Announce settings save status to screen readers - #1851
Open
benitoalba wants to merge 2 commits into
Open
Conversation
Contributor
📝 WalkthroughWalkthroughThe settings page now renders saved-status notices, conditionally loads ChangesSettings save status
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant SettingsPage
participant AdminApp
participant AnnounceSettingsSaveStatus
participant WpA11y
SettingsPage->>AdminApp: Render settings notice and initialize
AdminApp->>AnnounceSettingsSaveStatus: Pass window.wp?.a11y
AnnounceSettingsSaveStatus->>WpA11y: Speak notice message with politeness
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
benitoalba
force-pushed
the
fix/1757-announce-settings-save-status
branch
from
July 26, 2026 16:56
29edf88 to
81c93ac
Compare
benitoalba
marked this pull request as ready for review
July 26, 2026 17:07
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/phpunit/partials/SettingsPagePartialTest.php`:
- Line 41: Update tests/phpunit/partials/SettingsPagePartialTest.php at lines
41-41 by renaming the test method to test_partial_renders_settings_saved_notice
and adding a : void return type. Update tests/phpunit/Admin/EnqueueAdminTest.php
at lines 106-106 by renaming the test method to
test_enqueue_base_script_with_wp_a11y_on_settings_page and adding a : void
return type, preserving each test’s existing behavior.
- Line 13: Move SettingsPagePartialTest into the
EqualizeDigital\AccessibilityChecker\Tests\Partials namespace and add the
corresponding PSR-4 mapping for tests/phpunit/ in composer.json or the PHPUnit
bootstrap/autoload configuration, ensuring the test remains discoverable and
runnable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 729ea44f-a921-401a-8e60-8bc88e122e04
📒 Files selected for processing (7)
admin/class-enqueue-admin.phppartials/settings-page.phpsrc/admin/index.jssrc/admin/settings/announce-settings-save-status.jstests/jest/admin/announce-settings-save-status.test.jstests/phpunit/Admin/EnqueueAdminTest.phptests/phpunit/partials/SettingsPagePartialTest.php
pattonwebz
requested changes
Aug 3, 2026
Comment on lines
+86
to
+91
| $admin_script_dependencies = [ 'jquery' ]; | ||
| if ( 'accessibility_checker_settings' === $page ) { | ||
| $admin_script_dependencies[] = 'wp-a11y'; | ||
| } | ||
|
|
||
| wp_enqueue_script( 'edac', plugin_dir_url( EDAC_PLUGIN_FILE ) . 'build/admin.bundle.js', $admin_script_dependencies, EDAC_VERSION, false ); |
Member
There was a problem hiding this comment.
It's probably worth just putting this in the deps array everywhere rather than just conditionally on the one page.
Suggested change
| $admin_script_dependencies = [ 'jquery' ]; | |
| if ( 'accessibility_checker_settings' === $page ) { | |
| $admin_script_dependencies[] = 'wp-a11y'; | |
| } | |
| wp_enqueue_script( 'edac', plugin_dir_url( EDAC_PLUGIN_FILE ) . 'build/admin.bundle.js', $admin_script_dependencies, EDAC_VERSION, false ); | |
| wp_enqueue_script( 'edac', plugin_dir_url( EDAC_PLUGIN_FILE ) . 'build/admin.bundle.js', [ 'jquery', 'wp-a11y' ], EDAC_VERSION, false ); |
Contributor
Author
There was a problem hiding this comment.
Updated as suggested: edac now includes wp-a11y on every admin page, and the non-settings regression test was updated. PHPUnit passes with 27 tests and 56 assertions, along with PHPCS.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #1757.
wp.a11y.speak, using polite output for success and assertive output for errors.wp-a11yas a dependency of the admin bundle.Root cause
The settings form submits through WordPress's Settings API, which stores the standard result notice, but the custom settings page did not call
settings_errors(). The page therefore refreshed without showing or announcing the save result.The settings page now renders the standard visible notice immediately after its heading. On DOM ready, the admin bundle detects the notice and schedules the announcement for one second after the page finishes loading. This prevents the polite status from being lost among the screen reader's page-load announcements and does not move keyboard focus.
Validation
npm run buildgit diff --checkChecklist