@alexhollender, should we update the notification drawer shadow?

In T231205#5437984, @Niedzielski wrote:

@alexhollender, should we update the notification drawer shadow?

I'm not opposed to that, though if we proceed with the changes proposed in T226125 I don't think it will matter.

Imported into GCI as https://codein.withgoogle.com/tasks/6173552525443072/

Change 560192 had a related patch set uploaded (by N3rsti; owner: N3rsti):
[mediawiki/extensions/MobileFrontend@master] mobile.variables.less: added drawerShadow variable Drawer.less added shadow to navigation-drawer class

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

Change 560192 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] mobile.variables.less: added drawerShadow variable Drawer.less added shadow to navigation-drawer class

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

Seeing a regression from this change. I will revert.

@Jdlrobson My GCI task was already approved, but I see that there is a problem with it. Can I fix it?

In T231205#5761498, @N3rsti wrote:

@Jdlrobson My GCI task was already approved, but I see that there is a problem with it. Can I fix it?

Sure - we'll be happy to see that fixed!

I tried to reconstruct this bug on devtools and I see this shadow on the left side (which I will try to fix), but I don't see the shadow on the burger menu. On which website have you seen this bug?

In T231205#5761513, @N3rsti wrote:

I tried to reconstruct this bug on devtools and I see this shadow on the left side (which I will try to fix), but I don't see the shadow on the burger menu. On which website have you seen this bug?

Hi, this change hasn't been deployed to Wikimedia wikis yet. So you can not reproduce this regression on mediawiki.org. It seems the screenshot is of the main page of English Wikipedia beta cluster.

In T231205#5761520, @Masumrezarock100 wrote:

In T231205#5761513, @N3rsti wrote:

I tried to reconstruct this bug on devtools and I see this shadow on the left side (which I will try to fix), but I don't see the shadow on the burger menu. On which website have you seen this bug?

Hi, this change hasn't been deployed to Wikimedia wikis yet. So you can not reproduce this regression on mediawiki.org. It seems the screenshot is of the main page of English Wikipedia beta cluster.

It is probably also worth mentioning that you will no longer be able to replicate this bug on the Beta cluster, as the change was reverted. You will have to debug this on your local test instance.

In T231205#5761525, @IAmNetx wrote:

It is probably also worth mentioning that you will no longer be able to replicate this bug on the Beta cluster, as the change was reverted. You will have to debug this on your local test instance.

Right. I didn't realize it was already reverted.

Change 560442 had a related patch set uploaded (by N3rsti; owner: N3rsti):
[mediawiki/extensions/MobileFrontend@master] mobile.variables.less: added drawerShadow variable Drawer.less fixed shadow in navigation-drawer class

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

It should be working now, it it's not, please let me know and I will try to fix it.

On this page, where the bug was, I didn't add any shadow, because there was already one (it's a little bit different, but it's not a big difference in my opinion), but if you want to have the exact same shadow like on other pages, I can fix it.

In T231205#5761570, @N3rsti wrote:

It should be working now, it it's not, please let me know and I will try to fix it.

On this page, where the bug was, I didn't add any shadow, because there was already one (it's a little bit different, but it's not a big difference in my opinion), but if you want to have the exact same shadow like on other pages, I can fix it.

Of course! I'll be more responsive this time!

(it's a little bit different, but it's not a big difference in my opinion), but if you want to have the exact same shadow like on other page

The goal here is consistency so we want to make sure every box shadow is the same. So I see the following todos..

• The color for all the above should be defined in a control variable in minerva.less/minerva.variables.less
• .drawer currently has box-shadow: 0 -1px 8px 0 rgba( 0, 0, 0, 0.35 ); (remains unchanged) and the color uses the new variable.
• #mw-mf-page-left currently has no box-shadow. Needs box-shadow: 1px 0px 8px 0px rgba(0,0,0,0.35);
• .notifications-overlay.navigation-drawer currently has box-shadow -5px 0 0 0 rgba(0,0,0,0.3). Alpha color should be changed to 0.35

Change 560519 had a related patch set uploaded (by N3rsti; owner: N3rsti):
[mediawiki/skins/MinervaNeue@master] minerva.variables.less: added gray color variable MainMenu.less: added shadow to navigation NotificationOverlay.less: changed alpha color of shadow drawers.less: replaced rgba color with the same color variable

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

It should be fixed now. I'm not sure if I created variable in the right place, but it's easy to fix in necessary.

Change 560519 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Fix shadow on notification overlay and navigation-drawer

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

Leaving this open so we can run this by our designer when he's back in January.

On beta it looks like the correct shadow is being applied.

screenshot of beta	shadow css

There is a 2px border-radius being applied via .toggle-list__list that should not be there (or should be overridden).

Also I'm curious where else we use .toggle-list__list? It has a box-shadow of its own that is quite nice, and a little less intense than our current default.

@alexhollender I haven't changed border-radius, because from the task I had to change color only. I did everything from this list:

In T231205#5761710, @Jdlrobson wrote:

The goal here is consistency so we want to make sure every box shadow is the same. So I see the following todos..

• The color for all the above should be defined in a control variable in minerva.less/minerva.variables.less
• .drawer currently has box-shadow: 0 -1px 8px 0 rgba( 0, 0, 0, 0.35 ); (remains unchanged) and the color uses the new variable.
• #mw-mf-page-left currently has no box-shadow. Needs box-shadow: 1px 0px 8px 0px rgba(0,0,0,0.35);
• .notifications-overlay.navigation-drawer currently has box-shadow -5px 0 0 0 rgba(0,0,0,0.3). Alpha color should be changed to 0.35

