Skip to content

Add Row Editing Highlight and Tap-to-Edit support for Row selection Mode #342

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.

Sign up for GitHub

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

Open
rashmi-thakurr wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Add Row Editing Highlight and Tap-to-Edit support for Row selection Mode #342

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7264709
cell editing in row mode highlighting fixes
Apr 9, 2026
b8001a1
Add row editing highlight overlay, TapToEdit property, and lifecycle-…
Apr 15, 2026
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
64 changes: 63 additions & 1 deletion src/TableView.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@
private RowDefinition? _headerRowDefinition;
private bool _shouldThrowSelectionModeChangedException;
private bool _ensureColumns = true;
private TableViewRow? _editingHighlightRow;
private int _editingHighlightRowIndex = -1;
private readonly List<TableViewRow> _rows = [];
private readonly CollectionView _collectionView = [];

Expand Down Expand Up @@ -103,6 +105,17 @@
{
base.PrepareContainerForItemOverride(element, item);

// Reset editing highlight state on recycled containers to prevent
// stale _hasEditingHighlight from blocking EnsureAlternateColors.
if (element is TableViewRow { } recycledRow)
{
recycledRow.ApplyEditingHighlight(false);
if (_editingHighlightRow == recycledRow)
{
_editingHighlightRow = null;
}
}

DispatcherQueue.TryEnqueue(() =>
{
if (element is TableViewRow row)
Expand All @@ -111,10 +124,25 @@
row.ApplyCellsSelectionState();
row.RowPresenter?.ApplyDetailsPaneState(item);

// Reset current cell border on all cells in recycled containers
// to clear stale "Current" visual state from previous use.
foreach (var cell in row.Cells)
{
cell.ApplyCurrentCellState();
}

if (CurrentCellSlot.HasValue)
{
row.ApplyCurrentCellState(CurrentCellSlot.Value);
}

// Apply editing highlight when the editing row scrolls into view
var rowIndex = Items.IndexOf(item);
if (_editingHighlightRowIndex >= 0 && rowIndex == _editingHighlightRowIndex)
{
_editingHighlightRow = row;
row.ApplyEditingHighlight(true);
}
}
});
}
Expand Down Expand Up @@ -184,7 +212,7 @@

do
{
newSlot = GetNextSlot(newSlot, shiftKey, e.Key is VirtualKey.Enter);
newSlot = GetNextSlot(newSlot, shiftKey, e.Key is VirtualKey.Enter || (e.Key is VirtualKey.Tab && SelectionUnit is TableViewSelectionUnit.Row));

@w-ahmad w-ahmad Apr 13, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I guess this isn't right because the tab key should always put the next editable cell in the same row. If there is no editable cell in the same row, it will automatically jump to the next row or previous row with shift + tab.

@rashmi-thakurr rashmi-thakurr Apr 15, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

You're right, GetNextSlot already cycles through editable cells and wraps to the next row on its own.

What we were going for is the File Explorer–style behavior where Tab during rename jumps to the next row and starts editing it. Tab feels natural for quick back-to-back editing since it's the standard "next field" key. We tied it to SelectionUnit.Row because in Row mode the user is already working row by row, so it felt like a natural fit.

If we want to keep this separate from selection mode, we could add a TabNavigationMode property (Horizontal/Vertical) on TableView , similar to how TapToEdit works. It would just be one extra check in HandleNavigations, no changes to GetNextSlot itself. Should we go with that, or do you think it should be handled differently?

@w-ahmad w-ahmad Apr 16, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I get your point that in File Explorer, pressing the Tab key while editing a file name jumps to the next row. The key thing to note is that File Explorer only has one editable column and it's the Name column. So, TableView would behave exactly the same when there’s only one editable column in a row, jumping across rows with the Tab key.

@w-ahmad w-ahmad Apr 13, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I don’t think this change is necessary because the Tab key should always move to the next editable cell and put it in editing mode. If there’s no editable cell in the current row, it will automatically move to the next row.


} while (isEditing && Columns[newSlot.Column].IsReadOnly);

