feat(ui): refreshHighlights, onHighlightError, sign-out guard on highlights by cameronapak · Pull Request #132 · youversion/platform-sdk-reactnative-expo

cameronapak · 2026-08-12T22:49:16Z

Stacked on #100 (highlights).

This PR adds three host APIs on the highlights stack.

What changed

BibleReaderHandle.refreshHighlights()
The host calls this through a React 19 ref. The call runs highlights.refresh.
onHighlightError
This optional callback is on BibleReader. The payload type is HighlightWriteError.
The reader calls it only for queued, or for error with reason transient.
The reader does not call it for ok, noop, invalid, auth, or not-signed-in.
useSignOutGuard
This hook is the shared sign-out confirm. BibleReader and YouVersionAuthButton both use it.

Host
ref.refreshHighlights -> highlights.refresh
apply or remove highlight
queued or transient error -> onHighlightError
else -> no callback
sign-out press
Alert always shows
confirm -> signOut, then clearAuthState
cancel -> stay signed in

Sign-out behavior

This path is not the same as PR #104.

If the user confirms, the app calls signOut. Then core clearAuthState clears the queue and the cache.
If the user cancels, the app does not sign out.
The app does not call discardPendingHighlights or clearHighlightQueue.
The Alert always shows (normal copy or pending copy). The app does not sign out in silence when the queue is empty.

Test plan

typecheck
tests
lint
format check
use-sign-out-guard pending then alert then confirm then signOut. No discard. Cancel does not sign out. Alert always shows.
YouVersionAuthButton guarded sign-out when items are queued
BibleReader refreshHighlights runs highlights.refresh. onHighlightError fires for queued and transient only.

Greptile Summary

The PR adds consumer APIs for refreshing and reporting highlight writes, and centralizes guarded sign-out behavior across the reader and authentication button.

Exposes BibleReaderHandle.refreshHighlights() through React 19 refs.
Reports queued and transient highlight outcomes through onHighlightError while containing callback failures.
Adds and exports useSignOutGuard, including web and unauthenticated behavior.
Updates tests, exports, documentation, and release notes.

Confidence Score: 5/5 ·

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename	Overview
packages/ui/src/lib/report-highlight-write-error.ts	Adds the filtered highlight-write callback adapter and contains both synchronous callback throws and asynchronous rejections.
packages/ui/src/native/use-sign-out-guard.ts	Centralizes localized sign-out confirmation, preserves the web fallback, and contains rejected sign-out promises.
packages/ui/src/native/bible-reader.tsx	Exposes highlight refresh through a ref, reports selected write outcomes, and routes sign-out through the shared guard.
packages/ui/src/native/youversion-auth-button.tsx	Routes explicit and automatic sign-out actions through the shared guard while preserving sign-in behavior.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  Host[Host application] -->|ref.refreshHighlights| Reader[BibleReader]
  Reader -->|refresh| Highlights[Highlights state]
  Reader -->|apply or remove| Write[Highlight write]
  Write -->|queued or transient error| Callback[onHighlightError]
  Reader --> Guard[useSignOutGuard]
  AuthButton[YouVersionAuthButton] --> Guard
  Guard -->|web or signed out| SignOut[signOut]
  Guard -->|authenticated native| Alert[Localized confirmation alert]
  Alert -->|confirm| SignOut
  Alert -->|cancel| StaySignedIn[Remain signed in]

_{Reviews (7): Last reviewed commit: "fix(ui): explicit sign-out still clears ..." | Re-trigger Greptile}

…lights Port three consumer-facing UI deltas onto the highlights branch: - BibleReaderHandle.refreshHighlights via React 19 ref + useImperativeHandle - onHighlightError for queued and transient write outcomes only - useSignOutGuard shared by BibleReader toolbar and YouVersionAuthButton Sign-out guard preserves highlights-tip behavior: always Alert (normal or pending copy), confirm calls signOut() only — no discard-first path. Tests cover the hook, auth button parity, ref refresh, and error filtering. Co-authored-by: Cameron Pak <cameronandrewpak@gmail.com>

