Add Row Editing Highlight and Tap-to-Edit support for Row selection Mode#342
Add Row Editing Highlight and Tap-to-Edit support for Row selection Mode#342rashmi-thakurr wants to merge 2 commits intow-ahmad:mainfrom
Conversation
cf3ad20 to
5dbf5b6
Compare
5dbf5b6 to
7264709
Compare
| 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)); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
| 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)); |
There was a problem hiding this comment.
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.
| { | ||
| SetIsEditing(false); | ||
| } | ||
| else if (SelectionUnit is TableViewSelectionUnit.Row or TableViewSelectionUnit.CellOrRow && newSlot.Row != currentCell.Slot.Row) |
There was a problem hiding this comment.
Use Cell.PrepareForEdit and Cell.EndEditing methods to control row highlighting.
There was a problem hiding this comment.
Done, moved the highlight into Cell.PrepareForEdit and Cell.EndEditing since they fire for every editing trigger (tap, double-tap, keyboard, F2).
|
|
||
| IsEditing = value; | ||
| UpdateCornerButtonState(); | ||
|
|
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
| TableView.CurrentRowIndex = Index; | ||
| TableView.LastSelectionUnit = TableViewSelectionUnit.Row; | ||
| } | ||
|
|
There was a problem hiding this comment.
we handle cell editing inside the cell, why do we need this code?
There was a problem hiding this comment.
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.
| /// <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(); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
|
|
||
| if (e.Handled) { base.OnDoubleTapped(e); return; } | ||
|
|
||
| if (TableView?.SelectionUnit is TableViewSelectionUnit.Row |
There was a problem hiding this comment.
we handle cell editing inside the cell, why do we need this code?
There was a problem hiding this comment.
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.
| MakeSelection(); | ||
| e.Handled = true; | ||
| } | ||
| else if (TableView?.SelectionUnit is TableViewSelectionUnit.CellOrRow |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Done, added a TapToEdit bool DP on TableView, defaults to false. When true, tapping an already-selected cell starts editing regardless of SelectionUnit.
|
Thank you for your contribution @rashmi-thakurr! I’ve left some feedback in the review to help improve things. Let me know if you’d like clarification on any of the points. |
…driven cell editing
|
Thanks for the thorough review, @w-ahmad Really appreciate the feedback - it pushed the implementation in a much cleaner direction. Here's what we've updated:
One thing we'd love your input on, we currently make Tab wrap to the next row in Row mode (like File Explorer). Would you prefer we make this configurable with something like a TabNavigationMode property, or keep Tab horizontal-only? Let us know! |
|
Thanks for the updates @rashmi-thakurr! I tested your new changes and noticed that when a cell exits edit mode, the row shifts slightly to the left. I’m not sure about the reason, but this newly added border needs to be rearranged, similar to how we handle the root panel and other elements of the row presenter. Screen.Recording.2026-04-16.200644.mp4I'm not great at UI design, but I suggest using a border with rounded corners, 1px border thickness and a slightly accent-tinted background to highlight the row. Here's a demo of what I mean... <!--TODO: put these property values to the theme resources-->
<Border BorderThickness="1"
CornerRadius="{ThemeResource ControlCornerRadius}"
BorderBrush="{ThemeResource AccentFillColorDefaultBrush}">
<Border.Background>
<SolidColorBrush Color="{StaticResource SystemAccentColor}"
Opacity="0.2" />
</Border.Background>
</Border>Screen.Recording.2026-04-16.201550.mp4 |
Summary
Adds visual row-editing highlight and tap-to-edit support for Row and CellOrRow selection modes, so the active row is clearly indicated while a cell is being edited, matching Windows File Explorer's behavior.
Closes #336
Key Implementation Details
and is completely independent of the row's background — no interaction with EnsureAlternateColors, no platform branching.
Double-tap editing is not guarded by this property.
SelectionPage.xamlto demonstrate the tap-to-edit feature.Screen.Recording.2026-04-14.112409.mp4
Test Coverage
8 new tests in TableViewRowEditingTests.cs covering: editing highlight in Row mode, editing highlight in CellOrRow mode, pointer hover prerequisites, tap-to-edit prerequisites, virtualization recycling,
current-cell state reset, and editing state transitions across rows.
All 174 tests pass (existing + new).
Files Changed
9 files changed, +404 / −3 lines
src/TableView.csPrepareContainerForItemOverride, Tab-wraps-row inGetNextSlotsrc/TableView.Properties.csTapToEditdependency property and CLR propertysrc/TableViewCell.csPrepareForEdit/EndEditing, TapToEdit guard inOnTapped, CellOrRow pointer hoversrc/TableViewRow.csApplyEditingHighlightdelegates to presentersrc/TableViewRowPresenter.csEditingHighlightOverlaytemplate child,ApplyEditingHighlightmethodsrc/Themes/TableViewRowPresenter.xamlEditingHighlightOverlayBorder afterRootPanelsrc/Themes/Resources.xamlTableViewRowEditingHighlightBackgroundfor Light, Dark, HighContrastsamples/.../SelectionPage.xamltests/TableViewRowEditingTests.cs