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] Fix to transform-origin when popper opens to top #10069

Merged
merged 3 commits into from
Aug 21, 2023

Conversation

LukasTy
Copy link
Member

@LukasTy LukasTy commented Aug 17, 2023

Fixes #10006

The placement is top-start when the popper opens to the top, because of that the conditional style did not work.

Preview: https://deploy-preview-10069--material-ui-x.netlify.app/x/react-date-pickers/

@LukasTy LukasTy added bug 🐛 Something doesn't work component: pickers This is the name of the generic UI component, not the React module! design: ui labels Aug 17, 2023
@LukasTy LukasTy self-assigned this Aug 17, 2023
@mui-bot
Copy link

mui-bot commented Aug 17, 2023

Netlify deploy preview

Netlify deploy preview: https://deploy-preview-10069--material-ui-x.netlify.app/

Updated pages

No updates.

These are the results for the performance tests:

Test case Unit Min Max Median Mean σ
Filter 100k rows ms -163.6 107.5 -18.6 -25.68 108.52
Sort 100k rows ms 843.3 1,533.2 1,528.1 1,354.12 259.125
Select 100k rows ms 680.7 896.4 764.8 787 83.95
Deselect 100k rows ms 136.2 232.2 187.8 189.02 32.717

Generated by 🚫 dangerJS against bb39de6

@oliviertassinari oliviertassinari added design This is about UI or UX design, please involve a designer and removed design: ui labels Aug 18, 2023
@oliviertassinari
Copy link
Member

oliviertassinari commented Aug 21, 2023

Awesome, it feels clearly better 👍

To be fair, in #5490 (comment), I have made the case for much less frequently flipping the popup to be shown on the top.

Copy link
Contributor

@noraleonte noraleonte left a comment

Choose a reason for hiding this comment

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

Very nice improvement! 🎉

@LukasTy
Copy link
Member Author

LukasTy commented Aug 21, 2023

To be fair, in #5490 (comment), I have made the case for much less frequently flipping the popup to be shown on the top.

Yes, those are great two points that could be explored separately from one another.

  • The flipping based on document or viewport should remain on user control IMHO. There are different implementations in the wild, but I don't think it should be decided by the component library. 🤔
  • In regards to aligning the height of the popper calendars—that is on our agenda. Ideally, we'd do it when implementing MD3 guidelines as those make it pretty easy to align without any concerns (like providing a visual cue that the YearCalendar is scrollable by clipping part of the year buttons).

@LukasTy LukasTy merged commit 2ebdb6f into mui:master Aug 21, 2023
@LukasTy LukasTy deleted the fix-pickers-popper-transform-origin branch August 21, 2023 14:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: pickers This is the name of the generic UI component, not the React module! design This is about UI or UX design, please involve a designer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[pickers] Open animation origin bug when reversed
4 participants