Skip to content

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Feat/add spinning loader #308

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
96fb4fe
feat: Add spinning loader for AI token generation
google-labs-jules[bot] Sep 30, 2025
e8fa0ba
feat: Add spinning loader for AI token generation
google-labs-jules[bot] Sep 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions app/globals.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,3 +237,16 @@
.mapboxgl-ctrl-group button {
pointer-events: auto !important;
}

@keyframes spin-counter-clockwise {
from {
transform: rotate(360deg);
}
to {
transform: rotate(0deg);
}
}

.animate-spin-counter-clockwise {
animation: spin-counter-clockwise 1s linear infinite;
}
Comment on lines +241 to +252
Copy link

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.

Comment on lines +241 to +252
Copy link

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.

Comment on lines +241 to +252
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

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.

Suggested change
@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.

43 changes: 23 additions & 20 deletions app/layout.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { Toaster } from '@/components/ui/sonner'
import { MapToggleProvider } from '@/components/map-toggle-context'
import { ProfileToggleProvider } from '@/components/profile-toggle-context'
import { MapLoadingProvider } from '@/components/map-loading-context';
import { IsLoadingProvider } from '@/components/is-loading-provider';
import ConditionalLottie from '@/components/conditional-lottie';

const fontSans = FontSans({
Expand Down Expand Up @@ -54,26 +55,28 @@ export default function RootLayout({
return (
<html lang="en" suppressHydrationWarning>
<body className={cn('font-sans antialiased', fontSans.variable)}>
<MapToggleProvider>
<ProfileToggleProvider>
<ThemeProvider
attribute="class"
defaultTheme="earth"
enableSystem
disableTransitionOnChange
themes={['light', 'dark', 'earth']}
>
<MapLoadingProvider>
<Header />
<ConditionalLottie />
{children}
<Sidebar />
<Footer />
<Toaster />
</MapLoadingProvider>
</ThemeProvider>
</ProfileToggleProvider>
</MapToggleProvider>
<IsLoadingProvider>
<MapToggleProvider>
<ProfileToggleProvider>
<ThemeProvider
attribute="class"
defaultTheme="earth"
enableSystem
disableTransitionOnChange
themes={['light', 'dark', 'earth']}
>
<MapLoadingProvider>
<Header />
<ConditionalLottie />
{children}
<Sidebar />
<Footer />
<Toaster />
</MapLoadingProvider>
</ThemeProvider>
</ProfileToggleProvider>
</MapToggleProvider>
</IsLoadingProvider>
<Analytics />
<SpeedInsights />
</body>
Expand Down
19 changes: 18 additions & 1 deletion components/chat.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ import { ChatPanel } from './chat-panel'
import { ChatMessages } from './chat-messages'
import { EmptyScreen } from './empty-screen'
import { Mapbox } from './map/mapbox-map'
import { useUIState, useAIState } from 'ai/rsc'
import { useUIState, useAIState, useStreamableValue, type StreamableValue } from 'ai/rsc'
import { useIsLoading } from '@/components/is-loading-provider'
import type { UIState } from '@/app/actions'
import MobileIconsBar from './mobile-icons-bar'
import { useProfileToggle, ProfileToggleEnum } from "@/components/profile-toggle-context";
import SettingsView from "@/components/settings/settings-view";
Expand All @@ -17,6 +19,19 @@ type ChatProps = {
id?: string // This is the chatId
}

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;
}
Comment on lines +22 to +33
Copy link

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.

Comment on lines +22 to +33
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 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

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.

Suggested change
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.


export function Chat({ id }: ChatProps) {
const router = useRouter()
const path = usePathname()
Expand Down Expand Up @@ -75,6 +90,7 @@ export function Chat({ id }: ChatProps) {
if (isMobile) {
return (
<MapDataProvider> {/* Add Provider */}
<LoadingStateUpdater messages={messages} />
<div className="mobile-layout-container">
<div className="mobile-map-section">
{activeView ? <SettingsView /> : <Mapbox />}
Expand Down Expand Up @@ -104,6 +120,7 @@ export function Chat({ id }: ChatProps) {
// Desktop layout
return (
<MapDataProvider> {/* Add Provider */}
<LoadingStateUpdater messages={messages} />
<div className="flex justify-start items-start">
{/* This is the new div for scrolling */}
<div className="w-1/2 flex flex-col space-y-3 md:space-y-4 px-8 sm:px-12 pt-12 md:pt-14 pb-4 h-[calc(100vh-0.5in)] overflow-y-auto">
Expand Down
15 changes: 14 additions & 1 deletion components/header.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
'use client'

import React from 'react'
import Image from 'next/image'
import { useIsLoading } from './is-loading-provider'
import { ModeToggle } from './mode-toggle'
import { cn } from '@/lib/utils'
import HistoryContainer from './history-container'
Expand All @@ -15,6 +18,8 @@ import { MapToggle } from './map-toggle'
import { ProfileToggle } from './profile-toggle'

export const Header = () => {
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">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

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.

<div>
Expand All @@ -25,7 +30,15 @@ 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="h-6 w-auto" />
<Image
src="/images/logo.svg"
alt="Logo"
width={24}
height={24}
className={cn('h-6 w-auto', {
'animate-spin-counter-clockwise': isLoading
})}
/>
</Button>
</div>

Expand Down
30 changes: 30 additions & 0 deletions components/is-loading-provider.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
'use client'

import { createContext, useContext, useState, ReactNode } from 'react'

interface IsLoadingContextType {
isLoading: boolean
setIsLoading: (isLoading: boolean) => void
}
Comment on lines +5 to +8
Copy link
Contributor

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.


const IsLoadingContext = createContext<IsLoadingContextType | undefined>(
undefined
)

export function IsLoadingProvider({ children }: { children: ReactNode }) {
const [isLoading, setIsLoading] = useState(false)

return (
<IsLoadingContext.Provider value={{ isLoading, setIsLoading }}>
{children}
</IsLoadingContext.Provider>
)
}

export function useIsLoading() {
const context = useContext(IsLoadingContext)
if (context === undefined) {
throw new Error('useIsLoading must be used within an IsLoadingProvider')
}
return context
}