[HOLD for payment 2023-12-28] [$500] Expense report header is missing the three dots icon #32414

m-natarajan · 2023-12-03T15:33:35Z

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: 1.4.7
Reproducible in staging?: y
Reproducible in production?: y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: @shawnborton
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1701391916879549

Action Performed:

Go to an expense report, look at the top header area

Expected Result:

The header area should have a three dots (ellipsis overflow) menu in the top right. In this menu, there should at least be a pin and call option)

Actual Result:

There is no overflow ellipsis menu here

Workaround:

unknown

Platforms:

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

Screenshots/Videos

Add any screenshot/video evidence

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~01896b1282e560a2b7
Upwork Job ID: 1731335924328407040
Last Price Increase: 2023-12-10
Automatic offers:

aimane-chnaif | Reviewer | 28037783
mkhutornyi | Contributor | 28037784

melvin-bot · 2023-12-03T15:33:40Z

Triggered auto assignment to @kevinksullivan (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

melvin-bot · 2023-12-03T15:33:42Z

Bug0 Triage Checklist (Main S/O)

This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
This bug is not a duplicate report (check E/App issues and #expensify-bugs)
- If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
This bug is reproducible using the reproduction steps in the OP. S/O
- If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
- If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
This issue is filled out as thoroughly and clearly as possible
- Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

melvin-bot · 2023-12-03T15:33:47Z

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

melvin-bot · 2023-12-03T15:33:52Z

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

mkhutornyi · 2023-12-03T15:54:11Z

Proposal

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

Three dots menu is missing in expense request header

What is the root cause of that problem?

New feature

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

In MoneyReportHeader, pass threeDotsMenuItems, shouldShowThreeDotsButton, threeDotsAnchorPosition props to HeaderWithBackButton

                threeDotsMenuItems={threeDotMenuItems}
                shouldShowThreeDotsButton
                threeDotsAnchorPosition={styles.threeDotsPopoverOffsetNoCloseButton(windowWidth)}

We can discuss what options to put but for now, Zoom and Google Meet is added

Edit: (for pin / unpin option)

App/src/pages/home/report/ContextMenu/ContextMenuActions.js

Lines 420 to 421 in 3827a9c

    
           shouldShow: (type, reportAction, isArchivedRoom, betas, anchor, isChronosReport, reportID, isPinnedChat) => 
        
               type === CONTEXT_MENU_TYPES.REPORT && !isPinnedChat && !ReportUtils.isMoneyRequestReport(reportID),

App/src/pages/home/report/ContextMenu/ContextMenuActions.js

Lines 434 to 435 in 3827a9c

    
           shouldShow: (type, reportAction, isArchivedRoom, betas, anchor, isChronosReport, reportID, isPinnedChat) => 
        
               type === CONTEXT_MENU_TYPES.REPORT && isPinnedChat && !ReportUtils.isMoneyRequestReport(reportID),

Here, we should remove && !ReportUtils.isMoneyRequestReport(reportID) condition

And also add to threeDotMenuItems array. This logic can be copied from

App/src/pages/home/HeaderView.js

Lines 155 to 180 in 3827a9c

    
           threeDotMenuItems.push(HeaderUtils.getPinMenuItem(props.report)); 
        
           if (isConcierge && props.guideCalendarLink) { 
        
               threeDotMenuItems.push({ 
        
                   icon: Expensicons.Phone, 
        
                   text: translate('videoChatButtonAndMenu.tooltip'), 
        
                   onSelected: Session.checkIfActionIsAllowed(() => { 
        
                       Link.openExternalLink(props.guideCalendarLink); 
        
                   }), 
        
               }); 
        
           } else if (!isAutomatedExpensifyAccount && !isTaskReport && !isArchivedRoom) { 
        
               threeDotMenuItems.push({ 
        
                   icon: ZoomIcon, 
        
                   text: translate('videoChatButtonAndMenu.zoom'), 
        
                   onSelected: Session.checkIfActionIsAllowed(() => { 
        
                       Link.openExternalLink(CONST.NEW_ZOOM_MEETING_URL); 
        
                   }), 
        
               }); 
        
               threeDotMenuItems.push({ 
        
                   icon: GoogleMeetIcon, 
        
                   text: translate('videoChatButtonAndMenu.googleMeet'), 
        
                   onSelected: Session.checkIfActionIsAllowed(() => { 
        
                       Link.openExternalLink(CONST.NEW_GOOGLE_MEET_MEETING_URL); 
        
                   }), 
        
               }); 
        
           }

To hide pin when RBR shows, add && !hasBrickError to this condition:

App/src/components/LHNOptionsList/OptionRowLHN.js

Line 303 in 3827a9c

{!shouldShowGreenDotIndicator && optionItem.isPinned && (

yh-0218 · 2023-12-03T15:59:11Z

Proposal

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

Three dots menu is missing in expense request header

What is the root cause of that problem?

We didn't handle Three dots menu on MoneyReportHeader

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

App/src/components/MoneyReportHeader.js

Line 101 in 48b5a29

<HeaderWithBackButton

we need to add

     shouldShowThreeDotsButton
     threeDotsMenuItems={threeDotsMenuItems}
     threeDotsAnchorPosition={styles.threeDotsPopoverOffsetNoCloseButton(windowWidth)}

We can add more menu items

    const threeDotsMenuItems = [HeaderUtils.getPinMenuItem(moneyRequestReport)];
    if (!ReportUtils.isArchivedRoom(chatReport)) {
        threeDotsMenuItems.push({
            icon: ZoomIcon,
            text: translate('videoChatButtonAndMenu.zoom'),
            onSelected: Session.checkIfActionIsAllowed(() => {
                Link.openExternalLink(CONST.NEW_ZOOM_MEETING_URL);
            }),
        });
        threeDotsMenuItems.push({
            icon: GoogleMeetIcon,
            text: translate('videoChatButtonAndMenu.googleMeet'),
            onSelected: Session.checkIfActionIsAllowed(() => {
                Link.openExternalLink(CONST.NEW_GOOGLE_MEET_MEETING_URL);
            }),
        });
    }

What alternative solutions did you explore? (Optional)

DylanDylann · 2023-12-03T18:57:57Z

Proposal

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

Expense report header is missing the three dots icon

What is the root cause of that problem?

We don't have logic to display three-dot button on the IOU/Expense Report

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

In MoneyReportHeader we should add 3 props: shouldShowThreeDotsButton, threeDotsMenuItems, threeDotsAnchorPosition

App/src/components/MoneyReportHeader.js

Line 101 in cc4add5

<HeaderWithBackButton

In threeDotsMenuItems, we add pin/unpin functionality as we did here

App/src/components/MoneyRequestHeader.js

Line 97 in cc4add5

const threeDotsMenuItems = [HeaderUtils.getPinMenuItem(report)];

Note that, we also add pin/unpin functionality in the context menu (when right click to IOU/Expense report in LHN) for consistency by removing !ReportUtils.isMoneyRequestReport in both places

App/src/pages/home/report/ContextMenu/ContextMenuActions.js

Line 435 in cc4add5

    
           type === CONTEXT_MENU_TYPES.REPORT && isPinnedChat && !ReportUtils.isMoneyRequestReport(reportID),

App/src/pages/home/report/ContextMenu/ContextMenuActions.js

Line 421 in cc4add5

    
           type === CONTEXT_MENU_TYPES.REPORT && !isPinnedChat && !ReportUtils.isMoneyRequestReport(reportID),

In three dot, we also consider adding Zoom and Google meet options as we did here

App/src/pages/home/HeaderView.js

Lines 165 to 179 in cc4add5

    
           } else if (!isAutomatedExpensifyAccount && !isTaskReport && !isArchivedRoom) { 
        
               threeDotMenuItems.push({ 
        
                   icon: ZoomIcon, 
        
                   text: translate('videoChatButtonAndMenu.zoom'), 
        
                   onSelected: Session.checkIfActionIsAllowed(() => { 
        
                       Link.openExternalLink(CONST.NEW_ZOOM_MEETING_URL); 
        
                   }), 
        
               }); 
        
               threeDotMenuItems.push({ 
        
                   icon: GoogleMeetIcon, 
        
                   text: translate('videoChatButtonAndMenu.googleMeet'), 
        
                   onSelected: Session.checkIfActionIsAllowed(() => { 
        
                       Link.openExternalLink(CONST.NEW_GOOGLE_MEET_MEETING_URL); 
        
                   }), 
        
               });

In LHN Row we don't want to display 3 icons so that in here

App/src/components/LHNOptionsList/OptionRowLHN.js

Lines 303 to 310 in 3827a9c

    
           {!shouldShowGreenDotIndicator && optionItem.isPinned && ( 
        
               <View 
        
                   style={styles.ml2} 
        
                   accessibilityLabel={translate('sidebarScreen.chatPinned')} 
        
               > 
        
                   <Icon src={Expensicons.Pin} /> 
        
               </View> 
        
           )}

We also should hide pin icon if RBR appears by updating condition like this

                            {!shouldShowGreenDotIndicator && !hasBrickError && optionItem.isPinned && (

What alternative solutions did you explore? (Optional)

kurakurasakti · 2023-12-04T11:06:58Z

Proposal

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

Three dots menu on header is missing when open expense request.

What is the root cause of that problem?

missing 3 dots handler on MoneyReportHeader so when the component load, header doesn't read the condition

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

Send props to MoneyReportHeader and also the condition to show the 3 dots.

What alternative solutions did you explore? (Optional)

melvin-bot · 2023-12-04T11:07:01Z

📣 @kurakurasakti! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

Make sure you've read and understood the contributing guidelines.
Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.

Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

kurakurasakti · 2023-12-04T11:26:38Z

Proposal

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

the 3 dots menu is missing in expense request header

What is the root cause of that problem?

You miss the handler of 3 dots. After the component loads, the header does not render the 3 dots.

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

you need to add handler to pass the In MoneyReportHeader components, pass threeDotsMenuItems, shouldShowThreeDotsButton, threeDotsAnchorPosition props

      // Shows border if no buttons or next steps are showing below the header
                shouldShowBorderBottom={!(shouldShowAnyButton && isSmallScreenWidth) && !(shouldShowNextSteps && !isSmallScreenWidth)}
                threeDotsMenuItems
                shouldShowThreeDotsButton

What alternative solutions did you explore? (Optional)

aimane-chnaif · 2023-12-06T13:42:03Z

reviewing proposals

aimane-chnaif · 2023-12-07T00:25:47Z

After internal discussion, here's the expected behavior

Options to show:

Pin / Unpin
Zoom
Google Meet

We don't like having three icons in a row, so when show GBR/RBR on pinned report, pin should be hidden

mkhutornyi · 2023-12-07T00:37:54Z

Proposal

Updated

DylanDylann · 2023-12-07T01:59:18Z

Updated proposal to cover this case:

when showing GBR/RBR on the pinned report, the pin should be hidden.

Just a note: My proposal is the first one that suggest do display pin option in three dots and Context menu of LHN Row

mkhutornyi · 2023-12-07T02:02:49Z

It's easy & straightforward fix to add pin option in context menu.
I did not mention it originally because it was disabled intentionally in #20771 but now reverted back.
I clearly said: We can discuss what options to put but for now, Zoom and Google Meet is added

yh-0218 · 2023-12-07T04:02:06Z

I did mention all from first.

melvin-bot · 2023-12-10T16:00:04Z

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

aimane-chnaif · 2023-12-11T19:21:56Z

Thanks for the proposals everyone.
The issue is straightforward and I think we should go with @mkhutornyi who proposed first.
I understand there were some debates about pin option but I also thought pin should be disabled as this was bug originally, which led me to raise discussion on slack. So IMO it should not affect proposal selection.
Giving the final call to assigned engineer.
🎀 👀 🎀 C+ reviewed

melvin-bot · 2023-12-21T05:12:10Z

Reviewing label has been removed, please complete the "BugZero Checklist".

melvin-bot · 2023-12-21T05:12:14Z

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.14-6 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

show 3 dots menu in expense report header #32961

If no regressions arise, payment will be issued on 2023-12-28. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

External issue reporter
Contributor that fixed the issue
Contributor+ that helped on the issue and/or PR

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

@aimane-chnaif requires payment automatic offer (Reviewer)
@mkhutornyi requires payment automatic offer (Contributor)

melvin-bot · 2023-12-21T05:12:15Z

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

[@aimane-chnaif] The PR that introduced the bug has been identified. Link to the PR:
[@aimane-chnaif] 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:
[@aimane-chnaif] 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:
[@aimane-chnaif] Determine if we should create a regression test for this bug.
[@aimane-chnaif] 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.
[@kevinksullivan] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

kevinksullivan · 2023-12-22T17:57:54Z

Hi @aimane-chnaif please complete the checklist above when you can!

iwiznia · 2023-12-29T18:40:26Z

Bump @aimane-chnaif

aimane-chnaif · 2023-12-29T18:46:18Z

This is not regression but design change.

We should update TestRail.
https://github.com/Expensify/Expensify/issues/298942
@iwiznia can you please help with it as I don't have access?

iwiznia · 2023-12-29T19:13:46Z

Ah ok, I can open the issue for you, but can you write down the contents please?

aimane-chnaif · 2023-12-29T19:24:12Z

Regression Test Proposal

Go to any IOU or expense report
Verify that header has 3 dots menu icon in the top right
Click on the icon
Verify that Pin / Unpin, Zoom and Google Meet options show and they work properly
Right click this report in LHN
Verify that Pin / Unpin option shows and it works properly. If it shows GBR/RBR, verify that pin icon is hidden

iwiznia · 2023-12-29T19:33:51Z

Nice, is that a new test? I thought we needed to update one and if so, do you happen to know which?

aimane-chnaif · 2023-12-29T19:45:13Z

I thought we needed to update one

I am not sure. https://github.com/Expensify/Expensify/issues/298942 doesn't have TestRail link?

aimane-chnaif · 2023-12-29T19:45:54Z

Let's treat as new test case. I believe QA team will update TestRail properly

iwiznia · 2023-12-29T19:47:37Z

Ohh there's a link here did not see it 👍

iwiznia · 2023-12-29T19:49:20Z

Created it here https://github.com/Expensify/Expensify/issues/355542

melvin-bot · 2024-01-02T17:25:12Z

@iwiznia, @kevinksullivan, @aimane-chnaif, @mkhutornyi Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

melvin-bot · 2024-01-04T17:10:53Z

@iwiznia, @kevinksullivan, @aimane-chnaif, @mkhutornyi Huh... This is 4 days overdue. Who can take care of this?

iwiznia · 2024-01-05T21:25:20Z

@kevinksullivan ?

kevinksullivan · 2024-01-08T20:40:22Z

payments made

m-natarajan added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Dec 3, 2023

melvin-bot bot assigned kevinksullivan Dec 3, 2023

m-natarajan added the External Added to denote the issue can be worked on by a contributor label Dec 3, 2023

melvin-bot bot changed the title ~~Expense report header is missing the three dots icon~~ [$500] Expense report header is missing the three dots icon Dec 3, 2023

melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Dec 3, 2023

melvin-bot bot assigned aimane-chnaif Dec 3, 2023

melvin-bot bot added the Overdue label Dec 6, 2023

melvin-bot bot removed the Overdue label Dec 6, 2023

melvin-bot bot added Overdue and removed Overdue labels Dec 10, 2023

melvin-bot bot assigned iwiznia Dec 11, 2023

melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production labels Dec 21, 2023

melvin-bot bot changed the title ~~[$500] Expense report header is missing the three dots icon~~ [HOLD for payment 2023-12-28] [$500] Expense report header is missing the three dots icon Dec 21, 2023

melvin-bot bot removed the Reviewing Has a PR in review label Dec 21, 2023

melvin-bot bot added Daily KSv2 Overdue and removed Weekly KSv2 labels Dec 27, 2023

melvin-bot bot removed the Overdue label Dec 29, 2023

melvin-bot bot added the Overdue label Jan 1, 2024

melvin-bot bot added Overdue and removed Overdue labels Jan 5, 2024

kevinksullivan closed this as completed Jan 8, 2024

melvin-bot bot removed the Overdue label Jan 8, 2024

[HOLD for payment 2023-12-28] [$500] Expense report header is missing the three dots icon #32414

[HOLD for payment 2023-12-28] [$500] Expense report header is missing the three dots icon #32414

Comments

m-natarajan commented Dec 3, 2023 • edited by melvin-bot bot Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

melvin-bot bot commented Dec 3, 2023

melvin-bot bot commented Dec 3, 2023

Bug0 Triage Checklist (Main S/O)

melvin-bot bot commented Dec 3, 2023

melvin-bot bot commented Dec 3, 2023

mkhutornyi commented Dec 3, 2023 • edited Loading

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?

yh-0218 commented Dec 3, 2023

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)

DylanDylann commented Dec 3, 2023 • edited Loading

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)

kurakurasakti commented Dec 4, 2023

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)

melvin-bot bot commented Dec 4, 2023

kurakurasakti commented Dec 4, 2023 • edited Loading

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)

aimane-chnaif commented Dec 6, 2023

aimane-chnaif commented Dec 7, 2023

mkhutornyi commented Dec 7, 2023

Proposal

DylanDylann commented Dec 7, 2023 • edited Loading

mkhutornyi commented Dec 7, 2023

yh-0218 commented Dec 7, 2023

melvin-bot bot commented Dec 10, 2023

aimane-chnaif commented Dec 11, 2023

melvin-bot bot commented Dec 21, 2023

melvin-bot bot commented Dec 21, 2023

melvin-bot bot commented Dec 21, 2023

kevinksullivan commented Dec 22, 2023

iwiznia commented Dec 29, 2023

aimane-chnaif commented Dec 29, 2023

iwiznia commented Dec 29, 2023

aimane-chnaif commented Dec 29, 2023

Regression Test Proposal

iwiznia commented Dec 29, 2023

aimane-chnaif commented Dec 29, 2023

aimane-chnaif commented Dec 29, 2023

iwiznia commented Dec 29, 2023

iwiznia commented Dec 29, 2023

melvin-bot bot commented Jan 2, 2024

melvin-bot bot commented Jan 4, 2024

iwiznia commented Jan 5, 2024

kevinksullivan commented Jan 8, 2024 • edited Loading

m-natarajan commented Dec 3, 2023 •

edited by melvin-bot bot

Loading

mkhutornyi commented Dec 3, 2023 •

edited

Loading

DylanDylann commented Dec 3, 2023 •

edited

Loading

kurakurasakti commented Dec 4, 2023 •

edited

Loading

DylanDylann commented Dec 7, 2023 •

edited

Loading

kevinksullivan commented Jan 8, 2024 •

edited

Loading