From 0ee7f3b336b02796f7c82de5e4ccd040b356b8b6 Mon Sep 17 00:00:00 2001
From: tellthemachines <tellthemachines@users.noreply.github.com>
Date: Wed, 10 Nov 2021 15:19:37 +1100
Subject: [PATCH] Fix Navigation accessibility issues (#36292)

* Fix display contents accessibility issue in Navigation

* Remove aria-hidden from navigation wrapper.

* Fix content justification in overlay nav.

* Fix for alignment inside overlay menu.

* Move text align rule.

* Revert "Fix content justification in overlay nav."

This reverts commit 5170bb5bb3b9ead314a1ecec68768f2fe2511804.

* Fix php lint errors.

Co-authored-by: jasmussen <joen@automattic.com>
---
 lib/block-supports/layout.php                 | 18 ++++++++--
 packages/block-editor/src/layouts/flex.js     | 32 ++++++++++++++----
 .../src/navigation-submenu/style.scss         |  4 +++
 .../block-library/src/navigation/block.json   |  3 +-
 .../src/navigation/edit/responsive-wrapper.js |  6 +---
 .../block-library/src/navigation/index.php    |  2 +-
 .../block-library/src/navigation/style.scss   | 33 ++++++++++---------
 .../block-library/src/page-list/style.scss    |  7 ++--
 8 files changed, 70 insertions(+), 35 deletions(-)

diff --git a/lib/block-supports/layout.php b/lib/block-supports/layout.php
index 3ba5aca6a35c6d..dee0e654c43c65 100644
--- a/lib/block-supports/layout.php
+++ b/lib/block-supports/layout.php
@@ -103,14 +103,26 @@ function gutenberg_get_layout_style( $selector, $layout, $has_block_gap_support
 			 */
 			if ( ! empty( $layout['justifyContent'] ) && array_key_exists( $layout['justifyContent'], $justify_content_options ) ) {
 				$style .= "justify-content: {$justify_content_options[ $layout['justifyContent'] ]};";
-				// --justification-setting allows children to inherit the value regardless or row or column direction.
-				$style .= "--justification-setting: {$justify_content_options[ $layout['justifyContent'] ]};";
+				if ( ! empty( $layout['setCascadingProperties'] ) && $layout['setCascadingProperties'] ) {
+					// --layout-justification-setting allows children to inherit the value regardless or row or column direction.
+					$style .= "--layout-justification-setting: {$justify_content_options[ $layout['justifyContent'] ]};";
+					$style .= '--layout-direction: row;';
+					$style .= "--layout-wrap: $flex_wrap;";
+					$style .= "--layout-justify: {$justify_content_options[ $layout['justifyContent'] ]};";
+					$style .= '--layout-align: center;';
+				}
 			}
 		} else {
 			$style .= 'flex-direction: column;';
 			if ( ! empty( $layout['justifyContent'] ) && array_key_exists( $layout['justifyContent'], $justify_content_options ) ) {
 				$style .= "align-items: {$justify_content_options[ $layout['justifyContent'] ]};";
-				$style .= "--justification-setting: {$justify_content_options[ $layout['justifyContent'] ]};";
+				if ( ! empty( $layout['setCascadingProperties'] ) && $layout['setCascadingProperties'] ) {
+					// --layout-justification-setting allows children to inherit the value regardless or row or column direction.
+					$style .= "--layout-justification-setting: {$justify_content_options[ $layout['justifyContent'] ]};";
+					$style .= '--layout-direction: column;';
+					$style .= '--layout-justify: initial;';
+					$style .= "--layout-align: {$justify_content_options[ $layout['justifyContent'] ]};";
+				}
 			}
 		}
 		$style .= '}';
diff --git a/packages/block-editor/src/layouts/flex.js b/packages/block-editor/src/layouts/flex.js
index c929a67440b7a2..968e7694db634d 100644
--- a/packages/block-editor/src/layouts/flex.js
+++ b/packages/block-editor/src/layouts/flex.js
@@ -85,7 +85,10 @@ export default {
 		);
 	},
 	save: function FlexLayoutStyle( { selector, layout } ) {
-		const { orientation = 'horizontal' } = layout;
+		const {
+			orientation = 'horizontal',
+			setCascadingProperties = false,
+		} = layout;
 		const blockGapSupport = useSetting( 'spacing.blockGap' );
 		const hasBlockGapStylesSupport = blockGapSupport !== null;
 		const justifyContent =
@@ -94,21 +97,36 @@ export default {
 		const flexWrap = flexWrapOptions.includes( layout.flexWrap )
 			? layout.flexWrap
 			: 'wrap';
-		// --justification-setting allows children to inherit the value
-		// regardless or row or column direction.
-		const rowOrientation = `
+		let rowOrientation = `
 		flex-direction: row;
 		align-items: center;
 		justify-content: ${ justifyContent };
-		--justification-setting: ${ justifyContent };
 		`;
+		if ( setCascadingProperties ) {
+			// --layout-justification-setting allows children to inherit the value
+			// regardless or row or column direction.
+			rowOrientation += `
+			--layout-justification-setting: ${ justifyContent };
+			--layout-direction: row;
+			--layout-wrap: ${ flexWrap };
+			--layout-justify: ${ justifyContent };
+			--layout-align: center;
+			`;
+		}
 		const alignItems =
 			alignItemsMap[ layout.justifyContent ] || alignItemsMap.left;
-		const columnOrientation = `
+		let columnOrientation = `
 		flex-direction: column;
 		align-items: ${ alignItems };
-		--justification-setting: ${ alignItems };
 		`;
+		if ( setCascadingProperties ) {
+			columnOrientation += `
+			--layout-justification-setting: ${ alignItems };
+			--layout-direction: column;
+			--layout-justify: initial;
+			--layout-align: ${ alignItems };
+			`;
+		}
 		return (
 			<style>{ `
 				${ appendSelectors( selector ) } {
diff --git a/packages/block-library/src/navigation-submenu/style.scss b/packages/block-library/src/navigation-submenu/style.scss
index 00105a188c207c..d78ae384144ebc 100644
--- a/packages/block-library/src/navigation-submenu/style.scss
+++ b/packages/block-library/src/navigation-submenu/style.scss
@@ -13,6 +13,10 @@ button.wp-block-navigation-item__content {
 	color: currentColor;
 	font-size: inherit;
 	font-family: inherit;
+
+	// Buttons default to center alignment. This becomes visible
+	// when a menu item label is long enough to wrap.
+	text-align: left;
 }
 
 .wp-block-navigation-submenu__toggle {
diff --git a/packages/block-library/src/navigation/block.json b/packages/block-library/src/navigation/block.json
index 931010815030e2..f4a574b2f92409 100644
--- a/packages/block-library/src/navigation/block.json
+++ b/packages/block-library/src/navigation/block.json
@@ -114,7 +114,8 @@
 			"allowSwitching": false,
 			"allowInheriting": false,
 			"default": {
-				"type": "flex"
+				"type": "flex",
+				"setCascadingProperties": true
 			}
 		}
 	},
diff --git a/packages/block-library/src/navigation/edit/responsive-wrapper.js b/packages/block-library/src/navigation/edit/responsive-wrapper.js
index 7030e8e7914abc..0a85b1cfc629ad 100644
--- a/packages/block-library/src/navigation/edit/responsive-wrapper.js
+++ b/packages/block-library/src/navigation/edit/responsive-wrapper.js
@@ -61,11 +61,7 @@ export default function ResponsiveWrapper( {
 				</Button>
 			) }
 
-			<div
-				className={ responsiveContainerClasses }
-				id={ modalId }
-				aria-hidden={ ! isOpen }
-			>
+			<div className={ responsiveContainerClasses } id={ modalId }>
 				<div
 					className="wp-block-navigation__responsive-close"
 					tabIndex="-1"
diff --git a/packages/block-library/src/navigation/index.php b/packages/block-library/src/navigation/index.php
index 6f52c41c71d7d1..a1bac8453072bb 100644
--- a/packages/block-library/src/navigation/index.php
+++ b/packages/block-library/src/navigation/index.php
@@ -354,7 +354,7 @@ function( $block ) {
 
 	$responsive_container_markup = sprintf(
 		'<button aria-expanded="false" aria-haspopup="true" aria-label="%3$s" class="%6$s" data-micromodal-trigger="modal-%1$s"><svg width="24" height="24" xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24" role="img" aria-hidden="true" focusable="false"><rect x="4" y="7.5" width="16" height="1.5" /><rect x="4" y="15" width="16" height="1.5" /></svg></button>
-			<div class="%5$s" id="modal-%1$s" aria-hidden="true">
+			<div class="%5$s" id="modal-%1$s">
 				<div class="wp-block-navigation__responsive-close" tabindex="-1" data-micromodal-close>
 					<div class="wp-block-navigation__responsive-dialog" role="dialog" aria-modal="true" aria-labelledby="modal-%1$s-title" >
 							<button aria-label="%4$s" data-micromodal-close class="wp-block-navigation__responsive-container-close"><svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24" width="24" height="24" role="img" aria-hidden="true" focusable="false"><path d="M13 11.8l6.1-6.3-1-1-6.1 6.2-6.1-6.2-1 1 6.1 6.3-6.5 6.7 1 1 6.5-6.6 6.5 6.6 1-1z"></path></svg></button>
diff --git a/packages/block-library/src/navigation/style.scss b/packages/block-library/src/navigation/style.scss
index 8547d420c91b72..ca6eb0b4b702a5 100644
--- a/packages/block-library/src/navigation/style.scss
+++ b/packages/block-library/src/navigation/style.scss
@@ -304,6 +304,11 @@
 
 // Navigation block inner container.
 .wp-block-navigation__container {
+	display: flex;
+	flex-wrap: var(--layout-wrap, wrap);
+	flex-direction: var(--layout-direction, initial);
+	justify-content: var(--layout-justify, initial);
+	align-items: var(--layout-align, initial);
 
 	// Reset the default list styles
 	list-style: none;
@@ -329,7 +334,11 @@
 	bottom: 0;
 
 	.wp-block-navigation__responsive-container-content {
-		display: contents;
+		display: flex;
+		flex-wrap: var(--layout-wrap, wrap);
+		flex-direction: var(--layout-direction, initial);
+		justify-content: var(--layout-justify, initial);
+		align-items: var(--layout-align, initial);
 	}
 
 	// Overlay menu.
@@ -350,8 +359,6 @@
 		// Add extra top padding so items don't conflict with close button.
 		padding: 72px 24px 24px 24px;
 		background-color: inherit;
-		// Fallback to inheritance in case the --justification-setting variable fails.
-		align-items: inherit;
 
 		.wp-block-navigation__responsive-container-content {
 			// Override the container flex layout settings
@@ -360,7 +367,11 @@
 			display: flex;
 			flex-direction: column;
 			// Inherit alignment settings from container.
-			align-items: var(--justification-setting, inherit);
+			align-items: var(--layout-justification-setting, inherit);
+
+			// Always align the contents of the menu to the top.
+			justify-content: flex-start;
+
 			// Allow menu to scroll.
 			overflow: auto;
 			padding: 0;
@@ -412,7 +423,7 @@
 				display: flex;
 				flex-direction: column;
 				// Inherit alignment settings from container.
-				align-items: var(--justification-setting, inherit);
+				align-items: var(--layout-justification-setting, initial);
 			}
 		}
 
@@ -435,7 +446,7 @@
 	@include break-small() {
 		&:not(.hidden-by-default) {
 			&:not(.is-menu-open) {
-				display: contents;
+				display: block;
 				width: 100%;
 				position: relative;
 				z-index: 2;
@@ -518,13 +529,3 @@ html.has-modal-open {
 	overflow: hidden;
 }
 
-.wp-block-navigation__responsive-close,
-.wp-block-navigation__responsive-dialog,
-.wp-block-navigation__container {
-	display: contents;
-
-	.is-menu-open & {
-		// Fallback to inheritance in case the --justification-setting variable fails.
-		align-items: inherit;
-	}
-}
diff --git a/packages/block-library/src/page-list/style.scss b/packages/block-library/src/page-list/style.scss
index e31bafeaf2d585..5623c2dab9275f 100644
--- a/packages/block-library/src/page-list/style.scss
+++ b/packages/block-library/src/page-list/style.scss
@@ -1,8 +1,11 @@
 // The Page List block should inherit navigation styles when nested within it
 .wp-block-navigation {
 	.wp-block-page-list {
-		display: contents;
-		flex-wrap: wrap;
+		display: flex;
+		flex-direction: var(--layout-direction, initial);
+		justify-content: var(--layout-justify, initial);
+		align-items: var(--layout-align, initial);
+		flex-wrap: var(--layout-wrap, wrap);
 		background-color: inherit;
 	}