fix: minimize label on initial render with a value

[RichardLindhout]

Update: see the right files here: #1441
This one is for history but something went wrong with rebasing.

Motivation

Fixes
#1380
#1354

The code here is never ran when you have an initial value

https://github.com/callstack/react-native-paper/blob/master/src/components/TextInput/TextInput.tsx#L210

There is an initial value available in the state so it's never changed there
https://github.com/callstack/react-native-paper/blob/master/src/components/TextInput/TextInput.tsx#L191

Test plan

Tested on Android, iOS and web

Extra

We should maybe rewrite this using hooks, that should prevent these bugs from happening since we could use 'useEffect'

[callstack-bot]

Hey @RichardLindhout, thank you for your pull request 🤗. The documentation from this branch can be viewed here.

[satya164]

We should maybe rewrite this using hooks

Unfortunately it'll break compatibility with older versions of RN. Maybe sometime for next major release.

[RichardLindhout]

Yeah feels a little bit wrong to minimize on every render. Maybe we should check if the labelLayout has changed and value is set and then to the minimize

[satya164]

Shouldn't the this.state.value check be enough for this? Since we synchronise this.props.value with this.state.value

[RichardLindhout]

Yes but it's done in the constructor too, so it does not change. Thats why minimize is never run (labelLayout should be calculated first)

[RichardLindhout]

I changed this check to be more performant.

[satya164]

I'm not following. Why wouldn't it change if it's done in constructor? Maybe we want to add prevState.labelLayout !== this.state.labelLayout here instead?

[RichardLindhout]

first render ->
props.value= 'dd'
state.value= 'dd'

componentDidUpdate() {
  if (prevState.value !== this.state.value) {
     // this will never run since values are the same
     // but label needs to minimize
  }
}

[RichardLindhout]

Do you see it?

[satya164]

I both navigated to the view and reloaded the page. It's not a fast refresh.

What React Native version are you on? I'm guessing it happens on new RN versions since I don't see it in our example app which has RN 0.59, and both the linked issues are on RN 0.61.

Which makes me think that there is some regression in new RN version. And if the change works around the issue, we can add it with a note that it's an workaround for bug in RN 0.61 where updated values don't apply for interpolation.

For the check, prevState.labelLayout !== this.state.labelLayout should be enough.

[RichardLindhout]

I'm on 0.61.3

[satya164]

Ok, so it's a regression in RN. Let's change the condition to add prevState.labelLayout !== this.state.labelLayout and add a comment that this is to workaround regression in RN 0.61

[RichardLindhout]

Did that, thanks for the all your help with this pull request.

[RichardLindhout]

@satya164 This should prevent unnecessary minimizeLabel calls, maybe I can change my code into this. Works on all platforms.

  (prevState.labelLayout !== this.state.labelLayout && this.props.value) ||
  (prevState.labelLayout !== this.state.labelLayout && this.props.defaultValue) ||

[satya164]

Do we need to check for this.props.value and this.props.defaultValue?

Also since this is an "or" clause, does it actually prevent the inner code from being run? since state.value is going to change every render anyway, triggering the condition.

[RichardLindhout]

This is only to prevent the Animated call when a parent component is changed, then the component will be rendered again.

E.g. when you have 1000 textinputs, and in 1 you type they will all 1000 be calling an animated minimizeLabel

[satya164]

I think that's because the conditions are wrong here: this.props.defaultValue and this.props.value shouldn't be in this condition at all. The main issue is this: #1440 (comment)

[RichardLindhout]

I think you have a valid point with default value, but the problem is with value. This fixed both unneeded minimizeLabel calls with defaultValue + fixes the transform origin on label when the textinput is rendered for the first time with a value prop.

[RichardLindhout]

But will interpolate run when nothing has changed parentState.labeled and start is not run?

Then it will never run with the new baseLabelTranslateX?

[satya164]

Interpolation runs every render. If anything inside the interpolation changes, it will apply the update.

[RichardLindhout]

Perhaps only when the animated value is changed somehow, thats the only thing that could cause this bug.

[satya164]

Can you rebase against master?

[RichardLindhout]

Done? I don't fully understand rebasing. I don't get why there are still conflicts since I pulled the upstream master.

[satya164]

@RichardLindhout you can try doing a merge instead.

[RichardLindhout]

I don't know, I did something wrong with merging.
@satya164 New pull request here: #1441