Make WordPress Core

Opened 6 years ago

Closed 5 years ago

Last modified 5 years ago

#44640 closed defect (bug) (fixed)

Use current `.notice` CSS classes for settings errors

Reported by: flixos90's profile flixos90 Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 5.3 Priority: normal
Severity: normal Version:
Component: Administration Keywords: has-patch
Focuses: Cc:

Description

The CSS classes error and updated for settings errors are legacy, as numerous releases ago more proper classes notice, notice-success, notice-error etc. were introduced. However, the settings_errors() function still prints divs with the old classes.

We should consider adjusting that for consistency:

  • Map updated to notice-success.
  • Map error to notice-error.

Attachments (3)

44640.diff (1.0 KB) - added by donmhico 5 years ago.
44640.2.diff (975 bytes) - added by donmhico 5 years ago.
Change how the CSS is created to support other types of notices (info, warning).
44640.3.diff (977 bytes) - added by donmhico 5 years ago.
Corrected @since to adhere to WPCS.

Download all attachments as: .zip

Change History (10)

#1 @desrosj
6 years ago

  • Keywords needs-patch added; 2nd-opinion removed
  • Milestone changed from Awaiting Review to Future Release

I think this is reasonable. Moving to Future Release so a patch can be created.

#2 @toddhalfpenny
6 years ago

  • Keywords has-patch added; needs-patch removed

Wondering if the following approach is valid. I did wonder about putting the mapping into it's own function, but seemed overkill.

WAS

        foreach ( $settings_errors as $key => $details ) {
                $css_id    = 'setting-error-' . $details['code'];
                $css_class = $details['type'] . ' settings-error notice is-dismissible';
                $output   .= "<div id='$css_id' class='$css_class'> \n";
                $output   .= "<p><strong>{$details['message']}</strong></p>";
                $output   .= "</div> \n";
        }

Proposed

        foreach ( $settings_errors as $key => $details ) {

                $type_css_class = '';
                switch ( $details['type'] ) {
                        case 'error':
                                $type_css_class = 'notice-error';
                                break;
                        case 'updated':
                                $type_css_class = 'notice-success';
                                break;
                }

                $css_id    = 'setting-error-' . $details['code'];
                $css_class = $type_css_class . ' settings-error notice is-dismissible';
                $output   .= "<div id='$css_id' class='$css_class'> \n";
                $output   .= "<p><strong>{$details['message']}</strong></p>";
                $output   .= "</div> \n";
        }

Tested (by saving "General Settings" with empty email address.
If this looks good will create a patch.

@donmhico
5 years ago

#3 @donmhico
5 years ago

I created a patch, 44640.diff, based on @toddhalfpenny code above.

@donmhico
5 years ago

Change how the CSS is created to support other types of notices (info, warning).

#4 @donmhico
5 years ago

Please use the latest patch - 44640.2.diff . It will handle other notice types passed on add_settings_error() better than the first patch I submitted. See #44941.

Reference:
https://developer.wordpress.org/reference/functions/add_settings_error/

@donmhico
5 years ago

Corrected @since to adhere to WPCS.

#5 @SergeyBiryukov
5 years ago

  • Milestone changed from Future Release to 5.3
  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing

#6 @SergeyBiryukov
5 years ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 45814:

Administration: In add_settings_error(), add warning and info as possible values for message type.

Account for these new values in settings_errors(), resulting in notice-warning and notice-info CSS classes.

Map legacy error and updated CSS classes to notice-error and notice-success.

Props donmhico, toddhalfpenny, flixos90, desrosj, javorszky, SergeyBiryukov.
Fixes #44640, #44941.

#7 @SergeyBiryukov
5 years ago

In 45818:

Administration: Replace legacy updated message type in add_settings_error() calls with success.

See #44640.

Note: See TracTickets for help on using tickets.