Refactor settings_errors() to use wp_admin_notice() internally. by afercia · Pull Request #13669 · WordPress/wordpress-develop

afercia · 2026-09-22T20:21:31Z

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

Use of AI Tools

None

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 · 2026-09-22T20:21:45Z

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 afercia, westonruter, joedolson.

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

afercia · 2026-09-22T20:24:13Z

Cc @pattonwebz

github-actions · 2026-09-22T20:24:44Z

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.

afercia · 2026-09-23T09:04:35Z

Honestly, I would just remove testing for this and remove the workarounds I had to introduce to support this passed notice type.
But:
It was introduced in https://core.trac.wordpress.org/changeset/45873 to account for an edge case reported by... myself. Still, I'd consider to remove it also because wp_get_admin_notice() doesn't accept a type string that contains spaces.
Open to feedback.

afercia · 2026-09-23T09:05:54Z

-		if ( 'updated' === $details['type'] ) {
-			$details['type'] = 'success';
+		// Backward compatibility for passed types that may wrongly contain `notice-`.
+		$type = str_replace( 'notice-', '', trim( $details['type'] ) );


Passed types should no longer contain notice- as wp_get_admin_notice() already adds this prefix.

Could this accidentally remove notice- from the unintended class? What if someone passs in a type like 'error my-notice-is-awesome hello world'?

westonruter · 2026-09-23T15:23:10Z

-		$css_class = sprintf(
-			'notice %s settings-error is-dismissible',
-			esc_attr( $details['type'] )
+			esc_attr( $details['code'] ),


Minor: This escaping is redundant. It is already handled by wp_get_admin_notice().

Suggested change

esc_attr( $details['code'] ),

$details['code'],

westonruter · 2026-09-23T15:23:59Z

+			$additional_classes = explode( ' ', $type . ' settings-error' );
+			// Set the type to empty string.
+			$type = '';
+


Suggested change

westonruter · 2026-09-23T15:24:18Z

+		if ( str_contains( $type, ' ' ) ) {
+			// Use ths substrings as additional classes.
+			$additional_classes = explode( ' ', $type . ' settings-error' );
+			// Set the type to empty string.


This is obvious 😄

Suggested change

// Set the type to empty string.

westonruter · 2026-09-23T15:27:11Z

-		if ( 'updated' === $details['type'] ) {
-			$details['type'] = 'success';
+		// Backward compatibility for passed types that may wrongly contain `notice-`.
+		$type = str_replace( 'notice-', '', trim( $details['type'] ) );


Could this accidentally remove notice- from the unintended class? What if someone passs in a type like 'error my-notice-is-awesome hello world'?

afercia · 2026-09-23T16:53:57Z

Currently, this PR introduces a change in the behavior for potential custom notice types.
Before, a passed type like for example calltoaction worked and was printed out without prefixing notice-.
After, it gets prefixed and becomes notice-calltoaction.

This is inherently the behavior of wp_admin_notice(() which always adds the notice- prefix.
A decision shoudl be made on whether to allow custom types without the notice- prefix.

afercia · 2026-09-23T20:27:25Z

I addressed the backward compatibility issue in a hopefully better way and added more test cases. It covers cases when the $type passed to add_settings_error() may be like:

'error'
'success'
'warning'
'info'
'updated'
'notice-error'
'custom'
'notice-custom'
'error my-own-css-class hello world'
'error my-notice-is-awesome hello world'

This should cover the previous behavior.
Note that custom types may produce CSS classes that are rendered in a different order, but the classes are all there.

joedolson

A minor change suggestion in a comment, for improved clarity. Otherwise, I think this looks good.

joedolson · 2026-09-24T23:11:31Z

+		 * classes are rendered in the default order determined by `wp_admin_notice()`.
+		 * This includes the old type `updated`. Instead, anything else e.g.
+		 * custom notice types or strings with spaces is now passed as
+		 * additional_classes and appended to the end of the string,


This makes it sound like $additional_classes is a string, when it's an array. I think you should limit the comment to addressing how custom classes are handled here, and not address how they will be handled once they're inside wp_admin_notices().

…pected order.

The amdin notices rendered in the Settings pages use their own function to echo the notices, which is a little legacy. Today, Core provides a standardized way to render the admin notices by the means of `wp_admin_notice()`. - Refactors `settings_errors()` to use `wp_admin_notice()` internally. - Expands the related tests. Added bonus: The Settings pages notices will now fire the some hooks and filters fired by `wp_admin_notice()`, e.g. `wp_admin_notice` and `wp_admin_notice_markup`, which is a necessary preparation for #50486. Developed in #13669 Props afercia, westonruter, joedolson. See #50486. Fixes #66161. git-svn-id: https://develop.svn.wordpress.org/trunk@63943 602fd350-edb4-49c9-b593-d223f7449a82

github-actions · 2026-09-26T12:38:16Z

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

SVN changeset: 63943
GitHub commit: aaea5ab

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

The amdin notices rendered in the Settings pages use their own function to echo the notices, which is a little legacy. Today, Core provides a standardized way to render the admin notices by the means of `wp_admin_notice()`. - Refactors `settings_errors()` to use `wp_admin_notice()` internally. - Expands the related tests. Added bonus: The Settings pages notices will now fire the some hooks and filters fired by `wp_admin_notice()`, e.g. `wp_admin_notice` and `wp_admin_notice_markup`, which is a necessary preparation for #50486. Developed in WordPress/wordpress-develop#13669 Props afercia, westonruter, joedolson. See #50486. Fixes #66161. Built from https://develop.svn.wordpress.org/trunk@63943 git-svn-id: http://core.svn.wordpress.org/trunk@63109 1a063a9b-81f0-0310-95a4-ce76da25c4cd

afercia requested a review from joedolson September 22, 2026 20:23

afercia commented Sep 23, 2026

View reviewed changes

afercia mentioned this pull request Sep 23, 2026

Admin notices accessibility #13671

Closed

westonruter reviewed Sep 23, 2026

View reviewed changes

afercia force-pushed the admin-notices-a11y branch from 33891a0 to 9c14839 Compare September 23, 2026 20:21

afercia force-pushed the admin-notices-a11y branch 2 times, most recently from 32664c6 to 8ecee8f Compare September 23, 2026 22:24

joedolson requested changes Sep 24, 2026

View reviewed changes

afercia force-pushed the admin-notices-a11y branch from 8ecee8f to 7df6fa0 Compare September 25, 2026 07:09

afercia added the props-bot Adding this label triggers the Props Bot workflow for a PR. label Sep 26, 2026

github-actions Bot removed the props-bot Adding this label triggers the Props Bot workflow for a PR. label Sep 26, 2026

afercia added 10 commits September 26, 2026 14:31

Refactor settings_errors() to use wp_admin_notice() internally.

5fca0b1

Adjustments to make tests pass and for backward compatibility.

4900e9c

Better address backward compatibility for custom admin notice types.

4b3febf

Remove no longer necessary esc_attr.

e83cce5

Simplify test by expecting the full string with CSS classes in the ex…

e7e9cc0

…pected order.

Revert unintentional change to user.php file.

b6ad7ef

Add the notice class to the expected output where missing.

7b31839

Clarify comment.

85bafe5

Remove redundant test case.

0ec409b

Improve comments.

8ce4b17

afercia force-pushed the admin-notices-a11y branch from edc789e to 8ce4b17 Compare September 26, 2026 12:31

github-actions Bot closed this Sep 26, 2026

Conversation

afercia commented Sep 22, 2026

Use of AI Tools

Uh oh!

github-actions Bot commented Sep 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

afercia commented Sep 22, 2026

Uh oh!

github-actions Bot commented Sep 22, 2026

Test using WordPress Playground

Some things to be aware of

Uh oh!

afercia Sep 23, 2026

Choose a reason for hiding this comment

Uh oh!

afercia Sep 23, 2026

Choose a reason for hiding this comment

Uh oh!

westonruter Sep 23, 2026

Choose a reason for hiding this comment

Uh oh!

westonruter Sep 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

westonruter Sep 23, 2026

Choose a reason for hiding this comment

Uh oh!

westonruter Sep 23, 2026

Choose a reason for hiding this comment

Uh oh!

westonruter Sep 23, 2026

Choose a reason for hiding this comment

Uh oh!

afercia commented Sep 23, 2026

Uh oh!

afercia commented Sep 23, 2026

Uh oh!

joedolson left a comment

Choose a reason for hiding this comment

Uh oh!

joedolson Sep 24, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Sep 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions Bot commented Sep 22, 2026 •

edited

Loading

westonruter Sep 23, 2026 •

edited

Loading