Skip to content

Conversation

@jeremy-visionaid
Copy link
Contributor

@jeremy-visionaid jeremy-visionaid commented May 27, 2025

Using flags for KeyboardKeys is not valid since there are too many distinct keys.

Fixes: #19

@jeremy-visionaid
Copy link
Contributor Author

@davidortinau This is a somewhat minimal change to move towards getting things working. Other problems remain - it works OK for iOS and Windows, and improves but not fixes the behaviour for MacCatalyst.

@davidortinau davidortinau requested a review from Copilot May 28, 2025 15:11
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR replaces flag-based key handling with direct single-key mappings to support a larger set of distinct keys.

  • Iterates actual key usages from events instead of flag combinations.
  • Introduces a flat KeyboardKeys enum and direct extension methods for platform key conversions.
  • Updates triggers and markup extensions to use single-key properties.

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/Plugin.Maui.KeyListener/UIPressesData.cs Iterate over presses set directly instead of event flags.
src/Plugin.Maui.KeyListener/KeyboardPageViewController.cs Loop per key usage and assign KeyPressedEventArgs for each key.
src/Plugin.Maui.KeyListener/KeyboardMarkupExtensions.cs Simplify parsing to a single KeyboardKeys value or None.
src/Plugin.Maui.KeyListener/KeyboardKeysExtensions.iOS.cs Remove flag-based mapping; add ToUIKeyboardHidUsage/ToKeyboardKeys.
src/Plugin.Maui.KeyListener/KeyboardKeysExtensions.Windows.cs Remove flag-based mapping; add ToVirtualKey/ToKeyboardKeys.
src/Plugin.Maui.KeyListener/KeyboardKeys.cs Add new flat KeyboardKeys enum listing all keys.
src/Plugin.Maui.KeyListener/KeyboardFlags.cs Remove flag-based KeyboardKeys; keep only KeyboardModifiers.
src/Plugin.Maui.KeyListener/KeyboardBehaviorTrigger.iOS.cs Change to single PlatformKey and adjust equality/hash logic.
src/Plugin.Maui.KeyListener/KeyboardBehaviorTrigger.cs Rename Keys to Key property and wire single-key logic.
Comments suppressed due to low confidence (2)

src/Plugin.Maui.KeyListener/KeyboardKeysExtensions.iOS.cs:8

  • Consider adding unit tests for this mapping extension to verify each KeyboardKeys value is correctly converted to UIKeyboardHidUsage.
public static UIKeyboardHidUsage ToUIKeyboardHidUsage(this KeyboardKeys keyboardKey) => keyboardKey switch

src/Plugin.Maui.KeyListener/KeyboardKeysExtensions.Windows.cs:9

  • Add unit tests covering all cases in this switch to ensure every KeyboardKeys value is mapped correctly to VirtualKey.
public static VirtualKey ToVirtualKey(KeyboardKeys keyboardKey) => keyboardKey switch

{
Modifiers = modifiers,
Keys = key.ToKeyboardKeys(),
KeyChar = key.ToString().FirstOrDefault(),
Copy link

Copilot AI May 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using key.ToString().FirstOrDefault() derives the character from the enum name rather than the actual pressed character. Consider storing and using the original UIKey.CharactersIgnoringModifiers value in UIPressesData for accurate character reporting.

Suggested change
KeyChar = key.ToString().FirstOrDefault(),
KeyChar = (key as UIKey)?.CharactersIgnoringModifiers?.FirstOrDefault() ?? '\0',

Copilot uses AI. Check for mistakes.
{
int hash = 17;
hash = hash * 23 + obj.Keys.GetHashCode();
hash = hash * 23 + obj.Key.GetHashCode();
Copy link

Copilot AI May 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Equals implementation compares PlatformKey, but the hash uses Key. To maintain the hash–equals contract, use obj.PlatformKey.GetHashCode() here.

Suggested change
hash = hash * 23 + obj.Key.GetHashCode();
hash = hash * 23 + obj.PlatformKey.GetHashCode();

Copilot uses AI. Check for mistakes.
@davidortinau davidortinau merged commit 94d0f78 into davidortinau:main May 28, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Incorrect values for KeyPressedEventArgs.Keys when pressing multiple keys

2 participants