WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 6 years ago

#30488 closed defect (bug) (fixed)

Switching between Admin Color Schemes does not immediately update the body class.

Reported by: valendesigns Owned by: SergeyBiryukov
Milestone: 4.2 Priority: normal
Severity: normal Version: 3.8
Component: Administration Keywords: has-patch dev-feedback
Focuses: ui, javascript, administration Cc:

Description

Recently I was working on a plugin that required different menu icons based on the color scheme currently activated. However, I found that when you switch between schemes in your profile the body class is not updated until you manually refresh the page. Which means any styles you've added via admin_head to change your icon globally are not immediately reflected in the UI.

I'm submitting this ticket with a simple patch. The changes I've made are to pass the value of $color_scheme to the wp_send_json_success() function via its $data parameter, and a bit of JavaScript that removes the old color scheme and adds the newly selected one to the body class.

Attachments (3)

30488.diff (1.1 KB) - added by valendesigns 6 years ago.
30488.1.diff (1.2 KB) - added by valendesigns 6 years ago.
30488.2.diff (1.2 KB) - added by valendesigns 6 years ago.

Download all attachments as: .zip

Change History (16)

@valendesigns
6 years ago

#1 @valendesigns
6 years ago

  • Keywords has-patch added

#2 @florianziegler
6 years ago

Tested. Works.

Would be nice to get this committed asap, as we also need this for #28599, see my comment for reference.

Last edited 6 years ago by florianziegler (previous) (diff)

#3 @valendesigns
6 years ago

  • Keywords dev-feedback added

Thank you for testing the patch @florianziegler. Hopefully this can make it into 4.1.0, which seems like a reasonable request IMO.

#4 follow-up: @johnbillion
6 years ago

  • Milestone changed from Awaiting Review to Future Release

I'd be interested in knowing what changes you make to your icon based on the colour scheme.

#5 in reply to: ↑ 4 @valendesigns
6 years ago

Replying to johnbillion:

I'd be interested in knowing what changes you make to your icon based on the colour scheme.

Originally I was going to simply replace the darker version with a lighter version for certain schemes by hiding the image and adding the alternative to the background of the container. However, due to the body class limitation I went a different route. In the end I created an svg and turned it into a font. Which was the best solution, and much more seamless for UX. Still, having the class properly switch makes sense; even if I don't need it anymore. Not everyone has the option to turn their icon into a font.

#6 @johnbillion
6 years ago

  • Version changed from trunk to 3.8

#7 @valendesigns
6 years ago

@johnbillion Could we please get this added early in 4.2?

Cheers!

#8 @SergeyBiryukov
6 years ago

  • Milestone changed from Future Release to 4.2

This ticket was mentioned in Slack in #core by drew. View the logs.


6 years ago

#10 follow-up: @ocean90
6 years ago

To get rid of the RegExp, what about returning the old and the new CSS class in wp_send_json_success() to make a simple .replace() call?

#11 in reply to: ↑ 10 @valendesigns
6 years ago

Replying to ocean90:

To get rid of the RegExp, what about returning the old and the new CSS class in wp_send_json_success() to make a simple .replace() call?

Sounds good. I'll update the patch.

@valendesigns
6 years ago

@valendesigns
6 years ago

#12 @valendesigns
6 years ago

Patch 30488.2.diff lets wp_ajax_save_user_color_scheme take care of the logic, and the JavaScript simply removes and adds the classes that get passed in from the wp_send_json_success array. Thank you @ocean90 for the suggestion, it's much better this way.

#13 @SergeyBiryukov
6 years ago

  • Owner set to SergeyBiryukov
  • Resolution set to fixed
  • Status changed from new to closed

In 31400:

Update body class when switching between admin color schemes.

props valendesigns.
fixes #30488.

Note: See TracTickets for help on using tickets.