Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow theme variables to be modified from outside #8

Open
bebraw opened this issue Jun 4, 2015 · 7 comments
Open

Allow theme variables to be modified from outside #8

bebraw opened this issue Jun 4, 2015 · 7 comments

Comments

@bebraw
Copy link
Member

bebraw commented Jun 4, 2015

Got this tip from @Morhaus:

sass.renderSync({
  data: '$foo: getVar("foo")',
  functions: {
    'getVar($varname)': function(varname) {
      var varname = varname.getValue();
      return sass .types.{String,Color,Number}(vars[varname]);
    },
  }
});

Alternatively we could do something more generic like use templating for variables files.

@eldh
Copy link
Member

eldh commented Jun 7, 2015

With sass-loader 1.0 and node-sass 3 this should not be needed though. @import '~antwar-theme/vars'; would work....

@bebraw
Copy link
Member Author

bebraw commented Jun 7, 2015

What if we went through aliasing? Ie. in this case you would provide the alias target through theme configuration (variables: path.join(__dirname, 'styles/variables.sass')). If that's not provided it would resolve to default.

I know it's not generic but there's no way we can get a generic solution anyhow given normal CSS doesn't support variables by default and the situation is always a little different. Interestingly cssnext has support for Custom Property Value Syntax so that's one potential alternative for theme authors.

Anyhow I suggest we push this problem to the user side instead of trying to solve it ourselves. Maybe just go through alias in this case. Just need to make sure core supports that. The theme should be able to inject the configuration it needs.

@eldh
Copy link
Member

eldh commented Jun 7, 2015

yeah. It's something that the theme has to expose though. We don't want to force it from core. I think.

@bebraw
Copy link
Member Author

bebraw commented Jun 7, 2015

yeah. It's something that the theme has to expose though. We don't want to force it from core. I think.

Exactly.

I think we may need to rethink theme definition to give more power to theme authors, though. Essentially you'll want to be able to alter configuration based on given parameters. Consider the example below:

antwar.config.js

var defaultTheme = require('antwar-default-theme');

module.exports = {
  theme: defaultTheme({
    variables: path.join(__dirname, 'styles/variables.sass')
  }),
};

So the definition is analogous to plugins.

@eldh
Copy link
Member

eldh commented Jun 7, 2015

Could do this, to avoid spilling too much of the internals:

var defaultTheme = require('antwar-default-theme');

module.exports = {
  theme: defaultTheme({
    variables: 'styles/variables.sass'
  }),
};

@bebraw
Copy link
Member Author

bebraw commented Jun 7, 2015

@eldh Ok. I did that path.join to be explicit about the path but I suppose we could assume site directory by default.

@eldh
Copy link
Member

eldh commented Jun 7, 2015

Yeah, I think that would make sense to people.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants