Accessibility: Require Alt key for arrow navigation in theme and media modals

[Sukhendu2002]

Trac ticket: https://core.trac.wordpress.org/ticket/63760

Use of AI Tools

---

This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

[github-actions]

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

• All changes will be lost when closing a tab with a Playground instance.
• All changes will be lost when refreshing the page.
• A fresh instance is created each time the link below is clicked.
• Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
  it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

[joedolson]

In addition to the comment on media modal navigation, I think both keyboard navigations should trigger a wp.a11y.speak() notification telling the user what media item/theme they're now viewing. Otherwise, the feedback for screen reader users is not very good.

[joedolson]

This change breaks repeated use of keyboard navigation to switch, because focus is moved to the prev/next buttons after triggering navigation.

Preventing these events while on interactive elements was specifically necessary because the key event was triggered on single arrows, but with a combination key event, that's not so relevant.

[Sukhendu2002]

Thanks for the review. I updated the patch to add wp.a11y.speak() feedback for both media and theme Alt+arrow navigation, so screen reader users hear what item they are now viewing. I also removed the BUTTON and A exclusions so repeated navigation still works after focus moves to the prev/next buttons. I kept SELECT treated as a form field, but can remove that too if preferred.

[github-actions]

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.

Core Committers: Use this line as a base for the props when committing in SVN:

Props sukhendu2002, joedolson, afercia.

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

[Copilot]

Pull request overview

This PR updates keyboard navigation in the Themes and Media modals to require the Alt modifier when using left/right arrow keys, and adds new localized strings to announce the newly shown theme/media item via wp.a11y.speak() for improved screen reader feedback.

Changes:

• Require Alt + Left/Right Arrow for previous/next navigation in the theme preview/overlay and the media edit-attachments modal.
• Add wp.a11y.speak() announcements when navigating to a new theme/media item.
• Add new i18n strings (themeViewed, mediaItemViewed) to support the announcements.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
src/wp-includes/media.php	Adds a localized announcement string for the media modal (“Viewing media item: %s”).
src/wp-admin/themes.php	Adds a localized announcement string for themes browsing (“Viewing theme: %s”).
src/wp-admin/theme-install.php	Adds the same localized announcement string for the theme installer context.
src/js/media/views/frame/edit-attachments.js	Requires Alt+Arrow navigation and announces the current media item after navigation.
src/js/_enqueues/wp/theme.js	Requires Alt+Arrow navigation in theme overlays/previews and announces the current theme after navigation.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[joedolson]

Three changes: use event.originalEvent.repeat instead of event.repeat, update the shortcut documentation in the Help panel, and add missing support for navigating themes in the Customizer. No need to add the spoken message in the customizer, as it is already present in the showDetails method.

[afercia]

Thank you for the PR, I think it is going in the right direction.
I will add some consideration points in a following comment.

[afercia]

A few considerations:

In the Media Library, the usage of event.originalEvent.repeat changes the current behavior. Previously, keeping the arrow keys pressed allowed to navigate attachments very quickly without interruption. Now, the navigation works one attachment at a time, requiring to press Alt + arrow keys multiple times to navigate multiple attachments.

Personally, I liked the previous behavior because it allows a very quick navigation. Now, this is no longer possible. We should make a decision on whether to preserve the 'press and hold' behavior. If so, I think it should be used also in the theme browser, for consistency.

The speak announcements should be debounced.
When clicking multiple times the next and previous buttons or using the keyboard shortcuts multiple times very quickly, users would receive multiple, consecutive, speak announcements at any attachment / theme change. Ideally, the announcement should be triggered when we detect users stop navigating.
If we implement the 'press and hold' behavior from point 1), that would be evrn worse.
Ideally, the announcements should be debounced as in: prevented while the navigation methods are called multiple times within a certain amount of time. If that turns to be difficult to implement, maybe we should try a simple timeout. The announcement should be canceled if users close the modal dialogs within the debounce interval.

The message of the announcement for the themes is slightly different. I would suggest to make them consistent:

• new message: 'themeViewed' => __( 'Viewing theme: %s' ),
• existing string: 'announceThemeDetails' => __( 'Showing details for theme: %s' ),

Ideally, the shorter the better. However, I'm not sure 'Viewing theme' is accurate. Actually, the modal dialogs don't show the theme. They show the theme details.

In the theme browser there's no help to inform users they can use alt / option + the left or right arrow keys or at least I couldn't find any.

If we think Alt + arrow keys is the best option, we should create a new ticket to evaluate whether there are other places in Core and Gutenberg where this pattern should be implemented. This pattern should be consistent everywhere.

[joedolson]

1. In all honesty, I never use keyboard navigation in the media library, and wouldn't have even thought of that. But given that change, I think it's better to return to the previous behavior.
2. Agreed.
3. Shorter is better, but only if it's accurate. I'd be fine with shifting both just to 'Theme details: %s'; I feel like the context of 'showing' or 'viewing' can be assumed.
4. I assume you're referring to the screen at Appearance > Themes. The rest of the navigation guidance is in the Overview panel, so I think we could add instructions there.
5. Agreed. I don't know that Alt + arrow is the best option, but I don't have a better idea. It is certainly better than a single-key trigger that's needed elsewhere, however.

[afercia]

Missing docblock.

