Skip to content

fix(ui): close editor mode when opening settings#1422

Merged
arnestrickmann merged 4 commits intogeneralaction:mainfrom
shreyaspapi:fix/settings-blocked-by-editor
Mar 13, 2026
Merged

fix(ui): close editor mode when opening settings#1422
arnestrickmann merged 4 commits intogeneralaction:mainfrom
shreyaspapi:fix/settings-blocked-by-editor

Conversation

@shreyaspapi
Copy link
Contributor

@shreyaspapi shreyaspapi commented Mar 11, 2026

Summary

When the in-app editor (EditorMode) is open, clicking the Settings button had no visible effect — it set showSettingsPage=true but the MainContentArea where SettingsPage renders was hidden via display: none behind the editor overlay.

Fix: call setShowEditorMode(false) before opening settings in both entry points:

  • Titlebar Settings button (handleToggleSettingsPage)
  • Native OS menu Settings handler (onMenuOpenSettings)

Fixes

Fixes #1367

Snapshot

Screen.Recording.2026-03-12.at.12.20.24.AM.mov

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Mandatory Tasks

  • I have self-reviewed the code
  • A decent size PR without self-review might be rejected

Checklist

  • I have read the contributing guide
  • My code follows the style guidelines of this project (pnpm run format)
  • I have checked if my changes generate no new warnings (pnpm run lint)
  • I have checked if new and existing unit tests pass locally with my changes

@vercel
Copy link

vercel bot commented Mar 11, 2026

@shreyaspapi is attempting to deploy a commit to the General Action Team on Vercel.

A member of the Team first needs to authorize it.

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 11, 2026

Greptile Summary

This PR fixes a bug where opening settings while the in-app CodeEditor overlay was active had no visible effect, because MainContentArea (where SettingsPage renders) was obscured behind the editor. The fix adds setShowEditorMode(false) inside handleToggleSettingsPage before calling openSettingsPage(), and correctly adds setShowEditorMode to the useCallback dependency array.

Key changes:

  • handleToggleSettingsPage now closes editor mode before opening settings, making the main content area visible.
  • The useCallback dependency array is updated to include setShowEditorMode.

Issue found:

  • The native OS menu "Settings" handler (onMenuOpenSettings listener, line 160) still calls openSettingsPage() directly without first closing editor mode, leaving the same visual bug reproducible via the menu bar.

Confidence Score: 3/5

  • Safe to merge with minor risk — the primary bug is fixed, but one secondary path to open settings still has the same issue.
  • The core fix is correct and the dependency array update is accurate. However, the native OS menu "Settings" handler is an identically-structured code path that still reproduces the original bug, so the fix is incomplete.
  • src/renderer/views/Workspace.tsx — specifically the onMenuOpenSettings effect at line 160.

Important Files Changed

Filename Overview
src/renderer/views/Workspace.tsx Fix correctly closes editor mode before opening settings via the titlebar button, but the same code path bug remains in the native OS menu "Settings" handler (line 160) which still calls openSettingsPage() directly without dismissing the editor overlay.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A([User triggers Settings]) --> B{Entry point}
    B -->|Titlebar button| C[handleToggleSettingsPage]
    B -->|Native OS menu| D[onMenuOpenSettings handler]
    B -->|Command palette| E[openSettingsPage directly]
    B -->|Update notifier| F[openSettingsPage directly]

    C --> G{showSettingsPage?}
    G -->|Yes| H[handleCloseSettingsPage]
    G -->|No| I[setShowEditorMode false ✅ FIXED]
    I --> J[openSettingsPage]

    D --> K[openSettingsPage ⚠️ skips setShowEditorMode]
    E --> K
    F --> K

    J --> L{showEditorMode was true?}
    K --> L
    L -->|No| M[Settings renders visibly ✅]
    L -->|Yes| N[CodeEditor overlay still active ❌ Settings hidden]
Loading

Comments Outside Diff (1)

  1. src/renderer/views/Workspace.tsx, line 158-164 (link)

    Native menu "Settings" still bypasses the fix

    The same bug this PR addresses — settings rendered hidden behind the CodeEditor overlay when editor mode is active — still exists when the user opens settings via the native OS menu bar. The onMenuOpenSettings handler calls openSettingsPage() directly without first calling setShowEditorMode(false).

Last reviewed commit: 220b2a3

Copy link
Contributor

@yashdev9274 yashdev9274 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hey @shreyaspapi
this fix is still incomplete. there are 2 additional entry points that still have this bug:

im mentioning them along with the changes so you can fix them directly

1. Update notifier (line ~260)

```tsx
// Current (needs fix):
onOpenSettings: () => openSettingsPage('general')

// Fix:
onOpenSettings: () => { setShowEditorMode(false); openSettingsPage('general'); }
```

2. Command palette handlers (lines ~476-477)

```tsx
// Current (needs fix):
handleOpenSettings={() => openSettingsPage()}
handleOpenKeyboardShortcuts={() => openSettingsPage('interface')}

// Fix:
handleOpenSettings={() => { setShowEditorMode(false); openSettingsPage(); }}
handleOpenKeyboardShortcuts={() => { setShowEditorMode(false); openSettingsPage('interface'); }}
```

do address them as well

@shreyaspapi
Copy link
Contributor Author

shreyaspapi commented Mar 11, 2026

Fixed the two remaining entry points in c9d5332:

@arnestrickmann
Copy link
Contributor

Thanks for opening this PR and for taking this on.

I am wondering why this would be desired behaviour.

CleanShot 2026-03-11 at 19 04 53@2x

I think it would be a better fix to keep the left file editor sidebar open and just open the settings in the center of the page.

Wdyt?

@shreyaspapi
Copy link
Contributor Author

I think it would be a better fix to keep the left file editor sidebar open and just open the settings in the center of the page.

Wdyt?

Yeah maybe that would be better

@arnestrickmann
Copy link
Contributor

Could you do that change?

@shreyaspapi
Copy link
Contributor Author

shreyaspapi commented Mar 12, 2026 via email

@shreyaspapi
Copy link
Contributor Author

Screenshot 2026-03-13 at 2 46 10 AM

…tor mode

Keep the file explorer sidebar visible when opening settings from any
entry point (titlebar, menu, command palette, update notifier). Settings
now renders in the CodeEditor content area, replacing the Monaco editor,
while the file tree stays intact. The right sidebar is auto-collapsed
while settings is shown and restored on close.
@shreyaspapi shreyaspapi force-pushed the fix/settings-blocked-by-editor branch from ef4ecb9 to 79557c6 Compare March 12, 2026 21:17
@arnestrickmann
Copy link
Contributor

@shreyaspapi thanks!

@shreyaspapi
Copy link
Contributor Author

Screenshot 2026-03-13 at 2 48 25 AM

I tried adding it in the middle area, but it isn’t looking good on my screen size at least.

@arnestrickmann
Copy link
Contributor

How does it look now in the latest iteration?

@shreyaspapi
Copy link
Contributor Author

Screenshot 2026-03-13 at 2 46 10 AM

This is the latest one this looks fine the spacing is good as well.

@arnestrickmann arnestrickmann merged commit e90d878 into generalaction:main Mar 13, 2026
2 of 3 checks passed
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.

[bug]: can't open settings when in-app editor is open

3 participants