Page MenuHomePhabricator

PopupWidget at the bottom of the container is unreadable
Closed, ResolvedPublic

Description

When the PopupWidget is at the bottom of its container, it will be so thin that it completely obscures the content.

Event Timeline

Tgr created this task.Sep 24 2016, 9:35 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptSep 24 2016, 9:35 AM

The usual solution to this is to specify a $overlay in which to place the popup, so that it can extend beyond the boundaries of the container of the button. (See "Dialog with dropdowns ($overlay test)" in the dialogs demo to see what this does.)

But apparently PopupButtonWidget doesn't yet support this :/

You could probably implement it by mixing in FloatableElement into PopupWidget, and then adding $overlay to config of PopupElement and passing it as $floatableContainer to PopupWidget. See FloatingMenuSelectWidget and DropdownWidget for an example. I have not tried, but it will likely just work.

Another option would be T114612 and using that to render the popup "upside down" (appearing above the button, with the little arrow pointing down).

Change 313623 had a related patch set uploaded (by Gergő Tisza):
[TEST] Make PopupElement extend over containing blocks

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

Tgr added a comment.Sep 30 2016, 9:45 PM

You could probably implement it by mixing in FloatableElement into PopupWidget, and then adding $overlay to config of PopupElement and passing it as $floatableContainer to PopupWidget. See FloatingMenuSelectWidget and DropdownWidget for an example. I have not tried, but it will likely just work.

Tried that but probably misunderstood what needs to be done (see patch above); there was no visible change.

Another option would be T114612 and using that to render the popup "upside down" (appearing above the button, with the little arrow pointing down).

Tried that too but it's beyond my CSS fu. There are at least two nontrivial problems:

  • how do you apply the drop shadow to the anchor? The diagonal border trick does not work; using a css-rotated square would but I vaguely remember that the OOUI people avoided that for a reason.
  • how do you position to the top of the target? Bottom positioning just works, because that's how auto is defined for absolute positioning. To position to the top, one would have to offset it from JS by the height of the target element, which might be an inline element; that seems fragile.

I think the approaches are too complicated so far.
The Gods of CSS gave us min-height – let's use it.

I also hope, that I haven't missed something that would backfire here. Just in case, @matmarex you've added two patches for ClippableElement and FloatableElement in v0.16.3 to never exceed browser viewport. If we're rendering content unreadable/unusable I think that should allow my approach. Objections?

Change 330351 had a related patch set uploaded (by VolkerE):
ClippableElement: Add min-height for usability in edge cases

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

Volker_E moved this task from Backlog to Reviewing on the OOUI board.Jan 4 2017, 1:49 AM

I don't see how this is an improvement.

Previously you got this:

Now you get this:

Or for PopupButtonWidget specifically:

@matmarex The improvement: You can use it again?! There's a scrollbar on canvas if the popup overflows and you're able to scroll and read within the popup.
I don't say that this is the ultimate solution, but it makes something unusable at least usable. It's more an intermediate step to mitigate the current bad situation we're putting the user in.
What's your take on the issue?

OK, I looked at it again more carefully (and at the very helpful demo examples which Prateek added, that I missed entirely at first) and I see how it helps in certain edge cases. Screenshots of the new demo:

BeforeAfter

Change 330351 merged by jenkins-bot:
ClippableElement: Add min-height for usability in edge cases

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

@matmarex Thanks for looking closer into it and reconsideration!

Volker_E closed this task as Resolved.Jan 4 2017, 7:59 PM
Volker_E claimed this task.
Volker_E triaged this task as Medium priority.
Volker_E removed a project: Patch-For-Review.
Volker_E moved this task from Reviewing to OOjs-UI-0.18.4 on the OOUI board.
Volker_E edited projects, added OOUI (OOjs-UI-0.18.4); removed OOUI.

Change 330462 had a related patch set uploaded (by Bartosz Dziewoński):
PopupButtonWidget: Add $overlay config option

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

matmarex reopened this task as Open.Jan 4 2017, 8:09 PM

You could probably implement it by mixing in FloatableElement into PopupWidget, and then adding $overlay to config of PopupElement and passing it as $floatableContainer to PopupWidget. See FloatingMenuSelectWidget and DropdownWidget for an example. I have not tried, but it will likely just work.

Tried that but probably misunderstood what needs to be done (see patch above); there was no visible change.

I implemented that too now, see patch above. It turned out to be a little more complicated than I was hoping, but not terrible.

Screenshots of another new demo:

Screenshot of ApiSandbox, with that patch and updated https://gerrit.wikimedia.org/r/#/c/313623/:

Change 330462 merged by jenkins-bot:
PopupButtonWidget: Add $overlay config option

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

matmarex closed this task as Resolved.Jan 24 2017, 11:36 AM
matmarex removed a project: Patch-For-Review.