From b047890a254f3904f58cec1deabb8a244346eade Mon Sep 17 00:00:00 2001
From: laurelfulford <laurelfulford@git.wordpress.org>
Date: Mon, 31 Dec 2018 17:48:08 +0000
Subject: [PATCH] Twenty Nineteen: Improve menu semantics and keyboard
 navigation.

The menu's original markup included some non-semantic tags and an unnecessary `tabindex` attribute that made it difficult to navigate via keyboard.

Props allancole, anevins, kjellr.
Fixes #45713.



git-svn-id: https://develop.svn.wordpress.org/trunk@44376 602fd350-edb4-49c9-b593-d223f7449a82
---
 .../twentynineteen/inc/template-functions.php | 22 +++++-----
 .../js/touch-keyboard-navigation.js           |  6 ++-
 .../navigation/_menu-main-navigation.scss     | 40 +++++++++++++++++++
 .../themes/twentynineteen/style-rtl.css       | 38 ++++++++++++++++++
 .../themes/twentynineteen/style.css           | 38 ++++++++++++++++++
 .../template-parts/header/site-branding.php   |  2 +-
 6 files changed, 132 insertions(+), 14 deletions(-)

diff --git a/src/wp-content/themes/twentynineteen/inc/template-functions.php b/src/wp-content/themes/twentynineteen/inc/template-functions.php
index d7baff327c..2dbd0c5eba 100644
--- a/src/wp-content/themes/twentynineteen/inc/template-functions.php
+++ b/src/wp-content/themes/twentynineteen/inc/template-functions.php
@@ -203,18 +203,18 @@ function twentynineteen_add_ellipses_to_nav( $nav_menu, $args ) {
 	if ( 'menu-1' === $args->theme_location ) :
 
 		$nav_menu .= '<div class="main-menu-more">';
-		$nav_menu .= '<ul class="main-menu" tabindex="0">';
+		$nav_menu .= '<ul class="main-menu">';
 		$nav_menu .= '<li class="menu-item menu-item-has-children">';
-		$nav_menu .= '<a href="#" class="screen-reader-text" aria-label="More" aria-haspopup="true" aria-expanded="false">' . esc_html__( 'More', 'twentynineteen' ) . '</a>';
-		$nav_menu .= '<span class="submenu-expand main-menu-more-toggle is-empty" tabindex="-1">';
+		$nav_menu .= '<button class="submenu-expand main-menu-more-toggle is-empty" tabindex="-1" aria-label="More" aria-haspopup="true" aria-expanded="false">';
+		$nav_menu .= '<span class="screen-reader-text">' . esc_html__( 'More', 'twentynineteen' ) . '</span>';
 		$nav_menu .= twentynineteen_get_icon_svg( 'arrow_drop_down_ellipsis' );
-		$nav_menu .= '</span>';
+		$nav_menu .= '</button>';
 		$nav_menu .= '<ul class="sub-menu hidden-links">';
 		$nav_menu .= '<li id="menu-item--1" class="mobile-parent-nav-menu-item menu-item--1">';
-		$nav_menu .= '<span class="menu-item-link-return">';
+		$nav_menu .= '<button class="menu-item-link-return">';
 		$nav_menu .= twentynineteen_get_icon_svg( 'chevron_left' );
 		$nav_menu .= esc_html__( 'Back', 'twentynineteen' );
-		$nav_menu .= '</span>';
+		$nav_menu .= '</button>';
 		$nav_menu .= '</li>';
 		$nav_menu .= '</ul>';
 		$nav_menu .= '</li>';
@@ -269,11 +269,11 @@ function twentynineteen_add_dropdown_icons( $output, $item, $depth, $args ) {
 		// Inject the keyboard_arrow_left SVG inside the parent nav menu item, and let the item link to the parent item.
 		// @todo Only do this for nested submenus? If on a first-level submenu, then really the link could be "#" since the desire is to remove the target entirely.
 		$link = sprintf(
-			'<span class="menu-item-link-return" tabindex="-1">%s',
+			'<button class="menu-item-link-return" tabindex="-1">%s',
 			twentynineteen_get_icon_svg( 'chevron_left', 24 )
 		);
 
-		// replace opening <a> with <span>
+		// replace opening <a> with <button>
 		$output = preg_replace(
 			'/<a\s.*?>/',
 			$link,
@@ -281,10 +281,10 @@ function twentynineteen_add_dropdown_icons( $output, $item, $depth, $args ) {
 			1 // Limit.
 		);
 
-		// replace closing </a> with </span>
+		// replace closing </a> with </button>
 		$output = preg_replace(
 			'#</a>#i',
-			'</span>',
+			'</button>',
 			$output,
 			1 // Limit.
 		);
@@ -295,7 +295,7 @@ function twentynineteen_add_dropdown_icons( $output, $item, $depth, $args ) {
 		$icon = twentynineteen_get_icon_svg( 'keyboard_arrow_down', 24 );
 
 		$output .= sprintf(
-			'<span class="submenu-expand" tabindex="-1">%s</span>',
+			'<button class="submenu-expand" tabindex="-1">%s</button>',
 			$icon
 		);
 	}
diff --git a/src/wp-content/themes/twentynineteen/js/touch-keyboard-navigation.js b/src/wp-content/themes/twentynineteen/js/touch-keyboard-navigation.js
index a466195c16..2fa1905624 100644
--- a/src/wp-content/themes/twentynineteen/js/touch-keyboard-navigation.js
+++ b/src/wp-content/themes/twentynineteen/js/touch-keyboard-navigation.js
@@ -112,7 +112,7 @@
 		currentSubMenu.parentElement.lastElementChild.className += ' expanded-true';
 
 		// Update aria-expanded state
-		toggleAriaExpandedState( currentSubMenu.previousSibling );
+		toggleAriaExpandedState( currentSubMenu );
 	}
 
 	/**
@@ -230,7 +230,8 @@
 				openSubMenu(event.target);
 
 			// Check if child of .submenu-expand is touched
-			} else if ( null != getCurrentParent( event.target, '.submenu-expand' ) && getCurrentParent( event.target, '.submenu-expand' ).matches( '.submenu-expand' ) ) {
+			} else if ( null != getCurrentParent( event.target, '.submenu-expand' ) &&
+								getCurrentParent( event.target, '.submenu-expand' ).matches( '.submenu-expand' ) ) {
 				openSubMenu( getCurrentParent( event.target, '.submenu-expand' ) );
 
 			// Check if .menu-item-link-return is touched
@@ -254,6 +255,7 @@
 			if ( null != mainNav && hasClass( mainNav, '.main-navigation' ) ) {
 				// Prevent default mouse events
 				event.preventDefault();
+
 			} else if (
 				event.target.matches('.submenu-expand') ||
 				null != getCurrentParent( event.target, '.submenu-expand' ) &&
diff --git a/src/wp-content/themes/twentynineteen/sass/navigation/_menu-main-navigation.scss b/src/wp-content/themes/twentynineteen/sass/navigation/_menu-main-navigation.scss
index 0e13b0e5dd..61d943a774 100644
--- a/src/wp-content/themes/twentynineteen/sass/navigation/_menu-main-navigation.scss
+++ b/src/wp-content/themes/twentynineteen/sass/navigation/_menu-main-navigation.scss
@@ -13,6 +13,39 @@
 		display: inline;
 	}
 
+	/* Un-style buttons */
+	button {
+		display: inline-block;
+		border: none;
+		padding: 0;
+		margin: 0;
+		font-family: $font__heading;
+		font-weight: 700;
+		line-height: $font__line-height-heading;
+		text-decoration: none;
+		background: transparent;
+		color: inherit;
+		cursor: pointer;
+		transition: background 250ms ease-in-out,
+		            transform 150ms ease;
+		-webkit-appearance: none;
+		-moz-appearance: none;
+
+		&:hover,
+		&:focus {
+			background: transparent;
+		}
+
+		&:focus {
+			outline: 1px solid transparent;
+			outline-offset: -4px;
+		}
+
+		&:active {
+			transform: scale(0.99);
+		}
+	}
+
 	.main-menu {
 
 		display: inline-block;
@@ -183,6 +216,13 @@
 				}
 			}
 
+			> .menu-item-link-return {
+				width: 100%;
+				font-size: $font__size_base;
+				font-weight: normal;
+				text-align: left;
+			}
+
 			> a:empty {
 				display: none;
 			}
diff --git a/src/wp-content/themes/twentynineteen/style-rtl.css b/src/wp-content/themes/twentynineteen/style-rtl.css
index f4569a3f30..d86d83bf28 100644
--- a/src/wp-content/themes/twentynineteen/style-rtl.css
+++ b/src/wp-content/themes/twentynineteen/style-rtl.css
@@ -1027,6 +1027,7 @@ a:focus {
 .main-navigation {
   display: block;
   margin-top: 0.25rem;
+  /* Un-style buttons */
   /*
 	 * Sub-menu styles
 	 *
@@ -1049,6 +1050,36 @@ body.page .main-navigation {
   display: inline;
 }
 
+.main-navigation button {
+  display: inline-block;
+  border: none;
+  padding: 0;
+  margin: 0;
+  font-family: -apple-system, BlinkMacSystemFont, "Segoe UI", "Roboto", "Oxygen", "Ubuntu", "Cantarell", "Fira Sans", "Droid Sans", "Helvetica Neue", sans-serif;
+  font-weight: 700;
+  line-height: 1.2;
+  text-decoration: none;
+  background: transparent;
+  color: inherit;
+  cursor: pointer;
+  transition: background 250ms ease-in-out, transform 150ms ease;
+  -webkit-appearance: none;
+  -moz-appearance: none;
+}
+
+.main-navigation button:hover, .main-navigation button:focus {
+  background: transparent;
+}
+
+.main-navigation button:focus {
+  outline: 1px solid transparent;
+  outline-offset: -4px;
+}
+
+.main-navigation button:active {
+  transform: scale(0.99);
+}
+
 .main-navigation .main-menu {
   display: inline-block;
   margin: 0;
@@ -1207,6 +1238,13 @@ body.page .main-navigation {
   background: #005177;
 }
 
+.main-navigation .sub-menu > li > .menu-item-link-return {
+  width: 100%;
+  font-size: 22px;
+  font-weight: normal;
+  text-align: right;
+}
+
 .main-navigation .sub-menu > li > a:empty {
   display: none;
 }
diff --git a/src/wp-content/themes/twentynineteen/style.css b/src/wp-content/themes/twentynineteen/style.css
index c10e8701b2..06c0717cee 100644
--- a/src/wp-content/themes/twentynineteen/style.css
+++ b/src/wp-content/themes/twentynineteen/style.css
@@ -1027,6 +1027,7 @@ a:focus {
 .main-navigation {
   display: block;
   margin-top: 0.25rem;
+  /* Un-style buttons */
   /*
 	 * Sub-menu styles
 	 *
@@ -1049,6 +1050,36 @@ body.page .main-navigation {
   display: inline;
 }
 
+.main-navigation button {
+  display: inline-block;
+  border: none;
+  padding: 0;
+  margin: 0;
+  font-family: -apple-system, BlinkMacSystemFont, "Segoe UI", "Roboto", "Oxygen", "Ubuntu", "Cantarell", "Fira Sans", "Droid Sans", "Helvetica Neue", sans-serif;
+  font-weight: 700;
+  line-height: 1.2;
+  text-decoration: none;
+  background: transparent;
+  color: inherit;
+  cursor: pointer;
+  transition: background 250ms ease-in-out, transform 150ms ease;
+  -webkit-appearance: none;
+  -moz-appearance: none;
+}
+
+.main-navigation button:hover, .main-navigation button:focus {
+  background: transparent;
+}
+
+.main-navigation button:focus {
+  outline: 1px solid transparent;
+  outline-offset: -4px;
+}
+
+.main-navigation button:active {
+  transform: scale(0.99);
+}
+
 .main-navigation .main-menu {
   display: inline-block;
   margin: 0;
@@ -1207,6 +1238,13 @@ body.page .main-navigation {
   background: #005177;
 }
 
+.main-navigation .sub-menu > li > .menu-item-link-return {
+  width: 100%;
+  font-size: 22px;
+  font-weight: normal;
+  text-align: left;
+}
+
 .main-navigation .sub-menu > li > a:empty {
   display: none;
 }
diff --git a/src/wp-content/themes/twentynineteen/template-parts/header/site-branding.php b/src/wp-content/themes/twentynineteen/template-parts/header/site-branding.php
index e1a5b823d9..03bc53b802 100644
--- a/src/wp-content/themes/twentynineteen/template-parts/header/site-branding.php
+++ b/src/wp-content/themes/twentynineteen/template-parts/header/site-branding.php
@@ -36,7 +36,7 @@
 				array(
 					'theme_location' => 'menu-1',
 					'menu_class'     => 'main-menu',
-					'items_wrap'     => '<ul id="%1$s" class="%2$s" tabindex="0">%3$s</ul>',
+					'items_wrap'     => '<ul id="%1$s" class="%2$s">%3$s</ul>',
 				)
 			);
 			?>