[CHECKLIST] [$250] Android & iOS - [Search v2.1] - Selected option is not highlighted

[IuliiaHerets]

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

Action Performed:

1. Launch New Expensify app.
2. Go to Search.
3. Tap Expenses dropdown button.

Expected Result:

The selected option will be highlighted.

Actual Result:

The selected option is not highlighted.

Workaround:

Unknown

Platforms:

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Bug6592419_1725416489226.ScreenRecording_09-04-2024_10-19-14_1.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0107ad6c6463841aef
• Upwork Job ID: 1831310673079177453
• Last Price Increase: 2024-09-04
• Automatic offers:
• ikevin127 | Reviewer | 103915892

Issue Owner

Current Issue Owner: @ikevin127

[melvin-bot]

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

[melvin-bot]

Triggered auto assignment to @garrettmknight (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]

👋 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.

[lakchote]

Not a blocker as it's a styling issue, rather than a functionality issue.

Putting the external label, as contributors can solve this.

[melvin-bot]

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

[melvin-bot]

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

[bernhardoj]

Edited by proposal-police: This proposal was edited at 2024-09-04 12:56:31 UTC.

Proposal

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

Selected search type option isn't highlighted.

What is the root cause of that problem?

We recently added the highlighted background in this commit which requires the item to be selected.

App/src/components/PopoverMenu.tsx

Line 274 in 2ef63e7

style={{backgroundColor: item.isSelected ? theme.activeComponentBG : undefined}}

However, we never pass the isSelected property.

App/src/pages/Search/SearchTypeMenuNarrow.tsx

Lines 35 to 48 in 2ef63e7

	const popoverMenuItems = typeMenuItems.map((item, index) => {
	const isSelected = title ? false : index === activeItemIndex;
	return {
	text: item.title,
	onSelected: singleExecution(() => Navigation.navigate(item.route)),
	icon: item.icon,
	iconFill: isSelected ? theme.iconSuccessFill : theme.icon,
	iconRight: Expensicons.Checkmark,
	shouldShowRightIcon: isSelected,
	success: isSelected,
	containerStyle: isSelected ? [{backgroundColor: theme.border}] : undefined,
	};
	});

The style passed will be combined with the combinedStyle array.

App/src/components/MenuItem.tsx

Line 442 in 2ef63e7

const combinedStyle = [styles.popoverMenuItem, style];

It will then be used as the pressable style.

App/src/components/MenuItem.tsx

Lines 577 to 580 in 2ef63e7

	style={({pressed}) =>
	[
	containerStyle,
	combinedStyle,

Notice that combinedStyle is put after containerStyle, so it'll override the containerStyle. In our case, we already set the highlighted background when the item is selected through containerStyle.

App/src/pages/Search/SearchTypeMenuNarrow.tsx

Line 46 in 2ef63e7

containerStyle: isSelected ? [{backgroundColor: theme.border}] : undefined,

But because we started to pass the highlighted background from style, the background color is overridden.

[{backgroundColor: theme.border}], // containerStyle
[{backgroundColor: undefined}], // combinedStyle

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

Pass isSelected to the object. (pass it here too)

return {
    ...,
    isSelected,

OR

Only apply the background-color style if isSelected is true.

App/src/components/PopoverMenu.tsx

Line 274 in 2ef63e7

style={{backgroundColor: item.isSelected ? theme.activeComponentBG : undefined}}

style={item.isSelected ? {backgroundColor: theme.activeComponentBG} : {}}

[ikevin127]

Thanks for the detailed RCA! @bernhardoj's updated proposal looks good to me. The RCA is correct and I think the first solution of passing isSelected to the objects makes more sense.

🎀👀🎀 C+ reviewed

[melvin-bot]

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

[melvin-bot]

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

[ikevin127]

@lakchote For reviewed proposal assignment.

[lakchote]

Thanks for your review @ikevin127.

@bernhardoj's proposal LGTM

[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

[bernhardoj]

PR is ready

cc: @ikevin127

[ikevin127]

⚠️ Automation failed here -> this should be on [HOLD for Payment 2024-09-23] according to 2 days ago's production deploy from #49058 (comment).

cc @garrettmknight

[garrettmknight]

Payment Summary:

• Contributor: @bernhardoj $250
• Reviewer: @ikevin127 $250 via upwork

@ikevin127 can you complete the checklist?

[bernhardoj]

Requested in ND.

[JmillsExpensify]

$250 approved for @bernhardoj

[ikevin127]

• [@ikevin127] The PR that introduced the bug has been identified. Link to the PR: Add CSV import flow to Categories page #47827.
• [@ikevin127] 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: https://github.com/Expensify/App/pull/47827/files#r1772422205.
• [@ikevin127] A discussion in #expensify-bugs 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.
• [@ikevin127] Determine if we should create a regression test for this bug.
• [@ikevin127] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.

Regression Test Proposal

1. Open the Search page.
2. Press the search type button.
3. Verify that the selected type is highlighted.

Native / mWeb Only (narrow layout)

4. Open the filter page and apply any filter.
5. Press the search type button again.
6. Verify that the filter text menu item is highlighted.

Do we agree 👍 or 👎.