From 864a1766a7e547c4a75f64ee4fe730b47fb885d9 Mon Sep 17 00:00:00 2001 From: Jan Drewniak Date: Mon, 25 Feb 2019 20:50:56 +0100 Subject: [PATCH] Refactor pageActions menu to accommodate AMC mode This patch refactors the markup and JS associated with the page actions menu in order to achieve greater flexibility in its presentation. The menu items are now positioned via flexbox and rendered using a mustache template in PHP. The goal of this refactor is to accommodate both AMC mode and default mode with the same markup. No changes should be visible for non-AMC users with this refactor. No changes to AMC mode have been made in this patch either. This patch includes temporary workarounds to avoid problems caused by HTML caching. Changes include: - Changing the data structure of the page_actions property in SkinMinerva.php - Passing that modified data structure into a new mustache template, PageActionMenu.mustache - Adding new CSS for the new page actions menu HTML - changing the query selectors in JS to match the new markup - Making the JS-modified page-actions compatible with the new markup - Keeping existing CSS and JS to avoid breaking cached HTML Bug: T213352 Depends-On: I95cf726c4b6d8c3895a26aa6e07f4b1747ee30fe Change-Id: I5a7d73b20617cb3c6d6379084ac4bea23ec3bc74 --- includes/skins/MinervaTemplate.php | 18 +---- includes/skins/SkinMinerva.php | 73 +++++++++---------- includes/skins/pageActionMenu.mustache | 11 +++ .../pageactions.less | 67 ++++++++++++++++- .../downloadPageAction.js | 14 +++- resources/skins.minerva.scripts/init.js | 17 +++-- 6 files changed, 137 insertions(+), 63 deletions(-) create mode 100644 includes/skins/pageActionMenu.mustache diff --git a/includes/skins/MinervaTemplate.php b/includes/skins/MinervaTemplate.php index e0dcd92..0481488 100644 --- a/includes/skins/MinervaTemplate.php +++ b/includes/skins/MinervaTemplate.php @@ -92,25 +92,15 @@ class MinervaTemplate extends BaseTemplate { /** * Get the HTML for rendering the available page actions - * @param array $data Data used to build page actions * @return string */ - protected function getPageActionsHtml( $data ) { + protected function getPageActionsHtml() { + $templateParser = new TemplateParser( __DIR__ ); $actions = $this->getPageActions(); $html = ''; - $isJSOnly = true; + if ( $actions ) { - foreach ( $actions as $key => $val ) { - if ( isset( $val['is_js_only'] ) ) { - if ( !$val['is_js_only'] ) { - $isJSOnly = false; - } - unset( $val['is_js_only'] ); // no need to output this attribute - } - $html .= $this->makeListItem( $key, $val ); - } - $additionalClasses = $isJSOnly ? 'jsonly' : ''; - $html = ''; + $html = $templateParser->processTemplate( 'pageActionMenu', [ 'pageactions' => $actions ] ); } return $html; } diff --git a/includes/skins/SkinMinerva.php b/includes/skins/SkinMinerva.php index 348b55f..445115f 100644 --- a/includes/skins/SkinMinerva.php +++ b/includes/skins/SkinMinerva.php @@ -1137,18 +1137,17 @@ class SkinMinerva extends SkinTemplate { /** * Prepare configured and available page actions * - * When adding new page actions make sure each menu item has - * is_js_only key set to true or false. - * The key will be used to decide whether to display the page actions - * wrapper on the front end. The key will be considered false if not set. + * If a page action should display a placeholder element + * (i.e. it will be hydrated on the client with JS) add the 'jsonly' CSS class to + * the 'class' key of its array. * * @param BaseTemplate $tpl */ protected function preparePageActions( BaseTemplate $tpl ) { $menu = []; - if ( $this->isAllowedPageAction( 'edit' ) ) { - $menu['edit'] = $this->createEditPageAction(); + if ( $this->isAllowedPageAction( 'switch-language' ) ) { + $menu[] = $this->createSwitchLanguageAction(); } if ( $this->isAllowedPageAction( 'watch' ) ) { @@ -1156,11 +1155,11 @@ class SkinMinerva extends SkinTemplate { // Pass these actions in as context for #createWatchPageAction. $actions = $tpl->data['content_navigation']['actions']; - $menu['watch'] = $this->createWatchPageAction( $actions ); + $menu[] = $this->createWatchPageAction( $actions ); } - if ( $this->isAllowedPageAction( 'switch-language' ) ) { - $menu['switch-language'] = $this->createSwitchLanguageAction(); + if ( $this->isAllowedPageAction( 'edit' ) ) { + $menu[] = $this->createEditPageAction(); } $tpl->set( 'page_actions', $menu ); @@ -1170,7 +1169,8 @@ class SkinMinerva extends SkinTemplate { * Creates the "edit" page action: the well-known pencil icon that, when tapped, will open an * editor with the lead section loaded. * - * @return array A map compatible with BaseTemplate#makeListItem + * @return array A map of HTML attributes and a "text" property to be used with the + * pageActionMenu.mustache template. */ protected function createEditPageAction() { $title = $this->getTitle(); @@ -1187,18 +1187,12 @@ class SkinMinerva extends SkinTemplate { $userCanEdit = $userQuickEditCheck && !$userBlockInfo; return [ - 'id' => 'ca-edit', - 'text' => '', - 'itemtitle' => $this->msg( 'mobile-frontend-pageaction-edit-tooltip' ), - 'class' => MinervaUI::iconClass( $userCanEdit ? 'edit-enabled' : 'edit', 'element' ), - 'links' => [ - 'edit' => [ - 'href' => $title->getLocalURL( $editArgs ), - 'msg' => 'mobile-frontend-editor-edit', - 'class' => 'edit-page', - ], - ], - 'is_js_only' => false, + 'id' => 'ca-edit', + 'href' => $title->getLocalURL( $editArgs ), + 'class' => 'edit-page ' + . MinervaUI::iconClass( $userCanEdit ? 'edit-enabled' : 'edit', 'element' ), + 'title' => $this->msg( 'mobile-frontend-pageaction-edit-tooltip' ), + 'text' => $this->msg( 'mobile-frontend-editor-edit' ) ]; } @@ -1208,7 +1202,8 @@ class SkinMinerva extends SkinTemplate { * will direct the user's UA to Special:Login. * * @param array $actions - * @return array A map compatible with BaseTemplate#makeListItem + * @return array A map of HTML attributes and a "text" property to be used with the + * pageActionMenu.mustache template. */ protected function createWatchPageAction( $actions ) { $title = $this->getTitle(); @@ -1222,8 +1217,9 @@ class SkinMinerva extends SkinTemplate { $baseResult = [ 'id' => 'ca-watch', // Use blank icon to reserve space for watchstar icon once JS loads - 'class' => MinervaUI::iconClass( $icon, 'element', 'watch-this-article' ), - 'is_js_only' => true + 'class' => MinervaUI::iconClass( $icon, 'element', 'watch-this-article' ) . ' jsonly', + 'title' => $this->msg( 'watchthispage' ), + 'text' => $this->msg( 'watchthispage' ) ]; if ( isset( $actions['watch'] ) ) { @@ -1231,11 +1227,10 @@ class SkinMinerva extends SkinTemplate { } elseif ( isset( $actions['unwatch'] ) ) { $result = array_merge( $actions['unwatch'], $baseResult ); $result['class'] .= ' watched'; + $result[ 'text' ] = $this->msg( 'unwatchthispage' ); } else { // placeholder for not logged in $result = array_merge( $baseResult, [ - // FIXME: makeLink (used by makeListItem) when no text is present defaults to use the key - 'text' => '', 'href' => $ctaUrl, ] ); } @@ -1247,26 +1242,26 @@ class SkinMinerva extends SkinTemplate { * Creates the "switch-language" action: the icon that, when tapped, opens the language * switcher. * - * @return array A map compatible with BaseTemplate#makeListItem + * @return array A map of HTML attributes and a 'text' property to be used with the + * pageActionMenu.mustache template. */ protected function createSwitchLanguageAction() { - $languageSwitcherLinks = []; - $languageSwitcherClasses = 'language-selector'; + $languageSwitcherLink = false; + $languageSwitcherClasses = ' language-selector'; if ( $this->doesPageHaveLanguages ) { - $languageSwitcherLinks['mobile-frontend-language-article-heading'] = [ - 'href' => SpecialPage::getTitleFor( 'MobileLanguages', $this->getTitle() )->getLocalURL() - ]; + $languageSwitcherLink = SpecialPage::getTitleFor( + 'MobileLanguages', + $this->getTitle() + )->getLocalURL(); } else { $languageSwitcherClasses .= ' disabled'; } - return [ - 'text' => '', - 'itemtitle' => $this->msg( 'mobile-frontend-language-article-heading' ), - 'class' => MinervaUI::iconClass( 'language-switcher', 'element', $languageSwitcherClasses ), - 'links' => $languageSwitcherLinks, - 'is_js_only' => false + 'class' => MinervaUI::iconClass( 'language-switcher', 'element', $languageSwitcherClasses ), + 'href' => $languageSwitcherLink, + 'title' => $this->msg( 'mobile-frontend-language-article-heading' ), + 'text' => $this->msg( 'mobile-frontend-language-article-heading' ) ]; } diff --git a/includes/skins/pageActionMenu.mustache b/includes/skins/pageActionMenu.mustache new file mode 100644 index 0000000..d4efa50 --- /dev/null +++ b/includes/skins/pageActionMenu.mustache @@ -0,0 +1,11 @@ + diff --git a/resources/skins.minerva.base.styles/pageactions.less b/resources/skins.minerva.base.styles/pageactions.less index 61f18a7..79f3e9d 100644 --- a/resources/skins.minerva.base.styles/pageactions.less +++ b/resources/skins.minerva.base.styles/pageactions.less @@ -15,7 +15,70 @@ margin-bottom: 12px; } -#page-actions { +// used to disable the languages icon. +.mw-ui-icon-element.disabled { + cursor: default; + opacity: 0.25; +} + +.page-actions-menu { + box-sizing: border-box; + border-top: 1px solid @colorGray14; + border-bottom: 1px solid @colorGray12; +} + +.page-actions-menu__list { + display: flex; + justify-content: space-between; + height: 44px; // total height is 46px. 2px added by border on .page-actions-menu +} + +.page-actions-menu__list-item { + position: relative; + display: flex; + flex-basis: 4em; + justify-content: flex-end; + align-items: center; + + // overriding default icon styles + .mw-ui-icon-element { + // The page actions menu icons are ever so slightly larger + // than standard icons. + @pageActionsIconSize: @iconSize + 0.15; + position: relative; + min-width: @pageActionsIconSize; + width: @pageActionsIconSize; + height: @pageActionsIconSize; + + &:hover { + box-shadow: none; + } + + &:before { + margin: 0; + // `.mw-ui-icon` absolutely positions this pseudo-element but only + // positions right & left. This ensures icon stretches 100% height and + // stretches the entire height of its parent element. + top: 0; + bottom: 0; + } + } +} + +// default layout - spacing out the first menu item +.page-actions-menu__list-item:first-child { + flex-grow: 1; + justify-content: flex-start; +} + +// overriding common.less `display:inherit` (which causes `display: flex;` in this instance). +.client-js .jsonly#ca-watch { + display: list-item; +} + +// TODO: T213352 Delete this nested block after varnish cache has cleared and selectors +// no longer apply. +#page-actions:not( .page-actions-menu__list ) { font-size: @pageActionFontSize; float: none; border: 0; @@ -70,7 +133,7 @@ // FIXME: cached HTML. Can be removed when work on T212216 // has been deployed and varnish cache cleared. -.heading-holder #page-actions:first-child { +.heading-holder #page-actions:not( .page-actions-menu__list ):first-child { position: absolute; bottom: 0; } diff --git a/resources/skins.minerva.scripts/downloadPageAction.js b/resources/skins.minerva.scripts/downloadPageAction.js index cdd147a..87e7193 100644 --- a/resources/skins.minerva.scripts/downloadPageAction.js +++ b/resources/skins.minerva.scripts/downloadPageAction.js @@ -125,7 +125,12 @@ * @returns {jQuery.Object|null} */ function downloadPageAction( skin, supportedNamespaces, windowObj ) { - var icon, spinner = icons.spinner(); + var icon, spinner = icons.spinner(), + // TODO: T213352 Temporary cache compatibility - to be deleted. + // Any conditionals using this boolean should be DELETED when the + // old page action menu is no longer being served to users. + // eslint-disable-next-line jquery/no-global-selector + oldPageActionsDOM = $( '#page-actions.hlist' ).length > 0; if ( isAvailable( windowObj, skin.page, navigator.userAgent, @@ -136,12 +141,17 @@ glyphPrefix: 'minerva', title: msg( 'minerva-download' ), name: GLYPH, + tagName: oldPageActionsDOM ? 'div' : 'button', events: { // will be bound to `this` click: getOnClickHandler( skin, spinner ) } } ); - return $( '
  • ' ).append( icon.$el ).append( spinner.$el.hide() ); + if ( oldPageActionsDOM ) { + return $( '
  • ' ).append( icon.$el ).append( spinner.$el.hide() ); + } else { + return $( '
  • ' ).addClass( 'page-actions-menu__list-item' ).append( icon.$el ).append( spinner.$el.hide() ); + } } else { return null; } diff --git a/resources/skins.minerva.scripts/init.js b/resources/skins.minerva.scripts/init.js index 70271c9..09fab7f 100644 --- a/resources/skins.minerva.scripts/init.js +++ b/resources/skins.minerva.scripts/init.js @@ -236,14 +236,19 @@ */ function appendDownloadButton() { var $downloadAction = downloadPageAction( skin, - config.get( 'wgMinervaDownloadNamespaces', [] ), window ); + config.get( 'wgMinervaDownloadNamespaces', [] ), window ), + // TODO: T213352 Temporary cache compatibility - to be deleted. + // Any conditionals using this boolean should be DELETED when the + // old page action menu is no longer being served to users. + // eslint-disable-next-line jquery/no-global-selector + oldPageActionsDOM = $( '#page-actions.hlist' ).length > 0; if ( $downloadAction ) { - // Because the page actions are floated to the right, their order in the - // DOM is reversed in the display. The watchstar is last in the DOM and - // left-most in the display. Since we want the download button to be to - // the left of the watchstar, we put it after it in the DOM. - $downloadAction.insertAfter( '#ca-watch' ); + if ( oldPageActionsDOM ) { + $downloadAction.insertAfter( '#ca-watch' ); + } else { + $downloadAction.insertAfter( '.page-actions-menu__list-item:first-child' ); + } track( 'minerva.downloadAsPDF', { action: 'buttonVisible' } );