fixed .svg render issue in IE

[aintJoshinya]

I'm pretty new to GitHub, so let me know if I messed anything up with reporting this issue and proposing a fix.

When using an .svg file for the star/icon and using IE, the star/icon is not correctly repeated. To make it to repeat correctly, the background-size css property should be set to the "starwidth" and "starheight" values for the .rateit-selected, .rateit-hover, and range elements.

as an example, in IE11, when using an outlined star svg and a filled star svg for the stars, this is how they are rendered:

this can be fixed by setting the background-size, as described above.

as far as I can tell, this does not impact other major browsers (chrome, firefox, edge)

[gjunge]

Welcome and thank you for helping RateIt get better!!! It looks good, on Sunday or Monday I'll just run some compatibility checks, and will then merge your pull request, and update all the packages.

So I'll ping you back as soon as I've ran the tests.

Once again, welcome and thanks! Gidon

[aintJoshinya]

thanks for the quick reply!

I did a little testing to try and help and realized that my fix also broke the default functionality using the included gif image. My bad. I was only using svg images and didn't notice it breaking the gif.

I think setting the "background-size" is only necessary when using an svg.

if there's a better way to share my updated code, please let me know. I'll include it here for now.

I created two functions, one to test if the html element has an svg as its background image, and one to explicitly set the background size.

function backgroundImageIsSVG (element) {
    var bi = element.css("background-image");
    var extension = bi.slice(bi.lastIndexOf(".")+1,  -2);

    return extension.toLowerCase() == "svg";
};

function explicitBackgroundSize (element) {
    element.css('background-size', itemdata('starwidth') + "px " + itemdata('starheight') + "px");
}

then the code is changed to this (I included a few lines around it for context)

//resize the height of all elements, 
if (!isfont) {
    item.find('.rateit-selected, .rateit-hover').height(itemdata('starheight'));
    //added to adjust background image size. added to provide compatibility with IE. IE will not properly 
    //repeat items unless the background-size is set.
    item.find('.rateit-selected, .rateit-hover').each(function () {
        if (backgroundImageIsSVG($(this))) {
            explicitBackgroundSize($(this));
        }
    });
}

and...

else {
    //set the range element to fit all the stars.
    range.width(itemdata('starwidth') * (itemdata('max') - itemdata('min'))).height(itemdata('starheight'));
    //added to adjust background image size. added to provide compatibility with IE. IE will not properly 
    //repeat items unless the background-size is set.
    if (backgroundImageIsSVG(range)) {
        explicitBackgroundSize(range);
    }
}

I hope this helps!

[gjunge]

I think the problem with the backgroundImageIsSVG is that it is too naive. The SVG might also be embedded in the stylesheet, so then we do not have a SVG extension.

Perhaps it is better to let the user add a rateit-svg class if the item is svg. This way we just have to check for the existence of this class.

We do have to update the documentation for this. What do you think?

[aintJoshinya]

That is true, I hadn't thought of embeded SVGs. Explicitly adding a class when using SVGs is better than testing for an svg extension.

It is possible that a user specifies different image types for the different parts of rateit( rateit-reset, rateit-hover, etc). Maybe this is over complicating things, but should we provide part specific classes as well?

eg, if the user wanted to use an svg for all parts of the rateit rating, the could apply the rateit-svg class to it. if only a single part was going to be an svg, they could apply a more specific class, like rateit-hover-svg.

I'm not sure if this is an unneeded over complication, but I thought I'd throw it out there.

[gjunge]

Hi, I'm sorry for the delay. Got a little swamped.

I would like to keep it simple for now and go for the 80-20 rule.