Flow toolbar overlaps with mention search
Closed, ResolvedPublic1 Story Points

Description

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.

Restricted Application added a project: Collaboration-Team-Triage. · View Herald TranscriptJun 28 2017, 1:06 PM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript
Trizek-WMF triaged this task as High priority.Jun 28 2017, 1:06 PM
Trizek-WMF moved this task from Untriaged to Hot on the Collaboration-Team-Triage board.
Etonkovidova added a subscriber: Etonkovidova.EditedJun 28 2017, 2:40 PM

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.

Probably my changes from T167616 caused this :(

Etonkovidova added a comment.EditedJun 28 2017, 7:33 PM

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


(2) Check for editing to see if the following has been fixed.
misaligned icons

(3) the link inspector cannot be dismissed

Catrope added a subscriber: Catrope.

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!

matmarex claimed this task.Jul 10 2017, 4:37 PM

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:

Inspector looks okay…
The context menu looks okay…
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

matmarex added a comment.EditedJul 10 2017, 9:50 PM

(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 set the point value for this task to 1.
Jdforrester-WMF closed this task as Resolved.
Restricted Application added a project: User-Ryasmeen. · View Herald TranscriptJul 10 2017, 11:11 PM