fix(code-snippet): add light styles for all variations and update storybook knobs #4342

jendowns · 2019-10-15T19:10:19Z

Proposal to add light styles to all variations of the CodeSnippet, since the light prop is available to use for the component but currently only applies styles for the inline variation.

Changelog

Changed

update .bx--snippet--light styles to apply to all variations of the CodeSnippet (and also update button hover colors to accurately reflect these changes) -- NOTE: I realize the original inline + light variation used $field-01 props, however the $ui-* tokens I picked out are the same and are good foils for the non-light $ui-* tokens used.
update story to include light prop knobs for all variations of CodeSnippet

Testing / Reviewing

Check out the carbon-components-react Netlify deployment for this PR & use the knobs to toggle the light state for each CodeSnippet variation.

packages/components/src/components/code-snippet/_code-snippet.scss

netlify · 2019-10-15T19:21:30Z

Deploy preview for the-carbon-components ready!

Built with commit b21ea99

https://deploy-preview-4342--the-carbon-components.netlify.com

netlify · 2019-10-15T19:25:21Z

Deploy preview for carbon-elements ready!

Built with commit b21ea99

https://deploy-preview-4342--carbon-elements.netlify.com

netlify · 2019-10-15T19:25:59Z

Deploy preview for carbon-components-react failed.

Built with commit b21ea99

https://app.netlify.com/sites/carbon-components-react/deploys/5da6199dc05f250007807620

netlify · 2019-10-15T19:37:13Z

Deploy preview for carbon-components-react ready!

Built with commit b21ea99

https://deploy-preview-4342--carbon-components-react.netlify.com

netlify · 2019-10-15T19:37:51Z

Deploy preview for carbon-elements ready!

Built with commit baf682e

https://deploy-preview-4342--carbon-elements.netlify.com

netlify · 2019-10-15T19:45:00Z

Deploy preview for the-carbon-components ready!

Built with commit baf682e

https://deploy-preview-4342--the-carbon-components.netlify.com

asudoh

LGTM 👍 - Thanks @jendowns!

emyarod

looks good to me, pending design review

dakahn

WCAG 1.4.11 requires a 3:1 contrast ratio for non-text interface elements and their adjacent colors. Common ways of meeting this requirement are 3:1 ratio contrast borders or improving the contrast between the interface element and it's adjacent colors (page background, parent background etc).

Currently our code snippet does not meet these requirements. While most code-snippets are not considered "interface components" ours are since they are interactable.

This situation is made worse in lighter themes by allowing an even lower contrast variant background color for these interface components.

jendowns · 2019-10-16T19:13:00Z

@dakahn could you describe which parts of the light variation of the CodeSnippet you are referring to?

dakahn · 2019-10-16T19:15:31Z

@jendowns The interface element's background color must meet a 3:1 contrast ratio against adjacent colors.

jendowns · 2019-10-16T19:19:47Z

@dakahn 🤔 So do you mean that a "light" CodeSnippet technically can't really exist, because that background would be too close to the $ui-background?

dakahn · 2019-10-16T19:22:29Z

@dakahn precisely. Since it's interactable it must meet 1.4.11 contrast requirements (3:1) for non-text interface elements. Worth noting -- it currently doesn't meet this requirement today.

jendowns · 2019-10-16T19:26:27Z

@dakahn Are we definitely sure we should consider all variations of the CodeSnippet interactive?

I can definitely see it for the inline one because you click it to copy etc.
But what about the single and multi-line versions? Technically they have interactive buttons, but the body of the components (where code is) isn't interactive? 🤔

jendowns · 2019-10-16T19:30:42Z

Also something else to consider... in our case, we are using the light prop to distinguish the CodeSnippet from a ui-01 or similar background where it would normally have been the same color as the background color.

Here's an example of a light + inline snippet with some context (in a panel):

(so maybe what you are seeing in the storybook isn't a good in-product example of how it would be used anyway? 🤔 )

dakahn

Okay! If we're only using light on dark themes and we have docs and guidance coming to back that up. I'm cool with this. 👍

jendowns · 2019-10-17T16:21:57Z

@dakahn Per our Slack convo, I added a conditional wrapper class & a light prop message to help people use the prop correctly. So if you look at the CodeSnippet variations in different themes and toggle the light knob, you'll see something like this:

dakahn · 2019-10-17T16:45:16Z

@joshblack How do you feel about this in-Storybook warning. I thought this would be cool as a stop-gap until we figure out how we're doing guidance in this thing.

joshblack · 2019-10-17T17:12:40Z

I'm down if it helps 👍 what do you think @jillianhowarth? Is this okay from the design perspective to help educate devs who might mistakenly use this prop?

jillianhowarth · 2019-10-17T18:14:09Z

packages/react/src/components/CodeSnippet/CodeSnippet-story.js

@@ -48,12 +50,23 @@ const props = {
  }),
 };

