[HOLD for payment 2022-12-20] [$1000] Profile : Upload/Delete Photo popup not hiding

[kavimuru]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Actions Performed:

1. Go to Settings -> Profile
2. Click Profile Photo
3. Select Upload Photo

Expected Result:

Should hide options after selecting one of them

Actual Result:

Not hiding even after selecting one of the options. Need to click outside the options to make it go away

Workaround:

unknown

Platform:

Where is this issue occurring?

• Web

Version Number: 1.2.37-0
Reproducible in staging?: y
Reproducible in production?: n
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos:

Screen.Recording.2022-12-07.at.10.30.59.PM.mov

Recording.3212.mp4

Expensify/Expensify Issue URL:
Issue reported by: @Santhosh-Sellavel / Applause internal team
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1670432916695299

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01a23347fca2f83eaf
• Upwork Job ID: 1600971997335502848

[melvin-bot]

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

[francoisl]

I can reproduce. @isabelastisser can you please apply the Engineering label to get it assigned. Doesn't work if I do it since I'm already on the engineering team.

I'll start investigating in the meantime.

[melvin-bot]

Triggered auto assignment to @marcaaron (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[francoisl]

Ok I just ran a git bisect, and it seems to come from this PR, which makes sense because it changed PopoverMenu quite a bit. cc @aimane-chnaif if you have any idea of the specific issue here.

[francoisl]

Ok so I managed to fix it by adding a this.setState({isMenuVisible: false}); here in each menu item's onSelected, but the fact that it broke here for this componenet makes me think it might also be broken in other places we use PopoverMenu, so I don't know if it's the best possible fix.

[marcaaron]

@Luke9389 want to assist with this one?

I don't have much context on the linked PR so would probably just revert it so it can go back to reviews.

@francoisl feels like the popover should be closing. I tested in a few other places and it closes. For some reason it doesn't close here. So, the workaround you mentioned might work, but doesn't explain the root cause.

[0xmiros]

I found exact root cause (not related to that PR though it is a regression). I will post here with solution shortly.

[0xmiros]

Root cause:

App/src/components/AttachmentPicker/index.js

Line 49 in cb256f9

this.fileInput.click();

This fires parent component's onPress event here:

App/src/components/AvatarWithImagePicker.js

Line 250 in cb256f9

onPress={() => this.setState({isMenuVisible: true})}

so isMenuVisible (which was set to false) set to true again

Solution:
https://github.com/Expensify/App/blob/main/src/components/AttachmentPicker/index.js

                <input
                    hidden
                    type="file"
                    ref={el => this.fileInput = el}
                    onChange={(e) => {
                        const file = e.target.files[0];

                        if (file) {
                            file.uri = URL.createObjectURL(file);
                            this.onPicked(file);
                        }

                        // Cleanup after selecting a file to start from a fresh state
                        this.fileInput.value = null;
                    }}
+                   onClick={e => e.stopPropagation()}
                    accept={getAcceptableFileTypes(this.props.type)}
                />

reference:
https://stackoverflow.com/questions/1369035/how-do-i-prevent-a-parents-onclick-event-from-firing-when-a-child-anchor-is-cli

Why this issue not exist before making that PR?

This was original code when Upload photo is clicked.
item.onSelected(); fires this.fileInput.click() and this fires onPress event of Pressable and this.setState({isMenuVisible: true}) is called.
And then props.onItemSelected(item); calls this.setState({isMenuVisible: false})
So isMenuVisible is set to true and immediately set to false and user feels popup hidden.

I checked all usages of PopoverMenu and the issue exists only here.

[marcaaron]

@0xmiroslav 👏 excellent!

So the parent onPress event of a Pressable can be called when the child click happens. I guess even if it is programmatically triggered that event can bubble and should be stopped. On the hidden input this makes perfect sense to me as nothing needs to know about that except the input.

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

📣 @0xmiroslav You have been assigned to this job by @marcaaron!
Please apply to this job in Upwork 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 📖

[marcaaron]

@thesahindia removing you here as we already have a workable solution

[0xmiros]

Thanks for assigning me @marcaaron

[marcaaron]

@allroundexperts I minimized your comment because it's not really germane to this issue. If you have any specific concerns about our process in general feel free to bring those up in Slack, thanks!

[marcaaron]

@s77rt take it to Slack please. It is not relevant to this issue.

[chiragsalian]

Not a deploy blocker since the fix was CP-d so I'm removing deployBlockerCash label. I also tested on staging and it looks like the issue is resolved.

[melvin-bot]

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

• fix "Upload photo" popup menu not hiding #13448

If no regressions arise, payment will be issued on 2022-12-20. 🎊

After the hold period, please check if any of the following need payment for this issue, and if so check them off after paying:

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

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

• Merged PR within 3 business days of assignment - 50% bonus
• Merged PR more than 9 business days after assignment - 50% penalty

[melvin-bot]

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:

• [@marcaaron] The PR that introduced the bug has been identified. Link to the PR: fix "Closing split bill doesn't refocus on composer" on web/desktop #12031
• [@marcaaron] 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: fix "Closing split bill doesn't refocus on composer" on web/desktop #12031 (comment)
• [@marcaaron] 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: https://expensify.slack.com/archives/C01GTK53T8Q/p1670971237240229
• [@isabelastisser] A regression test has been added or updated so that the same bug will not reach production again. Link to the GH issue for creating the test here: Add a step to Profile - Edit Information in Profile test case here.

[isabelastisser]

@0xmiroslav, I sent you an invitation for the job in Upwork, please apply and I will hire you and issue the payment ASAP. Thanks!

https://www.upwork.com/jobs/~01a23347fca2f83eaf

[isabelastisser]

• @0xmiroslav was paid in Upwork.

• The contract has ended.

• The job is closed.

[Santhosh-Sellavel]

@isabelastisser Reporting bonus not issued!

[isabelastisser]

@Santhosh-Sellavel sorry about that! I invited you to the job in Upwork now, please accept it and I will extend the contract and issue the payment. Thanks!

[Santhosh-Sellavel]

Applied @isabelastisser

[isabelastisser]

@Santhosh-Sellavel paid. All set!