Skip to content
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

Update to use a11y script module package in Core. #65539

Merged
merged 1 commit into from
Sep 26, 2024

Conversation

sirreal
Copy link
Member

@sirreal sirreal commented Sep 20, 2024

See WordPress/wordpress-develop#7304.

What?

Update the interactivity-router package to always use the @wordpress/a11y module for its aria-live regions.

This was behind a Gutenberg-only flag, but since landing in Core in 6.7 (with compatibility in Gutenberg for previous versions) the flag can be removed to always use the a11y module.

Why?

It's preferable to use the shared package that implements this functionality instead of a one-off implementation. Now that a11y is available as a script module, it should be used.

How?

Remove the IS_GUTENBERG_PLUGIN conditionals.

Testing Instructions

This PR is difficult to test on its own because it requires the package to be updated as a Core dependency for proper testing. It does not affect Gutenberg behavior, only the package's behavior in Core. There are other ways of doing it but this works:

  • Get a checkout of Core from Interactivity Router: Update for latest GB changes. wordpress-develop#7304. This is the build that will be running.
  • Run npm ci in the Core checkout to install packages.
  • Build the package from this PR (npm run build:packages in Gutenberg).
  • Replace the @wordpress/interactivity-router package in Core's node_modules (node_modules/@wordpress/interactivity-router) with the package directory in Gutenberg (remove the directory and copy the directory from Gutenberg packages/interactivity-router).
  • In Core, run the npm build script you use, in my case npm run dev.

Then test out that the interactivity router a11y functionality is working. A good way to test is the query block with "force page reload" option disabled. Specifically, the aria-live regions of the page should be updated (with localized text if in non-English locale) announcing "Page loaded."

@sirreal sirreal added [Type] Task Issues or PRs that have been broken down into an individual action to take [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Package] Interactivity Router /packages/interactivity-router labels Sep 20, 2024
@sirreal sirreal force-pushed the update/interactivity-router-use-a11y-core branch from 4277fc8 to fb4eba7 Compare September 24, 2024 09:35
@sirreal sirreal added the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Sep 25, 2024
@sirreal sirreal force-pushed the update/interactivity-router-use-a11y-core branch from fb4eba7 to 4aa1449 Compare September 25, 2024 19:07
@sirreal sirreal marked this pull request as ready for review September 26, 2024 09:35
Copy link

github-actions bot commented Sep 26, 2024

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: sirreal <[email protected]>
Co-authored-by: gziolo <[email protected]>
Co-authored-by: michalczaplinski <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@sirreal sirreal requested review from gziolo and michalczaplinski and removed request for gziolo September 26, 2024 09:42
@sirreal sirreal force-pushed the update/interactivity-router-use-a11y-core branch from 4aa1449 to 9487bb1 Compare September 26, 2024 09:42
@sirreal sirreal requested a review from gziolo September 26, 2024 09:42
@@ -403,30 +397,28 @@ function a11ySpeak( messageKey: keyof typeof navigationTexts ) {
} catch {}
} else {
// Fallback to localized strings from Interactivity API state.
// @todo This block is for Core < 6.7.0. Remove when support is dropped.

// @ts-expect-error
if ( state.navigation.texts?.loading ) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just confirming, does this state come from the server in WP 6.6 and 6.5? Was it removed in WP core for WP 6.7?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WordPress/wordpress-develop#7304 is expected to remove it in 6.7 and switch to the script module data passing. It was in the Interactivity API store state before that change, so in WordPress 6.6 it was in the Interactivity API store state.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent. Thank you for confirmation.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I may not have answered that fully. Yes, it's server provided state in WordPress 6.6.

@michalczaplinski
Copy link
Contributor

michalczaplinski commented Sep 26, 2024

I'm testing using the "Testing Instructions" from PR description (using Core https://github.com/WordPress/wordpress-develop/pull/7304/files + copied packages from this PR).

I see an odd issue: the aria-live message has a trailing &nbsp every other navigation:

output_6de84c.mp4

It might be something silly or even unrelated but let's confirm it first 🙂

@sirreal
Copy link
Member Author

sirreal commented Sep 26, 2024

Thanks for the thorough testing!

I see an odd issue: the aria-live message has a trailing \n every other navigation:

This is expected behavior, the speak package adds a trailing &nbsp; on repeated identical messages to encourage accessibility software to announce them:

/*
* Safari + VoiceOver don't announce repeated, identical strings. We use
* a `no-break space` to force them to think identical strings are different.
*/
if ( previousMessage === message ) {
message += '\u00A0';
}

Copy link
Contributor

@michalczaplinski michalczaplinski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great to me 👍 Thank you for working on this Jon!

@sirreal sirreal merged commit 7d883a4 into trunk Sep 26, 2024
66 of 67 checks passed
@sirreal sirreal deleted the update/interactivity-router-use-a11y-core branch September 26, 2024 12:14
@github-actions github-actions bot added this to the Gutenberg 19.4 milestone Sep 26, 2024
@github-actions github-actions bot removed the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Sep 26, 2024
gutenbergplugin pushed a commit that referenced this pull request Sep 26, 2024
Update the interactivity-router package to use the @wordpress/a11y module for its aria-live regions.

This was behind a Gutenberg-only flag. Since the a11y module is landing in Core in 6.7, the flag can be removed to always use the a11y module.

See WordPress/wordpress-develop#7304.
@github-actions github-actions bot added the Backported to WP Core Pull request that has been successfully merged into WP Core label Sep 26, 2024
Copy link

I just cherry-picked this PR to the wp/6.7 branch to get it included in the next release: 20ae57d

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backported to WP Core Pull request that has been successfully merged into WP Core [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Package] Interactivity Router /packages/interactivity-router [Type] Task Issues or PRs that have been broken down into an individual action to take
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants