Skip to content

🎨 Palette: [UX improvement] Fix Hotkey section accessibility#419

Open
google-labs-jules[bot] wants to merge 3 commits into
mainfrom
palette-accessibility-hotkey-13154857657782863106
Open

🎨 Palette: [UX improvement] Fix Hotkey section accessibility#419
google-labs-jules[bot] wants to merge 3 commits into
mainfrom
palette-accessibility-hotkey-13154857657782863106

Conversation

@google-labs-jules

Copy link
Copy Markdown
Contributor

💡 What: Added AutomationProperties.LabeledBy to the Global Hotkey modifier ComboBox and key TextBox in SettingsWindow.xaml.
🎯 Why: Enhances the interface for screen readers, providing proper context when interacting with hotkey input elements.
♿ Accessibility: Ensures that users relying on screen reading software understand which setting the ComboBox and TextBox refer to by correctly linking them to their corresponding visual header.


PR created automatically by Jules for task 13154857657782863106 started by @mikekthx

@google-labs-jules

Copy link
Copy Markdown
Contributor Author

👋 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 @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.


For security, I will only act on instructions from the user who triggered this task.

@google-labs-jules google-labs-jules Bot requested a review from mikekthx as a code owner June 24, 2026 11:34
@github-actions github-actions Bot added documentation Improvements or additions to READMEs, wikis, or inline code comments. ui Front-end changes, WinUI layouts, styling (e.g., Acrylic), or system tray updates. labels Jun 24, 2026
Comment thread .jules/palette.md Outdated
Comment thread SettingsWindow.xaml Outdated
Comment thread SettingsWindow.xaml Outdated
@mikekthx

Copy link
Copy Markdown
Owner

Review: Fix Hotkey Section Accessibility

What this PR does: Adds x:Name to the "Global Hotkey" TextBlock header and wires both the modifier ComboBox and key TextBox to it via AutomationProperties.LabeledBy="{Binding ElementName=…}", with a corresponding palette learning note.

Overall assessment: The intent is good and the XAML pattern is technically correct ({Binding ElementName=…} is the required form per CLAUDE.md, not {x:Bind}). However, there are two blocking issues.


Blocking issues

1. LabeledBy overrides existing, more specific automation names (SettingsWindow.xaml:109, 113)

The .resw files already set per-control accessible names via x:Uid:

  • Settings_HotkeyModifier.AutomationProperties.Name"Hotkey modifier"
  • Settings_HotkeyKey.AutomationProperties.Name"Hotkey key"

In UIA, LabeledBy causes a screen reader to use the referenced element's text as the control's primary accessible name, overriding AutomationProperties.Name. The result is that after this PR both controls are announced identically as "Global Hotkey" — losing the per-control specificity that was already there. This is a regression, not an improvement.

