[$500] Menu - Pin & Delete request appear on Receipt image when quickly tap on receipt and 3-Dot menu
If you havenβt already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!
Version Number: 1.4.16-3 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: Applause-Internal Team Slack conversation: @
Action Performed:
- Navigate to New dot app as Employee
- Go to workspace chat> create Scan request
- On detail page tap receipt and 3-Dot menu in a quick manner
Expected Result:
The receipt should be open and no 3-Dot menu should be open
Actual Result:
3-Dot menu (Pin & Delete request) from detail page appear on Receipt image when quickly tap on receipt and 3-Dot menu
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
- [x] Android: Native
- [ ] Android: mWeb Chrome
- [ ] iOS: Native
- [ ] iOS: mWeb Safari
- [x] MacOS: Chrome / Safari
- [ ] MacOS: Desktop
Screenshots/Videos
https://github.com/Expensify/App/assets/115492554/75e058b7-68dd-4ab6-94e7-bc978c6122ea
Upwork Automation - Do Not Edit
- Upwork Job URL: https://www.upwork.com/jobs/~01ea7a6ca457ab8814
- Upwork Job ID: 1738519644108128256
- Last Price Increase: 2024-01-06
- Automatic offers:
- c3024 | Contributor | 28106449
Job added to Upwork: https://www.upwork.com/jobs/~01ea7a6ca457ab8814
Triggered auto assignment to @Christinadobrzyn (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.
Bug0 Triage Checklist (Main S/O)
- [ ] This "bug" occurs on a supported platform (ensure
Platformsin 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
Triggered auto assignment to Contributor-plus team member for initial proposal review - @c3024 (External)
Proposal
Please re-state the problem that we are trying to solve in this issue.
Menu - Pin & Delete request appear on Receipt image when quickly tap on receipt and 3-Dot menu
What is the root cause of that problem?
The root cause is that the Receipt image view and 3-dot menu are modals. The modals always open on top of screen in react native and the last opened modal will be on top
What changes do you think we should make in order to solve the problem?
I think would be better if we use one component for displaying any modals in root of project . But in this case, we can fix the problem with the following steps.
- Add call
willAlertModalBecomeVisible(true)in this function - Add this call
willAlertModalBecomeVisible(false);in this function - Use getModalState function or wrap ThreeDotsMenu component in
withOnyxto access to modal state in component
export default withOnyx({
modal: {
key: ONYXKEYS.MODAL,
},
})(ThreeDotsMenu);
- Change this function like below
const showPopoverMenu = () => {
if (!modal.willAlertModalBecomeVisible) {
setPopupMenuVisible(true);
}
};
What alternative solutions did you explore? (Optional)
We can add another key in Onyx.modal like willAlertModalBecomeVisible and use it to avoid using the key which seems to be for modal alert only
Proposal
Please re-state the problem that we are trying to solve in this issue.
The three-dot menu shows on the receipt image attachment page.
What is the root cause of that problem?
This issue happens because of a delay. If the three dots menu shows after we are navigated to the receipt image attachment page, then we will see the menu on that page. We have faced a similar issue before here. Basically the same root cause.
What changes do you think we should make in order to solve the problem?
The solution also will be the same as https://github.com/Expensify/App/issues/29379, we just need to apply it to the three-dot menu.
First, close the menu when the screen blurs.
useFocusEffect(
useCallback(() => {
return () => hidePopoverMenu();
}, []),
);
Next, pass the focused state to the popover isVisible prop.
const isFocused = useIsFocused();
isVisible={isPopupMenuVisible && isFocused}
Optionally, prevent the press when the screen is not focused.
onPress={() => {
if (!isFocused) return;
....
})
(it's optional because so far, we already have a singleExecution apply to BaseGenericPressable which disables the pressable when an interaction is happening, including the navigation transition)
(this issue can happen on every usage of modal, to fix it globally, we can apply the fix to BaseModal, we will call onClose when the screen blurs)
Proposal
Please re-state the problem that we are trying to solve in this issue.
3-Dot menu (Pin & Delete request) from detail page appear on Receipt image when quickly tap on receipt and 3-Dot menu
What is the root cause of that problem?
If we click receipt image and 3 dot menu quickly, the receipt image will be opened and then 3 dot menu will be opened on top of it, and currently we don't have any logic to prevent 3 dot menu that are behind modal from being opened on top of the modal.
What changes do you think we should make in order to solve the problem?
We should use similar approach as this PR that fixes a similar issue. Basically if the three dots menu is behind a modal, its popover should not be visible.
- Connect to
ONYXKEYS.MODALhere
export default withOnyx({
modal: {
key: ONYXKEYS.MODAL,
},
})(ThreeDotsMenu);
- If the three dots menu is behind a modal, hide the popover if it's currently opened
(We need to check
!shouldOverlaybelow because ifshouldOverlayis true, that means the current three dots itself is inside a modal: AttachmentModal)
const isBehindModal = modal.willAlertModalBecomeVisible && !shouldOverlay;
useEffect(() => {
if (isBehindModal && isPopupMenuVisible) {
hidePopoverMenu();
}
}, [isBehindModal, isPopupMenuVisible]);
- Add
isBehindModalto the visible condition as well
isVisible={isPopupMenuVisible && !isBehindModal}
- We can optionally add
|| isBehindModalto thedisabledcondition here
This is basically the same approach already used in this PR for another component, just that in here we relies on the modal visibility rather than screen focus. This is a global fix that makes sure no such issue like this will occur again for the three dots menu, and rely on existing values/lifecycle of willAlertModalBecomeVisible rather than manipulating its lifecycle to workaround the race conditions as suggested in the above proposal, which unfortunately still doesn't work if pressing the receipt image and 3 dot very quickly.
What alternative solutions did you explore? (Optional)
The shouldOverlay name is a bit vague and can be considered renaming to isInModal for clarity.
We can consider relying on modal.isVisible (or both willAlertModalBecomeVisible and isVisible)
I find this extremely difficult to reproduce on my device and emulator. I am not able to quickly press the three dot menu after pressing on the receipt. Any tips to consistently reproduce this?
I find this extremely difficult to reproduce on my device and emulator. I am not able to quickly press the three dot menu after pressing on the receipt. Any tips to consistently reproduce this?
@c3024 I run on my physical Android device and press the receipt with 1 finger and immediately press the three dot using another finger and can reproduce it every time, hope that helps π
Proposal
Please re-state the problem that we are trying to solve in this issue.
Menu - Pin & Delete request appear on Receipt image when quickly tap on receipt and 3-Dot menu
What is the root cause of that problem?
When the user clicks on 3-Dot menu, the menu will be open while the menu is rendering if a user quickly clicks on an attachment the Modal for attachment is also triggered to open, and this issue always happens when device performance is slow. We can perform by using scripting or putting a device to low-performance mode
let menu = document.querySelectorAll('button[aria-label="More"]')[0];
let attachment = document.querySelectorAll('[aria-label="View attachment"]')[0];
menu?.click();
attachment?.click()
Reproduce
https://github.com/Expensify/App/assets/11959869/ee0483dd-75f4-4175-9d62-b3edcb6e1753
What changes do you think we should make in order to solve the problem?
Due to this issue can happen anywhere in the app so we need to handle it generally. We will create ThreeDotsMenuManager look like ReportActionComposeFocusManager has function isMenuOpen, openMenu, closeMenu
When showModal we will call ThreeDotsMenuManager.closeMenu() if we have any ThreeDotsMenu has opened state
const showModal = () => {
+ ThreeDotsMenuManager.closeMenu()
...
onModalShow?.();
};
What alternative solutions did you explore? (Optional)
Instead of create ThreeDotsMenuManager we can create PopoverManager
Reviewing
π£ It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? πΈ
Is there a similar case anywhere else where an undesired three dot menu modal appears over another modal so that a global fix is warranted?
@c3024 I think a global fix should be made around a new feature, not around bug. Because AttachmentModal is not associated with other modals and works separately .
looks like we're reviewing this, @c3024 let me know if you need any help from me!
Is there a similar case anywhere else where an undesired three dot menu modal appears over another modal so that a global fix is warranted?
@c3024 it happens on the normal chat report as well, when pressing three dots and sent attachment at the same time. I believe it happens basically everywhere an AttachmentModal and three dot menu appears together. So yeah I think we need a global fix
I have not been able to reproduce this anywhere else. Could you post a video of it?
π£ It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? πΈ
@Christinadobrzyn @c3024 this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!
I can't reproduce this either - @tienifr are you able to take a video of what you see?
I'm going ooo next week so going to assign a buddy to monitor this for me
status - we're trying to reproduce this (and probably update the OP with the correct steps)
Triggered auto assignment to @puneetlath (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.
Bug0 Triage Checklist (Main S/O)
- [ ] This "bug" occurs on a supported platform (ensure
Platformsin 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
I have not been able to reproduce this anywhere else. Could you post a video of it?
@c3024 @Christinadobrzyn here you go, same issue with normal Attachment in chat report, I can reproduce every time, make sure to follow this step
https://github.com/Expensify/App/assets/113963320/4b75afd4-f42c-4f73-8893-7413760763c5
Issue not reproducible during KI retests. (First week)
The bug is still reproducible
https://github.com/Expensify/App/assets/46921547/961a682e-8faa-4a15-97e7-2e641fcc5150
Thank you all for your proposals.
willAlertModalBecomeVisible is largely modified in the base components.
So, I think the approach of not showing the popover when the three dot menu is behind a modal is the better way of fixing this than changing the willAlertModalBecomeVisible values in the AttachmentModal. This approach also fixes all possible cases where the three dot menu can be behind any modal.
Proposal here by @tienifr looks good to me.
π π π C+ Reviewed
Current assignee @puneetlath is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.
@c3024 @tienifr Did you try opening only the 3-dot menu ? I faced this problem when I used @tienifr's approach
https://github.com/Expensify/App/assets/46921547/eb2fef43-05df-45e5-8fe9-2c05fa3a1f90
@puneetlath @Christinadobrzyn @c3024 this issue is now 3 weeks old. There is one more week left before this issue breaks WAQ and will need to go internal. What needs to happen to get a PR in review this week? Please create a thread in #expensify-open-source to discuss. Thanks!
Did you try opening only the 3-dot menu ? I faced this problem when I used @tienifr's approach
@shahinyan11 There'll be a isPopover field accompanying the willAlertModalBecomeVisible, so the isBehindModal won't be true if the modal that's "becoming visible" is the PopoverMenu itself.
I'll handle that during the PR.