Conversation
- Remove gap between animation loops (200ms restart delay vs 12s interval) - Add animation for all 3 table rows in Recent Activity section - Each row gets selected with crosshair, shows component name, and copies - Animation flows continuously: Export → MetricCard → StatValue → SignupRow → OrderRow → PaymentRow → repeat Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
- Use for..of with entries() instead of index-based loop - Remove unnecessary null check (we control the data) - Inline initializeAnimationLoop into direct runAnimationLoop call - Simplify ref callback syntax Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
|
Cursor Agent can help with this pull request. Just |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
@react-grab/cli
grab
@react-grab/ami
@react-grab/amp
@react-grab/claude-code
@react-grab/codex
@react-grab/cursor
@react-grab/droid
@react-grab/gemini
@react-grab/opencode
react-grab
@react-grab/relay
@react-grab/utils
commit: |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR successfully achieves continuous animation and table row selections. Found one critical race condition with visibility change handling that could cause multiple concurrent animation loops. The implementation is otherwise solid.
| const handleVisibilityChange = (): void => { | ||
| if (document.visibilityState === "visible") { | ||
| isCancelled = true; | ||
| clearInterval(animationInterval); | ||
| resetAnimationState(); | ||
| setTimeout(initializeAnimationLoop, 100); | ||
| isCancelled = false; | ||
| setTimeout(runAnimationLoop, 100); | ||
| } |
There was a problem hiding this comment.
Critical race condition: When the tab becomes visible, isCancelled is set to true (line 556), then immediately back to false (line 558), but the existing runAnimationLoop from line 564 might not have exited yet since the cancellation check happens at discrete await points. This could result in multiple concurrent animation loops running simultaneously.
The previous interval-based approach avoided this because clearInterval immediately stopped scheduling new executions.
Fix: Wait for the current loop to fully exit before starting a new one. Consider:
const handleVisibilityChange = (): void => {
if (document.visibilityState === "visible") {
isCancelled = true;
resetAnimationState();
// Wait for current loop to exit at next cancellation check
setTimeout(() => {
isCancelled = false;
runAnimationLoop();
}, 200);
}
};| for (const [i, rowPos] of activityRowPositions.current.entries()) { | ||
| if (isCancelled) return; | ||
|
|
||
| const rowCenter = getElementCenter(rowPos); | ||
| setCursorPos(rowCenter); | ||
| await wait(350); | ||
| if (isCancelled) return; | ||
|
|
||
| setSelectionBox(createSelectionBox(rowPos, SELECTION_PADDING_PX)); | ||
| displaySelectionLabel( | ||
| rowPos.x + rowPos.width / 2, | ||
| rowPos.y + rowPos.height + 10, | ||
| ACTIVITY_DATA[i].component, |
There was a problem hiding this comment.
Array access without bounds check: If activityRowPositions.current.length doesn't match ACTIVITY_DATA.length (e.g., due to measurement timing or filtering out null refs), accessing ACTIVITY_DATA[i] at line 533 could fail or use the wrong index.
Since activityRowPositions is filtered from activityRowRefs (lines 326-336), if any refs are null during measurement, the arrays will be misaligned.
Suggestion: Use the filtered data structure directly or add a bounds check:
if (i >= ACTIVITY_DATA.length) continue;Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
- Table rows now show comment input UI instead of grab/copy - Simulates typing into 'Add context' field - Shows submit button and 'Sent' confirmation - Matches react-grab comment mode interaction Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
- Remove slice(0, -1) to include all three activity rows in animation - Remove duplicate runAnimationLoop call to prevent concurrent animations
|
Bugbot Autofix prepared fixes for 2 of the 2 bugs found in the latest run.
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 1 file
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/website/components/mobile-demo-animation.tsx">
<violation number="1" location="packages/website/components/mobile-demo-animation.tsx:558">
P1: Race condition in cancellation logic. Toggling `isCancelled` to `true` and then immediately back to `false` synchronously will fail to stop the previous async loop.
When `executeAnimationSequence` is paused at an `await`, it will only check `isCancelled` after the promise resolves. By that time, `handleVisibilityChange` has already reset it to `false`, so the old loop sees `false` and continues running. This results in two competing animation loops.
**Fix**: Use a `ref` object (e.g., `useRef({ id: 0 })`) to track the current "run ID" and pass it to the animation sequence, or ensure `isCancelled` remains true for the duration of the old loop's potential wake-up (which is difficult to time). A ref-based token is the most robust solution.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| clearInterval(animationInterval); | ||
| resetAnimationState(); | ||
| setTimeout(initializeAnimationLoop, 100); | ||
| isCancelled = false; |
There was a problem hiding this comment.
P1: Race condition in cancellation logic. Toggling isCancelled to true and then immediately back to false synchronously will fail to stop the previous async loop.
When executeAnimationSequence is paused at an await, it will only check isCancelled after the promise resolves. By that time, handleVisibilityChange has already reset it to false, so the old loop sees false and continues running. This results in two competing animation loops.
Fix: Use a ref object (e.g., useRef({ id: 0 })) to track the current "run ID" and pass it to the animation sequence, or ensure isCancelled remains true for the duration of the old loop's potential wake-up (which is difficult to time). A ref-based token is the most robust solution.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/website/components/mobile-demo-animation.tsx, line 558:
<comment>Race condition in cancellation logic. Toggling `isCancelled` to `true` and then immediately back to `false` synchronously will fail to stop the previous async loop.
When `executeAnimationSequence` is paused at an `await`, it will only check `isCancelled` after the promise resolves. By that time, `handleVisibilityChange` has already reset it to `false`, so the old loop sees `false` and continues running. This results in two competing animation loops.
**Fix**: Use a `ref` object (e.g., `useRef({ id: 0 })`) to track the current "run ID" and pass it to the animation sequence, or ensure `isCancelled` remains true for the duration of the old loop's potential wake-up (which is difficult to time). A ref-based token is the most robust solution.</comment>
<file context>
@@ -499,35 +518,53 @@ export const MobileDemoAnimation = (): ReactElement => {
- clearInterval(animationInterval);
resetAnimationState();
- setTimeout(initializeAnimationLoop, 100);
+ isCancelled = false;
+ setTimeout(runAnimationLoop, 100);
}
</file context>
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
| setCursorPos(rowCenter); | ||
| await wait(350); | ||
| if (isCancelled) return; | ||
|
|
There was a problem hiding this comment.
Race condition in handleVisibilityChange allows concurrent animation loops to run simultaneously when tab visibility changes
| const runAnimationLoop = async (): Promise<void> => { | ||
| while (!isCancelled) { | ||
| await executeAnimationSequence(); | ||
| } |
There was a problem hiding this comment.
Animation restart has a race condition when tab becomes visible - old and new animation loops execute concurrently and call setState from overlapping animation sequences
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/website/components/mobile-demo-animation.tsx">
<violation number="1" location="packages/website/components/mobile-demo-animation.tsx:548">
P2: Potential index mismatch: if any `activityRowPositions` entries are null or filtered out, the index `i` from `.entries()` won't align correctly with `ACTIVITY_DATA[i]`. Consider iterating with the actual position data that includes its corresponding activity data, or ensure both arrays are guaranteed to have matching indices.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
- Fix Bug 1: Preserve original array indices in activityRowPositions to prevent mismatch with ACTIVITY_DATA when refs are null
- Fix Bug 2: Track and display correct text ('Copied' vs 'Sent') during label fade-out animation
|
Bugbot Autofix prepared fixes for 2 of the 2 bugs found in the latest run.
|
|
Bugbot Autofix prepared fixes for 1 of the 1 bugs found in the latest run.
|
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/website/components/mobile-demo-animation.tsx">
<violation number="1" location="packages/website/components/mobile-demo-animation.tsx:633">
P2: Starting a new animation loop on visibility change without waiting for the existing loop to stop can create multiple concurrent loops because the cancellation flag is reset to false before the prior loop observes it. This can cause overlapping animations and extra state updates after returning to the tab.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| resetAnimationState(); | ||
| setTimeout(initializeAnimationLoop, 100); | ||
| isCancelled = false; | ||
| runAnimationLoop(); |
There was a problem hiding this comment.
P2: Starting a new animation loop on visibility change without waiting for the existing loop to stop can create multiple concurrent loops because the cancellation flag is reset to false before the prior loop observes it. This can cause overlapping animations and extra state updates after returning to the tab.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/website/components/mobile-demo-animation.tsx, line 633:
<comment>Starting a new animation loop on visibility change without waiting for the existing loop to stop can create multiple concurrent loops because the cancellation flag is reset to false before the prior loop observes it. This can cause overlapping animations and extra state updates after returning to the tab.</comment>
<file context>
@@ -630,6 +630,7 @@ export const MobileDemoAnimation = (): ReactElement => {
isCancelled = true;
resetAnimationState();
isCancelled = false;
+ runAnimationLoop();
}
};
</file context>
Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/website/components/mobile-demo-animation.tsx">
<violation number="1" location="packages/website/components/mobile-demo-animation.tsx:569">
P2: The activity-row animation now skips the final row because the loop stops at `length - 1`. This prevents the PaymentRow from ever being selected/animated.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| await simulateClickAndCopy(valuePos); | ||
| if (isCancelled) return; | ||
|
|
||
| for (let i = 0; i < ACTIVITY_DATA.length - 1; i++) { |
There was a problem hiding this comment.
P2: The activity-row animation now skips the final row because the loop stops at length - 1. This prevents the PaymentRow from ever being selected/animated.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/website/components/mobile-demo-animation.tsx, line 569:
<comment>The activity-row animation now skips the final row because the loop stops at `length - 1`. This prevents the PaymentRow from ever being selected/animated.</comment>
<file context>
@@ -566,7 +566,7 @@ export const MobileDemoAnimation = (): ReactElement => {
if (isCancelled) return;
- for (let i = 0; i < ACTIVITY_DATA.length; i++) {
+ for (let i = 0; i < ACTIVITY_DATA.length - 1; i++) {
if (isCancelled) return;
const rowPos = activityRowPositions.current[i];
</file context>
| for (let i = 0; i < ACTIVITY_DATA.length - 1; i++) { | |
| for (let i = 0; i < ACTIVITY_DATA.length; i++) { |
- ExportBtn: comment 'add CSV option' - MetricCard: comment 'show graph' - StatValue: comment 'format as USD' - SignupRow: comment 'add avatar' - OrderRow: grab/copy (last interaction) Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
| setSuccessFlash(HIDDEN_BOX); | ||
| await fadeOutSelectionLabel(); | ||
| setCursorType("crosshair"); | ||
| await wait(300); |
There was a problem hiding this comment.
Dead code: drag selection feature never used
Low Severity
The animateDragSelection function is defined but never called - the code that previously invoked it was removed. Related state (dragBox, isDragging) is declared and reset but never actually used: dragBox.visible is always false so its render block never executes, and isDragging is only set to false. This dead code should be removed.
Additional Locations (1)
2 participants
Make the mobile demo animation continuous and include table row selections.
Previously, the animation had a 12-second pause between loops and did not animate the "Recent Activity" table rows. This change removes the pause and extends the animation to sequentially select and "copy" each table row.
Summary by cubic
Made the mobile demo animation continuous and switched most interactions to comment input/submit. The loop no longer pauses and steps through Export → Metric card → Stat value → Signup (comment) → Order (copy), with clear labels and reliable restarts.
New Features
Bug Fixes
Written for commit dbde78d. Summary will update on new commits.
Note
Low Risk
UI-only animation/visual state changes with no backend or data-handling impact; main risk is timing/loop-cancellation edge cases affecting the demo’s smoothness.
Overview
Makes the
MobileDemoAnimationloop continuously (replacing the fixed 12s interval) and improves restart behavior on tab visibility changes.Extends the scripted interactions to measure and target
Recent Activityrows, adds a new “commenting” flow (typed text, submit icon, “Sent” confirmation), and refines label rendering/positioning (supports above/below placement and correct fade-out text for “Copied” vs “Sent”).Written by Cursor Bugbot for commit dbde78d. This will update automatically on new commits. Configure here.