feat(desktop): add app icon switcher

apps/desktop/e2e/settings.spec.ts

@@ -19,6 +19,23 @@
 
 import { test, expect, COMPOSER_INPUT } from './fixtures';
 
+async function choiceContentGeometry(card: import('@playwright/test').Locator) {
+  return card.evaluate((element) => {
+    const content = Array.from(element.children).find((child) => child.tagName !== 'INPUT');
+    if (!(content instanceof HTMLElement)) {
+      throw new Error('SelectableCard content is missing');
+    }
+    const cardRect = element.getBoundingClientRect();
+    const contentRect = content.getBoundingClientRect();
+    return {
+      cardHeight: cardRect.height,
+      contentHeight: contentRect.height,
+      topGap: contentRect.top - cardRect.top,
+      bottomGap: cardRect.bottom - contentRect.bottom,
+    };
+  });
+}
+
 test('opening settings commits an active titlebar rename', async ({ window: page }) => {
   const composer = page.locator(COMPOSER_INPUT);
   await composer.fill('create a session for settings rename');
@@ -104,3 +121,23 @@ test('wide settings gutters scroll the whole main pane', async ({ window: page }
   await expect.poll(() => pane.evaluate((element) => element.scrollTop)).toBeGreaterThan(0);
   await expect(content).toBeVisible();
 });
+
+test('appearance choice content stays vertically centered in stretched grid rows', async ({ window: page }) => {
+  await page.evaluate(async () => {
+    await window.maka.settings.update({ personalization: { uiLocale: 'en' } });
+  });
+  await page.reload();
+  await page.waitForSelector(COMPOSER_INPUT);
+  await page.setViewportSize({ width: 1650, height: 992 });
+  await page.getByRole('button', { name: 'Settings' }).click();
+  await page.getByRole('button', { name: 'Appearance', exact: true }).click();
+  await expect(page.getByRole('heading', { name: 'App icon' })).toBeVisible();
+
+  for (const name of ['Azure', 'Classic']) {
+    const card = page.getByRole('checkbox', { name, exact: true }).locator('..');
+    await expect(card).toBeVisible();
+    const geometry = await choiceContentGeometry(card);
+    expect(geometry.cardHeight).toBeGreaterThan(geometry.contentHeight);
+    expect(Math.abs(geometry.topGap - geometry.bottomGap)).toBeLessThanOrEqual(1);
+  }
+});

apps/desktop/src/main/__tests__/app-icon-ipc.test.ts

@@ -0,0 +1,223 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import assert from 'node:assert/strict';
+import { mkdtemp, mkdir, readdir, writeFile } from 'node:fs/promises';
+import { tmpdir } from 'node:os';
+import { join } from 'node:path';
+import { test } from 'node:test';
+import type { IpcMainInvokeEvent } from 'electron';
+import type { AppSettings, UpdateAppSettingsInput } from '@maka/core/settings';
+import { registerAppIconIpc } from '../app-icon-ipc.js';
+import { customAppIconDirectory, resolveCustomAppIconPath } from '../custom-app-icon-store.js';
+
+const ID = 'c'.repeat(32);
+const ICON = `custom:${ID}`;
+
+type Handler = (event: IpcMainInvokeEvent, ...args: unknown[]) => unknown;
+
+async function harness(selected: string, options: {
+    onCompareAndSet?: () => void;
+    onApply?: () => void;
+    onShowOpenDialog?: () => Promise<void>;
+  } = {}) {
+  const root = await mkdtemp(join(tmpdir(), 'maka-icon-ipc-'));
+  await mkdir(customAppIconDirectory(root), { recursive: true });
+  await writeFile(resolveCustomAppIconPath(root, ID), 'x');
+
+  const handlers = new Map<string, Handler>();
+  let settings = { appearance: { theme: 'auto', appIcon: selected } } as unknown as AppSettings;
+  const applied: AppSettings[] = [];
+
+  registerAppIconIpc({
+    // Typed, not cast: a stub that stops matching the real dependencies should
+    // fail the build rather than keep passing against a shape that is gone.
+    ipcMain: { handle: (channel: string, handler: Handler) => void handlers.set(channel, handler) },
+    showOpenDialog: async () => {
+      await options.onShowOpenDialog?.();
+      return { canceled: true, filePaths: [] };
+    },
+    listPreviews: async () => [],
+    importArtwork: async () => 'default',
+    settingsStore: {
+      update: async (patch: UpdateAppSettingsInput) => {
+        settings = {
+          ...settings,
+          appearance: { ...settings.appearance, ...patch.appearance },
+        } as AppSettings;
+        return settings;
+      },
+      updateIf: async (
+        predicate: (current: AppSettings) => boolean,
+        patch: UpdateAppSettingsInput,
+      ) => {
+        // The real store evaluates the predicate and writes on one queue. The
+        // hook stands in for whatever else reached that queue first.
+        options.onCompareAndSet?.();
+        if (!predicate(settings)) return { applied: false, settings };
+        settings = {
+          ...settings,
+          appearance: { ...settings.appearance, ...patch.appearance },
+        } as AppSettings;
+        return { applied: true, settings };
+      },
+    },
+    applySettings: async (next: AppSettings) => {
+      options.onApply?.();
+      applied.push(next);
+    },
+    userDataPath: () => root,
+  });
+
+  return {
+    root,
+    applied,
+    remove: (icon: unknown) =>
+      handlers.get('app:removeIcon')!(undefined as unknown as IpcMainInvokeEvent, icon),
+    current: () => settings.appearance.appIcon,
+    select: (icon: unknown) =>
+      handlers.get('app:selectIcon')!(undefined as unknown as IpcMainInvokeEvent, icon),
+    importIcon: () =>
+      handlers.get('app:importIcon')!(undefined as unknown as IpcMainInvokeEvent),
+    // Stands in for whatever else reached the settings queue first; the IPC
+    // path above is the one under test.
+    forceSelect: (icon: string) => {
+      settings = {
+        ...settings,
+        appearance: { ...settings.appearance, appIcon: icon },
+      } as AppSettings;
+    },
+  };
+}
+
+test('removing the current icon resets the selection before the file goes away', async () => {
+  const h = await harness(ICON);
+  const result = (await h.remove(ICON)) as { ok: boolean; selection?: string };
+
+  assert.equal(result.ok, true);
+  assert.equal(result.selection, 'default');
+  // Both halves moved, and the setting is the half that moved first.
+  assert.equal(h.current(), 'default');
+  assert.deepEqual(await readdir(customAppIconDirectory(h.root)), []);
+  // The OS surface was told, so the dock is not still holding the deleted art.
+  assert.equal(h.applied.length, 1);
+});
+
+test('removing an icon that is not selected leaves the selection alone', async () => {
+  const h = await harness('sky');
+  const result = (await h.remove(ICON)) as { ok: boolean; selection?: string };
+
+  assert.equal(result.ok, true);
+  assert.equal(result.selection, 'sky');
+  assert.equal(h.current(), 'sky');
+  assert.deepEqual(await readdir(customAppIconDirectory(h.root)), []);
+  assert.equal(h.applied.length, 0);
+});
+
+/**
+ * The shipped set is not the user's to delete, and a malformed reference names
+ * no artwork at all — neither may reach the store, which would otherwise turn
+ * the string into a path.
+ */
+test('shipped ids and malformed references are refused without touching disk', async () => {
+  const h = await harness(ICON);
+  for (const bad of ['default', 'sky', 'custom:../../etc/passwd', 'custom:', 42, null]) {
+    const result = (await h.remove(bad)) as { ok: boolean; reason?: string };
+    assert.equal(result.ok, false, `${String(bad)} should be refused`);
+    assert.equal(result.reason, 'invalid_id');
+  }
+  assert.deepEqual(await readdir(customAppIconDirectory(h.root)), [`${ID}.png`]);
+  assert.equal(h.current(), ICON);
+});
+
+/**
+ * The gap a busy flag cannot close: the selection can move between the read
+ * and the write, and on the far side of an IPC boundary at that. Resetting
+ * unconditionally would stamp `default` over a choice the user just made.
+ */
+test('a selection landing during removal wins, and the file still goes', async () => {
+  const newer = 'sky';
+  const h = await harness(ICON, { onCompareAndSet: () => h.forceSelect(newer) });
+
+  const result = (await h.remove(ICON)) as { ok: boolean; selection?: string };
+
+  assert.equal(result.ok, true);
+  // The newer choice is the authority, and it is what the caller is told.
+  assert.equal(result.selection, newer);
+  assert.equal(h.current(), newer);
+  // Nothing was applied, because nothing about the selection changed here.
+  assert.equal(h.applied.length, 0);
+  // The artwork is still deleted: it is no longer in use, which is what was asked.
+  assert.deepEqual(await readdir(customAppIconDirectory(h.root)), []);
+});
+
+/**
+ * The window a compare-and-set around the settings write could not close:
+ * applying the icon yields, and a selection arriving in that gap would be left
+ * pointing at a file the removal is about to delete. Serializing the three
+ * operations removes the window rather than guarding it, so a selection issued
+ * mid-removal cannot interleave — it runs after, and is refused because the
+ * artwork it names is gone.
+ */
+test('a selection issued mid-removal cannot land between reset, apply and delete', async () => {
+  const observed: string[] = [];
+  const h = await harness(ICON, {
+    onCompareAndSet: () => observed.push('compare-and-set'),
+    onApply: () => observed.push('apply'),
+  });
+
+  const removal = h.remove(ICON);
+  // Issued without awaiting the removal: this is the interleaving attempt.
+  const selection = h.select(ICON);
+  const [removed, selected] = (await Promise.all([removal, selection])) as [
+    { ok: boolean },
+    { ok: boolean; reason?: string },
+  ];
+
+  assert.equal(removed.ok, true);
+  // It ran after the removal, not inside it, and the artwork was already gone.
+  assert.equal(selected.ok, false);
+  assert.equal(selected.reason, 'missing_artwork');
+  assert.equal(h.current(), 'default');
+  assert.deepEqual(await readdir(customAppIconDirectory(h.root)), []);
+  // Nothing ran between the reset and the delete.
+  assert.deepEqual(observed, ['compare-and-set', 'apply']);
+});
+
+/**
+ * The dialog is user time, not work time. Holding the owner while it is open
+ * would block selection and removal in every other window on someone reading a
+ * file list, so only the copy-into-place is serialized.
+ */
+test('an open file dialog does not hold the queue', async () => {
+  let releaseDialog: () => void = () => {};
+  const dialogOpen = new Promise<void>((resolve) => {
+    releaseDialog = resolve;
+  });
+  const h = await harness('sky', { onShowOpenDialog: () => dialogOpen });
+
+  const importing = h.importIcon();
+  // The dialog is still open; a selection issued now must not wait for it.
+  const selected = (await h.select('ink')) as { ok: boolean };
+  assert.equal(selected.ok, true);
+  assert.equal(h.current(), 'ink');
+
+  releaseDialog();
+  await importing;
+});