google / accompanist

A collection of extension libraries for Jetpack Compose
https://google.github.io/accompanist
Apache License 2.0
7.48k stars 602 forks source link

[WebView] Suggestion for persistent WebView upon configuration change or navigating to/from WebView composable #1178

Closed jacobmichaelis closed 1 year ago

jacobmichaelis commented 2 years ago

Describe the bug

Android Bug: When rotating the screen, or navigating to another composable and back, the WebView recomposes with no way to show an existing stored WebView.

To Reproduce

Expected behavior

The website shown in the WebView doesn't reload upon configuration change/navigating back to WebView. Suggestion: I am storing my WebView on the ViewModel, but there is no way to just show the existing cached WebView I have when recomposing the WebView by Accompanist. My suggestion is that you add an existingWebView: WebView? = null to the WebView composable and then change var webView by remember { mutableStateOf<WebView?>(null) } to be mutableStateOf(existingWebView). Then in the AndroidView do webView ?: WebView(context).apply { .... I have tested this in my project by copying your code and altering it and it worked as I had hoped. This is just a suggestion, I'm sure you will have a much more brilliant way of doing what I suggested, but I hope it will be available in one way or another in the future.

Environment:

bentrengrove commented 2 years ago

Thanks for the report, this is definitely something we need to improve.

Just as a side note, I wouldn't store a view in the viewmodel, so be careful. That sounds like an almost certain memory leak on rotation as that view will be holding on to a context reference. The proper way to handle this is by saving and restoring state, there isn't a great way to do that right now unfortunately.

One thing that might help though, if you are in a fully Compose app you can pretty safely opt out of rotation by adding

android:configChanges="orientation|screenSize|screenLayout"

to your activity in your AndroidManifest. That should help this issue.

jacobmichaelis commented 2 years ago

@bentrengrove thanks for the advice. If I'm going to be honest, I am actually going to force portrait on the page, screen rotation was just the easiest way to illustrate the issue. The real issue I wanted to overcome was navigating to another page in the nav graph and then navigating back would cause a reload that I don't want. If you have any other suggestions on where to store that let me know, and I'm assuming you are someone who could make changes to the Accompanist library, I'd love to hear your thought process on approaching the solution to this.

agent10 commented 2 years ago

My workaround for that issue now is using a fullscreen dialog(or bottom sheet also will work I guess) nav destination instead of "composable". In that case your screen won't be disposed and webview isn't needed to be reloaded.

bharath1997 commented 2 years ago

Hey guys, I wanted to avoid the reload of webview after navigation, I don't think storing it in ViewModel would be a proper solution too, but this is kind of a serious problem as we are currently developing a hybrid app similar to amazon(native and webview)completely with compose, the main content will be shown in webview, don't want to show it within bottom sheet or fullscreen dialog as it would be the main content to be shown to user after launch, can someone please recommend some other solution or a workaround for this if possible?

github-actions[bot] commented 2 years ago

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

github-actions[bot] commented 2 years ago

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

workspace commented 2 years ago

I tried to solve this issue with caching WebView instance for each route in navigation. It worked well as I wanted, however, I'm not sure it is the best solution.

bharath1997 commented 2 years ago

@workspace can you provide the solution for this in simple code or can you elaborate on what you did?

github-actions[bot] commented 2 years ago

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

nevenisnoob commented 2 years ago

@bentrengrove (Sorry for the direct mention) The bot closed this unsolved issue, please reopen it.

darienalvarez commented 2 years ago

Hello. Any solution to this issue? This is very important for apps who handle hybrid features.

wogg commented 2 years ago

RescueTime is also struggling with this problem, in a hybrid view. An example of smuggling a handle to webview by "caching WebView instance for each route in navigation" would be very help to see @workspace

darienalvarez commented 2 years ago

@wogg I ended creating an Activity with a WebView

<activity
            android:name=".ui.WebActivity"
            android:launchMode="singleInstance"
            android:configChanges="orientation|screenSize"
            android:theme="@style/Theme.WebActivity"
            android:exported="false" />

in the oncreate

 if (savedInstanceState != null) {
            webView.restoreState(savedInstanceState.getBundle(WEB_VIEW_KEY) ?: bundleOf())

            webView.reload()
        } else {
            webView.load(url)
        }

and also

