From ac1fa83c200a126c452c06cd6d1e6289b4aaa380 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Tue, 17 Aug 2021 22:05:49 +0200 Subject: [PATCH 1/3] Ensure initial value of generic wxSpinCtrl is always valid Under MSW creating a wxSpinCtrl with a range of, say, 1..10 and the default initial value of 0 sets its initial value to 1 (i.e. the closest valid value) as expected, but the generic version still set it to the invalid value of 0, which was unexpected, inconsistent and not useful. Fix the generic version to follow MSW behaviour now and add a test checking for this. --- include/wx/generic/spinctlg.h | 3 +++ src/generic/spinctlg.cpp | 24 +++++++++++++++++------- tests/controls/spinctrltest.cpp | 11 +++++++++++ 3 files changed, 31 insertions(+), 7 deletions(-) diff --git a/include/wx/generic/spinctlg.h b/include/wx/generic/spinctlg.h index 64d9cb95c5..db7162751a 100644 --- a/include/wx/generic/spinctlg.h +++ b/include/wx/generic/spinctlg.h @@ -145,6 +145,9 @@ protected: // check if the value is in range bool InRange(double n) const { return (n >= m_min) && (n <= m_max); } + // adjust the value to fit the range and snap it to ticks if necessary + double AdjustAndSnap(double value) const; + // ensure that the value is in range wrapping it round if necessary double AdjustToFitInRange(double value) const; diff --git a/src/generic/spinctlg.cpp b/src/generic/spinctlg.cpp index ae47212210..7e87d1937a 100644 --- a/src/generic/spinctlg.cpp +++ b/src/generic/spinctlg.cpp @@ -222,11 +222,14 @@ bool wxSpinCtrlGenericBase::Create(wxWindow *parent, return false; } - m_value = initial; - m_min = min; - m_max = max; + m_min = min; + m_max = max; m_increment = increment; + // Note that AdjustAndSnap() uses the variables set above, so only call it + // after assigning the values to them. + m_value = AdjustAndSnap(initial); + // the string value overrides the numeric one (for backwards compatibility // reasons and also because it is simpler to specify the string value which // comes much sooner in the list of arguments and leave the initial @@ -235,7 +238,7 @@ bool wxSpinCtrlGenericBase::Create(wxWindow *parent, { double d; if ( DoTextToValue(value, &d) ) - m_value = d; + m_value = AdjustAndSnap(d); } m_textCtrl = new wxSpinCtrlTextGeneric(this, DoValueToText(m_value), style); @@ -529,10 +532,8 @@ void wxSpinCtrlGenericBase::SetValue(const wxString& text) } } -bool wxSpinCtrlGenericBase::DoSetValue(double val, SendEvent sendEvent) +double wxSpinCtrlGenericBase::AdjustAndSnap(double val) const { - wxCHECK_MSG( m_textCtrl, false, wxT("invalid call to wxSpinCtrl::SetValue") ); - if ( val < m_min ) val = m_min; if ( val > m_max ) @@ -551,6 +552,15 @@ bool wxSpinCtrlGenericBase::DoSetValue(double val, SendEvent sendEvent) } } + return val; +} + +bool wxSpinCtrlGenericBase::DoSetValue(double val, SendEvent sendEvent) +{ + wxCHECK_MSG( m_textCtrl, false, wxT("invalid call to wxSpinCtrl::SetValue") ); + + val = AdjustAndSnap(val); + wxString str(DoValueToText(val)); if ((val != m_value) || (str != m_textCtrl->GetValue())) diff --git a/tests/controls/spinctrltest.cpp b/tests/controls/spinctrltest.cpp index fa2662deee..e7262e83ed 100644 --- a/tests/controls/spinctrltest.cpp +++ b/tests/controls/spinctrltest.cpp @@ -124,6 +124,17 @@ TEST_CASE_METHOD(SpinCtrlTestCase1, "SpinCtrl::Init4", "[spinctrl]") CHECK(m_spin->GetValue() == 99); } +TEST_CASE_METHOD(SpinCtrlTestCase1, "SpinCtrl::InitOutOfRange", "[spinctrl]") +{ + m_spin->Create(wxTheApp->GetTopWindow(), wxID_ANY, "", + wxDefaultPosition, wxDefaultSize, 0, + 10, 20, 0); + + // Recreate the control with another "initial" outside of the valid range: + // it shouldn't be taken into account. + CHECK(m_spin->GetValue() == 10); +} + TEST_CASE_METHOD(SpinCtrlTestCase1, "SpinCtrl::NoEventsInCtor", "[spinctrl]") { // Verify that creating the control does not generate any events. This is From 2727926608657409215a5c646524aeb3bb6a32c6 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Tue, 17 Aug 2021 23:23:53 +0200 Subject: [PATCH 2/3] Add wxLIST_DEFAULT_COL_WIDTH constant The same value of 80px was used in both the generic and MSW versions of wxListCtrl, so introduce a symbolic name for it and define it only once, similarly to how it's already done for wxDVC_DEFAULT_WIDTH and WXGRID_DEFAULT_COL_WIDTH. No real changes. --- include/wx/listbase.h | 5 +++-- interface/wx/listctrl.h | 2 +- src/generic/listctrl.cpp | 5 +---- src/msw/listctrl.cpp | 2 +- 4 files changed, 6 insertions(+), 8 deletions(-) diff --git a/include/wx/listbase.h b/include/wx/listbase.h index 7d8e8f68b4..677e16a29c 100644 --- a/include/wx/listbase.h +++ b/include/wx/listbase.h @@ -135,11 +135,12 @@ enum wxListColumnFormat wxLIST_FORMAT_CENTER = wxLIST_FORMAT_CENTRE }; -// Autosize values for SetColumnWidth +// Values for SetColumnWidth() enum { wxLIST_AUTOSIZE = -1, - wxLIST_AUTOSIZE_USEHEADER = -2 // partly supported by generic version + wxLIST_AUTOSIZE_USEHEADER = -2, // partly supported by generic version + wxLIST_DEFAULT_COL_WIDTH = 80 }; // Flag values for GetItemRect diff --git a/interface/wx/listctrl.h b/interface/wx/listctrl.h index 1ebea5d1bf..b3caa95761 100644 --- a/interface/wx/listctrl.h +++ b/interface/wx/listctrl.h @@ -89,7 +89,7 @@ enum wxListColumnFormat wxLIST_FORMAT_CENTER = wxLIST_FORMAT_CENTRE }; -/// Autosize values for SetColumnWidth +/// Values for SetColumnWidth() enum { wxLIST_AUTOSIZE = -1, diff --git a/src/generic/listctrl.cpp b/src/generic/listctrl.cpp index f0d513a102..cb28fba2a4 100644 --- a/src/generic/listctrl.cpp +++ b/src/generic/listctrl.cpp @@ -96,9 +96,6 @@ static const int MARGIN_BETWEEN_ROWS = 6; // when autosizing the columns, add some slack static const int AUTOSIZE_COL_MARGIN = 10; -// default width for the header columns -static const int WIDTH_COL_DEFAULT = 80; - // the space between the image and the text in the report mode static const int IMAGE_MARGIN_IN_REPORT_MODE = 5; @@ -336,7 +333,7 @@ void wxListHeaderData::SetHeight( int h ) void wxListHeaderData::SetWidth( int w ) { - m_width = w < 0 ? WIDTH_COL_DEFAULT : w; + m_width = w < 0 ? wxLIST_DEFAULT_COL_WIDTH : w; } void wxListHeaderData::SetState( int flag ) diff --git a/src/msw/listctrl.cpp b/src/msw/listctrl.cpp index 922a9809d8..593dcf840e 100644 --- a/src/msw/listctrl.cpp +++ b/src/msw/listctrl.cpp @@ -2057,7 +2057,7 @@ long wxListCtrl::DoInsertColumn(long col, const wxListItem& item) // always give some width to the new column: this one is compatible // with the generic version lvCol.mask |= LVCF_WIDTH; - lvCol.cx = 80; + lvCol.cx = wxLIST_DEFAULT_COL_WIDTH; } long n = ListView_InsertColumn(GetHwnd(), col, &lvCol); From 96d36383bd4b7b7ce2e016324a72a1c7161ee037 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Tue, 17 Aug 2021 23:28:24 +0200 Subject: [PATCH 3/3] Use sensible default column width in generic wxListCtrl too Set width of the new columns to wxLIST_DEFAULT_COL_WIDTH and not 0 in the generic version: this is more compatible with the MSW version and more useful. Document that omitting list column width in XRC results in columns of default, rather than null, as previously, width. --- docs/doxygen/overviews/xrc_format.h | 2 +- src/generic/listctrl.cpp | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/docs/doxygen/overviews/xrc_format.h b/docs/doxygen/overviews/xrc_format.h index c01cae2896..bf9d0d7417 100644 --- a/docs/doxygen/overviews/xrc_format.h +++ b/docs/doxygen/overviews/xrc_format.h @@ -1379,7 +1379,7 @@ following properties (all of them optional): @row3col{text, @ref overview_xrcformat_type_text, The title of the column. } @row3col{width, integer, - The column width. } + The column width. @c wxLIST_DEFAULT_COL_WIDTH is used by default. } @row3col{image, integer, The zero-based index of the image associated with the item in the 'small' image list. } @endTable diff --git a/src/generic/listctrl.cpp b/src/generic/listctrl.cpp index cb28fba2a4..81fe66ca8a 100644 --- a/src/generic/listctrl.cpp +++ b/src/generic/listctrl.cpp @@ -313,8 +313,9 @@ void wxListHeaderData::SetItem( const wxListItem &item ) if ( m_mask & wxLIST_MASK_FORMAT ) m_format = item.m_format; - if ( m_mask & wxLIST_MASK_WIDTH ) - SetWidth(item.m_width); + // Always give some initial width to the new columns (it's still possible + // to set the width to 0 explicitly, however). + SetWidth(m_mask & wxLIST_MASK_WIDTH ? item.m_width : wxLIST_DEFAULT_COL_WIDTH); if ( m_mask & wxLIST_MASK_STATE ) SetState(item.m_state);