Page MenuHomePhabricator

FloatableElement with overlay doesn't work if anchor is not in a scrollable area
Closed, ResolvedPublic

Description

Or more precisely, if the closest scrollable of the anchor and the closest scrollable of the overlay are the same. This is a regression from https://gerrit.wikimedia.org/r/#/c/276664/ (for T129521). The culprit is this.needsCustomPosition = closestScrollableOfContainer !== closestScrollableOfFloatable;. This is plainly false, because (like in our case) the floatable can be in an overlay at (0,0) while the container is somewhere else, in which case positioning is definitely needed, but depending on the scrollbar situation the closest scrollable container could be the window for both of them.

This caused T130153: [betalabs] Regression: 'Mark as read/unread' box is misplaced in Echo, where I worked around it by ensuring the anchor was in an always-scrollable container.

The demo doesn't expose this, because it only tests overlays with dialogs where there are several layers of scrollables separating the anchor from the overlay. https://gerrit.wikimedia.org/r/277931 adds a demo that exposes the broken behavior.

Event Timeline

Change 277931 had a related patch set uploaded (by Catrope):
[WIP] Failing demo for DropdownWidget with an overlay

https://gerrit.wikimedia.org/r/277931

Sorry, I've been working on UploadWizard issues all day today. @Catrope, if you have the time, I'd appreciate if you took a stab at this (especially since you already know what is going on?). If I don't have time for it before next release, I guess we'll just revert that patch.

matmarex lowered the priority of this task from High to Medium.
matmarex edited projects, added Regression; removed Patch-For-Review.
matmarex added a subscriber: matmarex.

Since Echo was the only affected user and there's a solid workaround there, this isn't really very high priority, and I don't really have time to work on it soon. Sorry. :(

Change 332758 had a related patch set uploaded (by Bartosz Dziewoński):
FloatableElement: More correctly decide if we need custom position

https://gerrit.wikimedia.org/r/332758

Change 277931 merged by jenkins-bot:
demo: Failing demo for DropdownWidget with an overlay

https://gerrit.wikimedia.org/r/277931

Change 332758 merged by jenkins-bot:
FloatableElement: More correctly decide if we need custom position

https://gerrit.wikimedia.org/r/332758