-
Notifications
You must be signed in to change notification settings - Fork 201
refactor(datepicker)!: diy token migration #2185
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
Conversation
c345711
to
5571c71
Compare
🚀 Deployed on https://pr-2185--spectrum-css.netlify.app |
80948c4
to
044ba38
Compare
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 looks great. The only thing I see that needs adjustment is related to the spacing around the invalid icon. The quiet in particular has too much right padding, which causes the values to turn to ellipsis sooner than they should:
The regular one ("Range", invalid) also has 2px more padding than combobox has between the text and the invalid icon, which when fixed would mean a value like "10/30/2022" would no longer turn to ellipsis there. The placeholder format still would though, which leads me to...
Possible design change:
I'm wondering if it would be a good idea to increase the overall min-width of the range inputs, so that the placeholders and values do not turn to ellipsis when there is an invalid icon. This may be something to validate with the design team, as it'd be a change from what it is now. Otherwise the user experience of visually comparing what might be wrong or entering a value is hindered by not being able to see the entire value or expected format.
High contrast, quiet:
I saw this as well; the invalid icon is a little too close to the button border:
dd7d716
to
ddb638f
Compare
Description
Migrates DatePicker to new tokens.
DatePicker does not have a figma or XD files, however it is similar to Combo box which does.
Additionally:
Jira Ticket 579
How and where has this been tested?
Please tag yourself on the tests you've marked complete to confirm the tests have been run by someone other than the author.
Testing Outline
@jawinn
Regression testing
Validate:
Screenshots
Before
After
To-do list
I have read the contribution guidelines.
I have updated relevant storybook stories and templates.
I have tested these changes in Windows High Contrast mode.
If my change impacts other components, I have tested to make sure they don't break.
If my change impacts documentation, I have updated the documentation accordingly.
✨ This pull request is ready to merge. ✨