Skip to content

fix(ui): collapse the BibleCard error state into one alert region - #321

Merged
cameronapak merged 7 commits into
mainfrom
ype-2360-react-biblecard-error-state-needs-better-text
Aug 11, 2026
Merged

fix(ui): collapse the BibleCard error state into one alert region#321
cameronapak merged 7 commits into
mainfrom
ype-2360-react-biblecard-error-state-needs-better-text

Conversation

@cameronapak

@cameronapak cameronapak commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

YPE-2360 | Artifacts | Task

What problems was I solving

The BibleCard component shows an error state when a Bible passage does not load. This error state had two faults.

Fault 1. A screen reader announced two alerts for one failure.

The card showed two separate regions with role="alert". The first region was the word "ERROR" in the header slot. The second region was an icon plus a sentence that explains the error. Both regions were live regions. A screen reader therefore announced the word "Error" first, and then announced the sentence as a second alert.

Fault 2. An error was a dead end.

The card hid the version picker when passageError was set. A 404 error means the passage is not in the selected Bible version. The fix is to select a different version. The card removed that control at the moment the reader needed it.

After this change, a failed request announces one alert. That alert carries the sentence that explains the error. The reader can also select a different version without leaving the card.

What user-facing changes did I ship

  • packages/ui/src/components/bible-card.tsx - The word "ERROR" keeps its position and its style. It loses role="alert" and aria-live, so a screen reader no longer announces it as a second alert. The version picker now shows during an error.
  • packages/ui/src/components/verse.tsx - The icon now has aria-hidden, so a screen reader skips it. The block keeps role="alert" and aria-live="polite". Both attributes are load bearing, and the section below explains why.

VerseOfTheDay and standalone BibleTextView show the same message block. Both components get the icon fix. Their announcement stays polite and their visible text does not change. Neither component gets an "ERROR" label, because that label belongs to the BibleCard header slot.

This change adds no new i18n keys. The eight status-aware messages, their six locales, and the code that derives errors are all unchanged. This PR does not rewrite the error copy. See "Scope: the copy work is a separate ticket" below.

How I implemented it

One alert region

In bible-card.tsx, BibleCardHeaderError was a div with role="alert" and aria-live="polite" around one <h2>. It is now the <h2> alone. The div held no classes that changed the layout, so the card looks the same.

The <h2> uses the same classes as BibleCardHeaderReference. This reuse is deliberate. The card already puts an <h2> in that slot for the passage reference. The error label therefore adds no new heading level to the outline of the host page.

In verse.tsx, VerseUnavailableMessage keeps role="alert" and becomes the only alert region. It marks its icon aria-hidden. It also keeps aria-live="polite".

That aria-live="polite" is not redundant. In WAI-ARIA, role="alert" carries an implicit aria-live="assertive". An explicit aria-live="polite" on the same element overrides the implicit value downward. Deleting the attribute would therefore make the announcement assertive, which is a behavior change and not a cleanup. VerseOfTheDay renders this block on page load, and an assertive announcement would interrupt whatever the screen reader is already saying. An earlier commit on this branch deleted the attribute; review caught it and it is restored.

The header ternary keeps three branches instead of two. Two branches would fall through to the loading spinner during an error. The spinner would then turn forever.

The version picker during an error

In bible-card.tsx, the condition changed from showVersionPicker && !passageError to showVersionPicker. This is the whole source change. The picker reads versionNum and not the passage, so it works while passageError is set.

The header row needs no layout change. The word "ERROR" still sits on the left and the picker on the right. The class yv:justify-between is still correct.

Tests and the Storybook story

bible-card.test.tsx gets a new BibleCard - Error state block with four tests:

  • Exactly one role="alert" region shows.
  • That region carries the 503 sentence.
  • The header shows an <h2> that reads "Error".
  • No role="status" spinner shows during an error.

The version picker is covered by the Error story instead of by a fifth jsdom test. That jsdom test would have to hand-mock the five hooks that BibleVersionPicker.Root reads: useLanguages, useLanguage, useVersions, useFilteredVersions, and useOrganizations. Those mocks couple this file to the internals of a component it does not test. packages/ui/CLAUDE.md also says to prefer Storybook for UI component tests.

verse.test.tsx gets one test. One alert region shows. That region carries aria-live="polite". The icon has aria-hidden. The alert holds no "Error" text, because a standalone BibleTextView has no header slot.

