Shopify / slate

Slate is a toolkit for developing Shopify themes. It's designed to assist your workflow and speed up the process of developing, testing, and deploying themes.
https://shopify.github.io/slate
MIT License
1.28k stars 364 forks source link

CSS Custom Variable - Liquid variable or control flow tags #541

Open jonathanmoore opened 6 years ago

jonathanmoore commented 6 years ago

Problem

The current approach with passing liquid tags from snippets/css-variables.liquid to the CSS style assets does not allow for any variable (capture or assign) or control flow (if) tags. Shopify/starter-theme has a bug where liquid assigns variables are passed to layout.theme.css.liquid on build or deploy, however the values always return null. https://github.com/Shopify/starter-theme/issues/60

Replication steps

Here are two specific examples of challenges with theme customization settings and the way css-variables work:

Applying liquid filters to objects and then returning a variable

Below font_body_bold is a new drop from settings.font_body after applying the font_modify filter. This assignment will not carry over from a theme's snippets to the theme's assets (css/scss).

When Slate is deployed or built the liquid tag {{ font_body_bold.weight | default: 'bold' }} is dropped into the layout.theme.css.liquid file. However since the variable assignment happens in a snippet outside of the style asset the value of font_body_bold.weight is null which always defaults to bold regardless of the font_modify filters.

Invalid example from shopify/starter-theme

<style>
{%- assign font_body_bold = settings.font_body | font_modify: 'weight', 'bolder' -%}

:root {
  --font-body-bold-weight: {{ font_body_bold.weight | default: 'bold' }};
}
</style>

Using a boolean setting for CSS

Having a checkbox/boolean theme setting like Capitalize product titles is a common option many themes have. Currently there is not a way to take a setting's true/false value to apply CSS like text-transform: [uppercase|lowercase].

The expectation would be that you could use a basic if/else statement to assign the uppercase or lowercase value based on a theme's setting. Below is an example that I think many would expect to work, but only returns nil values.

<style>
{%- if settings.capitalize_product_title -%}
  {%- assign product_title_transform = 'uppercase' -%}
{%- else -%}
  {%- assign product_title_transform = 'normal' -%}
{%- endif -%}

:root {
  --product-title-transform: {{ product_title_transform }};
}
</style>

More Details

Similar issue from the perspective of SCSS functions https://github.com/Shopify/slate/issues/503 It's important to note that this specific issue does not occur while running yarn start and live processing the SCSS changes locally. In that case the snippet's CSS vars are doing the work to pass along anything processed with liquid.

jonathanmoore commented 6 years ago

One potential solution would be to copy and prepend any liquid outside of the <style> tags directly to the generated .css.liquid file during build and deploy. Using an approach like this it could solve this issue as well as some of #503 since some SCSS functionality could be recreated with liquid. This should not impact yarn start since the liquid variables will still work to pass along values to the CSS variables.

{%- assign font_body_bold = settings.font_body | font_modify: 'weight', 'bolder' -%}
{%- assign color_special = '#FF0000' -%}
{%- if settings.capitalize_product_title -%}
  {%- assign product_title_transform = 'uppercase' -%}
{%- else -%}
  {%- assign product_title_transform = 'normal' -%}
{%- endif -%}

<style>
  {{ settings.font_body | font_face }}
  {{ font_body_bold | font_face }}
  :root {
    --color-accent: {{ settings.color_accent }};
    --color-special: {{ color_special }};
    --font-body: {{ settings.font_body.family }}, {{ settings.font_body.fallback_families }};
    --font-body-weight: {{ settings.font_body.weight }};
    --font-body-style: {{ settings.font_body.style }};
    --font-body-bold-weight: {{ font_body_bold.weight | default: 'bold' }};
    --product-title-transform: {{ product_title_transform }};
  }
</style>

Then during the build or deploy process the liquid outside of the <style> tags would be added to the top of .css.liquid files.

