Skip to content

🎨 Palette: Improve accessibility and loading feedback #525

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
ngoiyaeric wants to merge 1 commit into
base: main
Choose a base branch
from
Open

🎨 Palette: Improve accessibility and loading feedback #525

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 3 additions & 0 deletions .Jules/palette.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
## 2025-05-14 - [Icon-only Button Accessibility]
**Learning:** Many icon-only buttons in the application were missing ARIA labels and titles, making them inaccessible to screen readers and providing no tooltips for sighted users. The logo functioning as a history toggle was particularly non-obvious.
**Action:** Always provide both `aria-label` and `title` for icon-only buttons. Ensure that branding elements used as interactive controls have clear descriptive labels.
33 changes: 27 additions & 6 deletions components/chat-panel.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ 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 { Spinner } from './ui/spinner'
import Textarea from 'react-textarea-autosize'
import { nanoid } from '@/lib/utils'
import { useSettingsStore } from '@/lib/store/settings'
Expand All @@ -20,19 +21,21 @@ interface ChatPanelProps {
input: string
setInput: (value: string) => void
onSuggestionsChange?: (suggestions: PartialRelated | null) => void
onSubmitting?: (isSubmitting: boolean) => void
}

export interface ChatPanelRef {
handleAttachmentClick: () => void
submitForm: () => void
}

