Skip to content

Conversation

boobam22
Copy link

@boobam22 boobam22 commented Oct 14, 2025

close #13990

Summary by CodeRabbit

  • Bug Fixes

    • Improves accuracy of move animations in grouped transitions, especially when elements use CSS transforms or non-default transform origins, reducing misalignment and flicker during enter/leave/move.
  • Refactor

    • Streamlined internal position calculation for transition tracking to enhance consistency and performance.

Copy link

coderabbitai bot commented Oct 14, 2025

Walkthrough

Reworks position tracking in TransitionGroup by introducing a Position type and a calculatePosition helper that factors in transform and transformOrigin. Replaces direct DOMRect usage and getBoundingClientRect calls with calculated positions, updating WeakMaps (positionMap, newPositionMap) and registration/recording routines accordingly. No exported/public declarations changed.

Changes

Cohort / File(s) Summary
Position calculation refactor
packages/runtime-dom/src/components/TransitionGroup.ts
Added internal Position { left, top } type; introduced calculatePosition(el) applying transformOrigin and transform to bounding rect; replaced getBoundingClientRect position reads with calculatePosition; updated positionMap/newPositionMap storage and recordPosition/initial child registration to use calculated positions; removed DOMRect typing from maps.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant TransitionGroup
  participant ChildEl as Child Element
  participant Calc as calculatePosition
  participant PosMap as positionMap
  participant NewPosMap as newPositionMap

  User->>TransitionGroup: mount/update
  loop initial registration
    TransitionGroup->>Calc: calculatePosition(ChildEl)
    Calc-->>TransitionGroup: Position { left, top }
    TransitionGroup->>PosMap: set(ChildEl, Position)
  end

  Note over TransitionGroup: On move/update
  TransitionGroup->>Calc: calculatePosition(ChildEl)
  Calc-->>TransitionGroup: Position { left, top }
  TransitionGroup->>NewPosMap: set(ChildEl, Position)
  TransitionGroup->>TransitionGroup: compute deltas and apply moves
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested labels

scope: transition

Suggested reviewers

  • edison1105

Poem

I hopped through DOM with nimble cheer,
From rects to poses crisp and clear—
A tiny shift, a clever tune,
Positions dance in perfect swoon.
With maps in paws and transforms bright,
I chart the lefts, the tops just right. 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title clearly and concisely describes the main change by indicating a fix for the runtime-dom's TransitionGroup behavior to compare positions based on transform-origin, which directly reflects the introduction of the calculatePosition helper and related updates.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 45547e6 and 4b191f8.

📒 Files selected for processing (1)
  • packages/runtime-dom/src/components/TransitionGroup.ts (3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/runtime-dom/src/components/TransitionGroup.ts (1)
packages/compiler-core/src/ast.ts (1)
  • Position (83-87)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Redirect rules
  • GitHub Check: Header rules
  • GitHub Check: Pages changed

Comment on lines +198 to +202
const [ox, oy] = style.transformOrigin.split(' ').map(v => parseFloat(v))
const point = new DOMPoint(ox, oy)
const transformed = point.matrixTransform(matrix)

return { left: transformed.x + rect.left, top: transformed.y + rect.top }
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Transform origin parsing breaks for percentages/keywords

parseFloat on style.transformOrigin treats 50% as 50 (interpreted as 50 px) and returns NaN for keywords such as center, left, etc. That makes calculatePosition report incorrect or NaN offsets when authors use the default 50% 50% origin or keyword-based origins, so moves are computed from the wrong point and transitions visibly jump. Please resolve the transform-origin string into actual pixel offsets (handle %, the keyword set, and fall back to the axis size) before feeding it to the DOMMatrix.

🤖 Prompt for AI Agents
In packages/runtime-dom/src/components/TransitionGroup.ts around lines 198-202,
the transform-origin parsing currently uses parseFloat which treats "50%" as 50
and returns NaN for keywords; update calculatePosition to resolve
transformOrigin tokens into pixel offsets before creating the DOMPoint: split
the transformOrigin into x/y (support single-value shorthand by using '50%' for
missing axis), for each token if it endsWith('%') compute
(parseFloat(token)/100) * axisSize (rect.width for x, rect.height for y), else
if it is a keyword map 'left'->0%,'center'->50%,'right'->100% and similarly
'top'->0%,'center'->50%,'bottom'->100%, else parseFloat and treat as pixels,
then use those computed ox/oy pixel values to construct the DOMPoint and
continue with matrixTransform and return offsets.

Copy link

pkg-pr-new bot commented Oct 15, 2025

Open in StackBlitz

@vue/compiler-core

npm i https://pkg.pr.new/@vue/compiler-core@13991

@vue/compiler-dom

npm i https://pkg.pr.new/@vue/compiler-dom@13991

@vue/compiler-sfc

npm i https://pkg.pr.new/@vue/compiler-sfc@13991

@vue/compiler-ssr

npm i https://pkg.pr.new/@vue/compiler-ssr@13991

@vue/reactivity

npm i https://pkg.pr.new/@vue/reactivity@13991

@vue/runtime-core

npm i https://pkg.pr.new/@vue/runtime-core@13991

@vue/runtime-dom

npm i https://pkg.pr.new/@vue/runtime-dom@13991

@vue/server-renderer

npm i https://pkg.pr.new/@vue/server-renderer@13991

@vue/shared

npm i https://pkg.pr.new/@vue/shared@13991

vue

npm i https://pkg.pr.new/vue@13991

@vue/compat

npm i https://pkg.pr.new/@vue/compat@13991

commit: 4b191f8

Copy link

Size Report

Bundles

File Size Gzip Brotli
runtime-dom.global.prod.js 102 kB (+200 B) 38.7 kB (+108 B) 34.8 kB (+87 B)
vue.global.prod.js 160 kB (+200 B) 58.8 kB (+94 B) 52.3 kB (+74 B)

Usages

Name Size Gzip Brotli
createApp (CAPI only) 46.7 kB 18.3 kB 16.7 kB
createApp 54.7 kB 21.3 kB 19.5 kB
createSSRApp 58.9 kB 23 kB 21 kB
defineCustomElement 60 kB 23 kB 21 kB
overall 68.8 kB 26.5 kB 24.2 kB

@edison1105
Copy link
Member

Thanks for this PR, but this PR is a duplicate of #6108.
see Playground with #6108

@boobam22 boobam22 closed this Oct 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TransitionGroup: Incorrect move animation when element has transform: scale()

2 participants