Nested menus/submenus

[MathiasWP]

Describe the bug

We are frequently using the sl-dropdown component for dropdowns. We have some use cases where it would be preferable to show a new dropdown within a dropdown. The problem/bug we are trying to solve is to use the hoist-property on the inner-most dropdown. The position of the inner dropdowns content does not match the trigger when hoist is set.

To Reproduce

See codepen below.

Demo

https://codepen.io/MathiasWP/pen/VwMpvWz

Screenshots

Browser / OS

• OS: Mac
• Browser: Tested on Brave, Safari, Firefox, Chrome and Opera.
• Browser version: Unsure, probably latest.

Additional information

I've tried to get a better understanding of how the dropdown-component works by reading through the source code. I see that the popover is created with the popper.js library, and that setting the host property is equal to setting the strategy to fixed. The thing that puzzles me is that in the popper.js documentation, they say that "If your reference element is in a fixed container, use the fixed strategy". My understanding of the popper.js library is that they use getBoundingClientRect to calculate coordinates, so i don't quite understand why this bug occurs.

[MathiasWP]

I've explored the popper.js library some more, and i think i've found some interesting stuff:

It seems like the calculation of the coordinates is a combination of multiple values from the position of the trigger and its parent. Moving the dropdown to the left of the viewport moves the popover with the same distance. This applies to both the X and Y axis:

I've attached a video here showing it in action. I feel like the popovers position is based on a combination of the trigger and its parent (the initial trigger button), but this is just a hypothesis. I do not have time to look deep into the source code of both projects to determine what the exact behaviour is.

Screen.Recording.2021-12-15.at.12.32.25.mov

Here is the repo i have played around with: https://github.com/MathiasWP/dropdown-hoist-bug-demo
To set it up, just run npm install and then npm run dev.

Having this feature has become important for our startup-company, but i do not have time myself to find a solution right now. Would you be able to look into this, @claviska? I am willing to donate some of my own money to support you and this great project - if that can help.

[claviska]

I wouldn't use the approach you described above because, even if you get positioning nailed down, it requires an additional click to open the submenu and keyboard interactions won't work as users expect.

To do this correctly, some work needs to be done in <sl-menu-item> to support a nested <sl-menu>. I don't think more than one layer of nesting should be a requirement, as each level of nesting makes it harder to use.

I think using the same slot="submenu" pattern makes sense here.

Here's an example of FAST's menu which does this well: https://jsfiddle.net/7qvzjcpe/

From the fiddle:

<fast-menu>
    <fast-menu-item>Menu item 1<fast-menu slot="submenu">
            <fast-menu-item>Menu item 1.1</fast-menu-item>
            <fast-menu-item>Menu item 1.2</fast-menu-item>
            <fast-menu-item>Menu item 1.3</fast-menu-item>
        </fast-menu>
    </fast-menu-item>
    <fast-menu-item>Menu item 2<fast-menu slot="submenu">
            <fast-menu-item>Menu item 2.1</fast-menu-item>
            <fast-menu-item>Menu item 2.2</fast-menu-item>
            <fast-menu-item>Menu item 2.3</fast-menu-item>
        </fast-menu>
    </fast-menu-item>
    <fast-menu-item>Menu item 3<fast-menu slot="submenu">
            <fast-menu-item>Menu item 3.1</fast-menu-item>
            <fast-menu-item>Menu item 3.2</fast-menu-item>
            <fast-menu-item>Menu item 3.3</fast-menu-item>
        </fast-menu>
    </fast-menu-item>
</fast-menu>

This isn't top of my radar, but it's a valid request and I'd like to add support to it. I just can't guarantee a time right now.

[MathiasWP]

Thank you for answering! I still mean that there are valid use cases where it makes sense to have nested dropdowns, even if it requires an additional click to open. Look at the example above from Notions menu.

The slot="submenu" is also a great feature, and i agree that it would also be nice to have.

It would be very useful for us to have this feature asap, could we maybe discuss ways to move this feature higher up on your radar? 😄

[claviska]

Ah, yeah we're talking about two different things. Sorry for the confusion!

Regarding your CodePen, the reason it happens when you use hoist is that it switches to a fixed positioning strategy to "break out" of any containing overflow. However, this makes the hoisted element relative to its containing block, which is usually the viewport. However, some properties can cause this to vary. It gets complicated.

I consider this a gap in the platform. It's one that many are aware of and there are proposals to resolve it. One is a CSS property (suggested by me) and another is an HTML Element.

As such, I don't think this is something we can "fix." The hoist property in Shoelace is already a workaround. Once we have a better solution, the positioning mechanism can adapt it and this will likely solve itself.

I'm aware that's not great news for you. Have you considered the alternative approach of reworking your overflows so you don't have to use hoist?

[MathiasWP]

No worries, thank you for the answer! I agree that it is a gap in the platform, but isn't that what popper.js is trying to solve with the strategy: fixed configuration? Because even though the containing block is the viewport, they try to solve this by using getBoundingClientRect to get the coordinates of the container, so the correct position can still be calculated.

I may be missing something in my intuition, so feedback is appreciated.

[ak-beam]

This may be a different feature request, but for our use cases the ability to have a collapsible indented menu would be more the behavior we need from a submenu.

Item 1
Item 2
Submenu Title (collapsed by default)
  Submenu Item 1
  Submenu Item 2

I could potentially spend some time implementing this if we can agree on what behavior the library should have out of the box.

[ak-beam]

Note: I ended up using the Tree / TreeItem components for my use case