Make keyboard focus visible on the composer and search fields - #394
Merged
Merged
Conversation
A new rendered test tabs through every stop in both schemes and compares each one unfocused and focused: the pixels that change must include a ring's worth whose colour moved by at least 3:1. It found the composer showed no such change at all. Its focus border was a 45% accent mix and its glow 12%, both too faint to see. It now takes a full-strength 2px ring (border plus a 1px spread) and keeps the soft glow outside it. The familiar search, find bar and screen-viewer fields showed focus only as a 1px border or a 15% halo; they take the same 2px ring. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Moderate findings remain around conditional-field coverage and screenshot baseline reliability.
Review effort: Lite
Findings: 1
Open (1)
What changed in this PR
Improves keyboard focus visibility for the composer, search, find-bar, and screen-viewer fields, with dark/light scheme E2E coverage.
Changes:
- Strengthens focus rings across affected controls.
- Adds screenshot-based keyboard focus checks.
Review findings:
- Moderate (3 votes): The sweep does not open the conditional find bar or screen viewer, leaving their focus rules untested.
- Moderate (1 vote): A single baseline screenshot can allow scrolling changes to produce false positives.
- Moderate (1 vote): The find-bar focus state is not visually asserted.
- Moderate (1 vote): The screen viewer uses an unavailable relay and is not rendered during the test.
| File | Description |
|---|---|
src/ui/ui.css |
Strengthens the composer focus ring. |
src/coven/screen-viewer.css |
Adds a visible screen-input focus ring. |
src/coven/chat-app.css |
Adds focus rings for search and find controls. |
e2e/app.tauri-mock.spec.ts |
Adds dark/light scheme focus-visibility testing. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Review noted both panels render only when open, so the sweep never reached the fields whose focus rings this branch changes. They are now opened before the baseline, and the sweep must reach "Find in conversation" and "Screen address". Fields are named by their label. With the previous styles the sweep fails on the search, find, screen address and password fields. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
New test. A rendered e2e test tabs through every stop in both colour schemes. For each stop it compares the screen before and after focus, and requires the change to include at least one ring's worth of pixels whose colour moved by 3:1 or more. That follows the WCAG 2.4.7 and 2.4.13 measures. For a field whose container shows focus, it measures the container: the composer, the familiar search and the find bar.
Fixes the test found:
Verification
playwright test: 55 passed, including the new focus test in both schemes.Message Local familiar: 0/1363.vitest run src/ui src/coven src/design: 305 passed.chat-app.css(.coven-find input:focus-visible), on a line this PR does not change.🤖 Generated with Claude Code