🎨 Palette: Add dynamic context to list accessibility#70
Conversation
Added dynamic, item-specific context to icon-only edit and delete buttons in SwiftUI repeated lists (`ScriptListView`, `MacroListView`, and `ChordSequenceListViews`) to disambiguate actions for screen readers and tooltip users. Updated `.Jules/palette.md` to document this learning. 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. |
📝 WalkthroughWalkthroughIcon-only action buttons (Edit, Delete) in Contextual accessibility labels for list row buttons
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Poem
🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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.
Inline comments:
In
`@XboxControllerMapper/XboxControllerMapper/Views/MainWindow/ChordSequenceListViews.swift`:
- Around line 118-127: The tooltip and accessibility text for chord actions
still use actionDisplayString, which collapses macro/script entries to generic
labels. Update ChordRow so its .help and .accessibilityLabel use the
already-resolved chord action name (for example via the existing resolved label
logic) instead of actionDisplayString, and apply the same change to both button
groups referenced in ChordSequenceListViews.
In
`@XboxControllerMapper/XboxControllerMapper/Views/Scripts/ScriptListView.swift`:
- Around line 132-133: The example card action is replacing the synthesized
accessibility label with only “Use … example,” which removes the visible
title/description context. In ScriptListView’s example card accessibility
modifier, keep the existing content from example.name and example.description
instead of setting a custom accessibilityLabel, and move the action wording into
the accessibility hint so the card remains fully descriptive to screen readers.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 5c1e52cc-c5a1-4180-880a-b113f63b00f8
📒 Files selected for processing (4)
.Jules/palette.mdXboxControllerMapper/XboxControllerMapper/Views/Macros/MacroListView.swiftXboxControllerMapper/XboxControllerMapper/Views/MainWindow/ChordSequenceListViews.swiftXboxControllerMapper/XboxControllerMapper/Views/Scripts/ScriptListView.swift
| .help("Edit chord \(chord.actionDisplayString)") | ||
| .accessibilityLabel("Edit chord \(chord.actionDisplayString)") | ||
|
|
||
| Button(action: onDelete) { | ||
| Image(systemName: "trash") | ||
| .foregroundColor(.red.opacity(0.8)) | ||
| } | ||
| .buttonStyle(.borderless) | ||
| .help("Delete") | ||
| .accessibilityLabel("Delete") | ||
| .help("Delete chord \(chord.actionDisplayString)") | ||
| .accessibilityLabel("Delete chord \(chord.actionDisplayString)") |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Use the resolved action name here, not actionDisplayString.
Lines 118-127 and 264-273 still become generic for macro/script mappings because actionDisplayString returns "Macro" / "Script" in those cases. ChordRow already resolves the actual macro/script name for the visible label, so the new tooltip/VoiceOver text remains ambiguous in repeated lists.
♿ Suggested direction
- .help("Edit chord \(chord.actionDisplayString)")
- .accessibilityLabel("Edit chord \(chord.actionDisplayString)")
+ .help("Edit chord \(resolvedChordActionName)")
+ .accessibilityLabel("Edit chord \(resolvedChordActionName)")
- .help("Delete sequence \(sequence.actionDisplayString)")
- .accessibilityLabel("Delete sequence \(sequence.actionDisplayString)")
+ .help("Delete sequence \(resolvedSequenceActionName)")
+ .accessibilityLabel("Delete sequence \(resolvedSequenceActionName)")private var resolvedChordActionName: String {
if let systemCommand = chord.systemCommand { return chord.hint ?? systemCommand.displayName }
if let macroId = chord.macroId,
let profile = profileManager.activeProfile,
let macroName = profile.macroDisplayName(for: macroId) {
return chord.hint ?? macroName
}
if let scriptId = chord.scriptId,
let profile = profileManager.activeProfile,
let script = profile.scripts.first(where: { $0.id == scriptId }) {
return chord.hint ?? script.name
}
return chord.hint ?? chord.actionDisplayString
}Also applies to: 264-273
🤖 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/MainWindow/ChordSequenceListViews.swift`
around lines 118 - 127, The tooltip and accessibility text for chord actions
still use actionDisplayString, which collapses macro/script entries to generic
labels. Update ChordRow so its .help and .accessibilityLabel use the
already-resolved chord action name (for example via the existing resolved label
logic) instead of actionDisplayString, and apply the same change to both button
groups referenced in ChordSequenceListViews.
| .help("Use \(example.name) example") | ||
| .accessibilityLabel("Use \(example.name) example") |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Don’t replace the example card’s synthesized label.
Line 133 likely strips out the visible description from the accessibility tree and reduces the button to just “Use … example”. These cards already expose example.name and example.description; keeping that content and moving the action wording to a hint preserves more context for screen-reader users.
♿ Suggested fix
.help("Use \(example.name) example")
-.accessibilityLabel("Use \(example.name) example")
+.accessibilityHint("Use \(example.name) example")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .help("Use \(example.name) example") | |
| .accessibilityLabel("Use \(example.name) example") | |
| .help("Use \(example.name) example") | |
| .accessibilityHint("Use \(example.name) example") |
🤖 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/Scripts/ScriptListView.swift`
around lines 132 - 133, The example card action is replacing the synthesized
accessibility label with only “Use … example,” which removes the visible
title/description context. In ScriptListView’s example card accessibility
modifier, keep the existing content from example.name and example.description
instead of setting a custom accessibilityLabel, and move the action wording into
the accessibility hint so the card remains fully descriptive to screen readers.
Added dynamic, item-specific context to icon-only edit and delete buttons in SwiftUI repeated lists (`ScriptListView`, `MacroListView`, and `ChordSequenceListViews`) to disambiguate actions for screen readers and tooltip users. Updated `.Jules/palette.md` to document this learning. Co-authored-by: NSEvent <44446865+NSEvent@users.noreply.github.com>
1 participant
🎨 Palette: Add dynamic context to list accessibility
💡 What:
Added dynamic, item-specific context to the
.help()and.accessibilityLabel()modifiers on icon-only edit and delete buttons in various list views (ScriptListView,MacroListView, andChordSequenceListViews).🎯 Why:
When rendering repeated lists (using
ForEach), providing static "Edit" or "Delete" labels for icon-only buttons makes the interface ambiguous for users relying on screen readers or pointer tooltips, as they lack context on which item the action targets. Dynamic context (e.g., "Edit [Macro Name]") disambiguates these actions and significantly improves accessibility.♿ Accessibility:
PR created automatically by Jules for task 13112545269818779337 started by @NSEvent
Summary by CodeRabbit