From c232e64d898e9116635c81b770caa003d9514ea2 Mon Sep 17 00:00:00 2001 From: Mario Santos Date: Tue, 19 Sep 2023 10:02:19 +0200 Subject: [PATCH 1/6] Remove navigation SSR attributes --- packages/block-library/src/navigation/index.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/block-library/src/navigation/index.php b/packages/block-library/src/navigation/index.php index 7d2a8888afde14..1d6ef4e9a27143 100644 --- 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 = ' From eb898f037e17bd890854dabcce4855440f3ac12e Mon Sep 17 00:00:00 2001 From: Mario Santos Date: Tue, 19 Sep 2023 10:10:37 +0200 Subject: [PATCH 2/6] Update `bind` directive to remove undefined attrs --- packages/interactivity/src/directives.js | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/packages/interactivity/src/directives.js b/packages/interactivity/src/directives.js index f0a3d7e32e09e1..fd5f7fb0cc2499 100644 --- a/packages/interactivity/src/directives.js +++ b/packages/interactivity/src/directives.js @@ -217,7 +217,12 @@ export default () => { const result = evaluate( path, { context: contextValue, } ); - element.props[ attribute ] = result; + if ( result !== null && result !== undefined ) { + element.ref.current?.setAttribute( attribute, result ); + } else { + element.ref?.current?.removeAttribute( attribute ); + } + // element.props[ 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,13 +246,12 @@ export default () => { attribute !== 'download' && attribute !== 'rowSpan' && attribute !== 'colSpan' && - attribute in el + el.getAttribute( attribute ) ) { try { - el[ attribute ] = - result === null || result === undefined - ? '' - : result; + if ( result === null || result === undefined ) { + el[ attribute ] = result; + } return; } catch ( err ) {} } From 9776616a6e0079235c631e44d479e11cba51c701 Mon Sep 17 00:00:00 2001 From: Mario Santos Date: Tue, 19 Sep 2023 10:11:24 +0200 Subject: [PATCH 3/6] Change `aria-modal` dynamically --- packages/block-library/src/navigation/view.js | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/block-library/src/navigation/view.js b/packages/block-library/src/navigation/view.js index b0d39ef3ca4d57..4aeefa31b177dc 100644 --- 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' - : ''; + : undefined; + }, + ariaModal: ( store ) => { + const { context, selectors } = store; + return context.core.navigation.type === 'overlay' && + selectors.core.navigation.isMenuOpen( store ) + ? 'true' + : undefined; }, isMenuOpen: ( { context } ) => // The menu is opened if either `click`, `hover` or `focus` is true. From aee3180434b8735f14da730f8c600f4f8316cb89 Mon Sep 17 00:00:00 2001 From: Mario Santos Date: Tue, 19 Sep 2023 16:46:41 +0200 Subject: [PATCH 4/6] Add workaround for the `role` attribute --- packages/interactivity/src/directives.js | 36 ++++++++++++++++-------- 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/packages/interactivity/src/directives.js b/packages/interactivity/src/directives.js index fd5f7fb0cc2499..ce3859c630231f 100644 --- 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'; /** @@ -217,12 +223,18 @@ export default () => { const result = evaluate( path, { context: contextValue, } ); - if ( result !== null && result !== undefined ) { - element.ref.current?.setAttribute( attribute, result ); - } else { - element.ref?.current?.removeAttribute( attribute ); - } - // element.props[ attribute ] = result; + 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 @@ -246,12 +258,14 @@ export default () => { attribute !== 'download' && attribute !== 'rowSpan' && attribute !== 'colSpan' && - el.getAttribute( attribute ) + attribute !== 'role' && + attribute in el ) { try { - if ( result === null || result === undefined ) { - el[ attribute ] = result; - } + el[ attribute ] = + result === null || result === undefined + ? '' + : result; return; } catch ( err ) {} } From 7c3a483653455c5d23f57ee7dd23ae3156de4352 Mon Sep 17 00:00:00 2001 From: Mario Santos Date: Tue, 19 Sep 2023 16:47:53 +0200 Subject: [PATCH 5/6] Use `null` instead of `undefined` --- packages/block-library/src/navigation/view.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/block-library/src/navigation/view.js b/packages/block-library/src/navigation/view.js index 4aeefa31b177dc..6a8e4979983b8a 100644 --- a/packages/block-library/src/navigation/view.js +++ b/packages/block-library/src/navigation/view.js @@ -78,14 +78,14 @@ wpStore( { return context.core.navigation.type === 'overlay' && selectors.core.navigation.isMenuOpen( store ) ? 'dialog' - : undefined; + : null; }, ariaModal: ( store ) => { const { context, selectors } = store; return context.core.navigation.type === 'overlay' && selectors.core.navigation.isMenuOpen( store ) ? 'true' - : undefined; + : null; }, isMenuOpen: ( { context } ) => // The menu is opened if either `click`, `hover` or `focus` is true. From c6dc211d4dac53da1784af33987cadf3432339db Mon Sep 17 00:00:00 2001 From: Mario Santos Date: Tue, 19 Sep 2023 17:30:45 +0200 Subject: [PATCH 6/6] Update interactivity package changelog --- packages/interactivity/CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/interactivity/CHANGELOG.md b/packages/interactivity/CHANGELOG.md index 19e82ff30471b4..fe201a7bca3a34 100644 --- a/packages/interactivity/CHANGELOG.md +++ b/packages/interactivity/CHANGELOG.md @@ -6,6 +6,10 @@ - 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)) + ## 2.2.0 (2023-08-31) ### Enhancements