Repository navigation
Conversation
A component that animates a pseudo-element (animatorFor(element, {pseudoElement: '::before'})) drew its motion where sampleCurve could not read it: it labelled samples by the element's own animations only and read the element's computed style. sampleCurve(locator, times, {pseudoElement}) now reads that pseudo-element's presented values, labelled by its own animations (those whose effect targets the element with that pseudo-element). Without the option nothing changes.
waitForRest already waits a pseudo-element's animation out: its read of getAnimations({subtree: true}) includes the element's pseudo-elements in Chromium and WebKit. A self-test now guards that.
Red first: on main's curves.ts the new self-test samples the box itself (opacity 1 at t 0, where the veil's is 0) in both engines; with the option it matches the veil's linear fade to 0.005 and its 40 px move to 0.05 px. The README describes the option in the curve-sampling paragraph, outside the module table.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
meta-cla
Bot
added
the
CLA Signed
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Requesting one small change before this lands; the sampleCurve pseudo-element option and its self-tests look correct, and sampling without the option is unchanged.
The JSDoc at internal/interaction-harness/src/curves.ts:592 cites animatorFor(element, {pseudoElement}), but AnimatorOptions on main has no pseudoElement option yet (it arrives with #8242), so please point to element.animate(keyframes, {pseudoElement}) as the self-test does, or drop the parenthetical until that option lands.
[Automated review]
| * `pseudoElement` samples the element's `::before` or `::after` instead: | ||
| * its presented values, labelled by its own animations (those whose effect | ||
| * targets the element with that pseudo-element), as a component that animates | ||
| * a pseudo-element (`animatorFor(element, {pseudoElement})`) draws it. |
There was a problem hiding this comment.
animatorFor(element, {pseudoElement}) doesn't exist on main: AnimatorOptions in packages/core/src/motion/animator.ts has only initial, now and requestFrame, and the pseudoElement option arrives with #8242. Until then this line could name the call that exists, as the new self-test does:
* a pseudo-element (`element.animate(keyframes, {pseudoElement})`) draws it.… call that exists on main
astracat found that the note cited `animatorFor(element, {pseudoElement})`, an animator option that arrives with meta#8242 and isn't on main. It now names `element.animate(keyframes, {pseudoElement})`, the call the self-test uses.
There was a problem hiding this comment.
Recommending approval: the new sampleCurve pseudoElement option reads a ::before or ::after timed by its own animations, sampling without the option is unchanged, and the option's self-test fails without the change and passes with it. The JSDoc at internal/interaction-harness/src/curves.ts:592 now names element.animate(keyframes, {pseudoElement}), which resolves the change requested on the previous revision, so nothing further is needed from you.
[Automated review]
Review only this PR's change: vjeux/astryx@90da4a7...93b46e7
Why
A component that animates a
::beforeor::after(animatorFor(element, {pseudoElement})) draws its motion wheresampleCurvecannot read it:sampleCurvereads the element's computed style, labelled by the element's own animations. Lightbox's chrome-fade proofs (#8242) sample a pseudo-element's fade three times.What led to it
The option lives only in #8242's line, on an older harness. The foundation re-fold doesn't carry it, so #8242's re-fold cannot take it without resolving the harness README by hand.
Options and tradeoffs
Why this choice
sampleCurve(locator, times, {pseudoElement: '::before'})reads that pseudo-element's presented values, labelled by its own animations: those whose effect targets the element with that pseudo-element. Without the option nothing changes.waitForRestalready waits a pseudo-element's animation out, since its subtree read includes the element's pseudo-elements, and a self-test now guards that. The README describes the option in the curve-sampling paragraph rather than the module table, which the re-fold re-pads.How to check it
pnpm test:interaction internal/interaction-harness/src/harness.interaction.browser.spec.ts --grep "::before animation": 2 pass in each engine.Evidence
curves.tsthe new self-test samples the box itself (opacity 1 at t 0, where the veil's is 0) in both engines; with the option it matches the veil's linear fade to 0.005 and its 40 px move to 0.05 px.pnpm check:repoand the node project (796 files) pass.