Improve documentation for how feature manager classes work.

This changes nothing about the existing classes, it just improves
how they are documented.

The FIXME comment was making things confusing.

Make sure every feature class is accounted for and audited and
throw a RuntimeException if we forget to document it.

Change-Id: I9d8f6553fe6b8c2ae80d8b2490c8895a8334a537
This commit is contained in:
Jon Robson 2024-07-08 09:25:30 -07:00 committed by Jdlrobson
parent 2b4abc891d
commit acb79c5e1b
3 changed files with 40 additions and 22 deletions

View file

@ -27,6 +27,7 @@ use MediaWiki\Skins\Vector\ConfigHelper;
use MediaWiki\Skins\Vector\Constants;
use MediaWiki\Skins\Vector\FeatureManagement\Requirements\SimpleRequirement;
use MediaWiki\User\Options\UserOptionsLookup;
use RuntimeException;
use Wikimedia\Assert\Assert;
/**
@ -179,6 +180,8 @@ class FeatureManager {
// Client side preferences
switch ( $featureName ) {
// This feature has 3 possible states: 0, 1, 2 and -excluded.
// It persists for all users.
case CONSTANTS::FEATURE_FONT_SIZE:
if ( ConfigHelper::shouldDisable(
$config->get( 'VectorFontSizeConfigurableOptions' ), $request, $title
@ -188,6 +191,8 @@ class FeatureManager {
$suffixEnabled = 'clientpref-' . $this->getUserPreferenceValue( CONSTANTS::PREF_KEY_FONT_SIZE );
$suffixDisabled = 'clientpref-0';
break;
// This feature has 4 possible states: day, night, os and -excluded.
// It persists for all users.
case CONSTANTS::PREF_NIGHT_MODE:
// if night mode is disabled for the page, add the exclude class instead and return early
if ( ConfigHelper::shouldDisable( $config->get( 'VectorNightModeOptions' ), $request, $title ) ) {
@ -208,17 +213,32 @@ class FeatureManager {
// So that editors can target the same class across skins
$prefix .= 'skin-theme-';
break;
// These features persist for all users and have two valid states: 0 and 1.
case CONSTANTS::FEATURE_LIMITED_WIDTH:
case CONSTANTS::FEATURE_TOC_PINNED:
case CONSTANTS::FEATURE_APPEARANCE_PINNED:
$suffixEnabled = 'clientpref-1';
$suffixDisabled = 'clientpref-0';
break;
default:
// FIXME: Eventually this should not be necessary.
// These features only persist for logged in users so do not contain the clientpref suffix.
// These features have two valid states: enabled and disabled. In future it would be nice if these
// were 0 and 1 so that the features.js module cannot be applied to server side only flags.
case CONSTANTS::FEATURE_MAIN_MENU_PINNED:
case CONSTANTS::FEATURE_PAGE_TOOLS_PINNED:
// Server side only feature flags.
// Note these classes are fixed and cannot be changed at runtime by JavaScript,
// only via modification to LocalSettings.php.
case Constants::FEATURE_NIGHT_MODE:
case Constants::FEATURE_APPEARANCE:
case Constants::FEATURE_LIMITED_WIDTH_CONTENT:
case Constants::FEATURE_LANGUAGE_IN_HEADER:
case Constants::FEATURE_LANGUAGE_IN_MAIN_PAGE_HEADER:
case Constants::FEATURE_STICKY_HEADER:
$suffixEnabled = 'enabled';
$suffixDisabled = 'disabled';
break;
default:
throw new RuntimeException( "Feature $featureName has no associated feature class." );
}
return $this->isFeatureEnabled( $featureName ) ?
$prefix . $suffixEnabled : $prefix . $suffixDisabled;

View file

@ -34,24 +34,23 @@ 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.
* @param {boolean} [isNotClientPreference] the feature is not a client preference,
* so does not persist for logged out users.
* @return {boolean} The new feature state (false=disabled, true=enabled).
* @throws {Error} if unknown feature toggled.
*/
function toggleDocClasses( name, override, isLegacy ) {
const suffixEnabled = isLegacy ? 'enabled' : 'clientpref-1';
const suffixDisabled = isLegacy ? 'disabled' : 'clientpref-0';
function toggleDocClasses( name, override, isNotClientPreference ) {
const suffixEnabled = isNotClientPreference ? 'enabled' : 'clientpref-1';
const suffixDisabled = isNotClientPreference ? 'disabled' : 'clientpref-0';
const featureClassEnabled = `vector-feature-${ name }-${ suffixEnabled }`,
classList = document.documentElement.classList,
featureClassDisabled = `vector-feature-${ name }-${ suffixDisabled }`,
// If neither of the classes can be found it is a legacy feature
isLegacyFeature = !classList.contains( featureClassDisabled ) &&
isLoggedInOnlyFeature = !classList.contains( featureClassDisabled ) &&
!classList.contains( featureClassEnabled );
// Check in legacy mode.
if ( isLegacyFeature && !isLegacy ) {
if ( isLoggedInOnlyFeature && !isNotClientPreference ) {
// try again using the legacy classes
return toggleDocClasses( name, override, true );
} else if ( classList.contains( featureClassDisabled ) || override === true ) {
@ -92,16 +91,15 @@ function isEnabled( name ) {
*
* @param {string} name
* @param {boolean} featureEnabled
* @param {boolean} [isLegacy] FIXME: this is for supporting cached HTML,
* but also features using the old feature class structure.
* @param {boolean} [isClientPreference] whether the feature is also a client preference
* @return {string}
*/
function getClass( name, featureEnabled, isLegacy ) {
function getClass( name, featureEnabled, isClientPreference ) {
if ( featureEnabled ) {
const suffix = isLegacy ? 'enabled' : 'clientpref-1';
const suffix = isClientPreference ? 'clientpref-1' : 'enabled';
return `vector-feature-${ name }-${ suffix }`;
} else {
const suffix = isLegacy ? 'disabled' : 'clientpref-0';
const suffix = isClientPreference ? 'clientpref-0' : 'disabled';
return `vector-feature-${ name }-${ suffix }`;
}
}

View file

@ -22,8 +22,8 @@ class FeatureManagerTest extends \MediaWikiIntegrationTestCase {
[ CONSTANTS::FEATURE_LIMITED_WIDTH, true, 'vector-feature-limited-width-clientpref-1' ],
[ CONSTANTS::FEATURE_TOC_PINNED, false, 'vector-feature-toc-pinned-clientpref-0' ],
[ CONSTANTS::FEATURE_TOC_PINNED, true, 'vector-feature-toc-pinned-clientpref-1' ],
[ 'default', false, 'vector-feature-default-disabled' ],
[ 'default', true, 'vector-feature-default-enabled' ],
[ CONSTANTS::FEATURE_NIGHT_MODE, false, 'vector-feature-night-mode-disabled' ],
[ CONSTANTS::FEATURE_NIGHT_MODE, true, 'vector-feature-night-mode-enabled' ],
];
}
@ -49,14 +49,14 @@ class FeatureManagerTest extends \MediaWikiIntegrationTestCase {
$featureManager = $this->newFeatureManager();
$featureManager->registerSimpleRequirement( 'requirement', true );
$featureManager->registerSimpleRequirement( 'disabled', false );
$featureManager->registerFeature( 'sticky-header', [ 'requirement' ] );
$featureManager->registerFeature( 'TableOfContents', [ 'requirement' ] );
$featureManager->registerFeature( 'Test', [ 'disabled' ] );
$featureManager->registerFeature( Constants::FEATURE_STICKY_HEADER, [ 'requirement' ] );
$featureManager->registerFeature( Constants::FEATURE_NIGHT_MODE, [ 'requirement' ] );
$featureManager->registerFeature( Constants::FEATURE_LIMITED_WIDTH_CONTENT, [ 'disabled' ] );
$this->assertEquals(
[
'vector-feature-sticky-header-enabled',
'vector-feature-table-of-contents-enabled',
'vector-feature-test-disabled'
'vector-feature-night-mode-enabled',
'vector-feature-limited-width-content-disabled'
],
$featureManager->getFeatureBodyClass()
);