+const lightPropMessage = (
+  <small style={{ display: 'block', paddingBottom: '1rem' }}>
+    {'Make sure to use a different background color for the '}


This copy is unclear - we should clarify that they should be using the prop with certain themes

Hey @jillianhowarth the prop is currently available regardless of theme. I'm happy to change the copy though -- what should it be?

It's safe to use with all themes actually. It's working correctly in the White and Gray 10 themes as well.

If we want to get more specific/technical we could say something like:

The snippet container should never be the same color as the page background.

Do not use the light variant on $ui-background or $ui-02.

aagonzales

The hover of the light snippet should be using hover-ui as well so it doesn't blind in.

jendowns · 2019-10-17T18:43:56Z

Thanks @aagonzales & @jillianhowarth!

I have updated that copy & you should see it reflected in the netlify preview soon.
But as for using $hover-ui for the hover color... currently it looks great for white and g10 themes but it is very hard to see for the dark themes g90 and g100 🤔 Seems to be because $hover-ui is very close to $ui-02 (the "light" variant bg color).

Here's g90 for example (I'm hovering over the "copy" button in the upper right of the multi line snippet):

aagonzales · 2019-10-17T19:00:22Z

Hmm I wonder if the new color updates made them too similar. hover-ui is the appropriate token so I'm wondering if we need to update that token value. Let me look at it in relation to some other things.

Or maybe its time to add an hover-ui-light token specifically for the light variants 🤔

These are the options

jendowns · 2019-10-17T21:48:53Z

@aagonzales Personally I'm leaning more toward the new token just so it doesn't affect any other components. 😅 Is that okay? If so, I could go ahead and add the new token in this PR if you happen to know what colors it should be per theme?

aagonzales · 2019-10-17T22:20:01Z

@jendowns actually I just talked to Josh about this, could we actually do a lighten/darken effect for the hover for these? That's the direction we're trying to go with hovers in v11 so we can get ahead of the game without adding a new token.

White and gray 10: darken by 8%
Gray 90 and 100: lighten by 8%

jendowns · 2019-10-21T15:59:12Z

Thank you @aagonzales! That makes sense. And for the v9 theme, should the color be darkened by 8% as well?

aagonzales · 2019-10-21T16:32:52Z

Hmm for V9, the icon color darkens not the background. I think its also controlled by a different token in v9 if I'm not mistaken.

joshblack · 2019-10-21T20:08:10Z

@jendowns sorry about all the changes here! Let me know if you need anything else from us 👀

joshblack · 2019-10-22T19:29:35Z

Chatted with @jendowns, I think we'll move forward with using the value out of the theme map and lighten/darken that instead of the property while we figure out the issue around CSS Custom Properties with lighten/darken values over in #4426

packages/components/src/globals/scss/_theme-tokens.scss

packages/components/src/components/code-snippet/_code-snippet.scss

Co-Authored-By: Josh Black <josh@josh.black>

…tion Co-Authored-By: Josh Black <josh@josh.black>

Jen Downs added 3 commits October 15, 2019 14:00

fix(code-snippet): add light styles for all variations

b628bff

docs(code-snippet): add light prop knob for single and multi-line

5bfa758

docs(code-snippet): fix typo in light prop description

b21ea99

jendowns requested a review from a team as a code owner October 15, 2019 19:10

ghost requested review from aledavila and asudoh October 15, 2019 19:10

jendowns commented Oct 15, 2019

View reviewed changes

packages/components/src/components/code-snippet/_code-snippet.scss Show resolved Hide resolved

Merge branch 'master' into code-snippet--light

f6acf52

jendowns mentioned this pull request Oct 15, 2019

The CodeSnippet component light theme isn't working carbon-design-system/ibm-security#147

Closed

jendowns changed the title ~~Code snippet light~~ fix(code-snippet): add light styles for all variations and update storybook knobs Oct 15, 2019

jendowns mentioned this pull request Oct 15, 2019

Styles - incorrectly passing hex values (instead of rgb values) to rgba() color function #4345

Closed

1 task

asudoh approved these changes Oct 15, 2019

View reviewed changes

emyarod approved these changes Oct 16, 2019

View reviewed changes

emyarod requested a review from a team October 16, 2019 13:30

ghost requested review from jillianhowarth and removed request for a team October 16, 2019 13:31

dakahn suggested changes Oct 16, 2019

View reviewed changes

dakahn approved these changes Oct 16, 2019

View reviewed changes

Jen Downs added 2 commits October 17, 2019 11:26

docs(code-snippet): adjust template strings in story for prettier

c94ed8e

docs(code-snippet): adjust light prop message styling

e9500f5

jillianhowarth reviewed Oct 17, 2019

View reviewed changes

docs(code-snippet): update light prop usage message

8fa40d4

aagonzales reviewed Oct 17, 2019

View reviewed changes

Merge branch 'master' into code-snippet--light

2f4282e

jendowns added 2 commits October 21, 2019 16:43

Merge branch 'master' into code-snippet--light

f9f2e52

Merge branch 'master' into code-snippet--light

00fc911

fix(theme-tokens): add hover-ui-light theme token and function

6d85473

