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

[Menu] Support cascading / nested menu #11723

Open
janhoeck opened this issue Jun 5, 2018 · 73 comments · May be fixed by nbv1982/material-ui#10 or hzhz/material-ui#96
Open

[Menu] Support cascading / nested menu #11723

janhoeck opened this issue Jun 5, 2018 · 73 comments · May be fixed by nbv1982/material-ui#10 or hzhz/material-ui#96
Labels
component: menu This is the name of the generic UI component, not the React module! new feature New feature or request waiting for 👍 Waiting for upvotes

Comments

@janhoeck
Copy link

janhoeck commented Jun 5, 2018

Hello,

I wrote some two advanced components, who can handle Submenuitems.
I defined an anchorElement, which will be passed as property to the Mui Menu Component.
Is there a way to define where the Popup Element should be displayed? Because now it will just overlap. Maybe I want to calculate it more left or more right.

Here is an demo: https://codesandbox.io/s/9ywowy18k4

Benchmark

@oliviertassinari oliviertassinari added docs Improvements or additions to the documentation component: menu This is the name of the generic UI component, not the React module! labels Jun 5, 2018
@oliviertassinari
Copy link
Member

oliviertassinari commented Jun 5, 2018

@janhoeck This is a nice demo! You can use the anchor playground to pick the right values.
Here is an example: https://codesandbox.io/s/lx1336xrx7

juin-05-2018 19-43-39

Do you want to add such example in the documentation? 😄

@oliviertassinari oliviertassinari added the good first issue Great for first contributions. Enable to learn the contribution process. label Jun 5, 2018
@janhoeck
Copy link
Author

janhoeck commented Jun 6, 2018

Thanks for your answer! Its working too when I align my button to the right.
I thought the sub menu item will overlap. But it won't. Here is what I mean: https://codesandbox.io/s/x99r38x6rz

Sure, I can add such an example in the documentation. How I do that? 👍
Maybe we can implement a sub menu component in the core code. I think some people would like this!

@oliviertassinari
Copy link
Member

How I do that?

:) https://github.com/mui-org/material-ui/blob/master/CONTRIBUTING.md#how-do-i-add-a-new-demo-in-the-documentation

Maybe we can implement a sub menu component in the core code. I think some people would like this!

Looking at your demo on codesandbox, the submenu implementation seems already straightforward. What abstraction do we miss?

@evilass
Copy link

evilass commented Jun 6, 2018

WOOOOW cool!

@janhoeck
Copy link
Author

janhoeck commented Jun 6, 2018

@oliviertassinari Uhm just implement a SubMenu Component in the core code so that the user does not have to write it on his own :). Its just an idea :).

I think I have time to add this to the documentation at the evening :)

@janhoeck
Copy link
Author

janhoeck commented Jun 7, 2018

Uhm @oliviertassinari I don't get material-ui running locally. But I removed mobx from the demo and use the normal setState functions.
Soooo maybe you could just copy/paste it in the docs?

Demo:
https://codesandbox.io/s/l7wm3rjxzm

menu.md Text:

Sub menus

If you want to have a menu with a sub menu tree (for example you want to add a "others" item as the last item of the parent menu)
then you can use the anchorElement function combining with Menu and MenuItem.

The EnhancedMenu is a wrapper for the Menu. The EnhancedMenu will iterate over the menuItems prop
and create a normal MenuItem or a new Menu with an anchorElement.

{{"demo": "pages/demos/menus/subMenu/Demo.js"}}

@oliviertassinari
Copy link
Member

I don't get material-ui running locally

@janhoeck Are you using yarn? Otherwise, you need to npm install each package. Thanks for simplifying the codesandbox! Also, if you could open a pull-request with any changes, I could update it to match your previous comment: #11723 (comment).

@janhoeck
Copy link
Author

@oliviertassinari FYI: Until now I did not find time to document it :/ And tomorrow I am 3 days on a business trip.

@pavanshinde47

This comment has been minimized.

@JeremyGrieshop
Copy link

@janhoeck Any idea how to enhance/correct the following issues with the SubMenu component you implemented?

  1. After expanding a SubMenu, and clicking away from the expanded SubMenu, it will only close the current SubMenu. This seems right, if "clicking away" means clicking its parent. But, the natural thing to do when clicking completely away is to dismiss everything. Consider a deeply nested submenu, where you intentionally need to "click away" N number of times in order to dismiss every menu window.

  2. Somewhat related to (1), but "clicking away" on another parent menu item that has a SubMenu should dismiss the current SubMenu and go ahead and open up the new SubMenu. I suspect figuring out focus would be difficult. See Office UI Fabric's ContextualMenu. They can even open things onMouseOver.

