[$250] Expense - On selecting a category/ tags by multiple tapping, user directed to conversation page

[IuliiaHerets]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Version Number: V9. 0.69-0
Reproducible in staging?: Y
Reproducible in production?: Y
Issue reported by: Applause Internal Team

Action Performed:

1. Go to https://staging.new.expensify.com/home
2. Go to workspace settings
3. Enable tags
4. Create few tags
5. Tap workspace chat
6. Create an expense with category and tag
7. Open the expense
8. Open category and select a category by multiple tapping
9. Open tag and select a tag by multiple tapping

Expected Result:

On selecting a category/ tags by multiple tapping, user must not be directed to conversation/merchant page instead of expense details page.

Actual Result:

On selecting a category/ tags by multiple tapping, user directed to conversation/merchant page instead of expense details page.

Workaround:

Unknown

Platforms:

• Android: Standalone
• Android: HybridApp
• Android: mWeb Chrome
• iOS: Standalone
• iOS: HybridApp
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Bug6680799_1732988031088.Screenrecorder-2024-11-30-22-56-36-735_compress_1.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021864001016141250370
• Upwork Job ID: 1864001016141250370
• Last Price Increase: 2024-12-17
• Automatic offers:
• Krishna2323 | Contributor | 105632281

Issue Owner

Current Issue Owner: @alitoshmatov

[melvin-bot]

