fix(ui): collapse the BibleCard error state into one alert region - #321
Open
cameronapak wants to merge 3 commits into
Open
fix(ui): collapse the BibleCard error state into one alert region#321cameronapak wants to merge 3 commits into
cameronapak wants to merge 3 commits into
Conversation
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 detectedLatest commit: f7fcc99 The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
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 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
YPE-2360 | Artifacts | Task
What problems was I solving
The
BibleCardcomponent 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
passageErrorwas 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
role="alert"andaria-live, so a screen reader no longer announces it as a second alert. The version picker now shows during an error.aria-live, becauserole="alert"already makes the block a live region. The icon now hasaria-hidden, so a screen reader skips it.VerseOfTheDayand standaloneBibleTextViewshow the same message block. Both components get the same two accessibility fixes. The text that they show does not change. Neither component gets an "ERROR" label, because that label belongs to theBibleCardheader 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.
How I implemented it
One alert region
In bible-card.tsx,
BibleCardHeaderErrorwas adivwithrole="alert"andaria-live="polite"around one<h2>. It is now the<h2>alone. Thedivheld no classes that changed the layout, so the card looks the same.The
<h2>uses the same classes asBibleCardHeaderReference. 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,
VerseUnavailableMessagekeepsrole="alert"and becomes the only alert region. It losesaria-liveand marks its iconaria-hidden.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 && !passageErrortoshowVersionPicker. This is the whole source change. The picker readsversionNumand not the passage, so it works whilepassageErroris 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-betweenis still correct.Tests and the Storybook story
bible-card.test.tsx gets a new
BibleCard - Error stateblock with five tests:role="alert"region shows.<h2>that reads "Error".role="status"spinner shows during an error.The picker test needed five more hook mocks:
useLanguages,useLanguage,useVersions,useFilteredVersions, anduseOrganizations. The file mocks the hooks package with a barevi.mockcall. Every export therefore returnsundefined, andBibleVersionPicker.Rootreads these five.verse.test.tsx gets one test. The alert has no
aria-live. The icon hasaria-hidden. The alert holds no "Error" text, because a standaloneBibleTextViewhas no header slot.bible-card.stories.tsx gets
showVersionPicker: trueon theErrorstory. Itsplayfunction now tests for one alert region and an enabled picker..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
VerseUnavailableMessagekeepsrole="alert", losesaria-live, and marks its iconaria-hidden.BibleCardHeaderErroris now a bare<h2>with the classes ofBibleCardHeaderReference.&& !passageError.errorHeadingkey comes from the existing locale files.Deviations and surprises
/NIV/i. This test is stricter than the outline, and it does not conflict with it..changeset/config.jsonalready puts the three packages in afixedgroup, so a changeset that namesplatform-react-uialone still bumps all three.pnpm changeset statusshows this result. A changeset that names all three would also copy this UI-only text intopackages/core/CHANGELOG.mdandpackages/hooks/CHANGELOG.md. The three most recent UI fixes onmain(b592e72,9c2e8e4,9a2b3e9) each name only the package that changed. The outline records this deviation in its Phase 3 section.Additions that are not in the plan
Doc comments above
BibleCardHeaderError, the header ternary, the version picker condition, andVerseUnavailableMessage. They put the reasoning of the outline into the source. They change no behavior.Items that were planned but not implemented
None.
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.
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-endchange and the empty-row check therefore left the outline before Phase 2 started.How to verify it
Run these commands first:
Manual tests
Automated tests
pnpm typecheck pnpm lint pnpm test pnpm --filter @youversion/platform-react-ui test:integrationIf you use a fresh worktree, do these two steps before you run the commands above:
packages/core/.env.exampletopackages/core/.env.local. Without this file,YVP_API_HOSTis 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.pnpm --filter @youversion/platform-react-ui build:css. Withoutdist/tailwind.css, twoWideContainerstories fail on the missingyv:card-contentcap. This failure is not related to this branch.Results on this branch:
pnpm testpasses 1065 tests (core 369, hooks 289, ui 407).test:integrationpasses 452 tests.pnpm build --forcepasses, includingverify:styles.Description for the changelog
Fix the
BibleCarderror state that announced two alerts instead of one, and keep the version picker usable during an error.Greptile Summary
The PR consolidates
BibleCarderrors into one alert region while preserving an error heading and allowing users to switch Bible versions during passage failures.Confidence Score: 5/5
The PR appears safe to merge, with the accessibility and error-recovery behavior consistently implemented and covered.
The card retains one reachable error alert, preserves a semantic error heading, and exposes the existing version picker during passage failures without a supported path to a render, state, or accessibility regression.
Important Files Changed
Reviews (1): Last reviewed commit: "chore: add changeset for the BibleCard e..." | Re-trigger Greptile