Fix: wp_admin_notice(): pass type instead of error/updated via additional_classes#12678
Fix: wp_admin_notice(): pass type instead of error/updated via additional_classes#12678hbhalodia wants to merge 2 commits into
type instead of error/updated via additional_classes#12678Conversation
|
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 Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Test using WordPress PlaygroundThe 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
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
There was a problem hiding this comment.
Pull request overview
This PR updates WordPress Core admin-notice call sites to use the supported type argument (error / success) instead of passing legacy notice classes (error / updated) via additional_classes, aligning usage with wp_admin_notice() / wp_get_admin_notice() (introduced in 6.4).
Changes:
- Replaces
additional_classes => array( 'error' )withtype => 'error'across various admin screens. - Replaces
additional_classes => array( 'updated' )withtype => 'success'across various admin screens. - Preserves non-type styling classes (e.g.
inline,fade,hide-if-js,hidden,error-div) viaadditional_classeswhere needed.
Reviewed changes
Copilot reviewed 32 out of 32 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/wp-includes/media-template.php | Uses type => 'error' for the deprecated Edit Media screen notice. |
| src/wp-admin/widgets-form.php | Converts widget admin notices to type-based success/error. |
| src/wp-admin/users.php | Converts multiple user-management notices to type-based success/error; preserves fade where used. |
| src/wp-admin/user-new.php | Converts user creation notices to type-based error/success while keeping paragraph_wrap behavior. |
| src/wp-admin/user-edit.php | Converts profile edit notices to type-based success/error; preserves inline where used. |
| src/wp-admin/upload.php | Converts Media Library notices to type and retains hide-if-js where relevant. |
| src/wp-admin/update-core.php | Converts update selection validation notices to type => 'error'. |
| src/wp-admin/themes.php | Converts theme-management notices to type-based success/error. |
| src/wp-admin/theme-install.php | Converts JS-required installer notice to type => 'error' and keeps hide-if-js. |
| src/wp-admin/theme-editor.php | Converts theme editor notices to type-based success/error. |
| src/wp-admin/plugins.php | Converts plugin screen notices to type-based success/error and updates shared notice args. |
| src/wp-admin/plugin-editor.php | Converts plugin editor notices to type-based error/success and uses dismissible. |
| src/wp-admin/options-general.php | Converts inline settings notice to type => 'success' while keeping inline. |
| src/wp-admin/network/themes.php | Converts network theme caution notices to type => 'error'. |
| src/wp-admin/nav-menus.php | Converts nav menu notices to type-based success/error throughout flows. |
| src/wp-admin/link-manager.php | Converts link manager notice to type => 'success'. |
| src/wp-admin/includes/user.php | Converts default password nag notice to type => 'error' and retains custom class. |
| src/wp-admin/includes/template.php | Converts import/upload and meta box compatibility notices to type-based errors. |
| src/wp-admin/includes/network.php | Converts network install notices to type-based error/success while keeping inline. |
| src/wp-admin/includes/nav-menu.php | Converts nav menu update notices to type-based error/success. |
| src/wp-admin/includes/file.php | Converts filesystem credentials error notice to type => 'error'. |
| src/wp-admin/includes/class-wp-plugin-install-list-table.php | Converts “no items” notice to type => 'error' and keeps inline. |
| src/wp-admin/includes/class-custom-image-header.php | Converts custom header update notice to type => 'success'. |
| src/wp-admin/includes/class-custom-background.php | Converts custom background update notice to type => 'success'. |
| src/wp-admin/includes/class-bulk-upgrader-skin.php | Converts bulk upgrader error notice to type => 'error'. |
| src/wp-admin/import.php | Converts importer-not-installed notice to type => 'error'. |
| src/wp-admin/edit.php | Converts post list “bulk action” notice to type => 'success'. |
| src/wp-admin/edit-tags.php | Converts tag/taxonomy screen notice to type-based success/error. |
| src/wp-admin/edit-link-form.php | Converts “Link added” notice to type => 'success'. |
| src/wp-admin/edit-form-advanced.php | Removes legacy updated class in favor of type => 'success'; converts hidden error notice to type => 'error'. |
| src/wp-admin/edit-comments.php | Converts comment moderation notices to type-based error/success. |
| src/wp-admin/async-upload.php | Converts async upload error notice to type => 'error' while keeping error-div. |
Comments suppressed due to low confidence (1)
src/wp-admin/plugins.php:663
$updated_notice_argsnow configures asuccess-type notice, so the variable name is misleading. Renaming it (and updating its usages below) would make the intent clearer and prevent future confusion.
$updated_notice_args = array(
'id' => 'message',
'type' => 'success',
'dismissible' => true,
);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
mukeshpanchal27
left a comment
mukeshpanchal27
left a comment
There was a problem hiding this comment.
Hi @hbhalodia, thanks for the PR!
Removing the updated class from additional_classes introduces a regression, so I think it's better to keep that as-is and adjust only the type. WDYT?
| implode( "<br />\n", $messages ), | ||
| array( | ||
| 'id' => 'moderated', | ||
| 'additional_classes' => array( 'updated' ), |
There was a problem hiding this comment.
Removing the .updated class will definitely introduce a regression in plugin notices. A quick search shows that many plugins rely on that class to style their notices: https://veloria.dev/search/28516758-5fa1-4e88-9012-19de975fe7f0
For example, LiteSpeed Cache uses .updated directly in its notice styles:
.litespeed-wrap .notice,
.litespeed-wrap div.updated,
.litespeed-wrap div.error {
border-left: 4px solid #fff;
box-shadow: 0 1px 1px 0 rgba(0, 0, 0, 0.1);
padding: 1px 12px;
}There was a problem hiding this comment.
Hi @mukeshpanchal27, Thanks for the review.
I am not sure on this, as per the example shared, it has div.error style is also added. So not only updated class, but also error could have effect. Because the new notice adds the class as notice notice-success or notice notice-error not notice error, so it would break for error as well.
What could be done here to not remove the additional-classes entirely instead, add the type error or success as an extra thing would be more approporate if the regression is concern.
| array( | ||
| 'id' => $error_description_id, | ||
| 'additional_classes' => array( 'error-div', 'error' ), | ||
| 'type' => 'error', | ||
| 'additional_classes' => array( 'error-div' ), | ||
| 'paragraph_wrap' => false, |
Trac ticket: https://core.trac.wordpress.org/ticket/65703
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.