mwrock / RequestReduce

Instantly makes your .net website faster by reducing the number and size of requests with almost no effort.
www.requestreduce.org
Apache License 2.0
228 stars 49 forks source link

Sprite generation doesn't use correct image size if nested too deep #183

Open bizzarry opened 12 years ago

bizzarry commented 12 years ago

Hi,

I haven't been able to determine if this is an issue with the dotless parser's output or the sprite generation's usage of that output, but I have mocked up a simple example. It might be hard to describe without just sending you the sample project. If you have a way that I could get that to you, please let me know.

Two sets of png images: 1) small-set: 16x16 2) large-set: 32x32

----Html Markup----

Spriting Test

Breaks

Works

---Broken .less styling---

.iconB { display: block; width: 16px; height: 16px;

background-position: 0 0;
background-repeat: no-repeat;

&.success {
    background-image: url('/Content/Images/small-icons/success.png');
}
&.warn {
    background-image: url('/Content/Images/small-icons/warn.png');
}
&.error {
    background-image: url('/Content/Images/small-icons/error.png');
}

&.large {
    width: 32px;
    height: 32px;

    &.success {
        background-image: url('/Content/Images/large-icons/success.png');
    }
    &.warn {
        background-image: url('/Content/Images/large-icons/warn.png');
    }
    &.error {
        background-image: url('/Content/Images/large-icons/error.png');
    }
}

} ---Working .less styling---

.iconW { display: block; width: 16px; height: 16px;

background-position: 0 0;
background-repeat: no-repeat;

&.success {
    background-image: url('/Content/Images/small-icons/success.png');
}
&.warn {
    background-image: url('/Content/Images/small-icons/warn.png');
}
&.error {
    background-image: url('/Content/Images/small-icons/error.png');
}

&.large {
    &.success {
        width: 32px;
        height: 32px;
        background-image: url('/Content/Images/large-icons/success.png');
    }
    &.warn {
        width: 32px;
        height: 32px;
        background-image: url('/Content/Images/large-icons/warn.png');
    }
    &.error {
        width: 32px;
        height: 32px;
        background-image: url('/Content/Images/large-icons/error.png');
    }
}

}

What I see: It looks like RR picks up the correct small and large images and puts them into the sprite. However, it crops the larger images to the width and height of the smaller ones. Screenshot: http://imgur.com/ChMWq

Apologies if this isn't clear. Here are some more screenshots of my example above: Before RR: http://imgur.com/pxH0C After RR: http://imgur.com/mCqGS

Regards, Chris

mwrock commented 12 years ago

looks like the markup got swallowed by the wiki. Can you try and send that again or you can email to me at matt@mattwrock.com thanks!

mwrock commented 12 years ago

Thanks for the email. Definitely not a less parser issue. It has to do with the way rr flattens multiple css classes with partial bg image properties. This is one of the trickiest bits of logic to get right. I completely reworked all this code over the last.holiday season. Initially I included width and height as attributes to flatten but ran in to some edge cases where it was breaking some sites. So since your CSS puts the dimensions in a separate class it gets dropped. I'm going to revisit this and try to remember the exact cases where this was breaking.

bizzarry commented 12 years ago

Hey Matt, no rush, the workaround I have in my sample works fine for us. Our live example isn't much more complicated than that so it's not the end of the work to duplicate some styling here or there.

All in all, RR is an awesome project. Thanks so much!!

Chris