-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
[POC] Refactor PickersDay
and DateRangePickerDay
components
#15921
base: master
Are you sure you want to change the base?
Conversation
Deploy preview: https://deploy-preview-15921--material-ui-x.netlify.app/ Updated pages: |
dayOfWeek: number; | ||
} | ||
|
||
export type OwnerState = Partial<EnhancedPickersDayProps> & { |
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'll go more in depth on the code changes next year.
But concerning this interface we need to make sure it's properly shaped:
-
Should be named
PickerDayOwnerState
-
Should not extend
EnhancedPickersDayProps
but only have the properties it actually needs (either to generate the classes or for the variants) and extendPickerOwnerState
. -
Every property should be named with the same approach (
isLastDayOfTheWeek
instead oflastDayOfWeek
for example), including the one that come from the props. -
Each property should have some JSDoc explaining the value provided.
Once we have this object clean, it will be easier to see if some properties are not that needed 👌
closes #14753
This is an exploration
I'm exploring the option of creating only one day component used by both simple and range pickers & writing the styles in a way that they are truly separated from each other. i.e. The
selected
state should not care whetherisStartOfPreviewing
is true or not. It just applies the styles related to selection.I reduced the DOM structure to a single element (ButtonBase) and the previewing and highlighting (now renamed to
selectedRange
) is handled with pseudo elements.The advantage of this is that we reduce the complexity of customizing this component a lot and we can avoid specificity issues. See new demo vs custom theme from the overview page
A downside is that the width and positioning of the pseudo elements is calculated based on the width and margins of the root => so any customizations of the dimensions or spacing of the root element need to be reflected in the calculation of the dimensions and positionings of the pseudo elements. We can try abstracting some of that and introducing css variables, but I think documenting this should be enough. Plus, it might even make it easier to achieve custom designs where the highlighting and previewing don't connect. (Like the picture)