microsoft / XamlBehaviorsWpf

Home for WPF XAML Behaviors on GitHub.
MIT License
839 stars 138 forks source link

Keytriggers with modifier invoke the action of the keytrigger with the same key and modifier set to None #162

Closed salvatorefrancostats closed 4 months ago

salvatorefrancostats commented 6 months ago

Describe the bug Keytriggers with modifier invoke the action of the keytrigger with the same key and modifier set to none

To Reproduce Steps to reproduce the behavior:

  1. Create a WPF Application
  2. Create MainWindow
  3. Add a keytrigger with Key="Up"
  4. Add an InvokeCommandAction and bind Command1
  5. Add a keytrigger with Key="Up" and Modifiers="Shift"
  6. Add an InvokeCommandAction and bind Command2
  7. Add breakpoints on both Command1 and Command2
  8. Run the application
  9. Press Shift+Up on the keyboard
  10. Both commands will be fired

Expected behavior Only Command2 will be fired

Screenshots Xaml Behaviors KeyTrigger Issue

Desktop (please complete the following information):

Reproduction Link https://github.com/microsoft/XamlBehaviorsWpf/blob/ca52233144a8969c96c1e911483830a9b3c1d43a/src/Microsoft.Xaml.Behaviors/Input/KeyTrigger.cs#L77

This check allow keytrigger with modifiers set to none to invoke action also if there is a specific keytrigger for the pressed keys

gianlucacuccostatsperform commented 5 months ago

@brianlagunas did you had a chance to check this issue ? we might help to fix?

brianlagunas commented 5 months ago

No, I haven't even looked at this yet. My day job has been keeping me extremely busy.

I'm not sure if this is by design, or an oversight in the original implementation.

gianlucacuccostatsperform commented 5 months ago

@brianlagunas thanks for your answer and for your time! from what we saw in the code we can't evaluate that and we can't understand why that has been changed the only think where we can confirm is this version is not supporting anymore 2 commands witch the first one just the fine the key and a second one that define the same key with the modifier because both will be then triggered. we highly use that in our applications and that is what it make us stuck to 1.1.39

brianlagunas commented 5 months ago

Looking at the history, that fix was added to address issue #77 .

If you have time, a PR would be great. We added tests for the original fix, so as long as those tests don't break, we should be okay. Also, we will need test for any additional changes in that area of the code. It would have been nice to have a test for this scenario so we could have seen the behavior change.

gianlucacuccostatsperform commented 4 months ago

@brianlagunas we found those 2 different behavior can't be used at the same time, modifiers can't be used as modifier and as key at same time, we have produce the following pr to keep both behavior and define witch one to apply with a flag UseModifiersAsKeys https://github.com/microsoft/XamlBehaviorsWpf/pull/166/commits/b724af2c16cb30813f318077e23f9aac6781e947

brianlagunas commented 4 months ago

@gianlucacuccostatsperform I just submitted a PR for this fix.

gianlucacuccostatsperform commented 4 months ago

@brianlagunas thanks for your time and your effort, i tested and it do perfectly work!