From d7e6690b52c06fa185ce866c4edf73bdd48a5d16 Mon Sep 17 00:00:00 2001 From: Jan Drewniak Date: Wed, 13 Apr 2022 23:58:05 -0400 Subject: [PATCH] Sticky header edit button A/B test bucketing (updated) Adds behaviour for conditionally adding the edit button to the sticky-header based on A/B test bucketing. This behaviour depends on having the `$wgVectorStickyHeaderEdit` config set to true for logged-in users: $wgVectorStickyHeaderEdit = [ "logged_in" => true, "logged_out" => false ]; as well as an AB test configured with the following buckets: $wgVectorWebABTestEnrollment = [ 'name' => 'vector.sticky_header_edit', 'enabled' => true, 'buckets' => [ 'unsampled' => [ 'samplingRate' => 0 ], 'stickyHeaderEditButtonControl' => [ 'samplingRate' => 0 ], 'stickyHeaderEditButtonTreatment' => [ 'samplingRate' => 1 ] ] ]; With that config, this change hides the sticky header for all users except those in the stickyHeaderEditButtonTreatment bucket. NOTE: This patch address the sticky header being visible on incorrect namespaces when the AB test is enabled and the revert of 42b808738a1aba7e6e94ae37554236e619bde38c. Bug: T299959 Bug: T309370 Change-Id: I3effbb3e5f0bb1c8663255936458e3849511dfca --- resources/skins.vector.es6/AB.js | 9 ++ resources/skins.vector.es6/main.js | 93 +++++++++++++++----- resources/skins.vector.es6/stickyHeader.js | 15 +++- resources/skins.vector.es6/stickyHeaderAB.js | 0 4 files changed, 91 insertions(+), 26 deletions(-) create mode 100644 resources/skins.vector.es6/stickyHeaderAB.js diff --git a/resources/skins.vector.es6/AB.js b/resources/skins.vector.es6/AB.js index 4059c868..5cb2435b 100644 --- a/resources/skins.vector.es6/AB.js +++ b/resources/skins.vector.es6/AB.js @@ -4,6 +4,15 @@ const EXCLUDED_BUCKET = 'unsampled'; const TREATMENT_BUCKET_SUBSTRING = 'treatment'; const WEB_AB_TEST_ENROLLMENT_HOOK = 'mediawiki.web_AB_test_enrollment'; +/** + * @typedef {Object} WebABTest + * @property {string} name + * @property {function(): string} getBucket + * @property {function(string): boolean} isInBucket + * @property {function(): boolean} isInSample + * @property {function(): boolean} isInTreatmentBucket + */ + /** * @typedef {Object} SamplingRate * @property {number} samplingRate The desired sampling rate for the group in the range [0, 1]. diff --git a/resources/skins.vector.es6/main.js b/resources/skins.vector.es6/main.js index 3e3e6ace..9141a124 100644 --- a/resources/skins.vector.es6/main.js +++ b/resources/skins.vector.es6/main.js @@ -44,6 +44,63 @@ const getHeadingIntersectionHandler = ( changeActiveSection ) => { }; }; +/** + * Initialize sticky header AB tests and determine whether to show the sticky header + * based on which buckets the user is in. + * + * @typedef {Object} InitStickyHeaderABTests + * @property {boolean} disableEditIcons - Should the sticky header have an edit icon + * @property {boolean} showStickyHeader - Should the sticky header be shown + * @param {ABTestConfig} abConfig + * @param {boolean} isStickyHeaderFeatureAllowed and the user is logged in + * @param {function(ABTestConfig): initExperiment.WebABTest} getEnabledExperiment + * @return {InitStickyHeaderABTests} + */ +function initStickyHeaderABTests( abConfig, isStickyHeaderFeatureAllowed, getEnabledExperiment ) { + let show = isStickyHeaderFeatureAllowed, + stickyHeaderExperiment, + noEditIcons = true; + + // Determine if user is eligible for sticky header AB test + if ( + isStickyHeaderFeatureAllowed && // The sticky header can be shown on the page + abConfig.enabled && // An AB test config is enabled + ( // One of the sticky-header AB tests is specified in the config + abConfig.name === stickyHeader.STICKY_HEADER_EXPERIMENT_NAME || + abConfig.name === stickyHeader.STICKY_HEADER_EDIT_EXPERIMENT_NAME + ) + ) { + // If eligible, initialize the AB test + stickyHeaderExperiment = getEnabledExperiment( abConfig ); + + // If running initial AB test, only show sticky header to treatment group. + if ( stickyHeaderExperiment.name === stickyHeader.STICKY_HEADER_EXPERIMENT_NAME ) { + show = stickyHeaderExperiment.isInTreatmentBucket(); + + // Remove class if present on the html element so that scroll + // padding isn't applied to users who don't experience the new treatment. + if ( !show ) { + document.documentElement.classList.remove( 'vector-sticky-header-enabled' ); + } + + } + + // If running edit-button AB test, show sticky header to all buckets + // and show edit button for treatment group + if ( stickyHeaderExperiment.name === stickyHeader.STICKY_HEADER_EDIT_EXPERIMENT_NAME ) { + show = true; + if ( stickyHeaderExperiment.isInTreatmentBucket() ) { + noEditIcons = false; + } + } + } + + return { + showStickyHeader: show, + disableEditIcons: noEditIcons + }; +} + /** * @return {void} */ @@ -71,25 +128,13 @@ const main = () => { allowedAction && 'IntersectionObserver' in window; - const isExperimentEnabled = - !!ABTestConfig.enabled && - ABTestConfig.name === stickyHeader.STICKY_HEADER_EXPERIMENT_NAME && - !mw.user.isAnon() && - isStickyHeaderAllowed; - - // If necessary, initialize experiment and fire the A/B test enrollment hook. - const stickyHeaderExperiment = isExperimentEnabled && - initExperiment( Object.assign( {}, ABTestConfig, { token: mw.user.getId() } ) ); - - // Remove class if present on the html element so that scroll padding isn't undesirably - // applied to users who don't experience the new treatment. - if ( stickyHeaderExperiment && !stickyHeaderExperiment.isInTreatmentBucket() ) { - document.documentElement.classList.remove( 'vector-sticky-header-enabled' ); - } - - const isStickyHeaderEnabled = stickyHeaderExperiment ? - stickyHeaderExperiment.isInTreatmentBucket() : - isStickyHeaderAllowed; + const { showStickyHeader, disableEditIcons } = initStickyHeaderABTests( + ABTestConfig, + isStickyHeaderAllowed && !mw.user.isAnon(), + ( config ) => initExperiment( + Object.assign( {}, config, { token: mw.user.getId() } ) + ) + ); // Table of contents const tocElement = document.getElementById( TOC_ID ); @@ -99,25 +144,26 @@ const main = () => { // Set up intersection observer for page title, used by sticky header const observer = scrollObserver.initScrollObserver( () => { - if ( isStickyHeaderAllowed && isStickyHeaderEnabled ) { + if ( isStickyHeaderAllowed && showStickyHeader ) { stickyHeader.show(); } scrollObserver.fireScrollHook( 'down', PAGE_TITLE_SCROLL_HOOK ); }, () => { - if ( isStickyHeaderAllowed && isStickyHeaderEnabled ) { + if ( isStickyHeaderAllowed && showStickyHeader ) { stickyHeader.hide(); } scrollObserver.fireScrollHook( 'up', PAGE_TITLE_SCROLL_HOOK ); } ); - if ( isStickyHeaderAllowed && isStickyHeaderEnabled ) { + if ( isStickyHeaderAllowed && showStickyHeader ) { stickyHeader.initStickyHeader( { header, userMenu, observer, - stickyIntersection + stickyIntersection, + disableEditIcons } ); } else if ( stickyIntersection ) { observer.observe( stickyIntersection ); @@ -222,6 +268,7 @@ const main = () => { module.exports = { main, test: { + initStickyHeaderABTests, getHeadingIntersectionHandler } }; diff --git a/resources/skins.vector.es6/stickyHeader.js b/resources/skins.vector.es6/stickyHeader.js index f74979f9..98317b35 100644 --- a/resources/skins.vector.es6/stickyHeader.js +++ b/resources/skins.vector.es6/stickyHeader.js @@ -13,6 +13,7 @@ const ULS_HIDE_CLASS = 'uls-dialog-sticky-hide', VECTOR_USER_LINKS_SELECTOR = '.vector-user-links', SEARCH_TOGGLE_SELECTOR = '.vector-sticky-header-search-toggle', + STICKY_HEADER_EDIT_EXPERIMENT_NAME = 'vector.sticky_header_edit', STICKY_HEADER_EXPERIMENT_NAME = 'vector.sticky_header'; /** @@ -212,6 +213,7 @@ function prepareEditIcons( if ( !primaryEditSticky || !wikitextSticky || !protectedSticky ) { return; } + if ( !primaryEdit ) { removeNode( protectedSticky ); removeNode( wikitextSticky ); @@ -343,13 +345,15 @@ function prepareUserMenu( userMenu ) { * @param {Element} userMenuStickyContainer * @param {IntersectionObserver} stickyObserver * @param {Element} stickyIntersection + * @param {boolean} disableEditIcons */ function makeStickyHeaderFunctional( header, userMenu, userMenuStickyContainer, stickyObserver, - stickyIntersection + stickyIntersection, + disableEditIcons ) { const userMenuStickyContainerInner = userMenuStickyContainer @@ -370,7 +374,9 @@ function makeStickyHeaderFunctional( const ceEdit = document.querySelector( '#ca-edit a' ); const protectedEdit = document.querySelector( '#ca-viewsource a' ); const isProtected = !!protectedEdit; - const primaryEdit = protectedEdit || ( veEdit || ceEdit ); + // For sticky header edit A/B test, conditionally remove the edit icon by setting null. + // Otherwise, use either protected, ve, or source edit (in that order). + const primaryEdit = disableEditIcons ? null : protectedEdit || veEdit || ceEdit; const secondaryEdit = veEdit ? ceEdit : null; const disableStickyHeader = () => { document.body.classList.remove( STICKY_HEADER_VISIBLE_CLASS ); @@ -434,6 +440,7 @@ function isAllowedAction( action ) { * @property {Element} userMenu * @property {IntersectionObserver} observer * @property {Element} stickyIntersection + * @property {boolean} disableEditIcons */ /** @@ -449,7 +456,8 @@ function initStickyHeader( props ) { props.userMenu, userMenuStickyContainer, props.observer, - props.stickyIntersection + props.stickyIntersection, + props.disableEditIcons ); setupSearchIfNeeded( props.header ); @@ -486,5 +494,6 @@ module.exports = { STICKY_HEADER_ID, FIRST_HEADING_ID, USER_MENU_ID, + STICKY_HEADER_EDIT_EXPERIMENT_NAME, STICKY_HEADER_EXPERIMENT_NAME }; diff --git a/resources/skins.vector.es6/stickyHeaderAB.js b/resources/skins.vector.es6/stickyHeaderAB.js new file mode 100644 index 00000000..e69de29b