{%- assign font_body_bold = settings.font_body | font_modify: 'weight', 'bolder' -%}
{%- assign color_special = '#FF0000' -%}
{%- if settings.capitalize_product_title -%}
  {%- assign product_title_transform = 'uppercase' -%}
{%- else -%}
  {%- assign product_title_transform = 'normal' -%}
{%- endif -%}

body{
  font-family:  {{ settings.font_body.family }};
}
a {
  color: {{ settings.color_accent }};
}
span.special{
  color:{{ color_special }};
}
.product_title{
  text-transform:{{ product_title_transform }};
}

This would also be a huge improvement for free / premium theme developers supporting merchants. Often when a merchant wants to make an adjustment to how much darker the accent color is on hover, or tweak another SCSS variable they can do so by changing one or two lines in the .scss.liquid file. Personally I have been concerned about the impact of loosing this since Slate 1.x moves towards compressed CSS with the liquid tags duplicated many times throughout the file.

With an approach of copying the liquid tags over we could have an organized approach where we assign all sorts of variables linked to theme settings or modifying theme settings using booleans, color filters, font filters, etc. This would give more knowledgable merchants and Shopify experts a much easier path to overwrite some style defaults—especially if liquid comments and line breaks are preserved.

{%- comment -%}
  Colors
{%- endcomment -%}
{%- assign color_text = settings.color_text -%}
{%- assign color_tex_title = settings.color_text | color_darken: 30 -%}
{%- assign color_accent = settings.color_accent -%}
{%- assign color_accent_hover = color_accent | color_darken: 20 -%}

{%- comment -%}
  Fonts
{%- endcomment -%}
{%- assign font_body = settings.font_body -%}
{%- assign font_bod_bold = font_body | font_modify: 'weight', 'bolder' -%}

{%- comment -%}
  Default variables
{%- endcomment -%}
{%- assign page_width = '1180px' -%}

body{......
jonathanmoore commented 6 years ago

I still believe that there are many normal theme use cases where it would make sense to incorporate a solution into Slate, however as a temporary solution on my end I created a new liquid-variables.liquid file that is imported into css-variables.liquid for development, and then during build a webpack plugin is used to inject the liquid-variables.liquid file at the top of all the .css.liquid files. https://github.com/jonathanmoore/starter-theme/pull/1

t-kelly commented 6 years ago

Thanks @jonathanmoore -- clearly I need to dig into our new styling solution and flush out some of these very important edge cases!

jonathanmoore commented 6 years ago

As a temporary solution, I'm solving this with a new snippets/liquid-variables.liquid file that is included at the top of css-variables.liquid. In the new snippet I am assigning all the liquid variables that are used for CSS vars. Finally using a webpack plugin and extending the production build the contents of the liquid-variables.liquid file is appended to the top of the dist/assets/layout.theme.css.liquid file.

// slate.config.js
/* eslint-disable no-undef */

const path = require('path');
const fs = require('fs');
const WrapperPlugin = require('wrapper-webpack-plugin');

const alias = {
  jquery: path.resolve('./node_modules/jquery'),
  'lodash-es': path.resolve('./node_modules/lodash-es'),
};

const liquidVariables = fs.readFileSync(
  'src/snippets/liquid-variables.liquid',
  'utf8',
);

module.exports = {
  slateCssVarLoader: {
    cssVarLoaderLiquidPath: ['src/snippets/css-variables.liquid'],
  },
  slateTools: {
    extends: {
      dev: {resolve: {alias}},
      prod: {
        resolve: {alias},
        module: {},
        plugins: [
          new WrapperPlugin({
            test: /\.css\.liquid$/,
            header: liquidVariables,
          }),
        ],
      },
    },
  },
};
montalvomiguelo commented 6 years ago

@jonathanmoore I have used this in my theme :)

https://github.com/montalvomiguelo/starter-debut/tree/liquid-variables

wrainbird commented 6 years ago

@jonathanmoore this is awesome, finally getting a chance to play around with this.

I extended it a little further, I really wanted the Liquid variables to be in the same file as the css custom variables. So am reading the css-variables file, then splitting the contents at the "style" tag and putting it to the top of the file.

