codeforamerica / honeycrisp-gem

A Rails gem with base styles and Javascript for Code for America products
http://honeycrisp.herokuapp.com
MIT License
24 stars 8 forks source link

Darkens unselected tabs by 5% in tab bar #249

Closed ash-rc closed 3 years ago

ash-rc commented 3 years ago

closes #152

It was originally proposed to set the background color of selected tabs to white in order to differentiate the selected tab from the unselected tabs in a tab bar.

After some discussion between @bengolder and @racheledelman , it was noted that the tab bar looks different in the styleguide than it does on most projects. The selected tab used $background-color, which was white for the styleguide, while the tab bar itself was set to $color-grey-light, so the selected tab stood out by accident in the styleguide and blended in in other places like GCF.

This PR sets the tab bar to use the background color darkened by 5%. A selected tab will show the normal background color. Below you can see the difference more clearly:

Before on GCF

Screen Shot 2021-02-26 at 11 25 14 AM Screen Shot 2021-02-26 at 11 25 04 AM

After on GCF

Screen Shot 2021-02-26 at 11 17 07 AM Screen Shot 2021-02-26 at 11 16 58 AM
racheledelman commented 3 years ago

Hm, this definitely looks good in your screenshots, where the selected tab matches the gray background color. However it still looks a little odd if the page background is another color, like on the documentation page, which is white. image

Do you think there's an easy way to peg the tab's color to the overall background color? That would be ideal, but I could see it being complicated, eg if the page background was one color, but the tab bar was inside an element with a different background. If not, this is still an improvement, because it differentiates selected and background tabs.

ash-rc commented 3 years ago

@racheledelman Huh! that's really weird actually, both selected and unselected tabs have their background color set to be the same as the background color for the page, see: https://github.com/codeforamerica/honeycrisp-gem/blob/d6142ee946e125c65f9fc0c237d12ecc1f4d50b0/app/assets/stylesheets/molecules/_tabs.scss#L11-L35

I'll look into the way Honeycrip is setting background color to see if it's displaying in a weird way

ash-rc commented 3 years ago

@racheledelman okay I think you actually solved it in your suggestion of why this would be difficult!

"That would be ideal, but I could see it being complicated, eg if the page background was one color, but the tab bar was inside an element with a different background."

all of the examples in the styleguide are within slabs, which have their own background color of white. Since the tabs go off of the page background color, they appear greyer than the slab color :(.

Do you think it's okay to go with this approach? I'm not sure how easy it would be to inherit the background color of the nearest parent but could try to see if someone else knows

racheledelman commented 3 years ago

I think that's okay, as long as it's easy to override.

ash-rc commented 3 years ago

@racheledelman should be! you can set a specific one in a customization.

I'll get this reviewed then!

bytheway875 commented 3 years ago

If you want to inherit the background color of the parent, you might be able to do background-color: inherit; https://www.w3schools.com/cssref/css_inherit.asp I don't think scss allows you to darken(inherit, 5%) though.

What if we created a variable called $unselected-tab-background where the default color is darken($background-color, 5%), but can be easily overriden by apps that want to do something different? I don't know that that's a pattern we use, but it reminds me of how Bootstrap recommends doing overrides -- override the variables instead of the class itself. That way, projects can set a default $unselected-tab-background that's consistent within their app usage.

ash-rc commented 3 years ago

@bytheway875 I like that suggestion! Would adding something like this to the _variables file fit what you're thinking?

$unselected-tab-background:   darken($color-background, 5%);
$selected-tab-background:     $color-background;
bytheway875 commented 3 years ago

@ash-rc Exactly!

ash-rc commented 3 years ago

@bytheway875 Awesome! Retook screenshots on GCF to make sure it didn't break anything. Also reformatted the variables file since it used both tabs and spaces for whitespace 😲

Screen Shot 2021-02-26 at 2 07 45 PM Screen Shot 2021-02-26 at 2 07 52 PM