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
Adds password component that shows and hides password on user click #213
Conversation
This pull request has been linked to Shortcut Story #13559: Show password with eyes icon. |
export function HidePassword() { | ||
return ( | ||
<svg | ||
width="30" |
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.
I want to note that I tried to make this icon smaller, but it never looked right, so I had to use the properties listed in the Snow Dashboard UI kit in Figma. This kit was used for the eye icon in the Figma design.
import { TextField } from '@rotational/beacon-core'; | ||
import { useState } from 'react'; | ||
|
||
import { HidePassword } from '@/components/icons/hidePassword'; | ||
import { ShowPassword } from '@/components/icons/showPassword'; | ||
|
||
export default function Password() { | ||
const [showEye, setShowEye] = useState(false); | ||
|
||
const toggleEyeIcon = () => { | ||
setShowEye(!showEye); | ||
}; | ||
|
||
return ( | ||
<div className="flex space-x-3"> | ||
<TextField | ||
label="Password" | ||
placeholder="Password" | ||
type={!showEye ? 'password' : 'text'} | ||
fullWidth | ||
data-testid="password" | ||
/> | ||
<button onClick={toggleEyeIcon} className="mt-4 self-center" data-testid="button"> | ||
{showEye ? <ShowPassword /> : <HidePassword />} | ||
<span className="sr-only" data-testid="screenReadText"> | ||
{showEye ? 'Hide Password' : 'Show Password'} | ||
</span> | ||
</button> | ||
</div> | ||
); | ||
} |
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.
It looks good.
Would you be able to apply those changes to the registrationForm component? This should be implemented directly on that view. It may not be necessary to have a separate password component.
export function ShowPassword() { | ||
return ( | ||
<> | ||
<svg | ||
width="30" | ||
height="20" | ||
viewBox="0 0 30 20" | ||
aria-hidden="true" | ||
focusable="false" | ||
fill="none" | ||
xmlns="http://www.w3.org/2000/svg" | ||
data-testid="showPassword" | ||
> | ||
<path | ||
fillRule="evenodd" | ||
clipRule="evenodd" | ||
d="M29.9138 10.4061C29.9138 10.4061 29.6346 11.0344 29.065 11.9391C29.065 11.9391 27.9427 13.7215 26.4571 15.2071C26.4571 15.2071 21.6642 20 15 20C15 20 8.33579 20 3.54289 15.2071C3.54289 15.2071 2.0573 13.7215 0.935017 11.9391C0.935017 11.9391 0.36542 11.0344 0.0861884 10.4061C-0.0287295 10.1476 -0.0287295 9.85243 0.0861885 9.59386C0.0861885 9.59386 0.365419 8.96559 0.935017 8.06094C0.935017 8.06094 2.0573 6.27848 3.54289 4.79289C3.54289 4.79289 8.33579 0 15 0C15 0 21.6642 0 26.4571 4.79289C26.4571 4.79289 27.9427 6.27847 29.065 8.06094C29.065 8.06094 29.6346 8.96558 29.9138 9.59386C30.0287 9.85243 30.0287 10.1476 29.9138 10.4061ZM27.3725 10.8734C27.3725 10.8734 27.6833 10.3798 27.8859 10C27.8859 10 27.6833 9.62016 27.3725 9.12656C27.3725 9.12656 26.3698 7.53401 25.0429 6.20711C25.0429 6.20711 20.8358 2 15 2C15 2 9.16422 2 4.95711 6.20711C4.95711 6.20711 3.6302 7.53401 2.62748 9.12656C2.62748 9.12656 2.31671 9.62015 2.11411 10C2.11411 10 2.31671 10.3799 2.62748 10.8734C2.62748 10.8734 3.6302 12.466 4.95711 13.7929C4.95711 13.7929 9.16422 18 15 18C15 18 20.8358 18 25.0429 13.7929C25.0429 13.7929 26.3698 12.466 27.3725 10.8734Z" | ||
fill="#1D65A6" | ||
/> | ||
<path | ||
fillRule="evenodd" | ||
clipRule="evenodd" | ||
d="M15 4C15 4 17.4853 4 19.2426 5.75736C19.2426 5.75736 21 7.51472 21 10C21 10 21 12.4853 19.2426 14.2426C19.2426 14.2426 17.4853 16 15 16C15 16 12.5147 16 10.7574 14.2426C10.7574 14.2426 9 12.4853 9 10C9 10 9 7.51472 10.7574 5.75736C10.7574 5.75736 12.5147 4 15 4ZM15 6C15 6 13.3431 6 12.1716 7.17157C12.1716 7.17157 11 8.34315 11 10C11 10 11 11.6569 12.1716 12.8284C12.1716 12.8284 13.3431 14 15 14C15 14 16.6569 14 17.8284 12.8284C17.8284 12.8284 19 11.6569 19 10C19 10 19 8.34315 17.8284 7.17157C17.8284 7.17157 16.6569 6 15 6Z" | ||
fill="#1D65A6" | ||
/> | ||
</svg> | ||
</> | ||
); | ||
} |
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.
can you create an icon component in icons folder? name it EyesIcon if possible.
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.
oh just checked again and it seems like it is already in the icons component folder. so you can forget about this comment
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.
but if you can review the naming it would be great.
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.
I think that naming the icons ShowPassword
and HidePassword
makes the code easier for readers to understand what the icons will do when users interact with them on the page.
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.
Honestly, I think it easy to have consistent naming. when I'm in the icons folder it is easy for me to have a name that reflects the folder. you can add the eyesClose-icon and eyesOpen-icon. HidePassword and showPassword look like a component names.
fullWidth | ||
data-testid="password" | ||
/> | ||
<button onClick={toggleEyeIcon} className="mt-4 self-center" data-testid="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.
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.
I moved the icon inside the field. If the icon's color needs to also be changed, please let me know.
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.
Here's a link showing how the icon appears now. I want to note that the icon does move a little when the user clicks it. When I added a bottom value, in addition to the top value, the icon didn't move. However, when a user deleted a password or if one wasn't entered, this would make the icon drop to the bottom of the text field. After trying several different things, this seemed to be the best option.
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.
To fix this behavior can you fixe the icon height size and set a good padding value? you can also make the icon smaller and set the color to gray.
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 suggestion worked. Thank you! I was able to decrease each icon's size by using scale. For the gray color, I used slate gray (-secondary-900)
from the design system. It passes a contrast checker. I checked lighter gray color values in the design system they did not pass the contrast checker. Here's a link to view how it looks now.
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.
Looks good. just a minor comment. If you feel okay you can merge it. we could make those updates later if it takes time.
Scope of changes
Adds a password component to the Beacon UI that shows an closed eye icon. When a user clicks on the closed eye icon, it should be replaced with an open eye icon and the password text should appear on the screen. When a user clicks on the opened eye icon, the closed eye icon should reappear and the password text should be hidden.
For accessibility,
aria-hidden="true"
andfocusable="false"
was added to each icon's svg file. Screen readers should recognize a button with the text "Show Password" or "Hide Password" instead of the icons.Fixes SC-13559
Type of change
Acceptance criteria
https://www.awesomescreenshot.com/video/14834557?key=f5ae3bfc7a820862dabd0b2b68c3ae7d
Author checklist
Reviewer(s) checklist