Page MenuHomePhabricator

User links: Standardize `.mw-ui-icon` to overhauled icon canvas size 20x20
Closed, ResolvedPublic5 Estimated Story Points

Assigned To
Authored By
Trizek-WMF
Mar 29 2018, 1:29 PM
Referenced Files
F34560296: Screen Shot 2021-07-22 at 2.52.32 PM.png
Jul 22 2021, 6:55 PM
F34560288: Screen Shot 2021-07-22 at 2.50.21 PM.png
Jul 22 2021, 6:55 PM
F34560290: Screen Shot 2021-07-22 at 2.50.30 PM.png
Jul 22 2021, 6:55 PM
F34560211: Screen Shot 2021-07-22 at 1.16.59 PM.png
Jul 22 2021, 6:55 PM
F34560301: language button shift.gif
Jul 22 2021, 6:55 PM
F34560214: Screen Shot 2021-07-22 at 1.23.34 PM.png
Jul 22 2021, 6:55 PM
F34560216: Screen Shot 2021-07-22 at 1.17.11 PM.png
Jul 22 2021, 6:55 PM
F34559044: image.png
Jul 21 2021, 8:20 PM

Description

Mediawiki ui icons are 24x24 however should be 20x20.
Minerva currently overrides the entire stylesheet to achieve this.

Changes implemented in T177432 resulted in .mw-ui-icon contained icons being blurry and labels being really close.

Capture d’écran_2018-03-29_15-25-45.png (288×178 px, 17 KB)

Acceptance criteria

  • The stylesheet inside Minerva is made the default in core so that it applies Vector.
  • This should feature flagged to allow for testing in the beta cluster (either with new module or ResourceLoader magic) and allow us to deploy when we are ready to. It should be disabled by default.
  • We should remove the Minerva override once the default is in place

Design review

This can skip QA provided the designer is happy and move to sign off. If there is any value in QA the designer should add some suggested QA instructions. If there is any issues requiring follow up please ping @Jdlrobson

NOTE: QA of icons impacted in page previews will be done as part of another task.

Details

ProjectBranchLines +/-Subject
mediawiki/coremaster+316 -6
mediawiki/skins/Vectormaster+20 -109
mediawiki/skins/Vectormaster+0 -3
mediawiki/extensions/UniversalLanguageSelectormaster+3 -3
mediawiki/skins/Vectormaster+4 -7
mediawiki/skins/Vectormaster+7 -8
mediawiki/skins/Vectormaster+2 -2
mediawiki/skins/Vectormaster+0 -3
mediawiki/extensions/MultimediaViewermaster+31 -4
mediawiki/skins/MinervaNeuemaster+39 -238
mediawiki/skins/Vectormaster+111 -29
mediawiki/coremaster+230 -98
mediawiki/extensions/Flowmaster+188 -156
mediawiki/skins/MinervaNeuemaster+0 -4
mediawiki/coremaster+12 -2
mediawiki/skins/Vectormaster+41 -43
mediawiki/skins/MinervaNeuemaster+0 -229
mediawiki/coremaster+224 -88
mediawiki/coremaster+7 -5
mediawiki/extensions/Flowmaster+52 -36
mediawiki/extensions/Flowmaster+7 -18
Show related patches Customize query in gerrit

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

Change 702456 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/skins/Vector@master] WIP: Use Minerva's mw-ui-icon implementation in Vector

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

Change 702475 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/core@master] Make user-menu list items produce HTML consistent with other menus

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

Change 702485 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/skins/Vector@master] Refactor the way we add classes to list items

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

@ovasileva FYI this should be considered a blocker now for the rollout. As I've begun implementing this I've found a few issues with the current implementation that are going to be much much easier to fix while this is feature flagged.

Change 702485 merged by jenkins-bot:

[mediawiki/skins/Vector@master] Refactor the way we add classes to list items

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

Change 702470 merged by jenkins-bot:

[mediawiki/core@master] Allow skins to wrap menu item link labels in spans

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

