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

@@ -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 = '<ul id="page-actions" class="hlist ' . $additionalClasses . '">' . $html . '</ul>';
+			$html = $templateParser->processTemplate( 'pageActionMenu',  [ 'pageactions' => $actions ] );
 		}
 		return $html;
 	}

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
-	 * <code>is_js_only</code> key set to <code>true</code> or <code>false</code>.
-	 * 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' )
 		];
 	}
 

includes/skins/pageActionMenu.mustache

@@ -0,0 +1,11 @@
+<nav class="page-actions-menu">
+	<ul id="page-actions" class="page-actions-menu__list">
+		{{#pageactions}}
+		<li class="page-actions-menu__list-item">
+			<a id="{{id}}" href="{{href}}" class="{{class}}" role="button" title="{{title}}">
+				{{text}}
+			</a>
+		</li>
+		{{/pageactions}}
+	</ul>
+</nav>

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;
 }

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 $( '<li>' ).append( icon.$el ).append( spinner.$el.hide() );
+			if ( oldPageActionsDOM ) {
+				return $( '<li>' ).append( icon.$el ).append( spinner.$el.hide() );
+			} else {
+				return $( '<li>' ).addClass( 'page-actions-menu__list-item' ).append( icon.$el ).append( spinner.$el.hide() );
+			}
 		} else {
 			return null;
 		}

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'
 			} );