feat: add simple pull-to-refresh to chat history sidebar #368
Conversation
📝 WalkthroughWalkthroughAdds cross-platform pull-to-refresh to the chat history by accepting an external Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Nav as Sidebar (nav / historyContainerRef)
participant Chat as ChatHistoryList (containerRef)
participant API as ConversationsAPI
participant UI as Indicator (RefreshCw)
Note over User,Nav: User pulls down on the sidebar/nav container
User->>Nav: pointer/touch down + drag
Nav->>Chat: pointer events observed via containerRef
Chat->>UI: update pullDistance (dampened transform / opacity)
alt pullDistance < threshold
UI-->>User: show partial indicator (translate / opacity)
else pullDistance ≥ threshold
Chat->>UI: set isPullRefreshing (start spinner)
Chat->>API: trigger refresh / poll conversations
API-->>Chat: return updated conversations
Chat->>UI: clear refreshing & animate release
UI-->>User: refreshed list visible
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
Deploying maple with
|
| Latest commit: |
9d8f01f
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://d298104c.maple-ca8.pages.dev |
| Branch Preview URL: | https://feat-simple-pull-to-refresh.maple-ca8.pages.dev |
Greptile SummaryThis PR simplifies pull-to-refresh by replacing the complex platform-specific approach from PR #367 with a unified touch/mouse drag implementation that works consistently across all platforms. Key Changes:
Previous Issues Addressed:
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant Container as Sidebar Container
participant ChatHistoryList
participant PTR as Pull-to-Refresh Handler
participant API as pollForUpdates()
Note over User,API: Touch/Mouse Pull-to-Refresh Flow
User->>Container: touchstart/mousedown (at top)
Container->>PTR: handleTouchStart/handleMouseDown
PTR->>PTR: Clear long-press timer
PTR->>PTR: Set isPulling=true, pullStartY
User->>Container: touchmove/mousemove (drag down)
Container->>PTR: handleTouchMove/handleMouseMove
PTR->>PTR: Calculate distance with resistance
PTR->>ChatHistoryList: Update pull indicator & content transform
Note over ChatHistoryList: Visual feedback: icon rotates, content shifts down
User->>Container: touchend/mouseup
Container->>PTR: handleTouchEnd/handleMouseUp
PTR->>PTR: Set isPulling=false
alt distance > 60px threshold
PTR->>PTR: Set isRefreshing=true
PTR->>ChatHistoryList: Animate to 60px, show spinner
PTR->>API: Call pollForUpdates()
API->>API: Fetch latest 20 conversations
API->>ChatHistoryList: Merge with existing conversations
API-->>PTR: Complete
PTR->>ChatHistoryList: Reset to 0px with transition
PTR->>PTR: Set isRefreshing=false
else distance <= 60px
PTR->>ChatHistoryList: Animate back to 0px
end
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
frontend/src/components/ChatHistoryList.tsx (2)
184-199: Consider cleanup for the setTimeout on unmount.If the component unmounts while refreshing, the
setTimeoutat line 193 will still execute and attempt to update state. While React 18 handles this gracefully, you could store the timeout ID in a ref and clear it on cleanup to avoid potential warnings.♻️ Optional fix to handle unmount during refresh
+ const refreshTimeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null); + const handleRefresh = useCallback(async () => { isRefreshingRef.current = true; setIsPullRefreshing(true); try { await pollForUpdates(); } catch (error) { console.error("Refresh failed:", error); } finally { - setTimeout(() => { + refreshTimeoutRef.current = setTimeout(() => { setIsPullRefreshing(false); setPullDistance(0); isRefreshingRef.current = false; }, 300); } }, [pollForUpdates]); + + // Add cleanup in a useEffect + useEffect(() => { + return () => { + if (refreshTimeoutRef.current) { + clearTimeout(refreshTimeoutRef.current); + } + }; + }, []);
240-245: Consider checking for left mouse button only.
handleMouseDowndoesn't verify which mouse button was pressed. Right-click (context menu) or middle-click could unintentionally start a pull gesture.♻️ Proposed fix to check for primary button
const handleMouseDown = (e: MouseEvent) => { + // Only handle left mouse button (button 0) + if (e.button !== 0) return; if (container.scrollTop === 0 && !isRefreshingRef.current) { pullStartY.current = e.clientY; isPulling.current = true; } };
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
frontend/src/components/ChatHistoryList.tsxfrontend/src/components/Sidebar.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use path aliases (@/*maps to./src/*) for imports in TypeScript/React files
Use 2-space indentation, double quotes, and enforce 100-character line limit in TypeScript/React code
Maintain strict TypeScript and avoid usinganytype
Use PascalCase for component names and camelCase for variables and function names
Use functional components with React hooks instead of class components
Use React context for global state management and TanStack Query for server state management
Runjust format,just lint, andjust buildafter making TypeScript/React changes to ensure code quality and compilation
Files:
frontend/src/components/Sidebar.tsxfrontend/src/components/ChatHistoryList.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). (6)
- GitHub Check: Greptile Review
- GitHub Check: build-linux
- GitHub Check: build-macos (universal-apple-darwin)
- GitHub Check: build-ios
- GitHub Check: build-android
- GitHub Check: Cloudflare Pages
🔇 Additional comments (5)
frontend/src/components/Sidebar.tsx (1)
106-106: LGTM!The
historyContainerRefis correctly created and wired to the<nav>element, then passed down toChatHistoryListfor pull-to-refresh gesture detection. The type assertions are necessary to satisfy TypeScript's strict type checking betweenMutableRefObjectandRefObject.Also applies to: 257-260, 269-269
frontend/src/components/ChatHistoryList.tsx (4)
9-10: LGTM!The new imports (
CheckSquare,RefreshCw) are appropriately used, and the optionalcontainerRefprop enables pull-to-refresh without breaking existing consumers.Also applies to: 35-35, 62-63
85-91: Good pattern for gesture state management.Using refs (
pullDistanceRef,isRefreshingRef) alongside state avoids stale closure issues in event handlers while still triggering re-renders when needed.
273-291: Event listener management looks solid.Good use of
passive: falsefortouchmoveto enablepreventDefault(), and attachingmousemove/mouseuptowindowensures the drag gesture works even when the cursor leaves the container.
706-722: Nice pull-to-refresh indicator implementation.The indicator provides good visual feedback: height and opacity scale with pull distance, the icon rotates during pull to indicate progress, and switches to
animate-spinonce refreshing. The opacity reaching 1.0 at the 60px threshold nicely signals the trigger point.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
frontend/src/components/ChatHistoryList.tsx (1)
710-726: LGTM! Nice visual feedback for pull-to-refresh.The indicator provides clear visual feedback with smooth transitions, progressive rotation during pull, and spin animation during refresh.
For enhanced accessibility, consider adding a keyboard shortcut (e.g.,
Cmd/Ctrl+R) or a refresh button in the UI to allow keyboard-only users to trigger a manual refresh. Not required for this PR, but worth considering for inclusive design.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/src/components/ChatHistoryList.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use path aliases (@/*maps to./src/*) for imports in TypeScript/React files
Use 2-space indentation, double quotes, and enforce 100-character line limit in TypeScript/React code
Maintain strict TypeScript and avoid usinganytype
Use PascalCase for component names and camelCase for variables and function names
Use functional components with React hooks instead of class components
Use React context for global state management and TanStack Query for server state management
Runjust format,just lint, andjust buildafter making TypeScript/React changes to ensure code quality and compilation
Files:
frontend/src/components/ChatHistoryList.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). (6)
- GitHub Check: Greptile Review
- GitHub Check: build-macos (universal-apple-darwin)
- GitHub Check: build-linux
- GitHub Check: build-android
- GitHub Check: build-ios
- GitHub Check: Cloudflare Pages
🔇 Additional comments (5)
frontend/src/components/ChatHistoryList.tsx (5)
9-10: LGTM! Clean import and prop additions.The RefreshCw icon import and optional containerRef prop are well-typed and integrate cleanly with the existing component structure.
Also applies to: 35-35
85-91: LGTM! Appropriate state management.Good separation between state (for UI updates) and refs (for tracking without re-renders). This prevents unnecessary render cycles during gesture tracking.
184-199: LGTM! Solid refresh handler.The function correctly gates concurrent refreshes with
isRefreshingRef, reuses the existingpollForUpdatesflow, and ensures UI state resets after 300ms regardless of outcome.
201-295: Excellent implementation of pull-to-refresh gesture handling.The previous review concerns have been successfully addressed:
- Primary button check prevents activation on right/middle clicks (line 241)
- Interactive element filtering prevents interference with buttons, links, and inputs (line 244)
preventDefault()on mouse move prevents text selection during drag (line 257)The unified touch/mouse approach is clean, with proper passive flags for performance, window-level mouse listeners for dragging outside the container, and thorough cleanup.
221-222: Consider verifying the pull threshold in user testing.The 0.4 resistance factor combined with the 60px threshold means users must pull ~150px raw distance to trigger refresh. While this prevents accidental refreshes, it may feel stiff on mobile devices.
Consider testing with users to ensure the gesture feels natural. If the threshold proves too high, you could:
- Increase resistance factor to 0.5 (requires 120px raw pull), or
- Lower threshold to 50px (requires 125px raw pull)
Also applies to: 232-232, 258-259, 269-269
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
frontend/src/components/ChatHistoryList.tsx (1)
184-199: Clean refresh implementation.The refresh flow correctly manages state and prevents concurrent refreshes using
isRefreshingRef. The 300ms delay provides smooth visual feedback.♻️ Optional: Extract magic number to constant
For maintainability, consider extracting the 300ms delay:
+const REFRESH_ANIMATION_DELAY = 300; + const handleRefresh = useCallback(async () => { isRefreshingRef.current = true; setIsPullRefreshing(true); try { await pollForUpdates(); } catch (error) { console.error("Refresh failed:", error); } finally { setTimeout(() => { setIsPullRefreshing(false); setPullDistance(0); isRefreshingRef.current = false; - }, 300); + }, REFRESH_ANIMATION_DELAY); } }, [pollForUpdates]);
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/src/components/ChatHistoryList.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use path aliases (@/*maps to./src/*) for imports in TypeScript/React files
Use 2-space indentation, double quotes, and enforce 100-character line limit in TypeScript/React code
Maintain strict TypeScript and avoid usinganytype
Use PascalCase for component names and camelCase for variables and function names
Use functional components with React hooks instead of class components
Use React context for global state management and TanStack Query for server state management
Runjust format,just lint, andjust buildafter making TypeScript/React changes to ensure code quality and compilation
Files:
frontend/src/components/ChatHistoryList.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). (5)
- GitHub Check: build-android
- GitHub Check: build-ios
- GitHub Check: build-linux
- GitHub Check: build-macos (universal-apple-darwin)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (6)
frontend/src/components/ChatHistoryList.tsx (6)
9-10: LGTM! Clean import additions for pull-to-refresh.The RefreshCw icon import is appropriate for the pull-to-refresh indicator.
35-35: Good prop design for external container control.The optional
containerRefprop enables external components to provide a scroll container reference for pull-to-refresh, maintaining good component boundaries.
85-91: Excellent state management approach.Good use of state for UI-driven values (
isPullRefreshing,pullDistance) and refs for tracking gesture state without triggering re-renders. This is a performant pattern.
206-246: Excellent touch event implementation with proper long-press handling.The touch handlers correctly:
- Cancel the long-press timer when pull gesture starts (lines 208-211, 225-228) — this addresses the previous concern about conflicts between pull-to-refresh and long-press selection
- Use
preventDefaultduring active pulls to prevent scrolling interference (line 224)- Apply resistance factor (0.4) and maximum distance (80px) for natural pull feedback
- Check scroll position to only activate at the top
248-291: Well-implemented mouse drag handlers with proper safeguards.The mouse handlers correctly address previous review concerns:
- preventDefault added: Line 269 prevents text selection during pull (previously missing)
- Long-press cancellation: Lines 253-256 and 270-273 cancel the long-press timer when pull gesture starts or continues
- Interactive element filtering: Line 252 prevents activation on buttons, links, and inputs
The implementation uses the primary mouse button check (line 249) and matches the touch handler behavior with consistent resistance factor and thresholds.
726-742: Polished pull-to-refresh indicator with great visual feedback.The indicator implementation provides excellent UX:
- Opacity gradually increases to 1.0 at the 60px threshold (line 732)
- Icon rotates proportionally to pull distance (
pullDistance * 3), giving immediate feedback- Switches to spin animation when actively refreshing
- Uses performant CSS transforms
|
@TestFlight build |
|
🚀 TestFlight deployment triggered! Check the Actions tab for progress. |
|
✅ TestFlight deployment completed successfully! |
Adds pull-to-refresh functionality using a unified approach: - Works on all platforms (mobile, desktop, web) with same behavior - Touch events for mobile devices - Mouse click-and-drag for desktop/web - Visual feedback with animated refresh icon - 60px pull threshold with resistance factor Fixes #366 Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
- Skip interactive elements (buttons, links, inputs, menu items) - Only trigger on primary mouse button - Prevent text selection while dragging Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
Prevents conflict between pull-to-refresh and long-press selection mode by canceling the long-press timer when a pull gesture begins. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
AnthonyRonning
force-pushed
the
feat/simple-pull-to-refresh
branch
from
ace6665 to
b77cf05
Compare
|
@TestFlight build |
|
🚀 TestFlight deployment triggered! Check the Actions tab for progress. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @frontend/src/components/ChatHistoryList.tsx:
- Around line 248-291: handleMouseMove/handleMouseUp fail to reset pullDistance
when the user moves back up past the start, which can leave
pullDistanceRef.current > 0 and wrongly trigger handleRefresh; in
handleMouseMove (function handleMouseMove) add the same upward-movement guard
used for touch handlers: when computed distance <= 0 or currentY <=
pullStartY.current, set pullDistanceRef.current = 0 and call setPullDistance(0)
(and clear any longPressTimerRef.current if present), and in handleMouseUp
ensure you also reset pullDistanceRef.current = 0 and setPullDistance(0) before
returning so no stale distance remains to trigger handleRefresh.
- Around line 206-246: Add a touchcancel handler and ensure pullDistance is
reset when the finger moves back above the start: implement handleTouchCancel
that clears longPressTimerRef, sets isPulling.current = false, sets
pullDistanceRef.current = 0 and calls setPullDistance(0) (mirroring
handleTouchEnd cleanup), and register it alongside handleTouchStart/Move/End in
the event listeners and cleanup. In handleTouchMove, when computing distance, if
distance <= 0 immediately reset pullDistanceRef.current = 0 and call
setPullDistance(0) (and clear any longPressTimerRef) so upward movement clears
the visual indicator and prevents triggering refresh based on peak distance;
keep the existing resistanceFactor, adjustedDistance clamp, and the existing
check for isRefreshingRef.current and container.scrollTop === 0. Ensure all
references use the existing refs/vars: handleTouchStart, handleTouchMove,
handleTouchEnd, longPressTimerRef, pullStartY, isPulling, pullDistanceRef,
setPullDistance, handleRefresh, and isRefreshingRef.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
frontend/src/components/ChatHistoryList.tsxfrontend/src/components/Sidebar.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use path aliases (@/*maps to./src/*) for imports in TypeScript/React files
Use 2-space indentation, double quotes, and enforce 100-character line limit in TypeScript/React code
Maintain strict TypeScript and avoid usinganytype
Use PascalCase for component names and camelCase for variables and function names
Use functional components with React hooks instead of class components
Use React context for global state management and TanStack Query for server state management
Runjust format,just lint, andjust buildafter making TypeScript/React changes to ensure code quality and compilation
Files:
frontend/src/components/ChatHistoryList.tsxfrontend/src/components/Sidebar.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). (5)
- GitHub Check: build-linux
- GitHub Check: build-macos (universal-apple-darwin)
- GitHub Check: build-android
- GitHub Check: build-ios
- GitHub Check: Greptile Review
🔇 Additional comments (9)
frontend/src/components/ChatHistoryList.tsx (6)
9-10: LGTM!The RefreshCw icon import is appropriate for the pull-to-refresh indicator.
35-35: LGTM!The optional
containerRefprop is a clean API extension that enables external control of the scroll container for pull-to-refresh functionality.
85-91: LGTM!The pull-to-refresh state management appropriately uses React state for values that drive rendering (isPullRefreshing, pullDistance) and refs for tracking gesture state without triggering re-renders.
184-199: LGTM!The refresh handler properly reuses the existing
pollForUpdatesmechanism and includes appropriate error handling and a visual feedback delay before clearing the refresh state.
293-311: Event listener setup looks good overall.The event listener configuration is appropriate:
- Touch events use correct passive flags
- Mouse move/up events on
windowproperly track drags that leave the container- Cleanup function removes all listeners
However, this is affected by the missing
touchcancelhandler issue flagged in the previous comment.
734-750: LGTM!The pull-to-refresh indicator provides excellent visual feedback:
- Height and opacity scale with pull distance
- Smooth transitions with
transition-all duration-200- Appropriate animation states: rotation during pull, spin during refresh
frontend/src/components/Sidebar.tsx (3)
106-106: LGTM!The
historyContainerRefis correctly typed and initialized for forwarding to the ChatHistoryList component.
257-260: LGTM!The
historyContainerRefis correctly attached to the nav element, which is the appropriate scroll container (withoverflow-y-auto) for the pull-to-refresh functionality.
269-269: LGTM!The
containerRefprop is correctly forwarded to ChatHistoryList, completing the ref forwarding pattern that enables pull-to-refresh functionality.
|
✅ TestFlight deployment completed successfully! |
Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
|
@TestFlight build |
|
🚀 TestFlight deployment triggered! Check the Actions tab for progress. |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @frontend/src/components/ChatHistoryList.tsx:
- Around line 275-303: The pull distance peak isn't cleared when the user moves
back up, so handleTouchMove must detect distance <= 0 and reset the pull state:
when distance <= 0 (and container.scrollTop <= 0) call setPullDistancePx(0) and
set pullDistanceRef.current = 0 (and optionally set isPulling.current = false)
to clear the stale peak; keep the existing longPressTimerRef and isRefreshingRef
guards. Also ensure handleTouchEnd uses the live pullDistanceRef.current value
(not a stale captured value) when deciding to call handleRefresh.
- Around line 320-348: The mouse move handler (handleMouseMove) currently only
updates pullDistance when distance>0, so if the user moves back above the start
point the peak value remains in pullDistanceRef and can falsely trigger a
refresh; update handleMouseMove to set pullDistance to 0 (via
setPullDistancePx(0)) and clear any longPressTimer when distance <= 0 (and/or
when container.scrollTop > 0) so pullDistanceRef.current is reset to the final
position, ensuring handleMouseUp's check of pullDistanceRef.current uses the
actual final distance; keep references to isPulling, pullStartY,
pullDistanceRef, and setPullDistancePx when implementing this fix.
- Around line 305-318: The pull-to-refresh activates too easily and clicks on
plain divs (conversation titles) can start a pull; update handleMouseDown to
ignore non-touch interactions on text/divs by checking target.closest for
div/textbox (or simply restrict mouse-based initiation), and add a minimum drag
threshold (e.g., 10px) to the gesture logic: when setting pullStartY in
handleMouseDown keep current behavior but ensure handleTouchMove and
handleMouseMove compute delta = currentY - pullStartY.current and only set
isPulling.current = true and call setPullDistancePx(delta) when delta >= 10;
also preserve clearing longPressTimerRef as before. Ensure references:
handleMouseDown, handleTouchMove, handleMouseMove, pullStartY, isPulling,
setPullDistancePx, longPressTimerRef.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
frontend/src/components/ChatHistoryList.tsxfrontend/src/components/Sidebar.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use path aliases (@/*maps to./src/*) for imports in TypeScript/React files
Use 2-space indentation, double quotes, and enforce 100-character line limit in TypeScript/React code
Maintain strict TypeScript and avoid usinganytype
Use PascalCase for component names and camelCase for variables and function names
Use functional components with React hooks instead of class components
Use React context for global state management and TanStack Query for server state management
Runjust format,just lint, andjust buildafter making TypeScript/React changes to ensure code quality and compilation
Files:
frontend/src/components/ChatHistoryList.tsxfrontend/src/components/Sidebar.tsx
🧬 Code graph analysis (1)
frontend/src/components/ChatHistoryList.tsx (2)
frontend/src/components/ui/checkbox.tsx (1)
Checkbox(26-26)frontend/src/components/ui/dropdown-menu.tsx (4)
DropdownMenu(170-170)DropdownMenuTrigger(171-171)DropdownMenuContent(172-172)DropdownMenuItem(173-173)
⏰ 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). (6)
- GitHub Check: Greptile Review
- GitHub Check: build-android
- GitHub Check: build-ios
- GitHub Check: build-linux
- GitHub Check: build-macos (universal-apple-darwin)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (10)
frontend/src/components/Sidebar.tsx (3)
106-106: LGTM!The ref is correctly typed and initialized.
257-257: LGTM!The ref is correctly attached to the scrollable nav container.
266-266: LGTM!The containerRef prop is correctly forwarded to enable pull-to-refresh functionality.
frontend/src/components/ChatHistoryList.tsx (7)
9-10: LGTM!Icon imports are correctly added for the new pull-to-refresh and selection features.
35-35: LGTM!The containerRef prop is correctly typed as optional, maintaining backward compatibility.
85-94: LGTM!The ref-based approach for pull distance tracking correctly avoids re-render jank during gestures, which is essential for smooth iOS performance.
96-147: LGTM!The imperative styling approach with RAF scheduling and GPU-accelerated transforms provides smooth pull-to-refresh animations without triggering React re-renders.
241-256: LGTM!The refresh handler correctly integrates with the existing
pollForUpdatesmechanism and provides appropriate user feedback.
793-806: LGTM!The pull indicator is correctly implemented with proper accessibility attributes and performance hints.
808-982: LGTM!The content wrapper and rendering logic correctly integrate pull-to-refresh while preserving all existing functionality.
|
✅ TestFlight deployment completed successfully! |
Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
frontend/src/components/ChatHistoryList.tsx (1)
237-253: Consider cleanup for async operations.The
setTimeoutcallback at lines 247-251 could execute after the component unmounts. While React 18 handles this gracefully (no crash), consider adding a cleanup pattern for a cleaner implementation.♻️ Optional: Add cleanup for setTimeout
const handleRefresh = useCallback(async () => { + let cancelled = false; isRefreshingRef.current = true; setIsPullRefreshing(true); setPullDistancePx(60, { transition: true }); try { await pollForUpdates(); } catch (error) { console.error("Refresh failed:", error); } finally { setTimeout(() => { + if (cancelled) return; setIsPullRefreshing(false); isRefreshingRef.current = false; setPullDistancePx(0, { transition: true }); }, 300); } + return () => { cancelled = true; }; }, [pollForUpdates, setPullDistancePx]);Note: This is a minor refinement; the current implementation works fine in practice.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/src/components/ChatHistoryList.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use path aliases (@/*maps to./src/*) for imports in TypeScript/React files
Use 2-space indentation, double quotes, and enforce 100-character line limit in TypeScript/React code
Maintain strict TypeScript and avoid usinganytype
Use PascalCase for component names and camelCase for variables and function names
Use functional components with React hooks instead of class components
Use React context for global state management and TanStack Query for server state management
Runjust format,just lint, andjust buildafter making TypeScript/React changes to ensure code quality and compilation
Files:
frontend/src/components/ChatHistoryList.tsx
🧬 Code graph analysis (1)
frontend/src/components/ChatHistoryList.tsx (2)
frontend/src/components/ui/checkbox.tsx (1)
Checkbox(26-26)frontend/src/components/ui/dropdown-menu.tsx (4)
DropdownMenu(170-170)DropdownMenuTrigger(171-171)DropdownMenuContent(172-172)DropdownMenuItem(173-173)
⏰ 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). (6)
- GitHub Check: Greptile Review
- GitHub Check: build-ios
- GitHub Check: build-macos (universal-apple-darwin)
- GitHub Check: build-linux
- GitHub Check: build-android
- GitHub Check: Cloudflare Pages
🔇 Additional comments (8)
frontend/src/components/ChatHistoryList.tsx (8)
9-11: LGTM!The new imports for
CheckSquareandRefreshCware appropriately added for the pull-to-refresh and selection features.
35-35: LGTM!The optional
containerRefprop is well-typed and provides a clean API for attaching pull-to-refresh to an external scrolling container.
85-94: LGTM!Well-structured refs for imperative gesture handling. Using refs instead of state for the pull gesture avoids unnecessary re-renders during the drag, and the RAF ref enables proper batching of style updates.
96-147: LGTM!The imperative style management is well-architected:
- Uses
translate3dfor GPU-accelerated animations- RAF batching prevents layout thrashing
- Proper cleanup of animation frames on unmount
- The transition option in
setPullDistancePxprovides smooth snapping
201-229: LGTM!The refactored
pollForUpdateslogic correctly addresses chat history ordering:
- Preserves server ordering as the source of truth for the latest page
- Maintains referential identity for unchanged items to minimize re-renders
- Retains paginated older items not in the latest response
- The equality check at line 224-226 is a good optimization to avoid unnecessary state updates
347-367: LGTM!Event listener setup is well-implemented:
- Correct use of
passive: falsefortouchmoveto allowpreventDefault()touchcancelhandler reuseshandleTouchEndappropriatelymousemove/mouseuponwindowenables proper drag tracking outside the container- All listeners properly cleaned up on effect teardown
790-803: LGTM!The pull indicator is well-structured:
- Positioned absolutely with
pointer-events-noneto not interfere with interactionsaria-hidden="true"for proper accessibilitywillChangehints optimize for the animated properties- Initial hidden state (opacity 0, transform -60px) prevents flash on mount
805-817: LGTM!The content wrapper properly uses
willChange: transformfor animation performance, and theselect-noneclass on conversation items helps prevent text selection during pull gestures.
Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
|
@TestFlight build |
|
🚀 TestFlight deployment triggered! Check the Actions tab for progress. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @frontend/src/components/ChatHistoryList.tsx:
- Around line 279-307: The pull distance peak isn't cleared when the user moves
back above the start, so update handleTouchMove to set pullDistance to 0 when
distance <= 0: when calculating distance in handleTouchMove (inside the block
that checks container.scrollTop), add an else branch that calls
setPullDistancePx(0) and also sets pullDistanceRef.current = 0 (and clear any
longPressTimerRef there as needed) so pullDistanceRef always matches the visible
state; keep existing logic for adjustedDistance when distance > 0 and ensure
handleTouchEnd continues to read pullDistanceRef.current to decide refresh.
🧹 Nitpick comments (1)
frontend/src/components/ChatHistoryList.tsx (1)
244-260: Refresh handler implementation is correct.The 300ms delay before resetting provides good visual feedback. Consider adding user-visible error feedback (e.g., toast notification) on refresh failure rather than just logging.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/src/components/ChatHistoryList.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use path aliases (@/*maps to./src/*) for imports in TypeScript/React files
Use 2-space indentation, double quotes, and enforce 100-character line limit in TypeScript/React code
Maintain strict TypeScript and avoid usinganytype
Use PascalCase for component names and camelCase for variables and function names
Use functional components with React hooks instead of class components
Use React context for global state management and TanStack Query for server state management
Runjust format,just lint, andjust buildafter making TypeScript/React changes to ensure code quality and compilation
Files:
frontend/src/components/ChatHistoryList.tsx
🧬 Code graph analysis (1)
frontend/src/components/ChatHistoryList.tsx (2)
frontend/src/components/ui/checkbox.tsx (1)
Checkbox(26-26)frontend/src/components/ui/dropdown-menu.tsx (4)
DropdownMenu(170-170)DropdownMenuTrigger(171-171)DropdownMenuContent(172-172)DropdownMenuItem(173-173)
⏰ 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). (6)
- GitHub Check: Greptile Review
- GitHub Check: build-android
- GitHub Check: build-macos (universal-apple-darwin)
- GitHub Check: build-linux
- GitHub Check: build-ios
- GitHub Check: Cloudflare Pages
🔇 Additional comments (10)
frontend/src/components/ChatHistoryList.tsx (10)
27-36: Props interface looks good.The optional
containerRefprop is correctly typed and allows the parent component to provide a container element for pull-to-refresh event handling.
86-95: Good use of refs for imperative animation.Using refs for pull state (
isPulling,pullDistanceRef,isRefreshingRef) avoids unnecessary re-renders during the gesture, whilepullRafRefcorrectly gates RAF scheduling to prevent duplicate frames.
97-148: Well-structured imperative animation logic.The RAF-gated scheduling, GPU-accelerated transforms (
translate3d), and conditional transitions provide smooth pull-to-refresh animations without React re-render overhead.
203-236: Solid deduplication logic with server-ordered merging.The algorithm correctly maintains server ordering for the latest page while preserving older paginated items. The referential equality optimization (line 216-218) only compares
title—if other metadata fields matter, they should be included in the comparison.
354-374: Event listener setup is correct.The
passive: falseontouchmoveenablespreventDefault(), and attachingmousemove/mouseuptowindowcorrectly tracks the cursor outside the container. The cleanup is complete.
390-435: Good guard against concurrent pagination requests.The
isLoadingMoreRefprevents duplicate API calls when the IntersectionObserver fires rapidly, and the deduplication logic ensures clean merging.
811-824: Pull indicator UI is well-structured.Good use of
aria-hidden="true"for the decorative indicator, GPU-optimized withwillChange, and theanimate-spinclass provides clear visual feedback during refresh.
826-918: Conversation list rendering is clean.The
select-noneclass on items prevents accidental text selection during gestures, and the conditional refs/handlers are correctly applied based on mode and position.
920-999: Loading and archived sections implemented correctly.The loading indicator provides clear pagination feedback, and archived chats correctly omit selection mode support as specified in the issue requirements.
324-352: Same upward movement bug in mouse handlers.The mouse handlers have the identical issue: moving back above the starting point doesn't reset
pullDistanceRef, which can cause incorrect refresh triggers.Likely an incorrect or invalid review comment.
|
✅ TestFlight deployment completed successfully! |
Implements a simplified pull-to-refresh for the chat history list in the sidebar.
Changes
Why this approach?
This is a simplified rewrite of PR #367 which had become too complex with platform-specific behaviors (wheel events for desktop, different handlers for mobile). The complexity led to scroll-related bugs on web.
This version uses one unified approach: click/touch and drag down. It works consistently everywhere and is much easier to maintain.
Fixes #366
Summary by CodeRabbit
New Features
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.