From b12de611f78f2dfcc4eb27659cb6b5462e70884d Mon Sep 17 00:00:00 2001 From: adilallo <39313955+adilallo@users.noreply.github.com> Date: Tue, 29 Sep 2026 14:05:30 -0600 Subject: [PATCH] Read the review card fill from localStorage before picking a new one. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The review screen’s effect ran before draft hydration, so reload always generated a fresh color and overwrote the stored one. Co-authored-by: Cursor --- .../create/hooks/usePreviewSurfaceColor.ts | 35 ++++----- .../screens/review/FinalReviewScreen.tsx | 21 ++++-- .../utils/resolvePreviewSurfaceFallback.ts | 64 +++++++++++++++++ .../FinalReviewPreviewSurface.test.tsx | 35 ++++++++- .../resolvePreviewSurfaceFallback.test.ts | 71 +++++++++++++++++++ 5 files changed, 199 insertions(+), 27 deletions(-) create mode 100644 app/(app)/create/utils/resolvePreviewSurfaceFallback.ts create mode 100644 tests/unit/resolvePreviewSurfaceFallback.test.ts diff --git a/app/(app)/create/hooks/usePreviewSurfaceColor.ts b/app/(app)/create/hooks/usePreviewSurfaceColor.ts index 542d560..ef39c1c 100644 --- a/app/(app)/create/hooks/usePreviewSurfaceColor.ts +++ b/app/(app)/create/hooks/usePreviewSurfaceColor.ts @@ -1,23 +1,8 @@ "use client"; import { useEffect, useState } from "react"; -import { - dominantColorFromImageUrl, - isPreviewSurfaceColor, - randomPreviewSurfaceColor, -} from "../../../../lib/create/previewSurfaceColor"; - -function storedOrRandom( - storedFallback: string | undefined, - persistFallback: (_color: string) => void, -): string { - if (storedFallback && isPreviewSurfaceColor(storedFallback)) { - return storedFallback; - } - const next = randomPreviewSurfaceColor(); - persistFallback(next); - return next; -} +import { dominantColorFromImageUrl } from "../../../../lib/create/previewSurfaceColor"; +import { resolvePreviewSurfaceFallback } from "../utils/resolvePreviewSurfaceFallback"; /** * Fill for the review preview card. An uploaded logo contributes its most @@ -27,7 +12,7 @@ function storedOrRandom( * * @param imageUrl Uploaded logo URL, or `null` when the user skipped the photo. * @param storedFallback Draft `previewSurfaceColor`, if already picked. - * @param persistFallback Writes the fallback onto the draft once. + * @param persistFallback Writes the fallback onto React state once. * @returns A CSS color, or `undefined` before the client picks one. * * @example @@ -42,15 +27,23 @@ export function usePreviewSurfaceColor( useEffect(() => { if (!imageUrl) { + const next = resolvePreviewSurfaceFallback(storedFallback); + persistFallback(next); // eslint-disable-next-line react-hooks/set-state-in-effect -- random fill is client-only; first paint stays on the fallback class - setColor(storedOrRandom(storedFallback, persistFallback)); + setColor(next); return; } let cancelled = false; - void dominantColorFromImageUrl(imageUrl).then((next) => { + void dominantColorFromImageUrl(imageUrl).then((extracted) => { if (cancelled) return; - setColor(next ?? storedOrRandom(storedFallback, persistFallback)); + if (extracted) { + setColor(extracted); + return; + } + const next = resolvePreviewSurfaceFallback(storedFallback); + persistFallback(next); + setColor(next); }); return () => { diff --git a/app/(app)/create/screens/review/FinalReviewScreen.tsx b/app/(app)/create/screens/review/FinalReviewScreen.tsx index 233305d..ae5c337 100644 --- a/app/(app)/create/screens/review/FinalReviewScreen.tsx +++ b/app/(app)/create/screens/review/FinalReviewScreen.tsx @@ -34,6 +34,10 @@ import { vectorMarkPath, } from "../../../../../lib/assetUtils"; import { isPreviewSurfaceColor } from "../../../../../lib/create/previewSurfaceColor"; +import { + persistDraftPreviewSurfaceColor, + readDraftPreviewSurfaceColor, +} from "../../utils/resolvePreviewSurfaceFallback"; const FACET_FALLBACK_ORDER: readonly TemplateFacetGroupKey[] = [ "coreValues", @@ -247,12 +251,19 @@ export function FinalReviewScreen({ : undefined; const persistFallback = useCallback( (color: string) => { - replaceState((prev) => - prev.previewSurfaceColor && - isPreviewSurfaceColor(prev.previewSurfaceColor) + replaceState((prev) => { + persistDraftPreviewSurfaceColor(color, prev); + const keep = + (prev.previewSurfaceColor && + isPreviewSurfaceColor(prev.previewSurfaceColor) + ? prev.previewSurfaceColor + : undefined) ?? + readDraftPreviewSurfaceColor() ?? + color; + return prev.previewSurfaceColor === keep ? prev - : { ...prev, previewSurfaceColor: color }, - ); + : { ...prev, previewSurfaceColor: keep }; + }); }, [replaceState], ); diff --git a/app/(app)/create/utils/resolvePreviewSurfaceFallback.ts b/app/(app)/create/utils/resolvePreviewSurfaceFallback.ts new file mode 100644 index 0000000..842eb2e --- /dev/null +++ b/app/(app)/create/utils/resolvePreviewSurfaceFallback.ts @@ -0,0 +1,64 @@ +import type { CreateFlowState } from "../types"; +import { + isPreviewSurfaceColor, + randomPreviewSurfaceColor, +} from "../../../../lib/create/previewSurfaceColor"; +import { + readAnonymousCreateFlowState, + writeAnonymousCreateFlowState, +} from "./anonymousDraftStorage"; + +function validPreviewSurfaceColor( + value: string | undefined, +): string | undefined { + return typeof value === "string" && isPreviewSurfaceColor(value) + ? value + : undefined; +} + +/** Existing draft fill from React state or localStorage, if it is a stored hsl. */ +export function readDraftPreviewSurfaceColor( + storedFallback?: string, +): string | undefined { + return ( + validPreviewSurfaceColor(storedFallback) ?? + validPreviewSurfaceColor( + readAnonymousCreateFlowState().previewSurfaceColor, + ) + ); +} + +/** + * Keep a fallback fill on the anonymous draft immediately. The provider’s + * first mirrored write is skipped, so waiting on React state loses the color + * across reload. `state` is merged so a write before hydrate does not drop + * other draft fields already in memory. + */ +export function persistDraftPreviewSurfaceColor( + color: string, + state: CreateFlowState = {}, +): void { + const next = validPreviewSurfaceColor(color); + if (!next) return; + const current = readAnonymousCreateFlowState(); + const stored = validPreviewSurfaceColor(current.previewSurfaceColor); + writeAnonymousCreateFlowState({ + ...current, + ...state, + previewSurfaceColor: stored ?? next, + }); +} + +/** + * Reuse the draft fill when one exists; otherwise pick one and write it now. + * Reads localStorage itself so a child effect can run before context hydrates. + */ +export function resolvePreviewSurfaceFallback( + storedFallback?: string, +): string { + const existing = readDraftPreviewSurfaceColor(storedFallback); + if (existing) return existing; + const color = randomPreviewSurfaceColor(); + persistDraftPreviewSurfaceColor(color); + return color; +} diff --git a/tests/components/FinalReviewPreviewSurface.test.tsx b/tests/components/FinalReviewPreviewSurface.test.tsx index 3aa2b75..28778b1 100644 --- a/tests/components/FinalReviewPreviewSurface.test.tsx +++ b/tests/components/FinalReviewPreviewSurface.test.tsx @@ -1,10 +1,11 @@ import { useEffect, useLayoutEffect } from "react"; -import { describe, expect, it, vi } from "vitest"; +import { beforeEach, describe, expect, it, vi } from "vitest"; import { waitFor } from "@testing-library/react"; import { renderWithProviders as render, screen } from "../utils/test-utils"; import "@testing-library/jest-dom/vitest"; import { FinalReviewScreen } from "../../app/(app)/create/screens/review/FinalReviewScreen"; import { useCreateFlow } from "../../app/(app)/create/context/CreateFlowContext"; +import { CREATE_FLOW_ANONYMOUS_KEY } from "../../app/(app)/create/utils/anonymousDraftStorage"; import { dominantColorFromImageUrl, randomPreviewSurfaceColor, @@ -60,6 +61,10 @@ function FinalReviewWithStateProbe({ } describe("Final review preview surface", () => { + beforeEach(() => { + window.localStorage.removeItem(CREATE_FLOW_ANONYMOUS_KEY); + vi.mocked(randomPreviewSurfaceColor).mockClear(); + }); it("fills the card from the uploaded logo’s most common color", async () => { render(); @@ -95,6 +100,34 @@ describe("Final review preview surface", () => { expect(vi.mocked(randomPreviewSurfaceColor)).toHaveBeenCalled(); }); + it("reuses a fallback saved only in localStorage across a fresh mount", async () => { + window.localStorage.setItem( + CREATE_FLOW_ANONYMOUS_KEY, + JSON.stringify({ + title: "Oak Park Commons", + communityContext: "Local mutual aid.", + previewSurfaceColor: "hsl(200, 60%, 80%)", + }), + ); + let latest: CreateFlowState = {}; + render( + { + latest = state; + }} + />, + ); + + await waitFor(() => { + expect(latest.previewSurfaceColor).toBe("hsl(200, 60%, 80%)"); + }); + expect(vi.mocked(randomPreviewSurfaceColor)).not.toHaveBeenCalled(); + }); + it("reuses the stored fallback instead of picking a new color", async () => { vi.mocked(randomPreviewSurfaceColor).mockClear(); render( diff --git a/tests/unit/resolvePreviewSurfaceFallback.test.ts b/tests/unit/resolvePreviewSurfaceFallback.test.ts new file mode 100644 index 0000000..74f7ccd --- /dev/null +++ b/tests/unit/resolvePreviewSurfaceFallback.test.ts @@ -0,0 +1,71 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { CREATE_FLOW_ANONYMOUS_KEY } from "../../app/(app)/create/utils/anonymousDraftStorage"; +import { + persistDraftPreviewSurfaceColor, + readDraftPreviewSurfaceColor, + resolvePreviewSurfaceFallback, +} from "../../app/(app)/create/utils/resolvePreviewSurfaceFallback"; +import { randomPreviewSurfaceColor } from "../../lib/create/previewSurfaceColor"; + +vi.mock("../../lib/create/previewSurfaceColor", async (importOriginal) => { + const actual = + await importOriginal< + typeof import("../../lib/create/previewSurfaceColor") + >(); + return { + ...actual, + randomPreviewSurfaceColor: vi.fn(() => "hsl(12, 50%, 80%)"), + }; +}); + +describe("resolvePreviewSurfaceFallback", () => { + beforeEach(() => { + window.localStorage.removeItem(CREATE_FLOW_ANONYMOUS_KEY); + vi.mocked(randomPreviewSurfaceColor).mockClear(); + }); + + it("reuses a valid fill already on React state", () => { + expect(resolvePreviewSurfaceFallback("hsl(200, 60%, 80%)")).toBe( + "hsl(200, 60%, 80%)", + ); + expect(randomPreviewSurfaceColor).not.toHaveBeenCalled(); + }); + + it("reuses a fill already on the anonymous draft", () => { + window.localStorage.setItem( + CREATE_FLOW_ANONYMOUS_KEY, + JSON.stringify({ + title: "Oak Park Commons", + previewSurfaceColor: "hsl(200, 60%, 80%)", + }), + ); + expect(resolvePreviewSurfaceFallback(undefined)).toBe( + "hsl(200, 60%, 80%)", + ); + expect(randomPreviewSurfaceColor).not.toHaveBeenCalled(); + }); + + it("picks once and writes it onto the anonymous draft", () => { + expect(resolvePreviewSurfaceFallback(undefined)).toBe( + "hsl(12, 50%, 80%)", + ); + expect(randomPreviewSurfaceColor).toHaveBeenCalledTimes(1); + expect(readDraftPreviewSurfaceColor()).toBe("hsl(12, 50%, 80%)"); + expect(resolvePreviewSurfaceFallback(undefined)).toBe( + "hsl(12, 50%, 80%)", + ); + expect(randomPreviewSurfaceColor).toHaveBeenCalledTimes(1); + }); + + it("does not replace a fill already stored on the draft", () => { + window.localStorage.setItem( + CREATE_FLOW_ANONYMOUS_KEY, + JSON.stringify({ + title: "Oak Park Commons", + previewSurfaceColor: "hsl(200, 60%, 80%)", + }), + ); + persistDraftPreviewSurfaceColor("hsl(12, 50%, 80%)"); + expect(readDraftPreviewSurfaceColor()).toBe("hsl(200, 60%, 80%)"); + }); +});