Skip to content

Conversation

@ThomasLandauer
Copy link
Contributor

Although common, placing the label into the placeholder is bad practice and reduces usability and accessibility. If people wanna do it this way, they still can; but it shouldn't be suggested :-)

If you merge it, I'll update the docs at https://symfony.com/doc/current/security/form_login_setup.html#generating-the-login-form too.

@romaricdrigon
Copy link
Contributor

Hello,
Thanks for the feedback and the PR.
I'm having mixed feelings, because we still had a <label> for screen reader, so it was IMO it was OK regarding accessibility. Only thing is that sr-only class require Bootstrap ; so maybe we should make default theme less Bootstrap dependant? Really, I don't know.

@ThomasLandauer
Copy link
Contributor Author

@weaverryan
Copy link
Member

This looks reasonable - and we're still using Bootstrap markup, so sure - let's do it :). Thanks Thomas!

weaverryan added a commit that referenced this pull request Nov 1, 2019
This PR was merged into the 1.0-dev branch.

Discussion
----------

Changed placeholder to real label

Although common, placing the label into the placeholder is bad practice and reduces usability and accessibility. If people wanna do it this way, they still can; but it shouldn't be suggested :-)

If you merge it, I'll update the docs at https://symfony.com/doc/current/security/form_login_setup.html#generating-the-login-form too.

Commits
-------

fd5a6d5 Changed placeholder to real label
@weaverryan weaverryan merged commit fd5a6d5 into symfony:master Nov 1, 2019
@weaverryan
Copy link
Member

Just tagged a patch release - so could you please update the docs @ThomasLandauer? Thanks!

@ThomasLandauer ThomasLandauer deleted the patch-1 branch November 1, 2019 20:22
@ThomasLandauer
Copy link
Contributor Author

ThomasLandauer commented Nov 1, 2019

Update the docs at 5.0 master https://symfony.com/doc/master/security/form_login_setup.html or which branch?

BTW: Is there a long-term plan to get rid of Bootstrap?

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.

3 participants