vaadin / sass-compiler

A Java Sass compiler implementation
53 stars 25 forks source link

Wrong source lookup with mixins and extended themes #278

Closed vaadin-bot closed 9 years ago

vaadin-bot commented 9 years ago

Originally by mkn@compart.com


Extending a theme that uses mixins that itself uses mixins that contains url-references will be resolved in a wrong way. The start lokation for the lookup will be the extended theme, instead of the the original one.

reindeed -> common-theme -> projectspecific-theme

common theme is build up like this:

common.scss:

@import "../reindeer/reindeer.scss";
@import "button/button.scss";

@mixin common{
  @include reindeer;
  @include buttons;
}

@mixin default-button($image, $hoverimage) {
    background-image: $image;
    &:active, &:hover {
      background-image: $hoverimage;
    }
    &.v-disabled, &.v-disabled:active, &.v-disabled:hover {
      background-image: $image;
    }
}

and button.scss:

@mixin buttons{

 .mybutton{
    @include default-button(jump-last-page-button, url(../buttons/arrows/mybutton.png), url(../buttons/arrows/mybutton_hover.png));
}

The common theme is extended like this:

@import "../common/common.scss";

@mixin webviewer-test {
  @include common;

   /* ...more css... */
}

The button-images will be resolved in a wrong way. Define them as "../common/buttons/arrows/mybutton.png" works, but of course then they are marked as "not available" in the IDE

See https://vaadin.com/forum#!/thread/8453453 for more details.


Imported from https://dev.vaadin.com/ issue #15115

vaadin-bot commented 9 years ago

Originally by proaccountapp


Updated prioritization date.

vaadin-bot commented 9 years ago

Originally by Mika Murtojarvi


I am not totally sure whether I understand the directory structure correctly. Is it possible for you to submit the relevant files, say, as a zip package containing the themes folder?

vaadin-bot commented 9 years ago

Originally by mkn@compart.com


Attachment added: theme-sample.zip (49.8 KiB) theme example structure

vaadin-bot commented 9 years ago

Originally by mkn@compart.com


pretty much as I described, hope this helps.

vaadin-bot commented 9 years ago

Originally by Mika Murtojarvi


At this point of investigation it appears that the sass-lang compiler (version 3.2.12) outputs the same url's as the Vaadin Sass compiler for the custom button images. This would indicate that the compiler works as intended. However, I will still do some further tests regarding the url handling of our compiler.

vaadin-bot commented 9 years ago

Originally by mkn@compart.com


Thanks for the feedback! this would be really helpful.

We have kind of a workaround, to define url-variables inside the common.scss file and the prefix "../common/" which then by chance will fit the lookup path.

vaadin-bot commented 9 years ago

Originally by Mika Murtojarvi


It appears that it is actually the handling of url's in simple properties (such as background-image: url(...); ) that is incompatible with the sass-lang compiler. The sass-lang compiler basically outputs url's as they are typed in the source scss files, unlike our compiler. See https://github.com/sass/sass/issues/1015 for discussion on the subject.

However, changing the handling of url's to be compatible with sass-lang would likely break many themes including our own. I will update this ticket after discussing the issue with others.

vaadin-bot commented 9 years ago

Originally by @Legioth


Based on the investigation by Mika, it seems that the only sensible thing would be to make sure we are compatible with how sass-lang works, even though that would mean doing the opposite of what is actually reported in this ticket.

Changing this would not be 100% backwards compatible with existing themes, so instead of including this fix in e.g. Sass Compiler 0.9.11, we should instead bump the version number to 0.10.0 and make the next minor release of the framework (i.e. 7.4.0) use the 0.10.x series while Vaadin 7.3.x would continue using Sass Compiler 0.9.x that preserves the invalid behavior discussed in this ticket.

vaadin-bot commented 9 years ago

Originally by @Legioth


I accidentally left this in the designing state even though no further designing would be needed to get it to progress, but rather someone would simply need to make the envisioned changes to the Sass compiler so we can see whether this is feasible and with which version it can actually be released.

vaadin-bot commented 9 years ago

Originally by Mika Murtojarvi


https://dev.vaadin.com/review/#/c/6705/