Repository navigation
Add public TableView.BeginEditAsync(slot) to start an edit like F2 (#430) - #431
SolidRockProgrammer wants to merge 6 commits into
Conversation
The control begins editing on a double tap and on F2/Enter, but exposes no public way to ask for it. A column whose resting cell carries its own affordance - a drop-down arrow, a picker button - needs to open the editor on a single click of that affordance, which the control cannot infer from the gesture. BeginEdit() wraps the existing internal BeginCellEditing(RoutedEventArgs) and applies the same preconditions as the double-tap path, so it adds no behaviour beyond the verb the control already performs.
There was a problem hiding this comment.
🟡 Changes recommended
Editing a non-current cell can desynchronize the table’s edit lifecycle, and the new API lacks coverage.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds a public API for externally starting a TableViewCell edit session.
Changes:
- Adds
TableViewCell.BeginEdit()with editability guards. - Documents behavior and return semantics.
File summaries
| File | Description |
|---|---|
src/TableViewCell.cs |
Exposes and documents programmatic cell editing. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 3
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| public bool BeginEdit() | ||
| { | ||
| if (IsReadOnly || TableView is null || TableView.IsEditing || Column?.UseSingleElement is not false) | ||
| { | ||
| return false; | ||
| } | ||
|
|
||
| return BeginCellEditing(new RoutedEventArgs()); |
| /// The control starts an edit session from <see cref="OnDoubleTapped"/> and from the | ||
| /// <c>TableView</c> key handler, but offers no public way to ask for one. A column whose RESTING | ||
| /// cell carries an affordance of its own - a drop-down arrow, a picker button - needs to open the | ||
| /// editor on a single click of that affordance, and that is not a gesture the control can infer. |
| public bool BeginEdit() | ||
| { | ||
| if (IsReadOnly || TableView is null || TableView.IsEditing || Column?.UseSingleElement is not false) |
|
@SolidRockProgrammer, Copilot is right—calling Cell.BeginEdit() alone won’t notify TableView about the edit and even won't make the cell as current cell. I believe, we should create a method that works like pressing F2 and also sets the cell as CurrentCell. This will help prevent unwanted behaviors later on. |
Review feedback on w-ahmad#431: calling BeginEdit() on a cell entered edit mode without telling the TableView and without making that cell the current cell, so later keyboard navigation and commit behaviour would start from the wrong cell. BeginEditAsync now follows the same sequence as the existing gestures: commit an edit in progress on another cell (as tap/Tab do), make the target cell current and selected (as keyboard navigation does), then begin the edit as F2 does. The edit starts from OnCurrentCellChanged after the cell has been scrolled into view and focused, so that focus call cannot pull focus back out of the new editing element. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A negative control showed the focus assertion passes whether the edit starts before or after the current-cell change is processed, so it pinned nothing about the ordering; it also failed once on a cold first deploy, which would make it a flake in CI. The comment on the hand-off now states what the ordering does rather than a failure it prevents. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
SolidRockProgrammer
changed the title
|
Thanks, agreed — the cell-level method left the TableView with a different current cell from the one being edited. I've replaced it with |
ScrollRowIntoView always yields, so BeginEditAsync could never complete before its caller's event handler returned - not even for the current, visible cell, where F2 begins synchronously. A key or character handler needs to know whether the edit began before it returns (found by a consumer that starts an edit when a character is typed on a selected cell). A realized current cell is now edited directly; only a cell scrolled out of view waits for the scroll. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
One follow-up from integrating this in our app: |
…vigateFromCurrentCell (#5) * DH-1831 Add TableView.BeginEditAsync(slot) (w-ahmad#431) Makes a cell current and selected through MakeSelection and begins editing it as F2 does, committing any edit in progress on another cell first; a realized current cell is edited synchronously. Brought onto the 1.5.0 line Hub ships so it can be tested in Hub before w-ahmad#431 goes further upstream. TableViewCell.BeginEdit() is kept so current callers still compile. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * DH-1981 Add spreadsheet navigation options and commit/navigate APIs EnterKeyNavigation (Down by default, Right moves to the next cell and wraps, as gINT does); ContinueEditingOnNavigation (true by default; false makes Tab/Enter commit and only select the next cell, as Excel does); CommitEdit() and NavigateFromCurrentCell(key) so a host can implement entry mode (an arrow key that commits and moves). The arrow-slot computation is extracted so the key handler and the public method share it. Defaults keep the stock behaviour. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * DH-1981 Pin the navigation tests to their declared columns AutoGenerateColumns defaults true, so the test grid carried three generated columns beside the three declared ones and the wrap test's last column was not the last (CI run 36832285874). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * DH-1981 Test Tab and Enter while editing; fix a doc comment The commit-and-move path ran only from OnKeyDown, which needs a KeyRoutedEventArgs no test can construct, so ContinueEditingOnNavigation and EnterKeyNavigation=Right while editing had no test at all. - Factor the Tab/Enter branch into internal HandleTabOrEnter (same behaviour; a cancelled commit still leaves the key unhandled). - Four tests: default continues editing; Right + not continuing commits and only selects; Tab wraps and only selects; a cancelled commit moves nothing. - GetArrowSlot had been inserted under GetNextSlot's summary, giving it two summaries and GetNextSlot none. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Adds a public
TableView.BeginEditAsync(TableViewCellSlot)that makes a cell the current cell and begins editing it, as pressing F2 on that cell does.Closes #430.
Why
TableViewbegins editing on a double tap (TableViewCell.OnDoubleTapped) and on F2 / Tab / Enter (TableViewkey handling), but exposes no public way to ask for one:BeginCellEditing(RoutedEventArgs)isinternal, and settingCurrentCellSlotonly selects.A column whose resting cell carries an affordance of its own (a drop-down arrow, a picker button) needs to open the editor on a single click of that affordance, and the control cannot infer that from the gesture.
UseSingleElement = trueis the only public alternative, and it puts a live editor in every cell of the column: right for a check box, costly for a combo box, and a focusedTextBoxswallows Left/Right, which is the grid's own cell navigation.What it does
An earlier revision added
TableViewCell.BeginEdit(). As review pointed out, that entered edit mode without making the cell current, so theTableViewwas left with a different current cell from the one being edited. It is replaced by a method onTableViewthat follows the same sequence as the existing gestures:CellEditEndingcancels the commit, no edit begins.MakeSelection, the same call keyboard navigation uses, so it is scrolled into view and focused byOnCurrentCellChanged.BeginningEditis raised and a cancelling handler is honoured.The edit in step 3 starts from inside
OnCurrentCellChanged, after that handler has scrolled to and focused the cell, so the editing element is the last thing to take focus. If the cell is already current, it is scrolled into view and edited directly.It returns
falsewithout editing for a read-only table, column or cell, for a column that draws itself throughUseSingleElement(a double tap does not begin an edit there either), for a slot outside the table, and when that cell is already being edited. A second call made before a first one has started its edit supersedes it.It is
asyncbecause the target cell may need scrolling into view before it exists, whichScrollCellIntoViewalready models as aTask.Testing
New
tests/TableViewBeginEditTests.cs(8 tests): the cell becomes current and editing; calling it on the current cell; committing an edit in progress on another cell (oneCellEditEndedwithCommit); calling it on the cell already being edited; a cancelledBeginningEdit; a read-only column; a read-only table; a slot outside the table.Full suite run locally through
WinUI.TableView.Tests.build.appxrecipe(x64 Debug,net10.0-windows10.0.26100.0): 372 / 372 passed. The branch hasmainmerged in.I also tried a test asserting that focus ends up in the editing element. It passed, but it passed equally when the edit was started before the current-cell change (checked by temporarily reordering the code), so it did not pin anything about the ordering, and it failed once on a cold first deploy. I left it out rather than add a test that could flake in CI without protecting anything.
Context
We ship this control in a geotechnical data-entry application and currently reach the internal method by reflection, which we would rather not do. Same motivation as #428 / #429.