[$500] Different behavior for mobile and mWeb when user opens an attachment and tries to go back - Reported by @thesahindia

[mvtglobally]

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

---

Action Performed:

1. Open app on mobile app
2. Send a message with an attachment
3. Click on the attachment to preview
4. Hit back button
5. Repeat steps on the same device in mWeb

Expected Result:

Same behavior should be across platforms

Actual Result:

Different behavior for mobile and mobile web when the user opens an attachment and tries to go back using the native back button.
Mobile :- The user go back to the chat.
Mobile web :- The chat closes itself.

Workaround:

Unknown

Platform:

Where is this issue occurring?

• iOS
• Android
• Mobile Web

Version Number: 1.1.24-0
Reproducible in staging?: Y
Reproducible in production?: Y
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation

Screen.Recording.2021-12-25.at.1.35.57.AM.mov

Screen.Recording.2021-12-25.at.1.35.22.AM.mov

Expensify/Expensify Issue URL:
Issue reported by: @thesahindia
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1640376771111300

View all open jobs on GitHub

[MelvinBot]

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

[stitesExpensify]

Yep definitely a bug. @thesahindia do you want to propose a solution?

[MelvinBot]

Triggered auto assignment to @NicMendonca (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

[NicMendonca]

Upwork job post: https://www.upwork.com/jobs/~0167b446c2be6b25f4

[MelvinBot]

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

[MelvinBot]

Triggered auto assignment to @roryabraham (Exported), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[phivh]

My proposal:

There is a code block that shows the image thumbnail until the image is loaded. From my point of view, it could show ActivityIndicator for better UX on the mobile app.

File: src/components/ImageView/index.native.js

Before:

// Display thumbnail until Image size calculation is complete
if (!this.state.imageWidth || !this.state.imageHeight) {
  return (
      <View
          style={[
              styles.w100,
              styles.h100,
              styles.alignItemsCenter,
              styles.justifyContentCenter,
              styles.overflowHidden,
              styles.errorOutline,
          ]}
      >
          <Image
              source={{uri: this.props.url}}
              style={StyleUtils.getWidthAndHeightStyle(this.state.thumbnailWidth, this.state.thumbnailHeight)}
              resizeMode={Image.resizeMode.contain}
          />
      </View>
  );
}

After

// Display indicator until Image size calculation is complete
if (!this.state.imageWidth || !this.state.imageHeight) {
  return (
      <View>
          <ActivityIndicator
              size="small"
              color={themeColors.textSupporting}
          />
      </View>
  );
}

Review:

ios_issue7010.mp4

[parasharrajat]

First of all, Same behavior should be across platforms does not tell at all what is the expected behavior. Could we please pay more attention while creating the issue or during issue triaging? cc: @mallenexpensify @NicMendonca

---

@phivh I think you misunderstood the issue.

Different behavior for mobile and mobile web when the user opens an attachment and tries to go back using the native back button.
Mobile :- The user go back to the chat.
Mobile web :- The chat closes itself.

which says that we want the user to land on Chat page for both Native Mobile and Mobile web.

[stitesExpensify]

IMO the expected behavior is that the user ends up back at the chat. Do you agree @NicMendonca ?

[NicMendonca]

Yep, I agree with that 👍

[NicMendonca]

@phivh do you have any questions after reading @parasharrajat's feedback?

[phivh]

@NicMendonca Nope, I will look again the navigation, then give a proposal if I have.

[NicMendonca]

Doubled the price to $500: https://www.upwork.com/jobs/~0167b446c2be6b25f4

[parasharrajat]

I have done some research on this and it seems that the solution will be complicated and it requires hacks. There is no default way of doing this using browser technologies.

1. we first need to add an empty entry to the history state whenever a modal is opened.
2. so when the user will close the modal of pressing the back button that entry will be popped instead of going back.
3. Now attach an event handler to popstate event to check if our entry was popped out then close the modal.

Based on the discussion necolas/react-native-web#1738. RN-web authors considers this behavior hacky and will not implement it in their repo. Nor this can be considered a part of react-native-modal repo as it depends on BackHandler or onClose event on Modal component. Thus if RN-web implements this onClose for Modal then it will start working.

In Short, popstate event can not be canceled or prevented so it requires hacks to overcome this.

So, I suggest we close this as it is an edge case and most of the mobile-web users would be used to closing modal via the cross button.

[phivh]

@parasharrajat Understand your point. I also checked this issue and trying with window popstate listener and it seems tricky for now but at least somehow it works but not perfectly. I have an idea from my mind. Let me try to add a hash when view image modal open then will see what happen. I will show you guys then.

[mallenexpensify]

@stitesExpensify @roryabraham you two cool with closing this based on Rajat's reasoning above?

[stitesExpensify]

I'm fine with closing this. @roryabraham can have the final call

[roryabraham]

Yeah, I'm fine with keeping things simple and closing this. I like keeping things consistent where possible, but in this case we can consider it a platform limitation. It's not a severe bug either, just a mild inconsistency.

[melvin-bot]

⚠️ Looks like this issue was linked to a possible regression on PRODUCTION here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a production regression has occurred a Root Cause Analysis is required. Please follow the instructions here.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.