Skip to content

Commit 3f69494

Browse files
YPE-953: fix(ui): fix/refactor the footnotes logic (#94)
* fix(ui): fix how the parsed html and notes gets rendered - the dom was getting re-rendered multiple times especially when adding the bible reader settings. - this change allows the verse text to only be re-rendered when a different html string is passed in explicitly. * refactor(ui): sanitize html before parsing/transforming * refactor(ui): use react portals for footnotes * fix(ui): fix footnotes popover - uses new props on our popover component to reduce redundancy in the header of the popover. * test(ui): fix flaky test * fix(ui): fix footnotes icon positioning and size as font size changes * feat(ui): rewrite footnote extraction with proper verse boundary detection - Fix verse boundaries to work across paragraph elements using TreeWalker - Add `v` and `usfm` to DOMPurify allowed attributes (was stripping verse markers) - Place footnote icons at actual verse end, not next verse's paragraph - Exclude headers (.yv-h) from popover verse text - Add spacing between paragraphs in reconstructed verse text - Style footnote markers (A, B, C) with muted foreground color - Inherit reader font size in popover verse content - Remove verse number from popover verse text (shown in header already) - Add tests for footnote extraction and placement - Simplify and refactor extraction logic (~145 to ~85 lines) * fix: resolve type linting * docs(verse): add comments to improve code readability * YPE-999: feat(ui): add dark mode to footnotes popover feat(ui): add dark mode to footnotes * test(verse): add footnote popover interaction test Add test for footnote button click event Verify popover dialog renders with correct list items * refactor: remove dead code from footnote text extraction * fix: Remove unused background prop The `background` prop was removed from the `BibleReader` component and its usage in `Content` and `BibleTextView` was also removed as it was not being utilized. * test(ui): add @testing-library/user-event dependency Introduces @testing-library/user-event to the ui package to provide more realistic simulation of user interactions in tests compared to fireEvent. Updates the Verse.Html - Footnotes test to use userEvent.click * refactor(packages): move testing package to devDependencies * refactor(packages): accurately pass theme prop to HTML.Verse component --------- Co-authored-by: cameronapak <[email protected]>
1 parent 6874d27 commit 3f69494

File tree

10 files changed

+514
-135
lines changed

10 files changed

+514
-135
lines changed

.changeset/puny-pigs-sniff.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
'@youversion/platform-react-hooks': patch
3+
'@youversion/platform-core': patch
4+
'@youversion/platform-react-ui': patch
5+
---
6+
7+
Refactors footnotes implementation to use React portals, improves HTML sanitization, and fixes footnote popover behavior.

packages/ui/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@
7373
"@tailwindcss/cli": "4.1.15",
7474
"@testing-library/jest-dom": "6.9.1",
7575
"@testing-library/react": "16.3.0",
76+
"@testing-library/user-event": "^14.6.1",
7677
"@types/react": "19.2.2",
7778
"@types/react-dom": "19.2.2",
7879
"@vitejs/plugin-react": "5.0.4",

packages/ui/src/components/bible-reader.stories.tsx

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,3 +225,118 @@ export const RealAPI: Story = {
225225
</div>
226226
),
227227
};
228+
229+
export const FootnotesPersistAfterFontSizeChange: Story = {
230+
tags: ['integration'],
231+
args: {
232+
versionId: 111,
233+
book: 'JHN',
234+
chapter: '1',
235+
background: 'light',
236+
},
237+
render: (args) => (
238+
<div className="yv:h-screen yv:bg-background">
239+
<BibleReader.Root {...args}>
240+
<BibleReader.Content />
241+
<BibleReader.Toolbar />
242+
</BibleReader.Root>
243+
</div>
244+
),
245+
play: async ({ canvasElement }) => {
246+
await waitFor(
247+
async () => {
248+
const verseContainer = canvasElement.querySelector('[data-slot="yv-bible-renderer"]');
249+
await expect(verseContainer).toBeInTheDocument();
250+
},
251+
{ timeout: 5000 },
252+
);
253+
254+
const getFootnoteButtons = () => canvasElement.querySelectorAll('[data-verse-footnote] button');
255+
256+
await waitFor(
257+
async () => {
258+
const footnoteButtons = getFootnoteButtons();
259+
await expect(footnoteButtons.length).toBe(9);
260+
},
261+
{ timeout: 5000 },
262+
);
263+
264+
const initialFootnoteCount = getFootnoteButtons().length;
265+
266+
const settingsButton = screen.getByRole('button', { name: /settings/i });
267+
await userEvent.click(settingsButton);
268+
269+
await waitFor(async () => {
270+
await expect(await screen.findByText('Reader Settings')).toBeInTheDocument();
271+
});
272+
273+
const increaseFontButton = screen.getByTestId('increase-font-size');
274+
await userEvent.click(increaseFontButton);
275+
276+
await waitFor(async () => {
277+
const footnoteButtons = getFootnoteButtons();
278+
await expect(footnoteButtons.length).toBe(initialFootnoteCount);
279+
});
280+
281+
const decreaseFontButton = screen.getByTestId('decrease-font-size');
282+
await userEvent.click(decreaseFontButton);
283+
await userEvent.click(decreaseFontButton);
284+
285+
await waitFor(async () => {
286+
const footnoteButtons = getFootnoteButtons();
287+
await expect(footnoteButtons.length).toBe(initialFootnoteCount);
288+
});
289+
},
290+
};
291+
292+
export const ThemeOverridesProvider: Story = {
293+
tags: ['integration'],
294+
args: {
295+
versionId: 111,
296+
book: 'JHN',
297+
chapter: '1',
298+
background: 'light',
299+
},
300+
globals: {
301+
theme: 'dark',
302+
},
303+
render: (args) => (
304+
<div className="yv:h-screen yv:bg-background">
305+
<BibleReader.Root {...args}>
306+
<BibleReader.Content />
307+
<BibleReader.Toolbar />
308+
</BibleReader.Root>
309+
</div>
310+
),
311+
play: async ({ canvasElement }) => {
312+
await waitFor(
313+
async () => {
314+
const verseContainer = canvasElement.querySelector('[data-slot="yv-bible-renderer"]');
315+
await expect(verseContainer).toBeInTheDocument();
316+
},
317+
{ timeout: 5000 },
318+
);
319+
320+
const readerTheme = canvasElement.querySelector('[data-yv-theme="light"]');
321+
await expect(readerTheme).toBeInTheDocument();
322+
323+
await waitFor(
324+
async () => {
325+
const footnoteButton = canvasElement.querySelector('[data-verse-footnote] button');
326+
await expect(footnoteButton).toBeInTheDocument();
327+
},
328+
{ timeout: 5000 },
329+
);
330+
331+
const footnoteButton = canvasElement.querySelector('[data-verse-footnote] button');
332+
await expect(footnoteButton?.closest('[data-yv-theme="light"]')).toBeInTheDocument();
333+
334+
await userEvent.click(footnoteButton!);
335+
336+
await waitFor(async () => {
337+
const popover = document.querySelector('[data-slot="popover-content"]');
338+
await expect(popover).toBeInTheDocument();
339+
await expect(popover?.closest('[data-yv-theme="light"]')).toBeInTheDocument();
340+
});
341+
},
342+
};

