[$125] [Search v1.2] - Hold expense modal appears on the top left when expense is held in Search

@rojiphil

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: 9.0.35-0
Reproducible in staging?: Y
Reproducible in production?: Y
Email or phone of affected tester (no customers): applausetester+kh010901@applause.expensifail.com
Issue reported by: Applause Internal Team

Action Performed:

Go to staging.new.expensify.com
Go to DM.
Submit two expenses.
Go to Search.
Click on the submitted expense.
Click on the header > Hold.
Enter reason and save it.

Expected Result:

Hold expense modal will open as RHP.

Actual Result:

Hold expense modal appears on the top left when expense is held in Search.

Workaround:

Unknown

Platforms:

Screenshots/Videos

Bug6604298_1726402698827!Screenshot_2024-09-15_at_20 14 25

Bug6604298_1726402698833.20240915_201411.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~021835782276333225172
Upwork Job ID: 1835782276333225172
Last Price Increase: 2024-09-23
Automatic offers:

rojiphil | Reviewer | 104106001
Krishna2323 | Contributor | 104106003

Issue Owner

Current Issue Owner: @rojiphil

@sakluger

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

@sakluger

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

Proposal

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

Hold expense modal appears on the top left when expense is held in Search.

What is the root cause of that problem?

In here we are passing down the prop shouldUseNarrowLayout, and in web screen, its value will be true when we are in RHP on the Search page logic here. As a result, it will show this popover instead of redirecting to the route PROCESS_MONEY_REQUEST_HOLD.

App/src/components/MoneyReportHeader.tsx

Lines 255 to 262 in 84793c8

    
               if (shouldUseNarrowLayout) { 
        
                   if (Navigation.getActiveRoute().slice(1) === ROUTES.PROCESS_MONEY_REQUEST_HOLD) { 
        
                       Navigation.goBack(); 
        
                   } 
        
               } else { 
        
                   Navigation.navigate(ROUTES.PROCESS_MONEY_REQUEST_HOLD); 
        
               } 
        
           }, [shouldUseNarrowLayout, shouldShowHoldMenu]);

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

We should pass isSmallScreenWidth instead of shouldUseNarrowLayout in this case like we did here and here. And do the same with MoneyRequestHeader

          <MoneyReportHeader
              report={report}
              policy={policy}
              transactionThreadReportID={transactionThreadReportID}
              reportActions={reportActions}
              shouldUseNarrowLayout={isSmallScreenWidth}
              onBackButtonPress={onBackButtonPress}
          />

App/src/hooks/useResponsiveLayout/index.ts

Line 68 in 84793c8

const shouldUseNarrowLayout = isSmallScreenWidth || isInNarrowPaneModal;

What alternative solutions did you explore? (Optional)

NA

Screen.Recording.2024-09-16.at.22.22.28.mov

Proposal 

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

Search - Hold expense modal appears on the top left when expense is held in Search

What is the root cause of that problem?

shouldUseNarrowLayout is used to display the modal and shouldUseNarrowLayout will be true even if isInNarrowPaneModal is true.

App/src/components/MoneyReportHeader.tsx

Line 425 in 8e3821c

