WordPress.org

Make WordPress Core

Opened 17 months ago

Closed 17 months ago

Last modified 10 months 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 17 months ago.
video demostrating the issue
48598.diff (858 bytes) - added by larrach 17 months ago.
patch1
test-patch.png (135.9 KB) - added by audrasjb 17 months ago.
Testing 48598.diff with the 4 first color schemes
test-patch2.2.png (133.0 KB) - added by audrasjb 17 months ago.
Testing 48598.diff with the 3 last color schemes
test-patch2.png (133.0 KB) - added by audrasjb 17 months ago.
Testing 48598.diff with the 3 last color schemes
1.png (6.4 KB) - added by azaozz 17 months ago.
(Adding couple more screenshots to better illustrate the issue.)
2.png (6.6 KB) - added by azaozz 17 months ago.
48598.2.diff (3.1 KB) - added by melchoyce 17 months ago.
Think between this and my patch in #48585 I've fixed this.
48598.3.diff (930 bytes) - added by audrasjb 17 months ago.
Patch refreshed

Download all attachments as: .zip

Change History (34)

@studiotwee
17 months ago

video demostrating the issue

#2 @studiotwee
17 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
17 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
17 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
17 months ago

  • Description modified (diff)

Done.

#6 @afercia
17 months ago

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

#7 @audrasjb
17 months ago

#48661 was marked as a duplicate.

#8 @audrasjb
17 months ago

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

@larrach
17 months ago

patch1

#9 @larrach
17 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
17 months ago

Testing 48598.diff with the 4 first color schemes

@audrasjb
17 months ago

Testing 48598.diff with the 3 last color schemes

@audrasjb
17 months ago

Testing 48598.diff with the 3 last color schemes

#10 @audrasjb
17 months ago

  • Keywords has-screenshots added

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

#11 @afercia
17 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 17 months ago by afercia (previous) (diff)

#13 @azaozz
17 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
17 months ago

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

@azaozz
17 months ago

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


17 months ago

#15 @audrasjb
17 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.


17 months ago

@melchoyce
17 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.


17 months ago

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


17 months ago

#19 @SergeyBiryukov
17 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
17 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
17 months ago

Patch refreshed

#21 @audrasjb
17 months ago

Ah great, thanks Sergey! :D

#22 @afercia
17 months ago

  • Keywords needs-dev-note added

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


14 months ago

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


14 months ago

#25 @desrosj
10 months ago

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