Remove role attribute when set to null in data-wp-bind (#54608)

* Remove navigation SSR attributes * Update `bind` directive to remove undefined attrs * Change `aria-modal` dynamically * Add workaround for the `role` attribute * Use `null` instead of `undefined` * Update interactivity package changelog
WordPress · Sep 20, 2023 · 481783b · 481783b
1 parent 08b6f1c
commit 481783b
Show file tree

Hide file tree

Showing 4 changed files with 31 additions and 5 deletions.
diff --git a/packages/block-library/src/navigation/index.php b/packages/block-library/src/navigation/index.php
@@ -713,10 +713,8 @@ function render_block_core_navigation( $attributes, $content, $block ) {
 			tabindex="-1"
 		';
 		$responsive_dialog_directives    = '
-			data-wp-bind--aria-modal="selectors.core.navigation.isMenuOpen"
-			aria-modal="false"
+			data-wp-bind--aria-modal="selectors.core.navigation.ariaModal"
 			data-wp-bind--role="selectors.core.navigation.roleAttribute"
-			role=""
 			data-wp-effect="effects.core.navigation.focusFirstElement"
 		';
 		$close_button_directives         = '

diff --git a/packages/block-library/src/navigation/view.js b/packages/block-library/src/navigation/view.js
@@ -78,7 +78,14 @@ wpStore( {
 					return context.core.navigation.type === 'overlay' &&
 						selectors.core.navigation.isMenuOpen( store )
 						? 'dialog'
-						: '';
+						: null;
+				},
+				ariaModal: ( store ) => {
+					const { context, selectors } = store;
+					return context.core.navigation.type === 'overlay' &&
+						selectors.core.navigation.isMenuOpen( store )
+						? 'true'
+						: null;
 				},
 				isMenuOpen: ( { context } ) =>
 					// The menu is opened if either `click`, `hover` or `focus` is true.

diff --git a/packages/interactivity/CHANGELOG.md b/packages/interactivity/CHANGELOG.md
@@ -6,6 +6,9 @@
 
 -   Improve `navigate()` to render only the result of the last call when multiple happen simultaneously. ([#54201](https://github.com/WordPress/gutenberg/pull/54201))
 
+### Bug Fix
+
+-   Remove `role` attribute when set to `null` in `data-wp-bind`. ([#54608](https://github.com/WordPress/gutenberg/pull/54608))
 -   Add `timeout` option to `navigate()`, with a default value of `10000` milliseconds. ([#54474](https://github.com/WordPress/gutenberg/pull/54474))
 
 ## 2.2.0 (2023-08-31)

diff --git a/packages/interactivity/src/directives.js b/packages/interactivity/src/directives.js
@@ -1,7 +1,13 @@
 /**
  * External dependencies
  */
-import { useContext, useMemo, useEffect, useRef } from 'preact/hooks';
+import {
+	useContext,
+	useMemo,
+	useEffect,
+	useRef,
+	useLayoutEffect,
+} from 'preact/hooks';
 import { deepSignal, peek } from 'deepsignal';
 
 /**
@@ -218,6 +224,17 @@ export default () => {
 						context: contextValue,
 					} );
 					element.props[ attribute ] = result;
+					// Preact doesn't handle the `role` attribute properly, as it doesn't remove it when `null`.
+					// We need this workaround until the following issue is solved:
+					// https://github.com/preactjs/preact/issues/4136
+					useLayoutEffect( () => {
+						if (
+							attribute === 'role' &&
+							( result === null || result === undefined )
+						) {
+							element.ref.current.removeAttribute( attribute );
+						}
+					}, [ attribute, result ] );
 
 					// This seems necessary because Preact doesn't change the attributes
 					// on the hydration, so we have to do it manually. It doesn't need
@@ -241,6 +258,7 @@ export default () => {
 							attribute !== 'download' &&
 							attribute !== 'rowSpan' &&
 							attribute !== 'colSpan' &&
+							attribute !== 'role' &&
 							attribute in el
 						) {
 							try {