-
-
Notifications
You must be signed in to change notification settings - Fork 44
Add ListView-like hotkeys support to TableView #251
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Add ListView-like hotkeys support to TableView #251
Conversation
Introduced a new `UseListViewHotkeys` dependency property to the `TableView` class, allowing users to enable ListView-like hotkeys for navigation and selection. This feature is opt-in and defaults to `false` to ensure backward compatibility. Enhanced the `OnKeyDown` method to support additional keyboard navigation and selection scenarios when `UseListViewHotkeys` is enabled. Added support for toggling row selection with `Enter` and improved navigation using keys like `Up`, `Down`, `Home`, `End`, `PageUp`, and `PageDown`. Added helper methods `ToggleCurrentRowSelection`, `GetFocusedRowIndex`, and `GetRowFromElement` to manage row selection and focus. Updated `using` directives to include `Microsoft.UI.Xaml.Media` for visual tree traversal.
There was a problem hiding this 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 adds ListView-style keyboard navigation support to TableView through a new opt-in UseListViewHotkeys property. The feature enables standard ListView hotkey behavior in Multiple selection mode (Up/Down to move focus, Enter to toggle selection, Shift+Up/Down for extended selection) while maintaining TableView's existing cell editing shortcuts.
Key Changes:
- Introduced
UseListViewHotkeysproperty (defaults to false for backward compatibility) - Added keyboard navigation logic that conditionally delegates to ListView's base behavior when the property is enabled
- Implemented helper methods for row selection toggling and focus management
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 13 comments.
| File | Description |
|---|---|
| src/TableView.Properties.cs | Added UseListViewHotkeys dependency property declaration and accessor |
| src/TableView.cs | Added IsRowKeyboardContext property, modified OnKeyDown to handle ListView-style navigation, and added three helper methods (ToggleCurrentRowSelection, GetFocusedRowIndex, GetRowFromElement) with visual tree traversal support |
Comments suppressed due to low confidence (1)
src/TableView.cs:135
- The expression 'A == true' can be simplified to 'A'.
(UseListViewHotkeys == true) && //Not sure if I'm solving a bug or adding a feature, so I've made it apply only when the UseListViewHotkeys is set to true just in case.
| } | ||
|
|
||
|
|
||
|
|
Copilot
AI
Dec 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ToggleCurrentRowSelection method is missing XML documentation. Add a summary explaining that this method toggles the selection state of the currently focused or selected row when in Multiple selection mode.
| /// <summary> | |
| /// Toggles the selection state of the currently focused or selected row when in Multiple selection mode. | |
| /// </summary> |
|
|
||
| return new TableViewCellSlot(nextRow, nextColumn); | ||
| } | ||
|
|
Copilot
AI
Dec 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The GetFocusedRowIndex method is missing XML documentation. Add a summary explaining that this method retrieves the index of the currently focused row, or returns -1 if no row has focus or if XamlRoot is not available.
| /// <summary> | |
| /// Retrieves the index of the currently focused row, or returns -1 if no row has focus or if XamlRoot is not available. | |
| /// </summary> | |
| /// <returns> | |
| /// The index of the currently focused row, or -1 if no row has focus or if XamlRoot is not available. | |
| /// </returns> |
| var row = GetRowFromElement(focused); | ||
| return row?.Index ?? -1; | ||
| } | ||
|
|
Copilot
AI
Dec 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The GetRowFromElement method is missing XML documentation. Add a summary and param documentation explaining that this method traverses the visual tree upward from the given element to find the containing TableViewRow, returning null if no row is found.
| /// <summary> | |
| /// Traverses the visual tree upward from the specified element to find the containing <see cref="TableViewRow"/>. | |
| /// </summary> | |
| /// <param name="element">The starting element from which to search up the visual tree for a containing <see cref="TableViewRow"/>.</param> | |
| /// <returns> | |
| /// The <see cref="TableViewRow"/> that contains the provided element, or <c>null</c> if no containing row is found. | |
| /// </returns> |
|
|
||
| private void ToggleCurrentRowSelection() | ||
| { | ||
|
|
Copilot
AI
Dec 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is an unnecessary blank line at the start of this method. Remove it to maintain consistency with the coding style used elsewhere in the codebase.
|
|
||
| SelectionStartRowIndex = index; | ||
| CurrentRowIndex = index; | ||
|
|
Copilot
AI
Dec 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is an unnecessary blank line at the end of this method. Remove it to maintain consistency with the coding style used elsewhere in the codebase.
|
|
||
|
|
Copilot
AI
Dec 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are excessive blank lines here (two consecutive empty lines). According to typical code style practices and for consistency with the rest of the codebase, there should be only one blank line between method declarations.
| index = rowIndex; | ||
| } | ||
|
|
||
|
|
Copilot
AI
Dec 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is an unnecessary blank line here. Remove it to maintain consistent spacing within the method.
| } | ||
|
|
||
| /// <summary> | ||
| /// Gets or sets whether the TableView should use ListView like hotkeys for navigation and selection. |
Copilot
AI
Dec 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The documentation for this property should be more specific and descriptive. It should explain what "ListView like hotkeys" means in practical terms, such as: "Gets or sets a value indicating whether the TableView uses ListView-style keyboard navigation in Multiple selection mode, where Up/Down move focus, Enter toggles selection, and Shift+Up/Down extends selection without deselecting other items." This helps developers understand when and why they would use this property.
| /// Gets or sets whether the TableView should use ListView like hotkeys for navigation and selection. | |
| /// Gets or sets a value indicating whether the <see cref="TableView"/> uses ListView-style keyboard | |
| /// navigation in multiple selection mode, where Up/Down move the focused row, Enter toggles selection | |
| /// of the focused row, and Shift+Up/Down extends the selection without deselecting other items. |
| (SelectionUnit == TableViewSelectionUnit.CellOrRow && | ||
| LastSelectionUnit == TableViewSelectionUnit.Row)); | ||
|
|
||
|
|
Copilot
AI
Dec 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are excessive trailing spaces after the closing brace. Remove the unnecessary whitespace to maintain clean code formatting.
| /// <summary> | ||
| /// Identifies the UseListViewHotkeys dependency property. | ||
| /// </summary> | ||
| public static readonly DependencyProperty UseListViewHotkeysProperty = DependencyProperty.Register(nameof(UseListViewHotkeys), typeof(bool), typeof(TableView), new PropertyMetadata(false)); |
Copilot
AI
Dec 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The XML documentation comment for this dependency property declaration is incomplete. It's missing the closing summary tag on line 262, which causes the next documentation comment to be malformed. The summary should be properly closed before the next field declaration.
| public static readonly DependencyProperty UseListViewHotkeysProperty = DependencyProperty.Register(nameof(UseListViewHotkeys), typeof(bool), typeof(TableView), new PropertyMetadata(false)); | |
| public static readonly DependencyProperty UseListViewHotkeysProperty = DependencyProperty.Register(nameof(UseListViewHotkeys), typeof(bool), typeof(TableView), new PropertyMetadata(false)); | |
| /// <summary> |
Not sure if this is fixing a bug or adding feature so I've added a property, UseListViewHotkeys that makes TableView in SelectionMode="Multiple" use ListView hotkeys. It defaults to false, so there won't be any backwards compatibility issues.
In the default WinUI 3 ListView, when SelectionMode="Multiple,
The 'Up' and 'Down' keys move the selection border
'Enter' toggles selection.
'Shift'+'Up'/'Down' when moving from a SelectedItem selects the destination item without deselecting other items anywhere.
The new property causes TableView to be like this, whilst still maintaining "tab" to edit next cell, 'enter' to edit the below cell and all other hotkeys.
If set to false... it keeps the original behaviour.
In TableView, when SelectionMode="Multiple" and SelectionUnit is "Row".
(or the checkbox column if "RowOrColumn")
'Up' selects the destination item and keeps anything below it selected, deselecting all consequentive rows above it. (non-consequentive items stay selected
'Down' does the reverse.
'Left'/'Right' deselects all consequentive items but the current cell.
'Enter' behaves super weirdly, if the checkbox cell is highlighted, it selects the second row and moves the selection border to the second row whilst keeping the prior item selected. Otherwise it behaves like the 'Down' key.
'Shift' key has no effect. (I've checked TableView.SelectRows() so I'm surprised)
I also had a look at SelectionMode="Extended" but TableView/ListView behave the same.
Added helper methods
ToggleCurrentRowSelection,GetFocusedRowIndex, andGetRowFromElementto manage row selection and focus. Updatedusingdirectives to includeMicrosoft.UI.Xaml.Mediafor visual tree traversal.I did not change the VirtualKey.Left in the selection checkbox column, selecting the row. I'm assuming this is intended behaviour.