Precursor to YPE-641: transform html for highlights

[cameronapak]

If you want to understand the hardship as to why this change was needed, check out this Notion doc.

Play Video Overview

[changeset-bot]

🦋 Changeset detected

Latest commit: dc3f52d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages

Name	Type
@youversion/platform-react-ui	Minor
@youversion/platform-core	Minor
@youversion/platform-react-hooks	Minor
nextjs	Patch

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

[chatgpt-codex-connector]

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

[greptile-apps]

Greptile Summary

Refactored Bible verse HTML transformation to wrap verse content in yv-v elements, enabling simpler CSS targeting for highlights. The key changes:

• Verse wrapping: Transformed empty <span class="yv-v" v="1"></span> markers into containers that wrap their verse content: <span class="yv-v" v="1">...content...</span>
• Multi-paragraph support: When verses span multiple paragraphs, the PR creates duplicate yv-v wrappers in each paragraph (matching Bible.com's pattern)
• Simplified footnote extraction: Replaced complex TreeWalker-based footnote logic with simpler .closest('.yv-v[v]') approach that leverages the new wrapping structure
• CSS fix: Removed white-space: nowrap from .yv-v elements to allow proper text wrapping
• Test coverage: Added 6 comprehensive tests covering single verses, multi-paragraph verses, and header exclusion

This is a precursor to YPE-641 (highlights feature), establishing the DOM structure needed for CSS-based verse highlighting like .yv-v[v="1"] { background: yellow; }.

Confidence Score: 4/5

• This PR is safe to merge with minimal risk - well-tested refactoring with comprehensive test coverage
• The refactoring is well-designed and thoroughly tested with 6 new tests. The transformation logic handles edge cases (multi-paragraph verses, headers). One minor concern: the recursive nature of wrapRemainingParagraphs and wrapIntermediateParagraphs could theoretically cause issues with deeply nested or malformed HTML, but DOMPurify sanitization mitigates this risk.
• No files require special attention - all changes are well-tested and properly implemented

Important Files Changed

Filename	Overview
.gitignore	Added .opencode/plans directory to .gitignore - standard housekeeping change
packages/ui/src/styles/bible-reader.css	Removed white-space: nowrap from .yv-v to allow verse content to wrap properly
packages/ui/src/components/verse.test.tsx	Added comprehensive test suite covering verse wrapping functionality, multi-paragraph verses, and header exclusion
packages/ui/src/components/verse.tsx	Refactored HTML transformation to wrap verse content in yv-v elements, simplifying footnote extraction logic

Sequence Diagram

sequenceDiagram
    participant User
    participant Verse.Html
    participant yvDomTransformer
    participant wrapVerseContent
    participant extractNotesFromWrappedHtml
    participant HtmlWithNotes

    User->>Verse.Html: Render HTML with verse markers
    Verse.Html->>yvDomTransformer: Parse and transform HTML
    yvDomTransformer->>DOMParser: Parse HTML string
    yvDomTransformer->>DOMPurify: Sanitize HTML
    yvDomTransformer->>wrapVerseContent: Wrap verses in yv-v elements
    
    Note over wrapVerseContent: Find all .yv-v[v] markers
    wrapVerseContent->>wrapVerseContent: Transform empty markers into wrappers
    wrapVerseContent->>wrapVerseContent: Handle multi-paragraph verses
    wrapVerseContent->>wrapVerseContent: Skip headers (.yv-h)
    
    yvDomTransformer->>extractNotesFromWrappedHtml: Extract footnotes (if enabled)
    
    Note over extractNotesFromWrappedHtml: Use .closest('.yv-v[v]') to find verse
    extractNotesFromWrappedHtml->>extractNotesFromWrappedHtml: Group footnotes by verse
    extractNotesFromWrappedHtml->>extractNotesFromWrappedHtml: Build verse HTML with A/B/C markers
    extractNotesFromWrappedHtml->>extractNotesFromWrappedHtml: Insert placeholders
    extractNotesFromWrappedHtml->>extractNotesFromWrappedHtml: Remove footnote elements
    
    yvDomTransformer->>yvDomTransformer: Add non-breaking space to verse labels
    yvDomTransformer->>yvDomTransformer: Fix irregular tables
    yvDomTransformer-->>Verse.Html: Return transformed HTML + notes
    
    Verse.Html->>HtmlWithNotes: Render with footnote buttons
    HtmlWithNotes->>HtmlWithNotes: Find placeholders in DOM
    HtmlWithNotes->>HtmlWithNotes: Portal VerseFootnoteButton to placeholders
    HtmlWithNotes-->>User: Display verse with interactive footnotes

Loading

[cameronapak]

@greptileai, what improvements would you suggest?

[greptile-apps]

Looking at this PR that transforms HTML for highlights by wrapping verse content, I can see several areas for improvement:

Performance & Efficiency

1. DOM Traversal Optimization: The current implementation does multiple passes through the DOM. You could combine wrapVerseContent and extractNotesFromWrappedHtml into a single traversal for better performance.

2. TreeWalker Reuse: In extractNotesFromWrappedHtml, you're creating a new TreeWalker for each verse wrapper. Consider reusing a single walker or using more efficient DOM traversal methods.

Code Quality & Maintainability

3. Extract Constants: Magic selectors like .p, p, div.p and .s1, .s2, .s3, .s4, .ms appear multiple times. Extract these to constants:

const PARAGRAPH_SELECTORS = '.p, p, div.p';
const HEADER_SELECTORS = '.s1, .s2, .s3, .s4, .ms';
const VERSE_MARKER_SELECTOR = '.yv-v[v]';

4. Simplify Paragraph Detection: Create a helper function instead of repeating the same logic:

function isParagraphElement(element: Element): boolean {
  return element.classList.contains('p') || 
         element.tagName === 'P' || 
         (element.tagName === 'DIV' && element.classList.contains('p'));
}

5. Error Boundary: Add null checks and error handling for edge cases where DOM structure might be unexpected.

Test Coverage Improvements

6. Edge Case Testing: Your tests are good, but consider adding:
   • Empty verse content handling
   • Malformed HTML scenarios
   • Verses with only headers (no text content)
   • Performance tests with large HTML documents

Algorithm Clarity

7. Document Processing Flow: The current approach processes verses sequentially, but the paragraph-spanning logic is complex. Consider a different approach:

   • First pass: identify all verse boundaries
   • Second pass: wrap content within those boundaries
   • Third pass: handle footnotes

8. Memory Efficiency: The nodesToWrap array could be large for verses with lots of content. Consider processing nodes as you iterate rather than collecting them all first.

CSS Integration

9. Style Coordination: Since this change affects how highlights work, ensure the CSS changes in bible-reader.css (removing white-space: nowrap) are sufficient. You might need additional styles for the new wrapping structure.

Would you like me to elaborate on any of these suggestions or help implement specific improvements?

[cameronapak]

Alright... My buddy Amp Code said,

Looking at this code, I'll be honest: these optimizations are probably not needed for your use case.

Why the current approach is fine:

1. Small DOM size - Bible passages are typically 1-30 verses. Even a full chapter is ~50 verses with maybe 200 DOM nodes total. Modern browsers handle this in microseconds.
2. Runs once per render - yvDomTransformer runs in useEffect when html changes. Users aren't rapidly changing passages.
3. TreeWalker is already fast - Creating a TreeWalker per verse (typically <30 walkers) is negligible overhead.
4. Premature optimization - Combining wrapVerseContent and extractNotesFromWrappedHtml would make the code significantly harder to understand and maintain for marginal gains.

When you'd actually want to optimize:

Rendering 100+ passages simultaneously
User is rapidly switching passages (carousel/swipe)
Measurable performance issues in profiler

Brenden and I were processing overhead of TreeWalker for each verse, so I asked my buddy OpenCode and it said:

Analysis: 176 verses is still TRIVIAL for modern browsers, including low-end devices
Evidence from research:

1. TreeWalker Performance Benchmarks

• morphdom benchmark: 3.8ms for 100 iterations = 38 microseconds per TreeWalker
• StackOverflow: TreeWalker is fastest native DOM traversal API (faster than querySelectorAll, NodeIterator)

2. Low-End Device Calculations
   Using 4-6x CPU throttling (standard proxy for mobile/low-end devices):
   176 TreeWalkers × 38μs = ~6.7ms (high-end device)
   6.7ms × 4 (slow device) = ~27ms
   6.7ms × 6 (very slow device) = ~40ms
   40ms is imperceptible to users and happens once per chapter load.
3. Key Advantages of Current Approach

• Small traversal scope: Each walker only visits its verse wrapper (~5-20 nodes), not entire document
• Native API: TreeWalker is browser-optimized C++ code, not JavaScript
• Creation overhead: Microseconds per instance, not milliseconds

4. Context: Bible App Reality

• Psalm 119 (176 verses) is the extreme outlier - 99% of chapters < 50 verses
• Typical chapter (30 verses, 2 paragraphs each) = ~60 TreeWalkers = ~2-3ms even on low-end
• This is a one-time initialization cost per chapter render
• Alternatives would be slower: Single walker = traverse entire DOM (1000+ nodes), manual traversal = 19% slower per benchmark

5. Real-World Low-End Data
   From Nolan Lawson's research:

• Android 2GB RAM: Tested for real low-end scenarios
• Optimizations saved 847ms on heavy operations
• TreeWalker overhead is not in the optimization category - it's already optimal
  Conclusion
  The code review comment is invalid. Creating TreeWalker per verse is:
• ✅ Fastest approach per benchmarks
• ✅ Negligible overhead even on low-end devices (< 50ms total)
• ✅ Correct architectural choice (small traversal scope per verse)
• ✅ One-time cost (not repeated during interaction)
  No optimization needed. The current implementation is already using the most efficient DOM traversal method available.

[bmanquen]

Great work! This is going to be such an upgrade from the previous version and help us interact with the DOM much easier, this is amazing!

Just a few comments to work through.

[cameronapak]

@davidfedor @bbelyeu here is the approach I've taken to prep the HTML returned from YVDOM so that highlights can be easily styled.

If you want to provide feedback, it's important that you have read through this Notion doc.

Here's a video overview of the update working

[davidfedor]

Wonderful readability improvements, Cam!