Fix login screen accessibility: add form labels, lang attribute, and update link contrast (#1373) #2252
Conversation
…update link contrast (autolab#1373)
📝 WalkthroughWalkthroughThe pull request includes updates to two files. The Changes
Assessment against linked issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (2)
app/views/devise/sessions/new.html.erb (2)
9-12: Consider removing redundant placeholder textThe label implementation looks good and follows accessibility best practices. However, the placeholder text duplicates the label which is redundant. Consider removing the placeholder or using it for format hints instead (e.g., "[email protected]").
- <%= f.email_field :email, autofocus: true, placeholder: "Email Address" %> + <%= f.email_field :email, autofocus: true, placeholder: "[email protected]" %>
13-16: Add password requirements hint for better accessibilityWhile the label implementation is correct, consider adding password requirements using
aria-describedbyto help users understand password constraints before submission.- <%= f.label :password, "Password" %> - <%= f.password_field :password, placeholder: "Password" %> + <%= f.label :password, "Password" %> + <span id="password-requirements" class="sr-only">Password must be at least 8 characters long</span> + <%= f.password_field :password, + "aria-describedby": "password-requirements", + placeholder: "Enter your password" %>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
app/assets/stylesheets/_variables.scss(1 hunks)app/views/devise/sessions/new.html.erb(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- app/assets/stylesheets/_variables.scss
|
Hi David, it looks like all the checks have passed on your PR. It's great that you are making changes that makes the website more accessible. One thing that might be nice for you to add is to explain how you know the updated color adheres to the AA guidelines. Otherwise, it looks like you've address all three pain points outlined in the issue. The language is specified, you have modified the color to adhere to AA guidelines, and the email and password fields now have labels for voice navigation. |
…update link contrast (#1373)
Description
Added accessibility improvements to the login screen:
Motivation and Context
Fixes #1373
These changes improve accessibility for users relying on screen readers and those with visual impairments by providing proper form labels, language identification, and sufficient color contrast.
How Has This Been Tested?
Tested locally by:
Types of changes
Checklist:
overcommit --install && overcommit --signto use pre-commit hook for linting