Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions .changeset/recent-highlight-colors.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
'@youversion/platform-react-ui': patch
---

BibleReader's verse action popover now shows the user's recent highlight colors and marks active swatches with a checkmark (YPE-1034 PR3, still behind the internal `HIGHLIGHTS_LIVE` flag).

- **Recent colors**: when highlights are live (flag on + auth provider + signed in), the color row renders the server's recent-colors list (recently used first, then defaults) via `useBibleReaderHighlights` → `useHighlights.getRecentColors`. The list is deduped first-occurrence-wins, normalized (lowercase, leading `#` stripped, non-`/^[0-9a-f]{6}$/` entries dropped), never reordered (server order is the truth), and the row scrolls horizontally if it overflows. It falls back to the hardcoded `HIGHLIGHT_COLORS` palette whenever recents are unavailable — flag off, no auth provider, signed out, fetch pending, or fetch failed. The fetch runs once per live edge (not per tap or per navigation).
- **Checkmark swap**: active/remove swatches now render a 24px checkmark (`icons/check`) instead of the X, matching iOS (platform-sdk-swift #179). Same theme-invariant `#121212` fill and identical behavior — tapping still removes the highlight.
- **Type**: `HighlightColor` (exported from `verse-action-popover`) is widened from the five-literal palette union to `string`, since recents can be any hex. This is a loosening (every prior value still assignable), so it is non-breaking for consumers. `HIGHLIGHT_COLORS` remains exported as the default palette.
12 changes: 12 additions & 0 deletions docs/adr/YPE-642-verse-action-popover.md
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,18 @@ Selection is always enabled in BibleReader (no opt-out prop for now; YAGNI).
- Tunable; swap to a ring/bg later if Figma says otherwise.

## As-built notes (deviations from the design above)
- **ADR-005 active-swatch icon (YPE-1034 PR3):** the 24px **X** on active/remove
swatches was replaced with a 24px **checkmark** (`icons/check`), matching iOS
(platform-sdk-swift #179). Same Text/Everdark (`--yv-gray-50` = `#121212`,
theme-invariant) fill and identical behavior — tapping still removes the
highlight; the swatch's `Clear highlight` aria-label is unchanged.
- **Recent colors (YPE-1034 PR3):** when highlights are live (flag on + auth
provider + signed in) the color row renders the server's recent-colors list
(recently used first, then defaults) in server order, deduped, and
horizontally scrollable if it overflows. It falls back to the hardcoded
`HIGHLIGHT_COLORS` palette whenever recents are unavailable (flag off, no auth
provider, signed out, fetch pending or failed). `HighlightColor` was widened
from the five-literal union to `string` since recents can be any hex.
- **ADR-004 revised:** selection + highlights live in `Content`, **not** Root
context. Copy/Share/anchor all need the rendered verse DOM (which lives in
Content), so Root ownership would fragment the feature. BibleTextView stays
Expand Down
2 changes: 2 additions & 0 deletions packages/ui/src/components/bible-reader.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -518,6 +518,7 @@ function Content() {

const {
highlightedVerses,
recentColors,
apply: applyHighlight,
remove: removeHighlight,
permissionDialogOpen,
Expand Down Expand Up @@ -744,6 +745,7 @@ function Content() {
activeHighlights={activeHighlights}
selectedVerses={selectedVerses}
highlightedVerses={highlightedVerses}
recentColors={recentColors}
anchorElement={anchorElement}
scrollRoot={scrollContainerRef.current}
onHighlight={handleHighlight}
Expand Down
19 changes: 19 additions & 0 deletions packages/ui/src/components/icons/check.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import type { ComponentProps, ReactElement } from 'react';

export function CheckIcon(props: ComponentProps<'svg'>): ReactElement {
return (
<svg
width="24"
height="24"
viewBox="0 0 24 24"
fill="none"
xmlns="http://www.w3.org/2000/svg"
{...props}
>
<path
d="M19.6627 6.13882C20.0663 6.51547 20.0888 7.14784 19.7124 7.55165L19.7007 7.56339C19.693 7.57162 19.6815 7.58408 19.6661 7.60068C19.6349 7.63447 19.5879 7.68562 19.5266 7.75259C19.404 7.88675 19.2237 8.08624 18.9942 8.343C18.5352 8.85666 17.8803 9.60146 17.1022 10.5223C15.5446 12.3656 13.497 14.9071 11.5296 17.708C11.3594 17.9502 11.1725 18.22 10.9889 18.4172C10.7971 18.6231 10.45 18.9278 9.92408 18.942C9.39431 18.9563 9.02907 18.666 8.8282 18.4718C8.63358 18.2836 8.43144 18.0219 8.24539 17.7854C7.25297 16.5234 6.27013 15.4686 5.53573 14.7291C5.16905 14.3598 4.8656 14.0704 4.6553 13.8749C4.5502 13.7771 4.46885 13.7022 4.41431 13.6532C4.38726 13.6289 4.36672 13.611 4.35354 13.5993C4.34694 13.5935 4.34194 13.5888 4.33904 13.5862C4.33767 13.585 4.33607 13.5846 4.33558 13.5842C3.91934 13.2213 3.8761 12.589 4.23891 12.1727C4.60179 11.7565 5.23338 11.7139 5.64967 12.0767L5.65036 12.076L5.65796 12.0836C5.66273 12.0878 5.67017 12.094 5.67867 12.1016C5.69565 12.1166 5.72014 12.1379 5.75118 12.1658C5.81368 12.222 5.90392 12.3044 6.01772 12.4102C6.24522 12.6218 6.56756 12.9297 6.95478 13.3197C7.72857 14.0989 8.76556 15.2122 9.81704 16.5493C9.83185 16.5681 9.84664 16.5862 9.86055 16.6039C9.87102 16.589 9.88266 16.574 9.89369 16.5583C11.9059 13.6936 13.9928 11.1031 15.5747 9.23103C16.3663 8.29427 17.0331 7.53573 17.5027 7.01027C17.7375 6.7475 17.9228 6.54269 18.0503 6.40329C18.1138 6.33381 18.1632 6.28075 18.1967 6.24447C18.2133 6.22647 18.2266 6.2125 18.2353 6.20304C18.2397 6.1983 18.2434 6.19382 18.2457 6.1913L18.2492 6.18784C18.6259 5.784 19.2588 5.76208 19.6627 6.13882Z"
fill="currentColor"
/>
</svg>
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -105,3 +105,135 @@ describe('useBibleReaderHighlights — real useHighlights/useApiData', () => {
expect(getHighlights).toHaveBeenCalledTimes(1);
});
});

describe('useBibleReaderHighlights — recent colors', () => {
function stubHighlights() {
vi.spyOn(HighlightsClient.prototype, 'getHighlights').mockResolvedValue({
data: [],
next_page_token: null,
});
}

it('fetches recent colors when auth resolves after mount (signed-out → signed-in flip)', async () => {
stubHighlights();
const getRecentColors = vi
.spyOn(HighlightsClient.prototype, 'getRecentColors')
.mockResolvedValue(['00d6ff', 'ff0000', '5dff79']);

// Mount signed out — mirrors YouVersionAuthProvider, which initializes
// userInfo to null and resolves the session asynchronously. The production
// edge is always this flip, never a synchronously-signed-in mount.
const { result, rerender } = renderHook(() => useBibleReaderHighlights(defaultOptions), {
wrapper: Providers,
});

expect(getRecentColors).not.toHaveBeenCalled();
expect(result.current.recentColors).toBeNull();

signedIn = true;
rerender();

await waitFor(() => {
expect(result.current.recentColors).toEqual(['00d6ff', 'ff0000', '5dff79']);
});
expect(getRecentColors).toHaveBeenCalledTimes(1);
});

it('reports null until the fetch resolves, then swaps in the server list', async () => {
stubHighlights();
let resolveColors!: (colors: string[]) => void;
vi.spyOn(HighlightsClient.prototype, 'getRecentColors').mockReturnValue(
new Promise<string[]>((resolve) => {
resolveColors = resolve;
}),
);

signedIn = true;
const { result } = renderHook(() => useBibleReaderHighlights(defaultOptions), {
wrapper: Providers,
});

// Pending fetch → null, so the popover renders its default palette meanwhile.
expect(result.current.recentColors).toBeNull();

await act(async () => {
resolveColors(['abcdef']);
await Promise.resolve();
});
expect(result.current.recentColors).toEqual(['abcdef']);
});

it('does not fetch and stays null when signed out', async () => {
stubHighlights();
const getRecentColors = vi
.spyOn(HighlightsClient.prototype, 'getRecentColors')
.mockResolvedValue(['00d6ff']);

signedIn = false;
const { result } = renderHook(() => useBibleReaderHighlights(defaultOptions), {
wrapper: Providers,
});

await act(async () => {
await Promise.resolve();
});
expect(result.current.recentColors).toBeNull();
expect(getRecentColors).not.toHaveBeenCalled();
});

it('does not fetch and stays null when the flag is off', async () => {
setHighlightsLive(false);
stubHighlights();
const getRecentColors = vi
.spyOn(HighlightsClient.prototype, 'getRecentColors')
.mockResolvedValue(['00d6ff']);

signedIn = true;
const { result } = renderHook(() => useBibleReaderHighlights(defaultOptions), {
wrapper: Providers,
});

await act(async () => {
await Promise.resolve();
});
expect(result.current.recentColors).toBeNull();
expect(getRecentColors).not.toHaveBeenCalled();
});

it('falls back to null when the fetch fails (e.g. 403 without permission)', async () => {
stubHighlights();
vi.spyOn(HighlightsClient.prototype, 'getRecentColors').mockRejectedValue(
Object.assign(new Error('HTTP 403'), { status: 403 }),
);

signedIn = true;
const { result } = renderHook(() => useBibleReaderHighlights(defaultOptions), {
wrapper: Providers,
});

// Give the rejected promise a tick to settle.
await act(async () => {
await Promise.resolve();
await Promise.resolve();
});
expect(result.current.recentColors).toBeNull();
});

it('drops recent colors on sign-out', async () => {
stubHighlights();
vi.spyOn(HighlightsClient.prototype, 'getRecentColors').mockResolvedValue(['00d6ff']);

signedIn = true;
const { result, rerender } = renderHook(() => useBibleReaderHighlights(defaultOptions), {
wrapper: Providers,
});

await waitFor(() => {
expect(result.current.recentColors).toEqual(['00d6ff']);
});

signedIn = false;
rerender();
expect(result.current.recentColors).toBeNull();
});
});
46 changes: 45 additions & 1 deletion packages/ui/src/components/use-bible-reader-highlights.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,13 @@ export type UseBibleReaderHighlightsOptions = {
export type UseBibleReaderHighlightsReturn = {
/** Verse number → hex color (lowercase, no `#`) for the current chapter. */
highlightedVerses: Record<number, string>;
/**
* The user's recent highlight colors (recently used first, then defaults) as
* hex strings, or `null` when unavailable — highlights off, no session, or the
* fetch hasn't resolved / failed. The color row falls back to its default
* palette while this is `null`. Server-ordered; not normalized here.
*/
recentColors: string[] | null;
/**
* Highlights the given verses in `color`. Bridge-safe: primitives only.
* When the user has a session and the highlights permission this writes
Expand Down Expand Up @@ -139,14 +146,50 @@ export function useBibleReaderHighlights({
} = useHighlightAuthActions();

const chapterUsfm = `${book}.${chapter}`;
const { highlights, createHighlight, deleteHighlight } = useHighlights(
const { highlights, createHighlight, deleteHighlight, getRecentColors } = useHighlights(
{ version_id: versionId, passage_id: chapterUsfm },
{ enabled: live },
);

const [overlay, setOverlay] = useState<HighlightOverlay>({});
const [permissionDialogOpen, setPermissionDialogOpen] = useState(false);

// The user's recent highlight colors, fetched once the feature is live. `null`
// means "use the default palette": the feature is inert, or the fetch is still
// pending or failed. Recent colors are user-global (not per chapter/version),
// so they're fetched on the `live` edge only — never per navigation or per tap.
//
// DEFERRED (accepted for dark launch): an account switch WITHOUT an
// intervening sign-out (userInfo A → userInfo B while `live` stays true)
// never re-runs this effect, so user A's recents stay rendered for user B.
// Hosts normally sign out between users, which drops recents via the `!live`
// branch. Follow-up: key the fetch on the user identity as well as `live`.
const [recentColors, setRecentColors] = useState<string[] | null>(null);
const getRecentColorsRef = useRef(getRecentColors);
getRecentColorsRef.current = getRecentColors;
useEffect(() => {
if (!live) {
// Sign-out / flag-off must drop recents synchronously-ish so the row falls
// back to the palette rather than showing another session's colors.
setRecentColors(null);
return;
}
let cancelled = false;
getRecentColorsRef
.current()
.then((colors) => {
if (!cancelled) setRecentColors(colors);
})
.catch(() => {
// A failed/forbidden fetch is non-fatal: keep `null` so the row renders
// the default palette. (Signed-in-without-permission 403s land here.)
if (!cancelled) setRecentColors(null);
});
return () => {
cancelled = true;
};
}, [live]);

// Drop the optimistic overlay synchronously (during render) the moment the
// scope changes, so an in-flight overlay never paints over another
// chapter's or version's verses — their verse numbers collide.
Expand Down Expand Up @@ -563,6 +606,7 @@ export function useBibleReaderHighlights({

return {
highlightedVerses,
recentColors,
apply,
remove,
permissionDialogOpen,
Expand Down
Loading
Loading