IE8 support

[tomchiverton]

IE8 requires the ability to use the alternative directive syntax : <div rzslider ... >

Fixes #313

[codecov-io]

Current coverage is 100.00%

Merging #314 into master will not affect coverage as of c6be5d0

@@            master    #314   diff @@
======================================
  Files            1       1       
  Stmts          701     701       
  Branches         0       0       
  Methods          0       0       
======================================
  Hit            701     701       
  Partial          0       0       
  Missed           0       0       

Review entire Coverage Diff as of c6be5d0

Powered by Codecov. Updated on successful CI builds.

[tomchiverton]

The only problem this leaves is that the CSS rules all need to be changed : s/^rzslider/.rzslider/g and I'm not sure how you'd do that. Would we change all the rules to have selectors that covered both cases ?

[ValentinH]

What about using replace: true and wrapping the template inside a div with the .rz-slider class? Then we can simply replace the rzslider rule by .rzslider in the LESS file?

[tomchiverton]

Did you mean "the template inside a div with the .rzslider class" ?
I think that upsets initElemHandles() because when I did that, FireFox complains that this.selbar is null.

[ValentinH]

Yes the initElemHandles probably needs some refactoring too. However, if simply using the attribute syntax is OK for IE8 then this is probably simpler. What do you think?

[tomchiverton]

New template, SASS source, and change to initElemhandles() now pushed.

[ValentinH]

Why did you use class=\"rzslider\" for the template? It shoud be directly class="rzslider" I think.

[tomchiverton]

@ValentinH not sure what you mean. The slashes have to be there to escape the speech marks.

[ValentinH]

they have to be here when used directly in the JS file but here it's an HTML one. ;)

[tomchiverton]

@ValentinH oh, right. I'm not sure how that file ends up in the output .js file. I will change on the branch.

[ValentinH]

OK, I need to test it to ensure that there is no breaking change due to this change. Some unit tests are failing and they are probably about the vertical option.

[ValentinH]

Sorry for the delay of my response. I have just tried you changed and indeed, the vertical sliders are broken.
You just need to add replace: true to the directive config and remove your changes in the initElemHandles() method. ;)

[ValentinH]

@tomchiverton any update about this PR?

[tomchiverton]

I've not had a chance to add vertical sliders to our IE8-requiring application to confirm the suggestion above.

[ValentinH]

I tried it and it should work. Could you update your PR so I can merge it?

[tomchiverton]

Sure. I didn't realise Github wouldn't let you make these changes pre-merge.

[ValentinH]

Thanks! Just before I merge, can you rebase/squash all the commits into one please so it's cleaner in the commits list?

[tomchiverton]

How do I do that ? Pretend I'm a Subversion user who only uses Git because lots of projects I depend on do.

[ValentinH]

Check this: https://github.com/angular-slider/angularjs-slider/blob/master/CONTRIBUTING.md ;)

[ValentinH]

Also it will be released under 3.0 version since existing implementations can be broken if style are defined using the rzslider selector.

[tomchiverton]

Apparently you can do this for this and all future requests : https://github.com/blog/2141-squash-your-commits

[ValentinH]

Indeed, I saw it when they added it but forgot it! :D Thanks 👍

[ValentinH]

I'll release it a little bit later because I would like to implement what I explained in #328 so I only do only major release.