Skip to content

Commit

Permalink
fix(range-slider): Fix slider boundaries (#2408)
Browse files Browse the repository at this point in the history
* fix(connectRangeSlider): update getConfiguration for handle with previous configuration
* feat(RangeSlider): add stories for disabled
* refactor(connectRangeSlider): add function for compute range
* refactor(connectRangeSlider): update function for compute refinement from stats
* refactor(connectRangeSlider): add test for getConfiguration function
* refactor(connectRangeSlider): extract condition for bounds
* style(connectRangeSlider): just refactor the style of the function
* refactor(connectRangeSlider): extract refine on widget
* fix(connectRangeSlider): trggier refinement when refine value is equal to bounds
* refactor(connectRangeSlider): rename variable for avoid confusion
* refactor(Slider): always call refine with values
* refactor(Slider): rename test for consistency
* fix(range-slider-widget): update test according to the new refine function
* refactor: reoder widget functions
* docs(connectRangeSlider): update incorrect options name and signature function
* refactor(connectRangeSlider): restore initial behaviour for fallback refinements values
* feat(range-slider): add stories
* refactor(connectRangeSlider): restore Infinity defaut refinement
* refactor(range-slider): restore start override in widget
* fix(connectRangeSlider): update wrong destructuring
* fix(connectorRangeSlider): dont refine on bounds
* feat(range-slider): add stories for min only and max only bounds
* fix(connectRangeSlider): update expectations
* refactor(connectRangeSlider): use destructuring in refinement
* feat(connectRangeSlider): add function for test if able to refine
* feat(connectRangeSlider): allow disable slider without alterate the results
* refactor(connectRangeSlider): remove compute refinement from stats
* fix(range-slider): restore initial behaviour
* refactor(connectRangeSlider): rename variables
* feat(range-slider-stories): increase step value for step stories
* fix(connectRangeSlider): use matches iteratee shorthand
* refactor: avoid usage of useless fat arrow
* fix(connectRangeSlider): avoid refine out of range
* refactor(connectRangeSlider): use isFinite instead of condittion
* refactor(connectRangeSlider): use ternary instead of let bindings
* chore(dev-novel): Add more meaningful values to min & max story
* refactor(Slider): rename named export to RawSlider
* refactor(connectRangeSlider): rename is(Min|Max)Bounds to has(Min|Max)Bound
* refactor(connectRangeSlider): rename (min|max)Bounds to (min|max)Bound
* refactor(connectRangeSlider): avoid using _isAbleToRefine
* refactor(connectRangeSlider): use array as return value for _getCurrentRefinement
* refactor(connectRangeSlider): rewrite test without mock helper
* refactor(range-slider-test): reorder test for coherence with Jest output
* fix(range-slider): clamp values to range
* fix(conncetRangeSlider): avoid to round refinement values

Fix #2386 

- The boundaries were ignored in `getConfiguration` for the first render
- They were ignored when a refinement was triggered and reset
  • Loading branch information
samouss authored and bobylito committed Oct 2, 2017
1 parent 6207b93 commit bea43db
Show file tree
Hide file tree
Showing 9 changed files with 798 additions and 198 deletions.
104 changes: 99 additions & 5 deletions dev/app/init-builtin-widgets.js
Original file line number Diff line number Diff line change
Expand Up @@ -511,8 +511,44 @@ export default () => {
templates: {
header: 'Price',
},
max: 500,
step: 10,
tooltips: {
format(rawValue) {
return `$${Math.round(rawValue).toLocaleString()}`;
},
},
})
);
})
)
.add(
'disabled',
wrapWithHits(container => {
window.search.addWidget(
instantsearch.widgets.rangeSlider({
container,
attributeName: 'price',
templates: {
header: 'Price',
},
min: 100,
max: 50,
tooltips: {
format(rawValue) {
return `$${Math.round(rawValue).toLocaleString()}`;
},
},
})
);
})
)
.add(
'with step',
wrapWithHits(container => {
window.search.addWidget(
instantsearch.widgets.rangeSlider({
container,
attributeName: 'price',
step: 500,
tooltips: {
format(rawValue) {
return `$${Math.round(rawValue).toLocaleString()}`;
Expand All @@ -529,10 +565,68 @@ export default () => {
instantsearch.widgets.rangeSlider({
container,
attributeName: 'price',
min: 0,
max: 500,
pips: false,
step: 10,
tooltips: {
format(rawValue) {
return `$${Math.round(rawValue).toLocaleString()}`;
},
},
})
);
})
)
.add(
'with min boundaries',
wrapWithHits(container => {
window.search.addWidget(
instantsearch.widgets.rangeSlider({
container,
attributeName: 'price',
templates: {
header: 'Price',
},
min: 36,
tooltips: {
format(rawValue) {
return `$${Math.round(rawValue).toLocaleString()}`;
},
},
})
);
})
)
.add(
'with max boundaries',
wrapWithHits(container => {
window.search.addWidget(
instantsearch.widgets.rangeSlider({
container,
attributeName: 'price',
templates: {
header: 'Price',
},
max: 36,
tooltips: {
format(rawValue) {
return `$${Math.round(rawValue).toLocaleString()}`;
},
},
})
);
})
)
.add(
'with min / max boundaries',
wrapWithHits(container => {
window.search.addWidget(
instantsearch.widgets.rangeSlider({
container,
attributeName: 'price',
templates: {
header: 'Price',
},
min: 10,
max: 500,
tooltips: {
format(rawValue) {
return `$${Math.round(rawValue).toLocaleString()}`;
Expand Down
19 changes: 5 additions & 14 deletions src/components/Slider/Slider.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,8 @@ import times from 'lodash/times';
import range from 'lodash/range';
import has from 'lodash/has';

import PropTypes from 'prop-types';

import React, { Component } from 'react';
import PropTypes from 'prop-types';
import Rheostat from 'rheostat';
import cx from 'classnames';

Expand All @@ -13,7 +12,7 @@ import Pit from './Pit.js';
import autoHideContainerHOC from '../../decorators/autoHideContainer.js';
import headerFooterHOC from '../../decorators/headerFooter.js';

class Slider extends Component {
export class RawSlider extends Component {
static propTypes = {
refine: PropTypes.func.isRequired,
min: PropTypes.number.isRequired,
Expand All @@ -31,17 +30,9 @@ class Slider extends Component {
return this.props.min >= this.props.max;
}

handleChange = ({ min, max, values }) => {
// when Slider is disabled, we alter `min, max` values
// in order to render a "disabled state" Slider. Since we alter
// theses values, Rheostat trigger a "change" event which trigger a new
// search to Algolia with wrong values.
handleChange = ({ values }) => {
if (!this.isDisabled) {
const { refine } = this.props;
refine([
min === values[0] ? undefined : values[0],
max === values[1] ? undefined : values[1],
]);
this.props.refine(values);
}
};

Expand Down Expand Up @@ -120,4 +111,4 @@ class Slider extends Component {
}
}

export default autoHideContainerHOC(headerFooterHOC(Slider));
export default autoHideContainerHOC(headerFooterHOC(RawSlider));
28 changes: 25 additions & 3 deletions src/components/Slider/__tests__/Slider-test.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import React from 'react';
import renderer from 'react-test-renderer';
import { shallow } from 'enzyme';

import Slider from '../Slider';
import Slider, { RawSlider } from '../Slider';

describe('Slider', () => {
it('should render correctly', () => {
it('expect to render correctly', () => {
const tree = renderer
.create(
<Slider
Expand All @@ -22,7 +23,7 @@ describe('Slider', () => {
expect(tree).toMatchSnapshot();
});

it('should render without pips', () => {
it('expect to render without pips', () => {
const tree = renderer
.create(
<Slider
Expand All @@ -39,4 +40,25 @@ describe('Slider', () => {
.toJSON();
expect(tree).toMatchSnapshot();
});

it('expect to call handleChange on change', () => {
const props = {
refine: jest.fn(),
min: 0,
max: 500,
values: [0, 0],
pips: true,
step: 2,
tooltips: true,
shouldAutoHideContainer: false,
};

shallow(<RawSlider {...props} />)
.find('Rheostat')
.simulate('change', {
values: [0, 100],
});

expect(props.refine).toHaveBeenCalledWith([0, 100]);
});
});
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`Slider should render correctly 1`] = `
exports[`Slider expect to render correctly 1`] = `
<div
style={
Object {
Expand Down Expand Up @@ -465,7 +465,7 @@ exports[`Slider should render correctly 1`] = `
</div>
`;

exports[`Slider should render without pips 1`] = `
exports[`Slider expect to render without pips 1`] = `
<div
style={
Object {
Expand Down
Loading

0 comments on commit bea43db

Please sign in to comment.