Skip to content

fix(paste): improve keyboard navigation in Advanced Paste window#45256

Open
yeelam-gordon wants to merge 2 commits intomainfrom
issue/33000
Open

fix(paste): improve keyboard navigation in Advanced Paste window#45256
yeelam-gordon wants to merge 2 commits intomainfrom
issue/33000

Conversation

@yeelam-gordon
Copy link
Contributor

Summary of the Pull Request

Improves keyboard navigation in the Advanced Paste window to provide a more natural and accessible user experience. Key improvements:

  • Smart initial focus: When AI is disabled, focus starts on "Paste as plain text" instead of the AI prompt textbox; when AI is enabled, focus remains on the prompt textbox
  • Arrow key navigation: Enables arrow key navigation through paste options using XYFocusKeyboardNavigation and SingleSelectionFollowsFocus
  • Clipboard history flyout: Right arrow key (Left in RTL) opens the clipboard history flyout; Escape or Left arrow (Right in RTL) closes it and returns focus
  • Enter key support: Pressing Enter on a selected paste format invokes it

PR Checklist

  • Closes: Keyboard navigation in Advanced Paste window isn't good #33000
  • Communication: I've discussed this with core contributors already. If the work hasn't been agreed, this work might be rejected
  • Tests: Added/updated and all pass
  • Localization: All end-user-facing strings can be localized
  • Dev docs: Added/updated

Detailed Description of the Pull Request / Additional comments

Files Changed

  • src/modules/AdvancedPaste/AdvancedPaste/AdvancedPasteXAML/MainWindow.xaml.cs - Modified SetFocus() to conditionally focus paste options list when AI is disabled
  • src/modules/AdvancedPaste/AdvancedPaste/AdvancedPasteXAML/Pages/MainPage.xaml - Added keyboard navigation attributes and named elements for flyout control
  • src/modules/AdvancedPaste/AdvancedPaste/AdvancedPasteXAML/Pages/MainPage.xaml.cs - Added keyboard event handlers for navigation:
    • SetInitialFocusToPasteOptions() - Sets focus to first paste option
    • PasteOptionsListView_KeyDown() - Handles Enter key to invoke selected format
    • ClipboardHistoryButton_KeyDown() - Opens flyout with Right arrow (RTL-aware)
    • ClipboardHistoryItemsView_KeyDown() - Closes flyout with Escape or Left arrow
    • ClipboardHistoryFlyout_Closed() - Returns focus to button when flyout closes

Behavioral Changes

Scenario Before After
Initial focus (AI off) AI prompt textbox "Paste as plain text" option
Arrow keys on paste list No navigation Navigate through options
Enter on selected option N/A Invokes paste action
Right arrow on Clipboard History Nothing Opens flyout
Left arrow/Escape in flyout Nothing Closes flyout, returns focus

Fixes #33000

Validation Steps Performed

  1. Manual testing with AI disabled:

    • Opened Advanced Paste window
    • Verified focus starts on "Paste as plain text"
    • Used arrow keys to navigate through paste options
    • Pressed Enter to invoke selected paste format
  2. Manual testing with AI enabled:

    • Opened Advanced Paste window
    • Verified focus starts on AI prompt textbox
  3. Clipboard history navigation:

    • Navigated to Clipboard history button
    • Pressed Right arrow to open flyout
    • Pressed Left arrow/Escape to close and return focus
  4. RTL support:

    • Verified Left/Right arrow behavior respects FlowDirection

@github-actions

This comment has been minimized.

@yeelam-gordon
Copy link
Contributor Author

Test comment

@yeelam-gordon
Copy link
Contributor Author

Code Review Summary - PR #45256

Review iteration: 1
Changed files: 3 | Medium severity issues: 2

This PR improves keyboard navigation in Advanced Paste. Overall, well-implemented with good RTL support. A few items need attention.


Medium Severity Findings

1. Telemetry Source Mismatch (MainPage.xaml.cs:262)

{"file":"src/modules/AdvancedPaste/AdvancedPaste/AdvancedPasteXAML/Pages/MainPage.xaml.cs","start_line":262,"end_line":262,"severity":"medium","tags":["functionality","pr-45256"],"body":"PasteOptionsListView_KeyDown uses PasteActionSource.ContextMenu for keyboard-invoked actions. This misreports telemetry since the action is triggered via Enter key, not context menu click. Fix: Use PasteActionSource.InAppKeyboardShortcut instead."}

Problem: The PasteOptionsListView_KeyDown method uses PasteActionSource.ContextMenu when invoked via keyboard Enter key.

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)

{"file":"src/modules/AdvancedPaste/AdvancedPaste/AdvancedPasteXAML/Pages/MainPage.xaml.cs","start_line":18,"end_line":18,"severity":"medium","tags":["code-quality","pr-45256"],"body":"The Microsoft.UI.Xaml.Controls.Primitives using directive appears unused. The Flyout class used in the code comes from Microsoft.UI.Xaml.Controls, not Primitives. Remove if build confirms it's unused."}

Problem: using Microsoft.UI.Xaml.Controls.Primitives; was added but the Flyout class used is from Microsoft.UI.Xaml.Controls.

Why it matters: Unused imports clutter code and may indicate incomplete changes.

Fix: Verify via build, then remove if unused.


Low Severity (Informational)

  • Cross-ListView Selection: The two ListViews share the same KeyDown handler but have independent selection states. Selection persists when navigating between lists. Consider if this is intended UX.

  • Accessibility consideration: Good RTL support via GetForwardKey()/GetBackwardKey() helpers. 👍

Copy link
Contributor Author

@yeelam-gordon yeelam-gordon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review with inline comments for medium+ severity findings.

- Changed PasteActionSource.ContextMenu to PasteActionSource.InAppKeyboardShortcut for
  Enter key invocation in PasteOptionsListView_KeyDown
- Removed unused Microsoft.UI.Xaml.Controls.Primitives import
@github-actions
Copy link

@check-spelling-bot Report

🔴 Please review

See the 📂 files view, the 📜action log, or 📝 job summary for details.

Unrecognized words (4)

Gotchas
Metacharacter
registryroot
regroot

These words are not needed and should be removed gotcha ROOTOWNER

Some files were automatically ignored 🙈

These sample patterns would exclude them:

^src/modules/powerrename/unittests/testdata/avif_test\.avif$
^src/modules/powerrename/unittests/testdata/heif_test\.heic$

You should consider adding them to:

.github/actions/spell-check/excludes.txt

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 patterns.txt to exclude portions, add items to the dictionary (e.g. by adding them to allow.txt), or fix typos.

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
on the issue/33000 branch (ℹ️ how do I use this?):

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 ⚠️ (2)

See the 📂 files view, the 📜action log, or 📝 job summary for details.

⚠️ Warnings Count
⚠️ ignored-expect-variant 2
⚠️ large-file 2

See ⚠️ Event descriptions for more information.

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.txt file 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 the patterns.txt file.

    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.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Keyboard navigation in Advanced Paste window isn't good

1 participant