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. diff --git a/packages/ui/src/components/bible-card.stories.tsx b/packages/ui/src/components/bible-card.stories.tsx index 52649147..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: { @@ -206,12 +207,26 @@ 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.', + ); + + // 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 3f58a99e..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'); @@ -156,6 +165,90 @@ 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(); + }); + + 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', () => { 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..3bec741b 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 ? (
@@ -164,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 ? ( { }); }); + 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(
);