Skip to content

Add project tab-navigation + Focus Sidebar key bindings#154

Open
aryeh wants to merge 4 commits into
thdxg:mainfrom
aryeh:additional_key_bindings
Open

Add project tab-navigation + Focus Sidebar key bindings#154
aryeh wants to merge 4 commits into
thdxg:mainfrom
aryeh:additional_key_bindings

Conversation

@aryeh

@aryeh aryeh commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

What

Add Key Bindings to

  • Rearrange tab higher in project

  • Rearrange tab lower in project

  • Select previous tab in project

  • Select next tab in project

  • Focus and Open SideBar temporarily until new tab is selected, or ESC defocuses SideBar

  • Allow navigating SideBar with arrow keys

  • Prevent active tab from grabbing focus when a tab in the SideBar is selected/highlighted but not formally chosen

Why

Allow for keyboard driven reorganizing and managing tabs.

Closes #113

How

Verified

  • mise run format, mise run lint, and mise run test all pass
  • Built and ran the change in the app (mise run run) and confirmed the behavior
  • Added or updated tests for new model / persistence / palette / hotkey logic
  • Checked by testing app

Notes for reviewers

@github-actions github-actions Bot added area:ui Views, Settings UI area:state AppState, models, persistence area:hotkeys Key routing, hotkeys area:tests Test changes labels Jul 6, 2026
@aryeh aryeh changed the title Add project tab-navigation + Focus Sidebar key bindings and off-scree… Add project tab-navigation + Focus Sidebar key bindings Jul 6, 2026
@aryeh aryeh closed this Jul 6, 2026
@aryeh aryeh reopened this Jul 6, 2026
@thdxg

thdxg commented Jul 7, 2026

Copy link
Copy Markdown
Owner

Nice work — the wrapping move semantics and the ring/focus-mode split are clean, and the test coverage is solid (Workspace moves, the sidebar ring, and the arrow-key modifier regression test all pull their weight). A few notes:

1. HotkeyShortcut.matches duplicates comparableModifierMask (Macterm/App/Hotkeys.swift)

let relevant: NSEvent.ModifierFlags = [.command, .control, .option, .shift]
return event.modifierFlags.intersection(relevant) == modifiers

This is the same set as HotkeyRegistry.comparableModifierMask, which already documents why .function/.numericPad are excluded. Since that constant already masked those out, this inline copy is a behavioral no-op — the actual guard is the new matches_arrow_with_function_and_numericPad_flags test (👍). Suggest using the named constant so the two definitions can't drift:

return event.modifierFlags.intersection(HotkeyRegistry.comparableModifierMask) == modifiers

2. The if let mainWindow guard change is the one real behavioral change to existing routing (Macterm/App/Responders.swift)

if let mainWindow, let keyWindow = NSApp.keyWindow, keyWindow !== mainWindow, 

When mainWindow is nil the old code entered this block and restricted which hotkeys fired; now it's skipped and all routed hotkeys proceed. The rationale in the comment makes sense, but since this affects every keybind (not just the new ones) during the pre-wired-window window, could you confirm nothing relied on the old swallow-when-nil behavior across app-launch / window-teardown timing?

3. Two MainWindow changes look unrelated to keybindings — intentional?

  • navigationSplitViewColumnWidth(min: 140, ideal: 180, max: .infinity) (was max: 280) — lets the sidebar be dragged arbitrarily wide.
  • Folding columnVisibility into a computed binding drops the previous withAnimation, so toggling the sidebar (Cmd+) now snaps rather than animates.

Both may be deliberate, just flagging since they're outside the stated scope.

4. sidebarArrowSwitch may stick at list boundaries. In click-into-list mode, the key responder sets sidebarArrowSwitch = true and passes the arrow through; the flag is only cleared in onChange(of: selection). A native List doesn't wrap, so an arrow at the top/bottom row won't change selection and the flag stays set — the next tab click would then be treated as an arrow switch and skip restoreFocusToActivePane(), leaving focus in the sidebar. Worth a quick check.

Minor: making saveLayout/applyLayout rebindable (defaulting to none) is a reasonable addition but is scope beyond the tab-nav work — fine to keep, just noting it for the squash subject.

@aryeh

aryeh commented Jul 7, 2026

Copy link
Copy Markdown
Contributor Author

I will need to pick through these when I have a moment to spare, to remember exactly what was done why.

Removing the sidebar max-width was added and I forgot to document it. The sidebar hides the long named items and it is difficult to differentiate one similar tab from another..

The original sidebar and activeTab would fignt against keyboard navigating the project and tab list. The moment a tab in the sidebar was highlighted the activeTab would switch and grab focus preventing you from actually keeping the sidebar+tab list in focus and navigating it. That was where a log this comes from. The project list and tab list in the sidebar was not a first-class tool that allows navigation by keyboard, it was a presentation of the tabs and projects that only allows a single select/click and that was it, and then it was defocused and you are thrown into the activeTab.

Much of my extra code over simply adding key bindings was fighting the sidebar that wasn't first class item that allows true interaction.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:hotkeys Key routing, hotkeys area:state AppState, models, persistence area:tests Test changes area:ui Views, Settings UI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Some MacTerm UI Improvements

2 participants