[go: up one dir, main page]

Page MenuHomePhabricator

[EPIC, Visual refinements] Standardize icon buttons & links (Vector 2022)
Closed, ResolvedPublic

Description

Description

Our icon buttons & links are not currently consistent with each other, and are not following the design style guide spec. We need to update the color of the icons, the total size (i.e. hover area), and the hover color.

We're working on updates as part of a new feature:
https://en.wikipedia.beta.wmflabs.org/wiki/User:JDrewniak?vectorvisualenhancementnext=1

To update:

image.png (2×3 px, 418 KB)

Design spec

image.png (776×992 px, 54 KB)

Prototype

https://di-collapsible-menus.web.app/Moss

Sequencing

Without feature flag

Behind feature flag

QA

{F35815929}

Sign off step

Related Objects

Event Timeline

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

We talked about this with the DST team and it sounds like we have no disagreements with what we're doing, but it's unclear how and when we make these changes, particularly while changes are in flux. It also doesn't really make sense for web team to maintain https://github.com/wikimedia/mediawiki/blob/master/resources/src/mediawiki.ui.button/button.less (buttons) and https://github.com/wikimedia/mediawiki/blob/master/resources/src/mediawiki.ui.icon/icons-2.less (icon) given we don't have the insight into the work the design systems team is doing and the goal of these stylesheets is to always be in sync with Codex!

Roan pointed out that using the token system in Vector might be worth focusing on first to reduce technical debt.

The suggestion on the table is to defer this change until later and not make it a blocker for deploy, but we want to check that with Alex. We'll then update design systems with what we decide.

Change 824302 abandoned by Jdlrobson:

[mediawiki/core@master] MediaWiki UI: Set correct icon font size, padding and hover color

Reason:

Pending further conversation, I don't think it's a good time to make these changes right now.

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

Test wiki created on Patch demo by Jdlrobson using patch(es) linked to this task:
https://patchdemo.wmflabs.org/wikis/dcb2317c72/w

The above patchdemo doesn't address the issues in this ticket yet, so please don't design review against the acceptance criteria in this ticket just yet.

Discussed with Alex and confirming that this is not considered a deployment blocker

Tagging design systems backlog as this is a potentially collaborative web/design systems task. From the web side, it is not a blocker for further desktop improvements deployment, so moving back to the backlog
@DAbad

In T314323#8142432, @bmartinezcalvo wrote:@Volker_E @DAbad should we create a task to define the badge component in our system?

There's already T280708: [EPIC] Badge: Add Badge component to Codex filed.

One added usability complexity: While it might make sense to have the article links and tools have the margin on the encompassing list element, for something like the watchstar we should expand the clickable area

image.png (208×778 px, 22 KB)

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

[mediawiki/skins/Vector@master] Storybook: Update Legacy/codex comparison table

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

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

[mediawiki/core@master] MediaWiki UI: Bring styles in line with latest Codex

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

Test wiki created on Patch demo by Jdlrobson using patch(es) linked to this task:
https://patchdemo.wmflabs.org/wikis/94e79f6a53/w

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

[mediawiki/skins/Vector@master] Visual enhancements next: Fix watchstar alignment

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

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

https://patchdemo.wmflabs.org/wikis/94e79f6a53/w/

Change 831993 merged by jenkins-bot:

[mediawiki/skins/Vector@master] Storybook: Update Legacy/codex comparison table

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

Hey @alexhollender_WMF I talked to Volker about this today, and we've come up with a plan for updating the icons/buttons that side steps breaking touch areas on mobile.

The icons will adapt based on breakpoint (so on mobile and tablet they will increase in touch area size). We'll work with design systems team on this one, and then switch to web team to finish the rest of the work here.

To give you a sense of what this will look like here's a patch demo with proposed changes for phase 1:
https://patchdemo.wmflabs.org/wikis/26f259e6be/wiki/Main_Page?useskin=vector-2022

Note, for now this only addresses Update mw-ui-icon spec (blocked on talking to design systems team) and Change small icons padding from 9px to 6px TODO items. Other items can be done without DST assistance.

