App: [HOLD for payment 2022-11-29] [$250] Android/iOS - Chat and the FAB menu options open on the chat page when tapping both in quick succession.

Looking into the diff and share the update asap.

I thought, I already did. Forgot to leave a comment on Saturday. Will re-review it in 2 hours.

In normal use with a speedy app, I think it will be incredibly unlikely that you can quickly tap both of these with the same thumb before the first action is registered.

Nevertheless, I’ve been testing using two thumbs to quickly tap multiple pieces of UI on a few native apps (i.e WhatsApp, Twitter etc) and they all ignore the final action. I think we’re right to stick with this:

The first action should take priority

Settled up these contracts, so we’re all square here. 🎉

we probably should add some regression test for this though

I don’t think it’s worth adding one for an edge case that in practice is incredibly hard to reproduce. Unless the app is very, very slow, you don’t get the chance to tap both a chat row and the FAB in quick succession unless you force it using two thumbs or something.

Going to close this out, but let me know if you disagree.

I think this “issue” has been around forever, no specific bug - so I think we can check off the first 3 points - we probably should add some regression test for this though

Ah, I couldn’t find that one! Cool, so we’ll use:

• That job for the bonus ($250 for getting it merged in +3 days)
• This one for the fix ($500 for the issues combined).

Yep, we’ll combine the payment for the two. Here’s the job to accept the offer on (@parasharrajat as well).

I’m going to close the other issue out.

Cool, so the job is here and I’ve sent you the offer @aimane-chnaif, and @parasharrajat for C+.

Look forward to the PR! 🚀

Agree with @parasharrajat 👍 And yes, your spidey-senses were correct @trjExpensify 😉

Cool, so the “we” in this I presume means it’s approved by @Beamanator as well at this point yeah? Let me know, and I’ll get going on the job offer.

Yeah, give me like 2 hours. Not started working yet.

@parasharrajat will review updated proposal soon 👍

Proposal

Below code fixes both this GH and https://github.com/Expensify/App/issues/11708

https://github.com/Expensify/App/blob/main/src/pages/home/sidebar/SidebarScreen/BaseSidebarScreen.js

+ import withNavigationFocus from '../../../../components/withNavigationFocus';

+   componentDidUpdate(prevProps) {
+       if (((this.props.isFocused || !prevProps.isFocused) && (this.props.isDrawerOpen || !prevProps.isDrawerOpen)) || !this.state.isCreateMenuActive) {
+           return;
+       }
+       this.hideCreateMenu();
+   }

    showCreateMenu() {
+       if (!this.props.isFocused || (this.props.isSmallScreenWidth && !this.props.isDrawerOpen)) {
+           return;
+       }
        this.setState({
            isCreateMenuActive: true,
        });
        this.props.onShowCreateMenu();
    }

- export default withDrawerState(BaseSidebarScreen);
+ export default withDrawerState(withNavigationFocus(BaseSidebarScreen));

Code explanation:

1. call this.hideCreateMenu(); in componentDidUpdate !this.props.isFocused && prevProps.isFocused: This is the case when try to open other pages like Search, Profile. !this.props.isDrawerOpen && prevProps.isDrawerOpen: This is the case when try to open Report page. this.state.isCreateMenuActive condition is just to prevent redundant call when menu is already hidden. This fixes part of this GH so manually hide popup menu when user opens other pages quickly after clicking FAB icon. This also fixes https://github.com/Expensify/App/issues/11708 so manually hide popup menu when user opens other pages using shortcut key on web/desktop.

2. conditional showCreateMenu() call !this.props.isFocused: this is the case when click FAB icon quickly after clicking search icon (to open search page) or after clicking avatar icon (to open profile page). !this.props.isDrawerOpen: this is the case when click FAB icon quickly after clicking any chat raw on LHN (to open Report page). isDrawerOpen is always false in medium/large screens so this.props.isSmallScreenWidth condition should be added as well. This fixes part of this GH so prevent showing popup menu when user click FAB icon quickly after opening other pages on iOS/Android/mWeb.

Yeah, I agree with the first action taking priority.

Hmm good points all around. Honestly I don’t think this is a big problem for the user either way - if they click on a chat row & FAB at nearly the same time, they can’t expect one to work and not the other, right? They obviously accidentally clicked one and will be ok fixing their click (assuming it doesn’t go where they wanted)

In this case I call upon the @Expensify/design team for ideas!

Ah, yeah. The OP didn’t get updated. Fixed! To confirm, yes, we’re good with this approach:

the first trigger action should take priority which is the chat opening action in our case.

Job on Upwork here: https://www.upwork.com/jobs/~019fb94172f17c76bc

Proposal

1. For this we can simply navigate user back to HOME screen if he accidentally presses on the FAB menu.

Also tested both scenarios:

1. Press on FAB and then press on a chat.
2. Press on chat and then press on FAB.

Code:

Fixed Video:

https://user-images.githubusercontent.com/78416198/192174264-62510594-a28d-4699-8cd8-29038fffa2c4.mov