Problem/Motivation
When I asked to be co-maintainer of this module, one of the tasks I wanted to take care of was making sure the functionality was a bit more accessible.
At this moment the module uses a <span> tag to render an icon and JS to toggle the display of the password.
I think this might be a problem for the following reasons:
- Visually impaired users browse the web with e-readers. These cannot identify the icons (such as the eye) and convey their meaning to the end user (make the password visible). The result is that visually impaired users won't even know that there is a way to toggle the password visibility (if the password is visible, the e-reader can read it back to the user and they can validate their input).
- Many users use the keyboard to navigate the websites.
<span>elements are not focusable by default. The result is that users that navigate the site with a keyboard will not be able to interact with the eye icon, and therefore neither use the functionality of this module.
Steps to reproduce
Just try to navigate to the icon with the keyboard (it won't be focused), or use a reader to get the meaning of the icon (you won't get "hide or show password" or anything similar).
Proposed resolution
Change the <span> tag for a <button> tag. I know this is an important change that might affect the styling of the sites, but I believe it's the right thing to do, as I quote this line from https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/bu...
Where possible, it is recommended to use native HTML buttons (
<button>,<input type="button">,<input type="submit">,<input type="reset">and<input type="image">) rather than the button role, as native HTML buttons are supported by all user agents and assistive technology and provide keyboard and focus requirements by default, without need for additional customization.
Other changes include:
- Adding a aria-label attribute to the button indicating what the action of clicking the button does: "Show password" or "Hide password".
- Providing multilingual support by allowing those strings to be translatable.
Remaining tasks
Please test this and provide feedback.
User interface changes
The <span> element will change to a <button> element.
Data model changes
The module will define two strings that the site builders will be able to translate within Drupal's translation interface: "Show password" and "Hide password".
A patch will be submitted below.
| Comment | File | Size | Author |
|---|
Issue fork view_password-3191748
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #4
anacolautti commentedAdding the patch.
Comment #5
anacolautti commentedComment #6
anacolautti commentedComment #7
anacolautti commentedThe code needs to be updated, it won't merge to 8.x-5.x (dev) anymore.
Comment #8
anacolautti commentedComment #9
anacolautti commentedComment #11
cesarmiquel commentedComment #13
cesarmiquel commentedComment #14
matthew.page commentedI have reviewed and tested this patch. Submitting patch for review and implementation. Thank you.
Comment #15
anacolautti commentedThanks @matthew.page! What's the difference with the submitted changes in the merge request created by @cesarmiquel? I noticed differences in indentations and JS code structuring. Why is that? Thank you!
Comment #16
matthew.page commentedHi @anacolautti! Sorry! I apologize. I thought the code structure was incorrect. But I was mistaken. Your patch works fine. I just tested it on my local and I didn't run into any issues. You can remove my patch if you want.
Comment #17
anacolautti commentedThanks for the review! I marked the issue as reviewed and tested by the community. I'll proceed to merge soon!
Comment #18
anacolautti commentedComment #19
anacolautti commentedClosing this issue. Merging to 6.0.x branch because this changes the markup.
Edit: When merging from the Issue page I accidentally merged to the 8.x-5.x branch. I merged manually to 6.0.x and reverted the commit in 8.x-5.x to avoid forcing the markup changes onto older branches.
Comment #22
anacolautti commented