fix(desktop): improve Linux desktop experience#1208
Conversation
Add Linux build job to CI workflow (ubuntu-latest, x64) with AppImage and deb targets. Update release workflow to create stable-named Linux artifacts and fix shell fallback from /bin/zsh to /bin/sh for cross-platform compatibility.
Move GATED_FEATURES and GatedFeature type from usePaywall.ts to constants.ts to break the circular import chain that caused "Cannot access 'GATED_FEATURES' before initialization" at runtime.
Remove blanket disableHardwareAcceleration() that forced CPU-only rendering on Linux. Add GPU optimization flags (gpu-rasterization, zero-copy, ignore-gpu-blocklist) and provide SUPERSET_DISABLE_GPU=1 escape hatch for users with problematic drivers.
- Add no-drag class to WindowControls so buttons are clickable - Add Linux CLI command map for external apps (fixes spawn open ENOENT) - Make trafficLightPosition macOS-only to avoid warnings on Linux
Default to maximized on first launch. When moving from a smaller screen to a larger one, auto-scale window to 90% of work area if saved bounds cover less than 60% of current display.
- Enable GPU acceleration flags (rasterization, zero-copy, Vaapi, Ozone) - Enable auto-updater for Linux (AppImage native support) - Add PipeWire → PulseAudio → ALSA audio fallback chain - Add Linux-compatible terminal fonts (Ubuntu Mono, DejaVu, Liberation) - Skip port scanning when no terminal sessions are active - Reduce terminal scrollback to 5000 lines for lower memory usage - Add prefers-reduced-motion CSS for accessibility
📝 WalkthroughWalkthroughThis PR introduces Linux desktop application support alongside existing macOS and Windows builds, adds CI/CD infrastructure for Linux artifact generation and release workflows, implements platform-specific GPU and audio optimizations, reorganizes Paywall module exports, and applies various UX improvements including proportional window scaling, accessibility features, and updated default settings. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Pull request overview
This PR enhances the Linux desktop experience by enabling GPU acceleration, auto-updates, and improving cross-platform compatibility for audio, fonts, and external applications.
Changes:
- Enables GPU acceleration flags for Linux with SUPERSET_DISABLE_GPU=1 escape hatch
- Enables auto-updater for Linux AppImage distributions via electron-updater
- Adds Linux audio fallback chain (PipeWire → PulseAudio → ALSA) for notification sounds
- Adds Linux-compatible terminal fonts and external app CLI commands
- Optimizes performance by skipping port scanning when no sessions are active
- Reduces terminal scrollback buffer from 10000 to 5000 lines
- Adds prefers-reduced-motion CSS for accessibility
Reviewed changes
Copilot reviewed 17 out of 18 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| bun.lock | Version bump from 0.0.65 to 0.0.67 |
| apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/config.ts | Adds Linux terminal fonts (Ubuntu Mono, DejaVu Sans Mono, Liberation Mono) and Consolas |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/components/WindowControls/WindowControls.tsx | Adds no-drag class to window controls container |
| apps/desktop/src/renderer/globals.css | Adds prefers-reduced-motion media query for accessibility |
| apps/desktop/src/renderer/components/Paywall/usePaywall.ts | Extracts GATED_FEATURES and GatedFeature to constants.ts |
| apps/desktop/src/renderer/components/Paywall/index.ts | Updates exports to reference constants.ts |
| apps/desktop/src/renderer/components/Paywall/constants.ts | Moves GATED_FEATURES and GatedFeature definitions here |
| apps/desktop/src/main/windows/main.ts | Conditionally applies trafficLightPosition only for macOS |
| apps/desktop/src/main/terminal-host/session.ts | Reduces scrollback from 10000 to 5000 lines, changes default shell from /bin/zsh to /bin/sh |
| apps/desktop/src/main/lib/window-state/bounds-validation.ts | Adds auto-scaling logic for small windows on large screens, maximizes by default on first launch |
| apps/desktop/src/main/lib/terminal/port-manager.ts | Skips port scanning when no sessions are active |
| apps/desktop/src/main/lib/notification-sound.ts | Adds PipeWire → PulseAudio → ALSA fallback chain for Linux audio |
| apps/desktop/src/main/lib/auto-updater.ts | Enables auto-updater for Linux by checking PLATFORM.IS_WINDOWS instead of !PLATFORM.IS_MAC |
| apps/desktop/src/lib/trpc/routers/external/helpers.ts | Adds Linux CLI commands map and Windows support for external app launching |
| apps/desktop/src/lib/electron-app/factories/app/setup.ts | Adds GPU acceleration flags for Linux and SUPERSET_DISABLE_GPU=1 escape hatch |
| apps/desktop/create-release.sh | Updates to display Linux download URLs alongside macOS |
| .github/workflows/release-desktop.yml | Adds Linux artifact handling (AppImage and DEB) |
| .github/workflows/build-desktop.yml | Adds Linux build job for x64 architecture |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Allow users with problematic GPU drivers to disable hardware acceleration | ||
| if (process.env.SUPERSET_DISABLE_GPU === "1") { | ||
| app.disableHardwareAcceleration(); | ||
| } | ||
|
|
||
| // Enable GPU optimizations on Linux | ||
| if (PLATFORM.IS_LINUX) { | ||
| app.commandLine.appendSwitch("enable-gpu-rasterization"); | ||
| app.commandLine.appendSwitch("enable-zero-copy"); | ||
| app.commandLine.appendSwitch("ignore-gpu-blocklist"); | ||
| // Canvas OOP rasterization offloads canvas rendering to GPU process | ||
| app.commandLine.appendSwitch( | ||
| "enable-features", | ||
| "CanvasOopRasterization,VaapiVideoDecodeLinuxGL", | ||
| ); | ||
| // Auto-detect Wayland vs X11 for native compositor integration | ||
| app.commandLine.appendSwitch("ozone-platform-hint", "auto"); | ||
| } |
There was a problem hiding this comment.
The GPU acceleration escape hatch (SUPERSET_DISABLE_GPU=1) doesn't fully disable GPU features on Linux. When SUPERSET_DISABLE_GPU=1 is set, disableHardwareAcceleration() is called, but then the Linux-specific GPU switches (enable-gpu-rasterization, enable-zero-copy, etc.) are still applied unconditionally. This creates inconsistent behavior where the user attempts to disable GPU acceleration, but GPU-specific features are still enabled via command-line switches. The Linux GPU optimization block should be wrapped in a condition that checks if GPU acceleration wasn't disabled.
| // Windows: use start command | ||
| const appName = MAC_APP_NAMES[app]; | ||
| if (!appName) return null; | ||
| return { command: "open", args: ["-a", appName, targetPath] }; | ||
| return { command: "cmd", args: ["/c", "start", "", appName, targetPath] }; |
There was a problem hiding this comment.
The Windows implementation in getAppCommand uses MAC_APP_NAMES which contains application display names like "Visual Studio Code", "IntelliJ IDEA", etc. These are not valid executable names for Windows. The cmd /c start command expects either executable names (like "code.exe") or properly registered application names. This implementation will fail to open applications on Windows. A WINDOWS_APP_COMMANDS map similar to LINUX_APP_COMMANDS should be created with appropriate Windows executable names or registered application names.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@apps/desktop/src/lib/electron-app/factories/app/setup.ts`:
- Around line 77-89: In the PLATFORM.IS_LINUX block update the GPU flags: remove
the unsupported switches passed via
app.commandLine.appendSwitch("enable-gpu-rasterization"), ("enable-zero-copy")
and ("ignore-gpu-blocklist") and instead use a supported alternative for forcing
discrete GPU when needed (e.g., the Electron-supported
force_high_performance_gpu switch) via app.commandLine.appendSwitch; also update
the enable-features value passed to
app.commandLine.appendSwitch("enable-features", ...) to replace the deprecated
"VaapiVideoDecodeLinuxGL" with the modern names
"AcceleratedVideoDecodeLinuxGL,AcceleratedVideoDecodeLinuxZeroCopyGL" while
keeping CanvasOopRasterization as appropriate. Ensure all changes are made
inside the same PLATFORM.IS_LINUX conditional around
app.commandLine.appendSwitch.
In `@apps/desktop/src/main/lib/notification-sound.ts`:
- Around line 64-74: The nested execFile callbacks currently swallow errors and
shadow the same `error` name; update the Linux audio fallback block (where
execFile is called with "pw-play", "paplay", "aplay" and uses `soundPath`) to
use distinct callback parameter names (e.g., errPw, errPap, errAplay) and, if
the final execFile call fails, log a helpful error message including the
soundPath and which players were attempted (using console.error or the project
logger). Ensure each intermediate failure is either logged or propagated as
needed and avoid reusing the same `error` variable in nested callbacks.
In `@apps/desktop/src/main/lib/window-state/bounds-validation.ts`:
- Around line 73-93: Change scaling to use the work area of the display that
contains the saved bounds rather than the primary display: locate where
workAreaSize is used and replace it with a display-specific work area obtained
from a helper (e.g., getDisplayForBounds or similar) using
savedState.width/height and savedState.x/y; extract the magic numbers into
module-level constants (e.g., MIN_AREA_RATIO = 0.6, WORKAREA_SCALE = 0.9) and
use them instead of 0.6/0.9; and update clampToWorkArea to accept an object
parameter (e.g., clampToWorkArea({ width, height })) and update its call site
here accordingly. Ensure the scaling math (savedAspect,
targetWidth/targetHeight) uses the display-specific workAreaSize and keep the
same aspect-preserving logic.
🧹 Nitpick comments (7)
apps/desktop/src/renderer/components/Paywall/usePaywall.ts (1)
2-7: Consider removing redundant re-exports from this file.The
GATED_FEATURESandGatedFeatureare now re-exported from bothusePaywall.ts(line 7) andindex.ts(lines 2-3). Sinceindex.tsserves as the barrel file for the Paywall module, the re-export here inusePaywall.tsmay be unnecessary and creates multiple import paths to the same symbols.If existing consumers import from
usePaywall.tsdirectly, keeping the re-export maintains backwards compatibility. Otherwise, consider removing line 7 to enforce a single import path throughindex.ts.♻️ Optional: Remove redundant re-export
import { authClient } from "renderer/lib/auth-client"; import { type GatedFeature, GATED_FEATURES } from "./constants"; import { paywall } from "./Paywall"; type UserPlan = "free" | "pro"; -export { type GatedFeature, GATED_FEATURES }; - export function usePaywall() {apps/desktop/src/main/lib/auto-updater.ts (1)
200-216: Comment references macOS-specific artifact but code now runs on Linux too.Line 212's comment mentions
latest-mac.yml, but with the platform gating change this code now also runs on Linux, where electron-updater will look forlatest-linux.yml. The comment should be updated to be platform-agnostic.📝 Suggested comment update
// Use generic provider with explicit feed URL - // This ensures we always fetch latest-mac.yml from the correct GitHub release + // This ensures we always fetch the platform-specific yml (latest-mac.yml or latest-linux.yml) from the correct GitHub release autoUpdater.setFeedURL({apps/desktop/src/main/terminal-host/session.ts (2)
135-140: Extract the scrollback default to a named constant.Avoid inline magic numbers so the default can be tuned in one place.
As per coding guidelines, extract hardcoded magic numbers, strings, and enums to named constants at module top instead of leaving them inline in logic.♻️ Suggested change
+const DEFAULT_SCROLLBACK_LINES = 5_000; ... this.emulator = new HeadlessEmulator({ cols: options.cols, rows: options.rows, - scrollback: options.scrollbackLines ?? 5000, + scrollback: options.scrollbackLines ?? DEFAULT_SCROLLBACK_LINES, });
939-943: Promote the/bin/shfallback to a module constant.This avoids a hardcoded path literal inside logic and makes future platform overrides safer.
As per coding guidelines, extract hardcoded magic numbers, strings, and enums to named constants at module top instead of leaving them inline in logic.♻️ Suggested change
+const DEFAULT_FALLBACK_SHELL = "/bin/sh"; ... private getDefaultShell(): string { if (process.platform === "win32") { return process.env.COMSPEC || "cmd.exe"; } - return process.env.SHELL || "/bin/sh"; + return process.env.SHELL || DEFAULT_FALLBACK_SHELL; }apps/desktop/src/main/windows/main.ts (1)
85-87: Consider usingPLATFORM.IS_MACfor consistency.The platform check works correctly, but the codebase has a
PLATFORM.IS_MACconstant inshared/constants.tsthat could be used here for consistency with other platform checks (e.g., line 91 insetup.tsusesPLATFORM.IS_WINDOWS).♻️ Suggested refactor
+import { PLATFORM } from "shared/constants"; + // In createWindow options: - ...(process.platform === "darwin" + ...(PLATFORM.IS_MAC ? { trafficLightPosition: { x: 16, y: 16 } } : {}),apps/desktop/src/lib/trpc/routers/external/helpers.ts (2)
32-57: Linux app commands mapping is well-structured.Good approach marking macOS-only apps (xcode, iterm, warp, appcode) as
nulland using standard CLI commands for cross-platform apps.One note:
x-terminal-emulator(line 42) is Debian/Ubuntu specific (alternatives system). On Fedora, Arch, etc., this command won't exist. Consider adding a fallback chain or detecting the available terminal emulator.💡 Optional: Add terminal fallback detection
// Could add a helper to detect available terminal: // gnome-terminal, konsole, xfce4-terminal, xterm as fallbacks
67-82: Platform branching logic is clear and maintainable.The explicit platform checks with early returns are readable. However, note that the Windows fallback (line 80) uses
MAC_APP_NAMESwhich contains macOS-specific application names like "Visual Studio Code". While the Windowsstartcommand may resolve these through shell path lookup, this is fragile and may not work for all applications.Consider adding a
WINDOWS_APP_COMMANDSmapping similar toLINUX_APP_COMMANDSfor more reliable Windows support:💡 Suggested improvement for Windows
+/** Map of app IDs to their Windows executable names */ +const WINDOWS_APP_COMMANDS: Record<ExternalApp, string | null> = { + finder: null, + vscode: "code", + "vscode-insiders": "code-insiders", + cursor: "cursor", + // ... etc +}; // Windows: use start command -const appName = MAC_APP_NAMES[app]; -if (!appName) return null; -return { command: "cmd", args: ["/c", "start", "", appName, targetPath] }; +const command = WINDOWS_APP_COMMANDS[app]; +if (!command) return null; +return { command: "cmd", args: ["/c", command, targetPath] };
| // Enable GPU optimizations on Linux | ||
| if (PLATFORM.IS_LINUX) { | ||
| app.commandLine.appendSwitch("enable-gpu-rasterization"); | ||
| app.commandLine.appendSwitch("enable-zero-copy"); | ||
| app.commandLine.appendSwitch("ignore-gpu-blocklist"); | ||
| // Canvas OOP rasterization offloads canvas rendering to GPU process | ||
| app.commandLine.appendSwitch( | ||
| "enable-features", | ||
| "CanvasOopRasterization,VaapiVideoDecodeLinuxGL", | ||
| ); | ||
| // Auto-detect Wayland vs X11 for native compositor integration | ||
| app.commandLine.appendSwitch("ozone-platform-hint", "auto"); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find package.json in the desktop app to check Electron version
find . -name "package.json" -type f | head -20Repository: superset-sh/superset
Length of output: 586
🏁 Script executed:
# Search for electron in package files
rg "electron" --type json -l | head -10Repository: superset-sh/superset
Length of output: 163
🏁 Script executed:
cat ./apps/desktop/package.json | rg "electron" -A 1 -B 1Repository: superset-sh/superset
Length of output: 1388
🌐 Web query:
Electron 39 Chromium version compatibility
💡 Result:
Electron 39.x is built on Chromium 142 (Chrome/Chromium “M142”). (endoflife.date)
Exact Chromium patch versions depend on the Electron 39 minor/patch release, for example:
- Electron 39.0.0 → Chromium 142.0.7444.52 (releases.electronjs.org)
- Electron 39.1.1 → Chromium 142.0.7444.59 (releases.electronjs.org)
- Electron 39.2.5 → Chromium 142.0.7444.177 (releases.electronjs.org)
To verify for any specific 39.x version, check its entry on Electron Releases (it lists the bundled Chromium/Node/V8). (releases.electronjs.org)
Citations:
- 1: https://endoflife.date/electron?utm_source=openai
- 2: https://releases.electronjs.org/release/v39.0.0?utm_source=openai
- 3: https://releases.electronjs.org/release/v39.1.1?utm_source=openai
- 4: https://releases.electronjs.org/release/v39.2.5?utm_source=openai
- 5: https://releases.electronjs.org/release/v39.0.0?utm_source=openai
🌐 Web query:
VaapiVideoDecodeLinuxGL Chromium flag electron 39
💡 Result:
Electron 39 ships with Chromium 142.0.7444.52 (releases.electronjs.org), so you’re dealing with “modern” Linux VA-API behavior/flags.
What VaapiVideoDecodeLinuxGL is
--enable-features=VaapiVideoDecodeLinuxGL is the Chromium feature-flag that enables VA-API hardware video decoding on Linux via the GL path (best-effort / not officially supported). Chromium’s own VA-API doc lists it as the decode switch and also calls out the commonly-paired GL/ANGLE switches: --use-gl=angle and --use-angle=gl. (chromium.googlesource.com)
How to pass it in Electron 39
Add the switches before app.whenReady():
const { app } = require('electron');
app.commandLine.appendSwitch('enable-features', 'VaapiVideoDecodeLinuxGL');
app.commandLine.appendSwitch('use-gl', 'angle');
app.commandLine.appendSwitch('use-angle', 'gl');
// optional, sometimes needed:
// app.commandLine.appendSwitch('ignore-gpu-blocklist');
app.whenReady().then(() => { /* create windows */ });Important: flag rename in newer Chromium
Multiple Linux guides report that starting around Chromium 131, VA-API decode flags were effectively renamed from Vaapi* to Accelerated* (and --use-gl=angle --use-angle=gl may no longer be required on some setups). The commonly suggested replacements are:
--enable-features=AcceleratedVideoDecodeLinuxGL,AcceleratedVideoDecodeLinuxZeroCopyGL(plusAcceleratedVideoEncoderif you care about encode)- Sometimes also
VaapiIgnoreDriverChecks(esp. non-Intel / finicky configs) (dev.to) (wiki.debian.org)
So on Electron 39, if VaapiVideoDecodeLinuxGL doesn’t do anything, try:
app.commandLine.appendSwitch(
'enable-features',
'AcceleratedVideoDecodeLinuxGL,AcceleratedVideoDecodeLinuxZeroCopyGL'
);Verify it worked
Use Chromium’s recommended check:
chrome://media-internals→ for the playing video, look forvideo_decodershowing a GPU/VAAPI decoder. (chromium.googlesource.com)
Citations:
- 1: https://releases.electronjs.org/release/v39.0.0?utm_source=openai
- 2: https://chromium.googlesource.com/chromium/src/%2B/refs/tags/124.0.6356.6/docs/gpu/vaapi.md?utm_source=openai
- 3: https://dev.to/archerallstars/chrome-flags-latest-2024-update-web-browser-video-hardware-acceleration-on-linux-34k1?utm_source=openai
- 4: https://wiki.debian.org/HardwareVideoAcceleration?utm_source=openai
- 5: https://chromium.googlesource.com/chromium/src.git/%2B/354b19a49c2fb545a398517c04ecb2f11f20a05f/docs/gpu/vaapi.md?utm_source=openai
🌐 Web query:
enable-gpu-rasterization enable-zero-copy ignore-gpu-blocklist electron 39 deprecated
💡 Result:
Those three flags are Chromium “about:flags”/GPU tuning switches, not Electron APIs. In Electron v39, they’re commonly reported as “deprecated/ignored” because Electron only guarantees a limited set of supported command-line switches, and unsupported switches “will have no effect” (even if Chromium used to accept them). [1]
What to do instead:
- Don’t rely on
--enable-gpu-rasterization,--enable-zero-copy,--ignore-gpu-blocklistfor Electron v39—they’re not in Electron’s supported-switch list, so behavior can change or become a no-op. [1] - If your goal is “use the discrete GPU,” use Electron-supported switches like
--force_high_performance_gpu(or the low-power counterpart). [1] - If you’re trying to tweak Chromium rendering behavior, Electron recommends discovering the current Chromium-in-Electron switches via Chromium’s flags/version pages and noting many modern toggles are now
--enable-features=...entries rather than standalone switches. [1] - For background on GPU rasterization being a Chromium feature primarily controlled via
chrome://flags/#enable-gpu-rasterization/ related mechanisms, see Chromium’s graphics docs. [2]
Sources: Electron “Supported Command Line Switches” (unsupported switches have no effect; guidance on Chromium flags) [1]; Chromium graphics doc on enabling GPU rasterization [2].
Remove or replace unsupported GPU flags for Electron 39.
The flags enable-gpu-rasterization, enable-zero-copy, and ignore-gpu-blocklist are not in Electron's supported command-line switches list and will have no effect in Electron 39. Remove these or replace with Electron-supported alternatives like --force_high_performance_gpu if targeting discrete GPU usage.
Additionally, VaapiVideoDecodeLinuxGL is deprecated in Chromium 142 (bundled with Electron 39.x) in favor of AcceleratedVideoDecodeLinuxGL and AcceleratedVideoDecodeLinuxZeroCopyGL. Update the enable-features flag to use the modern accelerated video decode names for better compatibility.
Current code
app.commandLine.appendSwitch("enable-gpu-rasterization");
app.commandLine.appendSwitch("enable-zero-copy");
app.commandLine.appendSwitch("ignore-gpu-blocklist");
app.commandLine.appendSwitch(
"enable-features",
"CanvasOopRasterization,VaapiVideoDecodeLinuxGL",
);
🤖 Prompt for AI Agents
In `@apps/desktop/src/lib/electron-app/factories/app/setup.ts` around lines 77 -
89, In the PLATFORM.IS_LINUX block update the GPU flags: remove the unsupported
switches passed via app.commandLine.appendSwitch("enable-gpu-rasterization"),
("enable-zero-copy") and ("ignore-gpu-blocklist") and instead use a supported
alternative for forcing discrete GPU when needed (e.g., the Electron-supported
force_high_performance_gpu switch) via app.commandLine.appendSwitch; also update
the enable-features value passed to
app.commandLine.appendSwitch("enable-features", ...) to replace the deprecated
"VaapiVideoDecodeLinuxGL" with the modern names
"AcceleratedVideoDecodeLinuxGL,AcceleratedVideoDecodeLinuxZeroCopyGL" while
keeping CanvasOopRasterization as appropriate. Ensure all changes are made
inside the same PLATFORM.IS_LINUX conditional around
app.commandLine.appendSwitch.
| // Linux - try common audio players in order of preference: | ||
| // pw-play (PipeWire), paplay (PulseAudio), aplay (ALSA) | ||
| execFile("pw-play", [soundPath], (error) => { | ||
| if (error) { | ||
| execFile("aplay", [soundPath]); | ||
| execFile("paplay", [soundPath], (error) => { | ||
| if (error) { | ||
| execFile("aplay", [soundPath]); | ||
| } | ||
| }); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Add error logging when all audio players fail.
If all three audio players fail, the error is silently swallowed. This makes debugging audio issues on Linux unnecessarily difficult. Additionally, consider using distinct parameter names to avoid shadowing error in the nested callbacks.
As per coding guidelines: "Never swallow errors silently; at minimum log errors with context before rethrowing or handling them explicitly."
🔊 Proposed fix to add error logging
// Linux - try common audio players in order of preference:
// pw-play (PipeWire), paplay (PulseAudio), aplay (ALSA)
- execFile("pw-play", [soundPath], (error) => {
- if (error) {
- execFile("paplay", [soundPath], (error) => {
- if (error) {
- execFile("aplay", [soundPath]);
+ execFile("pw-play", [soundPath], (pwError) => {
+ if (pwError) {
+ execFile("paplay", [soundPath], (paError) => {
+ if (paError) {
+ execFile("aplay", [soundPath], (alsaError) => {
+ if (alsaError) {
+ console.warn(
+ `[notification-sound] All audio players failed for: ${soundPath}`
+ );
+ }
+ });
}
});
}
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Linux - try common audio players in order of preference: | |
| // pw-play (PipeWire), paplay (PulseAudio), aplay (ALSA) | |
| execFile("pw-play", [soundPath], (error) => { | |
| if (error) { | |
| execFile("aplay", [soundPath]); | |
| execFile("paplay", [soundPath], (error) => { | |
| if (error) { | |
| execFile("aplay", [soundPath]); | |
| } | |
| }); | |
| } | |
| }); | |
| // Linux - try common audio players in order of preference: | |
| // pw-play (PipeWire), paplay (PulseAudio), aplay (ALSA) | |
| execFile("pw-play", [soundPath], (pwError) => { | |
| if (pwError) { | |
| execFile("paplay", [soundPath], (paError) => { | |
| if (paError) { | |
| execFile("aplay", [soundPath], (alsaError) => { | |
| if (alsaError) { | |
| console.warn( | |
| `[notification-sound] All audio players failed for: ${soundPath}` | |
| ); | |
| } | |
| }); | |
| } | |
| }); | |
| } | |
| }); |
🤖 Prompt for AI Agents
In `@apps/desktop/src/main/lib/notification-sound.ts` around lines 64 - 74, The
nested execFile callbacks currently swallow errors and shadow the same `error`
name; update the Linux audio fallback block (where execFile is called with
"pw-play", "paplay", "aplay" and uses `soundPath`) to use distinct callback
parameter names (e.g., errPw, errPap, errAplay) and, if the final execFile call
fails, log a helpful error message including the soundPath and which players
were attempted (using console.error or the project logger). Ensure each
intermediate failure is either logged or propagated as needed and avoid reusing
the same `error` variable in nested callbacks.
| let { width, height } = clampToWorkArea(savedState.width, savedState.height); | ||
|
|
||
| // Scale up if saved bounds are much smaller than current work area | ||
| // This handles moving from a small screen to a large screen | ||
| const areaRatio = | ||
| (width * height) / (workAreaSize.width * workAreaSize.height); | ||
| if (areaRatio < 0.6) { | ||
| // Saved window covers less than 60% of current screen → scale up | ||
| // Use 90% of work area while maintaining aspect ratio | ||
| const savedAspect = width / height; | ||
| const targetWidth = Math.round(workAreaSize.width * 0.9); | ||
| const targetHeight = Math.round(targetWidth / savedAspect); | ||
|
|
||
| if (targetHeight <= workAreaSize.height) { | ||
| width = targetWidth; | ||
| height = targetHeight; | ||
| } else { | ||
| height = Math.round(workAreaSize.height * 0.9); | ||
| width = Math.round(height * savedAspect); | ||
| } | ||
| } |
There was a problem hiding this comment.
Scale against the display that contains the saved bounds.
Right now scaling is based on the primary display’s work area. If the window was saved on a smaller secondary display, the scaled bounds can exceed that display and be treated as off-screen, causing an unexpected recenter onto the primary display. Consider deriving the work area from the display matching the saved bounds before applying the scale. Also, please extract the scaling thresholds to constants and switch clampToWorkArea to object params for consistency.
🔧 Suggested update (display-aware scaling + constants + object params)
const MIN_VISIBLE_OVERLAP = 50;
const MIN_WINDOW_SIZE = 400;
+const SCALE_UP_AREA_RATIO_THRESHOLD = 0.6;
+const SCALE_UP_WORKAREA_RATIO = 0.9;
function clampToWorkArea(
- width: number,
- height: number,
+ { width, height }: { width: number; height: number },
): { width: number; height: number } {
const { workAreaSize } = screen.getPrimaryDisplay();
return {
width: Math.min(Math.max(width, MIN_WINDOW_SIZE), workAreaSize.width),
height: Math.min(Math.max(height, MIN_WINDOW_SIZE), workAreaSize.height),
};
}
export function getInitialWindowBounds(
savedState: WindowState | null,
): InitialWindowBounds {
- const { workAreaSize } = screen.getPrimaryDisplay();
+ const primaryWorkArea = screen.getPrimaryDisplay().workAreaSize;
// No saved state → maximize by default for best first-launch experience
if (!savedState) {
return {
- width: workAreaSize.width,
- height: workAreaSize.height,
+ width: primaryWorkArea.width,
+ height: primaryWorkArea.height,
center: true,
isMaximized: true,
};
}
- let { width, height } = clampToWorkArea(savedState.width, savedState.height);
+ let { width, height } = clampToWorkArea({
+ width: savedState.width,
+ height: savedState.height,
+ });
+ const baseBounds: Rectangle = {
+ x: savedState.x,
+ y: savedState.y,
+ width,
+ height,
+ };
+ const { workAreaSize } = screen.getDisplayMatching(baseBounds);
// Scale up if saved bounds are much smaller than current work area
// This handles moving from a small screen to a large screen
const areaRatio =
(width * height) / (workAreaSize.width * workAreaSize.height);
- if (areaRatio < 0.6) {
+ if (areaRatio < SCALE_UP_AREA_RATIO_THRESHOLD) {
// Saved window covers less than 60% of current screen → scale up
// Use 90% of work area while maintaining aspect ratio
const savedAspect = width / height;
- const targetWidth = Math.round(workAreaSize.width * 0.9);
+ const targetWidth = Math.round(
+ workAreaSize.width * SCALE_UP_WORKAREA_RATIO,
+ );
const targetHeight = Math.round(targetWidth / savedAspect);
if (targetHeight <= workAreaSize.height) {
width = targetWidth;
height = targetHeight;
} else {
- height = Math.round(workAreaSize.height * 0.9);
+ height = Math.round(
+ workAreaSize.height * SCALE_UP_WORKAREA_RATIO,
+ );
width = Math.round(height * savedAspect);
}
}As per coding guidelines, Extract hardcoded magic numbers, strings, and enums to named constants at module top instead of leaving them inline in logic; Use object parameters for functions with 2 or more parameters instead of positional arguments.
🤖 Prompt for AI Agents
In `@apps/desktop/src/main/lib/window-state/bounds-validation.ts` around lines 73
- 93, Change scaling to use the work area of the display that contains the saved
bounds rather than the primary display: locate where workAreaSize is used and
replace it with a display-specific work area obtained from a helper (e.g.,
getDisplayForBounds or similar) using savedState.width/height and
savedState.x/y; extract the magic numbers into module-level constants (e.g.,
MIN_AREA_RATIO = 0.6, WORKAREA_SCALE = 0.9) and use them instead of 0.6/0.9; and
update clampToWorkArea to accept an object parameter (e.g., clampToWorkArea({
width, height })) and update its call site here accordingly. Ensure the scaling
math (savedAspect, targetWidth/targetHeight) uses the display-specific
workAreaSize and keep the same aspect-preserving logic.
Summary
prefers-reduced-motionCSS media query for accessibilityTest plan
SUPERSET_DISABLE_GPU=1escape hatch disables GPU flagsSummary by CodeRabbit
Release Notes
New Features
Improvements