WordPress.org

Make WordPress Core

Opened 9 months ago

Closed 5 weeks ago

Last modified 4 weeks 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 9 months ago.
48709.diff (989 bytes) - added by melchoyce 8 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 4 months ago.
Unique CSS styles for all color schemes and buttons types
48709.2.diff (617 bytes) - added by larrach 5 weeks ago.
Apply disabled css style proposed by audrasjb to all colors schemes
48709 disabled comparison.png (25.2 KB) - added by afercia 5 weeks ago.
Screen Shot 2020-07-02 at 16.28.40.png (1014.9 KB) - added by nrqsnchz 5 weeks ago.
48709.3.diff (601 bytes) - added by audrasjb 5 weeks ago.
Patch refresh with color proposed by @melchoyce
Capture d’écran 2020-07-06 à 13.55.17.png (32.2 KB) - added by audrasjb 5 weeks ago.
Testing 48709.3.diff (same styles for all alternate color schemes)
48709.4.diff (1.4 KB) - added by audrasjb 5 weeks ago.
Include default styles for both primary and secondary buttons to the changes
48709.5.diff (1.5 KB) - added by youknowriad 5 weeks 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 5 weeks 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.


9 months ago

#2 @SergeyBiryukov
9 months ago

  • Component changed from General to Administration

Related: #48598

#3 @afercia
9 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
9 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 4 months ago by audrasjb (previous) (diff)

#5 @melchoyce
8 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
8 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
8 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.


6 months ago

#8 @joedolson
6 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
6 months ago

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

#10 @audrasjb
4 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
4 months ago

Unique CSS styles for all color schemes and buttons types

#11 @audrasjb
4 months ago

CSS declarations used in the above screenshot:

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

#12 @audrasjb
4 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.


8 weeks ago

@larrach
5 weeks ago

Apply disabled css style proposed by audrasjb to all colors schemes

#14 @melchoyce
5 weeks 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
5 weeks 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
5 weeks ago

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

#17 @afercia
5 weeks 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
5 weeks ago

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

#19 @audrasjb
5 weeks 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
5 weeks 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
5 weeks 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
5 weeks 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
5 weeks ago

Patch refresh with color proposed by @melchoyce

@audrasjb
5 weeks 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.


5 weeks ago

#24 @youknowriad
5 weeks 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
5 weeks 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
5 weeks 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 5 weeks ago by audrasjb (previous) (diff)

#27 @mapk
5 weeks 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
5 weeks 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
5 weeks 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
5 weeks ago

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

#31 @audrasjb
5 weeks 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
5 weeks ago

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

@youknowriad
5 weeks ago

Remove disabled state from the admin scheme specific styles.

#32 @youknowriad
5 weeks 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
5 weeks ago

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

@audrasjb
5 weeks ago

Testing 48709.5.diff

#34 @audrasjb
5 weeks 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
5 weeks 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
4 weeks ago

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