Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 7 additions & 0 deletions .Jules/palette.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
## 2025-05-15 - [Consistency in Icon-Only Button Feedback]
**Learning:** Sighted users rely on visual tooltips (like the native `title` attribute or custom `Tooltip` components) to understand icon-only buttons. Removing these without providing a replacement degrades the UX. Accessibility improvements (`aria-label`) should complement, not replace, visual labels.
**Action:** Always ensure icon-only buttons have either a `title` attribute or are wrapped in a `Tooltip` component for visual feedback, in addition to `aria-label` for screen readers.

## 2025-05-15 - [Portal-based Component Injection]
**Learning:** When using portals to inject buttons (e.g., `HeaderSearchButton`), the target container must exist in the DOM for the button to appear. If the target container is moved or replaced, the portal ID must be maintained or updated.
**Action:** When refactoring layouts that host portal targets, verify that the target IDs (like `mobile-header-search-portal`) are preserved or correctly re-implemented.
Comment on lines +1 to +7
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Address markdown formatting issues.

The static analysis tool flags minor markdown formatting violations:

  • Headings should be surrounded by blank lines (MD022)
  • First line should be a top-level heading (MD041)
📝 Proposed fix for markdown formatting
+# Palette Learnings
+
 ## 2025-05-15 - [Consistency in Icon-Only Button Feedback]
+
 **Learning:** Sighted users rely on visual tooltips (like the native `title` attribute or custom `Tooltip` components) to understand icon-only buttons. Removing these without providing a replacement degrades the UX. Accessibility improvements (`aria-label`) should complement, not replace, visual labels.
+
 **Action:** Always ensure icon-only buttons have either a `title` attribute or are wrapped in a `Tooltip` component for visual feedback, in addition to `aria-label` for screen readers.

 ## 2025-05-15 - [Portal-based Component Injection]
+
 **Learning:** When using portals to inject buttons (e.g., `HeaderSearchButton`), the target container must exist in the DOM for the button to appear. If the target container is moved or replaced, the portal ID must be maintained or updated.
+
 **Action:** When refactoring layouts that host portal targets, verify that the target IDs (like `mobile-header-search-portal`) are preserved or correctly re-implemented.
📝 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
## 2025-05-15 - [Consistency in Icon-Only Button Feedback]
**Learning:** Sighted users rely on visual tooltips (like the native `title` attribute or custom `Tooltip` components) to understand icon-only buttons. Removing these without providing a replacement degrades the UX. Accessibility improvements (`aria-label`) should complement, not replace, visual labels.
**Action:** Always ensure icon-only buttons have either a `title` attribute or are wrapped in a `Tooltip` component for visual feedback, in addition to `aria-label` for screen readers.
## 2025-05-15 - [Portal-based Component Injection]
**Learning:** When using portals to inject buttons (e.g., `HeaderSearchButton`), the target container must exist in the DOM for the button to appear. If the target container is moved or replaced, the portal ID must be maintained or updated.
**Action:** When refactoring layouts that host portal targets, verify that the target IDs (like `mobile-header-search-portal`) are preserved or correctly re-implemented.
# Palette Learnings
## 2025-05-15 - [Consistency in Icon-Only Button Feedback]
**Learning:** Sighted users rely on visual tooltips (like the native `title` attribute or custom `Tooltip` components) to understand icon-only buttons. Removing these without providing a replacement degrades the UX. Accessibility improvements (`aria-label`) should complement, not replace, visual labels.
**Action:** Always ensure icon-only buttons have either a `title` attribute or are wrapped in a `Tooltip` component for visual feedback, in addition to `aria-label` for screen readers.
## 2025-05-15 - [Portal-based Component Injection]
**Learning:** When using portals to inject buttons (e.g., `HeaderSearchButton`), the target container must exist in the DOM for the button to appear. If the target container is moved or replaced, the portal ID must be maintained or updated.
**Action:** When refactoring layouts that host portal targets, verify that the target IDs (like `mobile-header-search-portal`) are preserved or correctly re-implemented.
🧰 Tools
🪛 markdownlint-cli2 (0.20.0)

[warning] 1-1: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below

(MD022, blanks-around-headings)


[warning] 1-1: First line in a file should be a top-level heading

(MD041, first-line-heading, first-line-h1)


[warning] 5-5: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below