joshblack suggested changes Oct 24, 2019

View reviewed changes

packages/components/src/globals/scss/_theme-tokens.scss Outdated Show resolved Hide resolved

packages/components/src/globals/scss/_theme-tokens.scss Outdated Show resolved Hide resolved

packages/components/src/components/code-snippet/_code-snippet.scss Outdated Show resolved Hide resolved

jendowns and others added 5 commits October 24, 2019 10:00

fix(theme-tokens): use feature-flags-enabled in get-light-value function

0aa8d97

Co-Authored-By: Josh Black <josh@josh.black>

fix(theme-tokens): use custom-property-prefix in get-light-value func…

9bed966

…tion Co-Authored-By: Josh Black <josh@josh.black>

Merge branch 'master' into code-snippet--light

450626e

fix(code-snippet): flatten all light variation styles

d2e186b

Merge branch 'master' into code-snippet--light

baf682e

joshblack approved these changes Oct 25, 2019

View reviewed changes

joshblack added status: ready to merge 🎉 and removed status: visual review 🔍 labels Oct 25, 2019

joshblack merged commit d6498e9 into carbon-design-system:master Oct 25, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(code-snippet): add light styles for all variations and update storybook knobs #4342

fix(code-snippet): add light styles for all variations and update storybook knobs #4342

jendowns commented Oct 15, 2019

netlify bot commented Oct 15, 2019

netlify bot commented Oct 15, 2019

netlify bot commented Oct 15, 2019

netlify bot commented Oct 15, 2019 •

edited

Loading

netlify bot commented Oct 15, 2019 •

edited

Loading

netlify bot commented Oct 15, 2019 •

edited

Loading

asudoh left a comment

emyarod left a comment

dakahn left a comment •

edited

Loading

jendowns commented Oct 16, 2019

dakahn commented Oct 16, 2019

jendowns commented Oct 16, 2019

dakahn commented Oct 16, 2019

jendowns commented Oct 16, 2019

jendowns commented Oct 16, 2019

dakahn left a comment

jendowns commented Oct 17, 2019

dakahn commented Oct 17, 2019

joshblack commented Oct 17, 2019 •

edited

Loading

jillianhowarth Oct 17, 2019

jendowns Oct 17, 2019

aagonzales Oct 17, 2019 •

edited

Loading

aagonzales left a comment

jendowns commented Oct 17, 2019

aagonzales commented Oct 17, 2019 •

edited

Loading

jendowns commented Oct 17, 2019

aagonzales commented Oct 17, 2019

jendowns commented Oct 21, 2019

aagonzales commented Oct 21, 2019 •

edited

Loading

joshblack commented Oct 21, 2019

joshblack commented Oct 22, 2019

fix(code-snippet): add light styles for all variations and update storybook knobs #4342

fix(code-snippet): add light styles for all variations and update storybook knobs #4342

Conversation

jendowns commented Oct 15, 2019

Changelog

Testing / Reviewing

netlify bot commented Oct 15, 2019

netlify bot commented Oct 15, 2019

netlify bot commented Oct 15, 2019

netlify bot commented Oct 15, 2019 • edited Loading

netlify bot commented Oct 15, 2019 • edited Loading

netlify bot commented Oct 15, 2019 • edited Loading

asudoh left a comment

Choose a reason for hiding this comment

emyarod left a comment

Choose a reason for hiding this comment

dakahn left a comment • edited Loading

Choose a reason for hiding this comment

jendowns commented Oct 16, 2019

dakahn commented Oct 16, 2019

jendowns commented Oct 16, 2019

dakahn commented Oct 16, 2019

jendowns commented Oct 16, 2019

jendowns commented Oct 16, 2019

dakahn left a comment

Choose a reason for hiding this comment

jendowns commented Oct 17, 2019

dakahn commented Oct 17, 2019

joshblack commented Oct 17, 2019 • edited Loading

jillianhowarth Oct 17, 2019

Choose a reason for hiding this comment

jendowns Oct 17, 2019

Choose a reason for hiding this comment

aagonzales Oct 17, 2019 • edited Loading

Choose a reason for hiding this comment

aagonzales left a comment

Choose a reason for hiding this comment

jendowns commented Oct 17, 2019

aagonzales commented Oct 17, 2019 • edited Loading

jendowns commented Oct 17, 2019

aagonzales commented Oct 17, 2019

jendowns commented Oct 21, 2019

aagonzales commented Oct 21, 2019 • edited Loading

joshblack commented Oct 21, 2019

joshblack commented Oct 22, 2019

netlify bot commented Oct 15, 2019 •

edited

Loading

netlify bot commented Oct 15, 2019 •

edited

Loading

netlify bot commented Oct 15, 2019 •

edited

Loading

dakahn left a comment •

edited

Loading

joshblack commented Oct 17, 2019 •

edited

Loading

aagonzales Oct 17, 2019 •

edited

Loading

aagonzales commented Oct 17, 2019 •

edited

Loading

aagonzales commented Oct 21, 2019 •

edited

Loading