fix(settings): toggle settings shortcut

[janburzinski]

summary

toggle settings with cmd,

[greptile-apps]

Greptile Summary

This PR makes the settings view toggleable (open and close) with Cmd+,, where previously the shortcut only opened it. It extracts a toggleSettingsView helper, tracks lastNonSettingsView in NavigationStore so closing settings returns to the prior context, and adds an Escape handler in AppKeyboardShortcuts for closing settings when no modal is open.

• settings-toggle.ts — new singleton helper with a 150 ms dedup guard to prevent the macOS menu accelerator and renderer hotkey from double-firing on a single Cmd+, press.
• NavigationStore — gains lastNonSettingsView (updated on every non-settings navigation) and a small fix to navigate() that uses stored view params when none are supplied, preserving context on settings close.
• AppKeyboardShortcuts — adds a closeModal-key handler that navigates back from settings when no modal is blocking, wired to the same lastNonSettingsView.

Confidence Score: 4/5

Safe to merge for most sessions, but quitting while settings is open causes the close-settings navigation to land on 'home' instead of the user's prior view on the next launch.

lastNonSettingsView is tracked in the store but never written to or read from the persisted NavigationSnapshot. If the app is quit while in settings, the restored session starts in settings with lastNonSettingsView defaulting to 'home', and the first Cmd+, or Escape press silently drops the user's actual prior context. Everything else — the dedup guard, the Escape handler, the store tracking during a live session — is correct.

src/renderer/lib/stores/navigation-store.ts — lastNonSettingsView must be added to the snapshot getter and NavigationSnapshot type to survive session restore.

Important Files Changed

Filename	Overview
src/renderer/lib/stores/navigation-store.ts	Adds lastNonSettingsView tracking and fixes history params on navigate; the field is correctly updated in _applyNavigation and restoreSnapshot, but it is not included in the snapshot getter or NavigationSnapshot type, so it is lost when the app quits while settings is the active view.
src/renderer/lib/layout/settings-toggle.ts	New helper that centralises the open/close toggle logic with a 150 ms dedup guard to prevent double-fire from the macOS menu accelerator and the renderer hotkey; logic is correct for the current two callers.
src/renderer/lib/components/app-keyboard-shortcuts.tsx	Adds a closeModal hotkey handler that navigates back from settings when no modal is open; the logic is correct, but the entire file is indented with two extra leading spaces and is missing a trailing newline — likely an editor formatting artifact from the rewrite.
src/renderer/app/app-menu-events.tsx	Correctly gates onOpenSettings to the open-only path and delegates to toggleSettingsView; minor blank-line addition in the unrelated notification handler.
src/renderer/lib/layout/navigation-provider.tsx	Exports NonSettingsViewId type, strengthens currentView to ViewId, and exposes lastNonSettingsView from the store via useWorkspaceSlots; clean change.
src/renderer/lib/commands/app-commands.ts	Replaces direct navigate('settings') with toggleSettingsView, passing the bound navigate, current view, and lastNonSettingsView from appState; straightforward migration.

Comments Outside Diff (1)

1. src/renderer/lib/stores/navigation-store.ts, line 85-99 (link)

   lastNonSettingsView dropped when session is snapshotted in settings

   lastNonSettingsView is not included in the snapshot getter or in NavigationSnapshot, so it is never persisted. When the app quits while in settings view, snapshot.currentViewId === 'settings', which causes restoreSnapshot to skip the lastNonSettingsView update — leaving it at the default 'home'. The next Cmd+, press or Escape then navigates to 'home' instead of the user's actual prior context (e.g. 'task'). To fix this, add lastNonSettingsView to NavigationSnapshot, include it in snapshot, and restore it in restoreSnapshot.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: src/renderer/lib/stores/navigation-store.ts
   Line: 85-99
   
   Comment:
   **`lastNonSettingsView` dropped when session is snapshotted in settings**
   
   `lastNonSettingsView` is not included in the `snapshot` getter or in `NavigationSnapshot`, so it is never persisted. When the app quits while in settings view, `snapshot.currentViewId === 'settings'`, which causes `restoreSnapshot` to skip the `lastNonSettingsView` update — leaving it at the default `'home'`. The next Cmd+, press or Escape then navigates to `'home'` instead of the user's actual prior context (e.g. `'task'`). To fix this, add `lastNonSettingsView` to `NavigationSnapshot`, include it in `snapshot`, and restore it in `restoreSnapshot`.
   
   How can I resolve this? If you propose a fix, please make it concise.

Prompt To Fix All With AI

Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
src/renderer/lib/stores/navigation-store.ts:85-99
**`lastNonSettingsView` dropped when session is snapshotted in settings**

