From e5bf8adad7959a5634a6a797be71684aa08f495a Mon Sep 17 00:00:00 2001 From: Jon Robson Date: Tue, 11 Jul 2023 16:13:57 -0700 Subject: [PATCH] Limited width uses new client preferences system * Update classes to use clientpref-1 and clientpref-0 suffix for limited width I've limited this to the only client preference for now to reduce risk. * For cached HTML retain existing CSS rules, and continue saving a cookie * Migrate cookie if found for newly generated pages. This will be to ensure the old cookie and new cookie are in sync (this should be a one time operation) Depends-On: I1e635f843ac9b2f248b1f7618134598e80291b38 Bug: T341641 Change-Id: I120f8f7114b33d2cfbd1c3c57ebf41f8b2d7fec4 --- includes/FeatureManagement/FeatureManager.php | 16 +++++++- resources/skins.vector.js/features.js | 37 +++++++++++++++---- .../skins.vector.js/limitedWidthToggle.js | 28 ++++++++++++++ .../skins.vector.styles/layouts/grid.less | 6 ++- .../skins.vector.styles/layouts/preZebra.less | 2 + .../skins.vector.styles/layouts/screen.less | 2 + .../layouts/grid.less | 2 + .../layouts/screen.less | 4 ++ skinStyles/mediawiki.action.edit.less | 2 + 9 files changed, 89 insertions(+), 10 deletions(-) diff --git a/includes/FeatureManagement/FeatureManager.php b/includes/FeatureManagement/FeatureManager.php index d766e90da..4c553b524 100644 --- a/includes/FeatureManagement/FeatureManager.php +++ b/includes/FeatureManagement/FeatureManager.php @@ -137,8 +137,22 @@ class FeatureManager { return array_map( static function ( $featureName ) use ( $featureManager ) { // switch to lower case and switch from camel case to hyphens $featureClass = ltrim( strtolower( preg_replace( '/[A-Z]([A-Z](?![a-z]))*/', '-$0', $featureName ) ), '-' ); + + // Client side preferences + switch ( $featureClass ) { + case 'limited-width': + $suffixEnabled = 'clientpref-1'; + $suffixDisabled = 'clientpref-0'; + break; + default: + // FIXME: Eventually this should not be necessary. + $suffixEnabled = 'enabled'; + $suffixDisabled = 'disabled'; + break; + } $prefix = 'vector-feature-' . $featureClass . '-'; - return $featureManager->isFeatureEnabled( $featureName ) ? $prefix . 'enabled' : $prefix . 'disabled'; + return $featureManager->isFeatureEnabled( $featureName ) ? + $prefix . $suffixEnabled : $prefix . $suffixDisabled; }, array_keys( $this->features ) ); } diff --git a/resources/skins.vector.js/features.js b/resources/skins.vector.js/features.js index 177c8e399..1a49056b7 100644 --- a/resources/skins.vector.js/features.js +++ b/resources/skins.vector.js/features.js @@ -13,11 +13,20 @@ function save( feature, enabled ) { if ( !mw.user.isNamed() ) { switch ( feature ) { case 'limited-width': + // FIXME: Cookie is set for cached HTML using the older inline script. + // When removing this code, + // please retain `mw.cookie.set( 'mwclientprefs', null );` for 1-4 weeks + // to clean up after ourselves so users don't have a non-expiring cookie which + // serves no purpose. if ( enabled ) { mw.cookie.set( 'mwclientprefs', null ); } else { mw.cookie.set( 'mwclientprefs', 'vector-feature-limited-width' ); } + + // Save the setting under the new system + // @ts-ignore https://github.com/wikimedia/typescript-types/pull/44 + mw.user.clientPrefs.set( `vector-feature-${feature}`, enabled ? '1' : '0' ); break; default: // not a supported anonymous preference @@ -35,14 +44,18 @@ function save( feature, enabled ) { * * @param {string} name feature name * @param {boolean} [override] option to force enabled or disabled state. - * + * @param {boolean} [isLegacy] should we search for legacy classes + * FIXME: this is for supporting cached HTML, + * this should be removed 1-4 weeks after the patch has been in production. * @return {boolean} The new feature state (false=disabled, true=enabled). * @throws {Error} if unknown feature toggled. */ -function toggleDocClasses( name, override ) { - const featureClassEnabled = `vector-feature-${name}-enabled`, +function toggleDocClasses( name, override, isLegacy ) { + const suffixEnabled = isLegacy ? 'enabled' : 'clientpref-1'; + const suffixDisabled = isLegacy ? 'disabled' : 'clientpref-0'; + const featureClassEnabled = `vector-feature-${name}-${suffixEnabled}`, classList = document.documentElement.classList, - featureClassDisabled = `vector-feature-${name}-disabled`; + featureClassDisabled = `vector-feature-${name}-${suffixDisabled}`; if ( classList.contains( featureClassDisabled ) || override === true ) { classList.remove( featureClassDisabled ); @@ -52,6 +65,9 @@ function toggleDocClasses( name, override ) { classList.add( featureClassDisabled ); classList.remove( featureClassEnabled ); return false; + } else if ( !isLegacy ) { + // try again using the legacy classes + return toggleDocClasses( name, override, true ); } else { throw new Error( `Attempt to toggle unknown feature: ${name}` ); } @@ -73,7 +89,8 @@ function toggle( name ) { * @return {boolean} */ function isEnabled( name ) { - return document.documentElement.classList.contains( getClass( name, true ) ); + return document.documentElement.classList.contains( getClass( name, true ) ) || + document.documentElement.classList.contains( getClass( name, true, true ) ); } /** @@ -81,13 +98,17 @@ function isEnabled( name ) { * * @param {string} name * @param {boolean} featureEnabled + * @param {boolean} [isLegacy] FIXME: this is for supporting cached HTML, + * this should be removed 1-4 weeks after the patch has been in production. * @return {string} */ -function getClass( name, featureEnabled ) { +function getClass( name, featureEnabled, isLegacy ) { if ( featureEnabled ) { - return `vector-feature-${name}-enabled`; + const suffix = isLegacy ? 'enabled' : '1'; + return `vector-feature-${name}-${suffix}`; } else { - return `vector-feature-${name}-disabled`; + const suffix = isLegacy ? 'disabled' : '0'; + return `vector-feature-${name}-${suffix}`; } } diff --git a/resources/skins.vector.js/limitedWidthToggle.js b/resources/skins.vector.js/limitedWidthToggle.js index eb980ef8b..8a639da33 100644 --- a/resources/skins.vector.js/limitedWidthToggle.js +++ b/resources/skins.vector.js/limitedWidthToggle.js @@ -34,6 +34,34 @@ function init() { const settings = /** @type {HTMLElement|null} */ ( document.querySelector( '.vector-settings' ) ); const toggle = /** @type {HTMLElement|null} */ ( document.querySelector( '.vector-limited-width-toggle' ) ); const toggleIcon = /** @type {HTMLElement|null} */ ( document.querySelector( '.vector-limited-width-toggle .vector-icon' ) ); + const cookie = mw.cookie.get( 'mwclientprefs' ); + // @ts-ignore https://github.com/wikimedia/typescript-types/pull/42 + const existingValue = mw.user.clientPrefs.get( 'vector-feature-limited-width' ); + // On a cached page existingValue will return false. + // If the cookie is set then we need to add the appropriate class to the page + // so that the toggle works correctly. + if ( !existingValue ) { + document.documentElement.classList.add( + `vector-feature-limited-width-clientpref-${cookie ? '0' : '1'}` + ); + document.documentElement.classList.remove( + 'vector-feature-limited-width-disabled', + 'vector-feature-limited-width-enabled' + ); + } + // Sync cookie value with clientPrefs if the two are misaligned. + if ( cookie && existingValue === '1' ) { + // @ts-ignore https://github.com/wikimedia/typescript-types/pull/42 + mw.user.clientPrefs.set( 'vector-feature-limited-width', '0' ); + // Update the display. + document.documentElement.classList.remove( + 'vector-feature-limited-width-clientpref-1', + 'vector-feature-limited-width-enabled' + ); + document.documentElement.classList.add( + 'vector-feature-limited-width-clientpref-0' + ); + } if ( !settings || !toggle || !toggleIcon ) { return; diff --git a/resources/skins.vector.styles/layouts/grid.less b/resources/skins.vector.styles/layouts/grid.less index 22772eb4b..d414711a1 100644 --- a/resources/skins.vector.styles/layouts/grid.less +++ b/resources/skins.vector.styles/layouts/grid.less @@ -60,7 +60,9 @@ } } } - +// FIXME: Remove -disabled when cache has cleared. +&.vector-feature-limited-width-clientpref-0 .mw-content-container, +&.vector-feature-limited-width-clientpref-0 .mw-table-of-contents-container, &.vector-feature-limited-width-disabled .mw-content-container, &.vector-feature-limited-width-content-disabled .mw-content-container, &.vector-feature-limited-width-disabled .mw-table-of-contents-container, @@ -122,6 +124,8 @@ column-gap: @grid-column-gap; } + // FIXME: Remove -disabled when cache has cleared. + &.vector-feature-limited-width-clientpref-0 .mw-body, &.vector-feature-limited-width-disabled .mw-body, &.vector-feature-limited-width-content-disabled .mw-body { grid-template-columns: ~'minmax(0, 1fr) min-content'; diff --git a/resources/skins.vector.styles/layouts/preZebra.less b/resources/skins.vector.styles/layouts/preZebra.less index 774d83a67..1168eface 100644 --- a/resources/skins.vector.styles/layouts/preZebra.less +++ b/resources/skins.vector.styles/layouts/preZebra.less @@ -4,6 +4,8 @@ .mixin-page-container(); } +// FIXME: Remove -disabled when cache has cleared. +&.vector-feature-limited-width-clientpref-0 .vector-header-container, &.vector-feature-limited-width-disabled .vector-header-container { max-width: none; } diff --git a/resources/skins.vector.styles/layouts/screen.less b/resources/skins.vector.styles/layouts/screen.less index 15f737492..febfcac8a 100644 --- a/resources/skins.vector.styles/layouts/screen.less +++ b/resources/skins.vector.styles/layouts/screen.less @@ -129,6 +129,8 @@ body { } } +// FIXME: Remove -disabled when cache has cleared. +&.vector-feature-limited-width-clientpref-0 .mw-page-container, &.vector-feature-limited-width-disabled .mw-page-container { max-width: none; } diff --git a/resources/skins.vector.zebra.styles/layouts/grid.less b/resources/skins.vector.zebra.styles/layouts/grid.less index a032f6acc..6480c7f20 100644 --- a/resources/skins.vector.zebra.styles/layouts/grid.less +++ b/resources/skins.vector.zebra.styles/layouts/grid.less @@ -115,6 +115,8 @@ column-gap: @grid-column-gap; } + // FIXME: Remove -disabled when cache has cleared. + &.vector-feature-limited-width-clientpref-0 .mw-body, &.vector-feature-limited-width-disabled .mw-body, &.vector-feature-limited-width-content-disabled .mw-body { grid-template-columns: ~'minmax(0, 1fr) min-content'; diff --git a/resources/skins.vector.zebra.styles/layouts/screen.less b/resources/skins.vector.zebra.styles/layouts/screen.less index 370d45f1b..bcccc7401 100644 --- a/resources/skins.vector.zebra.styles/layouts/screen.less +++ b/resources/skins.vector.zebra.styles/layouts/screen.less @@ -183,6 +183,10 @@ body { .mixin-vector-page-container-sizing(); } +// FIXME: Remove -disabled when cache has cleared. +&.vector-feature-limited-width-clientpref-0 .mw-page-container, +&.vector-feature-limited-width-clientpref-0 .vector-sticky-header, +&.vector-feature-limited-width-clientpref-0 .mw-header, &.vector-feature-limited-width-disabled .mw-page-container, &.vector-feature-limited-width-disabled .vector-sticky-header, &.vector-feature-limited-width-disabled .mw-header { diff --git a/skinStyles/mediawiki.action.edit.less b/skinStyles/mediawiki.action.edit.less index ef47d1f21..66544aa89 100644 --- a/skinStyles/mediawiki.action.edit.less +++ b/skinStyles/mediawiki.action.edit.less @@ -1,5 +1,7 @@ @import '../resources/common/variables.less'; +// FIXME: Remove -enabled when cache has cleared. +.vector-feature-limited-width-clientpref-1 #wikiPreview, .vector-feature-limited-width-enabled #wikiPreview { // @max-width-content-container is defined in ems, so the final width depends on font-size, // but this element is nested in an element with a different font-size. Divide by the