Page MenuHomePhabricator

Flow toolbar overlaps with mention search
Closed, ResolvedPublic1 Estimated Story Points

Assigned To
Authored By
Trizek-WMF
Jun 28 2017, 1:06 PM
Referenced Files
F8694170: image.png
Jul 10 2017, 7:18 PM
F8694166: image.png
Jul 10 2017, 7:18 PM
F8694175: image.png
Jul 10 2017, 7:18 PM
F8694191: image.png
Jul 10 2017, 7:18 PM
F8548972: Screen Shot 2017-06-28 at 11.53.34 AM.png
Jun 28 2017, 7:33 PM
F8548979: Screen Shot 2017-06-28 at 12.25.17 PM.png
Jun 28 2017, 7:33 PM
F8548976: Screen Shot 2017-06-28 at 12.11.33 PM.png
Jun 28 2017, 7:33 PM
F8547935: Screen Shot 2017-06-28 at 7.39.23 AM.png
Jun 28 2017, 2:40 PM
Tokens
"Like" token, awarded by Elitre.

Description

Capture d’écran_2017-06-28_15-02-42.png (280×434 px, 25 KB)

I have the bug on Mediawiki wiki. I don't have this bug on Hebrew Wikipedia ATM, which means it is due to something in the deployment train.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript
Trizek-WMF moved this task from Untriaged to Hot on the Collaboration-Team-Triage board.

Yes, betalabs displays the same issuse for the link and mention inspectors.
Note: There is no such "transparency" for link ink inspector on non-Flow pages.

Screen Shot 2017-06-28 at 7.39.23 AM.png (258×493 px, 34 KB)

Probably my changes from T167616 caused this :(

To check after the fix: since it's possible to do VE insert in Flow via cmd+?, the following needs to be checked.
(1) In Flow post, do additional checking for some other insert (hieroglyph, chemical or math formulas etc).

Screen Shot 2017-06-28 at 11.53.34 AM.png (292×499 px, 17 KB)

(2) Check for editing to see if the following has been fixed.
misaligned icons
Screen Shot 2017-06-28 at 12.11.33 PM.png (265×786 px, 28 KB)

(3) the link inspector cannot be dismissed
Screen Shot 2017-06-28 at 12.25.17 PM.png (292×809 px, 31 KB)

Catrope subscribed.

Looks like this is a bug with VisualEditor's bottom toolbar support

@matmarex Can you take a look at this when you get a chance? Thanks!

This is actually more difficult than it looks like, since inspectors are in the same z-index "layer" as e.g. surface highlights and context menus. If we were to simply change the order so that toolbar (.ve-ui-toolbar > .oo-ui-toolbar-bar) has lower z-index than the overlay (.ve-ui-overlay), we'd instead get this:

image.png (204×754 px, 7 KB)
image.png (204×754 px, 8 KB)
Inspector looks okay…
image.png (204×754 px, 10 KB)
The context menu looks okay…
image.png (204×754 px, 9 KB)
But toolbar menus are not okay

Generally, the more I think about this, the more I feel that bottom-positioned toolbars are just fundamentally a bad idea. Consider the case in the third screenshot above – should we have the context menu cover the bottom toolbar (potentially making its tools inaccessible when the cursor is in the wrong place), or should we have the bottom toolbar cut off the context menu (potentially making its actions inaccessible)? Toolbars on top do not have this problem.

Actually, as far as I can tell, the broken behavior I described above is what was happening before my changes from T167616, and apparently everyone was happy with it in spite of the fundamental brokenness, so who am I to argue. I don't have Flow set up locally, so I'd appreciate if someone could verify that it was broken that way before (surface highlights appearing above open toolbar menus). If it was somehow working correctly, then we should revert the fixes for T167616 and then cry.

Change 364323 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[VisualEditor/VisualEditor@master] Workaround for Flow bottom toolbars conflicting with inspectors

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

I don't have Flow set up locally, so I'd appreciate if someone could verify that it was broken that way before (surface highlights appearing above open toolbar menus). If it was somehow working correctly, then we should revert the fixes for T167616 and then cry.

I set it up and tested, and it wasn't broken before. I cried, but then instead of reverting I realized we could do the thing that the patch above does.

Change 364323 merged by jenkins-bot:
[VisualEditor/VisualEditor@master] Workaround for Flow bottom toolbars conflicting with inspectors

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

(See also T128088 about moving the toolbar to the top. I repeated my general comments there.)

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

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

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

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

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