[$250] [Search v2.3] Popup colors are incorrect

[lakchote]

See #48566 (comment)

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021836682888839715103
• Upwork Job ID: 1836682888839715103
• Last Price Increase: 2024-09-19
• Automatic offers:
• ishpaul777 | Contributor | 104052106
• jaydamani | Contributor | 104052748

Issue Owner

Current Issue Owner: @mollfpr

[melvin-bot]

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

[melvin-bot]

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

[cretadn22]

@lakchote Could you provide the correct design?

[jaydamani]

Proposal

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

Incorrect color in three dot menu

What is the root cause of that problem?

The popup comes from ThreeDotMenu which renders popOverMenu. PopOverMenu does not pass isPaneMenu prop for MenuItem so the default icon color is theme.icon (green) and color on focus is theme.iconMenu (black). The colors are decided based on below logic (active means hovered).

https://github.com/Expensify/App/blob/main/src/styles/utils/index.ts#L1442-L1460

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

The colors come from common components so we can do one of the following:

Option 1

Only update the menu for saved search.

To do this, add isPaneMenu as prop to ThreeDotMenu and popOverMenu to pass it down to MenuItem

Option 2

Update all pop-up menus in the app to be consitent.

Add isPaneMenu prop here or remove the logic from here by updating to below. We can also remove isPaneMenu prop after this.

    switch (buttonState) {
        case CONST.BUTTON_STATES.ACTIVE:
        case CONST.BUTTON_STATES.PRESSED:
            return theme.iconHovered;
        case CONST.BUTTON_STATES.COMPLETE:
            return theme.iconSuccessFill;
        case CONST.BUTTON_STATES.DEFAULT:
        case CONST.BUTTON_STATES.DISABLED:
        default:
            if (isMenuIcon) {
                return theme.iconMenu;
            }
            return theme.icon;
    }

[NaveedShaikh78]

Proposal

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

Popup colors are incorrect

What is the root cause of that problem?

The deault value for menu item is wrong and but if we change the value it is not reflecting due to the changes
done revision

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

MenuItem.tsx mak isPaneMenu flag to true

What alternative solutions did you explore? (Optional)

Reverting the changes revision. can fix the problem

[melvin-bot]

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

[lakchote]

Assigning @ishpaul777 since he's available to review now.

[ishpaul777]

Thanks, looking at proposals 👀

[ishpaul777]

@lakchote is this the correct colors ?

Screen.Recording.2024-09-20.at.2.31.15.PM.mov

asking coz all other menu in app is of same color as now do we need to make all consistent with this colors

Screen.Recording.2024-09-20.at.2.33.55.PM.mov

[ishpaul777]

if we want to make this change to all popover menu items in app, we can go with option 2 from @jaydamani's Proposal if only specific to saved Search popover menu then option 1. In any case @jaydamani Proposal LGTM!

🎀 👀 🎀 C+ reviewed

[melvin-bot]

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

[lakchote]

Let's go with option 1 @jaydamani as we don't want to introduce unwanted side effects in the app, as isPaneMenu must have been there for a reason.

[melvin-bot]

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

[jaydamani]

I received the offer. I will work on this over the weekend and raise PR by monday. Let me know if this needs to be done today.

[lakchote]

I received the offer. I will work on this over the weekend and raise PR by monday. Let me know if this needs to be done today.

@jaydamani if you could work on this today and raise a PR, that'd be great!

[jaydamani]

Created PR here

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