When generating a theme, and using theme-options.yml the variable overrides are not respected.
Somehow the following lines got removed from gulpfile.js and thus are no longer copied over to sub themes:
// Load theme options if theme-options.yml file exist and merge it with
// options.sass variable.
if (fs.existsSync('./theme-options.yml')) {
var themeOptions = yaml.safeLoad(fs.readFileSync('./theme-options.yml', 'utf8'));
if (themeOptions && typeof themeOptions.sass != 'undefined') {
options.sass = merge(options.sass, themeOptions.sass);
}
}This also raised questions to me:
Tried to document that here: https://www.drupal.org/docs/8/themes/bs-base/theme-development/sass-vari...
| Comment | File | Size | Author |
|---|---|---|---|
| #7 | interdiff-3071854-2-7.txt | 1.09 KB | pivica |
| #7 | 3071854-respect_theme_options-7.patch | 1.65 KB | pivica |
Comments
Comment #2
ytsurkHere the patch re-introducing the left-out code.
Not sure if we should provide an update hook to check the file in existing theme(s).
I would say no, as we're still in alpha ...
Comment #3
ytsurkComment #4
ytsurkComment #5
ytsurkComment #6
ytsurkComment #7
pivica commentedThx for spotting this, checked and this is indeed a bug. Provided a new patch - just some comments improvements.
> 1. What's the difference between sass/variables/_theme_name.scss and theme-options.yml ?
The only difference is that theme-options.yml will be loaded first and then the _theme_name.scss.
> 2. Should the theme-options.yml from parent themes also taken into the chain (right now, they're not) ?
Hmmm, good question.
Currently, theme-options.yml is more an exotic feature of bs_base theme. We have an idea in the future to offer inline code editor so site admins can edit this file, save it and the theme will autogenerate then CSS. This is the main and only reason this support for yaml SASS variable file exists. However
This would mean that theme-options.yml is really only intended for end custom child themes and not custom base themes.
So i would say, yes it would be correct to include this yaml files from parent themes if they exist. However due to other things i don't think it makes sense to spent time on this now.
Let us see what will happen with this yaml support. It does not offer really any benefit from regular SASS variables file and only imposes a lot of limitation - for example, you can not import custom mixins and functions that you would like to use it in it, etc.
> 3. Variables from parent themes should be available in both files ?
No, idea is that this files sits on top and that they can override variables from parent themes. For this to happen we need first to load these files and then variable files from parent themes. This is how SASS variable override works with !default. I know it is annoying when you want to use some parent variable in this files, but the only thing you can do is to define this parent variable in this files again, you can not reuse them.
Comment #8
pivica commentedComment #10
pivica commentedCommitted.
@ytsurk i am not sure do we need some follow-up issus for this, feel free to create some if you think so.
Comment #11
ytsurkI think for now the behaviour is documented and shall stay as is ..