Menu

#411 feat(dashboard): extract real per-app icons for Time-by-app

closed
nobody
None
2026-07-09
2026-07-08
Anonymous
No

Originally created by: adityaharishch

Summary

  • New get_app_icon Tauri command resolves an installed .app bundle via NSWorkspace, extracts its NSImage, caches a PNG under ~/.meridian/icon-cache/, served to the webview through the asset protocol (bridge.ts's assetUrl, ui/lib/app-icons.ts's useAppIconUrl)
  • Adds a curated BRAND_ICONS wordmark table (via simple-icons) as a mid-tier fallback between the real icon and the letter monogram
  • AppGlyph now renders: real icon → brand wordmark → monogram, in that priority order

Stacked on prior PRs in this chain.

Test plan

  • [x] cargo build -p meridian-tray, cargo clippy -- -D warnings
  • [x] npm run build in ui/

Related

Tickets: #415

Discussion

  • Anonymous

    Anonymous - 2026-07-08

    Originally posted by: coderabbitai[bot]

    [!IMPORTANT]

    Review skipped

    Auto reviews are disabled on base/target branches other than the default branch.

    Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.


    ⚙️ Run configuration

    Configuration used: Organization UI

    Review profile: ASSERTIVE

    Plan: Pro Plus

    Run ID: 2725b37b-201d-4efe-ae5f-ff7a67779def

    You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

    Use the checkbox below for a quick retry:
    - [ ] 🔍 Trigger review

    ✨ Finishing Touches
    🧪 Generate unit tests (beta) - [ ] Create PR with unit tests - [ ] Commit unit tests in branch `feat/app-icon-extraction`

    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.

    ❤️ Share - [X](https://twitter.com/intent/tweet?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A&url=https%3A//coderabbit.ai) - [Mastodon](https://mastodon.social/share?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A%20https%3A%2F%2Fcoderabbit.ai) - [Reddit](https://www.reddit.com/submit?title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&text=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code.%20Check%20it%20out%3A%20https%3A//coderabbit.ai) - [LinkedIn](https://www.linkedin.com/sharing/share-offsite/?url=https%3A%2F%2Fcoderabbit.ai&mini=true&title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&summary=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code)

    Comment @coderabbitai help to get the list of available commands.

     
  • Anonymous

    Anonymous - 2026-07-08

    Originally posted by: Akarsh-Hegde

    Review — extract real per-app icons for Time-by-app

    Summary: A cleanly-scoped, well-documented feature that swaps letter monograms for real .app-bundle icons (with a vendored brand-wordmark mid-tier fallback); no blocking issues found.

    Blocking / correctness issues

    None. I checked the highest-risk item — the asset protocol. The <img src> relies on convertFileSrc + Tauri's asset protocol, which needs security.assetProtocol.enable + a scope covering the cache dir. This diff doesn't touch tauri.conf.json, but its base branch already sets assetProtocol.enable: true with scope: ["$HOME/.meridian/icon-cache/*"], and the cache writes flat files (icon-cache/<key>.png), so the single-level * glob matches. Custom #[tauri::command]s need no capability entry in Tauri v2, so registering get_app_icon in invoke_handler! (lib.rs:494) is complete.

    Suggestions / nits

    • simple-icons is a runtime dependency but never imported (ui/package.json). The path data in ui/lib/brand-icons.ts is hand-vendored (no import ... from 'simple-icons'). It's a build-time reference source only — belongs in devDependencies (or drop it). As-is it bloats installs without shipping anything. This is the one I'd fix before merge.
    • Disk cache never invalidates (app_icons.rs:241). Once ~/.meridian/icon-cache/<key>.png exists it's served forever — a stale icon survives an app update/reinstall (no TTL or bundle-mtime check). Acceptable for v1; worth a comment noting the cache is permanent.
    • cache_key collisions (app_icons.rs:175): lowercasing + mapping every non-alphanumeric to _ means distinct app_names can collapse to the same key ("Foo Bar" vs "Foo-Bar") and get served the wrong cached icon. Low probability, but combined with the permanent cache it's a sticky wrong-icon — a short hash suffix removes the risk.
    • Sync command + AppKit main-thread affinity (app_icons.rs:222): keeping get_app_icon synchronous is correct — Tauri v2 runs non-async commands on the main thread, where NSWorkspace/NSImage want to be. Worth a one-line comment saying so, because the instinct to make the blocking fs + PNG-encode async would move the raw unsafe AppKit calls off the main thread. Footgun for the next editor, not a bug today.
    • .map_err(|e| e.to_string()) (app_icons.rs:238,257) drops the anyhow-context style the repo prefers; minor given how shallow these are.

    Strengths

    • Frontend dedup is right: a module-level Map + in-flight-promise cache in ui/lib/app-icons.ts means one invoke per distinct app name for the whole session, not per render/instance.
    • Three-tier fallback (real icon → brand wordmark → monogram) degrades gracefully; useAppIconUrl returns null off-Tauri / on failure so the existing glyph always renders; cancelled flag handles unmount/appName-change cleanly.
    • iconForFile: only called after resolve_bundle_path confirms the bundle exists, avoiding NSWorkspace's generic-icon trap. File headers on line 1, #[tracing::instrument] + debug/warn, per-icon viewBox stored, license/trademark provenance documented, hermetic unit tests for resolve_bundle_path_in/cache_key.

    Verdict: Approve with minor nits. No correctness blockers; the simple-icons dependency placement is the one to fix before merge, the rest are follow-up comments.

    🤖 Automated review via Claude Code

     
  • Anonymous

    Anonymous - 2026-07-09

    Ticket changed by: adityaharishch

    • status: open --> closed
     

Log in to post a comment.