-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
fix(TextInput): fixes hover, focus issues with visibility icon #7680
fix(TextInput): fixes hover, focus issues with visibility icon #7680
Conversation
Deploy preview for carbon-elements ready! Built with commit a7c09c7 |
Deploy preview for carbon-components-react ready! Built without sensitive environment variables with commit a7c09c7 https://deploy-preview-7680--carbon-components-react.netlify.app |
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.
yay! 🌮
Still needs visual review
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!
Closes #7678
The visibility icon was inheriting
currentColor
which was causing issues in dark themes. This increases specificity so that the proper colors are used across all themes.While working on this, I also noticed issues with keyboard navigation, so this also adds in focus styles / cleans up the box model
Changelog
New
Changed
Testing / Reviewing
Ensure you can tab to password visibility icon, and see focus styles
Ensure the hover fill is correct across themes