Skip to content

Commit

Permalink
Merge branch 'master' into multiselect-inline-error-fix
Browse files Browse the repository at this point in the history
  • Loading branch information
tw15egan authored Jan 17, 2020
2 parents 95de92f + 044145c commit 622a941
Show file tree
Hide file tree
Showing 11 changed files with 373 additions and 21 deletions.
203 changes: 203 additions & 0 deletions docs/style.md
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,18 @@ row before the </tbody></table> line.
- [React](#react)
- [Guidelines](#guidelines)
- [Writing a component](#writing-a-component)
- [Style](#style-1)
- [Naming event handlers](#naming-event-handlers)
- [Sass](#sass)
- [Guidelines](#guidelines-1)
- [Author component styles using mixins](#author-component-styles-using-mixins)
- [Use design tokens where appropriate](#use-design-tokens-where-appropriate)
- [Avoid nesting selectors](#avoid-nesting-selectors)
- [Use only as much specificity as needed](#use-only-as-much-specificity-as-needed)
- [Use the global `$prefix` variable](#use-the-global-prefix-variable)
- [Annotate relevant Sass values with SassDoc](#annotate-relevant-sass-values-with-sassdoc)
- [Style](#style-2)
- [Comments](#comments)

<!-- END doctoc generated TOC please keep comment here to allow auto update -->
<!-- prettier-ignore-end -->
Expand Down Expand Up @@ -269,3 +281,194 @@ When writing event handlers, we prefer using the exact name, `onClick` to a
shorthand. If that name is already being used in a given scope, which often
happens if the component supports a prop `onClick`, then we prefer to specify
the function as `handleOnClick`.

## Sass

### Guidelines

#### Author component styles using mixins

When authoring the styles for a component, it's important that we use
[`mixins`](https://sass-lang.com/documentation/at-rules/mixin) to allow
developers to control when the CSS for a specific component gets emitted. For
example:

```scss
// src/components/accordion/_accordion.scss

/// Accordion
/// @access private
/// @group accordion
@mixin accordion {
.#{$prefix}--accordion {
// ...
}
}
```

Authoring component styles under a mixin allows the design system to:

- Control when the CSS for accordion gets emitted, or not emitted, from the
library
- Allows us to author experimental or future styles in a separate mixin and
toggle its inclusion through feature flags
- Could allow developers consuming the design system to control when styles get
emitted

#### Use design tokens where appropriate

We have a number of Sass variables available in the project to be used as design
tokens when building out a component. Almost always you will want to leverage
these instead of hard coding values for colors, type, or even spacing. You can
visit the following SassDoc links to view all of the design tokens relevant to
this project:

- [Color](../packages/theme/docs/sass.md)
- [Layout](../packages/layout/docs/sass.md)
- [Motion](../packages/motion/docs/sass.md)
- [Type](../packages/type/docs/sass.md)

#### Avoid nesting selectors

Nesting selectors is often a convenient and fast way to author styles in Sass.
Unfortunately, they also add a performance and maintenance burden for the design
system. The performance burden is due to the generated nature of selectors which
can lead to unexpected CSS bundle bloat. The maintenance burden manifests itself
in a way that makes it harder to find specific selectors while working on the
codebase. For example, if we are looking for the selector `.component:focus` in
the following file:

```scss
// Early on in the file
.component {
// ...
}

// ...

// Later on in the file
.component {
&:focus {
// ...
}
}
```

It can be difficult to track down the specific `.component:focus` selector
without having to navigate through the file and relevant matches to see where
`&:focus` is being defined. While this may be hard to visualize in a short code
snippet, as file size grows and our Sass is rewritten or updated, this problem
becomes increasingly obvious.

#### Use only as much specificity as needed

It's important that we write selectors that use only as much specificity as
needed. Ideally, we would only need one selector per component but this is
rarely the case. As a result, adding specificity should be done sparingly or
when including it is helpful when building a component. For example, if you
would like to enforce a specific element or ARIA attribute then using this
attribute in a selector would be appropriate:

```scss
button[aria-expanded='false'] {
// ...
}
```

If we compared this to a class selector, for example `.my-component__button`,
then we may consider this as adding more specificity than needed. However, for
the design system it is more important that the component itself implements this
contract for accessibility.

#### Use the global `$prefix` variable

When writing selectors, always include the global `$prefix` variable. This value
is used to namespace all of the selectors that we ship in the design system.

<table>
<thead><tr><th>Unpreferred</th><th>Preferred</th></tr></thead>
<tbody>
<tr><td>

```scss
.my-component {
// ...
}
```

</td><td>

```scss
.#{$prefix}--my-component {
// ...
}
```

</td></tr>
</tbody></table>

#### Annotate relevant Sass values with SassDoc

When authoring functions, mixins, or values that are intended to be shared, you
should leverage SassDoc. The typical format we use includes the following
information:

```scss
/// <Details about the mixin>
/// @access <public|private>
/// @group <name-of-group>
@mixin my-component {
// ...
}
```

### Style

#### Comments

When annotating individual selectors or properties, you should add an inline
comment above the piece of code you're commenting.

<table>
<thead><tr><th>Unpreferred</th><th>Preferred</th></tr></thead>
<tbody>
<tr><td>

```scss
.#{$prefix}--my-component {
width: 100%; // Comment about why we need 100% width
}
```

</td><td>

```scss
.#{$prefix}--my-component {
// Comment about why we need 100% width
width: 100%;
}
```

</td></tr>
</tbody></table>

When annotating a section of a Sass file, it is helpful to use the following
banner comment style:

```scss
//----------------------------------------------------------------------------
// Section name
//----------------------------------------------------------------------------
```

_Note: this banner should be formatted to span 80 columns_

When writing SassDoc comments, you should use three forward slashes:

```scss
/// This is a comment for SassDoc
/// @access public
.#{$prefix}--my-component {
// ...
}
```
53 changes: 48 additions & 5 deletions packages/components/docs/sass.md
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@
- [✅text-03 [variable]](#text-03-variable)
- [✅text-04 [variable]](#text-04-variable)
- [✅text-05 [variable]](#text-05-variable)
- [✅text-error [variable]](#text-error-variable)
- [✅icon-01 [variable]](#icon-01-variable)
- [✅icon-02 [variable]](#icon-02-variable)
- [✅icon-03 [variable]](#icon-03-variable)
Expand Down Expand Up @@ -2161,6 +2162,7 @@ Generate a media query up to the width of the given breakpoint name
- **Used by**:
- [carbon--breakpoint-between [mixin]](#carbon--breakpoint-between-mixin)
- [carbon--breakpoint [mixin]](#carbon--breakpoint-mixin)
- [accordion [mixin]](#accordion-mixin)

### ✅carbon--breakpoint-down [mixin]

Expand Down Expand Up @@ -2205,7 +2207,6 @@ Generate a media query for the maximum width of the given styles
- [carbon--is-smallest-breakpoint [function]](#carbon--is-smallest-breakpoint-function)
- **Used by**:
- [carbon--breakpoint-between [mixin]](#carbon--breakpoint-between-mixin)
- [accordion [mixin]](#accordion-mixin)
- [carbon-header [mixin]](#carbon-header-mixin)
- [carbon-side-nav [mixin]](#carbon-side-nav-mixin)

Expand Down Expand Up @@ -4104,6 +4105,7 @@ Define theme variables from a map of tokens
$text-03: map-get($theme, 'text-03') !global;
$text-04: map-get($theme, 'text-04') !global;
$text-05: map-get($theme, 'text-05') !global;
$text-error: map-get($theme, 'text-error') !global;
$icon-01: map-get($theme, 'icon-01') !global;
$icon-02: map-get($theme, 'icon-02') !global;
$icon-03: map-get($theme, 'icon-03') !global;
Expand Down Expand Up @@ -4277,6 +4279,10 @@ Define theme variables from a map of tokens
--#{$custom-property-prefix}-text-05,
map-get($theme, 'text-05')
) !global;
$text-error: var(
--#{$custom-property-prefix}-text-error,
map-get($theme, 'text-error')
) !global;
$icon-01: var(
--#{$custom-property-prefix}-icon-01,
map-get($theme, 'icon-01')
Expand Down Expand Up @@ -4670,6 +4676,10 @@ Define theme variables from a map of tokens
@include custom-property('text-05', map-get($theme, 'text-05'));
}

@if should-emit($theme, $carbon--theme, 'text-error', $emit-difference) {
@include custom-property('text-error', map-get($theme, 'text-error'));
}

@if should-emit($theme, $carbon--theme, 'icon-01', $emit-difference) {
@include custom-property('icon-01', map-get($theme, 'icon-01'));
}
Expand Down Expand Up @@ -5444,6 +5454,7 @@ Define theme variables from a map of tokens
- [text-03 [variable]](#text-03-variable)
- [text-04 [variable]](#text-04-variable)
- [text-05 [variable]](#text-05-variable)
- [text-error [variable]](#text-error-variable)
- [icon-01 [variable]](#icon-01-variable)
- [icon-02 [variable]](#icon-02-variable)
- [icon-03 [variable]](#icon-03-variable)
Expand Down Expand Up @@ -5605,6 +5616,7 @@ $carbon--theme--g90: map-merge(
text-02: #c6c6c6,
text-03: #6f6f6f,
text-05: #8d8d8d,
text-error: #ffb3b8,
icon-01: #f4f4f4,
icon-02: #c6c6c6,
link-01: #78a9ff,
Expand All @@ -5613,7 +5625,7 @@ $carbon--theme--g90: map-merge(
field-02: #525252,
inverse-01: #161616,
inverse-02: #f4f4f4,
support-01: #fa4d56,
support-01: #ff8389,
support-02: #42be65,
support-04: #4589ff,
inverse-support-01: #da1e28,
Expand Down Expand Up @@ -5676,6 +5688,7 @@ $carbon--theme--g100: map-merge(
text-02: #c6c6c6,
text-03: #6f6f6f,
text-05: #8d8d8d,
text-error: #ff8389,
icon-01: #f4f4f4,
icon-02: #c6c6c6,
link-01: #78a9ff,
Expand Down Expand Up @@ -5749,6 +5762,7 @@ $carbon--theme--v9: map-merge(
text-02: #5a6872,
text-03: #cdd1d4,
text-05: #5a6872,
text-error: #e0182d,
icon-01: #3d70b2,
icon-02: #5a6872,
link-01: #3d70b2,
Expand Down Expand Up @@ -5826,6 +5840,7 @@ $carbon--theme: (
text-03: if(global-variable-exists('text-03'), $text-03, map-get($carbon--theme--white, 'text-03')),
text-04: if(global-variable-exists('text-04'), $text-04, map-get($carbon--theme--white, 'text-04')),
text-05: if(global-variable-exists('text-05'), $text-05, map-get($carbon--theme--white, 'text-05')),
text-error: if(global-variable-exists('text-error'), $text-error, map-get($carbon--theme--white, 'text-error')),
icon-01: if(global-variable-exists('icon-01'), $icon-01, map-get($carbon--theme--white, 'icon-01')),
icon-02: if(global-variable-exists('icon-02'), $icon-02, map-get($carbon--theme--white, 'icon-02')),
icon-03: if(global-variable-exists('icon-03'), $icon-03, map-get($carbon--theme--white, 'icon-03')),
Expand Down Expand Up @@ -6463,6 +6478,29 @@ $text-05: if(
- [search [mixin]](#search-mixin)
- [time-picker [mixin]](#time-picker-mixin)

### ✅text-error [variable]

<details>
<summary>Source code</summary>

```scss
$text-error: if(
global-variable-exists('carbon--theme') and map-has-key(
$carbon--theme,
'text-error'
),
map-get($carbon--theme, 'text-error'),
#da1e28
);
```

</details>

- **Group**: [@carbon/themes](#carbonthemes)
- **Type**: `{undefined}`
- **Used by**:
- [carbon--theme [mixin]](#carbon--theme-mixin)

### ✅icon-01 [variable]

Primary icons
Expand Down Expand Up @@ -12617,12 +12655,17 @@ Accordion styles
// Transition property for when the accordion closes
transition: padding motion(standard, productive) $duration--fast-02;
padding-left: $carbon--spacing-05;
padding-right: 25%;
padding-right: $carbon--spacing-05;

@include carbon--breakpoint-down('md') {
// Custom breakpoints based on issue #4993
@include carbon--breakpoint-up(480px) {
padding-right: $carbon--spacing-09;
}

@include carbon--breakpoint-up(640px) {
padding-right: 25%;
}

p {
@include type-style('body-long-01');
}
Expand Down Expand Up @@ -12717,7 +12760,7 @@ Accordion styles

- **Group**: [accordion](#accordion)
- **Requires**:
- [carbon--breakpoint-down [mixin]](#carbon--breakpoint-down-mixin)
- [carbon--breakpoint-up [mixin]](#carbon--breakpoint-up-mixin)
- [prefix [variable]](#prefix-variable)
- [ui-03 [variable]](#ui-03-variable)
- [text-01 [variable]](#text-01-variable)
Expand Down
9 changes: 7 additions & 2 deletions packages/components/src/components/accordion/_accordion.scss
Original file line number Diff line number Diff line change
Expand Up @@ -98,12 +98,17 @@
// Transition property for when the accordion closes
transition: padding motion(standard, productive) $duration--fast-02;
padding-left: $carbon--spacing-05;
padding-right: 25%;
padding-right: $carbon--spacing-05;

@include carbon--breakpoint-down('md') {
// Custom breakpoints based on issue #4993
@include carbon--breakpoint-up(480px) {
padding-right: $carbon--spacing-09;
}

@include carbon--breakpoint-up(640px) {
padding-right: 25%;
}

p {
@include type-style('body-long-01');
}
Expand Down
Loading

0 comments on commit 622a941

Please sign in to comment.