packages/ui/src/components/bible-reader.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -151,14 +151,14 @@ function Root({
151151

152152
function Content() {
153153
const {
154+
background,
154155
book,
155156
chapter,
156157
versionId,
157158
currentFontSize,
158159
currentFontFamily,
159160
lineHeight,
160161
showVerseNumbers,
161-
background,
162162
} = useBibleReaderContext();
163163
const { books } = useBooks(versionId);
164164
const { version } = useVersion(versionId);
@@ -186,13 +186,13 @@ function Content() {
186186
</h1>
187187

188188
<BibleTextView
189-
theme={background}
190189
reference={usfmReference}
191190
versionId={versionId}
192191
fontFamily={currentFontFamily}
193192
fontSize={currentFontSize}
194193
lineHeight={lineHeight}
195194
showVerseNumbers={showVerseNumbers}
195+
theme={background}
196196
/>
197197

198198
{version?.copyright && (

packages/ui/src/components/bible-widget-view.stories.tsx

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,10 @@ export const WithVersionPicker: Story = {
9292
);
9393
});
9494

95-
await expect(screen.getByText(/luke 1:39-45 amp/i)).toBeVisible();
95+
await waitFor(async () => {
96+
const heading = screen.getByRole('heading', { level: 2, name: /luke 1:39-45/i });
97+
await expect(heading).toHaveTextContent(/amp/i);
98+
});
9699
},
97100
};
98101

packages/ui/src/components/verse.stories.tsx

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -261,3 +261,86 @@ export const DarkMode: Story = {
261261
</section>
262262
),
263263
};
264+
265+
export const FootnotePopoverThemeLight: Story = {
266+
args: {
267+
reference: 'JHN.1',
268+
versionId: 111,
269+
renderNotes: true,
270+
showVerseNumbers: true,
271+
theme: 'light',
272+
},
273+
tags: ['integration'],
274+
play: async ({ canvasElement }) => {
275+
await waitFor(
276+
async () => {
277+
const verseContainer = canvasElement.querySelector('[data-slot="yv-bible-renderer"]');
278+
await expect(verseContainer).toBeInTheDocument();
279+
},
280+
{ timeout: 5000 },
281+
);
282+
283+
await waitFor(
284+
async () => {
285+
const footnoteButton = canvasElement.querySelector('[data-verse-footnote] button');
286+
await expect(footnoteButton).toBeInTheDocument();
287+
},
288+
{ timeout: 5000 },
289+
);
290+
291+
const footnoteButton = canvasElement.querySelector('[data-verse-footnote] button');
292+
await expect(footnoteButton?.closest('[data-yv-theme="light"]')).toBeInTheDocument();
293+
294+
await userEvent.click(footnoteButton!);
295+
296+
await waitFor(async () => {
297+
const popover = document.querySelector('[data-slot="popover-content"]');
298+
await expect(popover).toBeInTheDocument();
299+
await expect(popover?.closest('[data-yv-theme="light"]')).toBeInTheDocument();
300+
});
301+
},
302+
};
303+
304+
export const FootnotePopoverThemeDark: Story = {
305+
args: {
306+
reference: 'JHN.1',
307+
versionId: 111,
308+
renderNotes: true,
309+
showVerseNumbers: true,
310+
theme: 'dark',
311+
},
312+
tags: ['integration'],
313+
render: (args) => (
314+
<div className="yv:dark">
315+
<BibleTextView {...args} />
316+
</div>
317+
),
318+
play: async ({ canvasElement }) => {
319+
await waitFor(
320+
async () => {
321+
const verseContainer = canvasElement.querySelector('[data-slot="yv-bible-renderer"]');
322+
await expect(verseContainer).toBeInTheDocument();
323+
},
324+
{ timeout: 5000 },
325+
);
326+
327+
await waitFor(
328+
async () => {
329+
const footnoteButton = canvasElement.querySelector('[data-verse-footnote] button');
330+
await expect(footnoteButton).toBeInTheDocument();
331+
},
332+
{ timeout: 5000 },
333+
);
334+
335+
const footnoteButton = canvasElement.querySelector('[data-verse-footnote] button');
336+
await expect(footnoteButton?.closest('[data-yv-theme="dark"]')).toBeInTheDocument();
337+
338+
await userEvent.click(footnoteButton!);
339+
340+
await waitFor(async () => {
341+
const popover = document.querySelector('[data-slot="popover-content"]');
342+
await expect(popover).toBeInTheDocument();
343+
await expect(popover?.closest('[data-yv-theme="dark"]')).toBeInTheDocument();
344+
});
345+
},
346+
};

packages/ui/src/components/verse.test.tsx

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
*/
44
import { describe, it, expect } from 'vitest';
55
import { render, waitFor } from '@testing-library/react';
6+
import userEvent from '@testing-library/user-event';
67
import { Verse } from './verse';
78

89
describe('Verse.Html - XSS Protection', () => {
@@ -220,6 +221,104 @@ describe('Verse.Html - XSS Protection', () => {
220221
});
221222
});
222223

224+
describe('Verse.Html - Footnotes', () => {
225+
it('should extract footnotes and create placeholders', async () => {
226+
const htmlWithFootnotes = `
227+
<div class="p">
228+
<span class="yv-v" v="5"></span><span class="yv-vlbl">5</span>The light shines in the darkness, and the
229+
darkness has not overcome<span class="yv-n f"><span class="fr">1:5 </span><span class="ft">Or </span><span class="fqa">understood</span></span>
230+
it.
231+
</div>
232+
`;
233+
234+
const { container } = render(<Verse.Html html={htmlWithFootnotes} renderNotes={true} />);
235+
236+
await waitFor(() => {
237+
const placeholder = container.querySelector('[data-verse-footnote="5"]');
238+
expect(placeholder).not.toBeNull();
239+
});
240+
});
241+
242+
it('should remove original footnote elements', async () => {
243+
const htmlWithFootnotes = `
244+
<div class="p">
245+
<span class="yv-v" v="5"></span><span class="yv-vlbl">5</span>The light shines<span class="yv-n f"><span class="fr">1:5 </span><span class="ft">Or understood</span></span> it.
246+
</div>
247+
`;
248+
249+
const { container } = render(<Verse.Html html={htmlWithFootnotes} renderNotes={true} />);
250+
251+
await waitFor(() => {
252+
const footnoteElements = container.querySelectorAll('.yv-n.f');
253+
expect(footnoteElements.length).toBe(0);
254+
});
255+
});
256+
257+
it('should place footnote at end of correct verse (v42 not v43)', async () => {
258+
const htmlWithFootnotes = `
259+
<div class="p">
260+
<span class="yv-v" v="42"></span><span class="yv-vlbl">42</span>And he brought him to Jesus.
261+
</div>
262+
<div class="p">
263+
Jesus looked at him and said,
264+
<span class="wj">"You are Simon son of John. You will be called Cephas"</span>
265+
(which, when translated, is Peter<span class="yv-n f"><span class="fr">1:42 </span><span class="fq">Cephas </span><span class="ft">(Aramaic) and </span><span class="fq">Peter </span><span class="ft">(Greek) both mean </span><span class="fqa">rock.</span></span>).
266+
</div>
267+
<div class="s1 yv-h">Jesus Calls Philip and Nathanael</div>
268+
<div class="p">
269+
<span class="yv-v" v="43"></span><span class="yv-vlbl">43</span>The next day Jesus decided to leave for Galilee.
270+
</div>
271+
`;
272+
273+
const { container } = render(<Verse.Html html={htmlWithFootnotes} renderNotes={true} />);
274+
275+
await waitFor(() => {
276+
const placeholder42 = container.querySelector('[data-verse-footnote="42"]');
277+
expect(placeholder42).not.toBeNull();
278+
279+
const verse43Marker = container.querySelector('.yv-v[v="43"]');
280+
expect(verse43Marker).not.toBeNull();
281+
282+
const position42 = placeholder42?.compareDocumentPosition(verse43Marker!);
283+
expect(position42! & Node.DOCUMENT_POSITION_FOLLOWING).toBeTruthy();
284+
});
285+
});
286+
287+
it('should handle multiple footnotes in a single verse', async () => {
288+
const htmlWithMultipleNotes = `
289+
<div class="p">
290+
<span class="yv-v" v="51"></span><span class="yv-vlbl">51</span>He then added,
291+
<span class="wj">"Very truly I tell you,</span><span class="yv-n f"><span class="fr">1:51 </span><span class="ft">The Greek is plural.</span></span>
292+
<span class="wj">you</span><span class="yv-n f"><span class="fr">1:51 </span><span class="ft">The Greek is plural.</span></span>
293+
<span class="wj">will see heaven open."</span>
294+
</div>
295+
`;
296+
297+
const { container } = render(<Verse.Html html={htmlWithMultipleNotes} renderNotes={true} />);
298+
299+
await waitFor(() => {
300+
const placeholder = container.querySelector('[data-verse-footnote="51"]');
301+
expect(placeholder).not.toBeNull();
302+
303+
const footnoteElements = container.querySelectorAll('.yv-n.f');
304+
expect(footnoteElements.length).toBe(0);
305+
});
306+
307+
const footnoteButton = container.querySelector('[data-verse-footnote="51"] button');
308+
expect(footnoteButton).not.toBeNull();
309+
310+
await userEvent.click(footnoteButton!);
311+
312+
await waitFor(() => {
313+
const popover = document.body.querySelector('[role="dialog"]');
314+
expect(popover).not.toBeNull();
315+
316+
const listItems = popover?.querySelectorAll('ul li');
317+
expect(listItems?.length).toBe(2);
318+
});
319+
});
320+
});
321+
223322
describe('Verse.Text', () => {
224323
it('should render verse with number and text (default size)', () => {
225324
const { container } = render(<Verse.Text number={1} text="In the beginning" />);

0 commit comments

Comments
 (0)