Page MenuHomePhabricator

[SPIKE 10hrs] Investigate how skin suboptions can be presented in Special:Preferences
Closed, ResolvedPublicSpike

Description

Background

We would like to present the option to select legacy (old) vector within the preferences page as a suboption of the vector skin:

Acceptance criteria

Answer the following questions

  • Does the preferences page currently allow for this?
  • If not, what changes would we need to make to the preferences page to create this option?

Event Timeline

ovasileva created this task.Nov 6 2019, 5:59 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptNov 6 2019, 5:59 PM
ovasileva triaged this task as High priority.Nov 6 2019, 5:59 PM

@alexhollender, is it important to represent this new setting as a suboption? E.g.:

  • Skin A
  • Vector
    • Enable old mode
  • Skin C

I haven't looked yet but it may be easier on the tech side to move to the top level so I was curious. E.g.:

  • Skin A
  • Vector (new)
  • Vector (old)
  • Skin C

Beyond presentation, question for devs: how should the suboption interact with storage?

@alexhollender, is it important to represent this new setting as a suboption?
...
I haven't looked yet but it may be easier on the tech side to move to the top level so I was curious.

We discussed this idea briefly (see the "Skinning a cat" artboard here for additional context). Here is the relevant mock:

I like this approach because it feels straightforward, whereas having a sub-option within a skin is a new concept and could therefore be confusing. However since New Vector isn't actually a new skin (technically) I wonder if it would lead to confusion from other perspectives, e.g. for community developers? Generally speaking I think the more the UI matches what's happening behind the scenes in the software the better. Perhaps said a different way, it makes me wonder: what is a skin? From the perspective of the appearance of the site the difference between New Vector and Old Vector will be analogous to the difference between Skin A and Skin B, however maybe from other perspectives it will not.

Other things to consider:

  • For a while the two Vector experiences will be more similar than different. And if it's only a matter of presentation we could consider making this change at some point in the future, once the two Vector experiences are more divergent.
  • Which approach better communicates (and better sets us up for success regarding) our plan to eventually transition the default to the updated Vector experience? I'm not yet able to come up with any clear differences between the two options from this perspective.

If possible, I would like us to be able to present the option as it is in the task itself (as a suboption under vector). I think it send a very strong message that we are building improvements to the vector skin and that while we are still supporting the old version of the skin, we encourage people to use the new version. I think presenting it as a separate option should only be used as a fallback if the original idea is exceptionally high risk.

ovasileva updated the task description. (Show Details)Nov 13 2019, 6:07 PM
ovasileva renamed this task from [SPIKE] Investigate how skin suboptions can be presented in Special:Preferences to [SPIKE 10hrs] Investigate how skin suboptions can be presented in Special:Preferences.Nov 13 2019, 6:11 PM
Masumrezarock100 changed the subtype of this task from "Task" to "Spike".Nov 14 2019, 9:21 AM

I've looked into this some. Here's my understanding.

