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

Style mixin syntax is incompatible with Sass #1373

Closed
jouni opened this issue Apr 6, 2015 · 6 comments
Closed

Style mixin syntax is incompatible with Sass #1373

jouni opened this issue Apr 6, 2015 · 6 comments
Assignees

Comments

@jouni
Copy link

jouni commented Apr 6, 2015

Running the following through the Sass compiler:

:host {
  --my-toolbar-theme: {
    background-color: green;
    border-radius: 4px;
    border: 1px solid gray;
  }
}

Results in:

:host {
  --my-toolbar-theme-background-color: green;
  --my-toolbar-theme-border-radius: 4px;
  --my-toolbar-theme-border: 1px solid gray;
}

Which is obviously unwanted.

The only workaround I could think right away is to wrap the rule set in quotes (make it a string), and then interpolate that to the output:

:host {
  $theme: "{
    background-color: green;
    border-radius: 4px;
    border: 1px solid gray;
  }";
  --my-toolbar-theme: #{$theme};
}

Not a huge issue, but it would be great if the custom mixin syntax would work with the Sass compiler as well.

@sorvell
Copy link
Contributor

sorvell commented Apr 9, 2015

Thanks for the heads up. If we can make these play nicer together we'll do so.

@sorvell
Copy link
Contributor

sorvell commented May 22, 2015

This is an issue with the SASS compiler and should be filed with them. --my-toolbar-theme: begins a css property and the token stream after it should not be parsed as a rule. This can be seen on Firefox, for example: http://jsbin.com/siwaxeneka/1/edit?html,console,output

@voondo
Copy link

voondo commented Sep 10, 2015

@sorvell : The SASS guys probably don't want to fix that because, according the W3C specs, the "CSS identifiers (including element names, classes, and IDs in selectors) can contain only the characters [a-zA-Z0-9] and ISO 10646 characters U+00A0 and higher, plus the hyphen (-) and the underscore (_); they cannot start with a digit, two hyphens, or a hyphen followed by a digit."
Consequently --my-toolbar-theme is an invalid selector.
(see sass/sass#1832)

Maybe should you consider another way to declare your mixins because it conflicts with existing specs ?Why not _@_mixin like SASS ? (By the way, we already have _@_apply)

@jouni
Copy link
Author

jouni commented Sep 10, 2015

Alright, thanks for the update.

We’ve since moved away from using Sass (at least for the time being), so this is not a problem for us at the moment.

I’m just wondering how “dangerous” the Polymer mixin syntax is, if it’s not considered valid CSS? I know Tab Atkins has a proposal for @apply to become a standard at some point, but is there any comments from the Polymer team regarding this? Is this actively being pushes as an upcoming standard, or will it stay Polymer specific for the foreseeable future and always require JS to parse it?

@ebidel
Copy link
Contributor

ebidel commented Sep 10, 2015

The Polymer is is working with Tab and the spec authors on this one. It's one reason we've moved to css custom properties and mixins to jump ahead of the evolving standards.

Note --my-toolbar-theme: is how you define a custom property. If the sass compiler can't handle that syntax, then it's a bug. Properties are landing in Chrome and already in FF.

@jouni
Copy link
Author

jouni commented Sep 10, 2015

Great to hear it’s moving forward, really waiting for these two (custom properties and mixins) to become native so we can take advantage of native cascade with these as well!

I think Sass can handle the custom properties syntax alright, but gets thrown off when you specify a block instead of one property value with it. Anyway, it’s going to be the Sass compiler’s problem, not Polymer’s. So I think you can close this issue :)

Edit: oh, it was closed already :D

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

No branches or pull requests

5 participants