WordPress.org

Make WordPress Core

Opened 11 months ago

Closed 4 months ago

Last modified 3 months ago

#48709 closed defect (bug) (fixed)

Disabled button doesn't look disabled

Reported by: drw158 Owned by: audrasjb
Milestone: 5.5 Priority: normal
Severity: normal Version:
Component: Administration Keywords: has-patch has-dev-note
Focuses: ui, administration Cc:

Description

Midnight:

http://cldup.com/4lUDc58Vns.png

Default:

http://cldup.com/V3xFKZ3jlY.png

The Delete Permanently button is disabled because I haven't selected anything yet.

The problem is that it doesn't look disabled at all. The problem seems more prominent in the midnight color scheme. The text color seems different for some reason.

To reproduce:

  • Go to Media
  • Click Bulk Select

Attachments (11)

Screenshot 2019-11-18 at 22.27.26.png (88.5 KB) - added by afercia 11 months ago.
48709.diff (989 bytes) - added by melchoyce 11 months ago.
Not working for the Light color scheme, which takes a different approach to primary buttons. @ryelle can you take a look?
c35b6b54f3439e69a133deae6c499a4b.gif (22.4 KB) - added by audrasjb 6 months ago.
Unique CSS styles for all color schemes and buttons types
48709.2.diff (617 bytes) - added by larrach 4 months ago.
Apply disabled css style proposed by audrasjb to all colors schemes
48709 disabled comparison.png (25.2 KB) - added by afercia 4 months ago.
Screen Shot 2020-07-02 at 16.28.40.png (1014.9 KB) - added by nrqsnchz 4 months ago.
48709.3.diff (601 bytes) - added by audrasjb 4 months ago.
Patch refresh with color proposed by @melchoyce
Capture d’écran 2020-07-06 à 13.55.17.png (32.2 KB) - added by audrasjb 4 months ago.
Testing 48709.3.diff (same styles for all alternate color schemes)
48709.4.diff (1.4 KB) - added by audrasjb 4 months ago.
Include default styles for both primary and secondary buttons to the changes
48709.5.diff (1.5 KB) - added by youknowriad 4 months ago.
Remove disabled state from the admin scheme specific styles.
Capture d’écran 2020-07-06 à 22.00.38.png (116.2 KB) - added by audrasjb 4 months ago.
Testing 48709.5.diff

Download all attachments as: .zip

Change History (47)

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


11 months ago

#2 @SergeyBiryukov
11 months ago

  • Component changed from General to Administration

Related: #48598

#3 @afercia
11 months ago

Not sure this is related to #48598 or any other change in WordPress 5.3. It's just the usual disabled style for primary buttons (see screenshot below from WordPress 5.2). Not saying it wouldn't benefit from some improvements.

#4 @kennethroberson5556
11 months ago

Totally agreed. I felt confused working on a recent site, too. The color schemes are very odd. In my opinion, it shouldn't be only enabled but also highlighted when the pictures are selected.

Last edited 6 months ago by audrasjb (previous) (diff)

#5 @melchoyce
11 months ago

Yeah, I'd say this is an issue with the core disabled button styling that's exacerbated by color schemes. I've gotten the style working better for most schemes, but I agree we should address the underlying issue of having better disabled primary button styles.

@melchoyce
11 months ago

Not working for the Light color scheme, which takes a different approach to primary buttons. @ryelle can you take a look?

#6 @afercia
11 months ago

Worth noting that introducing new color variables would need a dev note. There are plugins that extend the color schemes e.g. and they should be informed there are new variables to use.

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


9 months ago

#8 @joedolson
9 months ago

On the accessibility side, we don't have any issues with the design team making a call on changing this. Contrast requirements don't apply to disabled interface controls, so we don't have any concerns.

#9 @valentinbora
8 months ago

  • Focuses administration added
  • Keywords has-patch needs-dev-note added
  • Milestone changed from Awaiting Review to 5.5

#10 @audrasjb
6 months ago

  • Keywords needs-refresh needs-design-feedback added

@melchoyce what about using the same CSS styles for all the admin color schemes, including default and alternative color schemes, and apply them to both primary and secondary buttons?

See proposal below :)

@audrasjb
6 months ago

Unique CSS styles for all color schemes and buttons types

#11 @audrasjb
6 months ago

CSS declarations used in the above screenshot:

background: #eee;
border-color: #aaa;
color: #aaa;
cursor: not-allowed;

#12 @audrasjb
6 months ago

  • Owner set to audrasjb
  • Status changed from new to accepted

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


4 months ago

@larrach
4 months ago

Apply disabled css style proposed by audrasjb to all colors schemes

#14 @melchoyce
4 months ago

We talked about this in today's #design triage.

Can we use this approach taken in Gutenberg? https://github.com/WordPress/gutenberg/pull/23229

If not, I'd recommend using these colors instead:

color: #606A73;
border-color: #a2aab2;
background: #fbfbfc;

Drawn from https://codepen.io/hugobaeta/full/RNOzoV/.

#15 @afercia
4 months ago

I'd argue that, although disabled controls don't have color contrast requirements, color alone can't be used to distinguish a disabled control from an enabled one.

For a user who can't distinguish colors, or with low vision, etc. the Gutenberg implementation @melchoyce linked above is not ideal and should be reviewed. Basically, it doesn't differ that much from the core primary buttons.

There's only a change in the color background and the button text gets "opaque". This isn't sufficient to distinguish a disabled button from an enabled one.

