From a0d633360bde4ac6aba15eb9b6e8c8892c33809b Mon Sep 17 00:00:00 2001 From: Ben Trengrove Date: Mon, 16 May 2022 12:47:59 +1000 Subject: [PATCH 1/2] Fix url in remember not causing recomposition --- .../sample/webview/BasicWebViewSample.kt | 12 +++-- .../com/google/accompanist/web/WebTest.kt | 50 +++++++++++++++++++ .../com/google/accompanist/web/WebView.kt | 20 +++++--- 3 files changed, 69 insertions(+), 13 deletions(-) diff --git a/sample/src/main/java/com/google/accompanist/sample/webview/BasicWebViewSample.kt b/sample/src/main/java/com/google/accompanist/sample/webview/BasicWebViewSample.kt index 8373e829c..70dd5dc18 100644 --- a/sample/src/main/java/com/google/accompanist/sample/webview/BasicWebViewSample.kt +++ b/sample/src/main/java/com/google/accompanist/sample/webview/BasicWebViewSample.kt @@ -39,8 +39,10 @@ import androidx.compose.material.TopAppBar import androidx.compose.material.icons.Icons import androidx.compose.material.icons.filled.ArrowBack import androidx.compose.material.icons.filled.Error +import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember +import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.graphics.Color @@ -49,7 +51,6 @@ import androidx.compose.ui.unit.dp import com.google.accompanist.sample.AccompanistSampleTheme import com.google.accompanist.web.AccompanistWebViewClient import com.google.accompanist.web.LoadingState -import com.google.accompanist.web.WebContent import com.google.accompanist.web.WebView import com.google.accompanist.web.rememberWebViewNavigator import com.google.accompanist.web.rememberWebViewState @@ -60,9 +61,10 @@ class BasicWebViewSample : ComponentActivity() { super.onCreate(savedInstanceState) setContent { AccompanistSampleTheme { - val state = rememberWebViewState(url = "https://google.com") + var url by remember { mutableStateOf("https://google.com") } + val state = rememberWebViewState(url = url) val navigator = rememberWebViewNavigator() - val (textFieldValue, setTextFieldValue) = remember(state.content) { + var textFieldValue by remember(state.content.getCurrentUrl()) { mutableStateOf(state.content.getCurrentUrl() ?: "") } @@ -96,14 +98,14 @@ class BasicWebViewSample : ComponentActivity() { OutlinedTextField( value = textFieldValue, - onValueChange = setTextFieldValue, + onValueChange = { textFieldValue = it }, modifier = Modifier.fillMaxWidth() ) } Button( onClick = { - state.content = WebContent.Url(textFieldValue) + url = textFieldValue }, modifier = Modifier.align(Alignment.CenterVertically) ) { diff --git a/web/src/androidTest/kotlin/com/google/accompanist/web/WebTest.kt b/web/src/androidTest/kotlin/com/google/accompanist/web/WebTest.kt index ebda63f1a..40314bc24 100644 --- a/web/src/androidTest/kotlin/com/google/accompanist/web/WebTest.kt +++ b/web/src/androidTest/kotlin/com/google/accompanist/web/WebTest.kt @@ -21,6 +21,7 @@ import android.webkit.WebView import androidx.compose.material.MaterialTheme import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect +import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember import androidx.compose.runtime.snapshotFlow import androidx.compose.ui.Modifier @@ -48,6 +49,7 @@ import org.hamcrest.CoreMatchers.containsString import org.hamcrest.CoreMatchers.equalTo import org.junit.After import org.junit.Before +import org.junit.Ignore import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith @@ -178,6 +180,53 @@ class WebTest { .check(webMatches(getCurrentUrl(), containsString(LINK_URL))) } + @Test + fun testUrlUpdatedWithRemember() { + val url = mutableStateOf("https://google.com") + rule.setContent { + @Composable + fun MyWebView(url: String) { + val webViewState = rememberWebViewState(url = url) + WebTestContent(webViewState = webViewState, idlingResource = idleResource) + } + + MyWebView(url = url.value) + } + + // Ensure the data is loaded first + onWebView() + .check(webMatches(getCurrentUrl(), containsString("google.com"))) + + url.value = "https://github.com" + + onWebView() + .check(webMatches(getCurrentUrl(), containsString("github.com"))) + } + + @Test + fun testDataUpdatedWithRemember() { + val data = mutableStateOf(TEST_DATA) + rule.setContent { + @Composable + fun MyWebView(data: String) { + val webViewState = rememberWebViewStateWithHTMLData(data = data) + WebTestContent(webViewState = webViewState, idlingResource = idleResource) + } + + MyWebView(data = data.value) + } + + // Ensure the data is loaded first + onWebView() + .withElement(findElement(Locator.TAG_NAME, "a")) + .check(webMatches(getText(), containsString(LINK_TEXT))) + + data.value = TEST_TITLE_DATA + + onWebView() + .check(webMatches(getTitle(), containsString(TITLE_TEXT))) + } + @Test fun testCustomClientIsAssigned() { lateinit var state: WebViewState @@ -510,6 +559,7 @@ class WebTest { } @FlakyTest + @Ignore("Slow and flaky: https://github.com/google/accompanist/issues/1085") @Test fun testAdditionalHttpHeaders() { val mockServer = MockWebServer() diff --git a/web/src/main/java/com/google/accompanist/web/WebView.kt b/web/src/main/java/com/google/accompanist/web/WebView.kt index 2b06c9141..453484f7f 100644 --- a/web/src/main/java/com/google/accompanist/web/WebView.kt +++ b/web/src/main/java/com/google/accompanist/web/WebView.kt @@ -80,6 +80,13 @@ fun WebView( with(navigator) { webView?.handleNavigationEvents() } } + // Set the state of the client and chrome client + // This is done internally to ensure they always are the same instance as the + // parent Web composable + client.state = state + client.navigator = navigator + chromeClient.state = state + AndroidView( factory = { context -> WebView(context).apply { @@ -90,13 +97,6 @@ fun WebView( ViewGroup.LayoutParams.MATCH_PARENT ) - // Set the state of the client and chrome client - // This is done internally to ensure they always are the same instance as the - // parent Web composable - client.state = state - client.navigator = navigator - chromeClient.state = state - webChromeClient = chromeClient webViewClient = client }.also { webView = it } @@ -410,6 +410,8 @@ data class WebViewError( */ @Composable fun rememberWebViewState(url: String, additionalHttpHeaders: Map = emptyMap()) = + // Rather than using .apply {} here we will recreate the state, this prevents + // a recomposition loop when the webview updates the url itself. remember(url, additionalHttpHeaders) { WebViewState( WebContent.Url( @@ -426,4 +428,6 @@ fun rememberWebViewState(url: String, additionalHttpHeaders: Map */ @Composable fun rememberWebViewStateWithHTMLData(data: String, baseUrl: String? = null) = - remember(data, baseUrl) { WebViewState(WebContent.Data(data, baseUrl)) } + remember(data, baseUrl) { + WebViewState(WebContent.Data(data, baseUrl)) + } From 1884715e13b810103d093745da5230b70fa34b39 Mon Sep 17 00:00:00 2001 From: Ben Trengrove Date: Tue, 17 May 2022 11:08:03 +1000 Subject: [PATCH 2/2] Add wait in updated url test --- .../kotlin/com/google/accompanist/web/WebTest.kt | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/web/src/androidTest/kotlin/com/google/accompanist/web/WebTest.kt b/web/src/androidTest/kotlin/com/google/accompanist/web/WebTest.kt index 40314bc24..6fb3670d2 100644 --- a/web/src/androidTest/kotlin/com/google/accompanist/web/WebTest.kt +++ b/web/src/androidTest/kotlin/com/google/accompanist/web/WebTest.kt @@ -182,7 +182,7 @@ class WebTest { @Test fun testUrlUpdatedWithRemember() { - val url = mutableStateOf("https://google.com") + val url = mutableStateOf("about:blank") rule.setContent { @Composable fun MyWebView(url: String) { @@ -195,12 +195,14 @@ class WebTest { // Ensure the data is loaded first onWebView() - .check(webMatches(getCurrentUrl(), containsString("google.com"))) + .check(webMatches(getCurrentUrl(), containsString("about:blank"))) + + url.value = LINK_URL - url.value = "https://github.com" + rule.waitForIdle() onWebView() - .check(webMatches(getCurrentUrl(), containsString("github.com"))) + .check(webMatches(getCurrentUrl(), containsString(LINK_URL))) } @Test