feat(ui): swap the serif font from Source Serif 4 to Untitled Serif (YPE-1350, YPE-1910) - #304
Open
cameronapak wants to merge 8 commits into
Conversation
🦋 Changeset detectedLatest commit: efc2e24 The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
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 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
cameronapak
commented
Jul 28, 2026
cameronapak
changed the title
cameronapak
commented
Jul 29, 2026
cameronapak
added a commit
that referenced
this pull request
Jul 29, 2026
Review feedback on PR #304. Aktiv Grotesk was never published — it landed in a changeset and was reverted before release — so naming it in the ADRs and style comments documents a decision about a font we do not use and never shipped. Untitled Serif is cleared, so anything implying otherwise is wrong too. - ADR-0001: retitle without "pending licensing", drop the Aktiv Grotesk bullet and the licensing narrative. What remains is the decision the code actually depends on: a hardcoded @font-face is the wrong delivery pattern for an SDK that renders inside third-party apps, so brand fonts load from /v1/fonts/{font_id}/stylesheet. Reduced from two re-introduction conditions to that one. - ADR-0003: drop the Aktiv Grotesk mentions and the "licensing is cleared" paragraph; state the sans stack is simply unchanged. - theme.css / global.css / font-tokens.test.ts: same, in the comments. - bible-chapter-picker.test.tsx: two test names still said "Aktiv" while asserting only size and weight. Renamed to what they check. Filename kept as 0001-revert-brand-fonts-pending-licensing.md so the three published CHANGELOG links keep resolving; those changelogs are release history and are left as-is. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
cameronapak
changed the title
cameronapak
requested review from
Dustin-Kelley,
camrun91 and
jaredhightower-youversion
July 29, 2026 15:24
cameronapak
added a commit
that referenced
this pull request
Jul 30, 2026
Review feedback on PR #304. Aktiv Grotesk was never published — it landed in a changeset and was reverted before release — so naming it in the ADRs and style comments documents a decision about a font we do not use and never shipped. Untitled Serif is cleared, so anything implying otherwise is wrong too. - ADR-0001: retitle without "pending licensing", drop the Aktiv Grotesk bullet and the licensing narrative. What remains is the decision the code actually depends on: a hardcoded @font-face is the wrong delivery pattern for an SDK that renders inside third-party apps, so brand fonts load from /v1/fonts/{font_id}/stylesheet. Reduced from two re-introduction conditions to that one. - ADR-0003: drop the Aktiv Grotesk mentions and the "licensing is cleared" paragraph; state the sans stack is simply unchanged. - theme.css / global.css / font-tokens.test.ts: same, in the comments. - bible-chapter-picker.test.tsx: two test names still said "Aktiv" while asserting only size and weight. Renamed to what they check. Filename kept as 0001-revert-brand-fonts-pending-licensing.md so the three published CHANGELOG links keep resolving; those changelogs are release history and are left as-is. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
cameronapak
force-pushed
the
ype-1350-react-sdk-biblereader-use-correct-serif-font-swap-source-serif-for-untitled
branch
from
July 30, 2026 15:27
c7e3ebb to
b24b9a1
Compare
…-1910) Serif stack becomes 'Untitled Serif', 'Source Serif 4', serif in both declarations — core's --yv-font-serif and ui's @theme inline --font-serif. They are literal duplicates, not aliases, so font-tokens.test.ts now guards them against drift (the first test to assert a font token's value). Delivery is a new <YvFonts /> rendered from YouVersionProvider: a React 19 hoisted <link> to /v1/fonts/1/stylesheet, the gated endpoint ADR-0001 named as the required consumption pattern. This is the SDK's first runtime-value- dependent stylesheet — the URL needs the app key, which __YV_STYLES__ cannot know at build time. It renders only in the normal branch; no key, no font. Source Serif 4 stays loaded as the fallback, so nothing regresses when the request is blocked or the host has no key. Reader body text still renders Source Serif 4: Verse.Html sets --yv-reader-font-family inline from the reader's own state, which shadows the token. Phase 2 changes that constant. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…eference (YPE-1350) The reader pins its own font: Verse.Html sets --yv-reader-font-family inline from component state, which shadows the CSS token. So the Phase 1 stack change could not reach passage body text until the constant behind that inline value changed. This is that change. - UNTITLED_SERIF_FONT is the new default; SOURCE_SERIF_FONT stays exported as @deprecated because the hydrate-time migration still needs to recognize it. - Returning readers who chose serif before this shipped stored the old stack. Without mapping it forward they would hydrate to a value matching neither picker button — no button active, and Source Serif 4 forever. The migration is deliberately narrow rather than full validation: FontFamily is an open type on purpose, so a host passing defaultFontFamily="Georgia" must still round-trip. A test covers that non-legacy values pass through untouched. - bible-card.tsx had the same inline pin hardcoded; swapped too. YPE-1910 names the Bible card explicitly. - Locale files trade sourceSerifFontName for untitledSerifFontName ("Untitled", per the ticket wording). They are SDK-private, so removal is safe. Verified in Chrome: body text renders Untitled Serif, picker shows "Untitled" active, a seeded legacy value migrates and re-persists, and Inter survives a reload unmigrated. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…350) ADR-0001 is Accepted and names two conditions for re-introducing brand fonts. Shipping the font without touching it would leave the repo's own documentation contradicting its code. ADR-0003 records both conditions as met, and is deliberately blunt about what the access control actually is: gated discovery plus revocable CDN URLs, not file-level authentication. ADR-0001's finding that the woff2 is publicly downloadable is unchanged and still true — what changed is the reading of the licence constraint, which is YouVersion's to interpret. Saying so keeps the two ADRs consistent; the alternative was a quiet contradiction. The sign-off is verbal, from Klim directly, relayed and confirmed by Cameron Pak on 2026-07-28. The ADR names it as verbal rather than implying a paper trail exists, and says to amend if written confirmation surfaces. Aktiv Grotesk stays reverted; ADR-0001 is superseded in part only. Also: the README's CSP section was stale — it still listed storage.googleapis.com for Aktiv Grotesk, which no longer ships. Replaced with the hosts actually used now. That section is the answer for strict-CSP consumers, since there is deliberately no opt-out prop. Changeset is minor across all three packages (core's CSS changes too) and calls out the new outbound request, the CSP entries, and the default font change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…e claim This repo is public and both ADRs are published with it. Strip the licensing narrative from ADR-0001 and ADR-0003 down to what the code needs to know: which face is cleared, and that it must be served by the Fonts API rather than shipped in the repo. Licensing specifics are tracked internally. Also drop ADR-0003's CDN probe table, which published an unauthenticated font URL and a compliance argument that don't belong in a public repo. Code changes: - yv-fonts.tsx / yv-fonts.test.tsx: the comments asserted React "only suspends for stylesheets inserted during a transition". React's docs say the component rendering a `precedence` stylesheet suspends while it loads, without scoping that to transitions. The test proves the synchronous path only, so say that and flag the transition path as uncovered. - yv-fonts.test.tsx: the missing-key test goes through the provider's MissingAppKey guard, so YvFonts' own guard was never executed. Cover it directly for undefined / empty / whitespace keys, and restore the console spy it left installed. - README: drop the `font-display: swap` claim. That property comes from a stylesheet the Fonts API authors, not this SDK, so we can't promise it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Review feedback on PR #304. Aktiv Grotesk was never published — it landed in a changeset and was reverted before release — so naming it in the ADRs and style comments documents a decision about a font we do not use and never shipped. Untitled Serif is cleared, so anything implying otherwise is wrong too. - ADR-0001: retitle without "pending licensing", drop the Aktiv Grotesk bullet and the licensing narrative. What remains is the decision the code actually depends on: a hardcoded @font-face is the wrong delivery pattern for an SDK that renders inside third-party apps, so brand fonts load from /v1/fonts/{font_id}/stylesheet. Reduced from two re-introduction conditions to that one. - ADR-0003: drop the Aktiv Grotesk mentions and the "licensing is cleared" paragraph; state the sans stack is simply unchanged. - theme.css / global.css / font-tokens.test.ts: same, in the comments. - bible-chapter-picker.test.tsx: two test names still said "Aktiv" while asserting only size and weight. Renamed to what they check. Filename kept as 0001-revert-brand-fonts-pending-licensing.md so the three published CHANGELOG links keep resolving; those changelogs are release history and are left as-is. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ADR 0003 was taken by the locale-ownership-gate ADR merged in #308. Renames the file and heading, updates cross-references in ADR-0001, the changeset, theme.css, global.css, and yv-fonts.tsx. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
cameronapak
force-pushed
the
ype-1350-react-sdk-biblereader-use-correct-serif-font-swap-source-serif-for-untitled
branch
from
July 30, 2026 15:43
b24b9a1 to
a374c35
Compare
Collaborator
Author
|
Ready for review @jaredhightower-youversion or @camrun91 P.S. Dustin and I are out of office the rest of this week due to being over hours and will resume on Monday |
…erif-font-swap-source-serif-for-untitled
Collaborator
Review notes — worth fixing before mergeTwo items from review that I'd like addressed before this lands: [High] Suspending stylesheet not containedFiles:
Combined with no opt-out, every consumer inherits first-nav latency to Suggested fix: wrap the font link in a local boundary inside the provider: <Suspense fallback={null}>
<YvFonts … />
</Suspense>That keeps suspension scoped to the provider instead of bubbling up to the app. [Medium] Changeset is stale vs the branchFile: The changeset text doesn't match what's actually in the branch:
Those lines ship into published CHANGELOGs, so please update the changeset (and the ADR wording to match) before merge. |
camrun91
requested changes
Jul 31, 2026
camrun91
left a comment
camrun91
left a comment
Collaborator
There was a problem hiding this comment.
See above request of changes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Warning
YPE-1350 | YPE-1910
What problems was I solving
The SDK's serif face was Source Serif 4, a Google Fonts stand-in adopted when ADR-0001 pulled the hardcoded brand
@font-faceblocks out of the SDK. The BibleReader has therefore never rendered in YouVersion's brand serif.ADR-0001's finding was about delivery, not the typeface: a hardcoded
@font-facepointing at a CDN woff2 puts the file in front of third-party developer apps with nothing in the SDK able to gate it. So it set one condition for bringing a brand font back — load it via/v1/fonts/{font_id}/stylesheet, which requires an app key. Untitled Serif now does exactly that.Shipped, this means: the BibleReader and every other serif surface render in Untitled Serif, delivered from the Fonts API using the app key consumers already supply — no font file in the repo, no new prop, no setup. A consumer who blocks the new hosts falls back to Source Serif 4 with no layout break, so the downside case is exactly today's behavior.
The sans stack is untouched and stays
'Inter', sans-serif. This is a serif-only change.What user-facing changes did I ship
--yv-font-serifbecomes'Untitled Serif', 'Source Serif 4', serif, so every serif surface follows at once: BibleReader body text, the Bible card,BibleText, the version-picker abbreviation tile, footnotes, chapter headings, and thelgVerse of the Day card (this is YPE-1910's scope, done in the same pass)<link rel="stylesheet">tohttps://api.youversion.com/v1/fonts/1/stylesheet, plus woff2 fetches fromcdn.youversion.com. No new prop and no opt-outhttps://api.youversion.cominstyle-srcandhttps://cdn.youversion.cominfont-src. This also removes a stalestorage.googleapis.comentry for a host the SDK no longer callsHow I implemented it
Five commits. The walkthrough artifact has the same journey with inline diffs.
1 · Load the font, move the tokens (
7607cb5)https://{apiHost}/v1/fonts/1/stylesheet?app_key={key}and renders<link rel="stylesheet" precedence="yv-sdk-fonts">. This is the SDK's first runtime-value-dependent stylesheet: the URL needs the app key, which is only known at render time, so it can't live in the build-time-frozen__YV_STYLES__blob. React 19 hoists the link into<head>and dedupes byhref, so multiple providers still yield one link. Returnsnullon a missing/whitespace key — no key, nothing to 401MissingAppKeyguard, so only the has-a-key branch requests it@theme inlinevalues are inlined into utilities rather than emitted as runtime custom properties. Both got the same literal, both got comments saying so@import-ed. Nothing else catches drift; jsdom never loads either sheetapiHost, missing key, dedupe across two roots. The notable assertion is that children still commit: React can suspend a commit on<link precedence>until the sheet loads and jsdom never firesload, which would have meant mounting the provider could blank a consumer's tree2 · Move the reader (
03f6aca)The token swap alone can't reach reader body text.
bible-reader.tsxwrites--yv-reader-font-familyinline from its own state, andbible-reader.cssapplies it to every descendant — since that inline property is always defined, it wins over--yv-font-serif. So the reader needed its own changes:UNTITLED_SERIF_FONT;SOURCE_SERIF_FONTis@deprecatedbut kept, because the migration needs the literal to compare againstsavedFontFamily === SOURCE_SERIF_FONT ? UNTITLED_SERIF_FONT : savedFontFamily. Deliberately not validation —FontFamilyis(string & {})on purpose, so a host-supplieddefaultFontFamily="Georgia"must still round-trip. Without the migration, a reader who'd already picked serif would reload to a picker with neither button activefontFamilyexplicitly rather than inheriting the token, so it needed the same swapLoadsSavedPreferencesFromLocalStoragestill seeds the legacy value, then asserts the migrated result on the CSS custom property, in localStorage, and on the active picker button3 · Record the decision (
65c8815)4 · Review passes (
f6a53a2,c7e3ebb)2ba9e6dadded it and0d184fcreverted it, both landing in2.2.0, so the released package never carried it. Anything implying Untitled Serif is not cleared came out too. Two bible-chapter-picker.test.tsx test names still referenced it while asserting only size and weight — renamed to what they checkyv-fonts.tsx/yv-fonts.test.tsxasserted React "only suspends for stylesheets inserted during a transition." React's docs say the component rendering aprecedencestylesheet suspends while it loads, with no transition caveat. The test proves the synchronous path only, so it now says that and flags the transition path as uncovered — a latency risk on a transition-driven mount, not a hang, since failures settle tooYvFonts' own guard. The missing-key test routes through the provider'sMissingAppKeybranch, soYvFonts'!appKey?.trim()guard never executed. Now tested directly for undefined / empty / whitespace, and the leakedconsole.errorspy is restoredfont-display: swapclaim. That property comes from a stylesheet the Fonts API authors, not this SDKVerified against the endpoint directly:
GET /v1/fonts/1/stylesheetreturns401 Failed to resolve API Keywith no key and401 Invalid ApiKeywith?app_key=bogus— so the query-param form the SDK builds is accepted by the gateway.Deviations from the plan
Plan: 04-structure-outline-untitled-serif-swap.md
Implemented as planned
<YvFonts />matches the plan's snippet exactly — hardcoded font id 1,apiHostdefault,precedence="yv-sdk-fonts",nullon empty keyDeviations/surprises
--yv-reader-font-familypin was found. The code follows the corrected plan. Not an implementation deviation — a plan self-correction--yv-reader-font-family. Reaching that property requires rendering<BibleReader.Content />, which callsusePassageand throwsYouVersion context not found— that test file mocks the hooks package but renders no provider. The custom-property assertion lives in the story instead. Pre-flagged in the planAdditions not in plan
font-tokens.test.tsalso guards the sans token pair for drift, not just serif — same pattern, no reason to leave the other half unguardedc7e3ebbdocs cleanup was not planned work; it came out of reviewItems planned but not implemented
None.
Open item — locale ownership
This is the one red check.
.github/scripts/check-locale-ownership.shfails any PR touchingpackages/ui/src/i18n/locales/**unless it's aplatform-localizationsync PR. This PR renamessourceSerifFontName→untitledSerifFontNamein all three.It's a chicken-and-egg, and both halves were reproduced locally: revert the locale files and
i18n Checkfails instead, becausebible-reader.tsxcallst('untitledSerifFontName')and the key wouldn't exist — that check hard-fails on at()reference missing fromen.json.The only clean path is the documented one in
docs/i18n-guidelines.md:react.untitledSerifFontNameupstream inplatform-localization(sources/common/en.json)distribute-react.ymlopens achore/localization-sync-react-*PR hereUntil that lands, this PR can't go green either way. Removing
react.sourceSerifFontNameupstream should be a follow-up after this merges, not part of step 1 — removing it whilemainstill callst('sourceSerifFontName')would turni18n Checkred onmain.Not changed on purpose
packages/{ui,core,hooks}/CHANGELOG.mdstill carry the original 2.2.0 entries, including the font naming this PR cleaned out of the ADRs. Rewriting shipped release notes falsifies the record…-pending-licensing.mdfilename, even though the title no longer says that. Three published CHANGELOG entries link to that path and renaming would break them. Happy to rename if you'd rather take the dead linksHow to verify it
git fetch origin ype-1350-react-sdk-biblereader-use-correct-serif-font-swap-source-serif-for-untitled git checkout ype-1350-react-sdk-biblereader-use-correct-serif-font-swap-source-serif-for-untitled pnpm install && pnpm buildManual Testing
pnpm dev:webwith a real app key → BibleReader body text renders Untitled Serif (not Source Serif 4). DevTools → Network shows one request toapi.youversion.com/v1/fonts/1/stylesheetand woff2 fetches fromcdn.youversion.comlocalStorage.setItem('youversion-platform:reader:font-family', '"Source Serif 4", serif')→ after reload the value is the Untitled stack and the picker still shows serif activedefaultFontFamily="Georgia", pick it, reload → still Georgia (migration must not clobber host values)api.youversion.com(DevTools request blocking or a strict CSP) → serif text falls back to Source Serif 4 with no layout breakBibleText, footnotes, chapter headings, and thelgVOTD card all render Untitled Serif (YPE-1910 scope)Automated Tests
Description for the changelog
Swap the SDK's serif face from Source Serif 4 to Untitled Serif, YouVersion's brand serif, loaded from the YouVersion Fonts API — strict-CSP consumers must allowlist
api.youversion.com(style-src) andcdn.youversion.com(font-src).Greptile Summary
The PR adopts Untitled Serif as the SDK-wide serif face.
Confidence Score: 5/5
The PR appears safe to merge.
No blocking failure remains; the previously reported locale-ownership issue was resolved by removing locale files from this PR after the synchronized English key landed.
Important Files Changed
Sequence Diagram
Reviews (6): Last reviewed commit: "Merge branch 'main' into ype-1350-react-..." | Re-trigger Greptile