Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ranges with allowCross=false result in wrong handle moving when dragging #226

Closed
Oddsor opened this issue Feb 20, 2017 · 8 comments
Closed
Assignees
Labels

Comments

@Oddsor
Copy link

Oddsor commented Feb 20, 2017

Couldn't think of a better title to this

I encountered a strange issue when attempting to implement a sort of wrapper component around the rc-slider.

When implementing an onChange containing a setState, the third value in the range slider is set to the same value as the second when dragging the first. The setState does not necessarily need to modify values used by the rc-slider for this to break.

For instance: A range of [20, 66, 70] becomes [21, 66, 66] when dragging the first handle. This also fails when dragging the last handle.

I tracked this to the "ensureValueNotConflict"-function in Range.js component on line 336. It is called in the componentWillReceiveProps, and this is why it breaks:

if (!allowCross && handle != null) {
  if (handle > 0 && val <= bounds[handle - 1]) {
    return bounds[handle - 1];
  }
  if (handle < bounds.length - 1 && val >= bounds[handle + 1]) {
    return bounds[handle + 1];
  }
}

A hacky fix I came up with is to pass the index to ensureValueNotConflict and then use that to skip values that aren't next to the dragged value

Range.prototype.componentWillReceiveProps = function componentWillReceiveProps(nextProps) {
    var _this2 = this;

    if (!('value' in nextProps || 'min' in nextProps || 'max' in nextProps)) return;
    var bounds = this.state.bounds;

    var value = nextProps.value || bounds;
    var nextBounds = value.map(function (v, i) { <----- Index!
      return _this2.trimAlignValue(v, i, nextProps);
    });

And then modifying ensureValueNotConflict like this:

if (!allowCross && handle != null) {
  if (handle - 1 != index && handle + 1 != index){
    return val;
  }
  if (handle > 0 && val <= bounds[handle - 1]) {
    return bounds[handle - 1];
  }
  if (handle < bounds.length - 1 && val >= bounds[handle + 1]) {
    return bounds[handle + 1];
  }
}

This seems to solve my issue

@Oddsor Oddsor changed the title Ranges with allowCross=false result in wrong value array Ranges with allowCross=false result in wrong handle moving when dragging Feb 20, 2017
@benjycui
Copy link
Member

Could you provide re-producible online demo? And some steps to re-produce it. Thanks.

@Oddsor
Copy link
Author

Oddsor commented Feb 21, 2017

I could put one up on codepen for instance, but I couldn't figure out how to add rc-slider as an external resource there.

@benjycui
Copy link
Member

Please provide a re-producible demo: http://codepen.io/benjycui/pen/aJooRE?editors=0011

@Oddsor
Copy link
Author

Oddsor commented Feb 26, 2017

Thanks a lot for the help! Been a busy week so I didn't find the energy to answer this before now, but here's an example that hopefully shows the issue with allowCross by adding a comparison!

NB: The bug only shows when you actually hold and drag on an endpoint, clicking along the range works fine.

http://codepen.io/oddsor/pen/GWJooK

@simgot
Copy link

simgot commented Mar 8, 2017

I'm encountering the same issue,
hoping for fast fix
Thanks @Oddsor for idea of a work around 👍

@saturation
Copy link

Any news on this one? Is there any other workaround instead of hacking rc-slider code?

paranoidjk added a commit that referenced this issue Jul 3, 2017
@paranoidjk
Copy link
Member

Fixed in rc-slider@8.1.4.

paranoidjk added a commit that referenced this issue Jul 3, 2017
@Oddsor
Copy link
Author

Oddsor commented Dec 15, 2017

This issue seems to have been marked as fixed, but the functionality is (partially?) broken again in the latest version (8.5.0)

Can be reproduced on the examples-page for this component (http://react-component.github.io/slider/examples/range.html), on the following examples:

  • Controlled Range, not allow across
  • Controlled Range, not allow across, pushable

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants