Fix: back button does not work when ZoomableImage() is focused

saket · Jul 19, 2024 · 95a0ba6 · 95a0ba6
1 parent 8e66fc8
commit 95a0ba6
Show file tree

Hide file tree

Showing 3 changed files with 40 additions and 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
@@ -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
@@ -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
@@ -3,30 +3,29 @@ 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
 
 /**
  * Makes a composable focusable and forwards its focus events to another composable.
  */
 @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() })
 }