See the attached screenshot, in order:

  • core primary buttons (not ideal)
  • latest Gutenberg plugin implementation (not ideal)
  • core default buttons: better, as the disabled buttons almost "fades out" and also its border is way less visible

#16 @melchoyce
4 months ago

I was thinking we'd still use cursor: not-allowed; — does that improve either methods at all?

#17 @afercia
4 months ago

Hm @melchoyce no, please :) Cursor values have well defined usages and that wouldn't be appropriate.

Aside: also the pointer cursor used on all buttons is, technically, non-standard. See #47171 (migrated from the WPCampus accessibility report on Gutenberg).

#18 @melchoyce
4 months ago

Thanks @afercia. Do you have any examples of good disabled buttons we could use for inspiration?

#19 @audrasjb
4 months ago

  • Keywords needs-refresh removed

Ok, thanks all for the discussion, very interesting 👍

So @afercia, concerning color schemes, what about using the approach I initially suggested and implemented in 48709.2.diff (same disabled buttons styles for all color schemes). Indeed, it simplifies the problem (and its maintainance) by using a unique style for each color scheme. I think it makes sense for that kind of not-usable UI components.

#20 @afercia
4 months ago

same disabled buttons styles for all color schemes

Yep, I was thinking at this. RIght now, the only "disabled" style that can be clearly distinguished is the one use for the default buttons.

@melchoyce from a design perspective: do primary buttons need to look "primary" also when disabled? Wouldn't be way simpler and usable to make them look like the default disabled buttons as @audrasjb suggested?

#21 @nrqsnchz
4 months ago

do primary buttons need to look "primary"

I think they do. Even if disabled, identifying the primary action provides a lot of context.

While I understand that color alone cannot be used to distinguish one from the other, it's important to note that there's also the factor of luminosity and saturation.

The gray disabled style proposed is using desaturation to convey the disabled meaning, while the ones in Core and Gutenberg use both a bit of desaturation and a change in luminosity.

Please see the attached image for a comparison with different filters emulating various sight and color conditions.

#22 @audrasjb
4 months ago

Thanks @nrqsnchz, but I still think all disabled buttons should have the same "disabled" look.
They are disabled functionalities (= not usable) that may benefit from looking disabled the same way for each color scheme.

The fact that those UI elements are disabled is more important than their supposed role.

@audrasjb
4 months ago

Patch refresh with color proposed by @melchoyce

@audrasjb
4 months ago

Testing 48709.3.diff (same styles for all alternate color schemes)

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


4 months ago

#24 @youknowriad
4 months ago

The last patch seems to use static colors for all admin schemes. That's fine but does that mean the styles should be removed from the sass files entirely and just included I the regular button stylesheet instead, or is there a specificity issue that needs to be taken into consideration?

#25 @youknowriad
4 months ago

Regardless of the outcome of this ticket, it would be cool to follow-up with a Gutenberg PR to update the @wordpress/components Buttons too and potentially add the disabled variation to this story https://wordpress.github.io/gutenberg/?path=/story/components-button--buttons

#26 @audrasjb
4 months ago

  • Keywords needs-design-feedback removed

In the future, indeed, we may also want to use those disabled styles on the default color scheme, but for now, the point of this ticket is only to address the issue that using opacities for each color schemes doesn't work very well. It doesn't touch default scheme’s behavior which works fine :)

Last edited 4 months ago by audrasjb (previous) (diff)

#27 @mapk
4 months ago

The Design Team just discussed this in today's triage https://wordpress.slack.com/archives/C02S78ZAL/p1594054528371100. There was agreement to render a primary button that is disabled with the default disabled state. No need to keep the primary styles.

#28 @youknowriad
4 months ago

In Gutenberg, a change to the default scheme is automatically applied to all schemes. The default scheme is basically considered just another color scheme. Any particular reason for the special treatment it gets in Core.

If we don't align here, it's going to be harder to backport this to Gutenberg.

#29 @audrasjb
4 months ago

In that case, I'd say we should also move those styles to core buttons.css stylesheet, as you proposed.
I can update the current patch if needed @youknowriad.

#30 @youknowriad
4 months ago

I'd be in favor of that personally (consistency)

#31 @audrasjb
4 months ago

  • Keywords needs-refresh added

Makes sense. Thanks for sharing your thoughts, I'm now convinced.
I'll add a new patch in few mins.

@audrasjb
4 months ago

Include default styles for both primary and secondary buttons to the changes

@youknowriad
4 months ago

Remove disabled state from the admin scheme specific styles.

#32 @youknowriad
4 months ago

Not sure if I'm testing things properly but the last patch doesn't seem to work for admin scheme colors. The background of the primary buttons is still "colored". I updated a patch with something that works for me but I'd appreciate confirmation.

#33 @audrasjb
4 months ago

No it's not you, @youknowriad, I messed up with my patch.
Testing yours right now.

@audrasjb
4 months ago

Testing 48709.5.diff

#34 @audrasjb
4 months ago

  • Keywords commit added; needs-refresh removed

@youknowriad I tested your patch and it works fine (see screenshot above).

Marking this for commit

#35 @whyisjake
4 months ago

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

In 48360:

Administration: Ensure that disabled buttons look disabled.

This change removes the disabled state from the admin scheme specific styles.

Fixes #48709.

Props drw158, SergeyBiryukov, afercia, kennethroberson5556, melchoyce, joedolson, valentinbora, audrasjb, larrach, nrqsnchz, youknowriad.

#36 @desrosj
3 months ago

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