-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Implementing an inline toggle button to show/hide password. #20611
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements a password visibility toggle button for the login screen, allowing users to show/hide their password input. The implementation adds an inline toggle button with SVG icons that appears on hover or focus, and refactors the CSS to support the new layout.
- Adds a toggle button with eye/eye-off SVG icons that switches between showing and hiding the password
- Wraps the password input in a span container to accommodate the toggle button
- Updates CSS to target specific input elements and style the new password input container with toggle button
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| src/Umbraco.Web.UI.Login/src/auth.element.ts | Adds createButton and createShowPasswordToggleItem functions, modifies createFormLayoutItem to optionally include the toggle button, and updates component initialization |
| src/Umbraco.Web.UI.Login/src/auth-styles.css | Refactors CSS to target username and password inputs separately, adds styles for the password container span and toggle button |
| const createButton = (opts: { id: string; name: string }) => { | ||
| const button = document.createElement('button'); | ||
| button.id = opts.id; | ||
| button.ariaLabel = 'Show password'; |
Copilot
AI
Oct 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The aria-label should be localized using the same localization approach as other UI elements in this codebase. Consider using an umb-localize element or similar localization mechanism instead of hardcoded English text.
| button.onclick = () => { | ||
| const passwordInput = document.getElementById('password-input') as HTMLInputElement; | ||
| passwordInput.type = passwordInput.type === 'password' ? 'text' : 'password'; | ||
| button.ariaLabel = passwordInput.type === 'password' ? 'Show password' : 'Hide password'; |
Copilot
AI
Oct 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The aria-label text should be localized to support multiple languages. The hardcoded English strings 'Show password' and 'Hide password' should use the localization system.
| if (passwordInput.type === 'text') { | ||
| button.innerHTML = ` | ||
| <svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="1.75" stroke-linecap="round" stroke-linejoin="round"> | ||
| <path d="M9.88 9.88a3 3 0 1 0 4.24 4.24"></path> | ||
| <path d="M10.73 5.08A10.43 10.43 0 0 1 12 5c7 0 10 7 10 7a13.16 13.16 0 0 1-1.67 2.68"></path> | ||
| <path d="M6.61 6.61A13.526 13.526 0 0 0 2 12s3 7 10 7a9.74 9.74 0 0 0 5.39-1.61"></path> | ||
| <line x1="2" x2="22" y1="2" y2="22"></line> | ||
| </svg> | ||
| `; | ||
| } else { | ||
| button.innerHTML = ` | ||
| <svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="1.75" stroke-linecap="round" stroke-linejoin="round"> | ||
| <path d="M2 12s3-7 10-7 10 7 10 7-3 7-10 7-10-7-10-7Z"></path> | ||
| <circle cx="12" cy="12" r="3"></circle> | ||
| </svg> | ||
| `; | ||
| } |
Copilot
AI
Oct 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The SVG markup is duplicated in three places (lines 52-57, 65-72, and 74-79). Consider extracting the SVG strings into constants (e.g., EYE_VISIBLE_SVG and EYE_HIDDEN_SVG) to reduce duplication and make the code more maintainable.
| flex-wrap: nowrap; | ||
| position: relative; | ||
| vertical-align: middle; | ||
| column-gap: 0px; |
Copilot
AI
Oct 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The value '0px' should be simplified to '0' as zero values don't require units in CSS. This is a common best practice for cleaner, more maintainable stylesheets.
| column-gap: 0px; | |
| column-gap: 0; |
| padding: var(--uui-size-1, 3px) var(--uui-size-space-4, 9px); | ||
| } | ||
| #umb-login-form #password-input-span input { | ||
| flex-basis: 0px; |
Copilot
AI
Oct 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The value '0px' should be simplified to '0' as zero values don't require units in CSS.
| flex-basis: 0px; | |
| flex-basis: 0; |
| flex-shrink: 99; | ||
| flex-grow: 1; | ||
| align-self: stretch; | ||
| min-width: 0px; |
Copilot
AI
Oct 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The value '0px' should be simplified to '0' as zero values don't require units in CSS.
| min-width: 0px; | |
| min-width: 0; |
…changed the css to accommodate these changes
Added the svg's to their own const for easy reuse Added localization for the arialabel on the button Seperated the createFormLayoutItem so there is a seperate for the password input Moved all the conditional logic in the onclick event to fit inside one if/else statement
…enough for localization to load, and replaced it with a function. The function will try and resolve the promise by checking if the localize.terms methods returns a changed value, if not then it retries every 50ms or untill it hits a max retry of 40/2 seconds.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We had said in the past that the real solution was to use the <uui-input-password /> component, which supports this OOTB. However, it is not possible to use that component because many password managers do not support Shadow DOM, which is why we create the input elements in the root and move them in through slots to their final place.
I don't mind adding this to the login screen, but we need to ensure that we support the same browsers as the UI library and that we do not introduce the same bugs again. So I left a comment down below:
9139429 to
e294b05
Compare
…he uui libary. Now the button is always visible instead of appearing on hover or when in focus
| border-color: var(--uui-input-border-color-hover, var(--uui-color-border-standalone, #c2c2c2)); | ||
| } | ||
|
|
||
| #umb-login-form #password-input-span:hover button, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.

Implemented an inline toggle button to show/hide your password, also changed the css to accommodate these changes, for the login screen
Description
Created methods to add a button for the toggle as well as a method for creating the span element that contains the button.
The button has an SVG attached that changes depending on the toggle state.
The button's onClick event targets the field for the password and changes its type from "password" to "text"
The button only appears if you hover the span or have focus-within in the span element.
Wrapped the password input in a new span element so I could manipulate the input field's length
I have removed some of the styling that wasn't used, as well as added CSS that specifically targets each input element (username and password).