-
Notifications
You must be signed in to change notification settings - Fork 1
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: DatePicker and DateRangePicker #1544
Conversation
/> | ||
</UnstyledButton> | ||
</div> | ||
{(isInvalid || state.isInvalid) && ( |
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.
question: are there instances where isInvalid
and state.isInvalid
would be different?
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.
state.isInvalid
is determined by useDatePickerState
. For example, it handles end date before start date error and provides a default error message.
Users pass isInvalid
props to DatePicker
based on their business logic.
@@ -1,6 +1,8 @@ | |||
@use "../styles/common" as *; | |||
.CalendarGrid { | |||
border-collapse: collapse; | |||
z-index: 999; |
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.
question: does it make sense for us to add a z-index token for calendars?
{
"z-index": {
"input_icon": { "value": "1" },
"calendar": { "value": "999" },
"nav": { "value": "1000" },
"drawer": { "value": "1200" },
"modal": { "value": "1300" },
"notification": { "value": "999999" }
}
}
or is this z-index specific to a certain interaction and doesnt really make sense to have a z-index token?
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.
after looking at it again, z-index
should be on the diaglog
to show on top of the popover
instead of Calendar.
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.
pointer-events: auto; | ||
outline: none; | ||
position: relative; | ||
z-index: design-token("z-index.input_icon"); |
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.
suggestion: up to you and a minor thing, but i think we could just use the value of 1 here or define a component level token. generally so far we've been assigning z-index tokens in association with some semantics
π Changes
β Checklist
Easy UI has certain UX standards that must be met. In general, non-trivial changes should meet the following criteria:
Visuals match Design Specs in Figma(There is no design spec)Strikethroughany items that are not applicable to this pull request.