Sass guidelines #1257

snide · 2018-10-23T18:08:07Z

Summary

Adds guidelines for our Sass layer with some examples of what the output is.

Checklist

~~- [ ] This was checked in mobile~~
~~- [ ] This was checked in IE11~~
~~- [ ] This was checked in dark mode~~ Has a similar issue to color guidelines where the colors don't appear correctly with theme.
~~- [ ] Any props added have proper autodocs~~

Documentation examples were added
A changelog entry exists and is marked appropriately
~~- [ ] This was checked for breaking changes and labeled appropriately~~
~~- [ ] Jest tests were updated or added to match the most common scenarios~~
~~- [ ] This was checked against keyboard-only and screenreader scenarios~~

snide · 2018-10-25T23:50:52Z

src-docs/src/views/guidelines/colors.js

-                        <EuiDescriptionList>
-                          <EuiDescriptionListTitle>
+                        <EuiDescriptionList compressed>
+                          <EuiDescriptionListTitle className="guidelineColor__title">


Added a small fix for the tooltip popup on the color page while i was in the guidelines.

The main thing that bugs me about the presentation of this section is that you have to hover to see the namings of the combinations. I'd really like to see the "Text" actually say the color name and then put a header before each section with the background color name and just leave the tooltip to show the contrast level.

Prolly outside the scope of this PR, but if you want to give it a shot, go for it.

snide · 2018-10-25T23:51:43Z

src/components/flex/_flex_grid.scss

@@ -51,3 +51,9 @@ $fractions: (
    }
  }
 }
+@include euiBreakpoint('xs','s') {


Noticed a bug where the negative margins from EuiFlexGrid was messing up mobile.

snide · 2018-10-25T23:53:26Z

src/global_styling/variables/_colors.scss

@@ -43,7 +43,7 @@ $euiColorVis9: #920000 !default;

 // Code

-$euiCodeBlockBackgroundColor: $euiColorLightestShade !default;
+$euiCodeBlockBackgroundColor: rgba(0,0,0,.05) !default;


Moved our code background color to an opacity. It fixes the issue where these things showed up ugly on things like callouts.

Is that enough and does it work in dark mode?

Or in a tooltip?

No, but i'll fix. I think I'll just remove this from the PR so it can be tackled separately.

So I had a solution for this with opacities, but it was a little hacky. I'm instead reverting the color change so I can merge this (but keeping some of the more general bugs we had with inline code in EuiText). I'll discuss my hacky solution with you another time. :)

snide · 2018-10-25T23:54:55Z

src-docs/src/views/guidelines/sass.js

@@ -0,0 +1,712 @@
+import React from 'react';
+import lightColors from '!!sass-vars-to-js-loader?preserveKeys=true!../../../../src/global_styling/variables/_colors.scss';


@chandlerprall Is there any way I can get the theme context in the lower level pages. To make this stuff work in dark mode I'd need to know what theme this page is viewing.

This issue is the same problem as #605 so I'm going to leave it for now. We can address it in a later PR.

snide · 2018-10-29T16:53:33Z

@daveyholler and @ryankeairns ^^ do you mind taking a look at this from a content perspective? This kind of doc is aimed at people who are writing Sass but not super familiar with what all the variables and mixins are.

Hopefully this should make clear the following.

How to format and namespace your Sass.
How light dark theming works
Some color functions available for contrast and theming
The variables and mixins needed for the global scope

If there's anything you have questions on, ask them, and I'll make sure to include it.

daveyholler · 2018-10-29T19:03:00Z

@snide Here are my suggestions:

Give some overview as to how to use sizing
- Should we avoid using pixel values wherever possible?
- Are they based on rems
Grammar error in second paragraph on Theming patterns
- ... traditional color ~~function~~ functions don't give enough...
I'm not sure what the graphic for Shadows hide overflowing content is conveying 😁
Maybe some explanation on when to and when not to use colors on shadows?
I'd recommend a link to the BEM principal for the uninitiated

ryankeairns · 2018-10-29T19:21:43Z

In the comment above the tintOrShade function, describe what the parameters of the function are // tintOrShade(color, tint value, shade value) will tint...

ryankeairns · 2018-10-29T19:23:27Z

The example callouts appear to use $euiColorPrimary, but this code block says $euiColorSecondary :

