diff --git a/.changeset/fix-multi-window-theme-flicker.md b/.changeset/fix-multi-window-theme-flicker.md new file mode 100644 index 00000000..594493f6 --- /dev/null +++ b/.changeset/fix-multi-window-theme-flicker.md @@ -0,0 +1,5 @@ +--- +"@inkeep/open-knowledge": patch +--- + +Fix the appearance theme toggle flickering other open project windows. With more than one project open, switching Light/Dark/System made every non-focused window flicker rapidly before settling on the right appearance. The cause was the window-chrome translucency material being re-applied to every open window on every theme change — work that scales with the number of windows and rebuilds the macOS vibrancy view each time, even though the material never needs to change on a light/dark switch. The desktop app now skips re-applying a window's translucency when it is unchanged, so theme switches are flicker-free across all open windows. Genuine "Reduce transparency" accessibility changes still apply to every window. diff --git a/packages/desktop/src/main/reduced-transparency-handler.ts b/packages/desktop/src/main/reduced-transparency-handler.ts index d954ef3a..3d7b7925 100644 --- a/packages/desktop/src/main/reduced-transparency-handler.ts +++ b/packages/desktop/src/main/reduced-transparency-handler.ts @@ -2,6 +2,7 @@ export type VibrancyMaterial = 'sidebar' | 'window'; export interface BrowserWindowVibrancyTarget { isDestroyed?: () => boolean; + readonly id?: number; setVibrancy: (mat: VibrancyMaterial | null) => void; } @@ -11,21 +12,36 @@ export interface ReducedTransparencyDeps { warn?: (line: string) => void; } +const lastAppliedMaterial = new WeakMap(); + export function applyReducedTransparency( deps: ReducedTransparencyDeps, reducedTransparency: boolean, ): void { const material: VibrancyMaterial | null = reducedTransparency ? null : deps.defaultVibrancy; let windowCount = 0; + let skippedCount = 0; + let failedCount = 0; + let destroyedCount = 0; for (const win of deps.getAllWindows()) { - if (win.isDestroyed?.() === true) continue; + if (win.isDestroyed?.() === true) { + destroyedCount += 1; + continue; + } + if (lastAppliedMaterial.get(win) === material) { + skippedCount += 1; + continue; + } try { win.setVibrancy(material); + lastAppliedMaterial.set(win, material); windowCount += 1; } catch (err) { + failedCount += 1; deps.warn?.( JSON.stringify({ event: 'reduced-transparency-window-failed', + windowId: win.id, vibrancy: material, error: err instanceof Error ? err.message : String(err), }), @@ -38,6 +54,9 @@ export function applyReducedTransparency( reducedTransparency, vibrancy: material, windowCount, + skippedCount, + failedCount, + destroyedCount, }), ); } diff --git a/packages/desktop/tests/main/reduced-transparency-handler.test.ts b/packages/desktop/tests/main/reduced-transparency-handler.test.ts index d692603d..f1461b96 100644 --- a/packages/desktop/tests/main/reduced-transparency-handler.test.ts +++ b/packages/desktop/tests/main/reduced-transparency-handler.test.ts @@ -56,6 +56,20 @@ describe('applyReducedTransparency — disable path (reduced=true)', () => { expect(live.setVibrancy).toHaveBeenCalledTimes(1); expect(dead.setVibrancy).not.toHaveBeenCalled(); }); + + test('a window stub without isDestroyed does not throw (optional-chain guard)', () => { + const setVibrancy = mock(() => {}); + const win: BrowserWindowVibrancyTarget = { + setVibrancy: setVibrancy as unknown as (mat: 'sidebar' | 'window' | null) => void, + }; + const deps: ReducedTransparencyDeps = { + getAllWindows: () => [win], + defaultVibrancy: 'sidebar', + }; + + expect(() => applyReducedTransparency(deps, true)).not.toThrow(); + expect(setVibrancy).toHaveBeenCalledWith(null); + }); }); describe('applyReducedTransparency — restore path (reduced=false)', () => { @@ -128,7 +142,7 @@ describe('applyReducedTransparency — diagnostic logging', () => { expect(parsed.windowCount).toBe(1); }); - test('windowCount counts only non-destroyed windows', () => { + test('windowCount counts only successfully-applied windows; destroyed windows go to destroyedCount', () => { const live = makeWindow(); const dead = makeWindow({ destroyed: true }); const warn = mock(() => {}); @@ -142,6 +156,25 @@ describe('applyReducedTransparency — diagnostic logging', () => { const parsed = JSON.parse(warn.mock.calls[0]?.[0] as unknown as string); expect(parsed.windowCount).toBe(1); + expect(parsed.destroyedCount).toBe(1); + }); + + test('summary partition is complete: all counters sum to getAllWindows().length', () => { + const a = makeWindow(); + const b = makeWindow({ destroyed: true }); + const warn = mock(() => {}); + const deps: ReducedTransparencyDeps = { + getAllWindows: () => [a.win, b.win], + defaultVibrancy: 'sidebar', + warn, + }; + + applyReducedTransparency(deps, true); + + const parsed = JSON.parse(warn.mock.calls[0]?.[0] as unknown as string); + expect( + parsed.windowCount + parsed.skippedCount + parsed.failedCount + parsed.destroyedCount, + ).toBe(deps.getAllWindows().length); }); test('omitting warn dep does not throw (optional sink)', () => { @@ -252,5 +285,133 @@ describe('applyReducedTransparency — per-window throw isolation', () => { const lines = warn.mock.calls.map((call) => JSON.parse(call[0] as unknown as string)); const summary = lines.find((l) => l.event === 'reduced-transparency-applied'); expect(summary?.windowCount).toBe(1); + expect(summary?.skippedCount).toBe(0); + expect(summary?.failedCount).toBe(1); + }); + + test('per-window failure warn carries the window id for attribution', () => { + const a = makeWindow(); + const setVibrancy = mock(() => { + throw new Error('setVibrancy: native failure'); + }); + const b: BrowserWindowVibrancyTarget = { + id: 42, + isDestroyed: () => false, + setVibrancy: setVibrancy as unknown as (mat: 'sidebar' | 'window' | null) => void, + }; + const warn = mock(() => {}); + const deps: ReducedTransparencyDeps = { + getAllWindows: () => [a.win, b], + defaultVibrancy: 'sidebar', + warn, + }; + + applyReducedTransparency(deps, true); + + const failed = warn.mock.calls + .map((call) => JSON.parse(call[0] as unknown as string)) + .find((l) => l.event === 'reduced-transparency-window-failed'); + expect(failed?.windowId).toBe(42); + }); +}); + +describe('applyReducedTransparency — per-window idempotence memo (flicker guard)', () => { + function summaries(warn: ReturnType) { + return warn.mock.calls + .map((call) => JSON.parse(call[0] as unknown as string)) + .filter((l) => l.event === 'reduced-transparency-applied'); + } + + test('skips a window already at the target material on a repeat call (no redundant setVibrancy)', () => { + const a = makeWindow(); + const warn = mock(() => {}); + const deps: ReducedTransparencyDeps = { + getAllWindows: () => [a.win], + defaultVibrancy: 'sidebar', + warn, + }; + + applyReducedTransparency(deps, false); + applyReducedTransparency(deps, false); + + expect(a.setVibrancy).toHaveBeenCalledTimes(1); + expect(a.setVibrancy.mock.calls[0]).toEqual(['sidebar']); + + const lines = summaries(warn); + expect(lines[0]).toMatchObject({ windowCount: 1, skippedCount: 0 }); + expect(lines[1]).toMatchObject({ windowCount: 0, skippedCount: 1 }); + }); + + test('re-applies when the material actually changes (genuine reduce-transparency toggle)', () => { + const a = makeWindow(); + const deps: ReducedTransparencyDeps = { + getAllWindows: () => [a.win], + defaultVibrancy: 'sidebar', + }; + + applyReducedTransparency(deps, false); // material 'sidebar' + applyReducedTransparency(deps, true); // material null — changed → re-applies + + expect(a.setVibrancy).toHaveBeenCalledTimes(2); + expect(a.setVibrancy.mock.calls[0]).toEqual(['sidebar']); + expect(a.setVibrancy.mock.calls[1]).toEqual([null]); + }); + + test('applies once to a newly-opened window while skipping already-memoized windows', () => { + const a = makeWindow(); + const b = makeWindow(); + const warn = mock(() => {}); + + applyReducedTransparency( + { getAllWindows: () => [a.win], defaultVibrancy: 'sidebar', warn }, + true, + ); + applyReducedTransparency( + { getAllWindows: () => [a.win, b.win], defaultVibrancy: 'sidebar', warn }, + true, + ); + + expect(a.setVibrancy).toHaveBeenCalledTimes(1); + expect(b.setVibrancy).toHaveBeenCalledTimes(1); + expect(b.setVibrancy.mock.calls[0]).toEqual([null]); + expect(summaries(warn).at(-1)).toMatchObject({ windowCount: 1, skippedCount: 1 }); + }); + + test('per-window memo tracks each transition (apply / skip / re-apply on real change)', () => { + const a = makeWindow(); + const deps: ReducedTransparencyDeps = { + getAllWindows: () => [a.win], + defaultVibrancy: 'sidebar', + }; + + applyReducedTransparency(deps, false); // sidebar — applied + applyReducedTransparency(deps, false); // sidebar — skipped + applyReducedTransparency(deps, true); // null — changed, applied + applyReducedTransparency(deps, true); // null — skipped + applyReducedTransparency(deps, false); // sidebar — changed back, applied + + expect(a.setVibrancy.mock.calls).toEqual([['sidebar'], [null], ['sidebar']]); + }); + + test('a window that throws is not memoized — the next apply retries instead of skipping', () => { + let calls = 0; + const setVibrancy = mock(() => { + calls += 1; + if (calls === 1) throw new Error('setVibrancy: transient native failure'); + }); + const win: BrowserWindowVibrancyTarget = { + isDestroyed: () => false, + setVibrancy: setVibrancy as unknown as (mat: 'sidebar' | 'window' | null) => void, + }; + const deps: ReducedTransparencyDeps = { + getAllWindows: () => [win], + defaultVibrancy: 'sidebar', + }; + + applyReducedTransparency(deps, false); // throws — NOT memoized + applyReducedTransparency(deps, false); // retries (would skip if memoized on failure) + + expect(setVibrancy).toHaveBeenCalledTimes(2); + expect(setVibrancy.mock.calls).toEqual([['sidebar'], ['sidebar']]); }); });