diff --git a/.changeset/wire-highlights-api-to-reader.md b/.changeset/wire-highlights-api-to-reader.md new file mode 100644 index 00000000..f717b92d --- /dev/null +++ b/.changeset/wire-highlights-api-to-reader.md @@ -0,0 +1,13 @@ +--- +'@youversion/platform-core': minor +'@youversion/platform-react-hooks': minor +'@youversion/platform-react-ui': minor +--- + +Wire the Highlights API into the Bible reader, replacing the localStorage-only highlight store. + +- `BibleReader` now loads and saves verse highlights via the Highlights API for signed-in readers (chapter-scoped `GET`, optimistic `create`/`delete`), with the server as the source of truth. The previous `localStorage` persistence (`youversion-platform:highlights:`) is removed. +- The reader's sign-in (and the example navbar button) now request the `highlights` data-exchange permission so the token is granted highlights access. +- `@youversion/platform-react-hooks` now exports `YouVersionAuthContext`, enabling null-safe reads of signed-in state without `useYVAuth()` throwing when no auth provider is mounted. + +When not signed in, highlighting is currently ephemeral session state (not persisted); the dedicated not-signed-in / permission flow is a follow-up. diff --git a/docs/adr/YPE-642-verse-action-popover.md b/docs/adr/YPE-642-verse-action-popover.md index ec13acf8..ee8c8dcb 100644 --- a/docs/adr/YPE-642-verse-action-popover.md +++ b/docs/adr/YPE-642-verse-action-popover.md @@ -174,3 +174,37 @@ Selection is always enabled in BibleReader (no opt-out prop for now; YAGNI). (SSR) says prefer `useEffect`. Pre-existing; clean up while here. - #131 leftovers to handle: replace the old verse-selection Storybook story + remove demo highlight CSS; add a changeset. + +## As-built addendum — YPE-1034 (Highlights API wiring) + +**ADR-001 (localStorage only) is superseded.** The follow-up ticket it named +landed: highlights now sync via the Highlights API for signed-in readers, and +the localStorage store is removed entirely. + +- **Persistence:** `bible-reader.tsx` `Content` no longer owns a + localStorage-backed `highlightStore` / `persistHighlights`. The + `youversion-platform:highlights:` key is gone and is never written. + Highlight state now lives in the new `lib/use-reader-highlights.ts` hook. +- **Signed in:** `useReaderHighlights` fetches via `useHighlights` (chapter-scoped + `GET /v1/highlights`), and apply/remove call `createHighlight` / + `deleteHighlight` optimistically per verse. The server is the source of truth — + the store is rebuilt from the fetched collection, filtered to the current + `version_id` + chapter prefix so a stale mid-refetch collection can't leak + another scope's colors (this replaces ADR-001's version-keyed reset guard). +- **Signed out (interim):** highlights are ephemeral session state — applied to + the in-memory store, never persisted, no API calls. This is a placeholder: the + not-signed-in / permission state machine (a later ticket) will replace it with + a pending-highlight → sign-in flow that defers the highlight until the + `highlights` permission is granted. ADR-002's API-shaped model made this swap + mechanical, as intended. +- **Auth read:** `useReaderHighlights` reads sign-in state via the raw + `YouVersionAuthContext` (now exported from `@youversion/platform-react-hooks`) + rather than `useYVAuth()`, which throws when no auth provider is mounted — the + reader must keep working for consumers who don't enable auth. +- **Permission at sign-in:** the reader's `handleSignIn` (and the example navbar + button) request `permissions: ['highlights']` so the token is actually granted + highlights access. This is a data-exchange **permission** + (`requested_permissions[]`), not an OIDC scope — see the corrected + `HighlightsClient` docs. +- **Out of scope (unchanged):** failure/revert UX (optimistic + `console.error` + only) and `getRecentColors` (popover keeps its hardcoded colors). diff --git a/examples/vite-react/src/components/navbar.tsx b/examples/vite-react/src/components/navbar.tsx index a9c9ecb6..71a3f6c2 100644 --- a/examples/vite-react/src/components/navbar.tsx +++ b/examples/vite-react/src/components/navbar.tsx @@ -3,7 +3,11 @@ import { Menu } from 'lucide-react'; import { Button } from '@/components/ui/button'; import { Sheet, SheetContent, SheetHeader, SheetTitle, SheetTrigger } from '@/components/ui/sheet'; import { ModeToggle } from '@/components/mode-toggle'; -import { useYVAuth, YouVersionAuthButton } from '@youversion/platform-react-ui'; +import { + SignInWithYouVersionPermission, + useYVAuth, + YouVersionAuthButton, +} from '@youversion/platform-react-ui'; import type { Page } from '@/App'; const navItems: { label: string; page: Page }[] = [ @@ -88,6 +92,7 @@ export function Navbar({ currentPage, onNavigate }: NavbarProps) { size="short" onAuthError={(err) => console.error('Auth error:', err)} scopes={['profile', 'email']} + permissions={[SignInWithYouVersionPermission.highlights]} /> )} diff --git a/packages/core/src/highlights.ts b/packages/core/src/highlights.ts index c1f3354a..6d42db68 100644 --- a/packages/core/src/highlights.ts +++ b/packages/core/src/highlights.ts @@ -30,9 +30,12 @@ export type DeleteHighlightOptions = { /** * Client for interacting with Highlights API endpoints. - * Note: All endpoints require OAuth authentication with appropriate scopes - * (`read_highlights` / `write_highlights`), passed as an - * `Authorization: Bearer ` header. + * Note: All endpoints require an OAuth access token passed as an + * `Authorization: Bearer ` header, and the user must have granted the + * `highlights` data-exchange permission at sign-in. That permission is + * requested via the `requested_permissions[]` authorize param (see + * `SignInWithYouVersionPermission`) — it is NOT an OIDC scope and never appears + * in the token's `scope`. A token without it yields 403/404 from these routes. */ export class HighlightsClient { private client: ApiClient; @@ -129,7 +132,7 @@ export class HighlightsClient { /** * Fetches a collection of highlights for a user. * The response will return a color per verse without ranges. - * Requires OAuth with read_highlights scope. + * Requires the `highlights` permission (see class note). * @param options Query parameters scoping the fetch. `version_id` and `passage_id` * (verse or chapter USFM, e.g. "JHN.3") are both required by the API. * @param lat Optional long access token. If not provided, retrieves from YouVersionPlatformConfiguration. @@ -166,7 +169,7 @@ export class HighlightsClient { /** * Fetches the user's recently used highlight colors, followed by the default * colors, as hex strings (no `#`). Useful for building a color picker. - * Requires OAuth with read_highlights scope. + * Requires the `highlights` permission (see class note). * @param lat Optional long access token. If not provided, retrieves from YouVersionPlatformConfiguration. * @returns An ordered list of hex color strings. */ @@ -193,7 +196,7 @@ export class HighlightsClient { /** * Creates or updates a highlight on a passage. * Verse ranges may be used in the passage_id attribute. - * Requires OAuth with write_highlights scope. + * Requires the `highlights` permission (see class note). * @param data The highlight data to create or update. * @param lat Optional long access token. If not provided, retrieves from YouVersionPlatformConfiguration. * @returns The created or updated Highlight object. @@ -229,7 +232,7 @@ export class HighlightsClient { /** * Clears highlights for a passage. - * Requires OAuth with write_highlights scope. + * Requires the `highlights` permission (see class note). * @param passageId The passage identifier (USFM format, e.g., "MAT.1.1" or "MAT.1.1-5"). * @param options Query parameters; `version_id` is required by the API (sent as `bible_id`). * @param lat Optional long access token. If not provided, retrieves from YouVersionPlatformConfiguration. diff --git a/packages/hooks/src/context/index.ts b/packages/hooks/src/context/index.ts index e2183601..ddb69413 100644 --- a/packages/hooks/src/context/index.ts +++ b/packages/hooks/src/context/index.ts @@ -1,2 +1,3 @@ export * from './YouVersionContext'; export * from './YouVersionProvider'; +export * from './YouVersionAuthContext'; diff --git a/packages/ui/src/components/bible-reader.tsx b/packages/ui/src/components/bible-reader.tsx index 99ee9e07..0e219cb7 100644 --- a/packages/ui/src/components/bible-reader.tsx +++ b/packages/ui/src/components/bible-reader.tsx @@ -2,11 +2,16 @@ import i18n from '@/i18n'; import { useDelayedLoading } from '@/lib/use-delayed-loading'; +import { useReaderHighlights } from '@/lib/use-reader-highlights'; import { cn } from '@/lib/utils'; import { INTER_FONT, SOURCE_SERIF_FONT, type FontFamily } from '@/lib/verse-html-utils'; import { useControllableState } from '@radix-ui/react-use-controllable-state'; import type { BibleBook } from '@youversion/platform-core'; -import { DEFAULT_LICENSE_FREE_BIBLE_VERSION, getAdjacentChapter } from '@youversion/platform-core'; +import { + DEFAULT_LICENSE_FREE_BIBLE_VERSION, + getAdjacentChapter, + SignInWithYouVersionPermission, +} from '@youversion/platform-core'; import { useBooks, usePassage, @@ -503,40 +508,22 @@ function Content() { }, [book, chapter]); // ---- Verse selection + highlights ------------------------------------------ - // Selection is ephemeral; highlights persist to localStorage only for now - // (ADR-001) in the future `highlight` API shape: keyed by full passage_id USFM - // and scoped by versionId/bible_id (ADR-002). The reader DOM ref lets us anchor - // the popover and pull clean verse text for Copy / Share. + // Selection is ephemeral. Highlights sync via the Highlights API for signed-in + // readers (chapter-scoped, keyed by full passage_id USFM); when not signed in + // they are ephemeral session state (never persisted) — see useReaderHighlights. + // The reader DOM ref lets us anchor the popover and pull clean verse text for + // Copy / Share. const readerRef = useRef(null); const [selectedVerses, setSelectedVerses] = useState([]); const [popoverOpen, setPopoverOpen] = useState(false); const [anchorElement, setAnchorElement] = useState(null); const lastSelectionRef = useRef([]); - const [highlightStore, setHighlightStore] = useState>({}); - - const highlightsStorageKey = `youversion-platform:highlights:${versionId}`; - - // Clear the store synchronously (during render) the moment the version key - // changes. The load effect below runs *after* paint, so without this the - // previous version's highlights would paint over the new text for one frame — - // their `${book}.${chapter}.N` keys collide with the new version's verses. - const [loadedHighlightsKey, setLoadedHighlightsKey] = useState(highlightsStorageKey); - if (loadedHighlightsKey !== highlightsStorageKey) { - setLoadedHighlightsKey(highlightsStorageKey); - setHighlightStore({}); - } - // Load this version's highlights when the version changes (client-only). - useEffect(() => { - let data: Record = {}; - try { - const raw = localStorage.getItem(highlightsStorageKey); - if (raw) data = JSON.parse(raw) as Record; - } catch { - // Ignore (unavailable or malformed storage). - } - setHighlightStore(data); - }, [highlightsStorageKey]); + const { highlightsByPassageId, applyHighlight, removeHighlight } = useReaderHighlights({ + versionId, + book, + chapter, + }); // Navigating away (book/chapter/version) drops the selection — those verses no // longer exist on screen (ADR-007). @@ -551,13 +538,13 @@ function Content() { const chapterPrefix = `${book}.${chapter}.`; const highlightedVerses = useMemo(() => { const map: Record = {}; - for (const [passageId, color] of Object.entries(highlightStore)) { + for (const [passageId, color] of Object.entries(highlightsByPassageId)) { if (!passageId.startsWith(chapterPrefix)) continue; const verseNum = parseInt(passageId.slice(chapterPrefix.length), 10); if (verseNum) map[verseNum] = color; } return map; - }, [highlightStore, chapterPrefix]); + }, [highlightsByPassageId, chapterPrefix]); // Distinct colors present in the current selection → drives the X (remove) circles. const activeHighlights = useMemo( @@ -570,14 +557,6 @@ function Content() { [selectedVerses, highlightedVerses], ); - function persistHighlights(next: Record) { - try { - localStorage.setItem(highlightsStorageKey, JSON.stringify(next)); - } catch { - // Ignore (private mode / quota exceeded). - } - } - function closeAndClearSelection() { setPopoverOpen(false); setSelectedVerses([]); @@ -607,23 +586,12 @@ function Content() { } function handleHighlight(color: string) { - const next = { ...highlightStore }; - for (const verse of selectedVerses) { - next[`${book}.${chapter}.${verse}`] = color; - } - setHighlightStore(next); - persistHighlights(next); + applyHighlight(selectedVerses, color); closeAndClearSelection(); } function handleClearHighlight(color: string) { - const next = { ...highlightStore }; - for (const verse of selectedVerses) { - const passageId = `${book}.${chapter}.${verse}`; - if (next[passageId] === color) delete next[passageId]; - } - setHighlightStore(next); - persistHighlights(next); + removeHighlight(selectedVerses, color); // Multiple colors active → keep open so the user can remove others (AC 8a); // last color removed → dismiss (AC 8). @@ -844,7 +812,10 @@ function UserMenu() { if (onSignInPress) { onSignInPress(); } else { - void signIn({ scopes: ['profile'] }); + void signIn({ + scopes: ['profile'], + permissions: [SignInWithYouVersionPermission.highlights], + }); } }; const handleSignOut = () => { diff --git a/packages/ui/src/lib/use-reader-highlights.test.tsx b/packages/ui/src/lib/use-reader-highlights.test.tsx new file mode 100644 index 00000000..32129966 --- /dev/null +++ b/packages/ui/src/lib/use-reader-highlights.test.tsx @@ -0,0 +1,238 @@ +import { act, renderHook } from '@testing-library/react'; +import { createElement, type ContextType, type ReactNode } from 'react'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +// Mock only `useHighlights`; keep the real `YouVersionAuthContext` so we can +// drive signed-in state through a Provider. +const { useHighlightsMock, createHighlight, deleteHighlight } = vi.hoisted(() => ({ + useHighlightsMock: vi.fn(), + createHighlight: vi.fn(), + deleteHighlight: vi.fn(), +})); + +vi.mock('@youversion/platform-react-hooks', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, useHighlights: useHighlightsMock }; +}); + +import { YouVersionAuthContext } from '@youversion/platform-react-hooks'; +import { useReaderHighlights } from './use-reader-highlights'; + +type Highlight = { version_id: number; passage_id: string; color: string }; + +const signedIn = { + userInfo: { userId: 'user-1' }, + setUserInfo: () => {}, + isLoading: false, + error: null, +} as unknown as ContextType; + +function wrapper({ children }: { children: ReactNode }) { + return createElement(YouVersionAuthContext.Provider, { value: signedIn }, children); +} + +function mockServerHighlights(data: Highlight[]) { + useHighlightsMock.mockReturnValue({ + highlights: { data, next_page_token: null }, + loading: false, + error: null, + refetch: vi.fn(), + createHighlight, + deleteHighlight, + getRecentColors: vi.fn(), + }); +} + +const JHN3 = { versionId: 111, book: 'JHN', chapter: '3' }; + +describe('useReaderHighlights (signed in)', () => { + beforeEach(() => { + vi.clearAllMocks(); + createHighlight.mockResolvedValue({}); + deleteHighlight.mockResolvedValue(undefined); + mockServerHighlights([]); + }); + + it('fetches highlights for the current version + chapter scope', () => { + renderHook(() => useReaderHighlights(JHN3), { wrapper }); + expect(useHighlightsMock).toHaveBeenCalledWith( + { version_id: 111, passage_id: 'JHN.3' }, + { enabled: true }, + ); + }); + + it('builds the map from server data, filtered to the current version + chapter', () => { + mockServerHighlights([ + { version_id: 111, passage_id: 'JHN.3.16', color: 'ffff00' }, + { version_id: 111, passage_id: 'JHN.3.17', color: 'aabbcc' }, + { version_id: 111, passage_id: 'GEN.1.1', color: 'ddeeff' }, // other chapter → excluded + { version_id: 999, passage_id: 'JHN.3.16', color: '000000' }, // other version → excluded + ]); + + const { result } = renderHook(() => useReaderHighlights(JHN3), { wrapper }); + + expect(result.current.highlightsByPassageId).toEqual({ + 'JHN.3.16': 'ffff00', + 'JHN.3.17': 'aabbcc', + }); + }); + + it('normalizes server highlight colors to lowercase so removeHighlight matches popover palette', () => { + mockServerHighlights([{ version_id: 111, passage_id: 'JHN.3.16', color: 'FFFF00' }]); + + const { result } = renderHook(() => useReaderHighlights(JHN3), { wrapper }); + expect(result.current.highlightsByPassageId).toEqual({ 'JHN.3.16': 'ffff00' }); + + // Popover emits lowercase palette values; removal must match the normalized store. + act(() => result.current.removeHighlight([16], 'ffff00')); + + expect(deleteHighlight).toHaveBeenCalledWith('JHN.3.16', { version_id: 111 }); + expect(result.current.highlightsByPassageId).toEqual({}); + }); + + it('applyHighlight posts one createHighlight per verse with USFM passage_id and optimistically paints', () => { + const { result } = renderHook(() => useReaderHighlights(JHN3), { wrapper }); + + act(() => result.current.applyHighlight([16, 17], 'ffff00')); + + expect(createHighlight).toHaveBeenCalledTimes(2); + expect(createHighlight).toHaveBeenCalledWith({ + version_id: 111, + passage_id: 'JHN.3.16', + color: 'ffff00', + }); + expect(createHighlight).toHaveBeenCalledWith({ + version_id: 111, + passage_id: 'JHN.3.17', + color: 'ffff00', + }); + expect(result.current.highlightsByPassageId).toEqual({ + 'JHN.3.16': 'ffff00', + 'JHN.3.17': 'ffff00', + }); + }); + + it('removeHighlight deletes only passages matching the color and clears them from the map', () => { + mockServerHighlights([ + { version_id: 111, passage_id: 'JHN.3.16', color: 'ffff00' }, + { version_id: 111, passage_id: 'JHN.3.17', color: 'aabbcc' }, + ]); + + const { result } = renderHook(() => useReaderHighlights(JHN3), { wrapper }); + + // Remove ffff00 across both verses; only 3.16 matches that color. + act(() => result.current.removeHighlight([16, 17], 'ffff00')); + + expect(deleteHighlight).toHaveBeenCalledTimes(1); + expect(deleteHighlight).toHaveBeenCalledWith('JHN.3.16', { version_id: 111 }); + expect(result.current.highlightsByPassageId).toEqual({ 'JHN.3.17': 'aabbcc' }); + }); +}); + +describe('useReaderHighlights (auth transitions)', () => { + function authValue(userId: string | null): ContextType { + return { + userInfo: userId ? { userId } : null, + setUserInfo: () => {}, + isLoading: false, + error: null, + } as unknown as ContextType; + } + + // Wrapper reads the current auth from a mutable ref so a `rerender` can flip + // the signed-in user (or sign out) without remounting the hook. + function renderWithAuth(initialUserId: string | null) { + let auth = authValue(initialUserId); + const dynamicWrapper = ({ children }: { children: ReactNode }) => + createElement(YouVersionAuthContext.Provider, { value: auth }, children); + const utils = renderHook(() => useReaderHighlights(JHN3), { wrapper: dynamicWrapper }); + const setUser = (userId: string | null) => { + auth = authValue(userId); + utils.rerender(); + }; + return { ...utils, setUser }; + } + + beforeEach(() => { + vi.clearAllMocks(); + createHighlight.mockResolvedValue({}); + deleteHighlight.mockResolvedValue(undefined); + }); + + it('clears the store when the reader signs out (userId -> null)', () => { + mockServerHighlights([{ version_id: 111, passage_id: 'JHN.3.16', color: 'ffff00' }]); + + const { result, setUser } = renderWithAuth('user-1'); + expect(result.current.highlightsByPassageId).toEqual({ 'JHN.3.16': 'ffff00' }); + + act(() => setUser(null)); + + expect(result.current.highlightsByPassageId).toEqual({}); + }); + + it("clears the previous user's highlights when a different user signs in", () => { + mockServerHighlights([{ version_id: 111, passage_id: 'JHN.3.16', color: 'ffff00' }]); + + const { result, setUser } = renderWithAuth('user-1'); + expect(result.current.highlightsByPassageId).toEqual({ 'JHN.3.16': 'ffff00' }); + + // user-2 signs in but the fetch hasn't refetched for them: `useHighlights` + // still returns user-1's collection (same reference). It must NOT be painted. + act(() => setUser('user-2')); + + expect(result.current.highlightsByPassageId).toEqual({}); + }); + + it("adopts the new user's highlights once their fetch resolves", () => { + mockServerHighlights([{ version_id: 111, passage_id: 'JHN.3.16', color: 'ffff00' }]); + + const { result, setUser } = renderWithAuth('user-1'); + act(() => setUser('user-2')); + expect(result.current.highlightsByPassageId).toEqual({}); + + // Fresh collection for user-2 (new reference) is trusted and painted. + mockServerHighlights([{ version_id: 111, passage_id: 'JHN.3.19', color: 'aabbcc' }]); + act(() => setUser('user-2')); + + expect(result.current.highlightsByPassageId).toEqual({ 'JHN.3.19': 'aabbcc' }); + }); +}); + +describe('useReaderHighlights (signed out)', () => { + // No wrapper: `useContext(YouVersionAuthContext)` is null, so `isSignedIn` is + // false and highlights stay ephemeral (never persisted to the API). + beforeEach(() => { + vi.clearAllMocks(); + mockServerHighlights([]); + }); + + it('disables the fetch when there is no signed-in user', () => { + renderHook(() => useReaderHighlights(JHN3)); + expect(useHighlightsMock).toHaveBeenCalledWith( + { version_id: 111, passage_id: 'JHN.3' }, + { enabled: false }, + ); + }); + + it('applyHighlight paints ephemerally without calling createHighlight', () => { + const { result } = renderHook(() => useReaderHighlights(JHN3)); + + act(() => result.current.applyHighlight([16, 17], 'ffff00')); + + expect(createHighlight).not.toHaveBeenCalled(); + expect(result.current.highlightsByPassageId).toEqual({ + 'JHN.3.16': 'ffff00', + 'JHN.3.17': 'ffff00', + }); + }); + + it('removeHighlight clears ephemerally without calling deleteHighlight', () => { + const { result } = renderHook(() => useReaderHighlights(JHN3)); + + act(() => result.current.applyHighlight([16], 'ffff00')); + act(() => result.current.removeHighlight([16], 'ffff00')); + + expect(deleteHighlight).not.toHaveBeenCalled(); + expect(result.current.highlightsByPassageId).toEqual({}); + }); +}); diff --git a/packages/ui/src/lib/use-reader-highlights.ts b/packages/ui/src/lib/use-reader-highlights.ts new file mode 100644 index 00000000..a1dd2792 --- /dev/null +++ b/packages/ui/src/lib/use-reader-highlights.ts @@ -0,0 +1,137 @@ +'use client'; + +import { useHighlights, YouVersionAuthContext } from '@youversion/platform-react-hooks'; +import { useCallback, useContext, useEffect, useState } from 'react'; + +type UseReaderHighlightsArgs = { + versionId: number; + book: string; + chapter: string; +}; + +type UseReaderHighlightsResult = { + /** Full passage_id (e.g. "JHN.3.16") -> 6-char lowercase hex, no `#`. */ + highlightsByPassageId: Record; + applyHighlight: (verses: number[], color: string) => void; + removeHighlight: (verses: number[], color: string) => void; +}; + +/** + * Encapsulates the reader's highlight state. When the user is signed in, + * highlights load from and persist to the Highlights API (chapter-scoped); when + * not signed in they are ephemeral session state (never persisted). + * + * Reads auth via the raw `YouVersionAuthContext` (null when no auth provider is + * mounted) rather than `useYVAuth()`, which throws — the reader must keep + * working for consumers who don't enable auth. + * + * Failure UX is intentionally out of scope: mutations apply optimistically and + * log on error (no revert/toast). Reconciliation against the server (source of + * truth) happens via refetch — `useHighlights` refetches on a successful + * mutation, and on failure we refetch here so the optimistic store can't stay + * permanently out of sync until the next navigation. + */ +export function useReaderHighlights({ + versionId, + book, + chapter, +}: UseReaderHighlightsArgs): UseReaderHighlightsResult { + const authContext = useContext(YouVersionAuthContext); + const userId = authContext?.userInfo?.userId ?? null; + const isSignedIn = !!authContext?.userInfo; + + const chapterPassageId = `${book}.${chapter}`; + const chapterPrefix = `${chapterPassageId}.`; + + const { highlights, createHighlight, deleteHighlight, refetch } = useHighlights( + { version_id: versionId, passage_id: chapterPassageId }, + { enabled: isSignedIn }, + ); + + // Optimistic, client-side view of the current scope's highlights. + const [optimisticStore, setOptimisticStore] = useState>({}); + + // Clear synchronously (during render) the moment the scope key changes, so the + // previous version's colors for a shared passage_id (e.g. "JHN.3.16") can't + // paint over the new scope for one frame before the fetch resolves. The user + // identity is part of the key: signing out (userId -> null) or switching users + // resets the store immediately, so one reader can't keep painting another + // user's server-backed highlights until the next navigation/remount. + const scopeKey = `${userId ?? 'signed-out'}:${versionId}:${chapterPassageId}`; + const [loadedScopeKey, setLoadedScopeKey] = useState(scopeKey); + const [staleHighlights, setStaleHighlights] = useState(null); + if (loadedScopeKey !== scopeKey) { + setLoadedScopeKey(scopeKey); + setOptimisticStore({}); + setStaleHighlights(highlights); + } + + // Server is source of truth when signed in. Rebuild the store from the fetched + // collection, filtered to the current scope so a stale collection (mid-refetch + // after a scope change) can never leak another version's/chapter's entries. + useEffect(() => { + if (!isSignedIn || !highlights) return; + if (highlights === staleHighlights) return; + const next: Record = {}; + for (const highlight of highlights.data) { + if (highlight.version_id !== versionId) continue; + if (!highlight.passage_id.startsWith(chapterPrefix)) continue; + next[highlight.passage_id] = highlight.color.toLowerCase(); + } + setOptimisticStore(next); + }, [isSignedIn, highlights, staleHighlights, versionId, chapterPrefix]); + + const applyHighlight = useCallback( + (verses: number[], color: string) => { + setOptimisticStore((prev) => { + const next = { ...prev }; + for (const verse of verses) next[`${chapterPrefix}${verse}`] = color; + return next; + }); + if (!isSignedIn) return; + for (const verse of verses) { + createHighlight({ + version_id: versionId, + passage_id: `${chapterPrefix}${verse}`, + color, + }).catch((error) => { + console.error(error); + // Success already refetches inside `useHighlights`; on failure we + // reconcile the optimistic store against the server here. + refetch(); + }); + } + }, + [isSignedIn, versionId, chapterPrefix, createHighlight, refetch], + ); + + const removeHighlight = useCallback( + (verses: number[], color: string) => { + // Compute the affected passages from current state before mutating, so the + // API calls (a side effect) stay out of the state updater — which React + // may invoke twice in StrictMode. + const cleared = verses + .map((verse) => `${chapterPrefix}${verse}`) + .filter((passageId) => optimisticStore[passageId] === color); + if (cleared.length === 0) return; + + setOptimisticStore((prev) => { + const next = { ...prev }; + for (const passageId of cleared) delete next[passageId]; + return next; + }); + if (!isSignedIn) return; + for (const passageId of cleared) { + deleteHighlight(passageId, { version_id: versionId }).catch((error) => { + console.error(error); + // Success already refetches inside `useHighlights`; on failure we + // reconcile the optimistic store against the server here. + refetch(); + }); + } + }, + [optimisticStore, isSignedIn, versionId, chapterPrefix, deleteHighlight, refetch], + ); + + return { highlightsByPassageId: optimisticStore, applyHighlight, removeHighlight }; +}