-
Notifications
You must be signed in to change notification settings - Fork 24
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
feat(styles): synced disabled and error states for select #1342
base: develop
Are you sure you want to change the base?
Conversation
This pull request is automatically being deployed by Amplify Hosting (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 realize this file hasn't been touched in a while and has some old patterns, but our new guidelines specify these should be component level variables.
example:
:root {
--select-background-color-disabled: var(--field-background-color-disabled)
}
.cauldron--theme-light {
--select-background-color-disabled: /* whatever color is supposed to be here */
}
.Field__select--disabled select {
background: var(--select-background-color-disabled);
}
...
It's okay if this takes a little more time since we're trying to align our component css patterns as well.
…uldron into fix/sync-select-with-design
box-shadow: rgba(0, 0, 0, 0.05) 0 1px 2px 0 inset, | ||
var(--field-border-color-error-focus-glow) 0 0 4px 0; | ||
var(--select-field-border-color-error) 0 0 0 1px, |
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.
This was done after a discussion with @dequelabs/cauldron-design in which we decided a temporary solution to address an a11y issue would be to increase the size of the focus ring upon focus.
Closes: #1006