Context Navigation

← Previous Ticket
Next Ticket →

#44640 closed defect (bug) (fixed)

Use current `.notice` CSS classes for settings errors

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

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 7 years ago.
44640.2.diff (975 bytes ) - added by donmhico 7 years ago.: Change how the CSS is created to support other types of notices (info, warning).
44640.3.diff (977 bytes ) - added by donmhico 7 years ago.: Corrected @since to adhere to WPCS.

Download all attachments as: .zip

Change History (10)

#1 @desrosj
7 years ago

Keywords needs-patch added; 2nd-opinion removed
Milestone Awaiting Review → Future Release

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

#2 @toddhalfpenny
7 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
7 years ago

Attachment 44640.diff added

#3 @donmhico
7 years ago

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

@donmhico
7 years ago

Attachment 44640.2.diff added

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

#4 @donmhico
7 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/