Page MenuHomePhabricator

#p-personal hides OO.ui.Dialog in Vector skin
Closed, ResolvedPublic1 Story Points

Description

The z-index of #p-personal is set to 100 and thus it renders over the dialog. I understand that OOjs shouldn't be making mediawiki specific changes like this, but right now OO.uiDialog has a z-index of just 2. Is this something we might change?

VE handles it by creating an extra .ve-ui-overlay-global with a z-index of 101.

See also - https://www.mediawiki.org/wiki/Manual:Coding_conventions/CSS#z-index

Event Timeline

Prtksxna created this task.Jul 9 2015, 9:36 AM
Prtksxna updated the task description. (Show Details)
Prtksxna raised the priority of this task from to Normal.
Prtksxna added a project: OOUI.
Prtksxna added subscribers: Prtksxna, matmarex.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJul 9 2015, 9:36 AM
Prtksxna set Security to None.Jul 9 2015, 9:36 AM
Prtksxna added a subscriber: Jdforrester-WMF.

This is even worse on mobile where because of the personal bar you can't even exit from the dialog.

TheDJ added a subscriber: TheDJ.Jul 9 2015, 10:42 AM

I don't think it's even needed anymore to have that personal toolbar have z-index. Wasn't it for IE5 or something? Or was it for IE6 ? in that case we can just make it an IE specific CSS hack. Seems like it would be a safer long term solution.

I don't think it's even needed anymore to have that personal toolbar have z-index. Wasn't it for IE5 or something? Or was it for IE6 ? in that case we can just make it an IE specific CSS hack. Seems like it would be a safer long term solution.

Looks like a fix for T39158: Page-top tabs and personal menus may overlap in the Vector skin and T50078: VisualEditor: Vector drop down menu hidden behind toolbar.

Avoid 101 as that's still within the same range and will likely be overlapped by other elements in the same category in the future. Use something like 200 or 1000 instead.

At the moment 1-10 is used for within relative scopes of components, 100 to jump out within the same scope (e.g. on top of the page, but still "on" the page). It would make sense to reserve 1000+ for transient modals and overlays.

Although in the mid- to long term we should see to cut down on the z-indexes and go back to natural enforcement through relative scopes and DOM ordering. This is getting a bit out of hand. The only reason p-personal has that high z-index is to avoid user-generated content from overlapping interface controls (both for functional and security reasons).

Prtksxna claimed this task.Jul 20 2015, 2:07 PM

Change 225871 had a related patch set uploaded (by Prtksxna):
Increase z-index of .oo-ui-dialog to 200

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

Change 225871 merged by jenkins-bot:
Dialog: Increase z-index of .oo-ui-dialog to 1000

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

matmarex closed this task as Resolved.Jul 24 2015, 11:33 AM

The issue was fixed in OOUI and will be fixed in Vector after next OOUI release.

The fix for a vector hack should be in vector or MW core, not in the OOUI theme, which contains sensible z-indexes. It should also be more targeted than just all dialogs, as the above patch was causing bugs with the local vs global overlay in VE mobile breaking the link inspector, so I have submitted a revert.

Change 228844 had a related patch set uploaded (by Esanders):
Revert "Dialog: Increase z-index of .oo-ui-dialog to 1000 "

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

Specifically this caused T107619

The fix for a vector hack should be in vector or MW core, not in the OOUI theme, which contains sensible z-indexes.

Unless the z-index of dialogs is increased, we can't fix this in Vector or MW, as there are no z-indices inbetween that we could use for the personal menu. (Unless we refactor the skin more, or unless we override OOjs UI's styles, I guess.)

Jdforrester-WMF reopened this task as Open.Aug 4 2015, 3:12 PM

Low-priority bug doesn't get to break everything in its fix.

Change 228844 merged by jenkins-bot:
Revert "Dialog: Increase z-index of .oo-ui-dialog to 1000 "

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

Change 229146 had a related patch set uploaded (by Esanders):
Apply personal bar z-index hack to modal window managers in Vector

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

Change 229425 had a related patch set uploaded (by Jforrester):
Apply personal bar z-index hack to modal OOUI window managers

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

Legoktm renamed this task from #p-personal hides OO.ui.Dialog to #p-personal hides OO.ui.Dialog in Vector skin.Aug 5 2015, 5:34 PM
Legoktm added a project: Vector.
Jdforrester-WMF moved this task from Backlog to Doing on the OOUI board.Aug 7 2015, 6:22 PM

Change 229146 abandoned by Jforrester:
Apply personal bar z-index hack to modal window managers in Vector

Reason:
Consensus to do this in Vector, not core.

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

Change 229425 merged by jenkins-bot:
Apply personal bar z-index hack to modal OOUI window managers

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

Jdforrester-WMF moved this task from Doing to Reviewing on the OOUI board.Sep 18 2015, 3:23 PM
matmarex reopened this task as Open.Dec 18 2015, 9:32 PM

This didn't actually fix the issue at all…

Change 260084 had a related patch set uploaded (by Bartosz Dziewoński):
Actually apply personal bar z-index hack to modal OOUI window managers

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

Change 260084 merged by jenkins-bot:
Actually apply personal bar z-index hack to modal OOUI window managers

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

matmarex closed this task as Resolved.Dec 18 2015, 10:54 PM
matmarex removed a project: Patch-For-Review.