From 95e6c7e97066b2fd7b33a30b72bb6cd870c37c26 Mon Sep 17 00:00:00 2001 From: Cameron Pak Date: Fri, 7 Aug 2026 12:03:18 -0500 Subject: [PATCH 1/3] fix(ui): collapse the BibleCard error state into one alert region 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 --- .../ui/src/components/bible-card.stories.tsx | 12 +++-- .../ui/src/components/bible-card.test.tsx | 49 +++++++++++++++++++ packages/ui/src/components/bible-card.tsx | 21 ++++++-- packages/ui/src/components/verse.test.tsx | 28 +++++++++++ packages/ui/src/components/verse.tsx | 11 +++-- 5 files changed, 108 insertions(+), 13 deletions(-) diff --git a/packages/ui/src/components/bible-card.stories.tsx b/packages/ui/src/components/bible-card.stories.tsx index 52649147..75d419c7 100644 --- a/packages/ui/src/components/bible-card.stories.tsx +++ b/packages/ui/src/components/bible-card.stories.tsx @@ -206,12 +206,16 @@ export const Error: Story = { play: async ({ canvasElement }) => { const canvas = within(canvasElement); + // The header slot carries the "Error" label; the body block is the one alert. await waitFor(async () => { await expect(canvas.getByRole('heading', { level: 2, name: /error/i })).toBeInTheDocument(); - const errorMessages = canvas.getAllByText( - 'The Bible service is having trouble right now. Please try again in a moment.', - ); - await expect(errorMessages.length).toBeGreaterThan(0); }); + + const alerts = canvas.getAllByRole('alert'); + + await expect(alerts).toHaveLength(1); + await expect(alerts[0]).toHaveTextContent( + 'The Bible service is having trouble right now. Please try again in a moment.', + ); }, }; diff --git a/packages/ui/src/components/bible-card.test.tsx b/packages/ui/src/components/bible-card.test.tsx index 3f58a99e..7ab6f707 100644 --- a/packages/ui/src/components/bible-card.test.tsx +++ b/packages/ui/src/components/bible-card.test.tsx @@ -156,6 +156,55 @@ describe('BibleCard - Delayed spinner', () => { }); }); +describe('BibleCard - Error state', () => { + function createError(message: string, status?: number): Error { + return Object.assign(new Error(message), status === undefined ? {} : { status }); + } + + beforeEach(() => { + vi.mocked(useTheme).mockReturnValue('light'); + vi.mocked(useVersion).mockReturnValue({ + version: mockVersion, + loading: false, + error: null, + refetch: vi.fn(), + }); + vi.mocked(usePassage).mockReturnValue({ + passage: null, + loading: false, + error: createError('Request failed with status 503', 503), + refetch: vi.fn(), + }); + }); + + it('should render exactly one alert region', () => { + const { container } = render(); + + expect(within(container).getAllByRole('alert')).toHaveLength(1); + }); + + it('should show the status message in that one alert region', () => { + const { container } = render(); + const alert = within(container).getByRole('alert'); + + expect(alert).toHaveTextContent( + 'The Bible service is having trouble right now. Please try again in a moment.', + ); + }); + + it('should render the error heading in the header slot', () => { + const { container } = render(); + + expect(within(container).getByRole('heading', { level: 2 })).toHaveTextContent('Error'); + }); + + it('should not render a loading spinner while an error is set', () => { + const { container } = render(); + + expect(within(container).queryByRole('status')).toBeNull(); + }); +}); + describe('BibleCard - onFootnotePress callback', () => { const mockPassageWithFootnote: BiblePassage = { id: 'JHN.1', diff --git a/packages/ui/src/components/bible-card.tsx b/packages/ui/src/components/bible-card.tsx index f505b457..596eb1d8 100644 --- a/packages/ui/src/components/bible-card.tsx +++ b/packages/ui/src/components/bible-card.tsx @@ -26,14 +26,21 @@ export type BibleCardProps = { onFootnotePress?: (data: FootnoteData) => void; }; +/** + * The "Error" label for the header slot. + * + * It matches `BibleCardHeaderReference` exactly. The card already renders an + * `

` in this slot for the passage reference, so this injects no new heading + * level into the host page's outline. It carries no `role="alert"` and no + * `aria-live`: the body block stays the single alert region, so screen readers + * announce one alert. + */ function BibleCardHeaderError(): React.ReactNode { const { t } = useTranslation(undefined, { i18n }); return ( -
-

- {t('errorHeading')} -

-
+

+ {t('errorHeading')} +

); } @@ -151,6 +158,10 @@ export function BibleCard({ >
+ {/* + The error branch stays separate rather than folding into the loading + branch, which would spin forever on error. + */} {passage && !passageError ? (
diff --git a/packages/ui/src/components/verse.test.tsx b/packages/ui/src/components/verse.test.tsx index ece936e3..9bebc023 100644 --- a/packages/ui/src/components/verse.test.tsx +++ b/packages/ui/src/components/verse.test.tsx @@ -1044,6 +1044,34 @@ describe('BibleTextView - Error messaging', () => { }); }); + it('should render one alert region with a hidden icon and no heading line', async () => { + const { getAllByRole, getByRole } = render( + , + ); + + await waitFor(() => { + expect(getByRole('alert')).toHaveTextContent( + 'The Bible service is having trouble right now. Please try again in a moment.', + ); + }); + + const alert = getByRole('alert'); + + expect(getAllByRole('alert')).toHaveLength(1); + expect(alert).not.toHaveAttribute('aria-live'); + expect(alert.querySelector('svg')).toHaveAttribute('aria-hidden', 'true'); + // Standalone BibleTextView has no header slot, so no "Error" label renders. + expect(alert).not.toHaveTextContent('Error'); + }); + it('should prioritize 5xx errors over "not found" text in the message', async () => { const { getByRole } = render(
); From 52a1b965e02319e89f2853e8bd252a5e59143799 Mon Sep 17 00:00:00 2001 From: Cameron Pak Date: Fri, 7 Aug 2026 12:10:42 -0500 Subject: [PATCH 2/3] fix(ui): keep the BibleCard version picker usable during an error 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 --- .../ui/src/components/bible-card.stories.tsx | 11 +++++ .../ui/src/components/bible-card.test.tsx | 48 ++++++++++++++++++- packages/ui/src/components/bible-card.tsx | 6 ++- 3 files changed, 62 insertions(+), 3 deletions(-) diff --git a/packages/ui/src/components/bible-card.stories.tsx b/packages/ui/src/components/bible-card.stories.tsx index 75d419c7..ef2a9ad9 100644 --- a/packages/ui/src/components/bible-card.stories.tsx +++ b/packages/ui/src/components/bible-card.stories.tsx @@ -186,6 +186,7 @@ export const Error: Story = { args: { reference: 'LUK.1.39-45', versionId: 111, + showVersionPicker: true, }, tags: ['integration'], parameters: { @@ -217,5 +218,15 @@ export const Error: Story = { await expect(alerts[0]).toHaveTextContent( 'The Bible service is having trouble right now. Please try again in a moment.', ); + + // The picker is the in-card recovery path: a 404 is fixed by switching versions. + const versionPickerButton = await canvas.findByRole('button', { + name: /change bible version/i, + }); + + await waitFor(async () => { + await expect(versionPickerButton).toBeEnabled(); + await expect(versionPickerButton).toHaveTextContent(/NIV/i); + }); }, }; diff --git a/packages/ui/src/components/bible-card.test.tsx b/packages/ui/src/components/bible-card.test.tsx index 7ab6f707..b48b398a 100644 --- a/packages/ui/src/components/bible-card.test.tsx +++ b/packages/ui/src/components/bible-card.test.tsx @@ -6,8 +6,17 @@ import { render, act, within, waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { BibleCard } from './bible-card'; import type { FootnoteData } from './verse'; -import { usePassage, useVersion, useTheme } from '@youversion/platform-react-hooks'; -import type { BiblePassage, BibleVersion } from '@youversion/platform-core'; +import { + useFilteredVersions, + useLanguage, + useLanguages, + useOrganizations, + usePassage, + useTheme, + useVersion, + useVersions, +} from '@youversion/platform-react-hooks'; +import type { BiblePassage, BibleVersion, Language } from '@youversion/platform-core'; vi.mock('@youversion/platform-react-hooks'); @@ -203,6 +212,41 @@ describe('BibleCard - Error state', () => { expect(within(container).queryByRole('status')).toBeNull(); }); + + it('should keep the version picker usable so a bad version can be swapped', () => { + // The version picker mounts its own hook tree. The file-level auto-mock + // returns undefined for each one, so give them values here. + 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() }); + + const { container } = render( + , + ); + + const picker = within(container).getByRole('button', { name: /change bible version/i }); + + expect(picker).toBeInTheDocument(); + expect(picker).toBeEnabled(); + expect(picker).toHaveTextContent('BSB'); + }); }); describe('BibleCard - onFootnotePress callback', () => { diff --git a/packages/ui/src/components/bible-card.tsx b/packages/ui/src/components/bible-card.tsx index 596eb1d8..3bec741b 100644 --- a/packages/ui/src/components/bible-card.tsx +++ b/packages/ui/src/components/bible-card.tsx @@ -175,7 +175,11 @@ export function BibleCard({ )} - {showVersionPicker && !passageError ? ( + {/* + The picker stays available during an error. A 404 means the passage + is not in the selected version, so switching versions is the fix. + */} + {showVersionPicker ? ( Date: Fri, 7 Aug 2026 12:17:18 -0500 Subject: [PATCH 3/3] chore: add changeset for the BibleCard error state fix Co-Authored-By: Claude Opus 5 --- .changeset/biblecard-single-error-alert.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .changeset/biblecard-single-error-alert.md diff --git a/.changeset/biblecard-single-error-alert.md b/.changeset/biblecard-single-error-alert.md new file mode 100644 index 00000000..7f05a1a5 --- /dev/null +++ b/.changeset/biblecard-single-error-alert.md @@ -0,0 +1,7 @@ +--- +'@youversion/platform-react-ui': patch +--- + +Fix the `BibleCard` error state announcing two alerts, and keep the version picker usable while an error is showing. The "Error" label stays in the header slot but drops its `role="alert"` and `aria-live`, leaving the message block in the card body as the only alert region. The picker no longer disappears on error, so a 404 has an in-card fix: switch to a version that carries the passage. + +The shared message block also drops a redundant `aria-live` and hides its icon with `aria-hidden`, so `VerseOfTheDay` and standalone `BibleTextView` pick up the same accessibility fixes. Their visible text is unchanged, and neither gains an "Error" label. The eight status-aware messages, their six locales, and how errors are derived are untouched.