Repository navigation
perf(Layer): read an entering layer's box without writing styles - #10240
Conversation
settledBox overrode the layer's transforms with none !important, read the box and restored them, every time it read a layer during its entrance. In Chromium that is 8 style writes and a forced style recalculation and layout twice per read, inside the frame that reads it. An entrance only translates and scales the layer, so the box now comes from the one transformed read: its layout size is the transformed size over the scale, and its position follows from the transform origin. The derived edges are moved onto the 1/64 px layout grid when they are within 1/1024 px of it, so the box equals the layout box exactly. A rotated or skewed layer keeps the override read, and a layer that transitions a transform keeps its read-only path. untransform stays, rewritten without DOMMatrix.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
PR Analysis Report🔎 Preview ·
|
| Component | Size | LoC | Visual · A11y · RTL |
|---|---|---|---|
| LayerProvider | 4.1 kB (0) · 20 B CSS (0) | — | ✓ · ✓ · ✗ |
| PowerSearch | 277.5 kB (+677 B) · 23.2 kB CSS (0) | — | — · — · — |
| DateTimeInput | 200.1 kB (+670 B) · 18.2 kB CSS (0) | — | — · — · — |
112 more components
| Component | Size | LoC | Visual · A11y · RTL |
|---|---|---|---|
| charts/ChartTooltip | 158.7 kB (+664 B) · 423 B CSS (0) | — | — · — · — |
| lab/CommentThread | 303.5 kB (+662 B) · 2.4 kB CSS (0) | — | — · — · — |
| DateRangeInput | 87.7 kB (+661 B) · 12.2 kB CSS (0) | — | — · — · — |
| charts/Chart | 181.3 kB (+659 B) · 494 B CSS (0) | — | — · — · — |
| CommandPalette | 67.8 kB (+658 B) · 12.4 kB CSS (0) | — | — · — · — |
| Calendar | 63.7 kB (+653 B) · 10.0 kB CSS (0) | — | — · — · — |
| TopNavMegaMenu | 62.1 kB (+653 B) · 11.3 kB CSS (0) | — | — · — · — |
| SideNavItem | 62.7 kB (+652 B) · 10.5 kB CSS (0) | — | — · — · — |
| HoverCard | 25.3 kB (+648 B) · 4.3 kB CSS (0) | — | — · — · — |
| SideNavCollapseButton | 53.6 kB (+648 B) · 9.5 kB CSS (0) | — | — · — · — |
| TimeInput | 154.5 kB (+647 B) · 15.6 kB CSS (0) | — | — · — · — |
| ToggleButton | 53.6 kB (+647 B) · 9.5 kB CSS (0) | — | — · — · — |
| lab/TransferListSelector | 99.0 kB (+647 B) · 1.9 kB CSS (0) | — | — · — · — |
| richtext/RichTextView | 272.2 kB (+647 B) · 2.3 kB CSS (0) | — | — · — · — |
| BaseTypeahead | 63.0 kB (+646 B) · 10.4 kB CSS (0) | — | — · — · — |
| Button | 52.7 kB (+645 B) · 9.4 kB CSS (0) | — | — · — · — |
| DateInput | 177.8 kB (+645 B) · 16.5 kB CSS (0) | — | — · — · — |
| richtext/RichTextEditor | 292.0 kB (+645 B) · 2.6 kB CSS (0) | — | — · — · — |
| MobileNav | 62.9 kB (+644 B) · 10.1 kB CSS (0) | — | — · — · — |
| StatusDot | 28.5 kB (+644 B) · 5.3 kB CSS (0) | — | — · — · — |
| ChatLayoutScrollButton | 53.2 kB (+643 B) · 9.6 kB CSS (0) | — | — · — · — |
| Banner | 54.8 kB (+641 B) · 9.9 kB CSS (0) | — | — · — · — |
| TableSelectionToolbar | 62.3 kB (+641 B) · 12.5 kB CSS (0) | — | — · — · — |
| lab/ListInput | 97.3 kB (+641 B) · 1.6 kB CSS (0) | — | — · — · — |
| ChatLayout | 56.0 kB (+640 B) · 9.9 kB CSS (0) | — | — · — · — |
| ChatComposerInput | 67.4 kB (+639 B) · 10.7 kB CSS (0) | — | — · — · — |
| ChatComposerTokenElement | 67.4 kB (+639 B) · 10.7 kB CSS (0) | — | — · — · — |
| DialogHeader | 55.5 kB (+638 B) · 10.9 kB CSS (0) | — | — · — · — |
| MultiSelector | 90.1 kB (+638 B) · 16.4 kB CSS (0) | — | — · — · — |
| Tokenizer | 86.8 kB (+638 B) · 18.2 kB CSS (0) | — | — · — · — |
| lab/EmojiPicker | 82.3 kB (+638 B) · 751 B CSS (0) | — | — · — · — |
| lab/SelectableCardGroup | 60.6 kB (+638 B) · 162 B CSS (0) | — | — · — · — |
| ListItem | 65.8 kB (+637 B) · 11.0 kB CSS (0) | — | — · — · — |
| SegmentedControl | 90.5 kB (+637 B) · 6.8 kB CSS (0) | — | — · — · — |
| MobileNavToggle | 53.1 kB (+636 B) · 9.4 kB CSS (0) | — | — · — · — |
| TextInput | 63.8 kB (+636 B) · 10.9 kB CSS (0) | — | — · — · — |
| AvatarGroup | 43.1 kB (+635 B) · 5.6 kB CSS (0) | — | — · — · — |
| ChatComposer | 69.3 kB (+635 B) · 11.3 kB CSS (0) | — | — · — · — |
| Markdown | 163.9 kB (+635 B) · 19.6 kB CSS (0) | — | — · — · — |
| NumberInput | 67.8 kB (+635 B) · 11.2 kB CSS (0) | — | — · — · — |
| Pagination | 105.0 kB (+635 B) · 17.3 kB CSS (0) | — | — · — · — |
| AppShell | 82.7 kB (+634 B) · 12.9 kB CSS (0) | — | — · — · — |
| Lightbox | 104.0 kB (+634 B) · 9.9 kB CSS (0) | — | — · — · — |
| ChatSendButton | 53.2 kB (+633 B) · 9.4 kB CSS (0) | — | — · — · — |
| IconButton | 52.7 kB (+633 B) · 9.4 kB CSS (0) | — | — · — · — |
| TabMenu | 64.1 kB (+633 B) · 10.5 kB CSS (0) | — | — · — · — |
| lab/DiffViewer | 79.1 kB (+633 B) · 2.1 kB CSS (0) | — | — · — · — |
| Selector | 94.8 kB (+632 B) · 17.0 kB CSS (0) | — | — · — · — |
| SideNav | 82.4 kB (+632 B) · 10.8 kB CSS (0) | — | — · — · — |
| AvatarGroupOverflow | 45.4 kB (+631 B) · 5.5 kB CSS (0) | — | — · — · — |
| ChatDictationButton | 53.3 kB (+631 B) · 9.5 kB CSS (0) | — | — · — · — |
| DrawerHeader | 55.7 kB (+631 B) · 12.3 kB CSS (0) | — | — · — · — |
| Typeahead | 75.4 kB (+631 B) · 12.2 kB CSS (0) | — | — · — · — |
| CodeBlock | 67.5 kB (+630 B) · 10.7 kB CSS (0) | — | — · — · — |
| ComplexSelector | 66.8 kB (+630 B) · 11.5 kB CSS (0) | — | — · — · — |
| Stepper | 54.4 kB (+630 B) · 9.6 kB CSS (0) | — | — · — · — |
| AlertDialog | 96.9 kB (+629 B) · 13.8 kB CSS (0) | — | — · — · — |
| Avatar | 40.4 kB (+629 B) · 5.5 kB CSS (0) | — | — · — · — |
| InputGroup | 53.0 kB (+629 B) · 7.8 kB CSS (0) | — | — · — · — |
| RadioListItem | 63.6 kB (+629 B) · 9.8 kB CSS (0) | — | — · — · — |
| Thumbnail | 57.0 kB (+628 B) · 11.0 kB CSS (0) | — | — · — · — |
| Link | 49.7 kB (+627 B) · 7.0 kB CSS (0) | — | — · — · — |
| Popover | 80.6 kB (+627 B) · 13.5 kB CSS (0) | — | — · — · — |
| TopNavMenu | 66.9 kB (+627 B) · 10.6 kB CSS (0) | — | — · — · — |
| CheckboxList | 55.2 kB (+626 B) · 8.0 kB CSS (0) | — | — · — · — |
| InfoTip | 44.2 kB (+625 B) · 5.3 kB CSS (0) | — | — · — · — |
| lab/ChatReactionBar | 99.7 kB (+625 B) · 1.1 kB CSS (0) | — | — · — · — |
| RadioList | 52.8 kB (+623 B) · 7.2 kB CSS (0) | — | — · — · — |
| DropdownMenuSubMenu | 65.6 kB (+622 B) · 9.4 kB CSS (0) | — | — · — · — |
| BreadcrumbItem | 77.0 kB (+621 B) · 11.6 kB CSS (0) | — | — · — · — |
| Toast | 90.3 kB (+621 B) · 9.7 kB CSS (0) | — | — · — · — |
| CheckboxListItem | 66.8 kB (+619 B) · 11.0 kB CSS (0) | — | — · — · — |
| richtext/RichTextEditorToolbar | 246.0 kB (+619 B) · 454 B CSS (0) | — | — · — · — |
| Tooltip · Storybook | 26.8 kB (+618 B) · 4.3 kB CSS (0) | 820 (0) | ✓ · ✓ · ✓ |
| TopNavHeading | 67.2 kB (+618 B) · 10.5 kB CSS (0) | — | — · — · — |
| lab/CommentPin | 53.6 kB (+618 B) · 1.2 kB CSS (0) | — | — · — · — |
| Field | 51.4 kB (+617 B) · 7.2 kB CSS (0) | — | — · — · — |
| SideNavHeading | 67.8 kB (+614 B) · 10.7 kB CSS (0) | — | — · — · — |
| FileInput | 65.2 kB (+613 B) · 10.7 kB CSS (0) | — | — · — · — |
| FieldLabel | 48.0 kB (+611 B) · 5.6 kB CSS (0) | — | — · — · — |
| MoreMenu | 88.6 kB (+611 B) · 16.8 kB CSS (0) | — | — · — · — |
| lab/ChatEmojiPicker | 98.5 kB (+611 B) · 773 B CSS (0) | — | — · — · — |
| lab/TourStep | 86.8 kB (+611 B) · 590 B CSS (0) | — | — · — · — |
| lab/TransferList | 92.0 kB (+611 B) · 1.7 kB CSS (0) | — | — · — · — |
| TabList | 41.1 kB (+610 B) · 6.4 kB CSS (0) | — | — · — · — |
| TextArea | 58.8 kB (+609 B) · 9.2 kB CSS (0) | — | — · — · — |
| Table | 101.5 kB (+608 B) · 17.3 kB CSS (0) | — | — · — · — |
| TopNav | 64.5 kB (+608 B) · 10.3 kB CSS (0) | — | — · — · — |
| Slider | 102.7 kB (+607 B) · 8.7 kB CSS (0) | — | — · — · — |
| Switch | 97.3 kB (+607 B) · 9.9 kB CSS (0) | — | — · — · — |
| CheckboxInput | 60.7 kB (+604 B) · 9.1 kB CSS (0) | — | — · — · — |
| DropdownMenu | 88.3 kB (+604 B) · 16.8 kB CSS (0) | — | — · — · — |
| lab/CommentComposer | 139.8 kB (+601 B) · 1.9 kB CSS (0) | — | — · — · — |
| lab/Comments | 308.6 kB (+595 B) · 2.5 kB CSS (0) | — | — · — · — |
| Carousel | 61.5 kB (+594 B) · 10.4 kB CSS (0) | — | — · — · — |
| Toolbar | 40.4 kB (+593 B) · 7.9 kB CSS (0) | — | — · — · — |
| TableCell | 93.2 kB (+586 B) · 17.1 kB CSS (0) | — | — · — · — |
| lab/Schedule | 100.7 kB (+586 B) · 3.9 kB CSS (0) | — | — · — · — |
| TableHeaderCell | 92.8 kB (+583 B) · 16.8 kB CSS (0) | — | — · — · — |
| ContextMenu | 92.0 kB (+574 B) · 16.7 kB CSS (0) | — | — · — · — |
| charts/ChartLegend | 35.8 kB (+3 B) · 224 B CSS (0) | — | — · — · — |
| Timestamp | 23.2 kB (+2 B) · 9.7 kB CSS (0) | — | — · — · — |
| ChatMessageList | 23.9 kB (+1 B) · 7.0 kB CSS (0) | — | — · — · — |
| ChatToolCalls | 27.8 kB (+1 B) · 8.3 kB CSS (0) | — | — · — · — |
| CommandPaletteInput | 27.9 kB (+1 B) · 7.5 kB CSS (0) | — | — · — · — |
| ProgressBar | 11.5 kB (+1 B) · 6.2 kB CSS (0) | — | — · — · — |
| Heading | 8.2 kB (0) · 5.4 kB CSS (0) | — | — · — · — |
| Spinner | 20.5 kB (0) · 6.1 kB CSS (0) | — | — · — · — |
| Text | 7.6 kB (0) · 5.4 kB CSS (0) | — | — · — · — |
| Timer | 9.7 kB (0) · 5.9 kB CSS (0) | — | — · — · — |
| layer | — | — | — · ✓ · — |
| Layer | — | — | ✓ · ✓ · — |
Package size: @astryxdesign/core 6.83 MB packed (+3.3 kB); no new dependencies.
API changes: none in this pull request
Committed API report behind this build: cli 19 rows, core 24 rows; run pnpm api:report.
Touched components checked; add ci:run-visual or ci:run-rtl to run the full suites.
Accessibility: 1 violation(s)
Accessibility Audit
Status: 1 accessibility violation(s) found — 1 serious.
Tooltip - 1 issue(s)
- 🟠 serious: Ensure elements that have scrollable content are accessible by keyboard in Safari
- Rule:
scrollable-region-focusable· Affects 1/22 stories · Learn more - WCAG: 2.1.1, 2.1.3 (Level A)
- Rule:
Generated by PR Enrichment workflow | View full report
There was a problem hiding this comment.
Changes requested. Reads on running entrance frames now return the identical box with no style writes, but three cases read the wrong box where the old override was exact.
- In Chromium the read made in the call that shows a layer sees its entrance animation still pending, when
getComputedStylealready reports the first keyframe butgetBoundingClientRectdoes not include it yet, sosettledBoxundoes a transform the box does not carry and reads it 7–9 px further from the trigger; a Tooltip opened near the top of the viewport then stays over its trigger instead of moving beside it, and taking the existing override path while any of the layer's animations is pending fixes this. - Under CSS
zoomother than 1 the computed translation andtransform-originare in the layer's own px while the box is zoomed, andtranslateis read withparseFloat, so a zoomed layer reads up to 5.4 px toward its trigger at zoom 2 and a percentagetranslatereads 35.8 px off; falling back to the override whencurrentCSSZoomis not 1 or atranslatecomponent is not in px fixes both. - The changeset's "the result is the same box" and the description's identical-box measurement do not hold for those reads, because stepped frames never reach the pending show read; the fixes above make the claim true, or the wording can be narrowed.
Please cover the show read with a browser test (jsdom has no pending animation state) and add unit cases for the zoom and translate fallbacks. As an optional note, the 1/64 px grid named in the comment at settledBox.ts:24-29 is the Chromium and WebKit unit (Firefox uses 1/60 px).
[Automated review]
…m and a percentage translate The calculation takes the transform out only where the computed transform and the box agree: an entrance that is running, at a zoom of 1, with a translate in px. While an animation is pending, under a CSS zoom other than 1, or for a percentage translate, settledBox reads under the override as it did before. A browser test opens a tall Tooltip with motion allowed and checks that it opens beside its trigger and no entrance frame covers it. The grid tolerance is 1/2048 px, so Firefox's 1/60 px values are never moved onto the 1/64 px grid.
There was a problem hiding this comment.
Approved pending CI: CI has not run on this head yet.
The show read, CSS zoom other than 1 and a percentage translate now keep the override read, so settledBox returns the same box as before in each case, while reads during a running entrance no longer write styles.
Non-blocking:
- The new
TooltipTallContentmotion-allowed case also passes without the pending fallback, so it does not cover what its title names; rename it to what it checks, or add a case that fails without the fallback (for example a zoomed Tooltip whose trigger resizes during its entrance). - The layout-grid comment in
settledBox.tsstill has a 31-word passive sentence; a shorter rewrite is suggested inline.
[Automated review]
…rries it Right after a layer shows, Chromium can report its entrance's first frame in the computed style while getBoundingClientRect still returns the box laid out without it. Undoing that transform from such a box read the layer several px off, so a Tooltip near the top of the viewport could read as covering its trigger and move beside it (seen as a docsite Tooltip that covered the links beside its button). The box carries the scale only when its size is clearly nearer the layout size times the scale than the layout size itself. The layout size is the used border-box size from the computed style, so no rounding hides a small scale. Otherwise settledBox reads under the one-read override, as before.
There was a problem hiding this comment.
Changes requested. Reads on the middle frames of an entrance now return the same box with no style writes, but two things need fixing first:
- Regression: the changeset still says entrance reads stop writing styles except for the showing read, CSS
zoomand a percentagetranslate, but the new box-carries-scale fallback also keeps the override on the last frames of every entrance and whenever the box does not carry the entrance (in Chromium, a layer at a fallback position), so please list those cases. settledBox.ts(lines 142 and 148) andsettledBox.test.ts(lines 50-51) are not Prettier-formatted, so the formatting step of the requiredlintcheck will fail; runningpnpm exec prettier --writeon both files fixes it.
A non-blocking comment rewrite is suggested inline. CI has not run on this head yet because the pull request is a draft.
[Automated review]
…e every override case The computed translate keeps a percentage, so the matrix built from it returns null instead of reading -50% as -50 px. The changeset now names every read that still uses the override.
There was a problem hiding this comment.
Changes requested. The earlier changeset and formatting points are fixed, and reads on the middle frames of an entrance still return the same box without writing styles, but two things need fixing first:
- Regression: merged with current
main, the exit copy's unit test ("holds where a layer still entering was drawn",layerExitCopy.test.ts) fails because its stand-in box sits off the 1/64 px layout grid and thissettledBoxsnaps it (40.391pxbecomes40.390625px), so the requiredtestcheck will fail; please mergemainand put that case's box on the layout grid with matching expectations. transformOfnow rejects a one-valuetranslatesuch as8px, which is how browsers report a translate with no y offset, so a layer that transitionstransformgets its raw box back (8 px off in Chromium, wheremainreads it exactly); checking only the components present,translate.split(' ').every(value => value.endsWith('px')), fixes it, ideally with a unit case.
CI has not run on this head yet because the pull request is a draft.
[Automated review]
…xture out on the grid Chromium reports translate: 8px 0px as 8px. The px check now covers only the components present, so a transitioned layer with a one-value translate is still read exactly. The exit copy's mid-entrance case lays its stand-in box out on the 1/64 px grid, as a real engine does.
There was a problem hiding this comment.
Approved pending CI: CI has not run on this head yet, because the pull request is a draft; it runs once the pull request is marked ready for review.
Both earlier points are fixed: a one-value translate such as 8px is read again, so a transitioned layer gets the same box as on main (pinned by the new unit case), and the exit copy's mid-entrance test now lays its box on the 1/64 px layout grid and passes with this settledBox. Reads on the middle frames of an entrance still return the same box without writing styles.
- Non-blocking: the description says edges within 1/1024 px snap onto the grid, while the code uses 1/2048 px (
settledBox.ts:32).
[Automated review]
Maintainer impact
While a Tooltip, Popover, menu or other anchored layer plays its entrance, the layer runtime reads the layer's box to decide whether it covers its trigger or must be capped above a software keyboard. Each of those reads wrote
transform: none !importantto the layer (four properties), read the box, and wrote the old values back. In Chromium every such read forced two style recalculations and two layouts inside the frame that made it. A Tooltip reads its box at show, on every size change and in the frame after every anchor move, so a Tooltip that opens during a scroll paid this on each frame of its entrance.Intended invariant
architecture:react-component-runtimeINV18 (layout is read as little as possible).spec:AST-059FR4, FR8, FR9 andarchitecture:layer-runtimeINV12: the box is read from the layer's settled box, without its entrance transform. This stays true; only how the transform is taken out changes.No behavior change: the box that
settledBoxreturns is the same box, so placement, the anchor-clear switch, the keyboard cap and the entrance animation are unchanged. Where the calculation could disagree with the box, the old override read stays.Change
Every entrance only translates and scales the layer. For such a transform,
settledBoxnow takes the transform out of the one box read by calculation: the layout size is the transformed size over the scale, and the position follows from the transform origin. The computed transform is rounded when the engine serializes it, so the derived edges can be a few millionths of a px off; an edge within 1/1024 px of the 1/64 px layout grid is moved onto it, so the result equals the layout box exactly.currentCSSZoom) is 1, anytranslateis in px, and the box read carries the scale (its size is clearly nearer the used border-box size times the scale than the used size itself). Atranslatewith a percentage makes the computed matrix unknown, so that layer also takes the override. Right after a show, Chromium can report the entrance's first frame in the computed style while the box read is still laid out without it; undoing a transform the box does not carry read a Tooltip as covering its trigger. In every other case (the read that shows the layer, the frames before the box carries the entrance, a layer at a fallback position whose box does not carry the entrance in Chromium, a scale too near 1 to tell apart, a zoomed layer, a percentagetranslate) the layer is read under the one-read override, as before.untransformstays, rewritten to compose the 2D matrix itself instead of throughDOMMatrix, and to use the exact size for a translate-and-scale transform.Evidence
Failure before the change: run against
main'ssettledBox, the newsettledBox.test.tscases "reads an entering layer at its layout box, exactly, and writes no style" and "reads a layer that scales unevenly, or mirrors" fail (setPropertycalled 4 times: the override; the restore is 4 more writes). The transitioned-rotation case also fails there, becausemainneedsDOMMatrix, which jsdom lacks.Success after the change, measured in Chromium (Playwright's headless Chromium) on an anchored popover with the shipped entrance (from 8 px below at 95 % scale), one
settledBoxread in each frame of the entrance. The first frame after the show still has a pending animation and keeps the override read; most later frames read without a write:getBoundingClientRectper readgetComputedStyleper readAt rest (no transform) both are one
getBoundingClientRect.Show read with a pending entrance: a new
TooltipTallContentbrowser case opens a tall Tooltip with motion allowed and checks that it opens beside its trigger and that no frame of the entrance covers the trigger. It passes at this head. It also passes against a build of the previous head, which had no pending-animation fallback: in these Chromium runs, the box at the show read already carried the first keyframe. So the fallback is kept as a safeguard for the engine state the review describes, at the cost of one override read per entrance.Regression found by CI on an earlier head and fixed here: the docsite's "one info banner at 1440px in dark" cases failed because the banner's dismiss Tooltip, opened by hover, moved beside its button and covered the links. Reproduced on that head's Storybook (a top-edge Tooltip opened by hover under a fake clock moved beside its trigger in 2 of 8 runs; never on
main). Cause: the box read did not carry the transform the computed style reported. With the box-carries-scale check: 0 of 10 runs. A unit case pins that state (a 300×48 box with a computedmatrix(0.97, …)).Product/runtime behavior verified unchanged: in the same page, both implementations read every frame of the entrance (up to 19 distinct transforms) at device scale 1.5 and 2; every box was identical, edge for edge (largest difference 0 px), at device scale 1, 1.5 and 2 (at this head). Existing
useLayer.test.tsxanduseLayerKeyboardClearance.test.tsxpass unchanged, including the keyboard cap of a layer that is still entering.Scope
Testing
vitest run --project ui packages/core/src/Layer/settledBox.test.ts packages/core/src/Layer/useLayer.test.tsx packages/core/src/Layer/useLayerKeyboardClearance.test.tsx(145 tests, including unit cases for the pending, stale-box, near-1 scale, zoom and percentage-translatefallbacks);tsc -p packages/core; ESLint on the changed files.playwright test TooltipTallContent -g "desktop|motion allowed"(Chromium) against a Storybook built at this head.pnpm review:pre: 0 findings.pnpm specs:for --diff origin/main: AST-059 FR4/FR8/FR9 and layer-runtime INV12 keep their wording; the box is still read without the entrance transform.