Problem/Motivation

We need to start leveraging css variables, they are well supported and will help us reduce our code base, making our themes faster for the end user and easier to code and maintain

Proposed resolution

We will start implementing css variables for colors and then continue the work on other sass variables we currently use, for spacing of elements for example

Remaining tasks

* Implement css variables for colors
* Break the dependency of the favicon gulp task on the #color-spot-1 variable
* Update the readme for the favicon gulp task

User interface changes

There should be no visual difference for the end user

Note:

Work will be done on an issue fork but will branch of feature/css-vars-dart-sass for latest dart sass implementation

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Jolidog created an issue. See original summary.

Jolidog’s picture

Issue summary: View changes

Jolidog’s picture

Assigned: Jolidog » rembrandx
Status: Active » Needs review

Implemented color variables.

TODO

  • Change the storybook code that handles the color swatches, so it leverages the now available color variables instead of creating new ones.
  • Place the color name below the swatch, so we can remove the set-color() functions and other helper functions

NEXT STEPS

  • Start leveraging the color variables to reduce code, for example, heading color when inside a background color section, can now be adjusted with 1 line of code, producing much less compiled code, which in turn will make the css smaller and therefor faster

NOTE

Includes the code from https://www.drupal.org/project/rocketship_theme_generator/issues/3274780

rembrandx’s picture

Status: Needs review » Reviewed & tested by the community

@Jolidog: currently testing out the implementation.
Found 1 issue in the new color variables definition file: if a var refers to another var, it needs to be wrapped or it will not work. I will try to fix and commit.

Eg. for style="backround-color: var(--clr-link)"

Defining the link var this way, does not work:
--clr-link: --clr-spot-1;

But this does:
--clr-link: var(--clr-spot-1);

rembrandx’s picture

Status: Reviewed & tested by the community » Needs work
rembrandx’s picture

Status: Needs work » Needs review

@Jolidog: I've added corrections to the color vars (mentioned in prev. comment) + picked up the 'storybook colors' TO DO's.
Added it to an issue branch, you can review the merge request:
https://git.drupalcode.org/project/rocketship_theme_generator/-/merge_re...

rubendello’s picture

In a recent project, I've used the 3.x version of the theme to make use of the color variables.

Works really nice, but during setup, and especially when doing my first commit, I ran into some issues (mostly caused by lintr on precommit). See below.

STARTER THEME

  • Some files have a typo in using the math.div.
    Find max-width: math.div(100%, 12 * 9);, and replace with max-width: (math.div(100%, 12) * 9);
  • In 05-pages/page--layout-builder/_layout-builder.scss, the following should be added:
    #layout-builder-modal {
      .cke_maximized {
        position: fixed !important;
      }
    }
    
  • stylelint: /themes/custom/rocketship_theme_starter/components/00-theme/00-base/05-grid/_00-grid-flex.scss:38:33: Expected single space after "," (function-comma-space-after)
    stylelint: /themes/custom/rocketship_theme_starter/components/00-theme/00-base/05-grid/_00-grid-flex.scss:39:34: Expected single space after "," (function-comma-space-after)
    

    Spaces need to be corrected

  • All colors: stylelint: /themes/custom/rocketship_theme_starter/components/00-theme/00-base/07-colors/_vars-00-colors.scss:11:16: Expected single space after ":" (declaration-colon-space-after)
    

    Lintr doesn't like the tabs added between the variables and values. Probably because they are wrapped in the :root element?

  • Gulp config.js still has the path of the old color variable file. This should be changed to the new file? Implications on certain gulp tasks (favicon)?
  • stylelint: /themes/custom/rocketship_theme_starter/components/00-theme/00-base/07-colors/styleguide/_02-colors-library.scss:32:3: Unexpected longhand value '.2rem 2rem .5rem 2rem' instead of '.2rem 2rem .5rem' (shorthand-property-no-redundant-values)
    
  • components/00-theme/00-base/07-colors/styleguide/colors.twig still contains {% include "@rocketship-theme-demo-atoms..." instead of the theme name
  • Colors are only imported in style.scss. I needed to add an import for the colors in style.content-blocks.scss & style.editor.scss to have the colors working for content blocks & inside ckeditor.
  • The theme makes use of lighten & darken functions, which don't work anymore. Files: _tables.scss & _status.scss
  • Use of variables in button mixins result in hsl function being called in hsl function, which doesn't work: background, color, hover, hover-bg
  • _00-content-block.scss: bg color added twice (line340), the first one should be removed.
    .layout--content-blocks .content-bg--spot-2 > .cb-content {
      background-color: hsl(var(--clr-spot-1));
      background-color: hsl(var(--clr-spot-2));
    }
    

MINIMAL THEME

  • stylelint: /themes/custom/rocketship_theme_starter/components/01-content-blocks/02-molecules/003-faq/_cb-faq.scss
    

    Lots of indentation errors. Indentation is wrong + 1 closing curly brace too much in comment

  • rembrandx committed 1c28631 on issue/rocketship_theme_generator-3274783-3274783-implement-css-variables authored by Jolidog
    Resolve #3274783 "Implement css variables"
    

  • rembrandx committed 341c4d2 on issue/rocketship_theme_generator-3274783-3274783-implement-css-variables
    Issue #3274783 by Jolidog, rubendello: linting fixes and other small...

  • rembrandx committed 341c4d2 on 3.x
    Issue #3274783 by Jolidog, rubendello: linting fixes and other small...
rembrandx’s picture

Status: Needs review » Reviewed & tested by the community

Added the feedback/fixes to the 3.x dev branch.
Will check a couple other tickets as well before creating a new release with this.
Further 'CSS variables' development will be going in different new tickets.

rembrandx’s picture

Version: 3.x-dev » 3.7.0
rembrandx’s picture

Status: Reviewed & tested by the community » Fixed

Last part I added was leveraging the CSS Variables for colors, used with Content Blocks and Layout Sections. So it's easier to change the entire color scheme of those (or make a 'negative' style). That is in 3.7.1
Rest of color variable changes are in 3.7.0

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.