(MD022, blanks-around-headings)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.Jules/palette.md around lines 1 - 7, The markdown file lacks a top-level
heading and blank lines around headings causing MD041 and MD022 failures; add a
first-line H1 (e.g., a descriptive top-level heading) so the file starts with a
level-1 heading, and ensure every existing heading (like "2025-05-15 -
[Consistency in Icon-Only Button Feedback]" and "2025-05-15 - [Portal-based
Component Injection]") has a blank line both before and after it so headings are
properly surrounded by blank lines.

23 changes: 13 additions & 10 deletions app/layout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { HistoryToggleProvider } from '@/components/history-toggle-context'
import { HistorySidebar } from '@/components/history-sidebar'
import { MapLoadingProvider } from '@/components/map-loading-context';
import ConditionalLottie from '@/components/conditional-lottie';
import { TooltipProvider } from '@/components/ui/tooltip'
import { MapProvider as MapContextProvider } from '@/components/map/map-context'

const fontSans = FontSans({
Expand Down Expand Up @@ -107,16 +108,18 @@ export default function RootLayout({
disableTransitionOnChange
themes={['light', 'dark', 'earth']}
>
<MapContextProvider>
<MapLoadingProvider>
<Header />
<ConditionalLottie />
{children}
<HistorySidebar />
<Footer />
<Toaster />
</MapLoadingProvider>
</MapContextProvider>
<TooltipProvider>
<MapContextProvider>
<MapLoadingProvider>
<Header />
<ConditionalLottie />
{children}
<HistorySidebar />
<Footer />
<Toaster />
</MapLoadingProvider>
</MapContextProvider>
</TooltipProvider>
Comment on lines +111 to +122

Choose a reason for hiding this comment

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

TooltipProvider is added without any configuration. Many tooltip implementations (e.g., Radix-based) default to non-zero delays and may show tooltips on touch/keyboard in ways that can feel noisy across the whole app.

Given the PR goal is UX/accessibility, it’s worth explicitly setting provider props (delay, skipDelay, disableHoverableContent) to a deliberate global behavior rather than relying on library defaults.

Suggestion

Consider configuring the provider explicitly, e.g.:

<TooltipProvider delayDuration={200} skipDelayDuration={0} disableHoverableContent>
  ...
</TooltipProvider>

This makes the global UX predictable and avoids regressions if library defaults change. Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this suggestion.

</ThemeProvider>
</UsageToggleProvider>
</ProfileToggleProvider>
Expand Down
64 changes: 40 additions & 24 deletions components/chat-panel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@ import { cn } from '@/lib/utils'
import { UserMessage } from './user-message'
import { Button } from './ui/button'
import { ArrowRight, Plus, Paperclip, X, Sprout } from 'lucide-react'
import {
Tooltip,
TooltipContent,
TooltipTrigger
} from '@/components/ui/tooltip'
import Textarea from 'react-textarea-autosize'
import { nanoid } from '@/lib/utils'
import { useSettingsStore } from '@/lib/store/settings'
Expand Down Expand Up @@ -169,17 +174,22 @@ export const ChatPanel = forwardRef<ChatPanelRef, ChatPanelProps>(({ messages, i
'fixed bottom-4 left-4 flex justify-start items-center pointer-events-none z-50'
)}
>
<Button
type="button"
variant={'ghost'}
size={'icon'}
className="rounded-full transition-all hover:scale-110 pointer-events-auto text-primary"
onClick={() => handleClear()}
data-testid="new-chat-button"
title="New Chat"
>
<Sprout size={28} className="fill-primary/20" />
</Button>
<Tooltip>
<TooltipTrigger asChild>
<Button
type="button"
variant={'ghost'}
size={'icon'}
className="rounded-full transition-all hover:scale-110 pointer-events-auto text-primary"
onClick={() => handleClear()}
data-testid="new-chat-button"
aria-label="New Chat"
>
<Sprout size={28} className="fill-primary/20" />
</Button>
</TooltipTrigger>
<TooltipContent>New Chat</TooltipContent>
</Tooltip>
</div>
)
}
Expand Down Expand Up @@ -216,18 +226,24 @@ export const ChatPanel = forwardRef<ChatPanelRef, ChatPanelProps>(({ messages, i
accept="text/plain,image/png,image/jpeg,image/webp"
/>
{!isMobile && (
<Button
type="button"
variant={'ghost'}
size={'icon'}
className={cn(
'absolute top-1/2 transform -translate-y-1/2 left-3'
)}
onClick={handleAttachmentClick}
data-testid="desktop-attachment-button"
>
<Paperclip size={isMobile ? 18 : 20} />
</Button>
<Tooltip>
<TooltipTrigger asChild>
<Button
type="button"
variant={'ghost'}
size={'icon'}
className={cn(
'absolute top-1/2 transform -translate-y-1/2 left-3'
)}
onClick={handleAttachmentClick}
data-testid="desktop-attachment-button"
aria-label="Attach file"
>
<Paperclip size={isMobile ? 18 : 20} />
</Button>
</TooltipTrigger>
<TooltipContent>Attach file</TooltipContent>
</Tooltip>
Comment on lines 228 to +246
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

Dead branch: isMobile ternary always evaluates to false here.

Line 228 guards this block with !isMobile, so isMobile ? 18 : 20 on line 242 will always resolve to 20. Simplify to a literal.

Proposed fix
-                  <Paperclip size={isMobile ? 18 : 20} />
+                  <Paperclip size={20} />
📝 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
{!isMobile && (
<Button
type="button"
variant={'ghost'}
size={'icon'}
className={cn(
'absolute top-1/2 transform -translate-y-1/2 left-3'
)}
onClick={handleAttachmentClick}
data-testid="desktop-attachment-button"
>
<Paperclip size={isMobile ? 18 : 20} />
</Button>
<Tooltip>
<TooltipTrigger asChild>
<Button
type="button"
variant={'ghost'}
size={'icon'}
className={cn(
'absolute top-1/2 transform -translate-y-1/2 left-3'
)}
onClick={handleAttachmentClick}
data-testid="desktop-attachment-button"
aria-label="Attach file"
>
<Paperclip size={isMobile ? 18 : 20} />
</Button>
</TooltipTrigger>
<TooltipContent>Attach file</TooltipContent>
</Tooltip>
{!isMobile && (
<Tooltip>
<TooltipTrigger asChild>
<Button
type="button"
variant={'ghost'}
size={'icon'}
className={cn(
'absolute top-1/2 transform -translate-y-1/2 left-3'
)}
onClick={handleAttachmentClick}
data-testid="desktop-attachment-button"
aria-label="Attach file"
>
<Paperclip size={20} />
</Button>
</TooltipTrigger>
<TooltipContent>Attach file</TooltipContent>
</Tooltip>
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@components/chat-panel.tsx` around lines 228 - 246, The Paperclip icon size
uses a redundant ternary (isMobile ? 18 : 20) inside the block already guarded
by !isMobile; update the Paperclip usage in the ChatPanel render so it uses the
literal 20 size (replace Paperclip size prop with 20) to remove the dead branch;
locate the JSX around Tooltip/TooltipTrigger/Button where handleAttachmentClick,
Paperclip, and isMobile are referenced and make the size change there.

)}
<Textarea
ref={inputRef}
Expand Down Expand Up @@ -295,7 +311,7 @@ export const ChatPanel = forwardRef<ChatPanelRef, ChatPanelProps>(({ messages, i
<span className="text-sm text-muted-foreground truncate max-w-xs">
{selectedFile.name}
</span>
<Button variant="ghost" size="icon" onClick={clearAttachment} data-testid="clear-attachment-button">
<Button variant="ghost" size="icon" onClick={clearAttachment} data-testid="clear-attachment-button" aria-label="Clear attachment">
<X size={16} />
</Button>
</div>
Expand Down
2 changes: 2 additions & 0 deletions components/header-search-button.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,8 @@ export function HeaderSearchButton() {
size="icon"
onClick={handleResolutionSearch}
disabled={isAnalyzing || !map || !actions}
data-testid="resolution-search"
aria-label="Analyze current map view"
title="Analyze current map view"
>
{isAnalyzing ? (
Expand Down
55 changes: 39 additions & 16 deletions components/header.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@ import { useUsageToggle } from './usage-toggle-context'
import { useProfileToggle } from './profile-toggle-context'
import { useHistoryToggle } from './history-toggle-context'
import { useState, useEffect } from 'react'
import {
Tooltip,
TooltipContent,
TooltipTrigger
} from '@/components/ui/tooltip'

export const Header = () => {
const { toggleCalendar } = useCalendarToggle()
Expand Down Expand Up @@ -52,15 +57,21 @@ export const Header = () => {
</div>

<div className="absolute left-1 flex items-center">
<Button variant="ghost" size="icon" onClick={toggleHistory} data-testid="logo-history-toggle">
<Image
src="/images/logo.svg"
alt="Logo"
width={20}
height={20}
className="h-5 w-auto"
/>
</Button>
<Tooltip>
<TooltipTrigger asChild>
<Button variant="ghost" size="icon" onClick={toggleHistory} data-testid="logo-history-toggle">
<Image
src="/images/logo.svg"
alt="Logo"
width={20}
height={20}
className="h-5 w-auto"
/>
<span className="sr-only">Toggle history</span>
</Button>
</TooltipTrigger>
<TooltipContent>Toggle history</TooltipContent>
</Tooltip>
Comment on lines +60 to +74
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Consider using aria-label instead of sr-only span on the logo button.

The button contains an <Image alt="Logo"> and a <span className="sr-only">Toggle history</span>. Screen readers may announce both — "Logo Toggle history" — because the accessible name is computed from all descendant text content. An explicit aria-label="Toggle history" on the <Button> would give a single, clean accessible name, consistent with how other buttons in this PR use aria-label.

Proposed fix
-            <Button variant="ghost" size="icon" onClick={toggleHistory} data-testid="logo-history-toggle">
+            <Button variant="ghost" size="icon" onClick={toggleHistory} data-testid="logo-history-toggle" aria-label="Toggle history">
               <Image
                 src="/images/logo.svg"
-                alt="Logo"
+                alt=""
                 width={20}
                 height={20}
                 className="h-5 w-auto"
               />
-              <span className="sr-only">Toggle history</span>
             </Button>
📝 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
<Tooltip>
<TooltipTrigger asChild>
<Button variant="ghost" size="icon" onClick={toggleHistory} data-testid="logo-history-toggle">
<Image
src="/images/logo.svg"
alt="Logo"
width={20}
height={20}
className="h-5 w-auto"
/>
<span className="sr-only">Toggle history</span>
</Button>
</TooltipTrigger>
<TooltipContent>Toggle history</TooltipContent>
</Tooltip>
<Tooltip>
<TooltipTrigger asChild>
<Button variant="ghost" size="icon" onClick={toggleHistory} data-testid="logo-history-toggle" aria-label="Toggle history">
<Image
src="/images/logo.svg"
alt=""
width={20}
height={20}
className="h-5 w-auto"
/>
</Button>
</TooltipTrigger>
<TooltipContent>Toggle history</TooltipContent>
</Tooltip>
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@components/header.tsx` around lines 60 - 74, Replace the visually-hidden span
used for the button label with an explicit aria-label on the Button component to
avoid duplicate accessible names; specifically, on the Button (the one with
variant="ghost" size="icon" data-testid="logo-history-toggle" and
onClick={toggleHistory}) add aria-label="Toggle history" and remove the <span
className="sr-only">Toggle history</span> descendant so the Image alt="Logo"
remains for the image only and the Button has a single, clear accessible name.

<h1 className="text-2xl font-poppins font-semibold text-primary">
QCX
</h1>
Expand All @@ -71,15 +82,27 @@ export const Header = () => {

<MapToggle />

<Button variant="ghost" size="icon" onClick={toggleCalendar} title="Open Calendar" data-testid="calendar-toggle">
<CalendarDays className="h-[1.2rem] w-[1.2rem]" />
</Button>
<Tooltip>
<TooltipTrigger asChild>
<Button variant="ghost" size="icon" onClick={toggleCalendar} data-testid="calendar-toggle">
<CalendarDays className="h-[1.2rem] w-[1.2rem]" />
<span className="sr-only">Open Calendar</span>
</Button>
</TooltipTrigger>
<TooltipContent>Open Calendar</TooltipContent>
</Tooltip>

<div id="header-search-portal" className="contents" />

<Button variant="ghost" size="icon" onClick={handleUsageToggle}>
<TentTree className="h-[1.2rem] w-[1.2rem]" />
</Button>
<Tooltip>
<TooltipTrigger asChild>
<Button variant="ghost" size="icon" onClick={handleUsageToggle}>
<TentTree className="h-[1.2rem] w-[1.2rem]" />
<span className="sr-only">Usage & Credits</span>
</Button>
</TooltipTrigger>
<TooltipContent>Usage & Credits</TooltipContent>
</Tooltip>

<ModeToggle />

Expand All @@ -89,7 +112,7 @@ export const Header = () => {
{/* Mobile menu buttons */}
<div className="flex md:hidden gap-2">

<Button variant="ghost" size="icon" onClick={handleUsageToggle}>
<Button variant="ghost" size="icon" onClick={handleUsageToggle} aria-label="Usage & Credits">
<TentTree className="h-[1.2rem] w-[1.2rem]" />
</Button>
<ProfileToggle/>
Expand Down
19 changes: 18 additions & 1 deletion components/history.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,19 @@ import { Button } from '@/components/ui/button'
import { ChevronLeft, Menu } from 'lucide-react'
import { cn } from '@/lib/utils'
import { useHistoryToggle } from './history-toggle-context'
import {
Tooltip,
TooltipContent,
TooltipTrigger
} from '@/components/ui/tooltip'

type HistoryProps = {
location: 'sidebar' | 'header'
}

export function History({ location }: HistoryProps) {
const { toggleHistory } = useHistoryToggle()
return (
const button = (
<Button
variant="ghost"
size="icon"
Expand All @@ -19,8 +24,20 @@ export function History({ location }: HistoryProps) {
})}
data-testid="history-button"
onClick={toggleHistory}
aria-label="Toggle history"
>
{location === 'header' ? <Menu /> : <ChevronLeft size={16} />}
</Button>
)

if (location === 'header') {
return (
<Tooltip>
<TooltipTrigger asChild>{button}</TooltipTrigger>
<TooltipContent>Toggle history</TooltipContent>
</Tooltip>
)
}

return button
}
14 changes: 6 additions & 8 deletions components/mobile-icons-bar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,26 +37,24 @@ export const MobileIconsBar: React.FC<MobileIconsBarProps> = ({ onAttachmentClic

return (
<div className="mobile-icons-bar-content">
<Button variant="ghost" size="icon" onClick={handleNewChat} data-testid="mobile-new-chat-button">
<Button variant="ghost" size="icon" onClick={handleNewChat} data-testid="mobile-new-chat-button" aria-label="New Chat">
<Plus className="h-[1.2rem] w-[1.2rem]" />
</Button>
<ProfileToggle />
<MapToggle />
<Button variant="ghost" size="icon" onClick={toggleCalendar} title="Open Calendar" data-testid="mobile-calendar-button">
<Button variant="ghost" size="icon" onClick={toggleCalendar} aria-label="Open Calendar" data-testid="mobile-calendar-button">
<CalendarDays className="h-[1.2rem] w-[1.2rem] transition-all rotate-0 scale-100" />
</Button>
<Button variant="ghost" size="icon" data-testid="mobile-search-button">
<Search className="h-[1.2rem] w-[1.2rem] transition-all rotate-0 scale-100" />
</Button>
<div id="mobile-header-search-portal" className="contents" />
<a href="https://buy.stripe.com/14A3cv7K72TR3go14Nasg02" target="_blank" rel="noopener noreferrer">
<Button variant="ghost" size="icon">
<Button variant="ghost" size="icon" aria-label="Usage & Credits">
<TentTree className="h-[1.2rem] w-[1.2rem] transition-all rotate-0 scale-100" />
</Button>
</a>
<Button variant="ghost" size="icon" onClick={onAttachmentClick} data-testid="mobile-attachment-button">
<Button variant="ghost" size="icon" onClick={onAttachmentClick} data-testid="mobile-attachment-button" aria-label="Attach file">
<Paperclip className="h-[1.2rem] w-[1.2rem] transition-all rotate-0 scale-100" />
Comment on lines 39 to 55

Choose a reason for hiding this comment

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

Replacing the mobile search button with a portal mount fixes the missing target issue, but it also removes the visible search affordance if the portal injection fails for any reason (timing, conditional rendering, regression elsewhere). Right now there’s no fallback UI, so mobile users could lose search entirely without a clear signal.

Suggestion

Add a lightweight fallback search button when the portal injection is absent, or at least leave a non-interactive placeholder that clearly indicates search is loading/unavailable. One pragmatic approach is to render a disabled search button until the portal content mounts (if you can detect that), or keep the old button wired to the same handler used by the portal content.

Reply with "@CharlieHelps yes please" if you’d like me to add a commit that introduces a safe fallback while preserving the portal target.

</Button>
Comment on lines 49 to 56

Choose a reason for hiding this comment

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

Mobile Usage & Credits is implemented as an <a> wrapping a <Button>. This often results in nested interactive semantics (a button inside a link), which is problematic for accessibility and can cause inconsistent click/keyboard behavior.

Since this PR is explicitly about accessibility, this should be addressed here rather than left as-is.

Suggestion

Render the button as a link (or style an anchor) instead of nesting. If your Button supports asChild:

<Button asChild variant="ghost" size="icon" aria-label="Usage & Credits">
  <a href="..." target="_blank" rel="noopener noreferrer">
    <TentTree ... />
  </a>
</Button>

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

<Button variant="ghost" size="icon" data-testid="mobile-submit-button" onClick={onSubmitClick}>
<Button variant="ghost" size="icon" data-testid="mobile-submit-button" onClick={onSubmitClick} aria-label="Send message">
<ArrowRight className="h-[1.2rem] w-[1.2rem] transition-all rotate-0 scale-100" />
</Button>
<History location="header" />
Expand Down