🎨 Palette: Add focus-visible styles to ConversionAssets buttons#956
🎨 Palette: Add focus-visible styles to ConversionAssets buttons#956
Conversation
Co-authored-by: anchapin <6326294+anchapin@users.noreply.github.com>
|
👋 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. |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdds keyboard-accessible :focus-visible outline styles to all ConversionAssets action buttons and documents the accessibility pattern in the Jules palette notes. State diagram for button visual states with focus-visible stylesstateDiagram-v2
[*] --> Idle
Idle --> Hover
Idle --> FocusVisible
Hover --> Idle
Hover --> FocusVisible
FocusVisible --> Idle
state Idle {
[*] --> Default
Default: No_outline
}
state Hover {
[*] --> Hovered
Hovered: Hover_styles_only
}
state FocusVisible {
[*] --> KeyboardFocused
KeyboardFocused: Outline_2px_solid_3b82f6_with_outline_offset_2px
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The hard-coded
#3b82f6focus outline color should likely be replaced with a Palette design token or existing theme variable so it stays consistent with the rest of the system and is easier to update later. - The long selector list for
:focus-visiblesuggests these buttons could share a common class (e.g..ca-button) so the focus style is defined once and new buttons automatically inherit it.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The hard-coded `#3b82f6` focus outline color should likely be replaced with a Palette design token or existing theme variable so it stays consistent with the rest of the system and is easier to update later.
- The long selector list for `:focus-visible` suggests these buttons could share a common class (e.g. `.ca-button`) so the focus style is defined once and new buttons automatically inherit it.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Pull request overview
This PR improves keyboard accessibility in the Conversion Assets UI by adding explicit :focus-visible styling for key action buttons, helping keyboard users identify the currently focused control.
Changes:
- Added
:focus-visibleoutline/offset styling for multiple button classes inConversionAssets.css. - Added a Palette learning note documenting the accessibility pattern and recommendation.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| frontend/src/components/ConversionAssets/ConversionAssets.css | Adds component-scoped :focus-visible styles for ConversionAssets action buttons. |
| .jules/palette.md | Documents the accessibility learning/action for using :focus-visible focus indicators. |
| /* Accessibility: Focus-visible for action buttons */ | ||
| .refresh-button:focus-visible, | ||
| .retry-button:focus-visible, | ||
| .action-button:focus-visible, | ||
| .upload-button:focus-visible, | ||
| .close-button:focus-visible, | ||
| .edit-metadata-button:focus-visible, | ||
| .save-metadata-button:focus-visible, | ||
| .convert-asset-button:focus-visible, | ||
| .close-details-button:focus-visible, | ||
| .convert-all-button:focus-visible { | ||
| outline: 2px solid #3b82f6; | ||
| outline-offset: 2px; |
There was a problem hiding this comment.
The PR description says this change improves keyboard focus "without affecting mouse users", but these rules only style :focus-visible. Mouse clicks can still trigger the global button:focus outline from frontend/src/index.css (button:focus, button:focus-visible), so mouse users may still see a focus outline. If the intent is to show focus rings only for keyboard navigation, consider removing/adjusting the global button:focus rule or explicitly overriding :focus (non-focus-visible) for these button classes.
Co-authored-by: anchapin <6326294+anchapin@users.noreply.github.com>
💡 What: Added
:focus-visiblestyles to interactive buttons inConversionAssets.css.🎯 Why: Enhances keyboard accessibility by providing clear visual focus indicators, ensuring keyboard users can easily see which element is active without affecting mouse users.
♿ Accessibility: Improved keyboard navigation support for ConversionAssets UI elements.
PR created automatically by Jules for task 59125363831365138 started by @anchapin
Summary by Sourcery
Add keyboard-focus visible styling to ConversionAssets action buttons and document the accessibility pattern in Palette guidelines.
Enhancements:
Documentation: