🎨 Palette: Replace .onTapGesture with Button in list views for better accessibility - #96
🎨 Palette: Replace .onTapGesture with Button in list views for better accessibility#96NSEvent wants to merge 1 commit into
.onTapGesture with Button in list views for better accessibility#96Conversation
…tter accessibility - Refactored `MacroListView.swift` and `ScriptListView.swift` to use `Button` wrapping instead of `.onTapGesture` on list items. - Added proper `.help()` and `.accessibilityLabel()` dynamically bound to the item's name. - Ensures the UI remains keyboard navigable (e.g., standard tab order and focus states) and accessible to VoiceOver. - Logged accessibility learnings to `.Jules/palette.md`. Co-authored-by: NSEvent <44446865+NSEvent@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. |
📝 WalkthroughWalkthroughMacro and script row edit interactions now use plain-styled SwiftUI buttons instead of tap gestures, with rectangular hit areas and accessibility labels/tooltips. The palette documentation records the keyboard navigation and accessibility rationale. ChangesAccessible row edit actions
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
XboxControllerMapper/XboxControllerMapper/Views/Macros/MacroListView.swift (1)
190-190: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd fallback text for empty macro names.
For consistency with the
.helpand.accessibilityLabelmodifiers below, consider providing a fallback string whenmacro.nameis empty. This also aligns with how empty names are handled inScriptListView.swift.♻️ Proposed refactor
- Text(macro.name) + Text(macro.name.isEmpty ? "Unnamed Macro" : macro.name)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@XboxControllerMapper/XboxControllerMapper/Views/Macros/MacroListView.swift` at line 190, Update the macro name display in MacroListView to use the same fallback string as its nearby help and accessibilityLabel modifiers when macro.name is empty. Preserve the existing macro.name value for non-empty names and align the behavior with ScriptListView.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@XboxControllerMapper/XboxControllerMapper/Views/Macros/MacroListView.swift`:
- Line 190: Update the macro name display in MacroListView to use the same
fallback string as its nearby help and accessibilityLabel modifiers when
macro.name is empty. Preserve the existing macro.name value for non-empty names
and align the behavior with ScriptListView.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 1c21e7c5-d208-45e4-9835-2d78b4ad0644
📒 Files selected for processing (3)
.Jules/palette.mdXboxControllerMapper/XboxControllerMapper/Views/Macros/MacroListView.swiftXboxControllerMapper/XboxControllerMapper/Views/Scripts/ScriptListView.swift
💡 What:
Replaced standard
HStackrows equipped with.onTapGesturemodifiers with semanticButtoncomponents using.buttonStyle(.plain)inMacroListView.swiftandScriptListView.swift. Added dynamic.help()and.accessibilityLabel()bindings to these new interactive areas.🎯 Why:
Using
.onTapGesturedirectly on views effectively breaks native keyboard navigation; these items lose their focus states and are removed from the native tab order. Additionally, without a semantic interactive trait, screen readers struggle to convey what action the user can perform. By migrating to aButton, we restore these vital interactive behaviors and accessibility affordances without altering the visual design of the lists.♿ Accessibility:
.accessibilityLabelindicating exactly which macro or script is being edited (e.g., "Edit 'My Script'")..helpstrings to provide identical context via visual tooltips on hover.PR created automatically by Jules for task 6216224137302109714 started by @NSEvent
Summary by CodeRabbit