Change 704431 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/core@master] Version MediaWiki UI to allow us to roll out new specifications more carefully

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

LGoto added a subscriber: Jdrewniak.

Change 704984 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/skins/Vector@master] Remove Vector skinStyles for icons

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

Change 704987 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/core@master] Use MediaWiki UI icon specification 2

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

Change 704991 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/skins/MinervaNeue@master] Use MediaWiki UI version 2 from core

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

Change 704994 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/extensions/Flow@master] Prepare for mw-ui-icon changes, fixes padding on icons in mobile

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

Jdlrobson added a subscriber: RHo.

After chatting with the Growth team, @RHo said it was worthwhile fixing Flow so I've added a patch https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Flow/+/704994 - however it will need someone from the Growth team to review. The change should not impact desktop but will fix mobile and avoid a regression when the other changes here are completed.

After checking mobile Flow UI, I filed T286812 - there are several mobile Flow UI issues. I wasn't sure whether those issues are related to some changes done in this ticket patches, so I filed a separate ticket.

_QA note:_For testing after deployment.
The current UI (wmf.14) before the fix:

Screen Shot 2021-07-16 at 11.09.29 AM.png (788×2 px, 107 KB)
Screen Shot 2021-07-16 at 11.31.59 AM.png (934×708 px, 63 KB)
Screen Shot 2021-07-16 at 11.09.19 AM.png (922×2 px, 135 KB)

Change 705015 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/skins/MinervaNeue@master] Prepare for existing bundlesize change

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

After checking mobile Flow UI, I filed T286812 - there are several mobile Flow UI issues. I wasn't sure whether those issues are related to some changes done in this ticket patches, so I filed a separate ticket.

_QA note:_For testing after deployment.
The current UI (wmf.14) before the fix:

Screen Shot 2021-07-16 at 11.09.29 AM.png (788×2 px, 107 KB)
Screen Shot 2021-07-16 at 11.31.59 AM.png (934×708 px, 63 KB)
Screen Shot 2021-07-16 at 11.09.19 AM.png (922×2 px, 135 KB)

These are not related to the work here.

Change 705509 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/extensions/MultimediaViewer@master] Prepare for MediaWiki UI version 2

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

Change 705510 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/extensions/Popups@master] Remove the page preview icon hacks

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

Change 705015 merged by jenkins-bot:

[mediawiki/skins/MinervaNeue@master] Prepare for existing bundlesize change

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

Change 705509 merged by jenkins-bot:

[mediawiki/extensions/MultimediaViewer@master] Prepare for MediaWiki UI version 2

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

Change 702456 merged by jenkins-bot:

[mediawiki/skins/Vector@master] Prepare for updated mw-ui-icon implementation in Vector

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

Change 704994 merged by jenkins-bot:

[mediawiki/extensions/Flow@master] Prepare for mw-ui-icon changes, fixes padding on icons in mobile

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

Change 704987 merged by jenkins-bot:

[mediawiki/core@master] Use MediaWiki UI icon specification 2

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

Change 704991 merged by jenkins-bot:

[mediawiki/skins/MinervaNeue@master] Use MediaWiki UI version 2 from core in Minerva

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

There are separate issues for the regressions this caused, with the exception of the language button fix.
Once https://gerrit.wikimedia.org/r/c/mediawiki/skins/Vector/+/704984 is merged please move this into design review.

Checked in betalabs - both desktop and mobile, looks good.

Screen Shot 2021-07-20 at 4.40.32 PM.png (356×930 px, 70 KB)
Screen Shot 2021-07-20 at 4.42.11 PM.png (403×357 px, 27 KB)
Screen Shot 2021-07-20 at 4.40.40 PM.png (336×934 px, 75 KB)

Change 704984 merged by jenkins-bot:

[mediawiki/skins/Vector@master] Remove Vector skinStyles for icons and fix language button

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

Jdlrobson updated the task description. (Show Details)
Jdlrobson added a subscriber: bwang.

@Jdlrobson here's what I'm seeing:

Topic 1

