Make WordPress Core

Opened 2 days ago

Last modified 40 hours ago

#65703 accepted defect (bug)

wp_admin_notice(): pass `type` instead of `error/updated` via additional_classes

Reported by: hbhalodia Owned by: joedolson
Priority: normal Milestone: Awaiting Review
Component: Administration Version:
Severity: normal Keywords: good-first-bugs has-patch
Cc: Focuses: administration, coding-standards

Description

While working on this #65690 and PR - , I got one feedback from the copilot here.

The feedback says that,

wp_admin_notice() supports a type argument (e.g. error), which produces the canonical notice notice-error class. Using additional_classes => array( 'error', ... ) is legacy-style and makes the class semantics less clear for new notices; prefer type => 'error' and keep hide-if-js as an additional class.

When I looked at the codebase, I found inconsistency accross the codebase, some use legacy-style and use type argument to construct the notice.

I would like to propose issue here and improve code quality, so that we can consistently use the new variation i.e., to add the type argument, instead of legacy-style i.e., to add classes to the admin notices.

Below is the detailed version created by AI, Claude Code, Opus 4.8 with details like where is inconsistency accross the codebase.


AI Summary

Since WordPress 6.4, wp_admin_notice() and wp_get_admin_notice() accept a type argument (error, success, warning, info) that outputs the standard notice notice-<type> classes.

Core is inconsistent in how it uses this. Some calls correctly pass type, while many others pass the legacy status class (error or updated) directly through additional_classes. Both render the same (core styles div.error / .notice-error and div.updated / .notice-success identically), but using type makes the intent explicit and keeps every call site consistent.

Correct usagesource:trunk/src/wp-login.php:

<?php
wp_admin_notice(
        $errors,
        array(
                'type'           => 'error',
                'id'             => 'login_error',
                'paragraph_wrap' => false,
        )
);

Inconsistent usagesource:trunk/src/wp-admin/upload.php:

<?php
wp_admin_notice(
        $js_required_message,
        array(
                'additional_classes' => array( 'error', 'hide-if-js' ),
        )
);

Proposed change

Convert the legacy status class to the type argument, keeping any other (non-status) classes in additional_classes:

  • additional_classes => array( 'error' )type => 'error'
  • additional_classes => array( 'updated' )type => 'success' (updated is the legacy green/success class)
  • additional_classes => array( 'error', 'hidden' )type => 'error' + additional_classes => array( 'hidden' )

Output markup is equivalent, so this is low risk and can be split across multiple patches or in a single PR depends.

Line numbers are against trunk at the time of writing.


Group 1 — 'error'type => 'error' (50)

File Line(s) Classes to keep
src/wp-admin/import.php 73
src/wp-admin/edit-form-advanced.php 469 hidden
src/wp-admin/theme-editor.php 316, 353
src/wp-admin/themes.php 304, 321, 350
src/wp-admin/update-core.php 1078, 1088
src/wp-admin/edit-comments.php 308
src/wp-admin/plugins.php 350, 364, 652, 704, 723
src/wp-admin/users.php 328, 720, 740, 760, 790, 816
src/wp-admin/upload.php 243 hide-if-js
src/wp-admin/user-new.php 407, 436
src/wp-admin/widgets-form.php 433
src/wp-admin/plugin-editor.php 37
src/wp-admin/theme-install.php 189 hide-if-js
src/wp-admin/user-edit.php 256
src/wp-admin/async-upload.php 137 error-div
src/wp-admin/nav-menus.php 316, 348, 391, 449, 478, 494
src/wp-admin/network/themes.php 142, 153
src/wp-admin/includes/file.php 2524
src/wp-admin/includes/class-wp-plugin-install-list-table.php 298 inline
src/wp-admin/includes/network.php 127, 168, 284, 435, 458 284: inline
src/wp-admin/includes/template.php 1023, 1447 1447: inline
src/wp-admin/includes/user.php 559 default-password-nag
src/wp-admin/includes/nav-menu.php 1464
src/wp-admin/includes/class-bulk-upgrader-skin.php 199
src/wp-includes/media-template.php 389


Group 2 — 'updated'type => 'success' (39)

File Line(s) Classes to keep
src/wp-admin/edit-form-advanced.php 454
src/wp-admin/theme-editor.php 199
src/wp-admin/themes.php 266, 276, 285, 295, 312, 329, 338
src/wp-admin/options-general.php 284 inline
src/wp-admin/edit-comments.php 424
src/wp-admin/edit-link-form.php 100
src/wp-admin/plugins.php 661
src/wp-admin/nav-menus.php 288, 325, 361, 538, 704
src/wp-admin/plugin-editor.php 204 also is-dismissibledismissible => true
src/wp-admin/user-new.php 419
src/wp-admin/widgets-form.php 423
src/wp-admin/users.php 656, 682, 700, 710, 729, 749, 769, 780, 798 780, 798: fade
src/wp-admin/upload.php 229, 446
src/wp-admin/user-edit.php 233, 599 599: inline
src/wp-admin/link-manager.php 129
src/wp-admin/edit.php 476
src/wp-admin/includes/nav-menu.php 1519
src/wp-admin/includes/class-custom-background.php 271
src/wp-admin/includes/class-custom-image-header.php 540


Group 3 — Dynamic status class (2)

These build the class from a variable and need a conditional type instead:

File Line Detail
src/wp-admin/edit-tags.php 360 $class = isset( $_REQUEST['error'] ) ? 'error' : 'updated'; — set type to 'error' / 'success'
src/wp-admin/includes/network.php 234 $message_class is '', 'updated', or 'error' (set at lines 201–213) — map to type; keep inline


Not included (correct as-is)

These additional_classes values are behavioural/modifier classes with no type equivalent and should stay: notice-alt, inline, hidden, fade, hide-if-js / hide-if-no-js, below-h2, update-message, update-nag (a distinct legacy style, not one of the four types), the login-specific message / reset-pass / register, and notice-missing-attachment. Substrings such as error-div, community-events-errors and policy-text-updated are unrelated class names, not status types.

Thanks,

Change History (2)

This ticket was mentioned in PR #12678 on WordPress/wordpress-develop by @hbhalodia.


45 hours ago
#1

  • Keywords has-patch added

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

## Use of AI Tools

  • Yes
  • Claude Code, Opus 4.8
  • Used for drafting the issue description and also for performing the code updates.

#2 @joedolson
40 hours ago

  • Owner set to joedolson
  • Status newaccepted

To provide a little history on this, when we implemented wp_admin_notice() across core, we made the decision not to update all the legacy classes to the modern classes, so that we reduced churn on the output in that single release. It should be safe; it's unlikely that there are many plugins out there depending on a given notice using the .updated class at this point.

Note: See TracTickets for help on using tickets.