Replace usage of KeyDownHandler with Compose onKeyEvent

The KeyDownHandler was using each platform's native key handling mechanism, which doesn't play nice with Compose's key events system and it caused bugs. One example would be trying to hide a Menu on JVM by pressing Esc. The Menu could not consume the Compose key event, as a result other onKeyEvent listeners in the tree are triggered.
composablehorizons · Oct 16, 2024 · 8ac1671 · 8ac1671
1 parent 69236de
commit 8ac1671
Show file tree

Hide file tree

Showing 16 changed files with 168 additions and 297 deletions.
diff --git a/core/src/androidMain/kotlin/Modal.android.kt b/core/src/androidMain/kotlin/Modal.android.kt
@@ -4,14 +4,14 @@ import android.os.Build
 import android.view.Window
 import android.view.WindowManager
 import androidx.activity.ComponentDialog
-import androidx.compose.runtime.Composable
-import androidx.compose.runtime.CompositionLocalProvider
-import androidx.compose.runtime.DisposableEffect
-import androidx.compose.runtime.rememberCompositionContext
+import androidx.compose.foundation.layout.Box
+import androidx.compose.runtime.*
 import androidx.compose.runtime.saveable.rememberSaveable
-import androidx.compose.runtime.staticCompositionLocalOf
+import androidx.compose.ui.Modifier
 import androidx.compose.ui.graphics.Color
 import androidx.compose.ui.graphics.toArgb
+import androidx.compose.ui.input.key.KeyEvent
+import androidx.compose.ui.input.key.onKeyEvent
 import androidx.compose.ui.platform.ComposeView
 import androidx.compose.ui.platform.LocalContext
 import androidx.compose.ui.platform.LocalView
@@ -23,11 +23,12 @@ import androidx.lifecycle.setViewTreeLifecycleOwner
 import androidx.lifecycle.setViewTreeViewModelStoreOwner
 import androidx.savedstate.findViewTreeSavedStateRegistryOwner
 import androidx.savedstate.setViewTreeSavedStateRegistryOwner
-import java.util.UUID
+import java.util.*
 
 @Composable
 internal actual fun Modal(
     protectNavBars: Boolean,
+    onKeyEvent: (KeyEvent) -> Boolean,
     content: @Composable () -> Unit
 ) {
     val parentView = LocalView.current
@@ -46,7 +47,9 @@ internal actual fun Modal(
                     val localWindow = window
                         ?: error("Attempted to get the dialog's window without content. This should never happen and it's a bug in the library. Kindly open an issue with the steps to reproduce so that we fix it ASAP: https://github.com/composablehorizons/composables-core/issues/new")
                     CompositionLocalProvider(LocalModalWindow provides localWindow) {
-                        content()
+                        Box(Modifier.onKeyEvent(onKeyEvent)) {
+                            content()
+                        }
                     }
                 }
             }

diff --git a/core/src/androidMain/kotlin/Utils.kt b/core/src/androidMain/kotlin/Utils.kt
diff --git a/core/src/appleMain/kotlin/Modal.apple.kt b/core/src/appleMain/kotlin/Modal.apple.kt
@@ -1,21 +1,12 @@
 package com.composables.core
 
-import androidx.compose.ui.window.DialogProperties as ComposeDialogProperties
 import androidx.compose.runtime.Composable
-import androidx.compose.ui.graphics.Color
-import androidx.compose.ui.window.Dialog
+import androidx.compose.ui.input.key.KeyEvent
+import internal.ComposeDialog
 
 @Composable
-internal actual fun Modal(protectNavBars: Boolean, content: @Composable () -> Unit) {
-    Dialog(
-        onDismissRequest = {},
-        properties = ComposeDialogProperties(
-            dismissOnBackPress = false,
-            dismissOnClickOutside = false,
-            usePlatformInsets = false,
-            usePlatformDefaultWidth = false,
-            scrimColor = Color.Transparent
-        ),
-        content = content
-    )
-}
+internal actual fun Modal(
+    protectNavBars: Boolean,
+    onKeyEvent: (KeyEvent) -> Boolean,
+    content: @Composable () -> Unit
+) = ComposeDialog(onKeyEvent, content)
diff --git a/core/src/appleMain/kotlin/Utils.kt b/core/src/appleMain/kotlin/Utils.kt
diff --git a/core/src/commonMain/kotlin/Dialog.kt b/core/src/commonMain/kotlin/Dialog.kt
@@ -9,19 +9,15 @@ import androidx.compose.foundation.focusable
 import androidx.compose.foundation.gestures.detectTapGestures
 import androidx.compose.foundation.layout.Box
 import androidx.compose.foundation.layout.fillMaxSize
-import androidx.compose.runtime.Composable
-import androidx.compose.runtime.LaunchedEffect
-import androidx.compose.runtime.Stable
-import androidx.compose.runtime.getValue
-import androidx.compose.runtime.mutableStateOf
-import androidx.compose.runtime.remember
+import androidx.compose.runtime.*
 import androidx.compose.runtime.saveable.mapSaver
 import androidx.compose.runtime.saveable.rememberSaveable
-import androidx.compose.runtime.setValue
 import androidx.compose.ui.Alignment
 import androidx.compose.ui.Modifier
 import androidx.compose.ui.graphics.Color
 import androidx.compose.ui.input.key.Key
+import androidx.compose.ui.input.key.KeyEvent
+import androidx.compose.ui.input.key.key
 import androidx.compose.ui.input.pointer.pointerInput
 import androidx.compose.ui.semantics.dialog
 import androidx.compose.ui.semantics.semantics
@@ -87,19 +83,21 @@ public fun Dialog(
     }
 
     if (scope.visibleState.currentState || scope.visibleState.targetState || scope.visibleState.isIdle.not()) {
-        Modal {
-            if (properties.dismissOnBackPress) {
-                KeyDownHandler { event ->
-                    return@KeyDownHandler when (event.key) {
-                        Key.Back, Key.Escape -> {
-                            scope.dialogState.visible = false
-                            true
-                        }
-
-                        else -> false
+        val onKeyEvent: ((KeyEvent) -> Boolean) = if (properties.dismissOnBackPress) {
+            { event: KeyEvent ->
+                when (event.key) {
+                    Key.Back, Key.Escape -> {
+                        scope.dialogState.visible = false
+                        true
                     }
+
+                    else -> false
                 }
             }
+        } else {
+            { false }
+        }
+        Modal(onKeyEvent = onKeyEvent) {
             Box(
                 modifier = Modifier.fillMaxSize()
                     .let {

diff --git a/core/src/commonMain/kotlin/Menu.kt b/core/src/commonMain/kotlin/Menu.kt
@@ -1,3 +1,5 @@
+@file:OptIn(InternalComposeUiApi::class)
+
 package com.composables.core
 
 import androidx.compose.animation.AnimatedVisibility
@@ -12,9 +14,10 @@ import androidx.compose.foundation.layout.Box
 import androidx.compose.foundation.layout.Column
 import androidx.compose.runtime.*
 import androidx.compose.ui.Alignment
+import androidx.compose.ui.InternalComposeUiApi
 import androidx.compose.ui.Modifier
 import androidx.compose.ui.focus.*
-import androidx.compose.ui.input.key.Key
+import androidx.compose.ui.input.key.*
 import androidx.compose.ui.platform.LocalDensity
 import androidx.compose.ui.platform.LocalFocusManager
 import androidx.compose.ui.semantics.Role
@@ -37,54 +40,29 @@ public fun Menu(
 }
 
 @Composable
-public fun Menu(
-    state: MenuState, modifier: Modifier = Modifier, content: @Composable MenuScope.() -> Unit
-) {
+public fun Menu(state: MenuState, modifier: Modifier = Modifier, content: @Composable MenuScope.() -> Unit) {
     val scope = remember(state.expanded) { MenuScope(state) }
     val coroutineScope = rememberCoroutineScope()
-    var hasFocus by remember { mutableStateOf(false) }
-
-    if (hasFocus) {
-        KeyDownHandler { event ->
-            when (event.key) {
-                Key.DirectionDown -> {
-                    if (scope.menuState.expanded.not()) {
-                        scope.menuState.expanded = true
-                        coroutineScope.launch {
-                            // wait for the Popup to be displayed.
-                            // There is no official API to wait for this to happen
-                            delay(50)
-                            state.menuFocusRequester.requestFocus()
-                            state.currentFocusManager?.moveFocus(FocusDirection.Enter)
-                        }
-                        true
-                    } else {
-                        if (state.hasMenuFocus.not()) {
-                            state.menuFocusRequester.requestFocus()
-                            state.currentFocusManager?.moveFocus(FocusDirection.Enter)
-                        } else {
-                            state.currentFocusManager?.moveFocus(FocusDirection.Next)
-                        }
-                        true
-                    }
-                }
-
-                Key.DirectionUp -> {
-                    state.currentFocusManager?.moveFocus(FocusDirection.Previous)
-                    true
-                }
 
-                Key.Escape -> {
-                    state.expanded = false
-                    state.currentFocusManager?.clearFocus()
+    Box(modifier.onKeyEvent { event ->
+        if (event.type != KeyEventType.KeyDown) return@onKeyEvent false
+        when (event.key) {
+            Key.DirectionDown -> {
+                if (scope.menuState.expanded.not()) {
+                    scope.menuState.expanded = true
+                    coroutineScope.launch {
+                        // wait for the Popup to be displayed.
+                        // There is no official API to wait for this to happen
+                        delay(50)
+                        state.menuFocusRequester.requestFocus()
+                        state.currentFocusManager?.moveFocus(FocusDirection.Enter)
+                    }
                     true
-                }
-
-                else -> false
+                } else false
             }
+            else -> false
         }
-    }
-    Box(modifier.onFocusChanged { hasFocus = it.hasFocus }) {
+    }) {
         state.currentFocusManager = LocalFocusManager.current
         scope.content()
     }
@@ -200,13 +178,38 @@ public fun MenuScope.MenuContent(
             popupPositionProvider = positionProvider,
         ) {
             menuState.currentFocusManager = LocalFocusManager.current
-            AnimatedVisibility(visibleState = expandedState,
+            AnimatedVisibility(
+                visibleState = expandedState,
                 enter = enter,
                 exit = exit,
                 modifier = Modifier.onFocusChanged {
                     menuState.hasMenuFocus = it.hasFocus
-                }) {
+                }.onKeyEvent { event ->
+                    if (event.type != KeyEventType.KeyDown) return@onKeyEvent false
+
+                    return@onKeyEvent when (event.key) {
+                        Key.DirectionDown -> {
+                            menuState.currentFocusManager!!.moveFocus(FocusDirection.Next)
+                            true
+                        }
+
+                        Key.DirectionUp -> {
+                            menuState.currentFocusManager!!.moveFocus(FocusDirection.Previous)
+                            true
+                        }
+
+                        Key.Escape -> {
+                            menuState.expanded = false
+                            true
+                        }
+                        else -> false
+                    }
+                }
+            ) {
                 Column(modifier.focusRequester(menuState.menuFocusRequester)) {
+                    LaunchedEffect(Unit) {
+                        menuState.menuFocusRequester.requestFocus()
+                    }
                     contents()
                 }
             }

diff --git a/core/src/commonMain/kotlin/Modal.kt b/core/src/commonMain/kotlin/Modal.kt
@@ -1,6 +1,11 @@
 package com.composables.core
 
 import androidx.compose.runtime.Composable
+import androidx.compose.ui.input.key.KeyEvent
 
 @Composable
-internal expect fun Modal(protectNavBars: Boolean = false, content: @Composable () -> Unit)
+internal expect fun Modal(
+    protectNavBars: Boolean = false,
+    onKeyEvent: (KeyEvent) -> Boolean = { false },
+    content: @Composable () -> Unit
+)
diff --git a/core/src/commonMain/kotlin/ModalBottomSheet.kt b/core/src/commonMain/kotlin/ModalBottomSheet.kt
@@ -11,20 +11,14 @@ import androidx.compose.foundation.focusable
 import androidx.compose.foundation.gestures.detectTapGestures
 import androidx.compose.foundation.layout.Box
 import androidx.compose.foundation.layout.fillMaxSize
-import androidx.compose.runtime.Composable
-import androidx.compose.runtime.CompositionLocalProvider
-import androidx.compose.runtime.LaunchedEffect
-import androidx.compose.runtime.compositionLocalOf
-import androidx.compose.runtime.derivedStateOf
-import androidx.compose.runtime.getValue
-import androidx.compose.runtime.mutableStateOf
-import androidx.compose.runtime.remember
+import androidx.compose.runtime.*
 import androidx.compose.runtime.saveable.mapSaver
 import androidx.compose.runtime.saveable.rememberSaveable
-import androidx.compose.runtime.setValue
 import androidx.compose.ui.Modifier
 import androidx.compose.ui.graphics.Color
 import androidx.compose.ui.input.key.Key
+import androidx.compose.ui.input.key.KeyEvent
+import androidx.compose.ui.input.key.key
 import androidx.compose.ui.input.pointer.pointerInput
 import androidx.compose.ui.unit.Dp
 import androidx.compose.ui.unit.dp
@@ -150,7 +144,26 @@ public fun ModalBottomSheet(
         scope.visibleState.targetState = state.currentDetent != SheetDetent.Hidden
 
         if (scope.visibleState.currentState || scope.visibleState.targetState || scope.visibleState.isIdle.not()) {
-            Modal(protectNavBars = true) {
+
+            val onKeyEvent: (KeyEvent) -> Boolean = if (scope.sheetState.isIdle && properties.dismissOnBackPress) {
+                // AnchoredDraggableState jumps to 1.0f progress as soon as we change the current value
+                // while moving. This causes the sheet to disappear instead of animating away nicely.
+                // Because of this, we only manage back presses when the sheet is idle
+                { event ->
+                    when (event.key) {
+                        Key.Back, Key.Escape -> {
+                            scope.sheetState.currentDetent = SheetDetent.Hidden
+                            true
+                        }
+
+                        else -> false
+                    }
+                }
+            } else {
+                { false }
+            }
+
+            Modal(protectNavBars = true, onKeyEvent = onKeyEvent) {
                 Box(Modifier
                     .fillMaxSize()
                     .let {
@@ -209,23 +222,6 @@ public fun ModalBottomSheetScope.Sheet(
         }
     }
 
-    val properties = LocalModalProperties.current
-    if (sheetState.isIdle && properties.dismissOnBackPress) {
-        // AnchoredDraggableState jumps to 1.0f progress as soon as we change the current value
-        // while moving. This causes the sheet to disappear instead of animating away nicely.
-        // Because of this, we only manage back presses when the sheet is idle
-        KeyDownHandler { event ->
-            return@KeyDownHandler when (event.key) {
-                Key.Back, Key.Escape -> {
-                    sheetState.currentDetent = SheetDetent.Hidden
-                    true
-                }
-
-                else -> false
-            }
-        }
-    }
-
     if (hasBeenIntroduced) {
         LaunchedEffect(sheetState.currentDetent) {
             if (sheetState.currentDetent == SheetDetent.Hidden && sheetState.targetDetent == sheetState.currentDetent) {