Triggered auto assignment to @puneetlath (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.

[Krishna2323]

Edited by proposal-police: This proposal was edited at 2024-11-30 21:59:49 UTC.

Proposal


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

Expense - On selecting a category/ tags by multiple tapping, user directed to conversation page

What is the root cause of that problem?

• The tag/category option can be clicked/pressed twice until the page is completely removed and when we press twice, the updateTag function is called twice and navigates back twice.
  

  App/src/pages/iou/request/step/IOURequestStepTag.tsx

  Lines 74 to 94 in 3cc88f5

  	const navigateBack = () => {
  	Navigation.goBack(backTo);
  	};
  	const updateTag = (selectedTag: Partial<ReportUtils.OptionData>) => {
  	const isSelectedTag = selectedTag.searchText === tag;
  	const searchText = selectedTag.searchText ?? '';
  	const updatedTag = IOUUtils.insertTagIntoTransactionTagsString(transactionTag, isSelectedTag ? '' : searchText, tagListIndex);
  	if (isEditingSplitBill) {
  	IOU.setDraftSplitTransaction(transactionID, {tag: updatedTag});
  	navigateBack();
  	return;
  	}
  	if (isEditing) {
  	IOU.updateMoneyRequestTag(transactionID, report?.reportID ?? '-1', updatedTag, policy, policyTags, policyCategories);
  	navigateBack();
  	return;
  	}
  	IOU.setMoneyRequestTag(transactionID, updatedTag);
  	navigateBack();
  	};

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


• We should return early from updateTag or similar functions on other selection list page if the isFocused state from @react-navigation/native is false.
• If we want we can apply this logic directly in SelectionList & SelectionListWithModal

What alternative solutions did you explore? (Optional)

• We can apply this logic in ScreenWrapper and few other selector modals by using pointerEvents prop. This way, press events won't be triggered when screen is not focused.
• We can also add a prop in ScreenWrapper to apply this logic conditionally.

            <View
                ref={ref}
                style={[styles.flex1, {minHeight}]}
                // eslint-disable-next-line react/jsx-props-no-spreading, react-compiler/react-compiler
                {...panResponder.panHandlers}
                testID={testID}
                pointerEvents={isFocused ? 'auto' : 'none'}
            >

App/src/components/ScreenWrapper.tsx

Lines 262 to 268 in 3cc88f5

	<View
	ref={ref}
	style={[styles.flex1, {minHeight}]}
	// eslint-disable-next-line react/jsx-props-no-spreading, react-compiler/react-compiler
	{...panResponder.panHandlers}
	testID={testID}
	>

Result

[melvin-bot]

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

[melvin-bot]

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

[dominictb]

Edited by proposal-police: This proposal was edited at 2024-12-04 10:01:24 UTC.

Proposal

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

Expense - On selecting a category/ tags by multiple tapping, user directed to conversation page

What is the root cause of that problem?

When updating tag, we'll redirect to previous page

App/src/pages/iou/request/step/IOURequestStepTag.tsx

Line 78 in ce282d3

const updateTag = (selectedTag: Partial<ReportUtils.OptionData>) => {

If users press twice quickly, this function will be triggered 2 times so we call goBack 2 times

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

We shouldn't lean on isFocused since we still can call onSelectRow multiple times when the page is focused.

We should fix this issue on BaseSelectionList to prevent the bug like this.

1. Define the new prop called shouldCloseListAfterSelectingRow, false by default. Then enable it in case we want to close list (by navigation,...).
2. Creating isExecuting ref, then update it in selectRow function

    const isExecuting = useRef(false);
 const selectRow = useCallback(

        (item: TItem, indexToFocus?: number) => {
            if(shouldCloseListAfterSelectingRow){
                if(isExecuting.current){
                    return;
                }
                isExecuting.current = true
        }
...

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

What alternative solutions did you explore? (Optional)

N/A

[dominictb]

@alitoshmatov Can you take a look at my proposal? Thanks

[melvin-bot]

@puneetlath, @alitoshmatov Eep! 4 days overdue now. Issues have feelings too...

[melvin-bot]

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

[puneetlath]

Bumped @alitoshmatov in Slack.

[melvin-bot]

@puneetlath, @alitoshmatov Still overdue 6 days?! Let's take care of this!

[alitoshmatov]

@Krishna2323 Can you recheck your solution, I am still able to reproduce the issue

[Krishna2323]

@alitoshmatov, the solutions works well. Can you please tell me the platform you are testing on? FYI, the isFocused solution is already applied in SelectionListWithModal.

App/src/components/SelectionListWithModal/index.tsx

Lines 81 to 86 in bcb8641

	const handleLongPressRow = (item: TItem) => {
	// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
	if (!turnOnSelectionModeOnLongPress || !isSmallScreenWidth || item?.isDisabled || item?.isDisabledCheckbox || (!isFocused && !isScreenFocused)) {
	return;
	}
	setLongPressedItem(item);

Monosnap.screencast.2024-12-11.21-09-01.mp4

[alitoshmatov]

Oh, my bad, I was changing tag component and testing category. Now it is working.

[alitoshmatov]

Thank you @dominictb for proposal

We shouldn't lean on isFocused since we still can call onSelectRow multiple times when the page is focused.

Can you elaborate, are you suggesting that we can still reproduce the issue with @Krishna2323 proposal.

In your solution it seems that after first selection you just disable selecting row, can it cause some issue where we might allow another selection after first one?

[melvin-bot]

@puneetlath @alitoshmatov 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!

[dominictb]

@alitoshmatov

Can you elaborate, are you suggesting that we can still reproduce the issue with @Krishna2323 proposal.

Yes, we can still can reproduce, but it's quite hard. Navigating is the async action, so we can perform multiple actions while navigating (We can't guarantee isFocus is false after the first click).

In your solution it seems that after first selection you just disable selecting row, can it cause some issue where we might allow another selection after first one?

This way won't cause the regression since shouldCloseListAfterSelectingRow is false by default, we just enable it in case we want close the list after selecting row.

[Krishna2323]

Yes, we can still can reproduce, but it's quite hard. Navigating is the async action, so we can perform multiple actions while navigating (We can't guarantee isFocus is false after the first click).

I don't think that's correct because the same solution has been applied in multiple places, and we have never faced the issue again. I don't think this issue can even be reproduced after applying the solution.

[melvin-bot]

@puneetlath, @alitoshmatov Huh... This is 4 days overdue. Who can take care of this?

[alitoshmatov]

@puneetlath I reviewed proposal and selected here. Please review my selection when you have time

[Krishna2323]

@puneetlath friendly bump for assignment based on the C+ decision.

[melvin-bot]

📣 @Krishna2323 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[puneetlath]

Sorry for the delay. I'm good with going with @Krishna2323's proposal.

[Krishna2323]

@alitoshmatov, the PR has been ready for a week. I tagged you in the PR but forgot to mention it here—sorry about that. You can start reviewing it when you get a chance. Thanks!

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.91-2 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• fix: On selecting a category/ tags by multiple tapping, user directed to conversation page. #55036

If no regressions arise, payment will be issued on 2025-02-06. 🎊

For reference, here are some details about the assignees on this issue:

• @alitoshmatov requires payment through NewDot Manual Requests
• @Krishna2323 requires payment automatic offer (Contributor)

[melvin-bot]

@alitoshmatov @alitoshmatov The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]

[puneetlath]

This was reverted, so not ready for payment.

[Krishna2323]

@puneetlath I have raised a new PR here, but I'm not sure if the issue is still reproducible. I haven't been able to reproduce it myself. @alitoshmatov could you please try🙏🏻