fix(EMI-1396): Fix Android hardware back button in WebViews #9354

MrSltun · 2023-09-28T14:51:48Z

This PR resolves EMI-1396

Description

This PR upgrades react-native-webview, and adds a handler for the back button in WebViews

Videos

	Before	After
Android	android-before.mp4	android-after.mp4
iOS	ios-before.mov	ios-after.mov

PR Checklist

I have tested my changes on iOS and Android.
I hid my changes behind a feature flag, or they don't need one.
I have included screenshots or videos, or I have not changed the UI.
I have added tests, or my changes don't require any.
I added an app state migration, or my changes do not require one.
I have documented any follow-up work that this PR will require, or it does not require any.
I have added a changelog entry below, or my changes do not require one.

To the reviewers 👀

I would like at least one of the reviewers to run this PR on the simulator or device.

Changelog updates

Cross-platform user-facing changes

Add back handler for web views on Android - mrsltun

iOS user-facing changes

Android user-facing changes

Dev changes

Need help with something? Have a look at our docs, or get in touch with us.

Co-authored-by: George Kartalis <g.kartalis7@gmail.com>

ArtsyOpenSource · 2023-09-28T14:55:21Z

This PR contains the following changes:

Cross-platform user-facing changes (Add back handler for web views on Android - mrsltun)

Generated by 🚫 dangerJS against f2d51cc

MrSltun · 2023-09-28T14:53:57Z

src/app/Components/ArtsyWebView.tsx

-          userAgent={userAgent}
+          applicationNameForUserAgent={userAgent}


Updated the userAgent due to this reported issue

MrSltun · 2023-09-28T14:56:44Z

src/app/Components/ArtsyWebView.tests.tsx

+jest.mock("react-native-webview", () => {
+  const React = require("react")
+  const { View } = require("react-native")
+
+  return {
+    __esModule: true,
+    default: React.forwardRef((props: any, ref: any) => {
+      return <View ref={ref} {...props} />
+    }),
+  }
+})
+


We have to mock the package after the update due to the new architecture (react-native-webview/react-native-webview#2686 (comment))

gkartalis

The tests should be fixed if you move the lines below to setupJEst file:

jest.mock("react-native-webview", () => {
  const React = require("react")
  const { View } = require("react-native")

  return {
    __esModule: true,
    default: React.forwardRef((props: any, ref: any) => {
      return <View ref={ref} {...props} />
    }),
  }
})

gkartalis · 2023-09-28T14:52:55Z

src/app/Components/ArtsyWebView.tests.tsx

+jest.mock("react-native-webview", () => {
+  const React = require("react")
+  const { View } = require("react-native")
+
+  return {
+    __esModule: true,
+    default: React.forwardRef((props: any, ref: any) => {
+      return <View ref={ref} {...props} />
+    }),
+  }
+})
+


gkartalis · 2023-09-28T15:05:08Z

src/app/Components/ArtsyWebView.tsx

-                  }
-                }
-          }
+          onLeftButtonPress={useRightCloseButton && !canGoBack ? undefined : handleBackButtonPress}


nitpick(non-blocking): could we move this to an actual function with explicit returns over implicit ternary hell?

Suggested change

onLeftButtonPress={useRightCloseButton && !canGoBack ? undefined : handleBackButtonPress}

const handleLeftButtonPress = () => {

if (useRightCloseButton && !canGoBack) {

return

}

handleBackButtonPress()

}

I think you may have to make the onLeftOnButtonPress undefined explicitly if I remember correct otherwise the component renders buttons or something, 🤔 I may be remembering wrong but something to check

@brainbicycle Yes! I tried a return when the condition is met, and there was a button showing. I'll try to explicitly return undefined and see

It always shows a button unless the prop is undefined (not a function) 🤷
But I will extract the trenary fo readability

maybe we can update the component to not do that at some point, but def can be separate from this!

brainbicycle · 2023-09-28T15:32:06Z

package.json

@@ -197,7 +197,7 @@
    "react-native-url-polyfill": "1.3.0",
    "react-native-view-shot": "3.4.0",
    "react-native-vimeo-iframe": "1.2.1",
-    "react-native-webview": "11.22.7",


👋 bye bye old dependency

brainbicycle

