diff --git a/.changeset/fast-places-juggle.md b/.changeset/fast-places-juggle.md new file mode 100644 index 0000000..3446273 --- /dev/null +++ b/.changeset/fast-places-juggle.md @@ -0,0 +1,7 @@ +--- +'@youversion/platform-react-ui': minor +'@youversion/platform-core': minor +'@youversion/platform-react-hooks': minor +--- + +Refactor verse HTML transformation to support verse-level highlighting. Extract HTML processing logic to `verse-html-utils.ts` with new `wrapVerseContent()` function that wraps verse content in CSS-targetable `` elements. Simplify footnote extraction using wrapped verse structure. Remove CSS rule preventing text wrapping. Add comprehensive test coverage for verse wrapping behavior. diff --git a/.gitignore b/.gitignore index 24df55d..f52839a 100644 --- a/.gitignore +++ b/.gitignore @@ -71,3 +71,5 @@ storybook-static !.yarn/sdks !.yarn/versions .yarn/install-state.gz + +.opencode/plans diff --git a/packages/ui/src/components/verse.test.tsx b/packages/ui/src/components/verse.test.tsx index ffc191b..c2746b7 100644 --- a/packages/ui/src/components/verse.test.tsx +++ b/packages/ui/src/components/verse.test.tsx @@ -319,6 +319,125 @@ describe('Verse.Html - Footnotes', () => { }); }); +describe('Verse.Html - Verse Wrapping', () => { + it('should wrap verse content in yv-v elements', async () => { + const html = ` +
+ 1In the beginning was the Word. +
+ `; + + const { container } = render(); + + await waitFor(() => { + const verseWrapper = container.querySelector('.yv-v[v="1"]'); + expect(verseWrapper).not.toBeNull(); + expect(verseWrapper?.textContent).toContain('In the beginning was the Word'); + }); + }); + + it('should wrap multiple verses in same paragraph', async () => { + const html = ` +
+ 1First verse. + 2Second verse. +
+ `; + + const { container } = render(); + + await waitFor(() => { + const verse1 = container.querySelector('.yv-v[v="1"]'); + const verse2 = container.querySelector('.yv-v[v="2"]'); + + expect(verse1).not.toBeNull(); + expect(verse2).not.toBeNull(); + expect(verse1?.textContent).toContain('First verse'); + expect(verse2?.textContent).toContain('Second verse'); + }); + }); + + it('should duplicate yv-v wrapper when verse spans multiple paragraphs', async () => { + const html = ` +
+ 39"Come," he replied. +
+
So they went and saw where he was staying.
+
+ 40Andrew was one of the two. +
+ `; + + const { container } = render(); + + await waitFor(() => { + const verse39Wrappers = container.querySelectorAll('.yv-v[v="39"]'); + expect(verse39Wrappers.length).toBe(2); + + expect(verse39Wrappers[0]?.textContent).toContain('Come'); + expect(verse39Wrappers[1]?.textContent).toContain('So they went'); + }); + }); + + it('should enable CSS selection of individual verses', async () => { + const html = ` +
+ 1First verse text. + 2Second verse text. +
+ `; + + const { container } = render(); + + await waitFor(() => { + const verse1 = container.querySelector('.yv-v[v="1"]'); + const verse2 = container.querySelector('.yv-v[v="2"]'); + + expect(verse1).not.toBeNull(); + expect(verse2).not.toBeNull(); + expect(verse1).not.toBe(verse2); + }); + }); + + it('should preserve verse label inside wrapper', async () => { + const html = ` +
+ 5The light shines. +
+ `; + + const { container } = render(); + + await waitFor(() => { + const verseWrapper = container.querySelector('.yv-v[v="5"]'); + const label = verseWrapper?.querySelector('.yv-vlbl'); + + expect(label).not.toBeNull(); + expect(label?.textContent).toContain('5'); + }); + }); + + it('should not wrap header elements in verse spans', async () => { + const html = ` +
+ 42And he brought him to Jesus. +
+
Jesus Calls Philip
+
+ 43The next day Jesus decided to leave. +
+ `; + + const { container } = render(); + + await waitFor(() => { + const header = container.querySelector('.yv-h'); + expect(header).not.toBeNull(); + expect(header?.closest('.yv-v')).toBeNull(); + }); + }); +}); + describe('Verse.Text', () => { it('should render verse with number and text (default size)', () => { const { container } = render(); diff --git a/packages/ui/src/components/verse.tsx b/packages/ui/src/components/verse.tsx index 4b33257..a77f52c 100644 --- a/packages/ui/src/components/verse.tsx +++ b/packages/ui/src/components/verse.tsx @@ -1,160 +1,32 @@ 'use client'; -import { Popover, PopoverContent, PopoverTrigger } from '@/components/ui/popover'; import { usePassage, useTheme } from '@youversion/platform-react-hooks'; import DOMPurify from 'isomorphic-dompurify'; import { forwardRef, memo, + type ReactNode, useEffect, useLayoutEffect, useRef, useState, - type ReactNode, } from 'react'; import { createPortal } from 'react-dom'; +import { Popover, PopoverContent, PopoverTrigger } from '@/components/ui/popover'; +import { + extractNotesFromWrappedHtml, + LETTERS, + NON_BREAKING_SPACE, + type VerseNotes, + wrapVerseContent, +} from '@/lib/verse-html-utils'; import { Footnote } from './icons/footnote'; -const NON_BREAKING_SPACE = '\u00A0'; - -const LETTERS = 'abcdefghijklmnopqrstuvwxyz'; - -type VerseNotes = { - verseHtml: string; - notes: string[]; -}; - type ExtractedNotes = { html: string; notes: Record; }; -/** - * Checks if a node should be excluded from verse text reconstruction. - * Excludes: verse markers (.yv-v), verse labels (.yv-vlbl), headers (.yv-h), and footnotes (.yv-n). - */ -function isExcludedNode(node: Node): boolean { - if (!(node instanceof Element)) return false; - if (node.classList.contains('yv-v') || node.classList.contains('yv-vlbl')) return true; - if (node.classList.contains('yv-h') || node.closest('.yv-h')) return true; - if (node.classList.contains('yv-n') || node.closest('.yv-n')) return true; - return false; -} - -/** - * Extracts footnotes from Bible HTML and prepares data for footnote popovers. - * - * This function does three things: - * 1. Identifies verse boundaries using `.yv-v[v]` markers (verses can span multiple paragraphs) - * 2. For each verse with footnotes, builds a plain-text version with A/B/C markers for the popover - * 3. Inserts placeholder spans at the end of each verse (where the footnote icon will render) - * - * The challenge: verses don't respect paragraph boundaries. A verse starts at `.yv-v[v="X"]` - * and ends at the next `.yv-v[v]` marker, potentially spanning multiple `
` elements. - * We use a TreeWalker to flatten the DOM into document order, then use index ranges to define verses. - * - * @returns Modified HTML with footnotes removed and placeholders inserted, plus notes data for popovers - */ -function extractNotesFromHtml(html: string): ExtractedNotes { - if (typeof window === 'undefined') return { html, notes: {} }; - - const doc = new DOMParser().parseFromString( - DOMPurify.sanitize(html, DOMPURIFY_CONFIG), - 'text/html', - ); - const verseMarkers = Array.from(doc.querySelectorAll('.yv-v[v]')); - if (!verseMarkers.length) return { html: doc.body.innerHTML, notes: {} }; - - // Flatten DOM into document order so we can define verse boundaries by index ranges - const walker = doc.createTreeWalker(doc.body, NodeFilter.SHOW_ELEMENT | NodeFilter.SHOW_TEXT); - const allNodes: Node[] = []; - do { - allNodes.push(walker.currentNode); - } while (walker.nextNode()); - - const nodeIndex = new Map(allNodes.map((n, i) => [n, i])); - const footnotes = doc.querySelectorAll('.yv-n.f'); - - // Define verse boundaries: each verse spans from its marker to the next marker (or end of content) - const verses = verseMarkers.map((marker, i) => { - const nextMarker = verseMarkers[i + 1]; - return { - num: marker.getAttribute('v') || '0', - start: nodeIndex.get(marker) ?? 0, - end: nextMarker ? (nodeIndex.get(nextMarker) ?? allNodes.length) : allNodes.length, - fns: [] as Element[], - }; - }); - - // Assign each footnote to its containing verse (find verse whose range contains the footnote) - footnotes.forEach((fn) => { - const idx = nodeIndex.get(fn); - if (idx !== undefined) { - const verse = [...verses].reverse().find((v) => idx > v.start); - if (verse) verse.fns.push(fn); - } - }); - - const withNotes = verses.filter((v) => v.fns.length > 0); - - const notes: Record = {}; - withNotes.forEach((verse) => { - // Build plain-text verse content for popover, replacing footnotes with A/B/C markers - let text = ''; - let noteIdx = 0; - let lastP: Element | null = null; - - for (let i = verse.start; i < verse.end; i++) { - const node = allNodes[i]; - if (!node) continue; - const parent = node.parentNode as Element | null; - - if (node instanceof Element) { - if (node.classList.contains('yv-h') || node.closest('.yv-h')) continue; - if (node.classList.contains('yv-n') && node.classList.contains('f')) { - text += `${LETTERS[noteIdx++] || noteIdx}`; - } - } else if (node.nodeType === Node.TEXT_NODE && parent) { - if (parent.closest('.yv-h') || parent.closest('.yv-n.f')) continue; - if (parent.classList.contains('yv-v') || parent.classList.contains('yv-vlbl')) continue; - // Add space when transitioning between paragraphs (verses can span multiple

elements) - const curP = parent.closest('.p, p, div.p'); - if (lastP && curP && lastP !== curP) text += ' '; - text += node.textContent || ''; - if (curP) lastP = curP; - } - } - - notes[verse.num] = { verseHtml: text, notes: verse.fns.map((fn) => fn.innerHTML) }; - - // Insert placeholder at end of verse content (walk backwards to find last text node) - for (let i = verse.end - 1; i > verse.start; i--) { - const node = allNodes[i]; - if (!node) continue; - const parent = node.parentNode as Element | null; - if ( - node.nodeType === Node.TEXT_NODE && - node.textContent?.trim() && - parent && - !isExcludedNode(parent) && - !parent.closest('.yv-n') && - !parent.closest('.yv-h') - ) { - const placeholder = doc.createElement('span'); - placeholder.setAttribute('data-verse-footnote', verse.num); - parent.insertBefore(placeholder, node.nextSibling); - break; - } - } - }); - - footnotes.forEach((fn) => { - fn.remove(); - }); - - return { html: doc.body.innerHTML, notes }; -} - const VerseFootnoteButton = memo(function VerseFootnoteButton({ verseNum, verseNotes, @@ -272,28 +144,25 @@ function yvDomTransformer(html: string, extractNotes: boolean = false): Extracte return { html, notes: {} }; } - let extractedNotes: Record = {}; - let processedHtml = html; + // Parse and sanitize HTML + const doc = new DOMParser().parseFromString( + DOMPurify.sanitize(html, DOMPURIFY_CONFIG), + 'text/html', + ); - if (extractNotes) { - const result = extractNotesFromHtml(html); - processedHtml = result.html; - extractedNotes = result.notes; - } else { - processedHtml = DOMPurify.sanitize(html, DOMPURIFY_CONFIG); - } + // Wrap verse content FIRST (enables simple footnote extraction) + wrapVerseContent(doc); - // Safely parse and modify HTML to add spaces to paragraph elements - const parser = new DOMParser(); - const doc = parser.parseFromString(processedHtml, 'text/html'); + // Extract footnotes using the wrapped verse structure + const extractedNotes = extractNotes ? extractNotesFromWrappedHtml(doc) : {}; // Adds non-breaking space to the end of verse labels for better copying and pasting // (i.e. "3For God so loved..." to "3 For God so loved...") - const paragraphs = doc.querySelectorAll('.yv-vlbl'); - paragraphs.forEach((p) => { - const text = p.textContent || ''; + const verseLabels = doc.querySelectorAll('.yv-vlbl'); + verseLabels.forEach((label) => { + const text = label.textContent || ''; if (!text.endsWith(NON_BREAKING_SPACE)) { - p.textContent = text + NON_BREAKING_SPACE; + label.textContent = text + NON_BREAKING_SPACE; } }); diff --git a/packages/ui/src/lib/verse-html-utils.ts b/packages/ui/src/lib/verse-html-utils.ts new file mode 100644 index 0000000..be52643 --- /dev/null +++ b/packages/ui/src/lib/verse-html-utils.ts @@ -0,0 +1,237 @@ +export const NON_BREAKING_SPACE = '\u00A0'; + +export const LETTERS = 'abcdefghijklmnopqrstuvwxyz'; + +export type VerseNotes = { + verseHtml: string; + notes: string[]; +}; + +/** + * Wraps verse content in `yv-v` elements for easier CSS targeting. + * + * Transforms empty verse markers into wrapping containers. When a verse spans + * multiple paragraphs, creates duplicate wrappers in each paragraph (Bible.com pattern). + * + * Before: 1Text... + * After: 1Text... + * + * This enables simple CSS selectors like `.yv-v[v="1"] { background: yellow; }` + */ +export function wrapVerseContent(doc: Document): void { + /** + * Wraps all content in a paragraph with a verse span. + */ + function wrapParagraphContent(doc: Document, paragraph: Element, verseNum: string): void { + const children = Array.from(paragraph.childNodes); + if (children.length === 0) return; + + const wrapper = doc.createElement('span'); + wrapper.className = 'yv-v'; + wrapper.setAttribute('v', verseNum); + + const firstChild = children[0]; + if (firstChild) { + paragraph.insertBefore(wrapper, firstChild); + } + children.forEach((child) => { + wrapper.appendChild(child); + }); + } + + /** + * Wraps paragraphs between startParagraph and an optional endParagraph boundary. + * If no endParagraph is provided, wraps until a verse marker is found or siblings are exhausted. + */ + function wrapParagraphsUntilBoundary( + doc: Document, + verseNum: string, + startParagraph: Element | null, + endParagraph?: Element | null, + ): void { + if (!startParagraph) return; + + let currentP: Element | null = startParagraph.nextElementSibling; + + while (currentP && currentP !== endParagraph) { + // Skip heading elements - these are structural, not verse content + // See iOS implementation: https://github.com/youversion/platform-sdk-swift/blob/main/Sources/YouVersionPlatformUI/Views/Rendering/BibleVersionRendering.swift + const isHeading = + currentP.classList.contains('yv-h') || + currentP.matches('.s1, .s2, .s3, .s4, .ms, .ms1, .ms2, .ms3, .ms4, .mr, .sp, .sr, .qa, .r'); + if (isHeading) { + currentP = currentP.nextElementSibling; + continue; + } + + if (currentP.querySelector('.yv-v[v]')) break; + + if ( + currentP.classList.contains('p') || + currentP.tagName === 'P' + ) { + wrapParagraphContent(doc, currentP, verseNum); + } + + currentP = currentP.nextElementSibling; + } + } + + function handleParagraphWrapping( + doc: Document, + currentParagraph: Element | null, + nextParagraph: Element | null, + verseNum: string, + ): void { + if (!currentParagraph) return; + + if (!nextParagraph) { + wrapParagraphsUntilBoundary(doc, verseNum, currentParagraph); + return; + } + + if (currentParagraph !== nextParagraph) { + wrapParagraphsUntilBoundary(doc, verseNum, currentParagraph, nextParagraph); + } + } + + function processVerseMarker(marker: Element, index: number, markers: Element[]): void { + const verseNum = marker.getAttribute('v'); + if (!verseNum) return; + + const nextMarker = markers[index + 1]; + + const nodesToWrap = collectNodesBetweenMarkers(marker, nextMarker); + if (nodesToWrap.length === 0) return; + + const currentParagraph = marker.closest('.p, p, div.p'); + const nextParagraph = nextMarker?.closest('.p, p, div.p') || null; + const doc = marker.ownerDocument; + + wrapNodesInVerse(marker, verseNum, nodesToWrap); + handleParagraphWrapping(doc, currentParagraph, nextParagraph, verseNum); + } + + function wrapNodesInVerse(marker: Element, verseNum: string, nodes: Node[]): void { + const wrapper = marker.ownerDocument.createElement('span'); + wrapper.className = 'yv-v'; + wrapper.setAttribute('v', verseNum); + + const firstNode = nodes[0]; + if (firstNode) { + marker.parentNode?.insertBefore(wrapper, firstNode); + } + + nodes.forEach((node) => { + wrapper.appendChild(node); + }); + marker.remove(); + } + + function shouldStopCollecting(node: Node, endMarker: Element | undefined): boolean { + if (node === endMarker) return true; + if (endMarker && node instanceof Element && node.contains(endMarker)) return true; + return false; + } + + function shouldSkipNode(node: Node): boolean { + return node instanceof Element && node.classList.contains('yv-h'); + } + + function collectNodesBetweenMarkers(startMarker: Element, endMarker: Element | undefined): Node[] { + const nodes: Node[] = []; + let current: Node | null = startMarker.nextSibling; + + while (current && !shouldStopCollecting(current, endMarker)) { + if (shouldSkipNode(current)) { + current = current.nextSibling; + continue; + } + nodes.push(current); + current = current.nextSibling; + } + + return nodes; + } + + const verseMarkers = Array.from(doc.querySelectorAll('.yv-v[v]')); + verseMarkers.forEach(processVerseMarker); +} + + +/** + * Extracts footnotes from wrapped verse HTML and prepares data for footnote popovers. + * + * This function assumes verses are already wrapped in `.yv-v[v]` elements (by wrapVerseContent). + * It uses `.closest('.yv-v[v]')` to find which verse each footnote belongs to. + * + * @returns Notes data for popovers, keyed by verse number + */ +export function extractNotesFromWrappedHtml(doc: Document): Record { + const footnotes = Array.from(doc.querySelectorAll('.yv-n.f')); + if (!footnotes.length) return {}; + + // Group footnotes by verse number using closest wrapper + const footnotesByVerse = new Map(); + footnotes.forEach((fn) => { + const verseNum = fn.closest('.yv-v[v]')?.getAttribute('v'); + if (verseNum) { + let arr = footnotesByVerse.get(verseNum); + if (!arr) { + arr = []; + footnotesByVerse.set(verseNum, arr); + } + arr.push(fn); + } + }); + + const notes: Record = {}; + + footnotesByVerse.forEach((fns, verseNum) => { + // Find all wrappers for this verse (could be multiple if verse spans paragraphs) + const verseWrappers = Array.from(doc.querySelectorAll(`.yv-v[v="${verseNum}"]`)); + + // Build verse HTML with A/B/C markers for popover display + let verseHtml = ''; + let noteIdx = 0; + + verseWrappers.forEach((wrapper, wrapperIdx) => { + if (wrapperIdx > 0) verseHtml += ' '; + + const walker = doc.createTreeWalker(wrapper, NodeFilter.SHOW_ELEMENT | NodeFilter.SHOW_TEXT); + while (walker.nextNode()) { + const node = walker.currentNode; + if (node instanceof Element) { + if (node.classList.contains('yv-n') && node.classList.contains('f')) { + verseHtml += `${LETTERS[noteIdx++] || noteIdx}`; + } + } else if (node.nodeType === Node.TEXT_NODE) { + const parent = node.parentElement; + if (parent?.closest('.yv-n.f') || parent?.closest('.yv-h')) continue; + if (parent?.classList.contains('yv-vlbl')) continue; + verseHtml += node.textContent || ''; + } + } + }); + + notes[verseNum] = { + verseHtml, + notes: fns.map((fn) => fn.innerHTML), + }; + + // Insert placeholder at end of last verse wrapper + const lastWrapper = verseWrappers[verseWrappers.length - 1]; + if (lastWrapper) { + const placeholder = doc.createElement('span'); + placeholder.setAttribute('data-verse-footnote', verseNum); + lastWrapper.appendChild(placeholder); + } + }); + + // Remove all footnotes from DOM + footnotes.forEach((fn) => { + fn.remove(); + }); + + return notes; +} diff --git a/packages/ui/src/styles/bible-reader.css b/packages/ui/src/styles/bible-reader.css index 8fa4ef4..d7862a9 100644 --- a/packages/ui/src/styles/bible-reader.css +++ b/packages/ui/src/styles/bible-reader.css @@ -100,7 +100,6 @@ /* Wrap verse + label + content together to prevent breaking */ & .yv-v, & .verse { - white-space: nowrap; display: inline; }