-
Notifications
You must be signed in to change notification settings - Fork 3
Auto-transform the Bible HTML from getPassage so the consumer doesn't have extra steps #216
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
1c57836
789c2a1
453b0d3
aece62a
20c1599
c1a61db
753776e
4494016
36e2adb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| --- | ||
| "@youversion/platform-core": minor | ||
| "@youversion/platform-react-hooks": minor | ||
| "@youversion/platform-react-ui": minor | ||
| --- | ||
|
|
||
| Auto-transform Bible HTML in `getPassage` — verse wrapping, footnote extraction, sanitization, and table fixes now happen automatically. Consumers no longer need to call `transformBibleHtml` manually. Uses native DOMParser in browser, dynamic `import('jsdom')` on server. `jsdom` is now declared as an optional peer dependency so install logs surface it for server consumers. Added `data-yv-transformed` idempotency marker so double-transforms are a no-op. Pass `transform: false` to receive raw, untransformed HTML (useful for simple display or when `jsdom` is unavailable). Bible reader CSS now handles verse label spacing for untransformed HTML automatically. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -308,7 +308,7 @@ describe('transformBibleHtml - sanitization', () => { | |
| const result = transformBibleHtml(html, createAdapters()); | ||
|
|
||
| expect(result.html).not.toContain('onclick'); | ||
| expect(result.html).toContain('<p>'); | ||
| expect(result.html).toContain('<p'); | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. note: if you're wondering why this tag is seemingly cut off, it's because the tags would contain a data attribute in this new PR, which would then make it where the tag is something like
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can this |
||
| expect(result.html).toContain('Click me'); | ||
| }); | ||
|
|
||
|
|
@@ -336,7 +336,7 @@ describe('transformBibleHtml - sanitization', () => { | |
| const result = transformBibleHtml(html, createAdapters()); | ||
|
|
||
| expect(result.html).not.toContain('style'); | ||
| expect(result.html).toContain('<div>'); | ||
| expect(result.html).toContain('<div'); | ||
| expect(result.html).toContain('text'); | ||
| }); | ||
|
|
||
|
|
@@ -387,6 +387,34 @@ describe('transformBibleHtml - sanitization', () => { | |
| }); | ||
| }); | ||
|
|
||
| describe('transformBibleHtml - idempotency', () => { | ||
| it('should add data-yv-transformed marker after transforming', () => { | ||
| const html = | ||
| '<div><div class="p"><span class="yv-v" v="1"></span><span class="yv-vlbl">1</span>Text.</div></div>'; | ||
| const result = transformBibleHtml(html, createAdapters()); | ||
|
|
||
| expect(result.html).toContain('data-yv-transformed'); | ||
| }); | ||
|
|
||
| it('should short-circuit when HTML is already transformed', () => { | ||
| const html = | ||
| '<div><div class="p"><span class="yv-v" v="1"></span><span class="yv-vlbl">1</span>Text.</div></div>'; | ||
| const first = transformBibleHtml(html, createAdapters()); | ||
| const second = transformBibleHtml(first.html, createAdapters()); | ||
|
|
||
| expect(second.html).toBe(first.html); | ||
| }); | ||
|
|
||
| it('should produce identical output when transformed twice (idempotent)', () => { | ||
| const html = | ||
| '<div><div class="p"><span class="yv-v" v="1"></span><span class="yv-vlbl">1</span>Verse text<span class="yv-n f"><span class="ft">A note</span></span>.</div></div>'; | ||
| const first = transformBibleHtml(html, createAdapters()); | ||
| const second = transformBibleHtml(first.html, createAdapters()); | ||
|
|
||
| expect(second.html).toBe(first.html); | ||
| }); | ||
| }); | ||
|
|
||
| describe('transformBibleHtmlForBrowser - DOMParser fallback', () => { | ||
| it('should throw when DOMParser is unavailable', () => { | ||
| const original = globalThis.DOMParser; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@types/jsdom@^28.0.1is 4 major versions ahead of the pinned runtime"jsdom": "24.0.0"devDependency. TypeScript validates the code against jsdom 28 type definitions while the actual test/dev environment runs jsdom 24. If any jsdom 28 API used here was added, removed, or changed between versions 24 and 28, compile-time checking would pass while the code silently breaks at runtime. The type package should match the pinned runtime version.Prompt To Fix With AI
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!