Skip to content

chore(upstream): PR5 hotkeys — v1 directional pane focus + Cmd+Alt+Arrow restoration (#3460 #3472)#180

Merged
MocA-Love merged 3 commits intomainfrom
upstream-merge/pr5-hotkeys
Apr 15, 2026
Merged

chore(upstream): PR5 hotkeys — v1 directional pane focus + Cmd+Alt+Arrow restoration (#3460 #3472)#180
MocA-Love merged 3 commits intomainfrom
upstream-merge/pr5-hotkeys

Conversation

@MocA-Love
Copy link
Copy Markdown
Owner

概要

12コミット取り込み最後のPR。方針はユーザー明示指示で upstream に合わせる、ただし fork 固有の BROWSER_RELOAD / BROWSER_HARD_RELOAD は温存します。

取り込む upstream コミット

# Commit 内容
1 #3460 2bf1049a45cf73af1c feat(desktop/hotkeys): v1 directional pane focus + best-effort v1 override migrator
2 #3472 1294a7df6f7729b4bd feat(desktop/hotkeys): restore Cmd+Alt+Arrow for tab/workspace nav

方針変更: fork の Cmd+Alt+Arrow 扱い

fork は過去 PR #3 で upstream superset-sh#3403 の Cmd+Alt+Arrow → FOCUS_PANE_* を積極採用し、さらに独自調整 (ce0fb6134 post-cherry-pick fixes for spatial pane navigation) まで入れていました。今回の upstream superset-sh#3472 はこれを default から外し、Cmd+Alt+Arrow は PREV/NEXT_TAB/WORKSPACE に戻す方針転換をしています。

fork を upstream の最終判断 (Cmd+Alt+Arrow = tab/workspace nav) に合わせます。FOCUS_PANE_* は registry に残り unbound 扱いなので、pane focus を使いたいユーザーは Settings → Keyboard から任意のキーに再バインドできます。

変更後のキーバインド (上流一致)

Hotkey ID macOS Windows / Linux
PREV_TAB meta+alt+left ctrl+shift+alt+left
NEXT_TAB meta+alt+right ctrl+shift+alt+right
PREV_WORKSPACE meta+alt+up ctrl+shift+alt+up
NEXT_WORKSPACE meta+alt+down ctrl+shift+alt+down
FOCUS_PANE_{LEFT,RIGHT,UP,DOWN} null (unbound) null (unbound)
SCROLL_TO_BOTTOM meta+shift+down ctrl+end (was ctrl+shift+alt+down)

SCROLL_TO_BOTTOM Windows/Linux を ctrl+shift+alt+downctrl+end に移動したのは、復元された NEXT_WORKSPACE と同じ chord になるのを避けるため (upstream superset-sh#3472)。

維持される fork 固有 hotkeys

Hotkey ID macOS Windows / Linux
BROWSER_RELOAD meta+alt+r ctrl+shift+r
BROWSER_HARD_RELOAD meta+shift+alt+r ctrl+alt+shift+e

どれも上記新 defaults と canonical chord レベルで衝突しません (Codex で検証済み)。

手動 conflict 解決 (3ファイル)

1. apps/desktop/src/renderer/hotkeys/migrate.ts

fork は localStorage に既に hotkey-overrides がある場合に早期 return して tRPC からの stale 上書きを防いでいましたが、upstream superset-sh#3460-v2 marker bump でそのレイヤーこそ再サニタイズさせたい層でした。

最終解決: 既存 hotkey-overrides がある場合は parse して各 override を sanitizeOverride (新 detectUSLayout 経由) にかけて re-sanitize して書き戻す。tRPC からの stale 上書きは発生させない fork 意図を維持しつつ、upstream の sanitizer も走るように。

2. apps/desktop/src/renderer/routes/_authenticated/_dashboard/workspace/$workspaceId/page.tsx

fork の extractPaneIdsFromLayout import と upstream 新規 type FocusDirection import を両方取り込み。moveFocusDirectional + useHotkey("FOCUS_PANE_*") 4 つを v1 workspace に追加 (upstream superset-sh#3460)。fork 固有の tRPC-based prev/next workspace コメントは温存。

3. apps/desktop/src/renderer/routes/_authenticated/settings/keyboard/page.tsx

CATEGORY_ORDER に upstream の "Navigation" と fork の "Browser" を両方含めた (順序: Navigation, Browser, Workspace, Terminal, Layout, Window, Help)。

Codex 事前レビュー 2ラウンド

Round 1 (2件指摘, 両方修正):

  1. High: migrate.tsexistingRaw 早期 return が -v2 sanitizer を bypass → 上記「feat(desktop): add Excel/spreadsheet viewer with diff support #1 最終解決」で対応
  2. Medium: v1 FOCUS_PANE_* 4 つに enabled: isActive が無く、KeepAliveWorkspaces で mounted な非アクティブ workspace でも hotkey fire → 4 つ全てに { enabled: isActive } を追加

Round 2: Yes。コード上の blocker は解消済み。

テスト

  • bun install
  • bun run lint (biome pass、check-git-ref-strings pass、pre-existing projects.ts simple-git 違反のみ残存)
  • bun run typecheck: /tmp worktree の @types/node 解決問題、CI 側に委ねる
  • 実機スモーク:
    • Cmd+Alt+Arrow で v1/v2 両方の workspace が prev/next tab/workspace navigation
    • BROWSER_RELOAD / BROWSER_HARD_RELOAD が壊れていない
    • v1 workspace で FOCUS_PANE_* を Settings から再バインドして pane focus 動作
    • hotkey override migration (マーカー -v2) が既存 localStorage を re-sanitize する

次のステップ

これで 12 コミットの取り込みが完了します。PR5 マージ後、必要なら最終的な audit merge (-s ours) で upstream との同期点をマークできます。

saddlepaddle and others added 3 commits April 16, 2026 00:06
…rride migrator (superset-sh#3460)

* feat(desktop/hotkeys): directional pane focus for v1 + Mac alt modifier

Restores keyboard pane navigation to v1 workspaces with the same
Cmd+Alt+Arrow chords v2 uses, via a standalone spatial neighbor walker
over v1's MosaicNode<string> tree (no cross-package coupling to v2).
Also lets the recorder accept alt-only chords on Mac and warns when
bound alt-only chords could mask typing special characters.

* feat(desktop/hotkeys): best-effort migrator for v1 event.key overrides

The v1 recorder stored chords from event.key (layout-dependent raw
glyphs like "meta+,", "ctrl+shift+@", "meta+alt+ª"), while the new
store matches on event.code names. Extends sanitizeOverride with a
token rewrite pre-pass covering US-ANSI punctuation, shifted glyphs,
and macOS Option dead-keys; gates the Option dead-key table behind a
navigator.keyboard-based US-layout probe so non-US Mac users aren't
silently rebound to the wrong physical key.
…uperset-sh#3472)

Flip Cmd+Alt+Arrow (mac) / Ctrl+Shift+Alt+Arrow (win/linux) back to
the pre-superset-sh#3403 behavior:

- Cmd+Alt+Left / Right → Previous / Next Tab
- Cmd+Alt+Up / Down   → Previous / Next Workspace

FOCUS_PANE_{LEFT,RIGHT,UP,DOWN} remain registered but are now
unbound by default — users who want directional pane focus can
rebind them in Settings → Keyboard. PREV_TAB_ALT / NEXT_TAB_ALT
(Ctrl+Tab / Ctrl+Shift+Tab) are unchanged.

Also move SCROLL_TO_BOTTOM on Windows/Linux from ctrl+shift+alt+down
to ctrl+end to avoid the silent collision with NEXT_WORKSPACE that
this restoration would otherwise introduce. buildRegisteredAppChords
uses a Map keyed by canonical chord, so duplicate chords silently
clobber each other in the reverse lookup. mac SCROLL_TO_BOTTOM
(meta+shift+down) is unchanged.

Registered users who previously overrode any of these IDs keep their
overrides — defaults are just fallbacks.
Two blocking issues Codex found on the conflict resolution:

1. apps/desktop/src/renderer/hotkeys/migrate.ts — the fork guard that
   short-circuited when localStorage already had a hotkey-overrides entry
   bypassed upstream superset-sh#3460's new sanitizeOverride / detectUSLayout logic.
   The whole point of the `-v2` marker bump is to re-sanitize existing
   localStorage overrides that a pre-sanitizer build wrote with corrupt
   values, so skipping them defeats the migration.

   Restructure: instead of an early return, parse the existing
   localStorage overrides, re-run them through sanitizeOverridesMap with
   the US-layout probe, and write the cleaned map back. Still avoids
   overwriting with stale tRPC data (the fork's original intent).

2. apps/desktop/src/renderer/routes/_authenticated/_dashboard/workspace/
   $workspaceId/page.tsx — the v1 FOCUS_PANE_{LEFT,RIGHT,UP,DOWN}
   hotkeys were missing `enabled: isActive`. Every other useHotkey in
   this file already has that guard because KeepAliveWorkspaces keeps
   inactive WorkspacePage instances mounted; without the guard, a user
   who rebinds the FOCUS_PANE_* ids would see the hotkey fire in hidden
   background workspaces as well.
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 15, 2026

Warning

Rate limit exceeded

@MocA-Love has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 1 minutes and 35 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 1 minutes and 35 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: dfc78acb-ebf7-40f5-87fd-fdb50e137fa8

📥 Commits

Reviewing files that changed from the base of the PR and between 6a41325 and 0b51f2d.

⛔ Files ignored due to path filters (1)
  • bun.lock is excluded by !**/*.lock
📒 Files selected for processing (9)
  • apps/desktop/src/renderer/hotkeys/hooks/useRecordHotkeys/useRecordHotkeys.ts
  • apps/desktop/src/renderer/hotkeys/migrate.ts
  • apps/desktop/src/renderer/hotkeys/registry.ts
  • apps/desktop/src/renderer/hotkeys/utils/detectUSLayout.ts
  • apps/desktop/src/renderer/hotkeys/utils/overrideSanitizer.test.ts
  • apps/desktop/src/renderer/hotkeys/utils/sanitizeOverride.ts
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/workspace/$workspaceId/page.tsx
  • apps/desktop/src/renderer/routes/_authenticated/settings/keyboard/page.tsx
  • apps/desktop/src/renderer/stores/tabs/utils.ts
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch upstream-merge/pr5-hotkeys

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@MocA-Love MocA-Love merged commit c9418eb into main Apr 15, 2026
14 checks passed
MocA-Love added a commit that referenced this pull request Apr 15, 2026
-s ours merge to record that upstream commits a3e34bf through
de70163 (13 commits) are semantically already present on origin/main
via the PR1-6 cherry-pick series (PRs #176, #177, #178, #179, #180,
#182), plus fork-adaptation fixes layered on top.

This merge target is de70163 specifically (not upstream/main) so
newer upstream commits (9fff075 and later) remain visible in future
behind counts.

Upstream commits covered by this audit merge:
- a3e34bf  fix(desktop): restore cmd+click requirement for v1 terminal file links (superset-sh#3457)  [PR1/#176]
- 57557f8  fix(desktop): gate v2 workspace children on collection readiness (superset-sh#3464)       [PR1/#176]
- 4ee2e61  fix(desktop): use native clipboard for copy path in v2 sidebar (superset-sh#3462)         [PR1/#176]
- 87d6e93  feat(desktop): close settings with Escape key (superset-sh#3466)                          [PR1/#176]
- 9c7f5f4  chore(desktop): auto-restart host-service on bundle change in dev (superset-sh#3461)      [PR1/#176]
- 93140d9  fix(mcp): accept MCP resource URL as valid OAuth audience (superset-sh#3459)              [PR2/#177]
- be9e000  fix(desktop): drive tray menu off events, fetch real org name (superset-sh#3458)          [PR2/#177]
- c5f791e  feat(v2): unify workspace delete through host-service (superset-sh#3443)                  [PR3/#178]
- 2c24d93  feat(desktop): paginated branch picker with checkout + open actions (superset-sh#3397)    [PR4/#179]
- 2bf1049  feat(desktop/hotkeys): v1 directional pane focus + best-effort v1 override migrator (superset-sh#3460)  [PR5/#180]
- 1294a7d  feat(desktop/hotkeys): restore Cmd+Alt+Arrow for tab/workspace nav (superset-sh#3472)    [PR5/#180]
- de70163  feat(desktop): v2 review tab first pass — PR info, checks, comments (superset-sh#3463)    [PR6/#182]

Intentionally skipped (version bump, fork has independent versioning):
- 1e23353  chore(desktop): bump version to 1.5.5 (superset-sh#3473)

Fork-adaptation fixes layered on top of the cherry-picks:
- PR1: host-service-coordinator alias import fix, settings Escape
       selector narrowing (role-based + popper wrapper), Escape
       close uses replace navigation
- PR2: dual quit mode preservation (requestQuit "release"/"stop"),
       trayUpdateToken guard for stale async fetchHostInfo results
- PR4: ChangesHeader.normalizeBranchName regex rewrite (lint false
       positive), worktree add uses fullRef for remote-tracking
       refs, syncTimedOut reset on pendingId change, GIT_REFS.md
       barrel example fix
- PR5: migrate.ts re-sanitize of existing localStorage overrides
       (v2 marker bump intent), FOCUS_PANE_* enabled:isActive for
       KeepAliveWorkspaces, CATEGORY_ORDER merges Navigation (upstream)
       and Browser (fork)
- PR6: normalizeThreadsToComments flattens all thread.comments (not
       just first), CommentPane overrides <a> (openUrl) and <img>
       (SafeImage), zero-badge suppression, pr-null comments gate

Fork features verified intact (Explore agent audit of combined
36d4de4..35d95f3 range):
- BROWSER_RELOAD / BROWSER_HARD_RELOAD hotkeys
- dual quit mode menu in tray
- v1 terminal cold-restore + retry reconnect (out of range but
  unaffected)
- KeepAliveWorkspaces (FOCUS_PANE_* gated on isActive)
- useCommandPalette + addMemoTab in v2 workspace
- host-service-coordinator rename alias pattern
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.

2 participants