.themedBox {
  background-color: tintOrShade($euiColorSecondary, 90%, 70%);
  border-left: $euiBorderThick;
  border-color: $euiColorSecondary;
  padding: $euiSize;
  color: $euiTextColor;
}```

ryankeairns · 2018-10-29T19:26:25Z

Where it shows the two different values for $euiColorPrimary, is that telling me that the two values happen automatically based upon the current theme or that I have to use the tintOrShade mixin to achieve that result? (I think it happens automatically and all I need to do is use $euiPrimaryColor, but this made me feel less sure)

ryankeairns · 2018-10-29T19:29:32Z

It might be helpful to denote which of these is the default text color... is it $euiTextColor or $euiColorDarkShade? Simply adding (default) after the color variable name would do the trick.

Text colors
$euiTextColor
$euiColorDarkShade
$euiLinkColor

Same for the text sizes, which of these is the default/base K7 font size? is it $euiFontSize ?

ryankeairns · 2018-10-29T19:33:01Z

Misspell on the variable name here (missing the 'k'):
Breakpoints in EUI are provided through the use of a sass mixin @include euiBreapoint() that accepts an array of sizes

ryankeairns · 2018-10-29T19:35:20Z

Should this be Target tablets and up not just tablet only? The word only made me think it targeted a fixed range between two pixel widths (which I don't think is the case):

Target tablets only

@include euiBreakpoint('m') {...}

ryankeairns · 2018-10-29T19:39:01Z

Perhaps toss the border variables into this bullet list? It's the only section of this page not reflected in the bullet points :)

Writing Sass the EUI way
In general, when writing new SCSS in a project that installs EUI as a dependency ry to follow these best practices:

- Utilize color variables and functions rather than hard-coded values
- Utilize the sizing variables for padding and margins
- Utilize the animation variables for animations when possible
- Utilize the responsive mixins for all screen width calculations
- Utilize the typography mixins and variables for all font family, weight and sizing
- Utilize the shadow mixins and z-index variables to manage depth
- Minimize your overwrites and try to make new Sass additive in nature

ryankeairns · 2018-10-29T19:41:19Z

This is really helpful, thanks for putting it together. This will save me from searching through code in VSCode for the actual values!

snide · 2018-10-30T23:26:27Z

Should this be Target tablets and up not just tablet only? The word only made me think it targeted a fixed range between two pixel widths (which I don't think is the case):
Target tablets only

@include euiBreakpoint('m') {...}

It is the case!

More range options: `ticks`, `valueAppend`

snide · 2018-10-31T03:00:49Z

OK. Feedback addressed. Gonna wait to merge till morning. Think there might be something goofy going on with the yarn.lock file and wanna consult with @chandlerprall

snide · 2018-10-31T20:12:08Z

@chandlerprall did a check on this. merging down now that feedback is addressed.

snide force-pushed the guidelines/sass branch 2 times, most recently from 8a9e56a to a8ffb2a Compare October 25, 2018 23:49

snide commented Oct 25, 2018

View reviewed changes

snide requested a review from daveyholler October 29, 2018 16:48

snide requested a review from ryankeairns October 29, 2018 16:53

snide added 11 commits October 30, 2018 19:39

sass guidelines

e3dff49

yarn

bc7e10b

BEM primer

a2cf1cf

font families

12bb7f7

borders, breakpoints

d77dc29

rearrange code, fix color page

2737412

fix pre / code alpha coloring

363d64c

zindex docs

f537403

imports example

6c1c325

animation examples

42fc0a7

breakpoint content trick

08451d3

snide and others added 14 commits October 30, 2018 19:39

fix grid responsive bug

81029e1

links and cleanup

2a232e7

more exmaples

73767a3

Idea for displaying colors and contrast

23b7fb6

Fix tooltip shadow

7b337bb

Better layout

3e832be

Added slider and copy

788a61c

address feedback

e9f8d35

small fixes

d18b60f

Adding contrast level range slider

044cf09

More range options: `ticks`, `valueAppend`

more feedback

cfb7613

better overflow example

9812e7a

more feedback

2274197

remove code block fix

cd23b76

snide force-pushed the guidelines/sass branch from 5c66680 to cd23b76 Compare October 31, 2018 02:40

snide added 2 commits October 30, 2018 19:47

some color guidelines linking

cfc98cf

cl

266a3f0

snide changed the title ~~[WIP] Sass guidelines~~ Sass guidelines Oct 31, 2018

snide merged commit 8902d68 into elastic:master Oct 31, 2018

snide deleted the guidelines/sass branch October 31, 2018 20:11

chandlerprall mentioned this pull request Nov 7, 2018

Update EUI to v5.0.0 elastic/kibana#25316

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sass guidelines #1257

Sass guidelines #1257

snide commented Oct 23, 2018 •

edited

Loading

snide Oct 25, 2018

cchaos Oct 26, 2018

snide Oct 26, 2018

snide Oct 25, 2018

snide Oct 25, 2018

cchaos Oct 26, 2018

cchaos Oct 26, 2018

snide Oct 26, 2018

snide Oct 31, 2018

snide Oct 25, 2018

snide Oct 31, 2018

snide commented Oct 29, 2018

daveyholler commented Oct 29, 2018

ryankeairns commented Oct 29, 2018 •

edited

Loading

ryankeairns commented Oct 29, 2018 •

edited

Loading

ryankeairns commented Oct 29, 2018

ryankeairns commented Oct 29, 2018 •

edited

Loading

ryankeairns commented Oct 29, 2018 •

edited

Loading

ryankeairns commented Oct 29, 2018

ryankeairns commented Oct 29, 2018

ryankeairns commented Oct 29, 2018

snide commented Oct 30, 2018

snide commented Oct 31, 2018

snide commented Oct 31, 2018

		@@ -0,0 +1,712 @@
		import React from 'react';
		import lightColors from '!!sass-vars-to-js-loader?preserveKeys=true!../../../../src/global_styling/variables/_colors.scss';

Sass guidelines #1257

Sass guidelines #1257

Conversation

snide commented Oct 23, 2018 • edited Loading

Summary

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

snide commented Oct 29, 2018

daveyholler commented Oct 29, 2018

ryankeairns commented Oct 29, 2018 • edited Loading

ryankeairns commented Oct 29, 2018 • edited Loading

ryankeairns commented Oct 29, 2018

ryankeairns commented Oct 29, 2018 • edited Loading

ryankeairns commented Oct 29, 2018 • edited Loading

ryankeairns commented Oct 29, 2018

ryankeairns commented Oct 29, 2018

ryankeairns commented Oct 29, 2018

snide commented Oct 30, 2018

snide commented Oct 31, 2018

snide commented Oct 31, 2018

snide commented Oct 23, 2018 •

edited

Loading

ryankeairns commented Oct 29, 2018 •

edited

Loading

ryankeairns commented Oct 29, 2018 •

edited

Loading

ryankeairns commented Oct 29, 2018 •

edited

Loading

ryankeairns commented Oct 29, 2018 •

edited

Loading