{shouldUseNarrowLayout && shouldShowHoldMenu && (

App/src/hooks/useResponsiveLayout/index.ts

Lines 60 to 68 in 8e3821c

    
           // The component calling this hook is in a "narrow pane modal" if: 
        
           const isInNarrowPaneModal = 
        
               // it's a child of the right-docked modal 
        
               activeModalType === CONST.MODAL.MODAL_TYPE.RIGHT_DOCKED || 
        
               // or there's a "right modal navigator" or "left modal navigator" on the top of the root navigation stack 
        
               // and the component calling this hook is not the child of another modal type, such as a confirm modal 
        
               (isDisplayedInNarrowModalNavigator && !activeModalType); 
        
           const shouldUseNarrowLayout = isSmallScreenWidth || isInNarrowPaneModal;

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

In MoneyReportHeader & MoneyRequestHeader we should get isSmallScreenWidth from useResponsiveLayout hook and use that instead of shouldUseNarrowLayout.

We also need to update the shouldUseNarrowLayout in the useEffect in both components.

App/src/components/MoneyReportHeader.tsx

Lines 250 to 262 in 8e3821c

    
           useEffect(() => { 
        
               if (!shouldShowHoldMenu) { 
        
                   return; 
        
               } 
        
               if (shouldUseNarrowLayout) { 
        
                   if (Navigation.getActiveRoute().slice(1) === ROUTES.PROCESS_MONEY_REQUEST_HOLD) { 
        
                       Navigation.goBack(); 
        
                   } 
        
               } else { 
        
                   Navigation.navigate(ROUTES.PROCESS_MONEY_REQUEST_HOLD); 
        
               } 
        
           }, [shouldUseNarrowLayout, shouldShowHoldMenu]);

Note

We should not pass shouldUseNarrowLayout={isSmallScreenWidth} to MoneyReportHeader & MoneyRequestHeader, passing isSmallScreenWidth to shouldUseNarrowLayout will cause issues as it is being also used for showing back button and money request buttons according to the IOU report layout size.

What alternative solutions did you explore? (Optional)

Result

This feels like a fairly minor issue and change, so I will set the price at $125. Feel free to leave a note if you disagree.

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

@rojiphil

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

@luacmartins

Current assignee @luacmartins is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

@rojiphil

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

Offer link
Upwork job

@Krishna2323

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

@Krishna2323

@Krishna2323 do you have a PR for this one yet?

@Krishna2323

Bump @Krishna2323.

PR will be up within 24 hours.

@rojiphil

@rojiphil, @sakluger, @luacmartins, @Krishna2323 Whoops! This issue is 2 days overdue. Let's get this updated quick!

@sakluger

@sakluger @rojiphil, this issue has been fixed in this PR. The comment mentioning the this bug the PR.

cc: @bernhardoj @rayane-djouah

@sakluger

this issue has been fixed in this PR

Oh! Then, let's close this issue and move on. cc @sakluger

Navigation Menu

Repository navigation

[$125] [Search v1.2] - Hold expense modal appears on the top left when expense is held in Search #49275

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

Result

16 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	if (shouldUseNarrowLayout) {
	if (Navigation.getActiveRoute().slice(1) === ROUTES.PROCESS_MONEY_REQUEST_HOLD) {
	Navigation.goBack();
	}
	} else {
	Navigation.navigate(ROUTES.PROCESS_MONEY_REQUEST_HOLD);
	}
	}, [shouldUseNarrowLayout, shouldShowHoldMenu]);

	// The component calling this hook is in a "narrow pane modal" if:
	const isInNarrowPaneModal =
	// it's a child of the right-docked modal
	activeModalType === CONST.MODAL.MODAL_TYPE.RIGHT_DOCKED \|\|
	// or there's a "right modal navigator" or "left modal navigator" on the top of the root navigation stack
	// and the component calling this hook is not the child of another modal type, such as a confirm modal
	(isDisplayedInNarrowModalNavigator && !activeModalType);

	const shouldUseNarrowLayout = isSmallScreenWidth \|\| isInNarrowPaneModal;

	useEffect(() => {
	if (!shouldShowHoldMenu) {
	return;
	}

	if (shouldUseNarrowLayout) {
	if (Navigation.getActiveRoute().slice(1) === ROUTES.PROCESS_MONEY_REQUEST_HOLD) {
	Navigation.goBack();
	}
	} else {
	Navigation.navigate(ROUTES.PROCESS_MONEY_REQUEST_HOLD);
	}
	}, [shouldUseNarrowLayout, shouldShowHoldMenu]);

[$125] [Search v1.2] - Hold expense modal appears on the top left when expense is held in Search #49275

Description

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

Activity

melvin-bot commented on Sep 16, 2024

IuliiaHerets commented on Sep 16, 2024

nkdengineer commented on Sep 16, 2024

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

Krishna2323 commented on Sep 16, 2024

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

Result

sakluger commented on Sep 16, 2024

melvin-bot commented on Sep 16, 2024

melvin-bot commented on Sep 16, 2024

16 remaining items

melvin-bot commented on Sep 24, 2024

melvin-bot commented on Sep 24, 2024

melvin-bot commented on Sep 24, 2024

sakluger commented on Sep 26, 2024

sakluger commented on Sep 27, 2024

Krishna2323 commented on Sep 27, 2024

melvin-bot commented on Sep 27, 2024

Krishna2323 commented on Sep 28, 2024

rojiphil commented on Sep 29, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

Proposal 

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