Skip to content

Fix numpad vs non-numpad key ambiguity in shortcut binding #10576

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
ivandrofly wants to merge 1 commit into
base: main
Choose a base branch
from
+22 −7
Draft

Fix numpad vs non-numpad key ambiguity in shortcut binding #10576

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions src/UI/Features/Options/Shortcuts/GetKeyViewModel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
using Avalonia.Input;
using CommunityToolkit.Mvvm.ComponentModel;
using CommunityToolkit.Mvvm.Input;
using Nikse.SubtitleEdit.Logic;
using Nikse.SubtitleEdit.Logic.Config;

namespace Nikse.SubtitleEdit.Features.Options.Shortcuts;
Expand Down Expand Up @@ -82,8 +83,9 @@ Key.LeftAlt or Key.RightAlt or
var shiftLabel = isMac ? Se.Language.Options.Shortcuts.ShiftMac : Se.Language.Options.Shortcuts.Shift;
var winLabel = isMac ? Se.Language.Options.Shortcuts.WinMac : Se.Language.Options.Shortcuts.Win;

PressedKey = e.Key.ToString();
PressedKeyOnly = PressedKey;
var effectiveKey = ShortcutManager.GetEffectiveKeyName(e.Key, e.PhysicalKey);
PressedKey = effectiveKey;
PressedKeyOnly = effectiveKey;
IsControlPressed = e.KeyModifiers.HasFlag(KeyModifiers.Control);
IsAltPressed = e.KeyModifiers.HasFlag(KeyModifiers.Alt);
IsShiftPressed = e.KeyModifiers.HasFlag(KeyModifiers.Shift);
Expand Down Expand Up @@ -119,8 +121,8 @@ Key.LeftAlt or Key.RightAlt or
infoText += winLabel + " + ";
}

PressedKey += e.Key;
infoText += e.Key;
PressedKey += effectiveKey;
infoText += effectiveKey;

InfoText = string.Format(Se.Language.Options.Shortcuts.PressedKeyX, infoText);
}
Expand Down
19 changes: 16 additions & 3 deletions src/UI/Logic/ShortcutManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,21 @@ namespace Nikse.SubtitleEdit.Logic;
public class ShortcutManager : IShortcutManager
{
private readonly HashSet<Key> _activeKeys = [];
private readonly HashSet<string> _activeEffectiveNames = [];
private readonly List<ShortCut> _shortcuts = [];
private FrozenDictionary<string, ShortCut>? _lookupTable;
private bool _isDirty = true;
private bool _isControlPressed = false;
private bool _isShiftPressed = false;

// When NumLock is off, numpad navigation keys (e.g. NumPad7) produce the same logical Key as
// their non-numpad counterparts (Key.Home). Use the PhysicalKey name to keep them distinct.
public static string GetEffectiveKeyName(Key logicalKey, PhysicalKey physicalKey)
{
var physicalName = physicalKey.ToString();
return physicalName.StartsWith("NumPad", StringComparison.Ordinal) ? physicalName : logicalKey.ToString();
}

public static string GetKeyDisplayName(string key)
{
bool isMac = OperatingSystem.IsMacOS();
Expand Down Expand Up @@ -51,6 +60,7 @@ Key.LeftAlt or Key.RightAlt or
Key.LWin or Key.RWin))
{
_activeKeys.Add(e.Key);
_activeEffectiveNames.Add(GetEffectiveKeyName(e.Key, e.PhysicalKey));
}

_isControlPressed = e.KeyModifiers.HasFlag(KeyModifiers.Control);
Expand All @@ -63,10 +73,12 @@ public void OnKeyReleased(object? sender, KeyEventArgs e)
Key.ImeAccept or Key.ImeModeChange or Key.DeadCharProcessed or Key.None)
{
_activeKeys.Clear();
_activeEffectiveNames.Clear();
}
else
{
_activeKeys.Remove(e.Key);
_activeEffectiveNames.Remove(GetEffectiveKeyName(e.Key, e.PhysicalKey));
}

_isControlPressed = e.KeyModifiers.HasFlag(KeyModifiers.Control);
Expand All @@ -76,6 +88,7 @@ public void OnKeyReleased(object? sender, KeyEventArgs e)
public void ClearKeys()
{
_activeKeys.Clear();
_activeEffectiveNames.Clear();
_isControlPressed = false;
_isShiftPressed = false;
}
Expand Down Expand Up @@ -125,11 +138,11 @@ private void RebuildLookupTable()
}

// Build the current state key list with initial capacity
var currentInputKeys = new List<string>(_activeKeys.Count + 2);
var currentInputKeys = new List<string>(_activeEffectiveNames.Count + 2);

foreach (var key in _activeKeys)
foreach (var name in _activeEffectiveNames)
{
currentInputKeys.Add(key.ToString());
currentInputKeys.Add(name);
}

// Add normalized modifiers based on the event state
Expand Down