feat(desktop): let the app icon differ between light and dark - #3941
Merged
Conversation
6 tasks
The icon set assumed one tile serves every context, but a tile built for a light dock disappears against a dark one and vice versa — the two dark-ground colourways were unusable for anyone on a light system, and the light ones washed out on a dark one. Appearance now selects the icon: - `appearance.appIconDark` is an optional second slot. Absent means "one icon everywhere", which is what every settings file written before this says, so an upgrade keeps showing the icon the user picked rather than silently gaining a second one. `normalizeSettings` reads that slot from the raw input rather than from the defaults-merged object, which is the only way absent can survive normalization. - The dock re-resolves on OS appearance flips. Nothing in the settings object moves when the system toggles, so the fingerprint comparison that guards every other client-settings effect reports "no change"; the icon is wired to `nativeTheme` separately and no-ops when the resolved tile is already up. - Windows are born with the tile for the appearance they open into, and the synchronous startup tile follows the OS, so a default install in dark mode no longer flashes the light icon before settings load. The shipped set grows to 41 colourways of the geometric mark, and the default is now `sky` in light and `ink` in dark. Note that `default` is no longer the default: that id is now one selectable icon among many (the original mascot mark), while what a fresh install gets is a separate decision. Every fallback that previously named the `default` id now names the constant instead, so changing the default again touches one line. Discussed on GitHub Discussions apache#3604. Generated-by: Claude Code
light-sky, baby and periwinkle were cut from the set during apache#3431 review as near-duplicates of sky, and re-adding them here was an oversight of that decision rather than a reversal of it. The blue family stays at sky, cyan, ice and pale-inverted. Also corrects the ordering note: the list follows the order the icon discussion used, but is not one-to-one with its numbering now that three entries are absent, so a number from that thread maps to a tile by id rather than by position. Generated-by: Claude Code
A fresh install now uses one icon in both appearances rather than two. The dark default is still `ink`, but it is offered rather than applied: the dark slot stays empty until the user turns the split on, and turning it on seeds that slot with `ink` and moves the picker to it, so they land on a sensible dark tile already selected instead of on a copy of the light one. This keeps a default install to a single tile — the same thing an existing settings file gets — so the only people who see two icons are the ones who asked for two. `startupAppIcon` now derives from the default appearance instead of restating the light/dark pair, since it has to agree with what a fresh install resolves to and a second copy of that decision is how the two drift apart. Generated-by: Claude Code
…table Review of apache#3916 found the change did not do what it says on two counts. The packaged bundle still pointed at assets/icon.png, so Finder, Launchpad, the .app/.exe and the installer all kept the old mascot; only the running process ever saw the new default. Both mac and win icons now name the shipped default, with a test that fails if DEFAULT_APP_ICON changes without the packaging config following — the config is read before the workspace is built, so it cannot import the constant. The artwork arrived as opaque PNGs with no way to regenerate or audit them, which is the opposite of the reproducibility the icon discussion adopted them for. The generator that produced them now ships alongside: the mark is a handful of stroked polylines defined by constants, so the provenance of the artwork is the source. `--check` re-renders and compares bytes, and a test runs it over a sample covering flat fills, angled gradients and stroke gradients, skipping cleanly where no python3 is on PATH. Two smaller correctness fixes in the same area: - Removing a custom icon only ever examined `appearance.appIcon`. An icon used only in dark mode was deleted from disk while `appIconDark` kept naming it — the dock fell back, but the setting pointed at nothing and the picker had no valid selection. Both slots are now checked, and separately, since `updateIf` takes a fixed patch and only the slots that actually match may be reset. - The generic settings channel filtered out `appIcon` but not `appIconDark`, so a write could reach the second slot without queueing behind the icon seam's import/removal serialization. It names artwork on the same terms and is now filtered the same way. Also pins the upgrade behaviour that review flagged as unclear: a settings file that recorded a choice keeps it, and one that never recorded a choice takes the new default. The second is deliberate — it is how a default changes for people who were shown the old mark rather than choosing it — and is now a test rather than an accident of normalization. Generated-by: Claude Code
Two follow-ups from review of apache#3916. Removing a custom icon held in both slots used two independent conditional writes. If the second failed after the first committed, the light slot was already persisted as the default while the dark slot still named deleted artwork, `applySettings` never ran, and the handler reported `reset_failed` over a state it had half changed — the persisted and live views disagreeing with no way back. `updateIf` now accepts a patch derived from the state its predicate just accepted, so a caller that must touch several fields but only the ones that matched can still do it in one queued write. Removal uses that: both slots move together or neither does. A test proves it by failing the write and asserting neither slot moved, nothing reached the dock, and the artwork survives. The two artwork drift checks also ran nowhere. `scripts/*.test.mjs` are named explicitly in ci.yml rather than discovered, and `verify-packaged-app-icons` had never been listed — so a change to the generator, to `DEFAULT_APP_ICON`, or to the packaging config could go green while the artwork it names no longer matched. Both now run after Build. Confirmed against the planner that an artwork-only diff still routes to `code=true`, so a PNG changed on its own reaches the step that would catch it. Generated-by: Claude Code
The drift check compared four tiles byte-for-byte and only checked filenames for the other 34. That does not deliver what the check exists to claim: any of the unsampled PNGs could be edited byte-wise, or a colourway constant could change without its artwork being regenerated, and CI would stay green. The check now covers the whole generated catalogue, and asserts the reported count matches the number of generated ids so it cannot silently narrow again. Rendering 38 tiles at 1024px is ~90s of single-threaded signed-distance evaluation, which is the reason it was a sample in the first place. The work is per-icon and pure, so the script now renders across all cores: ~16s on an 8-core machine, which is affordable in CI. Verified it catches both drift modes it is meant to catch — a byte edited in a previously unsampled PNG, and a colourway constant changed by one hex digit without regenerating — each turning the suite red. Generated-by: Claude Code
The subset comment and the module docstring still described the test as checking "a handful" of tiles because rendering all of them was too slow. Both stopped being true when the check went exhaustive and the renderer went parallel, leaving the source contradicting the test beside it. Generated-by: Claude Code
ARE404
force-pushed
the
feat/icon-theme-modes
branch
from
August 26, 2026 16:38
5c8fc72 to
8d1c8be
Compare
Astro-Han
approved these changes
Aug 26, 2026
Astro-Han
left a comment
Astro-Han
left a comment
Contributor
There was a problem hiding this comment.
LGTM. The feature has been discussed openly in discussion.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The app icon could not follow appearance: one tile has to serve both docks, and
a pale tile disappears on a light dock while a dark one disappears on a dark
one. This adds an optional second icon for dark mode, replaces the shipped
default, and grows the set to 38 colourways of the geometric mark.
Refs #3604 — the discussion that adopted the mark and settled the default.
The two highest-scoring proposals there were both pairs (1+8 and 36+18),
which is the same request arriving from people who picked different colours.
Defaults. A fresh install gets
sky, used in both appearances: thesplit ships switched off, so a default install shows one tile everywhere.
inkis the dark recommendation, seeded into the dark slot when the user turnsthe split on rather than applied for them.
defaultis no longer the default. That id is now one selectable iconamong many (the original mascot mark), while what a fresh install gets is a
separate decision. Every fallback that named the id now names
DEFAULT_APP_ICON, so changing the default again is a one-line edit — thepackaging config is the one place that still hardcodes it, because it is read
before the workspace is built, and a test fails if the two drift.
Upgrades. A settings file that recorded a choice keeps it. One that never
recorded a choice takes the new default — deliberate, and how a default changes
for people who were shown the old mark rather than choosing it. Both are tests.
Verification
Run from a clean tree in the worktree, at head
a10626971:npm run build(root)npm run typecheck(root)npm run lint/npm run format:checknpm --workspace @maka/core testnpm --workspace @maka/storage testnpm --workspace @maka/desktop testnode --test scripts/verify-packaged-app-icons.test.mjsnode --test scripts/generate-app-icons.test.mjsnpx knip --workspace apps/desktopnpx knip --workspace packages/uinode scripts/asf-license-headers.mjs checkBoth artwork drift checks now run in CI after Build — they were listed
nowhere before, and
scripts/*.test.mjsare named explicitly inci.ymlrather than discovered, so a change to the generator, to
DEFAULT_APP_ICON,or to the packaging config could previously go green while the artwork it
names no longer matched. Verified against the CI planner that an artwork-only
diff still routes to
code=true.The generator check covers every shipped tile, not a sample, and asserts
the compared count matches the number of generated ids so it cannot silently
narrow. Rendering all 38 at 1024px is ~90s single-threaded, so the script
renders across all cores (~16s). Confirmed it catches both drift modes: a byte
edited in a PNG, and a colourway constant changed by one hex digit without
regenerating.
New coverage: per-appearance resolution, the upgrade cases above, clearing the
dark slot, a failed removal committing nothing (neither slot moves, nothing
reaches the dock, the artwork survives), an OS theme flip re-applying the icon, a flip changing nothing when
the split is off, an explicit
darkpreference ignoring the OS, removal of acustom icon used only in dark mode, and the generic settings channel refusing
both icon slots.
Not run: the packaging pipeline itself (
electron-builder) — no signingmaterial locally, so the bundle icon change is covered by the config test
rather than by a built artifact.
Manually exercised in a local dev build — Settings → Appearance, confirmed
visually:
before
after
skyselectedfollows the selected target
carry their zh copy with no missing strings
(An earlier revision of this description claimed the section failed to render.
That was my misreading of a screenshot, not a defect — it renders correctly.)
Evidence gap: the dock tile itself. A dev run hides the dock icon, so that
one surface is covered by unit tests rather than a capture.
Artwork
24 new PNGs. The generator that produces them ships in this PR
(
scripts/generate-app-icons.py): the mark is a handful of stroked polylinesdefined by constants, not traced, not drawn over stock art, and not generated
by a model — so the provenance of the artwork is the source.
--checkre-renders and compares bytes; a test runs it over a sample covering flat
fills, angled gradients and stroke gradients.
Geometry was measured against shipped system icons before adoption: block is
824/1024 (80.5% of canvas) with a 9.77% margin on all four sides, and the
corner profile matches Chrome and Notes to within a pixel at 256px.
AI use
Select exactly one:
Tool(s) and scope: Claude Code — wrote the implementation, tests and artwork
generator in this PR under my direction, and ran the checks listed above. I
reviewed every change. All commits carry
Generated-by: Claude Code.Checklist
Does this PR entail a change in behavior?