Close Twenty Twenty and Twenty Twenty-One mobile menus when clicking on in-page anchor link menu items (#5720)

Co-authored-by: Pierre Gordon <[email protected]>

Author: westonruter

includes/sanitizers/class-amp-core-theme-sanitizer.php

@@ -1885,6 +1885,9 @@ public function add_twentytwenty_modals() {
 			// the actionable elements in the modal. Therefore, the lightbox represents the "background".
 			$close_button_xpaths[] = "//*[ @id = '{$modal_id}' ]";
 
+			// Ensure anchor links also close the modal the same as the the Close Menu button.
+			$close_button_xpaths[] = sprintf( "//*[ @id = '{$modal_id}' ]//a[ @href and contains( @href, '#' ) ]" );
+
 			// Then, add the inner element of the lightbox as an open button xpath.
 			// This is done to prevent the above close action from closing the modal when an inner element is clicked.
 			// Workaround found here: https://stackoverflow.com/a/45971501 .
@@ -2159,6 +2162,21 @@ public function add_twentytwentyone_mobile_modal() {
 		AMP_DOM_Utils::add_amp_action( $menu_toggle, 'tap', "{$body_id}.toggleClass(class=primary-navigation-open)" );
 		AMP_DOM_Utils::add_amp_action( $menu_toggle, 'tap', "{$body_id}.toggleClass(class=lock-scrolling)" );
 		$menu_toggle->setAttribute( 'data-amp-bind-aria-expanded', "{$state_string} ? 'true' : 'false'" );
+
+		// Close the mobile modal when clicking in-page anchor links in the menu.
+		foreach ( $this->dom->xpath->query( '//*[ @id = "site-navigation" ]//a[ @href and contains( @href, "#" ) ]' ) as $link ) {
+			/** @var DOMElement $link */
+			AMP_DOM_Utils::add_amp_action( $link, 'tap', "AMP.setState({{$state_string}: false})" );
+			AMP_DOM_Utils::add_amp_action( $link, 'tap', "{$body_id}.toggleClass(class=primary-navigation-open,force=false)" );
+			AMP_DOM_Utils::add_amp_action( $link, 'tap', "{$body_id}.toggleClass(class=lock-scrolling,force=false)" );
+
+			// Ensure target is scrolled into view. Note that in-page anchor links currently do not work in the non-AMP
+			// version. Normally scrollTo shouldn't be necessary but it appears necessary due to scroll locking.
+			$target = preg_replace( '/.*#/', '', $link->getAttribute( 'href' ) );
+			if ( $target && $this->dom->getElementById( $target ) ) {
+				AMP_DOM_Utils::add_amp_action( $link, 'tap', "{$target}.scrollTo" );
+			}
+		}
 	}
 
 	/**

tests/php/test-class-amp-core-theme-sanitizer.php

@@ -514,7 +514,7 @@ public function test_amend_twentytwentyone_styles() {
 	 */
 	public function test_add_twentytwentyone_mobile_modal() {
 		$html = '
-			<nav>
+			<nav id="site-navigation">
 				<div class="menu-button-container">
 					<button id="primary-mobile-menu">
 						<span class="dropdown-icon open">Menu</span>
@@ -523,12 +523,14 @@ public function test_add_twentytwentyone_mobile_modal() {
 				</div>
 				<div class="primary-menu-container">
 					<ul id="primary-menu-list">
-						<li id="menu-item-1">Foo</li>
-						<li id="menu-item-2">Bar</li>
-						<li id="menu-item-3">Baz</li>
+						<li id="menu-item-1"><a href="https://example.com/">Foo</a></li>
+						<li id="menu-item-2"><a href="#colophon">Bar</a></li>
+						<li id="menu-item-3"><a href="https://example.com/#site-footer">Baz</a></li>
+						<li id="menu-item-2"><a href="' . esc_url( home_url( '/#colophon' ) ) . '">Quux</a></li>
 					</ul>
 				</div>
 			</nav>
+			<div id="colophon"></div>
 		';
 
 		$dom       = AMP_DOM_Utils::get_dom_from_content( $html );
@@ -539,6 +541,23 @@ public function test_add_twentytwentyone_mobile_modal() {
 		$query = $dom->xpath->query( '//button[ @id = "primary-mobile-menu" and @data-amp-bind-aria-expanded and @on ]' );
 
 		$this->assertEquals( 1, $query->length );
+
+		$query = $dom->xpath->query( '//a[ @href ]' );
+		$this->assertSame( 4, $query->length );
+		foreach ( $query as $link ) {
+			/** @var DOMElement $link */
+			$href = $link->getAttribute( 'href' );
+			if ( false !== strpos( $href, '#' ) ) {
+				$this->assertTrue( $link->hasAttribute( 'on' ) );
+				if ( false !== strpos( $href, '#colophon' ) ) {
+					$this->assertStringContains( 'colophon.scrollTo', $link->getAttribute( 'on' ) );
+				} else {
+					$this->assertStringNotContains( 'colophon.scrollTo', $link->getAttribute( 'on' ) );
+				}
+			} else {
+				$this->assertFalse( $link->hasAttribute( 'on' ) );
+			}
+		}
 	}
 
 	/**