-
-
Couldn't load subscription status.
- Fork 6
Feat/add spinning loader #308
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
This commit introduces a spinning loader on the top-left plant icon to indicate when the AI is generating tokens. A new `IsLoadingProvider` is created to manage the loading state, which is updated from the `Chat` component based on the `isGenerating` streamable value. The `Header` component consumes this state to conditionally apply a counter-clockwise spinning animation.
This commit introduces a spinning loader on the top-left plant icon to indicate when the AI is generating tokens. A new `IsLoadingProvider` is created to manage the loading state, which is updated from the `Chat` component based on the `isGenerating` streamable value. The `Header` component consumes this state to conditionally apply a counter-clockwise spinning animation.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
|
WalkthroughAdds a global CSS counter-clockwise spin animation. Introduces an IsLoadingProvider and hook, wraps app layout with it, and uses the loading state to animate the header logo. Chat now updates loading status based on message generation and triggers a server action when map drawing data changes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Chat as Chat UI
participant Updater as LoadingStateUpdater
participant LoadCtx as IsLoadingProvider
participant Header as Header Logo
User->>Chat: Send prompt / observe messages
Chat-->>Updater: Last message isGenerating (stream)
Updater->>LoadCtx: setIsLoading(true/false)
LoadCtx-->>Header: isLoading context value
opt isLoading === true
Header->>Header: Apply .animate-spin-counter-clockwise
end
sequenceDiagram
autonumber
participant Chat as Chat UI
participant MapCtx as useMapData
participant Server as updateDrawingContext (server action)
Chat-->>MapCtx: drawnFeatures updated
MapCtx->>Server: updateDrawingContext(drawnFeatures)
Server-->>MapCtx: ack/updated context
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
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 |
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨Explore these optional code suggestions:
|
||||||||||||||
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.
- Potential stuck spinner if
Chatunmounts whileisLoadingis true; add an effect cleanup to reset the state. LoadingStateUpdateris duplicated across mobile and desktop branches; inlining the logic inChatreduces duplication and complexity.- The new animation does not respect
prefers-reduced-motion; add a media query override for accessibility. - Consider dynamically updating the logo
alt/titlewhen loading to improve assistive feedback.
Additional notes (1)
- Maintainability |
components/chat.tsx:93-93
LoadingStateUpdateris rendered in both the mobile and desktop branches, creating duplication and room for drift. You can inline the effect intoChatitself and remove the extra component entirely, simplifying control flow and reducing re-renders.
Summary of changes
- Added a global CSS counter-clockwise spin animation (
spin-counter-clockwise) and utility class.animate-spin-counter-clockwise. - Introduced a new
IsLoadingProviderReact context to share anisLoadingboolean across the app. - Wrapped the app in
IsLoadingProvider(inapp/layout.tsx) soHeaderandChatcan share loading state. - In
Chat, addedLoadingStateUpdaterthat derivesisLoadingfrom the latest message’sisGeneratingstreamable value and updates context. - Marked
Headeras a client component and conditionally added a spinning animation to the logo whenisLoadingis true.
| function LoadingStateUpdater({ messages }: { messages: UIState }) { | ||
| const { setIsLoading } = useIsLoading(); | ||
| const lastMessage = messages[messages.length - 1]; | ||
| const isGenerating = lastMessage?.isGenerating; | ||
| const [loading] = useStreamableValue(isGenerating); | ||
|
|
||
| useEffect(() => { | ||
| setIsLoading(loading || false); | ||
| }, [loading, setIsLoading]); | ||
|
|
||
| return null; | ||
| } |
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.
When Chat unmounts (e.g., on route changes), the isLoading state can remain true, leaving the header icon spinning indefinitely. Add a cleanup in the effect to reset loading on unmount to avoid a stuck spinner.
Suggestion
Consider adding a cleanup that resets the flag:
useEffect(() => {
setIsLoading(Boolean(loading));
return () => setIsLoading(false);
}, [loading, setIsLoading]);Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this change.
| @keyframes spin-counter-clockwise { | ||
| from { | ||
| transform: rotate(360deg); | ||
| } | ||
| to { | ||
| transform: rotate(0deg); | ||
| } | ||
| } | ||
|
|
||
| .animate-spin-counter-clockwise { | ||
| animation: spin-counter-clockwise 1s linear infinite; | ||
| } |
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.
To respect users’ reduced-motion preferences and improve accessibility, consider disabling the spin animation when prefers-reduced-motion: reduce is set.
Suggestion
Add a reduced-motion override:
@media (prefers-reduced-motion: reduce) {
.animate-spin-counter-clockwise {
animation: none;
}
}Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this CSS.
| @keyframes spin-counter-clockwise { | ||
| from { | ||
| transform: rotate(360deg); | ||
| } | ||
| to { | ||
| transform: rotate(0deg); | ||
| } | ||
| } | ||
|
|
||
| .animate-spin-counter-clockwise { | ||
| animation: spin-counter-clockwise 1s linear infinite; | ||
| } |
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.
The keyframes use from { transform: rotate(360deg); } to { transform: rotate(0deg); }. Because 0deg and 360deg are equivalent, some browsers may normalize angles and produce no visible animation or inconsistent direction. For a robust counter-clockwise spin, explicitly rotate from 0deg to -360deg. Also consider honoring prefers-reduced-motion to avoid unnecessary motion for sensitive users.
Suggestion
Consider changing the keyframes to rotate from 0deg to -360deg and add a reduced-motion rule:
@keyframes spin-counter-clockwise {
from { transform: rotate(0deg); }
to { transform: rotate(-360deg); }
}
.animate-spin-counter-clockwise {
animation: spin-counter-clockwise 1s linear infinite;
}
@media (prefers-reduced-motion: reduce) {
.animate-spin-counter-clockwise {
animation: none;
}
}Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this suggestion.
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.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/chat.tsx (1)
82-87: Debounce and harden server action calls on drawnFeatures changes.Calling
updateDrawingContexton every reference change can spam the server; also avoid unhandled rejections and remove console noise.Apply inside the effect:
- useEffect(() => { - if (id && mapData.drawnFeatures && mapData.drawnFeatures.length > 0) { - console.log('Chat.tsx: drawnFeatures changed, calling updateDrawingContext', mapData.drawnFeatures); - updateDrawingContext(id, mapData.drawnFeatures); - } - }, [id, mapData.drawnFeatures]); + useEffect(() => { + if (!id || !mapData.drawnFeatures || mapData.drawnFeatures.length === 0) return; + const key = JSON.stringify(mapData.drawnFeatures); + if (key === lastFeaturesKeyRef.current) return; + lastFeaturesKeyRef.current = key; + void updateDrawingContext(id, mapData.drawnFeatures).catch(console.error); + }, [id, mapData.drawnFeatures]);And add once above the effect:
// Track last sent payload to avoid duplicate calls const lastFeaturesKeyRef = useRef<string | null>(null);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
app/globals.css(1 hunks)app/layout.tsx(2 hunks)components/chat.tsx(4 hunks)components/header.tsx(3 hunks)components/is-loading-provider.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
app/globals.css (1)
components/ui/spinner.tsx (1)
svg(6-19)
components/is-loading-provider.tsx (1)
components/map-loading-context.tsx (3)
children(11-18)MapLoadingContextType(4-7)context(20-26)
components/header.tsx (2)
components/is-loading-provider.tsx (1)
useIsLoading(24-30)lib/utils/index.ts (1)
cn(11-13)
app/layout.tsx (2)
components/is-loading-provider.tsx (1)
IsLoadingProvider(14-22)components/header.tsx (1)
Header(20-79)
components/chat.tsx (2)
app/actions.tsx (1)
UIState(292-297)components/is-loading-provider.tsx (1)
useIsLoading(24-30)
🔇 Additional comments (2)
components/header.tsx (1)
21-21: LGTM: clean integration of loading state into header logo.Conditional class application via cn is correct and minimal.
Also applies to: 33-41
app/layout.tsx (1)
58-79: LGTM: Provider wiring is correct and orders Header under IsLoadingProvider.No issues spotted with composition.
| @keyframes spin-counter-clockwise { | ||
| from { | ||
| transform: rotate(360deg); | ||
| } | ||
| to { | ||
| transform: rotate(0deg); | ||
| } | ||
| } | ||
|
|
||
| .animate-spin-counter-clockwise { | ||
| animation: spin-counter-clockwise 1s linear infinite; | ||
| } |
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.
Respect prefers-reduced-motion for the new spinner.
Add a reduced‑motion override to avoid continuous rotation for users who request less motion.
@keyframes spin-counter-clockwise {
from {
transform: rotate(360deg);
}
to {
transform: rotate(0deg);
}
}
.animate-spin-counter-clockwise {
animation: spin-counter-clockwise 1s linear infinite;
}
+
+@media (prefers-reduced-motion: reduce) {
+ .animate-spin-counter-clockwise {
+ animation: none;
+ }
+}📝 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.
| @keyframes spin-counter-clockwise { | |
| from { | |
| transform: rotate(360deg); | |
| } | |
| to { | |
| transform: rotate(0deg); | |
| } | |
| } | |
| .animate-spin-counter-clockwise { | |
| animation: spin-counter-clockwise 1s linear infinite; | |
| } | |
| @keyframes spin-counter-clockwise { | |
| from { | |
| transform: rotate(360deg); | |
| } | |
| to { | |
| transform: rotate(0deg); | |
| } | |
| } | |
| .animate-spin-counter-clockwise { | |
| animation: spin-counter-clockwise 1s linear infinite; | |
| } | |
| @media (prefers-reduced-motion: reduce) { | |
| .animate-spin-counter-clockwise { | |
| animation: none; | |
| } | |
| } |
🤖 Prompt for AI Agents
In app/globals.css around lines 241 to 252, the new spin-counter-clockwise
animation does not respect users' prefers-reduced-motion setting; add a
prefers-reduced-motion override so users requesting reduced motion do not see
continuous rotation by adding a CSS media query @media (prefers-reduced-motion:
reduce) that disables the animation for .animate-spin-counter-clockwise (set
animation: none or animation-duration: 0s and remove animation-iteration-count)
so the spinner remains static for those users.
| function LoadingStateUpdater({ messages }: { messages: UIState }) { | ||
| const { setIsLoading } = useIsLoading(); | ||
| const lastMessage = messages[messages.length - 1]; | ||
| const isGenerating = lastMessage?.isGenerating; | ||
| const [loading] = useStreamableValue(isGenerating); | ||
|
|
||
| useEffect(() => { | ||
| setIsLoading(loading || false); | ||
| }, [loading, setIsLoading]); | ||
|
|
||
| return null; | ||
| } |
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.
🧩 Analysis chain
Ensure cleanup so the global loading indicator never gets stuck.
When Chat unmounts during generation, isLoading can remain true. Reset on unmount.
useEffect(() => {
- setIsLoading(loading || false);
- }, [loading, setIsLoading]);
+ setIsLoading(Boolean(loading));
+ return () => setIsLoading(false);
+ }, [loading, setIsLoading]);If useStreamableValue from ai/rsc receives undefined, does it safely return [undefined] without throwing? If not, guard against an undefined isGenerating before calling it.
🌐 Web query:
Does the Vercel AI SDK (`ai/rsc`) hook `useStreamableValue` accept `undefined` safely? Link to docs/source.
💡 Result:
Short answer: No — useStreamableValue expects a StreamableValue created by createStreamableValue and does not accept undefined safely. The docs state the hook "takes a streamable value created using createStreamableValue" (so pass the streamable.value), and the RSC utilities/type code shows the streamable value shape is required. See the docs and implementation/discussion for details. [1][2]
Sources
- useStreamableValue docs. [1] https://sdk.vercel.ai/docs/reference/ai-sdk-rsc/use-streamable-value
- readStreamableValue / StreamableValue discussion (implementation/type details). [2] How to read streamable value on Server Actions? vercel/ai#3719
Guard streamable value and reset loading on unmount
Wrap useStreamableValue to avoid passing undefined, and add an unmount cleanup:
function LoadingStateUpdater({ messages }: { messages: UIState }) {
const { setIsLoading } = useIsLoading();
const lastMessage = messages[messages.length - 1];
const isGenerating = lastMessage?.isGenerating;
- const [loading] = useStreamableValue(isGenerating);
+ const [loading] = isGenerating != null
+ ? useStreamableValue(isGenerating)
+ : [false];
useEffect(() => {
- setIsLoading(loading || false);
- }, [loading, setIsLoading]);
+ setIsLoading(loading);
+ return () => setIsLoading(false);
+ }, [loading, setIsLoading]);
return null;
}📝 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.
| function LoadingStateUpdater({ messages }: { messages: UIState }) { | |
| const { setIsLoading } = useIsLoading(); | |
| const lastMessage = messages[messages.length - 1]; | |
| const isGenerating = lastMessage?.isGenerating; | |
| const [loading] = useStreamableValue(isGenerating); | |
| useEffect(() => { | |
| setIsLoading(loading || false); | |
| }, [loading, setIsLoading]); | |
| return null; | |
| } | |
| function LoadingStateUpdater({ messages }: { messages: UIState }) { | |
| const { setIsLoading } = useIsLoading(); | |
| const lastMessage = messages[messages.length - 1]; | |
| const isGenerating = lastMessage?.isGenerating; | |
| const [loading] = isGenerating != null | |
| ? useStreamableValue(isGenerating) | |
| : [false]; | |
| useEffect(() => { | |
| setIsLoading(loading); | |
| return () => setIsLoading(false); | |
| }, [loading, setIsLoading]); | |
| return null; | |
| } |
🤖 Prompt for AI Agents
In components/chat.tsx around lines 22 to 33, the call to useStreamableValue can
receive undefined and the effect never resets loading on unmount; change the
input to a guarded boolean (e.g. !!lastMessage?.isGenerating) so
useStreamableValue never gets undefined, and add a cleanup to the useEffect that
calls setIsLoading(false) on unmount (and keep setIsLoading dependency) so
loading is reset when the component unmounts.
| const { isLoading } = useIsLoading() | ||
|
|
||
| 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"> |
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.
Fix a11y: non-functional Button and expose loading state to ATs.
- The icon Button has no onClick/href, creating a focusable control that does nothing.
- Expose loading via aria-busy to announce to assistive tech.
Apply:
@@
- <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">
+ <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"
+ aria-busy={isLoading}
+ >
@@
- <Button variant="ghost" size="icon">
- <Image
+ <Button variant="ghost" size="icon" asChild>
+ <a href="/" aria-label="Home">
+ <Image
src="/images/logo.svg"
alt="Logo"
width={24}
height={24}
className={cn('h-6 w-auto', {
'animate-spin-counter-clockwise': isLoading
})}
- />
- </Button>
+ />
+ </a>
+ </Button>Also applies to: 32-42
🤖 Prompt for AI Agents
In components/header.tsx around lines 24 and 32-42, the icon Button is focusable
but has no onClick/href and the component doesn't expose loading state to ATs;
either make the control functional (add an onClick handler or href and forward
it from props) or if it's purely decorative convert it to a non-interactive
element (e.g., span/div) and remove button semantics/keyboard focus;
additionally expose the loading state via aria-busy (on the actionable element
or a containing region) and/or aria-disabled when appropriate, and forward a
loading prop so assistive tech can detect busy status.
| interface IsLoadingContextType { | ||
| isLoading: boolean | ||
| setIsLoading: (isLoading: boolean) => void | ||
| } |
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.
🛠️ Refactor suggestion | 🟠 Major
Broaden setIsLoading type to match React’s setState.
Current type blocks functional updates. Use Dispatch<SetStateAction>.
-import { createContext, useContext, useState, ReactNode } from 'react'
+import { createContext, useContext, useState, ReactNode, type Dispatch, type SetStateAction } from 'react'
@@
interface IsLoadingContextType {
isLoading: boolean
- setIsLoading: (isLoading: boolean) => void
+ setIsLoading: Dispatch<SetStateAction<boolean>>
}Also applies to: 15-15
🤖 Prompt for AI Agents
In components/is-loading-provider.tsx around lines 5 to 8 (and also at line 15),
the setIsLoading type is currently (isLoading: boolean) => void which prevents
functional updates; change the type to
React.Dispatch<React.SetStateAction<boolean>> and import Dispatch and
SetStateAction (or use React.Dispatch/React.SetStateAction) from React, then
update the useState typing/assignment at line 15 to use the same
Dispatch<SetStateAction<boolean>> type so callers can pass either a boolean or
an updater function.
PR Type
Enhancement
Description
Add spinning loader animation to plant icon during AI generation
Create global loading state management with React context
Integrate loading state with AI streaming responses
Apply counter-clockwise spinning animation with CSS keyframes
Diagram Walkthrough
File Walkthrough
globals.css
Add counter-clockwise spinning animation stylesapp/globals.css
animate-spin-counter-clockwiseutility classlayout.tsx
Integrate loading provider in app layoutapp/layout.tsx
IsLoadingProvidercontextchat.tsx
Add loading state tracking for AI responsescomponents/chat.tsx
LoadingStateUpdatercomponent to monitor AI generationuseStreamableValueto track streaming stateheader.tsx
Add spinning animation to header logocomponents/header.tsx
useIsLoadinghook to consume loading contextis-loading-provider.tsx
Create loading state context providercomponents/is-loading-provider.tsx
Summary by CodeRabbit