Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions src/TableView.Properties.cs
Original file line number Diff line number Diff line change
Expand Up @@ -256,8 +256,11 @@
public static readonly DependencyProperty CanReorderColumnsProperty = DependencyProperty.Register(nameof(CanReorderColumns), typeof(bool), typeof(TableView), new PropertyMetadata(true));

/// <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.
/// Identifies the <see cref="ConditionalCellStyles"/> dependency property.
/// </summary>

Check warning on line 263 in src/TableView.Properties.cs

View workflow job for this annotation

GitHub Actions / build

XML comment has badly formed XML -- 'End tag was not expected at this location.'

Check warning on line 263 in src/TableView.Properties.cs

View workflow job for this annotation

GitHub Actions / build

XML comment has badly formed XML -- 'End tag was not expected at this location.'

Check warning on line 263 in src/TableView.Properties.cs

View workflow job for this annotation

GitHub Actions / build

XML comment has badly formed XML -- 'End tag was not expected at this location.'

Check warning on line 263 in src/TableView.Properties.cs

View workflow job for this annotation

GitHub Actions / build

XML comment has badly formed XML -- 'End tag was not expected at this location.'
public static readonly DependencyProperty ConditionalCellStylesProperty = DependencyProperty.Register(nameof(ConditionalCellStyles), typeof(IList<TableViewConditionalCellStyle>), typeof(TableView), new PropertyMetadata(default));

/// <summary>
Expand Down Expand Up @@ -768,6 +771,15 @@
set => SetValue(CanReorderColumnsProperty, value);
}

/// <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.
/// </summary>
public bool UseListViewHotkeys
{
get => (bool)GetValue(UseListViewHotkeysProperty);
set => SetValue(UseListViewHotkeysProperty, value);
}

/// <summary>
/// Handles changes to the ItemsSource property.
/// </summary>
Expand Down
121 changes: 121 additions & 0 deletions src/TableView.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
using Microsoft.UI.Xaml.Controls.Primitives;
using Microsoft.UI.Xaml.Data;
using Microsoft.UI.Xaml.Input;
using Microsoft.UI.Xaml.Media;
using System;
using System.Collections;
using System.Collections.Generic;
Expand Down Expand Up @@ -129,6 +130,13 @@ protected override DependencyObject GetContainerForItemOverride()
_rows.Add(row);
return row;
}

private bool IsRowKeyboardContext =>
(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 comparison UseListViewHotkeys == true is redundant. Since UseListViewHotkeys is already a boolean, it can be used directly in the boolean expression. Change this to just UseListViewHotkeys for cleaner, more idiomatic code.

Suggested change
(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.
UseListViewHotkeys && //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 uses AI. Check for mistakes.
(SelectionUnit == TableViewSelectionUnit.Row ||
(SelectionUnit == TableViewSelectionUnit.CellOrRow &&
LastSelectionUnit == TableViewSelectionUnit.Row));
Comment on lines +134 to +138
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 IsRowKeyboardContext property is missing XML documentation. According to the project's coding guidelines (CS1591 enforced as error), all public types and members must have XML documentation comments. Private members should also be documented when their purpose is not immediately obvious. Add a summary explaining that this property determines when ListView-style keyboard navigation should be applied based on the UseListViewHotkeys setting and current selection context.

Copilot uses AI. Check for mistakes.


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.
/// <inheritdoc/>
protected override async void OnKeyDown(KeyRoutedEventArgs e)
Expand All @@ -142,9 +150,94 @@ protected override async void OnKeyDown(KeyRoutedEventArgs e)
return;
}

if (!IsEditing && IsRowKeyboardContext)
{
if (!shiftKey &&
!ctrlKey &&
SelectionMode == ListViewSelectionMode.Multiple &&
e.Key == VirtualKey.Enter)
{
ToggleCurrentRowSelection();
e.Handled = true;
return;
}

var rowSelectionOnly = SelectionUnit == TableViewSelectionUnit.Row;

if (e.Key is VirtualKey.Up or VirtualKey.Down or
VirtualKey.Home or VirtualKey.End or
VirtualKey.PageUp or VirtualKey.PageDown ||
(rowSelectionOnly && (e.Key is VirtualKey.Left or VirtualKey.Right)))
{
int? prevCellRow = null;
if (SelectionUnit == TableViewSelectionUnit.Row && CurrentCellSlot.HasValue)
{
prevCellRow = CurrentCellSlot.Value.Row;
}

base.OnKeyDown(e); // Let ListView move the row focus / selection

var focusedIndex = GetFocusedRowIndex();
if (focusedIndex >= 0)
{
CurrentRowIndex = focusedIndex;
SelectionStartRowIndex ??= focusedIndex;
}

if (SelectionUnit == TableViewSelectionUnit.Row &&
prevCellRow.HasValue &&
focusedIndex >= 0 &&
focusedIndex != prevCellRow.Value)
{
CurrentCellSlot = null; // will un-apply old cell's current-state border
}
return;
}
}
// Everything else (cell nav, F2 in cell mode, Space, etc.)
await HandleNavigations(e, shiftKey, ctrlKey);
}



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.
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.
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.
var index = GetFocusedRowIndex();

if (index < 0)
{
var rowIndex = CurrentRowIndex ?? SelectedIndex;

if (rowIndex is < 0 || rowIndex >= Items.Count)
{
return;
}

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.
if (index < 0 || index >= Items.Count) { return; }

Comment on lines +220 to +222
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.

This condition check is redundant. The index value is already validated in lines 208-218 where it's either set from GetFocusedRowIndex() (which returns -1 if invalid) or from CurrentRowIndex/SelectedIndex (which are validated against the same bounds). If index passes the earlier checks, it cannot be less than 0 or greater than or equal to Items.Count at this point.

Suggested change
if (index < 0 || index >= Items.Count) { return; }

Copilot uses AI. Check for mistakes.
var isSelected = SelectedRanges.Any(r => r.IsInRange(index));

var singleIndexRange = new ItemIndexRange(index, 1u);

if (isSelected)
{
DeselectRange(singleIndexRange);
}
else
{
SelectRange(singleIndexRange);
}

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.
}

/// <summary>
/// Handles navigation keys.
/// </summary>
Expand Down Expand Up @@ -385,6 +478,34 @@ private TableViewCellSlot GetNextSlot(TableViewCellSlot? currentSlot, bool isShi
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.
private int GetFocusedRowIndex()
{
if (XamlRoot is null)
return -1;

var focused = FocusManager.GetFocusedElement(XamlRoot) as DependencyObject;
if (focused is null)
return -1;

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 static TableViewRow? GetRowFromElement(DependencyObject element)
{
var current = element;

while (current is not null)
{
if (current is TableViewRow row)
return row;

current = VisualTreeHelper.GetParent(current);
}

return null;
}

/// <summary>
/// Copies the selected rows or cells content to the clipboard.
/// </summary>
Expand Down
Loading