[go: up one dir, main page]

Page MenuHomePhabricator

Replace legacy breakpoint value tokens in all Wikimedia web codebases
Open, MediumPublic

Description

Background

As stated in T331403, from the 10 deprecated legacy tokens in WikimediaUI Base identified, changing the breakpoint variables has some disruptive potential. The change in user-interface presentation needs to be approached sensitively.

See current use of legacy breakpoint tokens (CodeSearch)

Goal

Why is it important?

  • We need to enable sunsetting WikimediaUI Base We went down a path, that removed this relation successfully
  • In order to not have devs looking at different sources of truth, needing to find their path towards Codex or even wrongly copying WikimediaUI Base vars over.
  • Having different breakpoints blocks further work like unified responsiveness on design systems patterns or infrastructure like the coming Grid. See T337282: Implement Grid in Codex for the Codex part of it.

What's changing?

  1. Tablet breakpoint is decreased by 80px, pushing lesser devices into a mobile phone only experience.
  2. Desktop and desktop-wide kicks in later, by 120px and 480px each.

Specifically latter change will need a closer look when testing the patches.

@width-breakpoint-tablet: 720px; /* Codex: @min-width-breakpoint-tablet is 640px */
@width-breakpoint-desktop: 1000px; /* Codex: @min-width-breakpoint-desktop is 1120px */

Update Codex 1.4.0. These breakpoints haven't been used anywhere in our environment again and were removed in 1.4.0.

// Removed in v1.4.0
@width-breakpoint-desktop-wide: 1200px; /* Codex: @min-width-breakpoint-desktop-wide is 1680px */
@width-breakpoint-desktop-extrawide: 2000px; /* Neither A nor B: unique value, no replacement suggested */

Note, that extrawide is not used in our environment. It was defined in Flow, but never got any other use case.

Acceptance criteria

Replace all mentioned tokens from

How to fix

  • Replace @width-breakpoint-* CodeSearch
  • Replace equivalent numeric pixel values in JS window.matchMedia() occurrences
  • Remove them from Codex
  • Remove them from mediawiki.skin.defaults.less

Non-blocking

Details

SubjectRepoBranchLines +/-
mediawiki/coremaster+1 -1
mediawiki/services/chromium-rendermaster+2 -1
mediawiki/coremaster+2 -3
mediawiki/coremaster+596 -247
mediawiki/coremaster+2 -2
design/codexmain+0 -24
mediawiki/coremaster+2 -2
mediawiki/skins/Vectormaster+2 -13
mediawiki/skins/MinervaNeuemaster+88 -48
mediawiki/skins/Vectormaster+0 -11
mediawiki/coremaster+26 -22
mediawiki/extensions/FlaggedRevsmaster+1 -1
mediawiki/extensions/RelatedArticlesmaster+1 -1
mediawiki/extensions/WikimediaMessagesmaster+1 -1
mediawiki/coremaster+9 -9
mediawiki/coremaster+0 -1
mediawiki/coremaster+190 -114
design/codexmain+0 -8
Show related patches Customize query in gerrit

Related Objects

Event Timeline

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

Change #1036773 merged by jenkins-bot:

[mediawiki/extensions/WikimediaMessages@master] styles: Replace deprecated breakpoint tokens with Codex defined ones

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

The scope is these three, core, MN and Vector, as they are blocking the removal of the legacy breakpoint and also impact the possible addition of Grid.
I'm not sure what you refer to with 'For core, none…'? For the extensions I will care about the replacement as I already did for all of the scoped ones.

I just need Web team to review and make sure this is QA'd and you're aware in case any edge case might get flagged afterwards.

To be more explicit:

  • https://gerrit.wikimedia.org/r/c/mediawiki/core/+/945931 has various styles that the web team doesn't maintain or is familiar with. The only change there that seems in scope for our team is the comment addition to Skin.php For example Growth team maintains RCFilters according to https://www.mediawiki.org/wiki/Developers/Maintainers and search platform maintains the search CSS. We are not setup to QA any of these these pages.
  • Vector doesn't seem to use legacy tokens (unless the URL in description is incorrect? If you need changes there - happy to review them I just don't understand what changes are needed!)

The scope is these three, core, MN and Vector, as they are blocking the removal of the legacy breakpoint and also impact the possible addition of Grid.
I'm not sure what you refer to with 'For core, none…'? For the extensions I will care about the replacement as I already did for all of the scoped ones.

I just need Web team to review and make sure this is QA'd and you're aware in case any edge case might get flagged afterwards.

To be more explicit:

