Skip to content

🎨 Palette: Improve accessibility with ARIA labels for icon-only buttons #530

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 with ARIA labels for icon-only buttons #530

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-15 - [Accessibility: Icon-Only Button Labels]
**Learning:** Icon-only buttons without explicit `aria-label` attributes are inaccessible to screen reader users, as they often lack descriptive text content. While `title` attributes provide some information on hover, they are not a substitute for `aria-label` in terms of accessibility standards.
**Action:** Always ensure icon-only buttons have an `aria-label` or an `sr-only` span describing their action. This project now consistently uses `aria-label` for common interface actions like toggling history, opening calendars, and submitting forms.
Comment on lines +1 to +3
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 18, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Fix markdown heading level and missing blank line (MD041, MD022).

The file opens with a level-2 heading (##) — MD041 requires the first line of a file to be a top-level heading (#). MD022 also requires a blank line after the heading.

📝 Proposed fix 
-## 2025-05-15 - [Accessibility: Icon-Only Button Labels]
+# 2025-05-15 - [Accessibility: Icon-Only Button Labels]
+
 **Learning:** Icon-only buttons without explicit `aria-label` attributes...
📝 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 - [Accessibility: Icon-Only Button Labels]
**Learning:** Icon-only buttons without explicit `aria-label` attributes are inaccessible to screen reader users, as they often lack descriptive text content. While `title` attributes provide some information on hover, they are not a substitute for `aria-label` in terms of accessibility standards.
**Action:** Always ensure icon-only buttons have an `aria-label` or an `sr-only` span describing their action. This project now consistently uses `aria-label` for common interface actions like toggling history, opening calendars, and submitting forms.
# 2025-05-15 - [Accessibility: Icon-Only Button Labels]
**Learning:** Icon-only buttons without explicit `aria-label` attributes are inaccessible to screen reader users, as they often lack descriptive text content. While `title` attributes provide some information on hover, they are not a substitute for `aria-label` in terms of accessibility standards.
**Action:** Always ensure icon-only buttons have an `aria-label` or an `sr-only` span describing their action. This project now consistently uses `aria-label` for common interface actions like toggling history, opening calendars, and submitting forms.
🧰 Tools
🪛 markdownlint-cli2 (0.21.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)

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

In @.jules/palette.md around lines 1 - 3, Change the level-2 heading to a
top-level heading and add a blank line after it to satisfy MD041 and MD022:
replace "## 2025-05-15 - [Accessibility: Icon-Only Button Labels]" with a single
leading '#' heading and insert an empty line immediately after that heading so
the file starts with a top-level heading and a following blank line.

10 changes: 9 additions & 1 deletion components/calendar-notepad.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ export function CalendarNotepad({ chatId }: CalendarNotepadProps) {
<button
onClick={() => setDateOffset(dateOffset - 7)}
className="p-2 text-muted-foreground hover:text-foreground"
aria-label="Previous week"
>
<ChevronLeft className="h-5 w-5" />
</button>
Expand Down Expand Up @@ -124,6 +125,7 @@ export function CalendarNotepad({ chatId }: CalendarNotepadProps) {
<button
onClick={() => setDateOffset(dateOffset + 7)}
className="p-2 text-muted-foreground hover:text-foreground"
aria-label="Next week"
>
<ChevronRight className="h-5 w-5" />
</button>
Expand All @@ -138,10 +140,12 @@ export function CalendarNotepad({ chatId }: CalendarNotepadProps) {
placeholder="Add note... (⌘+Enter to save, @mention, #location)"
className="w-full p-2 bg-input rounded-md border focus:ring-ring focus:ring-2 focus:outline-none pr-10"
rows={3}
aria-label="Add a note"
/>
<button
onClick={handleTagLocation}
className="absolute right-2 top-1/2 -translate-y-1/2 text-muted-foreground hover:text-foreground"
aria-label="Tag current location"
>
<MapPin className="h-5 w-5" />
</button>
Expand All @@ -160,7 +164,11 @@ export function CalendarNotepad({ chatId }: CalendarNotepadProps) {
<p className="text-sm whitespace-pre-wrap break-words">{note.content}</p>
</div>
{note.locationTags && (
<button onClick={() => handleFlyTo(note.locationTags)} className="text-muted-foreground hover:text-foreground ml-2">
<button
onClick={() => handleFlyTo(note.locationTags)}
className="text-muted-foreground hover:text-foreground ml-2"
aria-label="Fly to tagged location"
>
<MapPin className="h-5 w-5" />
</button>
)}
Expand Down
4 changes: 3 additions & 1 deletion components/chat-panel.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,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 +226,7 @@ export const ChatPanel = forwardRef<ChatPanelRef, ChatPanelProps>(({ messages, i
)}
onClick={handleAttachmentClick}
data-testid="desktop-attachment-button"
aria-label="Attach file"
>
<Paperclip size={isMobile ? 18 : 20} />
</Button>
Expand Down Expand Up @@ -295,7 +297,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
1 change: 1 addition & 0 deletions components/followup-panel.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ export function FollowupPanel() {
disabled={input.length === 0}
variant={'ghost'}
className="absolute right-1"
aria-label="Send message"
>
<ArrowRight size={20} />
</Button>
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
31 changes: 27 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,13 @@ 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"
>
<Image
src="/images/logo.svg"
alt="Logo"
Expand All @@ -71,13 +77,25 @@ 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"
data-testid="calendar-toggle"
aria-label="Open calendar"
>
<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="Toggle usage"
>
<TentTree className="h-[1.2rem] w-[1.2rem]" />
</Button>

Expand All @@ -89,7 +107,12 @@ 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="Toggle usage"
>
<TentTree className="h-[1.2rem] w-[1.2rem]" />
</Button>
<ProfileToggle/>
Expand Down
1 change: 1 addition & 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,7 @@ export function History({ location }: HistoryProps) {
})}
data-testid="history-button"
onClick={toggleHistory}
aria-label="Toggle history"
>
{location === 'header' ? <Menu /> : <ChevronLeft size={16} />}
</Button>
Expand Down
12 changes: 6 additions & 6 deletions components/mobile-icons-bar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,26 +37,26 @@ 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} title="Open Calendar" data-testid="mobile-calendar-button" aria-label="Open calendar">
<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">
<Search className="h-[1.2rem] w-[1.2rem] transition-all rotate-0 scale-100" />
</Button>
Comment on lines +48 to 50
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 18, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Labeled but non-functional button worsens screen reader UX.

The Search button has aria-label="Search" but no onClick handler. Before this change the unlabeled icon was unlikely to be interacted with by assistive technology users. Adding a label now makes it fully discoverable, so screen reader users will attempt to activate it and receive no response — a silent failure. Either wire up the handler or add disabled until the feature is implemented.

🛡️ Minimal fix until onClick is available 
-      <Button variant="ghost" size="icon" data-testid="mobile-search-button" aria-label="Search">
+      <Button variant="ghost" size="icon" data-testid="mobile-search-button" aria-label="Search" disabled>
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@components/mobile-icons-bar.tsx` around lines 48 - 50, The Search Button
(Button with data-testid="mobile-search-button" and aria-label="Search") is
labeled but has no onClick handler, causing a silent failure for screen-reader
users; either wire this Button to the correct click action (add the onClick
handler that triggers the mobile search flow) or mark it as non-interactive
until implemented (add disabled={true} and ensure aria-disabled="true") and keep
the aria-label; update the Button in components/mobile-icons-bar.tsx accordingly
so the accessible state matches functionality.

<a href="https://buy.stripe.com/14A3cv7K72TR3go14Nasg02" target="_blank" rel="noopener noreferrer">
<Button variant="ghost" size="icon">
<Button variant="ghost" size="icon" aria-label="Upgrade">
<TentTree className="h-[1.2rem] w-[1.2rem] transition-all rotate-0 scale-100" />
</Button>
</a>
Comment on lines 51 to 55
Copy link

@charliecreates charliecreates bot Feb 18, 2026

Choose a reason for hiding this comment

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

For links that wrap an icon-only button, the accessible name should ideally be on the interactive element that receives focus/activation. Depending on the underlying Button implementation, focus may land on the <a> rather than the inner control, and the label on the inner button may not be used.

Consider moving the accessible name to the anchor (or ensuring the anchor is the only interactive element) to avoid nested interactive semantics and to guarantee the label is announced correctly.

Suggestion

Avoid nested interactive controls and place the label on the element that actually receives focus:

<a
  href="..."
  target="_blank"
  rel="noopener noreferrer"
  aria-label="Upgrade"
  className="inline-flex"
>
  <TentTree ... />
</a>

Alternatively, use your Button component’s asChild pattern (if available) so the anchor is the rendered element while keeping button styling. Reply with "@CharlieHelps yes please" if you want me to prepare a commit applying this pattern here.

<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" />
</Button>
<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