Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Try: Show background and increased opacity on disabled switcher #13721

Merged
merged 2 commits into from
Feb 12, 2019

Conversation

jasmussen
Copy link
Contributor

@jasmussen jasmussen commented Feb 7, 2019

This one goes out to all the @chrivanpatten's in the crowd!

Well, specifically, it is in response to this conversation we had here: #11669 (comment)

The problem: we always show the Block Switcher interface, even if no transformations are available. When no transformations are available, the buttonis disabled, which usually means it needs to have very contrast.

However in this case, the block switcher also works as a block type indicator, which makes it valuable to be able to see the icon in question.

This PR tries to marry the two challenges and shows the button as disabled adding a light gray background, but still increasing the opacity.

Thoughts?

Before:

screenshot 2019-02-07 at 09 59 37

After:

screenshot 2019-02-07 at 10 13 08

screenshot 2019-02-07 at 10 13 19

This one goes out to all the @chrivanpatten's in the crowd!

Well, specifically, it is in response to this conversation we had here: #11669 (comment)

The problem: we always show the Block Switcher interface, even if no transformations are available. When no transformations are available, the buttonis _disabled_, which usually means it needs to have very contrast.

However in this case, the block switcher also works as a block type indicator, which makes it valuable to be able to see the icon in question.

This PR tries to marry the two challenges and shows the button as disabled adding a light gray background, but still increasing the opacity.

Thoughts?
@jasmussen jasmussen added [Type] Enhancement A suggestion for improvement. [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). Needs Accessibility Feedback Need input from accessibility labels Feb 7, 2019
@jasmussen jasmussen self-assigned this Feb 7, 2019
@jasmussen jasmussen requested review from chrisvanpatten and a team February 7, 2019 09:12
@youknowriad
Copy link
Contributor

I personally don't understand that this is a disabled button :(

@gziolo
Copy link
Member

gziolo commented Feb 7, 2019

I personally don't understand that this is a disabled button :(

It needs to be disabled when there is no action available to make sure it's skipped when using keyboard navigation, right?

@gziolo gziolo added this to the 5.1 (Gutenberg) milestone Feb 7, 2019
@jasmussen jasmussen added the Needs Design Feedback Needs general design feedback. label Feb 7, 2019
@jasmussen
Copy link
Contributor Author

Added "needs design feedback" label to get some additional thoughts.

@ZebulanStanphill
Copy link
Member

On my laptop screen, I can barely tell that the button has a light gray background; if my screen is tilted a bit too much in one direction, it just looks white.

@kjellr
Copy link
Contributor

kjellr commented Feb 7, 2019

I like this approach. I do think it's not 100% clear that this is disabled, but that could be partially because the gray is so light. Perhaps we darken slightly to $light-gray-200? In any case, I think after hovering over it once, I'd get the message.

As you note, this area serves two purposes:

  1. Primarily, it tells you what block you're currently using.
  2. Secondarily, it allows you to swap it for a different block, or to choose a different block style.

The current, low opacity approach is designed primarily to address 2. It actually hurts 1 because it makes the indicator harder to see.

This new approach keeps 1 working well, and also addresses 2 as a secondary concern. That makes sense to me.

@jasmussen
Copy link
Contributor Author

Great feedback, everyone. I tried darkening the background color, that helps. I also tried to make the icon itself monochrome, which I forgot from the conversation we had on the initial thread. I think that helps a lot:

screenshot 2019-02-08 at 10 06 57

screenshot 2019-02-08 at 10 07 03

When will the YouTube icon be red, then, you ask? Glad you asked. It still has its colors in the Block Library:

screenshot 2019-02-08 at 10 08 54

What do you think?

@youknowriad
Copy link
Contributor

@jasmussen That's better for me 👍 Thanks for the updates

@jasmussen
Copy link
Contributor Author

Awesome, thanks. I'm wondering why the tests fail?

@youknowriad
Copy link
Contributor

@jasmussen Probably unrelated (related to animations), I'll look at those separately.

Copy link
Contributor

@kjellr kjellr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

screen shot 2019-02-11 at 8 06 02 am

One small note is that the placeholder and the disabled switcher colors are not exactly identical ($dark-opacity-light-200 vs. $light-gray-200) in my browser at least. This nags at me just a bit, but that may just be me being too nitpicky. Inspecting these shows that they're only a couple hex values off, so it's probably not a huge deal.

@jasmussen
Copy link
Contributor Author

Thank you Kjell. Going to merge this one.

I've been thinking that we should revisit the placeholder pattern separately in a holistic way to see how we can improve it. I think there's some simplification we can do here that might make the gray unnecessary.

@jasmussen jasmussen merged commit 33e6d42 into master Feb 12, 2019
@jasmussen jasmussen deleted the try/gray-bg-disabled-block-switcher branch February 12, 2019 08:17
youknowriad pushed a commit that referenced this pull request Mar 6, 2019
* Try: Show background and increased opacity on disabled switcher

This one goes out to all the @chrivanpatten's in the crowd!

Well, specifically, it is in response to this conversation we had here: #11669 (comment)

The problem: we always show the Block Switcher interface, even if no transformations are available. When no transformations are available, the buttonis _disabled_, which usually means it needs to have very contrast.

However in this case, the block switcher also works as a block type indicator, which makes it valuable to be able to see the icon in question.

This PR tries to marry the two challenges and shows the button as disabled adding a light gray background, but still increasing the opacity.

Thoughts?

* Try darker gray and monochrome icon.
youknowriad pushed a commit that referenced this pull request Mar 6, 2019
* Try: Show background and increased opacity on disabled switcher

This one goes out to all the @chrivanpatten's in the crowd!

Well, specifically, it is in response to this conversation we had here: #11669 (comment)

The problem: we always show the Block Switcher interface, even if no transformations are available. When no transformations are available, the buttonis _disabled_, which usually means it needs to have very contrast.

However in this case, the block switcher also works as a block type indicator, which makes it valuable to be able to see the icon in question.

This PR tries to marry the two challenges and shows the button as disabled adding a light gray background, but still increasing the opacity.

Thoughts?

* Try darker gray and monochrome icon.
This was referenced Apr 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). Needs Accessibility Feedback Need input from accessibility Needs Design Feedback Needs general design feedback. [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants