[HOLD for payment 2024-11-11] [$250] Invoices - Unable to select payment option in the dropdown in expense preview in the main chat

[lanitochka17]

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: 9.0.50-0
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: N/A
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause - Internal Team

Action Performed:

1. Go to staging.new.expensify.com
2. [User A] Send an invoice to User B
3. [User B] Go to invoice chat
4. [User B] Click on the pay button dropdown in the preview in the main chat (not the thread)
5. [User B] Select any option

Expected Result:

User will be able to select payment option from the dropdown

Actual Result:

User is unable to select payment option from the dropdown. App does not proceed after selecting any option from the dropdown

Workaround:

Unknown

Platforms:

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

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

Screenshots/Videos

Add any screenshot/video evidence

Bug6637233_1729136962344.20241017_114642.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021848792881281684450
• Upwork Job ID: 1848792881281684450
• Last Price Increase: 2024-10-22
• Automatic offers:
• truph01 | Contributor | 104576764

Issue Owner

Current Issue Owner: @OfstadC

[melvin-bot]

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

[lanitochka17]

@OfstadC 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

[lanitochka17]

We think that this bug might be related to #vip-bills

[abzokhattab]

Edited by proposal-police: This proposal was edited at 2024-10-17 15:38:53 UTC.

Proposal

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

Invoices - Unable to select payment option in the dropdown in expense preview in the main chat

What is the root cause of that problem?

In the ReportPreview component, when users click on options in the SettlementButton that have submenus, the submenu fails to display properly. Instead, the popover menu blinks

The issue occurs because the menuItems prop passed to the PopoverMenu component changes on every render due to unstable function references. Specifically:

• In SettlementButton, the onOptionSelected function changes on every render because it's recreated each time without memoization.

• In ButtonWithDropdownMenu, the menuItems array is recreated on every render because it depends on onOptionSelected and options, which are changing references.
  This causes the PopoverMenu to reset its internal state on each render, preventing the submenu from displaying correctly.

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

1. Memoize menuItems in ButtonWithDropdownMenu:

• Use useMemo to memoize the menuItems array, ensuring it only changes when options or onOptionSelected change.

• This prevents unnecessary re-renders and state resets in PopoverMenu.

// In ButtonWithDropdownMenu.js
const menuItems = useMemo(
    () =>
        options.map((item, index) => ({
            ...item,
            onSelected: item.onSelected
                ? () => item.onSelected?.()
                : () => {
                      onOptionSelected?.(item);
                      setSelectedItemIndex(index);
                  },
            shouldCallAfterModalHide: true,
        })),
    [options, onOptionSelected],
);

2. Memoize onOptionSelected in SettlementButton:

• Use useCallback to memoize the onOptionSelected function in SettlementButton, ensuring it has a stable reference.

• This prevents onOptionSelected from changing on every render, which stabilizes the menuItems prop.

// In SettlementButton.js
const onOptionSelected = useCallback(
    (option) => {
        if (policyID === '-1') {
            return;
        }
        savePreferredPaymentMethod(policyID, option.value);
    },
    [policyID],
);

By making these changes, we stabilize the menuItems prop passed to PopoverMenu, which prevents it from resetting its state and allows submenus to display correctly.

POC

Screen.Recording.2024-10-17.at.17.37.00.mov

What alternative solutions did you explore? (Optional)

[truph01]

Edited by proposal-police: This proposal was edited at 2024-10-19 06:03:19 UTC.

Proposal

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

User is unable to select payment option from the dropdown. App does not proceed after selecting any option from the dropdown

What is the root cause of that problem?

1. users open payment popover, hovered is true

2. they hover to payment option -> hovered is false

3. they select any option, we will render the sublist

App/src/components/PopoverMenu.tsx

Line 179 in 66cf824

setCurrentMenuItems([...selectedItem.subMenuItems]);

The hover event will be detected at the moment when we remove the main list then show the sublist

-> The menuItems will be re-calculated

If the menuItems get changed, we will reset the current menu items

App/src/components/PopoverMenu.tsx

Lines 261 to 267 in 66cf824

	useLayoutEffect(() => {
	if (menuItems.length === 0) {
	return;
	}
	setEnteredSubMenuIndexes(CONST.EMPTY_ARRAY);
	setCurrentMenuItems(menuItems);
	}, [menuItems]);

