[context menu] Create new ContextMenu component#1665
Conversation
✅ Deploy Preview for base-ui ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
4d55392 to
a9acad9
Compare
|
We haven't discussed the API of it. I'm wondering if an ordinary Menu with a different Trigger wouldn't be enough: <Menu.Root>
<Menu.ContextTrigger />
... |
|
@michaldudak discoverability-wise, it might be better as a component for Radix parity? The Root needs to know if it should act as a context menu, where a component provider works nicely in the API, since otherwise it would need a prop in conjunction with using
|
|
Testing on an iPhone SE (iOS 17.7.2), a long press on the trigger area when the context menu is already open will make a large text selection: RPReplay_Final1744034094.mp4I screen-capped Safari only but it repros on Firefox and Chrome as well According to react-aria it requires quite a heavy handed approach to handle this for all browsers: https://github.com/adobe/react-spectrum/blob/main/packages/%40react-aria/interactions/src/textSelection.ts |
06e668f to
850dee6
Compare
|
@mj12albert looks like |
Nice, glad we can avoid that extra Safari handling ~ |
|
We could add an attribute indicating how the menu was open (touch or right-click). It seems that on mobile menus are often placed above the trigger (so they are not covered by user's hand). A callback variant of |
| } = useMenuRootContext(); | ||
| const { side, align, floatingContext } = useMenuPositionerContext(); | ||
| const contextMenuContext = useContextMenuRootContext(); | ||
| const hasContextMenuParent = Boolean(contextMenuContext) && !nested; |
There was a problem hiding this comment.
In the Menubar PR I implemented a more universal solution to determine the menu's parent and its capabilities: https://github.com/mui/base-ui/pull/1684/files#diff-6da7b84ff007a487d62a9d48ba9fb32ad4b286ef666fd3d0419b14e9ebbe472a.
There was a problem hiding this comment.
Context menus don't have any parents though, so I think this should suffice?
There was a problem hiding this comment.
I'd like to avoid having all kinds of boolean values like nested, isInContextMenu, isInMenubar, (possibly) isInNavigationMenu. The approach I took in the Menubar PR lets a menu part know what kind of menu it's in in a standard way and provides a type-safe way to access the parent context (without having to check for its existence)
There was a problem hiding this comment.
Alright 👍 I'll wait for Menubar to be merged and then use its logic for Menu
|
@michaldudak if you mean it doesn't lock scroll in all situations, that's from #1890 |
|
@ImSingee sorry for the super late reply but I see why this doesn't work (but should). Working to fix |
Signed-off-by: atomiks <cc.glows@gmail.com>
|
No, I mean you are able to click on elements outside of it. As if the internal backdrop doesn't exist. |
|
@michaldudak ah right. I switched outside press dismissal from |
|
Changed to a 500ms grace period to block outside press after long press, so we can still use Fixes inner/outer nesting: https://codesandbox.io/p/sandbox/base-ui-context-menu-stack-forked-z2hclk |
|
Screen.Recording.2025-05-22.at.11.20.38.movEDIT: Never mind, it's an issue with the CSB built-in console. The browser console doesn't show duplicates. |
|
There's a bug with return focus causing a scroll to the previously focused element if it was out of the viewport, but will be updated in Floating UI before release |
Closes #51
Preview: https://deploy-preview-1665--base-ui.netlify.app/react/components/context-menu