Page MenuHomePhabricator

[EPIC] Split Vector into 2 separate skins with 2 different keys
Open, HighPublic

Description

Specific: What do we want to achieve?

The skinversion system in Vector adds unnecessary friction in the following ways:

  • We have to add a skinversion field to any schema for any form of instrumentation to distinguish between the two experiences
  • There is a risk of fatals in production. We have to exercise caution in code, as constructing a Skin is supposed to be cheap, and cannot access session information e.g. user preferences in certain parts of the flow.
  • There is no way for extensions or user gadgets to ship different styles on the two Vectors. As a result, we are accumulating technical debt by shipping styles for two experiences together.
  • Certain behaviours of skins are tied. As a result we've had no choice but to make changes that impact legacy and modern Vector (for example wrapping menu items in span in legacy Vector)

There are some benefits of the existing system that we want to keep:

  • Ability for user gadgets/styles/scripts to load in both experiences.

Measurable: How will we know when we've reached our goal?

There should be two skins in the user preferences for Vector. These should behave like any other skin e.g. MonoBook / Timeless

Achievable: What support will we need to achieve our goal?

The performance team have raised various concerns with the approach so are blockers for completing this work.

Relevant: Is this goal worthwhile?

This goal should certainly be completed before the end of the project. The skin version code is very domain-specific, and will likely cause confusion if we were to switch projects and come back to it, say a year later.

Separating the skins also allows us to retire/hide the legacy skin in the long-term future (although I don't think we've ever do this in practice), as well as allowing other teams to maintain ownership. More importantly it allows us to work on modern Vector without having concern for impacting the legacy experience which it is tied to.

This should lead to less CSS being shipped to both modern and legacy Vector (currently skinStyles is shared therefore many unnecessary styles are loaded on both experiences) e.g. https://gerrit.wikimedia.org/r/c/mediawiki/skins/Vector/+/744706/2/skinStyles/mediawiki.notification.less#25

It should also reduce the potential of caching incidents. e.g. https://phabricator.wikimedia.org/T296210

It should also simplify instrumentation as all instrumentation can use the skin field rather than needing a custom skinversion field.

Time-bound: What is the time frame? Can we achieve this goal in the timeframe I've set?

???

More information

One of the goals of the desktop improvements project was to minimize disruption to end-users gadgets and site scripts and ensure that the new Vector is compatible with all existing gadgets. For this reason, we should split the skins into 2 different keys with shared wiki pages/gadgets

Phase 1 - Skin separation - Acceptance criteria

https://gerrit.wikimedia.org/r/c/mediawiki/skins/Vector/+/713001

  • Vector will be broken into two internal skins that are hidden in user preferences. These will have skin keys vector19 and vector21
  • Vector will be the default skin. The Vector constructor, will make use of SkinFactory::getSkinOptions to work out which skins to apply
  • The skin key for Vector will be "vector"
  • No change to end user experience.

Phase 2 - Prepare and QA the skins - Acceptance criteria

  • All skinStyles that declare vector, will now declare vector19 and vector22. Currently this looks like it will be a manual job (see T288857)
  • We will undergo QA on both skins: ?useskin=vector19 and ?useskin=vector22 and ensure they are compatible with common gadgets and scripts.
  • Both of the new skins should load MediaWiki:Vector.css and MediaWiki:Vector.js. This is possible by using ResourceLoaderSiteModulePages and ResourceLoaderSiteStylesModulePages hooks.
  • A user script/stylesheet e.g User:Jdlrobson/Vector.css should apply to both skins. A new hook is needed to support this use case.
  • Review the skin preference instrumentation with a QA engineer. Verify it gives us information we need and document any changes.
  • Review the skin preference version workflow with designer.

Phase 3 - Split the skins - Acceptance criteria

  • The skins vector19 and vector21 will be made internal inside configuration using wgSkipSkins
  • The internal flag will be dropped inside Vector skin.json
  • Once the train has rolled out we will update the configuration, changing the default skin to "vector19" or "vector21" on desktop improvements projects. vector will be added to wgSkipSkins and vector19 and vector21 removed.
  • Provided everything has gone to plan, the "vector" key inside Vector's skin registration will be removed and removed from wgSkipSkins

Event Timeline

Jdlrobson renamed this task from Split Vector into 2 separate skins with 2 different keys to [EPIC] Split Vector into 2 separate skins with 2 different keys.Sep 15 2021, 5:00 PM

This is causing lots of problems in our day to day work and should be prioritized.

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

[mediawiki/skins/Vector@master] Vector is split into 2 skins

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

@Jdlrobson just out of interest, why did you opt to have "skin versions" or "vector uses two internal skins", instead of just having a new vector21, make it default whenever you like, and drop "vector" whenever you like?

@Jdlrobson just out of interest, why did you opt to have "skin versions" or "vector uses two internal skins", instead of just having a new vector21, make it default whenever you like, and drop "vector" whenever you like?

A requirement of the project from the product department was to minimize disruption to users by ensuring that their gadgets and user scripts continued to work. A new skin would have required users to copy across all their gadgets.

A second benefit of this was that we are able to keep the two skins aligned as we refactored the code, which has made it more maintainable, however at this point when that work has been mostly done, there's little value in continuing to keep them tied to the same skin key. Hope this context is helpful.