Skip to content

Commit

Permalink
fix(select): added check for nativeControl element before calling pro…
Browse files Browse the repository at this point in the history
…ps.setRippleCenter in NativeControl (#478)
  • Loading branch information
Matt Goo authored Nov 30, 2018
1 parent ce0c314 commit 5baad76
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 23 deletions.
13 changes: 5 additions & 8 deletions packages/select/NativeControl.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,12 @@ import PropTypes from 'prop-types';
import classnames from 'classnames';

export default class NativeControl extends React.Component {
nativeControl_ = React.createRef();

componentDidUpdate(prevProps) {
if (this.props.disabled !== prevProps.disabled) {
this.props.handleDisabled(this.props.disabled);
}

if (this.props.value !== prevProps.value) {
this.props.onChange({target: {value: this.props.value}});
}
}

get classes() {
Expand Down Expand Up @@ -64,9 +62,9 @@ export default class NativeControl extends React.Component {
onTouchStart(evt);
}

setRippleCenter = (clientX, target) => {
setRippleCenter = (xCoordinate, target) => {
if (target !== this.nativeControl_.current) return;
const targetClientRect = target.getBoundingClientRect();
const xCoordinate = clientX;
const normalizedX = xCoordinate - targetClientRect.left;
this.props.setRippleCenter(normalizedX);
}
Expand All @@ -82,7 +80,6 @@ export default class NativeControl extends React.Component {
handleDisabled,
onFocus,
onBlur,
onChange,
onTouchStart,
onMouseDown,
setRippleCenter,
Expand All @@ -94,12 +91,12 @@ export default class NativeControl extends React.Component {
<select
onFocus={this.handleFocus}
onBlur={this.handleBlur}
onChange={onChange}
onMouseDown={this.handleMouseDown}
onTouchStart={this.handleTouchStart}
disabled={disabled}
value={value}
className={this.classes}
ref={this.nativeControl_}
{...otherProps}
>
{children}
Expand Down
12 changes: 10 additions & 2 deletions packages/select/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,10 @@ export default class Select extends React.Component {
}

componentDidUpdate(prevProps, prevState) {
// this is to fix onChange being called twice
if (this.props.value !== prevProps.value) {
this.setState({value: this.props.value});
}
if (this.state.value !== prevState.value) {
this.foundation_.handleChange();
}
Expand Down Expand Up @@ -133,6 +137,10 @@ export default class Select extends React.Component {
);
}


setRippleCenter = (lineRippleCenter) => this.setState({lineRippleCenter});
setDisabled = (disabled) => this.setState({disabled});

/**
* render methods
*/
Expand Down Expand Up @@ -167,9 +175,9 @@ export default class Select extends React.Component {
<NativeControl
className={nativeControlClassName}
foundation={this.foundation_}
handleDisabled={(disabled) => this.setState({disabled})}
handleDisabled={this.setDisabled}
onChange={this.onChange}
setRippleCenter={(lineRippleCenter) => this.setState({lineRippleCenter})}
setRippleCenter={this.setRippleCenter}
{...otherProps}
>
{this.renderOptions()}
Expand Down
44 changes: 31 additions & 13 deletions test/unit/select/NativeControl.test.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React from 'react';
import td from 'testdouble';
import {assert} from 'chai';
import {shallow} from 'enzyme';
import {shallow, mount} from 'enzyme';
import NativeControl from '../../../packages/select/NativeControl';

suite('Select Native Input');
Expand All @@ -28,14 +28,6 @@ test('calls props.handleDisabled if props.disabled updates', () => {
td.verify(handleDisabled(true), {times: 1});
});

test('calls props.onChange if props.value updates', () => {
const onChange = td.func();
const wrapper = shallow(<NativeControl onChange={onChange} />);
const value = 'orange-peel';
wrapper.setProps({value});
td.verify(onChange({target: {value}}), {times: 1});
});

test('#event.focus calls #foundation.handleFocus', () => {
const foundation = {handleFocus: td.func()};
const wrapper = shallow(<NativeControl foundation={foundation} />);
Expand Down Expand Up @@ -78,14 +70,23 @@ test('#event.mousedown calls #props.onMouseDown', () => {
td.verify(onMouseDown(testEvt), {times: 1});
});

test('#event.mousedown calls #props.setRippleCenter', () => {
test('#event.mousedown calls #props.setRippleCenter if target is nativeControl', () => {
const setRippleCenter = td.func();
const wrapper = shallow(<NativeControl setRippleCenter={setRippleCenter} />);
const wrapper = mount(<NativeControl setRippleCenter={setRippleCenter} />);
wrapper.instance().nativeControl_ = {current: testEvt.target};
wrapper.simulate('mousedown', testEvt);
const left = testEvt.target.getBoundingClientRect().left;
td.verify(setRippleCenter(testEvt.clientX - left), {times: 1});
});

test('#event.mousedown does not call #props.setRippleCenter if target is not nativeControl', () => {
const setRippleCenter = td.func();
const wrapper = mount(<NativeControl setRippleCenter={setRippleCenter} />);
wrapper.simulate('mousedown', testEvt);
const left = testEvt.target.getBoundingClientRect().left;
td.verify(setRippleCenter(testEvt.clientX - left), {times: 0});
});

test('#event.touchstart calls #props.onTouchStart', () => {
const onTouchStart = td.func();
const wrapper = shallow(<NativeControl onTouchStart={onTouchStart} />);
Expand All @@ -101,9 +102,9 @@ test('#event.touchstart calls #props.onTouchStart', () => {
td.verify(onTouchStart(evt), {times: 1});
});

test('#event.touchstart calls #props.setRippleCenter', () => {
test('#event.touchstart calls #props.setRippleCenter if target is nativeControl', () => {
const setRippleCenter = td.func();
const wrapper = shallow(<NativeControl setRippleCenter={setRippleCenter} />);
const wrapper = mount(<NativeControl setRippleCenter={setRippleCenter} />);
const evt = {
test: 'test',
touches: [{clientX: 20}],
Expand All @@ -112,11 +113,28 @@ test('#event.touchstart calls #props.setRippleCenter', () => {
value: 'value',
},
};
wrapper.instance().nativeControl_ = {current: evt.target};
wrapper.simulate('touchstart', evt);
const left = evt.target.getBoundingClientRect().left;
td.verify(setRippleCenter(20 - left), {times: 1});
});

test('#event.touchstart does not call #props.setRippleCenter if target is not nativeControl', () => {
const setRippleCenter = td.func();
const wrapper = mount(<NativeControl setRippleCenter={setRippleCenter} />);
const evt = {
test: 'test',
touches: [{clientX: 20}],
target: {
getBoundingClientRect: () => ({left: 15}),
value: 'value',
},
};
wrapper.simulate('touchstart', evt);
const left = evt.target.getBoundingClientRect().left;
td.verify(setRippleCenter(20 - left), {times: 0});
});

test('renders children', () => {
const wrapper = shallow(<NativeControl>
<option value='test'>test</option>
Expand Down
7 changes: 7 additions & 0 deletions test/unit/select/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,13 @@ test('#foundation_.handleChange gets called when state.value updates', () => {
td.verify(wrapper.instance().foundation_.handleChange(), {times: 1});
});

test('state.value updates when props.value changes', () => {
const wrapper = shallow(<Select label='my label' value='test val'/>);
const updatedValue = 'new test value';
wrapper.setProps({value: updatedValue});
assert.equal(wrapper.state().value, updatedValue);
});

test('#componentWillUnmount destroys foundation', () => {
const wrapper = shallow(<Select label='my label' />);
const foundation = wrapper.instance().foundation_;
Expand Down

0 comments on commit 5baad76

Please sign in to comment.