From ac00ce985514b167579a6fcffa5afa418bd14fe1 Mon Sep 17 00:00:00 2001 From: heffneil Date: Wed, 10 Jun 2026 15:32:09 -0400 Subject: [PATCH 1/4] Property grid: Tab/Shift+Tab traverses to next/prev visible property Fixes #5697. Stock wxPropertyGrid handles Tab in an open editor by calling Navigate(IsForward), which moves keyboard focus to the next sibling widget of the grid rather than to the next property within it - so the user had to click their way back into the grid to edit the next field. Override Tab in xlPropertyGrid via wxEVT_CHAR_HOOK: - Plain Tab advances to the next visible property and focuses its editor; Shift+Tab moves backward. - Ctrl+Tab / Alt+Tab and Tab from a non-editor context fall through to wx default traversal (so the user can still exit the grid). - Tab at the first/last visible property also falls through. Committing the current edit fires wxEVT_PG_CHANGED, which can queue a model reload that rebuilds the entire grid and invalidates the property pointers we got from the iterator. To survive that, the focus shift is deferred via CallAfter and the next property is looked up by name (not by stale pointer). Co-Authored-By: Claude Opus 4.7 (1M context) --- src-ui-wx/shared/utils/xlPropertyGrid.h | 53 +++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/src-ui-wx/shared/utils/xlPropertyGrid.h b/src-ui-wx/shared/utils/xlPropertyGrid.h index 581f888e2d..766b06ef93 100644 --- a/src-ui-wx/shared/utils/xlPropertyGrid.h +++ b/src-ui-wx/shared/utils/xlPropertyGrid.h @@ -11,6 +11,7 @@ #include #include +#include #include // This is to workaround a crash in wxPropertyGrid where doing @@ -29,6 +30,13 @@ class xlPropertyGrid : public wxPropertyGrid { const wxString& name = wxASCII_STR(wxPropertyGridNameStr)) : wxPropertyGrid(parent, id, pos, size, style, name) { Connect(wxEVT_IDLE,(wxObjectEventFunction)&xlPropertyGrid::OnIdle, 0, this); + // Tab traversal: stock wxPropertyGrid sends focus to the next sibling + // widget on Tab (via Navigate()), so users have to click their way + // back into the grid to edit the next property. Intercept Tab via + // CHAR_HOOK on the grid and walk to the next/previous visible + // property instead, matching the behavior of every other property + // editor (Excel, Visual Studio properties pane, etc.). + Bind(wxEVT_CHAR_HOOK, &xlPropertyGrid::OnCharHook, this); } virtual ~xlPropertyGrid() { } @@ -74,6 +82,51 @@ class xlPropertyGrid : public wxPropertyGrid { wxPropertyGrid::Clear(); } + void OnCharHook(wxKeyEvent& event) { + // Only interested in plain Tab / Shift+Tab; Ctrl/Alt+Tab passes through. + if (event.GetKeyCode() != WXK_TAB || event.ControlDown() || event.AltDown()) { + event.Skip(); + return; + } + // If no editor is open, let wx do its default traversal so Tab can + // exit the grid normally when the user is just navigating. + if (!IsEditorFocused()) { + event.Skip(); + return; + } + wxPGProperty* sel = GetSelection(); + if (sel == nullptr) { + event.Skip(); + return; + } + wxPropertyGridIterator it = GetIterator(wxPG_ITERATE_VISIBLE, sel); + if (event.ShiftDown()) { + --it; + } else { + ++it; + } + if (it.AtEnd()) { + // Boundary: at the first/last property — fall through to default + // traversal so the user can Tab out of the grid entirely. + event.Skip(); + return; + } + // Commit the current editor's value, then jump to the next property. + // The commit fires wxEVT_PG_CHANGED, which queues a model reload that + // rebuilds the whole grid and invalidates property pointers. Defer the + // focus shift via CallAfter so it runs once the rebuild has settled, + // and look up the target by name (pointers from before the rebuild + // are dead). + const wxString nextName = (*it)->GetName(); + CommitChangesFromEditor(); + CallAfter([this, nextName]() { + if (wxPGProperty* p = GetPropertyByName(nextName)) { + SelectProperty(p, true); + EnsureVisible(p); + } + }); + } + private: bool m_hidingComboPopups = false; }; From 18b3536353b5c0b11108e021e0e8e5991239a398 Mon Sep 17 00:00:00 2001 From: heffneil Date: Wed, 10 Jun 2026 20:32:21 -0400 Subject: [PATCH 2/4] Address Copilot review on PR #6529 - Bail out if CommitChangesFromEditor() returns false so a validation rejection keeps focus on the current editor instead of silently moving on (Copilot comment #2). - Move OnCharHook to private; it is only used as a Bind-target and was never meant to be part of the public API (Copilot comment #3). Copilot comment #1 (iterator underflow) was investigated and dismissed: wxPropertyGridIteratorBase::Prev() correctly sets m_property = nullptr at the boundary and AtEnd() catches it; the suggested GetNextVisible/GetPrevVisible helpers do not exist on wxPropertyGridInterface in this wx fork. Co-Authored-By: Claude Opus 4.7 (1M context) --- src-ui-wx/shared/utils/xlPropertyGrid.h | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src-ui-wx/shared/utils/xlPropertyGrid.h b/src-ui-wx/shared/utils/xlPropertyGrid.h index 766b06ef93..76144f7e2c 100644 --- a/src-ui-wx/shared/utils/xlPropertyGrid.h +++ b/src-ui-wx/shared/utils/xlPropertyGrid.h @@ -82,6 +82,7 @@ class xlPropertyGrid : public wxPropertyGrid { wxPropertyGrid::Clear(); } +private: void OnCharHook(wxKeyEvent& event) { // Only interested in plain Tab / Shift+Tab; Ctrl/Alt+Tab passes through. if (event.GetKeyCode() != WXK_TAB || event.ControlDown() || event.AltDown()) { @@ -118,7 +119,11 @@ class xlPropertyGrid : public wxPropertyGrid { // and look up the target by name (pointers from before the rebuild // are dead). const wxString nextName = (*it)->GetName(); - CommitChangesFromEditor(); + // If validation rejects the value, stay on the current editor so the + // user can fix it rather than silently losing focus to another row. + if (!CommitChangesFromEditor()) { + return; + } CallAfter([this, nextName]() { if (wxPGProperty* p = GetPropertyByName(nextName)) { SelectProperty(p, true); @@ -127,6 +132,5 @@ class xlPropertyGrid : public wxPropertyGrid { }); } -private: bool m_hidingComboPopups = false; }; From c87a21facc1ab38d9fab011702f7cecb377355f4 Mon Sep 17 00:00:00 2001 From: heffneil Date: Mon, 22 Jun 2026 00:06:42 -0400 Subject: [PATCH 3/4] Property grid Tab: skip categories, advance from dropdowns, wrap-around, wheel no longer edits - Skip category/section headers when tabbing so focus lands on the next editable property - Gate on GetSelection() instead of IsEditorFocused() so combo/choice editors advance instead of dropping focus out of the grid - Wrap around at the ends (last->first, first->last) so Tab cycles - Redirect the mouse wheel from the active editor to the grid so scrolling never changes a spin/combo value (which rebuilt the grid and lost the selection) Co-Authored-By: Claude Opus 4.8 --- src-ui-wx/shared/utils/xlPropertyGrid.h | 56 ++++++++++++++++++------- 1 file changed, 41 insertions(+), 15 deletions(-) diff --git a/src-ui-wx/shared/utils/xlPropertyGrid.h b/src-ui-wx/shared/utils/xlPropertyGrid.h index 76144f7e2c..0741c2def4 100644 --- a/src-ui-wx/shared/utils/xlPropertyGrid.h +++ b/src-ui-wx/shared/utils/xlPropertyGrid.h @@ -37,6 +37,10 @@ class xlPropertyGrid : public wxPropertyGrid { // property instead, matching the behavior of every other property // editor (Excel, Visual Studio properties pane, etc.). Bind(wxEVT_CHAR_HOOK, &xlPropertyGrid::OnCharHook, this); + // Spin/combo editors capture the mouse wheel and change their value, + // which fires a rebuild that loses the selection. Redirect the wheel + // from the active editor to the grid so scrolling only scrolls. + Bind(wxEVT_PG_SELECTED, &xlPropertyGrid::OnPropertySelected, this); } virtual ~xlPropertyGrid() { } @@ -83,34 +87,56 @@ class xlPropertyGrid : public wxPropertyGrid { } private: + void OnPropertySelected(wxPropertyGridEvent& event) { + event.Skip(); + if (wxWindow* ed = GetEditorControl()) { + ed->Bind(wxEVT_MOUSEWHEEL, &xlPropertyGrid::OnEditorMouseWheel, this); + } + } + void OnEditorMouseWheel(wxMouseEvent& event) { + // Forward to the grid so it scrolls; don't Skip, so the editor never + // consumes the wheel to change its value. + wxMouseEvent fwd(event); + fwd.SetEventObject(this); + ProcessWindowEvent(fwd); + } void OnCharHook(wxKeyEvent& event) { // Only interested in plain Tab / Shift+Tab; Ctrl/Alt+Tab passes through. if (event.GetKeyCode() != WXK_TAB || event.ControlDown() || event.AltDown()) { event.Skip(); return; } - // If no editor is open, let wx do its default traversal so Tab can - // exit the grid normally when the user is just navigating. - if (!IsEditorFocused()) { - event.Skip(); - return; - } + // Nothing selected -> no active editor; let Tab traverse out of the grid. wxPGProperty* sel = GetSelection(); if (sel == nullptr) { event.Skip(); return; } + // Gate on selection rather than editor focus so combo/choice editors + // advance too (IsEditorFocused() is false for them, which used to drop + // focus out of the grid). Skip category headers since they have no editor. + auto step = [&event](wxPropertyGridIterator& i) { + if (event.ShiftDown()) { + --i; + } else { + ++i; + } + }; wxPropertyGridIterator it = GetIterator(wxPG_ITERATE_VISIBLE, sel); - if (event.ShiftDown()) { - --it; - } else { - ++it; - } + do { + step(it); + } while (!it.AtEnd() && (*it)->IsCategory()); if (it.AtEnd()) { - // Boundary: at the first/last property — fall through to default - // traversal so the user can Tab out of the grid entirely. - event.Skip(); - return; + // Past the last/first editable property: wrap to the other end so + // Tab cycles through the grid instead of leaving it. + it = GetIterator(wxPG_ITERATE_VISIBLE, event.ShiftDown() ? wxBOTTOM : wxTOP); + while (!it.AtEnd() && (*it)->IsCategory()) { + step(it); + } + if (it.AtEnd()) { + event.Skip(); + return; + } } // Commit the current editor's value, then jump to the next property. // The commit fires wxEVT_PG_CHANGED, which queues a model reload that From 575eeb3fa81d21f908ea3a61bee7be3b7646847c Mon Sep 17 00:00:00 2001 From: heffneil Date: Tue, 23 Jun 2026 10:21:46 -0400 Subject: [PATCH 4/4] Property grid Tab: stop on collapsed category headers Skip only EXPANDED category headers (descend to their first child); stop on COLLAPSED ones so the user can expand them (Right arrow) and Tab again to enter the section. Otherwise wxPG_ITERATE_VISIBLE hides a collapsed category's children and Tab skips the whole section. Applied to both the forward step and the wrap-around via a shared skippable() lambda. Co-Authored-By: Claude Opus 4.8 --- src-ui-wx/shared/utils/xlPropertyGrid.h | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src-ui-wx/shared/utils/xlPropertyGrid.h b/src-ui-wx/shared/utils/xlPropertyGrid.h index 0741c2def4..4fcf926c47 100644 --- a/src-ui-wx/shared/utils/xlPropertyGrid.h +++ b/src-ui-wx/shared/utils/xlPropertyGrid.h @@ -114,7 +114,9 @@ class xlPropertyGrid : public wxPropertyGrid { } // Gate on selection rather than editor focus so combo/choice editors // advance too (IsEditorFocused() is false for them, which used to drop - // focus out of the grid). Skip category headers since they have no editor. + // focus out of the grid). Skip expanded category headers (descend to + // their first child) but stop on collapsed ones so the user can expand + // them (Right arrow) and Tab again to enter the section. auto step = [&event](wxPropertyGridIterator& i) { if (event.ShiftDown()) { --i; @@ -122,15 +124,18 @@ class xlPropertyGrid : public wxPropertyGrid { ++i; } }; + auto skippable = [](wxPropertyGridIterator& i) { + return (*i)->IsCategory() && (*i)->IsExpanded(); + }; wxPropertyGridIterator it = GetIterator(wxPG_ITERATE_VISIBLE, sel); do { step(it); - } while (!it.AtEnd() && (*it)->IsCategory()); + } while (!it.AtEnd() && skippable(it)); if (it.AtEnd()) { - // Past the last/first editable property: wrap to the other end so - // Tab cycles through the grid instead of leaving it. + // Past the last/first stop: wrap to the other end so Tab cycles + // through the grid instead of leaving it. it = GetIterator(wxPG_ITERATE_VISIBLE, event.ShiftDown() ? wxBOTTOM : wxTOP); - while (!it.AtEnd() && (*it)->IsCategory()) { + while (!it.AtEnd() && skippable(it)) { step(it); } if (it.AtEnd()) {