search

radix-ui / primitives

Radix Primitives is an open-source UI component library for building high-quality, accessible design systems and web apps. Maintained by @workos.
https://radix-ui.com/primitives
MIT License
14.88k stars 724 forks source link

[DropdownMenu] Sub-menu doesn't handle narrow screen widths very well #1749

Open cpmsmith opened 1 year ago

cpmsmith commented 1 year ago

Bug report

Current Behavior

You could argue this is a feature request, but since the side/alignment of sub-menus is strictly managed by Radix, I think that makes this a bug. If there's not enough room right or left to show a submenu, it just overflows.

image

Expected behavior

I think if there's no horizontal room, it should fall back to opening downward or upward. The layout could be a little weird, but it's better than being unreadable.

Reproducible example

This is observable in the the default CodeSandbox for DropdownMenu if you enable responsive preview

Suggested solution

This can be achieved with the fallbackPlacements option for Floating UI's flip modifier.

Additional context

I know there's also other discussion around exposing more customization of the position logic (which I support). I haven't seen any discussion mentioning the fact that submenus expose even fewer options than other popovers, which should probably be considered in that other discussion.

Your environment

Software Name(s) Version
Radix Package(s) react-dropdown-menu 2.0.1
React n/a 18.3.0
Browser Firefox 106.0.3
Operating System macOS 12.3.1
benoitgrelard commented 1 year ago

Hey @cpmsmith, if we did this, it would likely cause cascading issues with regards to pointer handling. Submenus as they stand aren't really designed to be used on mobile, it's rather a desktop paradigm. I don't think we will "fix" the issue this way, rather I think it should be a different component to be used on mobile potentially.

cpmsmith commented 1 year ago

it would likely cause cascading issues with regards to pointer handling.

Yeah, I can certainly see it being complicated, though I suspect not impossible.

Submenus as they stand aren't really designed to be used on mobile, it's rather a desktop paradigm.

In general you're right, but I do have two quibbles.

  1. My primary use case here is a desktop application which supports being used in a narrow window in some cases. Obviously breaking out of the window would be the ideal, but is out of the question.
  2. This doesn't necessarily mean Radix has to fall in line or anything, but iOS has been shifting toward using this pattern lately, even including nested menus, such as this one in Reminders, so it's not unheard-of:
hichemfantar commented 1 year ago

We can keep the existing desktop behavior for nested menus. And when there's not enough space, change it to the way Canva handles it.

https://user-images.githubusercontent.com/34947993/210799817-859b2233-b12d-4356-8466-475daceb8ae3.mp4

There is also the Figma approach:

https://user-images.githubusercontent.com/34947993/210801174-ca674d96-943b-4230-b652-d144e2aaa290.mp4

misha-erm commented 1 year ago

stumbled upon this issue as well. For me it would be good enough if submenu could just shift along the side axis if it doesn't fit. E.g. this is how it works in linear.app

image

While I understand that SubMenu might not be the best component for mobile I still believe it should try to be accessible as long as it can be

might be related https://github.com/radix-ui/primitives/issues/1568#issuecomment-1435254665 (#1568)

paralin commented 4 months ago

This was raised in #2599 and closed as dupe of this issue.

Here is an additional reproduction of the issue for your reference;

image

In the above screenshot I show a radix dropdown taken from the example in the docs. It works properly except the sub-menu is going out of the viewport boundaries and is therefore clipped.

With collisionDetection enabled I expect the sub-menu to appear on top of the parent menu so as to not go out of the collision boundaries (the viewport).

See the CodeSandbox reproduction.

benoitgrelard commented 4 months ago

@paralin there is no need to repost your duplicate issue in this one. GitHub already links to it automatically (look just above your comment). Can you please delete your comment?

paralin commented 4 months ago

@benoitgrelard It has an additional example and demonstration of the problem. There is no need to delete.

benoitgrelard commented 4 months ago

@paralin The problem described is exactly the same, and your issue and example is still accessible either way.