"Learn more" z-index is placed above the transparent-shield leading to a white flash during slide in animation
Closed, ResolvedPublic3 Estimated Story Points
Actions

Assigned To

Authored By

	• alexhollender_WMF
	Jan 24 2019, 4:16 AM

Description

Note the related T206354

We have a z-index problem.

Description

When opening the main navigation menu, the dark overlay screen causes the fade next to the Learn more link to become visible as a white element on the screen. I'm seeing this on:

Android / Chrome, Firefox
iOS / Safari, Chrome, Firefox
MacOS / Safari, Chrome, Firefox

This only occurs during the animation and happens quite quickly (blink and you miss it!)

Documentation

page issue white flash.mov2 MBDownload

video at 0:04s

Development notes

This is a z-index issue.
.transparent-shield, .navigation-drawer has a z-index of 0 whereas .ambox-learn-more has a z-index of 1 (intentionally so that the gradient appears above any text). The learn more link however should appears below the shield.

Luckily z-index's are managed in a single file: mobile.less/mobile.variables.less with a few naughty CSS rules not using variables :

resources/mobile.notifications.overlay/NotificationsOverlay.less
MinervaNeue/resources/skins.minerva.content.styles/templates/ambox.less
MinervaNeue/skinStyles/ext.echo.styles.special/SpecialNotificationsOverlay.less

@z-indexBase: 0;
@z-indexOverlay: 1;
@z-indexOverOverlay: 2;

To fix this we will need to rethink z-indexes and ensure learn more is the same or lower than the transparent shield.
Increasing the z-index of transparent-shield will require increasing the z-index of other things e.g. @z-indexOverOverlay and z-indexBase so be careful!

QA instructions

Environment: staging
Browser & device: mobile (iOS/Safari, Android/Chrome), desktop (MacOS/Safari, Windows/Chrome)
Skin: mobile/default, desktop/Minerva
Steps:

visit a page on staging with a page issue banner, e.g. https://reading-web-staging.wmflabs.org/wiki/Devolution (if you don't see a page issue banner with a blue "Learn more" link you'll need to find a different page on staging that has an issue)
tap the hamburger button to open the side nav
verify that the white rectangle behind the "Learn more" link does not become visible as the dark overlay appears on top of the content page content (reference the images above for an example of what to look out for)

Acceptance criteria / sign off steps

AC1: Check all z-index values are defined by variable by running this code search as assuring there are no hard-coded values like in z-index: 1 in the results.
AC2: The bug is fixed

QA Results

AC	Status	Details
AC1	✅ Passed	T214550#4970746
AC2	✅ Passed	T214550#4963044

Details

	Subject	Repo	Branch	Lines /-
	Remove hard-coded z-index value from page-issues read-more link	mediawiki/skins/MinervaNeue	master	1 -1
	Change z-index of page-issues read-more link	mediawiki/skins/MinervaNeue	master	2 -2

Customize query in gerrit

Related Objects
Search...

		Status	Subtype	Assigned	Task
		Resolved		Jdlrobson	T214526 [EPIC] Mobile page issues banner: CSS/styling cleanup
		Resolved		ovasileva	T214550 "Learn more" z-index is placed above the transparent-shield leading to a white flash during slide in animation

Event Timeline

• alexhollender_WMF triaged this task as Medium priority.Jan 24 2019, 4:16 AM

• alexhollender_WMF created this task.

Jdrewniak added a project: Page-Issue-Warnings.Jan 24 2019, 9:37 AM

Aklapper renamed this task from Learn more white flash when opening navigation menu to "Learn more" white flash when opening navigation menu.Jan 24 2019, 11:23 AM

Jdlrobson moved this task from Incoming to Needs Prioritization on the Web-Team-Backlog board.Jan 24 2019, 6:34 PM

I cannot reproduce this on production. Are you sure the problem is still present?

I cannot reproduce this on production. Are you sure the problem is still present?

I captured that video yesterday, and am still seeing it. What device/browser are you using?

Okay, I see it now, but only on Android - not desktop Chrome. Blink and you miss it!

Jdlrobson renamed this task from "Learn more" white flash when opening navigation menu to "Learn more" z-index is placed above the transparent-shield leading to a white flash during slide in animation.Jan 24 2019, 10:35 PM

Jdlrobson updated the task description. (Show Details)

Jdlrobson moved this task from QA Tasks / Needs Reproduction to Triaged but Future on the Web-Team-Backlog board.

