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.

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

Conversation

acezard
Copy link
Contributor

@acezard acezard commented Jan 29, 2024

This will avoid a perpetual error state.
We reset the webview state on connection so it doesn't stay stuck

@acezard acezard force-pushed the fix--reset-cloud-webview-after-connection-retrieval branch from 28a766d to 1e253a1 Compare January 29, 2024 10:11
This will avoid a perpetual error state.
We reset the webview state on connection so it doesn't stay stuck
@acezard acezard force-pushed the fix--reset-cloud-webview-after-connection-retrieval branch from 1e253a1 to 0ad99b6 Compare January 30, 2024 09:10
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)

@acezard acezard merged commit c9d1cf8 into master Jan 31, 2024
1 check passed
@acezard acezard deleted the fix--reset-cloud-webview-after-connection-retrieval branch January 31, 2024 10:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants