Skip to content

Conversation

@AlexanderBlackman
Copy link
Contributor

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, and GetRowFromElement to manage row selection and focus. Updated using directives to include Microsoft.UI.Xaml.Media for 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.

hephaestusdailylog@gmail.com and others added 3 commits November 21, 2025 11:06
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.
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 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 UseListViewHotkeys property (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.

}



Copy link

Copilot AI Dec 18, 2025

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.

Suggested change
/// <summary>
/// Toggles the selection state of the currently focused or selected row when in Multiple selection mode.
/// </summary>

Copilot uses AI. Check for mistakes.

return new TableViewCellSlot(nextRow, nextColumn);
}

Copy link

Copilot AI Dec 18, 2025

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.

Suggested change
/// <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>

Copilot uses AI. Check for mistakes.
var row = GetRowFromElement(focused);
return row?.Index ?? -1;
}

Copy link

Copilot AI Dec 18, 2025

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.

Suggested change
/// <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>

Copilot uses AI. Check for mistakes.

private void ToggleCurrentRowSelection()
{

Copy link

Copilot AI Dec 18, 2025

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.

Suggested change

Copilot uses AI. Check for mistakes.

SelectionStartRowIndex = index;
CurrentRowIndex = index;

Copy link

Copilot AI Dec 18, 2025

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.

Suggested change

Copilot uses AI. Check for mistakes.
Comment on lines +201 to +202


Copy link

Copilot AI Dec 18, 2025

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.

Suggested change

Copilot uses AI. Check for mistakes.
index = rowIndex;
}


Copy link

Copilot AI Dec 18, 2025

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.

Suggested change

Copilot uses AI. Check for mistakes.
}

/// <summary>
/// Gets or sets whether the TableView should use ListView like hotkeys for navigation and selection.
Copy link

Copilot AI Dec 18, 2025

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.

Suggested change
/// 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.

Copilot uses AI. Check for mistakes.
(SelectionUnit == TableViewSelectionUnit.CellOrRow &&
LastSelectionUnit == TableViewSelectionUnit.Row));


Copy link

Copilot AI Dec 18, 2025

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.

Suggested change

Copilot uses AI. Check for mistakes.
/// <summary>
/// Identifies the UseListViewHotkeys dependency property.
/// </summary>
public static readonly DependencyProperty UseListViewHotkeysProperty = DependencyProperty.Register(nameof(UseListViewHotkeys), typeof(bool), typeof(TableView), new PropertyMetadata(false));
Copy link

Copilot AI Dec 18, 2025

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.

Suggested change
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>

Copilot uses AI. Check for mistakes.
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.

1 participant