Skip to content

Conversation

@jeremy-visionaid
Copy link
Contributor

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

Use MapVirtualKey to convert VirtualKey to char

Fixes: #25

@jeremy-visionaid
Copy link
Contributor Author

@davidortinau Maybe you'd like to use CsWin32 to provide the bindings, or know some other way of handling it without hitting the Win32 API directly? Either way, I'm happy to clean it up before merge.

@jeremy-visionaid jeremy-visionaid marked this pull request as ready for review May 28, 2025 22:30
@jeremy-visionaid
Copy link
Contributor Author

I've added CsWin32 as a build dependency on Windows. It's maybe a bit overkill for one function, but it's something others will be reasonably familiar with if more bindings are needed and it saves debate about how to manage p/invokes. I'd understand if it's not wanted though.

@jeremy-visionaid
Copy link
Contributor Author

Didn't take long to find that we do in fact need more bindings 😆 This is essentially a prerequisite for #27

@davidortinau davidortinau requested a review from Copilot May 29, 2025 18:12
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 adds proper Win32 interop to convert VirtualKey codes into characters and refactors Windows-specific key event handling to use these new extensions.

  • Introduce CsWin32 package and config to generate the MapVirtualKey P/Invoke binding
  • Add ToChar and ToKeyPressedEventArgs extension methods to centralize key-to-char conversion
  • Update KeyboardBehavior.Windows and sample to consume the new helpers

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/Plugin.Maui.KeyListener/Plugin.Maui.KeyListener.csproj Add Microsoft.Windows.CsWin32 package reference for Win32 APIs
src/Plugin.Maui.KeyListener/NativeMethods.txt List of native method names (MapVirtualKey) for CsWin32 generation
src/Plugin.Maui.KeyListener/NativeMethods.json CsWin32 configuration (wideCharOnly, emitSingleFile, public=false)
src/Plugin.Maui.KeyListener/KeyboardKeysExtensions.Windows.cs Implement ToChar and ToKeyPressedEventArgs extensions
src/Plugin.Maui.KeyListener/KeyboardBehavior.Windows.cs Refactor key event handlers to use ToKeyPressedEventArgs
samples/Plugin.Maui.KeyListener.Sample/MainPage.xaml.cs Update sample to show KeyChar in output
Comments suppressed due to low confidence (2)

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

  • The new ToKeyPressedEventArgs helper wraps key conversion logic. Consider adding unit tests to verify that Keys and KeyChar are set correctly across a variety of VirtualKey values and modifier states.
internal static KeyPressedEventArgs ToKeyPressedEventArgs(this KeyRoutedEventArgs e)

src/Plugin.Maui.KeyListener/NativeMethods.txt:1

  • [nitpick] It's not clear how NativeMethods.txt is consumed. Consider adding a header or comment explaining its purpose or remove it if it's no longer needed.
MapVirtualKey

_ => 0
};

public static char ToChar(this VirtualKey key) => (char)PInvoke.MapVirtualKey((uint)key, MAP_VIRTUAL_KEY_TYPE.MAPVK_VK_TO_CHAR);
Copy link

Copilot AI May 29, 2025

Choose a reason for hiding this comment

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

Using MapVirtualKey with MAPVK_VK_TO_CHAR ignores modifier keys (Shift, Ctrl) and current keyboard layout/layout-specific dead keys. Consider using ToUnicodeEx to accurately map key codes + modifiers to characters.

Copilot uses AI. Check for mistakes.

using Microsoft.UI.Xaml.Input;
using Windows.System;
using Windows.Win32;
Copy link

Copilot AI May 29, 2025

Choose a reason for hiding this comment

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

[nitpick] Importing the entire Windows.Win32 namespace is broad. You can narrow this to Windows.Win32.UI.Input.KeyboardAndMouse to limit scope and improve readability.

Suggested change
using Windows.Win32;

Copilot uses AI. Check for mistakes.
_ => 0
};

public static char ToChar(this VirtualKey key) => (char)PInvoke.MapVirtualKey((uint)key, MAP_VIRTUAL_KEY_TYPE.MAPVK_VK_TO_CHAR);
Copy link

Copilot AI May 29, 2025

Choose a reason for hiding this comment

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

The newly added ToChar extension lacks XML documentation. Adding a <summary> explaining expected behavior, limitations (e.g., no modifiers), and return value would help maintainers and API consumers.

Copilot uses AI. Check for mistakes.
@davidortinau davidortinau merged commit aaafdaa into davidortinau:main May 29, 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.KeyChar for non-alphanumeric virtual keys on Windows

2 participants