[Bug] [2020.04] TextInput onChange lose focus

[aljesusg]

After upgrade when the value change the textInput lost focus, it seems that is rendering again the component in onChange event. (See Gif)

Component

<TextInput
                value={this.state.addEgressHost.host}
                type="text"
                id="addEgressHost"
                key="addEgressHost"
                aria-describedby="add egress host"
                name="addHost"
                onChange={this.onAddHost}
                isValid={this.state.validEgressHost}
              />

OnAddHost method

  onAddHost = (value: string, _) => {
    const host = value.trim();
    this.setState({
      addEgressHost: {
        host: host
      },
      validEgressHost: isServerHostValid(host)
    });
  };

GIF

[aljesusg]

@redallen Do you know who I can talk to about this? Thanks !!!

[redallen]

Taking a look now.

[redallen]

I'm unable to reproduce this using your code (here's the codesandbox I made that doesn't seem to have your problem: https://codesandbox.io/s/autumn-framework-w26vd). The only change we made to TextInput in our 2020.04 release was my PR #3919 for fixing forward ref types, are you by chance passing a ref?

If you can link a reproducible example or tell me how to load this problematic page while running kiali I can help further.

[aljesusg]

Ok @redallen let me do some research. Thanks !!!

[aljesusg]

@redallen After some tests I found the problem, this happens when the component is in a table. So something changed here.

Here you have the sandbox https://codesandbox.io/s/texinput-table-ggx1l?file=/index.js

Is similar to our code https://github.com/kiali/kiali-ui/blob/master/src/pages/IstioConfigNew/GatewayForm.tsx

Somethign change here maybe we shopuld cast this to ICell ?? something change when PF render this.
Notrice that all textInputs inside the row are affected

[redallen]

I'm investigating this now and it seems to be a problem on any <input> element inside of a table row. It also appears to occur on "@patternfly/react-table": "2.24.41" which is what kiali is using. Will update once I find a fix...

[redallen]

Not quite sure the root cause yet, but a temporary fix is to use TextInputBase instead of TextInput. I'm debugging more of why our Table component doesn't like forwarded refs...

[aljesusg]

Ok thanks @redallen , I'm going to check TextInputBase. Thanks !!!

[redallen]

TLDR; Another table bug.

I've spent most of the day on this and am bewildered. My next step would be to make a minimal reproduction with a stripped down version of our Table component, but I'm not quite sure how to do that yet since I lack a good understanding of how Table works with React's lifecycle methods.

To sum up what I've found, using TextInputBase causes typing in the input to only update TextInputBase inside the <Table>. However, using React.forwardRef on TextInputBase causes the whole <RowWrapper> (and maybe even higher up the tree?) to rerender, and so the <input> loses focus. I assume this has to do internally with how React implements forwardRef when diff-checking. However, the fault is in our Table since <div>{this.rows()[0].cells}</div> works just fine.

The key implementation takeaway for TextInputBase vs TextInput is that TextInputBase is a normal React Component (that is typeof TextInputBase === 'function'), while TextInput is a special React object (that is typeof TextInput === 'object') with a .render prop.

I've created a branch for debugging if anyone wants to further investigate.

[aljesusg]

ok @redallen is working with TextInputBase we are moving forward with this and I'm going to document this to change it in next PF4 release or after fix. Thanks