Page MenuHomePhabricator

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

Description

Event Timeline

Esanders created this task.May 24 2017, 9:57 AM
Restricted Application added a project: Collaboration-Team-Triage. · View Herald TranscriptMay 24 2017, 9:57 AM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript

(1) To add more detailed screenshots:

  • the handle of the link inspector correctly displayed:

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

(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.

Jdforrester-WMF triaged this task as Normal priority.May 30 2017, 5:34 PM
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

Deskana closed this task as Resolved.May 10 2018, 1:02 PM