-
Notifications
You must be signed in to change notification settings - Fork 591
feat(FF): Add android in app update #10976
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
f5d5139
b8eefa4
c491446
b0c8926
36e97e7
7dfabb2
0c09eeb
6baca6a
a2a015b
d48442c
03052cd
a0650d9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -17,4 +17,18 @@ export const ArtsyNativeModule = { | |||||||||||||||
| Platform.OS === "ios" | ||||||||||||||||
| ? NativeModules.ArtsyNativeModule.isBetaOrDev | ||||||||||||||||
| : (NativeModules.ArtsyNativeModule.getConstants().isBeta as boolean) || __DEV__, | ||||||||||||||||
| checkForAppUpdate: | ||||||||||||||||
| Platform.OS === "ios" | ||||||||||||||||
| ? () => { | ||||||||||||||||
| console.error("checkForAppUpdate is not supported on iOS") | ||||||||||||||||
| } | ||||||||||||||||
| : NativeModules.ArtsyNativeModule.checkForAppUpdate, | ||||||||||||||||
| updateDownloaded: | ||||||||||||||||
| Platform.OS === "ios" | ||||||||||||||||
| ? false | ||||||||||||||||
| : (NativeModules.ArtsyNativeModule.getConstants().updateDownloaded as boolean), | ||||||||||||||||
| completeAppUpdate: | ||||||||||||||||
| Platform.OS === "ios" | ||||||||||||||||
| ? console.error("completeAppUpdate is unsupported on iOS") | ||||||||||||||||
| : NativeModules.ArtsyNativeModule.completeAppUpdate, | ||||||||||||||||
|
MrSltun marked this conversation as resolved.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This calls
Suggested change
|
||||||||||||||||
| } | ||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,57 @@ | ||||||||||||||||||
| import { useToast } from "app/Components/Toast/toastHook" | ||||||||||||||||||
| import { ArtsyNativeModule } from "app/NativeModules/ArtsyNativeModule" | ||||||||||||||||||
| import { useEffect } from "react" | ||||||||||||||||||
| import { NativeEventEmitter, NativeModules, Platform } from "react-native" | ||||||||||||||||||
|
|
||||||||||||||||||
| /** | ||||||||||||||||||
| * This is used to check for app updates on every app launch. | ||||||||||||||||||
| * Also monitors for downloaded updates and shows restart toast. | ||||||||||||||||||
| */ | ||||||||||||||||||
| export const usePromptForUpdate = () => { | ||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 No test covers this hook. Worth one that mocks |
||||||||||||||||||
| const toast = useToast() | ||||||||||||||||||
|
|
||||||||||||||||||
| useEffect(() => { | ||||||||||||||||||
| // Only run on Android | ||||||||||||||||||
| if (Platform.OS !== "android") { | ||||||||||||||||||
| return | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| const eventEmitter = new NativeEventEmitter(NativeModules.ArtsyNativeModule) | ||||||||||||||||||
|
|
||||||||||||||||||
| setTimeout(() => { | ||||||||||||||||||
| // delay prompt until homescreen loads | ||||||||||||||||||
| ArtsyNativeModule.checkForAppUpdate() | ||||||||||||||||||
| }, 12000) | ||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The home screen takes no more than 3 seconds to load - so we can maybe decrease this
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I tried 5,7, and 10, and the update prompt shows up so quickly that before the homescreen loads 😅 so I went with 15 and was too long, so I ended up with 12 and it was perfect
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. oki, thanks.
Comment on lines
+21
to
+24
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟢 The 12s timer isn't cleared on unmount. If the user leaves Home before it fires,
Suggested change
Then add |
||||||||||||||||||
|
|
||||||||||||||||||
| // Check if an update was already downloaded and show toast | ||||||||||||||||||
| if (ArtsyNativeModule.updateDownloaded) { | ||||||||||||||||||
| showUpdateDownloadedToast() | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| // Listen for update download events | ||||||||||||||||||
| const eventListener = eventEmitter.addListener("onAppUpdateDownloaded", () => { | ||||||||||||||||||
| showUpdateDownloadedToast() | ||||||||||||||||||
| }) | ||||||||||||||||||
|
|
||||||||||||||||||
| return () => { | ||||||||||||||||||
| eventListener.remove() | ||||||||||||||||||
| } | ||||||||||||||||||
| }, []) | ||||||||||||||||||
|
|
||||||||||||||||||
| const completeAppUpdate = async () => { | ||||||||||||||||||
| try { | ||||||||||||||||||
| await ArtsyNativeModule.completeAppUpdate() | ||||||||||||||||||
| } catch (error: any) { | ||||||||||||||||||
| console.log("Failed to complete app update:", error) | ||||||||||||||||||
| } | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| const showUpdateDownloadedToast = () => { | ||||||||||||||||||
| toast.show("Update downloaded. Reload to apply the update.", "bottom", { | ||||||||||||||||||
| backgroundColor: "green100", | ||||||||||||||||||
| cta: "Reload", | ||||||||||||||||||
| onPress: completeAppUpdate, | ||||||||||||||||||
| duration: "superLong", | ||||||||||||||||||
| }) | ||||||||||||||||||
| } | ||||||||||||||||||
| } | ||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
outside scope of this pr, but I wonder if there is a way to get errors at type check time or with eslint when we use a native module without a platform check.