`lastNonSettingsView` is not included in the `snapshot` getter or in `NavigationSnapshot`, so it is never persisted. When the app quits while in settings view, `snapshot.currentViewId === 'settings'`, which causes `restoreSnapshot` to skip the `lastNonSettingsView` update — leaving it at the default `'home'`. The next Cmd+, press or Escape then navigates to `'home'` instead of the user's actual prior context (e.g. `'task'`). To fix this, add `lastNonSettingsView` to `NavigationSnapshot`, include it in `snapshot`, and restore it in `restoreSnapshot`.

### Issue 2 of 2
src/renderer/lib/components/app-keyboard-shortcuts.tsx:1-17
The entire file is indented with two extra leading spaces on every line, which is inconsistent with the rest of the codebase. Additionally, there is no trailing newline at the end of the file. This appears to be an editor/formatting artifact introduced during the rewrite.

```suggestion
import { useHotkey } from '@tanstack/react-hotkeys';
import { useAppSettingsKey } from '@renderer/features/settings/use-app-settings-key';
import {
  getEffectiveHotkey,
  getHotkeyRegistration,
} from '@renderer/lib/hooks/useKeyboardShortcuts';
import { useTheme } from '@renderer/lib/hooks/useTheme';
import { useWorkspaceLayoutContext } from '@renderer/lib/layout/layout-provider';
import {
  useNavigate,
  useParams,
  useWorkspaceSlots,
} from '@renderer/lib/layout/navigation-provider';
import { useShowModal } from '@renderer/lib/modal/modal-provider';
import { modalStore } from '@renderer/lib/modal/modal-store';

export function AppKeyboardShortcuts() {
```

_{Reviews (4): Last reviewed commit: "fix(settings): preserve navigation when ..." | Re-trigger Greptile}

[greptile-apps]

onOpenSettings guard runs on the close path too

onOpenSettings?.() is now called even when currentView === 'settings' (i.e., the user is toggling settings off). In App.tsx, the concrete implementation calls setView('workspace') as a side effect every time — including on close — and returns false (blocking navigation) when the app is in onboarding. While those specific side effects are harmless today, the callback is semantically an "open" guard and was never designed to control the close path. The fix is to call onOpenSettings only when the intent is to open: check currentView !== 'settings' before invoking the callback.

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/renderer/app/app-menu-events.tsx
Line: 12-17

Comment:
**`onOpenSettings` guard runs on the close path too**

`onOpenSettings?.()` is now called even when `currentView === 'settings'` (i.e., the user is toggling settings *off*). In `App.tsx`, the concrete implementation calls `setView('workspace')` as a side effect every time — including on close — and returns `false` (blocking navigation) when the app is in onboarding. While those specific side effects are harmless today, the callback is semantically an "open" guard and was never designed to control the close path. The fix is to call `onOpenSettings` only when the intent is to open: check `currentView !== 'settings'` before invoking the callback.

How can I resolve this? If you propose a fix, please make it concise.

[greptile-apps]

Module-level previousView shared across all toggle callers

previousView and lastToggleAt are module-level singletons. Both AppMenuEvents and AppKeyboardShortcuts call toggleSettingsView, so a menu-triggered toggle and a keyboard-triggered toggle that happen to arrive outside the 150 ms dedup window (e.g., under load) will each read and overwrite the same previousView. More broadly, if a future caller opens settings through a different code path that doesn't go through this helper, previousView will be stale ('home') on the next close, silently dropping the user's actual context. The guard is correct for the current callers; the risk is for future additions.

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/renderer/lib/layout/settings-toggle.ts
Line: 5-7

Comment:
**Module-level `previousView` shared across all toggle callers**

`previousView` and `lastToggleAt` are module-level singletons. Both `AppMenuEvents` and `AppKeyboardShortcuts` call `toggleSettingsView`, so a menu-triggered toggle and a keyboard-triggered toggle that happen to arrive outside the 150 ms dedup window (e.g., under load) will each read and overwrite the same `previousView`. More broadly, if a future caller opens settings through a different code path that doesn't go through this helper, `previousView` will be stale (`'home'`) on the next close, silently dropping the user's actual context. The guard is correct for the current callers; the risk is for future additions.

How can I resolve this? If you propose a fix, please make it concise.

[janburzinski]

@greptile review

[greptile-apps]

Want your agent to iterate on Greptile's feedback? Try greploops.

[janburzinski]

woah you can talk back lol

[arnestrickmann]

can you please resolve the conflicts again:) ? @janburzinski

[janburzinski]

@arnestrickmann done :)

[arnestrickmann]

@greptile

[janburzinski]

@greptile