Shortcut Helper - Filter shortcut commands containing unsupported keys
Different keyboards have different keys, and some might not have the
keys specified by a shortcut command.
+ Refactors the repository to process all shortcut group sources as one
list
Test: ShortcutHelperCategoriesRepositoryTest
Flag: com.android.systemui.keyboard_shortcut_helper_rewrite
Fixes: 353894416
Change-Id: I4af5b80f64d36dd40095a287a032629b9a70c82a
diff --git a/packages/SystemUI/src/com/android/systemui/keyboard/shortcut/data/repository/ShortcutHelperCategoriesRepository.kt b/packages/SystemUI/src/com/android/systemui/keyboard/shortcut/data/repository/ShortcutHelperCategoriesRepository.kt
index 495e8f3..85bd0b0 100644
--- a/packages/SystemUI/src/com/android/systemui/keyboard/shortcut/data/repository/ShortcutHelperCategoriesRepository.kt
+++ b/packages/SystemUI/src/com/android/systemui/keyboard/shortcut/data/repository/ShortcutHelperCategoriesRepository.kt
@@ -71,6 +71,35 @@
stateRepository: ShortcutHelperStateRepository
) {
+ private val sources =
+ listOf(
+ InternalGroupsSource(
+ source = systemShortcutsSource,
+ isTrusted = true,
+ typeProvider = { System }
+ ),
+ InternalGroupsSource(
+ source = multitaskingShortcutsSource,
+ isTrusted = true,
+ typeProvider = { MultiTasking }
+ ),
+ InternalGroupsSource(
+ source = appCategoriesShortcutsSource,
+ isTrusted = true,
+ typeProvider = { AppCategories }
+ ),
+ InternalGroupsSource(
+ source = inputShortcutsSource,
+ isTrusted = false,
+ typeProvider = { InputMethodEditor }
+ ),
+ InternalGroupsSource(
+ source = currentAppShortcutsSource,
+ isTrusted = false,
+ typeProvider = { groups -> getCurrentAppShortcutCategoryType(groups) }
+ ),
+ )
+
private val activeInputDevice =
stateRepository.state.map {
if (it is Active) {
@@ -82,17 +111,20 @@
val categories: Flow<List<ShortcutCategory>> =
activeInputDevice
- .map {
- if (it == null) {
+ .map { inputDevice ->
+ if (inputDevice == null) {
return@map emptyList()
}
- return@map listOfNotNull(
- fetchSystemShortcuts(it),
- fetchMultiTaskingShortcuts(it),
- fetchAppCategoriesShortcuts(it),
- fetchImeShortcuts(it),
- fetchCurrentAppShortcuts(it),
- )
+ val groupsFromAllSources = sources.map { it.source.shortcutGroups(inputDevice.id) }
+ val supportedKeyCodes = fetchSupportedKeyCodes(inputDevice.id, groupsFromAllSources)
+ return@map sources.mapIndexedNotNull { index, internalGroupsSource ->
+ fetchShortcutCategory(
+ internalGroupsSource,
+ groupsFromAllSources[index],
+ inputDevice,
+ supportedKeyCodes,
+ )
+ }
}
.stateIn(
scope = backgroundScope,
@@ -100,49 +132,22 @@
initialValue = emptyList(),
)
- private suspend fun fetchSystemShortcuts(inputDevice: InputDevice) =
- toShortcutCategory(
- inputDevice.keyCharacterMap,
- System,
- systemShortcutsSource.shortcutGroups(inputDevice.id),
- keepIcons = true,
- )
-
- private suspend fun fetchMultiTaskingShortcuts(inputDevice: InputDevice) =
- toShortcutCategory(
- inputDevice.keyCharacterMap,
- MultiTasking,
- multitaskingShortcutsSource.shortcutGroups(inputDevice.id),
- keepIcons = true,
- )
-
- private suspend fun fetchAppCategoriesShortcuts(inputDevice: InputDevice) =
- toShortcutCategory(
- inputDevice.keyCharacterMap,
- AppCategories,
- appCategoriesShortcutsSource.shortcutGroups(inputDevice.id),
- keepIcons = true,
- )
-
- private suspend fun fetchImeShortcuts(inputDevice: InputDevice) =
- toShortcutCategory(
- inputDevice.keyCharacterMap,
- InputMethodEditor,
- inputShortcutsSource.shortcutGroups(inputDevice.id),
- keepIcons = false,
- )
-
- private suspend fun fetchCurrentAppShortcuts(inputDevice: InputDevice): ShortcutCategory? {
- val shortcutGroups = currentAppShortcutsSource.shortcutGroups(inputDevice.id)
- val categoryType = getCurrentAppShortcutCategoryType(shortcutGroups)
- return if (categoryType == null) {
+ private fun fetchShortcutCategory(
+ internalGroupsSource: InternalGroupsSource,
+ groups: List<KeyboardShortcutGroup>,
+ inputDevice: InputDevice,
+ supportedKeyCodes: Set<Int>,
+ ): ShortcutCategory? {
+ val type = internalGroupsSource.typeProvider(groups)
+ return if (type == null) {
null
} else {
toShortcutCategory(
inputDevice.keyCharacterMap,
- categoryType,
- shortcutGroups,
- keepIcons = false
+ type,
+ groups,
+ internalGroupsSource.isTrusted,
+ supportedKeyCodes,
)
}
}
@@ -162,13 +167,19 @@
type: ShortcutCategoryType,
shortcutGroups: List<KeyboardShortcutGroup>,
keepIcons: Boolean,
+ supportedKeyCodes: Set<Int>,
): ShortcutCategory? {
val subCategories =
shortcutGroups
.map { shortcutGroup ->
ShortcutSubCategory(
shortcutGroup.label.toString(),
- toShortcuts(keyCharacterMap, shortcutGroup.items, keepIcons)
+ toShortcuts(
+ keyCharacterMap,
+ shortcutGroup.items,
+ keepIcons,
+ supportedKeyCodes,
+ )
)
}
.filter { it.shortcuts.isNotEmpty() }
@@ -184,7 +195,15 @@
keyCharacterMap: KeyCharacterMap,
infoList: List<KeyboardShortcutInfo>,
keepIcons: Boolean,
- ) = infoList.mapNotNull { toShortcut(keyCharacterMap, it, keepIcons) }
+ supportedKeyCodes: Set<Int>,
+ ) =
+ infoList
+ .filter {
+ // Allow KEYCODE_UNKNOWN (0) because shortcuts can have just modifiers and no
+ // keycode, or they could have a baseCharacter instead of a keycode.
+ it.keycode == KeyEvent.KEYCODE_UNKNOWN || supportedKeyCodes.contains(it.keycode)
+ }
+ .mapNotNull { toShortcut(keyCharacterMap, it, keepIcons) }
private fun toShortcut(
keyCharacterMap: KeyCharacterMap,
@@ -268,6 +287,29 @@
return null
}
+ private suspend fun fetchSupportedKeyCodes(
+ deviceId: Int,
+ groupsFromAllSources: List<List<KeyboardShortcutGroup>>
+ ): Set<Int> =
+ withContext(backgroundDispatcher) {
+ val allUsedKeyCodes =
+ groupsFromAllSources
+ .flatMap { groups -> groups.flatMap { group -> group.items } }
+ .map { info -> info.keycode }
+ .distinct()
+ val keyCodesSupported =
+ inputManager.deviceHasKeys(deviceId, allUsedKeyCodes.toIntArray())
+ return@withContext allUsedKeyCodes
+ .filterIndexed { index, _ -> keyCodesSupported[index] }
+ .toSet()
+ }
+
+ private class InternalGroupsSource(
+ val source: KeyboardShortcutGroupsSource,
+ val isTrusted: Boolean,
+ val typeProvider: (groups: List<KeyboardShortcutGroup>) -> ShortcutCategoryType?,
+ )
+
companion object {
private const val TAG = "SHCategoriesRepo"
diff --git a/packages/SystemUI/tests/src/com/android/systemui/keyboard/shortcut/data/repository/ShortcutHelperCategoriesRepositoryTest.kt b/packages/SystemUI/tests/src/com/android/systemui/keyboard/shortcut/data/repository/ShortcutHelperCategoriesRepositoryTest.kt
index 14837f2..6e883c2 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/keyboard/shortcut/data/repository/ShortcutHelperCategoriesRepositoryTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/keyboard/shortcut/data/repository/ShortcutHelperCategoriesRepositoryTest.kt
@@ -16,12 +16,28 @@
package com.android.systemui.keyboard.shortcut.data.repository
+import android.hardware.input.fakeInputManager
+import android.view.KeyEvent.KEYCODE_A
+import android.view.KeyEvent.KEYCODE_B
+import android.view.KeyEvent.KEYCODE_C
+import android.view.KeyEvent.KEYCODE_D
+import android.view.KeyEvent.KEYCODE_E
+import android.view.KeyEvent.KEYCODE_F
+import android.view.KeyEvent.KEYCODE_G
+import android.view.KeyboardShortcutGroup
+import android.view.KeyboardShortcutInfo
import androidx.test.ext.junit.runners.AndroidJUnit4
import androidx.test.filters.SmallTest
import com.android.systemui.SysuiTestCase
import com.android.systemui.coroutines.collectLastValue
import com.android.systemui.keyboard.shortcut.data.source.FakeKeyboardShortcutGroupsSource
import com.android.systemui.keyboard.shortcut.data.source.TestShortcuts
+import com.android.systemui.keyboard.shortcut.shared.model.Shortcut
+import com.android.systemui.keyboard.shortcut.shared.model.ShortcutCategory
+import com.android.systemui.keyboard.shortcut.shared.model.ShortcutCategoryType
+import com.android.systemui.keyboard.shortcut.shared.model.ShortcutCommand
+import com.android.systemui.keyboard.shortcut.shared.model.ShortcutKey
+import com.android.systemui.keyboard.shortcut.shared.model.ShortcutSubCategory
import com.android.systemui.keyboard.shortcut.shortcutHelperAppCategoriesShortcutsSource
import com.android.systemui.keyboard.shortcut.shortcutHelperCategoriesRepository
import com.android.systemui.keyboard.shortcut.shortcutHelperCurrentAppShortcutsSource
@@ -47,13 +63,14 @@
private val fakeSystemSource = FakeKeyboardShortcutGroupsSource()
private val fakeMultiTaskingSource = FakeKeyboardShortcutGroupsSource()
+ private val fakeAppCategoriesSource = FakeKeyboardShortcutGroupsSource()
private val kosmos =
testKosmos().also {
it.testDispatcher = UnconfinedTestDispatcher()
it.shortcutHelperSystemShortcutsSource = fakeSystemSource
it.shortcutHelperMultiTaskingShortcutsSource = fakeMultiTaskingSource
- it.shortcutHelperAppCategoriesShortcutsSource = FakeKeyboardShortcutGroupsSource()
+ it.shortcutHelperAppCategoriesShortcutsSource = fakeAppCategoriesSource
it.shortcutHelperInputShortcutsSource = FakeKeyboardShortcutGroupsSource()
it.shortcutHelperCurrentAppShortcutsSource = FakeKeyboardShortcutGroupsSource()
}
@@ -61,6 +78,7 @@
private val repo = kosmos.shortcutHelperCategoriesRepository
private val helper = kosmos.shortcutHelperTestHelper
private val testScope = kosmos.testScope
+ private val fakeInputManager = kosmos.fakeInputManager
@Before
fun setUp() {
@@ -87,4 +105,74 @@
// though fetching shortcuts again would have returned a new result.
assertThat(secondCategories).isEqualTo(firstCategories)
}
+
+ @Test
+ fun categories_filtersShortcutsWithUnsupportedKeyCodes() =
+ testScope.runTest {
+ fakeSystemSource.setGroups(
+ listOf(
+ simpleGroup(
+ simpleShortcutInfo(KEYCODE_A),
+ simpleShortcutInfo(KEYCODE_B),
+ ),
+ simpleGroup(
+ simpleShortcutInfo(KEYCODE_C),
+ ),
+ )
+ )
+ fakeMultiTaskingSource.setGroups(
+ listOf(
+ simpleGroup(
+ simpleShortcutInfo(KEYCODE_D),
+ ),
+ simpleGroup(
+ simpleShortcutInfo(KEYCODE_E),
+ simpleShortcutInfo(KEYCODE_F),
+ ),
+ )
+ )
+ fakeAppCategoriesSource.setGroups(listOf(simpleGroup(simpleShortcutInfo(KEYCODE_G))))
+
+ fakeInputManager.removeKeysFromKeyboard(deviceId = 123, KEYCODE_A, KEYCODE_D, KEYCODE_G)
+ helper.toggle(deviceId = 123)
+
+ val categories by collectLastValue(repo.categories)
+ assertThat(categories)
+ .containsExactly(
+ ShortcutCategory(
+ ShortcutCategoryType.System,
+ listOf(
+ simpleSubCategory(simpleShortcut("B")),
+ simpleSubCategory(simpleShortcut("C")),
+ )
+ ),
+ ShortcutCategory(
+ ShortcutCategoryType.MultiTasking,
+ listOf(
+ simpleSubCategory(
+ simpleShortcut("E"),
+ simpleShortcut("F"),
+ ),
+ )
+ ),
+ )
+ }
+
+ private fun simpleSubCategory(vararg shortcuts: Shortcut) =
+ ShortcutSubCategory(simpleGroupLabel, shortcuts.asList())
+
+ private fun simpleShortcut(vararg keys: String) =
+ Shortcut(
+ label = simpleShortcutLabel,
+ commands = listOf(ShortcutCommand(keys.map { ShortcutKey.Text(it) }))
+ )
+
+ private fun simpleGroup(vararg shortcuts: KeyboardShortcutInfo) =
+ KeyboardShortcutGroup(simpleGroupLabel, shortcuts.asList())
+
+ private fun simpleShortcutInfo(keyCode: Int = 0) =
+ KeyboardShortcutInfo(simpleShortcutLabel, keyCode, /* modifiers= */ 0)
+
+ private val simpleShortcutLabel = "shortcut label"
+ private val simpleGroupLabel = "group label"
}
diff --git a/packages/SystemUI/tests/utils/src/android/hardware/input/FakeInputManager.kt b/packages/SystemUI/tests/utils/src/android/hardware/input/FakeInputManager.kt
index c4f93d1..6e7c05c 100644
--- a/packages/SystemUI/tests/utils/src/android/hardware/input/FakeInputManager.kt
+++ b/packages/SystemUI/tests/utils/src/android/hardware/input/FakeInputManager.kt
@@ -19,6 +19,8 @@
import android.view.InputDevice
import android.view.KeyCharacterMap
import android.view.KeyCharacterMap.VIRTUAL_KEYBOARD
+import android.view.KeyEvent
+import com.android.systemui.util.mockito.any
import com.android.systemui.util.mockito.mock
import com.android.systemui.util.mockito.whenever
import org.mockito.ArgumentMatchers.anyInt
@@ -38,6 +40,12 @@
.build()
private val devices = mutableMapOf<Int, InputDevice>(VIRTUAL_KEYBOARD to virtualKeyboard)
+ private val allKeyCodes = (0..KeyEvent.MAX_KEYCODE)
+ private val supportedKeyCodesByDeviceId =
+ mutableMapOf(
+ // Mark all keys supported by default
+ VIRTUAL_KEYBOARD to allKeyCodes.toMutableSet()
+ )
val inputManager =
mock<InputManager> {
@@ -61,13 +69,31 @@
whenever(enableInputDevice(anyInt())).thenAnswer { invocation ->
setDeviceEnabled(invocation, enabled = true)
}
+ whenever(deviceHasKeys(any(), any())).thenAnswer { invocation ->
+ val deviceId = invocation.arguments[0] as Int
+ val keyCodes = invocation.arguments[1] as IntArray
+ val supportedKeyCodes = supportedKeyCodesByDeviceId[deviceId]!!
+ return@thenAnswer keyCodes.map { supportedKeyCodes.contains(it) }.toBooleanArray()
+ }
}
+ fun addPhysicalKeyboardIfNotPresent(deviceId: Int, enabled: Boolean = true) {
+ if (devices.containsKey(deviceId)) {
+ return
+ }
+ addPhysicalKeyboard(deviceId, enabled)
+ }
+
fun addPhysicalKeyboard(id: Int, enabled: Boolean = true) {
check(id > 0) { "Physical keyboard ids have to be > 0" }
addKeyboard(id, enabled)
}
+ fun removeKeysFromKeyboard(deviceId: Int, vararg keyCodes: Int) {
+ addPhysicalKeyboardIfNotPresent(deviceId)
+ supportedKeyCodesByDeviceId[deviceId]!!.removeAll(keyCodes.asList())
+ }
+
private fun addKeyboard(id: Int, enabled: Boolean = true) {
devices[id] =
InputDevice.Builder()
@@ -77,6 +103,7 @@
.setEnabled(enabled)
.setKeyCharacterMap(keyCharacterMap)
.build()
+ supportedKeyCodesByDeviceId[id] = allKeyCodes.toMutableSet()
}
private fun InputDevice.copy(
diff --git a/packages/SystemUI/tests/utils/src/com/android/systemui/keyboard/shortcut/data/repository/ShortcutHelperTestHelper.kt b/packages/SystemUI/tests/utils/src/com/android/systemui/keyboard/shortcut/data/repository/ShortcutHelperTestHelper.kt
index 6ca5cd8..8b45662 100644
--- a/packages/SystemUI/tests/utils/src/com/android/systemui/keyboard/shortcut/data/repository/ShortcutHelperTestHelper.kt
+++ b/packages/SystemUI/tests/utils/src/com/android/systemui/keyboard/shortcut/data/repository/ShortcutHelperTestHelper.kt
@@ -95,7 +95,7 @@
}
fun toggle(deviceId: Int) {
- fakeInputManager.addPhysicalKeyboard(deviceId)
+ fakeInputManager.addPhysicalKeyboardIfNotPresent(deviceId)
fakeCommandQueue.doForEachCallback { it.toggleKeyboardShortcutsMenu(deviceId) }
}