The RCA of this issue is: hovered value change while showing the sublist (The RCA is not memorize problem since this issue doesn't happen on MoneyReportHeader)

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

1. We have the same issue here, and the PR author introduce shouldFreezeCapture ro freeze capture hover event when the popover is visible

so we should apply the same in here

<Hoverable
                shouldHandleScroll
                isDisabled={draftMessage !== undefined}
                shouldFreezeCapture={isPaymentMethodPopoverActive}
            >

2. We also need to prevent resetting hover in

App/src/components/Hoverable/ActiveHoverable.tsx

Line 129 in 66cf824

setIsHovered(false);

if shouldFreezeCapture is true

3. In case user press outside the action, we need to reset hover to false by triggering unsetHoveredIfOutside at the begining

we just update this line to

document.addEventListener('mouseover', unsetHoveredIfOutside, true);

What alternative solutions did you explore? (Optional)

[OfstadC]

Added to BillPay - just waiting on confirmation here

[melvin-bot]

@OfstadC Huh... This is 4 days overdue. Who can take care of this?

[melvin-bot]

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

[melvin-bot]

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

[jayeshmangwani]

Thanks for the proposals. I have tested both, and both will fix the issue. However, I agree with @truph01 that we should return early in the hoverable component to prevent flickering of the hover state value and freeze the hover, This approach will resolve our issue

[jayeshmangwani]

@truph01 's Proposal looks good to me. We can use shouldFreezeCapture when the popover is visible, so that the menuItems don't recalculate when pressing the SettlementButton.

🎀 👀 🎀 C+ reviewed

[melvin-bot]

Triggered auto assignment to @aldo-expensify, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[melvin-bot]

📣 @truph01 🎉 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 📖

[jayeshmangwani]

@truph01 , could you let us know when we can expect the PR?

[jayeshmangwani]

friendly bump @truph01

[truph01]

I will open PR in a few hours

[jayeshmangwani]

Thanks for the update

[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.56-9 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• fix: Unable to select payment option in the dropdown in expense preview in the main chat #51645

If no regressions arise, payment will be issued on 2024-11-11. 🎊

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

• @jayeshmangwani requires payment through NewDot Manual Requests
• @truph01 requires payment automatic offer (Contributor)

[melvin-bot]

@jayeshmangwani @OfstadC 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]

[OfstadC]

@jayeshmangwani Please complete the BZ checklist before the payment date. Thanks 😃

[OfstadC]

@jayeshmangwani Please complete the BZ checklist so I can issue payment

[OfstadC]

Payment Summary

• @jayeshmangwani due $250 via manual New Expensify request
• @truph01 paid $250 via Upwork

[jayeshmangwani]

Really sorry, I missed the BZ checklist , Let me complete it now!

[jayeshmangwani]

BugZero Checklist:

• [Contributor] Classify the bug:

Bug classification

Source of bug:

• 1a. Result of the original design (eg. a case wasn't considered)
• 1b. Mistake during implementation
• 1c. Backend bug
• 1z. Other: We changed a memoization condition, which resulted in this bug.

Where bug was reported:

• 2a. Reported on production
• 2b. Reported on staging (deploy blocker)
• 2c. Reported on a PR
• 2z. Other:

Who reported the bug:

• 3a. Expensify user
• 3b. Expensify employee
• 3c. Contributor
• 3d. QA
• 3z. Other:

• [Contributor] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake.

  Link to comment: use PopoverMenu for account switcher modal #49051 (comment)

• [Contributor] If the regression was CRITICAL (e.g. interrupts a core flow) A discussion in #expensify-open-source has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner.

  Link to discussion: N/A

• [Contributor] If it was decided to create a regression test for the bug, please propose the regression test steps using the template below to ensure the same bug will not reach production again.

Regression Test Proposal

1. Open the app on two devices as User A and User B.
2. [User A]: Send an invoice to User B.
3. [User B]: Open the invoice chat.
4. [User B]: In the main chat preview (not the thread), click on the Pay button dropdown.
5. [User B]: Select any payment option from the dropdown.
6. Verify that User B can select a payment option from the dropdown successfully.

Do we agree 👍 or 👎

[OfstadC]

https://github.com/Expensify/Expensify/issues/443345

[OfstadC]

Thanks @jayeshmangwani !

Payment Summary updated

[garrettmknight]

$250 approved for @jayeshmangwani