fix(text-field): remove foundation.setValue call during onChange #350
Conversation
…m:material-components/material-components-web-react into fix/text-field/validation-handling
Codecov Report
@@ Coverage Diff @@
## master #350 +/- ##
==========================================
- Coverage 96.97% 96.86% -0.12%
==========================================
Files 51 51
Lines 1816 1816
Branches 209 210 +1
==========================================
- Hits 1761 1759 -2
- Misses 55 57 +2
Continue to review full report at Codecov.
|
packages/text-field/index.js
Outdated
| @@ -262,7 +263,7 @@ class TextField extends React.Component { | |||
| key='text-field-container' | |||
| > | |||
| {leadingIcon ? this.renderIcon(leadingIcon) : null} | |||
| {this.renderInput()} | |||
| {this.foundation_ ? this.renderInput() : null} | |||
|
|
||
| // this should be in the componentDidMount, but foundation is not created | ||
| // at that time. | ||
| if (value && foundation !== prevProps.foundation) { |
There was a problem hiding this comment.
Once this #353 is fixed, I will move this back into componentDidMount
packages/text-field/Input.js
Outdated
| this.inputElement = React.createRef(); | ||
| } | ||
| inputElement = React.createRef(); | ||
| state = {disableProgramaticSetValue: false}; |
There was a problem hiding this comment.
Change this to enableProgrammaticSetValue, it's easier to reason about logic if the variable is positive
packages/text-field/Input.js
Outdated
| foundation.autoCompleteFocus(); | ||
| this.setState({disableProgramaticSetValue: true}); |
There was a problem hiding this comment.
So the onChange event is only emitted when it's from a user? Do programmatic changes emit any event?
There was a problem hiding this comment.
Right the onChange event is only triggered when a user types. BOTH events (user and programatic) trigger updates props, which calls componentDidUpdate.
packages/text-field/Input.js
Outdated
| if (!this.state.disableProgramaticSetValue) { | ||
| foundation.setValue(value); | ||
| } else { | ||
| this.setState({disableProgramaticSetValue: false}); |
There was a problem hiding this comment.
at what point does the Text Field's state.value get updated if the change is from a user? should we be calling handleValueChange in handleBlur?
There was a problem hiding this comment.
we call setValue in handleBlur, but its called within MDC's foundation.deactivateFocus
|
update - ready for re-review |
packages/text-field/Input.js
Outdated
| // this should be in the componentDidMount, but foundation is not created | ||
| // at that time. | ||
| // Will fix this in | ||
| // https://github.com/material-components/material-components-web-react/pull/350/files |
fixes #333
By removing the componentDidUpdate from the index.js file, we no longer call foundation.setValue every input change. MDC Web was not doing this, so this is the correct fix.