diff --git a/mobile/packages/expo-hardware-keyboard-navigation/android/build.gradle b/mobile/packages/expo-hardware-keyboard-navigation/android/build.gradle index 973ff6bb050..645040189f3 100644 --- a/mobile/packages/expo-hardware-keyboard-navigation/android/build.gradle +++ b/mobile/packages/expo-hardware-keyboard-navigation/android/build.gradle @@ -3,6 +3,11 @@ plugins { id 'expo-module-gradle-plugin' } +dependencies { + testImplementation 'junit:junit:4.13.2' + testImplementation 'org.robolectric:robolectric:4.16' +} + group = 'expo.modules.hardwarekeyboardnavigation' version = '0.0.1' diff --git a/mobile/packages/expo-hardware-keyboard-navigation/android/src/main/java/expo/modules/hardwarekeyboardnavigation/HardwareKeyboardActivityLifecycleListener.kt b/mobile/packages/expo-hardware-keyboard-navigation/android/src/main/java/expo/modules/hardwarekeyboardnavigation/HardwareKeyboardActivityLifecycleListener.kt index 0f88e20a79e..c5bb587f0a1 100644 --- a/mobile/packages/expo-hardware-keyboard-navigation/android/src/main/java/expo/modules/hardwarekeyboardnavigation/HardwareKeyboardActivityLifecycleListener.kt +++ b/mobile/packages/expo-hardware-keyboard-navigation/android/src/main/java/expo/modules/hardwarekeyboardnavigation/HardwareKeyboardActivityLifecycleListener.kt @@ -14,6 +14,7 @@ class HardwareKeyboardActivityLifecycleListener : ReactActivityLifecycleListener } override fun onDestroy(activity: Activity) { + HardwareKeyboardNavigationRegistry.clearCapturedKeys() val callback = activity.window.callback if (callback is HardwareKeyboardWindowCallback) { activity.window.callback = callback.delegate diff --git a/mobile/packages/expo-hardware-keyboard-navigation/android/src/main/java/expo/modules/hardwarekeyboardnavigation/HardwareKeyboardNavigationRegistry.kt b/mobile/packages/expo-hardware-keyboard-navigation/android/src/main/java/expo/modules/hardwarekeyboardnavigation/HardwareKeyboardNavigationRegistry.kt index 42cac3fef9e..49276bb5d4a 100644 --- a/mobile/packages/expo-hardware-keyboard-navigation/android/src/main/java/expo/modules/hardwarekeyboardnavigation/HardwareKeyboardNavigationRegistry.kt +++ b/mobile/packages/expo-hardware-keyboard-navigation/android/src/main/java/expo/modules/hardwarekeyboardnavigation/HardwareKeyboardNavigationRegistry.kt @@ -16,6 +16,7 @@ object HardwareKeyboardNavigationRegistry { @Volatile private var commands: List = emptyList() private val observers = mutableSetOf Unit>>() + private val capturedKeys = mutableSetOf>() fun setCommands(next: List) { commands = next @@ -34,8 +35,24 @@ object HardwareKeyboardNavigationRegistry { observers.removeAll { it.get() == null } } - fun dispatch(event: KeyEvent): Boolean { - if (event.action != KeyEvent.ACTION_DOWN || event.repeatCount > 0) { + fun clearCapturedKeys() { + capturedKeys.clear() + } + + fun dispatch(event: KeyEvent, canStartCapture: Boolean = true): Boolean { + val identity = event.deviceId to event.keyCode + if (event.action == KeyEvent.ACTION_UP) { + return capturedKeys.remove(identity) + } + if (event.action != KeyEvent.ACTION_DOWN) { + return false + } + if (event.repeatCount > 0) { + return identity in capturedKeys + } + // A fresh down supersedes an up lost during device/window changes. + capturedKeys.remove(identity) + if (!canStartCapture) { return false } val key = keyToken(event.keyCode) ?: return false @@ -52,6 +69,7 @@ object HardwareKeyboardNavigationRegistry { if (currentObservers.isEmpty()) { return false } + capturedKeys.add(identity) currentObservers.forEach { it(command) } return true } diff --git a/mobile/packages/expo-hardware-keyboard-navigation/android/src/main/java/expo/modules/hardwarekeyboardnavigation/HardwareKeyboardWindowCallback.kt b/mobile/packages/expo-hardware-keyboard-navigation/android/src/main/java/expo/modules/hardwarekeyboardnavigation/HardwareKeyboardWindowCallback.kt index dcd4f067c0a..11bc2ee28dd 100644 --- a/mobile/packages/expo-hardware-keyboard-navigation/android/src/main/java/expo/modules/hardwarekeyboardnavigation/HardwareKeyboardWindowCallback.kt +++ b/mobile/packages/expo-hardware-keyboard-navigation/android/src/main/java/expo/modules/hardwarekeyboardnavigation/HardwareKeyboardWindowCallback.kt @@ -12,6 +12,13 @@ class HardwareKeyboardWindowCallback( override fun dispatchKeyEvent(event: KeyEvent): Boolean { val text = (window.currentFocus as? TextView)?.editableText val composing = text != null && BaseInputConnection.getComposingSpanStart(text) >= 0 - return (!composing && HardwareKeyboardNavigationRegistry.dispatch(event)) || delegate.dispatchKeyEvent(event) + return HardwareKeyboardNavigationRegistry.dispatch(event, canStartCapture = !composing) || delegate.dispatchKeyEvent(event) + } + + override fun onWindowFocusChanged(hasFocus: Boolean) { + if (!hasFocus) { + HardwareKeyboardNavigationRegistry.clearCapturedKeys() + } + delegate.onWindowFocusChanged(hasFocus) } } diff --git a/mobile/packages/expo-hardware-keyboard-navigation/android/src/test/java/expo/modules/hardwarekeyboardnavigation/HardwareKeyboardNavigationOwnershipTest.kt b/mobile/packages/expo-hardware-keyboard-navigation/android/src/test/java/expo/modules/hardwarekeyboardnavigation/HardwareKeyboardNavigationOwnershipTest.kt new file mode 100644 index 00000000000..f3be09fbb68 --- /dev/null +++ b/mobile/packages/expo-hardware-keyboard-navigation/android/src/test/java/expo/modules/hardwarekeyboardnavigation/HardwareKeyboardNavigationOwnershipTest.kt @@ -0,0 +1,61 @@ +package expo.modules.hardwarekeyboardnavigation + +import android.view.KeyEvent +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config + +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [28], manifest = Config.NONE) +class HardwareKeyboardNavigationOwnershipTest { + private var sends = 0 + private val observer: (HardwareKeyboardCommand) -> Unit = { sends++ } + private val reference = HardwareKeyboardNavigationRegistry.addObserver(observer) + + init { + HardwareKeyboardNavigationRegistry.setCommands(listOf( + HardwareKeyboardCommand("tab.previousRecent", "Tab", true, false, false, false) + )) + } + + private fun key(action: Int, repeat: Int = 0, device: Int = 1) = KeyEvent( + 0, 0, action, KeyEvent.KEYCODE_TAB, repeat, KeyEvent.META_CTRL_ON, device, 0 + ) + + @After fun cleanup() { + HardwareKeyboardNavigationRegistry.removeObserver(reference) + HardwareKeyboardNavigationRegistry.setCommands(emptyList()) + HardwareKeyboardNavigationRegistry.clearCapturedKeys() + } + + @Test fun ownedRepeatsAndReleaseNeverReachTerminalAfterNavigation() { + assertTrue(HardwareKeyboardNavigationRegistry.dispatch(key(KeyEvent.ACTION_DOWN))) + HardwareKeyboardNavigationRegistry.setCommands(emptyList()) + assertTrue(HardwareKeyboardNavigationRegistry.dispatch(key(KeyEvent.ACTION_DOWN, 1))) + assertTrue(HardwareKeyboardNavigationRegistry.dispatch(key(KeyEvent.ACTION_UP))) + assertFalse(HardwareKeyboardNavigationRegistry.dispatch(key(KeyEvent.ACTION_UP))) + assertEquals(1, sends) + } + + @Test fun compositionDoesNotStartCaptureButStillFinishesOwnedKeys() { + assertFalse(HardwareKeyboardNavigationRegistry.dispatch(key(KeyEvent.ACTION_DOWN), false)) + assertTrue(HardwareKeyboardNavigationRegistry.dispatch(key(KeyEvent.ACTION_DOWN))) + assertTrue(HardwareKeyboardNavigationRegistry.dispatch(key(KeyEvent.ACTION_DOWN, 1), false)) + assertTrue(HardwareKeyboardNavigationRegistry.dispatch(key(KeyEvent.ACTION_UP), false)) + assertEquals(1, sends) + } + + @Test fun deviceOwnershipAndWindowCleanupDoNotSwallowUnrelatedKeys() { + assertTrue(HardwareKeyboardNavigationRegistry.dispatch(key(KeyEvent.ACTION_DOWN))) + assertFalse(HardwareKeyboardNavigationRegistry.dispatch(key(KeyEvent.ACTION_UP, device = 2))) + HardwareKeyboardNavigationRegistry.clearCapturedKeys() + assertFalse(HardwareKeyboardNavigationRegistry.dispatch(key(KeyEvent.ACTION_UP))) + assertTrue(HardwareKeyboardNavigationRegistry.dispatch(key(KeyEvent.ACTION_DOWN))) + assertEquals(2, sends) + } +} diff --git a/mobile/packages/expo-hardware-keyboard-navigation/ios/ExpoHardwareKeyboardNavigationModule.swift b/mobile/packages/expo-hardware-keyboard-navigation/ios/ExpoHardwareKeyboardNavigationModule.swift index e770917824f..2f2e65c7225 100644 --- a/mobile/packages/expo-hardware-keyboard-navigation/ios/ExpoHardwareKeyboardNavigationModule.swift +++ b/mobile/packages/expo-hardware-keyboard-navigation/ios/ExpoHardwareKeyboardNavigationModule.swift @@ -20,14 +20,21 @@ private struct HardwareKeyboardCommandIdentity: Hashable { } @MainActor -private final class HardwareKeyboardCommandRegistry { - static let shared = HardwareKeyboardCommandRegistry() +public final class HardwareKeyboardCommandRegistry { + public static let shared = HardwareKeyboardCommandRegistry() private weak var controller: UIViewController? private var commandPayloads: [HardwareKeyboardCommandIdentity: [String: String]] = [:] private var installedCommands: [UIKeyCommand] = [] var handler: (([String: String]) -> Void)? + public func owns(_ command: UIKeyCommand, in window: UIWindow?) -> Bool { + guard let window, controller?.viewIfLoaded?.window === window, + let identity = commandIdentity(input: command.input, modifierFlags: command.modifierFlags) + else { return false } + return commandPayloads[identity] != nil + } + func install(_ records: [HardwareKeyboardCommandRecord]) { guard let controller = rootViewController() else { return } if self.controller !== controller { diff --git a/mobile/packages/expo-hardware-keyboard/android/build.gradle b/mobile/packages/expo-hardware-keyboard/android/build.gradle index a873b5d8698..c38147e3546 100644 --- a/mobile/packages/expo-hardware-keyboard/android/build.gradle +++ b/mobile/packages/expo-hardware-keyboard/android/build.gradle @@ -3,6 +3,11 @@ plugins { id 'expo-module-gradle-plugin' } +dependencies { + testImplementation 'junit:junit:4.13.2' + testImplementation 'org.robolectric:robolectric:4.16' +} + group = 'expo.modules.hardwarekeyboard' version = '0.0.1' diff --git a/mobile/packages/expo-hardware-keyboard/android/src/main/java/expo/modules/hardwarekeyboard/HardwareKeyboardCaptureView.kt b/mobile/packages/expo-hardware-keyboard/android/src/main/java/expo/modules/hardwarekeyboard/HardwareKeyboardCaptureView.kt index 27620c93450..9a23b70eb30 100644 --- a/mobile/packages/expo-hardware-keyboard/android/src/main/java/expo/modules/hardwarekeyboard/HardwareKeyboardCaptureView.kt +++ b/mobile/packages/expo-hardware-keyboard/android/src/main/java/expo/modules/hardwarekeyboard/HardwareKeyboardCaptureView.kt @@ -2,6 +2,7 @@ package expo.modules.hardwarekeyboard import android.content.Context import android.view.KeyEvent +import android.view.KeyCharacterMap import android.view.inputmethod.BaseInputConnection import android.widget.EditText import expo.modules.kotlin.AppContext @@ -48,6 +49,9 @@ class HardwareKeyboardCaptureView(context: Context, appContext: AppContext) : val shift = (meta and KeyEvent.META_SHIFT_ON) != 0 val repeat = event.repeatCount > 0 if (captureMode == "submit") { + if (!isPhysicalKeyboardEvent(event)) { + return super.dispatchKeyEvent(event) + } val enter = event.keyCode == KeyEvent.KEYCODE_ENTER || event.keyCode == KeyEvent.KEYCODE_NUMPAD_ENTER if (!enter || ctrl || alt || shift) { return super.dispatchKeyEvent(event) @@ -161,3 +165,7 @@ class HardwareKeyboardCaptureView(context: Context, appContext: AppContext) : key.startsWith("F") } } + +internal fun isPhysicalKeyboardEvent(event: KeyEvent): Boolean = + event.flags and KeyEvent.FLAG_SOFT_KEYBOARD == 0 && + event.deviceId != KeyCharacterMap.VIRTUAL_KEYBOARD && event.device?.isVirtual != true diff --git a/mobile/packages/expo-hardware-keyboard/android/src/test/java/expo/modules/hardwarekeyboard/HardwareKeyboardSubmitEventTest.kt b/mobile/packages/expo-hardware-keyboard/android/src/test/java/expo/modules/hardwarekeyboard/HardwareKeyboardSubmitEventTest.kt new file mode 100644 index 00000000000..0a78d2937e9 --- /dev/null +++ b/mobile/packages/expo-hardware-keyboard/android/src/test/java/expo/modules/hardwarekeyboard/HardwareKeyboardSubmitEventTest.kt @@ -0,0 +1,31 @@ +package expo.modules.hardwarekeyboard + +import android.view.KeyCharacterMap +import android.view.KeyEvent +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config + +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [28], manifest = Config.NONE) +class HardwareKeyboardSubmitEventTest { + private fun enter(deviceId: Int, flags: Int = 0) = KeyEvent( + 0, 0, KeyEvent.ACTION_DOWN, KeyEvent.KEYCODE_ENTER, 0, 0, deviceId, 0, flags + ) + + @Test fun softwareReturnIsNotHardwareSubmit() { + assertFalse(isPhysicalKeyboardEvent(enter(1, KeyEvent.FLAG_SOFT_KEYBOARD))) + assertFalse(isPhysicalKeyboardEvent(enter(KeyCharacterMap.VIRTUAL_KEYBOARD))) + assertFalse(isPhysicalKeyboardEvent(enter( + KeyCharacterMap.VIRTUAL_KEYBOARD, + KeyEvent.FLAG_SOFT_KEYBOARD or KeyEvent.FLAG_KEEP_TOUCH_MODE + ))) + } + + @Test fun physicalReturnRemainsEligible() { + assertTrue(isPhysicalKeyboardEvent(enter(1))) + } +} diff --git a/mobile/packages/expo-hardware-keyboard/ios/ExpoHardwareKeyboard.podspec b/mobile/packages/expo-hardware-keyboard/ios/ExpoHardwareKeyboard.podspec index 8b02c960c53..757aff1f28d 100644 --- a/mobile/packages/expo-hardware-keyboard/ios/ExpoHardwareKeyboard.podspec +++ b/mobile/packages/expo-hardware-keyboard/ios/ExpoHardwareKeyboard.podspec @@ -16,5 +16,6 @@ Pod::Spec.new do |s| s.static_framework = true s.dependency 'ExpoModulesCore' + s.dependency 'ExpoHardwareKeyboardNavigation' s.source_files = '**/*.{h,m,mm,swift}' end diff --git a/mobile/packages/expo-hardware-keyboard/ios/HardwareKeyboardCaptureView.swift b/mobile/packages/expo-hardware-keyboard/ios/HardwareKeyboardCaptureView.swift index e55abec1d56..a31486ec079 100644 --- a/mobile/packages/expo-hardware-keyboard/ios/HardwareKeyboardCaptureView.swift +++ b/mobile/packages/expo-hardware-keyboard/ios/HardwareKeyboardCaptureView.swift @@ -1,4 +1,5 @@ import ExpoModulesCore +import ExpoHardwareKeyboardNavigation import UIKit // Captured keys stay in the focused field's responder chain. @@ -41,11 +42,17 @@ public class HardwareKeyboardCaptureView: ExpoView { command.wantsPriorityOverSystemBehavior = true return [command] } - return Self.terminalKeyCommands + return Self.terminalKeyCommands.filter { + !HardwareKeyboardCommandRegistry.shared.owns($0, in: window) + } } public override func canPerformAction(_ action: Selector, withSender sender: Any?) -> Bool { if action == #selector(handleKeyCommand(_:)) { + if captureMode == "terminal", let command = sender as? UIKeyCommand, + HardwareKeyboardCommandRegistry.shared.owns(command, in: window) { + return false + } guard enabled, let input = focusedTextInput(in: self) else { return false } return input.markedTextRange == nil } @@ -53,6 +60,9 @@ public class HardwareKeyboardCaptureView: ExpoView { } @objc func handleKeyCommand(_ sender: UIKeyCommand) { + if captureMode == "terminal", HardwareKeyboardCommandRegistry.shared.owns(sender, in: window) { + return + } guard enabled, let textInput = focusedTextInput(in: self), textInput.markedTextRange == nil else { return } diff --git a/mobile/src/hardware-keyboard/mobile-hardware-keyboard-registry.test.ts b/mobile/src/hardware-keyboard/mobile-hardware-keyboard-registry.test.ts index 8eb7613e1f5..1fd55223069 100644 --- a/mobile/src/hardware-keyboard/mobile-hardware-keyboard-registry.test.ts +++ b/mobile/src/hardware-keyboard/mobile-hardware-keyboard-registry.test.ts @@ -1,7 +1,9 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' const nativeRuntime = vi.hoisted(() => ({ - setCommands: vi.fn() + setCommands: vi.fn(), + policy: 'terminal-first' as 'terminal-first' | 'orca-first', + preferencesChanged: () => {} })) vi.mock('@orca/expo-hardware-keyboard-navigation', () => ({ @@ -14,10 +16,13 @@ vi.mock('react-native', () => ({ Platform: { OS: 'ios' } })) vi.mock('./mobile-hardware-keyboard-preferences', () => ({ getMobileHardwareKeyboardPreferences: () => ({ loaded: true, - terminalShortcutPolicy: 'terminal-first' + terminalShortcutPolicy: nativeRuntime.policy }), loadMobileHardwareKeyboardPreferences: () => Promise.resolve(), - subscribeMobileHardwareKeyboardPreferences: () => () => undefined + subscribeMobileHardwareKeyboardPreferences: (listener: () => void) => { + nativeRuntime.preferencesChanged = listener + return () => undefined + } })) import { registerMobileHardwareKeyboardScope } from './mobile-hardware-keyboard-registry' @@ -25,6 +30,7 @@ import { registerMobileHardwareKeyboardScope } from './mobile-hardware-keyboard- describe('mobile hardware keyboard registry', () => { beforeEach(() => { nativeRuntime.setCommands.mockClear() + nativeRuntime.policy = 'terminal-first' }) it('lets the newest scope suppress an overlapping action', () => { @@ -50,4 +56,40 @@ describe('mobile hardware keyboard registry', () => { unregisterTerminal() unregisterApp() }) + + it('publishes and releases terminal-overlapping chords when the policy changes', () => { + nativeRuntime.policy = 'orca-first' + const unregister = registerMobileHardwareKeyboardScope({ + actionIds: [ + 'tab.previousRecent', + 'tab.nextTerminal', + 'tab.previousTerminal', + 'tab.nextAllTypes' + ], + context: 'terminal', + handler: vi.fn() + }) + const registered = () => nativeRuntime.setCommands.mock.calls.at(-1)?.[0] ?? [] + expect(registered()).toEqual( + expect.arrayContaining([ + expect.objectContaining({ key: 'Tab', control: true, shift: false }), + expect.objectContaining({ key: 'PageDown', control: true, shift: false }), + expect.objectContaining({ key: 'PageUp', control: true, shift: false }) + ]) + ) + nativeRuntime.policy = 'terminal-first' + nativeRuntime.preferencesChanged() + expect(registered().map((command: { actionId: string }) => command.actionId)).toEqual([ + 'tab.previousRecent', + 'tab.nextTerminal', + 'tab.previousTerminal' + ]) + nativeRuntime.policy = 'orca-first' + nativeRuntime.preferencesChanged() + expect(registered()).toEqual( + expect.arrayContaining([expect.objectContaining({ actionId: 'tab.nextAllTypes' })]) + ) + unregister() + expect(registered()).toEqual([]) + }) })