When hovering the icon button on beta is 46x46px, but it should be 44x44px (not sure where the extra 2px are coming from):

current betacorrect
image.png (152×341 px, 20 KB)
image.png (142×338 px, 19 KB)
  • note: in case it's helpful, the icon button next to Create account is correct:
    Screen Shot 2021-07-21 at 4.15.07 PM.png (100×256 px, 8 KB)

Topic 2

It looks like some extra space is getting added between the icon and the label (again not sure where it's coming from):

current betacorrect
image.png (168×530 px, 18 KB)
image.png (142×510 px, 10 KB)

Topic 3

The user icon button does not have correct padding. It seems like this might be due to the down arrow next to the icon not being included in the box.

current betacorrect
Screen Shot 2021-07-21 at 4.14.12 PM.png (117×151 px, 6 KB)
Screen Shot 2021-07-21 at 4.13.40 PM.png (103×166 px, 6 KB)
link to reference

Change 706040 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/skins/Vector@master] Drop redundant border

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

Change 706041 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/skins/Vector@master] Removes additional space between language button label and icon

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

Change 706045 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/skins/Vector@master] Correct the user icon padding

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

@alexhollender I should have fixed the 3 changes above can you design review them and put in code review column when you are happy with them?

https://patchdemo.wmflabs.org/wikis/0f17b10236/wiki/Main_Page?useskinversion=2&vectoruserlinks=1
Remember to use Patch Demo / patchdemo1 to log in

Change 706040 merged by jenkins-bot:

[mediawiki/skins/Vector@master] Drop redundant border

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

Change 706041 merged by jenkins-bot:

[mediawiki/skins/Vector@master] Removes additional space between language button label and icon

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

@Jdlrobson confirming that the issues noted in T191021#7228591 have been fixed:

  1. Screen Shot 2021-07-22 at 1.16.59 PM.png (123×326 px, 16 KB)
  2. Screen Shot 2021-07-22 at 1.23.34 PM.png (635×751 px, 32 KB)
  3. Screen Shot 2021-07-22 at 1.17.11 PM.png (99×195 px, 6 KB)

Two new things:

1

The color of the hover backgrounds for the icon buttons are not consistent:

Screen Shot 2021-07-22 at 2.50.21 PM.png (82×74 px, 1 KB)
Screen Shot 2021-07-22 at 2.50.30 PM.png (87×112 px, 2 KB)

In terms of what is correct, I think the best thing would be to reference Minerva:

Screen Shot 2021-07-22 at 2.52.32 PM.png (205×501 px, 17 KB)

2

The language button shifts location slightly on page load. This is happening on beta. Not sure if it's related to any recent changes but it's the first time I noticed it so I wanted to point it out (click to open GIF):

language button shift.gif (465×384 px, 233 KB)

Change 706703 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/skins/Vector@master] Sidebar hover background should be consistent with mw-ui-icon

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

Change 706045 merged by jenkins-bot:

[mediawiki/skins/Vector@master] Correct the user icon padding

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

Change 706713 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/extensions/UniversalLanguageSelector@master] Restrict compact language button styles to legacy Vector

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

Change 706703 merged by jenkins-bot:

[mediawiki/skins/Vector@master] Sidebar hover background should be consistent with mw-ui-icon

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

Change 706827 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/skins/Vector@master] Restore the true height of the language button

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

Change 706713 merged by jenkins-bot:

[mediawiki/extensions/UniversalLanguageSelector@master] Restrict compact language button styles to legacy Vector

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

Change 706827 merged by jenkins-bot:

[mediawiki/skins/Vector@master] Restore the true height of the language button

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

Looks good, resolving!

Change 704431 abandoned by Jdlrobson:

[mediawiki/core@master] Version MediaWiki UI to allow us to roll out new specifications more carefully

Reason:

Am assuming this was all just a misunderstanding. Krinkle if there is anything left you want to clarify please feel free to do so.

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

Test wiki on Patch demo by Jdlrobson using patch(es) linked to this task was deleted:

https://patchdemo.wmflabs.org/wikis/0f17b10236/w/