Since the .resw keys already cover accessible naming, the simplest fix is to remove both LabeledBy attributes entirely. If section-context is desired without sacrificing specificity, AutomationProperties.FullDescription (supplementary text, doesn't replace the name) or per-control labels are the right approach.

2. Wrong date in .jules/palette.md (line 25)

The new entry is dated 2026-10-24 but today is 2026-06-24 — four months in the future. CLAUDE.md requires running date +%Y-%m-%d to confirm the actual date before writing any dated entry.


What's correct

  • {Binding ElementName=HotkeyHeader} syntax for LabeledBy is exactly right per CLAUDE.md (the {x:Bind} form causes a silent XAML compiler crash).
  • The + separator TextBlock already has AutomationProperties.AccessibilityView="Raw" — no change needed there.
  • No new .resw strings, P/Invoke declarations, or services are required for this change.

@jules please address the issues noted above

@google-labs-jules

Copy link
Copy Markdown
Contributor Author

Review: Fix Hotkey Section Accessibility

What this PR does: Adds x:Name to the "Global Hotkey" TextBlock header and wires both the modifier ComboBox and key TextBox to it via AutomationProperties.LabeledBy="{Binding ElementName=…}", with a corresponding palette learning note.

Overall assessment: The intent is good and the XAML pattern is technically correct ({Binding ElementName=…} is the required form per CLAUDE.md, not {x:Bind}). However, there are two blocking issues.


Blocking issues

1. LabeledBy overrides existing, more specific automation names (SettingsWindow.xaml:109, 113)

The .resw files already set per-control accessible names via x:Uid:

  • Settings_HotkeyModifier.AutomationProperties.Name"Hotkey modifier"
  • Settings_HotkeyKey.AutomationProperties.Name"Hotkey key"

In UIA, LabeledBy causes a screen reader to use the referenced element's text as the control's primary accessible name, overriding AutomationProperties.Name. The result is that after this PR both controls are announced identically as "Global Hotkey" — losing the per-control specificity that was already there. This is a regression, not an improvement.

Since the .resw keys already cover accessible naming, the simplest fix is to remove both LabeledBy attributes entirely. If section-context is desired without sacrificing specificity, AutomationProperties.FullDescription (supplementary text, doesn't replace the name) or per-control labels are the right approach.

2. Wrong date in .jules/palette.md (line 25)

The new entry is dated 2026-10-24 but today is 2026-06-24 — four months in the future. CLAUDE.md requires running date +%Y-%m-%d to confirm the actual date before writing any dated entry.


What's correct

  • {Binding ElementName=HotkeyHeader} syntax for LabeledBy is exactly right per CLAUDE.md (the {x:Bind} form causes a silent XAML compiler crash).
  • The + separator TextBlock already has AutomationProperties.AccessibilityView="Raw" — no change needed there.
  • No new .resw strings, P/Invoke declarations, or services are required for this change.

@jules please address the issues noted above

Thank you for the detailed feedback. I've removed the AutomationProperties.LabeledBy attributes from the ComboBox and TextBox to preserve their specific accessible names defined in the .resw files. I have also corrected the date in the .jules/palette.md journal entry to today's date.

Comment thread SettingsWindow.xaml
@mikekthx

Copy link
Copy Markdown
Owner

Review: thread resolution + remaining issue

What was resolved

All three prior threads are now resolved:

  • Wrong date in palette.md — corrected from 2026-10-24 to 2026-06-24. ✅
  • LabeledBy overriding AutomationProperties.Name on ComboBoxLabeledBy removed. ✅
  • LabeledBy overriding AutomationProperties.Name on TextBoxLabeledBy removed. ✅

The new palette.md entry is accurate and useful: it correctly captures that LabeledBy supersedes an element-level AutomationProperties.Name and recommends FullDescription as the right alternative when a specific name is already present.


Remaining blocking issue

x:Name="HotkeyHeader" is orphaned.

The name is added to the TextBlock, but no LabeledBy or FullDescription attribute on the ComboBox or TextBox below references it. The PR's original accessibility goal (giving screen readers section-level context for the hotkey controls) is not achieved in the current state — the XAML change is a no-op.

Two valid paths forward:

  1. Implement FullDescription — the approach already documented in palette.md. Add AutomationProperties.FullDescription="{Binding ElementName=HotkeyHeader, Path=Text}" to both the ComboBox and TextBox. This adds "Global Hotkey" as supplementary context without overriding the specific "Hotkey modifier" / "Hotkey key" names from the .resw.

  2. Remove the x:Name — if the accessibility annotation is out of scope for this PR, drop the dead name and ship just the palette.md documentation update.

See the inline comment for the exact suggestion.

@jules please address the issues noted above

@google-labs-jules

Copy link
Copy Markdown
Contributor Author

Review: thread resolution + remaining issue

What was resolved

All three prior threads are now resolved:

  • Wrong date in palette.md — corrected from 2026-10-24 to 2026-06-24. ✅
  • LabeledBy overriding AutomationProperties.Name on ComboBoxLabeledBy removed. ✅
  • LabeledBy overriding AutomationProperties.Name on TextBoxLabeledBy removed. ✅

The new palette.md entry is accurate and useful: it correctly captures that LabeledBy supersedes an element-level AutomationProperties.Name and recommends FullDescription as the right alternative when a specific name is already present.


Remaining blocking issue

x:Name="HotkeyHeader" is orphaned.

The name is added to the TextBlock, but no LabeledBy or FullDescription attribute on the ComboBox or TextBox below references it. The PR's original accessibility goal (giving screen readers section-level context for the hotkey controls) is not achieved in the current state — the XAML change is a no-op.

Two valid paths forward:

  1. Implement FullDescription — the approach already documented in palette.md. Add AutomationProperties.FullDescription="{Binding ElementName=HotkeyHeader, Path=Text}" to both the ComboBox and TextBox. This adds "Global Hotkey" as supplementary context without overriding the specific "Hotkey modifier" / "Hotkey key" names from the .resw.

  2. Remove the x:Name — if the accessibility annotation is out of scope for this PR, drop the dead name and ship just the palette.md documentation update.

See the inline comment for the exact suggestion.

@jules please address the issues noted above

Thank you for the guidance. I've updated both the ComboBox and the TextBox to use AutomationProperties.FullDescription="{Binding ElementName=HotkeyHeader, Path=Text}" instead. This will provide the section context as a supplementary description without overriding their specific names from the .resw.

@mikekthx

Copy link
Copy Markdown
Owner

Code Review — Synchronize (2026-06-24)

What the PR does

Adds x:Name="HotkeyHeader" to the Global Hotkey section heading in SettingsWindow.xaml and wires both the modifier ComboBox and key TextBox to it via AutomationProperties.FullDescription. Updates .jules/palette.md with the lesson learned about LabeledBy vs FullDescription.

Thread resolution

All three previously open threads are now resolved:

  • Date bug (palette.md) — fixed to 2026-06-24
  • LabeledBy overriding existing AutomationProperties.Name — switched to FullDescription
  • Dead x:Name="HotkeyHeader" — now referenced by FullDescription on both controls; resolved above ✓

Assessment

The implementation is correct and follows the patterns already established in the file:

  • {Binding ElementName=HotkeyHeader, Path=Text} binds a string (Text) to the FullDescription string property — the right binding type for each property.
  • Using {Binding ElementName=...} (rather than {x:Bind}) for the attached-property binding is consistent with how AutomationProperties.LabeledBy is used on FolderList and the View Mode ComboBox in the same file, and avoids any compiler edge-cases.
  • Mixing {Binding} and {x:Bind} on the same element is valid in WinUI 3.
  • The .jules/palette.md date is correct; the learning note accurately describes when to prefer FullDescription over LabeledBy.
  • No localization strings were added (no new visible text was introduced), so no .resw changes are needed.
  • No new testable logic was introduced, so no test changes are needed.

No blocking issues. This PR is ready to merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to READMEs, wikis, or inline code comments. ui Front-end changes, WinUI layouts, styling (e.g., Acrylic), or system tray updates.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant