Skip to content

Commit 6bd75d2

Browse files
authored
fix(settings): say why a shortcut with only Option or Shift is refused (#2462)
Claude-Session: https://claude.ai/code/session_014MXT7tDsb7Rsm1HhBnwhSj Signed-off-by: Ngô Quốc Đạt <datlechin@gmail.com>
1 parent cc17e34 commit 6bd75d2

5 files changed

Lines changed: 49 additions & 3 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
2222

2323
- A data grid shortcut bound to `Cmd+G` silently taking the key from Find Next, with no conflict warning.
2424
- Stale cells after fitting, hiding or reordering a data grid column on a result narrower than the window. (#2446)
25+
- A shortcut recorded with only Option or Shift beeping with no reason given, against docs that said it would work.
2526
- A rebound shortcut silently killing Quit, Minimize, Hide, Settings, Show Toolbar or Enter Full Screen.
2627
- `Ctrl+Cmd+J` accepted in Settings > Keyboard while the SQL editor kept it for Jump to Definition.
2728
- Autocomplete keeping an earlier prefix's ordering after the typed word becomes an exact match. (#2444)

TablePro/Views/Settings/KeyboardSettingsView.swift

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ struct KeyboardSettingsView: View {
132132
needsModifierAlert = nil
133133
}
134134
} message: {
135-
Text(String(localized: "This action needs a modifier key like ⌘ or ⌥. A plain key won't reach the menu reliably."))
135+
Text(String(localized: "This action needs ⌘ or ⌃. Option and Shift can join them, but cannot hold a shortcut on their own."))
136136
}
137137
.onAppear {
138138
SystemHotkeyChecker.shared.reload()
@@ -169,6 +169,9 @@ struct KeyboardSettingsView: View {
169169
},
170170
onClear: {
171171
settings.clearShortcut(for: action)
172+
},
173+
onUnusableModifiers: {
174+
needsModifierAlert = action
172175
}
173176
)
174177
.frame(width: 160, height: 24)

TablePro/Views/Settings/ShortcutRecorderView.swift

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,10 @@ final class ShortcutRecorderNSView: NSView {
1919
/// Callback when the shortcut is cleared (Delete key while recording)
2020
var onClear: (() -> Void)?
2121

22+
/// Callback when the captured combo carries no modifier the menu can use, so the
23+
/// caller can say why instead of leaving the recorder to beep with no explanation.
24+
var onUnusableModifiers: (() -> Void)?
25+
2226
/// The currently displayed key combo
2327
var currentCombo: BoundKey? {
2428
didSet {
@@ -149,7 +153,8 @@ final class ShortcutRecorderNSView: NSView {
149153
return nil
150154
}
151155
guard let combo = BoundKey(from: event) else {
152-
NSSound.beep()
156+
endRecording()
157+
onUnusableModifiers?()
153158
return nil
154159
}
155160
onRecord?(combo)
@@ -290,6 +295,9 @@ struct ShortcutRecorderView: NSViewRepresentable {
290295
/// Called when the shortcut is cleared
291296
var onClear: (() -> Void)?
292297

298+
/// Called when the pressed combo carries no modifier a menu key equivalent can use.
299+
var onUnusableModifiers: (() -> Void)?
300+
293301
func makeNSView(context: Context) -> ShortcutRecorderNSView {
294302
let view = ShortcutRecorderNSView()
295303
view.currentCombo = combo
@@ -299,12 +307,16 @@ struct ShortcutRecorderView: NSViewRepresentable {
299307
view.onClear = {
300308
onClear?()
301309
}
310+
view.onUnusableModifiers = {
311+
onUnusableModifiers?()
312+
}
302313
return view
303314
}
304315

305316
func updateNSView(_ nsView: ShortcutRecorderNSView, context: Context) {
306317
nsView.currentCombo = combo
307318
nsView.onRecord = { [onRecord] newCombo in onRecord?(newCombo) }
308319
nsView.onClear = { [onClear] in onClear?() }
320+
nsView.onUnusableModifiers = { [onUnusableModifiers] in onUnusableModifiers?() }
309321
}
310322
}

TableProTests/Models/KeyboardShortcutTests.swift

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
// and migration from the legacy character-string storage.
88
//
99

10+
import AppKit
1011
import Foundation
1112
@testable import TablePro
1213
import Testing
@@ -181,6 +182,35 @@ struct BareKeyValidationTests {
181182
#expect(!ShortcutAction.executeQuery.allowsBareKey)
182183
}
183184

185+
/// The recorder tells the user a shortcut needs Command or Control, so the capture rule has
186+
/// to be exactly that. Option and Shift only ever qualify a combo, they never carry one, and
187+
/// a letter key held with either alone is a text-input keystroke rather than a shortcut.
188+
@Test("Option and Shift cannot hold a shortcut without Command or Control")
189+
func optionAndShiftAloneAreNotRecordable() {
190+
func capture(_ flags: NSEvent.ModifierFlags) -> BoundKey? {
191+
let event = NSEvent.keyEvent(
192+
with: .keyDown,
193+
location: .zero,
194+
modifierFlags: flags,
195+
timestamp: 0,
196+
windowNumber: 0,
197+
context: nil,
198+
characters: "e",
199+
charactersIgnoringModifiers: "e",
200+
isARepeat: false,
201+
keyCode: 14
202+
)
203+
return event.flatMap(BoundKey.init(from:))
204+
}
205+
206+
#expect(capture(.option) == nil)
207+
#expect(capture(.shift) == nil)
208+
#expect(capture([.option, .shift]) == nil)
209+
#expect(capture(.command) != nil)
210+
#expect(capture(.control) != nil)
211+
#expect(capture([.command, .option]) != nil)
212+
}
213+
184214
@Test("hasModifier reflects the combo")
185215
func hasModifierReflectsCombo() {
186216
#expect(BoundKey.character("r", command: true).hasModifier)

docs/features/keyboard-shortcuts.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -270,7 +270,7 @@ Open **Settings > Keyboard** (`Cmd+,`). Each action has a recorder field: click
270270

271271
A combination already taken by another action in the same context raises a dialog naming it: **Cancel** keeps the existing assignment, **Reassign** moves the shortcut and clears it from the other action. Editor and grid actions can share a key without conflict.
272272

273-
Menu actions need a modifier (`Cmd`, `Option`, `Ctrl`, or `Shift`); function keys `F1` through `F12` work bare, as do grid actions that read the key directly, like Preview FK Reference (`Space`).
273+
Menu actions need `Cmd` or `Ctrl`, with `Option` and `Shift` available alongside them but not on their own; function keys `F1` through `F12` work bare, as do grid actions that read the key directly, like Preview FK Reference (`Space`).
274274

275275
<Info>
276276
Some shortcuts cannot be reassigned: editor built-ins such as `Cmd+/`, tab selection (`Cmd+1` through `Cmd+9`), text size (`Cmd+=`, `Cmd+-`), and macOS system shortcuts, which are read live from System Settings. The recorder warns if you try.

0 commit comments

Comments
 (0)