Expand All @@ -196,6 +224,21 @@
{
SetIsEditing(false);
}
else if (SelectionUnit is TableViewSelectionUnit.Row or TableViewSelectionUnit.CellOrRow && newSlot.Row != currentCell.Slot.Row)

@w-ahmad w-ahmad Apr 13, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Use Cell.PrepareForEdit and Cell.EndEditing methods to control row highlighting.

@rashmi-thakurr rashmi-thakurr Apr 15, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done, moved the highlight into Cell.PrepareForEdit and Cell.EndEditing since they fire for every editing trigger (tap, double-tap, keyboard, F2).

{
// Editing moved to a different row — move the highlight
_editingHighlightRow?.ApplyEditingHighlight(false);
_editingHighlightRowIndex = newSlot.Row;
if (ContainerFromIndex(newSlot.Row) is TableViewRow newRow)
{
_editingHighlightRow = newRow;
newRow.ApplyEditingHighlight(true);
}
else
{
_editingHighlightRow = null;
}
}
}

MakeSelection(newSlot, false);
Expand Down Expand Up @@ -597,7 +640,7 @@
}
else
{
foreach (var propertyInfo in dataType.GetProperties())

Check warning on line 643 in src/TableView.cs

View workflow job for this annotation

GitHub Actions / build 

'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicProperties' in call to 'System.Type.GetProperties()'. The return value of method 'WinUI.TableView.Extensions.ObjectExtensions.GetItemType(IEnumerable)' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to.
{
var displayAttribute = propertyInfo.GetCustomAttributes().OfType<DisplayAttribute>().FirstOrDefault();
var autoGenerateField = displayAttribute?.GetAutoGenerateField();
Expand Down Expand Up @@ -1502,6 +1545,25 @@

IsEditing = value;
UpdateCornerButtonState();

@w-ahmad w-ahmad Apr 13, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

From my testing, TableView doesn’t recycle the row when a cell is in edit mode. It could be a ListView behavior, but let’s keep it as is for now, and we don’t need to unhighlight the row once it’s highlighted.
On the other hand, Uno does recycle the row, but let’s not worry about that for now.

@rashmi-thakurr rashmi-thakurr Apr 15, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Good to know that ListView doesn't recycle the row while a cell is in edit mode, that simplifies things. We've removed the highlight management from SetIsEditing completely. The highlight is now purely driven by PrepareForEdit/EndEditing, so once it's applied, it stays until the cell's EndEditing clears it. No intermediate toggle.

We've kept a defensive ApplyEditingHighlight(false) in PrepareContainerForItemOverride for recycled containers as-is for now. Since you mentioned recycling doesn't happen during edit, want us to remove it?

if (value && SelectionUnit is TableViewSelectionUnit.Row or TableViewSelectionUnit.CellOrRow)
{
if (CurrentCellSlot.HasValue)
{
_editingHighlightRowIndex = CurrentCellSlot.Value.Row;
if (ContainerFromIndex(CurrentCellSlot.Value.Row) is TableViewRow row)
{
_editingHighlightRow = row;
row.ApplyEditingHighlight(true);
}
}
}
else if (!value)
{
_editingHighlightRow?.ApplyEditingHighlight(false);
_editingHighlightRow = null;
_editingHighlightRowIndex = -1;
}
}

/// <summary>
Expand Down
16 changes: 14 additions & 2 deletions src/TableViewCell.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,8 @@ protected override void OnPointerEntered(PointerRoutedEventArgs e)

if ((TableView?.SelectionMode is not ListViewSelectionMode.None
&& TableView?.SelectionUnit is not TableViewSelectionUnit.Row)
|| !TableView.IsReadOnly)
|| !TableView.IsReadOnly
|| (TableView?.SelectionUnit is TableViewSelectionUnit.Row or TableViewSelectionUnit.CellOrRow && !IsReadOnly))
{
VisualStates.GoToState(this, false, VisualStates.StatePointerOver);
}
Expand All @@ -203,7 +204,8 @@ protected override void OnPointerExited(PointerRoutedEventArgs e)

