🎨 Palette: Link hotkey labels in Settings - #446
Conversation
|
👋 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. |
Review: PR #446 — Link hotkey labels in SettingsWhat this PR doesAdds Overall assessmentThe intent is good, but the implementation introduces an accessibility regression and should not be merged as-is. Blocking issue — both controls share the same UIA labelIn UI Automation, Before this change,
These are specific, distinct, and already sufficient. The new Compare this to the existing Recommended fix: Remove Secondary issuesDead Wrong year in Inaccurate What is correct
@jules please address the issues noted above |
Thank you for the detailed feedback. I have reverted the changes to |
Review — Round 2 (synchronize)Thread resolutionAll 3 previously open threads were outdated and their concerns are now addressed — resolved above:
What the PR now doesAfter the XAML
Blocking issue: localization incompleteOnly
Per CLAUDE.md, new UI strings must be added to all 13 Minor:
|
I have updated all remaining 12 locale |
Review SummaryWhat this PR does: Embeds group context ("Global Hotkey") directly into the Thread resolution: The previous open thread (only Overall approach is correct. Embedding group context in distinct Issues to addressRedundant "hotkey" word in 5 locales (inline comments posted) The fix prepends the locale's "Global Hotkey" phrase to strings that already contained the word for "hotkey", resulting in that word appearing twice in the screen reader announcement — the opposite of the accessibility improvement intended.
The other 8 locales ( Code suggestions with the deduplicated strings are attached as inline comments on the modifier line (101) of each affected file; the key line (110) in each file needs the same treatment. @jules please address the issues noted above |
I have removed the redundant "hotkey" wording from the translated |
Review — synchronize pass (2026-07-29)What this PR doesOriginally added Threads resolved in this passAll six previously open threads are now resolved:
Remaining issues — awkward prepend pattern in 7 localesFive locales formed natural compounds (de, ja, ko, zh-Hans, zh-Hant ✅). Seven locales (es, fr, it, pl, pt-BR, ru, tr) mechanically prepended the section header text with a space, producing two disconnected noun phrases that a screen reader announces as unrelated strings. Examples:
Inline code suggestions have been posted for all 14 affected lines — they can be applied directly from the Files Changed tab. @jules please address the issues noted above |
I have corrected the awkward grammar in the 7 specified locales ( |
Review — synchronize (round 2)What the PR doesThe previous iteration added Thread resolutionAll 14 previously-open threads were outdated; the underlying issues have been addressed in this push:
One blocking issue found
Inline suggestion posted on the relevant line. Everything else ✅
@jules please address the issues noted above |
I have updated the Russian locale's key translation to "Клавиша глобальной горячей клавиши" so that it mirrors the modifier and distinctly indicates it is the key field. |
Thread resolution summaryThe last unresolved thread (Russian All previous blocking concerns are addressed. See the breakdown below. Review summary — synchronizeWhat the PR does: Improves screen-reader accessibility for the hotkey controls in Settings. The Approach is correct. The earlier Overall assessment: ready to merge. No new issues found. What was addressed in this sync
Spot-check of translationsAll 13 locales produce distinct, unambiguous labels for the modifier and key controls:
|
💡 What:
Linked the interactive Hotkey ComboBox and TextBox to their descriptive visual header in SettingsWindow.xaml.
🎯 Why:
To ensure proper screen reader support. Previously, these controls were isolated, meaning a screen reader would announce them without the context of 'Global Hotkey'. By naming the header and using AutomationProperties.LabeledBy, the relationship is explicit.
♿ Accessibility Impacts:
Screen reader users will now correctly hear 'Global Hotkey' when navigating into the modifier or key inputs, making the settings form far more intuitive.
PR created automatically by Jules for task 13764616024473328879 started by @mikekthx