Add counter-clockwise spinning plant icon during token generation #357
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,7 @@ | |
| import React from 'react' | ||
| import Image from 'next/image' | ||
| import { useCalendarToggle } from './calendar-toggle-context' | ||
| import { useStreaming } from './streaming-context' | ||
| import { ModeToggle } from './mode-toggle' | ||
| import { cn } from '@/lib/utils' | ||
| import HistoryContainer from './history-container' | ||
|
|
@@ -18,6 +19,7 @@ import { ProfileToggle } from './profile-toggle' | |
|
|
||
| export const Header = () => { | ||
| const { toggleCalendar } = useCalendarToggle() | ||
| const { isStreaming } = useStreaming() | ||
|
There was a problem hiding this comment.
SuggestionMove
'use client'
import { StreamingProvider } from '@/components/streaming-context'
export function Providers({ children }: { children: React.ReactNode }) {
return <StreamingProvider>{children}</StreamingProvider>
}
import { Providers } from './providers'
import { Header } from '@/components/header'
export default function RootLayout({ children }: { children: React.ReactNode }) {
return (
<html lang="en">
<body>
<Providers>
<Header />
{children}
</Providers>
</body>
</html>
)
}Then delete the Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this refactor. |
||
| return ( | ||
| <header className="fixed w-full p-1 md:p-2 flex justify-between items-center z-10 backdrop-blur md:backdrop-blur-none bg-background/80 md:bg-transparent"> | ||
| <div> | ||
|
|
@@ -28,7 +30,16 @@ export const Header = () => { | |
|
|
||
| <div className="absolute left-1"> | ||
| <Button variant="ghost" size="icon"> | ||
| <Image | ||
| src="/images/logo.svg" | ||
| alt="Logo" | ||
| width={24} | ||
| height={24} | ||
| className={cn( | ||
| "h-6 w-auto transition-transform duration-1000", | ||
| isStreaming && "animate-spin-ccw" | ||
| )} | ||
| /> | ||
| </Button> | ||
| </div> | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -7,9 +7,16 @@ import remarkGfm from 'remark-gfm' | |||||||||||||||||
| import remarkMath from 'remark-math' | ||||||||||||||||||
| import rehypeKatex from 'rehype-katex' | ||||||||||||||||||
| import 'katex/dist/katex.min.css' | ||||||||||||||||||
| import { useStreaming } from './streaming-context' | ||||||||||||||||||
| import { useEffect } from 'react' | ||||||||||||||||||
|
|
||||||||||||||||||
| export function BotMessage({ content }: { content: StreamableValue<string> }) { | ||||||||||||||||||
| const [data, error, pending] = useStreamableValue(content) | ||||||||||||||||||
| const { setIsStreaming } = useStreaming() | ||||||||||||||||||
|
|
||||||||||||||||||
| useEffect(() => { | ||||||||||||||||||
| setIsStreaming(pending) | ||||||||||||||||||
| }, [pending, setIsStreaming]) | ||||||||||||||||||
|
Comment on lines
+15
to
+19
There was a problem hiding this comment. Using a single boolean SuggestionRefactor the streaming context to track a reference count and expose
export interface StreamingContextType {
isStreaming: boolean
start: () => void
stop: () => void
}
export const StreamingProvider = ({ children }: { children: React.ReactNode }) => {
const [count, setCount] = useState(0)
const start = () => setCount(c => c + 1)
const stop = () => setCount(c => Math.max(0, c - 1))
return (
<StreamingContext.Provider value={{ isStreaming: count > 0, start, stop }}>
{children}
</StreamingContext.Provider>
)
}
const { start, stop } = useStreaming()
useEffect(() => {
if (pending) start()
else stop()
return () => { if (pending) stop() }
}, [pending, start, stop])Reply with "@CharlieHelps yes please" if you'd like me to add a commit implementing this safer ref-count approach.
Comment on lines
+17
to
+19
Contributor
There was a problem hiding this comment. 🧹 Nitpick | 🔵 Trivial Approve with suggestion: Add explanatory comment. The implementation correctly tracks streaming state by relying on stable Consider adding a comment explaining the behavior: + // Update global streaming state when this message's streaming status changes.
+ // Only the newest message will have pending=true, older messages remain pending=false.
useEffect(() => {
setIsStreaming(pending)
}, [pending, setIsStreaming])📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||
|
|
||||||||||||||||||
| // Currently, sometimes error occurs after finishing the stream. | ||||||||||||||||||
| if (error) return <div>Error</div> | ||||||||||||||||||
|
|
||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| 'use client' | ||
|
|
||
| import { createContext, useContext, useState, ReactNode } from 'react' | ||
|
|
||
| interface StreamingContextType { | ||
| isStreaming: boolean | ||
| setIsStreaming: (streaming: boolean) => void | ||
| } | ||
|
|
||
| const StreamingContext = createContext<StreamingContextType | undefined>(undefined) | ||
|
|
||
| export const useStreaming = () => { | ||
| const context = useContext(StreamingContext) | ||
| if (!context) { | ||
| // Return default values if used outside provider (e.g., during SSR) | ||
| return { isStreaming: false, setIsStreaming: () => {} } | ||
| } | ||
| return context | ||
| } | ||
|
|
||
| export const StreamingProvider = ({ children }: { children: ReactNode }) => { | ||
| const [isStreaming, setIsStreaming] = useState(false) | ||
|
|
||
| return ( | ||
| <StreamingContext.Provider value={{ isStreaming, setIsStreaming }}> | ||
| {children} | ||
| </StreamingContext.Provider> | ||
| ) | ||
| } | ||
|
Comment on lines
+21
to
+29
Contributor
There was a problem hiding this comment. 🧹 Nitpick | 🔵 Trivial Memoize the context value to prevent unnecessary re-renders. The context value object is recreated on every render, causing all consumers (Header, BotMessage) to re-render even when Apply this diff to memoize the value: -import { createContext, useContext, useState, ReactNode } from 'react'
+import { createContext, useContext, useState, ReactNode, useMemo } from 'react'
export const StreamingProvider = ({ children }: { children: ReactNode }) => {
const [isStreaming, setIsStreaming] = useState(false)
+ const value = useMemo(
+ () => ({ isStreaming, setIsStreaming }),
+ [isStreaming]
+ )
+
return (
- <StreamingContext.Provider value={{ isStreaming, setIsStreaming }}>
+ <StreamingContext.Provider value={value}>
{children}
</StreamingContext.Provider>
)
}🤖 Prompt for AI Agents |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Placing
StreamingProviderinsideChatmeansHeader(which usesuseStreaming) is very likely outside the provider, leading to runtime errors and/or state desynchronization. After lifting the provider toapp/layout.tsx(or another common root), these nested providers should be removed to prevent multiple isolated contexts that won’t share state.Suggestion
Remove the
StreamingProviderwrappers from both the mobile and desktop branches ofChatonce the provider is placed at the app root. For example:Reply with "@CharlieHelps yes please" if you'd like me to add a commit that removes these wrappers.