Page MenuHomePhabricator

Lookup menu "stuck" in scrolling Dialog
Closed, ResolvedPublic3 Estimated Story PointsBUG REPORT

Description

Steps to replicate the issue (include links if applicable):

  • Create a new Codex App on wiki (for example in a userscript, don't think this part is important)
  • Add a Dialog component with content that scrolls
  • Add a Lookup to the content of that Dialog
  • add that little App to the page and open the dialog
  • type a few characters to make the Lookup show some options
  • scroll the content of the Dialog

What happens?:
The menu of the Lookup stays stuck in place where it was first rendered.

What should have happened instead?:
The menu of the Lookup stays attached to the Lookup as the Dialog content is scrolled.

Other information (browser name/version, screenshots, etc.):
A minimal failing example: https://test.wikipedia.org/wiki/User:Zvpunry/codexLookupMenuStuck.js
Use it like mw.loader.load( 'https://test.wikipedia.org/w/index.php?title=User:Zvpunry/codexLookupMenuStuck.js&action=raw&ctype=text/javascript' );

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript
CCiufo-WMF set the point value for this task to 3.Aug 21 2023, 5:08 PM

Thanks @Celenduin for the repro userscript, that's very handy for debugging the CSS here.

What is happening?

The reason that this is happening is that the Menu element uses absolute positioning when it appears. Normally the absolutely-positioned Menu element will be a child of something like Select or Lookup, where the outer-most element uses position: relative so that the menu can "fly out" when it opens but remains connected to the input element above it, etc.

Our problem was that we wanted menu elements inside of Dialogs to be able to break free of the boundaries of the dialog if they needed to (per this example here). In order to do that, I added an override to the menu styles where their position would revert to position: static, meaning that the absolutely-positioned menus would be laid out relative to the next-nearest parent element that uses non-static positioning – in this case the Dialog backdrop itself (which wraps the Dialog and uses position: fixed – handled the same as absolute positioning in this situation).

Your example clearly demonstrates that I wasn't thinking about menu components appearing inside long-scrolling dialog content. I had hoped that by using v-bind in CSS to bind ensure that the menu remained the same width of its no-longer-relatively-positioned parent I could get around the limitations of this approach, but that doesn't take the scroll position into account.

Potential fixes
  • Just setting position: relative on some parent element in the Dialog (cdx-dialog__body maybe) doesn't really solve things – that fixes the broken behavior in your example (see screenshot below) but it means we lose the ability of menus to pop-out of the dialog boundaries, which is not some thing we want.
  • Another option to explore might be introducing some additional markup inside the menu-using components (Select, Lookup, and Combobox) where an additional wrapper element can surround both the menu as well as the input; maybe that element could act as a relatively-positioned container without breaking other parts of the layout.

I'm going to investigate the second option here to see how promising this is. If that approach also proves insufficient, then we may need to rely on the outcome of T345116: [SPIKE] Determine how to provide comprehensive solutions for overlay sizing and positioning – Codex will likely need one or more composables that offer similar functionality to OOUI's "clippableElement" mixin. But perhaps we can solve this particular problem in CSS as opposed to JS.

Screenshot 2023-08-29 at 9.43.52 AM.png (2×1 px, 247 KB)

I did a little surveying of how other UI libraries handle this problem. I believe that Bootstrap's Modal component demonstrates the behavior that we want in Codex: https://getbootstrap.com/docs/5.3/components/modal/#tooltips-and-popovers

  • Popover elements can break out of the dialog frame
  • Such elements remain "anchored" to whatever elements they are meant to appear visually connected to, even if the user scrolls the dialog content.

It looks like Bootstrap achieves this by doing the following:

  • A "popover" element creates a new, absolutely-positioned <div> that appears outside the .modal-dialog element entirely (despite the fact that the button which triggers the popover/tooltip lives inside the dialog body).
  • The popover uses the following styles:
    • position: absolute
    • inset values of either 0px or auto for each of the 4 directions (based on how the pop-up is positioned in relation to its trigger)
    • transform: translate3d( x, y, z ) – the Z value is always zero, but it looks like the other values are calculated based on getBoundingClientRect

As I was digging into the Bootstrap source for their dropdown / popover components, I saw that they were relying on a library called PopperJS.

It looks like the developers of Popper have created a new library called floating-ui which deals with similar concerns, and there are even bindings for Vue. So maybe we ought to look into either adapting or just incorporating FloatingUI into Codex.

After thinking a little more about this, I'm afraid that we cannot solve this issue without implementing a comprehensive solution for all "floating" elements in Codex.

I think that we should either incorporate the above-mentioned floating-ui library into Codex as a dependency, or we should re-implement the minimum parts of it that we need ourselves.

I think the task to discuss that larger project is T345116: [SPIKE] Determine how to provide comprehensive solutions for overlay sizing and positioning, so I'm going to merge this task there as a good example of the kind of layout problem that any solution we adopt will need to eventually solve.

Thanks @Celenduin for the bug report and repro example, and hopefully we'll have a comprehensive solution soon.

Re-opening after some team discussion – once we complete T345116, we will create a new task based on whichever implementation path seems most promising. This task will then be set as a sub-task of that one – consider it a test case for whether or not we've figured out the right solutionl

egardner subscribed.
Volker_E subscribed.

@Celenduin The Floating UI change has been made part of Codex v0.20.0 and is available on NPM since yesterday and got merged into MediaWiki as of today. It's available on the beta cluster as of today and will ride the normal train starting next Tuesday.
Resolving this, please feel free to re-open in case of unwanted results.

Thank you so much to the entire team! 🙏
I look forward to using it 😊