Repository navigation
feat(shell): startup window mode, --fullscreen switch and F11 toggle - #1514
Conversation
Settings > General gains "Window on startup" (normal / maximized / fullscreen), Electron only. The value is mirrored into electron-conf by the SETTINGS_UPDATE handler and read synchronously when the main window is created, the same pattern as embeddedMpvFrameCopy. `--fullscreen` on the command line forces one fullscreen launch without persisting it. F11 toggles OS-level fullscreen through a new WINDOW:TOGGLE_FULLSCREEN channel bound in WorkspaceKeyboardShortcutsService. It is the exit path on Windows/Linux, where the title bar is hidden and the custom window controls hide themselves while fullscreen, so the setting is only offered when the bridge exposes both halves. The key is skipped while the player owns document.fullscreenElement. Two platform facts shaped the implementation: macOS ignores the constructor `fullscreen` option on a hidden window, so ready-to-show repeats setFullScreen(true) after show() where it did not take; and Electron leaves only the HTML state when the player exits HTML fullscreen inside a natively fullscreen window, so attachWindowStateEvents now tracks native and HTML fullscreen as two flags OR-ed into WINDOW:STATE_CHANGED instead of clearing one flag on leave-html-full-screen. Closes #1455 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Greptile SummaryThe PR adds configurable Electron startup window modes, a one-shot
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Create main window] --> B{Resolve startup mode}
B -->|normal| C[Restore normal bounds]
B -->|maximized| D[Maximize at ready-to-show]
B -->|fullscreen| E[Request native fullscreen]
F[F11] --> G[Toggle against pending target or event-fed state]
E --> H[Native enter/leave events]
G --> H
H --> I[Update tracked native state]
J[HTML player fullscreen events] --> K[Update tracked HTML state]
I --> L[OR native and HTML fullscreen]
K --> L
L --> M[Publish window state to renderer]
Reviews (11): Last reviewed commit: "fix(shell): make the fullscreen tracker ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…toggle parity Review follow-ups for #1514: - The `--fullscreen` switch is consumed by the first window. On macOS the process outlives its last window and the Dock re-creates it through the same initMainWindow, which read the process switch again and forced the re-created window fullscreen; it now follows the stored setting only. - WINDOW:TOGGLE_FULLSCREEN keeps the pending transition target per window and decides the next toggle against it. setFullScreen() completes asynchronously and isFullScreen() reports the old value meanwhile, so two quick F11 presses both requested the same target and an enter-then-exit landed fullscreen. The entry is dropped once a transition event reports the target state, so a request the platform dropped is corrected by the next press instead of sticking. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
A pending target that the platform silently dropped stayed in place until the next press happened to match it, so F11 parity was off by one press. The toggle handler now re-issues the target when a transition lands on the other state (a queued request may have been dropped) and ignores entries older than the transition timeout, so a request that produced no event at all is simply forgotten. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d28934755d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| startupWindowMode === 'fullscreen' && | ||
| !App.mainWindow.isFullScreen() | ||
| ) { | ||
| App.mainWindow.setFullScreen(true); |
There was a problem hiding this comment.
When the fallback startup path is used—most notably on macOS—this starts an asynchronous fullscreen transition without registering its target in pendingFullScreenTransitions. If the user presses F11 before enter-full-screen lands, WINDOW_TOGGLE_FULLSCREEN still sees isFullScreen() === false and requests true again instead of exiting, so the advertised exit requires waiting or another keypress. Route this startup request through the same transition tracker, or otherwise expose its pending target to the toggle handler.
Useful? React with 👍 / 👎.
…nsition tracker On macOS the startup fallback started an asynchronous fullscreen transition without registering its target, so an F11 pressed during the animation read isFullScreen() === false and requested fullscreen again instead of leaving it. The pending-target tracker now lives in its own service (native-fullscreen-transitions.ts) and both the F11 toggle and the startup fallback go through it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. Bravo. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
…nt arrives late settle() read the pending map directly and bypassed the transition timeout, so an event arriving more than two seconds after its request could re-issue the stale target with a fresh timestamp and decide the next F11 against intent the user had long moved past. It now goes through the same getter as the toggle, which discards expired entries. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0bac3bbe6b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| pendingFullScreenTransitions.delete(win); | ||
| if (win.isFullScreen() !== pendingTarget) { |
There was a problem hiding this comment.
On Windows, App.attachWindowStateEvents already documents that isFullScreen() can still return the pre-transition value when the matching event fires. If two quick F11 presses queue enter then exit and the enter event arrives while this getter still returns false, this comparison mistakes that stale value for the requested exit having landed, deletes the pending false target, and skips reissuing the exit; if the window manager dropped the queued request, the window remains fullscreen. Register distinct enter/leave handlers and settle against the state conveyed by the event instead of re-reading the getter.
Useful? React with 👍 / 👎.
…t the getter On Windows isFullScreen() can still report the pre-transition value while the matching event fires (the rule attachWindowStateEvents already follows), so settle() re-reading the getter could mistake a landed enter for the requested exit, drop the pending exit and never re-issue it. The enter and leave listeners now hand settle() the state the event conveys. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
… never the getter isFullScreen() is stale for the whole transition and, on Windows, even while the matching event fires, so every variant of "read the getter at the right moment" left a window in which two toggles requested the same state. The tracker now keeps its own per-window fullscreen state — seeded once at window creation (trackNativeFullScreen in initMainWindow, while no transition can be in flight) and fed afterwards only by the enter/leave-full-screen events, which also cover transitions the app did not request — and a toggle is decided against the pending target if one is in flight, else against that tracked state. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2c8d72990c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // has long moved past, and re-issuing it would revive the stale | ||
| // intent with a fresh timestamp. | ||
| const pendingTarget = getPendingFullScreenTarget(win); | ||
| state.pending = undefined; |
There was a problem hiding this comment.
When three F11 presses occur before the first fullscreen transition event, the requests are true, false, true; the first enter-full-screen event then happens to match the latest pending target, so this unconditional clear discards that final intent. If the platform subsequently reports the queued leave but dropped the last enter request—the exact queue-loss case this tracker otherwise handles—the leave finds no pending target to reissue and the window ends windowed despite the final press requesting fullscreen. Retain enough request ordering information, or reassert the current target, when a matching event may belong to an older request.
Useful? React with 👍 / 👎.
Three presses before the first transition event request true, false, true; the first enter matched the latest target and cleared the record, so if the platform then reported the queued leave but dropped the last enter, the window ended windowed against the final intent. The pending record now carries the number of requests still owed an event: a matching event clears it only once every request has reported, a mismatching one re-issues the target as before. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
@codex review |
1 similar comment
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f77f09701e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // An earlier request landed, not the latest one. Ask again | ||
| // rather than trusting the platform to have queued it: a second | ||
| // identical request is a no-op at worst. | ||
| requestFullScreen(win, pending.target); |
There was a problem hiding this comment.
After two quick F11 presses, the enter event decrements outstanding but this corrective requestFullScreen immediately increments it again; when the subsequent leave event lands, the count remains 1 even though every real transition has completed. Until the two-second timeout, an unrelated native fullscreen action such as the macOS green button or Ctrl+Cmd+F is therefore mistaken for an old mismatching event and immediately reversed. Reissue the final target without adding another user-request debt, or track which original requests have actually settled.
Useful? React with 👍 / 👎.
…est debt The re-issue that corrects a transition landing on the wrong state went through requestFullScreen and incremented the outstanding count, so the pending record outlived the last real transition and, within the timeout, mistook an unrelated native action (green button, Ctrl+Cmd+F) for an old mismatch and reversed it. The correction now repeats setFullScreen without adding debt. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 97e1dfbf4a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (pending.outstanding === 0) { | ||
| state.pending = undefined; |
There was a problem hiding this comment.
When a rapid F11 burst is coalesced—for example, enter/leave/enter produces only a final enter event—outstanding falls from 3 to 2, so this branch retains the pending target even though the requested state has landed. The fresh final code avoids debt only for corrective reissues; every user request still increments the count despite the tracker's explicit allowance for platforms dropping queued transitions. A macOS green-button or Ctrl+Cmd+F change during the ensuing two-second window is then mistaken for an old mismatch and immediately reversed. Reconcile pending requests without assuming that each setFullScreen call emits a distinct event.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Agreed that the count assumes one event per request, which coalescing breaks. Rather than reconcile against an unobservable, the tracker no longer counts at all: the record is the latest target, cleared when an event lands on it, repeated once on a mismatch (an earlier request landed first), and cleared on a second mismatch so it can never fight a user action twice. That gives up the earlier "enter/leave/enter with the last enter dropped" case — the two scenarios are indistinguishable from events, Electron queues rather than drops on macOS and applies synchronously elsewhere, and a phantom-debt reversal is the worse failure. Documented in the module docblock.
…nting requests Events cannot say which request they belong to, so counting requests fixed "enter/leave/enter with the last enter dropped" only by breaking its mirror image: a platform coalescing the burst into one enter left phantom debt and made the tracker reverse the user's next green-button/Ctrl+Cmd+F action. Neither happens on a platform Electron supports. The record is now just the latest target: an event landing on it clears it, a mismatch repeats the target once, a second mismatch clears it, and the 2 s expiry stays. The one imperfect burst is documented in the module docblock. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
…g a target Any automatic "repeat the target on a mismatch" is indistinguishable from reversing the user's own green-button or Ctrl+Cmd+F action, since events cannot say which request they belong to. The tracker now never issues a request on its own: the record is the latest target, cleared when an event lands on it, kept when one lands on the other state (our request is still queued, so the next press still follows the latest intent), and ignored after the 2 s timeout. Should a platform ever drop a queued request — Electron queues them on macOS and applies them synchronously elsewhere — the record expires, the event-fed state takes over and the next press corrects the window. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. 👍 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Closes #1455. Also covers the "start maximized" ask from #217 and the fullscreen half of #606.
What
normal(last size, default) /maximized/fullscreen. Electron only; the select renders only when the bridge exposes bothupdateSettingsandtoggleFullScreenWindow(RuntimeCapabilitiesService.supportsStartupWindowMode), so the mode is never offered without its exit.iptvnator --fullscreenforces one fullscreen launch for TV/HTPC autostart scripts and is never persisted (app.commandLine.hasSwitch; the playlist-path extractor already skips--prefixed args).WINDOW:TOGGLE_FULLSCREENchannel, listed in the shortcuts help. This is the exit path on Windows/Linux, where the title bar is hidden, the menu is removed and the custom window controls hide themselves while fullscreen — a bare "start fullscreen" checkbox would have trapped those users. F11 is deliberately not gated byisTypingInInput, and is skipped while the player ownsdocument.fullscreenElement(its own F / Esc are in charge). The PWA keeps the browser's F11.How
SETTINGS_UPDATEhandler mirrorsstartupWindowModeinto electron-conf (STARTUP_WINDOW_MODE) — the same pattern asembeddedMpvFrameCopy— andinitMainWindowreads it synchronously. Both sides normalize throughnormalizeStartupWindowMode, so junk never reaches the config or the window options.fullscreenis theBrowserWindowconstructor option. macOS ignores it on a hidden window (an NSWindow only toggles fullscreen once it is on screen — the first local e2e run caught this), soready-to-showrepeatssetFullScreen(true)aftershow()only whereisFullScreen()is still false.maximizedcallsmaximize()insideready-to-showright beforeshow(), never earlier (it would show a hidden window early).leave-full-screenfires.attachWindowStateEventsused to clearisFullScreenonleave-html-full-screen, which would un-hide the Windows/Linux window controls over a window that is still fullscreen after "start fullscreen → F → Esc". It now tracks native and HTML fullscreen as two flags OR-ed intoWINDOW:STATE_CHANGED.Deliberately out of scope: kiosk mode (removes the exit path), "remember last window state" (bounds persistence stores normal bounds only; an explicit choice is clearer), and a 10-foot UI (#1337).
Tests
startup-window-mode.spec.ts;app.spec.ts"startup window mode" (constructor option, macOS repeat, maximize-before-show ordering, CLI override, junk stored value);app-window-state.spec.tsnative+HTML round-trips;window.events.spec.tstoggle handler;keyboard-shortcuts.spec.ts+workspace-keyboard-shortcuts.service.spec.ts(F11 incl. input focus, HTML-fullscreen guard, modifiers/repeat, no bridge);runtime-capabilities,settings.events,settings-form.utils,settings-storecases.settings.e2e.ts): save fullscreen → relaunch is fullscreen → F11 out and back in;--fullscreenone-shot not persisted; maximized (skipped on Linux CI likewindow-controls.e2e.ts, xvfb has no WM).Validation
pnpm run typecheck:ci✅pnpm nx test electron-backend1847/1847 ✅ ·services,portal-shared-util,workspace-shell-feature,shared-interfaces1188/1188 ✅ ·websettings specs 161/161 ✅pnpm nx run electron-backend-e2e:e2e-ci--src/settings.e2e.ts17/17 ✅ (macOS)nx lint(7 projects),nx build web,release:notes:validate, i18n coverage + placeholder gates ✅Docs / notes
docs/architecture/workspace-shell.md: new "Startup window mode" section + the two-flag fullscreen rule;CLAUDE.md,README.md(F11 row,--fullscreennote);.changes/shell-startup-window-mode.md; 6 i18n keys × 18 locales via thei18n-fillmerger.Still manual
🤖 Generated with Claude Code