Skip to content
This repository has been archived by the owner on Dec 30, 2022. It is now read-only.

Commit

Permalink
fix(RangeInput): fix compatibility with React 16 & Panel
Browse files Browse the repository at this point in the history
* refactor(Panel): extract canRefine on class instead of declare it in consctructor

* fix(RangeInput): only call setState & canRefine when real changes happen

* doc(RangeInput): add note on componentWillReceiveProps

* fix(RangeInput): update state when canRefine change
  • Loading branch information
samouss authored and mthuret committed Oct 16, 2017
1 parent eb45d9a commit 3f218db
Show file tree
Hide file tree
Showing 3 changed files with 163 additions and 14 deletions.
11 changes: 8 additions & 3 deletions packages/react-instantsearch/src/components/Panel.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,16 @@ class Panel extends Component {

constructor(props) {
super(props);
this.state = { canRefine: true };
this.canRefine = canRefine => {
this.setState({ canRefine });

this.state = {
canRefine: true,
};
}

canRefine = canRefine => {
this.setState({ canRefine });
};

render() {
return (
<div {...cx('root', !this.state.canRefine && 'noRefinement')}>
Expand Down
45 changes: 36 additions & 9 deletions packages/react-instantsearch/src/components/RangeInput.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@ import classNames from './classNames.js';

const cx = classNames('RangeInput');

class RangeInput extends Component {
export class RawRangeInput extends Component {
static propTypes = {
canRefine: PropTypes.bool.isRequired,
translate: PropTypes.func.isRequired,
refine: PropTypes.func.isRequired,
min: PropTypes.number,
Expand All @@ -16,7 +17,10 @@ class RangeInput extends Component {
min: PropTypes.number,
max: PropTypes.number,
}),
canRefine: PropTypes.bool.isRequired,
};

static defaultProps = {
currentRefinement: {},
};

static contextTypes = {
Expand All @@ -25,23 +29,46 @@ class RangeInput extends Component {

constructor(props) {
super(props);
this.state = this.props.canRefine
? { from: props.currentRefinement.min, to: props.currentRefinement.max }
: { from: '', to: '' };

this.state = {
from: this.props.canRefine ? props.currentRefinement.min : '',
to: this.props.canRefine ? props.currentRefinement.max : '',
};
}

componentWillMount() {
if (this.context.canRefine) this.context.canRefine(this.props.canRefine);
if (this.context.canRefine) {
this.context.canRefine(this.props.canRefine);
}
}

componentWillReceiveProps(nextProps) {
if (nextProps.canRefine) {
// In react@16.0.0 the call to setState on the inputs trigger this lifecycle hook
// because the context has changed (for react). I don't think that the bug is related
// to react because I failed to reproduce it with a simple hierarchy of components.
// The workaround here is to check the differences between previous & next props in order
// to avoid to override current state when values are not yet refined. In the react documentation,
// they DON'T categorically say that setState never run componentWillReceiveProps.
// see: https://reactjs.org/docs/react-component.html#componentwillreceiveprops

if (
nextProps.canRefine &&
(this.props.canRefine !== nextProps.canRefine ||
this.props.currentRefinement.min !== nextProps.currentRefinement.min ||
this.props.currentRefinement.max !== nextProps.currentRefinement.max)
) {
this.setState({
from: nextProps.currentRefinement.min,
to: nextProps.currentRefinement.max,
});
}
if (this.context.canRefine) this.context.canRefine(nextProps.canRefine);

if (
this.context.canRefine &&
this.props.canRefine !== nextProps.canRefine
) {
this.context.canRefine(nextProps.canRefine);
}
}

onSubmit = e => {
Expand Down Expand Up @@ -92,4 +119,4 @@ class RangeInput extends Component {
export default translatable({
submit: 'ok',
separator: 'to',
})(RangeInput);
})(RawRangeInput);
121 changes: 119 additions & 2 deletions packages/react-instantsearch/src/components/RangeInput.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@ import PropTypes from 'prop-types';

import React from 'react';
import renderer from 'react-test-renderer';
import Enzyme, { mount } from 'enzyme';
import Enzyme, { mount, shallow } from 'enzyme';
import Adapter from 'enzyme-adapter-react-16';
Enzyme.configure({ adapter: new Adapter() });

import RangeInput from './RangeInput';
import RangeInput, { RawRangeInput } from './RangeInput';

describe('RangeInput', () => {
it('supports passing max/min values', () => {
Expand Down Expand Up @@ -46,6 +46,123 @@ describe('RangeInput', () => {
expect(tree).toMatchSnapshot();
});

it('expect to applies changes when props have changed ', () => {
const wrapper = shallow(
<RawRangeInput
createURL={() => '#'}
refine={() => {}}
min={0}
max={100}
currentRefinement={{ min: 0, max: 100 }}
canRefine={false}
translate={x => x}
/>
);

wrapper.setProps({
canRefine: true,
currentRefinement: {
min: 10,
max: 90,
},
});

wrapper.update();

expect(wrapper.state()).toEqual({
from: 10,
to: 90,
});
});

it("expect to don't applies changes when props don't have changed", () => {
const wrapper = shallow(
<RawRangeInput
createURL={() => '#'}
refine={() => {}}
min={0}
max={100}
currentRefinement={{ min: 0, max: 100 }}
canRefine={true}
translate={x => x}
/>
);

wrapper.setState({
from: 10,
to: 90,
});

wrapper.setProps({
canRefine: true,
currentRefinement: {
min: 0,
max: 100,
},
});

wrapper.update();

expect(wrapper.state()).toEqual({
from: 10,
to: 90,
});
});

it('expect to call context canRefine when props changed', () => {
const context = {
canRefine: jest.fn(),
};

const wrapper = shallow(
<RawRangeInput
createURL={() => '#'}
refine={() => {}}
min={0}
max={100}
currentRefinement={{ min: 0, max: 100 }}
canRefine={true}
translate={x => x}
/>,
{
context,
}
);

wrapper.setProps({
canRefine: false,
});

expect(context.canRefine).toHaveBeenCalledTimes(2);
});

it("expect to not call context canRefine when props don't have changed", () => {
const context = {
canRefine: jest.fn(),
};

const wrapper = shallow(
<RawRangeInput
createURL={() => '#'}
refine={() => {}}
min={0}
max={100}
currentRefinement={{ min: 0, max: 100 }}
canRefine={true}
translate={x => x}
/>,
{
context,
}
);

wrapper.setProps({
canRefine: true,
});

expect(context.canRefine).toHaveBeenCalledTimes(1);
});

it('refines its value on change', () => {
const refine = jest.fn();
const wrapper = mount(
Expand Down

0 comments on commit 3f218db

Please sign in to comment.