-
Notifications
You must be signed in to change notification settings - Fork 4.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Interactivity API: Use a11y Script Module in Gutenberg #65123
Interactivity API: Use a11y Script Module in Gutenberg #65123
Conversation
Size Change: -304 B (-0.02%) Total Size: 1.77 MB
ℹ️ View Unchanged
|
6296956
to
c0348dd
Compare
Flaky tests detected in 85821fd. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/10904903884
|
b63deed
to
8406792
Compare
f59a8be
to
4dd71f6
Compare
8406792
to
df831db
Compare
4dd71f6
to
a79ceda
Compare
afb51ff
to
6f6d8a2
Compare
d49068e
to
41d105d
Compare
359b640
to
14c1dcb
Compare
14c1dcb
to
4d8d9cb
Compare
I discovered some issues with setting up live regions and immediately updating them. This indicated a problem with the a11y module that I'm addressing in #65380. I've merged that PR here. |
It's certainly not optimal, it's been brought up before. I've been reluctant to add a module for loading modules, but I think ultimately it will need to happen and it's a question of focusing on it, although I agree this PR is not the place for it. |
c761be2
to
b4ae046
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Most of the changes come from #65380, which I reviewed first. I tested both PRs with this branch, and everything works as expected. I would appreciate the final check from @DAreRodz and @michalczaplinski. Do you think it's good enough to proceed with bringing the same set of changes to WordPress core in 6.7 release? To give an example of what it opens, it will be possible to reuse speak
from @wordpress/a11y
when implementing the lightbox for the Gallery block, which was raised during review by @afercia:
if ( ! hasLoadedNavigationTextsData ) { | ||
hasLoadedNavigationTextsData = true; | ||
const content = document.getElementById( | ||
'wp-script-module-data-@wordpress/interactivity-router' | ||
)?.textContent; | ||
if ( content ) { | ||
try { | ||
const parsed = JSON.parse( content ); | ||
if ( typeof parsed?.i18n?.loading === 'string' ) { | ||
navigationTexts.loading = parsed.i18n.loading; | ||
} | ||
if ( typeof parsed?.i18n?.loaded === 'string' ) { | ||
navigationTexts.loaded = parsed.i18n.loaded; | ||
} | ||
} catch {} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code eventually could get replaced with dynamically loaded @wordpress/i18n
.
if ( globalThis.IS_GUTENBERG_PLUGIN ) { | ||
import( '@wordpress/a11y' ).then( | ||
( { speak } ) => speak( message ), | ||
// Ignore failures to load the a11y module. | ||
() => {} | ||
); | ||
} else { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's behind the flag so the new approach will be only used in the Gutenberg plugin in case we decide against shipping it in the WordPress 6.7.
b4ae046
to
325249f
Compare
texts: { | ||
loading: '', | ||
loaded: '', | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would overwrite state from the server, it was breaking the server-provided localized strings previously.
hasLoadedNavigationTextsData = true; | ||
const content = document.getElementById( | ||
'wp-script-module-data-@wordpress/interactivity-router' | ||
)?.textContent; | ||
if ( content ) { | ||
try { | ||
const parsed = JSON.parse( content ); | ||
if ( typeof parsed?.i18n?.loading === 'string' ) { | ||
navigationTexts.loading = parsed.i18n.loading; | ||
} | ||
if ( typeof parsed?.i18n?.loaded === 'string' ) { | ||
navigationTexts.loaded = parsed.i18n.loaded; | ||
} | ||
} catch {} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The localized strings weren't working correctly before because the server-provided state was being overwritten with empty strings.
This form will work in Gutenberg but requires #65129 for it to work in Core.
There should be fallback code here to use the localized strings from Interactivity API state.
); | ||
} | ||
} | ||
add_action( 'after_setup_theme', 'gutenberg_register_interactivity_script_module_data_hooks', 20 ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With the latest changes to use the localized fallback strings (passed via Interactivity API), if I disable this filter it correctly uses the those strings as a fallback.
#65129 proposes adding this filter to Core.
What?
Change the Interactivity Router a11y implementation to use the a11y Script Module introduced in #65101.
The a11y Script Module will be used in Gutenberg only and if it cannot be loaded, the Interactivity API fallback will be used.
Builds on:
WordPress/wordpress-develop#7304 implements the same script data passing for translated strings in Core.
Why?
Instead of its own 1-off implementation, shared a11y functionality can be used.
Testing Instructions
Enable client side navigation, for example in the query block turn off the "force page reloads" option.
With the proper accessibility set up, for example macOS voiceover, "Page loaded." should be announced when navigating.
Alternatively, you can inspect the DOM with something like this in a console preview:
Screenshots or screencast
demo.mov