MainMenu is a controller not a View and server rendered

As Stephen pointed out somewhere, this is a bit of a micro-optimisation
Let's simplify this code by always rendering it in the HTML. MainMenu.js
as a result becomes a controller that just decides when to show it.

The geolocation check for Nearby is removed given the fact that all
grade A browsers for mediawiki have Geolocation support

Additional changes
* Browser support suggests "animations" class is redundant now
* `open` event no longer filed - not being used anywhere
* Transparent shield is now managed by the MainMenu controller not
the skin (which was confusing)
* Test geolocation using a simple feature tests
rather than abstracting it away inside Browser
* The main menu button is always hidden under either a translucent shield
and/or the main menu itself when it has been opened
so so it's not possible to ever click it while the menu is open
 - the click handler is thus simplified
removing a check for the class of the button

Depends-On: I7fd243366cceae780bd46e1aef2c08dae073f647
Change-Id: I3892afb5ed3df628e2845043cf3bbc22a9928921

includes/skins/MinervaTemplate.php

@@ -212,22 +212,16 @@ class MinervaTemplate extends BaseTemplate {
 	}
 
 	/**
-	 * Gets the main menu only on Special:MobileMenu.
-	 * On other pages the menu is rendered via JS.
+	 * Gets the main menu HTML.
 	 * @param array $data Data used to build the page
 	 * @return string
 	 */
 	protected function getMainMenuHtml( $data ) {
-		if ( $this->isSpecialMobileMenuPage ) {
-			$templateParser = new TemplateParser(
-				__DIR__ . '/../../resources/skins.minerva.scripts/menu/' );
+		$templateParser = new TemplateParser( __DIR__ );
 
 		return $templateParser->processTemplate( 'menu', $data['mainMenu']['items'] );
 	}
 
-		return '';
-	}
-
 	/**
 	 * Render the entire page
 	 * @param array $data Data used to build the page

includes/skins/SkinMinerva.php

@@ -790,8 +790,6 @@ class SkinMinerva extends SkinTemplate {
 			],
 			'wgMinervaFeatures' => $this->skinOptions->getAll(),
 			'wgMinervaDownloadNamespaces' => $this->getConfig()->get( 'MinervaDownloadNamespaces' ),
-			// hamburger icon is already rendered, pass only menu items
-			'wgMinervaMenuData' => $menuData['items']
 		];
 
 		return $vars;

includes/skins/minerva.mustache

@@ -1,9 +1,11 @@
 {{{headelement}}}
 <div id="mw-mf-viewport">
-	<nav id="mw-mf-page-left" class="navigation-drawer view-border-box">
+	<nav id="mw-mf-page-left" class="mw-mf-viewport__nav-placeholder navigation-drawer view-border-box">
 		{{{mainmenuhtml}}}
 	</nav>
 	<div id="mw-mf-page-center">
+		<!-- transparent-shield can be removed when Drawer updated -->
+		<a class="mw-mf-page-center__mask transparent-shield" href="#"></a>
 		<header class="header-container header-chrome">
 			<form class="header" action="{{wgScript}}" method="get">
 				<div>{{{menuButton}}}</div>

resources/skins.minerva.base.styles/ui.less

@@ -53,6 +53,7 @@ main {
 	}
 }
 
+.mw-mf-viewport__nav-placeholder,
 // if footer has last modified line hide it (T173545)
 #footer-info-lastmod {
 	display: none;
@@ -244,15 +245,6 @@ input.search {
 	}
 }
 
-.transparent-shield {
-	position: absolute;
-	background: @semiTransparent;
-	z-index: @z-indexAboveContent;
-	// don't use display: none because it's not animatable
-	visibility: hidden;
-	.transition( opacity 0.25s ease-in-out );
-}
-
 // It may be better to express these in a single class
 // or think about using extend or a mixin to stay with semantic selectors
 // https://css-tricks.com/the-extend-concept/

resources/skins.minerva.mainMenu.styles/MainMenu.less

@@ -12,6 +12,7 @@
 	left: 0;
 	bottom: 0;
 	min-width: 275px;
+	.transition( transform @menu-animation-duration @menu-animation-easing; );
 
 	@media screen and ( min-width: @width-breakpoint-tablet ) {
 		min-width: @width-breakpoint-mobile;
@@ -33,25 +34,11 @@
 	overflow: hidden;
 	// Necessary to disable scrolling on Android and iOS devices.
 	touch-action: none;
-}
 
-.animations {
-	// .menu
-	#mw-mf-page-left {
-		.transition( transform @menu-animation-duration @menu-animation-easing; );
-	}
-
-	&.primary-navigation-enabled {
 	// .menu
 	#mw-mf-page-left {
 		.transform( translate( 0, 0 ) );
 	}
-
-		.transparent-shield {
-			visibility: visible;
-			opacity: 0.5;
-		}
-	}
 }
 
 // Special:MobileMenu

resources/skins.minerva.mainMenu.styles/NotificationsOverlay.less

@@ -45,10 +45,3 @@
 		}
 	}
 }
-
-.secondary-navigation-enabled {
-	.transparent-shield {
-		visibility: visible;
-		opacity: 0.5;
-	}
-}

resources/skins.minerva.scripts/menu.js

@@ -12,20 +12,36 @@ var MainMenu = require( './menu/MainMenu.js' ),
  * @ignore
  */
 function createMainMenu() {
-	var options = mw.config.get( 'wgMinervaMenuData', {} );
-
-	options.activator = '.header .main-menu-button';
-
-	return new MainMenu( options );
+	return new MainMenu( {
+		activator: '.header .main-menu-button'
+	} );
 }
 