@janhoeck
Copy link
Author

@cobbs-totem You can define a boolean in a ContainerComponent. This boolean will be passed into all submenus. When you open a menu, set the boolean value to true.
After that check whenever you click away and set this boolean to false. If the boolean is false, than all menus should react to that and closed.

@JeremyGrieshop
Copy link

Thanks for the feedback! If I understand your proposal, I think the setback there is that clicking away only generates an event for the deepest child menu. So, the process would have to go in reverse (child onClose() calls props.onClose() to propagate the change upward to parents). This strategy will work when clicking away from all menus, as they will all get notified eventually, and all close out. However, if you click away from a child by clicking on the parent, the child gets the onClose() and tells all parents to close, including the parent in which you clicked inside. I haven't found a workaround for that.

@janhoeck
Copy link
Author

Just check in which container the user clicked. If the container is something else than a menu, then close all menus. And when the user clicked on a menu, than close all child menus of the menu

@IssueHuntBot
Copy link

@0maxxam0 funded this issue with $20. See it on IssueHunt

@janhoeck

This comment has been minimized.

@KJoyner
Copy link

KJoyner commented Nov 29, 2018

In the examples above, I am having trouble getting keyboard traversal to work in the nested menu. Any suggestions?

@Jak3b0
Copy link

Jak3b0 commented Jan 24, 2019

Hi,

does it support close all the sub-menus and menu when clicking outside of it? Right now, we need to click 2 times to close the whole menu.

Also, is possible to close all the sub-menu/menu when a user click on an item?

Thank you!

@janhoeck
Copy link
Author

@Jak3b0 You can do it with the ClickAwayListener. Take a look https://codesandbox.io/s/6voj4o458n

@Jak3b0
Copy link

Jak3b0 commented Jan 25, 2019 via email

@varshasaha
Copy link

Can I take this up?

@eps1lon
Copy link
Member

eps1lon commented Jan 31, 2019

Can I take this up?

That would be really great. The goal is to add a SubMenu (or cascading menu in material speak) to the demos. #8152 and #14345 (comment) are related.

@eps1lon eps1lon changed the title SubMenu should not overlap parent menu [Menu] Support cascading menus Jan 31, 2019
@JeremyGrieshop
Copy link

I agree, this would be awesome to have! I created a similar SubMenu component to do this, but it lacks all of the nuances with clicking away. For example, clicking away from all should close, but clicking away on another parent SubMenu should close all up to the clicked menu item.

I also kinda wish the ability to open on hover was an option, but that's probably for another task. Hover open gives you a much more responsive, desktop-like experience. Clicking and waiting for menu popup transitions takes forever when you need to navigate submenus 3 or 4 deep, for example.

@nkappler
Copy link

nkappler commented Aug 4, 2021

Since the new Popper is part of the MenuItem, the onMouseLeave event is not triggered for any submenus

@JeremyGrieshop
Copy link

Since the new Popper is part of the MenuItem, the onMouseLeave event is not triggered for any submenus

Got a link to a codesandbox?

@nkappler
Copy link

nkappler commented Aug 4, 2021

Got a link to a codesandbox?

https://codesandbox.io/s/stoic-mcnulty-ux2l8?file=/src/App.tsx

Here you go, there's really not much to it...

@nkappler
Copy link

nkappler commented Aug 4, 2021

The root menu has to be implemented with a Popper + ClickAwayListener, I think. Popover won't work because then the submenus event handlers don't work

@JeremyGrieshop
Copy link

JeremyGrieshop commented Aug 4, 2021

The root menu has to be implemented with a Popper + ClickAwayListener, I think. Popover won't work because then the submenus event handlers don't work

There's a lot I like about the simplicity of this solution! The current PR for this (#20591 ) solves some accessibility, keyboard navigation, RTL support, and additional styling, too. I had been using a custom solution myself, but I may try to convert it over to yours until the PR can get merged.

But, I'd probably add keyboard events and a transition/delay for the Poppers.

