Skip to content

Fix template column edit commit and context menu regression in DataGrid_LostFocus - #235

Merged
MrJul merged 3 commits into
AvaloniaUI:masterfrom
kdurane:master
May 28, 2026
Merged

Fix template column edit commit and context menu regression in DataGrid_LostFocus#235
MrJul merged 3 commits into
AvaloniaUI:masterfrom
kdurane:master

Conversation

@kdurane

@kdurane kdurane commented Apr 23, 2026

Copy link
Copy Markdown
Contributor

EditingColumnIndex property was disconnected from _editingColumnIndex, causing DataGrid_LostFocus and WaitForLostFocus to always see the wrong editing column

@kdurane

kdurane commented Apr 23, 2026

Copy link
Copy Markdown
Contributor Author

fixes issue #21210 (in Avalonia main)

@kdurane

kdurane commented Apr 23, 2026

Copy link
Copy Markdown
Contributor Author

addressess #209

@kdurane kdurane changed the title Fix EditingColumnIndex property not reading _editingColumnIndex backi… Fix template column edit commit and context menu regression in DataGrid_LostFocus Apr 23, 2026
@kdurane

kdurane commented Apr 23, 2026

Copy link
Copy Markdown
Contributor Author

Fix 1: EditingColumnIndex property not reading _editingColumnIndex backing field
EditingColumnIndex was declared as an auto-property, creating a compiler-generated backing field that nothing in the DataGrid was writing to. The internal field _editingColumnIndex was used throughout the rest of the DataGrid codebase, but DataGrid_LostFocus and WaitForLostFocus both reference the property rather than the field directly, meaning they always read the default value of 0 rather than the actual editing column index. This caused editingColumn to always resolve to column 0 regardless of which column was actually being edited, sending DataGrid_LostFocus down the wrong commit path for template columns.
Fix 2: Restore popup-aware focus tracking in DataGrid_LostFocus
I only discovered this second issue after noticing a reported regression (#234) pointing to commit 7fd1e70, which simplified the visual tree walk in DataGrid_LostFocus, replacing the popup-boundary-aware dataGridWillReceiveRoutedEvent logic with a simple IsVisualAncestorOf check. This removed the handling that correctly identified when focus had moved to a popup belonging to the editing element (such as a CalendarDatePicker dropdown or a TextBox context menu) rather than genuinely leaving the DataGrid. The result was that the DataGrid would incorrectly call CommitEdit and exit edit mode whenever a popup opened, dismissing context menus and preventing date selection from committing. The fix restores the popup detection using GetVisualRoot() to compare visual roots, which is the idiomatic v12 equivalent of the original approach.
Without fix 1, fix 2 has no effect — editingColumn is always null regardless of the focus logic, so the correct branch is never reached.

@MrJul MrJul left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

There is still EditingColumnIndex + _editingColumnIndex. The former is still used in several places, including in DataGridCell. I think we can just change the auto-property to be backed by this field instead.

@kdurane

kdurane commented May 28, 2026

Copy link
Copy Markdown
Contributor Author

Ah yes, of course, is that better

@kdurane
kdurane requested a review from MrJul May 28, 2026 14:41

@MrJul MrJul left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM!

@MrJul
MrJul merged commit 8b6ab3a into AvaloniaUI:master May 28, 2026
@kdurane

kdurane commented Jun 19, 2026

Copy link
Copy Markdown
Contributor Author

Hi @MrJul, when will an updated pack be released onto Nuget for datagrid? the last 2 fixes are whats holding me up from moving to version 12.

@MrJul

MrJul commented Jun 23, 2026

Copy link
Copy Markdown
Member

@kdurane The fixes are now released as part of Avalonia.Controls.DataGrid 12.0.1

This was referenced Aug 3, 2026
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.

2 participants