From dbe8e30b5680e6c5f991f6c92ab56adcac313d56 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Mon, 13 Dec 2021 13:51:38 +0000 Subject: [PATCH 1/7] Allow using column sort indicators in virtual wxListCtrl too There doesn't seem to be any reason to not show sort indicators in the columns of virtual list controls, so simply remove the test for IsVirtual() in DrawSortArrow(). This allows d8ec0aa001 (Support sort indicators in wxListCtrl header, 2021-11-28) to work for wxListCtrl with wxLC_VIRTUAL too. --- src/generic/listctrl.cpp | 2 +- src/msw/listctrl.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/generic/listctrl.cpp b/src/generic/listctrl.cpp index f62ad4408d..1e078b9f9a 100644 --- a/src/generic/listctrl.cpp +++ b/src/generic/listctrl.cpp @@ -1104,7 +1104,7 @@ void wxListHeaderWindow::OnPaint( wxPaintEvent &WXUNUSED(event) ) #endif wxHeaderSortIconType sortArrow = wxHDR_SORT_ICON_NONE; - if ( !m_owner->IsVirtual() && m_enableSortCol && i == m_sortCol ) + if ( m_enableSortCol && i == m_sortCol ) { if ( m_sortAsc ) sortArrow = wxHDR_SORT_ICON_UP; diff --git a/src/msw/listctrl.cpp b/src/msw/listctrl.cpp index 756c517d37..82900b7c96 100644 --- a/src/msw/listctrl.cpp +++ b/src/msw/listctrl.cpp @@ -3432,7 +3432,7 @@ void wxListCtrl::DrawSortArrow() { if ( ListView_GetColumn(GetHwnd(), col, &lvCol) ) { - if ( !IsVirtual() && m_enableSortCol && col == m_sortCol ) + if ( m_enableSortCol && col == m_sortCol ) { if ( m_sortAsc ) lvCol.fmt = (lvCol.fmt & ~HDF_SORTDOWN) | HDF_SORTUP; From 58290168e5d11b2654dfe16d5e21ebb491eb6034 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Mon, 13 Dec 2021 14:18:18 +0000 Subject: [PATCH 2/7] Remove top level "const" from {Enable,Show}SortIndicator() Don't use "const int" or "const bool" for parameter types, the "const" here is ignored and using it is inconsistent with all the rest of the library. No real changes. --- include/wx/generic/listctrl.h | 4 ++-- include/wx/listbase.h | 4 ++-- include/wx/msw/listctrl.h | 4 ++-- interface/wx/listctrl.h | 4 ++-- src/generic/listctrl.cpp | 4 ++-- src/msw/listctrl.cpp | 4 ++-- 6 files changed, 12 insertions(+), 12 deletions(-) diff --git a/include/wx/generic/listctrl.h b/include/wx/generic/listctrl.h index 774b20a7a3..8efc002910 100644 --- a/include/wx/generic/listctrl.h +++ b/include/wx/generic/listctrl.h @@ -115,9 +115,9 @@ public: bool IsItemChecked(long item) const wxOVERRIDE; void CheckItem(long item, bool check) wxOVERRIDE; - void EnableSortIndicator(const bool enable = true) wxOVERRIDE; + void EnableSortIndicator(bool enable = true) wxOVERRIDE; bool IsSortIndicatorEnabled() const wxOVERRIDE; - void ShowSortIndicator(const int idx, const bool ascending = true) wxOVERRIDE; + void ShowSortIndicator(int idx, bool ascending = true) wxOVERRIDE; void RemoveSortIndicator() wxOVERRIDE; int GetSortIndicator() const wxOVERRIDE; bool IsAscendingSortIndicator() const wxOVERRIDE; diff --git a/include/wx/listbase.h b/include/wx/listbase.h index ce4fc2aedb..32ab9aeb3a 100644 --- a/include/wx/listbase.h +++ b/include/wx/listbase.h @@ -447,9 +447,9 @@ public: virtual void CheckItem(long WXUNUSED(item), bool WXUNUSED(check)) { } // Sort indicator in header. - virtual void EnableSortIndicator(const bool WXUNUSED(enable) = true) { } + virtual void EnableSortIndicator(bool WXUNUSED(enable) = true) { } virtual bool IsSortIndicatorEnabled() const { return false; } - virtual void ShowSortIndicator(const int WXUNUSED(idx), const bool WXUNUSED(ascending) = true) { } + virtual void ShowSortIndicator(int WXUNUSED(idx), bool WXUNUSED(ascending) = true) { } virtual void RemoveSortIndicator() { } virtual int GetSortIndicator() const { return -1; } virtual bool IsAscendingSortIndicator() const { return true; } diff --git a/include/wx/msw/listctrl.h b/include/wx/msw/listctrl.h index 2314133ff4..02e891353e 100644 --- a/include/wx/msw/listctrl.h +++ b/include/wx/msw/listctrl.h @@ -226,9 +226,9 @@ public: void CheckItem(long item, bool check) wxOVERRIDE; // Sort indicator in header - void EnableSortIndicator(const bool enable = true) wxOVERRIDE; + void EnableSortIndicator(bool enable = true) wxOVERRIDE; bool IsSortIndicatorEnabled() const wxOVERRIDE; - void ShowSortIndicator(const int idx, const bool ascending = true) wxOVERRIDE; + void ShowSortIndicator(int idx, bool ascending = true) wxOVERRIDE; void RemoveSortIndicator() wxOVERRIDE; int GetSortIndicator() const wxOVERRIDE; bool IsAscendingSortIndicator() const wxOVERRIDE; diff --git a/interface/wx/listctrl.h b/interface/wx/listctrl.h index bc6c1ac9a4..1fb82c5f9d 100644 --- a/interface/wx/listctrl.h +++ b/interface/wx/listctrl.h @@ -1435,7 +1435,7 @@ public: @since 3.1.6 */ - void EnableSortIndicator(const bool enable); + void EnableSortIndicator(bool enable); /** Returns true if a sort indicator is enabled. @@ -1462,7 +1462,7 @@ public: @since 3.1.6 */ - void ShowSortIndicator(const int idx, const bool ascending = true); + void ShowSortIndicator(int idx, bool ascending = true); /** Remove the sort indicator from the column being used as sort key. diff --git a/src/generic/listctrl.cpp b/src/generic/listctrl.cpp index 1e078b9f9a..2e9d868d5a 100644 --- a/src/generic/listctrl.cpp +++ b/src/generic/listctrl.cpp @@ -5084,7 +5084,7 @@ bool wxGenericListCtrl::IsItemChecked(long item) const return m_mainWin->IsItemChecked(item); } -void wxGenericListCtrl::EnableSortIndicator(const bool enable) +void wxGenericListCtrl::EnableSortIndicator(bool enable) { if ( m_headerWin ) { @@ -5098,7 +5098,7 @@ bool wxGenericListCtrl::IsSortIndicatorEnabled() const return m_headerWin && m_headerWin->m_enableSortCol; } -void wxGenericListCtrl::ShowSortIndicator(const int idx, const bool ascending) +void wxGenericListCtrl::ShowSortIndicator(int idx, bool ascending) { if ( idx == -1 ) { diff --git a/src/msw/listctrl.cpp b/src/msw/listctrl.cpp index 82900b7c96..283b8b6b5b 100644 --- a/src/msw/listctrl.cpp +++ b/src/msw/listctrl.cpp @@ -1456,7 +1456,7 @@ bool wxListCtrl::IsItemChecked(long item) const return ListView_GetCheckState(GetHwnd(), (UINT)item) != 0; } -void wxListCtrl::EnableSortIndicator(const bool enable) +void wxListCtrl::EnableSortIndicator(bool enable) { m_enableSortCol = enable; DrawSortArrow(); @@ -1467,7 +1467,7 @@ bool wxListCtrl::IsSortIndicatorEnabled() const return m_enableSortCol; } -void wxListCtrl::ShowSortIndicator(const int idx, const bool ascending) +void wxListCtrl::ShowSortIndicator(int idx, bool ascending) { if ( idx == -1 ) { From c287840faa1ff7dcb8e5d052350b50a6b8c23ede Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Mon, 13 Dec 2021 14:29:59 +0000 Subject: [PATCH 3/7] Optimize changing sort indicator in wxGenericListCtrl Don't do anything at all if nothing changes and if the indicator does change, refresh only the header window and not the whole list control, which seems unnecessary. --- src/generic/listctrl.cpp | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/generic/listctrl.cpp b/src/generic/listctrl.cpp index 2e9d868d5a..a2366dce73 100644 --- a/src/generic/listctrl.cpp +++ b/src/generic/listctrl.cpp @@ -5086,10 +5086,10 @@ bool wxGenericListCtrl::IsItemChecked(long item) const void wxGenericListCtrl::EnableSortIndicator(bool enable) { - if ( m_headerWin ) + if ( m_headerWin && enable != m_headerWin->m_enableSortCol ) { m_headerWin->m_enableSortCol = enable; - Refresh(); + m_headerWin->Refresh(); } } @@ -5104,21 +5104,23 @@ void wxGenericListCtrl::ShowSortIndicator(int idx, bool ascending) { RemoveSortIndicator(); } - else if ( m_headerWin ) + else if ( m_headerWin && + (idx != m_headerWin->m_sortCol || + ascending != m_headerWin->m_sortAsc) ) { m_headerWin->m_sortCol = idx; m_headerWin->m_sortAsc = ascending; - Refresh(); + m_headerWin->Refresh(); } } void wxGenericListCtrl::RemoveSortIndicator() { - if ( m_headerWin ) + if ( m_headerWin && m_headerWin->m_sortCol != -1 ) { m_headerWin->m_sortCol = -1; m_headerWin->m_sortAsc = true; - Refresh(); + m_headerWin->Refresh(); } } From 30ce892ed5c48c91c1b6d74a69e00bfc5da30296 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Mon, 13 Dec 2021 14:31:49 +0000 Subject: [PATCH 4/7] Let wxListCtrl::ShowSortIndicator() implicitly enable indicators It doesn't seem right for ShowSortIndicator() to silently do nothing if EnableSortIndicator() hadn't been called before, so make it enable the sort indicators if they hadn't been enabled yet. The alternative would be to assert in this function, but this seems less useful. Also add some comments to wxMSW version. --- include/wx/msw/listctrl.h | 8 +++++++- interface/wx/listctrl.h | 7 ++++++- src/generic/listctrl.cpp | 1 + src/msw/listctrl.cpp | 25 ++++++++++++++++++++----- 4 files changed, 34 insertions(+), 7 deletions(-) diff --git a/include/wx/msw/listctrl.h b/include/wx/msw/listctrl.h index 02e891353e..f058fe69cc 100644 --- a/include/wx/msw/listctrl.h +++ b/include/wx/msw/listctrl.h @@ -426,6 +426,11 @@ protected: int m_colCount; // Windows doesn't have GetColumnCount so must // keep track of inserted/deleted columns + // m_sortCol and m_sortAsc are used only if m_enableSortCol is true. + // + // Note that m_sortCol may be set to -1, but this is not the same as + // setting m_enableSortCol to false, as the control updates the sort + // indicator on column click in the former case, but not in the latter. bool m_enableSortCol; bool m_sortAsc; int m_sortCol; @@ -457,7 +462,8 @@ private: // in-place editor control. void OnCharHook(wxKeyEvent& event); - // Draw the sort arrow arror in the header. + // Draw the sort arrow in the header. Should only be called when sort + // indicators are enabled. void DrawSortArrow(); // Object using for header custom drawing if necessary, may be NULL. diff --git a/interface/wx/listctrl.h b/interface/wx/listctrl.h index 1fb82c5f9d..e1b387f278 100644 --- a/interface/wx/listctrl.h +++ b/interface/wx/listctrl.h @@ -1428,6 +1428,9 @@ public: indicator in ascending order, or toggle it in the opposite order. To sort the list, call SortItems() in EVT_LIST_COL_CLICK. + Note that calling ShowSortIndicator() implicitly enables sort + indicators. + @note In wxMSW, this will disable the header icon of the column. @param enable @@ -1448,7 +1451,9 @@ public: /** Show the sort indicator of a specific column in a specific direction. - Sort indicators have to be enabled using EnableSortIndicator(). + + This function also enables showing sort indicators if + EnableSortIndicator() hadn't been called. @note This does not actually sort the list, use SortItems() for this. diff --git a/src/generic/listctrl.cpp b/src/generic/listctrl.cpp index a2366dce73..f7eb3f45b4 100644 --- a/src/generic/listctrl.cpp +++ b/src/generic/listctrl.cpp @@ -5108,6 +5108,7 @@ void wxGenericListCtrl::ShowSortIndicator(int idx, bool ascending) (idx != m_headerWin->m_sortCol || ascending != m_headerWin->m_sortAsc) ) { + m_headerWin->m_enableSortCol = true; m_headerWin->m_sortCol = idx; m_headerWin->m_sortAsc = ascending; m_headerWin->Refresh(); diff --git a/src/msw/listctrl.cpp b/src/msw/listctrl.cpp index 283b8b6b5b..6b7be675f2 100644 --- a/src/msw/listctrl.cpp +++ b/src/msw/listctrl.cpp @@ -1459,6 +1459,13 @@ bool wxListCtrl::IsItemChecked(long item) const void wxListCtrl::EnableSortIndicator(bool enable) { m_enableSortCol = enable; + + // This is the only place where we call this function even with disabled + // sort indicators because we may need to reset the currently shown + // indicator after disabling it. + // + // Also note that we should *not* skip calling it m_enableSortCol didn't + // change, as ShowSortIndicator() relies on it being called here. DrawSortArrow(); } @@ -1477,7 +1484,11 @@ void wxListCtrl::ShowSortIndicator(int idx, bool ascending) { m_sortCol = idx; m_sortAsc = ascending; - DrawSortArrow(); + + // We need to enable the sort indicators if they're not enabled yet and + // if they're already enabled, this will update the actually shown sort + // indicator. + EnableSortIndicator(); } } @@ -1485,7 +1496,9 @@ void wxListCtrl::RemoveSortIndicator() { m_sortCol = -1; m_sortAsc = true; - DrawSortArrow(); + + if ( IsSortIndicatorEnabled() ) + DrawSortArrow(); } int wxListCtrl::GetSortIndicator() const @@ -2091,7 +2104,8 @@ long wxListCtrl::DoInsertColumn(long col, const wxListItem& item) SetColumnWidth(n, wxLIST_AUTOSIZE_USEHEADER); } - DrawSortArrow(); + if ( IsSortIndicatorEnabled() ) + DrawSortArrow(); return n; } @@ -2472,7 +2486,8 @@ bool wxListCtrl::MSWOnNotify(int idCtrl, WXLPARAM lParam, WXLPARAM *result) else m_sortAsc = !m_sortAsc; m_sortCol = nmLV->iSubItem; - DrawSortArrow(); + if ( IsSortIndicatorEnabled() ) + DrawSortArrow(); eventType = wxEVT_LIST_COL_CLICK; event.m_itemIndex = -1; @@ -3432,7 +3447,7 @@ void wxListCtrl::DrawSortArrow() { if ( ListView_GetColumn(GetHwnd(), col, &lvCol) ) { - if ( m_enableSortCol && col == m_sortCol ) + if ( col == m_sortCol ) { if ( m_sortAsc ) lvCol.fmt = (lvCol.fmt & ~HDF_SORTDOWN) | HDF_SORTUP; From 52649cc5665248fcc7579bbe9ac0fc92e851e53c Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Tue, 14 Dec 2021 01:06:21 +0000 Subject: [PATCH 5/7] Remove wxListCtrl::EnableSortIndicator() The old API seems unnecessarily complex, it is simpler to just let the application call ShowSortIndicator() itself from its wxEVT_LIST_COL_CLICK handler, which needs to be defined anyhow in order to actually sort the items, rather than require it to enable sort indicator, explicitly set it initially and then remember to not set it any longer in response to the column clicks. Also make RemoveSortIndicator() non-virtual and implement it simply as ShowSortIndicator(-1) because this actually simplifies the code too. --- include/wx/generic/listctrl.h | 3 -- include/wx/generic/private/listctrl.h | 1 - include/wx/listbase.h | 4 +-- include/wx/msw/listctrl.h | 19 +++------- interface/wx/listctrl.h | 39 +++++--------------- samples/listctrl/listtest.cpp | 16 ++++++--- src/generic/listctrl.cpp | 42 ++-------------------- src/msw/listctrl.cpp | 51 +++------------------------ 8 files changed, 33 insertions(+), 142 deletions(-) diff --git a/include/wx/generic/listctrl.h b/include/wx/generic/listctrl.h index 8efc002910..ff2d2f9401 100644 --- a/include/wx/generic/listctrl.h +++ b/include/wx/generic/listctrl.h @@ -115,10 +115,7 @@ public: bool IsItemChecked(long item) const wxOVERRIDE; void CheckItem(long item, bool check) wxOVERRIDE; - void EnableSortIndicator(bool enable = true) wxOVERRIDE; - bool IsSortIndicatorEnabled() const wxOVERRIDE; void ShowSortIndicator(int idx, bool ascending = true) wxOVERRIDE; - void RemoveSortIndicator() wxOVERRIDE; int GetSortIndicator() const wxOVERRIDE; bool IsAscendingSortIndicator() const wxOVERRIDE; diff --git a/include/wx/generic/private/listctrl.h b/include/wx/generic/private/listctrl.h index 0996bed842..62e66edea0 100644 --- a/include/wx/generic/private/listctrl.h +++ b/include/wx/generic/private/listctrl.h @@ -381,7 +381,6 @@ public: int m_colToSend; int m_widthToSend; - bool m_enableSortCol; bool m_sortAsc; int m_sortCol; diff --git a/include/wx/listbase.h b/include/wx/listbase.h index 32ab9aeb3a..bb9098e4b3 100644 --- a/include/wx/listbase.h +++ b/include/wx/listbase.h @@ -447,10 +447,8 @@ public: virtual void CheckItem(long WXUNUSED(item), bool WXUNUSED(check)) { } // Sort indicator in header. - virtual void EnableSortIndicator(bool WXUNUSED(enable) = true) { } - virtual bool IsSortIndicatorEnabled() const { return false; } virtual void ShowSortIndicator(int WXUNUSED(idx), bool WXUNUSED(ascending) = true) { } - virtual void RemoveSortIndicator() { } + void RemoveSortIndicator() { ShowSortIndicator(-1); } virtual int GetSortIndicator() const { return -1; } virtual bool IsAscendingSortIndicator() const { return true; } diff --git a/include/wx/msw/listctrl.h b/include/wx/msw/listctrl.h index f058fe69cc..f796132418 100644 --- a/include/wx/msw/listctrl.h +++ b/include/wx/msw/listctrl.h @@ -226,10 +226,7 @@ public: void CheckItem(long item, bool check) wxOVERRIDE; // Sort indicator in header - void EnableSortIndicator(bool enable = true) wxOVERRIDE; - bool IsSortIndicatorEnabled() const wxOVERRIDE; void ShowSortIndicator(int idx, bool ascending = true) wxOVERRIDE; - void RemoveSortIndicator() wxOVERRIDE; int GetSortIndicator() const wxOVERRIDE; bool IsAscendingSortIndicator() const wxOVERRIDE; @@ -426,21 +423,16 @@ protected: int m_colCount; // Windows doesn't have GetColumnCount so must // keep track of inserted/deleted columns - // m_sortCol and m_sortAsc are used only if m_enableSortCol is true. - // - // Note that m_sortCol may be set to -1, but this is not the same as - // setting m_enableSortCol to false, as the control updates the sort - // indicator on column click in the former case, but not in the latter. - bool m_enableSortCol; - bool m_sortAsc; - int m_sortCol; - // all wxMSWListItemData objects we use wxVector m_internalData; // true if we have any items with custom attributes bool m_hasAnyAttr; + // m_sortAsc is only used if m_sortCol != -1 + bool m_sortAsc; + int m_sortCol; + private: // process NM_CUSTOMDRAW notification message WXLPARAM OnCustomDraw(WXLPARAM lParam); @@ -462,8 +454,7 @@ private: // in-place editor control. void OnCharHook(wxKeyEvent& event); - // Draw the sort arrow in the header. Should only be called when sort - // indicators are enabled. + // Draw the sort arrow in the header. void DrawSortArrow(); // Object using for header custom drawing if necessary, may be NULL. diff --git a/interface/wx/listctrl.h b/interface/wx/listctrl.h index e1b387f278..c107b4f6b2 100644 --- a/interface/wx/listctrl.h +++ b/interface/wx/listctrl.h @@ -1420,40 +1420,15 @@ public: */ void ExtendRulesAndAlternateColour(bool extend = true); - /** - Enable or disable showing a sort indicator in the header bar. - Sort indicators are only shown in report view. - - When clicking on the header of a column, this column will get the sort- - indicator in ascending order, or toggle it in the opposite order. To - sort the list, call SortItems() in EVT_LIST_COL_CLICK. - - Note that calling ShowSortIndicator() implicitly enables sort - indicators. - - @note In wxMSW, this will disable the header icon of the column. - - @param enable - If @true, enable showing a sort indicator, otherwise disable. - - @since 3.1.6 - */ - void EnableSortIndicator(bool enable); - - /** - Returns true if a sort indicator is enabled. - - @see EnableSortIndicator() - - @since 3.1.6 - */ - bool IsSortIndicatorEnabled() const; - /** Show the sort indicator of a specific column in a specific direction. - This function also enables showing sort indicators if - EnableSortIndicator() hadn't been called. + Sort indicators are only shown in report view and in the native wxMSW + version override any column icon, i.e. if the sort indicator is shown + for a column, no (other) icon is shown. + + This function should typically be called from EVT_LIST_COL_CLICK + handler. @note This does not actually sort the list, use SortItems() for this. @@ -1479,6 +1454,8 @@ public: /** Returns the column that shows the sort indicator. + Can return @c -1 if there is no sort indicator currently shown. + @since 3.1.6 */ int GetSortIndicator() const; diff --git a/samples/listctrl/listtest.cpp b/samples/listctrl/listtest.cpp index b563a72dfc..835b31e011 100644 --- a/samples/listctrl/listtest.cpp +++ b/samples/listctrl/listtest.cpp @@ -461,8 +461,6 @@ void MyFrame::RecreateList(long flags, bool withText) flags | wxBORDER_THEME | wxLC_EDIT_LABELS); - m_listCtrl->EnableSortIndicator(); - if ( old ) { wxSizer* const sizer = m_panel->GetSizer(); @@ -1091,10 +1089,18 @@ void MyListCtrl::OnColClick(wxListEvent& event) return; // clicked outside any column. } - if ( IsSortIndicatorEnabled() ) + // If clicking on the same column by which we already sort, toggle the sort + // direction, otherwise use ascending sort by default. + bool ascending; + if ( col == GetSortIndicator() ) + ascending = !IsAscendingSortIndicator(); + else + ascending = true; + + // sort on item data (SetItemData) + if ( SortItems(MyCompareFunction, ascending) ) { - // sort on item data (SetItemData), disable when sorting fails - EnableSortIndicator( SortItems(MyCompareFunction, IsAscendingSortIndicator()) ); + ShowSortIndicator(col, ascending); } // set or unset image diff --git a/src/generic/listctrl.cpp b/src/generic/listctrl.cpp index f7eb3f45b4..d8434a6960 100644 --- a/src/generic/listctrl.cpp +++ b/src/generic/listctrl.cpp @@ -982,7 +982,6 @@ wxListHeaderWindow::wxListHeaderWindow() m_owner = NULL; m_resizeCursor = NULL; - m_enableSortCol = false; m_sortAsc = true; m_sortCol = -1; } @@ -1104,7 +1103,7 @@ void wxListHeaderWindow::OnPaint( wxPaintEvent &WXUNUSED(event) ) #endif wxHeaderSortIconType sortArrow = wxHDR_SORT_ICON_NONE; - if ( m_enableSortCol && i == m_sortCol ) + if ( i == m_sortCol ) { if ( m_sortAsc ) sortArrow = wxHDR_SORT_ICON_UP; @@ -1334,12 +1333,6 @@ void wxListHeaderWindow::OnMouse( wxMouseEvent &event ) colItem.SetState(state & ~wxLIST_STATE_SELECTED); m_owner->SetColumn(i, colItem); } - - if ( m_sortCol != m_column ) - m_sortAsc = true; - else - m_sortAsc = !m_sortAsc; - m_sortCol = m_column; } SendListEvent( event.LeftDown() @@ -5084,47 +5077,18 @@ bool wxGenericListCtrl::IsItemChecked(long item) const return m_mainWin->IsItemChecked(item); } -void wxGenericListCtrl::EnableSortIndicator(bool enable) -{ - if ( m_headerWin && enable != m_headerWin->m_enableSortCol ) - { - m_headerWin->m_enableSortCol = enable; - m_headerWin->Refresh(); - } -} - -bool wxGenericListCtrl::IsSortIndicatorEnabled() const -{ - return m_headerWin && m_headerWin->m_enableSortCol; -} - void wxGenericListCtrl::ShowSortIndicator(int idx, bool ascending) { - if ( idx == -1 ) - { - RemoveSortIndicator(); - } - else if ( m_headerWin && + if ( m_headerWin && (idx != m_headerWin->m_sortCol || - ascending != m_headerWin->m_sortAsc) ) + (idx != -1 && ascending != m_headerWin->m_sortAsc)) ) { - m_headerWin->m_enableSortCol = true; m_headerWin->m_sortCol = idx; m_headerWin->m_sortAsc = ascending; m_headerWin->Refresh(); } } -void wxGenericListCtrl::RemoveSortIndicator() -{ - if ( m_headerWin && m_headerWin->m_sortCol != -1 ) - { - m_headerWin->m_sortCol = -1; - m_headerWin->m_sortAsc = true; - m_headerWin->Refresh(); - } -} - int wxGenericListCtrl::GetSortIndicator() const { if ( m_headerWin ) diff --git a/src/msw/listctrl.cpp b/src/msw/listctrl.cpp index 6b7be675f2..e7b14e03d5 100644 --- a/src/msw/listctrl.cpp +++ b/src/msw/listctrl.cpp @@ -278,7 +278,6 @@ void wxListCtrl::Init() m_colCount = 0; m_textCtrl = NULL; - m_enableSortCol = false; m_sortAsc = true; m_sortCol = -1; @@ -1456,49 +1455,15 @@ bool wxListCtrl::IsItemChecked(long item) const return ListView_GetCheckState(GetHwnd(), (UINT)item) != 0; } -void wxListCtrl::EnableSortIndicator(bool enable) -{ - m_enableSortCol = enable; - - // This is the only place where we call this function even with disabled - // sort indicators because we may need to reset the currently shown - // indicator after disabling it. - // - // Also note that we should *not* skip calling it m_enableSortCol didn't - // change, as ShowSortIndicator() relies on it being called here. - DrawSortArrow(); -} - -bool wxListCtrl::IsSortIndicatorEnabled() const -{ - return m_enableSortCol; -} - void wxListCtrl::ShowSortIndicator(int idx, bool ascending) { - if ( idx == -1 ) - { - RemoveSortIndicator(); - } - else + if ( idx != m_sortCol || (idx != -1 && ascending != m_sortAsc) ) { m_sortCol = idx; m_sortAsc = ascending; - // We need to enable the sort indicators if they're not enabled yet and - // if they're already enabled, this will update the actually shown sort - // indicator. - EnableSortIndicator(); - } -} - -void wxListCtrl::RemoveSortIndicator() -{ - m_sortCol = -1; - m_sortAsc = true; - - if ( IsSortIndicatorEnabled() ) DrawSortArrow(); + } } int wxListCtrl::GetSortIndicator() const @@ -2104,7 +2069,9 @@ long wxListCtrl::DoInsertColumn(long col, const wxListItem& item) SetColumnWidth(n, wxLIST_AUTOSIZE_USEHEADER); } - if ( IsSortIndicatorEnabled() ) + // Update the sort indicator if the index of the column for which it was + // set changed. Note that this condition works even if m_sortCol == -1. + if ( col <= m_sortCol ) DrawSortArrow(); return n; @@ -2481,14 +2448,6 @@ bool wxListCtrl::MSWOnNotify(int idCtrl, WXLPARAM lParam, WXLPARAM *result) break; case LVN_COLUMNCLICK: - if ( m_sortCol != nmLV->iSubItem ) - m_sortAsc = true; - else - m_sortAsc = !m_sortAsc; - m_sortCol = nmLV->iSubItem; - if ( IsSortIndicatorEnabled() ) - DrawSortArrow(); - eventType = wxEVT_LIST_COL_CLICK; event.m_itemIndex = -1; event.m_col = nmLV->iSubItem; From 0a8e82b01041c8931ff8c04b6fa9e6d6c865b2d3 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Tue, 14 Dec 2021 14:19:10 +0000 Subject: [PATCH 6/7] Rename formal parameter of ShowSortIndicator() to "col" No real changes, just use a more appropriate parameter name, as it's a column index and not just "index". --- include/wx/listbase.h | 2 +- interface/wx/listctrl.h | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/include/wx/listbase.h b/include/wx/listbase.h index bb9098e4b3..df569db9ee 100644 --- a/include/wx/listbase.h +++ b/include/wx/listbase.h @@ -447,7 +447,7 @@ public: virtual void CheckItem(long WXUNUSED(item), bool WXUNUSED(check)) { } // Sort indicator in header. - virtual void ShowSortIndicator(int WXUNUSED(idx), bool WXUNUSED(ascending) = true) { } + virtual void ShowSortIndicator(int WXUNUSED(col), bool WXUNUSED(ascending) = true) { } void RemoveSortIndicator() { ShowSortIndicator(-1); } virtual int GetSortIndicator() const { return -1; } virtual bool IsAscendingSortIndicator() const { return true; } diff --git a/interface/wx/listctrl.h b/interface/wx/listctrl.h index c107b4f6b2..a2620d35fe 100644 --- a/interface/wx/listctrl.h +++ b/interface/wx/listctrl.h @@ -1432,7 +1432,7 @@ public: @note This does not actually sort the list, use SortItems() for this. - @param idx + @param col The column to set the sort indicator for. If @c -1 is given, then the currently shown sort indicator will be removed. @@ -1442,7 +1442,7 @@ public: @since 3.1.6 */ - void ShowSortIndicator(int idx, bool ascending = true); + void ShowSortIndicator(int col, bool ascending = true); /** Remove the sort indicator from the column being used as sort key. From c834c0b8b70c5e62b7811cf0a8fd03edc82e4892 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Tue, 14 Dec 2021 14:22:08 +0000 Subject: [PATCH 7/7] Add GetUpdatedAscendingSortIndicator() helper function It seems like this function will need to be used in every implementation of EVT_LIST_COL_CLICK handler when using sorting, so it makes sense to provide it in the library itself. --- include/wx/listbase.h | 6 ++++++ interface/wx/listctrl.h | 25 +++++++++++++++++++++++++ samples/listctrl/listtest.cpp | 9 +-------- 3 files changed, 32 insertions(+), 8 deletions(-) diff --git a/include/wx/listbase.h b/include/wx/listbase.h index df569db9ee..9b200d1412 100644 --- a/include/wx/listbase.h +++ b/include/wx/listbase.h @@ -451,6 +451,12 @@ public: void RemoveSortIndicator() { ShowSortIndicator(-1); } virtual int GetSortIndicator() const { return -1; } virtual bool IsAscendingSortIndicator() const { return true; } + bool GetUpdatedAscendingSortIndicator(int col) const + { + // If clicking on the same column by which we already sort, toggle the sort + // direction, otherwise use ascending sort by default. + return col == GetSortIndicator() ? !IsAscendingSortIndicator() : true; + } protected: // Return pointer to the corresponding m_imagesXXX. diff --git a/interface/wx/listctrl.h b/interface/wx/listctrl.h index a2620d35fe..32aab20b5e 100644 --- a/interface/wx/listctrl.h +++ b/interface/wx/listctrl.h @@ -1460,6 +1460,31 @@ public: */ int GetSortIndicator() const; + /** + Returns the new value to use for sort indicator after clicking a + column. + + This helper function can be useful in the EVT_LIST_COL_CLICK handler + when it updates the sort indicator after the user clicked on a column. + + For example: + @code + void MyListCtrl::OnColClick(wxListEvent& event) + { + int col = event.GetColumn(); + if ( col == -1 ) + return; // clicked outside any column. + + const bool ascending = GetUpdatedAscendingSortIndicator(col); + SortItems(MyCompareFunction, ascending); + ShowSortIndicator(col, ascending); + } + @endcode + + @since 3.1.6 + */ + bool GetUpdatedAscendingSortIndicator(int col) const; + /** Returns @true if the sort indicator direction is ascending, @false when the direction is descending. diff --git a/samples/listctrl/listtest.cpp b/samples/listctrl/listtest.cpp index 835b31e011..4b2aae9e57 100644 --- a/samples/listctrl/listtest.cpp +++ b/samples/listctrl/listtest.cpp @@ -1089,15 +1089,8 @@ void MyListCtrl::OnColClick(wxListEvent& event) return; // clicked outside any column. } - // If clicking on the same column by which we already sort, toggle the sort - // direction, otherwise use ascending sort by default. - bool ascending; - if ( col == GetSortIndicator() ) - ascending = !IsAscendingSortIndicator(); - else - ascending = true; - // sort on item data (SetItemData) + const bool ascending = GetUpdatedAscendingSortIndicator(col); if ( SortItems(MyCompareFunction, ascending) ) { ShowSortIndicator(col, ascending);