fix(stackflow): scope swipe-back vars to activity roots to cut style recalculation#1204
Open
junghyeonsu wants to merge 1 commit intodevfrom
Open
fix(stackflow): scope swipe-back vars to activity roots to cut style recalculation#1204junghyeonsu wants to merge 1 commit intodevfrom
junghyeonsu wants to merge 1 commit intodevfrom
Conversation
|
Contributor
📝 Walkthrough개요스와이프백 기능의 내부 처리를 개선하기 위해 DOM 래핑, CSS 변수 기반 스타일 관리, 타겟 추적 메커니즘을 도입했습니다. 공개 API는 변경되지 않았으며, 공개 서명이 유지되었습니다. 변경 사항
예상 코드 리뷰 노력🎯 3 (보통) | ⏱️ ~22분 시
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
Deploying seed-design-v3 with
|
| Latest commit: |
df1ef83
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://aa8efb19.seed-design.pages.dev |
| Branch Preview URL: | https://fix-scope-swipe-back-vars.seed-design.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@packages/stackflow/src/primitive/GlobalInteraction/useGlobalInteraction.ts`:
- Around line 163-175: The velocity computation in moveSwipeBack can divide by
zero when t === swipeBackContextRef.current.t0; modify moveSwipeBack to compute
a safe delta time (e.g., deltaT = t - t0) and if deltaT is 0 or extremely small,
use a fallback (0 or a tiny epsilon) before calculating velocity, then update
setSwipeBackContext with the guarded velocity and keep the rest of the logic
(references: moveSwipeBack, swipeBackContextRef.current.t0, setSwipeBackContext,
setSwipeBackState, onSwipeBackMove).
- Around line 129-131: useLayoutEffect that calls
updateSwipeBackTargets(activities) lacks a cleanup; add a return cleanup
function in the useLayoutEffect inside useGlobalInteraction that resets the CSS
variables (--swipe-back-displacement, --swipe-back-displacement-ratio,
--swipe-back-target) on the elements previously targeted by
updateSwipeBackTargets (use the same activities or stored targets) when the
component unmounts or activities change; implement by capturing the list of
target elements (from activities or from updateSwipeBackTargets return value)
and in the cleanup loop remove or set those CSS custom properties to ''/null so
no stale styles remain.
Comment on lines
+129
to
+131
| useLayoutEffect(() => { | ||
| updateSwipeBackTargets(activities); | ||
| }, [activities, updateSwipeBackTargets]); |
Contributor
There was a problem hiding this comment.
useLayoutEffect에 cleanup 함수가 없습니다.
컴포넌트 언마운트 시 타겟 엘리먼트에 설정된 CSS 변수들(--swipe-back-displacement, --swipe-back-displacement-ratio, --swipe-back-target)이 정리되지 않습니다.
🧹 cleanup 함수 추가 제안
useLayoutEffect(() => {
updateSwipeBackTargets(activities);
+ return () => {
+ swipeBackTargetsRef.current.forEach(resetSwipeBackVars);
+ swipeBackTargetsRef.current = [];
+ };
}, [activities, updateSwipeBackTargets]);🤖 Prompt for AI Agents
In `@packages/stackflow/src/primitive/GlobalInteraction/useGlobalInteraction.ts`
around lines 129 - 131, useLayoutEffect that calls
updateSwipeBackTargets(activities) lacks a cleanup; add a return cleanup
function in the useLayoutEffect inside useGlobalInteraction that resets the CSS
variables (--swipe-back-displacement, --swipe-back-displacement-ratio,
--swipe-back-target) on the elements previously targeted by
updateSwipeBackTargets (use the same activities or stored targets) when the
component unmounts or activities change; implement by capturing the list of
target elements (from activities or from updateSwipeBackTargets return value)
and in the cleanup loop remove or set those CSS custom properties to ''/null so
no stale styles remain.
Comment on lines
+163
to
+175
| const moveSwipeBack = ({ x, t }: MoveSwipeBackProps) => { | ||
| const displacement = x - swipeBackContextRef.current.x0; | ||
| const displacementRatio = displacement / window.innerWidth; | ||
| const velocity = displacement / (t - swipeBackContextRef.current.t0); | ||
| setSwipeBackContext({ | ||
| ...swipeBackContextRef.current, | ||
| displacement, | ||
| displacementRatio, | ||
| velocity, | ||
| }); | ||
| setSwipeBackState((prev) => (prev === "swiping" ? prev : "swiping")); | ||
| onSwipeBackMove?.({ displacement, displacementRatio }); | ||
| }; |
Contributor
There was a problem hiding this comment.
0으로 나누기 가능성이 있습니다.
t와 t0가 동일한 타임스탬프일 경우 (터치 이벤트가 매우 빠르게 발생할 때) velocity 계산에서 0으로 나누기가 발생하여 Infinity 또는 NaN이 될 수 있습니다.
🛡️ 0으로 나누기 방어 코드 제안
const moveSwipeBack = ({ x, t }: MoveSwipeBackProps) => {
const displacement = x - swipeBackContextRef.current.x0;
const displacementRatio = displacement / window.innerWidth;
- const velocity = displacement / (t - swipeBackContextRef.current.t0);
+ const timeDelta = t - swipeBackContextRef.current.t0;
+ const velocity = timeDelta > 0 ? displacement / timeDelta : 0;
setSwipeBackContext({
...swipeBackContextRef.current,
displacement,
displacementRatio,
velocity,
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const moveSwipeBack = ({ x, t }: MoveSwipeBackProps) => { | |
| const displacement = x - swipeBackContextRef.current.x0; | |
| const displacementRatio = displacement / window.innerWidth; | |
| const velocity = displacement / (t - swipeBackContextRef.current.t0); | |
| setSwipeBackContext({ | |
| ...swipeBackContextRef.current, | |
| displacement, | |
| displacementRatio, | |
| velocity, | |
| }); | |
| setSwipeBackState((prev) => (prev === "swiping" ? prev : "swiping")); | |
| onSwipeBackMove?.({ displacement, displacementRatio }); | |
| }; | |
| const moveSwipeBack = ({ x, t }: MoveSwipeBackProps) => { | |
| const displacement = x - swipeBackContextRef.current.x0; | |
| const displacementRatio = displacement / window.innerWidth; | |
| const timeDelta = t - swipeBackContextRef.current.t0; | |
| const velocity = timeDelta > 0 ? displacement / timeDelta : 0; | |
| setSwipeBackContext({ | |
| ...swipeBackContextRef.current, | |
| displacement, | |
| displacementRatio, | |
| velocity, | |
| }); | |
| setSwipeBackState((prev) => (prev === "swiping" ? prev : "swiping")); | |
| onSwipeBackMove?.({ displacement, displacementRatio }); | |
| }; |
🤖 Prompt for AI Agents
In `@packages/stackflow/src/primitive/GlobalInteraction/useGlobalInteraction.ts`
around lines 163 - 175, The velocity computation in moveSwipeBack can divide by
zero when t === swipeBackContextRef.current.t0; modify moveSwipeBack to compute
a safe delta time (e.g., deltaT = t - t0) and if deltaT is 0 or extremely small,
use a fallback (0 or a tiny epsilon) before calculating velocity, then update
setSwipeBackContext with the guarded velocity and keep the rest of the logic
(references: moveSwipeBack, swipeBackContextRef.current.t0, setSwipeBackContext,
setSwipeBackState, onSwipeBackMove).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
전역 컨테이너에 CSS 변수를 매 프레임 갱신하던 구조를, 상단/뒤쪽 풀스크린 액티비티 루트로만 좁히도록 바꾼 게 핵심입니다. 변화점은 다음과 같습니다.
결과적으로 swipe move 중 스타일 재계산 범위가 “스택 전체” → “상단/뒤쪽 풀스크린 액티비티 루트”로 줄어들어 성능 이슈를 완화하도록 설계했습니다.
Summary by CodeRabbit
릴리스 노트