Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactored: works for multiple password inputs, fixes regarding autofill… #48

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

slackero
Copy link

@slackero slackero commented Nov 6, 2021

  • Works with multiple input[type=password].
  • The aria-label of the toggle button can be customized adding data-show and data-hide attributes to the password input.
  • The toggle button is added by JavaScript — remove existing <button> tags when upgrading.
  • The toggle button is always visible.
  • Fixes the problem that the toggle icon can be hidden when autofill is active.
  • Added aria-pressed attribute to the toggle button.
  • Added jQuery compatible script.
  • Updated, added demos.

- Works with multiple `input[type=password]`.
- The `aria-label` of the toggle button can be customized adding `data-show` and `data-hide` attributes to the password input.
- The toggle button is added by JavaScript, remove existing `<button>` tags.
- The toggle button works more stable.
- The toggle button is always visible.
Put the background icon to the button instead the password input to avoid hidden icon with autofill.
Copy link

@accesslint accesslint bot left a comment

Choose a reason for hiding this comment

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

There are accessibility issues in these changes.

js/show-password-toggle-jquery.min.js Outdated Show resolved Hide resolved
js/show-password-toggle-jquery.js Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
Copy link

@accesslint accesslint bot left a comment

Choose a reason for hiding this comment

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

There are accessibility issues in these changes.

README.md Show resolved Hide resolved
Copy link

@accesslint accesslint bot left a comment

Choose a reason for hiding this comment

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

There are accessibility issues in these changes.

README.md Show resolved Hide resolved
Copy link

@accesslint accesslint bot left a comment

Choose a reason for hiding this comment

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

There are accessibility issues in these changes.

README.md Show resolved Hide resolved
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.

1 participant