fix(paste): improve keyboard navigation in Advanced Paste window#45256
fix(paste): improve keyboard navigation in Advanced Paste window#45256yeelam-gordon wants to merge 2 commits intomainfrom
Conversation
This comment has been minimized.
This comment has been minimized.
|
Test comment |
Code Review Summary - PR #45256Review iteration: 1 This PR improves keyboard navigation in Advanced Paste. Overall, well-implemented with good RTL support. A few items need attention. Medium Severity Findings1. Telemetry Source Mismatch (MainPage.xaml.cs:262)Problem: The Why it matters: Telemetry analytics will incorrectly report keyboard navigation as context menu clicks, skewing usage data. Fix: await ViewModel.ExecutePasteFormatAsync(format, PasteActionSource.InAppKeyboardShortcut);2. Unused Import (MainPage.xaml.cs:18)Problem: Why it matters: Unused imports clutter code and may indicate incomplete changes. Fix: Verify via build, then remove if unused. Low Severity (Informational)
|
yeelam-gordon
left a comment
There was a problem hiding this comment.
Review with inline comments for medium+ severity findings.
src/modules/AdvancedPaste/AdvancedPaste/AdvancedPasteXAML/Pages/MainPage.xaml.cs
Show resolved
Hide resolved
src/modules/AdvancedPaste/AdvancedPaste/AdvancedPasteXAML/Pages/MainPage.xaml.cs
Outdated
Show resolved
Hide resolved
- Changed PasteActionSource.ContextMenu to PasteActionSource.InAppKeyboardShortcut for Enter key invocation in PasteOptionsListView_KeyDown - Removed unused Microsoft.UI.Xaml.Controls.Primitives import
@check-spelling-bot Report🔴 Please reviewSee the 📂 files view, the 📜action log, or 📝 job summary for details.Unrecognized words (4)Gotchas These words are not needed and should be removedgotcha ROOTOWNERSome files were automatically ignored 🙈These sample patterns would exclude them: You should consider adding them to: File matching is via Perl regular expressions. To check these files, more of their words need to be in the dictionary than not. You can use To accept these unrecognized words as correct, update file exclusions, and remove the previously acknowledged and now absent words, you could run the following commands... in a clone of the git@github.com:microsoft/PowerToys.git repository curl -s -S -L 'https://raw.githubusercontent.com/check-spelling/check-spelling/c635c2f3f714eec2fcf27b643a1919b9a811ef2e/apply.pl' |
perl - 'https://github.com/microsoft/PowerToys/actions/runs/21548295685/attempts/1' &&
git commit -m 'Update check-spelling metadata'Warnings
|
| Count | |
|---|---|
| 2 | |
| 2 |
See
If the flagged items are 🤯 false positives
If items relate to a ...
-
binary file (or some other file you wouldn't want to check at all).
Please add a file path to the
excludes.txtfile matching the containing file.File paths are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your files.
^refers to the file's path from the root of the repository, so^README\.md$would exclude README.md (on whichever branch you're using). -
well-formed pattern.
If you can write a pattern that would match it,
try adding it to thepatterns.txtfile.Patterns are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your lines.
Note that patterns can't match multiline strings.
There was a problem hiding this comment.
Pull request overview
This PR enhances keyboard navigation in the Advanced Paste window to provide a more accessible and intuitive user experience. The implementation addresses issue #33000 by adding smart focus management, arrow key navigation, and RTL-aware keyboard controls.
Changes:
- Conditional initial focus based on AI service state (prompt textbox when enabled, paste options list when disabled)
- Arrow key navigation through paste options with visual selection feedback
- Keyboard controls for clipboard history flyout (Right/Left arrow to open/close, with RTL support)
- Enter key support to invoke selected paste formats
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| MainWindow.xaml.cs | Modified SetFocus() to conditionally focus either the AI prompt textbox or paste options list based on IsCustomAIServiceEnabled state |
| MainPage.xaml | Changed ListView SelectionMode from "None" to "Single", enabled XYFocusKeyboardNavigation and SingleSelectionFollowsFocus for arrow key navigation, added KeyDown event handlers and named elements for flyout control |
| MainPage.xaml.cs | Added keyboard navigation methods including SetInitialFocusToPasteOptions(), RTL-aware key helpers, and event handlers for Enter key invocation and flyout keyboard controls |
Summary of the Pull Request
Improves keyboard navigation in the Advanced Paste window to provide a more natural and accessible user experience. Key improvements:
XYFocusKeyboardNavigationandSingleSelectionFollowsFocusPR Checklist
Detailed Description of the Pull Request / Additional comments
Files Changed
src/modules/AdvancedPaste/AdvancedPaste/AdvancedPasteXAML/MainWindow.xaml.cs- ModifiedSetFocus()to conditionally focus paste options list when AI is disabledsrc/modules/AdvancedPaste/AdvancedPaste/AdvancedPasteXAML/Pages/MainPage.xaml- Added keyboard navigation attributes and named elements for flyout controlsrc/modules/AdvancedPaste/AdvancedPaste/AdvancedPasteXAML/Pages/MainPage.xaml.cs- Added keyboard event handlers for navigation:SetInitialFocusToPasteOptions()- Sets focus to first paste optionPasteOptionsListView_KeyDown()- Handles Enter key to invoke selected formatClipboardHistoryButton_KeyDown()- Opens flyout with Right arrow (RTL-aware)ClipboardHistoryItemsView_KeyDown()- Closes flyout with Escape or Left arrowClipboardHistoryFlyout_Closed()- Returns focus to button when flyout closesBehavioral Changes
Fixes #33000
Validation Steps Performed
Manual testing with AI disabled:
Manual testing with AI enabled:
Clipboard history navigation:
RTL support:
FlowDirection