From 95a0ba64161afdcc9fb13f4adc4a62f30be076b6 Mon Sep 17 00:00:00 2001 From: Saket Narayan Date: Fri, 19 Jul 2024 03:17:57 -0400 Subject: [PATCH] Fix: back button does not work when ZoomableImage() is focused --- .../telephoto/zoomable/ZoomableImageTest.kt | 27 +++++++++++++++++++ .../saket/telephoto/zoomable/ZoomableImage.kt | 2 +- .../zoomable/internal/FocusForwarder.kt | 22 ++++++++------- 3 files changed, 40 insertions(+), 11 deletions(-) diff --git a/zoomable-image/core/src/androidTest/kotlin/me/saket/telephoto/zoomable/ZoomableImageTest.kt b/zoomable-image/core/src/androidTest/kotlin/me/saket/telephoto/zoomable/ZoomableImageTest.kt index b982128a..7287a88d 100644 --- a/zoomable-image/core/src/androidTest/kotlin/me/saket/telephoto/zoomable/ZoomableImageTest.kt +++ b/zoomable-image/core/src/androidTest/kotlin/me/saket/telephoto/zoomable/ZoomableImageTest.kt @@ -3,6 +3,7 @@ package me.saket.telephoto.zoomable import android.graphics.BitmapFactory +import android.view.KeyEvent import android.view.ViewConfiguration import androidx.activity.ComponentActivity import androidx.compose.animation.core.SnapSpec @@ -62,6 +63,7 @@ import androidx.compose.ui.test.junit4.StateRestorationTester import androidx.compose.ui.test.junit4.createAndroidComposeRule import androidx.compose.ui.test.longClick import androidx.compose.ui.test.onNodeWithTag +import androidx.compose.ui.test.onRoot import androidx.compose.ui.test.performClick import androidx.compose.ui.test.performKeyInput import androidx.compose.ui.test.performMultiModalInput @@ -1280,6 +1282,31 @@ class ZoomableImageTest { } } + @Test fun hardware_shortcuts_do_not_break_the_back_button() { + rule.setContent { + val focusRequester = remember { FocusRequester() } + ZoomableImage( + modifier = Modifier + .fillMaxSize() + .focusRequester(focusRequester) + .testTag("image"), + image = ZoomableImageSource.asset("cat_1920.jpg", subSample = false), + contentDescription = null, + ) + LaunchedEffect(Unit) { + focusRequester.requestFocus() + } + } + + rule.waitForIdle() + rule.runOnUiThread { + rule.activity.dispatchKeyEvent(KeyEvent(KeyEvent.ACTION_DOWN, KeyEvent.KEYCODE_BACK)) + rule.activity.dispatchKeyEvent(KeyEvent(KeyEvent.ACTION_UP, KeyEvent.KEYCODE_BACK)) + } + + rule.onRoot().assertDoesNotExist() + } + @Test fun calculate_content_bounds_for_full_quality_images( @TestParameter subSamplingStatus: SubSamplingStatus, ) { diff --git a/zoomable-image/core/src/main/kotlin/me/saket/telephoto/zoomable/ZoomableImage.kt b/zoomable-image/core/src/main/kotlin/me/saket/telephoto/zoomable/ZoomableImage.kt index c255531c..b8c6d9c1 100644 --- a/zoomable-image/core/src/main/kotlin/me/saket/telephoto/zoomable/ZoomableImage.kt +++ b/zoomable-image/core/src/main/kotlin/me/saket/telephoto/zoomable/ZoomableImage.kt @@ -88,7 +88,7 @@ fun ZoomableImage( // When ZoomableImage() is focused, the actual image underneath might not be displayed yet or // might change if the image source updates. Forward focus events to the active image so that - // it can receive keyboard and mouse shortcuts. + // it can receive key events for detecting keyboard shortcuts. val focusForwarder = remember { FocusForwarder() } Box( diff --git a/zoomable-image/core/src/main/kotlin/me/saket/telephoto/zoomable/internal/FocusForwarder.kt b/zoomable-image/core/src/main/kotlin/me/saket/telephoto/zoomable/internal/FocusForwarder.kt index a2c60459..1b3640b1 100644 --- a/zoomable-image/core/src/main/kotlin/me/saket/telephoto/zoomable/internal/FocusForwarder.kt +++ b/zoomable-image/core/src/main/kotlin/me/saket/telephoto/zoomable/internal/FocusForwarder.kt @@ -3,14 +3,16 @@ package me.saket.telephoto.zoomable.internal import android.annotation.SuppressLint import androidx.compose.foundation.focusable import androidx.compose.runtime.Stable +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.setValue +import androidx.compose.runtime.snapshotFlow import androidx.compose.ui.ExperimentalComposeUiApi import androidx.compose.ui.Modifier import androidx.compose.ui.focus.FocusRequester -import androidx.compose.ui.focus.FocusState import androidx.compose.ui.focus.focusRequester import androidx.compose.ui.focus.onFocusChanged import androidx.compose.ui.node.ModifierNodeElement -import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.launch /** @@ -18,15 +20,12 @@ import kotlinx.coroutines.launch */ @Stable internal class FocusForwarder { - private val isParentFocused = MutableStateFlow(false) + var isParentFocused by mutableStateOf(false) + var isChildFocused by mutableStateOf(false) val childFocusRequester = FocusRequester() - fun onParentFocusChanged(focusState: FocusState) { - check(isParentFocused.tryEmit(focusState.isFocused)) - } - suspend fun startForwardingFocus() { - isParentFocused.collect { isParentFocused -> + snapshotFlow { isParentFocused }.collect { if (isParentFocused) { childFocusRequester.requestFocus() } @@ -38,8 +37,10 @@ internal class FocusForwarder { internal fun Modifier.focusForwarder(forwarder: FocusForwarder, enabled: Boolean): Modifier { return if (enabled) { this - .onFocusChanged(forwarder::onParentFocusChanged) - .focusable() + .onFocusChanged { forwarder.isParentFocused = it.isFocused } + // The back button stops working if the parent + // remains focusable after the child receives focus + .focusable(enabled = !forwarder.isChildFocused) } else { this } @@ -49,6 +50,7 @@ internal fun Modifier.focusForwarder(forwarder: FocusForwarder, enabled: Boolean internal fun Modifier.focusable(forwarder: FocusForwarder): Modifier { return this .focusRequester(forwarder.childFocusRequester) + .onFocusChanged { forwarder.isChildFocused = it.isFocused } .then(OnAttachedNodeElement { forwarder.startForwardingFocus() }) }