Each test file builds its own error objects. verse.test.tsx keeps a local three-line createError and calls it nine times. bible-card.test.tsx needs one error object, so it writes that Object.assign inline.

bible-card.stories.tsx gets showVersionPicker: true on the Error story. Its play function asserts one alert region, aria-live="polite" on that region, and an enabled picker. It then walks the whole recovery path: open the picker, search for the Amplified Bible, select it, and assert that the alert clears and the passage renders.

That story also spreads globalHandlers back into its own msw.handlers array. A story-level handlers array replaces the preview-level array instead of merging with it. Without the spread, the picker's useLanguages and useVersions calls fell through to the live API, silently, because onUnhandledRequest is 'warn'. The 500 override is listed first, because MSW takes the first matching handler. Version 1588 (AMP) stays on globalHandlers and still resolves, which is what makes the recovery path testable.

.changeset/biblecard-single-error-alert.md is a patch that names @youversion/platform-react-ui.

Deviations from the plan

The plan artifact is a structure outline, not a plan file. It changed twice during the work, so it already describes the final direction. The code and the outline now agree on every phase.

Implemented as planned

  • VerseUnavailableMessage keeps role="alert" and marks its icon aria-hidden. It also keeps aria-live="polite", which is a correction to the outline. See "Review feedback" below.
  • BibleCardHeaderError is now a bare <h2> with the classes of BibleCardHeaderReference.
  • The header ternary keeps three branches.
  • The version picker condition loses && !passageError.
  • The tests and the story cover every case that the outline listed.
  • No new i18n keys. The errorHeading key comes from the existing locale files.

Deviations and surprises

  1. The outline said that the story must test that the picker shows and is enabled. The story also tests that the button text matches /NIV/i. This test is stricter than the outline, and it does not conflict with it.
  2. The changeset names one package. The outline asked for three. .changeset/config.json already puts the three packages in a fixed group, so a changeset that names platform-react-ui alone still bumps all three. pnpm changeset status shows this result. A changeset that names all three would also copy this UI-only text into packages/core/CHANGELOG.md and packages/hooks/CHANGELOG.md. The three most recent UI fixes on main (b592e72, 9c2e8e4, 9a2b3e9) each name only the package that changed. The outline records this deviation in its Phase 3 section.

Scope: the copy work is a separate ticket

YPE-2360 asks for two things. It asks for clear error text, and it asks for a review of that text against Figma. This PR delivers neither. It fixes the alert semantics and the recovery path instead, and it changes no strings.

This split is deliberate, for two reasons.

  1. The two jobs have different reviewers. The a11y and recovery fix is an engineering review. New copy needs a design and content review against the Figma web error states, and it lands in six locale files.
  2. The a11y fix is already correct and tested. Holding it behind a copy review would keep a real double-announcement bug in main.

Two claims in the ticket are already true in the code on main, before this PR. The message block uses yv:text-foreground, not a red destructive token, so the state is not styled as an alarm. The library also already picks from eight status-aware messages in packages/ui/src/lib/bible-text-error.ts, so a 404 and a 503 do not read the same. What is still open is whether those eight strings are the right strings.

The follow-up ticket for the copy work is YPE-4856. It is linked to YPE-2360.

Review feedback

Commit 9ba2dc2 responds to the review on this PR. Four changes:

  1. aria-live="polite" is restored on VerseUnavailableMessage. The earlier commit removed it as redundant. It is not redundant, because role="alert" implies aria-live="assertive". The removal would have moved VerseOfTheDay and standalone BibleTextView from a polite announcement to an assertive one. The changeset, the doc comment, and this body all now say the same thing.
  2. The Error story spreads globalHandlers back into its msw.handlers array. Mounting the picker under the old array sent the languages and versions requests to the live API.
  3. The Error story's play function walks the recovery path end to end. It switches to a version whose passage resolves and asserts that the alert clears. Before this, the story asserted only that the picker was enabled, which is a weaker claim than the one the PR makes.
  4. createError moved into packages/ui/src/test/errors.ts. Two test files declared the same helper. Commit d2abe0d reverses this one. Outside verse.test.tsx the shared module had exactly one caller, so it was an abstraction ahead of a second real consumer. verse.test.tsx keeps the local helper it started with. bible-card.test.tsx inlines its single call. The file is deleted.