One thing that would probably be difficult with your solution is the "triangulation" feature that was being discussed in the PR thread. Which helps with accidentally onMouseLeave() when you really just wanted to move at a cursor angle into the currently open submenu. See the PR for details.

@nkappler
Copy link

nkappler commented Aug 4, 2021

The current PR for this (#20591 ) solves some accessibility, keyboard navigation, RTL support, and additional styling, too.

Yes, that's a good point, I also had it in mind but just wanted to write a quick POC tailored to my needs

One thing that would probably be difficult with your solution is the "triangulation" feature that was being discussed in the PR thread. Which helps with accidentally onMouseLeave() when you really just wanted to move at a cursor angle into the currently open submenu.

I have noticed this already, too. Could be a bit tricky...

I have written a cascading (context-)menu once ( nkappler/ctxmenu - shameless plug, I know 😅) and I don't want to do it again, so I will probably use my solution for now and also wait for the PR you mentioned 😄

@nkappler
Copy link

nkappler commented Aug 5, 2021

One thing I just noticed, which the PR hasn't solved yet is parent MenuItems staying highlighted when a sub(sub...)menu is open.
This would be very easy to implement with my solution.

I have update the codesandbox to enable the highlighting
https://codesandbox.io/s/stoic-mcnulty-ux2l8

@mbrevda
Copy link

mbrevda commented Sep 30, 2021

This would be really cool. Any chance this could also enable nested tabs (think vertical tabs, with sub-tabs)?

@mgara

This comment was marked as off-topic.

@Dattaya
Copy link

Dattaya commented May 21, 2023

Another implementation that uses Menu underneath, but in Material UI 5.
It's also fairly simple, 70 lines of code and that includes keyboard navigation, unlimited nesting, and types: https://codesandbox.io/s/material-ui-submenu-sz73h7?file=/src/App.tsx

@mbrookes
Copy link
Member

@Dattaya Very impressive! Am I wrong to expect submenus to open with the spacebar, as they do for the main menu? I'm a little rusty in the keyboard navigation requirements. Aside from that (and that may not be an issue), it's impressive how few lines of code you achieved this in compared to alternative implementations!

@Dattaya
Copy link

Dattaya commented Jul 2, 2023

@Dattaya Very impressive! Am I wrong to expect submenus to open with the spacebar, as they do for the main menu? I'm a little rusty in the keyboard navigation requirements. Aside from that (and that may not be an issue), it's impressive how few lines of code you achieved this in compared to alternative implementations!

hey, I checked Chrome and VSCode and the spacebar didn’t do anything (Linux), but Enter on the other hand did, thanks for noticing that, it’s fixed now, and thanks for your kind words about the implementation. There’s still room for improvement like a dynamic hit area to prevent accidental menu closings, also, the component is not going to be as simple for those who want the previous sub-menu to stay open instead of everything collapsing immediately. It’s more than enough for me, because I only needed a one level deep submenu.

In addition I’d like to explain this line of code: TransitionProps={{ onExited: () => menuItemRef.current?.focus() }} that was added after I’d posted the message. There’s a bug that not many people are going to run into, as it only happens when combining both mouse and keyboard navigation. For some reason, TrapFocus focuses on the first item in the previous list before opening a submenu and as a result, when the submenus collapse, the focus jumps to the first menu item so I had to control it manually.

menu.mp4

@oliviertassinari oliviertassinari changed the title [Menu] Support cascading [Menu] Support cascading / nested menu Aug 30, 2023
@hzhz hzhz linked a pull request Nov 30, 2023 that will close this issue
@mj12albert mj12albert added the waiting for 👍 Waiting for upvotes label Dec 29, 2023
@Kepron
Copy link

Kepron commented Jan 24, 2024

I have a suggestion. Wouldn't it be better to have a structure that changes content instead of a menu that opens side by side?

Examples:

youtube.mp4
example.mp4

Source: https://www.primefaces.org/primereact-v8/slidemenu/

@JeremyGrieshop
Copy link

I have a suggestion. Wouldn't it be better to have a structure that changes content instead of a menu that opens side by side?

On mobile devices, I completely agree. Otherwise, it really depends. I often like to see the hierarchy, the ancestors and siblings, etc. all in one glance to be able to quickly change my selection without having to methodically traverse back through.

@yegor211
Copy link

yegor211 commented Feb 2, 2024

hey there guys

am I right that there is no implemented solution in the mui for the nested menus? it's odd to see that as it's a very common thing and the library is so huge and neat...

@sai6855
Copy link
Contributor

sai6855 commented Mar 25, 2024

Hey everyone, i've added Nested Menu demo to docs in this PR (not merged yet) #37570,
here is the demo link: https://deploy-preview-37570--material-ui.netlify.app/material-ui/react-menu#nested-menu. Can you guys try it out and see if the demo works for you.

Code turned out to be complex as it handled lot of edge cases and has to implement this behaviour: https://x.com/diegohaz/status/1283557901701918720?s=20

@redbmk
Copy link
Contributor

redbmk commented Jul 31, 2024

I was able to workaround this in Base UI's Menu as well. Feels a bit hacky and uses an undocumented feature from what I can tell, but it works.

I tried to simplify this as much as I could, but this is pretty much the bare minimum I could see necessary.

I'm wrapping the cascading menu in a group with the MenuItem and adding shortcuts to open and close the submenu with the right and left arrow keys. Most of the rest of the functionality is the same. If you're in a submenu, you need to Esc or click away twice to close both menus.

One improvement to this I could see is clicking an item in the submenu should close both rather than just the submenu

/* cascading-menu.module.css */
.menu,
.submenu {
  display: block;
  padding: 10px;
  border: 1px solid black;
  background: white;

  ul {
    list-style: none;
    padding: 0;
  }
}

.group {
  display: flex;
}
// CascadingMenu.tsx
import { MenuItem } from "@mui/base";
import { Dropdown } from "@mui/base/Dropdown";
import { Menu } from "@mui/base/Menu";
import { MenuButton } from "@mui/base/MenuButton";
import {
  type FocusEvent,
  type KeyboardEvent,
  type MouseEvent,
  useRef,
  useState,
} from "react";
import styles from "./cascading-menu.module.css";

const preventMuiDefault = (e: KeyboardEvent | MouseEvent | FocusEvent | null) => {
  if (!e) return;

  // @ts-expect-error undocumented MUI API
  e.defaultMuiPrevented = true;
};

const isSubmenuOpen = () =>
  Boolean(document.querySelector(`.${styles.submenu}.base--open`));

export function CascadingMenu() {
  const [submenuOpen, setSubmenuOpen] = useState(false);
  const menuItemWithSubmenu = useRef<HTMLLIElement>(null);

  return (
    <Dropdown>
      <MenuButton>Menu</MenuButton>
      <Menu
        className={styles.menu}
        slotProps={{
          root: { placement: "bottom-start" },
          listbox: {
            onKeyDown(e) {
              if (e.key === "ArrowRight" || isSubmenuOpen()) {
                preventMuiDefault(e);
              }
            },
            onBlur(e) {
              if (isSubmenuOpen()) {
                preventMuiDefault(e);
              }
            },
          },
        }}
      >
        <MenuItem onClick={() => alert("Hi!")}>Hi!</MenuItem>
        <div role="group" className={styles.group}>
          <MenuItem
            ref={menuItemWithSubmenu}
            onClick={() => alert("Hello!")}
            onKeyDown={(e) => {
              if (e.key === "ArrowRight") {
                setSubmenuOpen(true);
                preventMuiDefault(e);
              }
            }}
          >
            Hello!
          </MenuItem>
          <Dropdown
            open={submenuOpen}
            onOpenChange={(e, open) => {
              setSubmenuOpen(open);
              if (!open) {
                preventMuiDefault(e);
                menuItemWithSubmenu.current?.focus();
              }
            }}
          >
            <MenuButton slotProps={{ root: { tabIndex: -1, role: "presentation" } }}>
              ...
            </MenuButton>
            <Menu
              className={styles.submenu}
              slotProps={{
                root: { placement: "bottom-start" },
                listbox: {
                  onKeyDown(e) {
                    if (e.key === "ArrowLeft") {
                      setSubmenuOpen(false);
                      preventMuiDefault(e);
                      menuItemWithSubmenu.current?.focus();
                    }
                  },
                },
              }}
            >
              <MenuItem onClick={() => alert("Sup!")}>Sup!</MenuItem>
              <MenuItem onClick={() => alert("Yo!")}>Yo!</MenuItem>
            </Menu>
          </Dropdown>
        </div>
      </Menu>
    </Dropdown>
  );
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: menu This is the name of the generic UI component, not the React module! new feature New feature or request waiting for 👍 Waiting for upvotes
Projects
None yet