feat: add dynamic macOS app icon switching #2107
Conversation
- Add icon assets (nightly, dark, light, pro) for dynamic switching - Create Rust icon module with NSApplication.setApplicationIconImage - Add set_app_icon, reset_app_icon, get_available_icons commands - Update store schema with app_icon field for persistence - Add AppIconSettings UI component in settings - Wire up config registry with side effect to apply icon changes Co-Authored-By: [email protected] <[email protected]>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
❌ Deploy Preview for hyprnote failed.
|
❌ Deploy Preview for hyprnote-storybook failed.
|
📝 WalkthroughWalkthroughAdds an app icon feature: a new Desktop UI component for selecting app icons, configuration persistence via registry and store schema, Rust plugin commands and icon management (macOS implementation; non-macOS no-op), and Specta command exposure. Changes
Sequence DiagramsequenceDiagram
participant User
participant UI as Desktop UI
participant Form as Form State
participant Registry as Config Registry
participant CMD as windowsCommands
participant Plugin as Windows Plugin
participant Platform as Platform (macOS / Other)
User->>UI: choose icon
UI->>Form: onChange(selectedIcon)
Form->>Form: update app_icon
Form->>Registry: setPartialValues({ app_icon })
Registry->>Registry: persist value & trigger side effect
Registry->>CMD: windowsCommands.setAppIcon(app_icon)
CMD->>Plugin: invoke set_app_icon(app_icon)
alt macOS
Plugin->>Plugin: load PNG (include_bytes)
Plugin->>Plugin: create NSImage on main thread
Plugin->>Platform: set application icon
Platform-->>Plugin: success / error
else Non-macOS
Plugin->>Platform: no-op
Platform-->>Plugin: ok
end
Plugin-->>CMD: Result<(), String>
CMD-->>Registry: command result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
plugins/windows/src/lib.rs (1)
5-5: Icon module export and command registration look correct — don’t forget codegen/permissionsThe
iconmodule is correctly added and re‑exported, and the three icon commands are wired intomake_specta_builder, so they’ll be available to JS once bindings are regenerated.Per the plugins guidelines, after changing commands in
plugins/windows/src/lib.rsyou should also:
- Re‑run whatever
codegenstep generates@hypr/plugin-windows.- Update
plugins/windows/permissions/default.toml.- Update
apps/desktop/src-tauri/capabilities/default.json.Based on learnings, this keeps the new commands usable and properly permissioned end‑to‑end.
Also applies to: 12-12, 59-62
apps/desktop/src/components/settings/general/app-icon.tsx (1)
1-52: Typed AppIconSettings UI looks good; minorcnstyle nitThe component is nicely typed against the plugin’s
AppIconunion, the options array is explicit, and the selection state/visual feedback work as expected.One small style nit: project guidelines prefer passing an array to
cnfor more complex class combinations. Here you could do something like:className={cn([ "flex flex-col items-center gap-2 p-3 rounded-lg border-2 transition-all", value === option.value ? "border-blue-500 bg-blue-50" : "border-neutral-200 hover:border-neutral-300 hover:bg-neutral-50", ])}Functionally everything is correct; this is just for consistency.
apps/desktop/src/components/settings/general/index.tsx (1)
5-5:app_iconis correctly threaded through SettingsGeneralIncluding
"app_icon"inuseConfigValues, wiring it intouseForm’sdefaultValues, and exposing it via a<form.Field name="app_icon">bound toAppIconSettingslines up cleanly with the new config key and registry side effect.Assuming
useConfigValuesalways surfaces a validAppIcon(via the Zod default), theas AppIconcast is fine. If there’s any chance ofvalue.app_iconbeingundefinedon older installs, you might optionally default it here (e.g.value.app_icon ?? "dark" as AppIcon) to keep the UI selection deterministic.Also applies to: 10-10, 24-24, 53-53, 154-161
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (9)
apps/desktop/src-tauri/icons/dynamic/dark.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/dynamic/light.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/dynamic/nightly.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/dynamic/pro.pngis excluded by!**/*.pngapps/desktop/src/assets/icons/dark.pngis excluded by!**/*.pngapps/desktop/src/assets/icons/light.pngis excluded by!**/*.pngapps/desktop/src/assets/icons/nightly.pngis excluded by!**/*.pngapps/desktop/src/assets/icons/pro.pngis excluded by!**/*.pngplugins/windows/js/bindings.gen.tsis excluded by!**/*.gen.ts
📒 Files selected for processing (7)
apps/desktop/src/components/settings/general/app-icon.tsx(1 hunks)apps/desktop/src/components/settings/general/index.tsx(4 hunks)apps/desktop/src/config/registry.ts(3 hunks)packages/store/src/schema-internal.ts(3 hunks)plugins/windows/src/commands.rs(2 hunks)plugins/windows/src/icon.rs(1 hunks)plugins/windows/src/lib.rs(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Agent implementations should use TypeScript and follow the established architectural patterns defined in the agent framework
Agent communication should use defined message protocols and interfaces
Files:
packages/store/src/schema-internal.tsapps/desktop/src/config/registry.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Avoid creating a bunch of types/interfaces if they are not shared. Especially for function props, just inline them instead.
Never do manual state management for form/mutation. Use useForm (from tanstack-form) and useQuery/useMutation (from tanstack-query) instead for 99% of cases. Avoid patterns like setError.
If there are many classNames with conditional logic, usecn(import from@hypr/utils). It is similar toclsx. Always pass an array and split by logical grouping.
Usemotion/reactinstead offramer-motion.
Files:
packages/store/src/schema-internal.tsapps/desktop/src/components/settings/general/app-icon.tsxapps/desktop/src/config/registry.tsapps/desktop/src/components/settings/general/index.tsx
plugins/*/src/lib.rs
📄 CodeRabbit inference engine (plugins/AGENTS.md)
After updating commands in
plugins/<NAME>/src/lib.rs, runcodegen, updateplugins/<NAME>/permissions/default.toml, andapps/desktop/src-tauri/capabilities/default.json
Files:
plugins/windows/src/lib.rs
🧠 Learnings (2)
📚 Learning: 2025-11-27T11:40:22.782Z
Learnt from: CR
Repo: fastrepl/hyprnote PR: 0
File: plugins/AGENTS.md:0-0
Timestamp: 2025-11-27T11:40:22.782Z
Learning: Applies to plugins/*/src/lib.rs : After updating commands in `plugins/<NAME>/src/lib.rs`, run `codegen`, update `plugins/<NAME>/permissions/default.toml`, and `apps/desktop/src-tauri/capabilities/default.json`
Applied to files:
plugins/windows/src/lib.rsplugins/windows/src/commands.rsplugins/windows/src/icon.rs
📚 Learning: 2025-11-24T16:32:23.055Z
Learnt from: CR
Repo: fastrepl/hyprnote PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T16:32:23.055Z
Learning: Applies to **/*.{ts,tsx} : Never do manual state management for form/mutation. Use useForm (from tanstack-form) and useQuery/useMutation (from tanstack-query) instead for 99% of cases. Avoid patterns like setError.
Applied to files:
apps/desktop/src/components/settings/general/index.tsx
🧬 Code graph analysis (5)
apps/desktop/src/components/settings/general/app-icon.tsx (2)
packages/store/src/schema-internal.ts (1)
AppIcon(11-11)packages/utils/src/cn.ts (1)
cn(20-22)
apps/desktop/src/config/registry.ts (1)
packages/store/src/schema-internal.ts (1)
AppIcon(11-11)
plugins/windows/src/lib.rs (2)
plugins/windows/src/commands.rs (3)
set_app_icon(111-113)reset_app_icon(117-119)get_available_icons(123-125)plugins/windows/src/icon.rs (4)
set_app_icon(26-54)set_app_icon(57-59)reset_app_icon(62-74)reset_app_icon(77-79)
plugins/windows/src/icon.rs (2)
packages/store/src/schema-internal.ts (1)
AppIcon(11-11)plugins/windows/src/commands.rs (2)
set_app_icon(111-113)reset_app_icon(117-119)
apps/desktop/src/components/settings/general/index.tsx (2)
packages/store/src/schema-internal.ts (1)
AppIcon(11-11)apps/desktop/src/components/settings/general/app-icon.tsx (1)
AppIconSettings(21-52)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Devin
🔇 Additional comments (4)
packages/store/src/schema-internal.ts (1)
10-12: AppIcon schema and Tinybase mapping look consistentThe
appIconSchemavalues match the RustAppIcon’ssnake_caseserialization, and storingapp_iconas astringin the Tinybase schema is appropriate given the Zod enum. The"dark"default provides a sane baseline for both new and migrated data.Also applies to: 30-30, 71-71
apps/desktop/src/config/registry.ts (1)
8-11:app_iconregistry entry is correctly typed and wired to the pluginThe new
app_iconconfig key, itsAppIcon-typed default, and thesideEffectdelegating towindowsCommands.setAppIconare all consistent with the plugin API and the store schema. Calling into the plugin on non‑macOS targets is safe given the no‑op stubs, so this wiring looks good.Also applies to: 28-29, 163-169
plugins/windows/src/commands.rs (1)
1-1: Icon commands cleanly wrap the icon module APIThe new
set_app_icon,reset_app_icon, andget_available_iconscommands are straightforward wrappers over theiconmodule andAppIcon::all(), matching the signatures exposed inlib.rs. This keeps the JS surface small and leverages the shared enum nicely.Also applies to: 109-125
plugins/windows/src/icon.rs (1)
1-79: AppIcon enum and macOS implementation look sound and align with the TS sideThe
AppIconenum (withsnake_caseserde) plusall()provides a clean, serializable surface that matches the Zod/TSAppIconvalues consumed in the store and UI. The macOSset_app_icon/reset_app_iconimplementations correctly guard onMainThreadMarkerand useinclude_bytes!so the PNG assets are embedded at compile time, while the non‑macOS stubs safely no‑op.This is a solid foundation for the dynamic icon feature.
- Add get_build_channel command to detect nightly/stable/staging/dev - Update AppIconSettings to filter icons based on build channel and pro status - Nightly builds: only nightly icon - Stable builds: dark and light icons - Pro users on stable: all icons (dark, light, nightly, pro) - Auto-reset to valid icon if current selection becomes unavailable Co-Authored-By: [email protected] <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
apps/desktop/src/components/settings/general/app-icon.tsx (5)
24-37: Confirm icon availability rules for nightly channel and section visibility
getAvailableIconsForTierreturns only["nightly"]whenchannel === "nightly"(Line 28–29), regardless ofisPro, andvisibleOptions.length <= 1then hides the whole App Icon section (Lines 73–75). That means on nightly builds:
- Pro users can’t select Dark/Light/Pro at all.
- The settings UI disappears entirely instead of showing a read‑only “Nightly” preview.
If that’s intentional, this is fine; if Pro/nightly should still allow other icons or at least a visible preview, you may want to adjust either
getAvailableIconsForTieror thevisibleOptions.lengthguard.Also applies to: 73-75
45-47: Guard against overwriting a persisted Pro icon while billing state is still loadingWith
availableIconsderived fromchannel+isProand the effect that auto‑fixes invalid values (Lines 64–71), there’s a potential UX issue ifisProis initially false while billing is loading:
- Suppose a Pro user has
value === "pro"persisted.- On first mount,
isProdefaults to false, soavailableIconsis["dark","light"].- The effect sees
!availableIcons.includes("pro")and callsonChange(availableIcons[0]), resetting the icon (probably to"dark").- When
isProlater flips true,valueis already"dark", so the user never automatically gets their Pro icon back.If
useBillingAccessdoesn’t guarantee a fully resolvedisProbefore this component mounts, consider gating the correction effect on a “billing loaded” flag or only auto‑downgrading when you know the user is definitively non‑Pro.Also applies to: 54-57, 64-71
48-52: Handle getBuildChannel failures to avoid unhandled Promise rejections
windowsCommands.getBuildChannel()is used with.thenonly and no.catch(Lines 48–52). If that command ever rejects, you’ll get an unhandled rejection andchannelwill silently stay"dev".Consider adding basic error handling while keeping
"dev"as a safe default, for example:useEffect(() => { - windowsCommands.getBuildChannel().then((ch) => { - setChannel(ch as BuildChannel); - }); + void windowsCommands + .getBuildChannel() + .then((ch) => { + setChannel(ch as BuildChannel); + }) + .catch((err) => { + console.error("Failed to get build channel", err); + }); }, []);
89-94: Aligncnusage with project style (array-based grouping)Per the coding guidelines,
cnshould be called with an array and grouped logically. Right now it’s using separate string arguments (Lines 89–94).You could rewrite it along these lines:
- className={cn( - "flex flex-col items-center gap-2 p-3 rounded-lg border-2 transition-all", - value === option.value - ? "border-blue-500 bg-blue-50" - : "border-neutral-200 hover:border-neutral-300 hover:bg-neutral-50", - )} + className={cn([ + "flex flex-col items-center gap-2 p-3 rounded-lg border-2 transition-all", + value === option.value + ? "border-blue-500 bg-blue-50" + : "border-neutral-200 hover:border-neutral-300 hover:bg-neutral-50", + ])}As per coding guidelines, this keeps conditionals and base styles clearly grouped.
22-26: Consider reusing a shared BuildChannel type from the plugin (if available)You define a local
BuildChannelunion (Line 22) and then cast the result ofwindowsCommands.getBuildChannel()to it (Line 50). If@hypr/plugin-windowsalready exposes a typed build‑channel enum/union, importing that instead of maintaining a separate union here would avoid drift between the command’s return type and this file.If no such type exists, this is fine as‑is.
Also applies to: 45-51
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
plugins/windows/js/bindings.gen.tsis excluded by!**/*.gen.ts
📒 Files selected for processing (3)
apps/desktop/src/components/settings/general/app-icon.tsx(1 hunks)plugins/windows/src/commands.rs(2 hunks)plugins/windows/src/lib.rs(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- plugins/windows/src/commands.rs
- plugins/windows/src/lib.rs
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Avoid creating a bunch of types/interfaces if they are not shared. Especially for function props, just inline them instead.
Never do manual state management for form/mutation. Use useForm (from tanstack-form) and useQuery/useMutation (from tanstack-query) instead for 99% of cases. Avoid patterns like setError.
If there are many classNames with conditional logic, usecn(import from@hypr/utils). It is similar toclsx. Always pass an array and split by logical grouping.
Usemotion/reactinstead offramer-motion.
Files:
apps/desktop/src/components/settings/general/app-icon.tsx
🔇 Additional comments (1)
apps/desktop/src/components/settings/general/app-icon.tsx (1)
39-42: Component structure and selection UX look solidProp surface (
value+onChange), memoization ofavailableIcons/visibleOptions, and the grid‑based button UI (Lines 39–42 and 77–105) are all clear and straightforward. The selected‑state styling and copy make the behavior obvious to users.Also applies to: 77-105
Summary
Adds the ability for users to switch between 4 app icon variants (Dark, Light, Nightly, Pro) in the Settings panel. The icon change is applied immediately using macOS's
NSApplication.setApplicationIconImageAPI and persists across app restarts via the store.Changes:
apps/desktop/src-tauri/icons/dynamic/iconmodule in the windows plugin withset_app_icon,reset_app_icon, andget_available_iconscommandsapp_iconfield to the store schema with persistenceAppIconSettingsUI component in the general settings sectionReview & Testing Checklist for Human
NSApplication.setApplicationIconImagewhich is macOS-specific and could not be tested on my Linux environment. Verify the icon actually changes in the Dock when selecting different options.MainThreadMarker. If you see "Must be called from main thread" errors, the command may need to be wrapped withtauri::async_runtime::spawn_blockingor similar.Recommended test plan:
Notes
src-tauri/icons/dynamic/for Rustinclude_bytes!andsrc/assets/icons/for React UI previews)include_bytes!macro uses relative paths from the plugin directory which could be fragile if the project structure changesLink to Devin run: https://app.devin.ai/sessions/41c336d8475546b29a6bbdc68d920b04
Requested by: [email protected] (@ComputelessComputer)