WordPress.org

Make WordPress Core

Opened 5 months ago

Closed 5 months ago

Last modified 5 months ago

#50537 closed defect (bug) (fixed)

The buttons colors don't adapt properly to the admin scheme color on WP-Admin

Reported by: youknowriad Owned by: youknowriad
Milestone: 5.5 Priority: normal
Severity: normal Version: 5.4
Component: Administration Keywords: has-patch
Focuses: accessibility, css Cc:

Description

When you select a deferent admin-scheme, primary buttons change color, but secondary ones don't and the ".page-title-action" buttons don't adapt either.

Attachments (3)

Capture d’écran 2020-07-02 à 5.48.14 PM.png (188.5 KB) - added by youknowriad 5 months ago.
This screenshot shows the result after fixing the issues.
midnight-before.png (190.2 KB) - added by ocean90 5 months ago.
midnight-after.png (185.2 KB) - added by ocean90 5 months ago.

Download all attachments as: .zip

Change History (21)

@youknowriad
5 months ago

This screenshot shows the result after fixing the issues.

#1 @youknowriad
5 months ago

  • Status changed from assigned to accepted

This ticket was mentioned in PR #379 on WordPress/wordpress-develop by youknowriad.


5 months ago

  • Keywords has-patch added

#3 follow-up: @youknowriad
5 months ago

  • Focuses accessibility added
  • Keywords needs-design-feedback added

I'm not sure whether this requires an a11y feedback because it only touches the admin scheme colors and not the default one but I added the label regardless.

#4 in reply to: ↑ 3 @mapk
5 months ago

Replying to youknowriad:

I'm not sure whether this requires an a11y feedback because it only touches the admin scheme colors and not the default one but I added the label regardless.

Because it's only touching an alternate admin theme, I don't think it needs an accessibility review based on this slack comment from @joedolson: https://wordpress.slack.com/archives/C02S78ZAL/p1593629194193300 but I could be wrong.

I haven't tested your patch but the screenshot looks fantastic. I would also expect the primary color to change on the secondary buttons too.

#5 @mapk
5 months ago

  • Keywords needs-design-feedback removed

#6 @youknowriad
5 months ago

I just noticed the primary hover color is broken, I'll try to fix that a bit later.

#7 @michaelarestad
5 months ago

Tested. It looks particularly good for the most part. I also see busted primary button hover broken and some of the schemes are a bit light, but perhaps they need a minor updated.

#8 @Joen
5 months ago

Wonderful small iteration.

I look forward to helping #49999 with improving things overall, there's so much low hanging fruit that just needs a small tweak like this here and there.

#9 @youknowriad
5 months ago

Thanks all of you for the feedback, I fixed the hover issue for primary buttons.

Now about the low contrast in some admin scheme colors, I think the main issue is not from this ticket but I think that the "highlight color" for these admin schemes is just not a good color. The low contrast can be seen in other places too. So I was hesitant on whether to address it here or not.

@ryelle also proposed that we define a new Sass variable for the default/secondary button colors. In that same vein and since I believe the issue is not that the color between secondary and primary should be different, I'm going to define a variable that if set we fallback to the neutral secondary buttons. It's a measure to ensure the color schemes that don't have good colors won't be impacted by this PR while by default all schemes will adapt (just like the default admin scheme).

Last edited 5 months ago by youknowriad (previous) (diff)

#10 follow-up: @youknowriad
5 months ago

I updated the patch and kept the default colors for ocean and coffee themes. Let me know how looks it now. If folks agree it's a good shape, it's still time to ship on 5.5.

#11 @youknowriad
5 months ago

I'd also appreciate a review on #50536 (it should be more straightforward as just a simple fix)

Last edited 5 months ago by SergeyBiryukov (previous) (diff)

#12 @youknowriad
5 months ago

Here's another related ticket that I'd appreciate eyes on. #50547

Last edited 5 months ago by SergeyBiryukov (previous) (diff)

#13 in reply to: ↑ 10 @mcsf
5 months ago

Replying to youknowriad:

I updated the patch and kept the default colors for ocean and coffee themes. Let me know how looks it now. If folks agree it's a good shape, it's still time to ship on 5.5.

Reviewed, tested, and it looks great.

#14 @ocean90
5 months ago

It looks like the patch reverts parts of #48585 / #49003. Attached two before/after screenshots with the Midnight scheme.

#15 @youknowriad
5 months ago

@ocean90 Yes, that's intended, here's my reasoning for adapting the Midnight scheme buttons to the theme color;

  • We already use the red button for the primary button
  • The color has enough contrast
  • While I understand the "color look like a danger button" argument, it also applies to the primary button right now in trunk, IMO we should be consistent.

That said, it's just a variable change, I'd prefer adapting personally but I don't mind reverting too much.

Last edited 5 months ago by youknowriad (previous) (diff)

#16 @youknowriad
5 months ago

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

In 48342:

Administration: Adapts the secondary buttons colors to the admin schemes.

The admin schemes that don't meet the contrast guidelines are excluded from this change.

Props mapk, michaelarestad, mcsf, ocean90.
Closes #50537.

#17 @youknowriad
5 months ago

I think some colors in the admin schemes need to be rethought, I created this #50575 to continue the discussion.

Last edited 5 months ago by SergeyBiryukov (previous) (diff)

#18 @SergeyBiryukov
5 months ago

  • Milestone changed from Awaiting Review to 5.5
Note: See TracTickets for help on using tickets.