Fair. My understanding of our environment: if something is off after the change for volunteers, Web team would be the first to get it flagged.
I'm more than willing to take on any conversations about the change and even come up with single fine-tailored changes if everything else is not working. But a +1 by Web team is needed on the pieces that are Web related and awareness about all changes will help possible communication. One example is the change in page.ready.js in the core patch, the code was introduced by you.

  • Vector doesn't seem to use legacy tokens (unless the URL in description is incorrect? If you need changes there - happy to review them I just don't understand what changes are needed!)

Right, we've replaced them earlier on with the local variables, that are now part of this task. The local variables have to go though in order to make future amendments of breakpoints visible to DST instead of introducing more local variables mixed into the standard ones.

And none of any of those changes would be mergeable with the sanctus (minimally +1) by Web team.

Change #1037150 had a related patch set uploaded (by VolkerE; author: VolkerE):

[mediawiki/extensions/FlaggedRevs@master] styles: Replace deprecated breakpoint tokens with Codex defined ones

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

Jdlrobson renamed this task from Replace legacy breakpoint value tokens in (MediaWiki core and) Vector and MinervaNeue to Replace legacy breakpoint value tokens in all codebases.May 29 2024, 7:35 PM

From conversations with both, @Jdlrobson and DST peer engineers, my understanding on how to move this over the finish line:

  1. We (DST) are responsible for reviewing and merging the core and the FlaggedRevs patch, former is the predecessor for Vector and MinervaNeue ones, latter is dependent on the Vector one
  2. Web team is responsible for reviewing & merging Vector and MF/MinervaNeue and RelatedArticles ones. Latter is dependent on the MinervaNeue one
  3. All four should be merged on the same day, preferably a few hours after a Tuesday's cut

Does that sound good for you @Jdlrobson?

Change #1037181 had a related patch set uploaded (by VolkerE; author: VolkerE):

[mediawiki/extensions/RelatedArticles@master] styles: Replace deprecated breakpoint tokens with Codex defined ones

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

Change #1037184 had a related patch set uploaded (by VolkerE; author: VolkerE):

[mediawiki/extensions/MobileFrontend@master] styles: Replace deprecated breakpoint tokens with Codex defined ones

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

Just for clarification, both mentioned skins, MediaWiki-skins-Mirage and Femiwikiskin have their own @width- variables defined, they are not in scope here.
It's better practice to orient on @min- and @max- like Codex latest does, but it's not necessary.

Volker_E renamed this task from Replace legacy breakpoint value tokens in all codebases to Replace legacy breakpoint value tokens in all Wikimedia web codebases.May 30 2024, 1:00 AM
Volker_E updated the task description. (Show Details)
Volker_E updated the task description. (Show Details)

On a related note, could these breakpoint values be offered as CSS variables, just like we already have CSS variables for colors (e.g. --background-color-base)?

Use case: on frwiki there are some fine-tuned layout tweaks, for example using white-space: nowrap on large screens but not on small screens, and for consistency the same breakpoints as Codex are used, but currently the values have to be hardcoded, as there are no variables offered.

I was thinking the same thing, as I have always used the 720px breakpoint when making templates on dewiki responsive. Would be good to keep it variable based on skins.

I would suggest waiting for container queries to be well supported so that we get basically-skin-agnostic break points, for the content use case. (See also T331227.) WMF will still need their current set but we basically won't once we have those. (We can hardcode the like 2 gadgets that actually play with the full layout e.g. the experimental Timeless and Vector responsive gadgets en.wp has.)

Change #972327 had a related patch set uploaded (by VolkerE; author: VolkerE):

[mediawiki/skins/MinervaNeue@master] styles: Replace deprecated breakpoints

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

Change #945931 merged by jenkins-bot:

[mediawiki/core@master] styles: Replace deprecated breakpoints

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

Change #972331 merged by jenkins-bot:

[mediawiki/skins/Vector@master] styles: Replace deprecated breakpoints

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

Change #1037150 merged by jenkins-bot:

[mediawiki/extensions/FlaggedRevs@master] styles: Replace deprecated breakpoint tokens with Codex defined ones

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

Change #1037181 merged by jenkins-bot:

[mediawiki/extensions/RelatedArticles@master] styles: Replace deprecated breakpoint tokens with Codex defined ones

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

Change #972327 merged by jenkins-bot:

[mediawiki/skins/MinervaNeue@master] styles: Replace deprecated breakpoints

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

Thanks for your help on this @Volker_E and for flagging that it needed review from the web team - although at first glance these looked trivial you were right to flag this as more risky than most token changes!

It looks like this work uncovered some problems with the Minerva code, that we wouldn't have known about without doing this. I'm happy we managed to decouple those things and bring skins more in line with the design system team - that's going to reduce a lot of bugs on the long run. There are six visual changes with the above patch but all are improvements so it is an indication we are heading in the right direction. Thanks for the fruitful collaboration.

We will look at the last patch for Vector on Monday 💪

