feat: Visibility toggle for PasswordInput #377
Conversation
im-adithya
changed the title
|
@gabriellsh @MartinSchoeler @ggazzo Can you please check this and suggest changes if any? |
|
This pull request introduces 1 alert when merging 3f7be65 into 8a1b552 - view on LGTM.com new alerts:
|
|
This pull request introduces 1 alert when merging 121ec74 into b694b86 - view on LGTM.com new alerts:
|
There was a problem hiding this comment.
Hey, nice job! Can you make this changes to the password input field itself? I don't think we should have 2 components for passwords.
Also, you got to create the stories for the storybook (yarn storybook), and update loki afterwards (yarn update-storybook). Thanks!
Sure @gabriellsh 😄 |
|
@gabriellsh I have made the changes. But what should I change in the stories? (Now that we made changes to PasswordInput itself... Should I add some info about the toggle property?) |
|
Would be nice to have one story for the hidden and one for the text showing (with some default text of course). This way loki can test for the visual behavior. |
|
Done @gabriellsh! Please review it :) |
|
@tassoevan can you please review this as well? :) |
|
@im-adithya Is the toggle button accessible by tab navigation? |
No @tassoevan |
ggazzo
force-pushed
the
develop
branch
3 times, most recently
from
00a7874 to
a7f9c9a
Compare
tassoevan
changed the title
* Update fuselage.d.ts * Update index.js * Update spec.js * Update index.js * Update fuselage.d.ts * Update index.js * Update spec.js * Update PasswordInput.stories.mdx * Invert toggle state icon * Convert PasswordInput to TypeScript Co-authored-by: gabriellsh <[email protected]> Co-authored-by: Tasso Evangelista <[email protected]>
This PR closes #376
Forms._.Inputs._.PasswordInput.-.Default.Storybook.-.Google.Chrome.2021-01-26.22-43-56.mp4