From dbfa1c206ef33f7d8c05e65b122cae9489fc7ee8 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Mon, 1 Feb 2021 17:13:46 +0100 Subject: [PATCH 1/2] Rename and better document OnVerticalNavigation() helper function This is not an event handler itself, so it shouldn't use "On" prefix which is reserved, by convention, for the event handlers. It also doesn't need wxKeyEvent, but just wxKeyboardState, so change its signature accordingly. This will notably allow to use it from mouse event handlers too if necessary. No real changes. --- src/generic/datavgen.cpp | 34 +++++++++++++++++++++------------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/src/generic/datavgen.cpp b/src/generic/datavgen.cpp index 395403dd0f..1e415654cc 100644 --- a/src/generic/datavgen.cpp +++ b/src/generic/datavgen.cpp @@ -780,13 +780,21 @@ public: void OnPaint( wxPaintEvent &event ); void OnCharHook( wxKeyEvent &event ); void OnChar( wxKeyEvent &event ); - void OnVerticalNavigation(const wxKeyEvent& event, int delta); void OnLeftKey(wxKeyEvent& event); void OnRightKey(wxKeyEvent& event); void OnMouse( wxMouseEvent &event ); void OnSetFocus( wxFocusEvent &event ); void OnKillFocus( wxFocusEvent &event ); + // Go to the item at position delta rows away (delta may be positive or + // negative) from the current row. The new row is made current and the + // selection is changed or extended depending on the modifier keys flags in + // the keyboard state. + // + // If adding delta would result in an invalid item, it's clamped to the + // valid items range. + void GoToRelativeRow(const wxKeyboardState& kbdState, int delta); + void UpdateDisplay(); void RecalculateDisplay(); void OnInternalIdle() wxOVERRIDE; @@ -4609,11 +4617,11 @@ void wxDataViewMainWindow::OnChar( wxKeyEvent &event ) break; case WXK_UP: - OnVerticalNavigation(event, -1); + GoToRelativeRow(event, -1); break; case WXK_DOWN: - OnVerticalNavigation(event, +1); + GoToRelativeRow(event, +1); break; case '+': @@ -4645,19 +4653,19 @@ void wxDataViewMainWindow::OnChar( wxKeyEvent &event ) break; case WXK_END: - OnVerticalNavigation(event, +(int)GetRowCount()); + GoToRelativeRow(event, +(int)GetRowCount()); break; case WXK_HOME: - OnVerticalNavigation(event, -(int)GetRowCount()); + GoToRelativeRow(event, -(int)GetRowCount()); break; case WXK_PAGEUP: - OnVerticalNavigation(event, -(GetCountPerPage() - 1)); + GoToRelativeRow(event, -(GetCountPerPage() - 1)); break; case WXK_PAGEDOWN: - OnVerticalNavigation(event, +(GetCountPerPage() - 1)); + GoToRelativeRow(event, +(GetCountPerPage() - 1)); break; default: @@ -4665,7 +4673,7 @@ void wxDataViewMainWindow::OnChar( wxKeyEvent &event ) } } -void wxDataViewMainWindow::OnVerticalNavigation(const wxKeyEvent& event, int delta) +void wxDataViewMainWindow::GoToRelativeRow(const wxKeyboardState& kbdState, int delta) { // if there is no selection, we cannot move it anywhere if (!HasCurrentRow() || IsEmpty()) @@ -4689,7 +4697,7 @@ void wxDataViewMainWindow::OnVerticalNavigation(const wxKeyEvent& event, int del // in single selection we just ignore Shift as we can't select several // items anyhow - if ( event.ShiftDown() && !IsSingleSel() ) + if ( kbdState.ShiftDown() && !IsSingleSel() ) { RefreshRow( oldCurrent ); @@ -4714,12 +4722,12 @@ void wxDataViewMainWindow::OnVerticalNavigation(const wxKeyEvent& event, int del RefreshRow( oldCurrent ); // all previously selected items are unselected unless ctrl is held - if ( !event.ControlDown() ) + if ( !kbdState.ControlDown() ) UnselectAllRows(); ChangeCurrentRow( newCurrent ); - if ( !event.ControlDown() ) + if ( !kbdState.ControlDown() ) { SelectRow( m_currentRow, true ); SendSelectionChangedEvent(GetItemByRow(m_currentRow)); @@ -4875,7 +4883,7 @@ bool wxDataViewMainWindow::TryAdvanceCurrentColumn(wxDataViewTreeNode *node, wxK { // go to the first column of the next row: idx = 0; - OnVerticalNavigation(wxKeyEvent()/*dummy*/, +1); + GoToRelativeRow(wxKeyboardState()/*dummy*/, +1); } else { @@ -4893,7 +4901,7 @@ bool wxDataViewMainWindow::TryAdvanceCurrentColumn(wxDataViewTreeNode *node, wxK { // go to the last column of the previous row: idx = (int)GetOwner()->GetColumnCount() - 1; - OnVerticalNavigation(wxKeyEvent()/*dummy*/, -1); + GoToRelativeRow(wxKeyboardState()/*dummy*/, -1); } else { From bbd0c5244026b3d09797080857c7be26ebc0ed9b Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Mon, 1 Feb 2021 17:24:08 +0100 Subject: [PATCH 2/2] Fix Left/Right arrows in multi-selection generic wxDataViewCtrl They didn't reset the existing selection when used without Shift and didn't extend the selection when used with Shift. Fix both of these problems by reusing the existing logic from GoToRelativeRow() and, added in this commit, GoToRow() which was extracted from GoToRelativeRow(). Closes #19068. --- src/generic/datavgen.cpp | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/src/generic/datavgen.cpp b/src/generic/datavgen.cpp index 1e415654cc..164e2a6b0d 100644 --- a/src/generic/datavgen.cpp +++ b/src/generic/datavgen.cpp @@ -786,10 +786,15 @@ public: void OnSetFocus( wxFocusEvent &event ); void OnKillFocus( wxFocusEvent &event ); + // Go to the specified row, i.e. make it current and change or extend the + // selection extended depending on the modifier keys flags in the keyboard + // state. + // + // The row must be valid. + void GoToRow(const wxKeyboardState& state, unsigned int row); + // Go to the item at position delta rows away (delta may be positive or - // negative) from the current row. The new row is made current and the - // selection is changed or extended depending on the modifier keys flags in - // the keyboard state. + // negative) from the current row. // // If adding delta would result in an invalid item, it's clamped to the // valid items range. @@ -4689,8 +4694,14 @@ void wxDataViewMainWindow::GoToRelativeRow(const wxKeyboardState& kbdState, int if ( newRow >= rowCount ) newRow = rowCount - 1; + GoToRow(kbdState, newRow); +} + +void +wxDataViewMainWindow::GoToRow(const wxKeyboardState& kbdState, + unsigned int newCurrent) +{ unsigned int oldCurrent = m_currentRow; - unsigned int newCurrent = (unsigned int)newRow; if ( newCurrent == oldCurrent ) return; @@ -4780,12 +4791,7 @@ void wxDataViewMainWindow::OnLeftKey(wxKeyEvent& event) int parent = GetRowByItem( parent_node->GetItem() ); if ( parent >= 0 ) { - unsigned int row = m_currentRow; - SelectRow( row, false); - SelectRow( parent, true ); - ChangeCurrentRow( parent ); - GetOwner()->EnsureVisibleRowCol( parent, -1 ); - SendSelectionChangedEvent( parent_node->GetItem() ); + GoToRow(event, parent); } } } @@ -4813,12 +4819,7 @@ void wxDataViewMainWindow::OnRightKey(wxKeyEvent& event) else { // if the node is already open, we move the selection to the first child - unsigned int row = m_currentRow; - SelectRow( row, false ); - SelectRow( row + 1, true ); - ChangeCurrentRow( row + 1 ); - GetOwner()->EnsureVisibleRowCol( row + 1, -1 ); - SendSelectionChangedEvent( GetItemByRow(row+1) ); + GoToRelativeRow(event, +1); } } else