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: Improve appearance of hover on colored backgrounds. #14501

Merged
merged 2 commits into from
Mar 20, 2019

Conversation

kjellr
Copy link
Contributor

@kjellr kjellr commented Mar 18, 2019

The current hover state uses solid colors. These work nicely in many cases, but can also end up clashing with the document background if the theme uses a non white/black background color. As initially discussed in #14145 (comment), switching to a color value that uses opacity could positively impact the appearance in these cases.

This PR switches the following color values to their closest opacity equivalents:

  • Light backgrounds: $light-gray-500 to $dark-opacity-light-500
  • Dark backgrounds: $dark-gray-600 to $light-opacity-light-400

The main design issue to note is that the breadcrumb background colors are unchanged in this PR. This is required to ensure proper text contrast of the breadcrumb text. This eliminates a bit of the visual connection between the left border and the breadcrumb. It feels a little clumsy to me, and I'm not totally sold on this solution for that reason.

Screenshots (Light Backgrounds)

Light-01

Light-02

Light-03

Light-04

Screenshots (Dark Backgrounds)
Dark-01
Dark-02
Dark-03
Dark-04

@kjellr kjellr added [Type] Enhancement A suggestion for improvement. Needs Design Feedback Needs general design feedback. labels Mar 18, 2019
@kjellr kjellr self-assigned this Mar 18, 2019
@kjellr kjellr requested a review from jasmussen March 18, 2019 18:55
@kjellr
Copy link
Contributor Author

kjellr commented Mar 18, 2019

Two general ideas for dealing with the mismatched breadcrumbs:

  1. Remove them entirely. There's been some talk about this in other threads — I'm not sure if they're actually needed any in the hovers state.
  2. Instead of using a gray value, switch them to pure white or black. In this case, at least we're not adding another gray color to the mix.

Here's a screenshot of how option 2 might shake out:

Frame

Copy link
Contributor

@jasmussen jasmussen left a comment

Choose a reason for hiding this comment

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

You're great. Dig all of this a lot.

I would note that we can actually treat these hover labels as tooltips, i.e. they don't have to appear quickly. They can take as long as a normal tooltip takes to appear, if we feel that could make them less intrusive. Not something necessary for this PR, but worth noting. Here's a little more: https://www.nngroup.com/articles/tooltip-guidelines/ — this one does suggest, however, that if we do treat this more like a tooltip, it should probably look like all other tooltips, which is an interesting thought experiment for a different day.

@kjellr
Copy link
Contributor Author

kjellr commented Mar 20, 2019

this one does suggest, however, that if we do treat this more like a tooltip, it should probably look like all other tooltips, which is an interesting thought experiment for a different day.

That is interesting. Thanks for sharing. I'll merge this as is, and think about modifying the tooltips separately.

Thanks for the review!

@kjellr kjellr merged commit 233446b into master Mar 20, 2019
@kjellr kjellr deleted the try/transparent-hover-border branch March 20, 2019 14:46
@kjellr kjellr added this to the 5.4 (Gutenberg) milestone Mar 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

2 participants