Brand Refresh#3130
Conversation
📝 WalkthroughWalkthroughAdds conditional DMG background image support to Electron Builder and replaces workspace-name-based dock icon bitmap customization with build-type-based icon selection (dev/canary/fallback), removing OKLCH/color-hash and per-pixel border drawing. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ 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 |
Greptile SummaryThis PR introduces a brand refresh for the Superset desktop app, replacing the previous runtime pixel-manipulation approach (workspace-coloured borders drawn on the base icon) with pre-designed, per-build-type PNG assets ( Key changes:
Confidence Score: 4/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A([setWorkspaceDockIcon called]) --> B{platform === darwin?}
B -- No --> Z([no-op])
B -- Yes --> C[getIconPath]
C --> D[getIconsDir]
D --> E{app.isPackaged?}
E -- Yes --> F[process.resourcesPath/app.asar/resources/build/icons]
E -- No --> G{NODE_ENV === development?}
G -- Yes --> H[app.getAppPath/src/resources/build/icons]
G -- No --> I[__dirname/../resources/build/icons]
F & H & I --> J{NODE_ENV === development?}
J -- Yes --> K{existsSync icon-dev.png?}
K -- Yes --> L[return icon-dev.png]
K -- No --> M[return icon.png]
J -- No --> N{isCanaryBuild?}
N -- Yes --> O{existsSync icon-canary.png?}
O -- Yes --> P[return icon-canary.png]
O -- No --> M
N -- No --> M
L & P & M --> Q[nativeImage.createFromPath]
Q --> R{icon.isEmpty?}
R -- Yes --> S([warn and return])
R -- No --> T[app.dock.setIcon]
Reviews (1): Last reviewed commit: "feat(desktop): custom DMG installer back..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| function isCanaryBuild(): boolean { | ||
| const components = prerelease(app.getVersion()); | ||
| return components !== null && components.length > 0; | ||
| } |
There was a problem hiding this comment.
isCanaryBuild matches all prerelease channels
semver.prerelease() returns a truthy array for any prerelease tag (e.g. alpha, beta, rc, not just canary). If the project ever ships an alpha or rc build using a version string like 0.0.53-beta.1, this function will return true and the canary dock icon will be shown for those builds too.
A tighter check that matches the intent documented in the JSDoc:
| function isCanaryBuild(): boolean { | |
| const components = prerelease(app.getVersion()); | |
| return components !== null && components.length > 0; | |
| } | |
| function isCanaryBuild(): boolean { | |
| const components = prerelease(app.getVersion()); | |
| return components !== null && components.some((c) => String(c).toLowerCase() === "canary"); | |
| } |
| * Sets the macOS dock icon based on the current build type and macOS version. | ||
| * No-op on non-macOS platforms. | ||
| */ | ||
| export function setWorkspaceDockIcon(): void { |
There was a problem hiding this comment.
The function was previously responsible for setting a workspace-coloured border on the dock icon, but that concept has been fully removed. The new behaviour is simply "set the appropriate dock icon for this build type." The exported name setWorkspaceDockIcon will confuse future callers (and any documentation) — consider renaming it to setDockIcon or setBuildTypeDockIcon.
| export function setWorkspaceDockIcon(): void { | |
| export function setDockIcon(): void { |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/desktop/src/main/lib/dock-icon.ts`:
- Around line 15-26: getIconsDir currently falls back to a path built from
__dirname which fails in non-packaged, non-development runs because build/icons
isn’t copied into dist; update getIconsDir to use app.getAppPath() for the
fallback (instead of join(__dirname, "../resources/build/icons")) so the path
resolves the same in preview/dev/packaged modes (reference function getIconsDir,
app.isPackaged, env.NODE_ENV, and __dirname), or alternatively ensure
copyResourcesPlugin copies src/resources/build/icons into dist so the existing
__dirname-based fallback remains valid.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d38f6353-d126-44f2-8834-805659ab670d
⛔ Files ignored due to path filters (7)
apps/desktop/src/resources/build/icons/icon-canary.icois excluded by!**/*.icoapps/desktop/src/resources/build/icons/icon-canary.pngis excluded by!**/*.pngapps/desktop/src/resources/build/icons/icon-dev.icois excluded by!**/*.icoapps/desktop/src/resources/build/icons/icon-dev.pngis excluded by!**/*.pngapps/desktop/src/resources/build/icons/icon.icois excluded by!**/*.icoapps/desktop/src/resources/build/icons/icon.pngis excluded by!**/*.pngapps/desktop/src/resources/build/installer/background.tiffis excluded by!**/*.tiff
📒 Files selected for processing (5)
apps/desktop/electron-builder.tsapps/desktop/src/main/lib/dock-icon.tsapps/desktop/src/resources/build/icons/icon-canary.icnsapps/desktop/src/resources/build/icons/icon-dev.icnsapps/desktop/src/resources/build/icons/icon.icns
| /** | ||
| * Gets the path to the app icon PNG. | ||
| * Returns the icons directory path. | ||
| */ | ||
| function getIconPath(): string { | ||
| function getIconsDir(): string { | ||
| if (app.isPackaged) { | ||
| return join( | ||
| process.resourcesPath, | ||
| "app.asar/resources/build/icons/icon.png", | ||
| ); | ||
| return join(process.resourcesPath, "app.asar/resources/build/icons"); | ||
| } | ||
|
|
||
| if (env.NODE_ENV === "development") { | ||
| return join(app.getAppPath(), "src/resources/build/icons/icon.png"); | ||
| return join(app.getAppPath(), "src/resources/build/icons"); | ||
| } | ||
|
|
||
| return join(__dirname, "../resources/build/icons/icon.png"); | ||
| return join(__dirname, "../resources/build/icons"); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify the directory structure to ensure the fallback path is valid.
# Expected: The dist folder should have a structure where resources/build/icons
# is accessible via ../resources/build/icons from the lib folder.
# Check the typical dist structure for Electron main process files
fd -t f "dock-icon" --extension ts --extension js -x dirname {} \;
# Check if resources structure exists in expected locations
tree -L 4 -d apps/desktop/src/resources 2>/dev/null || echo "src/resources not found"
tree -L 4 -d apps/desktop/dist/resources 2>/dev/null || echo "dist/resources not found (may not be built)"Repository: superset-sh/superset
Length of output: 336
🏁 Script executed:
#!/bin/bash
# Check build configuration files in apps/desktop
find apps/desktop -maxdepth 2 -type f \( -name "package.json" -o -name "tsconfig*.json" -o -name "vite.config.*" -o -name "webpack.config.*" -o -name "*.config.js" \)Repository: superset-sh/superset
Length of output: 116
🏁 Script executed:
#!/bin/bash
# Check the package.json build scripts
cat apps/desktop/package.json | head -50Repository: superset-sh/superset
Length of output: 2336
🏁 Script executed:
#!/bin/bash
# Check tsconfig.json for output directory
cat apps/desktop/tsconfig.jsonRepository: superset-sh/superset
Length of output: 471
🏁 Script executed:
#!/bin/bash
# Check electron-vite configuration
find apps/desktop -name "electron.vite.config.*" -o -name "vite.config.*" | head -5Repository: superset-sh/superset
Length of output: 100
🏁 Script executed:
#!/bin/bash
# Look at electron-vite config if it exists
cat apps/desktop/electron.vite.config.ts 2>/dev/null | head -100Repository: superset-sh/superset
Length of output: 3434
🏁 Script executed:
#!/bin/bash
# Check if there's a script that copies resources to dist
grep -r "resources" apps/desktop/scripts --include="*.ts" --include="*.js" 2>/dev/null | head -20Repository: superset-sh/superset
Length of output: 173
🏁 Script executed:
#!/bin/bash
# Check the vite helpers to see what copyResourcesPlugin does
cat apps/desktop/vite/helpers.ts 2>/dev/null || cat apps/desktop/vite/helpers.js 2>/dev/null || find apps/desktop/vite -name "*.ts" -o -name "*.js" | xargs ls -laRepository: superset-sh/superset
Length of output: 2607
🏁 Script executed:
#!/bin/bash
# Also check what the resources variable is used for
cat apps/desktop/vite/helpers/copyResourcesPlugin.ts 2>/dev/null || find apps/desktop -name "*copyResources*"Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
#!/bin/bash
# Verify that build/icons is indeed not in the copy list by checking the entire flow
cat apps/desktop/vite/helpers.ts | grep -A 50 "RESOURCES_TO_COPY"Repository: superset-sh/superset
Length of output: 1943
🏁 Script executed:
#!/bin/bash
# Check if there's any other mechanism that copies build/ or icons
grep -r "build/icons" apps/desktop --include="*.ts" --include="*.js" --exclude-dir=node_modules | head -20Repository: superset-sh/superset
Length of output: 1012
🏁 Script executed:
#!/bin/bash
# Check the electron-builder config to see if icons are handled separately
find apps/desktop -name "electron-builder*" -type f | head -5Repository: superset-sh/superset
Length of output: 136
🏁 Script executed:
#!/bin/bash
# Check the electron-builder config to see how resources are handled
cat apps/desktop/electron-builder.ts 2>/dev/null | head -150Repository: superset-sh/superset
Length of output: 4622
🏁 Script executed:
#!/bin/bash
# Check specifically how pkg.resources is used
cat apps/desktop/electron-builder.ts | grep -B 5 -A 5 "pkg.resources"Repository: superset-sh/superset
Length of output: 1474
The fallback path will fail in non-packaged, non-development scenarios.
The third condition (line 25) is triggered when the app is not packaged and not in development mode—such as when running the built output in preview mode. In this case, __dirname resolves to dist/main/lib, and the path ../resources/build/icons becomes dist/resources/build/icons.
However, the build configuration only copies specific resources to dist via copyResourcesPlugin (sounds, tray, browser-extension, migrations, host-migrations, templates), not the build/icons directory. The icons directory only exists in src/resources and is copied to the package by electron-builder during the final packaging step. This means the fallback path will fail when trying to access icons.
Consider using app.getAppPath() for the fallback, which should work consistently across packaging modes, or add build/icons to the resources copied in copyResourcesPlugin.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/main/lib/dock-icon.ts` around lines 15 - 26, getIconsDir
currently falls back to a path built from __dirname which fails in non-packaged,
non-development runs because build/icons isn’t copied into dist; update
getIconsDir to use app.getAppPath() for the fallback (instead of join(__dirname,
"../resources/build/icons")) so the path resolves the same in
preview/dev/packaged modes (reference function getIconsDir, app.isPackaged,
env.NODE_ENV, and __dirname), or alternatively ensure copyResourcesPlugin copies
src/resources/build/icons into dist so the existing __dirname-based fallback
remains valid.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
…ution Replaces the workspace-name-colored border overlay with explicit icon files per build type. Resolution order: icon-dev.png → icon.png (dev), icon-canary.png → icon.png (canary), icon.png (stable).
Adds support for a custom background.tiff in the DMG installer window. Falls back to electron-builder's default if the file is absent.
michalkopanski
force-pushed
the
brand-refresh
branch
from
ad8a027 to
c4fc5e5
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
apps/desktop/src/main/lib/dock-icon.ts (1)
18-25:⚠️ Potential issue | 🟠 MajorPreview-mode fallback won't find the dock icons.
Line 25 resolves non-packaged, non-development runs relative to
__dirname. In preview/non-packaged production runs that path points at the built main output, butsrc/resources/build/iconsis not copied there, sonativeImage.createFromPath()gets a missing file and the branded dock icon never loads. Please either make this branch resolve from a path that exists in preview too, or copybuild/iconsinto the preview resource set.Expected result: the fallback branch points at
resources/build/icons, but the preview/dist resource-copy config does not includebuild/icons.#!/bin/bash set -euo pipefail echo "[dock-icon fallback branch]" sed -n '18,26p' apps/desktop/src/main/lib/dock-icon.ts echo echo "[desktop build-resource wiring]" rg -n -C3 'RESOURCES_TO_COPY|copyResourcesPlugin|build/icons|src/resources' apps/desktop --glob '*.ts' --glob 'package.json'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/main/lib/dock-icon.ts` around lines 18 - 25, The fallback branch in getIconsDir currently returns a path based on __dirname which doesn't contain src/resources in preview runs; update the final return to point at the runtime resources folder instead (e.g. return join(process.resourcesPath, "resources/build/icons")) so nativeImage.createFromPath can find icons in preview/non-dev runs; modify getIconsDir (and keep existing app.isPackaged/app.getAppPath branches) to use process.resourcesPath for the non-packaged non-development fallback.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@apps/desktop/src/main/lib/dock-icon.ts`:
- Around line 18-25: The fallback branch in getIconsDir currently returns a path
based on __dirname which doesn't contain src/resources in preview runs; update
the final return to point at the runtime resources folder instead (e.g. return
join(process.resourcesPath, "resources/build/icons")) so
nativeImage.createFromPath can find icons in preview/non-dev runs; modify
getIconsDir (and keep existing app.isPackaged/app.getAppPath branches) to use
process.resourcesPath for the non-packaged non-development fallback.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c7fcd31f-f9fe-4b75-bc4e-20525db09444
⛔ Files ignored due to path filters (11)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/assets/superset-empty-state-wordmark.svgis excluded by!**/*.svgapps/desktop/src/resources/build/icons/icon-canary.icois excluded by!**/*.icoapps/desktop/src/resources/build/icons/icon-canary.pngis excluded by!**/*.pngapps/desktop/src/resources/build/icons/icon-dev.icois excluded by!**/*.icoapps/desktop/src/resources/build/icons/icon-dev.pngis excluded by!**/*.pngapps/desktop/src/resources/build/icons/icon.icois excluded by!**/*.icoapps/desktop/src/resources/build/icons/icon.pngis excluded by!**/*.pngapps/desktop/src/resources/build/installer/background.tiffis excluded by!**/*.tiffapps/desktop/src/resources/tray/iconTemplate.pngis excluded by!**/*.pngapps/marketing/public/favicon-192.pngis excluded by!**/*.pngapps/marketing/public/title.svgis excluded by!**/*.svg
📒 Files selected for processing (5)
apps/desktop/electron-builder.tsapps/desktop/src/main/lib/dock-icon.tsapps/desktop/src/resources/build/icons/icon-canary.icnsapps/desktop/src/resources/build/icons/icon-dev.icnsapps/desktop/src/resources/build/icons/icon.icns
|
Closing in favor of #3367 — couldn't push to this branch, reopened the work there. |
2 participants
Summary
icon.png/icns/ico,icon-canary,icon-dev) with updated logo treatmenticonTemplate.png) with new designbackground.tiff) for macOSicon-dev.png, canary →icon-canary.png, stable →icon.pngtitle.svg) andfavicon-192.pngwith new logo designsuperset-empty-state-wordmark.svg) with new logo and dimensionsDesign Updates
How It Works
Build-type detection uses
semverprerelease tags — dev builds pickicon-dev, canary picksicon-canary, stable falls back toicon.png. The dock icon setter was simplified to set the image as-is (removed workspace-name border overlay and image processing).Test plan
icon-dev.pngin the dockicon-canary.pngin the dockicon.pngin the dockTesting
bun run typecheckbun run lintSummary by CodeRabbit