Update the input mapping system to not dispatch both a matched KeyDown and KeyDownNoRepeat

[Keavon]

For example, if we update input_mapping.rs like so:

- entry!(KeyDown(KeyA); action_dispatch=ToolMessage::ActivateToolPath),
+ entry!(KeyDownNoRepeat(KeyA); action_dispatch=ToolMessage::ActivateToolPath),

Then CtrlA will fire both the selection of all layers...

entry!(KeyDown(KeyA); modifiers=[Accel], action_dispatch=DocumentMessage::SelectAllLayers),

...as well as the change to the Path tool, since one gets matched on KeyDown and another on KeyDownNoRepeat. If both were KeyDown or KeyDownNoRepeat, only one would match. But currently, both are matching, and we only want one to match.

After implementing this fix, the following mappings should be changed from KeyDown to KeyDownNoRepeat:

Mappings to change

// TransformLayerMessage
entry!(KeyDown(KeyX); action_dispatch=TransformLayerMessage::ConstrainX),
entry!(KeyDown(KeyY); action_dispatch=TransformLayerMessage::ConstrainY),

// PathToolMessage
entry!(KeyDown(KeyG); action_dispatch=PathToolMessage::GRS { key: KeyG }),
entry!(KeyDown(KeyR); action_dispatch=PathToolMessage::GRS { key: KeyR }),
entry!(KeyDown(KeyS); action_dispatch=PathToolMessage::GRS { key: KeyS }),

// PenToolMessage
entry!(KeyDown(KeyG); action_dispatch=PenToolMessage::GRS { grab: KeyG, rotate: KeyR, scale: KeyS }),
entry!(KeyDown(KeyR); action_dispatch=PenToolMessage::GRS { grab: KeyG, rotate: KeyR, scale: KeyS }),
entry!(KeyDown(KeyS); action_dispatch=PenToolMessage::GRS { grab: KeyG, rotate: KeyR, scale: KeyS }),

// ToolMessage
entry!(KeyDown(KeyV); action_dispatch=ToolMessage::ActivateToolSelect),
entry!(KeyDown(KeyZ); action_dispatch=ToolMessage::ActivateToolNavigate),
entry!(KeyDown(KeyI); action_dispatch=ToolMessage::ActivateToolEyedropper),
entry!(KeyDown(KeyT); action_dispatch=ToolMessage::ActivateToolText),
entry!(KeyDown(KeyF); action_dispatch=ToolMessage::ActivateToolFill),
entry!(KeyDown(KeyH); action_dispatch=ToolMessage::ActivateToolGradient),
entry!(KeyDown(KeyA); action_dispatch=ToolMessage::ActivateToolPath),
entry!(KeyDown(KeyP); action_dispatch=ToolMessage::ActivateToolPen),
entry!(KeyDown(KeyN); action_dispatch=ToolMessage::ActivateToolFreehand),
entry!(KeyDown(KeyL); action_dispatch=ToolMessage::ActivateToolLine),
entry!(KeyDown(KeyM); action_dispatch=ToolMessage::ActivateToolRectangle),
entry!(KeyDown(KeyE); action_dispatch=ToolMessage::ActivateToolEllipse),
entry!(KeyDown(KeyY); action_dispatch=ToolMessage::ActivateToolPolygon),
entry!(KeyDown(KeyB); action_dispatch=ToolMessage::ActivateToolBrush),

// DocumentMessage
entry!(KeyDown(Space); modifiers=[Control], action_dispatch=DocumentMessage::GraphViewOverlayToggle),
entry!(KeyDown(KeyO); modifiers=[Alt], action_dispatch=DocumentMessage::ToggleOverlaysVisibility),
entry!(KeyDown(KeyS); modifiers=[Alt], action_dispatch=DocumentMessage::ToggleSnapping),
entry!(KeyDown(KeyL); modifiers=[Accel], action_dispatch=DocumentMessage::ToggleSelectedLocked),
entry!(KeyDown(KeyG); modifiers=[Alt], action_dispatch=DocumentMessage::ToggleGridVisibility),

// TransformLayerMessage
entry!(KeyDown(KeyG); action_dispatch=TransformLayerMessage::BeginGrab),
entry!(KeyDown(KeyR); action_dispatch=TransformLayerMessage::BeginRotate),
entry!(KeyDown(KeyS); action_dispatch=TransformLayerMessage::BeginScale),

// PortfolioMessage
entry!(KeyDown(KeyO); modifiers=[Accel], action_dispatch=PortfolioMessage::OpenDocument),
entry!(KeyDown(KeyI); modifiers=[Accel], action_dispatch=PortfolioMessage::Import),
entry!(KeyDown(KeyR); modifiers=[Alt], action_dispatch=PortfolioMessage::ToggleRulers),

// DialogMessage
entry!(KeyDown(KeyE); modifiers=[Accel], action_dispatch=DialogMessage::RequestExportDialog),
entry!(KeyDown(KeyN); modifiers=[Accel], action_dispatch=DialogMessage::RequestNewDocumentDialog),
entry!(KeyDown(Comma); modifiers=[Accel], action_dispatch=DialogMessage::RequestPreferencesDialog),

[tarunprabhu11]

Can I have a go at this?

[Keavon]

You're welcome to give it a try, sure. Just be aware that it is likely only something that can be successfully completed by someone with a good level of software engineering experience, since it's pretty tricky. I put a few hours into it and gave up. But if you're clever and have been programming for a while, you can probably figure it out better than I was able to.

[tarunprabhu11]

Hi @Keavon , just needed a few clarifications

• currently using crtl+a withentry!(KeyDownNoRepeat(KeyA); action_dispatch=ToolMessage::ActivateToolPath),will fire change to path tool and will not fire the selection of all layers. (can work on this)
• using ctrl+a with entry!(KeyDownNoRepeat(KeyA); modifiers=[Accel], action_dispatch=DocumentMessage::SelectAllLayers), will fire both actions.
  The other action remains KeyDown in both cases.
  Can I please get a recheck on the requirements.

[Keavon]

I don't understand what you've tested. Are you saying you changed KeyDown to KeyDownNoRepeat for both of your two makings you included in your comment, then attempted to use Ctrl+A? What is the outcome of that?

[tarunprabhu11]

When the change to path tool action is keydownnorepeat and the select all layers is keydown using ctrl+a would result in change to path tool only.
When the path tool is keydown and select all layers is keydownnorepeat using ctrl+a would result in both change to path tool and select all layers.

[tarunprabhu11]

Since it's a bit different from what's mentioned in the issue, I just wanted to reconfirm the desired working.

[Keavon]

It sounds like you're identifying examples of how this is broken. And yes, we know it's broken, so the goal here is to fix it. We want it to treat both as co-equals, so the existing manner in which a higher-precedence key supersedes a lower-precedence key works whether they are KeyDown or KeyDownNoRepeat or some mixture; instead of requiring they all be of the same type.