Skip to content

[TogglePasswordComponent] Add documentation for usage without Symfony Forms #1101

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

MirakuSan
Copy link

@MirakuSan MirakuSan commented Sep 7, 2023

Q A
Bug fix? no
New feature? no
Tickets n/a
License MIT

I would like to propose this improvement for the Toggle Password documentation.
I wanted to add this component to a login form and the documentation did not have example with native HTML forms.

So the example is the following :

    <div class="toggle-password-container"> // Add "toggle-password-container" or a class that applies position: relative to this container.
        <label for="password">Password</label>
        <input
            id="password"
            name="password"
            type="password"
            {{ stimulus_controller('symfony/ux-toggle-password/toggle-password', {
                    buttonClasses: ['toggle-password-button'], // Add as many classes as you wish. "toggle-password-button" is needed to activate the default CSS.
            }) }}
        >
    </div>

I really look forward for reviews to improve this example if needed.

Thanks a lot.

@MirakuSan MirakuSan force-pushed the improve-toggle-password-doc-form-natif branch from e94ea97 to 6f2810c Compare September 7, 2023 08:45
Copy link
Member

@weaverryan weaverryan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about the toggle-password-container class that should wrap the input (or at least something that has position: relative)?

placeholder="Password"
{{ stimulus_controller('@symfony/ux-toggle-password--toggle-password', {
visibleIcon: 'Default', // Here you can pass an SVG tag or a string. Default is the value for the default svg
visibleLabel: 'Show', // This field is optional
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
visibleLabel: 'Show', // This field is optional
visibleLabel: 'Show', // optional

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll move it to controller.ts too

Copy link
Author

@MirakuSan MirakuSan Sep 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@weaverryan I would like to propose this :

            {{ stimulus_controller('symfony/ux-toggle-password/toggle-password', {
                    {# visibleLabel: 'Show password', // If you want to modify this label. #}
                    {# visibleIcon: 'Some svg icon', // If you want to modify this icon. #}
                    {# hiddenLabel: 'Hide password', // If you want to modify this label. #}
                    {# hiddenIcon: 'Some svg icon', // If you want to modify this icon. #}
                    buttonClasses: ['toggle-password-button'], // Add as many classes as you wish. "toggle-password-button" is needed to activate the default CSS.
            }) }}

WDYT ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's a good idea, as these are different than the form field options (which are camelcase), so mentioning these here is a good idea 👍

Copy link
Author

@MirakuSan MirakuSan Sep 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have fixed this here : #1125 🙏 😄

@MirakuSan MirakuSan force-pushed the improve-toggle-password-doc-form-natif branch 2 times, most recently from 104b290 to 6817c63 Compare September 7, 2023 15:20
@MirakuSan
Copy link
Author

I have added the div with toggle-password-container and a comment

@weaverryan weaverryan force-pushed the improve-toggle-password-doc-form-natif branch from 6817c63 to 2890380 Compare September 19, 2023 12:55
@weaverryan
Copy link
Member

Thank you Damien!

@weaverryan weaverryan merged commit 4a5d9df into symfony:2.x Sep 19, 2023
@MirakuSan MirakuSan deleted the improve-toggle-password-doc-form-natif branch September 19, 2023 13:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants