Skip to content
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.

Sign up for GitHub

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

fix: reset cloud webview on connection retrieval (VO-269) #1149

Merged
merged 1 commit into from
Jan 31, 2024
Merged

fix: reset cloud webview on connection retrieval (VO-269) #1149

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion src/libs/services/NetService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,10 @@ const waitForOnline = (
const isCallbackFunction = typeof callbackArg === 'function'

if (state.isConnected) {
netLogger.debug('Online, using callback parameters')
netLogger.debug('Online, using callback parameters:', {
callbackArg,
params
})

try {
if (callbackArg === routes.stack)
Expand Down Expand Up @@ -85,8 +88,17 @@ const handleOffline = (
waitForOnline(callbackRoute, params)
}

const handleOfflineWithCallback = (
callback: (state: NetInfoState) => void
): void => {
navigate(routes.error, { type: strings.errorScreens.offline })

waitForOnline(callback)
}

export const NetService = {
handleOffline,
handleOfflineWithCallback,
isConnected,
isOffline,
waitForOnline
Expand Down
28 changes: 17 additions & 11 deletions src/screens/login/components/ClouderyViewSwitch.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import Minilog from 'cozy-minilog'

import React, {
forwardRef,
useEffect,
Expand Down Expand Up @@ -37,6 +38,7 @@ import {
import { LOGIN_FLAGSHIP_URL } from '/screens/login/components/functions/oidc'
import { jsPaddingInjection } from '/screens/login/components/functions/webViewPaddingInjection'
import { APPLICATION_NAME_FOR_USER_AGENT } from '/constants/userAgent'
import { navigationRef } from '/libs/RootNavigation'

const log = Minilog('ClouderyViewSwitchProps')

Expand Down Expand Up @@ -210,6 +212,21 @@ const ClouderyWebView = forwardRef(
onMessage?.(event)
}

const handleError = async (webviewErrorEvent: unknown): Promise<void> => {
try {
const isOffline = await NetService.isOffline()
isOffline &&
NetService.handleOfflineWithCallback(() => {
webviewRef.current?.reload() // Have to reload the webview when the user is back online or it will stay errored
navigationRef.navigate(routes.welcome) // Go back to the welcome screen to leave the offline screen (offline screen should be refactored to be a modal)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

offline screen should be refactored to be a modal

Can't we take the opportunity to do it? If @Ldoppea is agreed with this take?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think he would agree since he prefers modal based approach to route based like here.
I could do it if we have enough time, I think is needed something like half a day to one day.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, i'm not sure about what is the best option for the offline screen.

My arguments for using modals for lock screen or clouderyOffer screen is that they should not have any impact on the application state when displayed.

Here the offline screen is often displayed when the webviews are broken. So it may be useful to have a navigation that enforce reseting the view.

That being said, by reading your reload() call, I understand that state was not reseted with the current way of handling offline?

To sumarize, I see 2 approaches:

  • A screen that unmount the current screen while displaying the offline one
    • pro: we ensure the screen is reset and that no "offline" related bug remains
    • cons: we need to track context to reapply it when connexion is restored
  • A modal that keep the app's state intact
    • pro: we don't need to track context
    • cons: the screen need to reset itself when connexion is restored (this may be tricky is some screens)

})
} catch (error) {
log.error(error)
} finally {
log.error(webviewErrorEvent)
}
}

return (
<SupervisedWebView
applicationNameForUserAgent={APPLICATION_NAME_FOR_USER_AGENT}
Expand Down Expand Up @@ -254,17 +271,6 @@ const run = `
})();
`

const handleError = async (webviewErrorEvent: unknown): Promise<void> => {
try {
const isOffline = await NetService.isOffline()
isOffline && NetService.handleOffline(routes.onboarding)
} catch (error) {
log.error(error)
} finally {
log.error(webviewErrorEvent)
}
}

const styles = StyleSheet.create({
clouderyLoginView: {
position: 'absolute',
Expand Down
Loading