Make WordPress Core

Opened 19 months ago

Last modified 3 months ago

#39213 reviewing defect (bug)

Audit the network pages notices.

Reported by: afercia Owned by: jeremyfelt
Milestone: 5.0 Priority: normal
Severity: normal Version: 4.7
Component: Networks and Sites Keywords: has-ui-feedback good-first-bug needs-patch
Focuses: ui, multisite Cc:

Description (last modified by afercia)

Some notices and messages in the network pages are printed out in simple paragraphs and don't make use of the WordPress notices styling. For example, in my-sites.php:

_e( 'You must be a member of at least one site to use this page.' )

There are probably a few ones across the network screens. For design consistency, they should all use a wrapper <div> element with the proper CSS classes notice, notice-success, notice-error etc. Also, the old CSS classes updated and error should be replaced with the new ones, where applicable.

Attachments (3)

39213.diff (1.2 KB) - added by marksabbath 19 months ago.
39213.1.diff (1.2 KB) - added by marksabbath 11 months ago.
my-sites.diff (1.2 KB) - added by aryamaaru 3 weeks ago.
Please review my patch anybody

Download all attachments as: .zip

Change History (16)

#1 @afercia
19 months ago

  • Description modified (diff)

#2 @karmatosed
19 months ago

  • Keywords has-ui-feedback added; ui-feedback removed

Good catch, I agree this should have div added for design consistency and the proper class.

19 months ago

#3 @marksabbath
19 months ago

  • Keywords has-patch added
  • Version set to 4.7

Hello, people!

I've researched on this file and found only 2 occurrences:

Line 54 and 71.

Line 71 should be protected by the capabilities, but I've updated it as well. See attached the patch.

#4 @DrewAPicture
12 months ago

  • Owner set to marksabbath
  • Status changed from new to assigned

Assigning ownership to mark the good-first-bug as "claimed".

This ticket was mentioned in Slack in #core-multisite by stevenkword. View the logs.

11 months ago

#6 @stevenkword
11 months ago

  • Keywords needs-testing added

39213.diff still applies cleanly as of July 10, 2017.

Per multisite bug scrub: " @earnjam -- I don’t see any issues with the patch on 39213. Would just want to confirm there weren’t any other notices/messages that needed styling that were missed."

@marksabbath -- This looks largely complete. What tasks do you feel is missing in order to push this over the finish line? I'm adding the keyword, 'needs-user-testing' for now. Do you need any feedback?

Last edited 11 months ago by stevenkword (previous) (diff)

#7 @afercia
11 months ago

  • Keywords needs-refresh added

The elements nesting in the second notice in the patch is not correct:

<p><strong> ... </p></strong>

#8 @marksabbath
11 months ago

You're completely correct @afercia thanks for noticing and reporting that error. It should be fixed in this patch: https://core.trac.wordpress.org/attachment/ticket/39213/39213.1.diff

@stevenkword I think we're good to go. Thank you all for reviewing and sending feedbacks.

#9 @marksabbath
7 months ago

Hey @stevenkword anything that I could be doing to make this update moving forward?

This ticket was mentioned in Slack in #core-multisite by marksabbath. View the logs.

3 months ago

#11 @jeremyfelt
3 months ago

  • Keywords needs-testing needs-refresh removed
  • Milestone changed from Awaiting Review to 5.0
  • Owner changed from marksabbath to jeremyfelt
  • Status changed from assigned to reviewing

These changes look good. There's an extra > in the class attribute on the second message, but I'll take care of that in commit.

It takes messing with capabilities to even see the "You must be a member of at least one site to use this page." message as there are two other errors that would more likely show beforehand, but this is now displayed consistent with other error notices.

Thanks @marksabbath!

#12 @jeremyfelt
3 months ago

  • Keywords needs-patch added; has-patch removed

It does look like there several other notices that can be updated with notice-error and notice-success in src/wp-admin/network/*.php.

@marksabbath Would you be okay submitting an updated patch covering those as well?

#13 @marksabbath
3 months ago

Totes @jeremyfelt will work on that.

3 weeks ago

Please review my patch anybody

Note: See TracTickets for help on using tickets.