-
Notifications
You must be signed in to change notification settings - Fork 26
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(time-input): readonly fix #724
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.
Looks cool. I especially like the PR‘s summary and description.
@tdeekens I opened it as a draft so I could see the Percy diff and finish it up. 😢 |
value={this.props.value} | ||
onChange={this.props.onChange} | ||
onFocus={this.props.onFocus} | ||
onBlur={this.props.onBlur} | ||
{...filterDataAttributes(this.props)} | ||
/* ARIA */ | ||
role="textbox" | ||
aria-readonly={this.props.isReadOnly} |
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.
same as we do in the TextInput
@@ -20,7 +20,7 @@ const getIconTheme = (isDisabled, isMouseOver) => { | |||
export const ClearSection = props => ( | |||
<div | |||
onClick={props.isDisabled || props.isReadOnly ? undefined : props.onClear} | |||
css={getClearSectionStyles(props)} | |||
css={theme => getClearSectionStyles(props, theme)} |
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.
If only if was curried :)
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.
at least I have VRT to catch my mistakes 🙈
Just need to refactor the icons to use colors from css instead of Icon Themes to make theming work |
Actually a better way would just be to support Icon Theming. I'll do it in another PR though and put this on hold until then 🎈 |
00c28c3
to
f609809
Compare
* fix(time-input): readonly fix
This PRd does two things
isReadOnly
prop for theTimeInput
component.TimeInput
component to use design tokens and to be themeable.NOTE: Until we merge
10.0.0
, a themed TimeInput looks a bit weird because the icons aren't themable.Closes #723