Additions that are not in the plan

Doc comments above BibleCardHeaderError, the header ternary, the version picker condition, and VerseUnavailableMessage. They put the reasoning of the outline into the source. They change no behavior.

Items that were planned but not implemented

The jsdom test for the version picker during an error. The Error story covers that behavior, and it covers it better, because the story renders the real picker instead of five mocked hooks.

How the outline changed during the work

The outline changed shape twice. This history matters only if you read the artifacts next to the diff.

  1. The design discussion decided to remove the header label completely.
  2. A later outline revision put the word "Error" back, but inside the message block, above the sentence.
  3. On 2026-08-07, Cam reviewed the result in Storybook and reversed that decision. The label stays in the header slot. Only its alert semantics change. The code does this.

That reversal made two planned Phase 2 changes unnecessary. Both changes assumed an empty header slot. The header slot always has a child during an error. The justify-end change and the empty-row check therefore left the outline before Phase 2 started.

How to verify it

Run these commands first:

git fetch origin ype-2360-react-biblecard-error-state-needs-better-text
git checkout ype-2360-react-biblecard-error-state-needs-better-text
pnpm install
pnpm --filter @youversion/platform-core --filter @youversion/platform-react-hooks build
pnpm --filter @youversion/platform-react-ui storybook

Manual tests

  • Open Components/BibleCard → Error. Make sure that "ERROR" is in the header slot, above one block with the icon and the sentence.
  • Set the Storybook theme global to dark. Make sure that the story still reads correctly.
  • Start VoiceOver. Make sure that it announces one alert, and that this alert is the sentence.
  • Open Components/VerseOfTheDay with VoiceOver running. Make sure that the error announcement waits its turn instead of interrupting.
  • In the Error story, open the version picker and select a different version.
  • Make sure that the card recovers when the new version returns the passage.
  • Open Components/VerseOfTheDay and Components/BibleTextView. Both show the icon and the sentence, with no label.

Automated tests

pnpm typecheck
pnpm lint
pnpm test
pnpm --filter @youversion/platform-react-ui test:integration

If you use a fresh worktree, do these two steps before you run the commands above:

  1. Copy packages/core/.env.example to packages/core/.env.local. Without this file, YVP_API_HOST is empty and all 16 core suites fail to collect. The core tests use MSW mocks, so placeholder values are enough. CI supplies both values as secrets.
  2. Run pnpm --filter @youversion/platform-react-ui build:css. Without dist/tailwind.css, two WideContainer stories fail on the missing yv:card-content cap. This failure is not related to this branch.

Results on this branch, at commit 9ba2dc2: pnpm test passes 1064 tests (core 369, hooks 289, ui 406). test:integration passes 451 tests across 39 files. pnpm typecheck and pnpm lint both pass. Each count is one lower than the count before the review, which is the one removed jsdom picker test.

Description for the changelog

Fix the BibleCard error state that announced two alerts instead of one, and keep the version picker usable during an error.

Greptile Summary

The PR consolidates BibleCard failures into one polite alert while preserving the separate visual error heading and exposing the version picker as an in-card recovery path.

  • Removes live-region semantics from the BibleCard error heading.
  • Hides the decorative error icon from assistive technology.
  • Keeps the version picker available during passage errors.
  • Adds unit and Storybook coverage for alert semantics and version-switch recovery.
  • Adds a patch changeset for the UI package.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
packages/ui/src/components/bible-card.tsx Separates the visual error heading from the sole live alert and keeps the controlled version picker rendered during passage failures.
packages/ui/src/components/verse.tsx Preserves the polite alert behavior and removes the decorative icon from the accessibility tree.
packages/ui/src/components/bible-card.stories.tsx Restores global MSW handlers and verifies the complete transition from a failed NIV passage to a successful AMP passage.
packages/ui/src/components/bible-card.test.tsx Adds focused coverage for one alert, the separate error heading, status-aware copy, and absence of an error-state spinner.
packages/ui/src/components/verse.test.tsx Verifies the shared unavailable-message block remains one polite alert with a hidden icon.
.changeset/biblecard-single-error-alert.md Records the user-facing accessibility and recovery fixes as a UI package patch.

Reviews (4): Last reviewed commit: "chore: re-trigger the storybook test run" | Re-trigger Greptile

Context used:

