Page MenuHomePhabricator

Inspector's popup handle misplaced when close to edge of surface in Flow+VE
Closed, ResolvedPublic8 Estimated Story Points

Description

image.png (93×132 px, 2 KB)

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

(1) To add more detailed screenshots:

  • the handle of the link inspector correctly displayed:

Screen Shot 2017-05-30 at 11.50.03 AM.png (273×513 px, 22 KB)

  • a cursor is moved to the beginning of the word:

Screen Shot 2017-05-30 at 11.50.12 AM.png (215×497 px, 16 KB)

(2) (unrelated to the reported issue and not specific to Flow pages) There are cases when the entire link inspector gets misplaced (or de-attached). When links are entered continuously or when link were entered via source editing with forced new lines, or copy/paste links separated with newlines.

Screen Shot 2017-05-30 at 11.58.01 AM.png (276×1 px, 89 KB)

Screen Shot 2017-05-30 at 11.51.20 AM.png (717×455 px, 208 KB)

Screen Shot 2017-05-30 at 11.59.29 AM.png (507×593 px, 174 KB)

Jdforrester-WMF moved this task from To Triage to TR0: Interrupt on the VisualEditor board.
Jdforrester-WMF set the point value for this task to 8.

If I had to guess, I would guess that somewhere, a $container config option for this PopupWidget is set so that it is not allowed to extend past the boundaries of the edit field (or it is not set and the autodetected value is wrong).

This is ve.ui.DesktopContext#popup, I think.

Setting a breakpoint in OO.ui.PopupWidget.prototype.computePosition and doing this.$container = $( '#content' ); resolves the problem, so I guessed right. But that leaves two problems:

  • We'd need to set a different container for every skin, or just set it to use OO.ui.Element.static.getRootScrollableElement, but that will cause the popups to appear over the sidebar, which might be undesirable.
  • We'd need to somehow pass that from mw.flow.ve.Target (I think) to ve.ui.DesktopContext.

Change 429188 had a related patch set uploaded (by Esanders; owner: Esanders):
[VisualEditor/VisualEditor@master] Allow surfaces to change context popup's container

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

Change 429189 had a related patch set uploaded (by Esanders; owner: Esanders):
[mediawiki/extensions/VisualEditor@master] Set surface's $overlayContainer in DesktopArticleTarget

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

Change 429190 had a related patch set uploaded (by Esanders; owner: Esanders):
[mediawiki/extensions/Flow@master] Set surface's $overlayContainer in mw.flow.ve.Target

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

Change 429188 merged by jenkins-bot:
[VisualEditor/VisualEditor@master] Allow surfaces to change context popup's container

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

Change 429526 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (e673ad6de)

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

Change 429526 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (e673ad6de)

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

Change 429189 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] Set surface's $overlayContainer in DesktopArticleTarget

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

Change 429190 merged by jenkins-bot:
[mediawiki/extensions/Flow@master] Set surface's $overlayContainer in mw.flow.ve.Target

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