nice one!

gkartalis

🔥👏 awesome!

MounirDhahri · 2023-09-29T13:18:50Z

src/app/Components/ArtsyWebView.tsx

+      handleGoBack()
+    } else {
+      if (systemBackAction) {
+        systemBackAction()


thought: I assume systemBackAction does the same as above

if (isPresentedModally && !canGoBack) { dismissModal() } else if (!canGoBack) { handleGoBack() }

MounirDhahri · 2023-09-29T13:19:44Z

src/app/Components/ArtsyWebView.tsx

+    return true
+  })
+
+  const leftButton = useRightCloseButton && !canGoBack ? undefined : handleBackButtonPress


suggestion: onLeftButtonPress*

MrSltun and others added 3 commits September 28, 2023 16:35

update react-native-webview to v13.6.0

18af545

add android back handler

91b3866

fix tests

e5086ae

Co-authored-by: George Kartalis <g.kartalis7@gmail.com>

MrSltun requested review from starsirius, MounirDhahri, araujobarret, gkartalis, iskounen and brainbicycle September 28, 2023 14:51

MrSltun self-assigned this Sep 28, 2023

artsy-peril bot added the Jira Synced label Sep 28, 2023

MrSltun commented Sep 28, 2023

View reviewed changes

gkartalis reviewed Sep 28, 2023

View reviewed changes

brainbicycle reviewed Sep 28, 2023

View reviewed changes

brainbicycle previously approved these changes Sep 28, 2023

View reviewed changes

MrSltun added 2 commits September 28, 2023 18:07

move react-native-webview mock to setupJest

14e3255

extract onLeftButtonPress trenary to a const

f2d51cc

MrSltun dismissed brainbicycle’s stale review via f2d51cc September 28, 2023 16:08

gkartalis approved these changes Sep 29, 2023

View reviewed changes

MrSltun merged commit 6947da1 into main Sep 29, 2023
8 checks passed

MrSltun deleted the mrsltun/EMI-1396/fix-Android-hardware-back-button-in-webviews branch September 29, 2023 13:05

MounirDhahri reviewed Sep 29, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(EMI-1396): Fix Android hardware back button in WebViews #9354

fix(EMI-1396): Fix Android hardware back button in WebViews #9354

MrSltun commented Sep 28, 2023 •

edited by jira bot

Loading

ArtsyOpenSource commented Sep 28, 2023 •

edited

Loading

MrSltun Sep 28, 2023

MrSltun Sep 28, 2023

gkartalis left a comment

gkartalis Sep 28, 2023

gkartalis Sep 28, 2023

brainbicycle Sep 28, 2023

brainbicycle Sep 28, 2023

MrSltun Sep 28, 2023

MrSltun Sep 28, 2023

brainbicycle Sep 28, 2023 •

edited

Loading

brainbicycle Sep 28, 2023

gkartalis Sep 28, 2023

brainbicycle left a comment

gkartalis left a comment

MounirDhahri Sep 29, 2023

MounirDhahri Sep 29, 2023

		userAgent={userAgent}
		applicationNameForUserAgent={userAgent}

-          onLeftButtonPress={useRightCloseButton && !canGoBack ? undefined : handleBackButtonPress}
+const handleLeftButtonPress = () => {
+  if (useRightCloseButton && !canGoBack) {
+    return
+  }
+  handleBackButtonPress()
+}

fix(EMI-1396): Fix Android hardware back button in WebViews #9354

fix(EMI-1396): Fix Android hardware back button in WebViews #9354

Conversation

MrSltun commented Sep 28, 2023 • edited by jira bot Loading

Description

Videos

PR Checklist

To the reviewers 👀

Changelog updates

Cross-platform user-facing changes

iOS user-facing changes

Android user-facing changes

Dev changes

ArtsyOpenSource commented Sep 28, 2023 • edited Loading

This PR contains the following changes:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gkartalis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brainbicycle Sep 28, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brainbicycle left a comment

Choose a reason for hiding this comment

gkartalis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MrSltun commented Sep 28, 2023 •

edited by jira bot

Loading

ArtsyOpenSource commented Sep 28, 2023 •

edited

Loading

brainbicycle Sep 28, 2023 •

edited

Loading