[base-ui][Menu] Use Popup instead of Popper #40731
Conversation
michaldudak
added
component: menu
Netlify deploy preview@material-ui/unstyled: parsed: -0.03% 😍, gzip: -0.21% 😍 Bundle size reportDetails of bundle changes (Toolpad) |
michaldudak
changed the title
| @@ -71,7 +76,7 @@ const Menu = React.forwardRef((props, ref) => { | |||
| return { | |||
| ...resolvedSlotProps, | |||
| className: clsx( | |||
| 'text-sm box-border font-sans p-1.5 my-3 mx-0 rounded-xl overflow-auto outline-0 bg-white dark:bg-slate-900 border border-solid border-slate-200 dark:border-slate-700 text-slate-900 dark:text-slate-300 min-w-listbox shadow-md dark:shadow-slate-900', | |||
| 'text-sm box-border font-sans p-1.5 my-3 mx-0 rounded-xl overflow-auto outline-0 bg-white dark:bg-slate-900 border border-solid border-slate-200 dark:border-slate-700 text-slate-900 dark:text-slate-300 min-w-listbox shadow-md dark:shadow-slate-900 [.open_&]:opacity-100 [.open_&]:scale-100 transition-[opacity,transform] [.closed_&]:opacity-0 [.closed_&]:scale-90 [.placement-top_&]:origin-bottom [.placement-bottom_&]:origin-top', | |||
There was a problem hiding this comment.
Adding custom selectors seems like the Tailwind way of doing this instead of using classes. This is probably blocked by the Tailwind plugin after v1, though, right? Just checking.
There was a problem hiding this comment.
This is a custom class, so its selector is not included in the Tailwind config we use.
There was a problem hiding this comment.
And what would you think about not using CssTransition for the tailwind example and using the hooks directly instead? Wouldn't that be more idiomatic within tailwind users? This usage seems hard to find in the Tailwind docs
There was a problem hiding this comment.
Given my near-zero Tailwind experience, I don't know. @danilo-leal, I'd appreciate your input here, as you've used Tailwind more extensively than me.
There was a problem hiding this comment.
I have used arbitrary variant selectors with Tailwind very sparsely, but it doesn't feel too strange to me as the [...] brackets API is pretty familiar at this point. But maybe it would be indeed a bit easier to parse if we were using hooks — assuming we'd be able to do something like expanded && 'classes'.
(PS: Slightly off-topic, but I'm curious how this works with the ampersand being at the end 🤔 I'd intuitively assume it should be [&_.open]: instead of [.open_&], matching the example from the doc link Diego shared.)
There was a problem hiding this comment.
But maybe it would be indeed a bit easier to parse if we were using hooks
Could we merge this PR as is and work on the hook demo separately?
I'm curious how this works with the ampersand being at the end
The open class is applied to the parent element (popup), and we need to style the listbox conditionally depending on this class being present.
There was a problem hiding this comment.
Could we merge this PR as is and work on the hook demo separately?
Sure ✅
There was a problem hiding this comment.
Or we may end up redoing the demos if we decide to change the customization APIs
Replaced the Popper with Popup in the Menu implementation.
Similar to #40524, but with Menu.
Breaking change
slotProps.rootnow accepts thePopupPropsinstead of thePopperPropsPreview: https://deploy-preview-40731--material-ui.netlify.app/base-ui/react-menu/