From c9cdaadb5ea0a0048c5f2d7286489d99d86eb7fd Mon Sep 17 00:00:00 2001 From: Jon Robson Date: Wed, 2 Nov 2022 16:00:04 -0700 Subject: [PATCH] [Technical] Separate Dropdown template into 3 templates Use the new format to remove the need for getTemplateParser in SkinVector Bug: T319349 Change-Id: Ic4ac1d6d58099a689c29c16b3029bf43a849e50e --- includes/SkinVector.php | 28 +++++++---------- includes/templates/Dropdown.mustache | 31 ++----------------- includes/templates/Dropdown/Close.mustache | 3 ++ includes/templates/Dropdown/Open.mustache | 29 +++++++++++++++++ includes/templates/UserLinks.mustache | 21 ++++++++++++- includes/templates/UserLinks__logout.mustache | 4 --- .../__snapshots__/stickyHeader.test.js.snap | 4 +-- .../jest/__snapshots__/userLinks.test.js.snap | 10 +++--- tests/jest/stickyHeader.test.js | 8 ++--- tests/jest/userLinksData.js | 11 +++++-- 10 files changed, 85 insertions(+), 64 deletions(-) create mode 100644 includes/templates/Dropdown/Close.mustache create mode 100644 includes/templates/Dropdown/Open.mustache delete mode 100644 includes/templates/UserLinks__logout.mustache diff --git a/includes/SkinVector.php b/includes/SkinVector.php index d19cceac8..06c1b674b 100644 --- a/includes/SkinVector.php +++ b/includes/SkinVector.php @@ -221,14 +221,14 @@ abstract class SkinVector extends SkinMustache { * @param bool $isTempUser * @param bool $includeLearnMoreLink Pass `true` to include the learn more * link in the menu for anon users. This param will be inert for temp users. - * @return string + * @return array */ - private function getAnonMenuBeforePortletHTML( + private function getAnonMenuBeforePortletData( $returnto, $useCombinedLoginLink, $isTempUser, $includeLearnMoreLink - ) { + ): array { $templateParser = $this->getTemplateParser(); $loginLinkData = array_merge( $this->buildLoginData( $returnto, $useCombinedLoginLink ), [ 'class' => [ 'vector-menu-content-item', 'vector-menu-content-item-login' ], @@ -240,8 +240,6 @@ abstract class SkinVector extends SkinMustache { 'data-anon-editor' => [] ]; - $templateName = $isTempUser ? 'UserLinks__templogin' : 'UserLinks__login'; - if ( !$isTempUser && $includeLearnMoreLink ) { $learnMoreLinkData = [ 'text' => $this->msg( 'vector-anon-user-menu-pages-learn' )->text(), @@ -255,7 +253,7 @@ abstract class SkinVector extends SkinMustache { ]; } - return $templateParser->processTemplate( $templateName, $templateData ); + return $templateData; } /** @@ -263,16 +261,11 @@ abstract class SkinVector extends SkinMustache { * after the menu itself. * @return string */ - private function getLogoutHTML() { + private function getLogoutHTML(): string { $logoutLinkData = array_merge( $this->buildLogoutLinkData(), [ 'class' => [ 'vector-menu-content-item', 'vector-menu-content-item-logout' ], ] ); - $logoutLinkData = Hooks::updateLinkData( $logoutLinkData ); - - $templateParser = $this->getTemplateParser(); - return $templateParser->processTemplate( 'UserLinks__logout', [ - 'htmlLogout' => $this->makeLink( 'logout', $logoutLinkData ) - ] ); + return $this->makeLink( 'logout', Hooks::updateLinkData( $logoutLinkData ) ); } /** @@ -296,7 +289,7 @@ abstract class SkinVector extends SkinMustache { unset( $userMenuOverflowData[ 'label' ] ); if ( $isAnon || $isTempUser ) { - $userMenuData[ 'html-before-portal' ] .= $this->getAnonMenuBeforePortletHTML( + $additionalData = $this->getAnonMenuBeforePortletData( $returnto, $useCombinedLoginLink, $isTempUser, @@ -308,12 +301,13 @@ abstract class SkinVector extends SkinMustache { !$userMenuData['is-empty'] ); } else { - // Appending as to not override data potentially set by the onSkinAfterPortlet hook. - $userMenuData[ 'html-after-portal' ] .= $this->getLogoutHTML(); + $additionalData = []; } $moreItems = substr_count( $userMenuOverflowData['html-items'], ' $this->getLogoutHTML(), + 'is-temp-user' => $isTempUser, 'is-wide' => $moreItems > 3, 'data-user-menu-overflow' => $userMenuOverflowData, 'data-user-menu' => $userMenuData diff --git a/includes/templates/Dropdown.mustache b/includes/templates/Dropdown.mustache index 7214fd433..c5733168a 100644 --- a/includes/templates/Dropdown.mustache +++ b/includes/templates/Dropdown.mustache @@ -1,32 +1,5 @@ -{{! - See @typedef MenuDefinition -}} -
- {{! - Dropdown menus use the checkbox hack and require `input` and `label` elements. - `aria-label` is applied to the `input` which is semantically a button. - The `label` element is used as a visual button. - }} - - -
+{{>Dropdown/Open}} {{{html-before-portal}}}
    {{{html-items}}}
{{{html-after-portal}}} -
-
+{{>Dropdown/Close}} diff --git a/includes/templates/Dropdown/Close.mustache b/includes/templates/Dropdown/Close.mustache new file mode 100644 index 000000000..2da2875af --- /dev/null +++ b/includes/templates/Dropdown/Close.mustache @@ -0,0 +1,3 @@ +{{! this template must be used with Dropdown/Open to avoid unclosed HTML tags }} + + \ No newline at end of file diff --git a/includes/templates/Dropdown/Open.mustache b/includes/templates/Dropdown/Open.mustache new file mode 100644 index 000000000..ac7974906 --- /dev/null +++ b/includes/templates/Dropdown/Open.mustache @@ -0,0 +1,29 @@ +{{! + See @typedef MenuDefinition +}} +{{! this template must be used with Dropdown/Close to avoid unclosed HTML tags }} +
+ {{! + Dropdown menus use the checkbox hack and require `input` and `label` elements. + `aria-label` is applied to the `input` which is semantically a button. + The `label` element is used as a visual button. + }} + + +
+ diff --git a/includes/templates/UserLinks.mustache b/includes/templates/UserLinks.mustache index e91510b7e..d33e420f5 100644 --- a/includes/templates/UserLinks.mustache +++ b/includes/templates/UserLinks.mustache @@ -1,4 +1,23 @@ diff --git a/includes/templates/UserLinks__logout.mustache b/includes/templates/UserLinks__logout.mustache deleted file mode 100644 index aace29301..000000000 --- a/includes/templates/UserLinks__logout.mustache +++ /dev/null @@ -1,4 +0,0 @@ -{{!-- The #pt-logout ID is required for the AJAX enabled logout in mediawiki.page.ready to work.}} -
- {{{htmlLogout}}} -
diff --git a/tests/jest/__snapshots__/stickyHeader.test.js.snap b/tests/jest/__snapshots__/stickyHeader.test.js.snap index 44b9eed42..9a1bf4a2b 100644 --- a/tests/jest/__snapshots__/stickyHeader.test.js.snap +++ b/tests/jest/__snapshots__/stickyHeader.test.js.snap @@ -18,12 +18,12 @@ exports[`Sticky header renders 1`] = `
+
    -
    -
    +
    diff --git a/tests/jest/__snapshots__/userLinks.test.js.snap b/tests/jest/__snapshots__/userLinks.test.js.snap index e4a74d732..fa6acc74f 100644 --- a/tests/jest/__snapshots__/userLinks.test.js.snap +++ b/tests/jest/__snapshots__/userLinks.test.js.snap @@ -2,13 +2,14 @@ exports[`UserLinks renders 1`] = ` " + " `; diff --git a/tests/jest/stickyHeader.test.js b/tests/jest/stickyHeader.test.js index 4faf4793c..b8fc045ea 100644 --- a/tests/jest/stickyHeader.test.js +++ b/tests/jest/stickyHeader.test.js @@ -3,9 +3,8 @@ const fs = require( 'fs' ); const tocContainerTemplate = fs.readFileSync( 'includes/templates/TableOfContentsContainer.mustache', 'utf8' ); const stickyHeaderTemplate = fs.readFileSync( 'includes/templates/StickyHeader.mustache', 'utf8' ); const buttonTemplate = fs.readFileSync( 'includes/templates/Button.mustache', 'utf8' ); -const dropdownTemplate = fs.readFileSync( 'includes/templates/Dropdown.mustache', 'utf8' ); const sticky = require( '../../resources/skins.vector.es6/stickyHeader.js' ); -const { userLinksHTML } = require( './userLinksData.js' ); +const { userLinksHTML, dropdownPartials } = require( './userLinksData.js' ); const defaultButtonsTemplateData = [ { href: '#', @@ -89,11 +88,10 @@ const templateData = { 'data-buttons': defaultButtonsTemplateData.concat( editButtonsTemplateData ) }; -const renderedHTML = mustache.render( stickyHeaderTemplate, templateData, { +const renderedHTML = mustache.render( stickyHeaderTemplate, templateData, Object.assign( {}, dropdownPartials, { Button: buttonTemplate, - Dropdown: dropdownTemplate, SearchBox: '
    ' // ignore SearchBox for this test -} ); +} ) ); beforeEach( () => { document.body.innerHTML = renderedHTML; diff --git a/tests/jest/userLinksData.js b/tests/jest/userLinksData.js index 9e2a1905c..e9c0cee27 100644 --- a/tests/jest/userLinksData.js +++ b/tests/jest/userLinksData.js @@ -2,6 +2,8 @@ const mustache = require( 'mustache' ); const fs = require( 'fs' ); const userLinksTemplate = fs.readFileSync( 'includes/templates/UserLinks.mustache', 'utf8' ); const dropdownTemplate = fs.readFileSync( 'includes/templates/Dropdown.mustache', 'utf8' ); +const dropdownOpenTemplate = fs.readFileSync( 'includes/templates/Dropdown/Open.mustache', 'utf8' ); +const dropdownCloseTemplate = fs.readFileSync( 'includes/templates/Dropdown/Close.mustache', 'utf8' ); const templateData = { 'is-wide': false, @@ -35,10 +37,15 @@ const templateData = { } }; -const renderedHTML = mustache.render( userLinksTemplate, templateData, { +const dropdownPartials = { + 'Dropdown/Open': dropdownOpenTemplate, + 'Dropdown/Close': dropdownCloseTemplate, Dropdown: dropdownTemplate -} ); +}; + +const renderedHTML = mustache.render( userLinksTemplate, templateData, dropdownPartials ); module.exports = { + dropdownPartials, userLinksHTML: renderedHTML };