Skip to content

[pickers] Fix transformOrigin resolving based on popper placement #18206

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

Merged
merged 4 commits into from
Jun 4, 2025

Conversation

LukasTy
Copy link
Member

@LukasTy LukasTy commented Jun 3, 2025

Fix the resolving of transformOrigin here:

props: ({ popperPlacement }) => new Set(['top', 'top-start', 'top-end']).has(popperPlacement),
style: {
transformOrigin: 'bottom center',
},

ownerState always included the initial value of placement, regardless of the provided one.
This lead to the following problem:

Before

Screen.Recording.2025-06-03.at.16.26.36.mov

After

Screen.Recording.2025-06-03.at.16.26.58.mov

@LukasTy LukasTy self-assigned this Jun 3, 2025
@LukasTy LukasTy added type: regression A bug that reintroduces previously resolved issues or breaks existing functionality. scope: pickers Changes or issues related to the pickers product labels Jun 3, 2025
@mui-bot
Copy link

mui-bot commented Jun 3, 2025

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

Bundle size report

Total Size Change:${\tiny{\color{red}▲}}$+602B(0.00%) - Total Gzip Change:${\tiny{\color{red}▲}}$+149B(0.00%)
Files: 120 total (0 added, 0 removed, 14 changed)

Show 14 more bundle changes

@mui/x-date-pickersparsed:${\tiny{\color{red}▲}}$+43B(+0.02%) gzip:${\tiny{\color{red}▲}}$+19B(+0.03%)
@mui/x-date-pickers-proparsed:${\tiny{\color{red}▲}}$+43B(+0.01%) gzip:${\tiny{\color{red}▲}}$+6B(+0.01%)
@mui/x-date-pickers-pro/DateRangePickerparsed:${\tiny{\color{red}▲}}$+43B(+0.02%) gzip:${\tiny{\color{red}▲}}$+9B(+0.02%)
@mui/x-date-pickers-pro/DateTimeRangePickerparsed:${\tiny{\color{red}▲}}$+43B(+0.02%) gzip:${\tiny{\color{red}▲}}$+5B(+0.01%)
@mui/x-date-pickers-pro/DesktopDateRangePickerparsed:${\tiny{\color{red}▲}}$+43B(+0.03%) gzip:${\tiny{\color{red}▲}}$+8B(+0.02%)
@mui/x-date-pickers-pro/DesktopDateTimeRangePickerparsed:${\tiny{\color{red}▲}}$+43B(+0.02%) gzip:${\tiny{\color{red}▲}}$+19B(+0.03%)
@mui/x-date-pickers-pro/DesktopTimeRangePickerparsed:${\tiny{\color{red}▲}}$+43B(+0.03%) gzip:${\tiny{\color{red}▲}}$+10B(+0.02%)
@mui/x-date-pickers-pro/TimeRangePickerparsed:${\tiny{\color{red}▲}}$+43B(+0.03%) gzip:${\tiny{\color{red}▲}}$+11B(+0.03%)
@mui/x-date-pickers/DatePickerparsed:${\tiny{\color{red}▲}}$+43B(+0.03%) gzip:${\tiny{\color{red}▲}}$+10B(+0.02%)
@mui/x-date-pickers/DateTimePickerparsed:${\tiny{\color{red}▲}}$+43B(+0.02%) gzip:${\tiny{\color{red}▲}}$+10B(+0.02%)
@mui/x-date-pickers/DesktopDatePickerparsed:${\tiny{\color{red}▲}}$+43B(+0.03%) gzip:${\tiny{\color{red}▲}}$+8B(+0.02%)
@mui/x-date-pickers/DesktopDateTimePickerparsed:${\tiny{\color{red}▲}}$+43B(+0.02%) gzip:${\tiny{\color{red}▲}}$+7B(+0.01%)
@mui/x-date-pickers/DesktopTimePickerparsed:${\tiny{\color{red}▲}}$+43B(+0.03%) gzip:${\tiny{\color{red}▲}}$+6B(+0.02%)
@mui/x-date-pickers/TimePickerparsed:${\tiny{\color{red}▲}}$+43B(+0.03%) gzip:${\tiny{\color{red}▲}}$+21B(+0.05%)

Details of bundle changes

Generated by 🚫 dangerJS against db983cc

@@ -67,7 +67,7 @@ export interface PickerPopperSlotProps {
/**
* Props passed down to [Popper](https://mui.com/material-ui/api/popper/) component.
*/
popper?: SlotComponentPropsFromProps<PopperProps, {}, PickerPopperOwnerState>;
popper?: SlotComponentPropsFromProps<PopperProps, {}, PickerOwnerState>;
Copy link
Member Author

Choose a reason for hiding this comment

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

To have the final popperPlacement value here, we would need to resolve the slotProps.popper twice. 🙈
I opted for a slight BC by removing this field from the type to avoid inconsistencies or ugly and less performant code. 🙈

Copy link
Member

Choose a reason for hiding this comment

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

Not great but fine doing it :/

@@ -67,7 +67,7 @@ export interface PickerPopperSlotProps {
/**
* Props passed down to [Popper](https://mui.com/material-ui/api/popper/) component.
*/
popper?: SlotComponentPropsFromProps<PopperProps, {}, PickerPopperOwnerState>;
popper?: SlotComponentPropsFromProps<PopperProps, {}, PickerOwnerState>;
Copy link
Member

Choose a reason for hiding this comment

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

Not great but fine doing it :/

@LukasTy LukasTy enabled auto-merge (squash) June 4, 2025 10:16
@LukasTy LukasTy merged commit 5c8163d into mui:master Jun 4, 2025
22 checks passed
@LukasTy LukasTy deleted the fix-picker-popper-paper-owner-state branch June 4, 2025 10:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope: pickers Changes or issues related to the pickers product type: regression A bug that reintroduces previously resolved issues or breaks existing functionality.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants