[PAYMENT DUE] [$500] mWeb - Device back button with emoji picker opened leads to LHN instead of closing picker

[lanitochka17]

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.36-0
Reproducible in staging?: Y
**Reproducible in production?:**Y
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/4279504
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:

1. Log in to New Expensify
2. Navigate to any chat
3. Open emoji picker
4. Use device back button (twice if keyboard is opened)

Expected Result:

Emoji picker should be closed, user should remain in chat history

Actual Result:

User is redirected back to LHN

Workaround:

Unknown

Platforms:

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

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6365936_1706965847633.Record_2024-02-02-20-31-24.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0156cc7a92d86433bd
• Upwork Job ID: 1753804975067238400
• Last Price Increase: 2024-02-24

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[lanitochka17]

We think that this bug might be related to #vip-vsp
CC @quinthar

[Krishna2323]

Proposal

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

mWeb - Device back button with emoji picker opened leads to LHN instead of closing picker

What is the root cause of that problem?

We are call Navigation.goBack when back button is pressed.

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

Before navigating check if the emoji picker is visible or not, if visible close it and do not navigate. We can get emojiPicker status from EmojiPickerAction and call hideEmojiPicker.

Result

[suneox]

Proposal

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

mWeb - Device back button with emoji picker opened leads to LHN instead of closing picker

What is the root cause of that problem?

When emoji modal open we don't update window history state after that user clicks on "The browser back button" or "Device Back Button" it will navigate back to the previous page (base on browser history)

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

We will handle update the history state for Modal Web and to make sure this change only affects some the modals we can add new props shouldHandleNavigationBack for Modal

```diff
+function Modal({fullscreen = true, onModalHide = () => {}, type, onModalShow = () => {}, children, shouldHandleNavigationBack, ...rest}: BaseModalProps) {
    const theme = useTheme();
    const StyleUtils = useStyleUtils();
    const [previousStatusBarColor, setPreviousStatusBarColor] = useState<string>();

    const setStatusBarColor = (color = theme.appBG) => {
        if (!fullscreen) {
            return;
        }

        StatusBar.setBackgroundColor(color);
    };

    const hideModal = () => {
        setStatusBarColor(previousStatusBarColor);
        onModalHide();
+       if (window.history.state.shouldGoBack) {
+           window.history.back();
+       }
    };

+   const handlePopStateRef = useRef(() => {
+       rest.onClose();
+   });

    const showModal = () => {
        const statusBarColor = StatusBar.getBackgroundColor() ?? theme.appBG;

        ....

+       if (shouldHandleNavigationBack) {
+           window.history.pushState({shouldGoBack: true}, '', null);
+           window.addEventListener('popstate', handlePopStateRef.current);
+       }
        onModalShow?.();
    };

+   useEffect(
+       () => () => {
+           window.removeEventListener('popstate', handlePopStateRef.current);
+       },
+       [],
+   );

    return (
        <BaseModal
          ...

At EmojiPicker pass props shouldHandleNavigationBack to Popover with condition is Browser.isMobile()

POC

Screen.Recording.2024-02-12.at.23.29.35.mov

testing branch

What alternative solutions did you explore? (Optional)

[jeremy-croff]

Well opening up an emoji picker is not a navigation action.

Naturally clicking the back button would navigate back to the previous screen.

This is working as intended.

The only thing I see is that the background has a disabled state. We could improve the user experience if desired to disable all clicks and only close the emoji picker when clicking outside of it. But this is not what the Issue is asking for.

[hayata-suenaga]

This issue should be handled in this issue

Thank you everyone for your proposals, but we're closing this issue 🙇

[adamgrzybowski]

Hey @hayata-suenaga I agree with @jeremy-croff. It's not related to the goBack function and opening the emoji picker is not a navigation action.

If you want to implement this anyway, then this ticket should be separated form the goBack issue.

Not sure how the presented proposal will work on native platforms and how it interferes with web history though.

Personally I like the improvement mentioned by @jeremy-croff

The only thing I see is that the background has a disabled state. We could improve the user experience if desired to disable all clicks and only close the emoji picker when clicking outside of it. But this is not what the Issue is asking for.

[melvin-bot]

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

[hayata-suenaga]

re-posted about this issue in the #expensify-open-source channel

@sobitneupane please review the proposals when you have time 🙇

[suneox]

My proposal can work the same native behavior when pressing the back button on Android device (the bottom sheet will be closed)

Native

Screen.Recording.2024-02-13.at.00.02.24.mov

POC

Screen.Recording.2024-02-12.at.23.29.35.mov

[melvin-bot]

@sobitneupane, @zanyrenney Huh... This is 4 days overdue. Who can take care of this?

[adamgrzybowski]

hey @suneox do you have a draft branch? I would like to test a few things

[suneox]

hey @suneox do you have a draft branch? I would like to test a few things

@sobitneupane @adamgrzybowski You can try on this branch

[zanyrenney]

@sobitneupane it seems like Adam has reviewed the proposals above. Can you formally respond as to who we should be selecting here? If you do not have capacity as C+, please let me know and I will reassign the issue.

[kosmydel]

@adamgrzybowski is OOO and I will try to help.

@suneox I have quickly checked it, and for now, I have found one bigger issue in this PR:

1. Go to the Profile (must have avatar uploaded).
2. Press the Edit button on the Avatar.
3. Press "View photo"
4. It doesn't work.

I'm leaving also some notes:

1. What would happen in nested modals? I think it might break the back button behavior.
2. addEventListener should probably be in the useEffect as well.
3. We shouldn't add event listeners on non mobile devices.

Are you also 100% sure, it won't break the @react-navigation history as those changes are not synced with this library?

[sobitneupane]

Sure @zanyrenney. I will review the proposal asap.

[sobitneupane]

Thanks for the proposal @suneox

Proposal from @suneox looks good to me.

🎀 👀 🎀 C+ reviewed

[melvin-bot]

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

[suneox]

The PR has been deployed to production

[suneox]

Hi @zanyrenney how about payment for this issue has been deployed to production but the bot not updated status

[suneox]

Hi @zanyrenney this issue has been deployed to production 2 weeks, Can I get an offer for this issue?

[sobitneupane]

@zanyrenney This is ready for payment.

[Julesssss]

Yah, Zany has been OOO but will be back soon 🙂

[suneox]

Friendly bump @zanyrenney

[suneox]

Hi @sobitneupane how can I get payment for this issue? It has been deployed a month. Thank you!

[Julesssss]

Hey @zanyrenney, just wanted to check you've seen this payment request?

[zanyrenney]

on it, didn't see this with the lack of date automation.

[zanyrenney]

payment summary

@sobitneupane owed $500 - needs to request through NewDot (manual requests)
@suneox needs $500 from upwork.

[zanyrenney]

@suneox I can't find you on upwork.

Please can you manually apply to the job? https://www.upwork.com/jobs/~01386bcc6b2c6f0fdf

Feel free to DM me in slack when you have and i'll process the payment ASAP.

[suneox]

@suneox I can't find you on upwork.

Here is my profile

[zanyrenney]

Invited, please accept !

[suneox]

Invited, please accept !

I have accept this job, Thank you

[zanyrenney]

payment summary

@sobitneupane owed $500 - needs to request through NewDot (manual requests)
@suneox PAID $500 from upwork.

[melvin-bot]

@shawnborton @Julesssss @zanyrenney Be sure to fill out the Contact List!

[JmillsExpensify]

$500 approved for @sobitneupane