-
Notifications
You must be signed in to change notification settings - Fork 144
Fix a regression in scrolling when ComposeViewport is embedded in HTML scrollable container #3177
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: jb-main
Are you sure you want to change the base?
Changes from all commits
bb91048
d77e3fd
c442a25
15d3817
4ff8560
b427091
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 |
|---|---|---|
| @@ -0,0 +1,140 @@ | ||
| package androidx.compose.mpp.demo.embedded | ||
|
|
||
| import androidx.compose.foundation.border | ||
| import androidx.compose.foundation.layout.Box | ||
| import androidx.compose.foundation.layout.Column | ||
| import androidx.compose.foundation.layout.fillMaxSize | ||
| import androidx.compose.foundation.layout.fillMaxWidth | ||
| import androidx.compose.foundation.layout.padding | ||
| import androidx.compose.foundation.lazy.LazyColumn | ||
| import androidx.compose.material.Button | ||
| import androidx.compose.material.MaterialTheme | ||
| import androidx.compose.material.Text | ||
| import androidx.compose.runtime.Composable | ||
| 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.ExperimentalComposeUiApi | ||
| import androidx.compose.ui.Modifier | ||
| import androidx.compose.ui.graphics.Color | ||
| import androidx.compose.ui.unit.dp | ||
| import androidx.compose.ui.unit.sp | ||
| import androidx.compose.ui.window.ComposeViewport | ||
| import kotlinx.browser.document | ||
| import org.w3c.dom.HTMLDivElement | ||
| import org.w3c.dom.HTMLElement | ||
|
|
||
| @OptIn(ExperimentalComposeUiApi::class) | ||
| // A reproducer for https://youtrack.jetbrains.com/issue/CMP-10351 | ||
| fun embeddedScrollDemo(composeScroll: Boolean = true) { | ||
| // Override the default fullscreen styles to allow page scrolling | ||
| val style = document.createElement("style") | ||
| style.textContent = """ | ||
| html, body { | ||
| width: 100%; | ||
| height: auto !important; | ||
| margin: 0; | ||
| padding: 0; | ||
| overflow: auto !important; | ||
| } | ||
| body { | ||
| display: block !important; | ||
| } | ||
| #composeApplication { | ||
| display: none; | ||
| } | ||
| """.trimIndent() | ||
| document.head?.appendChild(style) | ||
|
|
||
| val body = document.body ?: return | ||
|
|
||
| // Title | ||
| val heading = document.createElement("h2") | ||
| heading.textContent = "Embedded ComposeViewport Scroll Demo" | ||
| (heading as HTMLElement).style.padding = "16px" | ||
| body.appendChild(heading) | ||
|
|
||
| val description = document.createElement("p") | ||
| description.textContent = "This demo shows a ComposeViewport embedded in a scrollable HTML page. " + | ||
| "Scroll the page to see Compose co-operating with native HTML scroll gestures." + | ||
| if (!composeScroll) " Compose content has no internal scroll." else "" | ||
| (description as HTMLElement).style.apply { | ||
| padding = "0 16px" | ||
| fontStyle = "italic" | ||
| color = "#777" | ||
| } | ||
| body.appendChild(description) | ||
|
|
||
| // HTML content before Compose | ||
| addHtmlParagraphs(body, 5, "Above Compose —") | ||
|
|
||
| // Compose container | ||
| val composeContainer = document.createElement("div") as HTMLDivElement | ||
| composeContainer.style.apply { | ||
| width = "100%" | ||
| height = "400px" | ||
| borderTop = "2px solid #1976D2" | ||
| borderBottom = "2px solid #1976D2" | ||
| margin = "16px 0" | ||
| } | ||
| body.appendChild(composeContainer) | ||
|
|
||
| // HTML content after Compose | ||
| addHtmlParagraphs(body, 10, "Below Compose —") | ||
|
|
||
| ComposeViewport(composeContainer) { | ||
| MaterialTheme { | ||
| var counter by remember { mutableStateOf(0) } | ||
| Column(modifier = Modifier.fillMaxSize()) { | ||
| Box( | ||
| modifier = Modifier.fillMaxWidth().padding(16.dp), | ||
| contentAlignment = Alignment.Center | ||
| ) { | ||
| Column(horizontalAlignment = Alignment.CenterHorizontally) { | ||
| Text("Compose content inside HTML scroll", fontSize = 18.sp) | ||
| Button(onClick = { counter++ }) { | ||
| Text("Clicked $counter times") | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if (composeScroll) { | ||
| ComposeScrollableContent() | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| @Composable | ||
| fun ComposeScrollableContent() { | ||
| LazyColumn(modifier = Modifier.fillMaxSize().padding(horizontal = 16.dp)) { | ||
| items(30) { index -> | ||
| Box( | ||
| modifier = Modifier | ||
| .fillMaxWidth() | ||
| .padding(vertical = 4.dp) | ||
| .border(1.dp, Color.LightGray) | ||
| .padding(12.dp) | ||
| ) { | ||
| Text("Compose LazyColumn item #$index") | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private fun addHtmlParagraphs(container: org.w3c.dom.Element, count: Int, prefix: String) { | ||
| for (i in 1..count) { | ||
| val p = document.createElement("p") | ||
| p.textContent = "$prefix paragraph $i: Lorem ipsum dolor sit amet, consectetur adipiscing elit. " + | ||
| "Sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. " + | ||
| "Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris." | ||
| (p as HTMLElement).style.apply { | ||
| padding = "0 16px" | ||
| lineHeight = "1.6" | ||
| } | ||
| container.appendChild(p) | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,6 +20,7 @@ package androidx.compose.ui.window | |
|
|
||
| import androidx.annotation.VisibleForTesting | ||
| import androidx.collection.mutableIntObjectMapOf | ||
| import androidx.collection.mutableIntSetOf | ||
| import androidx.compose.runtime.Composable | ||
| import androidx.compose.runtime.CompositionLocalProvider | ||
| import androidx.compose.runtime.InternalComposeApi | ||
|
|
@@ -91,7 +92,6 @@ import androidx.lifecycle.enableSavedStateHandles | |
| import kotlinx.browser.document | ||
| import kotlinx.browser.window | ||
| import kotlinx.coroutines.Dispatchers | ||
| import kotlinx.coroutines.MainScope | ||
| import kotlinx.coroutines.channels.Channel | ||
| import kotlinx.coroutines.channels.Channel.Factory.CONFLATED | ||
| import kotlinx.coroutines.coroutineScope | ||
|
|
@@ -311,7 +311,7 @@ internal class ComposeWindow( | |
|
|
||
| override val viewConfiguration = | ||
| object : ViewConfiguration by PlatformContext.DefaultViewConfiguration { | ||
| override val touchSlop: Float get() = with(density) { 18.dp.toPx() } | ||
| override val touchSlop: Float get() = with(density) { 8.dp.toPx() } | ||
| override val maximumFlingVelocity: Float | ||
| //https://cs.android.com/android/platform/superproject/+/android-latest-release:frameworks/base/core/java/android/view/ViewConfiguration.java;l=240;drc=733537294b158d22f2ae383f2ed77c93741798e9 | ||
| get() = with(density) { 8000.dp.toPx() } | ||
|
|
@@ -402,6 +402,10 @@ internal class ComposeWindow( | |
| } | ||
| } | ||
|
|
||
| // It helps Compose to co-operate with the browser's scroll when the ComposeViewport | ||
| // is nested in a scrollable html container | ||
| private val rootScrollObserver = RootScrollObserver() | ||
|
|
||
| private fun initEvents(canvas: HTMLCanvasElement) { | ||
|
|
||
| listOf( | ||
|
|
@@ -432,6 +436,23 @@ internal class ComposeWindow( | |
| } | ||
| } | ||
|
|
||
| // While we don't pass touchmove(s) to Compose, we need to track them to prevent the browser from taking over the gestures. | ||
| // https://developer.mozilla.org/en-US/docs/Web/CSS/Reference/Properties/touch-action | ||
| // > Applications using Touch events disable the browser handling of gestures by calling preventDefault() | ||
| addTypedEvent<TouchEvent>("touchmove") { evt -> | ||
| // This event happens after pointermove. | ||
| if (rootScrollObserver.consumedAnyScroll()) { | ||
| // By calling preventDefault here we prevent a browser from overtaking a scroll gesture. | ||
| evt.preventDefault() | ||
| } else if (!rootScrollObserver.hadAnyScroll() && !activeTouchPointersConsumedMoves.isEmpty()) { | ||
| // The pointermove(s) didn't hit any Compose scrollable content, but | ||
| // we registered at least 1 pointermove event for one or more currently active pointers, | ||
| // and that pointermove was consumed by Compose. | ||
| // So here we prevent the browser from taking over the gestures. | ||
| evt.preventDefault() | ||
| } | ||
| } | ||
|
eymar marked this conversation as resolved.
|
||
|
|
||
| addTypedEvent<WheelEvent>("wheel", passive = false) { event -> | ||
| onWheelEvent(event) | ||
| } | ||
|
|
@@ -496,8 +517,10 @@ internal class ComposeWindow( | |
| LocalComposeWindow provides this, | ||
| content = { | ||
| installFallbackFontDownloader() | ||
| interopContainer.TrackInteropPlacementContainer { | ||
| content() | ||
| WithNestedScrollObserver(rootScrollObserver) { | ||
|
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. Could you explain how it is handled in android? (I mean a nested compose scollable into an android view scrollable) |
||
| interopContainer.TrackInteropPlacementContainer { | ||
| content() | ||
| } | ||
| } | ||
|
|
||
| LaunchedEffect(Unit) { | ||
|
|
@@ -594,6 +617,7 @@ internal class ComposeWindow( | |
| } | ||
|
|
||
| private val activeTouchPointers = mutableIntObjectMapOf<TouchEventWithContainerOffset>() | ||
| private val activeTouchPointersConsumedMoves = mutableIntSetOf() | ||
| private val reusableTouchPointerList = mutableListOf<ComposeScenePointer>() | ||
| private fun getActivePointers(): MutableList<ComposeScenePointer> { | ||
| reusableTouchPointerList.clear() | ||
|
|
@@ -607,6 +631,7 @@ internal class ComposeWindow( | |
| if (event.type == "pointercancel") { | ||
| if (isTouchEvent(event)) { | ||
| activeTouchPointers.clear() | ||
| activeTouchPointersConsumedMoves.remove(event.pointerId) | ||
| activeTouchOffset = null | ||
| } else { | ||
| actualActivePointerButtons = null | ||
|
|
@@ -620,6 +645,7 @@ internal class ComposeWindow( | |
|
|
||
| val eventType = event.getPointerEventType() | ||
| var result: PointerEventResult? = null | ||
| var anyChangeConsumed = false | ||
|
|
||
| if (isMouseEvent(event)) { | ||
| keyboardModeState = KeyboardModeState.Hardware | ||
|
|
@@ -656,6 +682,10 @@ internal class ComposeWindow( | |
| return | ||
| } | ||
|
|
||
| if (activeTouchPointers.isEmpty()) { | ||
| rootScrollObserver.reset() | ||
|
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. I would add here something like: |
||
| } | ||
|
|
||
| // iOS Safari doesn't request focus when the page is shown, | ||
| // and the lifecycle doesn't trigger ON_RESUME. | ||
| // so, we decided to handle every touch | ||
|
|
@@ -714,6 +744,7 @@ internal class ComposeWindow( | |
| nativeEvent = coalescedEvent, | ||
| button = null | ||
| ) | ||
| anyChangeConsumed = anyChangeConsumed || result.anyChangeConsumed | ||
| } | ||
| } else { | ||
| result = scene.sendPointerEvent( | ||
|
|
@@ -726,16 +757,21 @@ internal class ComposeWindow( | |
| nativeEvent = event, | ||
| button = null | ||
| ) | ||
| anyChangeConsumed = result.anyChangeConsumed | ||
| } | ||
|
|
||
| activeTouchOffset = null | ||
|
|
||
| if (eventType == PointerEventType.Release) { | ||
| activeTouchPointers.remove(event.pointerId) | ||
| activeTouchPointersConsumedMoves.remove(event.pointerId) | ||
| } | ||
|
|
||
| if (result != null && result.anyChangeConsumed && event.cancelable) { | ||
| if (anyChangeConsumed && event.cancelable) { | ||
| event.preventDefault() | ||
| if (eventType == PointerEventType.Move) { | ||
| activeTouchPointersConsumedMoves.add(event.pointerId) | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
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.
https://android.googlesource.com/platform/frameworks/base/+/master/core/java/android/view/ViewConfiguration.java?pli=1#191