cameronapak and others added 3 commits August 7, 2026 12:03
A failed passage request rendered two competing role="alert" regions: the
"ERROR" label in the header slot and the icon plus status-aware sentence in
the body. Screen readers announced both.

The body block is now the only alert region. The header label keeps its place
and its styling but drops role="alert" and aria-live. The body block drops its
redundant aria-live and hides its icon with aria-hidden.

No new i18n keys, and no change to how errors are derived.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A 404 means the passage is missing from the selected version, and switching
versions is the fix. The picker was hidden whenever passageError was set, so
the card offered no way out.

The picker reads versionNum rather than the passage, so it renders and works
while an error is showing. The header row needs no change: the "ERROR" label
still sits on the left and the picker on the right.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@changeset-bot

changeset-bot Bot commented Aug 7, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: e719bbc

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages
Name Type
@youversion/platform-react-ui Patch
vite-react Patch
@youversion/platform-core Patch
@youversion/platform-react-hooks Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@cameronapak cameronapak self-assigned this Aug 10, 2026
Comment thread packages/ui/src/components/bible-card.stories.tsx
Comment thread packages/ui/src/components/bible-card.test.tsx Outdated

@bmanquen bmanquen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review of this branch against the repo's documented standards and against what the PR body says it ships. Four inline comments below; two of them I'd call blocking.

Blocking

  1. The aria-live removal in verse.tsx is a politeness change, not a de-duplication — it takes VerseOfTheDay and standalone BibleTextView from polite to assertive. The PR body and the changeset both call it "redundant"/"duplicate", and the source comment added in the same hunk says the opposite.
  2. Turning on showVersionPicker in the Error story makes an integration-tagged story fire unmocked network requests.

Non-blocking
3. The changeset text needs the same correction as (1).
4. createError is duplicated verbatim across the two test files, and the new jsdom tests re-assert what the Error play function already covers — packages/ui/AGENTS.md prefers Storybook for UI component tests.

Separately, a scoping question, not a code finding. YPE-2360 is titled "React BibleCard error state needs better text", and this branch changes zero user-visible strings — packages/ui/src/lib/bible-text-error.ts and all six locale files are untouched. The PR body confirms this ("The text that they show does not change"), but doesn't record it as a deviation. Alert semantics and picker availability are both real fixes; if the wording work was deliberately split off, worth saying so here so the ticket doesn't look closed by this PR.

Everything the PR body claims about the implementation checks out otherwise: exactly one role="alert" renders during an error, the header ternary really does keep three branches, the picker renders and stays enabled while passageError is set, no new i18n keys, and changeset status confirms the single-package changeset bumps all three via the fixed group.

Comment thread packages/ui/src/components/verse.tsx
Comment thread packages/ui/src/components/bible-card.stories.tsx
Comment thread .changeset/biblecard-single-error-alert.md Outdated
Comment thread packages/ui/src/components/bible-card.test.tsx Outdated
cameronapak and others added 2 commits August 11, 2026 09:32
Keep aria-live="polite" on VerseUnavailableMessage. role="alert" implies
assertive, so the attribute was an override, not a duplicate. Removing it
would have moved VerseOfTheDay and standalone BibleTextView from polite to
assertive announcements.

Spread globalHandlers back into the Error story's msw handlers. A
story-level handlers array replaces the preview-level one, so mounting the
version picker sent the languages and versions requests to the live API.

Extend the Error story's play function through the recovery path: switch to
a version whose passage resolves and assert the alert clears.

Drop the jsdom picker test that hand-mocked five BibleVersionPicker
internals; the story covers it. Move createError into a shared test helper.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Comment thread packages/ui/src/test/errors.ts Outdated
cameronapak and others added 2 commits August 11, 2026 10:41
Delete packages/ui/src/test/errors.ts. bible-card.test.tsx called it once,
so that call is now inlined. verse.test.tsx calls it nine times and keeps a
local three-line helper, which is where it started.

A shared module for one Object.assign was an abstraction ahead of a second
real consumer.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The prior run's only job failed on the known dom-vapor timeout flake, and
GitHub wedged the run record at in_progress so no re-run could be requested.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@cameronapak
cameronapak merged commit 4cae248 into main Aug 11, 2026
14 checks passed
@cameronapak
cameronapak deleted the ype-2360-react-biblecard-error-state-needs-better-text branch August 11, 2026 19:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants