fix(webapp):> Fix for mouse hover on a timeline not being updates after resize #2780
Conversation
Refactor MousePositionProvider to improve reactivity during resize, scroll, and layout shifts.
|
WalkthroughThe Timeline component gained mouse-position tracking and layout-resync logic: Estimated code review effort🎯 4 (Complex) | ⏱️ ~30–60 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/webapp/app/components/primitives/Timeline.tsx (1)
22-23: Consider clearinglastClienton mouse leave.The refs are correctly used to store state that shouldn't trigger re-renders. However,
lastClient.currentretains the last mouse position even after the mouse exits the container. While not critical, clearing it in theonMouseLeavehandler would be more precise.Apply this diff to clear the ref on mouse leave:
<div ref={ref} onMouseEnter={handleMouseMove} - onMouseLeave={() => setPosition(undefined)} + onMouseLeave={() => { + setPosition(undefined); + lastClient.current = null; + }} onMouseMove={handleMouseMove} style={{ width: "100%", height: "100%" }} >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/webapp/app/components/primitives/Timeline.tsx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
Files:
apps/webapp/app/components/primitives/Timeline.tsx
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use zod for validation in packages/core and apps/webapp
Files:
apps/webapp/app/components/primitives/Timeline.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Files:
apps/webapp/app/components/primitives/Timeline.tsx
apps/webapp/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Access all environment variables through the
envexport ofenv.server.tsinstead of directly accessingprocess.envin the Trigger.dev webapp
Files:
apps/webapp/app/components/primitives/Timeline.tsx
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: When importing from@trigger.dev/corein the webapp, use subpath exports from the package.json instead of importing from the root path
Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webapp
Files:
apps/webapp/app/components/primitives/Timeline.tsx
**/*.{js,ts,jsx,tsx,json,md,css,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier
Files:
apps/webapp/app/components/primitives/Timeline.tsx
⏰ 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). (23)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (4)
apps/webapp/app/components/primitives/Timeline.tsx (4)
8-8: LGTM!The addition of
useEffectis necessary for the new resize/scroll handling and animation frame logic.
25-41: LGTM!The
computeFromClienthelper effectively centralizes position calculation logic. The boundary checks correctly prevent position updates when the cursor is outside the container.
43-49: LGTM!The updated mouse handler correctly stores the last client coordinates and delegates calculation to
computeFromClient.
52-75: LGTM!The resize and scroll handling effectively addresses the core issue. Using
ResizeObserverfor container changes and scroll capture for ancestor scrolling are appropriate choices. The cleanup logic properly disconnects all observers and listeners.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/webapp/app/components/primitives/Timeline.tsx (1)
14-17: Prefertypeoverinterfaceper coding guidelines.As per the project's coding guidelines for TypeScript files, use
typeinstead ofinterface.-interface MousePosition { - x: number; - y: number; -} +type MousePosition = { + x: number; + y: number; +};
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/webapp/app/components/primitives/Timeline.tsx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
Files:
apps/webapp/app/components/primitives/Timeline.tsx
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use zod for validation in packages/core and apps/webapp
Files:
apps/webapp/app/components/primitives/Timeline.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Files:
apps/webapp/app/components/primitives/Timeline.tsx
apps/webapp/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Access all environment variables through the
envexport ofenv.server.tsinstead of directly accessingprocess.envin the Trigger.dev webapp
Files:
apps/webapp/app/components/primitives/Timeline.tsx
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: When importing from@trigger.dev/corein the webapp, use subpath exports from the package.json instead of importing from the root path
Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webapp
Files:
apps/webapp/app/components/primitives/Timeline.tsx
**/*.{js,ts,jsx,tsx,json,md,css,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier
Files:
apps/webapp/app/components/primitives/Timeline.tsx
⏰ 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). (23)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (4)
apps/webapp/app/components/primitives/Timeline.tsx (4)
22-23: LGTM!Good use of refs for tracking mutable state without triggering re-renders.
25-41: LGTM!Clean implementation with proper bounds checking and stable callback reference.
43-49: LGTM!Properly stores last client coordinates for recalculation on layout changes.
51-75: LGTM!Correct use of
ResizeObserverand event listeners with proper cleanup. The capture phase for scroll events appropriately catches ancestor scrolls that affect the bounding rect.
| const isAnimating = () => { | ||
| if (!ref.current) return false; | ||
| const styles = window.getComputedStyle(ref.current); | ||
| return styles.transition !== "none" || styles.animation !== "none"; | ||
| }; |
There was a problem hiding this comment.
isAnimating() check doesn't detect active animations correctly.
getComputedStyle().transition returns the defined CSS transition value (e.g., "width 0.3s ease"), not whether a transition is currently running. If the element or its ancestors have any CSS transitions defined, this will always return true, causing the RAF loop to run perpetually while hovering.
Use the Web Animations API to detect active animations:
const isAnimating = () => {
if (!ref.current) return false;
- const styles = window.getComputedStyle(ref.current);
- return styles.transition !== "none" || styles.animation !== "none";
+ return ref.current.getAnimations({ subtree: false }).length > 0;
};Alternatively, consider listening to transitionstart/transitionend events to set a flag, which avoids polling entirely.
📝 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.
| const isAnimating = () => { | |
| if (!ref.current) return false; | |
| const styles = window.getComputedStyle(ref.current); | |
| return styles.transition !== "none" || styles.animation !== "none"; | |
| }; | |
| const isAnimating = () => { | |
| if (!ref.current) return false; | |
| return ref.current.getAnimations({ subtree: false }).length > 0; | |
| }; |
🤖 Prompt for AI Agents
In apps/webapp/app/components/primitives/Timeline.tsx around lines 80-84,
replace the current isAnimating() logic (which checks computed styles) with a
real runtime check using the Web Animations API: call
ref.current.getAnimations({subtree: true}) and return true if any animation's
playState === 'running'; if getAnimations is not available (older browsers),
fall back to adding transitionstart/animationstart and
transitionend/animationend listeners on the element to set/clear an internal
isAnimatingFlag and have isAnimating() return that flag. This ensures the RAF
loop only runs while animations are actually active.
Refactor MousePositionProvider to improve reactivity during resize, scroll, and layout shifts.
Closes #
✅ Checklist
Testing
In the runs page, after clicking on a segment that opens the side menu the vertical line updates to the new position without mouse movement.
Changelog
Mose hover vertical line on a timeline updates after timeline resize