Vector/skin.json provides a [[ https://www.mediawiki.org/wiki/Manual:$wgValidSkinNames | ValidSkinNames ]] field. It currently only has a record for Vector but could be modified to include a second legacy skin:

	"ValidSkinNames": {
		"vector": "Vector",
		"legacyVector": "Legacy Vector"
	},

I believe this would at least be the beginning of fully supporting two skins in one repo. There may be other changes needed--I'm guessing we'll have to solicit folks with deep Core knowledge to have much confidence that we're not breaking anything.

Regardless, this change causes a second skin radio option to appear on Special:Preferences which is undesirable. This can be hidden by adding 'legacyVector' to [[ https://www.mediawiki.org/wiki/Manual:$wgSkipSkins | wgSkipSkins ]]. Doing so is a tiny bit confusing, but I think reasonable, because we'd still be exposing the setting as a suboption once we get that figured out.

I'm looking into how to actually support showing Legacy Vector as a suboption (which is the primary goal of this spike). That UI is built in Core's DefaultPreferencesFactory:

/**
  * @param User $user
  * @param IContextSource $context
  * @param array &$defaultPreferences
  * @return void
  */
protected function skinPreferences( User $user, IContextSource $context, &$defaultPreferences ) {
  # # Skin #####################################

  // Skin selector, if there is at least one valid skin
  $skinOptions = $this->generateSkinOptions( $user, $context );
  if ( $skinOptions ) {
    $defaultPreferences['skin'] = [
      'type' => 'radio',
      'options' => $skinOptions,
      'section' => 'rendering/skin',
    ];
  }

  $allowUserCss = $this->options->get( 'AllowUserCss' );
  $allowUserJs = $this->options->get( 'AllowUserJs' );
  # Create links to user CSS/JS pages for all skins
  # This code is basically copied from generateSkinOptions().  It'd
  # be nice to somehow merge this back in there to avoid redundancy.
  if ( $allowUserCss || $allowUserJs ) {
    $linkTools = [];
    $userName = $user->getName();

    if ( $allowUserCss ) {
      $cssPage = Title::makeTitleSafe( NS_USER, $userName . '/common.css' );
      $cssLinkText = $context->msg( 'prefs-custom-css' )->text();
      $linkTools[] = $this->linkRenderer->makeLink( $cssPage, $cssLinkText );
    }

    if ( $allowUserJs ) {
      $jsPage = Title::makeTitleSafe( NS_USER, $userName . '/common.js' );
      $jsLinkText = $context->msg( 'prefs-custom-js' )->text();
      $linkTools[] = $this->linkRenderer->makeLink( $jsPage, $jsLinkText );
    }

    $defaultPreferences['commoncssjs'] = [
      'type' => 'info',
      'raw' => true,
      'default' => $context->getLanguage()->pipeList( $linkTools ),
      'label-message' => 'prefs-common-config',
      'section' => 'rendering/skin',
    ];
  }
}

/**
  * @param User $user The User object
  * @param IContextSource $context
  * @return array Text/links to display as key; $skinkey as value
  */
protected function generateSkinOptions( User $user, IContextSource $context ) {
  $ret = [];

  $mptitle = Title::newMainPage();
  $previewtext = $context->msg( 'skin-preview' )->escaped();

  # Only show skins that aren't disabled in $wgSkipSkins
  $validSkinNames = Skin::getAllowedSkins();
  $allInstalledSkins = Skin::getSkinNames();

  // Display the installed skin the user has specifically requested via useskin=….
  $useSkin = $context->getRequest()->getRawVal( 'useskin' );
  if ( isset( $allInstalledSkins[$useSkin] )
    && $context->msg( "skinname-$useSkin" )->exists()
  ) {
    $validSkinNames[$useSkin] = $useSkin;
  }

  // Display the skin if the user has set it as a preference already before it was hidden.
  $currentUserSkin = $user->getOption( 'skin' );
  if ( isset( $allInstalledSkins[$currentUserSkin] )
    && $context->msg( "skinname-$currentUserSkin" )->exists()
  ) {
    $validSkinNames[$currentUserSkin] = $currentUserSkin;
  }

  foreach ( $validSkinNames as $skinkey => &$skinname ) {
    $msg = $context->msg( "skinname-{$skinkey}" );
    if ( $msg->exists() ) {
      $skinname = htmlspecialchars( $msg->text() );
    }
  }

  $defaultSkin = $this->options->get( 'DefaultSkin' );
  $allowUserCss = $this->options->get( 'AllowUserCss' );
  $allowUserJs = $this->options->get( 'AllowUserJs' );

  # Sort by the internal name, so that the ordering is the same for each display language,
  # especially if some skin names are translated to use a different alphabet and some are not.
  uksort( $validSkinNames, function ( $a, $b ) use ( $defaultSkin ) {
    # Display the default first in the list by comparing it as lesser than any other.
    if ( strcasecmp( $a, $defaultSkin ) === 0 ) {
      return -1;
    }
    if ( strcasecmp( $b, $defaultSkin ) === 0 ) {
      return 1;
    }
    return strcasecmp( $a, $b );
  } );

  $foundDefault = false;
  foreach ( $validSkinNames as $skinkey => $sn ) {
    $linkTools = [];

    # Mark the default skin
    if ( strcasecmp( $skinkey, $defaultSkin ) === 0 ) {
      $linkTools[] = $context->msg( 'default' )->escaped();
      $foundDefault = true;
    }

    # Create preview link
    $mplink = htmlspecialchars( $mptitle->getLocalURL( [ 'useskin' => $skinkey ] ) );
    $linkTools[] = "<a target='_blank' href=\"$mplink\">$previewtext</a>";

    # Create links to user CSS/JS pages
    if ( $allowUserCss ) {
      $cssPage = Title::makeTitleSafe( NS_USER, $user->getName() . '/' . $skinkey . '.css' );
      $cssLinkText = $context->msg( 'prefs-custom-css' )->text();
      $linkTools[] = $this->linkRenderer->makeLink( $cssPage, $cssLinkText );
    }

    if ( $allowUserJs ) {
      $jsPage = Title::makeTitleSafe( NS_USER, $user->getName() . '/' . $skinkey . '.js' );
      $jsLinkText = $context->msg( 'prefs-custom-js' )->text();
      $linkTools[] = $this->linkRenderer->makeLink( $jsPage, $jsLinkText );
    }

    $display = $sn . ' ' . $context->msg( 'parentheses' )
      ->rawParams( $context->getLanguage()->pipeList( $linkTools ) )
      ->escaped();
    $ret[$display] = $skinkey;
  }

  if ( !$foundDefault ) {
    // If the default skin is not available, things are going to break horribly because the
    // default value for skin selector will not be a valid value. Let's just not show it then.
    return [];
  }

  return $ret;
}

Some options that come to mind are:

  1. Update the data model and DefaultPreferencesFactory to allow a skin to specify a suboption. I don't know how involved this will be yet. A lot of this data format is baked into HTMLForm.
  2. Add a hook to allow a skin to specify an alternative to the inline HTML in generateSkinOptions(). This seems less desirable than #1 since all future edits would now have to reference both the data format _and_ the hook but could be more practical to _hack_.
  3. Subclass DefaultPreferencesFactory and somehow override the HTML construction (probably a bad idea for a number of reasons including that GlobalPreferencesFactory subclasses DefaultPreferencesFactory). I'm not planning to pursue this.
  4. Override the onGetPreferences() hook in Vector and overwrite the HTML for the Vector entry specifically (probably also a bad idea for a number of reasons including that this may not work well with the assumptions of the rest of the ecosystem and is fragile). I'm not planning to pursue this.

Still spiking. My next step is to dig into #1 and maybe #2. That's the current status today!

believe this would at least be the beginning of fully supporting two skins in one repo. There may be other changes needed--I'm guessing we'll have to solicit folks with deep Core knowledge to have much confidence that we're not breaking anything.

This has a huge downside however - mw.config.get('skin') and all associated skinStyles, gadgets and user scripts/styles specific to vector will no longer work. e.g. MediaWiki:Vector.css. In some cases this may be useful but in others it might cause us problems (e.g. we'll have to go into extensions and reenable the styles.

Taking this approach, while ensuring the goal we have of freezing current Vector would it make sense to keep the 'vector' key for the legacy version and use a new key for the new one. We'd likely still have to update skinStyles/gadgets etc in a lot of places for the new skin, but at least we'd have the time to do it rather than breaking existing experience.

Note: We did this when we pulled Minerva out of MobileFrontend. At one point there was a minerva and minervaneue key and we consolidated this into Minerva once the code in MobileFrontend had been dropped.

Regardless, this change causes a second skin radio option to appear on Special:Preferences which is undesirable. This can be hidden by adding 'legacyVector' to wgSkipSkins. Doing so is a tiny bit confusing, but I think reasonable, because we'd still be exposing the setting as a suboption once we get that figured out.

Yep wgSkipSkins could help here. However note if you've selected the skin it will show up as a separate radio.
You can try this out by going to https://en.wikipedia.org/wiki/Special:Preferences?useskin=cologneblue and enabling Cologne Blue and then refreshing the page without the useskin parameter.

In summary, the _user interface behavior_ wanted for the inner Legacy Vector setting is a nested checkbox that is interactive when the outer "Vector 2" radio is checked and non-interactive when the outer Vector 2 radio is unchecked. The _form submission data model_ wanted is still a radio where the wpskin parameter is set to exactly one skin. I think that divergent non-JS and JS experiences are necessary, unfortunately, and the former will not implement the mock. See the details below.


For non-JS, I'm aware of a few options:

  • Style the Legacy Vector radio as a checkbox via the experimental [appearance property](https://developer.mozilla.org/en-US/docs/Web/CSS/appearance) and/or images. The form data model and non-interactive appearance are correct but the user interface behavior is confusing since the inner Legacy Vector checkbox continues to behave like a radio button without any knowledge of the outer Vector 2 radio. I don't know of an additional CSS workaround to fix the interface behavior so I don't recommend this approach.
  • Use a checkbox for the Legacy Vector input (see below or playground). The inner checkbox can be forced to appear and behave as if disabled using CSS to set pointer-events: none; on the Legacy Vector checkbox and label when the outer Vector 2 radio is :not(:checked). This gives the user interface behavior wanted but the form's data model is incorrect since there's no way to disable the inner Legacy Checkbox without JS and the value of the checkbox is always sent. A new parameter could be used (e.g., skinVersion) but that does not fit into the preexisting design which assumes one entry per skin and may break things so I don't recommend this approach.
Server HTML
<form>
  <fieldset>
    <legend>Skin</legend>
    <ul>
      <li>
        <input type="radio" id="apple" name="wpskin" value="apple" />
        <label for="apple">Apple</label>
      </li>
      <li>
        <input type="radio" id="banana" name="wpskin" value="banana" checked />
        <label for="banana">Banana</label>
      </li>
      <li>
        <input type="radio" id="vector2" name="wpskin" value="vector2" />
        <label for="vector2">Vector</label>
        <div class="vectorOptions">
          <p>
            We are releasing updates to Vector gradually over the next few
            years. If Legacy Vector is enabled you will not see new features. We
            would love your feedback. <a href="#">Learn more</a>.
          </p>
          <input
            type="checkbox"
            id="vector"
            name="wpskin"
            value="vector"
            checked
          />
          <label class="vectorLabel" for="vector"
            >Enable Legacy Vector (frozen on December 2019)</label
          >
        </div>
      </li>
      <li>
        <input type="radio" id="carrot" name="wpskin" value="carrot" />
        <label for="carrot">Carrot</label>
      </li>
    </ul>
  </fieldset>
  <input id="wpEditToken" type="hidden" value="token+\" name="wpEditToken" />
  <input type="hidden" value="Special:Preferences" name="title" />
  <button type="submit" value="Save">Save</button>
</form>
Server CSS
li {
  list-style: none;
}

.vectorOptions {
  padding: 0.5rem 2rem;
  background: #eaf3ff;
}

#vector2:not(:checked) ~ .vectorOptions {
  background: #eaecf080;
}

#vector2:not(:checked) ~ .vectorOptions > #vector,
#vector2:not(:checked) ~ .vectorOptions > .vectorLabel {
  /* Disable checkbox state changes. */
  pointer-events: none;

  /* Make the checkbox appear disabled. */
  opacity: 0.5;
}
  • Continue using flat radio buttons for all skins. This is the approach I recommend since it promises to not break anything in itself, has the expected data model, and is the least amount of work. The user interface behavior is not ideal but is practical.

Open question: if we go with the last approach, is it worth adding the informational snippet? If so, I think that should be a distinct task.


For the JS experience, client layout changes will be necessary to achieve the design mocked. The reflow may be quite noticeable or may not be as a major layout change is already being performed on Special:Preferences page. The inner Legacy Vector checkbox disabled state will be updated by JS on radio checked state change (see below or playground). When the checkbox is disabled, only the outer radio state is used by the form which is the correct model. When it's enabled, both the outer Vector 2 radio wpskin value and inner Legacy Vector wpskin value are used. Since the latter appears last, this may be ok but the redundant outer value could be filtered out on save click if needed.

Client HTML
<form class="skin">
  <fieldset>
    <legend>Skin</legend>
    <ul>
      <li>
        <input type="radio" id="apple" name="wpskin" value="apple" />
        <label for="apple">Apple</label>
      </li>
      <li>
        <input type="radio" id="banana" name="wpskin" value="banana" checked />
        <label for="banana">Banana</label>
      </li>
      <li>
        <input type="radio" id="vector2" name="wpskin" value="vector2" />
        <label for="vector2">Vector</label>
        <div class="vectorOptions">
          <p>
            We are releasing updates to Vector gradually over the next few
            years. If Legacy Vector is enabled you will not see new features. We
            would love your feedback. <a href="#">Learn more</a>.
          </p>
          <input
            type="checkbox"
            id="vector"
            name="wpskin"
            value="vector"
            checked
            disabled
          />
          <label class="vectorLabel" for="vector"
            >Enable Legacy Vector (frozen on December 2019)</label
          >
        </div>
      </li>
      <li>
        <input type="radio" id="carrot" name="wpskin" value="carrot" />
        <label for="carrot">Carrot</label>
      </li>
    </ul>
  </fieldset>
  <input id="wpEditToken" type="hidden" value="token+\" name="wpEditToken" />
  <input type="hidden" value="Special:Preferences" name="title" />
  <button type="submit" value="Save">Save</button>
</form>
Client CSS
li {
  list-style: none;
}

.vectorOptions {
  padding: 0.5rem 2rem;
  background: #eaf3ff;
}

#vector2:not(:checked) ~ .vectorOptions {
  background: #eaecf080;
}
Client JS
const radios = document.querySelectorAll('input[type="radio"]')
const vector2 = document.getElementById('vector2')
const checkbox = document.getElementById('vector')

radios.forEach(radio =>
  radio.onchange = () => checkbox.disabled = !vector2.checked
)

I don't know how this will be integrated on the PHP side yet but it may be complicated. As mentioned earlier, the data model for skins seems strange. For example, the keys are the HTML and the skin names are the values:

HTMLForm API example
{
  "type": "radio",
  "options": {
    "Vector foo (default | <a target='_blank' href=\"/w/index.php?title=Main_Page&amp;useskin=vector\">Preview</a>)": "vector",
    "Eigenvector foo (<a target='_blank' href=\"/w/index.php?title=Main_Page&amp;useskin=eigenvector\">Preview</a>)": "eigenvector",
    "Legacy Vector foo (<a target='_blank' href=\"/w/index.php?title=Main_Page&amp;useskin=legacyVector\">Preview</a>)": "legacyVector",
    "MinervaNeue foo (<a target='_blank' href=\"/w/index.php?title=Main_Page&amp;useskin=minerva\">Preview</a>)": "minerva"
  },
  "section": "rendering/skin"
}

For the server experience, there shouldn't be any changes needed but I haven't looked into how the client handoff works yet.


I've revised my local patch to use 'vector' for legacy mode and 'vector2' for master per @Jdlrobson's recommendation. Alternative naming to "vector2" is welcome but this seemed the least surprising to me, albeit boring. Oh well.

Summary: The client changes are going to be challenging to write well, avoid regressions, and to get merged. I think the client changes, at least as I envision them, are dangerous and add significant technical debt. I'm happy to be shown otherwise but I don't see how to avoid these concerns practically using the preexisting architecture. Therefore, I recommend no client changes be made.

Adding a second skin

Regardless of whether client changes are made or not, skin.json will need to be revised to add "vector2" keys and update the now legacy Vector naming. Something like:

skin.json
"descriptionmsg": {
  "vector": "vector-skin-desc",
  "vector2": "vector2-skin-desc"
},
"namemsg": "skinname-vector2",
"ConfigRegistry": {
  "vector": "GlobalVarConfig::newInstance",
  "vector2": "GlobalVarConfig::newInstance"
},
"ValidSkinNames": {
  "vector": "LegacyVector",
  "vector2": "Vector"
},

I'm unsure what other places will need to be updated but it will probably be easiest to look to Timeless skin name usages as a reference.

Rollout

Changing skin.json has effect for all projects and all wikis. That is, there's no rollout. As soon as master is deployed, skin.json changes will be live regardless of whether any new features are actually available for the community to use. If this is correct, I think there are a few options:

  1. Make the change but defer skinname-vector and vector-skin-desc messaging updates by adding new initially unused keys, then use wgSkipSkins to hide vector2 everywhere. When new functionality is available, swap out skinname-vector and vector-skin-desc for the new keys and globally remove vector2 from wgSkipSkins. Note that the wgSkipSkins changes cannot be deployed selectively since that would cause the "Legacy Vector" option to be shown without the modern Vector alternative.
  2. Defer skin.json changes until new functionality is actually available.
  3. It may be possible to omit the vector2 addition to ValidSkinNames until new functionality is actually available. However, I'm unsure if adding a new skin but not registering it as a ValidSkinName is formally supported or not as I believe SkinFactory uses this field.

The effect of any option will globally enable the new Vector skin for all projects and all wikis so far as I know. There's still no selective rollout for any option. It's more about scheduling on / off.

Option #1 makes the most sense to me. I think it's lower risk than #2 since we can test changes in prod, and also #3 since that configuration may be unsupported. I think that having the skin available in a standard way from the beginning will also improve the development experience.

Server render

If client changes are not made, I don't see any additional changes to be made for the server render beyond adding a second skin. See T237561#5748785.

Client render

The Special:Preferences form lives in Core and changes to the client presentation would need to live there too. As such, I think the functionality would have to be sufficiently generalized (not Vector-specific) to be appropriate in Core. I can think of a couple options:

  • Create the concept of generic skin suboptions.
  • Following the precedent set by wgSkipSkins, add a wgLegacySkins option that maps old legacy skins to a skin to migrate to. E.g., $wgLegacySkins = [ 'foo' => 'bar', 'vector' => 'vector2' ];. The initial scope for this new configuration would be client-only. On the client, skins could be rendered as parent successor skin and child legacy skin as per the mock.

I think the functionality of the first option would be useful but probably require a major refactor to work with a "legacy mode" the way skins are implemented currently. The second option seems more practical.

Regardless, Moriel says there's no concept of a sub-checkbox in OOUI. Even if there were, I believe it would need to be integrated into PreferencesFormOOUI on the server and the client on top of the work to add the concept of a legacy skin. Moriel added that they had to implement a sub-checkbox pattern for the GlobalPreferences extension.

To enable skin section-specific rendering, I think the following changes would need to be made in addition to the client JavaScript:

.phan/config.php
...
'wgSkipSkins' => 'string[]',
'wgLegacySkins' => 'array<string,string>',
...
includes/DefaultSettings.php
...
$wgSkipSkins = [];

$wgLegacySkins = [];
...
includes/specials/SpecialPreferences.php
public function execute( $par ) {
  global $wgLegacySkins
  ...
  $out->addJsConfigVars( 'wgLegacySkins', $wgLegacySkins );
  ...
}
resources/Resources.php
...
  'mediawiki.special.preferences.ooui' => [
    'targets' => [ 'desktop', 'mobile' ],
    'scripts' => [
      ...,
      'resources/src/mediawiki.special.preferences.ooui/skin.js',
      ...
    ],
    ...
  ],
...

I'll also note for posterity that any "onGetPreferences()" hook specified in Vector/includes/Hooks.php seems to be invoked regardless of whether or not the skin is actually enabled by the user. This means that there is some flexibility here to do Vector-specific things but I don't think those sort of changes are going to be very sustainable for a fundamental option like skins since the rest of the system still needs to know about the old skin. This isn't my expertise but you can do stuff like replace the radio arbitrary HTML for better and worse:

Vector/includes/Hooks.php
public static function onGetPreferences( User $user, array &$prefs ) {
  foreach ( $prefs['skin']['options'] as $skinListItem => $skinName ) {
    if ($skinName === 'vector') {
      unset( $prefs['skin']['options'][$vectorSkinListItem] );
      $prefs['skin']['options'][ '<input type=checkbox>foooooo barrrrrr</input>' ] = [
        'type' => 'check',
        'options' => [
          'fooo' => 'vector'
        ],
        'section' => 'rendering/skin',
      ];
      break;
    }
  }
}

I think the GlobalPreferences client implementation will be similar but simpler than the legacy skin implementation needed because the presentation on the server and client is identical (and the server render doesn't seem to be supported at all for the local exceptions sub-checkbox).

The GlobalPreferences implementation shows how the sub-checkbox changes the outer radio disabled state and legacy skins would need something similar. If I'm not mistaken, it will get a little hairy:

  1. Hook into appearance tab infusion (filtering out the initial page load event fired as well as non-appearance tabs):
    1. Get the map of legacy to successor skins config.
    2. Identify and remove legacy skin radio inputs.
    3. For each legacy skin radio input removed:
      1. Identify the successor radio input.
      2. Inject a new adjacent OOUI LabelElement for the legacy skin. This may need styling as well.
  2. On radio state change:
    1. Enable any child legacy skin child checkbox for the currently checked radio successor (don't change the checked state though).
    2. Disable all legacy checkboxes that are not associated with the currently checked radio successor (don't change the checked state though).

Here's the pseudocode for step one:

resources/src/mediawiki.special.preferences.ooui/skin.js
/**
  * @param {HTMLElement} root
  * @return {void}
  */
function onAppearanceTabEnhanced( root ) {
  var
    labelElementSelector = '#mw-prefsection-rendering-skin .oo-ui-labelElement',
    labelElements = toArray( root.querySelectorAll( labelElementSelector ) );

  // Iterate over the label elements...
}

 /**
  * Wait for the tab to be infused by OOUI before manipulating it further. htmlform.enhance is
  * emitted on page load by resources/src/mediawiki.htmlform/htmlform.js, again for each tab
  * infused by resources/src/mediawiki.special.preferences.ooui/tabs.js, and possibly other
  * scenarios.
  * @param {any} $root
  * @return {void}
  */
function onHTMLFormEnhance( $root ) {
  var
    root = /** @type {Document|HTMLElement|undefined} */ ( $root[ 0 ] ),
    isTabInfusedEvent = root instanceof HTMLElement &&
      root.classList.contains( 'oo-ui-tabPanelLayout' ),
    isAppearanceTabEvent = isTabInfusedEvent && root.querySelector( '#mw-prefsection-rendering' );
  if ( isAppearanceTabEvent ) {
    mw.hook( 'htmlform.enhance' ).remove( onHTMLFormEnhance );
    onAppearanceTabEnhanced( /** @type {HTMLElement} */ ( root ) );
  }
}

mw.hook( 'htmlform.enhance' ).add( onHTMLFormEnhance );

I think skin.js will be the meat of the change. There may likely be additional unforeseen changes needed. We would also probably want to document that support is very primitive: 1) cycles like 'vector' => 'vector' are undefined, 2) multi-levels are unsupported liked 'vector' => 'vector2', 'monobook' => 'vector', and 3) only a single legacy mode is supported per skin (e.g., 'vector' => 'vector3', 'vector2' => 'vector3' is unsupported) 4) client-only.

Maybe there's some OOUI functionality that can help with parts but all in all, I think this is going to be a large body of clumsy (error-prone) code.

Niedzielski removed Niedzielski as the assignee of this task.Jan 3 2020, 10:02 PM

@ovasileva, @alexhollender I've discussed this task extensively with the devs in the last super-happy-dev-time which was very helpful to me (less so to them). Here are the options:

Option 1 Add a second skin

Add a second skin. There's nothing special about "vector2" except that it happens to live in the same repo as another skin, "vector". This will appear as a radio option like any other skin and is the lowest effort and lowest risk but there's no practical visual customizations possible except for changing the messaging. For example, the old Vector can be renamed to "Legacy Vector (frozen on December 2019)" to distinguish it from the new "Vector".

  • Pro: very straight forward, low effort, and low risk.
  • Pro: each skin is a first class citizen.
  • Con: limited rollout strategy available.
  • Con: skin preference is preexisting.
  • Con: presentation is not ideal:

Option 2 Add a Vector skin version preference

Add a version preference that is injected via Vector/Hooks.php (think Popups' enable / disable preference).

  • Pro: pretty straight forward and well-supported by the preferences page.
  • Con: Legacy Vector and Vector no longer have a first class skin distinction. Any tooling built around skins will only know about "Vector" and have no knowledge of versioning. For example, skin preview will use whatever the last skin version saved was (legacy or modern) or the default.
  • Pro: rollout is fully configurable as needed.
  • Pro/Con: presentation is reasonable but not ideal. I haven't figured out how to hide the entire section in the proper way yet but I think we'll be able to show / hide the entire Vector section based on the skin chosen. Currently, just the checkbox hides / unhides based on the skin option but this is using all stock MediaWiki functionality:

Shown (Vector skin selected):

Hidden (Non-Vector skin selected):

Option 3 Overhaul the skin system

Refactor MediaWiki's architecture to include the concept of a skin suboption, variant, or version. Lots of unknowns and possibly lengthy. Some similarities with https://github.com/wikimedia/mediawiki-extensions-Theme.

  • Pro: we can add whatever flexibility is needed now and for the future foreseen.
  • Con: high risk, lots of unknowns, unknown level of effort but probably big.

I'm happy to schedule a meeting or chat post-standup tomorrow. Let me know your thoughts and I can task out.

Discussed with @Niedzielski and @alexhollender and decided to go with Option 2 as the cons are somewhat minimal and it creates a less confusing experience for the user.

There are a few configs + the UI changes needed documented in a jumbo task in T242381. This can be broken up further as wanted.

Assigning to Olga and moving to design review to signal that @ovasileva and @alexhollender should review T242381.