Page MenuHomePhabricator

[EPIC] Refactor icon sizing in MF/Minerva to achieve consistency
Closed, ResolvedPublic8 Estimated Story Points

Description

Description

Currently there is not a consistent approach to icon sizing in MF/Minerva. There are generally three icon sizes (small, medium, and large) however the CSS that sizes individual icons is rather inconsistent. There is also inconsistency around the touch areas for icons. This problem became evident as we introduced several new icons into the interface as part of the AMC work. Our plan is to refactor the icon sizing, standardized around the following sizes:

smallmediumlarger than medium
icon area16x16px20x20px(no standard)
touch area34x34px44x44px(no standard)
example
small.jpg (160×192 px, 9 KB)
medium.jpg (204×236 px, 11 KB)
see T231683

This problem is made worse by the fact that the SVGs themselves are not sized consistently. Some are on 24x24px canvases, others are on 20x20px canvases, with varying degrees of padding within the SVGs themselves (not CSS padding/margins). The work related to replacing icons with properly sized ones will be tracked in T231613.

This is an incomplete reference to help clarify which icons should be sized small vs. medium:

image.png (1×2 px, 1 MB)

Example of current issue

  • Icons in the site header and page-actions toolbar are larger than icons in the menus.
  • Various icons, such as "talk" and "special pages", are being rendered larger than intended.
image.png (552×587 px, 99 KB)
image.png (669×554 px, 266 KB)

Developer notes

There seem to be two main causes of this issue:

  • The svgs themselves are not sized consistently. Some are on 24x24px canvases, others are on 20x20px canvases, with varying degrees of padding within the SVGs themselves (not CSS padding/margins)
  • The background-size: 100% auto; rule causes the width of the icons to 100%, which makes them appear larger if the SVG has a width smaller than 20px. See gif below.
    image.png (168×290 px, 28 KB)

icon-sizes-minerva.gif (554×600 px, 551 KB)

image.png (233×310 px, 38 KB)

QA

Run an exploratory test on https://en.m.wikipedia.beta.wmflabs.org/wiki/Spain - if any content is missing or looks different to production and the UI element involves an icon flag it.

Some examples we have found so far: T232800, T232792, T232798

Sign off steps

  • Make sure all subtasks are resolved.

QA Results

Details

SubjectRepoBranchLines +/-
mediawiki/extensions/MobileFrontendmaster+2 -3
mediawiki/extensions/MobileFrontendmaster+6 -8
mediawiki/skins/MinervaNeuemaster+1 -2
mediawiki/skins/MinervaNeuemaster+7 -6
mediawiki/skins/MinervaNeuemaster+34 -18
mediawiki/extensions/Thanksmaster+18 -4
mediawiki/skins/MinervaNeuemaster+1 -0
mediawiki/skins/MinervaNeuemaster+2 -2
mediawiki/extensions/MobileFrontendmaster+5 -0
mediawiki/extensions/MobileFrontendmaster+9 -8
mediawiki/extensions/MobileFrontendmaster+3 -28
mediawiki/extensions/MobileFrontendmaster+157 -134
mediawiki/skins/MinervaNeuemaster+136 -10
mediawiki/extensions/MobileFrontendmaster+6 -0
mediawiki/skins/MinervaNeuemaster+41 -59
mediawiki/extensions/MobileFrontendmaster+20 -41
mediawiki/skins/MinervaNeuemaster+94 -133
mediawiki/skins/MinervaNeuemaster+1 -0
mediawiki/extensions/MobileFrontendmaster+65 -50
mediawiki/skins/MinervaNeuemaster+26 -15
Show related patches Customize query in gerrit

Related Objects

StatusSubtypeAssignedTask
Resolvedovasileva
Resolved alexhollender_WMF
Resolved alexhollender_WMF
Resolvedovasileva
Resolvedovasileva
Resolvedovasileva
Resolvedovasileva
Resolvedovasileva
ResolvedBUG REPORTovasileva
DuplicateNone
ResolvedBUG REPORTovasileva
Resolvedovasileva
Resolvedovasileva
Resolvedovasileva
Resolvedovasileva
Resolvedovasileva
Resolvedovasileva
Resolvedovasileva
Resolvedovasileva
Resolvedovasileva
Duplicate alexhollender_WMF
Resolvedovasileva
Resolvedovasileva
Resolved alexhollender_WMF
Resolvedovasileva