cameronapak

Spec

Mostly pass vs the #104→#100 UI delta locks (refreshHighlights, onHighlightError filter, useSignOutGuard; warn-then-signOut only; always Alert; no discard; UI-only).

One Spec gap: YouVersionAuthButton always awaits guardedSignOut, while the reader still bypasses the Alert on web (Platform.OS === 'web' ? signOut : guardedSignOut). On RN-web, Alert.alert is a no-op, so the auth button can trap sign-out. Inline.

Standards

Mostly pass. Pure filter helper in lib/; React 19 ref without forwardRef; solid consumer-api + guard tests; no core edits.

Soft: bible-reader-consumer-api.test.tsx has two top-level describes (AGENTS Testing Musts prefer one module describe). Public re-export of useSignOutGuard is slightly beyond “wire into reader + button.” No changeset yet — fine while stacked/draft on #100, add before ship to main.

Code Reviewer bot, sent on behalf of Cam. Own-PR review → COMMENT (GitHub blocks self-approve). Draft PR reviewed at Implementor request.

cameronapak · 2026-08-12T22:53:27Z

          onSignInPress={signIn}
          // `Alert.alert` is a no-op on react-native-web, so web signs out unprompted.
-          onSignOutPress={Platform.OS === 'web' || !signOut ? signOut : handleSignOutPress}
+          onSignOutPress={Platform.OS === 'web' || !signOut ? signOut : guardedSignOut}


praise: Web bypass kept here so sign-out still works when Alert.alert is a no-op. The auth button needs the same treatment (see sibling issue).

Code Reviewer bot, sent on behalf of Cam.

cameronapak · 2026-08-12T22:53:27Z

