-
Notifications
You must be signed in to change notification settings - Fork 104
feat(calendar-month): add tokens #6297
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(calendar-month): add tokens #6297
Conversation
|
Preview: https://patternfly-pr-6297.surge.sh A11y report: https://patternfly-pr-6297-a11y.surge.sh |
andrew-ronaldson
left a comment
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.
Appppppproooooooooved
lboehling
left a comment
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.
lgtm!
mcoker
left a comment
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.
LGTM! Couple of comments, I wonder if they're worth making either now or as follow ups:
I think you can remove this? It doesn't appear to do anything:
| --#{$calendar-month}__dates-cell--m-selected__date--focus--BoxShadow: 0 0 #{pf-size-prem(5px)} var(--pf-t--global--background--color--primary--default); |
If an item is selected and hovered, the light blue hover border is still showing
And if it's focused and hovered:
I think you can fix that by adding --#{$calendar-month}__date--hover--BorderWidth: 0; to the selected block vars
patternfly/src/patternfly/components/CalendarMonth/calendar-month.scss
Lines 176 to 184 in 90d0a48
| &.pf-m-selected { | |
| --#{$calendar-month}__date--BackgroundColor: var(--#{$calendar-month}__dates-cell--m-selected__date--BackgroundColor); | |
| --#{$calendar-month}__date--Color: var(--#{$calendar-month}__dates-cell--m-selected__date--Color); | |
| --#{$calendar-month}__date--hover--BackgroundColor: var(--#{$calendar-month}__dates-cell--m-selected__date--hover--BackgroundColor); | |
| --#{$calendar-month}__date--focus--BackgroundColor: var(--#{$calendar-month}__dates-cell--m-selected__date--focus--BackgroundColor); | |
| --#{$calendar-month}__date--focus--after--BorderColor: var(--#{$calendar-month}__dates-cell--m-selected__date--focus--after--BorderColor); | |
| --#{$calendar-month}__date--focus--BoxShadow: var(--#{$calendar-month}__dates-cell--m-selected__date--focus--BoxShadow); | |
| --#{$calendar-month}__date--after--OutlineOffset: var(--#{$calendar-month}__date--after--focus--OutlineOffset); | |
| } |
Then it looks like this:
The year input is cut off - can see it in the screenshots above
thatblindgeye
left a comment
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.
Another thing that could be a followup is adding an icon to the helper text in the "Invalid" example
| // @debug $date-picker; // check your variable names located in src/patternfly/sass-utilities/component-namespaces.scss | ||
|
|
||
| .#{$date-picker} { | ||
| :root, .#{$date-picker} { |
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 could be a followup since we don't show examples with an input label like Figma seems to show ("Date of birth" above the datepicker), but we should add a var for spacing between a datepicker text label and the input itself (would also need to add an example).
|
@lboehling @andrew-ronaldson I just want to check, can you confirm I should take off the hover border for the selected date? |
mcoker
left a comment
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.
Nice!! Just one small nit on the date picker selector
|
|
||
| :root, .#{$date-picker} { | ||
| :where(:root), | ||
| .#{$date-picker} { |
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.
| .#{$date-picker} { | |
| :where(.#{$date-picker}) { |
thatblindgeye
left a comment
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.
Preview doesn't look to have updated, but ran your branch locally and looks good
|
🎉 This PR is included in version 6.0.0-alpha.80 🎉 The release is available on: Your semantic-release bot 📦🚀 |


Fixes #5734
Adds tokens
Changes the date select to a menu-toggle