Page MenuHomePhabricator

Menu should be able to extend past the edge of its container
Closed, ResolvedPublic

Description

When a menu appears in a height-limited container (or a container that is horizontally scrollable, even if it's not normally vertically scrollable), it can get taller than the menu even if the number of options is relatively small (10 in this case), and that causes the container to become scrollable if the menu is near the bottom of the container. The same thing will happen once we have dialogs, when dropdowns or other menus appear near the bottom of the dialog content.

Example: (the table has overflow-x: auto in this case)

Screenshot from 2022-04-27 11-10-09.png (376×745 px, 33 KB)
Screenshot from 2022-04-27 11-10-15.png (368×734 px, 36 KB)

Instead, we should allow the menu to extend past the edge of the container, like this:

Screenshot from 2022-04-27 11-18-57.png (548×773 px, 58 KB)

On a technical level, this would require the developer using the Menu to create an "overlay" (empty div at/near the bottom of the page; or can we perhaps use position: fixed and have the overlay be much closer to the Menu?), then pass that overlay to the Menu so that it can teleport itself into that div and be absolutely positioned in the right place, which requires measuring the position of the parent component's anchor element (the input in Lookup, or the "handle" in Select). We should consider how much of this can be handled by Menu itself, and how much of it needs to be handled by the parent components (at minimum, the parents would have to pass through the reference to the overlay, and pass in a reference to the anchor element).

When we implement the Dialog component, we can make each Dialog create its own overlay, and then provide() that overlay to its children, so that Menu automatically picks it up (inject()s) it and uses it. This would make putting a Select in a Dialog work automatically, without needing an overlay to be specified manually. In addition (or instead), we could consider creating an OverlayWrapper component that wraps its children and provides them with an overlay, if the position: fixed approach hinted at above is feasible.

Event Timeline

This is related to T304041: Consider enhancements for Selects and other menus with many items, and sort of part of it, except that it doesn't exclusively arise when there are many items.

STH triaged this task as Medium priority.Apr 30 2022, 3:58 PM
STH moved this task from Inbox to Up Next on the Design-System-Team board.

@Catrope it seems like this can be added to the acceptance criteria for T306932: Add configurable scroll behavior to the Menu component as a design note - what do you think?

It's somewhat related but largely separate, and the engineering impact is significant enough that I'd like it to be it's own task, if that's OK with you.

This is de facto being done as part of T284838

I think related to this task should also be the correct placement of the menu on the TOP of the component if there is not enough scrolling space at the bottom like OOUI currently does:

{F35248811}

As noted by @bmartinezcalvo in a Slack thread earlier today and Simone describes above, another solution for this problem is the menu opening up if there's not enough room for it below. However, we may need to do both, so that it works like this when a menu won't fit in its container:

  • If there's enough room for the menu above the component, show it above the component
  • If there's not enough room either below or above the component, show it below and let it be overlaid on top of other content

Update: we abandoned our original overlay approach for the Dialog component (T284838) and went with another solution: special styles for components with menus when they are inside a dialog. Here's the code for the Select component:

.cdx-select {
    position: relative;

    .cdx-dialog & {
        position: static;

        .cdx-menu {
            left: auto;
            // currentWidthPx is calculated in the script and passed into the CSS.
            width: v-bind( currentWidthInPx );
        }
    }
}

This means we haven't resolved this task, or the related bug T312121.

Yes this is done now