fulup-bzh / RangeSlider

Angular Foundation for Range Slider [vertical/horizontal, single/dual handles]
Other
5 stars 5 forks source link

Values conflict when using Foundation range slider #9

Closed miklosaubert closed 8 years ago

miklosaubert commented 8 years ago

This plugin is supposed to depend only on the CSS from the Foundation RangeSlider component, but when using it with the full component (CSS + JS), I am seeing some discrepancies between values in Angular's scope and in the DOM.

The Foundation component holds the sliders' values in a hidden input, which is actually set up in this plugin's directive template:

70    RangeSlider.directive('rangeSlider', ["$log", "$document", "$timeout", bzmFoundationSlider]);
71    function bzmFoundationSlider ($log, $document, $timeout) {
72
73        var template= '<div class="bzm-range-slider range-slider" title="{{title}}"data-slider>'
                  + '<span class="range-slider-handle handle-min" ng-mousedown="handleCB($event,0)" ng-focus="focusCB(true)" ng-blur="focusCB(false)" role="slider" tabindex="0"></span>'
                  + '<span class="handle-max" ng-mousedown="handleCB($event,1)" ng-focus="focusCB(true)" ng-blur="focusCB(false)" role="slider" tabindex="0"></span>'
                  + '<span class="range-slider-active-segment"></span>'
                  + '<span class="bzm-range-slider-start" ></span> '
                  + '<span class="bzm-range-slider-stop"></span> '
                  + '<input id={{sliderid}} type="hidden">'  // <<-- HERE
                + '</div>';

Basically, the mouseMove event handlers from the Foundation component calculate the value in a slightly different fashion than bzm-range-slider, which can result in values of 47 and 50 reported at the same time.

Could we use the value from this <input id={{sliderid}} type="hidden"> when the Foundation component is in use? I was thinking of opening a pull request with the following patch:

160,164d159
<             if (scope.vertical) {
<                 scope.relative[handle] = (offset - scope.bounds.handles[handle].getBoundingClientRect().height) / (scope.bounds.bar.getBoundingClientRect().height - scope.bounds.handles[handle].getBoundingClientRect().height);
<             } else {
<                 scope.relative[handle] = offset /  (scope.bounds.bar.getBoundingClientRect().width - scope.bounds.handles[handle].getBoundingClientRect().width);
<             }
166c161,168
<             var newvalue = scope.normalize (scope.relative[handle]);

---
>             if (Foundation.libs.slider) {
>                 var newvalue = parseFloat(document.getElementById(scope.sliderid).value);
>             } else {
>                 if (scope.vertical) {
>                 scope.relative[handle] = (offset - scope.bounds.handles[handle].getBoundingClientRect().height) / (scope.bounds.bar.getBoundingClientRect().height - scope.bounds.handles[handle].getBoundingClientRect().height);
>                 } else {
>                     scope.relative[handle] = offset /  (scope.bounds.bar.getBoundingClientRect().width - scope.bounds.handles[handle].getBoundingClientRect().width);
>                 }
167a170,171
>                 var newvalue = scope.normalize (scope.relative[handle]);
>             }
miklosaubert commented 8 years ago

I should add that this happens with Foundation version 5.4.7.

fulup-bzh commented 8 years ago

Could you propose a patch to fix this issue ? I would be more than happy to merge it.

miklosaubert commented 8 years ago

Pull request opened! Thanks for considering it.

On Tue, Jul 26, 2016 at 4:50 PM, Fulup Ar Foll notifications@github.com wrote:

Could you propose a patch to fix this issue ? I would be more than happy to merge it.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/fulup-bzh/RangeSlider/issues/9#issuecomment-235292302, or mute the thread https://github.com/notifications/unsubscribe-auth/AQzA2W6QxaET7V16s3qa-iemAqGjt01bks5qZh7IgaJpZM4JVBTo .

MIKLOS AUBERT BACKEND DEVELOPER EMAIL mat@trustpilot.com youremail@trustpilot.com PHONE +4531155565 DK Pilestraede 58, 5th Floor, 1112 Copenhagen K https://business.trustpilot.com/ http://www.facebook.com/trustpilot https://twitter.com/trustpilotdk https://www.linkedin.com/company/trustpilot https://plus.google.com/+TrustpilotReviews/posts https://www.youtube.com/user/trustpilot We're built on trust. Read about our values. https://www.trustpilot.com/trust EMAIL CONFIDENTIALITY NOTICE This email is from Trustpilot A/S registration No. DK30276582. The email and any attachments are confidential and may contain privileged information, and are intended for the named addressee(s) only. If you have received this message in error, please notify us and remove it from your system.

miklosaubert commented 8 years ago

Fixed with pull requests #10 and #11.