Skip to content
This repository has been archived by the owner on Sep 6, 2018. It is now read-only.

Fallback for imported variables #3

Closed
simurai opened this issue Dec 30, 2014 · 4 comments
Closed

Fallback for imported variables #3

simurai opened this issue Dec 30, 2014 · 4 comments
Assignees

Comments

@simurai
Copy link
Contributor

simurai commented Dec 30, 2014

The One Syntax theme breaks when using https://atom.io/themes/isotope-ui.

I think the problem is that Isotope tries to use the @syntax-background-color from One, but that one uses a variable imported from another LESS file (color.less), which Isotope does't (and shouldn't) import. https://github.com/atom/one-dark-syntax/blob/master/stylesheets/syntax-variables.less#L17

I'll have to look into it more, but maybe as a rule, the "official" syntax variables aren't allowed to be dependant on importing other files.

/cc @braver (no need to change anything in Isotope, just FYI)

@simurai simurai changed the title Fallback for variables Fallback for imported variables Dec 30, 2014
@simurai simurai self-assigned this Dec 30, 2014
@braver
Copy link

braver commented Dec 30, 2014

isotope-ui relies on the syntax-variables to set the @syntax-background-color. This creates a problem if the syntax-variables file contains variable names that are also used in the ui theme. Because my theme started as a copy of the One theme, that tends to happen :). In this case I'm also using a @theme-color variable and that creates a recursion.

So, I can (and will in a moment) fix it on my end.

However, we may need a rule that syntax-variables and ui-variables files stick to the required values, to make them safe for importing. These files being the "API" to pass style across packages might require some strictness here. Anything else should perhaps be moved to a different file.

@simurai
Copy link
Contributor Author

simurai commented Dec 31, 2014

@braver I think you can keep it as is and it should be fixed in One. Isotope works for all other themes I tested. It's just that I set it up like this:

// colors.less
@syntax-contrast: ...;
@syntax-base-color: ...;


// syntax-variables.less
@import "colors";
@syntax-background-color: darken(@syntax-base-color, @syntax-contrast);

Now when Isotope UI tries to use the syntax-variables.less there is an error because the variables from colors.less are missing.

So as a convention we could say that syntax-variables.less shouldn't rely on other files. That way other themes/packages can import it without breaking.

@braver
Copy link

braver commented Dec 31, 2014

I renamed @theme-color and Isotope now also works with One-syntax, so that made the problem go away for my theme. I still think that introducing undocumented variables in syntax-variables (or ui-variables for that matter) might cause problems for packages as well.

@simurai
Copy link
Contributor Author

simurai commented Jan 2, 2015

You're right.. the conflict was @theme-color. I thought that it would just get overridden if you import it.

Ok, closing this for now. But yeah.. we'll have to look into it once the theme settings get added.

@simurai simurai closed this as completed Jan 2, 2015
@simurai simurai added the label Jan 29, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants