[HOLD for payment 2024-07-24] [$250] [Uneven Splits] [Polish] Android - Split amount is not reset when tapping Reset button when keyboard is up

[m-natarajan]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Version Number: 1.4.71-0 Reproducible in staging?: y Reproducible in production?: new feature If this was caught during regression testing, add the test name, ID and link from TestRail: Email or phone of affected tester (no customers): Logs: https://stackoverflow.com/c/expensify/questions/4856 Expensify/Expensify Issue URL: Issue reported by: Applause internal team Slack conversation:

Action Performed:

1. Launch New Expensify app.
2. Go to FAB > Split expense.
3. Enter amount and select a few users.
4. Tap on split amount input.
5. Manually enter the split amount.
6. With keyboard up, tap on Reset button.

Expected Result:

The split amount will be reset (mweb behavior when keyboard is up).

Actual Result:

The keyboard is dismissed and the split amount is not reset.

Workaround:

unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• [x] Android: Native
• [ ] Android: mWeb Chrome
• [x] iOS: Native
• [ ] iOS: mWeb Safari
• [ ] MacOS: Chrome / Safari
• [ ] MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/Expensify/App/assets/38435837/bc3628b3-3d50-4a64-aff8-c68eef43c839

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~010af563a3cc74c7c2
• Upwork Job ID: 1787911203897004032
• Last Price Increase: 2024-05-28

Issue Owner

Current Issue Owner: @alexpensify

[melvin-bot[bot]]

