🎨 Palette: Add focus states and ARIA labels to Playground controls#61
🎨 Palette: Add focus states and ARIA labels to Playground controls#61aloewright wants to merge 1 commit into
Conversation
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
postpilot | 092ba1c | Jun 01 2026, 06:00 AM |
|
Warning Review limit reached
More reviews will be available in 29 minutes and 45 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request improves accessibility in the playground view by adding ARIA labels and visual focus-visible states to various form controls, as well as documenting a Playwright focus state verification technique in the learning log. Feedback suggests correcting the Playwright API reference in the log from the Python-specific page.eval_on_selector to the Node.js/TypeScript locator.evaluate(), and simplifying the Tailwind outline classes by using the theme-configured focus-visible:outline-lede utility instead of arbitrary values.
| ## 2024-06-01 - Focus state verification via Playwright | ||
| **Learning:** Found that when verifying focus states via headless Playwright scripts, using `page.eval_on_selector('selector', 'el => el.focus()')` may be more reliable for capturing visual focus indicators in screenshots than using standard `locator.focus()` or keyboard navigation. | ||
| **Action:** Use `page.eval_on_selector` to focus elements when capturing screenshots to verify `focus-visible` styling changes. |
There was a problem hiding this comment.
In Playwright for Node.js/TypeScript (which this project uses), there is no page.eval_on_selector method. That method name is specific to the Playwright Python API.
For Node.js/TypeScript, the correct and idiomatic way to evaluate a function on a selector is using page.$eval or, preferably, the modern locator.evaluate() API:
await page.locator('selector').evaluate(el => el.focus());Let's update this log entry to reflect the correct API for this codebase to avoid confusing other developers.
| ## 2024-06-01 - Focus state verification via Playwright | |
| **Learning:** Found that when verifying focus states via headless Playwright scripts, using `page.eval_on_selector('selector', 'el => el.focus()')` may be more reliable for capturing visual focus indicators in screenshots than using standard `locator.focus()` or keyboard navigation. | |
| **Action:** Use `page.eval_on_selector` to focus elements when capturing screenshots to verify `focus-visible` styling changes. | |
| ## 2024-06-01 - Focus state verification via Playwright | |
| **Learning:** Found that when verifying focus states via headless Playwright scripts, using page.locator('selector').evaluate(el => el.focus()) may be more reliable for capturing visual focus indicators in screenshots than using standard locator.focus() or keyboard navigation. | |
| **Action:** Use locator.evaluate to focus elements when capturing screenshots to verify focus-visible styling changes. |
| <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)]" |
There was a problem hiding this comment.
Since --color-lede is already mapped to var(--strand-color-accent-lede) in the Tailwind @theme configuration (defined in apps/quill/client/index.css), you can use the cleaner theme-configured utility focus-visible:outline-lede instead of the verbose arbitrary value syntax focus-visible:outline-[var(--strand-color-accent-lede)].
| className="w-full focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-[var(--strand-color-accent-lede)]" | |
| className="w-full focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-lede" |
| <textarea | ||
| className="min-h-[220px] w-full resize-y bg-transparent text-sm focus:outline-none" | ||
| aria-label="Input text" | ||
| 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)]" |
There was a problem hiding this comment.
Since --color-lede is already mapped to var(--strand-color-accent-lede) in the Tailwind @theme configuration (defined in apps/quill/client/index.css), you can use the cleaner theme-configured utility focus-visible:outline-lede instead of the verbose arbitrary value syntax focus-visible:outline-[var(--strand-color-accent-lede)].
| 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)]" | |
| 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-lede" |
There was a problem hiding this comment.
Pull request overview
This PR improves accessibility of the Quill Playground controls by adding keyboard-visible focus styles and providing accessible names for controls that are not programmatically labeled by visible text.
Changes:
- Added
aria-labelattributes to several Playground form controls. - Added
focus-visibleoutline styles to the temperature slider and input textarea. - Updated
.pp-selectstyling to include a:focus-visibleoutline for keyboard navigation.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| apps/quill/client/components/playground-view.tsx | Adds ARIA labeling and focus-visible styling to Playground controls (selects, range input, textarea) and updates .pp-select CSS for keyboard focus rings. |
| .Jules/palette.md | Documents an accessibility/testing “learning” related to verifying focus states with Playwright. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <Control label="Guide"> | ||
| <select | ||
| aria-label="Select guide" | ||
| className="pp-select" | ||
| onChange={(e) => setGuideSlug(e.target.value)} | ||
| value={guideSlug} |
| <Control label="Preset"> | ||
| <select | ||
| aria-label="Select preset" | ||
| className="pp-select" | ||
| onChange={(e) => setPresetSlug(e.target.value as UseCase | "")} | ||
| value={presetSlug} |
| <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)]" | ||
| max="1" |
💡 What: Added explicit
aria-labelattributes to the guide<select>, preset<select>, temperature<input type="range">, and input<textarea>elements in the Playground View. Addedfocus-visibleoutline styles directly via Tailwind classes to the<textarea>and temperature input, and updated.pp-selectCSS to include a focus ring on keyboard navigation.🎯 Why: The
<Control>and<Panel>UI wrappers inplayground-view.tsxdo not intrinsically associate a<label>element with their nested controls, making it difficult for screen readers to identify the input's purpose. Additionally, navigating these form controls via keyboard previously showed no visual focus indicators.📸 Before/After: Visually, the default rendering is unchanged. However, upon keyboard navigation, a clear focus ring using
--strand-color-accent-ledeappears around the currently focused dropdown, slider, or textarea.♿ Accessibility:
aria-labels to un-linked form controls for screen-reader support.focus-visiblestyling patterns across the form.PR created automatically by Jules for task 18261121751620298781 started by @aloewright