Skip to content

🎨 Palette: [UX improvement] Add ARIA labels and focus states to playground controls #39

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

🎨 Palette: [UX improvement] Add ARIA labels and focus states to playground controls #39

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
@@ -1,3 +1,6 @@
## 2025-03-05 - Form controls missing focus indicators and ARIA labels
**Learning:** Found that custom search inputs and select dropdowns were using `focus:outline-none` but missing visual focus states, impairing keyboard accessibility. Additionally, they were missing explicit ARIA labels.
**Action:** Always ensure any interactive control using `focus:outline-none` has a corresponding `focus-visible:` ring state added (using standard `focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-[var(--strand-color-accent-lede)]`), and verify inputs have `aria-label` or related `aria-labelledby`.
## 2025-05-24 - Form controls nested in generic wrapper components need ARIA labels
**Learning:** Custom UI wrappers like `<Control>` and `<Panel>` that rely exclusively on passing child inputs rather than using `htmlFor` and `id` cause interactive elements to be orphaned from a screen reader perspective, requiring explicit `aria-label` attributes directly on the child `select`, `input`, or `textarea`.
**Action:** Always verify elements nested inside `<Control>` and `<Panel>` have explicit `aria-label` attributes to ensure screen reader accessibility, alongside restoring keyboard focus states.

@gemini-code-assist gemini-code-assist Bot May 24, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The advice to 'Always verify elements... have explicit aria-label attributes' is slightly misleading. In HTML, nesting an input inside a <label> (as the <Control> component does) is a standard and valid way to provide an accessible name. Explicit aria-label attributes are only strictly necessary if the label text is not associated via nesting or id/htmlFor links.

Suggested change
**Action:** Always verify elements nested inside `<Control>` and `<Panel>` have explicit `aria-label` attributes to ensure screen reader accessibility, alongside restoring keyboard focus states.
**Action:** Always ensure interactive elements nested inside `<Control>` and `<Panel>` have accessible names, either via implicit nesting in a `<label>` or explicit `aria-label` attributes, alongside restoring keyboard focus states.

12 changes: 8 additions & 4 deletions apps/quill/client/components/playground-view.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,8 @@ export function PlaygroundView({
<div className="mb-6 grid gap-3 md:grid-cols-3">
<Control label="Guide">
<select
className="pp-select"
aria-label="Select guide"

@gemini-code-assist gemini-code-assist Bot May 24, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The aria-label values are inconsistent across the form controls (e.g., 'Select guide' vs 'Temperature'). For better accessibility and consistency with the visual labels, it is recommended to use simple noun-based labels that match the visual text. Additionally, since these elements are already nested within a <label> tag in the Control component, explicit aria-label attributes are technically redundant for modern screen readers.

Suggested change
aria-label="Select guide"
aria-label="Guide"

className="pp-select focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-[var(--strand-color-accent-lede)] focus:outline-none"

@gemini-code-assist gemini-code-assist Bot May 24, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The focus ring utility classes are repeated across multiple elements in this file (lines 299, 314, 332, 348). To improve maintainability and ensure a consistent focus style, consider defining these classes in a shared constant or adding them to the .pp-select CSS rule in the style block at the bottom of the file.

onChange={(e) => setGuideSlug(e.target.value)}
value={guideSlug}
>
Expand All @@ -309,7 +310,8 @@ export function PlaygroundView({

<Control label="Preset">
<select
className="pp-select"
aria-label="Select preset"

@gemini-code-assist gemini-code-assist Bot May 24, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Consistency: Use a noun-based label to match the visual label and other controls.

Suggested change
aria-label="Select preset"
aria-label="Preset"

className="pp-select focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-[var(--strand-color-accent-lede)] focus:outline-none"
onChange={(e) => setPresetSlug(e.target.value as UseCase | "")}
value={presetSlug}
>
Expand All @@ -326,7 +328,8 @@ export function PlaygroundView({

<Control label={`Temperature · ${temperature.toFixed(1)}`}>
<input
className="w-full"
aria-label="Temperature"
className="w-full focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-[var(--strand-color-accent-lede)] focus:outline-none"
max="1"
min="0"
onChange={(e) => setTemperature(Number.parseFloat(e.target.value))}
Expand All @@ -341,7 +344,8 @@ export function PlaygroundView({
<div className="grid gap-5 md:grid-cols-2">
<Panel label="Input">
<textarea
className="min-h-[220px] w-full resize-y bg-transparent text-sm focus:outline-none"
aria-label="Input text"

@gemini-code-assist gemini-code-assist Bot May 24, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Consistency: Use a noun-based label to match the visual label ('Input') and other controls.

Suggested change
aria-label="Input text"
aria-label="Input"

className="min-h-[220px] w-full resize-y bg-transparent text-sm focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-[var(--strand-color-accent-lede)] focus:outline-none"
onChange={(e) => setInput(e.target.value)}
placeholder="Paste a customer message, a prompt, a paragraph to rewrite…"
style={{ color: "var(--strand-color-ink-primary)" }}
Expand Down
Loading