LTiger14 commented 5 years ago

Come up with naming conventions and rules for the new css variables that will be added in the style refactoring. Also come up with general practices for the new styles after the refactoring.

LTiger14 commented 5 years ago

Draft 1:

Try to flatten the structure as much as possible (use css-grid) - accessibility?
No need to use BEM, simple class name will suffice because of styling encapsulation provided by the shadow DOM (emulated or not)
Certain features can be defined in the theme and activated via a flag. These flags could even be grouped in a mixin that activates a feature
Avoid unnecessary markup, avoid outer div. Instead, use the :host selector to apply style to the component
Try to avoid masking big duplicated chunks of html using css and rather use ts?

Css variables (custom properties)

A css variable should be provided where a rule is defined and likely to be changed.
Variables should be formatted like "--cx-{name of the rule}".
When assigning a value to a rule and also providing a css variable for the same rule, provide the default using the fallback. i.e. margin: var(--cx-margin, 0 10px 0 10px);.
Not everything need a variable. They should be added only if necessary and some styling should remain opinionated and not provide easy configuration.
We should not add variables without a fallback unless the value is specified at the root of the component: Bad
```
.test {
Margin: var(--cx-color);
}
```
Ok
```
:root {
--cx-color: red;
}
```

.test { Margin: var(--cx-color); }



Most css variables will be scoped to a class or element but we should still aim to provide variables with a global scope. To distinguish the two types of variables, we should use a separate naming convention for global variables. These variables should be named --cx-theme-{name of the rule}.

### Bonus

_Suggestion:_ Use css variables in the theme. At the moment, we are mostly using sass variables with at static value. These values should be replaced with a css var and a fallback. This allows easier customisation for theme variables used inside of the styles lib rather than outside. For example, the font sizes are used inside the type mixin. Therefore there is no easy way to change the value of those variables so they have an effect inside the mixin. To fix this, we can do : `$h1-font-size: var(--cx-theme-h1, $font-size-base * 2.25);`. Afterwards, this variable can be modified in the :root of the app styles.

tobi-or-not-tobi commented 5 years ago

@LTiger14 I actually thought this ticket was going to be only about the list of css variables :) That being said, this list is also needed, and should be based on https://github.com/SAP/cloud-commerce-spartacus-storefront/issues/11#issuecomment-438174669.

we could either change this ticket, or start with a new one to come up with the css variable list. Where I think we should have 2 groups:

Theme variables:

--cx-theme-border-radius
--cx-theme-transition-time
...

Component variables:

--cx-width
--cx-margin
...

The effort of this/that ticket would be to come up with an initial list.

btw. I wouldn't mind to find a shortcut for --cx-theme...

tobi-or-not-tobi commented 5 years ago

I'd prefer to transition this ticket and have it more focused on getting the list.

LTiger14 commented 5 years ago

Like @tobi-or-not-tobi mentioned, we should have to groups of css variables.

Theme variables:

--cx-theme-primary, --cx-theme-secondary (colors)
--cx-theme-border-radius
--cx-theme-transition-duration
--cx-theme-animation-duration
--cx-theme-gutter-width
--cx-theme-font-weight-normal/bold...
--cx-theme-spinner-size ?

Side note: In the styles library theme, there are multiple variables that aren't !default and therefore can't be easily overridden in their use within the styles lib. If the objective is also to override them we could add the following:

--cx-theme-font-size-base
--cx-theme-h1-font-size (h2,h3,h4...)
--cx-line-height-base
--cx-heading-line-height

Similarly :

--cx-theme-input-heigh (for example) i.e. $input-height: var (--cx-theme-input-heigh, 48px);

Component variables:

--cx-margin
--cx-padding
--cx-height
--cx-width
--cx-min-height
--cx-min-width
--cx-border-width/style/color
--cx-grid-row/column
--cx-grid-template-columns/rows
--cx-justify-self/--cx-align-self/--cx-place-self
--cx-justify-items/--cx-align-items
--cx-text-align
--cx-color
--cx-background-color
--cx-grid-column-gap/--cx-grid-row-gap
--cx-flex-direction
--cx-justify-content
--cx-align-self/items/content
--cx-zoom
--cx-opacity
--cx-position ?
--cx-display ?

tobi-or-not-tobi commented 5 years ago

Documentation added

SAP / spartacus

Come up with list of css variables #439

Draft 1:

Theme variables:

Component variables: