Page MenuHomePhabricator

[Spike] Determine remaining blockers (if any) for CSS component adoption
Closed, ResolvedPublic3 Estimated Story Points

Description

We'd like to adopt codex styles to replace mw-ui-button ,mw-ui-icon and typeahead search by changing our markup to match CSS components (see https://gerrit.wikimedia.org/r/c/mediawiki/skins/Vector/+/912974 and T322077)

The goal of the spike is to identify remaining blockers (if any)

TODO

  • Change the markup the CSS component markup
  • Run Pixel
  • Document the changes and identify recommended way to fix them.
  • Document the expected change in bytes for render blocking CSS

Event Timeline

Jdlrobson updated the task description. (Show Details)

I believe the CSS only button classes need to also work for <label> and <a> elements for us to fully replace core button styles with Codex.

https://phabricator.wikimedia.org/T333394#8809052
https://phabricator.wikimedia.org/T284273

I just realized that we make use of mw-ui-icon-small for the watchstar, I dont believe this has been covered by another task, @AnneT is this something on DSTs radar?

@bwang we now support 3 icon sizes in both the Vue and CSS-only icons, with associated size tokens that are also available. Will that be sufficient for your needs?

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

[mediawiki/core@master] Skin: Add icon data to the component data

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

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

[mediawiki/skins/Vector@master] [POC][WIP] Use Codex for many interface styles

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

Change 914420 merged by jenkins-bot:

[mediawiki/core@master] Skin: Add icon data to the component data

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

@AnneT Ah yeah thats perfect, I didnt see that before thank you

LGoto set the point value for this task to 3.May 4 2023, 5:33 PM

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

[mediawiki/extensions/UniversalLanguageSelector@master] Let Vector 2022 manage its own styles

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

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

[mediawiki/skins/Vector@master] Vector 2022 should manage styles for ext.uls.pt

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

Change 918606 merged by jenkins-bot:

[mediawiki/skins/Vector@master] Vector 2022 should manage styles for ext.uls.pt

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

Change 918605 merged by jenkins-bot:

[mediawiki/extensions/UniversalLanguageSelector@master] Let Vector 2022 manage its own styles

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

I generated a Pixel report and I'm not seeing anything major here that would count as a blocker:

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

I've updated T336526 with details of what I consider blockers.

@bwang can you confirm that nothing seems troublesome in the Pixel long screenshot to you?

@Jdlrobson I see quite a few unexpected changes in that pixel report and locally, most of them seem to be from this (https://phabricator.wikimedia.org/T335710#8817435). I also noticed that the flush mixin provided by Codex doesnt account for the fact that icon only buttons change size on smaller resolutions, which is resulting in pixel regressions in the heading and page title. These are primarily issues with Codex, so those things supposed to be fixed before T336526?

@bwang we hadn't implemented any responsiveness in the flush layout mixin until just now since we were blocked on how to implement T333392: Button: add "large" size. I just pushed a series of patches that will:

  • Add a new size prop to the Button component, which can be either 'medium' (the default) or 'large' (the min-size for interactive elements on mobile, 44px). For now, the team would like to just enable people to turn on the large size on all screen sizes, rather than specifically on mobile. We hope this is acceptable for now since icon buttons in Minerva are currently larger on all viewport widths. We may implement some kind of responsive size in the future
  • Update the flush layout mixin to include a param for button size. When it's an icon-only large button, the negative margin will increase to match the increased padding

Please let me know if you have any feedback on these patches, or would like to get together to talk about them sometime!

@Jdlrobson When we are replacing our typeahead search with the Codex CSS only component, can we also remove our implementation of the loader? This loader has been historically very difficult to reproduce and maintain, as it only shows when we are loading in Vue. Codex does provide a 'pending state' prop that shows a loader when search results are being fetched, but that isnt the same as our loader

Change 922928 had a related patch set uploaded (by Bernard Wang; author: Bernard Wang):

[mediawiki/skins/Vector@master] Replace Vector search markup and styles with Codex CSS Typeahead Search

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

@Jdlrobson When we are replacing our typeahead search with the Codex CSS only component, can we also remove our implementation of the loader? This loader has been historically very difficult to reproduce and maintain, as it only shows when we are loading in Vue. Codex does provide a 'pending state' prop that shows a loader when search results are being fetched, but that isnt the same as our loader

Yes definitely! That is T321106 and T322077 which can either be done next sprint or as subtasks of T336526 (note search is one of the TODOs there). Since this ticket is closed, let's continue the topic on those tasks (I've reassigned the patch).

Change 924556 had a related patch set uploaded (by Bernard Wang; author: Bernard Wang):

[mediawiki/extensions/UniversalLanguageSelector@master] Remove padding-left from .uls-trigger

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

Change 925853 had a related patch set uploaded (by Bernard Wang; author: Bernard Wang):

[mediawiki/extensions/Echo@master] Remove extraneous badgeButton.setLabel call when the badge is updated

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

Change 925853 abandoned by Bernard Wang:

[mediawiki/extensions/Echo@master] Remove extraneous badgeButton.setLabel call when the badge is updated

Reason:

no longer needed

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

Change 928114 had a related patch set uploaded (by Bernard Wang; author: Bernard Wang):

[mediawiki/skins/Vector@master] Remove custom TOC padding when in page title

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

Change 928114 abandoned by Bernard Wang:

[mediawiki/skins/Vector@master] Remove custom TOC padding when in page title

Reason:

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