Page MenuHomePhabricator

Reconsider usage of `mw-ui-icon-element` in Desktop Improvements buttons
Closed, ResolvedPublic



Through discussions in T283757, multiple button inconsistencies in modern Vector have been identified, specifically with the hover/focus/active states in the Sidebar, ULS, and User Links buttons.
Video of button state behaviors

These inconsistencies can mostly be attributed to two issues:

  1. Vector uses a mix of mw-ui-icon-element and mw-ui-button mw-ui-quiet classes across Desktop Refresh buttons. While both classes provide similar styles, mw-ui-icon-element doesn’t provide focus styles or distinguish between hover and active states. The mw-ui-icon-element class also seems unique to the WikimediaUI theme, as there aren’t any equivalent examples in either OOUI and WVUI. As far as I can tell, the more modern libraries recommend using an icon element inside of a quiet button, rather than applying styles directly to the icon.
  1. mw-ui-button mw-ui-quiet styles don’t match the styleguide to begin with. The focus styles override the active styles when they shouldn’t, and the focus outline is too thin. These 2 issues need to be fixed for the “Create account” and user page buttons.

Expected behavior

Buttons in Desktop Refresh have the same behavior and states, and they follow the “quiet button” spec in the styleguide and the WVUI docs.

Possible solutions:

1. Update buttons to align with style guide

Simplest way to achieve this is to override mw-ui-icon-element in modern Vector to apply the correct focus and active states
This option is likely the most involved for several reasons:

  • mw-ui-button mw-ui-quiet needs to be fixed in core (issue #2 above)
  • We can't easily add classes to the Echo icons, we need to add custom code to handle Echo.
  • The checkbox hack means we need to add custom code to handle the menu button (i.e. the user menu button)

Other options like trying to use both classes get really messy, as the classes have conflicting styles. The "correct" way to use these classes is probably something like this, but this would require some markup changes we currently don't support.

<h3 id=“p-personal-label” class=“vector-menu-heading mw-ui-button mw-ui-quiet”>
    <span class=“mw-ui-icon mw-ui-icon-wikimedia-userAvatar”>Personal tools</span>
2. Override mw-ui-button mw-ui-quiet to ensure all buttons consistently behave like mw-ui-icon-element

This option goes against the style guide and removes default button behavior, but it's a much simpler path to achieving consistency inside Vector. We would still have to fix mw-ui-button mw-ui-quiet in core. This would also make Vector's buttons consistent with MinervaNeue.

3. Do nothing and wait to redo the buttons with the new design system.

Considering mw-ui-icon-element and mw-ui-button mw-ui-quiet both don't match the style guide and that WikimediaUI will eventually be deprecated, we can choose to wait to redo these buttons at a later date when the design system and SSR is ready to use. This would mean we have to put up with some subtle button inconsistencies in Vector for the time being.

Event Timeline

bwang updated the task description. (Show Details)
cjming triaged this task as Medium priority.Aug 20 2021, 10:04 PM
cjming added a project: Web-Team-Backlog.
Jdlrobson subscribed.

We talked about this today in our engineers sync, and we would like to work out going into the sticky header what's the best way to mix mw-ui-button and mw-ui-icon. I'm going to make a spike for that and link it here (T289514)

mw-ui-button and mw-ui-icon were never meant to be mixed, and given we have this problem in Minerva we'd not rather tackle the greater problem here until later on in the project when we've learned more

@bwang given our conversations I'm wondering if this ticket is needed anymore. I think the short-term goal is make sure Minerva and Vector are using mw-ui-icon and mw-ui-button and those are aligned with WVUI and the long term goal is switching those out for WVUI.

@Jdlrobson The short and long term goals make sense to me. I actually think this ticket is handled by, as that patch basically removes "mw-ui-icon-element" from the user menu icon.

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

[mediawiki/skins/Vector@master] Separate icon classes from button classes in user links/language

Jdlrobson claimed this task.

Per T289320#7322653

Please reopen if I've misunderstood.