From 86150d492dc9492c922fe202f240c6a48540662b Mon Sep 17 00:00:00 2001 From: alistair3149 Date: Thu, 12 May 2022 22:37:28 -0400 Subject: [PATCH] refactor(core): clean up checkboxHack implementation Mostly based on Vector 2022. Now we target checkbox hacks by the HTML classes .mw-checkbox-hack-TYPE instead of defining each individually, which would make it more scalable. --- resources/skins.citizen.scripts/search.js | 49 ++------ resources/skins.citizen.scripts/skin.js | 131 ++++++++------------ resources/skins.citizen.search/typeahead.js | 2 - templates/Drawer.mustache | 2 +- templates/PersonalMenu.mustache | 2 +- templates/Search.mustache | 2 +- 6 files changed, 70 insertions(+), 118 deletions(-) diff --git a/resources/skins.citizen.scripts/search.js b/resources/skins.citizen.scripts/search.js index a589c4d8..a5daddd3 100644 --- a/resources/skins.citizen.scripts/search.js +++ b/resources/skins.citizen.scripts/search.js @@ -136,53 +136,30 @@ function bindExpandOnSlash( window, checkbox, input ) { window.addEventListener( 'keydown', onExpandOnSlash, true ); } -/** - * @param {Window} window - * @param {HTMLInputElement} input - * @param {HTMLElement} target - * @return {void} - */ -function initCheckboxHack( window, input, target ) { - const checkboxHack = require( './checkboxHack.js' ), - button = document.getElementById( 'citizen-search__buttonCheckbox' ), - checkbox = document.getElementById( 'citizen-search__checkbox' ); - - if ( checkbox instanceof HTMLInputElement && button ) { - checkboxHack.bindToggleOnClick( checkbox, button ); - checkboxHack.bindUpdateAriaExpandedOnInput( checkbox ); - checkboxHack.updateAriaExpanded( checkbox ); - checkboxHack.bindToggleOnEnter( checkbox ); - checkboxHack.bindDismissOnClickOutside( window, checkbox, button, target ); - checkboxHack.bindDismissOnFocusLoss( window, checkbox, button, target ); - checkboxHack.bindDismissOnEscape( window, checkbox ); - - bindExpandOnSlash( window, checkbox, input ); - - // Focus when toggled - checkbox.addEventListener( 'input', () => { - focusOnChecked( checkbox, input ); - } ); - } -} - /** * @param {Window} window * @return {void} */ function initSearch( window ) { const searchConfig = require( './config.json' ).wgCitizenEnableSearch, - searchForm = document.getElementById( 'searchform' ), - searchInput = document.getElementById( SEARCH_INPUT_ID ); + checkbox = document.getElementById( 'citizen-search__checkbox' ), + form = document.getElementById( 'searchform' ), + input = document.getElementById( SEARCH_INPUT_ID ); - initCheckboxHack( window, searchInput, searchForm ); + bindExpandOnSlash( window, checkbox, input ); + + // Focus when toggled + checkbox.addEventListener( 'input', () => { + focusOnChecked( checkbox, input ); + } ); if ( searchConfig ) { - setLoadingIndicatorListeners( searchForm, true, renderSearchLoadingIndicator ); - loadSearchModule( searchInput, 'skins.citizen.search', () => { - setLoadingIndicatorListeners( searchForm, false, renderSearchLoadingIndicator ); + setLoadingIndicatorListeners( form, true, renderSearchLoadingIndicator ); + loadSearchModule( input, 'skins.citizen.search', () => { + setLoadingIndicatorListeners( form, false, renderSearchLoadingIndicator ); } ); } else { - loadSearchModule( searchInput, 'mediawiki.searchSuggest', () => {} ); + loadSearchModule( input, 'mediawiki.searchSuggest', () => {} ); } } diff --git a/resources/skins.citizen.scripts/skin.js b/resources/skins.citizen.scripts/skin.js index 881435d2..936c9900 100644 --- a/resources/skins.citizen.scripts/skin.js +++ b/resources/skins.citizen.scripts/skin.js @@ -1,29 +1,14 @@ +const + checkboxHack = require( './checkboxHack.js' ), + CHECKBOX_HACK_CONTAINER_SELECTOR = '.mw-checkbox-hack-container', + CHECKBOX_HACK_CHECKBOX_SELECTOR = '.mw-checkbox-hack-checkbox', + CHECKBOX_HACK_BUTTON_SELECTOR = '.mw-checkbox-hack-button', + CHECKBOX_HACK_TARGET_SELECTOR = '.mw-checkbox-hack-target'; + /** - * Based on Vector - * Wait for first paint before calling this function. That's its whole purpose. - * - * Some CSS animations and transitions are "disabled" by default as a workaround to this old Chrome - * bug, https://bugs.chromium.org/p/chromium/issues/detail?id=332189, which otherwise causes them to - * render in their terminal state on page load. By adding the `vector-animations-ready` class to the - * `html` root element **after** first paint, the animation selectors suddenly match causing the - * animations to become "enabled" when they will work properly. A similar pattern is used in Minerva + * Wait for first paint before calling this function. * (see T234570#5779890, T246419). * - * Example usage in Less: - * - * ```less - * .foo { - * color: #f00; - * .transform( translateX( -100% ) ); - * } - * - * // This transition will be disabled initially for JavaScript users. It will never be enabled for - * // no-JS users. - * .citizen-animations-ready .foo { - * .transition( transform 100ms ease-out; ); - * } - * ``` - * * @param {Document} document * @return {void} */ @@ -32,57 +17,42 @@ function enableCssAnimations( document ) { } /** - * Initialize checkboxHacks - * TODO: Maybe ToC should init checkboxHack in its own RL module? + * Add the ability for users to toggle dropdown menus using the enter key (as + * well as space) using core's checkboxHack. * - * @param {Window} window - * @return {void} + * Based on Vector */ -function initCheckboxHack( window ) { - const checkboxHack = require( './checkboxHack.js' ), - drawer = { - button: document.getElementById( 'citizen-drawer__buttonCheckbox' ), - checkbox: document.getElementById( 'citizen-drawer__checkbox' ), - target: document.getElementById( 'citizen-drawer__card' ) - }, - personalMenu = { - button: document.getElementById( 'citizen-personalMenu__buttonCheckbox' ), - checkbox: document.getElementById( 'citizen-personalMenu__checkbox' ), - target: document.getElementById( 'citizen-personalMenu__card' ) - }, - checkboxObjs = [ drawer, personalMenu ]; +function bind() { + // Search for all dropdown containers using the CHECKBOX_HACK_CONTAINER_SELECTOR. + const containers = document.querySelectorAll( CHECKBOX_HACK_CONTAINER_SELECTOR ); - // This should be in ToC script - // And the media query needs to be synced with the less variable - // Also this does not monitor screen size changes - if ( document.querySelector( '.citizen-toc-enabled' ) && - window.matchMedia( 'screen and (max-width: 1300px)' ) ) { - const tocContainer = document.getElementById( 'toc' ); - if ( tocContainer ) { - const toc = { - button: tocContainer.querySelector( '.toctogglelabel' ), - checkbox: document.getElementById( 'toctogglecheckbox' ), - target: tocContainer.querySelector( 'ul' ) - }; - checkboxObjs.push( toc ); - } - } + containers.forEach( ( container ) => { + const + checkbox = container.querySelector( CHECKBOX_HACK_CHECKBOX_SELECTOR ), + button = container.querySelector( CHECKBOX_HACK_BUTTON_SELECTOR ), + target = container.querySelector( CHECKBOX_HACK_TARGET_SELECTOR ); - checkboxObjs.forEach( ( checkboxObj ) => { - if ( - checkboxObj.checkbox instanceof HTMLInputElement && - checkboxObj.button && - checkboxObj.target - ) { - checkboxHack.bindToggleOnClick( checkboxObj.checkbox, checkboxObj.button ); - checkboxHack.bindUpdateAriaExpandedOnInput( checkboxObj.checkbox ); - checkboxHack.updateAriaExpanded( checkboxObj.checkbox ); - checkboxHack.bindToggleOnEnter( checkboxObj.checkbox ); - checkboxHack.bindDismissOnClickOutside( - window, checkboxObj.checkbox, checkboxObj.button, checkboxObj.target - ); - checkboxHack.bindDismissOnEscape( window, checkboxObj.checkbox ); + if ( !( checkbox && button && target ) ) { + return; } + + checkboxHack.bind( window, checkbox, button, target ); + } ); +} + +/** + * T295085: Close all dropdown menus when page is unloaded to prevent them from + * being open when navigating back to a page. + * + * Based on Vector + */ +function bindCloseOnUnload() { + addEventListener( 'beforeunload', function () { + const checkboxes = document.querySelectorAll( CHECKBOX_HACK_CHECKBOX_SELECTOR + ':checked' ); + + checkboxes.forEach( ( checkbox ) => { + /** @type {HTMLInputElement} */ ( checkbox ).checked = false; + } ); } ); } @@ -114,25 +84,32 @@ function onTitleHidden( document ) { function main( window ) { const theme = require( './theme.js' ), search = require( './search.js' ), - toc = require( './tableOfContents.js' ); + tocContainer = document.getElementById( 'toc' ); enableCssAnimations( window.document ); theme.init( window ); search.init( window ); - initCheckboxHack( window ); onTitleHidden( window.document ); window.addEventListener( 'beforeunload', () => { document.documentElement.classList.add( 'citizen-loading' ); }, false ); - // TODO: This need some serious refactoring - // * Have a function to define checkbox targets then pass it to init - // * initCheckboxHack needs should take such objects as parameter - // * We shouldn't get the toc element multitple times - // * Need to consolidate scroll and intersection handlers - if ( document.getElementById( 'toc' ) ) { + bind(); + bindCloseOnUnload(); + + // Handle ToC + // TODO: There must be a cleaner way to do this + if ( tocContainer ) { + const toc = require( './tableOfContents.js' ); toc.init(); + + checkboxHack.bind( + window, + document.getElementById( 'toctogglecheckbox' ), + tocContainer.querySelector( '.toctogglelabel' ), + tocContainer.querySelector( 'ul' ) + ); } mw.loader.load( 'skins.citizen.preferences' ); diff --git a/resources/skins.citizen.search/typeahead.js b/resources/skins.citizen.search/typeahead.js index d17445d2..c9a689d4 100644 --- a/resources/skins.citizen.search/typeahead.js +++ b/resources/skins.citizen.search/typeahead.js @@ -276,7 +276,6 @@ function initTypeahead( searchForm, input ) { const clickInside = typeahead.contains( event.relatedTarget ); if ( !clickInside ) { - searchForm.setAttribute( 'aria-expanded', 'false' ); searchInput.setAttribute( 'aria-activedescendant', '' ); /* eslint-disable-next-line mediawiki/class-doc */ typeahead.classList.remove( expandedClass ); @@ -288,7 +287,6 @@ function initTypeahead( searchForm, input ) { const onFocus = function () { // Refresh the typeahead since the query will be emptied when blurred updateTypeahead( messages ); - searchForm.setAttribute( 'aria-expanded', 'true' ); /* eslint-disable-next-line mediawiki/class-doc */ typeahead.classList.add( expandedClass ); searchInput.addEventListener( 'keydown', keyboardEvents ); diff --git a/templates/Drawer.mustache b/templates/Drawer.mustache index 86e08c11..a0b3f006 100644 --- a/templates/Drawer.mustache +++ b/templates/Drawer.mustache @@ -2,7 +2,7 @@ string msg-citizen-drawer-toggle The label used by the drawer button. string msg-sitesubtitle the contents of the sitesubtitle message key }} -
+
+
+