Add layout="x" and layout="y" axis constraints#3627
Conversation
Allows limiting layout animations to a single axis. When layout="x", only the x-axis animates and y changes are applied instantly. When layout="y", only the y-axis animates and x changes are applied instantly. Works with both layout and layoutId. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Greptile SummaryThis PR introduces Key changes:
Issues found:
Confidence Score: 3/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[notifyLayoutUpdate called] --> B{animationType?}
B -->|size| C[Snap position, preserve size on both axes]
B -->|x or y| D[Determine snapAxis\nx animationType snaps y axis\ny animationType snaps x axis]
B -->|position| E[Preserve size, animate position]
B -->|both or default| F[Full layout animation]
D --> G{isShared?}
G -->|false| H[Update layoutBox snapAxis\nto match current layout]
G -->|true| I[Update measuredBox snapAxis\nto match current layout]
I --> J[if-isShared block re-updates\nmeasuredBox REDUNDANT\nshould update layoutBox]
H --> K[layoutDelta snapAxis = 0]
J --> L[layoutDelta snapAxis != 0 BUG]
I --> M[visualDelta snapAxis = 0]
H --> N[visualDelta snapAxis = 0]
K --> O[Correct: only non-snap axis animates]
N --> O
L --> P[layoutDelta non-zero on snap axis\nfor shared layout transitions]
M --> O
Last reviewed commit: 362e784 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if (isShared) { | ||
| snapshot.measuredBox[snapAxis].min = layout[snapAxis].min | ||
| snapshot.measuredBox[snapAxis].max = layout[snapAxis].max | ||
| } |
There was a problem hiding this comment.
Redundant assignment in the shared-layout branch
When isShared is true, axisSnapshot is already a reference to snapshot.measuredBox[snapAxis] (line 2124–2126). The subsequent if (isShared) block therefore writes the exact same values back to the exact same object — it is a no-op.
The intention was likely to also zero out the snap axis on snapshot.layoutBox, which drives the layoutDelta calculation at line 2157:
calcBoxDelta(layoutDelta, layout, snapshot.layoutBox)Because snapshot.layoutBox[snapAxis] is never updated for shared elements, layoutDelta on the snap axis will remain non-zero for layoutId-based (shared) transitions. The visual animation still appears correct because visualDelta is computed from measuredBox, but the stale layoutDelta could affect downstream didUpdate consumers and the hasLayoutChanged gate in unexpected ways.
The fix should update layoutBox in the isShared branch:
if (isShared) {
snapshot.layoutBox[snapAxis].min = layout[snapAxis].min
snapshot.layoutBox[snapAxis].max = layout[snapAxis].max
}| it(`It correctly fires layout="x" animations, only animating the x axis`, () => { | ||
| cy.visit("?test=layout&type=x") | ||
| .wait(50) | ||
| .get("#box") | ||
| .should(([$box]: any) => { | ||
| expectBbox($box, { | ||
| top: 0, | ||
| left: 0, | ||
| width: 100, | ||
| height: 200, | ||
| }) | ||
| }) | ||
| .trigger("click") | ||
| .wait(50) | ||
| .should(([$box]: any) => { | ||
| expectBbox($box, { | ||
| top: 100, | ||
| left: 100, | ||
| width: 200, | ||
| height: 300, | ||
| }) | ||
| }) | ||
| }) | ||
|
|
||
| it(`It correctly fires layout="y" animations, only animating the y axis`, () => { | ||
| cy.visit("?test=layout&type=y") | ||
| .wait(50) | ||
| .get("#box") | ||
| .should(([$box]: any) => { | ||
| expectBbox($box, { | ||
| top: 0, | ||
| left: 0, | ||
| width: 100, | ||
| height: 200, | ||
| }) | ||
| }) | ||
| .trigger("click") | ||
| .wait(50) | ||
| .should(([$box]: any) => { | ||
| expectBbox($box, { | ||
| top: 50, | ||
| left: 200, | ||
| width: 300, | ||
| height: 250, | ||
| }) | ||
| }) | ||
| }) |
There was a problem hiding this comment.
No test coverage for axis constraints with layoutId (shared layout)
The PR description states the feature "Works with both layout and layoutId", but both new Cypress tests use a plain layout="x" / layout="y" element without layoutId. The code path for shared layout transitions (isShared = true) takes a different branch inside notifyLayoutUpdate, and — as noted in the logic comment above — the if (isShared) block appears to contain a bug specific to that path.
Consider adding a test case that uses layout="x" (or "y") together with layoutId to verify that the snap axis truly snaps while the animated axis transitions correctly, and to prevent regressions in the shared-layout flow.
… block Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
amazing ship, thank you ❤️❤️🫡 |
2 participants
Summary
layout="x"andlayout="y"as new values for thelayoutproplayout="x", only the x-axis layout change is animated — y-axis changes apply instantlylayout="y", only the y-axis layout change is animated — x-axis changes apply instantlylayoutandlayoutIdFixes #3103
Fixes #1972
How it works
In the projection system's
notifyLayoutUpdate, whenanimationTypeis"x"or"y", the snapshot for the non-animated axis is set to match the current layout. This eliminates the delta on that axis so no animation occurs — the element snaps instantly to its final position on that axis while smoothly animating on the other.Test plan
layout="x"— verifies x-axis animates while y-axis snapslayout="y"— verifies y-axis animates while x-axis snaps🤖 Generated with Claude Code