if ((TableView?.SelectionMode is not ListViewSelectionMode.None
&& TableView?.SelectionUnit is not TableViewSelectionUnit.Row)
|| !TableView.IsReadOnly)
|| !TableView.IsReadOnly
|| (TableView?.SelectionUnit is TableViewSelectionUnit.Row or TableViewSelectionUnit.CellOrRow && !IsReadOnly))
{
VisualStates.GoToState(this, false, VisualStates.StateNormal);
}
Expand All @@ -229,6 +231,16 @@ protected override async void OnTapped(TappedRoutedEventArgs e)
MakeSelection();
e.Handled = true;
}
else if (TableView?.SelectionUnit is TableViewSelectionUnit.CellOrRow

@w-ahmad w-ahmad Apr 13, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Let's add a bool TapToEdit property to TableView, and only enable cell edit mode on tap when this property is true, regardless of the SelectionUnit setting.

@rashmi-thakurr rashmi-thakurr Apr 15, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done, added a TapToEdit bool DP on TableView, defaults to false. When true, tapping an already-selected cell starts editing regardless of SelectionUnit.

&& !IsReadOnly
&& TableView is not null
&& !TableView.IsEditing
&& Column?.UseSingleElement is not true)
{
// Second tap on an already-selected cell in CellOrRow mode — start editing
// (like File Explorer's tap-pause-tap to rename).
e.Handled = await BeginCellEditing(e);
}
}

/// <inheritdoc/>
Expand Down
78 changes: 75 additions & 3 deletions src/TableViewRow.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ public partial class TableViewRow : ListViewItem
private ListViewItemPresenter? _itemPresenter;
private Border? _selectionBackground;
private bool _ensureCells = true;
private bool _hasEditingHighlight;
private bool _isBeginningEdit;
private Brush? _cellPresenterBackground;
private Brush? _cellPresenterForeground;

Expand Down Expand Up @@ -177,7 +179,7 @@ protected override void OnPointerReleased(PointerRoutedEventArgs e)
}

/// <inheritdoc/>
protected override void OnTapped(TappedRoutedEventArgs e)
protected override async void OnTapped(TappedRoutedEventArgs e)
{
base.OnTapped(e);

Expand All @@ -186,15 +188,48 @@ protected override void OnTapped(TappedRoutedEventArgs e)
TableView.CurrentRowIndex = Index;
TableView.LastSelectionUnit = TableViewSelectionUnit.Row;
}

@w-ahmad w-ahmad Apr 13, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

we handle cell editing inside the cell, why do we need this code?

@rashmi-thakurr rashmi-thakurr Apr 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

We added this because we thought in Row mode, the row needed to catch the second tap (the tap-pause-tap like File Explorer rename) and forward it to the cell to start editing. But the cell already handles its own editing and now with the TapToEdit property guarding it in the cell's own OnTapped, the row doesn't need to be involved at all. Removed.

// When SelectionUnit is Row and the row is already selected, forward the
// tap to the target cell so editing can be initiated with a second tap
// (like File Explorer's tap-pause-tap to rename).
if (TableView?.SelectionUnit is TableViewSelectionUnit.Row
&& IsSelected
&& e.OriginalSource is DependencyObject source
&& source.FindAscendant<TableViewCell>() is { IsReadOnly: false } cell
&& !TableView.IsEditing
&& !_isBeginningEdit
&& cell.Column?.UseSingleElement is not true)
{
_isBeginningEdit = true;
TableView.MakeSelection(cell.Slot, false);
e.Handled = await cell.BeginCellEditing(e);
_isBeginningEdit = false;
}
}