Please signal if you think this is an improvement in the right direction (and are happy for it to be an intermediate state in resolving the issue once and for all.

Let me know if this looks good and move to either needs more work (and assign to me) or blocked (if happy with changes) and assigned to Volker so he knows it's okay to merge.
https://patchdemo.wmflabs.org/wikis/26f259e6be/wiki/Main_Page?useskin=vector-2022

To give you a sense of what this will look like here's a patch demo with proposed changes for phase 1:
https://patchdemo.wmflabs.org/wikis/26f259e6be/wiki/Main_Page?useskin=vector-2022
Please signal if you think this is an improvement in the right direction (and are happy for it to be an intermediate state in resolving the issue once and for all.

@Jdlrobson this is looking great. some notes/questions (many of these things might be already known/expected):

  • need to add the 8px spacing between the icon buttons
  • need to standardize the icon colors to #202122 (see the hamburger icon for example)
  • need to standardize the icon hover background color to #F8F9FA
  • the collapsed TOC icon that appears next to the page title (not the one in the sticky header) doesn't seem to have been updated
  • the collapsed TOC icon that appears floating in the corner doesn't seem to have been updated
  • what is the plan for the alerts (bell) and notices (inbox) icons? I don't think we would want to deploy this change without updating those two as well

based on the above not sure where to move the task to

@Jdlrobson this is looking great. some notes/questions (many of these things might be already known/expected):

Yes those would be handled by web team (see the two sets of TODO in description)

what is the plan for the alerts (bell) and notices (inbox) icons? I don't think we would want to deploy this change without updating those two as well

The plan is T257143 but the Growth team has no bandwidth to work on that. I think that's likely a blocker for this work continuing. I'll talk to Volker and see if he's comfortable with helping me make that change. Is that the only blocker your side from using this intermediate state or is it essentially all these changes go out together (for example if there's a week space between changes would that be a problem)?

@Jdlrobson and I just chatted about this. The plan is to break this into two parts:

Part 1:

  • Hover background colors
  • Icon glyph colors

Part 2 (visualenhancementsnext=1):

  • Update touch areas/padding
    • Including the Watch and Heart icons
  • Updating spacing/margins between the icons
  • Fix Echo icons to be consistent
Jdlrobson renamed this task from [Visual refinements] Standardize icon buttons & links (Vector 2022) to [EPIC, Visual refinements] Standardize icon buttons & links (Vector 2022).Sep 14 2022, 6:43 PM
Jdlrobson added a project: Epic.
Jdlrobson updated the task description. (Show Details)

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

[mediawiki/core@master] MediaWiki UI: Bring styles in line with latest Codex

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

ovasileva lowered the priority of this task from Medium to Low.Sep 15 2022, 5:02 PM

Test wiki created on Patch demo by Jdlrobson using patch(es) linked to this task:
https://patchdemo.wmflabs.org/wikis/df7bc7be01/w

I've listed the epics in the task description under "sequencing".

Once the task is unblocked, the remaining changes the web team needs to do should be small and I'll open separate tickets.

@alexhollender_WMF @Volker_E here's the latest and greatest version that we're working towards:

https://patchdemo.wmflabs.org/wikis/df7bc7be01/w/index.php?title=Spain&vectorvisualenhancementnext=1&useskin=vector-2022

Let me know if anything jumps out here as being overlooked.

@Jdlrobson the patchdemo is looking great 👍. Not sure what exactly is in scope here but the two things I'm seeing are:

  • the table of contents icon button has a touch area that's too large
  • the 8px spacing between the icon buttons is still missing

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

[mediawiki/skins/Vector@master] Technical: Generalize icon flushing

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

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

[mediawiki/skins/Vector@master] Prepare for icon touch area change

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

Change 836947 merged by jenkins-bot:

[mediawiki/skins/Vector@master] Technical: Generalize icon flushing

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

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

[mediawiki/skins/Vector@master] Prepare for icon touch area change

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

Change 836967 merged by jenkins-bot:

[mediawiki/skins/Vector@master] Prepare for icon touch area change

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

@alexhollender_WMF so all the work here should be done, and we should be ready to deploy this when needed. A preview of what we would deploy is currently at:

https://en.wikipedia.beta.wmflabs.org/wiki/Albert%20Einstein?vectorvisualenhancementnext=1

Alex the settings cog looks like this:

Screen Shot 2022-10-05 at 12.24.49 PM.png (221×402 px, 40 KB)

Any changes we want to make to that as part of deploying this change? If so I'll create follow up tickets so we don't lose track of them. If not, I'll work with Olga to schedule when to deploy this one.

@Jdlrobson ok cool, so you were able to address T319070#8284272?

the settings cog looks close enough, I think we can leave that as is.

Captura de Pantalla 2022-08-11 a las 12.16.48.png (468×1 px, 147 KB)

hey @bmartinezcalvo, can you clarify something for us: if there are several regular buttons in a row would they have the same 8px of spacing between them? For example:

image.png (372×786 px, 18 KB)

I assume it would have this spacing, but I also see this ButtonGroup component in Codex with no spacing, so I wasn't sure.

cc @bwang

Captura de Pantalla 2022-08-11 a las 12.16.48.png (468×1 px, 147 KB)

hey @bmartinezcalvo, can you clarify something for us: if there are several regular buttons in a row would they have the same 8px of spacing between them? For example:

image.png (372×786 px, 18 KB)

I assume it would have this spacing, but I also see this ButtonGroup component in Codex with no spacing, so I wasn't sure.

cc @bwang

@alexhollender_WMF we have 2 ways of combine buttons:

  1. With a ButtonGroup where we combine buttons with equally important actions. When we want to combine buttons whose actions are related to each other we use ButtonGroup component. Keep in mind that ButtonGroup is at the moment just implemented to combine Normal buttons and as specified in the spec sheet we should avoid combining buttons with different types of labels (text, numbers, icons) in the same ButtonGroup.
    Captura de Pantalla 2022-11-23 a las 10.39.09.png (414×1 px, 35 KB)
  2. We can simply place different type of buttons (text, icon, Primary, Normal, Quiet...) and separate them using an horizontal padding. In this case, we can use any of our spacing tokens according to each case (4px, 8px, 12px, 16px...) so 8px separation here is ok. I think your use case is more aligned with this second option.

@alexhollender_WMF I suggest we file any new bugs under T317632 and close this one out.

We should make sure T323176, T322673, T321504 and are resolved before resolving this particular ticket.

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

https://patchdemo.wmflabs.org/wikis/120dbeccb7/w/

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

https://patchdemo.wmflabs.org/wikis/dcb2317c72/w/

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

https://patchdemo.wmflabs.org/wikis/df7bc7be01/w/