MadLittleMods / postcss-css-variables

PostCSS plugin to transform CSS Custom Properties(CSS variables) syntax into a static representation
https://madlittlemods.github.io/postcss-css-variables/playground/
Other
536 stars 62 forks source link

Very cool, but incorrect variable overriding #2

Closed tunnckoCore closed 9 years ago

tunnckoCore commented 9 years ago

At first.. lib is awesome!

But I think following output for that fixture isn't correct.

:root {
    --some-width: 150px;
}

.box-foo {
    --some-width: 333px;
    width: var(--some-width);

    .box-bar {
        width: var(--some-width);
    }
}

@media (max-width: 800px) {
    .box-foo {
        --some-width: 300px;
    }
}

output

.box-foo {
    width: 333px;

    .box-bar {
        width: 333px;
    }

    @media (max-width: 800px) {

        .box-bar {
            width: 300px; /* here should be also 333px */
        }
    }
}

@media (max-width: 800px) {
    .box-foo {
        width: 300px;
    }
}

notice the .box-bar in media query. I think it should be 333px, not 300px as defined in box-foo media query?

tunnckoCore commented 9 years ago

Orrr if we accept that is correct, so it also should be 300px in box-bar outside the media query.

tunnckoCore commented 9 years ago

But yea, there have something wrong

The example below sets width of box-foo and box-bar in media queries to 550px which isn't correct, again.

btw, use css syntax highlighting in readme

:root {
    --some-width: 150px;
}

.box-foo {
    --some-width: 333px;
    width: var(--some-width);

    .box-bar {
        width: var(--some-width);
    }
}

.box-baz {
    --some-width: 200px;
    width: var(--some-width);
}

@media (max-width: 800px) {
    .box-baz {
        --some-width: 550px;
    }
}

outputs

.box-foo {
    width: 333px;

    .box-bar {
        width: 333px;
    }

    @media (max-width: 800px) {

        .box-bar {
            width: 550px;
        }
    }
}

@media (max-width: 800px) {

    .box-foo {
        width: 550px;
    }
}

.box-baz {
    width: 200px;
}

@media (max-width: 800px) {

    .box-baz {
        width: 550px;
    }
}
MadLittleMods commented 9 years ago

@tunnckoCore Thank you for bug report :grinning: - The first example input/output is as expected to my eyes.

But something is definitely going wrong in the second example. I hope to have a bug-fix done for this soon.

For future clarity and understanding; Add some comment arrows to point out declarations, etc you think are wrong. /* <------ wrong/unexpected */


I also just added syntax highlighting to the readme. Thank you for the suggestion.

tunnckoCore commented 9 years ago

@MadLittleMods haha yea first comment updated, review it again

It seems, .box-bar in media query is overrided by box-foo media query :laughing: absolutely no logic lol

MadLittleMods commented 9 years ago

@tunnckoCore Fixed in the latest v0.3.1 release.

Added test to make sure we don't regress: ./test/fixtures/cascade-on-nested-rules-in-proper-scope.css



Input:

:root {
    --some-width: 150px;
}

.box-foo {
    --some-width: 333px;
    width: var(--some-width);

    .box-bar {
        width: var(--some-width);
    }
}

.box-baz {
    --some-width: 200px;
    width: var(--some-width);
}

@media (max-width: 800px) {
    .box-baz {
        --some-width: 550px;
    }
}

Output:

.box-foo {
    width: 333px;

    .box-bar {
        width: 333px;
    }
}

.box-baz {
    width: 200px;
}

@media (max-width: 800px) {

    .box-baz {
        width: 550px;
    }
}
tunnckoCore commented 9 years ago

WOW, pretty cool and pretty fast.

MEGA FLEXIBLE and MEGA AWESOME plugin.