-$( function () {
-	// eslint-disable-next-line no-jquery/no-global-selector
-	if ( !$( '#mw-mf-page-left' ).find( '.menu' ).length ) {
-		// Now we have a main menu button register it.
+/**
+ * Wire up the main menu
+ */
+function init() {
 	mainMenu.registerClickEvents();
-		mainMenu.appendTo( '#mw-mf-page-left' );
+	/**
+	 * Close navigation if skin is tapped
+	 * @param {JQuery.Event} ev
+	 * @private
+	 */
+	function onSkinClick( ev ) {
+		mainMenu.closeNavigationDrawers();
+		ev.preventDefault();
 	}
-} );
+	// FIXME: This is for cached HTML and can be removed shortly.
+	// Ref: I3892afb5ed3df628e2845043cf3bbc22a9928921.
+	// eslint-disable-next-line no-jquery/no-global-selector
+	if ( $( '.mw-mf-page-center__mask' ).length === 0 ) {
+		$( '<a>' ).addClass( 'mw-mf-page-center__mask' ).prependTo( '#mw-mf-page-center' );
+	}
+	// eslint-disable-next-line no-jquery/no-global-selector
+	$( '.mw-mf-page-center__mask' ).on( 'click', onSkinClick );
+}
 
-module.exports = mainMenu;
+module.exports = {
+	mainMenu: mainMenu,
+	init: init
+};

resources/skins.minerva.scripts/menu/MainMenu.js

@@ -1,51 +1,23 @@
-( function ( M ) {
-	var
-		mobile = M.require( 'mobile.startup' ),
-		mfExtend = mobile.mfExtend,
-		browser = mobile.Browser.getSingleton(),
-		View = mobile.View;
-
+( function () {
 	/**
 	 * Representation of the main menu
 	 *
 	 * @class MainMenu
 	 * @extends View
 	 * @param {Object} options Configuration options
-	 */
-	function MainMenu( options ) {
-		this.activator = options.activator;
-		View.call( this, options );
-	}
-
-	mfExtend( MainMenu, View, {
-		isTemplateMode: true,
-		template: mw.template.get( 'skins.minerva.scripts', 'menu.mustache' ),
-		templatePartials: {
-			menuGroup: mw.template.get( 'skins.minerva.scripts', 'menuGroup.mustache' )
-		},
-
-		/**
-		 * @cfg {object} defaults Default options hash.
-		 * @cfg {string} defaults.activator selector for element that when clicked can open or
+	 * @param {string} options.activator selector for element that when clicked can open or
 	 *                                  close the menu
 	 */
-		defaults: {
-			activator: undefined
-		},
-
-		/**
-		 * Remove the nearby menu entry if the browser doesn't support geo location
-		 * @memberof MainMenu
-		 * @instance
-		 */
-		postRender: function () {
-			if ( !browser.supportsGeoLocation() ) {
-				this.$el.find( '.nearby' ).parent().remove();
-			}
+	function MainMenu( options ) {
+		// Remove `mw-mf-viewport__nav-placeholder` to signal the menu has been loaded
+		// eslint-disable-next-line no-jquery/no-global-selector
+		$( '#mw-mf-page-left' ).removeClass( 'mw-mf-viewport__nav-placeholder' );
+		this.activator = options.activator;
 
 		this.registerClickEvents();
-		},
+	}
 
+	MainMenu.prototype = {
 		/**
 		 * Registers events for opening and closing the main menu
 		 * @memberof MainMenu
@@ -58,11 +30,7 @@
 			$( this.activator )
 				.off( 'click' )
 				.on( 'click', function ( ev ) {
-					if ( self.isOpen() ) {
-						self.closeNavigationDrawers();
-					} else {
 					self.openNavigationDrawer();
-					}
 					ev.preventDefault();
 					// DO NOT USE stopPropagation or you'll break click tracking in WikimediaEvents
 				} );
@@ -109,12 +77,9 @@
 			$( document.body )
 				.toggleClass( 'navigation-enabled' )
 				.toggleClass( drawerType + '-navigation-enabled' );
-
-			this.emit( 'open' );
 		}
-	} );
+	};
 
 	module.exports = MainMenu;
 
-// eslint-disable-next-line no-restricted-properties
-}( mw.mobileFrontend ) );
+}() );

resources/skins.minerva.scripts/notifications.js

@@ -6,7 +6,8 @@ module.exports = function () {
 	var badge,
 		// eslint-disable-next-line no-restricted-properties
 		M = mw.mobileFrontend,
-		mainMenu = require( './menu.js' ),
+		menu = require( './menu.js' ),
+		mainMenu = menu.mainMenu,
 		router = require( 'mediawiki.router' ),
 		mobile = M.require( 'mobile.startup' ),
 		util = mobile.util,

resources/skins.minerva.scripts/preInit.js

@@ -9,29 +9,17 @@ module.exports = function () {
 	// eslint-disable-next-line no-restricted-properties
 	var M = mw.mobileFrontend,
 		mobile = M.require( 'mobile.startup' ),
-		skin = mobile.Skin.getSingleton(),
-		mainMenu = require( './menu.js' );
+		menus = require( './menu.js' );
 
-	/**
-	 * Close navigation if skin is tapped
-	 * @param {JQuery.Event} ev
-	 * @private
-	 */
-	function onSkinClick( ev ) {
-		var $target = $( ev.target );
+	// loads lazy loading images
+	mobile.Skin.getSingleton();
+	// remove transparent-shield
+	// FIXME: Remove when transparent-shield has been removed from Mobilefrontend Skin.js
+	// eslint-disable-next-line no-jquery/no-global-selector
+	$( '.transparent-shield:not( .mw-mf-page-center__mask )' ).remove();
 
-		// Make sure the menu is open and we are not clicking on the menu button
-		if (
-			mainMenu &&
-			mainMenu.isOpen() &&
-			// eslint-disable-next-line no-jquery/no-class-state
-			!$target.hasClass( 'main-menu-button' )
-		) {
-			mainMenu.closeNavigationDrawers();
-			ev.preventDefault();
-		}
-	}
-	skin.on( 'click', onSkinClick.bind( skin ) );
+	// setup main menu
+	menus.init();
 
 	( function ( wgRedirectedFrom ) {
 		// If the user has been redirected, then show them a toast message (see

resources/skins.minerva.scripts/styles.less

@@ -4,6 +4,31 @@
 
 @animationDuration: 0.3s;
 
+// transparent-shield class can be removed when removed from MobileFrontend Skin.js
+.transparent-shield,
+.mw-mf-page-center__mask {
+	position: absolute;
+	top: 0;
+	left: 0;
+	right: 0;
+	opacity: 0;
+	bottom: 0;
+	background: @semiTransparent;
+	z-index: @z-indexAboveContent;
+	// don't use display: none because it's not animatable
+	visibility: hidden;
+	.transition( opacity 0.25s ease-in-out );
+}
+
+.navigation-enabled {
+	// transparent-shield class can be removed when removed from MobileFrontend Skin.js
+	.transparent-shield,
+	.mw-mf-page-center__mask {
+		visibility: visible;
+		opacity: 0.5;
+	}
+}
+
 // Flip the arrow in table of contents when toggled
 .toctogglecheckbox:checked ~ .toctitle .mw-ui-icon:last-child {
 	&:before {

skin.json

@@ -567,9 +567,7 @@
 			],
 			"templates": {
 				"badge.mustache": "includes/skins/userNotifications.mustache",
-				"IssueNotice.mustache": "resources/skins.minerva.scripts/page-issues/overlay/IssueNotice.mustache",
-				"menu.mustache": "resources/skins.minerva.scripts/menu/menu.mustache",
-				"menuGroup.mustache": "resources/skins.minerva.scripts/menu/menuGroup.mustache"
+				"IssueNotice.mustache": "resources/skins.minerva.scripts/page-issues/overlay/IssueNotice.mustache"
 			},
 			"packageFiles": [
 				"resources/skins.minerva.scripts/init.js",

skinStyles/mobile.startup/skin.less

@@ -1,13 +1,3 @@
-// FIXME Should this be merged with .transparent-shield?
-.cloaked-element {
-	opacity: 0;
-	position: absolute;
-	top: 0;
-	left: 0;
-	right: 0;
-	bottom: 0;
-}
-
 .position-fixed {
 	// use !important to override more specific rules (e.g. in Overlay.less)
 	position: fixed !important;

tests/browser/features/support/pages/article_page.rb

@@ -148,7 +148,7 @@ class ArticlePage # rubocop:disable Metrics/ClassLength
   # loader
   element(:content_wrapper, 'main')
   div(:content, id: 'bodyContent')
-  div(:transparent_shield, css: '.transparent-shield')
+  a(:transparent_shield, css: '.mw-mf-page-center__mask')
   # secondary menu
   ## languages
   a(:switch_language_page_action, css: '#page-actions .language-selector')