Event Timeline

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

Change 533360 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] New mw-ui-icon spec for Minerva

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

Change 535970 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Update storybook to use new mw-ui-icon definition

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

Section collapse toggles look wrong when the collapsible section is a <h1> or <h3> instead of <h2>, as normal.

image.png (980×1 px, 66 KB)
image.png (980×1 px, 64 KB)
image.png (980×1 px, 62 KB)

This seems to be caused by this block in Toggler.less: https://phabricator.wikimedia.org/diffusion/EMFR/browse/master/resources/mobile.startup/Toggler.less$5

However, when I tried removing it, the styling for collapsible sections in other mobile skins got all messed up.

I'll leave it for you to untangle…

Change 535835 merged by Jdrewniak:
[mediawiki/extensions/MobileFrontend@master] Remove unneccessary icon CSS after icon refactor

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

@Jdlrobson @Jdrewniak here are the issues I'm seeing on beta:

Main site header
hamburger and search are too close to the edges
image.png (1×750 px, 218 KB)
Expand/collapse icon
too far over to the leftnot vertically centered with section heading
image.png (1×750 px, 82 KB)
image.png (1×798 px, 82 KB)
Overlays (talk, notifications, search)

The "X" has too much padding, which is causing two issues:

it should be closer to the edge of the screenit's pushing the element next to it too far over
image.png (1×750 px, 49 KB)
Screen Shot 2019-09-13 at 9.48.28 AM.png (79×186 px, 5 KB)
Main menu
alignment issue, text slightly too high for some menu items
image.png (1×750 px, 70 KB)
Search within page element
alignment issue, should be further to the right
image.png (1×750 px, 204 KB)
Log-out icon
en.m.wikipedia.beta.wmflabs.org_wiki_TemplateUsageArticle977(iPhone 6_7_8).png (1×750 px, 71 KB)
Bytes changed icon
alignment issue, should be further to the left
image.png (1×750 px, 184 KB)
Citation drawer
alignment issues, outer elements should be closer to edge of pagegeneral horizontal centering issues
image.png (1×750 px, 162 KB)
image.png (1×750 px, 159 KB)

Other issues, not sure if they are related to icon changes

close modal icon alignment issuepublish button clipped
en.m.wikipedia.beta.wmflabs.org_wiki_Anthony,_John,_and_Eustathios(iPhone 6_7_8).png (1×750 px, 698 KB)
en.m.wikipedia.beta.wmflabs.org_wiki_Anthony,_John,_and_Eustathios(iPhone 6_7_8) (1).png (1×750 px, 467 KB)
add discussion button on talk overlay
en.m.wikipedia.beta.wmflabs.org_wiki_Jam!(iPhone 6_7_8) (1).png (1×750 px, 45 KB)
last edited bar issues...
en.m.wikipedia.beta.wmflabs.org_wiki_Spain(iPhone 6_7_8).png (1×750 px, 86 KB)
en.m.wikipedia.beta.wmflabs.org_wiki_Breaking_Bad(iPhone 6_7_8).png (1×750 px, 76 KB)
thank button on diff page
en.m.wikipedia.beta.wmflabs.org_wiki_Special_MobileDiff_397227(iPhone 6_7_8).png (1×750 px, 162 KB)

Other issues, not sure if they are related to icon changes

close modal icon alignment issuepublish button clipped
en.m.wikipedia.beta.wmflabs.org_wiki_Anthony,_John,_and_Eustathios(iPhone 6_7_8).png (1×750 px, 698 KB)
en.m.wikipedia.beta.wmflabs.org_wiki_Anthony,_John,_and_Eustathios(iPhone 6_7_8) (1).png (1×750 px, 467 KB)

Close text appearing tracked in T232798

last edited bar issues...
en.m.wikipedia.beta.wmflabs.org_wiki_Spain(iPhone 6_7_8).png (1×750 px, 86 KB)
en.m.wikipedia.beta.wmflabs.org_wiki_Breaking_Bad(iPhone 6_7_8).png (1×750 px, 76 KB)

