Skip to content

Commit

Permalink
fix(text-field): label isn't float when set value with setState (#934)
Browse files Browse the repository at this point in the history
BREAKING CHANGE: handleValueChange is removed from Input's props.
  • Loading branch information
태재영 authored and Matt Goo committed Jun 19, 2019
1 parent cc06add commit f829e12
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 85 deletions.
39 changes: 12 additions & 27 deletions packages/text-field/Input.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,6 @@ export interface InputProps<T extends HTMLElement = HTMLInputElement> {
inputType?: 'input' | 'textarea';
isValid?: boolean;
foundation?: MDCTextFieldFoundation;
handleValueChange?: (
value: string | number | string[] | undefined,
cb: () => void
) => void;
syncInput?: (inputInstance: Input<T>) => void;
onBlur?: Pick<React.HTMLProps<T>, 'onBlur'>;
onChange?: Pick<React.HTMLProps<T>, 'onChange'>;
Expand Down Expand Up @@ -91,7 +87,6 @@ export default class Input<
inputType: 'input',
disabled: false,
id: '',
handleValueChange: () => {},
setDisabled: () => {},
setInputId: () => {},
handleFocusChange: () => {},
Expand All @@ -111,7 +106,6 @@ export default class Input<
value,
setInputId,
setDisabled,
handleValueChange,
foundation,
} = this.props;
if (syncInput) {
Expand All @@ -123,11 +117,8 @@ export default class Input<
if (setDisabled && disabled) {
setDisabled(true);
}
if (handleValueChange && value) {
handleValueChange(
value,
() => foundation && foundation.setValue(this.valueToString(value))
);
if (value && foundation) {
foundation.setValue(this.valueToString(value));
}
this.setState({isMounted: true});
}
Expand All @@ -139,7 +130,6 @@ export default class Input<
value,
disabled,
isValid,
handleValueChange,
setInputId,
setDisabled,
} = this.props;
Expand All @@ -163,16 +153,14 @@ export default class Input<
}

if (value !== prevProps.value) {
handleValueChange &&
handleValueChange(value, () => {
// only call #foundation.setValue on programatic changes;
// not changes by the user.
if (this.state.wasUserTriggeredChange) {
this.setState({wasUserTriggeredChange: false});
} else {
foundation && foundation.setValue(this.valueToString(value));
}
});
const inputValue = this.valueToString(value);
const notTriggeredChange = !this.state.wasUserTriggeredChange;
// only call #foundation.setValue on programatic changes;
// not changes by the user.
if (notTriggeredChange) {
foundation && foundation.setValue(inputValue);
}
this.setState({wasUserTriggeredChange: false});
}

if (isValid !== prevProps.isValid && foundation) {
Expand Down Expand Up @@ -263,10 +251,8 @@ export default class Input<
onTouchStart(evt);
};

