Repository navigation
Conversation
Supersedes #8230, rebased onto main. On iPhone and iPad the root Theme measures the reader's text size from one hidden, empty probe with font: -apple-system-body and gives the page that category's scales in an adopted stylesheet. Every theme writes its font sizes as its own values times the scale of the UIKit text style each plays, so only sizes move, and every scale is 1 at the default size and in every other browser. Changes from #8230's last head: root Themes in one document share the one probe and stylesheet, and the last one out removes them; the probe's mark moves to a private-attribute owner module; the header comments no longer cite records. Co-authored-by: Christopher Chedeau <vjeuxx@gmail.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
github-actions
Bot
added
the
needs:design-review
meta-cla
Bot
added
the
CLA Signed
PR Analysis Report
Generated by PR Enrichment workflow | View current CI run |
…be per document The spec owners' carve-out, verbatim, plus a verification row: one probe per document, none from a nested Theme, removed with the last root Theme, absent where font: -apple-system-body is unsupported.
|
AI review status for this pull request.
|
github-actions
Bot
added
the
needs:spec-owner-review
cixzhang
added
the
ci:run
|
/approve-spec 0d5da03 |
github-actions
Bot
removed
the
needs:spec-owner-review
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Changes requested. The approach matches spec:AST-102 FR3, and the INV7 carve-out has an engineering owner's approval for this head. Three things need fixing before approval:
packages/core/src/theme/dynamicType.ts:408-414creates its ownResizeObserverfor the probe; use Core's sharedobserveResize(utils/sharedResizeObserver.ts) instead, so text-size sync stays on the one resize-observation path (architecture:react-component-runtimeINV16).- The changeset says this works "On iPhone and iPad" and in "all" iPhone and iPad browsers, but the only device evidence is Safari on an iPhone simulator, so either add iPad Safari to the new run or narrow the wording.
- The only Safari-on-iOS run comes from the earlier pull request, recorded before the private
--_scaled-*caps and the shared probe, so please re-record Safari on iOS at this head (xs, default and an accessibility size, a branded theme, a live Settings change) and show that macOS Safari is unchanged, asspec:AST-102's browser evidence asks.
On this head, an emulated run in Chromium and WebKit matches the earlier simulator numbers and leaves the default size pixel-identical to main. Three small comment suggestions are inline.
[Automated review]
…he changeset claims only what is evidenced The probe's resize now goes through observeResize, which also takes the first reading. The changeset names Safari on iPhone, where the evidence was recorded, instead of every iPhone and iPad browser.
|
/approve-spec 71f6cd3 |
… and its open evidence gap
|
/approve-spec 36e968a |
github-actions
Bot
added
needs:spec-owner-review
There was a problem hiding this comment.
Changes requested. The shared observeResize and the narrowed changeset fix two of the three earlier findings, and the record edits have an engineering owner's approval for this head. Two things remain:
- Miss:
packages/cli/assets/docs/tree/type-scale.doc.mjs:155, the Dynamic Type story's description and the pull request title still say this works on iPhone and iPad and in every iOS browser, while the evidence and this pull request's ownspec:AST-102line cover only Safari on iPhone, so narrow them to Safari on iPhone as the changeset now does. spec:AST-102names Safari on iOS as the proof for FR3 and the only Safari run is of an earlier revision, so please record Safari on iPhone at this head (xs, default and an accessibility size, a branded theme, a live Settings change) and show that macOS Safari is unchanged.
An emulated run in Chromium on this head matches the earlier simulator numbers and leaves the default size pixel-identical to main. Two small non-blocking suggestions are inline.
[Automated review]
|
/approve-spec c664448 |
github-actions
Bot
added
needs:spec-owner-review
There was a problem hiding this comment.
Changes requested. The Dynamic Type mechanism still matches spec:AST-102 FR3, and the record edits have an engineering owner's approval for this head, but the new iOS-only gate brings three blocking items:
- The probe now also requires
-webkit-touch-callout, a feature the sync never uses, to tell iOS WebKit from macOS Safari (packages/core/src/theme/dynamicType.ts:388-390), andspec:AST-013FR6 allows such an engine workaround only for a reproduced defect with a regression fixture and a review or removal condition, so please either drop the check and the matching AST-102 line or record it with those three. - The AST-102 line saying Safari on iPhone was "recorded in the iOS simulator at the current implementation" (
spec.md:117-118) no longer holds, because that run predates the gate that decides whether iPhone mounts the probe, so please re-record Safari on iPhone on the head that keeps the gate or keep the line true to what was recorded. - The new AST-102 line that macOS Safari never mounts the probe (
spec.md:95-97) rests only on a stubbed unit case, so please add one macOS Safari run on the head that settles the gate.
An emulated run of this head in Chromium matches the earlier simulator numbers and leaves the default size pixel-identical to main. The earlier non-blocking suggestions still apply, and three comments now contradict the gate (see the inline notes).
[Automated review]
…gain The iOS-only touch-callout gate inferred the platform from a feature the sync never uses, so it goes, and so does the AST-102 line that rested on it. The probe and scale code match the recorded head again. Comments now name readerTextSizeEntries and the :root stylesheet, and the no-keyword Theme case checks that no stylesheet is adopted.
github-actions
Bot
added
the
needs:spec-owner-review
|
/approve-spec 7b10712 |
github-actions
Bot
removed
the
needs:spec-owner-review
There was a problem hiding this comment.
Changes requested. The Dynamic Type mechanism matches spec:AST-102 FR3, the record edits have an engineering owner's approval for this head, and the probe detects only the capability it uses. One blocking item remains:
- macOS Safari supports
font: -apple-system-body, so it mounts the probe too, and the claim that its fixed 13px Body leaves every theme's sizes unchanged (packages/core/src/theme/dynamicType.ts:25) rests only on a stubbed unit case, so please record one macOS Safari reading at the default text size (the probe's computedfont-size, and the story's body and supporting sizes) and, until it lands, keep macOS Safari in the AST-102 line's list of unverified browsers.
A non-blocking note on the length of the two new file headers is in the review.
[Automated review]
…namic-type # Conflicts: # packages/core/src/theme/Theme.test.tsx
|
/approve-spec c0e0b94 |
There was a problem hiding this comment.
Changes requested. The Dynamic Type mechanism matches spec:AST-102 FR3, and the record edits have an engineering owner's approval for this head. One blocking item remains:
- macOS Safari supports
font: -apple-system-body, so it mounts the probe too, and the claim that its fixed 13px Body leaves every theme's sizes unchanged (packages/core/src/theme/dynamicType.ts:25) rests only on a stubbed unit case, so please record one macOS Safari reading at the default text size (the probe's computedfont-size, and the story's body and supporting sizes) and, until it lands, add macOS Safari to the unverified browsers in thespec:AST-102line (docs/specs/AST-102-typography-theme-owned/spec.md:114-116).
The failing test-node (3/4) check comes from .github/scripts/ci-interaction-webkit.test.mjs, which fails the same way on main, so it is not caused by this pull request. A non-blocking note on the length of the two new file headers is in the review.
[Automated review]
…nverified until its reading
github-actions
Bot
added
the
needs:spec-owner-review
|
/approve-spec ab95939 |
github-actions
Bot
removed
the
needs:spec-owner-review
There was a problem hiding this comment.
Changes requested: the Dynamic Type mechanism matches spec:AST-102 FR3, and the record edits have an engineering owner's approval for this head. One blocking item remains:
- macOS Safari supports
font: -apple-system-body, so it mounts the probe too, and the claim that its fixed 13px Body leaves every theme's sizes unchanged (packages/core/src/theme/dynamicType.ts:25and:339) rests only on a stubbed unit case, so please record one macOS Safari reading at the default text size on this code (the probe's computedfont-size, and the Dynamic Type story's body and supporting sizes against the theme's own).
CI has not run on this head because the pull request is a draft. Two non-blocking suggestions on the length of the new file headers are inline.
[Automated review]
|
/approve-spec 0178ba3 |
There was a problem hiding this comment.
Changes requested: the Dynamic Type mechanism matches spec:AST-102 FR3, and the record edits have an engineering owner's approval for this head. One blocking item remains:
- macOS Safari supports
font: -apple-system-body, so it mounts the probe too, and the claim that its fixed 13px Body leaves every theme's sizes unchanged (packages/core/src/theme/dynamicType.ts:25and:339) rests only on a stubbed unit case, so please record one macOS Safari reading at the default text size on this code (the probe's computedfont-size, and the Dynamic Type story's body and supporting sizes against the theme's own).
CI has not run on this head because the pull request is a draft. Non-blocking: four new source comments run past the 25-word sentence limit for comments, which earlier reviews missed (suggestions inline), and the two file-header suggestions already on the pull request still apply.
[Automated review]
| /** | ||
| * @file dynamicType.ts | ||
| * @input The UIKit point sizes per text style (uikitTypography.ts), the raw | ||
| * font-size tokens (tokens.stylex.ts) | ||
| * @output The reader's text size categories, the category a measured Body | ||
| * size names, each raw font-size step's UIKit curve and scale, the rule that | ||
| * gives a category its scales, the scaled raw font-size declarations every | ||
| * theme emits and the semantic sizes it fixes, and the root Theme's text | ||
| * size sync | ||
| * @position Theme utility; read by generateThemeRules.ts (every theme's font | ||
| * sizes, as it writes them) and Theme.tsx (the root provider keeps the | ||
| * scales current) | ||
| * | ||
| * On iOS the reader picks a text size in Settings, and each UIKit text style | ||
| * follows it on its own curve. Mobile Safari shows the choice only through | ||
| * the `font: -apple-system-*` keywords, and the root font size stays 16px. | ||
| * So the root Theme runtime keeps one hidden, empty probe with | ||
| * `font: -apple-system-body` on `<body>`, reads the category from its size, | ||
| * and gives the page that category's scales in an adopted stylesheet. It | ||
| * reads again when the probe resizes. Each theme writes a font size as its | ||
| * own value times the scale of the text style it plays, so only sizes move. | ||
| * Every scale is 1 at the default category and in every other browser. | ||
| * macOS Safari knows the keyword, but its fixed 13px Body names no category. | ||
| * | ||
| * SYNC: When modified, update: | ||
| * - /packages/core/src/theme/dynamicType.test.ts | ||
| * - /packages/core/src/theme/generateThemeRules.ts (readerTextSizeEntries) | ||
| */ |
There was a problem hiding this comment.
Non-blocking: this header runs 28 lines, and its prose paragraph (lines 14-25) repeats what syncReaderTextSize's JSDoc (lines 335-349) says. architecture:knowledge-contracts INV29 keeps a file header to the @input, @output, @position and SYNC: tags, in about 12 lines, with no prose that repeats the main JSDoc. Keep the tags here and move anything the JSDoc does not already say into it.
| /** | ||
| * @file uikitTypography.ts | ||
| * @input Nothing; data only | ||
| * @output Exports `typography`: the point size of each UIKit text style at each | ||
| * content size category (Dynamic Type), from xSmall to AX5 | ||
| * @position Theme data: theme/dynamicType.ts scales a theme's own type scale | ||
| * on these curves when the reader changes their text size. | ||
| * | ||
| * Every row lists the twelve content size categories in order: xSmall, Small, | ||
| * Medium, Large (the default), xLarge, xxLarge, xxxLarge, then the five | ||
| * accessibility sizes AX1–AX5 that Larger Accessibility Sizes unlocks. A row | ||
| * is what `UIFont.preferredFont(forTextStyle:compatibleWith:)` returns for that | ||
| * style, and what Mobile Safari computes for the matching | ||
| * `font: -apple-system-*` keyword (`title0` is Large Title, `title1`–`title3` | ||
| * are Title 1–3). | ||
| * | ||
| * Only the styles a theme role scales on are listed (theme/dynamicType.ts maps | ||
| * each raw font-size step to one of them). | ||
| * | ||
| * Every row carries one or more `@source`: Apple's documentation, a WWDC | ||
| * session, WebKit's own source, or a measurement naming its subject, the iOS | ||
| * version and the date. dynamicType.test.ts enforces it. | ||
| * | ||
| * SYNC: When modified, update /packages/core/src/theme/dynamicType.ts | ||
| */ |
There was a problem hiding this comment.
Non-blocking: this header runs 25 lines; architecture:knowledge-contracts INV29 keeps a file header to its tags in about 12 lines. The column order and the source rule can move into typography's JSDoc, for example:
/**
* Point sizes per UIKit text style, one column per content size category,
* xSmall to AX5. Large is the default. Each row cites its `@source`.
*/
export const typography = {
Summary
People using Safari on iPhone who choose a text size in Settings (Display & Text Size, then Larger Text) get Astryx text at that size, in every theme. Native apps already follow that setting, but Astryx pages don't: a reader who needs 53 px body text still gets 16 px. At the default size, and in every other browser, nothing changes.
Supersedes #8230 (by vjeux), rebased onto current main with Co-authored-by. #8230 was closed only in a stale-PR triage, and nobody else is working on it.
What changes
theme/dynamicType.tsandtheme/uikitTypography.ts(from fix(core): text follows the reader's text size on iOS in every theme #8230): the UIKit point-size table per text style, the category a measured Body size names, and each raw font-size step's curve and scale.generateThemeRules.ts: every theme writes each raw font-size step as its own value times the scale of the UIKit text style that step plays. A semantic size a theme fixes at a length, such as Butter, Stone and Y2K's 12 px supporting text, scales on its default step's curve. Every scale is 1 unless the root sets it.Theme.tsx: the rootThemekeeps the scales current throughsyncReaderTextSize. Nested Themes don't.observeResize), not a ResizeObserver of its own;privateAttributes/textSizeProbe.stylex.ts);[fix]changeset for core and cli.The probe element (INV7)
The spec owners ruled the probe allowed, as a consistency point. This PR adds their carve-out to
architecture:component-style-authoringINV7, word for word: "The theme runtime may own one shared, empty, aria-hidden probe element per document, outside every component's DOM, used only to read a platform text size; it renders no content, is not a theming target, and is removed with the last root Theme."It also adds a verification row: one probe per document; none from a nested Theme; removed with the last root Theme; absent where
font: -apple-system-bodyis unsupported. Each is backed bydynamicType.test.tsandTheme.test.tsx.The probe itself:
<span aria-hidden="true">, empty and 1em square, withvisibility: hidden,position: fixedandpointer-events: none. It carriesfont: -apple-system-bodyand renders no text.Themes. It mounts on<body>, outside any component's DOM.font: -apple-system-body.Evidence
At l, no text-size scale applies. The phone values are larger than desktop Chromium's 14 / 12 because each theme has its own touch type sizes.
-apple-system-bodyto a fixed 13 px Body that names no category, so every scale stays 1 and body and supporting text stay at the theme's sizes. ThedynamicType.test.tscase "scales nothing where the keyword follows no reader setting (a fixed 13px Body, as on a Mac)" covers the logic; the reading itself is an owner check that is still pending.dynamicType.test.tsandTheme.test.tsxcover this.import {Theme}grew by about 1.6 kB gzip of JavaScript, with no CSS.Tests
dynamicType.test.ts: the category table and curves, the gate, the scales, follow-on-resize, cleanup, and a new case where two root Themes share one probe and the last one out removes it.tsc -p packages/core. Strict eslint is clean.Writing rubric results
spec:AST-102Current-state impact: the Dynamic Type line states the shipped, recorded state and what stays unverified (iPad Safari, other iOS browsers, macOS Safari at its default text size until its reading lands), in the present tense and with no references to pull requests.R1 narration: none; the INV7 sentence is in the present tense and was supplied verbatim by the spec owners.
R2 research in the record: none; the simulator numbers stay in this description.
R3 evidence naming a change: none.
R4 rejected alternatives: none added.
R5 pull request or issue references remaining: none.