tracked in T232800

Will probably have to open bugs for the rest

I found a weird issue on Firefox (on production) - The logout icon is misaligned -

image.png (532×969 px, 86 KB)

Change 536618 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/MobileFrontend@master] Flush reference drawer and Special:MobileDiff icons to relevant places

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

Change 536624 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/MobileFrontend@master] Search content margin should match

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

Change 536626 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/MinervaNeue@master] Drop width declaration on close button

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

Change 536624 merged by Jdlrobson:
[mediawiki/extensions/MobileFrontend@master] Search content margin should match search input

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

Change 536618 merged by Jdlrobson:
[mediawiki/extensions/MobileFrontend@master] Flush reference drawer and Special:MobileDiff icons to relevant places

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

In T229440#5490743, @alexhollender wrote:

@Jdlrobson @Jdrewniak here are the issues I'm seeing on beta:

Main site header
hamburger and search are too close to the edges

https://gerrit.wikimedia.org/r/#/c/mediawiki/skins/MinervaNeue/+/536174/

Expand/collapse icon
too far over to the leftnot vertically centered with section heading

https://gerrit.wikimedia.org/r/#/c/mediawiki/skins/MinervaNeue/+/536174/

Overlays (talk, notifications, search)

The "X" has too much padding, which is causing two issues:

it should be closer to the edge of the screenit's pushing the element next to it too far over

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

Main menu
alignment issue, text slightly too high for some menu items

STILL NEEDS A FIX

Search within page element
alignment issue, should be further to the right

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

Log-out icon
en.m.wikipedia.beta.wmflabs.org_wiki_TemplateUsageArticle977(iPhone 6_7_8).png (1×750 px, 71 KB)

tracked in T232792

Bytes changed icon
alignment issue, should be further to the left
image.png (1×750 px, 184 KB)

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

Citation drawer
alignment issues, outer elements should be closer to edge of pagegeneral horizontal centering issues

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

Other issues, not sure if they are related to icon changes

close modal icon alignment issuepublish button clipped

tracked in T232798

add discussion button on talk overlay
en.m.wikipedia.beta.wmflabs.org_wiki_Jam!(iPhone 6_7_8) (1).png (1×750 px, 45 KB)

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

last edited bar issues...

tracked in T232792

thank button on diff page

NEEDS FIX

Change 536661 had a related patch set uploaded (by Nray; owner: Nray):
[mediawiki/skins/MinervaNeue@master] Fix Main Menu text alignment problems

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

After Fix thanks extension works as expected

Change 536660 had a related patch set uploaded (by Pmiazga; owner: Jdlrobson):
[mediawiki/extensions/Thanks@master] Create mobile thanks using the mobile library

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

Change 536661 merged by Jdlrobson:
[mediawiki/skins/MinervaNeue@master] Fix Main Menu text alignment problems

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

Change 536626 merged by Jdlrobson:
[mediawiki/skins/MinervaNeue@master] Drop width declaration on close button

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

Change 536174 merged by Jdlrobson:
[mediawiki/skins/MinervaNeue@master] Utilize the mw-ui-icon-flush-left/right classes to align icons

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

Restricted Repository Identity mentioned this in rSKINc027d289cea0: Update git submodules.
Restricted Repository Identity mentioned this in rSKIN48bd971645df: Update git submodules.

Change 536660 merged by Jdlrobson:
[mediawiki/extensions/Thanks@master] Create mobile thanks using the mobile library

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

Change 536973 had a related patch set uploaded (by Jdrewniak; owner: Jdrewniak):
[mediawiki/skins/MinervaNeue@master] Align right-most header icon with content.

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

Change 536973 merged by Jdlrobson:
[mediawiki/skins/MinervaNeue@master] Align right-most header icon with content.

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

Change 537190 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/MobileFrontend@master] Search within pages icon should always be aligned with input icon

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

Change 537198 had a related patch set uploaded (by Jdrewniak; owner: Jdrewniak):
[mediawiki/skins/MinervaNeue@master] Fix icon alignment & padding of last-modified bar

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

Change 537198 merged by Jdlrobson:
[mediawiki/skins/MinervaNeue@master] Fix icon alignment & padding of last-modified bar

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

