Use PhysicalKey for shortcut Oem* keys on non-US layouts (#11082)#11100
Merged
Conversation
Avalonia derives Key.Oem* values from the produced character mapped against a US-keyboard table. On a Swedish layout Shift+. produces ':', which Avalonia reports as Key.OemSemicolon (since ':' is shift+; on US), and Shift+, also reports as OemSemicolon. Two distinct physical keys end up sharing a token and one shortcut silently overwrites the other. The Swedish '<' next to Z hits the same path and shows up as OemComma. Fall back to the layout-independent PhysicalKey whenever Key.ToString starts with "Oem". Existing user shortcut files use the old Oem* tokens, so add a one-shot migration that rewrites the SeShortCut.Keys list in place — the modern names then get persisted on the next save. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes shortcut-key collisions on non‑US keyboard layouts (e.g., Swedish on macOS) caused by Avalonia deriving Key.Oem* values from produced characters using a US layout mapping. The fix makes shortcut tokens stable and layout-independent by using PhysicalKey names for Oem* keys, and includes a migration so existing user shortcut JSON continues to work.
Changes:
- Update
ShortcutManager.GetShortcutKeyNameto returnPhysicalKey.ToString()forOem*keys, avoiding layout-based collisions. - Add legacy migration (
MigrateLegacyOemKeys) to rewrite storedOem*tokens to the newPhysicalKey-based tokens when constructingShortCut. - Add UI tests covering the Swedish collision cases and legacy token migration.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/UI/Logic/ShortcutManagerTests.cs | Adds regression tests for Oem* key collisions on non‑US layouts and tests for legacy token migration. |
| src/ui/Logic/ShortcutManager.cs | Switches Oem* token generation to PhysicalKey and adds a legacy Oem* → physical-key token migration map. |
| src/ui/Logic/Shortcut.cs | Runs legacy Oem* migration when ShortCut instances are constructed so existing saved shortcuts keep working. |
Comment on lines
+113
to
+122
| // as OemComma for the same reason. Fall back to the layout-independent | ||
| // PhysicalKey so each physical key gets a unique, stable token. | ||
| if (key.ToString().StartsWith("Oem", StringComparison.Ordinal)) | ||
| { | ||
| return physicalKey.ToString(); | ||
| } | ||
|
|
||
| return key.ToString(); | ||
| } | ||
|
|
The shortcut picker dropdown was built from Avalonia's Key enum and listed legacy Oem* names (OemPeriod, OemComma, ...) — after the PhysicalKey switch in GetShortcutKeyName, the capture path emits "Period", "Comma", "IntlBackslash" etc., so the dropdown could not produce a value that matched the captured token. Drop the Oem* entries and append the physical-key punctuation tokens that capture emits. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Shift+.andShift+,both detect asOemSemicolonbecause Avalonia derivesKey.Oem*from the produced character against a US-keyboard table. The two physical keys collide on one token and the second-bound shortcut silently wins. The Swedish<next toZlands on the same path and reports asOemComma.Key.ToString()starts withOem, fall back toPhysicalKey.ToString()inShortcutManager.GetShortcutKeyName. PhysicalKey is the layout-independent physical position — each physical key now gets a unique, stable token.Oem*tokens.MigrateLegacyOemKeysrewrites theSeShortCut.Keyslist in place when aShortCutis constructed, so old bindings keep working and the modern names get persisted on the next save.Closes #11082.
Test plan
Shift+.vsShift+,produce different tokens), the<mis-report case, and the legacy migration mapping.Shift+Ctrl+.to one action andShift+Ctrl+,to another — both should fire independently.OemPeriod/OemCommasaved shortcuts should still trigger after upgrade.🤖 Generated with Claude Code