// To stay in sync with the MDC React Text Field Component, handleValueChange()
// is called to update MDC React Text Field's state. That state variable
// is used to let other subcomponents and the foundation know what the current
// value of the input is.
// That state variable is used to let other subcomponents and
// the foundation know what the current value of the input is.
handleChange = (
evt: React.FormEvent<
T extends HTMLInputElement ? HTMLInputElement : HTMLTextAreaElement
Expand Down Expand Up @@ -332,7 +318,6 @@ export default class Input<
isValid,
value,
handleFocusChange,
handleValueChange,
setDisabled,
setInputId,
onFocus,
Expand Down
1 change: 0 additions & 1 deletion packages/text-field/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@ inputType | string | HTML tag to be used to render input element. 'input' (defau
className | String | Classes to be applied to the input element.
disabled | Function | Disables the input and the parent text field.
foundation | Function | The text field foundation.
handleValueChange | Function | A callback function to update React Text Field's value.
isValid | Boolean | If set, this value will override the native input's validation.
id | String | The `<input>` id attribute.
onBlur | Function | Blur event handler.
Expand Down
59 changes: 2 additions & 57 deletions test/unit/text-field/Input.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -180,24 +180,6 @@ test('#componentDidMount should not call any method if disabled and id do not ex
td.verify(setDisabled(td.matchers.isA(Boolean)), {times: 0});
});

test('#componentDidMount calls props.handleValueChange when the foundation initializes with a value', () => {
const handleValueChange = td.func();
const value = 'test value';
const props: any = {handleValueChange, value};
shallow(<Input {...props} />);
td.verify(handleValueChange(value, td.matchers.isA(Function)), {times: 1});
});

test('#componentDidMount does not call props.handleValueChange when there is no props.value', () => {
const handleValueChange = td.func();
const props: any = {handleValueChange};
shallow(<Input {...props} />);
td.verify(
handleValueChange(td.matchers.anything(), td.matchers.isA(Function)),
{times: 0}
);
});

test(
'#props.handleFocusChange is called when props.autoFocus is true' +
', there is a props.foundation, and component has mounted',
Expand Down Expand Up @@ -317,17 +299,6 @@ test(
}
);

test('#componentDidUpdate calls handleValueChange when the foundation initializes with a value', () => {
const setValue = td.func();
const handleValueChange = td.func();
const props: any = {handleValueChange};
const wrapper = shallow(<Input value='test value' {...props} />);
wrapper.setProps({foundation: buildFoundation({setValue})});
td.verify(handleValueChange('test value', td.matchers.isA(Function)), {
times: 1,
});
});

test('#componentDidUpdate calls setUseNativeValidation when isValid changes to undefined', () => {
const foundation: any = buildFoundation({setUseNativeValidation: td.func()});
const wrapper = shallow(
Expand All @@ -354,27 +325,9 @@ test('#componentDidUpdate calls setValid when isValid changes', () => {
td.verify(foundation.setValid(true), {times: 1});
});

test('props.handleValueChange() is called if this.props.value updates', () => {
const handleValueChange = td.func();
const props: any = {handleValueChange};
const wrapper = shallow(<Input {...props} />);
wrapper.setProps({value: 'meow'});
td.verify(handleValueChange('meow', td.matchers.isA(Function)), {times: 1});
});

test('foundation.setValue() is called if this.props.value updates', () => {
const foundation: any = buildFoundation({setValue: td.func()});
const handleValueChange: (
value: string | number | string[] | undefined,
cb: () => void
) => void = (value, cb) => value && cb();
const wrapper = shallow(
<Input
value='test value'
foundation={foundation}
handleValueChange={handleValueChange}
/>
);
const wrapper = shallow(<Input value='test value' foundation={foundation} />);
wrapper.setProps({value: 'meow'});
td.verify(foundation.setValue('meow'), {times: 1});
});
Expand Down Expand Up @@ -466,16 +419,8 @@ test('#event.onChange calls props.onChange()', () => {

test('wasUserTriggeredChange test', () => {
const foundation: any = buildFoundation();
const handleValueChange = (
value: string | number | string[] | undefined,
cb: () => void
) => value && cb();
const wrapper = mount<Input<HTMLInputElement>>(
<Input
value='test value'
foundation={foundation}
handleValueChange={handleValueChange}
/>
<Input value='test value' foundation={foundation} />
);
wrapper.simulate('change');
assert.isTrue(wrapper.instance().state.wasUserTriggeredChange);
Expand Down
19 changes: 19 additions & 0 deletions test/unit/text-field/index.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -770,3 +770,22 @@ test('Input component sync test in TextField', () => {
// and throw error because there is no inputComponent
assert.doesNotThrow(() => wrapper.instance().setState({disabled: true}));
});

test('FloatingLabel is floated even if value is changed programmatically', () => {
class TestComponent extends React.Component {
state = {value: ''};
render() {
return (
<TextField label='my label'>
<Input value={this.state.value} />
</TextField>
);
}
}
const wrapper = mount<TestComponent>(<TestComponent />);
const label = wrapper.find('.mdc-floating-label').getDOMNode();
const floatClass = 'mdc-floating-label--float-above';
assert.isFalse(label.className.includes(floatClass));
wrapper.setState({value: 'Test!'});
assert.isTrue(label.className.includes(floatClass));
});

0 comments on commit f829e12

Please sign in to comment.