[afercia]

I went ahead and changed the debounce() implementation as described in the previous code review.
Testing would be appreciated. HInt for testing purposes: temporarily increase the debounce delay from 500 to 2000.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

[afercia]

I think the cancelation is not needed when using the underscore debounce(). It would be needed when using setTimeout().

I stand partially corrected. The underscore.js _.debounce() has an internal debounced.cancel method that can be used when needed. It can be called directly on the wrapper function returned by _.debounce() to clear the internal timer, stopping any pending execution. For example: this.announceThemeDebounced.cancel().

We should test the scenario where users may close the modal dialog before the 500ms interval, to avoid an undesired announcement when the modal dialog is already closed. If the announcement is triggered, we should cancel the debounce.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (3)

src/js/_enqueues/wp/theme.js:40

• Using String.prototype.replace('%s', …) to fill a translatable string can break for locales that use numbered placeholders (e.g. "%1$s"). Using wp.i18n.sprintf() supports both numbered and unnumbered placeholders.

	name = model.get( 'name' ) || model.get( 'id' );

	if ( ! name ) {
		return;
	}

	wp.a11y.speak( l10n.themeViewed.replace( '%s', name ) );
}, 500 );

src/js/media/views/frame/edit-attachments.js:58

• Using String.prototype.replace('%s', …) to fill a translatable string can break for locales that use numbered placeholders (e.g. "%1$s"). Using wp.i18n.sprintf() supports both numbered and unnumbered placeholders.

		title = model.get( 'title' ) || model.get( 'filename' ) || model.get( 'id' );

		if ( ! title ) {
			return;
		}

		wp.a11y.speak( l10n.mediaItemViewed.replace( '%s', title ) );
	}, 500 ),

src/js/_enqueues/wp/customize/controls.js:1733

• Using String.prototype.replace('%s', …) to fill a translatable string can break for locales that use numbered placeholders (e.g. "%1$s"). Using wp.i18n.sprintf() supports both numbered and unnumbered placeholders.

			section.announceThemeDebounced = _.debounce( function( name ) {
				if ( ! name ) {
					return;
				}

				wp.a11y.speak( api.settings.l10n.announceThemeDetails.replace( '%s', name ) );
			}, 500 );

[afercia]

I updated the PR to my best. All the related announcement are now debounced. When closing the modal dialogs, the pending announcement are canceled. This behavior should be tested in the following 4 places:

Installed themes:
/wp-admin/themes.php

Theme installer:
/wp-admin/theme-install.php?browse=popular

Customizer theme preview (click the 'Change' button and then previwe the themes):
/wp-admin/customize.php?return=%2Fwp-admin%2Fthemes.php

Media grid:
/wp-admin/upload.php

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

src/js/_enqueues/wp/theme.js:1152

• themes.announceThemeDebounced can still fire after the theme details overlay is closed via the router theme:close path (triggered in route:themes). The close handler currently calls self.overlay.closeOverlay() but does not cancel the pending debounced announcement, so a queued screen-reader message may be spoken after the modal is gone.

		$( 'body' ).on( 'keydown.wp-themes', function( event ) {
			if ( ! self.overlay ) {
				return;
			}

tests/qunit/wp-admin/js/theme.js:96

• This test includes a leftover comment (“This test would need…”) that is now inaccurate, and the block’s spacing is inconsistent with nearby QUnit tests (e.g., } ) ); / } );). This makes the test harder to read and may trip style checks.

		QUnit.test( 'PreventDefault is called for arrow keys with Alt', function( assert ) {
			// This test would need to check if preventDefault was called
			var event = $.Event( 'keydown', {
				keyCode: 39,
				altKey: true,

[afercia]

Latest commit improves the Help text:

• Adds missing <code> tags.
• Better explains clicking a Theme will open a modal dialog.
• Changes Use the arrow buttons at the top of the dialog... to Use the buttons at the top of the dialog...
• Adds periods at the end of list items, for consistency.

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

src/js/media/views/frame/edit-attachments.js:49

• announceMediaItemDebounced is created on the prototype via _.debounce(...), which means the debounce timer/state (and .cancel()) can be shared across multiple EditAttachments frame instances. If more than one frame is ever instantiated, announcements can cancel/override each other unexpectedly. Prefer creating the debounced function per instance in initialize() and keeping the prototype property as null.

	announceMediaItemDebounced: _.debounce( function( model ) {
		var title;

		if ( ! model ) {
			return;

tests/qunit/wp-admin/js/theme.js:96

• The comment in this test is misleading (“would need to check if preventDefault was called”) even though the test does check it. Also, the new block deviates from the spacing used in the surrounding tests (e.g. } );), which makes the file inconsistent.

		QUnit.test( 'PreventDefault is called for arrow keys with Alt', function( assert ) {
			// This test would need to check if preventDefault was called
			var event = $.Event( 'keydown', {
				keyCode: 39,
				altKey: true,

[joedolson]

This looks good to me. I tested with VoiceOver, NVDA, and JAWS, and all three worked as expected with all browsers tested.

[github-actions]

A commit was made that fixes the Trac ticket referenced in the description of this pull request.

SVN changeset: 62878
GitHub commit: 5b9b41f

This PR will be closed, but please confirm the accuracy of this and reopen if there is more work to be done.