From bfd3f1b33fac8ca48656fb5a97719acdc6796e62 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Wed, 31 Jan 2018 23:47:15 +0100 Subject: [PATCH 01/16] Fix some typos in wxDataViewCtrl documentation No real changes. --- interface/wx/dataview.h | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/interface/wx/dataview.h b/interface/wx/dataview.h index 8d087a9367..e98ab8a387 100644 --- a/interface/wx/dataview.h +++ b/interface/wx/dataview.h @@ -155,7 +155,8 @@ public: @param column The column holding the items to be compared. @param ascending - The sort is being peformed in ascending or descending order. + Indicates whether the sort is being performed in ascending or + descending order. @return For an ascending comparison: a negative value if the item1 is less than (i.e. should appear above) item2, zero if the two items are @@ -163,11 +164,12 @@ public: appear below) the second one. The reverse for a descending comparison. @note If there can be multiple rows with the same value, consider - differentiating them form each other by their ID's rather than + differentiating them form each other by their IDs rather than returning zero. This to prevent rows with the same value jumping positions when items are added etc. For example: @code - // Differentiate items with the same value. + // Note that we need to distinguish between items with the same + // value. wxUIntPtr id1 = wxPtrToUInt(item1.GetID()), id2 = wxPtrToUInt(item2.GetID()); @@ -1956,7 +1958,7 @@ public: /** Sets the alignment of the renderer's content. - The default value of @c wxDVR_DEFAULT_ALIGMENT indicates that the content + The default value of @c wxDVR_DEFAULT_ALIGNMENT indicates that the content should have the same alignment as the column header. The method is not implemented under OS X and the renderer always aligns From 945cec4be14af13fd073dfc06d659ca1d2aaf1cd Mon Sep 17 00:00:00 2001 From: mikek Date: Thu, 1 Feb 2018 01:02:56 +0100 Subject: [PATCH 02/16] Refresh generic wxDataViewCtrl after disabling it Grey the control out immediately, instead of waiting until the next refresh to do it. Closes #17887. --- include/wx/generic/dataview.h | 2 +- src/generic/datavgen.cpp | 25 ++++++++++++++----------- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/include/wx/generic/dataview.h b/include/wx/generic/dataview.h index f4ebc64cae..5efc0e1eec 100644 --- a/include/wx/generic/dataview.h +++ b/include/wx/generic/dataview.h @@ -257,10 +257,10 @@ public: #if wxUSE_ACCESSIBILITY virtual bool Show(bool show = true) wxOVERRIDE; - virtual bool Enable(bool enable = true) wxOVERRIDE; virtual void SetName(const wxString &name) wxOVERRIDE; virtual bool Reparent(wxWindowBase *newParent) wxOVERRIDE; #endif // wxUSE_ACCESSIBILITY + virtual bool Enable(bool enable = true) wxOVERRIDE; virtual bool AllowMultiColumnSort(bool allow) wxOVERRIDE; virtual bool IsMultiColumnSortAllowed() const wxOVERRIDE { return m_allowMultiColumnSort; } diff --git a/src/generic/datavgen.cpp b/src/generic/datavgen.cpp index 9a4f08276e..34fdcc7409 100644 --- a/src/generic/datavgen.cpp +++ b/src/generic/datavgen.cpp @@ -5228,17 +5228,6 @@ bool wxDataViewCtrl::Show(bool show) return changed; } -bool wxDataViewCtrl::Enable(bool enable) -{ - bool changed = wxControl::Enable(enable); - if ( changed ) - { - wxAccessible::NotifyEvent(wxACC_EVENT_OBJECT_STATECHANGE, this, wxOBJID_CLIENT, wxACC_SELF); - } - - return changed; -} - void wxDataViewCtrl::SetName(const wxString &name) { wxControl::SetName(name); @@ -5257,6 +5246,20 @@ bool wxDataViewCtrl::Reparent(wxWindowBase *newParent) } #endif // wxUSE_ACCESIBILITY +bool wxDataViewCtrl::Enable(bool enable) +{ + bool changed = wxControl::Enable(enable); + if ( changed ) + { +#if wxUSE_ACCESSIBILITY + wxAccessible::NotifyEvent(wxACC_EVENT_OBJECT_STATECHANGE, this, wxOBJID_CLIENT, wxACC_SELF); +#endif // wxUSE_ACCESIBILITY + Refresh(); + } + + return changed; +} + bool wxDataViewCtrl::AssociateModel( wxDataViewModel *model ) { if (!wxDataViewCtrlBase::AssociateModel( model )) From 0d14dd8fe09e2a43eec043b798583e1f32fe3e16 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Thu, 1 Feb 2018 01:05:17 +0100 Subject: [PATCH 03/16] Test disabling wxDataViewCtrl in the dataview sample Just make it simple to verify whether disabling the control works as expected. See #14186. See #17887. --- samples/dataview/dataview.cpp | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/samples/dataview/dataview.cpp b/samples/dataview/dataview.cpp index a53b1c7be3..77af616a00 100644 --- a/samples/dataview/dataview.cpp +++ b/samples/dataview/dataview.cpp @@ -84,6 +84,7 @@ private: void OnCustomHeaderHeight(wxCommandEvent& event); #endif // wxHAS_GENERIC_DATAVIEWCTRL void OnGetPageInfo(wxCommandEvent& event); + void OnDisable(wxCommandEvent& event); void OnSetForegroundColour(wxCommandEvent& event); void OnIncIndent(wxCommandEvent& event); void OnDecIndent(wxCommandEvent& event); @@ -307,6 +308,7 @@ enum { ID_CLEARLOG = wxID_HIGHEST+1, ID_GET_PAGE_INFO, + ID_DISABLE, ID_BACKGROUND_COLOUR, ID_FOREGROUND_COLOUR, ID_CUSTOM_HEADER_ATTR, @@ -367,6 +369,7 @@ wxBEGIN_EVENT_TABLE(MyFrame, wxFrame) EVT_MENU( ID_CLEARLOG, MyFrame::OnClearLog ) EVT_MENU( ID_GET_PAGE_INFO, MyFrame::OnGetPageInfo ) + EVT_MENU( ID_DISABLE, MyFrame::OnDisable ) EVT_MENU( ID_FOREGROUND_COLOUR, MyFrame::OnSetForegroundColour ) EVT_MENU( ID_BACKGROUND_COLOUR, MyFrame::OnSetBackgroundColour ) EVT_MENU( ID_CUSTOM_HEADER_ATTR, MyFrame::OnCustomHeaderAttr ) @@ -460,6 +463,7 @@ MyFrame::MyFrame(wxFrame *frame, const wxString &title, int x, int y, int w, int wxMenu *file_menu = new wxMenu; file_menu->Append(ID_CLEARLOG, "&Clear log\tCtrl-L"); file_menu->Append(ID_GET_PAGE_INFO, "Show current &page info"); + file_menu->AppendCheckItem(ID_DISABLE, "&Disable\tCtrl-D"); file_menu->Append(ID_FOREGROUND_COLOUR, "Set &foreground colour...\tCtrl-S"); file_menu->Append(ID_BACKGROUND_COLOUR, "Set &background colour...\tCtrl-B"); file_menu->AppendCheckItem(ID_CUSTOM_HEADER_ATTR, "C&ustom header attributes"); @@ -830,6 +834,11 @@ void MyFrame::OnGetPageInfo(wxCommandEvent& WXUNUSED(event)) dvc->GetCountPerPage()); } +void MyFrame::OnDisable(wxCommandEvent& event) +{ + m_ctrl[m_notebook->GetSelection()]->Enable(!event.IsChecked()); +} + void MyFrame::OnSetForegroundColour(wxCommandEvent& WXUNUSED(event)) { wxDataViewCtrl * const dvc = m_ctrl[m_notebook->GetSelection()]; @@ -934,6 +943,8 @@ void MyFrame::OnPageChanged( wxBookCtrlEvent& WXUNUSED(event) ) GetMenuBar()->FindItem(id)->Check( m_ctrl[nPanel]->HasFlag(style) ); } + + GetMenuBar()->FindItem(ID_DISABLE)->Check(!m_ctrl[nPanel]->IsEnabled()); } void MyFrame::OnStyleChange( wxCommandEvent& WXUNUSED(event) ) From f1087d7fd51a1238a484c7e2761f62a5f9e04776 Mon Sep 17 00:00:00 2001 From: mikek Date: Thu, 1 Feb 2018 01:14:18 +0100 Subject: [PATCH 04/16] Fix missing selection event in generic wxDataViewCtrl No event was sent if the selection was narrowed by clicking on an already selected item without any key modifiers pressed. Fix this by generating an event always on click and not only when selecting a new item. Closes #17881. --- docs/changes.txt | 1 + src/generic/datavgen.cpp | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/changes.txt b/docs/changes.txt index 5f1698d1b7..1c82160ae2 100644 --- a/docs/changes.txt +++ b/docs/changes.txt @@ -248,6 +248,7 @@ wxMSW: - Fix focus-related problems when using native wxProgressDialog. - Fix crash when reparenting the currently focused window to another TLW. - Fix sending wxEVT_TEXT_ENTER when using auto-completion (Dubby). +- Fix missing selection event on click in multiselection wxDataViewCtrl (mikek). wxOSX: diff --git a/src/generic/datavgen.cpp b/src/generic/datavgen.cpp index 34fdcc7409..79daf5517d 100644 --- a/src/generic/datavgen.cpp +++ b/src/generic/datavgen.cpp @@ -4728,9 +4728,9 @@ void wxDataViewMainWindow::OnMouse( wxMouseEvent &event ) if ( UnselectAllRows(m_lineSelectSingleOnUp) ) { SelectRow( m_lineSelectSingleOnUp, true ); - SendSelectionChangedEvent( GetItemByRow(m_lineSelectSingleOnUp) ); } - //else: it was already selected, nothing to do + + SendSelectionChangedEvent( GetItemByRow(m_lineSelectSingleOnUp) ); } // If the user click the expander, we do not do editing even if the column From 0b1acae0a76829cd736f57b9b27cb2ca9ff0b3ca Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 3 Feb 2018 14:40:40 +0100 Subject: [PATCH 05/16] Fix drawing of expander buttons in wxDataViewCtrl under MSW The buttons were truncated, making them look ugly, because we didn't allocate enough space for them. Instead of complicating generic wxDataViewCtrl code even further with more MSW-specific code, just let DrawItemTreeButton() center the expander itself in the space allocated for it. This still involves guessing the width of the expander, but this doesn't need to be done as precisely, so it's still an advantage. Note that previously calculating expander size involved m_lineHeight, for some reason, but now we use GetCharWidth() for it, which is more appropriate. Closes #17863. --- src/generic/datavgen.cpp | 32 +++++++++++--------------------- 1 file changed, 11 insertions(+), 21 deletions(-) diff --git a/src/generic/datavgen.cpp b/src/generic/datavgen.cpp index 79daf5517d..801f9dd2ed 100644 --- a/src/generic/datavgen.cpp +++ b/src/generic/datavgen.cpp @@ -75,15 +75,6 @@ static const int SCROLL_UNIT_X = 15; // the cell padding on the left/right static const int PADDING_RIGHTLEFT = 3; -// the expander space margin -static const int EXPANDER_MARGIN = 4; - -#ifdef __WXMSW__ -static const int EXPANDER_OFFSET = 4; -#else -static const int EXPANDER_OFFSET = 1; -#endif - namespace { @@ -2540,23 +2531,20 @@ void wxDataViewMainWindow::OnPaint( wxPaintEvent &WXUNUSED(event) ) // Calculate the indent first indent = GetOwner()->GetIndent() * node->GetIndentLevel(); - // we reserve m_lineHeight of horizontal space for the expander - // but leave EXPANDER_MARGIN around the expander itself - int exp_x = cell_rect.x + indent + EXPANDER_MARGIN; + // We don't have any method to return the size of the expander + // button currently (TODO: add one to wxRendererNative), so + // just guesstimate it. + const int expWidth = 3*dc.GetCharWidth(); - indent += m_lineHeight; - - // draw expander if needed and visible - if ( node->HasChildren() && exp_x < cell_rect.GetRight() ) + // draw expander if needed + if ( node->HasChildren() ) { dc.SetPen( m_penExpander ); dc.SetBrush( wxNullBrush ); - int exp_size = m_lineHeight - 2*EXPANDER_MARGIN; - int exp_y = cell_rect.y + (cell_rect.height - exp_size)/2 - + EXPANDER_MARGIN - EXPANDER_OFFSET; - - const wxRect rect(exp_x, exp_y, exp_size, exp_size); + wxRect rect = cell_rect; + rect.x += indent; + rect.width = expWidth; int flag = 0; if ( m_underMouse == node ) @@ -2571,6 +2559,8 @@ void wxDataViewMainWindow::OnPaint( wxPaintEvent &WXUNUSED(event) ) wxRendererNative::Get().DrawTreeItemButton( this, dc, rect, flag); } + indent += expWidth; + // force the expander column to left-center align cell->SetAlignment( wxALIGN_CENTER_VERTICAL ); } From fd73ae16237f0800fa15f32c93f7a94bf074850a Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 3 Feb 2018 15:05:46 +0100 Subject: [PATCH 06/16] Add wxHD_BITMAP_ON_RIGHT style to wxHeaderCtrl Implement this style for wxMSW to allow putting the column images on the right side, for consistency with wxMSW wxListCtrl. See #13350. --- docs/changes.txt | 1 + include/wx/headerctrl.h | 3 +++ interface/wx/headerctrl.h | 8 ++++++++ src/msw/headerctrl.cpp | 3 +++ 4 files changed, 15 insertions(+) diff --git a/docs/changes.txt b/docs/changes.txt index 1c82160ae2..99680a7aac 100644 --- a/docs/changes.txt +++ b/docs/changes.txt @@ -181,6 +181,7 @@ All (GUI): - Add wxEVT_SEARCH[_CANCEL] synonyms for wxSearchCtrl events. - Generate wxEVT_SEARCH on Enter under all platforms. - Extend wxRendererNative::DrawGauge() to work for vertical gauges too. +- Add wxHD_BITMAP_ON_RIGHT style to wxHeaderCtrl. wxGTK: diff --git a/include/wx/headerctrl.h b/include/wx/headerctrl.h index 928ed7aa5e..f491fcbc7a 100644 --- a/include/wx/headerctrl.h +++ b/include/wx/headerctrl.h @@ -37,6 +37,9 @@ enum // right clicking the header wxHD_ALLOW_HIDE = 0x0002, + // force putting column images on right + wxHD_BITMAP_ON_RIGHT = 0x0004, + // style used by default when creating the control wxHD_DEFAULT_STYLE = wxHD_ALLOW_REORDER }; diff --git a/interface/wx/headerctrl.h b/interface/wx/headerctrl.h index 662d20df0e..8a9907d546 100644 --- a/interface/wx/headerctrl.h +++ b/interface/wx/headerctrl.h @@ -17,6 +17,9 @@ enum // right clicking the header wxHD_ALLOW_HIDE = 0x0002, + // force putting column images on right + wxHD_BITMAP_ON_RIGHT = 0x0004, + // style used by default when creating the control wxHD_DEFAULT_STYLE = wxHD_ALLOW_REORDER }; @@ -76,6 +79,11 @@ enum user to change the columns visibility on right mouse click. Notice that the program can always hide or show the columns, this style only affects the users capability to do it. + @style{wxHD_BITMAP_ON_RIGHT} + The column image, if any, will be shown on the right side if this style + is used. Note that this style is only implemented in wxMSW currently + and doesn't do anything under the other platforms. It is available + since wxWidgets 3.1.1. @style{wxHD_DEFAULT_STYLE} Symbolic name for the default control style, currently equal to @c wxHD_ALLOW_REORDER. diff --git a/src/msw/headerctrl.cpp b/src/msw/headerctrl.cpp index 267775f582..1694a8e288 100644 --- a/src/msw/headerctrl.cpp +++ b/src/msw/headerctrl.cpp @@ -314,6 +314,9 @@ void wxHeaderCtrl::DoInsertItem(const wxHeaderColumn& col, unsigned int idx) { hdi.mask |= HDI_IMAGE; + if ( HasFlag(wxHD_BITMAP_ON_RIGHT) ) + hdi.fmt |= HDF_BITMAP_ON_RIGHT; + if ( bmp.IsOk() ) { const int bmpWidth = bmp.GetWidth(), From 2600bc34ffd0647d7450024df3b20d95e52f9836 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 3 Feb 2018 15:08:29 +0100 Subject: [PATCH 07/16] Put column images on the right side in generic wxDataViewCtrl For consistency with wxListCtrl under MSW (which is the only platform where this change has any effect, as wxHD_BITMAP_ON_RIGHT is only supported there), put the column images on the right side in wxDataViewCtrl too. Closes #13350. --- samples/dataview/dataview.cpp | 2 ++ src/generic/datavgen.cpp | 4 +++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/samples/dataview/dataview.cpp b/samples/dataview/dataview.cpp index 77af616a00..26773c624d 100644 --- a/samples/dataview/dataview.cpp +++ b/samples/dataview/dataview.cpp @@ -23,6 +23,7 @@ #include "wx/wx.h" #endif +#include "wx/artprov.h" #include "wx/dataview.h" #include "wx/datetime.h" #include "wx/splitter.h" @@ -689,6 +690,7 @@ void MyFrame::BuildDataViewCtrl(wxPanel* parent, unsigned int nPanel, unsigned l wxDataViewColumn *column5 = new wxDataViewColumn( "custom", cr, 5, -1, wxALIGN_LEFT, wxDATAVIEW_COL_RESIZABLE ); + column5->SetBitmap(wxArtProvider::GetBitmap(wxART_INFORMATION, wxART_MENU)); m_ctrl[0]->AppendColumn( column5 ); diff --git a/src/generic/datavgen.cpp b/src/generic/datavgen.cpp index 801f9dd2ed..2dff6c1e3d 100644 --- a/src/generic/datavgen.cpp +++ b/src/generic/datavgen.cpp @@ -264,7 +264,9 @@ class wxDataViewHeaderWindow : public wxHeaderCtrl { public: wxDataViewHeaderWindow(wxDataViewCtrl *parent) - : wxHeaderCtrl(parent) + : wxHeaderCtrl(parent, wxID_ANY, + wxDefaultPosition, wxDefaultSize, + wxHD_DEFAULT_STYLE | wxHD_BITMAP_ON_RIGHT) { } From 4b69c1a71899df6d8eb73a1d0899f38a1f3918e2 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 3 Feb 2018 15:35:20 +0100 Subject: [PATCH 08/16] Remove m_penExpander from generic wxDataViewCtrl code This pen was never used as wxRendererNative::DrawTreeItemButton() always uses its own colours, even in the generic version. --- src/generic/datavgen.cpp | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/generic/datavgen.cpp b/src/generic/datavgen.cpp index 2dff6c1e3d..95a056e350 100644 --- a/src/generic/datavgen.cpp +++ b/src/generic/datavgen.cpp @@ -932,9 +932,6 @@ private: // the pen used to draw horiz/vertical rules wxPen m_penRule; - // the pen used to draw the expander and the lines - wxPen m_penExpander; - // This is the tree structure of the model wxDataViewTreeNode * m_root; int m_count; @@ -1959,10 +1956,6 @@ wxDataViewMainWindow::wxDataViewMainWindow( wxDataViewCtrl *parent, wxWindowID i m_penRule = wxPen(GetRuleColour()); - // compose a pen whichcan draw black lines - // TODO: maybe there is something system colour to use - m_penExpander = wxPen(wxColour(0,0,0)); - m_root = wxDataViewTreeNode::CreateRootNode(); // Make m_count = -1 will cause the class recaculate the real displaying number of rows. @@ -2541,9 +2534,6 @@ void wxDataViewMainWindow::OnPaint( wxPaintEvent &WXUNUSED(event) ) // draw expander if needed if ( node->HasChildren() ) { - dc.SetPen( m_penExpander ); - dc.SetBrush( wxNullBrush ); - wxRect rect = cell_rect; rect.x += indent; rect.width = expWidth; From 30f73aea6af19df18bfcf7ba394e357ced5ace62 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 3 Feb 2018 19:21:46 +0100 Subject: [PATCH 09/16] Make using custom colour for wxDataViewProgressRenderer work again This used to work in 3.0, but got broken with the switch to using a native function for rendering the gauge in generic wxDataViewCtrl 86cf756ba95be9ac6988bc466b462e899edf5c4b (see #16406). Restore it now, even if it requires not one, but two hacks: one at wxRendererGeneric level to guess whether a custom colour is being used or not by examining wxDC::GetBackground(), and the other one in wxDataViewProgressRenderer itself to fall back to wxRendererGeneric if the colour is specified. But it is arguably worth doing this to fix the regression, even if it would be nice to provide a better API instead (see #18076). Closes #17885. --- src/generic/datavgen.cpp | 13 ++++++++++++- src/generic/renderg.cpp | 14 +++++++++++++- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/src/generic/datavgen.cpp b/src/generic/datavgen.cpp index 95a056e350..d9840af8ab 100644 --- a/src/generic/datavgen.cpp +++ b/src/generic/datavgen.cpp @@ -1373,7 +1373,18 @@ wxString wxDataViewProgressRenderer::GetAccessibleDescription() const bool wxDataViewProgressRenderer::Render(wxRect rect, wxDC *dc, int WXUNUSED(state)) { - wxRendererNative::Get().DrawGauge( + const wxDataViewItemAttr& attr = GetAttr(); + if ( attr.HasColour() ) + dc->SetBackground(attr.GetColour()); + + // This is a hack, but native renderers don't support using custom colours, + // but typically gauge colour is important (e.g. it's commonly green/red to + // indicate some qualitative difference), so we fall back to the generic + // implementation which looks ugly but does support using custom colour. + wxRendererNative& renderer = attr.HasColour() + ? wxRendererNative::GetGeneric() + : wxRendererNative::Get(); + renderer.DrawGauge( GetOwner()->GetOwner(), *dc, rect, diff --git a/src/generic/renderg.cpp b/src/generic/renderg.cpp index 5cf2656cef..fc2f39645d 100644 --- a/src/generic/renderg.cpp +++ b/src/generic/renderg.cpp @@ -912,6 +912,18 @@ void wxRendererGeneric::DrawGauge(wxWindow* win, int max, int flags) { + // This is a hack, but we want to allow customizing the colour used for the + // gauge body, as this is important for the generic wxDataViewCtrl + // implementation which uses this method. So we assume that if the caller + // had set up a brush using background colour different from the default, + // it should be used. Otherwise we use the default one. + const wxBrush& bg = dc.GetBackground(); + wxColour colBar; + if ( bg.IsOk() && bg.GetColour() != win->GetBackgroundColour() ) + colBar = bg.GetColour(); + else + colBar = wxSystemSettings::GetColour(wxSYS_COLOUR_HIGHLIGHT); + // Use same background as text controls. DrawTextCtrl(win, dc, rect); @@ -929,7 +941,7 @@ void wxRendererGeneric::DrawGauge(wxWindow* win, progRect.width = wxMulDivInt32(progRect.width, value, max); } - dc.SetBrush(wxSystemSettings::GetColour(wxSYS_COLOUR_HIGHLIGHT)); + dc.SetBrush(colBar); dc.SetPen(*wxTRANSPARENT_PEN); dc.DrawRectangle(progRect); } From bb0a2952b19ff4b42a62c8ed8ed8ee36a5ab92f6 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 3 Feb 2018 20:00:08 +0100 Subject: [PATCH 10/16] Replace macros with wxVector<> for wxDataViewModelNotifiers Don't use deprecated macro-based linked list class, use wxVector<> instead for m_notifiers. Also make it private, as it should have been from the beginning. --- include/wx/dataview.h | 11 ++++++----- src/common/datavcmn.cpp | 28 +++++++++++++++++++++++----- 2 files changed, 29 insertions(+), 10 deletions(-) diff --git a/include/wx/dataview.h b/include/wx/dataview.h index 304864aab1..a3547fb13c 100644 --- a/include/wx/dataview.h +++ b/include/wx/dataview.h @@ -27,6 +27,7 @@ #include "wx/dataobj.h" #include "wx/withimages.h" #include "wx/systhemectrl.h" +#include "wx/vector.h" class WXDLLIMPEXP_FWD_CORE wxImageList; class wxItemAttr; @@ -179,8 +180,7 @@ private: // wxDataViewModel // --------------------------------------------------------- -WX_DECLARE_LIST_WITH_DECL(wxDataViewModelNotifier, wxDataViewModelNotifiers, - class WXDLLIMPEXP_ADV); +typedef wxVector wxDataViewModelNotifiers; class WXDLLIMPEXP_ADV wxDataViewModel: public wxRefCounter { @@ -273,8 +273,9 @@ public: virtual bool IsVirtualListModel() const { return false; } protected: - // the user should not delete this class directly: he should use DecRef() instead! - virtual ~wxDataViewModel() { } + // Dtor is protected because the objects of this class must not be deleted, + // DecRef() must be used instead. + virtual ~wxDataViewModel(); // Helper function used by the default Compare() implementation to compare // values of types it is not aware about. Can be overridden in the derived @@ -285,7 +286,7 @@ protected: return 0; } - +private: wxDataViewModelNotifiers m_notifiers; }; diff --git a/src/common/datavcmn.cpp b/src/common/datavcmn.cpp index 0f2ccc24b8..117dec03e3 100644 --- a/src/common/datavcmn.cpp +++ b/src/common/datavcmn.cpp @@ -95,9 +95,6 @@ wxFont wxDataViewItemAttr::GetEffectiveFont(const wxFont& font) const // wxDataViewModelNotifier // --------------------------------------------------------- -#include "wx/listimpl.cpp" -WX_DEFINE_LIST(wxDataViewModelNotifiers) - bool wxDataViewModelNotifier::ItemsAdded( const wxDataViewItem &parent, const wxDataViewItemArray &items ) { size_t count = items.GetCount(); @@ -134,7 +131,15 @@ bool wxDataViewModelNotifier::ItemsChanged( const wxDataViewItemArray &items ) wxDataViewModel::wxDataViewModel() { - m_notifiers.DeleteContents( true ); +} + +wxDataViewModel::~wxDataViewModel() +{ + wxDataViewModelNotifiers::const_iterator iter; + for (iter = m_notifiers.begin(); iter != m_notifiers.end(); ++iter) + { + delete *iter; + } } bool wxDataViewModel::ItemAdded( const wxDataViewItem &parent, const wxDataViewItem &item ) @@ -305,7 +310,20 @@ void wxDataViewModel::AddNotifier( wxDataViewModelNotifier *notifier ) void wxDataViewModel::RemoveNotifier( wxDataViewModelNotifier *notifier ) { - m_notifiers.DeleteObject( notifier ); + wxDataViewModelNotifiers::iterator iter; + for (iter = m_notifiers.begin(); iter != m_notifiers.end(); ++iter) + { + if ( *iter == notifier ) + { + delete notifier; + m_notifiers.erase(iter); + + // Skip the assert below. + return; + } + } + + wxFAIL_MSG(wxS("Removing non-registered notifier")); } int wxDataViewModel::Compare( const wxDataViewItem &item1, const wxDataViewItem &item2, From 342388a456b22cc7c1a25ef68017b66f178ba3c9 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 3 Feb 2018 20:19:14 +0100 Subject: [PATCH 11/16] Use wxCHECK_MSG() instead of wxLogError in wxDataViewTreeStore This is a check for a programming error and should never be shown to the user, so assert that the items being compared in wxDataViewTreeStore are under the same parent instead of using wxLogError. --- src/common/datavcmn.cpp | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/src/common/datavcmn.cpp b/src/common/datavcmn.cpp index 117dec03e3..c8d82f4131 100644 --- a/src/common/datavcmn.cpp +++ b/src/common/datavcmn.cpp @@ -2770,16 +2770,11 @@ int wxDataViewTreeStore::Compare( const wxDataViewItem &item1, const wxDataViewI if (!node1 || !node2) return 0; - wxDataViewTreeStoreContainerNode* parent1 = + wxDataViewTreeStoreContainerNode* const parent = (wxDataViewTreeStoreContainerNode*) node1->GetParent(); - wxDataViewTreeStoreContainerNode* parent2 = - (wxDataViewTreeStoreContainerNode*) node2->GetParent(); - if (parent1 != parent2) - { - wxLogError( wxT("Comparing items with different parent.") ); - return 0; - } + wxCHECK_MSG( node2->GetParent() == parent, 0 + wxS("Comparing items with different parent.") ); if (node1->IsContainer() && !node2->IsContainer()) return -1; From f7e592b33506440dac18b7f61eb7b9d3c43c5c71 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 3 Feb 2018 20:35:38 +0100 Subject: [PATCH 12/16] Use wxVector for storing wxDataViewTreeStoreNodes Replace the use of wxList macros with wxVector. Also make wxDataViewTreeStore::Compare() slightly more efficient by iterating over the children list only once instead of doing it twice. --- include/wx/dataview.h | 13 +++--- src/common/datavcmn.cpp | 90 ++++++++++++++++++++++++++++++----------- 2 files changed, 74 insertions(+), 29 deletions(-) diff --git a/include/wx/dataview.h b/include/wx/dataview.h index a3547fb13c..9162cd9d0d 100644 --- a/include/wx/dataview.h +++ b/include/wx/dataview.h @@ -1212,8 +1212,7 @@ private: wxClientData *m_data; }; -WX_DECLARE_LIST_WITH_DECL(wxDataViewTreeStoreNode, wxDataViewTreeStoreNodeList, - class WXDLLIMPEXP_ADV); +typedef wxVector wxDataViewTreeStoreNodes; class WXDLLIMPEXP_ADV wxDataViewTreeStoreContainerNode: public wxDataViewTreeStoreNode { @@ -1223,11 +1222,13 @@ public: wxClientData *data = NULL ); virtual ~wxDataViewTreeStoreContainerNode(); - const wxDataViewTreeStoreNodeList &GetChildren() const + const wxDataViewTreeStoreNodes &GetChildren() const { return m_children; } - wxDataViewTreeStoreNodeList &GetChildren() + wxDataViewTreeStoreNodes &GetChildren() { return m_children; } + wxDataViewTreeStoreNodes::iterator FindChild(wxDataViewTreeStoreNode* node); + void SetExpandedIcon( const wxIcon &icon ) { m_iconExpanded = icon; } const wxIcon &GetExpandedIcon() const @@ -1241,8 +1242,10 @@ public: virtual bool IsContainer() wxOVERRIDE { return true; } + void DestroyChildren(); + private: - wxDataViewTreeStoreNodeList m_children; + wxDataViewTreeStoreNodes m_children; wxIcon m_iconExpanded; bool m_isExpanded; }; diff --git a/src/common/datavcmn.cpp b/src/common/datavcmn.cpp index c8d82f4131..a0519fab0d 100644 --- a/src/common/datavcmn.cpp +++ b/src/common/datavcmn.cpp @@ -2439,9 +2439,6 @@ wxDataViewTreeStoreNode::~wxDataViewTreeStoreNode() delete m_data; } -#include "wx/listimpl.cpp" -WX_DEFINE_LIST(wxDataViewTreeStoreNodeList) - wxDataViewTreeStoreContainerNode::wxDataViewTreeStoreContainerNode( wxDataViewTreeStoreNode *parent, const wxString &text, const wxIcon &icon, const wxIcon &expanded, wxClientData *data ) @@ -2449,11 +2446,35 @@ wxDataViewTreeStoreContainerNode::wxDataViewTreeStoreContainerNode( , m_iconExpanded(expanded) { m_isExpanded = false; - m_children.DeleteContents(true); } wxDataViewTreeStoreContainerNode::~wxDataViewTreeStoreContainerNode() { + DestroyChildren(); +} + +wxDataViewTreeStoreNodes::iterator +wxDataViewTreeStoreContainerNode::FindChild(wxDataViewTreeStoreNode* node) +{ + wxDataViewTreeStoreNodes::iterator iter; + for (iter = m_children.begin(); iter != m_children.end(); ++iter) + { + if ( *iter == node ) + break; + } + + return iter; +} + +void wxDataViewTreeStoreContainerNode::DestroyChildren() +{ + wxDataViewTreeStoreNodes::const_iterator iter; + for (iter = m_children.begin(); iter != m_children.end(); ++iter) + { + delete *iter; + } + + m_children.clear(); } //----------------------------------------------------------------------------- @@ -2476,7 +2497,7 @@ wxDataViewItem wxDataViewTreeStore::AppendItem( const wxDataViewItem& parent, wxDataViewTreeStoreNode *node = new wxDataViewTreeStoreNode( parent_node, text, icon, data ); - parent_node->GetChildren().Append( node ); + parent_node->GetChildren().push_back( node ); return node->GetItem(); } @@ -2489,7 +2510,8 @@ wxDataViewItem wxDataViewTreeStore::PrependItem( const wxDataViewItem& parent, wxDataViewTreeStoreNode *node = new wxDataViewTreeStoreNode( parent_node, text, icon, data ); - parent_node->GetChildren().Insert( node ); + wxDataViewTreeStoreNodes& children = parent_node->GetChildren(); + children.insert(children.begin(), node); return node->GetItem(); } @@ -2505,12 +2527,13 @@ wxDataViewTreeStore::InsertItem(const wxDataViewItem& parent, if (!parent_node) return wxDataViewItem(0); wxDataViewTreeStoreNode *previous_node = FindNode( previous ); - int pos = parent_node->GetChildren().IndexOf( previous_node ); - if (pos == wxNOT_FOUND) return wxDataViewItem(0); + wxDataViewTreeStoreNodes& children = parent_node->GetChildren(); + const wxDataViewTreeStoreNodes::iterator iter = parent_node->FindChild( previous_node ); + if (iter == children.end()) return wxDataViewItem(0); wxDataViewTreeStoreNode *node = new wxDataViewTreeStoreNode( parent_node, text, icon, data ); - parent_node->GetChildren().Insert( (size_t) pos, node ); + children.insert(iter, node); return node->GetItem(); } @@ -2524,7 +2547,8 @@ wxDataViewItem wxDataViewTreeStore::PrependContainer( const wxDataViewItem& pare wxDataViewTreeStoreContainerNode *node = new wxDataViewTreeStoreContainerNode( parent_node, text, icon, expanded, data ); - parent_node->GetChildren().Insert( node ); + wxDataViewTreeStoreNodes& children = parent_node->GetChildren(); + children.insert(children.begin(), node); return node->GetItem(); } @@ -2541,7 +2565,7 @@ wxDataViewTreeStore::AppendContainer(const wxDataViewItem& parent, wxDataViewTreeStoreContainerNode *node = new wxDataViewTreeStoreContainerNode( parent_node, text, icon, expanded, data ); - parent_node->GetChildren().Append( node ); + parent_node->GetChildren().push_back( node ); return node->GetItem(); } @@ -2558,12 +2582,13 @@ wxDataViewTreeStore::InsertContainer(const wxDataViewItem& parent, if (!parent_node) return wxDataViewItem(0); wxDataViewTreeStoreNode *previous_node = FindNode( previous ); - int pos = parent_node->GetChildren().IndexOf( previous_node ); - if (pos == wxNOT_FOUND) return wxDataViewItem(0); + wxDataViewTreeStoreNodes& children = parent_node->GetChildren(); + const wxDataViewTreeStoreNodes::iterator iter = parent_node->FindChild( previous_node ); + if (iter == children.end()) return wxDataViewItem(0); wxDataViewTreeStoreContainerNode *node = new wxDataViewTreeStoreContainerNode( parent_node, text, icon, expanded, data ); - parent_node->GetChildren().Insert( (size_t) pos, node ); + children.insert(iter, node); return node->GetItem(); } @@ -2581,7 +2606,7 @@ wxDataViewItem wxDataViewTreeStore::GetNthChild( const wxDataViewItem& parent, u wxDataViewTreeStoreContainerNode *parent_node = FindContainerNode( parent ); if (!parent_node) return wxDataViewItem(0); - wxDataViewTreeStoreNodeList::compatibility_iterator node = parent_node->GetChildren().Item( pos ); + wxDataViewTreeStoreNode* const node = parent_node->GetChildren()[pos]; if (node) return wxDataViewItem(node->GetData()); @@ -2597,7 +2622,7 @@ int wxDataViewTreeStore::GetChildCount( const wxDataViewItem& parent ) const return 0; wxDataViewTreeStoreContainerNode *container_node = (wxDataViewTreeStoreContainerNode*) node; - return (int) container_node->GetChildren().GetCount(); + return (int) container_node->GetChildren().size(); } void wxDataViewTreeStore::SetItemText( const wxDataViewItem& item, const wxString &text ) @@ -2673,7 +2698,13 @@ void wxDataViewTreeStore::DeleteItem( const wxDataViewItem& item ) wxDataViewTreeStoreContainerNode *parent_node = FindContainerNode( parent_item ); if (!parent_node) return; - parent_node->GetChildren().DeleteObject( FindNode(item) ); + const wxDataViewTreeStoreNodes::iterator + iter = parent_node->FindChild(FindNode(item)); + if ( iter != parent_node->GetChildren().end() ) + { + delete *iter; + parent_node->GetChildren().erase(iter); + } } void wxDataViewTreeStore::DeleteChildren( const wxDataViewItem& item ) @@ -2681,7 +2712,7 @@ void wxDataViewTreeStore::DeleteChildren( const wxDataViewItem& item ) wxDataViewTreeStoreContainerNode *node = FindContainerNode( item ); if (!node) return; - node->GetChildren().clear(); + node->DestroyChildren(); } void wxDataViewTreeStore::DeleteAllItems() @@ -2751,14 +2782,14 @@ unsigned int wxDataViewTreeStore::GetChildren( const wxDataViewItem &item, wxDat wxDataViewTreeStoreContainerNode *node = FindContainerNode( item ); if (!node) return 0; - wxDataViewTreeStoreNodeList::iterator iter; + wxDataViewTreeStoreNodes::iterator iter; for (iter = node->GetChildren().begin(); iter != node->GetChildren().end(); ++iter) { wxDataViewTreeStoreNode* child = *iter; children.Add( child->GetItem() ); } - return node->GetChildren().GetCount(); + return node->GetChildren().size(); } int wxDataViewTreeStore::Compare( const wxDataViewItem &item1, const wxDataViewItem &item2, @@ -2767,13 +2798,13 @@ int wxDataViewTreeStore::Compare( const wxDataViewItem &item1, const wxDataViewI wxDataViewTreeStoreNode *node1 = FindNode( item1 ); wxDataViewTreeStoreNode *node2 = FindNode( item2 ); - if (!node1 || !node2) + if (!node1 || !node2 || (node1 == node2)) return 0; wxDataViewTreeStoreContainerNode* const parent = (wxDataViewTreeStoreContainerNode*) node1->GetParent(); - wxCHECK_MSG( node2->GetParent() == parent, 0 + wxCHECK_MSG( node2->GetParent() == parent, 0, wxS("Comparing items with different parent.") ); if (node1->IsContainer() && !node2->IsContainer()) @@ -2782,7 +2813,18 @@ int wxDataViewTreeStore::Compare( const wxDataViewItem &item1, const wxDataViewI if (node2->IsContainer() && !node1->IsContainer()) return 1; - return parent1->GetChildren().IndexOf( node1 ) - parent2->GetChildren().IndexOf( node2 ); + wxDataViewTreeStoreNodes::const_iterator iter; + for (iter = parent->GetChildren().begin(); iter != parent->GetChildren().end(); ++iter) + { + if ( *iter == node1 ) + return -1; + + if ( *iter == node2 ) + return 1; + } + + wxFAIL_MSG(wxS("Unreachable")); + return 0; } wxDataViewTreeStoreNode *wxDataViewTreeStore::FindNode( const wxDataViewItem &item ) const @@ -2951,7 +2993,7 @@ void wxDataViewTreeCtrl::DeleteChildren( const wxDataViewItem& item ) if (!node) return; wxDataViewItemArray array; - wxDataViewTreeStoreNodeList::iterator iter; + wxDataViewTreeStoreNodes::iterator iter; for (iter = node->GetChildren().begin(); iter != node->GetChildren().end(); ++iter) { wxDataViewTreeStoreNode* child = *iter; From 6c9ced9be238dee06edb573c7a9024d7ce380ec0 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 3 Feb 2018 20:42:03 +0100 Subject: [PATCH 13/16] Document wxDataViewListCtrl default sort order Also explain how to change it if the default behaviour of putting all the container items before all the leaves is undesirable. Closes #16105. --- interface/wx/dataview.h | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/interface/wx/dataview.h b/interface/wx/dataview.h index e98ab8a387..ed3a535159 100644 --- a/interface/wx/dataview.h +++ b/interface/wx/dataview.h @@ -3405,6 +3405,13 @@ public: without having to derive any class from it, but it is mostly used from within wxDataViewTreeCtrl. + Notice that by default this class sorts all items with children before the + leaf items. If this behaviour is inappropriate, you need to derive a custom + class from this one and override either its HasDefaultCompare() method to + return false, which would result in items being sorted just in the order in + which they were added, or its Compare() function to compare the items using + some other criterion, e.g. alphabetically. + @library{wxadv} @category{dvc} */ From 43c1baf1bd59ab36b53364a3aa5a28cb08578308 Mon Sep 17 00:00:00 2001 From: Hartwig Wiesmann Date: Sat, 3 Feb 2018 21:03:49 +0100 Subject: [PATCH 14/16] Fix wxDataViewColumn::SetSortOrder() under macOS Don't check if we already sort by the column in SetSortOrder() as this meant the sort order couldn't be changed programmatically at all. Closes #15405. --- docs/changes.txt | 1 + src/osx/cocoa/dataview.mm | 32 ++++++++++++++++++-------------- 2 files changed, 19 insertions(+), 14 deletions(-) diff --git a/docs/changes.txt b/docs/changes.txt index 99680a7aac..5fe6a0ba0b 100644 --- a/docs/changes.txt +++ b/docs/changes.txt @@ -282,6 +282,7 @@ wxOSX: - Fix selecting RGB bitmaps (with no alpha channel) into wxMemoryDC. - Fix updating radio groups when menu item is inserted/removed from wxMenu. - Allow changing alignment styles after wxTextCtrl creation (Andreas Falkenhahn). +- Fix wxDataViewColumn::SetSortOrder() (hartwigw). wxQt diff --git a/src/osx/cocoa/dataview.mm b/src/osx/cocoa/dataview.mm index c48c9c976e..e160b1ad92 100644 --- a/src/osx/cocoa/dataview.mm +++ b/src/osx/cocoa/dataview.mm @@ -3513,23 +3513,27 @@ void wxDataViewColumn::SetSortable(bool sortable) void wxDataViewColumn::SetSortOrder(bool ascending) { - if (m_ascending != ascending) + NSTableColumn* const tableColumn = m_NativeDataPtr->GetNativeColumnPtr(); + NSTableView* tableView = [tableColumn tableView]; + + wxCHECK_RET( tableView, wxS("Column has to be associated with a table view when the sorting order is set") ); + + if ( (m_ascending != ascending) || ([tableColumn sortDescriptorPrototype] == nil) ) { m_ascending = ascending; - if (IsSortKey()) - { - // change sorting order: - NSArray* sortDescriptors; - NSSortDescriptor* sortDescriptor; - NSTableColumn* tableColumn; - tableColumn = m_NativeDataPtr->GetNativeColumnPtr(); - sortDescriptor = [[NSSortDescriptor alloc] initWithKey:[[tableColumn sortDescriptorPrototype] key] ascending:m_ascending]; - sortDescriptors = [NSArray arrayWithObject:sortDescriptor]; - [tableColumn setSortDescriptorPrototype:sortDescriptor]; - [[tableColumn tableView] setSortDescriptors:sortDescriptors]; - [sortDescriptor release]; - } + // change sorting order for the native implementation (this will + // trigger a call to outlineView:sortDescriptorsDidChange: where the + // wxWidget's sort descriptors are going to be set): + NSSortDescriptor* const + sortDescriptor = [[NSSortDescriptor alloc] + initWithKey:[NSString stringWithFormat:@"%ld",(long)[tableView columnWithIdentifier:[tableColumn identifier]]] + ascending:m_ascending]; + + NSArray* sortDescriptors = [NSArray arrayWithObject:sortDescriptor]; + [tableColumn setSortDescriptorPrototype:sortDescriptor]; + [tableView setSortDescriptors:sortDescriptors]; + [sortDescriptor release]; } } From d7b88827b80bc5c0397ba0158ee18e65263e7f0f Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 3 Feb 2018 22:32:37 +0100 Subject: [PATCH 15/16] Avoid spurious focus events for composite windows in wxMSW Composite windows, i.e. wxWindows composed of several HWNDs at MSW level, didn't handle focus events correctly and, for example, generated wxEVT_KILL_FOCUS event when the focus simply switched from wxComboBox itself to the EDIT control inside it, which broke using non-readonly wxComboBox as in-place editor inside wxDataViewCtrl. Check if the focus event notifies about the focus switching to, or from, another HWND belonging to the same window, and suppress wx event generation in this case as they don't make sense because the focus doesn't change at wx level. See #17034. --- src/msw/window.cpp | 29 ++++++++++++++++++++++------- 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/src/msw/window.cpp b/src/msw/window.cpp index ead90f0321..0082cef587 100644 --- a/src/msw/window.cpp +++ b/src/msw/window.cpp @@ -4169,6 +4169,14 @@ bool wxWindowMSW::HandleSetFocus(WXHWND hwnd) return false; } + if ( ContainsHWND(hwnd) ) + { + // If another subwindow of this window already had focus before, this + // window should already have focus at wx level, no need for another + // event. + return false; + } + // notify the parent keeping track of focus for the kbd navigation // purposes that we got it wxChildFocusEvent eventFocus((wxWindow *)this); @@ -4193,6 +4201,20 @@ bool wxWindowMSW::HandleSetFocus(WXHWND hwnd) bool wxWindowMSW::HandleKillFocus(WXHWND hwnd) { + // Don't send the event when in the process of being deleted. This can + // only cause problems if the event handler tries to access the object. + if ( m_isBeingDeleted ) + { + return false; + } + + if ( ContainsHWND(hwnd) ) + { + // If the focus switches to another HWND which is part of the same + // wxWindow, we must not generate a wxEVT_KILL_FOCUS. + return false; + } + #if wxUSE_CARET // Deal with caret if ( m_caret ) @@ -4201,13 +4223,6 @@ bool wxWindowMSW::HandleKillFocus(WXHWND hwnd) } #endif // wxUSE_CARET - // Don't send the event when in the process of being deleted. This can - // only cause problems if the event handler tries to access the object. - if ( m_isBeingDeleted ) - { - return false; - } - wxFocusEvent event(wxEVT_KILL_FOCUS, m_windowId); event.SetEventObject(this); From d44dd8caf4f7cee320a096d8d80f4e1e85347681 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 4 Feb 2018 00:28:08 +0100 Subject: [PATCH 16/16] Document wxDataViewCtrl::GetMainWindow() This method can be useful, so make it part of the public API, similarly to e.g. wxGrid::GetGridWindow(). See #17786. --- interface/wx/dataview.h | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/interface/wx/dataview.h b/interface/wx/dataview.h index ed3a535159..8c2b303bdc 100644 --- a/interface/wx/dataview.h +++ b/interface/wx/dataview.h @@ -1442,6 +1442,16 @@ public: virtual wxRect GetItemRect(const wxDataViewItem& item, const wxDataViewColumn* col = NULL) const; + /** + Returns the window corresponding to the main area of the control. + + This is the window that actually shows the control items and may be + different from wxDataViewCtrl window itself in some ports (currently + this is only the case for the generic implementation used by default + under MSW). + */ + wxWindow* GetMainWindow(); + /** Returns pointer to the data model associated with the control (if any). */