Make WordPress Core

Opened 22 months ago

Closed 22 months ago

Last modified 22 months ago

#58096 closed defect (bug) (fixed)

Incorrect color for "Theme enabled." admin notice

Reported by: ocean90's profile ocean90 Owned by: audrasjb's profile audrasjb
Milestone: 6.2.1 Priority: normal
Severity: minor Version: 6.2
Component: Networks and Sites Keywords: good-first-bug has-patch
Focuses: ui Cc:

Description

This is a follow-up to #39213.

When enabling a theme for a network the notice is using the gray border color instead of green. This was introduced in [55418] by using a non-existent notice-updated class. It should be notice-success.

Attachments (6)

current.png (11.9 KB) - added by ocean90 22 months ago.
expected.png (12.2 KB) - added by ocean90 22 months ago.
58096.diff (772 bytes) - added by dhrumilk 22 months ago.
Updated diff.
58096.1.diff (1.3 KB) - added by dhrumilk 22 months ago.
Updated diff.
Capture d’écran 2023-04-06 à 14.18.23.png (199.7 KB) - added by audrasjb 22 months ago.
Theme enabled on network
Capture d’écran 2023-04-06 à 14.18.07.png (165.2 KB) - added by audrasjb 22 months ago.
Theme disabled on network

Download all attachments as: .zip

Change History (20)

@ocean90
22 months ago

@ocean90
22 months ago

#1 follow-up: @audrasjb
22 months ago

I can reproduce the issue on my side.
Looks like classes notice notice-updated is-dismissible shoulb be replaced with updated notice is-dismissible for this notice. Not CSS change needed.

This ticket was mentioned in PR #4309 on WordPress/wordpress-develop by MarineEvain.


22 months ago
#2

  • Keywords has-patch added; needs-patch removed

#3 @marineevain
22 months ago

Hi,

This is my first time patching WordPress code. If someone can review it and let me know if this helps...
Have a nice day!

#4 in reply to: ↑ 1 @SergeyBiryukov
22 months ago

Replying to audrasjb:

I can reproduce the issue on my side.
Looks like classes notice notice-updated is-dismissible should be replaced with updated notice is-dismissible for this notice. Not CSS change needed.

The updated class is legacy, see #44640, so we should just replace notice-updated with notice-success here:

notice notice-success is-dismissible

@dhrumilk
22 months ago

Updated diff.

#5 @chiragrathod103
22 months ago

Thanks, @dhrumilk for the patch.
The Patch looks good to me. but still need to update line 386 and 387

Last edited 22 months ago by chiragrathod103 (previous) (diff)

@dhrumilk
22 months ago

Updated diff.

#6 @chiragrathod103
22 months ago

Thanks, @dhrumilk for the patch.
Now Patch 58096.1.diff looks perfect to me.

@audrasjb
22 months ago

Theme enabled on network

@audrasjb
22 months ago

Theme disabled on network

#7 @audrasjb
22 months ago

  • Keywords commit added
  • Owner set to audrasjb
  • Status changed from new to accepted

#8 @audrasjb
22 months ago

Thanks everybody for your contribution :)
58096.1.diff is good to go.

#9 @audrasjb
22 months ago

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

In 55637:

Networks and Sites: Fix incorrect color for Theme enabling admin notices.

This changeset replaces a notice-updated class with notice-success to fix an issue where the notices were using a gray border color instead of green when enabling or disabling a theme for a network.

Follow-up to [55418].

Props ocean90, audrasjb, marineevain, SergeyBiryukov, dhrumilk, chiragrathod103.
Fixes #58096.

#10 @audrasjb
22 months ago

  • Keywords fixed-major added; commit removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening for branch 6.2 backport.

#12 @audrasjb
22 months ago

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

In 55638:

Networks and Sites: Fix incorrect color for Theme enabling admin notices.

This changeset replaces a notice-updated class with notice-success to fix an issue where the notices were using a gray border color instead of green when enabling or disabling a theme for a network.

Follow-up to [55418].

Props ocean90, audrasjb, marineevain, SergeyBiryukov, dhrumilk, chiragrathod103.
Merges [55584] to the 6.2 branch.
Fixes #58096.

#13 @audrasjb
22 months ago

Wrong “Merges” reference in the last changeset :\
This obviously merges [55637] to 6.2, not [55584].

#14 @audrasjb
22 months ago

  • Keywords fixed-major removed
Note: See TracTickets for help on using tickets.