Extend KeyDownTriggerBehavior with Modifiers and ability to handle already handled events

[Irina-Konovalova]

Fixes

This PR extends KeyDownTriggerBehavior.

PR Type

What kind of change does this PR introduce?

• Other... Please describe: Extend KeyDownTriggerBehavior with Modifiers and ability to handle already handled events.

What is the current behavior?

KeyDownTriggerBehavior does not support modifier keys or PreviewKeyDown, which limits its use in keyboard navigation scenarios.

What is the new behavior?

This PR extends KeyDownTriggerBehavior by adding support for modifier keys, PreviewKeyDown, and handled events. It also updates the XML documentation.

PR Checklist

Please check if your PR fulfills the following requirements:

• Created a feature/dev branch in your fork (vs. submitting directly from a commit on main)
• Based off latest main branch of toolkit
• Tested code with current supported SDKs
• New component
  • Documentation has been added
  • Sample in sample app has been added
  • Analyzers are passing for documentation and samples
  • Icon has been created (if new sample) following the Thumbnail Style Guide and templates
• Tests for the changes have been added (if applicable)
• Header has been added to all new source files
• Contains NO breaking changes

Other information

[Irina-Konovalova]

@dotnet-policy-service agree company="ECIERGE SOFTWARE DEV INC"

[Avid29]

I think this is a great PR. I love the additional functionality, and I appreciate the coding style such as using guard clauses. However, I dislike the repeated calls to InputKeyboardSource.GetKeyStateForCurrentThread. I do love small purposeful helper functions, but not at the expense of ideal behavior. Let's see about moving the state check into CheckModifiers and inlining IsDown inside Match using the state retrieved in CheckModifiers.

There's also a somewhat trivial code styling question spawned by this PR regarding the ordering of class members. Hoping to get a definitive answer on that soon.

[Avid29]

I pulled it down and tested it this time. Works great, just missing some namespace references in order to compile, but it also introduces a breaking change😬! I left a comment for how we can address that.

[Avid29]

Excellent. Looks good to me.

I'll pass it up the tree for a second maintainer to take a look at, after which we can get it merged!

[Avid29]

Hmm. The CI is giving compilation issues.

I had some issues formatting my missing namespace suggestion, and it looks like it indeed resulted in duplicate usings. There's also issues with an inaccessible API? This may be an Uno bug. I'll look into it properly when I get the chance.

[Irina-Konovalova]

Thanks for checking!
I removed the duplicate using.
I’ll wait for your follow‑up once you have a chance to look into the inaccessible API issue.

[Avid29]

So, the issue is that the InputKeyboardSource is a WinUI 3 API.

The ideal solution is to find an equivalent approach which works in WinUI 2, then maybe use conditional compilation to swap the implementation depending on if the existing approach is more ideal in WinUI 3.

As a backup, we can wrap all the new code in WinUI 3 conditional blocks and just make the modifiers part of the WinUI 3 API.

[Avid29]

This should fix the WinUI 2 issues

[Avid29]

Great! That's everything from me.

Note to self, we should add samples demonstrating the new functionality, but I can take care of that myself once this is merged.

[Arlodotexe]

One question for posterity and one naming suggestion to add from my end.

[Arlodotexe]

The docs for UIElement.PreviewKeyDownEvent describe it as:

Occurs when a keyboard key is pressed while the UIElement has focus.

This event uses the tunneling routing strategy. The corresponding bubbling event is KeyDown.

The docs for UIElement.KeyDown also describe:

Occurs when a keyboard key is pressed while the UIElement has focus.

The question is, why PreviewKeyDownEvent and not the KeyDown that we used before? both are routed event handlers, they both give you KeyRoutedEventArgs in the handler.

If they're behaviorally different, which do we need? If they're the same behaviorally, would just one be fine? Do we need both?

[Arlodotexe]

Maybe could use a better name for CheckModifierKeys, given its function. Check sounds more like a verb/method than a property value. Something like ModifiersEnabled might be more appropriate, given the existing property Modifiers that this gates the functionality of.