[HOLD for payment 2024-10-04][$250] [Search v2.4] [App] Extend the 🔍icon and search router to all pages by adding “Search”

[luacmartins]

Implement this section of the doc

cc @Kicu @289Adam289 @Guccio163 @SzymczakJ

Issue Owner

Current Issue Owner: @

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021834649747497603669
• Upwork Job ID: 1834649747497603669
• Last Price Increase: 2024-09-13
• Automatic offers:
• ikevin127 | Reviewer | 104036646

Issue Owner

Current Issue Owner: @sakluger

[melvin-bot]

Triggered auto assignment to @sakluger (NewFeature), see https://stackoverflowteams.com/c/expensify/questions/14418#:~:text=BugZero%20process%20steps%20for%20feature%20requests for more details. Please add this Feature request to a GH project, as outlined in the SO.

[SzymczakJ]

Hey! I’m Jakub Szymczak from Software Mansion, an expert agency, and I’d like to work on this issue!

[melvin-bot]

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

[melvin-bot]

Current assignee @ikevin127 is eligible for the External assigner, not assigning anyone new.

[melvin-bot]

@sakluger, @luacmartins, @ikevin127, @SzymczakJ Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[sakluger]

@luacmartins Just confirming that this should be daily?

[luacmartins]

We can move it to weekly while we work on the first few issues to unblock this one.

[Kicu]

I will start working on this task instead of @SzymczakJ, please reassign us

[melvin-bot]

📣 @ikevin127 🎉 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

[luacmartins]

I believe this was deployed to production last week. @Kicu are you still working on anything else related to this issue?

[Kicu]

Nope I don't think so, I'm only working on #49123 now

[sakluger]

Summarizing payment on this issue:

Contributor: @Kicu - no payment required
Contributor+: @ikevin127 $250, paid via Upwork

[melvin-bot]

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

[ntdiary]

Hi, @luacmartins, coming from #51192.
It seems that this issue and issue #49116 allow us to open a report using Search > Recent Chats on many pages?
Currently, this feature has some impact on navigation. I'm curious if the doc offers any guidance or suggestions on this? :)
e.g.,

1. When clicking the back button in the report page, the LHN briefly appears (Or event when swiping the report page, we can't return to the Search page).
2. Open a report on the Profile page, then swipe twice, it returns to the LHN instead of the Settings page.

test2.mp4

[luacmartins]

@ntdiary I don't think we covered that in the doc and it's something we should address. @Kicu for your thoughts.

[Kicu]

Hey, so first of all we treat this SearchRouter as a modal, so it's not a full fledged page (for example on web you will not see a dedicated URL for it).
So in theory this should not have much impact on navigation per se. It just appears when you open it - on top of whatever page was there, and then disappears after your interaction with it ends.

So if you open SearchRouter on Page X, then using either the back button or swipe should bring you back to Page X - I think this is the most natural transition.
So both 1 and 2 from @ntdiary comment sound like bugs that should be fixed. I think 🤔

Side note: calling @adamgrzybowski and @WojtekBoman who are working on navigation refactor, so it's possible this would be fixed on their PR, because the internal logic of screens, linkTo and back buttons will be changed after they are done.

On solution would be to wait, but I cannot guarantee how long this will take.
Not sure if that helps you a lot? 😓

[ntdiary]

Hey, so first of all we treat this SearchRouter as a modal, so it's not a full fledged page (for example on web you will not see a dedicated URL for it). So in theory this should not have much impact on navigation per se. It just appears when you open it - on top of whatever page was there, and then disappears after your interaction with it ends.

So if you open SearchRouter on Page X, then using either the back button or swipe should bring you back to Page X - I think this is the most natural transition. So both 1 and 2 from @ntdiary comment sound like bugs that should be fixed. I think 🤔

@Kicu, yeah, the navigation for SearchRouter modal itself works as expected. I think it seems to allow us to open a Report page from some other pages(e.g., Search page, Settings/Profile page), and at the same time, due to the logic within linkTo

App/src/libs/Navigation/linkTo/index.ts

Lines 107 to 112 in 4c04320

	if (topmostBottomTabRoute && (topmostBottomTabRoute.name !== matchingBottomTabRoute.name || isNewPolicyID || isOpeningSearch)) {
	root.dispatch({
	type: CONST.NAVIGATION.ACTION_TYPE.PUSH,
	payload: matchingBottomTabRoute,
	});
	}

app will push a Home screen (i.e., the LHN) into the BottomTabNavigator. So when we swipe to close the Report page, we end up seeing the LHN instead of the expected Search page. 😂

Although we can continue to use the beforeRemoveReportOpenedFromSearchRHP listener to remove the LHN when closing the Report, I don’t think it can resolve the LHN flickering effectively, so it's not very ideal. My initial guess is that a better solution might require changing the above push logic, so it might be better to take care of this case during the navigation refactor. 😄

[Kicu]

@ntdiary sounds good to me.