feat: fast-multi-click scrolling in tv-compose-material-carousel
Test: Added Instrumentation tests
Bug: b/263230588
Change-Id: I59d62afa2eedef07d576cd7db1336c72ab73dd79
diff --git a/tv/integration-tests/demos/src/main/java/androidx/tv/integration/demos/FeaturedCarousel.kt b/tv/integration-tests/demos/src/main/java/androidx/tv/integration/demos/FeaturedCarousel.kt
index 821281f..15d409d 100644
--- a/tv/integration-tests/demos/src/main/java/androidx/tv/integration/demos/FeaturedCarousel.kt
+++ b/tv/integration-tests/demos/src/main/java/androidx/tv/integration/demos/FeaturedCarousel.kt
@@ -66,7 +66,21 @@
)
}
}
- FeaturedCarousel()
+
+ FeaturedCarousel(Modifier.weight(1f))
+
+ Column(verticalArrangement = Arrangement.spacedBy(20.dp)) {
+ repeat(3) {
+ Box(
+ modifier = Modifier
+ .background(Color.Magenta.copy(alpha = 0.3f))
+ .width(50.dp)
+ .height(50.dp)
+ .drawBorderOnFocus()
+ .focusable()
+ )
+ }
+ }
}
}
items(2) { SampleLazyRow() }
@@ -83,18 +97,23 @@
@OptIn(ExperimentalTvMaterial3Api::class)
@Composable
-internal fun FeaturedCarousel() {
+internal fun FeaturedCarousel(modifier: Modifier = Modifier) {
val backgrounds = listOf(
Color.Red.copy(alpha = 0.3f),
Color.Yellow.copy(alpha = 0.3f),
- Color.Green.copy(alpha = 0.3f)
+ Color.Green.copy(alpha = 0.3f),
+ Color.Blue.copy(alpha = 0.3f),
+ Color.LightGray.copy(alpha = 0.3f),
+ Color.Magenta.copy(alpha = 0.3f),
+ Color.DarkGray.copy(alpha = 0.3f),
+ Color.LightGray.copy(alpha = 0.3f),
)
val carouselState = remember { CarouselState() }
Carousel(
slideCount = backgrounds.size,
carouselState = carouselState,
- modifier = Modifier
+ modifier = modifier
.height(300.dp)
.fillMaxWidth(),
carouselIndicator = {
@@ -113,7 +132,6 @@
Box(
modifier = Modifier
.background(backgrounds[itemIndex])
- .border(2.dp, Color.White.copy(alpha = 0.5f))
.fillMaxSize()
)
}
diff --git a/tv/tv-material/src/androidTest/java/androidx/tv/material3/CarouselTest.kt b/tv/tv-material/src/androidTest/java/androidx/tv/material3/CarouselTest.kt
index 2b0855f..6e600c6 100644
--- a/tv/tv-material/src/androidTest/java/androidx/tv/material3/CarouselTest.kt
+++ b/tv/tv-material/src/androidTest/java/androidx/tv/material3/CarouselTest.kt
@@ -14,7 +14,7 @@
* limitations under the License.
*/
-package androidx.tv.material
+package androidx.tv.material3
import androidx.compose.foundation.background
import androidx.compose.foundation.border
@@ -61,13 +61,6 @@
import androidx.compose.ui.unit.LayoutDirection
import androidx.compose.ui.unit.dp
import androidx.test.platform.app.InstrumentationRegistry
-import androidx.tv.material3.Carousel
-import androidx.tv.material3.CarouselDefaults
-import androidx.tv.material3.CarouselItem
-import androidx.tv.material3.CarouselItemDefaults
-import androidx.tv.material3.CarouselState
-import androidx.tv.material3.ExperimentalTvMaterial3Api
-import androidx.tv.material3.ScrollPauseHandle
import com.google.common.truth.Truth.assertThat
import org.junit.Rule
import org.junit.Test
@@ -572,6 +565,58 @@
}
@Test
+ fun carousel_manualScrolling_fastMultipleKeyPresses() {
+ val carouselState = CarouselState()
+ val tabs = listOf("Tab 1", "Tab 2", "Tab 3")
+
+ rule.setContent {
+ var selectedTabIndex by remember { mutableStateOf(0) }
+
+ Column {
+ TabRow(selectedTabIndex = selectedTabIndex) {
+ tabs.forEachIndexed { index, tab ->
+ Tab(
+ selected = index == selectedTabIndex,
+ onFocus = { selectedTabIndex = index },
+ ) {
+ Text(text = tab)
+ }
+ }
+ }
+
+ SampleCarousel(carouselState = carouselState, slideCount = 20) {
+ SampleCarouselSlide(modifier = Modifier.testTag("slide-$it"), index = it)
+ }
+ }
+ }
+
+ rule.waitForIdle()
+ rule.onNodeWithTag("pager").performSemanticsAction(SemanticsActions.RequestFocus)
+ rule.waitForIdle()
+
+ val slideProgression = listOf(6, 3, -4, 3, -6, 5, 3)
+
+ slideProgression.forEach {
+ if (it < 0) {
+ performKeyPress(NativeKeyEvent.KEYCODE_DPAD_LEFT, it * -1)
+ } else {
+ performKeyPress(NativeKeyEvent.KEYCODE_DPAD_RIGHT, it)
+ }
+ }
+
+ rule.mainClock.advanceTimeBy(animationTime + overlayRenderWaitTime)
+
+ val finalSlide = slideProgression.sum()
+ rule.onNodeWithText("Play $finalSlide").assertIsFocused()
+
+ performKeyPress(NativeKeyEvent.KEYCODE_DPAD_RIGHT, 3)
+
+ rule.mainClock.advanceTimeBy((animationTime + overlayRenderWaitTime) * 3)
+
+ rule.onNodeWithText("Play ${finalSlide + 3}").assertIsFocused()
+ }
+
+ @Test
fun carousel_manualScrolling_ltr() {
rule.setContent {
SampleCarousel { index ->
@@ -679,7 +724,7 @@
modifier = Modifier
.padding(5.dp)
.fillMaxWidth()
- .height(50.dp)
+ .height(200.dp)
.testTag("pager"),
carouselState = carouselState,
slideCount = slideCount,
@@ -702,10 +747,13 @@
@Composable
private fun SampleCarouselSlide(
index: Int,
+ modifier: Modifier = Modifier,
overlayRenderWaitTime: Long = CarouselItemDefaults.OverlayEnterTransitionStartDelayMillis,
content: (@Composable () -> Unit) = { SampleButton("Play $index") },
) {
+
CarouselItem(
+ modifier = modifier,
overlayEnterTransitionStartDelayMillis = overlayRenderWaitTime,
background = {
Box(
@@ -748,10 +796,11 @@
itemRect.bottom <= rootRect.bottom
}
-private fun performKeyPress(keyCode: Int, count: Int = 1) {
+private fun performKeyPress(keyCode: Int, count: Int = 1, afterEachPress: () -> Unit = { }) {
repeat(count) {
InstrumentationRegistry
.getInstrumentation()
.sendKeyDownUpSync(keyCode)
+ afterEachPress()
}
}
diff --git a/tv/tv-material/src/main/java/androidx/tv/material3/Carousel.kt b/tv/tv-material/src/main/java/androidx/tv/material3/Carousel.kt
index 60fa29f..ffcef53 100644
--- a/tv/tv-material/src/main/java/androidx/tv/material3/Carousel.kt
+++ b/tv/tv-material/src/main/java/androidx/tv/material3/Carousel.kt
@@ -17,6 +17,8 @@
package androidx.tv.material3
import android.view.KeyEvent.KEYCODE_BACK
+import android.view.KeyEvent.KEYCODE_DPAD_LEFT
+import android.view.KeyEvent.KEYCODE_DPAD_RIGHT
import androidx.compose.animation.AnimatedContent
import androidx.compose.animation.AnimatedVisibilityScope
import androidx.compose.animation.EnterTransition
@@ -41,20 +43,19 @@
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
-import androidx.compose.runtime.rememberUpdatedState
import androidx.compose.runtime.setValue
import androidx.compose.runtime.snapshotFlow
import androidx.compose.ui.Alignment
import androidx.compose.ui.ExperimentalComposeUiApi
import androidx.compose.ui.Modifier
import androidx.compose.ui.focus.FocusDirection
+import androidx.compose.ui.focus.FocusManager
import androidx.compose.ui.focus.FocusRequester
import androidx.compose.ui.focus.FocusState
-import androidx.compose.ui.focus.focusProperties
import androidx.compose.ui.focus.focusRequester
import androidx.compose.ui.focus.onFocusChanged
import androidx.compose.ui.graphics.Color
-import androidx.compose.ui.input.key.KeyEventType.Companion.KeyDown
+import androidx.compose.ui.input.key.KeyEventType.Companion.KeyUp
import androidx.compose.ui.input.key.key
import androidx.compose.ui.input.key.nativeKeyCode
import androidx.compose.ui.input.key.onKeyEvent
@@ -86,7 +87,6 @@
* @param carouselIndicator indicator showing the position of the current slide among all slides.
* @param content defines the slides for a given index.
*/
-
@Suppress("IllegalExperimentalApiUsage")
@OptIn(ExperimentalComposeUiApi::class, ExperimentalAnimationApi::class)
@ExperimentalTvMaterial3Api
@@ -127,27 +127,31 @@
Box(modifier = modifier
.bringIntoViewIfChildrenAreFocused()
.focusRequester(carouselOuterBoxFocusRequester)
- .onKeyEvent {
- if (it.key.nativeKeyCode == KEYCODE_BACK && it.type == KeyDown) {
- focusManager.moveFocus(FocusDirection.Exit)
- }
- false
- }
.onFocusChanged {
focusState = it
+
+ // When the carousel gains focus for the first time
if (it.isFocused && isAutoScrollActive) {
focusManager.moveFocus(FocusDirection.Enter)
}
}
- .manualScrolling(carouselState, slideCount, isLtr)
- .focusable()) {
+ .handleKeyEvents(
+ carouselState = carouselState,
+ outerBoxFocusRequester = carouselOuterBoxFocusRequester,
+ focusManager = focusManager,
+ slideCount = slideCount,
+ isLtr = isLtr,
+ )
+ .focusable()
+ ) {
AnimatedContent(
targetState = carouselState.activeSlideIndex,
transitionSpec = { enterTransition.with(exitTransition) }
) {
LaunchedEffect(Unit) {
this@AnimatedContent.onAnimationCompletion {
- if (isAutoScrollActive.not()) {
+ // Outer box is focused
+ if (!isAutoScrollActive && focusState?.isFocused == true) {
carouselOuterBoxFocusRequester.requestFocus()
focusManager.moveFocus(FocusDirection.Enter)
}
@@ -163,7 +167,7 @@
private fun shouldPerformAutoScroll(focusState: FocusState?): Boolean {
val carouselIsFocused = focusState?.isFocused ?: false
val carouselHasFocus = focusState?.hasFocus ?: false
- return (carouselIsFocused || carouselHasFocus).not()
+ return !(carouselIsFocused || carouselHasFocus)
}
@Suppress("IllegalExperimentalApiUsage")
@@ -182,19 +186,16 @@
doAutoScroll: Boolean,
onAutoScrollChange: (isAutoScrollActive: Boolean) -> Unit = {},
) {
- val currentTimeToDisplaySlideMillis by rememberUpdatedState(timeToDisplaySlideMillis)
- val currentSlideCount by rememberUpdatedState(slideCount)
-
if (doAutoScroll) {
LaunchedEffect(carouselState) {
while (true) {
yield()
- delay(currentTimeToDisplaySlideMillis)
+ delay(timeToDisplaySlideMillis)
if (carouselState.activePauseHandlesCount > 0) {
snapshotFlow { carouselState.activePauseHandlesCount }
.first { pauseHandleCount -> pauseHandleCount == 0 }
}
- carouselState.moveToNextSlide(currentSlideCount)
+ carouselState.moveToNextSlide(slideCount)
}
}
}
@@ -203,50 +204,58 @@
@Suppress("IllegalExperimentalApiUsage")
@OptIn(ExperimentalTvMaterial3Api::class, ExperimentalComposeUiApi::class)
-private fun Modifier.manualScrolling(
+private fun Modifier.handleKeyEvents(
carouselState: CarouselState,
+ outerBoxFocusRequester: FocusRequester,
+ focusManager: FocusManager,
slideCount: Int,
isLtr: Boolean
-): Modifier =
- this.focusProperties {
- exit = {
- val showPreviousSlideAndGetFocusRequester = {
- if (carouselState.isFirstSlide().not()) {
- carouselState.moveToPreviousSlide(slideCount)
- FocusRequester.Cancel
- } else {
- FocusRequester.Default
- }
- }
- val showNextSlideAndGetFocusRequester = {
- if (carouselState.isLastSlide(slideCount).not()) {
- carouselState.moveToNextSlide(slideCount)
- FocusRequester.Cancel
- } else {
- FocusRequester.Default
- }
- }
- when (it) {
- FocusDirection.Left -> {
- if (isLtr) {
- showPreviousSlideAndGetFocusRequester()
- } else {
- showNextSlideAndGetFocusRequester()
- }
- }
+): Modifier = onKeyEvent {
+ // Ignore KeyUp action type
+ if (it.type == KeyUp) {
+ return@onKeyEvent KeyEventPropagation.ContinuePropagation
+ }
- FocusDirection.Right -> {
- if (isLtr) {
- showNextSlideAndGetFocusRequester()
- } else {
- showPreviousSlideAndGetFocusRequester()
- }
- }
-
- else -> FocusRequester.Default
- }
+ val showPreviousSlideAndGetKeyEventPropagation = {
+ if (carouselState.isFirstSlide()) {
+ KeyEventPropagation.ContinuePropagation
+ } else {
+ carouselState.moveToPreviousSlide(slideCount)
+ outerBoxFocusRequester.requestFocus()
+ KeyEventPropagation.StopPropagation
}
}
+ val showNextSlideAndGetKeyEventPropagation = {
+ if (carouselState.isLastSlide(slideCount)) {
+ KeyEventPropagation.ContinuePropagation
+ } else {
+ carouselState.moveToNextSlide(slideCount)
+ outerBoxFocusRequester.requestFocus()
+ KeyEventPropagation.StopPropagation
+ }
+ }
+
+ when (it.key.nativeKeyCode) {
+ KEYCODE_BACK -> {
+ focusManager.moveFocus(FocusDirection.Exit)
+ KeyEventPropagation.ContinuePropagation
+ }
+
+ KEYCODE_DPAD_LEFT -> if (isLtr) {
+ showPreviousSlideAndGetKeyEventPropagation()
+ } else {
+ showNextSlideAndGetKeyEventPropagation()
+ }
+
+ KEYCODE_DPAD_RIGHT -> if (isLtr) {
+ showNextSlideAndGetKeyEventPropagation()
+ } else {
+ showPreviousSlideAndGetKeyEventPropagation()
+ }
+
+ else -> KeyEventPropagation.ContinuePropagation
+ }
+}
@OptIn(ExperimentalTvMaterial3Api::class)
@Composable
@@ -332,9 +341,11 @@
@OptIn(ExperimentalTvMaterial3Api::class)
internal class ScrollPauseHandleImpl(private val carouselState: CarouselState) : ScrollPauseHandle {
private var active by mutableStateOf(true)
+
init {
carouselState.activePauseHandlesCount += 1
}
+
/**
* Resumes the auto-scroll behaviour if there are no other active [ScrollPauseHandle]s.
*/
diff --git a/tv/tv-material/src/main/java/androidx/tv/material3/CarouselItem.kt b/tv/tv-material/src/main/java/androidx/tv/material3/CarouselItem.kt
index 0b10e6f..96114ca 100644
--- a/tv/tv-material/src/main/java/androidx/tv/material3/CarouselItem.kt
+++ b/tv/tv-material/src/main/java/androidx/tv/material3/CarouselItem.kt
@@ -76,24 +76,27 @@
overlay: @Composable () -> Unit
) {
val overlayVisible = remember { MutableTransitionState(initialState = false) }
- var focusState: FocusState? by remember { mutableStateOf(null) }
+ var containerBoxFocusState: FocusState? by remember { mutableStateOf(null) }
val focusManager = LocalFocusManager.current
var exitFocus by remember { mutableStateOf(false) }
LaunchedEffect(overlayVisible) {
overlayVisible.onAnimationCompletion {
// slide has loaded completely.
- if (focusState?.isFocused == true) { focusManager.moveFocus(FocusDirection.Enter) }
+ if (containerBoxFocusState?.isFocused == true) {
+ focusManager.moveFocus(FocusDirection.Enter)
+ }
}
}
+ // This box holds the focus until the overlay animation completes
Box(modifier = modifier
.onKeyEvent {
exitFocus = it.key.nativeKeyCode == KeyEvent.KEYCODE_BACK && it.type == KeyDown
false
}
.onFocusChanged {
- focusState = it
+ containerBoxFocusState = it
if (it.isFocused && exitFocus) {
focusManager.moveFocus(FocusDirection.Exit)
exitFocus = false
@@ -101,7 +104,8 @@
focusManager.moveFocus(FocusDirection.Enter)
}
}
- .focusable()) {
+ .focusable()
+ ) {
background()
LaunchedEffect(overlayVisible) {
@@ -112,8 +116,7 @@
}
AnimatedVisibility(
- modifier = Modifier
- .align(Alignment.BottomStart),
+ modifier = Modifier.align(Alignment.BottomStart),
visibleState = overlayVisible,
enter = overlayEnterTransition,
exit = overlayExitTransition
diff --git a/tv/tv-material/src/main/java/androidx/tv/material3/KeyEventPropagation.kt b/tv/tv-material/src/main/java/androidx/tv/material3/KeyEventPropagation.kt
new file mode 100644
index 0000000..a5d8ce8
--- /dev/null
+++ b/tv/tv-material/src/main/java/androidx/tv/material3/KeyEventPropagation.kt
@@ -0,0 +1,22 @@
+/*
+ * Copyright 2023 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package androidx.tv.material3
+
+internal object KeyEventPropagation {
+ const val StopPropagation = true
+ const val ContinuePropagation = false
+}
