-
Notifications
You must be signed in to change notification settings - Fork 0
🎨 Palette: Improve accessibility of playground inputs #51
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| 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-29 - Missing ARIA labels and focus states on interactive elements in playground | ||
| **Learning:** Verified that missing explicit `aria-label` attributes and visually prominent focus states (by stripping with `focus:outline-none`) are a recurring issue for custom inputs and selects, especially those nested inside wrapper components like `<Control>` that do not natively handle `htmlFor`. | ||
| **Action:** Always add explicit `aria-label` attributes to form elements where the visual label isn't directly bound, and replace `focus:outline-none` with `focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-[var(--strand-color-accent-lede)] focus:outline-none` to ensure keyboard accessibility. |
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -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="Guide" | ||||||||||||||
| className="pp-select focus:outline-none focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-[var(--strand-color-accent-lede)]" | ||||||||||||||
| onChange={(e) => setGuideSlug(e.target.value)} | ||||||||||||||
| value={guideSlug} | ||||||||||||||
| > | ||||||||||||||
|
|
@@ -309,7 +310,8 @@ export function PlaygroundView({ | |||||||||||||
|
|
||||||||||||||
| <Control label="Preset"> | ||||||||||||||
| <select | ||||||||||||||
| className="pp-select" | ||||||||||||||
| aria-label="Preset" | ||||||||||||||
| className="pp-select focus:outline-none focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-[var(--strand-color-accent-lede)]" | ||||||||||||||
|
Comment on lines
312
to
+314
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The More importantly, hardcoding
Suggested change
|
||||||||||||||
| onChange={(e) => setPresetSlug(e.target.value as UseCase | "")} | ||||||||||||||
| value={presetSlug} | ||||||||||||||
| > | ||||||||||||||
|
|
@@ -326,7 +328,8 @@ export function PlaygroundView({ | |||||||||||||
|
|
||||||||||||||
| <Control label={`Temperature · ${temperature.toFixed(1)}`}> | ||||||||||||||
| <input | ||||||||||||||
| className="w-full" | ||||||||||||||
| aria-label="Temperature" | ||||||||||||||
| className="w-full focus:outline-none focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-[var(--strand-color-accent-lede)]" | ||||||||||||||
| max="1" | ||||||||||||||
| min="0" | ||||||||||||||
| onChange={(e) => setTemperature(Number.parseFloat(e.target.value))} | ||||||||||||||
|
|
@@ -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" | ||||||||||||||
| className="min-h-[220px] w-full resize-y bg-transparent text-sm focus:outline-none focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-[var(--strand-color-accent-lede)]" | ||||||||||||||
| onChange={(e) => setInput(e.target.value)} | ||||||||||||||
| placeholder="Paste a customer message, a prompt, a paragraph to rewrite…" | ||||||||||||||
| style={{ color: "var(--strand-color-ink-primary)" }} | ||||||||||||||
|
|
@@ -363,6 +367,7 @@ export function PlaygroundView({ | |||||||||||||
| > | ||||||||||||||
| <input | ||||||||||||||
| checked={humanizeExtraPass} | ||||||||||||||
| className="focus:outline-none focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-[var(--strand-color-accent-lede)]" | ||||||||||||||
| disabled={isRunning} | ||||||||||||||
| onChange={(e) => setHumanizeExtraPass(e.target.checked)} | ||||||||||||||
| style={{ | ||||||||||||||
|
|
@@ -720,6 +725,7 @@ function HumanizeToggle({ | |||||||||||||
| > | ||||||||||||||
| <input | ||||||||||||||
| checked={on} | ||||||||||||||
| className="focus:outline-none focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-[var(--strand-color-accent-lede)]" | ||||||||||||||
| disabled={disabled || !authenticated} | ||||||||||||||
| onChange={onToggle} | ||||||||||||||
| style={{ accentColor: "var(--strand-color-accent-lede)" }} | ||||||||||||||
|
|
||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
<select>element is nested inside the<Control>component, which already renders an implicit HTML<label>wrapping its children. Because of this, the<select>is already implicitly associated with the label text 'Guide', making the explicitaria-label="Guide"redundant.More importantly, hardcoding
aria-labelintroduces a maintenance risk: if the visual label text in<Control>is ever updated or localized in the future, thearia-labelwill become out of sync. Consider removing the redundantaria-labelattribute.