Jdlrobson moved this task from Triaged but Future to Upcoming on the Web-Team-Backlog board.Feb 5 2019, 3:42 PM

Jdlrobson set the point value for this task to 3.Feb 5 2019, 5:48 PM

Jdlrobson moved this task from Upcoming to Readers-Web-Kanbanana-Board-2018-19-Q3 on the Web-Team-Backlog board.Feb 5 2019, 10:06 PM

Jdlrobson edited projects, added Web-Team-Backlog (Readers-Web-Kanbanana-Board-2018-19-Q3); removed Web-Team-Backlog.

Jdrewniak moved this task from To Do to Doing on the Web-Team-Backlog (Readers-Web-Kanbanana-Board-2018-19-Q3) board.Feb 7 2019, 8:02 PM

Jdlrobson assigned this task to Jdrewniak.Feb 7 2019, 8:31 PM

as far as I can tell, setting this line to z-index:0; (or @z-indexBase) seems to fix the issue. Here's a video with the fix.
https://drive.google.com/file/d/1uhFH9Dc4SXFPI3cTj50ZEf9XGx3-_noB/view?usp=sharing

Change 489112 had a related patch set uploaded (by Jdrewniak; owner: Jdrewniak):
[mediawiki/skins/MinervaNeue@master] Change z-index of page-issues read-more link

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

gerritbot added a project: Patch-For-Review.Feb 8 2019, 12:41 AM

It'd be good to put this on staging to double check, but I think the simple fix works.

FYI, re: z-index - it's a common misconception that z-index is a global value, but it's actually relative to a stacking context, which can be created by parent elements that are positioned absolute, or even a changes in opacity.

It seems as though both the read-more and transparent-shield are in the same stacking context, but for example, if I change the opacity to 0.9 on the .ambox, a new stacking context is created below the transparent-shield, and that also fixes the problem, see video 😳

Jdlrobson moved this task from Doing to Needs Design Review on the Web-Team-Backlog (Readers-Web-Kanbanana-Board-2018-19-Q3) board.Feb 8 2019, 2:14 AM

This is now on staging.

• alexhollender_WMF updated the task description. (Show Details)Feb 8 2019, 10:20 PM

@Jdrewniak the issue is fixed on mobile \o/

I noticed when testing on desktop that the "Learn more" link is still on top of the transparent-shield cloaked-element (in the video below you'll also notice that the color changes to a brighter blue). This isn't high priority to fix from a UX perspective, although I wanted to mention it because a) maybe it's something we want to fix from a technical perspective, b) if we don't fix it now I imagine we'll forget about it f o r e v e r

learn more shield index2.mov2 MBDownload

If you don't think it's worth fixing feel free to move this along to QA. I've added QA instructions.

Jdlrobson moved this task from Needs Design Review to Needs Code Review on the Web-Team-Backlog (Readers-Web-Kanbanana-Board-2018-19-Q3) board.Feb 8 2019, 10:53 PM

Change 489112 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Change z-index of page-issues read-more link

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

Jdlrobson moved this task from Needs Code Review to Ready for Signoff on the Web-Team-Backlog (Readers-Web-Kanbanana-Board-2018-19-Q3) board.Feb 8 2019, 11:58 PM

Jdlrobson moved this task from Ready for Signoff to Needs QA on the Web-Team-Backlog (Readers-Web-Kanbanana-Board-2018-19-Q3) board.

ReleaseTaggerBot added a project: MW-1.33-notes (1.33.0-wmf.17; 2019-02-12).Feb 9 2019, 12:01 AM

@alexhollender I'm not sure what changed but I'm unable to see the bug with the blue read-more text being clickable, either on staging or locally on master, so I'm handing this over to QA :)

ovasileva reassigned this task from • alexhollender_WMF to Edtadros.Feb 11 2019, 6:06 PM

@Jdrewniak & @alexhollender I'm not sure if this is a feature or a bug, but the "Learn more" link appears right justified when I check it with a phone aspect ratio but left justified when I check it with a tablet or desktop. I am happy to document the test for tablets or desktop, but when I click the hamburger with a phone emulation, the link goes off screen to the right so I cannot verify it. Let me know how you'd like me to proceed.

Tablet and Desktop:

Screen Shot 2019-02-11 at 9.25.50 PM.png (210×585 px, 24 KB)

Screen Shot 2019-02-11 at 9.26.03 PM.png (199×580 px, 19 KB)

Phone:

Screen Shot 2019-02-11 at 9.26.20 PM.png (287×379 px, 25 KB)