override fun onSaveInstanceState(outState: Bundle) {
        super.onSaveInstanceState(outState)

        Timber.i("onSaveInstanceState")

        val bundle = bundleOf()
        webView.saveState(bundle)
        outState.putBundle(WEB_VIEW_KEY, bundle)
    }
wogg commented 2 years ago

We are using jetpack compose and I am unsure how you would mix that approach in with a tabbed composable view, where the webview is on one of the tabs. Thank you for the example though.

bentrengrove commented 2 years ago

So there is a workaround now available at least for things like Pagers. You can now hoist the WebView instance above your Pager.

Very rough pseudocode, you'll probably have to improve it for your situation for instance making the WebView allocation lazy.

@Composable
fun Screen() {
   val pageCount = 5
   val context = LocalContext.current
   val webViews = remember { List(pageCount) { WebView(context) } }
   val webViewState = List(pageCount) { rememberWebViewState("YOUR_URL") }

   Pager(totalPages = pageCount) { page ->
      WebView(
         state = webViewState[page]
         factory = { webViews[page] }
      )
   }
}
yanzm commented 2 years ago

I used rememberSaveable to store/restore WebView state.

@Composable
fun AccompanistWebSample() {
    val initialUrl = "https://google.github.io/accompanist/web/"

    var lastUrl by rememberSaveable { mutableStateOf<String?>(null) }
    var bundle by rememberSaveable { mutableStateOf<Bundle?>(null) }

    val url = lastUrl ?: initialUrl

    val state = rememberWebViewState(url = url)

    WebView(
        state = state,
        modifier = Modifier.fillMaxSize(),
        onCreated = { webView ->
            bundle?.let {
                webView.restoreState(it)
                bundle = null
            }
        },
        onDispose = { webView ->
            bundle = Bundle().apply {
                webView.saveState(this)
            }
            lastUrl = webView.url
        }
    )
}
YugeCse commented 2 years ago

Yeah, I also think it's important to use a webview in a hybird app. But now, the webview cannot save the state. Is there another way to keep webview's state?

darienalvarez commented 1 year ago

@bentrengrove Do you have any progress on this issue??

bentrengrove commented 1 year ago

No progress, there isn't much we can do about this from the WebView wrapper. This fix needs to come from Compose itself adding a method to do this which I am pushing for. I am keeping this issue open as I know it's a common problem.

darienalvarez commented 1 year ago

@bentrengrove currently I regret to choose compose for my app. There are too many scenarios where compose is very complicated to use:

Those are common features in a lot of apps, it is unfortunate that google expends a lot of time in advertise a product that is far away to be complete or good enough.

bentrengrove commented 1 year ago

Thank you for your honest feedback @darienalvarez . I agree we definitely have room for improvement in those areas. I have passed this on to the wider team for discussion.

We also have an upstream issue for tracking the mechanism needed to fix this WebView issue. https://issuetracker.google.com/252891031

musilto7 commented 1 year ago

I had this problem also. I ended up with solution, which Uses standart WebView inside composable function.

var webView: WebView? by remember {
        mutableStateOf(null)
    }
    AndroidView(
        factory = { context ->
            webView = WebView(context).apply {
                val bundle: Bundle? = savedBundle
                if (bundle != null) {
                    restoreState(bundle)
                } else {
                    loadUrl(model.webViewUrl)
                }
            }
            webView!!
        }
    )

Then I am using Disposable effect for saving state.

 var savedBundle: Bundle? by rememberSaveable {
        mutableStateOf(null)
    }
    DisposableEffect(lifecycleOwner.lifecycle) {
        val observer = LifecycleEventObserver { _, event ->
            if (event == Lifecycle.Event.ON_STOP
                || event == Lifecycle.Event.ON_PAUSE
            ) {
                val bundle = Bundle()
                webView?.saveState(bundle)
                savedBundle = bundle
            }
        }

        lifecycleOwner.lifecycle.addObserver(observer)

        onDispose {
            lifecycleOwner.lifecycle.removeObserver(observer)
        }
    }
alirahimpour89 commented 1 year ago

As some mentioned, could this be a option until the real fix comes? If so I can send you a PR. https://github.com/google/accompanist/commit/98aadd9dd5385ca47aaf064fa53e9cc61745b215

anilateapp commented 1 year ago

@bentrengrove How long to wait for a fix? Is there a temporary solution to the problem?