Skip to content

Show/Hide Password: Accessibility on the button#508

Merged
nicosomb merged 3 commits intoPrestaShop:developfrom
yannicka:a11y-password-visibility
Jun 20, 2023
Merged

Show/Hide Password: Accessibility on the button#508
nicosomb merged 3 commits intoPrestaShop:developfrom
yannicka:a11y-password-visibility

Conversation

@yannicka
Copy link
Contributor

@yannicka yannicka commented Jun 13, 2023

Questions Answers
Description? Adds accessibility to the button to show/hide the password
Type? improvement
BC breaks? no
Deprecations? no
Fixed ticket? Improve #251
Sponsor company N/A
How to test? For a dev: Open dev console and sees aria-label and aria-expanded. (tested), for QA: see that showing/hiding password during registration works as expected.

icon.innerHTML = newType === 'text' ? 'visibility_off' : 'visibility';

const textShow = button.dataset.textShow;
const textHide = button.dataset.textHide;
Copy link
Contributor

@kpodemski kpodemski Jun 13, 2023

Choose a reason for hiding this comment

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

Suggested change
const textHide = button.dataset.textHide;
const { textHide, textShow } = button.dataset;

that should do it with eslint errors

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, updated!

Copy link
Contributor

@kpodemski kpodemski left a comment

Choose a reason for hiding this comment

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

eslint is 🔴

@kpodemski kpodemski added this to the Beta milestone Jun 16, 2023
@sallemiines sallemiines self-assigned this Jun 19, 2023
Copy link

@sallemiines sallemiines left a comment

Choose a reason for hiding this comment

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

Hello @yannicka

Thanx for the PR ! LGTM QA ✔️

You can check the attached screen recorder

810beta.1.mp4

Thank you !

@nicosomb nicosomb merged commit b85fd7c into PrestaShop:develop Jun 20, 2023
@nicosomb
Copy link
Contributor

Thank you @yannicka !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

5 participants