#753 fix: Remapping modifier keys breaks existing modifier key shortc…

…uts so automatically enable the "do not remap" trigger key option for modifier keys
keymapperorg · Jan 31, 2022 · ba6d96b · ba6d96b
1 parent d925152
commit ba6d96b
Show file tree

Hide file tree

Showing 4 changed files with 77 additions and 10 deletions.
diff --git a/app/src/main/java/io/github/sds100/keymapper/mappings/keymaps/ConfigKeyMapUseCase.kt b/app/src/main/java/io/github/sds100/keymapper/mappings/keymaps/ConfigKeyMapUseCase.kt
@@ -62,17 +62,24 @@ class ConfigKeyMapUseCaseImpl(
             }
         }
 
-        val newKeys = trigger.keys.toMutableList().apply {
+        val newKeys = trigger.keys.toMutableList()
 
-            val triggerKey = TriggerKey(
-                keyCode = keyCode,
-                device = device,
-                clickType = clickType
-            )
+        var consumeKeyEvent = true
 
-            add(triggerKey)
+        //Issue #753
+        if (KeyEventUtils.isModifierKey(keyCode)) {
+            consumeKeyEvent = false
         }
 
+        val triggerKey = TriggerKey(
+            keyCode = keyCode,
+            device = device,
+            clickType = clickType,
+            consumeKeyEvent = consumeKeyEvent
+        )
+
+        newKeys.add(triggerKey)
+
         val newMode = when {
             containsKey -> TriggerMode.Sequence
             newKeys.size <= 1 -> TriggerMode.Undefined

diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml
@@ -703,7 +703,7 @@
 
     <string name="flag_hold_down">Hold down</string>
     <string name="flag_hold_down_until_pressed_again">Hold down until pressed again</string>
-    <string name="flag_dont_override_default_action">Do not override default action</string>
+    <string name="flag_dont_override_default_action">Do not remap</string>
     <string name="flag_hold_down_until_swiped_again">Hold down until swiped again</string>
     <string name="flag_trigger_from_other_apps">Allow other apps to trigger this key map</string>
     <!--flags-->

diff --git a/app/src/test/java/io/github/sds100/keymapper/ConfigKeyMapUseCaseTest.kt b/app/src/test/java/io/github/sds100/keymapper/ConfigKeyMapUseCaseTest.kt
@@ -6,6 +6,7 @@ import io.github.sds100.keymapper.actions.TapCoordinateAction
 import io.github.sds100.keymapper.mappings.keymaps.ConfigKeyMapUseCaseImpl
 import io.github.sds100.keymapper.mappings.keymaps.KeyMap
 import io.github.sds100.keymapper.mappings.keymaps.KeyMapAction
+import io.github.sds100.keymapper.mappings.keymaps.trigger.TriggerKeyDevice
 import io.github.sds100.keymapper.system.keyevents.KeyEventUtils
 import io.github.sds100.keymapper.util.State
 import io.github.sds100.keymapper.util.dataOrNull
@@ -48,6 +49,60 @@ class ConfigKeyMapUseCaseTest {
         testDispatcher.cleanupTestCoroutines()
     }
 
+    /**
+     * Issue #753. If a modifier key is used as a trigger then it the
+     * option to not override the default action must be chosen so that the modifier
+     * key can still be used normally.
+     */
+    @Test
+    fun `when add modifier key trigger, enable do not remap option`() =
+        coroutineScope.runBlockingTest {
+            val modifierKeys = setOf(
+                KeyEvent.KEYCODE_SHIFT_LEFT,
+                KeyEvent.KEYCODE_SHIFT_RIGHT,
+                KeyEvent.KEYCODE_ALT_LEFT,
+                KeyEvent.KEYCODE_ALT_RIGHT,
+                KeyEvent.KEYCODE_CTRL_LEFT,
+                KeyEvent.KEYCODE_CTRL_RIGHT,
+                KeyEvent.KEYCODE_META_LEFT,
+                KeyEvent.KEYCODE_META_RIGHT,
+                KeyEvent.KEYCODE_SYM,
+                KeyEvent.KEYCODE_NUM,
+                KeyEvent.KEYCODE_FUNCTION
+            )
+
+            for (modifierKeyCode in modifierKeys) {
+                //GIVEN
+                useCase.mapping.value = State.Data(KeyMap())
+
+                //WHEN
+                useCase.addTriggerKey(modifierKeyCode, TriggerKeyDevice.Internal)
+
+                //THEN
+                val trigger = useCase.mapping.value.dataOrNull()!!.trigger
+
+                assertThat(trigger.keys[0].consumeKeyEvent, `is`(false))
+            }
+        }
+
+    /**
+     * Issue #753.
+     */
+    @Test
+    fun `when add non-modifier key trigger, do ont enable do not remap option`() =
+        coroutineScope.runBlockingTest {
+            //GIVEN
+            useCase.mapping.value = State.Data(KeyMap())
+
+            //WHEN
+            useCase.addTriggerKey(KeyEvent.KEYCODE_A, TriggerKeyDevice.Internal)
+
+            //THEN
+            val trigger = useCase.mapping.value.dataOrNull()!!.trigger
+
+            assertThat(trigger.keys[0].consumeKeyEvent, `is`(true))
+        }
+
     /**
      * issue #593
      */

diff --git a/docs/user-guide/keymaps.md b/docs/user-guide/keymaps.md
@@ -52,13 +52,18 @@ External devices are only listed if they are connected to your Android device wh
 
 This menu pops up if you press the 3 dot :fontawesome-solid-ellipsis-v: button on a key.
 
-#### Do not override default action
+#### Do not remap
 
 --8<-- "trigger-options/dont-override-default-action.md"
 
+This option doesn't remap the trigger key, which means it will continue do its normal thing when clicked as well as your
+action. This will be automatically
+
 #### Click type
 
-This will change the click type for a key in a sequence trigger. A parallel trigger key will not have the click type option because all the keys have the same click type. You will find the buttons to change a parallel trigger's click type above the trigger mode buttons as shown in the image at the top of this Trigger section.
+This will change the click type for a key in a sequence trigger. A parallel trigger key will not have the click type
+option because all the keys have the same click type. You will find the buttons to change a parallel trigger's click
+type above the trigger mode buttons as shown in the image at the top of this Trigger section.
 
 ## Customising actions