Block message drawer is all messed up:

image.png (461×1 px, 129 KB)

Block message drawer is all messed up:

image.png (461×1 px, 129 KB)

Hmm it doesnt seem to look like that in the storybook:

https://doc.wikimedia.org/MobileFrontend/master/js/ui/?path=/story/editor--sourceeditoroverlay-blocked-user

Will take a look tomorrow to see what's going on here unless you beat me to it. CSS leakage?

Change 537190 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Search within pages icon should always be aligned with input icon

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

Edtadros subscribed.

@alexhollender please take a look at the items with ❓ .

Test Result

Status: ⬜ In Progress
OS: macOS Mojave
Browser: Chrome
Device: MBP
Emulated Device: iPhoneX

Test Artifact(s):==== QA Steps

On https://en.m.wikipedia.beta.wmflabs.org/wiki/Spain verify the following appear per T229440#5490743

✅ AC1: Main site header

T229440-1.png (2×1 px, 385 KB)

❓ AC2: Expand/collapse icon

T229440-2.png (2×1 px, 305 KB)
T229440-2b.png (2×1 px, 305 KB)

❓ AC3: Overlays (talk, notifications, search)

T229440-3.png (2×1 px, 161 KB)

✅ AC4: Main menu

T229440-4.png (2×1 px, 178 KB)
T229440-4b.png (2×1 px, 135 KB)

✅ AC5: Search within page element

T229440-5.png (2×1 px, 387 KB)

✅ AC6: Log-out icon
See AC4 second image.

❓ AC7: Bytes changed icon

T229440-7b.png (2×1 px, 247 KB)
T229440-7.png (2×1 px, 204 KB)

❌ AC8: Citation drawer
The Citation icon is aligned with the text, but the X is aligned with the right margin.

T229440-8.png (2×1 px, 394 KB)

❓AC9: Close modal icon alignment issue

T229440-9.png (2×1 px, 1 MB)

❓ AC10: Publish button clipped
should "Publish" be right justified with the right margin? The "Next" button on the previous screen currently matches "Publish" with regards to the right margin.

T229440-9b.png (2×1 px, 1 MB)

✅ AC11: Add discussion button on talk overlay

T229440-11.png (2×1 px, 95 KB)

❌ AC12: Last edited bar issues
The text and the history icon appear to be too low.

T229440-12b.png (2×1 px, 456 KB)
T229440-12.png (2×1 px, 223 KB)

✅ AC13: Thank button on diff page
See AC7 second image.

Block message drawer is all messed up:

Hmm it doesnt seem to look like that in the storybook:

https://doc.wikimedia.org/MobileFrontend/master/js/ui/?path=/story/editor--sourceeditoroverlay-blocked-user

Will take a look tomorrow to see what's going on here unless you beat me to it. CSS leakage?

The CSS in the storybook is loaded in a different order. On the real page, this block:

.block-message-icon {
    float: left;
    width: 20%;
    margin-top: 0.3em;
}

Overrides styles from this block:

.mw-ui-icon {
    font-size: initial;
    position: relative;
    display: inline-block;
    box-sizing: content-box !important;
    width: 1.25em;
    …
}

The two 'width' rules have a different effect because of that.

Change 537497 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/MobileFrontend@master] BlockMessageDetails: Fix styling

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

Change 537497 abandoned by Bartosz Dziewoński:
BlockMessageDetails: Fix styling

Reason:
Let's go with https://gerrit.wikimedia.org/r/537500 instead, that also fixes the issue.

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

@alexhollender please take a look at the items with ❓ .

AC2 — fixed, but due to caching won't show up for another week or so https://phabricator.wikimedia.org/T232801
AC3 — fixed, looks good
AC7 — fixed, task for other issue is here https://phabricator.wikimedia.org/T233151
AC9T233171
AC10 — no issue
AC12T233172

Since everything is in separate tasks now I think it makes sense to do any further QA on those tasks themselves.

Note the remaining open bugs are not about sizing or consistency (while I could be convinced the former is, the issue with last modified is specific to that icon).

https://phabricator.wikimedia.org/T233046
https://phabricator.wikimedia.org/T233172