Skip to content
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

[pickers] Prepare compatibility with @mui/zero-runtime (stop using ownerState in styled) #12003

Merged
merged 34 commits into from
Mar 26, 2024

Conversation

flaviendelangle
Copy link
Member

@flaviendelangle flaviendelangle commented Feb 8, 2024

Part of #12277

Follow up

@flaviendelangle flaviendelangle self-assigned this Feb 8, 2024
@flaviendelangle flaviendelangle added the component: pickers This is the name of the generic UI component, not the React module! label Feb 8, 2024
@mui-bot
Copy link

mui-bot commented Feb 8, 2024

Deploy preview: https://deploy-preview-12003--material-ui-x.netlify.app/

Generated by 🚫 dangerJS against db6bd2d

* If `true`, the day can be dragged to change the current date range.
* @default false
*/
draggable?: boolean;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right now the typing comes from the DOM element which also accepts "true" and "false"
But we are only passing true of undefined so I restricted the typing so that our check in the styled component and the typing are coherent

@siriwatknp siriwatknp self-requested a review February 9, 2024 14:27
@flaviendelangle flaviendelangle changed the title [pickers] Explore usage of the new variants styling API [pickers] Prepare compatibility with @mui/zero-runtime (stop using ownerState in styled) Feb 23, 2024
@flaviendelangle flaviendelangle marked this pull request as ready for review February 23, 2024 14:11
@LukasTy LukasTy added the customization: css Design CSS customizability label Mar 6, 2024
Copy link
Member

@LukasTy LukasTy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work! 👏 🚀
Leaving some comments about a few regressions. 🙈 💡

@flaviendelangle flaviendelangle changed the base branch from next to master March 21, 2024 14:11
Copy link
Member

@LukasTy LukasTy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work, thank you! 👍 💯

@@ -76,24 +76,46 @@ const DateTimeRangePickerToolbarStart = styled(DateTimePickerToolbar, {
name: 'MuiDateTimeRangePickerToolbar',
slot: 'StartToolbar',
overridesResolver: (_, styles) => styles.startToolbar,
})<DateTimeRangePickerStartOrEndToolbarProps<any>>(({ ownerState }) => ({
})<DateTimeRangePickerStartOrEndToolbarProps<any>>({
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
})<DateTimeRangePickerStartOrEndToolbarProps<any>>({
})<DateTimeRangePickerStartOrEndToolbarProps<any> & { ownerState: DateTimeRangePickerStartOrEndToolbarProps<any> }>({

This would make sure that the styled-component receives ownerState.

Copy link
Member Author

@flaviendelangle flaviendelangle Mar 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DateTimeRangePickerStartOrEndToolbarProps already contains it 👍

type DateTimeRangePickerStartOrEndToolbarProps<TDate extends PickerValidDate> =
  DateTimePickerToolbarProps<TDate> & {
    ownerState?: DateTimeRangePickerToolbarProps<TDate>;
  };

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK the problem was that ownerState was optional in DateTimeRangePickerStartOrEndToolbarProps

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But the right API here is :

      props: ({ toolbarVariant }: DateTimeRangePickerStartOrEndToolbarProps<any>) => toolbarVariant !== 'desktop',

In which case I do need the typing because the types equals props and not props & ownerState (it should probably be fixed)

@flaviendelangle flaviendelangle merged commit 7f86c76 into mui:master Mar 26, 2024
17 checks passed
@flaviendelangle flaviendelangle deleted the poc-variants branch March 26, 2024 08:41
DungTiger pushed a commit to DungTiger/mui-x that referenced this pull request Jul 23, 2024
thomasmoon pushed a commit to thomasmoon/mui-x that referenced this pull request Sep 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: pickers This is the name of the generic UI component, not the React module! customization: css Design CSS customizability
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants