Page MenuHomePhabricator

[EPIC] Refactor icon sizing in MF/Minerva to achieve consistency
Closed, ResolvedPublic8 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
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:

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.

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.


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

Related Gerrit Patches:
mediawiki/extensions/MobileFrontend : masterBlockMessageDetails: Fix styling
mediawiki/extensions/MobileFrontend : masterSearch within pages icon should always be aligned with input icon
mediawiki/skins/MinervaNeue : masterFix icon alignment & padding of last-modified bar
mediawiki/skins/MinervaNeue : masterAlign right-most header icon with content.
mediawiki/skins/MinervaNeue : masterUtilize the mw-ui-icon-flush-left/right classes to align icons
mediawiki/extensions/Thanks : masterCreate mobile thanks using the mobile library
mediawiki/skins/MinervaNeue : masterFix Main Menu text alignment problems
mediawiki/skins/MinervaNeue : masterDrop width declaration on close button
mediawiki/extensions/MobileFrontend : masterSearch content margin should match search input
mediawiki/extensions/MobileFrontend : masterFlush reference drawer and Special:MobileDiff icons to relevant places
mediawiki/extensions/MobileFrontend : masterRemove unneccessary icon CSS after icon refactor
mediawiki/extensions/MobileFrontend : masterUpdate storybook to use new mw-ui-icon definition
mediawiki/skins/MinervaNeue : masterNew mw-ui-icon spec for Minerva
mediawiki/extensions/MobileFrontend : masterPrepare for the icon changes
mediawiki/skins/MinervaNeue : masterPrepare for new mw-ui-icon spec for Minerva
mediawiki/extensions/MobileFrontend : masterRemove all the icon hacks
mediawiki/skins/MinervaNeue : masterRemove the mw-ui-icon hacks and overrides
mediawiki/skins/MinervaNeue : masterAlso set `bottom` attribute on icon glyphs
mediawiki/extensions/MobileFrontend : masterSeparate text element from mw-ui-icon-before
mediawiki/skins/MinervaNeue : master[icon] revise icons

Related Objects

StatusAssignedTask
Openovasileva
Resolvedalexhollender
Resolvedalexhollender
Resolvedovasileva
Resolvedovasileva
Resolvedovasileva
Resolvedovasileva
Resolvedovasileva
Resolvedovasileva
DuplicateNone
Resolvedovasileva
Resolvedovasileva
Resolvedovasileva
Resolvedovasileva
OpenNone
Resolvedovasileva
Resolvedovasileva
Resolvedovasileva
Resolvedovasileva
Resolvedovasileva
Duplicatealexhollender
Resolvedovasileva
Resolvedovasileva
Resolvedalexhollender
OpenNone

Event Timeline

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

Change 536312 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Prepare for the icon changes

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

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

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

Jdlrobson updated the task description. (Show Details)Sep 13 2019, 12:00 AM

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.

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
Expand/collapse icon
too far over to the leftnot vertically centered with section heading
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
Main menu
alignment issue, text slightly too high for some menu items
Search within page element
alignment issue, should be further to the right
Log-out icon
Bytes changed icon
alignment issue, should be further to the left
Citation drawer
alignment issues, outer elements should be closer to edge of pagegeneral horizontal centering issues

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

close modal icon alignment issuepublish button clipped
add discussion button on talk overlay
last edited bar issues...
thank button on diff page

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

close modal icon alignment issuepublish button clipped

Close text appearing tracked in T232798

last edited bar issues...

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 -

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

@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

tracked in T232792

Bytes changed icon
alignment issue, should be further to the left

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

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

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:

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?

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 added a subscriber: Edtadros.

@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

❓ AC2: Expand/collapse icon

❓ AC3: Overlays (talk, notifications, search)

✅ AC4: Main menu

✅ AC5: Search within page element

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

❓ AC7: Bytes changed icon

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

❓AC9: Close modal icon alignment issue

❓ 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.

✅ AC11: Add discussion button on talk overlay

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

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

Edtadros updated the task description. (Show Details)Sep 17 2019, 2:33 PM

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

ovasileva closed this task as Resolved.Mon, Oct 21, 10:23 AM

WHOO!! Resolving