Skip to content

🎨 Palette: [UX improvement] Link Hotkey Inputs to Header#430

Open
google-labs-jules[bot] wants to merge 2 commits into
mainfrom
palette/settings-hotkey-a11y-11592666350873868872
Open

🎨 Palette: [UX improvement] Link Hotkey Inputs to Header#430
google-labs-jules[bot] wants to merge 2 commits into
mainfrom
palette/settings-hotkey-a11y-11592666350873868872

Conversation

@google-labs-jules

Copy link
Copy Markdown
Contributor

💡 What: Linked the global hotkey inputs to their visual header using AutomationProperties.LabeledBy.
🎯 Why: Provides clear context for screen reader users navigating directly to the hotkey fields.
♿ Accessibility impacts: Screen readers will now announce the 'Global Hotkey' context when focusing the combobox or textbox.


PR created automatically by Jules for task 11592666350873868872 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 July 8, 2026 11:45
@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 Jul 8, 2026
Comment thread SettingsWindow.xaml Outdated
Comment thread .jules/palette.md Outdated
@mikekthx

mikekthx commented Jul 8, 2026

Copy link
Copy Markdown
Owner

Review: Link Hotkey Inputs to Header

What this PR does: Adds x:Name="HotkeyHeader" to the Global Hotkey section TextBlock and sets AutomationProperties.LabeledBy="{Binding ElementName=HotkeyHeader}" on both the modifier ComboBox and the key TextBox, following the same pattern used by the View Mode and Folder List controls elsewhere in the file. Also records the lesson in .jules/palette.md.

Overall assessment: Needs changes — the intended improvement is a regression.


Blocking Issue

The hotkey controls already have per-field accessible names defined in all 13 .resw files:

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

AutomationProperties.LabeledBy takes precedence over AutomationProperties.Name in the UIA tree. After this change both the ComboBox and the TextBox will report the name "Global Hotkey" — the same string for both — instead of their distinct, specific names. A screen reader user navigating directly to either field can no longer tell them apart.

The existing .resw names already embed the "Hotkey" context word, so direct navigation is already accessible. The fix is to remove the LabeledBy attributes from both controls and leave the .resw values intact.


Secondary Issue

The new .jules/palette.md entry codifies the LabeledBy pattern without the critical caveat: only use it when no per-field AutomationProperties.Name exists. As written, the guidance will reproduce this same bug on any future control that already has a .resw name.


What's Fine

  • {Binding ElementName=HotkeyHeader} is the mandatory syntax for LabeledBy per CLAUDE.md (not {x:Bind}) — correctly applied.
  • XAML reformatting to multi-line attributes aligns with project conventions.
  • The AutomationProperties.AccessibilityView="Raw" on the + separator TextBlock is correct and untouched.

@jules please address the issues noted above

@google-labs-jules

Copy link
Copy Markdown
Contributor Author

Review: Link Hotkey Inputs to Header

What this PR does: Adds x:Name="HotkeyHeader" to the Global Hotkey section TextBlock and sets AutomationProperties.LabeledBy="{Binding ElementName=HotkeyHeader}" on both the modifier ComboBox and the key TextBox, following the same pattern used by the View Mode and Folder List controls elsewhere in the file. Also records the lesson in .jules/palette.md.

Overall assessment: Needs changes — the intended improvement is a regression.


Blocking Issue

The hotkey controls already have per-field accessible names defined in all 13 .resw files:

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

AutomationProperties.LabeledBy takes precedence over AutomationProperties.Name in the UIA tree. After this change both the ComboBox and the TextBox will report the name "Global Hotkey" — the same string for both — instead of their distinct, specific names. A screen reader user navigating directly to either field can no longer tell them apart.

The existing .resw names already embed the "Hotkey" context word, so direct navigation is already accessible. The fix is to remove the LabeledBy attributes from both controls and leave the .resw values intact.


Secondary Issue

The new .jules/palette.md entry codifies the LabeledBy pattern without the critical caveat: only use it when no per-field AutomationProperties.Name exists. As written, the guidance will reproduce this same bug on any future control that already has a .resw name.


What's Fine

  • {Binding ElementName=HotkeyHeader} is the mandatory syntax for LabeledBy per CLAUDE.md (not {x:Bind}) — correctly applied.
  • XAML reformatting to multi-line attributes aligns with project conventions.
  • The AutomationProperties.AccessibilityView="Raw" on the + separator TextBlock is correct and untouched.

@jules please address the issues noted above

I have reverted the SettingsWindow.xaml changes that added AutomationProperties.LabeledBy to the Hotkey controls since they already have specific accessible names defined in the .resw files. I have also corrected the entry in .jules/palette.md to specify that LabeledBy should only be used when no per-field AutomationProperties.Name exists. I am now pushing these corrections.

@github-actions github-actions Bot removed the ui Front-end changes, WinUI layouts, styling (e.g., Acrylic), or system tray updates. label Jul 8, 2026
@mikekthx

mikekthx commented Jul 8, 2026

Copy link
Copy Markdown
Owner

Code Review — Synchronize Event

Thread Resolution

Both prior review threads were outdated and their concerns are now addressed:

  • Thread 1 (Blocking — XAML regression): Resolved. The commit "Revert Hotkey LabeledBy changes and update memory instructions" removed the AutomationProperties.LabeledBy attributes from SettingsWindow.xaml. The per-field accessible names (AutomationProperties.Name via .resw) are now intact and neither ComboBox nor TextBox announce the same "Global Hotkey" header text.

  • Thread 2 (palette.md guidance incomplete): Resolved. The palette.md entry now includes the requested caveat: apply LabeledBy "ONLY IF no per-field AutomationProperties.Name already exists for those controls."


What this PR does (after sync)

The net change is a single addition to .jules/palette.md — a learning entry capturing when AutomationProperties.LabeledBy is and is not appropriate for hotkey-style grouped inputs. No production code, XAML, tests, or localization files are modified.

Assessment

No blocking issues. The change is safe to merge.

One minor observation: the Learning sentence in the new palette.md entry states that grouped inputs "lack context for screen reader users" as a blanket fact, but this is only true when no per-field AutomationProperties.Name is defined. The Action correctly restricts application with "ONLY IF no per-field name exists", so the guidance will not mislead in practice. Tightening the Learning statement to be conditional (e.g., "may lack context when no per-field name is defined") would make the entry more self-consistent, but this is a nit and not a blocker.

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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant