diff --git a/.changeset/bible-reader-controlled-mode.md b/.changeset/bible-reader-controlled-mode.md new file mode 100644 index 00000000..65e99592 --- /dev/null +++ b/.changeset/bible-reader-controlled-mode.md @@ -0,0 +1,10 @@ +--- +'@youversion/platform-react-ui': minor +--- + +BibleReader gains a controlled highlights mode (YPE-3705) for hosts that own highlight data themselves (e.g. React Native / Expo DOM hosts keeping the user token out of the WebView). + +- New `BibleReader.Root` props: `highlights?: Highlight[]` (core API shape; presence puts the highlight slice in controlled mode, latched at first mount), `onVerseSelect` (both modes, fires on every selection change — `verses: []` whenever a non-empty selection clears, including after highlight/copy/share actions, popover dismiss, and navigation), and `onHighlightApply` / `onHighlightRemove` (controlled mode only; ignored in self-contained mode). Payloads are serializable, bridge-safe objects (`BibleReaderVerseSelection`, `BibleReaderHighlightIntent`) with always-per-verse `passageIds`. +- In controlled mode the reader is a pure projection: highlights render solely from the prop (filtered by displayed version + chapter, range USFMs like `JHN.3.16-18` expanded per verse), color taps emit intents and paint nothing until the host round-trips an updated prop, and neither the highlights API nor any local store is touched. No auth surface can originate from the highlight path. +- Self-contained behavior (no `highlights` prop) uses the server-backed `useBibleReaderHighlights` path (YPE-1034), dark-launched behind `HIGHLIGHTS_LIVE`. +- `@youversion/platform-react-ui` now re-exports the `Highlight` type from `@youversion/platform-core`. diff --git a/CONTEXT.md b/CONTEXT.md index 1c3b95f1..4d283d04 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -45,6 +45,26 @@ gated on the internal `HIGHLIGHTS_LIVE` dark-launch flag and an authenticated user — while the flag is off or the user has no session, the reader is inert: no fetches, no writes, nothing rendered from the API. +## Controlled mode + +The alternate `BibleReader` posture (YPE-3705): the host owns highlight +data, auth, and persistence (e.g. a React Native / Expo DOM host keeping +the user token out of the WebView). The host passes `highlights: +Highlight[]` into `BibleReader.Root` and receives highlight intents; the +reader is a pure projection — no API calls, no local persistence, no +sign-in surface. Presence of the prop selects the posture (latched at +first mount). Controlled mode bypasses `HIGHLIGHTS_LIVE`: the color row +is always interactive so the public prop surface can ship while +self-contained stays dark. + +## Highlight intent + +The reader's request that a highlight be applied or removed +(`onHighlightApply` / `onHighlightRemove`). Always an intent, never a +completed fact — intent and fact are deliberately never given the same +name. In controlled mode the highlight appears only when the host +round-trips an updated `highlights` prop. + ## Verse action popover The floating action bar (YPE-642) that appears over a verse selection, diff --git a/docs/adr/YPE-3705-controlled-mode.md b/docs/adr/YPE-3705-controlled-mode.md new file mode 100644 index 00000000..5c545916 --- /dev/null +++ b/docs/adr/YPE-3705-controlled-mode.md @@ -0,0 +1,87 @@ +# YPE-3705 — Controlled (headless) BibleReader highlights + +Status: **Accepted** (grilling session 2026-07-20, Dustin + Claude) +Component: `packages/ui/src/components/bible-reader.tsx` (+ `packages/ui/src/lib/highlight-projection.ts`) +Epic: YPE-2894 (RN Expo Highlights) · Blocks: YPE-3710, YPE-3711, YPE-3712 +Related: PR #288 (highlights UI + state machine + API, open) · PR #269 (verse action popover) · YPE-1034 ADR (on PR #288) + +## Context + +Native hosts (the RN Expo SDK embedding the web reader as a DOM component) own +highlight data, auth, and persistence natively; the epic's goal is keeping the +**user token** out of the WebView. Bible *content* fetching is app-key catalog +traffic and is unchanged. + +The reader therefore has two postures for its highlight slice: +**self-contained** (YPE-1034 — the reader fetches and writes +highlights through the SDK's own auth session) and **controlled** (this ADR — +the host passes `highlights: Highlight[]` into `BibleReader.Root` and receives +intent events; presence of the prop selects the posture, latched at first +mount). The prop/event surface and its edge semantics are documented by the +public types and the tests in `bible-reader-controlled.test.tsx`; this ADR +records only what the code cannot show. + +## Coexistence model + +Controlled mode performs **no highlight persistence of any kind** — no API +calls, no localStorage, no sign-in/permission dialogs or redirects. The two +postures never mix: a controlled reader's highlight slice is a pure projection +of the prop, and a self-contained reader never calls the intent-event props. +`onVerseSelect` is a mode-independent observation; `onHighlightApply`/ +`onHighlightRemove` are controlled-only requests (a self-contained "applied" +notification would be a different event under a different name). + +## Non-obvious decisions + +- **Pure projection, no optimistic echo.** A color tap paints nothing; the + highlight appears when the host round-trips an updated prop. The native data + layer (YPE-3708) is the only optimistic layer — two optimistic layers cannot + agree on failure semantics, and only the host knows whether a write succeeded. +- **Controlled mode bypasses the `HIGHLIGHTS_LIVE` dark-launch flag** (a + PR #288 concept). The flag strictly means "is the self-contained server path + live"; the color row is always interactive in controlled mode. Consequence: + the controlled prop surface is public released API while self-contained + stays dark. + +## Rejected alternatives + +- **Fully headless reader (pure projection for everything)** — would require + the host to supply passage HTML, books, and version metadata as props, + tripling the bridge surface for app-key catalog data that isn't user data. + The pure-presentational layer already exists below (`BibleTextView`); the + epic's goal is native ownership of *user data and tokens*, so only the + highlight slice becomes a pure projection while the reader keeps fetching + content. +- **Explicit mode prop** — a second axis that can contradict the data; + presence-of-prop is the established controlled/uncontrolled idiom. +- **Chapter-scoped `Record` prop** — loses the identity fields + that make stale bridge data harmless, and overloads the internal + `highlightedVerses` name. +- **Reader-side optimism** — see above. +- **Machine-level controlled state** — controlled mode wants the machine + provably inert; that is an enable guard, not a statechart region. +- **Stacking on PR #288** — would gate the RN epic on #288's merge date; built + in parallel on `main` instead. The re-seat onto the shared adapter has since + landed (see below). + +## Integration with the self-contained path (landed) + +Controlled mode is seated in the `useBibleReaderHighlights` adapter (the +reader's single seam onto highlights, shared with YPE-1034's self-contained +path) via a `controlled` input, latched by `Root` at first mount: + +- When `controlled` is set, the `useHighlights` fetch stays `enabled: false` + and the adapter performs no writes and keeps no optimistic overlay — + controlled mode is a pure projection of the host's highlights + (`deriveHighlightedVerses`) plus intent forwarding from `apply`/`remove`. +- Controlled mode bypasses `isHighlightsLive()` — the dark-launch flag gates + only the self-contained server path, so the color row stays interactive and + the controlled prop surface ships while self-contained remains dark. +- The glossary (`CONTEXT.md`) defines "Controlled mode" and "Highlight + intent"; intent and fact are deliberately never given the same name. + +## Out of scope + +Notes, custom colors, >5 colors (epic fast-follow); self-contained notification +events; controlled *content* (the reader keeps fetching passages/books/versions); +offline/write-queue concerns (native-side, YPE-3717). diff --git a/packages/ui/AGENTS.md b/packages/ui/AGENTS.md index 29a1259e..aa9a56fb 100644 --- a/packages/ui/AGENTS.md +++ b/packages/ui/AGENTS.md @@ -19,7 +19,7 @@ src/index.ts # Entry point (re-exports components, types, hooks) - Components exported from `src/components/` - Re-exports from `@youversion/platform-core`: - `SignInWithYouVersionPermission`, `SignInWithYouVersionResult`, `YouVersionAPIUsers` - - `ApiConfig`, `AuthenticationState` types + - `ApiConfig`, `AuthenticationState`, `Highlight` types - Re-exports from `@youversion/platform-react-hooks`: - `YouVersionProvider`, `useYVAuth`, `UseYVAuthReturn` type diff --git a/packages/ui/src/components/bible-reader-controlled.test.tsx b/packages/ui/src/components/bible-reader-controlled.test.tsx new file mode 100644 index 00000000..c49711c3 --- /dev/null +++ b/packages/ui/src/components/bible-reader-controlled.test.tsx @@ -0,0 +1,521 @@ +/** + * @vitest-environment jsdom + */ +// Controlled-mode (YPE-3705) tests for BibleReader highlights: pure projection +// from the `highlights` prop, provable inertness (no highlight network or +// localStorage traffic), intent events, and mode latching. +// We stub ResizeObserver for jsdom (used by Radix/@floating-ui). The stub methods are intentionally no-ops. +/* eslint-disable @typescript-eslint/no-empty-function */ +import { describe, it, expect, vi, beforeEach, afterEach, type MockInstance } from 'vitest'; +import { render, screen, fireEvent, waitFor } from '@testing-library/react'; +import type { + BibleBook, + BiblePassage, + BibleVersion, + Highlight, + Language, +} from '@youversion/platform-core'; +import { + useBooks, + useFilteredVersions, + useHighlights, + useLanguage, + useLanguages, + useOrganizations, + usePassage, + useTheme, + useVersion, + useVersions, +} from '@youversion/platform-react-hooks'; +import { BibleReader, type BibleReaderRootProps } from './bible-reader'; +import { HIGHLIGHT_COLORS } from './verse-action-popover'; + +class ResizeObserverMock { + observe() {} + unobserve() {} + disconnect() {} +} +globalThis.ResizeObserver = ResizeObserverMock as unknown as typeof ResizeObserver; + +// jsdom under this Node version refuses localStorage for opaque origins +// (`SecurityError`). Theme-settings hydration in Root reads it on mount, so +// provide an in-memory stub for the suite. +const memoryStore = new Map(); +const localStorageMock: Storage = { + get length() { + return memoryStore.size; + }, + clear: () => memoryStore.clear(), + getItem: (key) => memoryStore.get(key) ?? null, + setItem: (key, value) => { + memoryStore.set(key, String(value)); + }, + removeItem: (key) => { + memoryStore.delete(key); + }, + key: (index) => [...memoryStore.keys()][index] ?? null, +}; +Object.defineProperty(globalThis, 'localStorage', { + value: localStorageMock, + configurable: true, + writable: true, +}); + +// jsdom does not implement Element#scrollTo (BibleReader.Content scrolls to +// top on chapter change). +if (!Element.prototype.scrollTo) { + Element.prototype.scrollTo = () => {}; +} + +vi.mock('@youversion/platform-react-hooks', async () => { + const actual = await vi.importActual('@youversion/platform-react-hooks'); + return { + ...actual, + useBooks: vi.fn(), + useFilteredVersions: vi.fn(), + useHighlights: vi.fn(), + useLanguage: vi.fn(), + useLanguages: vi.fn(), + useOrganizations: vi.fn(), + usePassage: vi.fn(), + useTheme: vi.fn(), + useVersion: vi.fn(), + useVersions: vi.fn(), + }; +}); + +const YELLOW = HIGHLIGHT_COLORS[0]; // fffe00 +const GREEN = HIGHLIGHT_COLORS[1]; // 5dff79 + +const mockBooks: BibleBook[] = [ + { + id: 'JHN', + title: 'John', + full_title: 'The Gospel According to John', + canon: 'new_testament', + abbreviation: 'John', + chapters: [ + { id: '1', title: '1', passage_id: 'JHN.1' }, + { id: '2', title: '2', passage_id: 'JHN.2' }, + ], + }, +]; + +const mockVersion = { + id: 111, + localized_abbreviation: 'NIV', + abbreviation: 'NIV', + title: 'New International Version', + language_tag: 'en', +} as BibleVersion; + +// Same shape as the real passages API mock data: `.yv-v` markers that the +// Bible HTML transformer expands into per-verse wrappers. +const mockPassage: BiblePassage = { + id: 'JHN.1', + content: + '
' + + '1In the beginning was the Word. ' + + '2He was with God in the beginning. ' + + '3Through him all things were made.' + + '
', + reference: 'John 1', +}; + +function setupDefaultMocks() { + vi.mocked(useTheme).mockReturnValue('light'); + vi.mocked(useBooks).mockReturnValue({ + books: { data: [...mockBooks], next_page_token: null }, + loading: false, + error: null, + refetch: vi.fn(), + }); + vi.mocked(useVersion).mockReturnValue({ + version: mockVersion, + loading: false, + error: null, + refetch: vi.fn(), + }); + vi.mocked(usePassage).mockReturnValue({ + passage: mockPassage, + loading: false, + error: null, + refetch: vi.fn(), + }); + // Self-contained path is inert in these tests (controlled mode owns the + // surface). Keep useHighlights stubbed so Content can mount without a + // YouVersionProvider. + vi.mocked(useHighlights).mockReturnValue({ + highlights: { data: [], next_page_token: null }, + loading: false, + error: null, + refetch: vi.fn(), + createHighlight: vi.fn(), + deleteHighlight: vi.fn(), + }); + vi.mocked(useLanguages).mockReturnValue({ + languages: { data: [] as Language[], next_page_token: null }, + loading: false, + error: null, + refetch: vi.fn(), + }); + vi.mocked(useLanguage).mockReturnValue({ + language: { id: 'en', language: 'English', display_names: { en: 'English' } } as Language, + loading: false, + error: null, + refetch: vi.fn(), + }); + vi.mocked(useVersions).mockReturnValue({ + versions: { data: [], next_page_token: null }, + loading: false, + error: null, + refetch: vi.fn(), + }); + vi.mocked(useFilteredVersions).mockReturnValue([]); + vi.mocked(useOrganizations).mockReturnValue({ organizations: new Map() }); +} + +// Toolbar is mounted alongside Content so the adversarial surface (chapter / +// version pickers, settings) is present in every test. +function readerJsx(props: Partial = {}) { + return ( + + + + + ); +} + +function renderReader(props: Partial = {}) { + return render(readerJsx(props)); +} + +function getVerseEl(container: HTMLElement, verse: number): HTMLElement { + const els = container.querySelectorAll(`.yv-v[v="${verse}"]`); + const el = els[els.length - 1]; + if (!el) throw new Error(`Verse ${verse} not rendered`); + return el; +} + +function selectVerse(container: HTMLElement, verse: number) { + fireEvent.click(getVerseEl(container, verse)); +} + +/** rgba() string the reader paints for a highlight fill (0.35 alpha). */ +function fillFor(hex: string): string { + const r = parseInt(hex.slice(0, 2), 16); + const g = parseInt(hex.slice(2, 4), 16); + const b = parseInt(hex.slice(4, 6), 16); + return `rgba(${r}, ${g}, ${b}, 0.35)`; +} + +function getApplyButtons() { + return screen + .getAllByRole('button') + .filter((btn) => btn.getAttribute('aria-label')?.includes('Apply')); +} + +function getClearButtons() { + return screen + .getAllByRole('button') + .filter((btn) => btn.getAttribute('aria-label')?.includes('Clear')); +} + +const selection = (verses: number[]) => ({ + versionId: 111, + book: 'JHN', + chapter: '1', + verses, + passageIds: verses.map((verse) => `JHN.1.${verse}`), +}); + +beforeEach(() => { + vi.clearAllMocks(); + localStorage.clear(); + setupDefaultMocks(); +}); + +describe('BibleReader controlled mode - pure projection', () => { + it('paints verses from the highlights prop, expanding range USFMs', () => { + const highlights: Highlight[] = [ + { version_id: 111, passage_id: 'JHN.1.1', color: YELLOW }, + { version_id: 111, passage_id: 'JHN.1.2-3', color: GREEN }, + ]; + const { container } = renderReader({ highlights }); + + expect(getVerseEl(container, 1).style.backgroundColor).toBe(fillFor(YELLOW)); + expect(getVerseEl(container, 2).style.backgroundColor).toBe(fillFor(GREEN)); + expect(getVerseEl(container, 3).style.backgroundColor).toBe(fillFor(GREEN)); + }); + + it('ignores entries for other versions and chapters', () => { + const highlights: Highlight[] = [ + { version_id: 222, passage_id: 'JHN.1.1', color: YELLOW }, + { version_id: 111, passage_id: 'JHN.2.2', color: YELLOW }, + { version_id: 111, passage_id: 'GEN.1.3', color: YELLOW }, + ]; + const { container } = renderReader({ highlights }); + + for (const verse of [1, 2, 3]) { + expect(getVerseEl(container, verse).style.backgroundColor).toBe(''); + } + }); + + it('ignores entries with colors outside the built-in swatches (no un-removable paint)', () => { + const highlights: Highlight[] = [ + { version_id: 111, passage_id: 'JHN.1.1', color: 'abcdef' }, + { version_id: 111, passage_id: 'JHN.1.2', color: YELLOW }, + ]; + const { container } = renderReader({ highlights }); + + expect(getVerseEl(container, 1).style.backgroundColor).toBe(''); + expect(getVerseEl(container, 2).style.backgroundColor).toBe(fillFor(YELLOW)); + }); + + it('re-projects when the highlights prop changes (host round-trip)', () => { + const { container, rerender } = renderReader({ highlights: [] }); + expect(getVerseEl(container, 1).style.backgroundColor).toBe(''); + + rerender( + readerJsx({ highlights: [{ version_id: 111, passage_id: 'JHN.1.1', color: YELLOW }] }), + ); + expect(getVerseEl(container, 1).style.backgroundColor).toBe(fillFor(YELLOW)); + }); +}); + +describe('BibleReader controlled mode - provable inertness', () => { + it('never touches the network or localStorage for highlights, even across select/apply/clear', async () => { + const fetchSpy = vi.fn(); + vi.stubGlobal('fetch', fetchSpy); + const getItemSpy = vi.spyOn(Storage.prototype, 'getItem'); + const setItemSpy = vi.spyOn(Storage.prototype, 'setItem'); + + try { + const { container } = renderReader({ + highlights: [{ version_id: 111, passage_id: 'JHN.1.2', color: GREEN }], + onHighlightApply: vi.fn(), + onHighlightRemove: vi.fn(), + }); + + // Apply a color to verse 1. + selectVerse(container, 1); + await waitFor(() => expect(getApplyButtons().length).toBeGreaterThan(0)); + fireEvent.click(getApplyButtons()[0]!); + + // Clear the highlight showing on verse 2. + selectVerse(container, 2); + await waitFor(() => expect(getClearButtons()).toHaveLength(1)); + fireEvent.click(getClearButtons()[0]!); + + const highlightKey = (key: unknown) => String(key).includes('highlights'); + expect(getItemSpy.mock.calls.some(([key]) => highlightKey(key))).toBe(false); + expect(setItemSpy.mock.calls.some(([key]) => highlightKey(key))).toBe(false); + expect(fetchSpy.mock.calls.some(([input]) => String(input).includes('/v1/highlights'))).toBe( + false, + ); + } finally { + vi.unstubAllGlobals(); + getItemSpy.mockRestore(); + setItemSpy.mockRestore(); + } + }); + + it('color taps paint nothing (no optimistic echo)', async () => { + const { container } = renderReader({ highlights: [], onHighlightApply: vi.fn() }); + + selectVerse(container, 1); + await waitFor(() => expect(getApplyButtons().length).toBeGreaterThan(0)); + fireEvent.click(getApplyButtons()[0]!); + + expect(getVerseEl(container, 1).style.backgroundColor).toBe(''); + }); +}); + +describe('BibleReader controlled mode - events', () => { + it('emits onVerseSelect on every selection change, [] on clear', () => { + const onVerseSelect = vi.fn(); + const { container } = renderReader({ highlights: [], onVerseSelect }); + + selectVerse(container, 2); + expect(onVerseSelect).toHaveBeenLastCalledWith(selection([2])); + + selectVerse(container, 1); + expect(onVerseSelect).toHaveBeenLastCalledWith(selection([1, 2])); + + selectVerse(container, 2); + expect(onVerseSelect).toHaveBeenLastCalledWith(selection([1])); + + selectVerse(container, 1); + expect(onVerseSelect).toHaveBeenLastCalledWith(selection([])); + expect(onVerseSelect).toHaveBeenCalledTimes(4); + }); + + it('emits onVerseSelect([]) when a color tap clears the selection', async () => { + const onVerseSelect = vi.fn(); + const { container } = renderReader({ + highlights: [], + onVerseSelect, + onHighlightApply: vi.fn(), + }); + + selectVerse(container, 1); + await waitFor(() => expect(getApplyButtons().length).toBeGreaterThan(0)); + onVerseSelect.mockClear(); + + fireEvent.click(getApplyButtons()[0]!); + expect(onVerseSelect).toHaveBeenCalledTimes(1); + expect(onVerseSelect).toHaveBeenCalledWith(selection([])); + }); + + it('emits onVerseSelect([]) when navigation clears the selection', async () => { + const onVerseSelect = vi.fn(); + const { container } = renderReader({ highlights: [], onVerseSelect }); + + selectVerse(container, 1); + onVerseSelect.mockClear(); + + fireEvent.click(screen.getByRole('button', { name: /next chapter/i })); + + await waitFor(() => expect(onVerseSelect).toHaveBeenCalledTimes(1)); + // Payload carries the new location and an empty selection. + expect(onVerseSelect).toHaveBeenCalledWith({ + versionId: 111, + book: 'JHN', + chapter: '2', + verses: [], + passageIds: [], + }); + }); + + it('emits onVerseSelect in self-contained mode too', () => { + const onVerseSelect = vi.fn(); + const { container } = renderReader({ onVerseSelect }); + + selectVerse(container, 3); + expect(onVerseSelect).toHaveBeenCalledWith(selection([3])); + }); + + it('emits onHighlightApply with the exact intent payload and clears the selection', async () => { + const onHighlightApply = vi.fn(); + const { container } = renderReader({ highlights: [], onHighlightApply }); + + selectVerse(container, 1); + selectVerse(container, 3); + await waitFor(() => expect(getApplyButtons().length).toBeGreaterThan(0)); + fireEvent.click(getApplyButtons()[0]!); + + expect(onHighlightApply).toHaveBeenCalledTimes(1); + expect(onHighlightApply).toHaveBeenCalledWith({ ...selection([1, 3]), color: YELLOW }); + + // Selection cleared per existing popover behavior. + await waitFor(() => { + expect(screen.queryByRole('dialog')).toBeNull(); + }); + expect(getVerseEl(container, 1).classList.contains('yv-v-selected')).toBe(false); + }); + + it('emits onHighlightRemove scoped to the selected verses showing that color', async () => { + const onHighlightRemove = vi.fn(); + const highlights: Highlight[] = [ + { version_id: 111, passage_id: 'JHN.1.1', color: YELLOW }, + { version_id: 111, passage_id: 'JHN.1.2', color: GREEN }, + ]; + const rerenderWith = (next: Highlight[]) => + rerender(readerJsx({ highlights: next, onHighlightRemove })); + const { container, rerender } = renderReader({ highlights, onHighlightRemove }); + + selectVerse(container, 1); + selectVerse(container, 2); + selectVerse(container, 3); + // Two active colors -> two clear circles, ordered yellow then green. + await waitFor(() => expect(getClearButtons()).toHaveLength(2)); + + fireEvent.click(getClearButtons()[0]!); + expect(onHighlightRemove).toHaveBeenCalledTimes(1); + expect(onHighlightRemove).toHaveBeenLastCalledWith({ ...selection([1]), color: YELLOW }); + + // Green still showing on a selected verse -> popover stays open (AC 8a). + expect(screen.getByRole('dialog')).toBeTruthy(); + + // Host round-trips the removal; the reader re-projects (verse 1 unpaints, + // one clear circle remains). + rerenderWith([{ version_id: 111, passage_id: 'JHN.1.2', color: GREEN }]); + expect(getVerseEl(container, 1).style.backgroundColor).toBe(''); + await waitFor(() => expect(getClearButtons()).toHaveLength(1)); + + fireEvent.click(getClearButtons()[0]!); + expect(onHighlightRemove).toHaveBeenCalledTimes(2); + expect(onHighlightRemove).toHaveBeenLastCalledWith({ ...selection([2]), color: GREEN }); + + // Last active color removed -> popover dismisses (AC 8). + await waitFor(() => { + expect(screen.queryByRole('dialog')).toBeNull(); + }); + }); + + it('ignores onHighlightApply / onHighlightRemove in self-contained mode', async () => { + const onHighlightApply = vi.fn(); + const onHighlightRemove = vi.fn(); + const { container } = renderReader({ onHighlightApply, onHighlightRemove }); + + selectVerse(container, 1); + // Self-contained with HIGHLIGHTS_LIVE off: the color row is hidden entirely + // (`highlightsEnabled` from PR-288). Copy/Share still open the popover. + // Controlled intent callbacks must never fire. + await waitFor(() => expect(screen.getByRole('dialog')).toBeTruthy()); + expect(getApplyButtons()).toHaveLength(0); + expect(getVerseEl(container, 1).style.backgroundColor).toBe(''); + expect(onHighlightApply).not.toHaveBeenCalled(); + expect(onHighlightRemove).not.toHaveBeenCalled(); + }); +}); + +describe('BibleReader controlled mode - latching', () => { + let warnSpy: MockInstance; + + beforeEach(() => { + warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + }); + + afterEach(() => { + warnSpy.mockRestore(); + }); + + it('stays controlled when highlights flips to undefined: warns and renders no highlights', () => { + const { container, rerender } = renderReader({ + highlights: [{ version_id: 111, passage_id: 'JHN.1.2', color: GREEN }], + }); + expect(getVerseEl(container, 2).style.backgroundColor).toBe(fillFor(GREEN)); + + rerender(readerJsx()); + + expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('latched at first mount')); + // Transient undefined renders as "no highlights" — still controlled. + expect(getVerseEl(container, 1).style.backgroundColor).toBe(''); + expect(getVerseEl(container, 2).style.backgroundColor).toBe(''); + }); + + it('stays self-contained when highlights appears after mount: warns and ignores the prop', () => { + const { container, rerender } = renderReader(); + expect(getVerseEl(container, 1).style.backgroundColor).toBe(''); + + rerender( + readerJsx({ highlights: [{ version_id: 111, passage_id: 'JHN.1.1', color: YELLOW }] }), + ); + + expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('latched at first mount')); + expect(getVerseEl(container, 1).style.backgroundColor).toBe(''); + }); + + it('treats [] as controlled while undefined means self-contained', () => { + const controlled = renderReader({ highlights: [] }); + expect(getVerseEl(controlled.container, 1).style.backgroundColor).toBe(''); + controlled.unmount(); + + // Self-contained with HIGHLIGHTS_LIVE off: also empty, but the mode is + // latched (a later highlights prop would be ignored — covered above). + const selfContained = renderReader(); + expect(getVerseEl(selfContained.container, 1).style.backgroundColor).toBe(''); + expect(warnSpy).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/ui/src/components/bible-reader.stories.tsx b/packages/ui/src/components/bible-reader.stories.tsx index 3d3c499c..5f9c44d8 100644 --- a/packages/ui/src/components/bible-reader.stories.tsx +++ b/packages/ui/src/components/bible-reader.stories.tsx @@ -1,11 +1,18 @@ +import { expandPassageId } from '@/lib/highlight-projection'; import { INTER_FONT, SOURCE_SERIF_FONT } from '@/lib/verse-html-utils'; import type { Meta, StoryObj } from '@storybook/react-vite'; +import type { Highlight } from '@youversion/platform-core'; import { delay, http, HttpResponse } from 'msw'; +import { useState } from 'react'; import { expect, fn, screen, spyOn, userEvent, waitFor } from 'storybook/test'; import mockBibles from '../test/mock-data/bibles.json'; import { globalHandlers } from '../test/mocks/handlers'; import { setupAuthenticatedUser } from '../test/utils'; -import { BibleReader } from './bible-reader'; +import { + BibleReader, + type BibleReaderHighlightIntent, + type BibleReaderRootProps, +} from './bible-reader'; let signInMock: ReturnType; @@ -803,6 +810,121 @@ export const JoshuaIntroChapter: Story = { }, }; +/** + * Controlled mode (YPE-3705): the host supplies highlight data via the + * `highlights` prop (core API shape, including range USFMs) and receives + * intent events; the reader performs no highlight persistence. Tweak the + * `highlights` arg and watch the paint follow; select verses and tap colors to + * see `onVerseSelect` / `onHighlightApply` / `onHighlightRemove` in the + * Actions panel. Note: taps paint nothing here — there is no host echoing the + * intents back (see `ControlledFakeHost` for the round-trip). + */ +export const Controlled: Story = { + args: { + defaultVersionId: 111, + defaultBook: 'JHN', + defaultChapter: '1', + highlights: [ + { version_id: 111, passage_id: 'JHN.1.1', color: 'fffe00' }, + { version_id: 111, passage_id: 'JHN.1.3-5', color: '5dff79' }, + ], + onVerseSelect: fn(), + onHighlightApply: fn(), + onHighlightRemove: fn(), + }, + render: (args) => ( +
+ + + + +
+ ), +}; + +/** + * Splits a stored highlight into per-verse entries so it can be matched + * against an intent's per-verse `passageIds`. API data may hold range USFMs + * (`JHN.1.3-5`); matching those as opaque strings would silently miss, and a + * partial removal (one verse out of a range) must split the range, not drop + * it. Unexpandable ids pass through untouched. + */ +const toPerVerse = (h: Highlight): Highlight[] => { + const expanded = expandPassageId(h.passage_id); + if (!expanded) return [h]; + return expanded.verses.map((verse) => ({ + version_id: h.version_id, + passage_id: `${expanded.book}.${expanded.chapter}.${verse}`, + color: h.color, + })); +}; + +/** True when a per-verse stored entry is covered by an intent's verses. */ +const matchesIntent = (h: Highlight, intent: BibleReaderHighlightIntent) => + h.version_id === intent.versionId && intent.passageIds.includes(h.passage_id); + +/** + * Controlled mode with a stateful fake host that echoes `onHighlightApply` / + * `onHighlightRemove` back into the `highlights` prop — the executable + * reference implementation of the round-trip contract for native hosts + * (RN Expo, YPE-3710). Selecting verses and tapping a color paints only via + * the prop update; tapping an X circle un-paints the same way. + */ +export const ControlledFakeHost: Story = { + name: 'Controlled (fake host)', + args: { + defaultVersionId: 111, + defaultBook: 'JHN', + defaultChapter: '1', + onVerseSelect: fn(), + onHighlightApply: fn(), + onHighlightRemove: fn(), + }, + render: function FakeHostStory(args: BibleReaderRootProps) { + // The host's own highlight store. A real native host owns this in its data + // layer (with the API, cache, and optimism behind it); the reader only ever + // sees the resulting array. Seeded with a range USFM on purpose — API data + // can hold ranges, and intents are per-verse, so every write normalizes the + // store through `toPerVerse` before matching. + const [highlights, setHighlights] = useState([ + { version_id: 111, passage_id: 'JHN.1.1', color: 'fffe00' }, + { version_id: 111, passage_id: 'JHN.1.3-5', color: '5dff79' }, + ]); + + const handleApply = (intent: BibleReaderHighlightIntent) => { + args.onHighlightApply?.(intent); + setHighlights((prev) => [ + // Replace any existing entry for these verses (last write wins). + ...prev.flatMap(toPerVerse).filter((h) => !matchesIntent(h, intent)), + ...intent.passageIds.map((passage_id) => ({ + version_id: intent.versionId, + passage_id, + color: intent.color, + })), + ]); + }; + + const handleRemove = (intent: BibleReaderHighlightIntent) => { + args.onHighlightRemove?.(intent); + setHighlights((prev) => prev.flatMap(toPerVerse).filter((h) => !matchesIntent(h, intent))); + }; + + return ( +
+ + + + +
+ ); + }, +}; + export const ChapterChangeLoadingOverlay: Story = { tags: ['integration'], args: { diff --git a/packages/ui/src/components/bible-reader.tsx b/packages/ui/src/components/bible-reader.tsx index 80b5d43b..a7c1ae5b 100644 --- a/packages/ui/src/components/bible-reader.tsx +++ b/packages/ui/src/components/bible-reader.tsx @@ -1,11 +1,12 @@ 'use client'; import i18n from '@/i18n'; +import { IS_PRODUCTION } from '@/lib/constants'; import { useDelayedLoading } from '@/lib/use-delayed-loading'; 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 type { BibleBook, Highlight } from '@youversion/platform-core'; import { DEFAULT_LICENSE_FREE_BIBLE_VERSION, getAdjacentChapter } from '@youversion/platform-core'; import { useBooks, @@ -38,8 +39,8 @@ import { PersonIcon } from './icons/person'; import { ProfileAvatar } from './profile-avatar'; import { Button } from './ui/button'; import { Popover, PopoverClose, PopoverContent, PopoverTrigger } from './ui/popover'; -import { useBibleReaderHighlights } from './use-bible-reader-highlights'; import { VerseActionPopover } from './verse-action-popover'; +import { useBibleReaderHighlights } from './use-bible-reader-highlights'; import { HighlightPermissionDialog } from './highlight-permission-dialog'; import { SignInDialog } from './sign-in-dialog'; import { BibleTextView, getCleanVerseText, type FootnoteData } from './verse'; @@ -71,6 +72,45 @@ type BibleReaderContextType = { onSignOutPress?: () => void; onCopy?: (data: BibleReaderShareData) => void | Promise; onShare?: (data: BibleReaderShareData) => void | Promise; + highlights?: Highlight[]; + isHighlightsControlled: boolean; + onVerseSelect?: (selection: BibleReaderVerseSelection) => void; + onHighlightApply?: (intent: BibleReaderHighlightIntent) => void; + onHighlightRemove?: (intent: BibleReaderHighlightIntent) => void; +}; + +/** + * Serializable payload describing the reader's current verse selection. + * Bridge-safe primitives only, so React Native / Expo DOM hosts can forward it + * across the native bridge. `passageIds` is deliberately redundant with + * `book`/`chapter`/`verses` (always per-verse, never ranges): native hosts feed + * the API without USFM string-building; analytics reads `verses` without USFM + * parsing. + */ +export type BibleReaderVerseSelection = { + /** Matches `Highlight.version_id`. */ + versionId: number; + /** USFM book code, e.g. `'JHN'`. */ + book: string; + /** Chapter id, e.g. `'3'`. */ + chapter: string; + /** Selected verse numbers, ascending and de-duplicated. `[]` on clear. */ + verses: number[]; + /** Per-verse USFM ids, e.g. `['JHN.3.16', 'JHN.3.17']`. Never ranges. */ + passageIds: string[]; +}; + +/** + * Serializable payload handed to `onHighlightApply` / `onHighlightRemove` in + * controlled mode. An intent is a request, not a fact: the reader paints + * nothing until the host round-trips an updated `highlights` prop. + */ +export type BibleReaderHighlightIntent = BibleReaderVerseSelection & { + /** + * 6-character lowercase hex, no `#`. For `onHighlightRemove`, the color + * being cleared. + */ + color: string; }; /** @@ -144,6 +184,49 @@ export type RootProps = { * default Web Share / clipboard flow — use for React Native / Expo hosts. */ onShare?: (data: BibleReaderShareData) => void | Promise; + /** + * Host-supplied highlights in the core API shape (`{ version_id, passage_id, + * color }`). Presence of this prop puts the reader's highlight slice in + * **controlled mode**: the reader renders highlights purely from this prop + * (filtered by the displayed version and chapter, range USFMs like + * `'JHN.3.16-18'` expanded per verse) and performs no highlight persistence + * of any kind — no network calls, no local store, no auth surface. Color + * taps emit {@link onHighlightApply} / {@link onHighlightRemove} instead of + * painting; nothing paints until the host round-trips an updated prop. + * + * The mode is latched at first mount: flipping between controlled and + * self-contained across renders is unsupported (dev warning). `[]` means + * "controlled, nothing highlighted"; leaving the prop off means + * self-contained. + * + * Entries whose color is outside the reader's five built-in swatches are + * ignored — the verse-action popover can only offer removal for its own + * palette, so an unmanageable color must not paint. + */ + highlights?: Highlight[]; + /** + * Called on every verse selection change with the selection payload — + * `verses: []` whenever a non-empty selection clears, whether by + * deselecting, after a highlight/copy/share action, on popover dismiss, or + * on navigation. Fires in both controlled and self-contained modes — it is + * an observation, not a request. + * + * A clear fired by navigation carries the *destination* book/chapter/version + * (the selection cleared because the reader moved there). Treat `verses: []` + * as "no selection anywhere" rather than keying off its location fields. + */ + onVerseSelect?: (selection: BibleReaderVerseSelection) => void; + /** + * Controlled mode only: called when the user taps a highlight color, with + * the intent payload. Ignored (never called) in self-contained mode. + */ + onHighlightApply?: (intent: BibleReaderHighlightIntent) => void; + /** + * Controlled mode only: called when the user taps a clear (X) circle, scoped + * to the selected verses currently showing that color. Ignored (never + * called) in self-contained mode. + */ + onHighlightRemove?: (intent: BibleReaderHighlightIntent) => void; children?: ReactNode; }; @@ -290,8 +373,30 @@ function Root({ onSignOutPress, onCopy, onShare, + highlights, + onVerseSelect, + onHighlightApply, + onHighlightRemove, children, }: RootProps) { + // Latched at first mount: a transient `undefined` on a controlled reader must + // render as "no highlights", never fall through to the self-contained path. + const isHighlightsControlledRef = useRef(highlights !== undefined); + const isHighlightsControlled = isHighlightsControlledRef.current; + const didWarnHighlightsModeFlipRef = useRef(false); + if ( + !IS_PRODUCTION && + (highlights !== undefined) !== isHighlightsControlled && + !didWarnHighlightsModeFlipRef.current + ) { + didWarnHighlightsModeFlipRef.current = true; + console.warn( + `BibleReader.Root: the \`highlights\` prop switched from ${ + isHighlightsControlled ? 'present to absent' : 'absent to present' + } after mount. The highlight mode (controlled vs self-contained) is latched at first mount and will not change. Pass \`highlights\` (use \`[]\` for "nothing highlighted") on every render for controlled mode, or never pass it for self-contained mode.`, + ); + } + const [book, setBook] = useControllableState({ prop: controlledBook, defaultProp: defaultBook, @@ -438,6 +543,11 @@ function Root({ onSignOutPress, onCopy, onShare, + highlights: isHighlightsControlled ? highlights : undefined, + isHighlightsControlled, + onVerseSelect, + onHighlightApply, + onHighlightRemove, }; return ( @@ -468,6 +578,11 @@ function Content() { onFootnotePress, onCopy, onShare, + highlights, + isHighlightsControlled, + onVerseSelect, + onHighlightApply, + onHighlightRemove, } = useBibleReaderContext(); const { version } = useVersion(versionId); @@ -509,11 +624,11 @@ function Content() { }, [book, chapter]); // ---- Verse selection + highlights ------------------------------------------ - // Selection is ephemeral (ADR-007 in YPE-642). Highlights are server-only - // account data (ADR-001 in YPE-1034): fetched and written through - // useBibleReaderHighlights, dark-launched behind the internal HIGHLIGHTS_LIVE - // flag. The reader DOM ref lets us anchor the popover and pull clean verse - // text for Copy / Share. + // Selection is ephemeral (ADR-007 in YPE-642). Highlights come from + // useBibleReaderHighlights: host-supplied in controlled mode (YPE-3705), or + // server-only account data in self-contained mode (YPE-1034 ADR-001), + // dark-launched behind HIGHLIGHTS_LIVE. The reader DOM ref anchors the + // popover and supplies clean verse text for Copy / Share. const readerRef = useRef(null); const [selectedVerses, setSelectedVerses] = useState([]); const [popoverOpen, setPopoverOpen] = useState(false); @@ -531,23 +646,43 @@ function Content() { signInDialogOpen, confirmSignInDialog, cancelSignInDialog, - } = useBibleReaderHighlights({ versionId, book, chapter }); + } = useBibleReaderHighlights({ + versionId, + book, + chapter, + controlled: isHighlightsControlled + ? { + highlights: highlights ?? [], + onApply: onHighlightApply, + onRemove: onHighlightRemove, + } + : undefined, + }); - // The color row / clear-highlight affordances only render when the highlights - // feature is live (dark-launch flag). Copy / Share are always available. - const highlightsEnabled = isHighlightsLive(); + // Color row is interactive when self-contained is live, or always in + // controlled mode (YPE-3705: controlled bypasses HIGHLIGHTS_LIVE). Copy / + // Share are always available. + const highlightsEnabled = isHighlightsControlled || isHighlightsLive(); // Copy shown to the sign-in dialog. Falls back to a neutral label when the // integrator hasn't set `YouVersionPlatformConfiguration.appName`. const signInAppName = YouVersionPlatformConfiguration.appName ?? t('signInAppNameFallback'); const signInPromptMessage = YouVersionPlatformConfiguration.signInPromptMessage; - // Navigating away (book/chapter/version) drops the selection — those verses no - // longer exist on screen (ADR-007). + // Read via ref in the navigation effect so an inline callback prop doesn't + // retrigger it every render. + const onVerseSelectRef = useRef(onVerseSelect); + onVerseSelectRef.current = onVerseSelect; + + // Navigating away (book/chapter/version) drops the selection — those verses + // no longer exist on screen (ADR-007). useEffect(() => { setSelectedVerses([]); setPopoverOpen(false); setAnchorElement(null); - lastSelectionRef.current = []; + if (lastSelectionRef.current.length > 0) { + lastSelectionRef.current = []; + onVerseSelectRef.current?.({ versionId, book, chapter, verses: [], passageIds: [] }); + } }, [book, chapter, versionId]); // Distinct colors present in the current selection → drives the X (remove) circles. @@ -565,13 +700,32 @@ function Content() { setPopoverOpen(false); setSelectedVerses([]); setAnchorElement(null); - lastSelectionRef.current = []; + // The deselect-tap path emits its own `[]` via handleVerseSelect. Read via + // ref: async callers (navigator.share().then) capture this closure, and a + // host-swapped callback must not go stale on that path. + if (lastSelectionRef.current.length > 0) { + lastSelectionRef.current = []; + onVerseSelectRef.current?.(buildVerseSelection([])); + } + } + + /** Builds the serializable selection payload (verses ascending, de-duped). */ + function buildVerseSelection(verses: number[]): BibleReaderVerseSelection { + const sorted = [...new Set(verses)].sort((a, b) => a - b); + return { + versionId, + book, + chapter, + verses: sorted, + passageIds: sorted.map((verse) => `${book}.${chapter}.${verse}`), + }; } function handleVerseSelect(verses: number[]) { const added = verses.find((verse) => !lastSelectionRef.current.includes(verse)); lastSelectionRef.current = verses; setSelectedVerses(verses); + onVerseSelect?.(buildVerseSelection(verses)); if (verses.length === 0) { setPopoverOpen(false); @@ -603,8 +757,10 @@ function Content() { removeHighlight(color, selectedVerses); // Multiple colors active → keep open so the user can remove others (AC 8a); - // last color removed → dismiss (AC 8). `highlightedVerses` still holds the - // pre-removal snapshot here — the optimistic overlay lands next render. + // last color removed → dismiss (AC 8). In self-contained mode + // `highlightedVerses` still holds the pre-removal snapshot here (the + // optimistic overlay lands next render); in controlled mode the host + // round-trip is what unpaints. const hasRemaining = selectedVerses.some((verse) => { const current = highlightedVerses[verse]; return current && current !== color; diff --git a/packages/ui/src/components/index.ts b/packages/ui/src/components/index.ts index 61c3a18b..c727e0a1 100644 --- a/packages/ui/src/components/index.ts +++ b/packages/ui/src/components/index.ts @@ -15,9 +15,11 @@ export { createBibleThemeSettingsContentHandlers, nextBibleReaderFontSizeDown, nextBibleReaderFontSizeUp, + type BibleReaderHighlightIntent, type BibleReaderRootProps, type BibleReaderShareData, type BibleReaderToolbarProps, + type BibleReaderVerseSelection, type BibleThemeSettingsContentProps, type BibleThemeSettingsSnapshot, type BibleThemeSettingsValues, diff --git a/packages/ui/src/components/use-bible-reader-highlights.test.tsx b/packages/ui/src/components/use-bible-reader-highlights.test.tsx index 644bef83..6e9b87e1 100644 --- a/packages/ui/src/components/use-bible-reader-highlights.test.tsx +++ b/packages/ui/src/components/use-bible-reader-highlights.test.tsx @@ -576,3 +576,96 @@ describe('useBibleReaderHighlights — scope changes', () => { consoleError.mockRestore(); }); }); + +describe('useBibleReaderHighlights — controlled mode (YPE-3705)', () => { + it('keeps the fetch disabled and projects from the host prop, even with the flag off', () => { + setHighlightsLive(false); + const mocked = mockUseHighlights({ + highlights: makeCollection([{ version_id: 111, passage_id: 'JHN.3.16', color: '00d6ff' }]), + }); + + const { result } = renderHook( + () => + useBibleReaderHighlights({ + ...defaultOptions, + controlled: { + highlights: [ + { version_id: 111, passage_id: 'JHN.3.16', color: 'fffe00' }, + { version_id: 111, passage_id: 'JHN.3.17-18', color: '5dff79' }, + { version_id: 111, passage_id: 'JHN.3.1', color: 'abcdef' }, // outside palette + ], + }, + }), + { wrapper: AuthWrapper }, + ); + + expect(vi.mocked(useHighlights)).toHaveBeenCalledWith( + { version_id: 111, passage_id: 'JHN.3' }, + { enabled: false }, + ); + expect(mocked.createHighlight).not.toHaveBeenCalled(); + expect(result.current.highlightedVerses).toEqual({ + 16: 'fffe00', + 17: '5dff79', + 18: '5dff79', + }); + }); + + it('emits apply/remove intents with no optimistic paint and no network', () => { + const onApply = vi.fn(); + const onRemove = vi.fn(); + const mocked = mockUseHighlights(); + + const { result, rerender } = renderHook( + ({ highlights }: { highlights: Highlight[] }) => + useBibleReaderHighlights({ + ...defaultOptions, + controlled: { highlights, onApply, onRemove }, + }), + { + wrapper: AuthWrapper, + initialProps: { + highlights: [{ version_id: 111, passage_id: 'JHN.3.16', color: 'fffe00' }], + }, + }, + ); + + expect(result.current.highlightedVerses).toEqual({ 16: 'fffe00' }); + + act(() => { + result.current.apply('5dff79', [17, 18]); + }); + // Pure projection: paint waits for the host round-trip. + expect(result.current.highlightedVerses).toEqual({ 16: 'fffe00' }); + expect(onApply).toHaveBeenCalledTimes(1); + expect(onApply).toHaveBeenCalledWith({ + versionId: 111, + book: 'JHN', + chapter: '3', + verses: [17, 18], + passageIds: ['JHN.3.17', 'JHN.3.18'], + color: '5dff79', + }); + expect(mocked.createHighlight).not.toHaveBeenCalled(); + + act(() => { + result.current.remove('fffe00', [16, 17]); + }); + expect(onRemove).toHaveBeenCalledTimes(1); + expect(onRemove).toHaveBeenCalledWith({ + versionId: 111, + book: 'JHN', + chapter: '3', + verses: [16], + passageIds: ['JHN.3.16'], + color: 'fffe00', + }); + expect(mocked.deleteHighlight).not.toHaveBeenCalled(); + // Still no optimistic un-paint. + expect(result.current.highlightedVerses).toEqual({ 16: 'fffe00' }); + + // Host round-trip. + rerender({ highlights: [] }); + expect(result.current.highlightedVerses).toEqual({}); + }); +}); diff --git a/packages/ui/src/components/use-bible-reader-highlights.ts b/packages/ui/src/components/use-bible-reader-highlights.ts index 18219fa7..03799d02 100644 --- a/packages/ui/src/components/use-bible-reader-highlights.ts +++ b/packages/ui/src/components/use-bible-reader-highlights.ts @@ -1,6 +1,8 @@ 'use client'; import { isHighlightsLive } from '@/lib/feature-flags'; +import { deriveHighlightedVerses } from '@/lib/highlight-projection'; +import type { Highlight } from '@youversion/platform-core'; import { bibleReaderHighlightsMachine, scopesEqual, @@ -16,11 +18,39 @@ import { } from '@youversion/platform-react-hooks'; import { useActorRef, useSelector } from '@xstate/react'; import { useContext, useEffect, useMemo, useRef } from 'react'; +import { HIGHLIGHT_COLORS } from './verse-action-popover'; + +/** + * Bridge-safe highlight intent emitted in controlled mode. Structurally + * identical to `BibleReaderHighlightIntent` on `BibleReader` — kept local so + * this adapter does not import from the compound component. + */ +export type BibleReaderHighlightIntentPayload = { + versionId: number; + book: string; + chapter: string; + verses: number[]; + passageIds: string[]; + color: string; +}; + +/** + * Host-owned highlight slice (YPE-3705). When present, the adapter is fully + * inert on the network: no fetch, no writes, pure projection from `highlights`, + * and `apply` / `remove` forward intents to the host. + */ +export type ControlledHighlightsInput = { + highlights: readonly Highlight[]; + onApply?: (intent: BibleReaderHighlightIntentPayload) => void; + onRemove?: (intent: BibleReaderHighlightIntentPayload) => void; +}; export type UseBibleReaderHighlightsOptions = { versionId: number; book: string; chapter: string; + /** Presence selects controlled mode (latched by the caller at first mount). */ + controlled?: ControlledHighlightsInput; }; export type UseBibleReaderHighlightsReturn = { @@ -35,6 +65,9 @@ export type UseBibleReaderHighlightsReturn = { * verses, or no auth provider). The caller uses the outcome to decide whether * to keep the verse selection: `'flow'` keeps it so cancelling the dialog * leaves the selection and popover intact. + * + * In controlled mode always emits an intent (no paint) and returns + * `'applied'` so the caller clears the selection as usual. */ apply: (color: string, verses: number[]) => 'applied' | 'flow' | 'noop'; /** Clears the given verses that are currently highlighted in `color`. */ @@ -84,30 +117,62 @@ function serverColorsEqual(a: ServerColors, b: ServerColors): boolean { return true; } +function buildHighlightIntent( + versionId: number, + book: string, + chapter: string, + color: string, + verses: number[], +): BibleReaderHighlightIntentPayload { + const sorted = [...new Set(verses)].filter((verse) => verse > 0).sort((a, b) => a - b); + return { + versionId, + book, + chapter, + verses: sorted, + passageIds: sorted.map((verse) => `${book}.${chapter}.${verse}`), + color: color.toLowerCase(), + }; +} + /** - * BibleReader's seam onto the highlights API (YPE-1034, self-contained mode). A - * THIN adapter over `bibleReaderHighlightsMachine` (PR-288): it reads auth + - * flag + fetched highlights from React and feeds them to the machine as events, - * exposes the machine's dialog states + write commands, and derives the rendered - * verse map. All flow/write logic (optimistic overlay, serialized writes, - * per-verse ownership, reconcile, auth flow, the vapor fix) lives in the - * machine; see that file for the invariants and the statechart. + * BibleReader's seam onto highlights (YPE-1034 self-contained + YPE-3705 controlled). * + * **Self-contained:** a THIN adapter over `bibleReaderHighlightsMachine` (PR-288): + * it reads auth + flag + fetched highlights from React and feeds them to the + * machine as events, exposes the machine's dialog states + write commands, and + * derives the rendered verse map. All flow/write logic (optimistic overlay, + * serialized writes, per-verse ownership, reconcile, auth flow, the vapor fix) + * lives in the machine; see that file for the invariants and the statechart. * Rendering and fetching are gated on `isHighlightsLive() && isAuthenticated`. * With no auth provider the reader keeps the PR-1 posture: no fetch, no writes, * and a color tap never enters the auth flow — copy/share still work. + * + * **Controlled:** when `controlled` is set, the fetch stays disabled, the + * machine is kept inert (`flagOn: false` — an enable guard, not a statechart + * region), the render map is a pure projection of the host's `highlights` (via + * `deriveHighlightedVerses`), and `apply` / `remove` emit intents with no + * optimistic paint. Controlled mode bypasses `isHighlightsLive` — the color + * row stays interactive so the public prop surface can ship while + * self-contained remains dark (YPE-3705 ADR). */ export function useBibleReaderHighlights({ versionId, book, chapter, + controlled, }: UseBibleReaderHighlightsOptions): UseBibleReaderHighlightsReturn { + const isControlled = controlled !== undefined; + // Read the auth context directly instead of `useYVAuth`, which throws when no - // auth provider is mounted. + // auth provider is mounted. No provider and signed out are the same state + // here for the self-contained path: no fetch, no writes, nothing rendered. const authContext = useContext(YouVersionAuthContext); const hasAuthProvider = authContext !== null; const isAuthenticated = Boolean(authContext?.userInfo); - const flagOn = isHighlightsLive(); + // Controlled mode keeps the machine disabled (provably inert) and never hits + // the network — the dark-launch flag only gates the self-contained server path. + const flagOn = !isControlled && isHighlightsLive(); const live = flagOn && isAuthenticated; const { @@ -124,6 +189,11 @@ export function useBibleReaderHighlights({ { enabled: live }, ); + // Stable refs so controlled apply/remove close over the latest callbacks + // without re-creating the functions every render. + const controlledRef = useRef(controlled); + controlledRef.current = controlled; + // A stable ref bag of the live SDK service closures. Passed once to the machine // via `input`; the machine reads `.current` at call time so it always sees the // latest closures without re-spawning. The ref is initialized with the first @@ -188,6 +258,19 @@ export function useBibleReaderHighlights({ const overlay = useSelector(actorRef, (state) => state.context.overlay); const machineScope = useSelector(actorRef, (state) => state.context.scope); const highlightedVerses = useMemo(() => { + // Controlled: pure projection from the host prop — no overlay, no fetch. + // Colors outside the popover swatches are ignored so un-removable paint + // cannot appear (YPE-3705). + if (isControlled) { + return deriveHighlightedVerses( + controlled?.highlights ?? [], + versionId, + book, + chapter, + HIGHLIGHT_COLORS, + ); + } + // Gate on `live`: sign-out or flag-off must render nothing this very render, // including optimistic overlay entries still in the machine. if (!live) return {}; @@ -198,7 +281,21 @@ export function useBibleReaderHighlights({ // collide across chapters. if (!scopesEqual(machineScope, scope)) return { ...serverColors }; return selectHighlightedVerses(serverColors, overlay); - }, [live, serverColors, overlay, machineScope, scope]); + }, [ + isControlled, + controlled?.highlights, + live, + serverColors, + overlay, + machineScope, + scope, + versionId, + book, + chapter, + ]); + + const highlightedVersesRef = useRef(highlightedVerses); + highlightedVersesRef.current = highlightedVerses; // ── Dialog state + commands ───────────────────────────────────────────────── const signInDialogOpen = useSelector(actorRef, (state) => @@ -222,10 +319,39 @@ export function useBibleReaderHighlights({ >( () => ({ apply: (color, verses) => { + const controlledInput = controlledRef.current; + if (controlledInput !== undefined) { + if (verses.length === 0) return 'noop'; + // Emit intent only — no optimistic paint (YPE-3705 ADR). + controlledInput.onApply?.( + buildHighlightIntent(scope.versionId, scope.book, scope.chapter, color, verses), + ); + return 'applied'; + } actorRef.send({ type: 'TAP_COLOR', color, verses }); return actorRef.getSnapshot().context.lastTapOutcome; }, remove: (color, verses) => { + const controlledInput = controlledRef.current; + if (controlledInput !== undefined) { + if (verses.length === 0) return; + // Only clear verses currently rendered in this color — that's the + // popover's per-color X semantics. + const normalizedColor = color.toLowerCase(); + const rendered = highlightedVersesRef.current; + const targetVerses = verses.filter((verse) => rendered[verse] === normalizedColor); + if (targetVerses.length === 0) return; + controlledInput.onRemove?.( + buildHighlightIntent( + scope.versionId, + scope.book, + scope.chapter, + normalizedColor, + targetVerses, + ), + ); + return; + } actorRef.send({ type: 'REMOVE', color, verses }); }, onPermissionDialogOpenChange: (open) => { @@ -238,7 +364,7 @@ export function useBibleReaderHighlights({ confirmSignInDialog: () => actorRef.send({ type: 'CONFIRM_SIGN_IN' }), cancelSignInDialog: () => actorRef.send({ type: 'DECLINE_SIGN_IN' }), }), - [actorRef], + [actorRef, scope], ); return { diff --git a/packages/ui/src/index.ts b/packages/ui/src/index.ts index 9304ceac..7eaf5116 100644 --- a/packages/ui/src/index.ts +++ b/packages/ui/src/index.ts @@ -12,6 +12,9 @@ export { // Authentication type ApiConfig, type AuthenticationState, + + // Highlights (the shape of BibleReader.Root's controlled `highlights` prop) + type Highlight, } from '@youversion/platform-core'; export { diff --git a/packages/ui/src/lib/constants.ts b/packages/ui/src/lib/constants.ts new file mode 100644 index 00000000..9eb34b90 --- /dev/null +++ b/packages/ui/src/lib/constants.ts @@ -0,0 +1,10 @@ +// `process` is not typed in this browser-targeting build. Consumers' bundlers +// statically replace `process.env.NODE_ENV` at build time, so this whole +// expression folds to a constant; the `typeof` guard treats unbundled browser +// environments (no `process` global) as production so dev-only behavior stays +// off there. +declare const process: { env: { NODE_ENV?: string } }; + +/** True in production builds. Dev-only warnings gate on `!IS_PRODUCTION`. */ +export const IS_PRODUCTION = + typeof process === 'undefined' || process.env.NODE_ENV === 'production'; diff --git a/packages/ui/src/lib/highlight-projection.test.ts b/packages/ui/src/lib/highlight-projection.test.ts new file mode 100644 index 00000000..1608ac4f --- /dev/null +++ b/packages/ui/src/lib/highlight-projection.test.ts @@ -0,0 +1,141 @@ +import { describe, it, expect } from 'vitest'; +import type { Highlight } from '@youversion/platform-core'; +import { deriveHighlightedVerses, expandPassageId } from './highlight-projection'; + +describe('expandPassageId', () => { + it('expands a single-verse USFM', () => { + expect(expandPassageId('JHN.3.16')).toEqual({ book: 'JHN', chapter: '3', verses: [16] }); + }); + + it('expands a verse-range USFM into every verse in the range', () => { + expect(expandPassageId('JHN.3.16-18')).toEqual({ + book: 'JHN', + chapter: '3', + verses: [16, 17, 18], + }); + }); + + it('expands a single-verse range (N-N) to one verse', () => { + expect(expandPassageId('PSA.23.1-1')).toEqual({ book: 'PSA', chapter: '23', verses: [1] }); + }); + + it('handles numbered book codes', () => { + expect(expandPassageId('1CO.13.4-7')).toEqual({ + book: '1CO', + chapter: '13', + verses: [4, 5, 6, 7], + }); + }); + + it('returns null for a chapter-scope USFM (not a highlightable unit)', () => { + expect(expandPassageId('JHN.3')).toBeNull(); + }); + + it('returns null for malformed input', () => { + expect(expandPassageId('')).toBeNull(); + expect(expandPassageId('JHN')).toBeNull(); + expect(expandPassageId('JHN.3.')).toBeNull(); + expect(expandPassageId('JHN..16')).toBeNull(); + expect(expandPassageId('.3.16')).toBeNull(); + expect(expandPassageId('JHN.3.abc')).toBeNull(); + expect(expandPassageId('JHN.3.16-')).toBeNull(); + expect(expandPassageId('JHN.3.-18')).toBeNull(); + expect(expandPassageId('JHN.3.16-18-20')).toBeNull(); + expect(expandPassageId('JHN.3.16.18')).toBeNull(); + }); + + it('returns null for a reversed range', () => { + expect(expandPassageId('JHN.3.18-16')).toBeNull(); + }); + + it('returns null for verse 0', () => { + expect(expandPassageId('JHN.3.0')).toBeNull(); + expect(expandPassageId('JHN.3.0-2')).toBeNull(); + }); + + it('returns null for an implausibly large range (defensive cap)', () => { + expect(expandPassageId('JHN.3.1-100000')).toBeNull(); + }); +}); + +describe('deriveHighlightedVerses', () => { + const highlight = (version_id: number, passage_id: string, color: string): Highlight => ({ + version_id, + passage_id, + color, + }); + + it('projects matching per-verse and range entries into a verse->color map', () => { + const map = deriveHighlightedVerses( + [highlight(111, 'JHN.3.16', 'fffe00'), highlight(111, 'JHN.3.18-19', '5dff79')], + 111, + 'JHN', + '3', + ); + expect(map).toEqual({ 16: 'fffe00', 18: '5dff79', 19: '5dff79' }); + }); + + it('ignores colors outside allowedColors, matching case-insensitively', () => { + const map = deriveHighlightedVerses( + [highlight(111, 'JHN.3.16', 'abcdef'), highlight(111, 'JHN.3.17', 'FFFE00')], + 111, + 'JHN', + '3', + ['fffe00'], + ); + expect(map).toEqual({ 17: 'fffe00' }); + }); + + it('accepts any color when allowedColors is omitted', () => { + const map = deriveHighlightedVerses([highlight(111, 'JHN.3.16', 'abcdef')], 111, 'JHN', '3'); + expect(map).toEqual({ 16: 'abcdef' }); + }); + + it('ignores entries for other versions', () => { + const map = deriveHighlightedVerses([highlight(222, 'JHN.3.16', 'fffe00')], 111, 'JHN', '3'); + expect(map).toEqual({}); + }); + + it('ignores entries for other books or chapters', () => { + const map = deriveHighlightedVerses( + [ + highlight(111, 'GEN.3.16', 'fffe00'), + highlight(111, 'JHN.4.16', 'fffe00'), + highlight(111, 'JHN.3.16', '00d6ff'), + ], + 111, + 'JHN', + '3', + ); + expect(map).toEqual({ 16: '00d6ff' }); + }); + + it('ignores unexpandable passage ids without dropping valid ones', () => { + const map = deriveHighlightedVerses( + [highlight(111, 'JHN.3', 'fffe00'), highlight(111, 'JHN.3.2', '5dff79')], + 111, + 'JHN', + '3', + ); + expect(map).toEqual({ 2: '5dff79' }); + }); + + it('lets later entries win on verse collisions', () => { + const map = deriveHighlightedVerses( + [highlight(111, 'JHN.3.16-17', 'fffe00'), highlight(111, 'JHN.3.17', 'ff95ef')], + 111, + 'JHN', + '3', + ); + expect(map).toEqual({ 16: 'fffe00', 17: 'ff95ef' }); + }); + + it('normalizes colors to lowercase (uppercase accepted at the boundary)', () => { + const map = deriveHighlightedVerses([highlight(111, 'JHN.3.16', 'FFFE00')], 111, 'JHN', '3'); + expect(map).toEqual({ 16: 'fffe00' }); + }); + + it('returns an empty map for an empty highlights array', () => { + expect(deriveHighlightedVerses([], 111, 'JHN', '3')).toEqual({}); + }); +}); diff --git a/packages/ui/src/lib/highlight-projection.ts b/packages/ui/src/lib/highlight-projection.ts new file mode 100644 index 00000000..640a93bf --- /dev/null +++ b/packages/ui/src/lib/highlight-projection.ts @@ -0,0 +1,78 @@ +import type { Highlight } from '@youversion/platform-core'; + +/** + * Defensive cap on how many verses a single range USFM may expand to. The + * longest chapter in any Bible (Psalm 119) has 176 verses, so any range longer + * than this is malformed data and is rejected rather than expanded. + */ +const MAX_RANGE_LENGTH = 250; + +/** A verse USFM (`JHN.3.16` / `JHN.3.16-18`) split into its parts. */ +export type ExpandedPassageId = { + book: string; + chapter: string; + /** Every verse number covered, ascending (a range expands to each verse). */ + verses: number[]; +}; + +/** + * Expands a verse or verse-range USFM passage id (`JHN.3.16`, `JHN.3.16-18`) + * into its book, chapter, and per-verse numbers. + * + * Returns `null` for anything that is not a highlightable verse unit: a + * chapter-scope USFM (`JHN.3`), malformed input, a reversed range, verse 0, or + * an implausibly large range. + */ +export function expandPassageId(passageId: string): ExpandedPassageId | null { + const parts = passageId.split('.'); + if (parts.length !== 3) return null; + const [book, chapter, versePart] = parts; + if (!book || !chapter || !versePart) return null; + + const match = /^(\d+)(?:-(\d+))?$/.exec(versePart); + if (!match) return null; + + const start = parseInt(match[1]!, 10); + const end = match[2] !== undefined ? parseInt(match[2], 10) : start; + if (start < 1 || end < start || end - start + 1 > MAX_RANGE_LENGTH) return null; + + const verses: number[] = []; + for (let verse = start; verse <= end; verse++) { + verses.push(verse); + } + return { book, chapter, verses }; +} + +/** + * Projects a host-supplied `Highlight[]` (core API shape) onto the displayed + * chapter as the reader's internal render map (verse number -> hex color). + * + * Entries for other versions, books, or chapters are ignored — every entry + * carries its full identity, so stale host data can never mispaint. Range + * passage ids are expanded per verse. Colors are normalized to lowercase (the + * API accepts uppercase at the boundary). Later entries win on collisions. + * + * When `allowedColors` is given, entries with any other color are ignored too: + * the verse-action popover can only offer removal for its own swatches, so a + * color the reader can't manage must not paint (it would be un-removable). + */ +export function deriveHighlightedVerses( + highlights: readonly Highlight[], + versionId: number, + book: string, + chapter: string, + allowedColors?: readonly string[], +): Record { + const map: Record = {}; + for (const { version_id, passage_id, color } of highlights) { + if (version_id !== versionId) continue; + const normalizedColor = color.toLowerCase(); + if (allowedColors && !allowedColors.includes(normalizedColor)) continue; + const expanded = expandPassageId(passage_id); + if (!expanded || expanded.book !== book || expanded.chapter !== chapter) continue; + for (const verse of expanded.verses) { + map[verse] = normalizedColor; + } + } + return map; +}