perf(stackflow): replace CSS animation with WAAPI for AppScreen transitions#1444
perf(stackflow): replace CSS animation with WAAPI for AppScreen transitions#1444junghyeonsu wants to merge 11 commits intodevfrom
Conversation
…creen transitions Migrate all AppScreen transition animations (push, pop, swipe back) from CSS recipe-based animations to Web Animations API (WAAPI). This eliminates the CSS-to-JS handoff that caused flicker, double-animation, and timing bugs during swipe-back gestures. Key changes: - Remove all transition animation selectors from app-screen.ts and app-bar.ts recipes - Add transition-animation.ts with WAAPI implementations for iOS/Android/fadeIn styles - Rewrite useGlobalInteraction to detect transitionState changes and trigger WAAPI - Add rAF lock pattern and passive-ready touchmove in useSwipeBack - Manage element positions via explicit inline styles instead of CSS variables Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 9718a76 The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthrough스택플로우 전환 로직을 WAAPI 기반 애니메이션으로 재구성하고, 스와이프 백을 ref 기반으로 변경했으며, DOM 타겟 검색·인라인 스타일 유틸리티와 데이터 속성 식별자를 추가했습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant User as 사용자 제스처
participant SB as useSwipeBack
participant GI as useGlobalInteraction
participant DOM as DOM 유틸리티
participant WAAPI as WAAPI 애니메이션
User->>SB: touchmove / touchend 이벤트
SB->>SB: RAF 스로틀 확인
SB->>GI: events.moveSwipeBack / events.endSwipeBack
GI->>DOM: findTransitionTargets(stackEl)
GI->>DOM: applySwipeStyles(targets, displacement, ratio)
DOM->>DOM: 인라인 transform/opacity 설정
alt 스와이프 종료(완료)
GI->>WAAPI: animateSwipeComplete(targets, displacement, velocity)
WAAPI->>DOM: 애니메이션으로 transform/opacity 변화
WAAPI-->>GI: finished Promise
else 스와이프 종료(취소)
GI->>WAAPI: animateSwipeCancel(targets, displacement, velocity)
WAAPI->>DOM: 복귀 애니메이션 실행
WAAPI-->>GI: finished Promise
end
GI->>DOM: setPostExitPositions(targets, style)
GI->>GI: 상태 및 인라인 스타일 정리
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
/snapshot |
Alpha Preview (Stackflow SPA)
|
Alpha Preview (Storybook)
|
📦 Snapshot Release
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/stackflow/src/primitive/GlobalInteraction/useSwipeBack.ts (1)
31-46:⚠️ Potential issue | 🟠 Major마지막
touchmove를 종료 전에 반영해야 합니다.지금은
moveSwipeBack()가 다음 frame으로 미뤄지기 때문에, 그 전에onTouchEnd/onTouchCancel이 오면endSwipeBack()가 이전displacement/velocity로 판정합니다. 빠른 스와이프가 취소로 오판될 수 있고, 다음 frame의 지연된moveSwipeBack()이 이미 시작된 cancel/completing 애니메이션 위에 다시 스타일을 덮어쓸 수도 있습니다. 마지막 좌표와 rAF id를 저장해 두고 종료 시 동기 flush 하거나, 최소한 pending frame을 취소한 뒤 종료하는 쪽이 안전합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/stackflow/src/primitive/GlobalInteraction/useSwipeBack.ts` around lines 31 - 46, The touchend/touchcancel path can race with a pending requestAnimationFrame in onTouchMove, causing endSwipeBack() to use stale displacement/velocity; modify useSwipeBack so onTouchMove saves the last touch {x,t} and the rAF id (e.g. rafIdRef) when calling requestAnimationFrame, and then in onTouchEnd and onTouchCancel first cancel any pending rAF (cancelAnimationFrame(rafIdRef.current)) and synchronously flush the last saved touch by calling events.moveSwipeBack with the saved {x,t} before calling events.endSwipeBack({}), ensuring rAFLockRef is reset appropriately to avoid deadlock.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/qvism-preset/src/stackflow/app-bar.ts`:
- Around line 231-235: 현재 transitionStyle의 slideFromRightIOS /
fadeFromBottomAndroid / fadeIn 변형을 비워서 transition-animation.ts의
applySwipeStyles()가 업데이트하는 CSS 변수 --swipe-back-displacement를 읽는 transform 동작이
사라졌습니다; transitionStyle 객체의 해당 variants에 swipe 중 요소 이동을 반영하는 transform(예:
translateX(var(--swipe-back-displacement))) 규칙을 복원하고, 이 파일의 AppBar 스타일 내
&:before(배경)에도 같은 transform(translateX(var(--swipe-back-displacement)))을 적용해
제목/아이콘과 동일하게 배경이 손가락을 따라 움직이도록 만드세요; applySwipeStyles()가 설정하는
--swipe-back-displacement 변수를 사용하도록 정확한 변수 이름을 참조하는지 확인하세요.
In `@packages/stackflow/src/primitive/GlobalInteraction/transition-animation.ts`:
- Around line 250-259: The problem is waitAll treats canceled animations as
successful by swallowing AbortError, causing callers (useGlobalInteraction.ts
callbacks that call setIdlePositions()/setPostExitPositions()) to run for
canceled animations; update waitAll to rethrow AbortError so Promise.all rejects
on cancellation (i.e., change the finished.catch handler inside waitAll to
rethrow when err.name === 'AbortError' or instanceof DOMException with name
'AbortError', but continue swallowing/ignoring other errors), ensuring canceled
animations are identified and downstream cleanup callbacks are skipped.
In `@packages/stackflow/src/primitive/GlobalInteraction/useGlobalInteraction.ts`:
- Around line 311-321: The returned swipeBackState currently exposes a snapshot
(swipeBackStateRef.current) so consumers never see updates when
setSwipeBackState is called; update the API by either removing swipeBackState
from the returned object or making it reactive — e.g., add a stable getter
getSwipeBackState() or maintain a synced piece of state inside
useGlobalInteraction that is updated inside setSwipeBackState (keep
setSwipeBackState, getSwipeBackEvents, stackProps as-is), and update the useMemo
to return the reactive getter/value (or remove swipeBackState) so consumers
observe changes; refer to useGlobalInteraction, swipeBackStateRef,
setSwipeBackState and getSwipeBackEvents when implementing.
- Around line 96-119: When starting a swipe in
useGlobalInteraction.startSwipeBack you cancel runningAnimsRef but you also must
cancel any scheduled frame-based push WAAPI reserved via pendingRAFRef; if you
don't, that RAF can fire after the swipe begins and apply push styles over the
swipe. Fix by checking pendingRAFRef.current in startSwipeBack, call
cancelAnimationFrame on it (or clearTimeout if using setTimeout), set
pendingRAFRef.current = null, and ensure any associated queued push state is
cleared so the reserved push won't run during the swipe.
---
Outside diff comments:
In `@packages/stackflow/src/primitive/GlobalInteraction/useSwipeBack.ts`:
- Around line 31-46: The touchend/touchcancel path can race with a pending
requestAnimationFrame in onTouchMove, causing endSwipeBack() to use stale
displacement/velocity; modify useSwipeBack so onTouchMove saves the last touch
{x,t} and the rAF id (e.g. rafIdRef) when calling requestAnimationFrame, and
then in onTouchEnd and onTouchCancel first cancel any pending rAF
(cancelAnimationFrame(rafIdRef.current)) and synchronously flush the last saved
touch by calling events.moveSwipeBack with the saved {x,t} before calling
events.endSwipeBack({}), ensuring rAFLockRef is reset appropriately to avoid
deadlock.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8d7b97e8-99d6-48cc-9bc7-e462e0c12b96
⛔ Files ignored due to path filters (10)
packages/css/all.cssis excluded by!packages/css/**/*packages/css/all.layered.cssis excluded by!packages/css/**/*packages/css/all.layered.min.cssis excluded by!packages/css/**/*packages/css/all.min.cssis excluded by!packages/css/**/*packages/css/recipes/app-bar-main.cssis excluded by!packages/css/**/*packages/css/recipes/app-bar-main.layered.cssis excluded by!packages/css/**/*packages/css/recipes/app-bar.cssis excluded by!packages/css/**/*packages/css/recipes/app-bar.layered.cssis excluded by!packages/css/**/*packages/css/recipes/app-screen.cssis excluded by!packages/css/**/*packages/css/recipes/app-screen.layered.cssis excluded by!packages/css/**/*
📒 Files selected for processing (5)
packages/qvism-preset/src/stackflow/app-bar.tspackages/qvism-preset/src/stackflow/app-screen.tspackages/stackflow/src/primitive/GlobalInteraction/transition-animation.tspackages/stackflow/src/primitive/GlobalInteraction/useGlobalInteraction.tspackages/stackflow/src/primitive/GlobalInteraction/useSwipeBack.ts
Alpha Preview (Docs)
|
Extract shared helpers to reduce duplication without changing behavior: - Unify iosAnimatePush/Pop into parameterized iosAnimate - Unify animateSwipeComplete/Cancel into parameterized animateSwipe - Extract collectAnimations helper for repeated filter+waitAll pattern - Fix file header comment to match actual fill:"forwards" strategy Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/stackflow/src/primitive/GlobalInteraction/transition-animation.ts (1)
346-352: 타입 단언 대신 명시적 타입 정의를 고려해보세요.
from.topTitle["transform"] as string패턴은 현재IOS_ONSCREEN/IOS_OFFSCREEN상수에서 안전하지만, 향후 유지보수 시 타입 안전성을 저해할 수 있습니다.♻️ 타입 안전성 개선 제안
interface IosPositions { topLayer: string; behindLayer: string; dim: string; - topTitle: Keyframe; - behindTitle: Keyframe; + topTitle: { opacity: string; transform: string }; + behindTitle: { opacity: string; transform: string }; topIconOpacity: string; behindIconOpacity: string; appBarPseudo: string; }이렇게 하면
from.topTitle.transform으로 직접 접근 가능하며 타입 단언이 불필요해집니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/stackflow/src/primitive/GlobalInteraction/transition-animation.ts` around lines 346 - 352, The code uses type assertions like from.topTitle["transform"] as string; instead, update the type definition for the topTitle object so transform is explicitly typed as string (e.g., refine the TopTitle type/interface or the IOS_ONSCREEN/IOS_OFFSCREEN constant typings) and then replace bracket-assertions with direct property access (from.topTitle.transform and to.topTitle.transform) to eliminate the need for "as string" and preserve type safety.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/stackflow/src/primitive/GlobalInteraction/transition-animation.ts`:
- Around line 346-352: The code uses type assertions like
from.topTitle["transform"] as string; instead, update the type definition for
the topTitle object so transform is explicitly typed as string (e.g., refine the
TopTitle type/interface or the IOS_ONSCREEN/IOS_OFFSCREEN constant typings) and
then replace bracket-assertions with direct property access
(from.topTitle.transform and to.topTitle.transform) to eliminate the need for
"as string" and preserve type safety.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 43afaef9-959e-48c7-9b48-e69533413d0d
📒 Files selected for processing (1)
packages/stackflow/src/primitive/GlobalInteraction/transition-animation.ts
setIdlePositions and setPostExitPositions now accept TransitionStyle parameter. iOS-specific behind layer offset (-30%) and title/icon hiding only apply to slideFromRightIOS. Android and fadeIn transitions no longer incorrectly offset the behind activity. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/stackflow/src/primitive/GlobalInteraction/transition-animation.ts (1)
34-37: 클래스 substring 셀렉터 사용에 대한 참고
[class*="seed-app-bar..."]패턴은 CSS-in-JS 환경에서 클래스명에 hash가 붙는 경우를 대응하기 위한 것으로 보입니다. 다만 이 방식은:
- 유사한 이름의 다른 클래스와 우발적 매칭 가능성
- 클래스 네이밍 컨벤션 변경 시 깨질 수 있음
현재 구현이 의도된 것이라면 문제없으나, 가능하다면
data-part속성처럼 명시적 attribute 기반 셀렉터가 더 안정적입니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/stackflow/src/primitive/GlobalInteraction/transition-animation.ts` around lines 34 - 37, The substring class selectors (SEL_APP_BAR_MAIN_ROOT, SEL_APP_BAR_ROOT, SEL_APP_BAR_ICON, SEL_APP_BAR_CUSTOM) are brittle and may match unintended hashed classnames; change the selectors to use an explicit data attribute (e.g., data-part="app-bar-main", "app-bar-root", "app-bar-icon", "app-bar-custom") and update the consuming components to render those data-part attributes so the animation code queries by attribute instead of class substring; ensure the new attribute names are documented and replace all uses of the four SEL_* constants in transition-animation.ts with the corresponding data-part selectors.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/stackflow/src/primitive/GlobalInteraction/useGlobalInteraction.ts`:
- Around line 87-230: The getSwipeBackEvents function violates React Hooks rules
by calling hooks (useCallbackRef, useCallback, useMemo) inside a callback; move
all hook calls to the top level of the useGlobalInteraction hook and have
getSwipeBackEvents only compose/return pre-created callbacks. Concretely: hoist
useCallbackRef calls for props.onSwipeBackStart/onSwipeBackMove/onSwipeBackEnd
out of getSwipeBackEvents into the top of useGlobalInteraction (keep names
onSwipeStartRef/onSwipeMoveRef/onSwipeEndRef or similar), create top-level
stable callbacks startSwipeBack, moveSwipeBack, endSwipeBack and reset with
useCallback that reference refs and refs like
swipeBackContextRef/cachedTargetsRef/runningAnimsRef, and create the returned
object with useMemo at top-level; then change getSwipeBackEvents to simply
return { startSwipeBack, moveSwipeBack, endSwipeBack, reset } (or remove it and
return that object directly) so no hooks are invoked inside a nested callback.
---
Nitpick comments:
In `@packages/stackflow/src/primitive/GlobalInteraction/transition-animation.ts`:
- Around line 34-37: The substring class selectors (SEL_APP_BAR_MAIN_ROOT,
SEL_APP_BAR_ROOT, SEL_APP_BAR_ICON, SEL_APP_BAR_CUSTOM) are brittle and may
match unintended hashed classnames; change the selectors to use an explicit data
attribute (e.g., data-part="app-bar-main", "app-bar-root", "app-bar-icon",
"app-bar-custom") and update the consuming components to render those data-part
attributes so the animation code queries by attribute instead of class
substring; ensure the new attribute names are documented and replace all uses of
the four SEL_* constants in transition-animation.ts with the corresponding
data-part selectors.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: eff0c742-5717-4f8e-856c-92761d806799
📒 Files selected for processing (2)
packages/stackflow/src/primitive/GlobalInteraction/transition-animation.tspackages/stackflow/src/primitive/GlobalInteraction/useGlobalInteraction.ts
…on.ts - Add data-part attributes to AppBarMain, AppBarIconButton, AppBarSlot to replace fragile className-based selectors - Split transition-animation.ts into: - dom.ts: DOM discovery (findTransitionTargets) and inline style management - animation.ts: WAAPI animation functions - Unify SwipeEndpoints with IOS_ONSCREEN/IOS_OFFSCREEN constants - Remove transition-animation.ts Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
♻️ Duplicate comments (4)
packages/stackflow/src/primitive/GlobalInteraction/useGlobalInteraction.ts (3)
98-121:⚠️ Potential issue | 🟠 Majorswipe 시작 시 대기 중인 push rAF도 취소해야 합니다.
Lines 261-263에서
enter-active는 다음 frame에 push WAAPI를 예약하는데,startSwipeBack에서는 실행 중인 animation만 취소하고pendingRAFRef는 그대로 둡니다. 사용자가 그 frame 전에 edge swipe를 시작하면 예약된 push가 뒤늦게 실행되어 swipe 스타일과 겹칠 수 있습니다.,
권장 수정
const startSwipeBack = useCallback( ({ x0, t0 }: StartSwipeBackProps) => { + // Cancel any pending RAF-scheduled push animation + if (pendingRAFRef.current !== null) { + cancelAnimationFrame(pendingRAFRef.current); + pendingRAFRef.current = null; + } + // Cancel any running transition animations cancelAll(runningAnimsRef.current); runningAnimsRef.current = [];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/stackflow/src/primitive/GlobalInteraction/useGlobalInteraction.ts` around lines 98 - 121, startSwipeBack currently cancels only running animations via cancelAll(runningAnimsRef.current) but does not clear any scheduled frame callbacks, so pending push WAAPI scheduled by enter-active (via pendingRAFRef) can still run during a swipe; update startSwipeBack to also cancel and clear pendingRAFRef (e.g., call cancelAnimationFrame or equivalent on pendingRAFRef.current and set pendingRAFRef.current = null) and ensure any helper that schedules push WAAPI uses pendingRAFRef so this cleanup prevents the delayed push from applying styles during a swipe.
313-324:⚠️ Potential issue | 🟠 Major반환하는
swipeBackState는 reactive하지 않습니다.Line 317에서
swipeBackStateRef.current스냅샷을 그대로 반환하므로,setSwipeBackState()가 호출되어도 hook 소비자는 변경된 값을 받지 못합니다. DOM dataset 갱신과는 별개로, 공개 API로 유지할 경우 reactive하게 노출하거나 API surface에서 제거하는 것이 안전합니다.,
권장 수정 옵션
옵션 1: 제거 — 외부에서 필요 없다면 반환 객체에서
swipeBackState제거옵션 2: getter 함수로 노출
return useMemo( () => ({ stackRef, swipeBackContextRef, - swipeBackState: swipeBackStateRef.current, + getSwipeBackState: () => swipeBackStateRef.current, setSwipeBackState, getSwipeBackEvents, stackProps, }),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/stackflow/src/primitive/GlobalInteraction/useGlobalInteraction.ts` around lines 313 - 324, The returned swipeBackState currently returns a non-reactive snapshot (swipeBackStateRef.current) from useGlobalInteraction; update the API so consumers see updates by either removing swipeBackState from the returned object or replacing it with a getter function that reads swipeBackStateRef.current (e.g., expose getSwipeBackState) or convert to React state updated by setSwipeBackState; adjust the returned object (which currently lists stackRef, swipeBackContextRef, swipeBackState: swipeBackStateRef.current, setSwipeBackState, getSwipeBackEvents, stackProps) and its dependency array accordingly so consumers receive reactive updates or the property is no longer exposed.
89-232:⚠️ Potential issue | 🔴 CriticalReact Hooks 규칙 위반: 콜백 내부에서 hooks 호출
getSwipeBackEvents는useCallback으로 감싸진 함수인데, 내부에서useCallbackRef(lines 94-96),useCallback(lines 98, 123, 147, 206),useMemo(line 223)를 호출하고 있습니다. Hooks는 반드시 컴포넌트/커스텀 hook의 최상위에서만 호출해야 합니다.hooks를
useGlobalInteraction최상위로 이동하고,getSwipeBackEvents는 이미 생성된 callbacks를 조합하여 반환하도록 리팩토링이 필요합니다.,
권장 리팩토링 방향
export function useGlobalInteraction() { const swipeBackStateRef = useRef<SwipeBackState>("idle"); // ...existing refs... + // Move callback refs to top level + const onSwipeStartRef = useRef<(() => void) | undefined>(); + const onSwipeMoveRef = useRef<((props: { displacement: number; displacementRatio: number }) => void) | undefined>(); + const onSwipeEndRef = useRef<((props: { swiped: boolean }) => void) | undefined>(); + + // Move callbacks to top level + const startSwipeBack = useCallback(({ x0, t0 }: StartSwipeBackProps) => { + // ... implementation using onSwipeStartRef.current ... + }, []); + + // ... other callbacks at top level ... const getSwipeBackEvents = useCallback((props: SwipeBackProps) => { - const onSwipeStart = useCallbackRef(props.onSwipeBackStart); + // Update refs instead of creating new hooks + onSwipeStartRef.current = props.onSwipeBackStart; + onSwipeMoveRef.current = props.onSwipeBackMove; + onSwipeEndRef.current = props.onSwipeBackEnd; + + return { startSwipeBack, moveSwipeBack, endSwipeBack, reset }; }, [startSwipeBack, moveSwipeBack, endSwipeBack, reset]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/stackflow/src/primitive/GlobalInteraction/useGlobalInteraction.ts` around lines 89 - 232, getSwipeBackEvents currently calls hooks (useCallbackRef, useCallback, useMemo) inside its body which violates React Hooks rules; move those hook calls to the top-level of the containing useGlobalInteraction hook and have getSwipeBackEvents only compose and return the already-created callbacks. Concretely: lift the useCallbackRef(...) for onSwipeStart/onSwipeMove/onSwipeEnd and the useCallback definitions for startSwipeBack, moveSwipeBack, endSwipeBack, reset, and the useMemo return wrapper out of getSwipeBackEvents into useGlobalInteraction's top scope; keep the internal logic of startSwipeBack/moveSwipeBack/endSwipeBack/reset identical but reference shared refs (swipeBackContextRef, cachedTargetsRef, runningAnimsRef, skipNextExitRef, setSwipeBackState, stackRef) from the outer hook, and change getSwipeBackEvents to simply return { startSwipeBack, moveSwipeBack, endSwipeBack, reset } (or a memoized object using the already-lifted useMemo) so no hooks are invoked inside getSwipeBackEvents itself.packages/stackflow/src/primitive/GlobalInteraction/animation.ts (1)
68-78:⚠️ Potential issue | 🟠 Major
cancel()된 animation도 정상 완료처럼 처리됩니다.
waitAll이AbortError를 삼킨 뒤 resolve하므로, 호출부의finished.then(...)이 cancel 경로에서도 항상 실행됩니다.useGlobalInteraction.ts에서는 그 callback 안에서setIdlePositions()/setPostExitPositions()를 호출하므로, 빠른 push→pop 전환 시 취소된 이전 animation의 callback이 뒤늦게 스타일을 덮어쓸 수 있습니다.,
권장 수정
cancel 여부를 식별하여 후속 cleanup을 건너뛰도록 수정:
function waitAll(animations: (Animation | null)[]): Promise<void> { const valid = animations.filter((a): a is Animation => a !== null); if (valid.length === 0) return Promise.resolve(); return Promise.all( valid.map((a) => a.finished.catch((err) => { - /* AbortError from cancel */ + // Re-throw AbortError so callers can distinguish cancellation + if (err instanceof DOMException && err.name === "AbortError") { + throw err; + } + // Swallow other errors }), ), ).then(() => {}); }또는 호출부에서 animation 참조가 유효한지 확인:
finished.then(() => { // Only apply if these are still the current animations if (runningAnimsRef.current === animations) { setIdlePositions(targets, style); cancelAll(animations); runningAnimsRef.current = []; } });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/stackflow/src/primitive/GlobalInteraction/animation.ts` around lines 68 - 78, waitAll currently swallows AbortError from cancelled animations so callers' finished.then callbacks run even for cancelled sequences; change waitAll to rethrow AbortError (or otherwise propagate cancellation) instead of swallowing it: inside waitAll's Promise.all mapping for each Animation a use a.finished.catch(err => { if (err && err.name === 'AbortError') throw err; /* swallow other errors if desired */ }); this preserves the existing behavior for non-cancellation errors but ensures cancelled animations cause the returned promise to reject so callers (e.g. useGlobalInteraction.ts finished handlers) can skip cleanup or verify runningAnimsRef/current animations before applying style changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/stackflow/src/primitive/GlobalInteraction/animation.ts`:
- Around line 68-78: waitAll currently swallows AbortError from cancelled
animations so callers' finished.then callbacks run even for cancelled sequences;
change waitAll to rethrow AbortError (or otherwise propagate cancellation)
instead of swallowing it: inside waitAll's Promise.all mapping for each
Animation a use a.finished.catch(err => { if (err && err.name === 'AbortError')
throw err; /* swallow other errors if desired */ }); this preserves the existing
behavior for non-cancellation errors but ensures cancelled animations cause the
returned promise to reject so callers (e.g. useGlobalInteraction.ts finished
handlers) can skip cleanup or verify runningAnimsRef/current animations before
applying style changes.
In `@packages/stackflow/src/primitive/GlobalInteraction/useGlobalInteraction.ts`:
- Around line 98-121: startSwipeBack currently cancels only running animations
via cancelAll(runningAnimsRef.current) but does not clear any scheduled frame
callbacks, so pending push WAAPI scheduled by enter-active (via pendingRAFRef)
can still run during a swipe; update startSwipeBack to also cancel and clear
pendingRAFRef (e.g., call cancelAnimationFrame or equivalent on
pendingRAFRef.current and set pendingRAFRef.current = null) and ensure any
helper that schedules push WAAPI uses pendingRAFRef so this cleanup prevents the
delayed push from applying styles during a swipe.
- Around line 313-324: The returned swipeBackState currently returns a
non-reactive snapshot (swipeBackStateRef.current) from useGlobalInteraction;
update the API so consumers see updates by either removing swipeBackState from
the returned object or replacing it with a getter function that reads
swipeBackStateRef.current (e.g., expose getSwipeBackState) or convert to React
state updated by setSwipeBackState; adjust the returned object (which currently
lists stackRef, swipeBackContextRef, swipeBackState: swipeBackStateRef.current,
setSwipeBackState, getSwipeBackEvents, stackProps) and its dependency array
accordingly so consumers receive reactive updates or the property is no longer
exposed.
- Around line 89-232: getSwipeBackEvents currently calls hooks (useCallbackRef,
useCallback, useMemo) inside its body which violates React Hooks rules; move
those hook calls to the top-level of the containing useGlobalInteraction hook
and have getSwipeBackEvents only compose and return the already-created
callbacks. Concretely: lift the useCallbackRef(...) for
onSwipeStart/onSwipeMove/onSwipeEnd and the useCallback definitions for
startSwipeBack, moveSwipeBack, endSwipeBack, reset, and the useMemo return
wrapper out of getSwipeBackEvents into useGlobalInteraction's top scope; keep
the internal logic of startSwipeBack/moveSwipeBack/endSwipeBack/reset identical
but reference shared refs (swipeBackContextRef, cachedTargetsRef,
runningAnimsRef, skipNextExitRef, setSwipeBackState, stackRef) from the outer
hook, and change getSwipeBackEvents to simply return { startSwipeBack,
moveSwipeBack, endSwipeBack, reset } (or a memoized object using the
already-lifted useMemo) so no hooks are invoked inside getSwipeBackEvents
itself.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e86140b6-9815-4f70-b4e5-b756c483a086
📒 Files selected for processing (5)
packages/stackflow/src/components/AppBar/AppBar.tsxpackages/stackflow/src/primitive/AppBar/AppBar.tsxpackages/stackflow/src/primitive/GlobalInteraction/animation.tspackages/stackflow/src/primitive/GlobalInteraction/dom.tspackages/stackflow/src/primitive/GlobalInteraction/useGlobalInteraction.ts
✅ Files skipped from review due to trivial changes (2)
- packages/stackflow/src/primitive/AppBar/AppBar.tsx
- packages/stackflow/src/components/AppBar/AppBar.tsx
…sitions Set start positions as inline styles before animation begins for fadeFromBottomAndroid (opacity:0 + translateY:8vh) and fadeIn (opacity:0), matching the existing pattern in iOS push. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…state useTopActivity().transitionStyle updates one render cycle late because it uses useState + useLayoutEffect. During push, the captured value was the previous activity's style, so fadeFromBottomAndroid/fadeIn always fell back to slideFromRightIOS. Add readTransitionStyle() that reads data-transition-style directly from the top activity DOM element at animation time, bypassing the stale state. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
For fadeFromBottomAndroid and fadeIn transitions, the behind activity stays in place (no parallax offset). After push completes, its appBar was visible behind the top activity's appBar. Hide it with opacity:0. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ment Root cause: clearAllStyles didn't clear transform/opacity on appBar roots, leaving stale inline styles from pre-animation setup (e.g. Android push sets transform:translate3d(0,8vh,0) which was never cleaned up). Pattern change: setIdlePositions and setPostExitPositions now call clearAllStyles first (clean slate), then set only non-default positions. No more selective clearing that misses elements. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Behind appBar is hidden (opacity:0) during idle to prevent bleed-through. Pop must restore it before animation starts so it doesn't flash in at the end. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
If a push rAF is scheduled but hasn't fired yet when the user starts swiping, the rAF would execute after swipe begins and overlay push animations on top of swipe styles. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
good |
2 participants
Summary
seed-enter/seed-exitkeyframes to Web Animations API (WAAPI)push,pop,idle,swipeBack*) fromapp-screen.tsandapp-bar.tsCSS recipes — static layout/color/z-index styles are preservedtransition-animation.tswith WAAPI implementations for all three transition styles (slideFromRightIOS, fadeFromBottomAndroid, fadeIn)useGlobalInteractionto detect stackflowtransitionStatechanges and trigger WAAPI, replacing CSS variable cascade with explicit inline style managementuseSwipeBackfor throttled touch event processingWhy
The previous CSS animation ↔ inline style handoff during swipe-back caused flicker, double-animation on exit-done, and AppBar desync bugs. Unifying all transitions under a single JS animation system eliminates the handoff entirely.
Test plan
bun test:all— 596 pass, 0 failexamples/stackflow-spa: push, pop, swipe complete, swipe cancel🤖 Generated with Claude Code
Co-Authored-By: Claude Opus 4.6 (1M context) noreply@anthropic.com
Summary by CodeRabbit
버그 수정
성능 개선
신규 기능