iTwin / appui

Monorepo for iTwin.js AppUi
MIT License
8 stars 2 forks source link

Keyboard shorcuts are called 2 times. And Control key flag is ignored. #876

Closed mathieu-fournier closed 1 week ago

mathieu-fournier commented 1 month ago

Describe the bug

Hi o/,

Issue

I have a shortcut liked to a letter. p Ex: image

When I press p, the execute function is called 2 times. I expect it to be called only one time.

An example of how this can be problematic. In my case, the shortcut opens a Dialog, which gets opened 2 times, one on top of each other...

Debug + fix :

I tracked where execute is called from, one is in accudraw code and the other one in UIFramework code : image

The accudraw code ignores all other checks. Ex : (isCtrlKeyRequired, isFocusOnHome). So, when I do ctrl + p, my shortcut is called from there. It should be ignored, like the UIFramework codes do.

I suggest to remove the whole accudraw handleKeyDown function and handle these shortcuts properly with the Framework.

Thanks. (:

To Reproduce

No response

Expected Behavior

No response

Screenshots

No response

Desktop (please complete the applicable information)

No response

Additional context

No response

GerardasB commented 1 month ago

I can not reproduce the described behavior of a shortcut executing twice. I assume it's due to the fact that keydown event of input is only emitted when the input is focused, but then isFocusOnHome getter returns false in FrameworkToolAdmin.

mathieu-fournier commented 4 weeks ago

Do you have accudraw / accusnap activated in your test app ?

mathieu-fournier commented 4 weeks ago

Steps to reproduce for the double call : 1 - Create a shortcut that is active when a element is selected (the shortcut is active when focus is not on home). 2 - Have accudraw active also. 3 - Press the shortcut. (accudraw and framework both fires up).

Steps to reproduce the ignored Control key : 1 - Create a shortcut that has isCtrlKeyRequired: true. 2 - Have accudraw activated. 3 - Press the shortcut without pressing ctrl. 4 - Notice that the shortcut fires up, it shouldnt have.

In both cases, accudraw code triggers the shortcut and it should not.

mathieu-fournier commented 4 weeks ago

Realted to : https://github.com/iTwin/appui/issues/819