Volker_E updated the task description. (Show Details)

Change #1040301 had a related patch set uploaded (by VolkerE; author: VolkerE):

[design/codex@main] tokens: Remove remaining deprecated `width-*` breakpoint tokens

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

Related, noticed that Echo defines custom breakpoint ‘@specialpage-hd-width: 982px;’

After T337954 we might now be able to replace that as well

Related, noticed that Echo defines custom breakpoint ‘@specialpage-hd-width: 982px;’

After T337954 we might now be able to replace that as well

Nice catch! I think it's better to tackle those in a different task. Researching also uncovered a few more similar, but slightly different breakpoints that would benefit from unification: https://codesearch-beta.wmcloud.org/search/?q=%5C%40media+all+and+%5C%28+min-width&files=&excludeFiles=&repos=#Extension:FlaggedRevs

Change #1040085 had a related patch set uploaded (by Jdlrobson; author: VolkerE):

[mediawiki/skins/Vector@master] Reapply "styles: Replace deprecated breakpoints"

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

Just for completion, an issue in the original implementation of Codex carrying MediaWiki skin variables was uncovered by the work here: T367103: Media queries using max width breakpoints are not working in Monobook and Fallback skin

Change #1040085 merged by jenkins-bot:

[mediawiki/skins/Vector@master] Reapply "styles: Replace deprecated breakpoints"

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

Change #1043218 had a related patch set uploaded (by VolkerE; author: VolkerE):

[mediawiki/core@master] mediawiki.diff: Replace deprecated breakpoint token

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

Change #1043218 merged by jenkins-bot:

[mediawiki/core@master] mediawiki.diff: Replace deprecated breakpoint token

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

Change #1043269 had a related patch set uploaded (by VolkerE; author: VolkerE):

[mediawiki/core@master] rcfilters: Replace deprecated breakpoint token

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

Change #1040301 merged by jenkins-bot:

[design/codex@main] tokens: Remove remaining deprecated `width-*` breakpoint tokens

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

Change #1043269 merged by jenkins-bot:

[mediawiki/core@master] rcfilters: Replace deprecated breakpoint token

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

How can one change the viewport back to 1000px?

Change #1049640 had a related patch set uploaded (by Eric Gardner; author: Eric Gardner):

[mediawiki/core@master] Update Codex from 1.7.0 to 1.8.0

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

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

Change #1049640 merged by jenkins-bot:

[mediawiki/core@master] Update Codex from 1.7.0 to 1.8.0

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

Change #1049955 had a related patch set uploaded (by VolkerE; author: VolkerE):

[mediawiki/core@master] mediawiki.less: Remove deprecated `@width-breakpoint-*` Less variables

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

Change #1049955 merged by jenkins-bot:

[mediawiki/core@master] mediawiki.less: Remove deprecated `@width-breakpoint-*` Less variables

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

Volker_E updated the task description. (Show Details)

Change #1075916 had a related patch set uploaded (by VolkerE; author: VolkerE):

[mediawiki/services/chromium-render@master] docs: Replace outdated skin Less variable with current one

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

Change #1075916 merged by jenkins-bot:

[mediawiki/services/chromium-render@master] docs: Replace outdated skin Less variable with current one

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

Hi! :) Just to clarify, is this task not subject to breakpoint changes? Or does the menu appear according to new breakpoints?
T330532: [Design] Re-evaluate breakpoint for hiding page tools/toc to keep content width more consistent

Change #1077052 had a related patch set uploaded (by VolkerE; author: VolkerE):

[mediawiki/core@master] docs: Amend to valid skin variable name for width breakpoint

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

Hi! :) Just to clarify, is this task not subject to breakpoint changes? Or does the menu appear according to new breakpoints?
T330532: [Design] Re-evaluate breakpoint for hiding page tools/toc to keep content width more consistent

Hi, no it's probably not. The task linked could point at either a wrong breakpoint choice or a gap in our current breakpoints or none of all of them. Will provide the current tokens breakpoints there.

Change #1077052 merged by jenkins-bot:

[mediawiki/core@master] docs: Amend to valid skin variable name for width breakpoint

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

Hi! :) Just to clarify, is this task not subject to breakpoint changes? Or does the menu appear according to new breakpoints?
T330532: [Design] Re-evaluate breakpoint for hiding page tools/toc to keep content width more consistent

Hi, no it's probably not. The task linked could point at either a wrong breakpoint choice or a gap in our current breakpoints or none of all of them. Will provide the current tokens breakpoints there.

Thanks for the answer :)

There are a few legacy breakpoints remaining, but not many: Code search.

Notably, addressing these in mediawiki-services-mobileapps would nicely complement fixing https://phabricator.wikimedia.org/T172078#8533594.