Conversation
✅ Deploy Preview for boosted ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
A thought for the reviewer: if we put the “accessibility-no-vision” icon in the OUDS Web sprite, we could have icons that change with the brand in the example (like loading buttons), but this icon is not strictly necessary for the OUDS documentation... |
# Conflicts: # site/src/assets/partials/snippets.js
There was a problem hiding this comment.
Pull request overview
This PR enhances the Password input documentation by adding a JavaScript-powered “show/hide password” live example (including a new “hide” icon in the shared SVG sprite), and includes a small documentation wording fix elsewhere.
Changes:
- Add a new “Show password” live example section to the Password input docs and wire it to a JS snippet.
- Introduce a new
hideicon symbol in OUDS SVG sprites for the Orange / Orange Compact / Sosh doc sites. - Fix “JavaScript” capitalization in the migration guide.
Reviewed changes
Copilot reviewed 3 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| site/static/sosh/docs/[version]/assets/img/ouds-web-sprite.svg | Adds hide icon symbol to support toggling icon in docs examples. |
| site/static/orange/docs/[version]/assets/img/ouds-web-sprite.svg | Adds hide icon symbol to support toggling icon in docs examples. |
| site/static/orange-compact/docs/[version]/assets/img/ouds-web-sprite.svg | Adds hide icon symbol to support toggling icon in docs examples. |
| site/src/content/docs/getting-started/migration-from-boosted.mdx | Fixes “JavaScript” capitalization in explanatory text. |
| site/src/content/docs/components/password-input.mdx | Adds JS guidance + a live example wired to the snippets file. |
| site/src/assets/partials/snippets.js | Adds the click handler used by the new live example. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
louismaximepiton
left a comment
There was a problem hiding this comment.
The whole thing work good, I'm just wondering if we should vocalize the password once we show the password ?
# Conflicts: # site/src/assets/partials/snippets.js
I found this article which suggests something about that: https://technology.blog.gov.uk/2021/04/19/simple-things-are-complicated-making-a-show-password-option/#:~:text=Screen%20reader%20users,to%20do%20so. However, it can be overkilling on that simple example... We have to discuss that. Some other recommendations for security are to set the type of the filed back to "password" when submitting... Should we mention that somewhere too? |
|
I also found something about the label which maybe shouldn't be changed: https://medium.com/@web-accessibility-education/dos-and-donts-of-accessible-show-password-buttons-9a5fbc2c566b#:~:text=Don%E2%80%99t%3A%20Toggle%20both,and%20vice%20versa. So many questions now ??? 😲 |
|
I'm fine with the changes, let's see what's the best from a11y pov |
|
About changing the label or not: https://www.w3.org/WAI/ARIA/apg/patterns/button/#:~:text=Toggle%20button%3A%20A,not%20be%20needed. NB : aria-pressed should be set to false at start if a toggle button is use. |
…ts, buttons in password input (spotted by copilot), add migration info
…ouds/main-his-show-passwd
# Conflicts: # site/src/content/docs/getting-started/migration.mdx
Related issues
Description
Add show password live example in paswword input page
Checklists
Progression (for Core Team only)
ouds/mainfollowing conventional commitLive previews