I think .toggle-list__list class is used in navigation bar on the wikipedia beta main page only (I haven't seen it anywhere else) and it has shadow from #mw-mf-page-left id so it overrides previous shadow. I can fix it with #mw-mf-page-left.navigation-drawer so it will be used only on pages like this . I'm not sure, but I think this file change was reverted so you can't see the shadow now (.primary-navigation-enabled is body class when navigation is opened so it only displayed nav shadow if opened). So I think things to do are:

• Remove border-radius
• Decide which shadow should be used in .toggle-list__list and if we should use previous one, I can fix it like I said before.

I've already completed this task because it was only to change colors and add variable, but I will be happy to help you with this problem.
Let me know if I can fix and which shadow should be used.

@N3rsti ok for now I think all that needs to be done is removing the border-radius.

@Jdrewniak do you know what the deal is with .toggle-list__list and why it has a box-shadow?

@Jdlrobson Why was this change reverted? I know you said that "this CSS block is dead", but now it doesn't show shadow in websites like this . There were no other shadow styles for this drawer.

Change 562336 had a related patch set uploaded (by N3rsti; owner: N3rsti):
[mediawiki/skins/MinervaNeue@master] Remove border-radius

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

I removed border-radius from .toggle-list__list. I haven't found any other border-radius styles that could override this so removing it from ToggleList.less should be fine, but I'm not sure why previous box-shadow change was reverted, like I said before.

In T231205#5779128, @N3rsti wrote:

@Jdlrobson Why was this change reverted? I know you said that "this CSS block is dead", but now it doesn't show shadow in websites like this . There were no other shadow styles for this drawer.

In T231205#5779128, @N3rsti wrote:

@Jdlrobson Why was this change reverted? I know you said that "this CSS block is dead", but now it doesn't show shadow in websites like this . There were no other shadow styles for this drawer.

Good question. I'm guessing there was a potential problem with the coordination of the two patches (the one you link and the one referenced in the commit message ( If4831fc700c7df3a2a389b5f95b6fbaea4b7d954 ) . https://en.m.wikipedia.beta.wmflabs.org/wiki/Main_Page runs the latest code and I see the box-shadow there but not the primary-navigation-enabled class. It's most likely this is a regression which will resolve itself in the next deploy.

There is a 2px border-radius being applied via .toggle-list__list that should not be there (or should be overridden).

The border-radius is being used on the overflow menu and now the main menu and this menu share code. I assume we want to keep the border radius here?

In T231205#5779359, @Jdlrobson wrote:

The border-radius is being used on the overflow menu and now the main menu and this menu share code. I assume we want to keep the border radius here?

Yes, just remove it from the main menu

In T231205#5779359, @Jdlrobson wrote:

https://en.m.wikipedia.beta.wmflabs.org/wiki/Main_Page runs the latest code and I see the box-shadow there but not the primary-navigation-enabled class. It's most likely this is a regression which will resolve itself in the next deploy.

Shouldn't the shadow be used on drawers on, for example this site? I know it's mediawiki website, not wikipedia beta, but GCI Task name and description was about MobileFrontend extension so I thought it should be used in mediawiki websites using MobileFrontend extension as well, but on mediawiki website i think it should be done in mobilefrontend code, not minerva skin (but i'm not sure)

In T231205#5779485, @N3rsti wrote:

In T231205#5779359, @Jdlrobson wrote:

https://en.m.wikipedia.beta.wmflabs.org/wiki/Main_Page runs the latest code and I see the box-shadow there but not the primary-navigation-enabled class. It's most likely this is a regression which will resolve itself in the next deploy.

Shouldn't the shadow be used on drawers on, for example this site? I know it's mediawiki website, not wikipedia beta, but GCI Task name and description was about MobileFrontend extension so I thought it should be used in mediawiki websites using MobileFrontend extension as well.

it should yes and this is likely because only 1 of the 2 patches got deployed, but I don't think it warrants further investigation as it will be fixed in the next round of deployments (code on wikipedia beta will be on mediawiki.org later tomorrow).

Ok, but I don't think it will work without this change which was reverted. Should I add it again (if it won't be a problem now)?

Change 562336 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Remove border-radius

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

In T231205#5779542, @N3rsti wrote:

Ok, but I don't think it will work without this change which was reverted.

are you seeing a problem with https://en.m.wikipedia.beta.wmflabs.org/ ? The box shadow looks fine to me there?

Should I add it again (if it won't be a problem now)?

No additional work is needed provided https://en.m.wikipedia.beta.wmflabs.org/ is working as expected. We deploy the code running on https://en.m.wikipedia.beta.wmflabs.org/ every week to all our wikis including mediawiki.org.

@alexhollender @N3rsti has fixed the border-radius issue so you should be able to test this on https://en.m.wikipedia.beta.wmflabs.org/wiki/Main_Page

It's working on wikipedia beta because it use #some_input_id:checked ~ #mw-mf-page-left (i don't remember id), but from what I've seen mediawiki is using anchor in this id so it can't be checked. Now I'm on mobile phone so I can't check dev tools, but I'm pretty sure there was anchor, not input on mediawiki

Edit: You're right. Sorry, I didn't know about HTML changes

Change 560442 abandoned by Jdlrobson:
mobile.variables.less: added drawerShadow variable Drawer.less fixed shadow in navigation-drawer class

Reason:
Taken care of in https://gerrit.wikimedia.org/r/#/c/mediawiki/skins/MinervaNeue/ /560519/

Thank you N3rsti!

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