Skip to content

Refactor: Move map analysis button to navigation bars #318

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

Closed
ngoiyaeric wants to merge 3 commits into from
Closed

Refactor: Move map analysis button to navigation bars #318

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9b92a09
Refactor: Move map analysis button to navigation bars
google-labs-jules[bot] Oct 7, 2025
658073c
Refactor: Move map analysis button to navigation bars
google-labs-jules[bot] Oct 7, 2025
2412eea
Refactor: Move map analysis button to navigation bars
google-labs-jules[bot] Oct 7, 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
18 changes: 12 additions & 6 deletions app/layout.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ 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 { MapProvider } from '@/components/map/map-context';
import { AnalysisToolProvider } from '@/components/analysis-tool-context';
import ConditionalLottie from '@/components/conditional-lottie';

const fontSans = FontSans({
Expand Down Expand Up @@ -64,12 +66,16 @@ export default function RootLayout({
themes={['light', 'dark', 'earth']}
>
<MapLoadingProvider>
<Header />
<ConditionalLottie />
{children}
<Sidebar />
<Footer />
<Toaster />
<MapProvider>
<AnalysisToolProvider>
<Header />
<ConditionalLottie />
{children}
<Sidebar />
<Footer />
<Toaster />
</AnalysisToolProvider>
</MapProvider>
</MapLoadingProvider>
</ThemeProvider>
</ProfileToggleProvider>
Expand Down
46 changes: 26 additions & 20 deletions components/analysis-tool.tsx → components/analysis-tool-context.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,25 @@
'use client'

import React, { useState } from 'react'
'use client'

Comment on lines 1 to +4
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Oct 7, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Remove duplicate 'use client' directive.

The 'use client' directive appears twice (lines 1 and 3), which is unnecessary and should be removed.

Apply this diff to remove the duplicate:

 'use client'
 
-'use client'
-
 import React, { createContext, useContext, useState, ReactNode } from 'react'
📝 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
'use client'
import React, { useState } from 'react'
'use client'
'use client'
import React, { createContext, useContext, useState, ReactNode } from 'react'
🤖 Prompt for AI Agents 
In components/analysis-tool-context.tsx around lines 1 to 4, there are two
identical 'use client' directives; remove the duplicate so only a single 'use
client' remains at the top of the file to avoid redundancy.

import React, { createContext, useContext, useState, ReactNode } from 'react'
import { useActions, useUIState } from 'ai/rsc'
import { AI } from '@/app/actions'
import { Button } from '@/components/ui/button'
import { Search } from 'lucide-react'
import { useMap } from './map/map-context'
import { nanoid } from 'nanoid'
import { UserMessage } from './user-message'
import { toast } from 'react-toastify'

export function AnalysisTool() {
type AnalysisToolContextType = {
isAnalyzing: boolean
handleResolutionSearch: () => void
}

const AnalysisToolContext = createContext<AnalysisToolContextType | undefined>(
undefined
)

export const AnalysisToolProvider = ({ children }: { children: ReactNode }) => {
const { map } = useMap()
const { submit } = useActions()
const [, setMessages] = useUIState<typeof AI>()
Expand Down Expand Up @@ -57,21 +66,18 @@ export function AnalysisTool() {
}

return (
<div className="absolute top-4 right-4 z-20">
<Button
variant="outline"
size="icon"
onClick={handleResolutionSearch}
disabled={isAnalyzing || !map}
title="Analyze current map view"
className="bg-background/80 backdrop-blur-sm hover:bg-background"
>
{isAnalyzing ? (
<div className="h-5 w-5 animate-spin rounded-full border-b-2 border-current"></div>
) : (
<Search className="h-5 w-5" />
)}
</Button>
</div>
<AnalysisToolContext.Provider
value={{ isAnalyzing, handleResolutionSearch }}
>
{children}
</AnalysisToolContext.Provider>
)
}

export const useAnalysisTool = (): AnalysisToolContextType => {
const context = useContext(AnalysisToolContext)
if (!context) {
throw new Error('useAnalysisTool must be used within an AnalysisToolProvider')
}
return context
}
88 changes: 39 additions & 49 deletions components/chat.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,6 @@ import SettingsView from "@/components/settings/settings-view";
import { MapDataProvider, useMapData } from './map/map-data-context'; // Add this and useMapData
import { MapProvider } from './map/map-context'
import { updateDrawingContext } from '@/lib/actions/chat'; // Import the server action
import dynamic from 'next/dynamic'

const AnalysisTool = dynamic(() => import('./analysis-tool').then(mod => mod.AnalysisTool), {
ssr: false,
})

type ChatProps = {
id?: string // This is the chatId
Expand Down Expand Up @@ -86,42 +81,17 @@ export function Chat({ id }: ChatProps) {
if (isMobile) {
return (
<MapDataProvider> {/* Add Provider */}
<MapProvider>
<div className="mobile-layout-container">
<div className="mobile-map-section">
{activeView ? <SettingsView /> : <Mapbox />}
</div>
<div className="mobile-icons-bar">
<MobileIconsBar onAttachmentClick={handleAttachment} />
</div>
<div className="mobile-chat-input-area">
<ChatPanel ref={chatPanelRef} messages={messages} input={input} setInput={setInput} />
</div>
<div className="mobile-chat-messages-area">
{showEmptyScreen ? (
<EmptyScreen
submitMessage={message => {
setInput(message)
}}
/>
) : (
<ChatMessages messages={messages} />
)}
</div>
</div>
</MapProvider>
</MapDataProvider>
);
}

// Desktop layout
return (
<MapDataProvider> {/* Add Provider */}
<MapProvider>
<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">
<ChatPanel messages={messages} input={input} setInput={setInput} />
<div className="mobile-layout-container">
<div className="mobile-map-section">
{activeView ? <SettingsView /> : <Mapbox />}
</div>
<div className="mobile-icons-bar">
<MobileIconsBar onAttachmentClick={handleAttachment} />
</div>
<div className="mobile-chat-input-area">
<ChatPanel ref={chatPanelRef} messages={messages} input={input} setInput={setInput} />
</div>
<div className="mobile-chat-messages-area">
{showEmptyScreen ? (
<EmptyScreen
submitMessage={message => {
Expand All @@ -131,16 +101,36 @@ export function Chat({ id }: ChatProps) {
) : (
<ChatMessages messages={messages} />
)}
</div>
<div
className="w-1/2 p-4 fixed h-[calc(100vh-0.5in)] top-0 right-0 mt-[0.5in]"
style={{ zIndex: 10 }} // Added z-index
>
{activeView ? <SettingsView /> : <Mapbox />}
<AnalysisTool />
</div>
</div>
</MapProvider>
</MapDataProvider>
);
}

// Desktop layout
return (
<MapDataProvider> {/* Add Provider */}
<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">
<ChatPanel messages={messages} input={input} setInput={setInput} />
{showEmptyScreen ? (
<EmptyScreen
submitMessage={message => {
setInput(message)
}}
/>
) : (
<ChatMessages messages={messages} />
)}
</div>
<div
className="w-1/2 p-4 fixed h-[calc(100vh-0.5in)] top-0 right-0 mt-[0.5in]"
style={{ zIndex: 10 }} // Added z-index
>
{activeView ? <SettingsView /> : <Mapbox />}
</div>
</div>
</MapDataProvider>
);
}
21 changes: 19 additions & 2 deletions components/header.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
'use client'

import React from 'react'
import Image from 'next/image'
import { ModeToggle } from './mode-toggle'
Expand All @@ -13,8 +15,13 @@ import {
} from 'lucide-react'
import { MapToggle } from './map-toggle'
import { ProfileToggle } from './profile-toggle'
import { useAnalysisTool } from './analysis-tool-context'
import { useMap } from './map/map-context'

export const Header = () => {
const { isAnalyzing, handleResolutionSearch } = useAnalysisTool()
const { map } = useMap()

Comment on lines +22 to +24
Copy link
Copy Markdown

@charliecreates charliecreates Bot Oct 7, 2025

Choose a reason for hiding this comment

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

Header now uses both useAnalysisTool() and useMap(), which will throw at runtime if Header is rendered outside of AnalysisToolProvider and MapProvider. In this diff, the providers are added inside Chat, but Header is not rendered by Chat (at least not in this diff), so there’s a high risk of a crash in any route/layout where Header sits outside these providers. Ensure Header is wrapped by both providers (and MapDataProvider if needed) at a common ancestor.

This is a correctness issue that will manifest as an immediate runtime error due to the explicit throw in useAnalysisTool and likely in useMap as well.

Suggestion

Wrap the part of the app that renders Header with the same providers used in Chat (preferably in a top-level layout):

// e.g., app/layout.tsx (or wherever Header is rendered)
import { MapDataProvider } from '@/components/map/map-data-context'
import { MapProvider } from '@/components/map/map-context'
import { AnalysisToolProvider } from '@/components/analysis-tool-context'
import { Header } from '@/components/header'

export default function RootLayout({ children }: { children: React.ReactNode }) {
  return (
    <html>
      <body>
        <MapDataProvider>
          <MapProvider>
            <AnalysisToolProvider>
              <Header />
              {children}
            </AnalysisToolProvider>
          </MapProvider>
        </MapDataProvider>
      </body>
    </html>
  )
}

Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this suggestion

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>
Expand All @@ -39,8 +46,18 @@ export const Header = () => {
<CalendarDays className="h-[1.2rem] w-[1.2rem]" />
</Button>

<Button variant="ghost" size="icon">
<Search className="h-[1.2rem] w-[1.2rem]" />
<Button
variant="ghost"
size="icon"
onClick={handleResolutionSearch}
disabled={isAnalyzing || !map}
title="Analyze current map view"
>
{isAnalyzing ? (
<div className="h-[1.2rem] w-[1.2rem] animate-spin rounded-full border-b-2 border-current"></div>
) : (
<Search className="h-[1.2rem] w-[1.2rem]" />
)}
</Button>
Comment on lines +49 to 61
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Oct 7, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Extract analysis button to eliminate duplication.

The analysis button implementation (lines 49-61) is identical to the one in components/mobile-icons-bar.tsx (lines 50-62). This duplication violates the DRY principle and makes maintenance harder.

Consider extracting a reusable AnalysisButton component:

// components/analysis-button.tsx
'use client'

import { Button } from '@/components/ui/button'
import { Search } from 'lucide-react'
import { useAnalysisTool } from './analysis-tool-context'
import { useMap } from './map/map-context'

export const AnalysisButton = () => {
  const { isAnalyzing, handleResolutionSearch } = useAnalysisTool()
  const { map } = useMap()

  return (
    <Button
      variant="ghost"
      size="icon"
      onClick={handleResolutionSearch}
      disabled={isAnalyzing || !map}
      title="Analyze current map view"
    >
      {isAnalyzing ? (
        <div className="h-[1.2rem] w-[1.2rem] animate-spin rounded-full border-b-2 border-current"></div>
      ) : (
        <Search className="h-[1.2rem] w-[1.2rem]" />
      )}
    </Button>
  )
}

Then replace the duplicated button code in both files with:

<AnalysisButton />
🤖 Prompt for AI Agents 
In components/header.tsx around lines 49-61 the Analysis button is duplicated
elsewhere (components/mobile-icons-bar.tsx lines ~50-62); extract a new reusable
component (e.g., components/analysis-button.tsx) that imports Button and Search,
uses useAnalysisTool and useMap hooks to derive isAnalyzing,
handleResolutionSearch and map, and renders the same Button JSX (include 'use
client' at top). Replace the duplicated JSX in both components/header.tsx and
components/mobile-icons-bar.tsx with a single <AnalysisButton /> import and
ensure the new file exports the component and both files import it.


<Button variant="ghost" size="icon">
Expand Down
18 changes: 16 additions & 2 deletions components/mobile-icons-bar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ import {
import { History } from '@/components/history'
import { MapToggle } from './map-toggle'
import { ModeToggle } from './mode-toggle'
import { useAnalysisTool } from './analysis-tool-context'
import { useMap } from './map/map-context'

interface MobileIconsBarProps {
onAttachmentClick: () => void;
Expand All @@ -25,6 +27,8 @@ interface MobileIconsBarProps {
export const MobileIconsBar: React.FC<MobileIconsBarProps> = ({ onAttachmentClick }) => {
const [, setMessages] = useUIState<typeof AI>()
const { clearChat } = useActions()
const { isAnalyzing, handleResolutionSearch } = useAnalysisTool()
const { map } = useMap()

const handleNewChat = async () => {
setMessages([])
Expand All @@ -43,8 +47,18 @@ export const MobileIconsBar: React.FC<MobileIconsBarProps> = ({ onAttachmentClic
<Button variant="ghost" size="icon">
<CalendarDays className="h-[1.2rem] w-[1.2rem] transition-all rotate-0 scale-100" />
</Button>
<Button variant="ghost" size="icon">
<Search className="h-[1.2rem] w-[1.2rem] transition-all rotate-0 scale-100" />
<Button
variant="ghost"
size="icon"
onClick={handleResolutionSearch}
disabled={isAnalyzing || !map}
title="Analyze current map view"
>
{isAnalyzing ? (
<div className="h-[1.2rem] w-[1.2rem] animate-spin rounded-full border-b-2 border-current"></div>
) : (
<Search className="h-[1.2rem] w-[1.2rem] transition-all rotate-0 scale-100" />
)}
</Button>
<Button variant="ghost" size="icon">
<TentTree className="h-[1.2rem] w-[1.2rem] transition-all rotate-0 scale-100" />
Expand Down
6 changes: 0 additions & 6 deletions dev_server.log
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1 @@
$ next dev --turbo
Copy link
Copy Markdown

@charliecreates charliecreates Bot Oct 7, 2025

Choose a reason for hiding this comment

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

Committing dev_server.log is not advisable. Build/dev logs are transient and should be excluded from source control to avoid noisy diffs and accidental leakage of local environment details.

Suggestion

Delete dev_server.log from the repo and add it to .gitignore to prevent future commits:

# logs
*.log
/dev_server.log

I can add a cleanup commit to remove the file and update .gitignore. Reply with "@CharlieHelps yes please" if you'd like me to do that

▲ Next.js 15.3.3 (Turbopack)
- Local: http://localhost:3000
- Network: http://192.168.0.2:3000
- Environments: .env.local, .env

✓ Starting...
Binary file removed jules-scratch/verification/error_screenshot.png
View file
Open in desktop
Binary file not shown.
66 changes: 0 additions & 66 deletions jules-scratch/verification/verify_e2e_analysis.py
View file
Open in desktop

This file was deleted.