WordPress.org

Make WordPress Core

Opened 8 months ago

Closed 7 months ago

Last modified 11 days ago

#48598 closed defect (bug) (fixed)

:active button color issue in all color schemes except for "Default"

Reported by: studiotwee Owned by: audrasjb
Milestone: 5.3.1 Priority: normal
Severity: normal Version: 5.3
Component: Administration Keywords: 5-3-admin-css-changes has-patch has-screenshots has-dev-note
Focuses: ui, accessibility Cc:

Description (last modified by afercia)

hi, in the latest update of WP the buttons had had a thick outline, which is great in terms of a11y. what’s less succesful is the :active state when selected the Midnight color scheme in wp-admin. See attached video demonstrating the issue.

BTW it's fine on the default color scheme.

Attachments (9)

2019-11-13_11-40-01.mp4 (119.5 KB) - added by studiotwee 8 months ago.
video demostrating the issue
48598.diff (858 bytes) - added by larrach 8 months ago.
patch1
test-patch.png (135.9 KB) - added by audrasjb 8 months ago.
Testing 48598.diff with the 4 first color schemes
test-patch2.2.png (133.0 KB) - added by audrasjb 8 months ago.
Testing 48598.diff with the 3 last color schemes
test-patch2.png (133.0 KB) - added by audrasjb 8 months ago.
Testing 48598.diff with the 3 last color schemes
1.png (6.4 KB) - added by azaozz 7 months ago.
(Adding couple more screenshots to better illustrate the issue.)
2.png (6.6 KB) - added by azaozz 7 months ago.
48598.2.diff (3.1 KB) - added by melchoyce 7 months ago.
Think between this and my patch in #48585 I've fixed this.
48598.3.diff (930 bytes) - added by audrasjb 7 months ago.
Patch refreshed

Download all attachments as: .zip

Change History (34)

@studiotwee
8 months ago

video demostrating the issue

#2 @studiotwee
8 months ago

The same issue is here with colorscheme Sunrise, Ectoplasma, Coffee, Blue and Ocean. It's on all color schemes, except for default.

#3 follow-up: @afercia
8 months ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 5.3.1

Good catch @studiotwee, thank you.

#4 in reply to: ↑ 3 @studiotwee
8 months ago

Replying to afercia:

Good catch @studiotwee, thank you.

very welcome. Can you do me a favor and remove the mp4 link from my initial ticket? I didnt know I could upload files with my ticket, which, in hindsight, is a very logical thing to be able to do.

#5 @afercia
8 months ago

  • Description modified (diff)

Done.

#6 @afercia
8 months ago

  • Keywords 5-3-admin-css-changes added

#7 @audrasjb
8 months ago

#48661 was marked as a duplicate.

#8 @audrasjb
8 months ago

  • Owner set to audrasjb
  • Severity changed from major to normal
  • Status changed from new to accepted

@larrach
8 months ago

patch1

#9 @larrach
8 months ago

  • Keywords has-patch added; needs-patch removed

Hi,
I made a patch. I created a new color variable from $text-color so the patch will work with possible future color schemes.
patch test on all color schemes.

@audrasjb
8 months ago

Testing 48598.diff with the 4 first color schemes

@audrasjb
8 months ago

Testing 48598.diff with the 3 last color schemes

@audrasjb
8 months ago

Testing 48598.diff with the 3 last color schemes

#10 @audrasjb
8 months ago

  • Keywords has-screenshots added

Thanks @larrach, I tested the patch and it looks good on my side.

#11 @afercia
8 months ago

  • Keywords needs-design-feedback added

Thanks for the patch @larrach 👋

I'm not sure the background color change should be kept though. That was a mistake introduced in [46423]

Instead, I'd tend to think the :active style should be consistent with the Gutenberg one. There's an inconsistency in Gutenberg as well, as the style varies across the color schemes. I just created an issue, thinking there should be some quick design feedback. See https://github.com/WordPress/gutenberg/issues/18577

Note: I think we can't use a new variable, as alternate color schemes provided by plugins would miss the variable. To test, install this plugin: https://wordpress.org/plugins/admin-color-schemes/

Last edited 8 months ago by afercia (previous) (diff)

#13 @azaozz
7 months ago

  • Summary changed from :active button color issue when Midnight color scheme has been selected to :active button color issue in all color schemes except for "Default"

@azaozz
7 months ago

(Adding couple more screenshots to better illustrate the issue.)

@azaozz
7 months ago

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


7 months ago

#15 @audrasjb
7 months ago

  • Keywords needs-design-feedback removed

We addressed this ticket during today's bug scrub, and given the short timeframe for 5.3.1, the current feeling would be to solve the issue with the current patch and to take time for a more global change on secondary buttons.

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


7 months ago

@melchoyce
7 months ago

Think between this and my patch in #48585 I've fixed this.

This ticket was mentioned in Slack in #design by melchoyce. View the logs.


7 months ago

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


7 months ago

#19 @SergeyBiryukov
7 months ago

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

In 46817:

Accessibility: Administration: Correct active button color for the alternative color schemes.

Follow-up to [46815].

Props melchoyce, ryelle, larrach, audrasjb, studiotwee, afercia, azaozz.
Fixes #48598.

#20 @SergeyBiryukov
7 months ago

In 46818:

Accessibility: Administration: Correct active button color for the alternative color schemes.

Follow-up to [46815].

Props melchoyce, ryelle, larrach, audrasjb, studiotwee, afercia, azaozz.
Merges [46817] to the 5.3 branch.
Fixes #48598.

@audrasjb
7 months ago

Patch refreshed

#21 @audrasjb
7 months ago

Ah great, thanks Sergey! :D

#22 @afercia
7 months ago

  • Keywords needs-dev-note added

This ticket was mentioned in Slack in #accessibility by afercia. View the logs.


4 months ago

This ticket was mentioned in Slack in #accessibility by afercia. View the logs.


4 months ago

#25 @desrosj
11 days ago

  • Keywords has-dev-note added; needs-dev-note removed
Note: See TracTickets for help on using tickets.