snippets/css-variables.liquid

<!-- Liquid Variables -->

{{ settings.font_heading | font_face }}
{{ settings.font_body | font_face }}

{%- assign font_body_bold = settings.font_body | font_modify: 'weight', 'bolder' -%}
{%- assign font_body_bold_italic = font_body_bold | font_modify: 'style', 'italic' -%}

{{ font_body_bold | font_face }}
{{ font_body_bold_italic | font_face }}

{% if settings.text_transform %}
  {% assign text_transform = 'uppercase' %}
{% else %}
  {% assign text_transform = 'normal' %}
{% endif %}

<style>
  :root {
    --color-accent: {{ settings.color_accent }};
    --color-body-text: {{ settings.color_body_text }};
    --color-main-background: {{ settings.color_main_bg }};
    --color-border: {{ settings.color_body_text | color_lighten: 50 }};

    --font-heading: {{ settings.font_heading.family }}, {{ settings.font_heading.fallback_families }};
    --font-body: {{ settings.font_body.family }}, {{ settings.font_body.fallback_families }};
    --font-body-weight: {{ settings.font_body.weight }};
    --font-body-style: {{ settings.font_body.style }};
    --font-body-bold-weight: {{ font_body_bold.weight | default: 'bold' }};

    --text_transform: {{ text_transform }};
  }
</style>

slate.config.js

const path = require('path');
const fs = require('fs');
const WrapperPlugin = require('wrapper-webpack-plugin');

const alias = {
  jquery: path.resolve('./node_modules/jquery'),
  'lodash-es': path.resolve('./node_modules/lodash-es'),
};

const cssVariableFile = fs.readFileSync(
  'src/snippets/css-variables.liquid',
  'utf8',
);

const liquidVariables = cssVariableFile.split("<style>")[0];

module.exports = {
  slateCssVarLoader: {
    cssVarLoaderLiquidPath: ['src/snippets/css-variables.liquid'],
  },
  slateTools: {
    extends: {
      dev: {resolve: {alias}},
      prod: {
        resolve: {alias},
        module: {},
        plugins: [
          new WrapperPlugin({
            test: /\.css\.liquid$/,
            header: liquidVariables,
          }),
        ],
      },
    },
  },
};
jonathanmoore commented 6 years ago

@t-kelly Any updates on this? As of right now I'm sort of stuck with 1.0.0 beta 7 and not sure if it's worth upgrading or taking the time to rework this into the new configs.

t-kelly commented 6 years ago

No updates yet -- but it should take less than 10 minutes to switch over your Slate config. Let me know if you have any questions!

wrainbird commented 5 years ago

@jonathanmoore did you have any luck migrating this over to the new config? Just looking at it now and running into some roadblocks getting it to work.

jonathanmoore commented 5 years ago

@rainerbird Same here. I have been short on time to figure out how to properly update it. Once I have some time to figure it out, I'll post it back in here.

wrainbird commented 5 years ago

@jonathanmoore

I had a go over the weekend and think I've got it working using this, hope this helps:

const plugins = [];

var liquidWrapper = new WrapperPlugin({
  test: /\.css\.liquid$/,
  header: liquidVariables,
});

if (process.env.NODE_ENV === 'production') {
  plugins.push(liquidWrapper);
}

module.exports = {

  'cssVarLoader.liquidPath': ['src/snippets/css-variables.liquid'],
  'webpack.extend': {
    resolve: {
      alias: {
        jquery: path.resolve('./node_modules/jquery'),
        'lodash-es': path.resolve('./node_modules/lodash-es'),
      },
    },
    plugins: plugins
  },
};
bitwitch commented 5 years ago

@rainerbird can confirm your updated config is working for me as well, im on slate v1.0.0-beta.12. Thank you and @jonathanmoore for figuring this out.

cooervo commented 4 years ago

native css variables should be supported by liquid and theme-kit

This would allow global vars in an easy way.