Screen Shot 2019-02-11 at 9.26.32 PM.png (289×379 px, 21 KB)

@Edtadros yup that's a feature.

As you mention, it's hard to take a screenshot of this bug because the "read more" goes off the screen. If you're able to do a screen-recording though, I think that would adequately let us compare the results to the video posted in the description. (If you're using an iPhone, it's got the screen-recording functionality built-in now!)

@Jdrewniak weird and cool — confirming that the issue I mentioned in T214550#4939796 seems to have resolved itself.

#JediMindTricks

Test Result

Acceptance Criteria #2: The bug is fixed.

Status: ✅ PASS
OS: iOS 12.1.4
Browser: Safari

Test Artifact(s):

Tablet:

iPhone:

IMG_F86B9D356431-1.jpeg (2×1 px, 783 KB)

IMG_1576.TRIM.MOV966 KBDownload

Edtadros updated the task description. (Show Details)Feb 19 2019, 12:57 AM

@Jdrewniak I am not sure how to verify the z-order variables. I'm not sure I have access. You can walk me through it, or just provide me a test artifact of some sort....screen shot, code snippet, etc...

Change 491459 had a related patch set uploaded (by Jdrewniak; owner: Jdrewniak):
[mediawiki/skins/MinervaNeue@master] Remove hard-coded z-index value from page-issues read-more link

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

Hey @Edtadros You can do a code-search through all our repos using this tool: https://codesearch.wmflabs.org/search/?q=z-index:&i=nope&files=&repos=Skin:MinervaNeue

There was one value that wasn't using the variables, but it was a unique use-case so I left it as is, however it just occurred to me that I could still symbolize that one value (the computed value remains the same, so there's no functional change) so I just pushed a small patch that does that.

Jdrewniak updated the task description. (Show Details)Feb 19 2019, 12:40 PM

Change 491459 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Remove hard-coded z-index value from page-issues read-more link

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

" AC1: Check all z-index values are defined by variable by runn" should pass now. I feel this is outside the scope of @Edtadros and it might be better for a developer to do this as part of sign off.

I'm reassigning to Edward so he can make that decision but also run any further tests that might be necessary!

ReleaseTaggerBot edited projects, added MW-1.33-notes (1.33.0-wmf.19; 2019-02-26); removed MW-1.33-notes (1.33.0-wmf.17; 2019-02-12).Feb 20 2019, 1:00 AM

Status: ✅ PASS

Acceptance Criteria #1

Test Artifact(s):

Screen Shot 2019-02-20 at 10.37.46 PM.png (1×988 px, 258 KB)

Thanks for the link @Jdrewniak! Sometimes all I need is the right tool for the job.

Edtadros reassigned this task from Edtadros to ovasileva.Feb 21 2019, 6:41 AM

Edtadros updated the task description. (Show Details)

Edtadros moved this task from Needs QA to Ready for Signoff on the Web-Team-Backlog (Readers-Web-Kanbanana-Board-2018-19-Q3) board.

Looks like we're done here. Thanks all!

ovasileva updated the task description. (Show Details)Feb 21 2019, 9:04 AM

Jdlrobson merged a task: T205148: [Bug] page issue banner fade element is visible when notifs drawer is open.Mar 6 2019, 6:49 PM

	F28256334: Screen Shot 2019-02-20 at 10.37.46 PM.png
	Feb 21 2019, 6:41 AM

	F28243119: Screen Shot 2019-02-11 at 9.26.03 PM.png
	Feb 19 2019, 12:48 AM

	F28198703: Screen Shot 2019-02-11 at 9.26.20 PM.png
	Feb 12 2019, 5:27 AM

	F28198704: Screen Shot 2019-02-11 at 9.26.32 PM.png
	Feb 12 2019, 5:27 AM

	F28198700: Screen Shot 2019-02-11 at 9.26.03 PM.png
	Feb 12 2019, 5:27 AM

	F28198699: Screen Shot 2019-02-11 at 9.25.50 PM.png
	Feb 12 2019, 5:27 AM

"Learn more" z-index is placed above the transparent-shield leading to a white flash during slide in animationClosed, ResolvedPublic3 Estimated Story PointsActions

Description

Description

Documentation

Development notes

QA instructions

Acceptance criteria / sign off steps

QA Results

Details

Related ObjectsSearch...

Event Timeline

Test Result

"Learn more" z-index is placed above the transparent-shield leading to a white flash during slide in animation
Closed, ResolvedPublic3 Estimated Story Points
Actions

Related Objects
Search...