You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
IOU - Unable to erase the first digit in BNP and Next button is disabled #9362
👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open StagingDeployCash deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:
Identify the pull request that introduced this issue and revert it.
it seems like this must be related to the Localize currency PR which refactored the page a bit as well. What's odd to me is that the production version is fine and the PR which added the blurOnSubmit prop is not causing it either, I tested that.
@amyevans and @parasharrajat Double-confirmed that reverting the #9351 does not fix this problem, I played around a bit more and I could not find what directly from the PR which is in production could cause this, but also I cant see anything in the current staging release which would touch this 😬
I have stopped my VM etc, but the only other PR which is distantly related to this could be this as there are some changes to the BaseTextInput which is used in this problem underneath. I would try to revert that PR locally and test if it prevails.
Proposal:
The issue here is that the this.props.value passed down to BaseTextInput is regarded as falsey for an empty string. This causes the second condition on Line 68 to be true since the value of inputValue matches the state value.
The solution seems to as simple as handling the empty string in the prop value separately.
componentDidUpdate() {
// Activate or deactivate the label when value is changed programmatically from outside
// Only update when value prop is provided
const inputValue = this.props.value || this.input.value;
if (_.isEmpty(inputValue) || (this.state.value === inputValue && _.isEmpty(this.props.value))) {
return;
}
if (inputValue === '' || _.isEmpty(this.props.value)) {
this.input.clear();
}
if (_.isEmpty(this.props.value)) {
this.deactivateLabel();
// eslint-disable-next-line react/no-did-update-set-state
this.setState({value: this.props.value});
return;
}
// eslint-disable-next-line react/no-did-update-set-state
this.setState({value: inputValue});
this.input.setNativeProps({text: inputValue});
// In some cases, When the value prop is empty, it is not properly updated on the TextInput due to its uncontrolled nature, thus manually clearing the TextInput.
if (inputValue) {
this.activateLabel();
} else if (!this.state.isFocused) {
this.deactivateLabel();
}
}
Activity
OSBotify commentedon Jun 8, 2022
👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open
StagingDeployCashdeploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:melvin-bot commentedon Jun 8, 2022
Triggered auto assignment to @amyevans (
Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.mountiny commentedon Jun 8, 2022
cc @mdneyazahmad and @parasharrajat
Updated: you all good @mdneyazahmad , sorry for the ping, it was different PR :)
it seems like this must be related to the Localize currency PR which refactored the page a bit as well. What's odd to me is that the production version is fine and the PR which added the blurOnSubmit prop is not causing it either, I tested that.
parasharrajat commentedon Jun 8, 2022
@mountiny did you try reverting the PR and see if that is really causing it? I tested this during PR review but I will look again to confirm this.
mountiny commentedon Jun 8, 2022
I tried reverting this PR and it does not cause it, even without it the problem occurs.
I havent tried reverting this PR because I confirmed it is not reproducible in production and that one is already in production.
mountiny commentedon Jun 8, 2022
@amyevans and @parasharrajat Double-confirmed that reverting the #9351 does not fix this problem, I played around a bit more and I could not find what directly from the PR which is in production could cause this, but also I cant see anything in the current staging release which would touch this 😬
Further investigation will be needed.
amyevans commentedon Jun 8, 2022
Okay, definitely appreciate the investigation you've done @mountiny! I'm taking a look now as well
mountiny commentedon Jun 8, 2022
I have stopped my VM etc, but the only other PR which is distantly related to this could be this as there are some changes to the BaseTextInput which is used in this problem underneath. I would try to revert that PR locally and test if it prevails.
amyevans commentedon Jun 8, 2022
Great suggestion - reverting #9056 locally does indeed fix the issue. This commit changing the
BaseTextInputis the source (cc @luacmartins)sig5 commentedon Jun 9, 2022
Proposal:
The issue here is that the
this.props.valuepassed down toBaseTextInputis regarded as falsey for an empty string. This causes the second condition on Line 68 to be true since the value ofinputValuematches thestatevalue.App/src/components/TextInput/BaseTextInput.js
Lines 66 to 70 in 9dfa7e4
The solution seems to as simple as handling the empty string in the prop value separately.
PR: #9367 (comment)
luacmartins commentedon Jun 9, 2022
Put up a fix here
luacmartins commentedon Jun 9, 2022
Sorry @sig5, but we will fix this one internally. Thanks for the interest though!
mountiny commentedon Jun 9, 2022
Fixed, nice work!