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:

  1. 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).
  2. 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:

  1. Adding a aria-label attribute to the button indicating what the action of clicking the button does: "Show password" or "Hide password".
  2. 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.

Command icon 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

anacolautti created an issue. See original summary.

anacolautti’s picture

Status: Active » Needs review
StatusFileSize
new3.31 KB

Adding the patch.

anacolautti’s picture

Issue summary: View changes
anacolautti’s picture

Issue summary: View changes
Status: Needs review » Postponed
anacolautti’s picture

Status: Postponed » Needs work

The code needs to be updated, it won't merge to 8.x-5.x (dev) anymore.

anacolautti’s picture

Issue summary: View changes
anacolautti’s picture

cesarmiquel made their first commit to this issue’s fork.

cesarmiquel’s picture

Assigned: anacolautti » cesarmiquel

cesarmiquel’s picture

Status: Needs work » Needs review
matthew.page’s picture

I have reviewed and tested this patch. Submitting patch for review and implementation. Thank you.

anacolautti’s picture

Thanks @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!

matthew.page’s picture

Hi @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.

anacolautti’s picture

Status: Needs review » Reviewed & tested by the community

Thanks for the review! I marked the issue as reviewed and tested by the community. I'll proceed to merge soon!

anacolautti’s picture

Version: 8.x-5.x-dev » 6.0.x-dev
anacolautti’s picture

Status: Reviewed & tested by the community » Fixed

Closing 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.

  • anacolautti committed fc28b69 on 8.x-5.x
    Revert "Issue #3191748 by cesarmiquel, anacolautti, matthew.page,...
anacolautti’s picture

Status: Fixed » Closed (fixed)