export const ChatPanel = forwardRef<ChatPanelRef, ChatPanelProps>(({ messages, input, setInput, onSuggestionsChange }, ref) => {
export const ChatPanel = forwardRef<ChatPanelRef, ChatPanelProps>(({ messages, input, setInput, onSuggestionsChange, onSubmitting }, ref) => {
const [, setMessages] = useUIState<typeof AI>()
const { submit, clearChat } = useActions()
const { mapProvider } = useSettingsStore()
const [isMobile, setIsMobile] = useState(false)
const [selectedFile, setSelectedFile] = useState<File | null>(null)
const [isSubmitting, setIsSubmitting] = useState(false)
const [suggestions, setSuggestionsState] = useState<PartialRelated | null>(null)
const setSuggestions = useCallback((s: PartialRelated | null) => {
setSuggestionsState(s)
Expand Down Expand Up @@ -121,8 +124,15 @@ export const ChatPanel = forwardRef<ChatPanelRef, ChatPanelProps>(({ messages, i
setInput('')
clearAttachment()

const responseMessage = await submit(formData)
setMessages(currentMessages => [...currentMessages, responseMessage as any])
setIsSubmitting(true)
onSubmitting?.(true)
try {
const responseMessage = await submit(formData)
setMessages(currentMessages => [...currentMessages, responseMessage as any])
Comment on lines +130 to +131
Copy link

@charliecreates charliecreates bot Feb 16, 2026

Choose a reason for hiding this comment

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

The response message is appended with responseMessage as any. Even if tsc passes, this is an unsafe escape hatch that can hide real shape/compat issues at runtime and makes the state harder to reason about.

Since this code path is central (chat submission), avoiding any here is worth it.

Suggestion

Avoid as any by tightening the return type of submit (or by normalizing the message into the expected UI message type before pushing into state).

If submit returns a union, consider a small runtime discriminator/normalizer so setMessages always receives the same message shape.

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

} finally {
setIsSubmitting(false)
onSubmitting?.(false)
}
Comment on lines +127 to +135
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 16, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Missing error handling in submit flow — unhandled rejection if submit() throws.

The try/finally resets the submitting state correctly, but there's no catch block. If submit(formData) rejects, the error propagates as an unhandled promise rejection. The user has already seen their message added optimistically (lines 108–114) and the input cleared (line 124), but will receive no error feedback.

🐛 Proposed fix: add error handling 
     setIsSubmitting(true)
     onSubmitting?.(true)
     try {
       const responseMessage = await submit(formData)
       setMessages(currentMessages => [...currentMessages, responseMessage as any])
+    } catch (error) {
+      console.error('Failed to submit message:', error)
+      // Consider showing a toast or inline error to the user
     } finally {
       setIsSubmitting(false)
       onSubmitting?.(false)
     }
📝 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
setIsSubmitting(true)
onSubmitting?.(true)
try {
const responseMessage = await submit(formData)
setMessages(currentMessages => [...currentMessages, responseMessage as any])
} finally {
setIsSubmitting(false)
onSubmitting?.(false)
}
setIsSubmitting(true)
onSubmitting?.(true)
try {
const responseMessage = await submit(formData)
setMessages(currentMessages => [...currentMessages, responseMessage as any])
} catch (error) {
console.error('Failed to submit message:', error)
// Consider showing a toast or inline error to the user
} finally {
setIsSubmitting(false)
onSubmitting?.(false)
}
🤖 Prompt for AI Agents 
In `@components/chat-panel.tsx` around lines 127 - 135, The submit call currently
lacks a catch, so if submit(formData) rejects the error bubbles as an unhandled
rejection; wrap the await submit(...) in a try/catch (keeping the existing
finally) and in the catch block handle the failure: log the error, call any
error callback if available (e.g., onSubmitting/onError), and update the
messages state via setMessages(currentMessages => [...currentMessages, /* an
error/result marker message to surface the failure */]) or set an error state so
the UI shows feedback. Ensure setIsSubmitting and onSubmitting toggles remain in
the finally block and reference the existing submit, setMessages and
onSubmitting identifiers when implementing.

}

const handleClear = async () => {
Expand Down Expand Up @@ -177,6 +187,7 @@ export const ChatPanel = forwardRef<ChatPanelRef, ChatPanelProps>(({ messages, i
onClick={() => handleClear()}
data-testid="new-chat-button"
title="New Chat"
aria-label="New Chat"
>
<Sprout size={28} className="fill-primary/20" />
</Button>
Expand Down Expand Up @@ -225,6 +236,8 @@ export const ChatPanel = forwardRef<ChatPanelRef, ChatPanelProps>(({ messages, i
)}
onClick={handleAttachmentClick}
data-testid="desktop-attachment-button"
aria-label="Attach File"
title="Attach File"
>
<Paperclip size={isMobile ? 18 : 20} />
</Button>
Expand Down Expand Up @@ -281,11 +294,12 @@ export const ChatPanel = forwardRef<ChatPanelRef, ChatPanelProps>(({ messages, i
'absolute top-1/2 transform -translate-y-1/2',
isMobile ? 'right-1' : 'right-2'
)}
disabled={input.length === 0 && !selectedFile}
disabled={(input.length === 0 && !selectedFile) || isSubmitting}
aria-label="Send message"
title="Send Message"
data-testid="chat-submit"
>
<ArrowRight size={isMobile ? 18 : 20} />
{isSubmitting ? <Spinner /> : <ArrowRight size={isMobile ? 18 : 20} />}
</Button>
</div>
</form>
Expand All @@ -295,7 +309,14 @@ 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="Remove Attachment"
title="Remove Attachment"
>
<X size={16} />
</Button>
</div>
Expand Down
9 changes: 8 additions & 1 deletion components/chat.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ export function Chat({ id }: ChatProps) {
const [input, setInput] = useState('')
const [showEmptyScreen, setShowEmptyScreen] = useState(false)
const [isSubmitting, setIsSubmitting] = useState(false)
const [isSubmittingAction, setIsSubmittingAction] = useState(false)
Comment on lines 40 to +41
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 16, 2026

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Confusing naming between isSubmitting and isSubmittingAction.

isSubmitting (line 40) drives suggestion-based form submission via useEffect, while isSubmittingAction (line 41) tracks the actual async submit state from ChatPanel. These names don't convey their distinct purposes. Consider renaming for clarity, e.g., shouldTriggerSubmit for the former and isSubmitting for the latter.

♻️ Proposed rename for clarity 
- const [isSubmitting, setIsSubmitting] = useState(false)
- const [isSubmittingAction, setIsSubmittingAction] = useState(false)
+ const [shouldTriggerSubmit, setShouldTriggerSubmit] = useState(false)
+ const [isSubmitting, setIsSubmitting] = useState(false)

Then update all references accordingly: setIsSubmitting(true) in suggestion onSelect → setShouldTriggerSubmit(true), and the useEffect at lines 89–94 would use shouldTriggerSubmit/setShouldTriggerSubmit. The MobileIconsBar and ChatPanel wiring would use isSubmitting/setIsSubmitting.

🤖 Prompt for AI Agents 
In `@components/chat.tsx` around lines 40 - 41, Rename the two confusing boolean
states to make their roles explicit: change const [isSubmitting,
setIsSubmitting] to const [shouldTriggerSubmit, setShouldTriggerSubmit] (this
state is used by the suggestion onSelect and the useEffect that triggers form
submission), and change const [isSubmittingAction, setIsSubmittingAction] to
const [isSubmitting, setIsSubmitting] (this state reflects the real async submit
status from ChatPanel/MobileIconsBar). Update all references accordingly:
replace setIsSubmitting(true) calls in suggestion onSelect with
setShouldTriggerSubmit(true), update the useEffect that watches submission
triggers to use shouldTriggerSubmit/setShouldTriggerSubmit, and wire the
ChatPanel and MobileIconsBar props/state to the new isSubmitting/setIsSubmitting
identifiers.

Comment on lines 38 to +41
Copy link

@charliecreates charliecreates bot Feb 16, 2026

Choose a reason for hiding this comment

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

There are now two similarly-named submission states in Chat: isSubmitting and isSubmittingAction. Only isSubmittingAction is wired into the new spinner/disable flow, while isSubmitting is still toggled from EmptyScreen.

This is a correctness risk because different UI elements may consult different flags (now or in future edits), leading to inconsistent submission UX.

Suggestion

Consolidate to a single submission flag in Chat (e.g., keep only isSubmittingAction) and update the EmptyScreen handler to set that same flag.

If isSubmitting is still required for other logic, rename both to clarify intent (e.g., isEmptyScreenSubmitting vs isChatActionSubmitting) and document which drives which UI.

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

const [suggestions, setSuggestions] = useState<PartialRelated | null>(null)
const chatPanelRef = useRef<ChatPanelRef>(null);

Expand Down Expand Up @@ -132,7 +133,11 @@ export function Chat({ id }: ChatProps) {
{activeView ? <SettingsView /> : isUsageOpen ? <UsageView /> : <MapProvider />}
</div>
<div className="mobile-icons-bar">
<MobileIconsBar onAttachmentClick={handleAttachment} onSubmitClick={handleMobileSubmit} />
<MobileIconsBar
onAttachmentClick={handleAttachment}
onSubmitClick={handleMobileSubmit}
isSubmitting={isSubmittingAction}
/>
</div>
<div className="mobile-chat-input-area">
<ChatPanel
Expand All @@ -141,6 +146,7 @@ export function Chat({ id }: ChatProps) {
input={input}
setInput={setInput}
onSuggestionsChange={setSuggestions}
onSubmitting={setIsSubmittingAction}
/>
</div>
<div className="mobile-chat-messages-area relative">
Expand Down Expand Up @@ -185,6 +191,7 @@ export function Chat({ id }: ChatProps) {
input={input}
setInput={setInput}
onSuggestionsChange={setSuggestions}
onSubmitting={setIsSubmittingAction}
/>
<div className="relative min-h-[100px]">
<div className={cn("transition-all duration-300", suggestions ? "blur-md pointer-events-none" : "")}>
Expand Down
1 change: 1 addition & 0 deletions components/header-search-button.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ export function HeaderSearchButton() {
onClick={handleResolutionSearch}
disabled={isAnalyzing || !map || !actions}
title="Analyze current map view"
aria-label="Analyze current map view"
>
{isAnalyzing ? (
<div className="h-5 w-5 animate-spin rounded-full border-b-2 border-current"></div>
Expand Down
34 changes: 30 additions & 4 deletions components/header.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,14 @@ export const Header = () => {
</div>

<div className="absolute left-1 flex items-center">
<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"
title="Toggle History"
>
<Image
src="/images/logo.svg"
alt="Logo"
Expand All @@ -71,13 +78,26 @@ export const Header = () => {

<MapToggle />

<Button variant="ghost" size="icon" onClick={toggleCalendar} title="Open Calendar" data-testid="calendar-toggle">
<Button
variant="ghost"
size="icon"
onClick={toggleCalendar}
title="Open Calendar"
aria-label="Open Calendar"
data-testid="calendar-toggle"
>
<CalendarDays className="h-[1.2rem] w-[1.2rem]" />
</Button>

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

<Button variant="ghost" size="icon" onClick={handleUsageToggle}>
<Button
variant="ghost"
size="icon"
onClick={handleUsageToggle}
aria-label="View Usage"
title="View Usage"
>
<TentTree className="h-[1.2rem] w-[1.2rem]" />
</Button>

Expand All @@ -89,7 +109,13 @@ 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="View Usage"
title="View Usage"
>
<TentTree className="h-[1.2rem] w-[1.2rem]" />
</Button>
<ProfileToggle/>
Expand Down
2 changes: 2 additions & 0 deletions components/history.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ export function History({ location }: HistoryProps) {
})}
data-testid="history-button"
onClick={toggleHistory}
aria-label="Toggle History"
title="Toggle History"
>
{location === 'header' ? <Menu /> : <ChevronLeft size={16} />}
</Button>
Expand Down
62 changes: 54 additions & 8 deletions components/mobile-icons-bar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,15 @@ import { MapToggle } from './map-toggle'
import { ModeToggle } from './mode-toggle'
import { ProfileToggle } from './profile-toggle'
import { useCalendarToggle } from './calendar-toggle-context'
import { Spinner } from './ui/spinner'

interface MobileIconsBarProps {
onAttachmentClick: () => void;
onSubmitClick: () => void;
isSubmitting?: boolean;
}

export const MobileIconsBar: React.FC<MobileIconsBarProps> = ({ onAttachmentClick, onSubmitClick }) => {
export const MobileIconsBar: React.FC<MobileIconsBarProps> = ({ onAttachmentClick, onSubmitClick, isSubmitting }) => {
const [, setMessages] = useUIState<typeof AI>()
const { clearChat } = useActions()
const { toggleCalendar } = useCalendarToggle()
Expand All @@ -37,27 +39,71 @@ 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"
title="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}
title="Open Calendar"
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">
<Button
variant="ghost"
size="icon"
data-testid="mobile-search-button"
aria-label="Search"
title="Search"
>
<Search className="h-[1.2rem] w-[1.2rem] transition-all rotate-0 scale-100" />
</Button>
Comment on lines +64 to 72
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 16, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Search button has no onClick handler — it's non-functional.

This button renders but does nothing when clicked. Was this intentional as a placeholder, or should it be wired to a search action?

#!/bin/bash
# Check if the mobile search button had an onClick in previous versions or if there's a related handler
rg -n 'mobile-search-button' --type=tsx --type=ts -C3
🤖 Prompt for AI Agents 
In `@components/mobile-icons-bar.tsx` around lines 64 - 72, The Search button
(Button with data-testid "mobile-search-button" and the Search icon) has no
onClick and is non-functional; add an onClick handler that triggers the
component's search action—either call an existing prop like
onSearchClick/onOpenSearch if MobileIconsBar accepts one, or wire it to the
local search-open function/state (e.g., call openSearch()/setSearchOpen(true) or
the hook that opens the search modal). Ensure you update MobileIconsBar's props
type to include the handler name you choose (onSearchClick/onOpenSearch) if
needed and pass that handler into the Button as onClick.

<a href="https://buy.stripe.com/14A3cv7K72TR3go14Nasg02" target="_blank" rel="noopener noreferrer">
<Button variant="ghost" size="icon">
<Button
variant="ghost"
size="icon"
aria-label="Purchase Credits"
title="Purchase 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"
title="Attach File"
>
<Paperclip className="h-[1.2rem] w-[1.2rem] transition-all rotate-0 scale-100" />
</Button>
<Button variant="ghost" size="icon" data-testid="mobile-submit-button" onClick={onSubmitClick}>
<ArrowRight className="h-[1.2rem] w-[1.2rem] transition-all rotate-0 scale-100" />
<Button
variant="ghost"
size="icon"
data-testid="mobile-submit-button"
onClick={onSubmitClick}
aria-label="Send Message"
title="Send Message"
disabled={isSubmitting}
>
{isSubmitting ? (
<Spinner />
) : (
<ArrowRight className="h-[1.2rem] w-[1.2rem] transition-all rotate-0 scale-100" />
)}
</Button>
<History location="header" />
<ModeToggle />
Expand Down