feat: upload dark logo

[mindspank]

Allows upload of a dark mode logo

Checklist:

• Covered by tests
• Ran it and it works as intended
• Reviewed the diff before requesting a review
• Checked for unhandled edge cases
• Linked the issues it closes
• Checked if the docs need to be updated. If so, create a separate Linear DOCS issue
• Intend to cherry-pick into the release branch
• I'm proud of this work!

[ericpgreen2]

Overall: Nice feature addition! A few design suggestions to consider:

1. Combine Logo Settings into One Section

Rather than two separate settings cards for Logo and Dark Logo, consider combining them into a single "Logo" section with both variants side-by-side. Benefits:

• Users setting one will naturally think about the other
• Side-by-side comparison helps ensure logos work well as a pair
• Reduces vertical space on the settings page
• Dark logo can be shown as optional ("Add dark variant")
• Reduces code duplication - LogoDarkSettings.svelte is nearly identical to LogoSettings.svelte

2. Simplify TopNavigationBar Props

Instead of passing two logo URL props and having the component select based on theme:

export let organizationLogoUrl: string | undefined;
export let organizationLogoDarkUrl: string | undefined;

$: logoUrl = theme === "dark" && organizationLogoDarkUrl
  ? organizationLogoDarkUrl
  : organizationLogoUrl;

Use a single prop, with the selection logic in a derived store in the themes feature directory:

// e.g., web-common/src/features/themes/organization-logo.ts
export const organizationLogoUrl = derived(
  [themeControl, organizationData],
  ([$theme, $org]) => $theme === "dark" && $org?.logoDarkUrl 
    ? $org.logoDarkUrl 
    : $org?.logoUrl
);

This keeps TopNavigationBar simpler and makes the logic reusable if logos appear elsewhere.

3. Simplify theme-control.ts

The ensure() method and initialized flag seem like unnecessary complexity. The constructor already runs once for the singleton, so it can handle all the setup directly:

class ThemeControl {
  private darkQuery = window.matchMedia("(prefers-color-scheme: dark)");
  // ...

  constructor() {
    const pref = get(this.preferenceStore);
    if (pref === "dark" || (pref === "system" && this.darkQuery.matches)) {
      this.setDark();
    }

    this.darkQuery.addEventListener("change", ({ matches }) => {
      if (get(this.preferenceStore) !== "system") return;
      matches ? this.setDark() : this.removeDark();
    });
  }

  // No ensure() needed, no initialized flag needed
}

Then remove the themeControl.ensure() call from +layout.svelte.

---

This comment was developed in collaboration between Eric and Claude Code.

[ericpgreen2]

See feedback above!

[ericpgreen2]

Thanks for the revisions! Two more issues to address:

1. Move organization-logo.ts to web-admin

The current location creates a layering violation - web-common is importing from web-admin, but the dependency should flow the other way.

2. Derive from TanStack Query, not a writable store

The organizationData writable store duplicates state that TanStack Query already manages. Use a helper function instead:

// web-admin/src/features/themes/organization-logo.ts
import type { V1Organization } from "@rilldata/web-admin/client";

export function getThemedLogoUrl(
  theme: "light" | "dark",
  org: V1Organization | undefined
): string | undefined {
  return theme === "dark" && org?.logoDarkUrl
    ? org.logoDarkUrl
    : org?.logoUrl;
}

// +layout.svelte
import { getThemedLogoUrl } from "@rilldata/web-admin/features/themes/organization-logo";

\$: organizationLogoUrl = getThemedLogoUrl(\$themeControl, organizationObj);

Then pass organizationLogoUrl as a prop to TopNavigationBar (as it was before) and remove the syncing logic.

---

This comment was developed in collaboration between Eric and Claude Code.

[ericpgreen2]

See above

[ericpgreen2]

Looks good! A few minor nits to consider before merging:

1. Redundant organization data - The layout now returns both the full organization object and individual URL properties (organizationLogoUrl, organizationLogoDarkUrl, etc.). Consider removing the individual properties and deriving them from organization where needed.

2. Unnecessary intermediate variable in TopNavigationBar.svelte - $: logoUrl = organizationLogoUrl; could just use organizationLogoUrl directly.

3. Type casts - $themeControl as "light" | "dark" in the layout is safe but indicates the Theme type could be narrower. Also url as URL and await parent() as {...} in +page.ts.

None of these are blocking - approving as-is.

---

This comment was developed in collaboration between Eric and Claude Code.