Triggered auto assignment to @rlinoz (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

[melvin-bot[bot]]

Triggered auto assignment to @isabelastisser (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[github-actions[bot]]

:wave: 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:

1. Identify the pull request that introduced this issue and revert it.
2. Find someone who can quickly fix the issue.
3. Fix the issue yourself.

[m-natarajan]

@isabelastisser FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors.

[m-natarajan]

We think this bug might be related to #vip-split

[rlinoz]

This is related to the new uneven split feature https://github.com/Expensify/App/pull/40386

I don't think this needs to be a blocker, as the reset feature still works, but curious for @youssef-lr's thoughts.

Also, are you taking this issues or should I make it external?

[melvin-bot[bot]]

Job added to Upwork: https://www.upwork.com/jobs/~010af563a3cc74c7c2

[melvin-bot[bot]]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @eVoloshchak (External)

[youssef-lr]

Thanks @amyevans, let's open this up for contributors as I haven't been able to figure it out yet and don't have time to do so.

[bernhardoj]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Can't press the reset button when the keyboard is visible.

What is the root cause of that problem?

It's the default behavior of a list view that when we press, the keyboard will close and the button won't receive the click/press. To customize that, we have keyboardShouldPersistTaps prop. We can set it to always so the view can receive click/press while the keyboard is shown.

The list used in the money request confirmation page is BaseOptionsSelector which uses BaseOptionsList. SectionList in BaseOptionsList already set it to always https://github.com/Expensify/App/blob/e0b6a06d2f34f35cda96ccc4d05252ede111c732/src/components/OptionsList/BaseOptionsList.tsx#L256

but we have a nested scroll view here that doesn't set the keyboardShouldPersistTaps. https://github.com/Expensify/App/blob/e0b6a06d2f34f35cda96ccc4d05252ede111c732/src/components/OptionsSelector/BaseOptionsSelector.js#L632-L640

What changes do you think we should make in order to solve the problem?

Set both ScrollView keyboardShouldPersistTaps to 'always'. This will reset the split amount, however, the focused input amount will only be updated after it's blurred because of formatAmountOnBlur prop. To fix it, we can manually dismiss the keyboard using Keyboard.dismiss in here. https://github.com/Expensify/App/blob/e0b6a06d2f34f35cda96ccc4d05252ede111c732/src/components/MoneyRequestConfirmationList.tsx#L528

https://github.com/Expensify/App/assets/50919443/af5c7c14-aaa5-4179-b0a1-925bed411bb7

[youssef-lr]

@bernhardoj is that the recording of the solution? if yes then I think we'll still have to tap twice on Reset?

[bernhardoj]

is that the recording of the solution?

Yes

if yes then I think we'll still have to tap twice on Reset?

Nope, only once.

[kaushiktd]

Please re-state the problem that we are trying to solve in this issue.

Android - Split - Split amount is not reset when tapping Reset button when keyboard is up

What is the root cause of that problem?

The root cause of the issue lies within the setNewAmount function. Upon analyzing the code, it was found that this function was not correctly updating the input field's value upon resetting. This behavior was attributed to the logic within setNewAmount, which failed to properly handle the reset action triggered by the reset button.

What changes do you think we should make in order to solve the problem?

To solve the problem, we need to introduce a flag resetClicked, which is set to true when the reset button is clicked. This flag will indicate that the input field's value should be reset. const [resetClicked, setResetClicked] = useState(false); The resetClicked state should be updated when the reset button is pressed: add after it https://github.com/Expensify/App/blob/e8ae3c5acedf0e6788dc574c7b6f3043ca37092a/src/components/MoneyRequestConfirmationList.tsx#L552 setResetClicked(true);

Next, we need to pass the resetClicked flag as a prop to the MoneyRequestAmountInput component: https://github.com/Expensify/App/blob/e8ae3c5acedf0e6788dc574c7b6f3043ca37092a/src/components/MoneyRequestConfirmationList.tsx#L499

<MoneyRequestAmountInput
    resetClicked={resetClicked}
    // Other props
/>

Within the MoneyRequestAmountInput component,

• Use a useRef flag (resetFlag) to manage the reset state.
• When resetClicked is true, set the resetFlag and call setNewAmount with the formatted amount.
• Adjust the selection state to move the cursor to the end.

const resetFlag = useRef(false);

Update the effect handling resetClicked to call setNewAmount with the formatted amount

   useEffect(() => {
        if (resetClicked) {
            resetFlag.current = true;
            setNewAmount(selectedAmountAsString);
            setSelection({start: formattedAmount.length, end: formattedAmount.length}); // Move cursor to the end

        }
    }, [resetClicked, selectedAmountAsString, setNewAmount]);

Now, we modify the setNewAmount function to use resetButtonState to determine whether to reset the input field's value or not: https://github.com/Expensify/App/blob/e8ae3c5acedf0e6788dc574c7b6f3043ca37092a/src/components/MoneyRequestAmountInput.tsx#L156

  const resultAmount = resetFlag.current ? selectedAmountAsString : strippedAmount;
                onAmountChange?.(resultAmount);

                resetFlag.current = false; // Reset the flag after handling
                return resultAmount;

With these changes, the MoneyRequestAmountInput component will correctly update its input field's value based on the resetClicked flag, ensuring that the reset action works as expected. video:-https://drive.google.com/file/d/1AH9jUNwuUnLvar-XtcnOgai2rAlCK0DH/view?usp=sharing

[youssef-lr]

Nope, only once.

Ah okay. So it seems like there is a slight delay due to having to dismiss the keyboard first. Anyway we can eliminate it @bernhardoj or there's really no way around it?

[youssef-lr]

Not overdue.

[bernhardoj]

@youssef-lr eliminate which part? The keyboard dismiss?

[youssef-lr]

The slight delay from when we press Reset and the shares are actually reset

[kaushiktd]

@youssef-lr I found the issue could be resolved without altering ScrollView components. Including Keyboard.dismiss() within resetSplitShares function effectively addressed the problem.

[bernhardoj]

Hmm, looks like the delay is because we dismissed the keyboard. Maybe the animation delays the onyx update. I think we need to either rework the formatOnBlur so the amount is updated when we reset it.

Or we can let the onyx complete first then dismiss the keyboard. IOU.resetSplitShares(transaction).then(Keyboard.dismiss)

https://github.com/Expensify/App/assets/50919443/ace4cf3e-9172-4c73-a6f3-6b76ff722fe7

[melvin-bot[bot]]

@eVoloshchak, @youssef-lr, @isabelastisser Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[isabelastisser]

@eVoloshchak, please provide an update. Thanks!

[eVoloshchak]

According to the issue description

Expected Result: The split amount will be reset (mweb behavior when keyboard is up).

Is the expected behavior is for the keyboard to stay open when the 'Reset' button is pressed? Seems like the desired behavior, but that's not the current behavior on mWeb

@youssef-lr, could you confirm what is the expected behavior here please?

[melvin-bot[bot]]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[youssef-lr]

Is the expected behavior is for the keyboard to stay open when the 'Reset' button is pressed? Seems like the desired behavior, but that's not the current behavior on mWeb

This would be great if possible

[eVoloshchak]

@bernhardoj, @kaushiktd, do you think this is achievable?

[melvin-bot[bot]]

@eVoloshchak, @youssef-lr, @isabelastisser Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[kaushiktd]

@eVoloshchak By simply adding the Keyboard.dismiss() statement within the resetSplitShares function, we effectively addressed the problem.

Currently, the keyboard behavior is such that it hides when the 'Reset' button is pressed.

Regarding the expected behavior, if the desired outcome is for the keyboard to remain open when the 'Reset' button is pressed, this deviates from the current solution where the keyboard is dismissed. If keeping the keyboard open is preferred, I can check its possibility. Will update you on the same soon.

[kaushiktd]

@eVoloshchak Proposal updated

[youssef-lr]

Not overdue.

[youssef-lr]

bump @eVoloshchak

[isabelastisser]

Hey @eVoloshchak, I DM'd you for visibility.

[melvin-bot[bot]]

@eVoloshchak, @youssef-lr, @isabelastisser Still overdue 6 days?! Let's take care of this!

[eVoloshchak]

@kaushiktd, thank you for the updated proposal! The behavior on the video matches the expected behavior, but the implementation can be improved

To solve the problem, we need to introduce a flag resetClicked, which is set to true when the reset button is clicked. This flag will indicate that the input field's value should be reset.

Why do we need to do that? Let's dig deeper and figure out the actual root cause for this, introducing an additional flag does look like a workaround. Is there a wrong newAmount passed to setNewAmount function?

[melvin-bot[bot]]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[melvin-bot[bot]]

@eVoloshchak @youssef-lr @isabelastisser this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[kaushiktd]

@eVoloshchak

Why do we need to do that?

The problem occurs because the input field remains focused, implying it's within the setNewAmount function. To signal a reset button click, we must set a resetClicked flag. This flag enables setNewAmount to detect the reset event and update the input field's value accordingly. Interestingly, in the web version, the reset button click doesn't maintain focus on the input field, ensuring proper detection of the reset event and correct value updating.

There is no problem in newAmount passed to setNewAmount function.

[youssef-lr]

To give some context, the amount is not updated if the input is focused. I think we should find a way to bypass this if the amount change is triggered from the reset button. The reason I coded things this way initially was to prevent auto formatting the amount (e.g. from 5 to 5.00) when the user is typing it, and only do so when the input is blurred.

[kaushiktd]

@youssef-lr
My code ensures that the auto-formatting functionality remains unaffected while allowing the input field to reset the amount when the reset button is clicked. Additionally, as you can see in the video, the cursor remains focused on the input field after the reset

[youssef-lr]

That's looking good, can we have the cursor move to the end when the amount is reset?

[youssef-lr]

@kaushiktd also do you have a test branch?

[kaushiktd]

That's looking good, can we have the cursor move to the end when the amount is reset?

Let me dig up something!

And what do you mean by test branch?

[kaushiktd]

@youssef-lr Proposal updated

[melvin-bot[bot]]

@eVoloshchak, @youssef-lr, @isabelastisser Whoops! This issue is 2 days overdue. Let's get this updated quick!

[melvin-bot[bot]]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[melvin-bot[bot]]

@eVoloshchak, @youssef-lr, @isabelastisser 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

[isabelastisser]

@eVoloshchak @youssef-lr can you please provide an update? Thanks!

[melvin-bot[bot]]

❌ There was an error making the offer to @kaushiktd for the Contributor role. The BZ member will need to manually hire the contributor.

[youssef-lr]

I think the proposal from @kaushiktd makes sense to me, please tag me once the PR is up

[kaushiktd]

Contributor details Your Expensify account email: prakash.techdoodles@gmail.com Upwork Profile Link: https://www.upwork.com/freelancers/~01e7f7b7a75799e9db

[kaushiktd]

My previous comment is for bot because I'm getting error in receiving offer for last couple of issue. So just tried to update the details.