fix: resolve thread dropdown flickering on open and navigation - #10
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes flicker/glitching in the thread history dropdown by adjusting fetch timing, animation behavior, focus handling, and navigation/toggle guards.
Changes:
- Fetch threads on open while preventing overlapping enter/exit animations (
AnimatePresence mode="wait"+ keyed dropdown). - Reset search/delete-confirm state on open and defer focusing the search input.
- Add navigation/toggle guards and avoid navigating when selecting the currently active thread.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Fetch on open, reset search | ||
| useEffect(() => { | ||
| if (isOpen) fetchThreads() | ||
| if (isOpen) { | ||
| setSearch("") | ||
| setConfirmDeleteId(null) | ||
| fetchThreads() | ||
| // Focus search input after render | ||
| requestAnimationFrame(() => searchInputRef.current?.focus()) | ||
| } | ||
| }, [isOpen, fetchThreads]) | ||
|
|
||
| // Debounced search | ||
| // Debounced search (only when user types, not on open) | ||
| const isFirstRender = useRef(true) | ||
| useEffect(() => { | ||
| if (!isOpen) return | ||
| if (!isOpen) { isFirstRender.current = true; return } | ||
| // Skip the first render (handled by the open effect above) | ||
| if (isFirstRender.current) { isFirstRender.current = false; return } | ||
| const timer = setTimeout(() => fetchThreads(search || undefined), 300) | ||
| return () => clearTimeout(timer) | ||
| }, [search, isOpen, fetchThreads]) |
There was a problem hiding this comment.
The setSearch("") inside the open effect can still trigger a second fetch via the debounced search effect when the previous search value was non-empty. On open: you fetch immediately, then setSearch("") causes a re-render where isFirstRender is already false, so the debounce schedules fetchThreads(undefined) again after 300ms.
To avoid reintroducing duplicate requests, consider moving the search/reset logic into handleToggle (before setting isOpen true) so the open render already has search === "", or add a dedicated “skip next search effect” ref that’s set when programmatically resetting search and consumed by the debounced effect.
| if (isOpen) { | ||
| setSearch("") | ||
| setConfirmDeleteId(null) | ||
| fetchThreads() | ||
| // Focus search input after render | ||
| requestAnimationFrame(() => searchInputRef.current?.focus()) | ||
| } |
There was a problem hiding this comment.
requestAnimationFrame(() => searchInputRef.current?.focus()) isn’t canceled on close/unmount. If the dropdown is opened and immediately closed (or the component unmounts during navigation), the queued callback may run after the input is removed, which can lead to inconsistent focus behavior.
Consider storing the RAF id and calling cancelAnimationFrame in the effect cleanup (or gate the focus with an isOpen/mounted ref).
| if (isOpen) { | |
| setSearch("") | |
| setConfirmDeleteId(null) | |
| fetchThreads() | |
| // Focus search input after render | |
| requestAnimationFrame(() => searchInputRef.current?.focus()) | |
| } | |
| if (!isOpen) return | |
| setSearch("") | |
| setConfirmDeleteId(null) | |
| fetchThreads() | |
| // Focus search input after render | |
| const rafId = requestAnimationFrame(() => searchInputRef.current?.focus()) | |
| return () => cancelAnimationFrame(rafId) |
| // Close immediately without animation to prevent flash during navigation | ||
| isNavigatingRef.current = true | ||
| setIsOpen(false) | ||
| router.push(`/chat?thread=${threadId}`) | ||
| // Reset navigation flag after transition | ||
| setTimeout(() => { isNavigatingRef.current = false }, 500) | ||
| } |
There was a problem hiding this comment.
The navigation guard uses a fixed setTimeout(..., 500) to clear isNavigatingRef. This is brittle: route transitions can be faster/slower than 500ms, and it can also leave a dangling timer on unmount.
A more robust approach is to clear the flag based on an actual navigation completion signal (e.g., useEffect that resets when currentThreadId changes, or when route/search params change), and clear any pending timeout in cleanup if you keep the timer.
| setIsOpen(false) | ||
| return | ||
| } | ||
| // Close immediately without animation to prevent flash during navigation |
There was a problem hiding this comment.
The inline comment says “Close immediately without animation to prevent flash during navigation”, but the dropdown still exits via AnimatePresence (now opacity-only). This can be misleading when debugging future flicker issues.
Consider updating the comment to reflect the actual behavior (e.g., “close before navigation” or “minimize exit animation”).
| // Close immediately without animation to prevent flash during navigation | |
| // Close before navigation to minimize visible flicker during the route change |
…p, use navigation signal
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
Fixes visual glitching in the thread history dropdown where it would flash/flicker when opening or selecting a thread.
mode="wait"to AnimatePresence to prevent enter/exit animation overlapautoFocuswith deferred ref-based focus to avoid layout thrashTest Plan