/// <inheritdoc/>
protected override void OnDoubleTapped(DoubleTappedRoutedEventArgs e)
protected override async void OnDoubleTapped(DoubleTappedRoutedEventArgs e)
{
var eventArgs = new TableViewRowDoubleTappedEventArgs(Index, this, Content);
TableView?.OnRowDoubleTapped(eventArgs);
e.Handled = eventArgs.Handled;

if (e.Handled) { base.OnDoubleTapped(e); return; }

if (TableView?.SelectionUnit is TableViewSelectionUnit.Row

@w-ahmad w-ahmad Apr 13, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

we handle cell editing inside the cell, why do we need this code?

@rashmi-thakurr rashmi-thakurr Apr 15, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Yeah, the cell already handles double-tap editing on its own. We added this thinking the row needed to be in charge of the whole flow, find the tapped cell, select it, start editing, and apply the highlight, all in one place. But the cell already knows how to edit itself, so the row was just doing the cell's job for it. Removed.

&& e.OriginalSource is DependencyObject source
&& source.FindAscendant<TableViewCell>() is { IsReadOnly: false } cell
&& !TableView.IsEditing
&& !_isBeginningEdit
&& cell.Column?.UseSingleElement is not true)
{
_isBeginningEdit = true;
TableView.MakeSelection(cell.Slot, false);
e.Handled = await cell.BeginCellEditing(e);
_isBeginningEdit = false;
return;
}

base.OnDoubleTapped(e);
}

Expand Down Expand Up @@ -584,7 +619,7 @@ private async void EnsureSelectionIndicatorPosition(double detailsHeight, Border
/// </summary>
internal void EnsureAlternateColors()
{
if (TableView is null || RowPresenter is null) return;
if (TableView is null || RowPresenter is null || _hasEditingHighlight) return;

RowPresenter.Background =
Index % 2 == 1 && TableView.AlternateRowBackground is not null ? TableView.AlternateRowBackground : _cellPresenterBackground;
Expand All @@ -603,6 +638,43 @@ internal void UpdateSelectCheckMarkOpacity()
}
}

/// <summary>
/// Highlights or unhighlights the row to indicate that a cell is being edited.
/// </summary>
internal void ApplyEditingHighlight(bool isEditing)
{
_hasEditingHighlight = isEditing;
if (isEditing)
{
#if WINDOWS
if (RowPresenter is not null && _itemPresenter?.PointerOverBackground is { } pointerOverBrush)
{
RowPresenter.Background = pointerOverBrush;
}
#else
if (_selectionBackground is not null)
{
_selectionBackground.Opacity = 1;
}
#endif
}
else
{
#if WINDOWS
if (RowPresenter is not null)
{
RowPresenter.Background = _cellPresenterBackground;
}
#else
if (_selectionBackground is not null)
{
_selectionBackground.Opacity = IsSelected ? 1 : 0;
}
#endif
EnsureAlternateColors();
}
}

Comment on lines +606 to +613

@w-ahmad w-ahmad Apr 13, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This code block seems a bit off. I suggest adding an accent-tinted border or rectangle in the TableViewRowPresenter above the RootPanel, and controlling its visibility from the code-behind. For that, you can use the Cell.PrepareForEdit and Cell.EndEditing methods.

@rashmi-thakurr rashmi-thakurr Apr 15, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Agreed, that code was a bit off. What we were trying to do was mimic the pointer-over look by directly swapping the row's background when editing started and restoring it when done. The problem was it kept clashing with EnsureAlternateColors (we had to add a guard to stop it from overwriting the highlight), and we needed #if WINDOWS branching because Uno handles it differently.

So we went with your suggestion, added a Border overlay in TableViewRowPresenter.xaml sitting above the RootPanel, hit-test invisible, collapsed by default. The code-behind just toggles its visibility. Much cleaner since the highlight is now completely separate from the row's background, no guards, no platform branching. We added a TableViewRowEditingHighlightBackground theme resource in Resources.xaml for Light, Dark and HighContrast. And the visibility is driven from PrepareForEdit/EndEditing as you suggested.

/// <summary>
/// Gets the height of the horizontal gridlines.
/// </summary>
Expand Down
Loading
Loading