+  if (onHighlightError === undefined) {
+    return
+  }
+  if (outcome.status === 'queued') {


praise: Filter matches the lock — queued and error+transient only; ok/noop/invalid/auth/not-signed-in stay silent. Tests pin it.

Code Reviewer bot, sent on behalf of Cam.

cameronapak · 2026-08-12T22:53:28Z

+          text: hasUnsentHighlights ? t('signOutPendingHighlightsConfirm') : t('signOut'),
+          style: 'destructive',
+          onPress: () => {
+            void signOut()


praise: Confirm → signOut() only (no discard). Always Alert with pending vs normal copy. Matches the highlights-tip + Cam locks.

Code Reviewer bot, sent on behalf of Cam.

cameronapak

Standards follow-up

Additional Standards after first COMMENT. Spec unchanged (web auth-button bypass).

Ship bar

Missing changeset before main (ok while draft on #100).
Collapse bible-reader-consumer-api.test.tsx to one module describe.
Move reportHighlightWriteError filter table to lib/tests (lightest falsifying).

Judgement

HighlightWriteError naming (queued is not an error) if #104 name not locked.
Root-export useSignOutGuard only if partners reuse it.
Prefer userEvent over fireEvent for new tests where practical.

Praise still stands: lib helper, guard extraction, React 19 ref, focused guard tests.

Code Reviewer bot, sent on behalf of Cam.

Co-authored-by: Cameron Pak <cameronandrewpak@gmail.com>

cursor · 2026-08-12T22:55:44Z

Standards follow-up addressed in cd3aa25:

Changeset — added .changeset/ype-104-ui-consumer-api.md (UI minor) covering refreshHighlights, onHighlightError, and the sign-out guard.
Filter tests — moved reportHighlightWriteError filter table to packages/ui/src/lib/__tests__/report-highlight-write-error.test.ts.
Describe flatten — bible-reader-consumer-api.test.tsx is now one module describe with flat its.

Spec web bypass for YouVersionAuthButton is in the same commit.

Code Implementor bot, sent on behalf of Cam. I am an AI.

cameronapak

Re-review @ `cd3aa25`

Spec

pass — all locks hold, including the web sign-out bypass on both surfaces.

refreshHighlights — React 19 ref + useImperativeHandle → highlights.refresh (no forwardRef).
onHighlightError — fires only for queued and error + transient via reportHighlightWriteError.
useSignOutGuard — shared by reader toolbar and YouVersionAuthButton; confirm calls signOut() only; no discard path; UI-only.
Web — both surfaces use Platform.OS === 'web' ? signOut : guardedSignOut (auth-button gap from the first review is closed).

Standards

Fixed since last review

Changeset present (.changeset/ype-104-ui-consumer-api.md).
One module describe in bible-reader-consumer-api.test.tsx.
Filter table moved to lib/__tests__/report-highlight-write-error.test.ts.

Hard

issue (hard): unused import reds Lint
youversion-auth-button.test.tsx still imports unused ReactNode. CI Lint fails with --max-warnings=0 (@typescript-eslint/no-unused-vars). Drop ReactNode from the import.

Judgement / nits (non-blocking)

suggestion (judgement): Mysterious Name — HighlightWriteError includes status: 'queued'. Keep if the #104 name is locked; otherwise a notice-style name reads truer.

suggestion (judgement): Speculative Generality — root-export of useSignOutGuard is fine if partners reuse it (changeset says so). If not, keep it barrel-private to the two SDK call sites.

nit: new consumer-api tests still use fireEvent; AGENTS prefers userEvent for new tests.

nit: Object.keys(ref) === ['refreshHighlights'] is close to framework-mechanics (layer-3 guidance).

Praise

Layer-1 helper tests for the filter.
Auth-button web bypass + test that pins no Alert on web.
Guard extraction and structural SignOutGuardAuth still look right.

Summary: Spec 0 findings. Standards: 1 hard (unused ReactNode / red Lint); rest nits/judgement. Worst Standards issue: drop unused ReactNode so Lint goes green.

Code Reviewer bot, sent on behalf of Cam.

Co-authored-by: Cameron Pak <cameronandrewpak@gmail.com>

cameronapak

Confirm @ `23b3847`

Spec

pass — no behavior change. Prior locks at cd3aa25 still hold (refresh handle, error filter, sign-out guard, web bypass on both surfaces).

Standards

Hard axis clear. Unused ReactNode import is gone from youversion-auth-button.test.tsx. Lint is green on this HEAD.

Non-blocking leftovers from the last review (nits / judgement only): fireEvent vs userEvent, Object.keys on the ref handle, HighlightWriteError naming, and whether useSignOutGuard stays a root export.

Summary: Spec 0. Standards hard 0. Worst remaining items are nits only.

Code Reviewer bot, sent on behalf of Cam.

Co-authored-by: Cameron Pak <cameronandrewpak@gmail.com>

cameronapak

Spec + Standards follow-up (`74c1d1a`)

Greptile P1 and P2 are closed. Lint, Typecheck, Test, and Build pass.

Spec

0 findings.

P1 closed: useSignOutGuard owns the web path. On web, the hook calls signOut() and does not raise Alert. On native, Alert still shows. Confirm still calls signOut() only. Callers do not keep a local web bypass.
P2 closed: reportHighlightWriteError wraps onHighlightError in try/catch. A throw in the callback does not become an unhandled rejection.
Prior spec is not regressed: refreshHighlights ref, error filter, shared guard, no discard-first path.

The PR body still says "Alert always shows". That line is leftover native-only wording. The intended spec is the hook-owned web fallback.

Standards

0 hard findings.

Judgement notes are on the snippets below.

Code Reviewer bot, sent on behalf of Cam.

cameronapak · 2026-08-13T15:14:00Z

+  message?: string
+}
+
+function invokeHighlightErrorHandler(


suggestion: This name only says that the function calls the handler.

The body isolates a throw from the consumer. A name that states that policy (for example callOnHighlightErrorSafely) is clearer.

Code Reviewer bot, sent on behalf of Cam.

cameronapak · 2026-08-13T15:14:00Z

+    expect(mockSignOut).not.toHaveBeenCalled()
+  })
+
+  it('signs out immediately on web without raising Alert', async () => {


nit: The hook test now covers the web path (use-sign-out-guard.test.tsx → describe('web')).

This RNTL case covers the same branch through the button. It is a duplicate, not a production split. You can keep it as a consumer smoke test, or drop it.

Code Reviewer bot, sent on behalf of Cam.

Co-authored-by: Cameron Pak <cameronandrewpak@gmail.com>

bmanquen

Two-axis review of origin/highlights...HEAD (5 commits): Standards (repo docs + a Fowler smell baseline) and Spec (this PR body + the changeset), run as separate passes so neither masks the other. Findings are inline below — 1 blocking, 3 issues, 1 todo, 2 suggestions, 1 question, 3 nitpicks.

praise: reportHighlightWriteError's filtering is exhaustive against HighlightWriteOutcome (ok | queued | noop | error{not-signed-in|auth|transient|invalid}) — only queued and error+transient fire, exactly as specified. All three reader write sites report. Confirm→signOut() only, cancel→nothing, no discardPendingHighlights/clearHighlightQueue: the sign-out flow matches the spec line for line.

Not an issue: "The Alert always shows" vs. the changeset's web bypass reconciles fine — "always" contrasts with signing out silently on an empty queue, and the web bypass is pre-existing documented reader behaviour now applied uniformly.

bmanquen · 2026-08-13T15:58:03Z

+  error: HighlightWriteError,
+): void {
+  try {
+    void Promise.resolve(onHighlightError(error)).catch((err) => {


suggestion (non-blocking): This rejection branch is unreachable through the public type.

The prop is typed (error: HighlightWriteError) => void on both HighlightWriteError and BibleReaderProps:167, so a type-conforming caller can't return a promise — meaning neither this .catch nor the test added in 916b636 can fire in practice. Either widen the prop to void | Promise<void> (as onCopy/onShare already are) and keep the containment, or drop the wrapper and rely on the surrounding try/catch.

bmanquen · 2026-08-13T15:58:03Z

+    invokeHighlightErrorHandler(onHighlightError, {
+      status: 'error',
+      reason: 'transient',
+      verses: outcome.failedVerses,


question (non-blocking): A partially-failed batch reports only its failures — verses: outcome.failedVerses, with succeededVerses dropped. Intended? The spec renames the type but says nothing about its shape, so flagging rather than asserting.

bmanquen · 2026-08-13T15:58:03Z

      if (swatch.state === 'remove') {
-        void removeHighlight(swatch.color, verses)
+        void removeHighlight(swatch.color, verses).then((outcome) =>
+          reportHighlightWriteError(outcome, onHighlightError),


nitpick (non-blocking): .then((outcome) => reportHighlightWriteError(outcome, onHighlightError)) repeats verbatim at lines 353, 371, and 399. One local reportWrite callback would capture onHighlightError once and drop it from three dep arrays.

bmanquen · 2026-08-13T15:58:03Z

+
+    await selectVerses(getByTestId)
+    await act(async () => {
+      fireEvent.press(getByTestId('trigger-apply-swatch'))


nitpick (non-blocking): AGENTS.md → Testing → Conventions: "Prefer userEvent over fireEvent for new tests." This file is new and uses fireEvent.press throughout (lines 196, 240, 268). The youversion-auth-button.test.tsx edits in this same PR do use userEvent.

bmanquen · 2026-08-13T15:58:03Z

+
+type AlertButton = { text?: string; style?: string; onPress?: () => void }
+
+function alertCall() {


nitpick (non-blocking): alertCall / pressAlertButton are re-implemented here and in youversion-auth-button.test.tsx. One shared helper would do — the Alert-mock shape is the thing most likely to drift between the two copies.

Co-authored-by: Cameron Pak <cameronandrewpak@gmail.com>

cameronapak commented Aug 12, 2026

View reviewed changes

fix(ui): bypass sign-out Alert on web for auth button

cd3aa25

Co-authored-by: Cameron Pak <cameronandrewpak@gmail.com>

cameronapak commented Aug 12, 2026

View reviewed changes

Comment thread packages/ui/src/native/__tests__/youversion-auth-button.test.tsx Outdated

chore(ui): drop unused ReactNode import in auth button test

23b3847

Co-authored-by: Cameron Pak <cameronandrewpak@gmail.com>

cameronapak commented Aug 12, 2026

View reviewed changes

cameronapak self-assigned this Aug 13, 2026

cameronapak marked this pull request as ready for review August 13, 2026 14:00

greptile-apps Bot reviewed Aug 13, 2026

View reviewed changes

Comment thread packages/ui/src/native/use-sign-out-guard.ts

Comment thread packages/ui/src/lib/report-highlight-write-error.ts Outdated

fix(ui): web sign-out in hook and isolate onHighlightError throws

74c1d1a

Co-authored-by: Cameron Pak <cameronandrewpak@gmail.com>

greptile-apps Bot reviewed Aug 13, 2026

View reviewed changes

Comment thread packages/ui/src/lib/report-highlight-write-error.ts

cameronapak commented Aug 13, 2026

View reviewed changes

fix(ui): swallow rejected async onHighlightError

916b636

Co-authored-by: Cameron Pak <cameronandrewpak@gmail.com>

bmanquen reviewed Aug 13, 2026

View reviewed changes

fix(ui): contain signOut rejections and gate signed-out guard

05aa013

Co-authored-by: Cameron Pak <cameronandrewpak@gmail.com>

greptile-apps Bot reviewed Aug 13, 2026

View reviewed changes

Comment thread packages/ui/src/native/use-sign-out-guard.ts

cursoragent and others added 2 commits August 13, 2026 18:10

fix(ui): contain rejecting web signOut in useSignOutGuard

95aa618

Co-authored-by: Cameron Pak <cameronandrewpak@gmail.com>

fix(ui): make HighlightWriteError a two-member union

d4ef5fb

Co-authored-by: Cameron Pak <cameronandrewpak@gmail.com>

greptile-apps Bot reviewed Aug 13, 2026

View reviewed changes

Comment thread packages/ui/src/native/use-sign-out-guard.ts Outdated

fix(ui): explicit sign-out still clears session when unauthenticated

75a3811

Co-authored-by: Cameron Pak <cameronandrewpak@gmail.com>

cameronapak merged commit 5dedcb6 into highlights Aug 13, 2026
8 checks passed


		type AlertButton = { text?: string; style?: string; onPress?: () => void }

		function alertCall() {

Uh oh!

Conversation

cameronapak commented Aug 12, 2026 • edited by greptile-apps Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changed

Sign-out behavior

Test plan

Greptile Summary

Confidence Score: 5/5 ·

Important Files Changed

Flowchart

Uh oh!

cameronapak left a comment

Choose a reason for hiding this comment

Spec

Standards

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cameronapak left a comment

Choose a reason for hiding this comment

Standards follow-up

Ship bar

Judgement

Uh oh!

cursor Bot commented Aug 12, 2026

Uh oh!

cameronapak left a comment

Choose a reason for hiding this comment

Re-review @ cd3aa25

Spec

Standards

Fixed since last review

Hard

Judgement / nits (non-blocking)

Praise

Uh oh!

Uh oh!

cameronapak left a comment

Choose a reason for hiding this comment

Confirm @ 23b3847

Spec

Standards

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cameronapak left a comment

Choose a reason for hiding this comment

Spec + Standards follow-up (74c1d1a)

Spec

Standards

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bmanquen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

cameronapak commented Aug 12, 2026 •

edited by greptile-apps Bot

Loading

Re-review @ `cd3aa25`

Confirm @ `23b3847`

Spec + Standards follow-up (`74c1d1a`)