🎨 Palette: Add dynamic accessibility context to list action buttons#75
🎨 Palette: Add dynamic accessibility context to list action buttons#75NSEvent wants to merge 1 commit into
Conversation
Added dynamic context to `.help()` and `.accessibilityLabel()` modifiers for the Edit and Delete buttons in `MacroListView.swift` and `ScriptListView.swift`. Previously, these icon-only buttons relied on static labels like "Edit" or "Delete", which caused disambiguation issues for screen reader users and tooltip confusion when navigating repeated lists. The labels now intelligently fallback to the item's name or a default if empty (e.g., "Edit Script Name" or "Delete Macro Name"), ensuring a fully accessible and intuitive micro-UX experience. 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. |
📝 WalkthroughWalkthroughThis PR updates edit and delete button accessibility metadata in MacroListView and ScriptListView to dynamically include the macro or script name in ChangesDynamic accessibility labels
Estimated code review effort: 1 (Trivial) | ~5 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
XboxControllerMapper/XboxControllerMapper/Views/Macros/MacroListView.swift (1)
158-159: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract repeated label computation to avoid help/accessibilityLabel drift.
The empty-name ternary is duplicated between
.help()and.accessibilityLabel()in bothSharedMacroRowandMacroRow. If the fallback text changes later, it's easy to update one and forget the other, causing tooltip/screen-reader text to diverge.♻️ Suggested refactor
Button(action: onEdit) { Image(systemName: "pencil") .foregroundColor(.accentColor) } .buttonStyle(.borderless) - .help(macro.name.isEmpty ? "Edit in shared library" : "Edit \(macro.name) in shared library") - .accessibilityLabel(macro.name.isEmpty ? "Edit in shared library" : "Edit \(macro.name) in shared library") + .help(editLabel) + .accessibilityLabel(editLabel)and similarly for
MacroRow's edit/delete buttons, derivingeditLabel/deleteLabelas computed properties on the row.Also applies to: 209-218
🤖 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` around lines 158 - 159, Extract the repeated edit/delete label text used by SharedMacroRow and MacroRow into shared computed values (for example editLabel and deleteLabel on each row) and reuse them for both .help() and .accessibilityLabel(). Keep the label derivation in one place so the tooltip and accessibility text stay identical when the fallback or macro-name formatting changes.XboxControllerMapper/XboxControllerMapper/Views/Scripts/ScriptListView.swift (2)
225-234: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSame duplicated ternary as MacroListView.swift.
Same DRY concern applies here — the empty-name fallback logic is repeated across
.help()/.accessibilityLabel()for both edit and delete.🤖 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 225 - 234, The edit/delete labels in ScriptListView duplicate the same ternary fallback logic across .help() and .accessibilityLabel(), so extract the shared display string for each action instead of repeating script.name.isEmpty checks. Update the Button section in ScriptListView to reuse a local label/value for the edit and delete text, similar to the DRY fix in MacroListView, while keeping the same accessibility and help behavior.
198-198: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winInconsistent empty-name fallback: display uses "Untitled Script" but a11y text drops the item type entirely.
Line 198 already establishes
"Untitled Script"as the canonical fallback name for this row. The edit/delete accessibility text at Lines 225-234 instead falls back to bare"Edit"/"Delete", losing the item-type context that the rest of the PR is specifically trying to add. Reusing the same fallback keeps tooltip/screen-reader text consistent with what's displayed on screen.♻️ Suggested fix
- .help(script.name.isEmpty ? "Edit" : "Edit \(script.name)") - .accessibilityLabel(script.name.isEmpty ? "Edit" : "Edit \(script.name)") + .help("Edit \(script.name.isEmpty ? "Untitled Script" : script.name)") + .accessibilityLabel("Edit \(script.name.isEmpty ? "Untitled Script" : script.name)") ... - .help(script.name.isEmpty ? "Delete" : "Delete \(script.name)") - .accessibilityLabel(script.name.isEmpty ? "Delete" : "Delete \(script.name)") + .help("Delete \(script.name.isEmpty ? "Untitled Script" : script.name)") + .accessibilityLabel("Delete \(script.name.isEmpty ? "Untitled Script" : script.name)")Also applies to: 225-234
🤖 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` at line 198, The row in ScriptListView is using different fallback text for display versus accessibility actions, so align the edit/delete labels with the same canonical empty-name fallback used by the script title. Update the accessibility text in the ScriptListView row so the labels built around the script’s name keep the item type context when script.name is empty, instead of falling back to bare action names. Reuse the same fallback logic already used by the Text display for the edit/delete accessibility strings.
🤖 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`:
- Around line 158-159: Extract the repeated edit/delete label text used by
SharedMacroRow and MacroRow into shared computed values (for example editLabel
and deleteLabel on each row) and reuse them for both .help() and
.accessibilityLabel(). Keep the label derivation in one place so the tooltip and
accessibility text stay identical when the fallback or macro-name formatting
changes.
In
`@XboxControllerMapper/XboxControllerMapper/Views/Scripts/ScriptListView.swift`:
- Around line 225-234: The edit/delete labels in ScriptListView duplicate the
same ternary fallback logic across .help() and .accessibilityLabel(), so extract
the shared display string for each action instead of repeating
script.name.isEmpty checks. Update the Button section in ScriptListView to reuse
a local label/value for the edit and delete text, similar to the DRY fix in
MacroListView, while keeping the same accessibility and help behavior.
- Line 198: The row in ScriptListView is using different fallback text for
display versus accessibility actions, so align the edit/delete labels with the
same canonical empty-name fallback used by the script title. Update the
accessibility text in the ScriptListView row so the labels built around the
script’s name keep the item type context when script.name is empty, instead of
falling back to bare action names. Reuse the same fallback logic already used by
the Text display for the edit/delete accessibility strings.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 1f5828ce-b64f-4629-a995-db752e57b145
📒 Files selected for processing (3)
.Jules/palette.mdXboxControllerMapper/XboxControllerMapper/Views/Macros/MacroListView.swiftXboxControllerMapper/XboxControllerMapper/Views/Scripts/ScriptListView.swift
1 participant
💡 What:
Added dynamic context to
.help()and.accessibilityLabel()modifiers for the Edit and Delete buttons inMacroRow,SharedMacroRow, andScriptRow.🎯 Why:
In repeated list views, generic static labels like "Edit" or "Delete" create significant confusion for screen reader users and provide incomplete context via tooltips. By incorporating the specific item's name (e.g.,
macro.nameorscript.name), each button becomes explicitly clear about what action it performs on which item.♿ Accessibility:
Resolves a WCAG violation regarding ambiguous control labels in repeated lists. Screen readers will now announce exactly which script or macro is being edited or deleted. Fallback logic is included to ensure empty names do not break string interpolation.
PR created automatically by Jules for task 6052923481835110309 started by @NSEvent
Summary by CodeRabbit