From 87afebd6f280b564dcefc27231dbd1dec9f71162 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Mon, 18 Dec 2017 18:28:53 +0100 Subject: [PATCH 01/28] Inline wxStaticBox ctors in wxGTK No real changes, just make the trivial ctors of this class inline for consistency with the new ctor about to be added. --- include/wx/gtk/statbox.h | 11 +++++++++-- src/gtk/statbox.cpp | 15 --------------- 2 files changed, 9 insertions(+), 17 deletions(-) diff --git a/include/wx/gtk/statbox.h b/include/wx/gtk/statbox.h index 7dd72b86ff..b019740133 100644 --- a/include/wx/gtk/statbox.h +++ b/include/wx/gtk/statbox.h @@ -16,14 +16,21 @@ class WXDLLIMPEXP_CORE wxStaticBox : public wxStaticBoxBase { public: - wxStaticBox(); + wxStaticBox() + { + } + wxStaticBox( wxWindow *parent, wxWindowID id, const wxString &label, const wxPoint &pos = wxDefaultPosition, const wxSize &size = wxDefaultSize, long style = 0, - const wxString &name = wxStaticBoxNameStr ); + const wxString &name = wxStaticBoxNameStr ) + { + Create( parent, id, label, pos, size, style, name ); + } + bool Create( wxWindow *parent, wxWindowID id, const wxString &label, diff --git a/src/gtk/statbox.cpp b/src/gtk/statbox.cpp index 797d80caf8..1e4a724497 100644 --- a/src/gtk/statbox.cpp +++ b/src/gtk/statbox.cpp @@ -59,21 +59,6 @@ static gboolean expose_event(GtkWidget* widget, GdkEventExpose*, wxWindow*) // wxStaticBox //----------------------------------------------------------------------------- -wxStaticBox::wxStaticBox() -{ -} - -wxStaticBox::wxStaticBox( wxWindow *parent, - wxWindowID id, - const wxString &label, - const wxPoint& pos, - const wxSize& size, - long style, - const wxString& name ) -{ - Create( parent, id, label, pos, size, style, name ); -} - bool wxStaticBox::Create( wxWindow *parent, wxWindowID id, const wxString& label, From aa47c15abdbd582c5a7c78d745412c15e4c41b45 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Tue, 19 Dec 2017 21:47:01 +0100 Subject: [PATCH 02/28] Add WXDestroyWithoutChildren() and use it from wxStaticBoxSizer Factor out the code from wxStaticBoxSizer dtor into a wxStaticBox method to improve encapsulation: the static box knows better than another class how to detach its children from it before destroying it. No real changes yet. --- include/wx/statbox.h | 6 ++++++ src/common/sizer.cpp | 15 +-------------- src/common/statboxcmn.cpp | 16 ++++++++++++++++ 3 files changed, 23 insertions(+), 14 deletions(-) diff --git a/include/wx/statbox.h b/include/wx/statbox.h index 968903b009..26f3f0dd14 100644 --- a/include/wx/statbox.h +++ b/include/wx/statbox.h @@ -45,6 +45,12 @@ public: *borderOther = BORDER; } + // This is an internal function currently used by wxStaticBoxSizer only. + // + // Reparent all children of the static box under its parent and destroy the + // box itself. + virtual void WXDestroyWithoutChildren(); + protected: // choose the default border for this window virtual wxBorder GetDefaultBorder() const wxOVERRIDE { return wxBORDER_NONE; } diff --git a/src/common/sizer.cpp b/src/common/sizer.cpp index ce59f88a1e..b0b94674b0 100644 --- a/src/common/sizer.cpp +++ b/src/common/sizer.cpp @@ -2582,20 +2582,7 @@ wxStaticBoxSizer::~wxStaticBoxSizer() // previous wxWidgets versions, so ensure they are left alive. if ( m_staticBox ) - { - // Notice that we must make a copy of the list as it will be changed by - // Reparent() calls in the loop. - const wxWindowList children = m_staticBox->GetChildren(); - wxWindow* const parent = m_staticBox->GetParent(); - for ( wxWindowList::const_iterator i = children.begin(); - i != children.end(); - ++i ) - { - (*i)->Reparent(parent); - } - - delete m_staticBox; - } + m_staticBox->WXDestroyWithoutChildren(); } void wxStaticBoxSizer::RecalcSizes() diff --git a/src/common/statboxcmn.cpp b/src/common/statboxcmn.cpp index 3539c3c356..68b42d7af4 100644 --- a/src/common/statboxcmn.cpp +++ b/src/common/statboxcmn.cpp @@ -36,6 +36,22 @@ wxStaticBoxBase::wxStaticBoxBase() #endif } +void wxStaticBoxBase::WXDestroyWithoutChildren() +{ + // Notice that we must make a copy of the list as it will be changed by + // Reparent() calls in the loop. + const wxWindowList children = GetChildren(); + wxWindow* const parent = GetParent(); + for ( wxWindowList::const_iterator i = children.begin(); + i != children.end(); + ++i ) + { + (*i)->Reparent(parent); + } + + delete this; +} + // ---------------------------------------------------------------------------- // XTI // ---------------------------------------------------------------------------- From 29bd25b757a9e18888f43be551367a8ac812665e Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Tue, 19 Dec 2017 22:07:15 +0100 Subject: [PATCH 03/28] Add GTKDoApplyWidgetStyle() helper This allows to call the protected wxWindowGTK::DoApplyWidgetStyle() method when it's really necessary, e.g. when forwarding to it from DoApplyWidgetStyle() implementation for another window. --- include/wx/gtk/window.h | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/include/wx/gtk/window.h b/include/wx/gtk/window.h index fd3f683491..d7b55ae685 100644 --- a/include/wx/gtk/window.h +++ b/include/wx/gtk/window.h @@ -206,6 +206,15 @@ public: virtual void GTKHandleRealized(); void GTKHandleUnrealize(); + // Apply the widget style to the given window. Should normally only be + // called from the overridden DoApplyWidgetStyle() implementation in + // another window and exists solely to provide access to protected + // DoApplyWidgetStyle() when it's really needed. + static void GTKDoApplyWidgetStyle(wxWindowGTK* win, GtkRcStyle *style) + { + win->DoApplyWidgetStyle(style); + } + protected: // for controls composed of multiple GTK widgets, return true to eliminate // spurious focus events if the focus changes between GTK+ children within @@ -426,8 +435,11 @@ protected: void GTKApplyWidgetStyle(bool forceStyle = false); - // helper function to ease native widgets wrapping, called by - // ApplyWidgetStyle -- override this, not ApplyWidgetStyle + // Helper function to ease native widgets wrapping, called by + // GTKApplyWidgetStyle() and supposed to be overridden, not called. + // + // And if you actually need to call it, e.g. to propagate style change to a + // composite control, use public static GTKDoApplyWidgetStyle(). virtual void DoApplyWidgetStyle(GtkRcStyle *style); void GTKApplyStyle(GtkWidget* widget, GtkRcStyle* style); From 7c849276f884de1471e56b3b262e408dcbf20cec Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Tue, 19 Dec 2017 21:29:32 +0100 Subject: [PATCH 04/28] Add support for using arbitrary windows as wxStaticBox labels This commit implements the new feature in wxGTK and updates the sample and the documentation. --- docs/changes.txt | 1 + docs/doxygen/mainpages/const_cpp.h | 2 + include/wx/gtk/statbox.h | 58 ++++++++++++++++++++++- interface/wx/statbox.h | 58 ++++++++++++++++++++++- samples/widgets/static.cpp | 49 +++++++++++++++++-- src/gtk/statbox.cpp | 75 +++++++++++++++++++++++++----- 6 files changed, 226 insertions(+), 17 deletions(-) diff --git a/docs/changes.txt b/docs/changes.txt index 676a77fc19..58207bbf06 100644 --- a/docs/changes.txt +++ b/docs/changes.txt @@ -113,6 +113,7 @@ All (GUI): - Allow wxWebView::RunScript() return values (Jose Lorenzo, GSoC 2017). - Allow using fractional pen widths with wxGraphicsContext (Adrien Tétar). - Add support for loading fonts from external files (Arthur Norman). +- Add support for using arbitrary windows as wxStaticBox labels. - Improve wxSVGFileDC to support more of wxDC API (Maarten Bent). - Add support for wxAuiManager and wxAuiPaneInfo to XRC (Andrea Zanellato). - Add support for wxSL_MIN_MAX_LABELS and wxSL_VALUE_LABEL to XRC (ousnius). diff --git a/docs/doxygen/mainpages/const_cpp.h b/docs/doxygen/mainpages/const_cpp.h index 1756d18a82..6114326616 100644 --- a/docs/doxygen/mainpages/const_cpp.h +++ b/docs/doxygen/mainpages/const_cpp.h @@ -194,6 +194,8 @@ Currently the following symbols exist: @itemdef{wxHAS_RAW_KEY_CODES, Defined if raw key codes (see wxKeyEvent::GetRawKeyCode are supported.} @itemdef{wxHAS_REGEX_ADVANCED, Defined if advanced syntax is available in wxRegEx.} @itemdef{wxHAS_TASK_BAR_ICON, Defined if wxTaskBarIcon is available on the current platform.} +@itemdef{wxHAS_WINDOW_LABEL_IN_STATIC_BOX, Defined if wxStaticBox::Create() + overload taking @c wxWindow* instead of the text label is available on the current platform.} @itemdef{wxHAS_MODE_T, Defined when wxWidgets defines @c mode_t typedef for the compilers not providing it. If another library used in a wxWidgets application, such as ACE (http://www.cs.wustl.edu/~schmidt/ACE.html), also diff --git a/include/wx/gtk/statbox.h b/include/wx/gtk/statbox.h index b019740133..64255f242d 100644 --- a/include/wx/gtk/statbox.h +++ b/include/wx/gtk/statbox.h @@ -18,6 +18,7 @@ class WXDLLIMPEXP_CORE wxStaticBox : public wxStaticBoxBase public: wxStaticBox() { + Init(); } wxStaticBox( wxWindow *parent, @@ -28,6 +29,21 @@ public: long style = 0, const wxString &name = wxStaticBoxNameStr ) { + Init(); + + Create( parent, id, label, pos, size, style, name ); + } + + wxStaticBox( wxWindow *parent, + wxWindowID id, + wxWindow* label, + const wxPoint &pos = wxDefaultPosition, + const wxSize &size = wxDefaultSize, + long style = 0, + const wxString &name = wxStaticBoxNameStr ) + { + Init(); + Create( parent, id, label, pos, size, style, name ); } @@ -37,7 +53,21 @@ public: const wxPoint &pos = wxDefaultPosition, const wxSize &size = wxDefaultSize, long style = 0, - const wxString &name = wxStaticBoxNameStr ); + const wxString &name = wxStaticBoxNameStr ) + { + return DoCreate( parent, id, &label, NULL, pos, size, style, name ); + } + + bool Create( wxWindow *parent, + wxWindowID id, + wxWindow* label, + const wxPoint &pos = wxDefaultPosition, + const wxSize &size = wxDefaultSize, + long style = 0, + const wxString &name = wxStaticBoxNameStr ) + { + return DoCreate( parent, id, NULL, label, pos, size, style, name ); + } virtual void SetLabel( const wxString &label ) wxOVERRIDE; @@ -52,13 +82,39 @@ public: virtual void AddChild( wxWindowBase *child ) wxOVERRIDE; + virtual void WXDestroyWithoutChildren() wxOVERRIDE; + protected: + // Common part of all ctors. + void Init() + { + m_labelWin = NULL; + } + + // Common implementation of both Create() overloads: exactly one of + // labelStr and labelWin parameters must be non-null. + bool DoCreate(wxWindow *parent, + wxWindowID id, + const wxString* labelStr, + wxWindow* labelWin, + const wxPoint& pos, + const wxSize& size, + long style, + const wxString& name); + virtual bool GTKWidgetNeedsMnemonic() const wxOVERRIDE; virtual void GTKWidgetDoSetMnemonic(GtkWidget* w) wxOVERRIDE; void DoApplyWidgetStyle(GtkRcStyle *style) wxOVERRIDE; + // If non-null, the window used as our label. This window is owned by the + // static box and will be deleted when it is. + wxWindow* m_labelWin; + wxDECLARE_DYNAMIC_CLASS(wxStaticBox); }; +// Indicate that we have the ctor overload taking wxWindow as label. +#define wxHAS_WINDOW_LABEL_IN_STATIC_BOX + #endif // _WX_GTKSTATICBOX_H_ diff --git a/interface/wx/statbox.h b/interface/wx/statbox.h index 46004f20f4..8627255027 100644 --- a/interface/wx/statbox.h +++ b/interface/wx/statbox.h @@ -84,6 +84,42 @@ public: long style = 0, const wxString& name = wxStaticBoxNameStr); + /** + Constructor for a static box using the given window as label. + + This constructor takes a pointer to an arbitrary window (although + usually a wxCheckBox or a wxRadioButton) instead of just the usual text + label and puts this window at the top of the box at the place where the + label would be shown. + + The @a label window must be a non-null, fully created window and will + become a child of this wxStaticBox, i.e. it will be owned by this + control and will be deleted when the wxStaticBox itself is deleted. + + An example of creating a wxStaticBox with window as a label: + @code + void MyFrame::CreateControls() + { + wxPanel* panel = new wxPanel(this); + wxCheckBox* checkbox = new wxCheckBox(panel, wxID_ANY, "Box checkbox"); + wxStaticBox* box = new wxStaticBox(panel, wxID_ANY, checkbox); + ... + } + @endcode + + Currently this constructor is only available in wxGTK, use + @c wxHAS_WINDOW_LABEL_IN_STATIC_BOX to check whether it can be used at + compile-time. + + @since 3.1.1 + */ + wxStaticBox(wxWindow* parent, wxWindowID id, + wxWindow* label, + const wxPoint& pos = wxDefaultPosition, + const wxSize& size = wxDefaultSize, + long style = 0, + const wxString& name = wxStaticBoxNameStr); + /** Destructor, destroying the group box. */ @@ -97,5 +133,25 @@ public: const wxPoint& pos = wxDefaultPosition, const wxSize& size = wxDefaultSize, long style = 0, const wxString& name = wxStaticBoxNameStr); -}; + /** + Creates the static box with the window as a label. + + This method can only be called for an object created using its default + constructor. + + See the constructor documentation for more details. + + Currently this overload is only available in wxGTK, use + @c wxHAS_WINDOW_LABEL_IN_STATIC_BOX to check whether it can be used at + compile-time. + + @since 3.1.1 + */ + wxStaticBox(wxWindow* parent, wxWindowID id, + wxWindow* label, + const wxPoint& pos = wxDefaultPosition, + const wxSize& size = wxDefaultSize, + long style = 0, + const wxString& name = wxStaticBoxNameStr); +}; diff --git a/samples/widgets/static.cpp b/samples/widgets/static.cpp index 36827f50ae..5e16c71d89 100644 --- a/samples/widgets/static.cpp +++ b/samples/widgets/static.cpp @@ -116,6 +116,9 @@ public: protected: // event handlers void OnCheckOrRadioBox(wxCommandEvent& event); +#ifdef wxHAS_WINDOW_LABEL_IN_STATIC_BOX + void OnBoxCheckBox(wxCommandEvent& event); +#endif // wxHAS_WINDOW_LABEL_IN_STATIC_BOX void OnButtonReset(wxCommandEvent& event); void OnButtonBoxText(wxCommandEvent& event); @@ -137,6 +140,9 @@ protected: // the check/radio boxes for styles wxCheckBox *m_chkVert, *m_chkGeneric, +#ifdef wxHAS_WINDOW_LABEL_IN_STATIC_BOX + *m_chkBoxWithCheck, +#endif // wxHAS_WINDOW_LABEL_IN_STATIC_BOX *m_chkAutoResize, *m_chkEllipsize; @@ -207,6 +213,9 @@ StaticWidgetsPage::StaticWidgetsPage(WidgetsBookCtrl *book, m_chkVert = m_chkAutoResize = m_chkGeneric = +#ifdef wxHAS_WINDOW_LABEL_IN_STATIC_BOX + m_chkBoxWithCheck = +#endif // wxHAS_WINDOW_LABEL_IN_STATIC_BOX #if wxUSE_MARKUP m_chkGreen = #endif // wxUSE_MARKUP @@ -243,6 +252,9 @@ void StaticWidgetsPage::CreateContent() m_chkGeneric = CreateCheckBoxAndAddToSizer(sizerLeft, "&Generic wxStaticText"); +#ifdef wxHAS_WINDOW_LABEL_IN_STATIC_BOX + m_chkBoxWithCheck = CreateCheckBoxAndAddToSizer(sizerLeft, "Checkable &box"); +#endif // wxHAS_WINDOW_LABEL_IN_STATIC_BOX m_chkVert = CreateCheckBoxAndAddToSizer(sizerLeft, "&Vertical line"); m_chkAutoResize = CreateCheckBoxAndAddToSizer(sizerLeft, "&Fit to text"); sizerLeft->Add(5, 5, 0, wxGROW | wxALL, 5); // spacer @@ -367,6 +379,9 @@ void StaticWidgetsPage::CreateContent() void StaticWidgetsPage::Reset() { m_chkGeneric->SetValue(false); +#ifdef wxHAS_WINDOW_LABEL_IN_STATIC_BOX + m_chkBoxWithCheck->SetValue(false); +#endif // wxHAS_WINDOW_LABEL_IN_STATIC_BOX m_chkVert->SetValue(false); m_chkAutoResize->SetValue(true); m_chkEllipsize->SetValue(true); @@ -469,10 +484,28 @@ void StaticWidgetsPage::CreateStatic() flagsText |= align; flagsBox |= align; - wxStaticBox *staticBox = new wxStaticBox(this, wxID_ANY, - m_textBox->GetValue(), - wxDefaultPosition, wxDefaultSize, - flagsBox); + wxStaticBox *staticBox; +#ifdef wxHAS_WINDOW_LABEL_IN_STATIC_BOX + if ( m_chkBoxWithCheck->GetValue() ) + { + wxCheckBox* const label = new wxCheckBox(this, wxID_ANY, + m_textBox->GetValue()); + label->Bind(wxEVT_CHECKBOX, &StaticWidgetsPage::OnBoxCheckBox, this); + + staticBox = new wxStaticBox(this, wxID_ANY, + label, + wxDefaultPosition, wxDefaultSize, + flagsBox); + } + else // normal static box +#endif // wxHAS_WINDOW_LABEL_IN_STATIC_BOX + { + staticBox = new wxStaticBox(this, wxID_ANY, + m_textBox->GetValue(), + wxDefaultPosition, wxDefaultSize, + flagsBox); + } + m_sizerStatBox = new wxStaticBoxSizer(staticBox, isVert ? wxHORIZONTAL : wxVERTICAL); @@ -559,6 +592,14 @@ void StaticWidgetsPage::OnCheckOrRadioBox(wxCommandEvent& event) CreateStatic(); } +#ifdef wxHAS_WINDOW_LABEL_IN_STATIC_BOX +void StaticWidgetsPage::OnBoxCheckBox(wxCommandEvent& event) +{ + wxLogMessage("Box check box has been %schecked", + event.IsChecked() ? "": "un"); +} +#endif // wxHAS_WINDOW_LABEL_IN_STATIC_BOX + void StaticWidgetsPage::OnButtonBoxText(wxCommandEvent& WXUNUSED(event)) { m_sizerStatBox->GetStaticBox()->SetLabel(m_textBox->GetValue()); diff --git a/src/gtk/statbox.cpp b/src/gtk/statbox.cpp index 1e4a724497..0d144e128b 100644 --- a/src/gtk/statbox.cpp +++ b/src/gtk/statbox.cpp @@ -59,13 +59,14 @@ static gboolean expose_event(GtkWidget* widget, GdkEventExpose*, wxWindow*) // wxStaticBox //----------------------------------------------------------------------------- -bool wxStaticBox::Create( wxWindow *parent, - wxWindowID id, - const wxString& label, - const wxPoint& pos, - const wxSize& size, - long style, - const wxString& name ) +bool wxStaticBox::DoCreate(wxWindow *parent, + wxWindowID id, + const wxString* labelStr, + wxWindow* labelWin, + const wxPoint& pos, + const wxSize& size, + long style, + const wxString& name) { if (!PreCreation( parent, pos, size ) || !CreateBase( parent, id, pos, size, style, wxDefaultValidator, name )) @@ -74,11 +75,41 @@ bool wxStaticBox::Create( wxWindow *parent, return false; } - m_widget = GTKCreateFrame(label); - g_object_ref(m_widget); + if ( labelStr ) + { + m_widget = GTKCreateFrame(*labelStr); - // only base SetLabel needs to be called after GTKCreateFrame - wxControl::SetLabel(label); + // only base SetLabel needs to be called after GTKCreateFrame + wxControl::SetLabel(*labelStr); + } + else // Use the given window as the label. + { + wxCHECK_MSG( labelWin, false, wxS("Label window can't be null") ); + + GtkWidget* const labelWidget = labelWin->m_widget; + wxCHECK_MSG( labelWidget, false, wxS("Label window must be created") ); + + // The widget must not have any parent at GTK+ level or setting it as + // label widget would fail. + GtkWidget* const oldParent = gtk_widget_get_parent(labelWidget); + gtk_container_remove(GTK_CONTAINER(oldParent), labelWidget); + gtk_widget_unparent(labelWidget); + + // It also should be our child at wx API level, but without being our + // child in wxGTK, i.e. it must not be added to the GtkFrame container, + // so we can't call Reparent() here (not even wxWindowBase version, as + // it still would end up in our overridden AddChild()), nor the normal + // AddChild() for the same reason. + labelWin->GetParent()->RemoveChild(labelWin); + wxWindowBase::AddChild(labelWin); + + m_labelWin = labelWin; + + m_widget = gtk_frame_new(NULL); + gtk_frame_set_label_widget(GTK_FRAME(m_widget), labelWidget); + } + + g_object_ref(m_widget); m_parent->DoAddChild( this ); @@ -121,16 +152,38 @@ void wxStaticBox::AddChild( wxWindowBase *child ) wxStaticBoxBase::AddChild(child); } +void wxStaticBox::WXDestroyWithoutChildren() +{ + // The label window doesn't count as our child, it's really a part of + // static box itself and it makes no sense to leave it alive when the box + // is destroyed, so do it even when it's supposed to be destroyed without + // destroying its children. + if ( m_labelWin ) + { + // By deleting it here, we indirectly remove this window from the list + // of our children and hence prevent the base class version of this + // method from reparenting it and thus keeping it alive. + delete m_labelWin; + m_labelWin = NULL; + } + + wxStaticBoxBase::WXDestroyWithoutChildren(); +} + void wxStaticBox::SetLabel( const wxString& label ) { wxCHECK_RET( m_widget != NULL, wxT("invalid staticbox") ); + wxCHECK_RET( !m_labelWin, wxS("Doesn't make sense when using label window") ); + GTKSetLabelForFrame(GTK_FRAME(m_widget), label); } void wxStaticBox::DoApplyWidgetStyle(GtkRcStyle *style) { GTKFrameApplyWidgetStyle(GTK_FRAME(m_widget), style); + if ( m_labelWin ) + GTKDoApplyWidgetStyle(m_labelWin, style); if (m_wxwindow) GTKApplyStyle(m_wxwindow, style); From 8c06a24da4ffdb0e4f3d9713491edb45f9fac318 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Wed, 20 Dec 2017 00:03:55 +0100 Subject: [PATCH 05/28] Move m_labelWin to wxStaticBoxBase itself It will be reused by all platforms and is not specific to wxGTK. This also means WXDestroyWithoutChildren() doesn't need to be virtual any longer. --- include/wx/gtk/statbox.h | 17 ----------------- include/wx/statbox.h | 6 +++++- src/common/statboxcmn.cpp | 12 +++++++++++- src/gtk/statbox.cpp | 18 ------------------ 4 files changed, 16 insertions(+), 37 deletions(-) diff --git a/include/wx/gtk/statbox.h b/include/wx/gtk/statbox.h index 64255f242d..cce262cbae 100644 --- a/include/wx/gtk/statbox.h +++ b/include/wx/gtk/statbox.h @@ -18,7 +18,6 @@ class WXDLLIMPEXP_CORE wxStaticBox : public wxStaticBoxBase public: wxStaticBox() { - Init(); } wxStaticBox( wxWindow *parent, @@ -29,8 +28,6 @@ public: long style = 0, const wxString &name = wxStaticBoxNameStr ) { - Init(); - Create( parent, id, label, pos, size, style, name ); } @@ -42,8 +39,6 @@ public: long style = 0, const wxString &name = wxStaticBoxNameStr ) { - Init(); - Create( parent, id, label, pos, size, style, name ); } @@ -82,15 +77,7 @@ public: virtual void AddChild( wxWindowBase *child ) wxOVERRIDE; - virtual void WXDestroyWithoutChildren() wxOVERRIDE; - protected: - // Common part of all ctors. - void Init() - { - m_labelWin = NULL; - } - // Common implementation of both Create() overloads: exactly one of // labelStr and labelWin parameters must be non-null. bool DoCreate(wxWindow *parent, @@ -107,10 +94,6 @@ protected: void DoApplyWidgetStyle(GtkRcStyle *style) wxOVERRIDE; - // If non-null, the window used as our label. This window is owned by the - // static box and will be deleted when it is. - wxWindow* m_labelWin; - wxDECLARE_DYNAMIC_CLASS(wxStaticBox); }; diff --git a/include/wx/statbox.h b/include/wx/statbox.h index 26f3f0dd14..360881031a 100644 --- a/include/wx/statbox.h +++ b/include/wx/statbox.h @@ -49,12 +49,16 @@ public: // // Reparent all children of the static box under its parent and destroy the // box itself. - virtual void WXDestroyWithoutChildren(); + void WXDestroyWithoutChildren(); protected: // choose the default border for this window virtual wxBorder GetDefaultBorder() const wxOVERRIDE { return wxBORDER_NONE; } + // If non-null, the window used as our label. This window is owned by the + // static box and will be deleted when it is. + wxWindow* m_labelWin; + wxDECLARE_NO_COPY_CLASS(wxStaticBoxBase); }; diff --git a/src/common/statboxcmn.cpp b/src/common/statboxcmn.cpp index 68b42d7af4..62c4b50134 100644 --- a/src/common/statboxcmn.cpp +++ b/src/common/statboxcmn.cpp @@ -31,6 +31,8 @@ extern WXDLLEXPORT_DATA(const char) wxStaticBoxNameStr[] = "groupBox"; wxStaticBoxBase::wxStaticBoxBase() { + m_labelWin = NULL; + #ifndef __WXGTK__ m_container.DisableSelfFocus(); #endif @@ -46,7 +48,15 @@ void wxStaticBoxBase::WXDestroyWithoutChildren() i != children.end(); ++i ) { - (*i)->Reparent(parent); + // The label window doesn't count as our child, it's really a part of + // static box itself and it makes no sense to leave it alive when the + // box is destroyed, so do it even when it's supposed to be destroyed + // without destroying its children -- by not reparenting it, we ensure + // that it's destroyed when this object itself is below. + if ( *i != m_labelWin ) + { + (*i)->Reparent(parent); + } } delete this; diff --git a/src/gtk/statbox.cpp b/src/gtk/statbox.cpp index 0d144e128b..6979591287 100644 --- a/src/gtk/statbox.cpp +++ b/src/gtk/statbox.cpp @@ -152,24 +152,6 @@ void wxStaticBox::AddChild( wxWindowBase *child ) wxStaticBoxBase::AddChild(child); } -void wxStaticBox::WXDestroyWithoutChildren() -{ - // The label window doesn't count as our child, it's really a part of - // static box itself and it makes no sense to leave it alive when the box - // is destroyed, so do it even when it's supposed to be destroyed without - // destroying its children. - if ( m_labelWin ) - { - // By deleting it here, we indirectly remove this window from the list - // of our children and hence prevent the base class version of this - // method from reparenting it and thus keeping it alive. - delete m_labelWin; - m_labelWin = NULL; - } - - wxStaticBoxBase::WXDestroyWithoutChildren(); -} - void wxStaticBox::SetLabel( const wxString& label ) { wxCHECK_RET( m_widget != NULL, wxT("invalid staticbox") ); From 91875045ac841f2c1d9d7d6b1c7539ba7d111b06 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Wed, 20 Dec 2017 00:06:08 +0100 Subject: [PATCH 06/28] Don't make wxStaticBoxBase::GetBordersForSizer() inline Move the function definition to the source file, there doesn't seem to be any reason to keep it in the header. --- include/wx/statbox.h | 8 +------- src/common/statboxcmn.cpp | 8 ++++++++ 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/include/wx/statbox.h b/include/wx/statbox.h index 360881031a..0e6e4f3ac3 100644 --- a/include/wx/statbox.h +++ b/include/wx/statbox.h @@ -37,13 +37,7 @@ public: // // the top border is the margin at the top (where the title is), // borderOther is the margin on all other sides - virtual void GetBordersForSizer(int *borderTop, int *borderOther) const - { - const int BORDER = FromDIP(5); // FIXME: hardcoded value - - *borderTop = GetLabel().empty() ? BORDER : GetCharHeight(); - *borderOther = BORDER; - } + virtual void GetBordersForSizer(int *borderTop, int *borderOther) const; // This is an internal function currently used by wxStaticBoxSizer only. // diff --git a/src/common/statboxcmn.cpp b/src/common/statboxcmn.cpp index 62c4b50134..c8427783f3 100644 --- a/src/common/statboxcmn.cpp +++ b/src/common/statboxcmn.cpp @@ -38,6 +38,14 @@ wxStaticBoxBase::wxStaticBoxBase() #endif } +void wxStaticBoxBase::GetBordersForSizer(int *borderTop, int *borderOther) const +{ + const int BORDER = FromDIP(5); // FIXME: hardcoded value + + *borderTop = GetLabel().empty() ? BORDER : GetCharHeight(); + *borderOther = BORDER; +} + void wxStaticBoxBase::WXDestroyWithoutChildren() { // Notice that we must make a copy of the list as it will be changed by From 67225fb07edb94deefc8d2e5b05431ad5e2e2d98 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Wed, 20 Dec 2017 00:29:03 +0100 Subject: [PATCH 07/28] Remove borders from wxStaticBoxSizer items in the widgets sample This allows to see better whether the borders returned by wxStaticBox::GetBordersForSizer() are correct. --- samples/widgets/static.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/samples/widgets/static.cpp b/samples/widgets/static.cpp index 5e16c71d89..8869bab39d 100644 --- a/samples/widgets/static.cpp +++ b/samples/widgets/static.cpp @@ -551,12 +551,12 @@ void StaticWidgetsPage::CreateStatic() isVert ? wxLI_VERTICAL : wxLI_HORIZONTAL); #endif // wxUSE_STATLINE - m_sizerStatBox->Add(m_statText, 0, wxGROW | wxALL, 5); + m_sizerStatBox->Add(m_statText, 0, wxGROW); #if wxUSE_STATLINE - m_sizerStatBox->Add(m_statLine, 0, wxGROW | wxALL, 5); + m_sizerStatBox->Add(m_statLine, 0, wxGROW | wxTOP | wxBOTTOM, 10); #endif // wxUSE_STATLINE #if wxUSE_MARKUP - m_sizerStatBox->Add(m_statMarkup, 0, wxALL, 5); + m_sizerStatBox->Add(m_statMarkup); #endif // wxUSE_MARKUP m_sizerStatic->Add(m_sizerStatBox, 0, wxGROW); From b34a1c036a18e1bf90cd81b264ee9dfe1d5382f3 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Wed, 20 Dec 2017 00:30:12 +0100 Subject: [PATCH 08/28] Account for label window in wxStaticBoxBase::GetBordersForSizer() Take the window used as label into account in the default implementation of this method. --- src/common/statboxcmn.cpp | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/common/statboxcmn.cpp b/src/common/statboxcmn.cpp index c8427783f3..4db934146a 100644 --- a/src/common/statboxcmn.cpp +++ b/src/common/statboxcmn.cpp @@ -42,7 +42,15 @@ void wxStaticBoxBase::GetBordersForSizer(int *borderTop, int *borderOther) const { const int BORDER = FromDIP(5); // FIXME: hardcoded value - *borderTop = GetLabel().empty() ? BORDER : GetCharHeight(); + if ( m_labelWin ) + { + *borderTop = m_labelWin->GetSize().y; + } + else + { + *borderTop = GetLabel().empty() ? BORDER : GetCharHeight(); + } + *borderOther = BORDER; } From 23a830ae16de33d30ee184097e42b5998c3ebb73 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Wed, 20 Dec 2017 22:53:12 +0100 Subject: [PATCH 09/28] Use symbolic names in wxMSW wxStaticBox drawing code Introduce symbolic constants instead of using raw magic numbers. No real changes and these numbers are still as magic as before, but at least they will be easier to change now. --- src/msw/statbox.cpp | 33 +++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/src/msw/statbox.cpp b/src/msw/statbox.cpp index 9ae5c230df..bcc79cd1f7 100644 --- a/src/msw/statbox.cpp +++ b/src/msw/statbox.cpp @@ -54,6 +54,21 @@ #define TMT_FONT 210 +namespace +{ + +// Offset of the first pixel of the label from the box left border. +// +// FIXME: value is hardcoded as this is what it is on my system, no idea if +// it's true everywhere +const int LABEL_HORZ_OFFSET = 9; + +// Extra borders around the label on left/right and bottom sides. +const int LABEL_HORZ_BORDER = 2; +const int LABEL_VERT_BORDER = 2; + +} // anonymous namespace + // ---------------------------------------------------------------------------- // wxWin macros // ---------------------------------------------------------------------------- @@ -443,23 +458,17 @@ void wxStaticBox::PaintForeground(wxDC& dc, const RECT&) dc.GetTextExtent(wxStripMenuCodes(label, wxStrip_Mnemonics), &width, &height); - int x; - int y = height; - // first we need to correctly paint the background of the label // as Windows ignores the brush offset when doing it - // - // FIXME: value of x is hardcoded as this is what it is on my system, - // no idea if it's true everywhere - RECT dimensions = {0, 0, 0, y}; - x = 9; + const int x = LABEL_HORZ_OFFSET; + RECT dimensions = { x, 0, 0, height }; dimensions.left = x; dimensions.right = x + width; // need to adjust the rectangle to cover all the label background - dimensions.left -= 2; - dimensions.right += 2; - dimensions.bottom += 2; + dimensions.left -= LABEL_HORZ_BORDER; + dimensions.right += LABEL_HORZ_BORDER; + dimensions.bottom += LABEL_VERT_BORDER; if ( UseBgCol() ) { @@ -489,7 +498,7 @@ void wxStaticBox::PaintForeground(wxDC& dc, const RECT&) } // now draw the text - RECT rc2 = { x, 0, x + width, y }; + RECT rc2 = { x, 0, x + width, height }; ::DrawText(hdc, label.t_str(), label.length(), &rc2, drawTextFlags); } From dfba063d53b0183196f1be7c49be18840ea475d8 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Wed, 20 Dec 2017 23:01:11 +0100 Subject: [PATCH 10/28] Adjust wxMSW wxStaticBox pixel constants for DPI Use FromDIP() with all constants expressed in pixels, this should result in better appearance when using high DPI. --- src/msw/statbox.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/msw/statbox.cpp b/src/msw/statbox.cpp index bcc79cd1f7..167de6c152 100644 --- a/src/msw/statbox.cpp +++ b/src/msw/statbox.cpp @@ -460,15 +460,15 @@ void wxStaticBox::PaintForeground(wxDC& dc, const RECT&) // first we need to correctly paint the background of the label // as Windows ignores the brush offset when doing it - const int x = LABEL_HORZ_OFFSET; + const int x = FromDIP(LABEL_HORZ_OFFSET); RECT dimensions = { x, 0, 0, height }; dimensions.left = x; dimensions.right = x + width; // need to adjust the rectangle to cover all the label background - dimensions.left -= LABEL_HORZ_BORDER; - dimensions.right += LABEL_HORZ_BORDER; - dimensions.bottom += LABEL_VERT_BORDER; + dimensions.left -= FromDIP(LABEL_HORZ_BORDER); + dimensions.right += FromDIP(LABEL_HORZ_BORDER); + dimensions.bottom += FromDIP(LABEL_VERT_BORDER); if ( UseBgCol() ) { From 900c6d5d75ae980c3ebf03c70bebcc319a44bfe2 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Wed, 20 Dec 2017 22:54:44 +0100 Subject: [PATCH 11/28] Slightly decrease the top margin of wxMSW wxStaticBox Use the same offset we already use in the drawing code, this is a bit less arbitrary than what we did before and looks slightly better too. --- src/msw/statbox.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/msw/statbox.cpp b/src/msw/statbox.cpp index 167de6c152..a178c4abcb 100644 --- a/src/msw/statbox.cpp +++ b/src/msw/statbox.cpp @@ -167,7 +167,7 @@ void wxStaticBox::GetBordersForSizer(int *borderTop, int *borderOther) const wxStaticBoxBase::GetBordersForSizer(borderTop, borderOther); // need extra space, don't know how much but this seems to be enough - *borderTop += GetCharHeight()/3; + *borderTop += FromDIP(LABEL_VERT_BORDER); } WXLRESULT wxStaticBox::MSWWindowProc(WXUINT nMsg, WXWPARAM wParam, WXLPARAM lParam) From 4a4d1643192d3d485ff073707ae941e4480132e3 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Wed, 20 Dec 2017 23:29:07 +0100 Subject: [PATCH 12/28] Keep attributes after recreating static controls in widgets sample Preserve the colours, font etc after recreating the widgets to facilitate testing. --- samples/widgets/static.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/samples/widgets/static.cpp b/samples/widgets/static.cpp index 8869bab39d..71b42b59b4 100644 --- a/samples/widgets/static.cpp +++ b/samples/widgets/static.cpp @@ -569,6 +569,8 @@ void StaticWidgetsPage::CreateStatic() staticBox->Connect(wxEVT_LEFT_UP, wxMouseEventHandler(StaticWidgetsPage::OnMouseEvent), NULL, this); + + SetUpWidget(); } // ---------------------------------------------------------------------------- From 598c62a26715e3e6ebdb1c9485b437fba649a40f Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Thu, 21 Dec 2017 13:46:32 +0100 Subject: [PATCH 13/28] Don't change wxCompositeWindow size from SetLayoutDirection() Remove wxSIZE_AUTO from the SetSize() call, this was completely unnecessary and unexpectedly (and wrongly) resized composite windows managed by sizers as SetLayoutDirection() side-effect. --- include/wx/compositewin.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/wx/compositewin.h b/include/wx/compositewin.h index b443eaceb5..48d0ae344e 100644 --- a/include/wx/compositewin.h +++ b/include/wx/compositewin.h @@ -109,7 +109,7 @@ public: // SetLayoutDirection(wxLayout_Default) wouldn't result in a re-layout // neither, but then we're not supposed to be called with it at all. if ( dir != wxLayout_Default ) - this->SetSize(-1, -1, -1, -1, wxSIZE_AUTO | wxSIZE_FORCE); + this->SetSize(-1, -1, -1, -1, wxSIZE_FORCE); } #if wxUSE_TOOLTIPS From 59ca9b93a0b36200e1bc15f39c85f78bb0ee7d38 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Thu, 21 Dec 2017 13:48:30 +0100 Subject: [PATCH 14/28] Make wxCompositeWindow ctor protected As this class is only supposed to be used as a base class, its ctor doesn't need to be, and hence ought not to be, public. Also update an outdated comment stating that the ctor didn't do anything when it, in fact, does perform an important task. --- include/wx/compositewin.h | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/include/wx/compositewin.h b/include/wx/compositewin.h index 48d0ae344e..ec26a414b5 100644 --- a/include/wx/compositewin.h +++ b/include/wx/compositewin.h @@ -33,17 +33,6 @@ class wxCompositeWindow : public W public: typedef W BaseWindowClass; - // Default ctor doesn't do anything. - wxCompositeWindow() - { - this->Connect - ( - wxEVT_CREATE, - wxWindowCreateEventHandler(wxCompositeWindow::OnWindowCreate) - ); - - } - // Override all wxWindow methods which must be forwarded to the composite // window parts. @@ -136,6 +125,17 @@ public: wxSetFocusToChild(this, NULL); } +protected: + // Default ctor sets things up for handling children events correctly. + wxCompositeWindow() + { + this->Connect + ( + wxEVT_CREATE, + wxWindowCreateEventHandler(wxCompositeWindow::OnWindowCreate) + ); + } + private: // Must be implemented by the derived class to return all children to which // the public methods we override should forward to. From 3ff9846a227b8eba066d12cedbc86d58cd61c192 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Thu, 21 Dec 2017 19:03:12 +0100 Subject: [PATCH 15/28] Document alignment styles in wxStaticBox Mention that they work in wxGTK, even if it seems to be the only platform supporting them currently. --- interface/wx/statbox.h | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/interface/wx/statbox.h b/interface/wx/statbox.h index 8627255027..d57949ee9b 100644 --- a/interface/wx/statbox.h +++ b/interface/wx/statbox.h @@ -71,7 +71,11 @@ public: Checkbox size. If ::wxDefaultSize is specified then a default size is chosen. @param style - Window style. See wxStaticBox. + Window style. There are no wxStaticBox-specific styles, but generic + ::wxALIGN_LEFT, ::wxALIGN_CENTRE_HORIZONTAL and ::wxALIGN_RIGHT can + be used here to change the position of the static box label when + using wxGTK (these styles are ignored under the other platforms + currently). @param name Window name. From 36c4b7651e83752745c1e10f6fe3d742b2ef9942 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 24 Dec 2017 19:54:41 +0100 Subject: [PATCH 16/28] Fix background of wxCheckBoxes inside wxStaticBox in wxMSW Erase background of the partially transparent native child controls, such as wxCheckBox, using our own background colour if we have it instead of using the parent's colour. For some reason, we -- seemingly intentionally, judging from the comment -- didn't do it before, but this meant that checkboxes inside static boxes didn't inherit the box background colour, if it was set, which was ugly and inconsistent with at least wxGTK. So do use our own background if we have it now by just reusing the existing PaintBackground() instead of manually using the parent background brush in WM_PRINTCLIENT handler. --- src/msw/statbox.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/msw/statbox.cpp b/src/msw/statbox.cpp index a178c4abcb..6596e1b573 100644 --- a/src/msw/statbox.cpp +++ b/src/msw/statbox.cpp @@ -204,9 +204,10 @@ WXLRESULT wxStaticBox::MSWWindowProc(WXUINT nMsg, WXWPARAM wParam, WXLPARAM lPar if ( !HandlePrintClient((WXHDC)wParam) ) { // no, we don't, erase the background ourselves - // (don't use our own) - see PaintBackground for explanation - wxBrush brush(GetParent()->GetBackgroundColour()); - wxFillRect(GetHwnd(), (HDC)wParam, GetHbrushOf(brush)); + RECT rc; + ::GetClientRect(GetHwnd(), &rc); + wxDCTemp dc((WXHDC)wParam); + PaintBackground(dc, rc); } return 0; From 687192d86a52b2779ff0c73a32403c1a0a257385 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 24 Dec 2017 20:07:05 +0100 Subject: [PATCH 17/28] Add support for arbitrary window labels in wxStaticBox to wxMSW Just reparent the label window and position it accordingly and, also, avoid painting over it in MSW-specific code. --- include/wx/msw/statbox.h | 20 ++++++++++++ interface/wx/statbox.h | 2 +- src/msw/statbox.cpp | 66 +++++++++++++++++++++++++++++++++++++--- 3 files changed, 83 insertions(+), 5 deletions(-) diff --git a/include/wx/msw/statbox.h b/include/wx/msw/statbox.h index 87ad173cba..89eb8fcdde 100644 --- a/include/wx/msw/statbox.h +++ b/include/wx/msw/statbox.h @@ -27,6 +27,16 @@ public: Create(parent, id, label, pos, size, style, name); } + wxStaticBox(wxWindow* parent, wxWindowID id, + wxWindow* label, + const wxPoint& pos = wxDefaultPosition, + const wxSize& size = wxDefaultSize, + long style = 0, + const wxString &name = wxStaticBoxNameStr) + { + Create(parent, id, label, pos, size, style, name); + } + bool Create(wxWindow *parent, wxWindowID id, const wxString& label, const wxPoint& pos = wxDefaultPosition, @@ -34,6 +44,13 @@ public: long style = 0, const wxString& name = wxStaticBoxNameStr); + bool Create(wxWindow *parent, wxWindowID id, + wxWindow* label, + const wxPoint& pos = wxDefaultPosition, + const wxSize& size = wxDefaultSize, + long style = 0, + const wxString& name = wxStaticBoxNameStr); + /// Implementation only virtual void GetBordersForSizer(int *borderTop, int *borderOther) const wxOVERRIDE; @@ -66,5 +83,8 @@ protected: wxDECLARE_DYNAMIC_CLASS_NO_COPY(wxStaticBox); }; +// Indicate that we have the ctor overload taking wxWindow as label. +#define wxHAS_WINDOW_LABEL_IN_STATIC_BOX + #endif // _WX_MSW_STATBOX_H_ diff --git a/interface/wx/statbox.h b/interface/wx/statbox.h index d57949ee9b..d7aae13cc0 100644 --- a/interface/wx/statbox.h +++ b/interface/wx/statbox.h @@ -111,7 +111,7 @@ public: } @endcode - Currently this constructor is only available in wxGTK, use + Currently this constructor is only available in wxGTK and wxMSW, use @c wxHAS_WINDOW_LABEL_IN_STATIC_BOX to check whether it can be used at compile-time. diff --git a/src/msw/statbox.cpp b/src/msw/statbox.cpp index 6596e1b573..9ca1b792bd 100644 --- a/src/msw/statbox.cpp +++ b/src/msw/statbox.cpp @@ -107,6 +107,27 @@ bool wxStaticBox::Create(wxWindow *parent, return true; } +bool wxStaticBox::Create(wxWindow* parent, + wxWindowID id, + wxWindow* labelWin, + const wxPoint& pos, + const wxSize& size, + long style, + const wxString& name) +{ + wxCHECK_MSG( labelWin, false, wxS("Label window can't be null") ); + + if ( !Create(parent, id, wxString(), pos, size, style, name) ) + return false; + + m_labelWin = labelWin; + m_labelWin->Reparent(this); + + m_labelWin->Move(FromDIP(LABEL_HORZ_OFFSET), 0); + + return true; +} + WXDWORD wxStaticBox::MSWGetStyle(long style, WXDWORD *exstyle) const { long styleWin = wxStaticBoxBase::MSWGetStyle(style, exstyle); @@ -269,7 +290,21 @@ void wxStaticBox::MSWGetRegionWithoutSelf(WXHRGN hRgn, int w, int h) GetBordersForSizer(&borderTop, &border); // top - SubtractRectFromRgn(hrgn, 0, 0, w, borderTop); + if ( m_labelWin ) + { + // Don't exclude the entire rectangle at the top, we do need to paint + // the background of the gap between the label window and the box + // frame. + const wxRect labelRect = m_labelWin->GetRect(); + const int gap = FromDIP(LABEL_HORZ_BORDER); + + SubtractRectFromRgn(hrgn, 0, 0, labelRect.GetLeft() - gap, borderTop); + SubtractRectFromRgn(hrgn, labelRect.GetRight() + gap, 0, w, borderTop); + } + else + { + SubtractRectFromRgn(hrgn, 0, 0, w, borderTop); + } // bottom SubtractRectFromRgn(hrgn, 0, h - border, w, h); @@ -415,7 +450,7 @@ void wxStaticBox::PaintForeground(wxDC& dc, const RECT&) // background mode doesn't change anything: the static box def window proc // still draws the label in its own colours, so we need to redraw the text // ourselves if we have a non default fg colour - if ( m_hasFgCol && wxUxThemeEngine::GetIfActive() ) + if ( m_hasFgCol && wxUxThemeEngine::GetIfActive() && !m_labelWin ) { // draw over the text in default colour in our colour HDC hdc = GetHdcOf(*impl); @@ -535,8 +570,31 @@ void wxStaticBox::OnPaint(wxPaintEvent& WXUNUSED(event)) GetBordersForSizer(&borderTop, &border); // top - dc.Blit(border, 0, rc.right - border, borderTop, - &memdc, border, 0); + if ( m_labelWin ) + { + // We also have to exclude the area taken by the label window, + // otherwise there would be flicker when it draws itself on top of it. + const wxRect labelRect = m_labelWin->GetRect(); + + // We also leave a small border around label window to make it appear + // more similarly to a plain text label. + const int gap = FromDIP(LABEL_HORZ_BORDER); + + dc.Blit(border, 0, + labelRect.GetLeft() - gap - border, + borderTop, + &memdc, border, 0); + dc.Blit(labelRect.GetRight() + gap, 0, + rc.right - (labelRect.GetRight() + gap), + borderTop, + &memdc, border, 0); + } + else + { + dc.Blit(border, 0, rc.right - border, borderTop, + &memdc, border, 0); + } + // bottom dc.Blit(border, rc.bottom - border, rc.right - border, border, &memdc, border, rc.bottom - border); From 329af399ebfeeb3c92c78d8ec3a9e876ed4c01f1 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 24 Dec 2017 21:43:57 +0100 Subject: [PATCH 18/28] Factor out wxCompositeWindowSettersOnly class Extract this class from wxCompositeWindow, as sometimes it can be convenient to just define the setter functions to do the right thing for a window containing sub-windows, but without dealing with focus too. This will be used in wxMSW wxStaticBox in the next commit. --- include/wx/compositewin.h | 68 +++++++++++++++++++++++++-------------- 1 file changed, 44 insertions(+), 24 deletions(-) diff --git a/include/wx/compositewin.h b/include/wx/compositewin.h index ec26a414b5..567d0e8999 100644 --- a/include/wx/compositewin.h +++ b/include/wx/compositewin.h @@ -26,9 +26,14 @@ class WXDLLIMPEXP_FWD_CORE wxToolTip; // base class name and implement GetCompositeWindowParts() pure virtual method. // ---------------------------------------------------------------------------- +// This is the base class of wxCompositeWindow which takes care of propagating +// colours, fonts etc changes to all the children, but doesn't bother with +// handling their events or focus. There should be rarely any need to use it +// rather than the full wxCompositeWindow. + // The template parameter W must be a wxWindow-derived class. template -class wxCompositeWindow : public W +class wxCompositeWindowSettersOnly : public W { public: typedef W BaseWindowClass; @@ -120,6 +125,44 @@ public: } #endif // wxUSE_TOOLTIPS +protected: + // Trivial but necessary default ctor. + wxCompositeWindowSettersOnly() + { + } + +private: + // Must be implemented by the derived class to return all children to which + // the public methods we override should forward to. + virtual wxWindowList GetCompositeWindowParts() const = 0; + + template + void SetForAllParts(R (wxWindowBase::*func)(TArg), T arg) + { + // Simply call the setters for all parts of this composite window. + const wxWindowList parts = GetCompositeWindowParts(); + for ( wxWindowList::const_iterator i = parts.begin(); + i != parts.end(); + ++i ) + { + wxWindow * const child = *i; + + // Allow NULL elements in the list, this makes the code of derived + // composite controls which may have optionally shown children + // simpler and it doesn't cost us much here. + if ( child ) + (child->*func)(arg); + } + } + + wxDECLARE_NO_COPY_TEMPLATE_CLASS(wxCompositeWindowSettersOnly, W); +}; + +// The real wxCompositeWindow itself, inheriting all the setters defined above. +template +class wxCompositeWindow : public wxCompositeWindowSettersOnly +{ +public: virtual void SetFocus() wxOVERRIDE { wxSetFocusToChild(this, NULL); @@ -137,10 +180,6 @@ protected: } private: - // Must be implemented by the derived class to return all children to which - // the public methods we override should forward to. - virtual wxWindowList GetCompositeWindowParts() const = 0; - void OnWindowCreate(wxWindowCreateEvent& event) { event.Skip(); @@ -206,25 +245,6 @@ private: event.Skip(); } - template - void SetForAllParts(R (wxWindowBase::*func)(TArg), T arg) - { - // Simply call the setters for all parts of this composite window. - const wxWindowList parts = GetCompositeWindowParts(); - for ( wxWindowList::const_iterator i = parts.begin(); - i != parts.end(); - ++i ) - { - wxWindow * const child = *i; - - // Allow NULL elements in the list, this makes the code of derived - // composite controls which may have optionally shown children - // simpler and it doesn't cost us much here. - if ( child ) - (child->*func)(arg); - } - } - wxDECLARE_NO_COPY_TEMPLATE_CLASS(wxCompositeWindow, W); }; From fdf47e8e1279f6becf013e1025c091a54caac894 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 24 Dec 2017 21:48:51 +0100 Subject: [PATCH 19/28] Fix colours and fonts of wxStaticBox label window in wxMSW Inherit from wxCompositeWindowSettersOnly<> to make sure all the usual setters, such as SetForegroundColour() and SetFont(), called on wxStaticBox are propagated to the label window too. However also prevent SetBackgroundColour() from being propagated unnecessarily -- because the checkbox already inherits the parent background colour by default in wxMSW anyhow -- and still override SetFont() to adjust the label window position after the font change, otherwise it could be truncated after increasing the font size, for example. Because of these issues, wxCompositeWindowSettersOnly is not ideally suited for its use here, but on balance it still seems to be better to use it rather than reimplement parts of its functionality here. --- include/wx/msw/statbox.h | 19 +++++++++++++++++-- src/msw/statbox.cpp | 40 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 56 insertions(+), 3 deletions(-) diff --git a/include/wx/msw/statbox.h b/include/wx/msw/statbox.h index 89eb8fcdde..294f1519ae 100644 --- a/include/wx/msw/statbox.h +++ b/include/wx/msw/statbox.h @@ -11,11 +11,16 @@ #ifndef _WX_MSW_STATBOX_H_ #define _WX_MSW_STATBOX_H_ +#include "wx/compositewin.h" + // Group box -class WXDLLIMPEXP_CORE wxStaticBox : public wxStaticBoxBase +class WXDLLIMPEXP_CORE wxStaticBox : public wxCompositeWindowSettersOnly { public: - wxStaticBox() { } + wxStaticBox() + : wxCompositeWindowSettersOnly() + { + } wxStaticBox(wxWindow *parent, wxWindowID id, const wxString& label, @@ -23,6 +28,7 @@ public: const wxSize& size = wxDefaultSize, long style = 0, const wxString& name = wxStaticBoxNameStr) + : wxCompositeWindowSettersOnly() { Create(parent, id, label, pos, size, style, name); } @@ -33,6 +39,7 @@ public: const wxSize& size = wxDefaultSize, long style = 0, const wxString &name = wxStaticBoxNameStr) + : wxCompositeWindowSettersOnly() { Create(parent, id, label, pos, size, style, name); } @@ -54,6 +61,9 @@ public: /// Implementation only virtual void GetBordersForSizer(int *borderTop, int *borderOther) const wxOVERRIDE; + virtual bool SetBackgroundColour(const wxColour& colour) wxOVERRIDE; + virtual bool SetFont(const wxFont& font) wxOVERRIDE; + virtual WXDWORD MSWGetStyle(long style, WXDWORD *exstyle) const wxOVERRIDE; // returns true if the platform should explicitly apply a theme border @@ -66,6 +76,8 @@ public: virtual WXLRESULT MSWWindowProc(WXUINT nMsg, WXWPARAM wParam, WXLPARAM lParam) wxOVERRIDE; protected: + virtual wxWindowList GetCompositeWindowParts() const wxOVERRIDE; + // return the region with all the windows inside this static box excluded virtual WXHRGN MSWGetRegionWithoutChildren(); @@ -80,6 +92,9 @@ protected: void OnPaint(wxPaintEvent& event); +private: + void PositionLabelWindow(); + wxDECLARE_DYNAMIC_CLASS_NO_COPY(wxStaticBox); }; diff --git a/src/msw/statbox.cpp b/src/msw/statbox.cpp index 9ca1b792bd..97a48d7bb6 100644 --- a/src/msw/statbox.cpp +++ b/src/msw/statbox.cpp @@ -123,11 +123,25 @@ bool wxStaticBox::Create(wxWindow* parent, m_labelWin = labelWin; m_labelWin->Reparent(this); - m_labelWin->Move(FromDIP(LABEL_HORZ_OFFSET), 0); + PositionLabelWindow(); return true; } +void wxStaticBox::PositionLabelWindow() +{ + m_labelWin->SetSize(m_labelWin->GetBestSize()); + m_labelWin->Move(FromDIP(LABEL_HORZ_OFFSET), 0); +} + +wxWindowList wxStaticBox::GetCompositeWindowParts() const +{ + wxWindowList parts; + if ( m_labelWin ) + parts.push_back(m_labelWin); + return parts; +} + WXDWORD wxStaticBox::MSWGetStyle(long style, WXDWORD *exstyle) const { long styleWin = wxStaticBoxBase::MSWGetStyle(style, exstyle); @@ -191,6 +205,30 @@ void wxStaticBox::GetBordersForSizer(int *borderTop, int *borderOther) const *borderTop += FromDIP(LABEL_VERT_BORDER); } +bool wxStaticBox::SetBackgroundColour(const wxColour& colour) +{ + // Do _not_ call the immediate base class method, we don't need to set the + // label window (which is the only sub-window of this composite window) + // background explicitly because it will almost always be a wxCheckBox or + // wxRadioButton which inherits its background from the box anyhow, so + // setting it would be at best useless. + return wxStaticBoxBase::SetBackgroundColour(colour); +} + +bool wxStaticBox::SetFont(const wxFont& font) +{ + if ( !wxCompositeWindowSettersOnly::SetFont(font) ) + return false; + + // We need to reposition the label as its size may depend on the font. + if ( m_labelWin ) + { + PositionLabelWindow(); + } + + return true; +} + WXLRESULT wxStaticBox::MSWWindowProc(WXUINT nMsg, WXWPARAM wParam, WXLPARAM lParam) { if ( nMsg == WM_NCHITTEST ) From d332ccfd6f8fe82d439a35dc22b5c9f8e6efd421 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Thu, 4 Jan 2018 22:58:50 +0100 Subject: [PATCH 20/28] Add support for wxStaticBoxSizer "windowlabel" property to XRC Allow specifying arbitrary windows as labels for the static boxes created from XRC. Note that wxStaticBox XRC handler itself wasn't updated to support this, as this handler seems to be quite useless because it's impossible to define any box children with it, so only wxStaticBoxSizer XRC handler really matters, anyhow. --- docs/doxygen/overviews/xrc_format.h | 3 ++ samples/xrc/rc/controls.xrc | 52 +++++++++++++++++++++++ src/xrc/xh_sizer.cpp | 65 +++++++++++++++++++++++++---- 3 files changed, 112 insertions(+), 8 deletions(-) diff --git a/docs/doxygen/overviews/xrc_format.h b/docs/doxygen/overviews/xrc_format.h index 187d8677d1..66dada62a6 100644 --- a/docs/doxygen/overviews/xrc_format.h +++ b/docs/doxygen/overviews/xrc_format.h @@ -2416,6 +2416,9 @@ support the following properties: Sizer orientation, "wxHORIZONTAL" or "wxVERTICAL" (default: wxHORIZONTAL).} @row3col{label, @ref overview_xrcformat_type_text, Label to be used for the static box around the sizer (default: empty).} +@row3col{windowlabel, any window, + Window to be used instead of the plain text label (default: none). + This property is only available since wxWidgets 3.1.1.}} @endTable @subsection overview_xrcformat_wxgridsizer wxGridSizer diff --git a/samples/xrc/rc/controls.xrc b/samples/xrc/rc/controls.xrc index bac4812e5e..8072505d22 100644 --- a/samples/xrc/rc/controls.xrc +++ b/samples/xrc/rc/controls.xrc @@ -983,6 +983,58 @@ lay them out using wxSizers, absolute positioning, everything you like! wxVERTICAL + + wxALL + 5 + + + + + + + wxALL + 5 + + + + + + wxALL + 5 + + + 1 + + + + + + wxGROW|wxALL + 5 + + wxVERTICAL + + + + 1 + + + + wxALL + 5 + + + + + + + wxALL + 5 + + + 1 + + diff --git a/src/xrc/xh_sizer.cpp b/src/xrc/xh_sizer.cpp index 8a91eb91e9..2a17184a55 100644 --- a/src/xrc/xh_sizer.cpp +++ b/src/xrc/xh_sizer.cpp @@ -320,14 +320,63 @@ wxSizer* wxSizerXmlHandler::Handle_wxBoxSizer() #if wxUSE_STATBOX wxSizer* wxSizerXmlHandler::Handle_wxStaticBoxSizer() { - return new wxStaticBoxSizer( - new wxStaticBox(m_parentAsWindow, - GetID(), - GetText(wxT("label")), - wxDefaultPosition, wxDefaultSize, - 0/*style*/, - GetName()), - GetStyle(wxT("orient"), wxHORIZONTAL)); + wxXmlNode* nodeWindowLabel = GetParamNode(wxS("windowlabel")); + wxString const& labelText = GetText(wxS("label")); + + wxStaticBox* box = NULL; + if ( nodeWindowLabel ) + { + if ( !labelText.empty() ) + { + ReportError("Either label or windowlabel can be used, but not both"); + return NULL; + } + +#ifdef wxHAS_WINDOW_LABEL_IN_STATIC_BOX + wxXmlNode* n = nodeWindowLabel->GetChildren(); + if ( !n ) + { + ReportError("windowlabel must have a window child"); + return NULL; + } + + if ( n->GetNext() ) + { + ReportError("windowlabel can only have a single child"); + return NULL; + } + + wxObject* const item = CreateResFromNode(n, m_parent, NULL); + wxWindow* const wndLabel = wxDynamicCast(item, wxWindow); + if ( !wndLabel ) + { + ReportError(n, "windowlabel child must be a window"); + return NULL; + } + + box = new wxStaticBox(m_parentAsWindow, + GetID(), + wndLabel, + wxDefaultPosition, wxDefaultSize, + 0/*style*/, + GetName()); +#else // !wxHAS_WINDOW_LABEL_IN_STATIC_BOX + ReportError("Support for using windows as wxStaticBox labels is " + "missing in this build of wxWidgets."); + return NULL; +#endif // wxHAS_WINDOW_LABEL_IN_STATIC_BOX/!wxHAS_WINDOW_LABEL_IN_STATIC_BOX + } + else // Using plain text label. + { + box = new wxStaticBox(m_parentAsWindow, + GetID(), + labelText, + wxDefaultPosition, wxDefaultSize, + 0/*style*/, + GetName()); + } + + return new wxStaticBoxSizer(box, GetStyle(wxS("orient"), wxHORIZONTAL)); } #endif // wxUSE_STATBOX From 1d037dd4c9d0096f1eb9493f88de4e239e3275fd Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 7 Jan 2018 01:14:17 +0100 Subject: [PATCH 21/28] Don't disable wxStaticBox window label when disabling the box This behaviour might be not completely intuitive, but it makes it much simpler to handle the box state using a checkbox as the label control (which is by far the most common case of using box window labels). Notice that while we could add a separate EnableWithoutLabel() method to wxStaticBox to make it possible to set the state of the box directly relatively easily, it wouldn't help with using wxEVT_UPDATE_UI for managing the box state indirectly as it relies on calling Enable() only. And this solution does allow wxEVT_UPDATE_UI handlers for the box itself to work (provided the handler takes care to check for the event object being the box itself, as otherwise it would still disable the child checkbox when its wxEVT_UPDATE_UI bubbles up to the box). --- include/wx/statbox.h | 1 + interface/wx/statbox.h | 26 ++++++++++++++++++++++++++ src/common/statboxcmn.cpp | 39 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 66 insertions(+) diff --git a/include/wx/statbox.h b/include/wx/statbox.h index 0e6e4f3ac3..ecb07ed92f 100644 --- a/include/wx/statbox.h +++ b/include/wx/statbox.h @@ -31,6 +31,7 @@ public: // overridden base class virtuals virtual bool HasTransparentBackground() wxOVERRIDE { return true; } + virtual bool Enable(bool enable = true) wxOVERRIDE; // implementation only: this is used by wxStaticBoxSizer to account for the // need for extra space taken by the static box diff --git a/interface/wx/statbox.h b/interface/wx/statbox.h index d7aae13cc0..c0fcac378e 100644 --- a/interface/wx/statbox.h +++ b/interface/wx/statbox.h @@ -158,4 +158,30 @@ public: const wxSize& size = wxDefaultSize, long style = 0, const wxString& name = wxStaticBoxNameStr); + + /** + Enables or disables the box without affecting its label window, if any. + + wxStaticBox overrides wxWindow::Enable() in order to avoid disabling + the control used as a label, if this box is using one. This is done in + order to allow using a wxCheckBox, for example, label and enable or + disable the box according to the state of the checkbox: if disabling + the box also disabled the checkbox in this situation, it would make it + impossible for the user to re-enable the box after disabling it, so the + checkbox stays enabled even if @c box->Enable(false) is called. + + However with the actual behaviour, implemented in this overridden + method, the following code (shown using C++11 only for convenience, + this behaviour is not C++11-specific): + @code + auto check = new wxCheckBox(parent, wxID_ANY, "Use the box"); + auto box = new wxStaticBox(parent, wxID_ANY, check); + check->Bind(wxEVT_CHECKBOX, + [box](wxCommandEvent& event) { + box->Enable(event.IsChecked()); + }); + @endcode + does work as expected. + */ + virtual bool Enable(bool enable = true); }; diff --git a/src/common/statboxcmn.cpp b/src/common/statboxcmn.cpp index 4db934146a..0e77dfe74f 100644 --- a/src/common/statboxcmn.cpp +++ b/src/common/statboxcmn.cpp @@ -78,6 +78,45 @@ void wxStaticBoxBase::WXDestroyWithoutChildren() delete this; } +bool wxStaticBoxBase::Enable(bool enable) +{ +#ifdef wxHAS_WINDOW_LABEL_IN_STATIC_BOX + // We want to keep the window label enabled even if the static box is + // disabled because this label is often used to enable/disable the box + // (e.g. a checkbox or a radio button is commonly used for this purpose) + // and it would be impossible to re-enable the box back if disabling it + // also disabled the label control. + // + // Unfortunately it is _not_ enough to just disable the box and then enable + // the label window as it would still remain disabled under some platforms + // (those where wxHAS_NATIVE_ENABLED_MANAGEMENT is defined, e.g. wxGTK) for + // as long as its parent is disabled. So we avoid disabling the box at all + // in this case and only disable its children, but still pretend that the + // box is disabled by updating its m_isEnabled, as it would be surprising + // if IsEnabled() didn't return false after disabling the box, for example. + if ( m_labelWin ) + { + if ( enable == IsThisEnabled() ) + return false; + + const wxWindowList& children = GetChildren(); + for ( wxWindowList::const_iterator i = children.begin(); + i != children.end(); + ++i ) + { + if ( *i != m_labelWin ) + (*i)->Enable(enable); + } + + m_isEnabled = enable; + + return true; + } +#endif // wxHAS_WINDOW_LABEL_IN_STATIC_BOX + + return wxNavigationEnabled::Enable(enable); +} + // ---------------------------------------------------------------------------- // XTI // ---------------------------------------------------------------------------- From 564df77283e4f88f4e85c6592dcebd02b44285ac Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 13 Jan 2018 22:26:07 +0100 Subject: [PATCH 22/28] Correct new wxStaticBox::Create() overload documentation Fix the method name and update its description to mention that it's also available in wxMSW. --- interface/wx/statbox.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/interface/wx/statbox.h b/interface/wx/statbox.h index c0fcac378e..f9174261d7 100644 --- a/interface/wx/statbox.h +++ b/interface/wx/statbox.h @@ -146,13 +146,13 @@ public: See the constructor documentation for more details. - Currently this overload is only available in wxGTK, use + Currently this overload is only available in wxGTK and wxMSW, use @c wxHAS_WINDOW_LABEL_IN_STATIC_BOX to check whether it can be used at compile-time. @since 3.1.1 */ - wxStaticBox(wxWindow* parent, wxWindowID id, + bool Create(wxWindow* parent, wxWindowID id, wxWindow* label, const wxPoint& pos = wxDefaultPosition, const wxSize& size = wxDefaultSize, From 2163b00b935a10804da08e8bd0f85d327f162343 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 13 Jan 2018 22:44:44 +0100 Subject: [PATCH 23/28] Move wxHAS_NATIVE_ENABLED_MANAGEMENT definition to the source file There doesn't seem to be any need to have this symbol in the header, when it's only used in NotifyWindowOnEnableChange() in wincmn.cpp. No real changes. --- include/wx/window.h | 16 ---------------- src/common/wincmn.cpp | 15 +++++++++++++++ 2 files changed, 15 insertions(+), 16 deletions(-) diff --git a/include/wx/window.h b/include/wx/window.h index 38d70ab7a8..8046223a4a 100644 --- a/include/wx/window.h +++ b/include/wx/window.h @@ -48,22 +48,6 @@ #define wxUSE_MENUS_NATIVE wxUSE_MENUS #endif // __WXUNIVERSAL__/!__WXUNIVERSAL__ - -// Define this macro if the corresponding operating system handles the state -// of children windows automatically when the parent is enabled/disabled. -// Otherwise wx itself must ensure that when the parent is disabled its -// children are disabled too, and their initial state is restored when the -// parent is enabled back. -#if defined(__WXMSW__) - // must do everything ourselves - #undef wxHAS_NATIVE_ENABLED_MANAGEMENT -#elif defined(__WXOSX__) - // must do everything ourselves - #undef wxHAS_NATIVE_ENABLED_MANAGEMENT -#else - #define wxHAS_NATIVE_ENABLED_MANAGEMENT -#endif - // ---------------------------------------------------------------------------- // forward declarations // ---------------------------------------------------------------------------- diff --git a/src/common/wincmn.cpp b/src/common/wincmn.cpp index f07acd465d..d398f1f7b0 100644 --- a/src/common/wincmn.cpp +++ b/src/common/wincmn.cpp @@ -1166,6 +1166,21 @@ bool wxWindowBase::IsEnabled() const return IsThisEnabled() && (IsTopLevel() || !GetParent() || GetParent()->IsEnabled()); } +// Define this macro if the corresponding operating system handles the state +// of children windows automatically when the parent is enabled/disabled. +// Otherwise wx itself must ensure that when the parent is disabled its +// children are disabled too, and their initial state is restored when the +// parent is enabled back. +#if defined(__WXMSW__) + // must do everything ourselves + #undef wxHAS_NATIVE_ENABLED_MANAGEMENT +#elif defined(__WXOSX__) + // must do everything ourselves + #undef wxHAS_NATIVE_ENABLED_MANAGEMENT +#else + #define wxHAS_NATIVE_ENABLED_MANAGEMENT +#endif + void wxWindowBase::NotifyWindowOnEnableChange(bool enabled) { // Under some platforms there is no need to update the window state From 967b4cdc1f848576be3b83fb07d59c9b670c3994 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 13 Jan 2018 22:45:39 +0100 Subject: [PATCH 24/28] Always call DoEnable() from NotifyWindowOnEnableChange() Simplify the code by replacing 2 conditionally-compiled DoEnable() calls with a single unconditional one. This doesn't change the behaviour of Enable(), as it always called DoEnable() on the window itself and only did it for its children when wxHAS_NATIVE_ENABLED_MANAGEMENT was not defined before and continues to do the same thing now, but it should fix a small bug in Reparent() as it didn't update the actual status of the window if it changed as the result of reparenting before, even though it was supposed to. --- src/common/wincmn.cpp | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/src/common/wincmn.cpp b/src/common/wincmn.cpp index d398f1f7b0..3daae78c26 100644 --- a/src/common/wincmn.cpp +++ b/src/common/wincmn.cpp @@ -1183,13 +1183,9 @@ bool wxWindowBase::IsEnabled() const void wxWindowBase::NotifyWindowOnEnableChange(bool enabled) { - // Under some platforms there is no need to update the window state - // explicitly, it will become disabled when its parent is. On other ones we - // do need to disable all windows recursively though. -#ifndef wxHAS_NATIVE_ENABLED_MANAGEMENT DoEnable(enabled); -#endif // !defined(wxHAS_NATIVE_ENABLED_MANAGEMENT) +#ifndef wxHAS_NATIVE_ENABLED_MANAGEMENT // Disabling a top level window is typically done when showing a modal // dialog and we don't need to disable its children in this case, they will // be logically disabled anyhow (i.e. their IsEnabled() will return false) @@ -1205,7 +1201,6 @@ void wxWindowBase::NotifyWindowOnEnableChange(bool enabled) // they would still show as enabled even though they wouldn't actually // accept any input (at least under MSW where children don't accept input // if any of the windows in their parent chain is enabled). -#ifndef wxHAS_NATIVE_ENABLED_MANAGEMENT for ( wxWindowList::compatibility_iterator node = GetChildren().GetFirst(); node; node = node->GetNext() ) @@ -1224,12 +1219,6 @@ bool wxWindowBase::Enable(bool enable) m_isEnabled = enable; - // If we call DoEnable() from NotifyWindowOnEnableChange(), we don't need - // to do it from here. -#ifdef wxHAS_NATIVE_ENABLED_MANAGEMENT - DoEnable(enable); -#endif // !defined(wxHAS_NATIVE_ENABLED_MANAGEMENT) - NotifyWindowOnEnableChange(enable); return true; From 3c29b3d0ce20df8246a32e8eaa3ed612c4fd720e Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 13 Jan 2018 22:50:26 +0100 Subject: [PATCH 25/28] Don't disable wxStaticBox children at wx level when disabling it Calling Enable() on all children from wxStaticBox::Enable() was wrong, the actual status of the child, returned by wxWindow::IsThisEnabled(), is not supposed to change just because its parent was disabled. Call NotifyWindowOnEnableChange() to avoid this, while still disabling the children visually. --- include/wx/window.h | 10 +++++----- src/common/statboxcmn.cpp | 6 +++++- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/include/wx/window.h b/include/wx/window.h index 8046223a4a..b8d9e8d1a3 100644 --- a/include/wx/window.h +++ b/include/wx/window.h @@ -1580,6 +1580,11 @@ public: return false; } + // Recursively call our own and our children DoEnable() when the + // enabled/disabled status changed because a parent window had been + // enabled/disabled. This is only used by the library implementation. + void NotifyWindowOnEnableChange(bool enabled); + protected: // helper for the derived class Create() methods: the first overload, with @@ -1891,11 +1896,6 @@ protected: static void NotifyCaptureLost(); private: - // recursively call our own and our children DoEnable() when the - // enabled/disabled status changed because a parent window had been - // enabled/disabled - void NotifyWindowOnEnableChange(bool enabled); - #if wxUSE_MENUS // temporary event handlers used by GetPopupMenuSelectionFromUser() void InternalOnPopupMenu(wxCommandEvent& event); diff --git a/src/common/statboxcmn.cpp b/src/common/statboxcmn.cpp index 0e77dfe74f..8620d4efd0 100644 --- a/src/common/statboxcmn.cpp +++ b/src/common/statboxcmn.cpp @@ -105,11 +105,15 @@ bool wxStaticBoxBase::Enable(bool enable) ++i ) { if ( *i != m_labelWin ) - (*i)->Enable(enable); + (*i)->NotifyWindowOnEnableChange(enable); } m_isEnabled = enable; + // Notice that we don't call DoEnable() on the box itself: under MSW it + // doesn't actually change anything and under GTK this would disable + // the label window. + return true; } #endif // wxHAS_WINDOW_LABEL_IN_STATIC_BOX From 88c6ce344e9b4adaef2ab7ec163ecfc051865dcf Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 13 Jan 2018 23:23:54 +0100 Subject: [PATCH 26/28] Fix bug with not re-enabling wxStaticBox with label sometimes In some specific scenario, described in the newly added comment in wxStaticBoxBase::Enable(), the box and its label could remain disabled after its parent was disabled and re-enabled. Fix this by continuing to use the derived class version for disabling the box children, but not when enabling them, as the base class version already does the right thing in this case. --- src/common/statboxcmn.cpp | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/src/common/statboxcmn.cpp b/src/common/statboxcmn.cpp index 8620d4efd0..50f8a932e6 100644 --- a/src/common/statboxcmn.cpp +++ b/src/common/statboxcmn.cpp @@ -94,7 +94,26 @@ bool wxStaticBoxBase::Enable(bool enable) // in this case and only disable its children, but still pretend that the // box is disabled by updating its m_isEnabled, as it would be surprising // if IsEnabled() didn't return false after disabling the box, for example. - if ( m_labelWin ) + // + // Finally note that this really needs to be done only when disabling the + // box and not when re-enabling it, at least for the platforms without + // native enabled state management (e.g. wxMSW) because otherwise we could + // have a bug in the following scenario: + // + // 0. The box is initially enabled + // 1. Its parent gets disabled, calling DoEnable(false) on the box and all + // its children from its NotifyWindowOnEnableChange() (including the + // label). + // 2. The box is itself disabled (for whatever app logic reason). + // 3. The parent gets enabled but this time doesn't do anything with the + // box, because it should continue being disabled. + // 4. The box is re-enabled -- but remains actually disabled as + // DoEnable(true) was never called on it (nor on the label). + // + // To avoid this possibility, we always call the base class version, which + // does call DoEnable(true) on the box itself and all its children, + // including the label, when re-enabling it. + if ( m_labelWin && !enable ) { if ( enable == IsThisEnabled() ) return false; @@ -110,10 +129,6 @@ bool wxStaticBoxBase::Enable(bool enable) m_isEnabled = enable; - // Notice that we don't call DoEnable() on the box itself: under MSW it - // doesn't actually change anything and under GTK this would disable - // the label window. - return true; } #endif // wxHAS_WINDOW_LABEL_IN_STATIC_BOX From a67f4b8254dd371e087f132e674e52e17fc8a72c Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Thu, 18 Jan 2018 23:14:36 +0100 Subject: [PATCH 27/28] Don't pretend static box with enabled label is disabled Trying to be smart by setting m_isEnabled to false in wxStaticBox::Enable() without actually disabling the box itself (because it can't be done if its label window is to remain enabled) didn't really work. For example, it was impossible to TAB to a checkbox label of the box when it was disabled, because keyboard navigation (correctly) doesn't recurse into disabled windows and there could be similar problems with any other code iterating over windows and skipping over the disabled ones. So, finally, simplify things and keep m_isEnabled in sync with the real box state, even if this, counter-intuitively, means that IsEnabled() on the box returns true after calling Enable(false) on it. This also reverts 4ee35cf5ee569b6ee6c7d0d5702484d4d2a20f96 ("Don't disable wxStaticBox children at wx level when disabling it") as we can't avoid really disabling the children any more now that their parent is not disabled: without this, their IsEnabled() would return true, i.e. they wouldn't be disabled at all, from the program point of view. This is unfortunate for the reasons that originally motivated that commit, i.e. if some wxStaticBox child is disabled, disabling and re-enabling the box will now re-enable this child, even if it shouldn't, but seems impossible to avoid. The only possible alternative is to modify IsEnabled() to add some wxStaticBox-specific hook to it, e.g. instead of calling GetParent()->IsEnabled() there, we could call some now AreChildrenEnable() method, which would delegate to IsEnabled() by default but overridden in wxStaticBox. However this seems complicated, and will add an extra virtual function call to all (frequently happening) IsEnabled() calls. --- include/wx/statbox.h | 7 +++++++ include/wx/window.h | 10 +++++----- interface/wx/statbox.h | 15 +++++++++++++++ src/common/statboxcmn.cpp | 39 +++++++++------------------------------ 4 files changed, 36 insertions(+), 35 deletions(-) diff --git a/include/wx/statbox.h b/include/wx/statbox.h index ecb07ed92f..d5d15a9f00 100644 --- a/include/wx/statbox.h +++ b/include/wx/statbox.h @@ -54,6 +54,13 @@ protected: // static box and will be deleted when it is. wxWindow* m_labelWin; + // For boxes with window label this member variable is used instead of + // m_isEnabled to remember the last value passed to Enable(). It is + // required because the box itself doesn't get disabled by Enable(false) in + // this case (see comments in Enable() implementation), and m_isEnabled + // must correspond to its real state. + bool m_areChildrenEnabled; + wxDECLARE_NO_COPY_CLASS(wxStaticBoxBase); }; diff --git a/include/wx/window.h b/include/wx/window.h index b8d9e8d1a3..8046223a4a 100644 --- a/include/wx/window.h +++ b/include/wx/window.h @@ -1580,11 +1580,6 @@ public: return false; } - // Recursively call our own and our children DoEnable() when the - // enabled/disabled status changed because a parent window had been - // enabled/disabled. This is only used by the library implementation. - void NotifyWindowOnEnableChange(bool enabled); - protected: // helper for the derived class Create() methods: the first overload, with @@ -1896,6 +1891,11 @@ protected: static void NotifyCaptureLost(); private: + // recursively call our own and our children DoEnable() when the + // enabled/disabled status changed because a parent window had been + // enabled/disabled + void NotifyWindowOnEnableChange(bool enabled); + #if wxUSE_MENUS // temporary event handlers used by GetPopupMenuSelectionFromUser() void InternalOnPopupMenu(wxCommandEvent& event); diff --git a/interface/wx/statbox.h b/interface/wx/statbox.h index f9174261d7..d9f2a564de 100644 --- a/interface/wx/statbox.h +++ b/interface/wx/statbox.h @@ -182,6 +182,21 @@ public: }); @endcode does work as expected. + + Please note that overriding Enable() to not actually disable this + window itself has two possibly unexpected consequences: + + - The box retains its enabled status, i.e. IsEnabled() still returns + @true, after calling @c Enable(false). + - The box children are enabled or disabled when the box is, which can + result in the loss of their original state. E.g. if a box child is + initially disabled, then the box itself is disabled and, finally, the + box is enabled again, this child will end up being enabled too (this + wouldn't happen with any other parent window as its children would + inherit the disabled state from the parent instead of being really + disabled themselves when it is disabled). To avoid this problem, + consider using ::wxEVT_UPDATE_UI to ensure that the child state is + always correct or restoring it manually after re-enabling the box. */ virtual bool Enable(bool enable = true); }; diff --git a/src/common/statboxcmn.cpp b/src/common/statboxcmn.cpp index 50f8a932e6..f8f489363b 100644 --- a/src/common/statboxcmn.cpp +++ b/src/common/statboxcmn.cpp @@ -32,6 +32,7 @@ extern WXDLLEXPORT_DATA(const char) wxStaticBoxNameStr[] = "groupBox"; wxStaticBoxBase::wxStaticBoxBase() { m_labelWin = NULL; + m_areChildrenEnabled = true; #ifndef __WXGTK__ m_container.DisableSelfFocus(); @@ -88,47 +89,25 @@ bool wxStaticBoxBase::Enable(bool enable) // also disabled the label control. // // Unfortunately it is _not_ enough to just disable the box and then enable - // the label window as it would still remain disabled under some platforms - // (those where wxHAS_NATIVE_ENABLED_MANAGEMENT is defined, e.g. wxGTK) for - // as long as its parent is disabled. So we avoid disabling the box at all - // in this case and only disable its children, but still pretend that the - // box is disabled by updating its m_isEnabled, as it would be surprising - // if IsEnabled() didn't return false after disabling the box, for example. - // - // Finally note that this really needs to be done only when disabling the - // box and not when re-enabling it, at least for the platforms without - // native enabled state management (e.g. wxMSW) because otherwise we could - // have a bug in the following scenario: - // - // 0. The box is initially enabled - // 1. Its parent gets disabled, calling DoEnable(false) on the box and all - // its children from its NotifyWindowOnEnableChange() (including the - // label). - // 2. The box is itself disabled (for whatever app logic reason). - // 3. The parent gets enabled but this time doesn't do anything with the - // box, because it should continue being disabled. - // 4. The box is re-enabled -- but remains actually disabled as - // DoEnable(true) was never called on it (nor on the label). - // - // To avoid this possibility, we always call the base class version, which - // does call DoEnable(true) on the box itself and all its children, - // including the label, when re-enabling it. - if ( m_labelWin && !enable ) + // the label window as it would still remain disabled for as long as its + // parent is disabled. So we avoid disabling the box at all in this case + // and only disable its children. + if ( m_labelWin ) { - if ( enable == IsThisEnabled() ) + if ( enable == m_areChildrenEnabled ) return false; + m_areChildrenEnabled = enable; + const wxWindowList& children = GetChildren(); for ( wxWindowList::const_iterator i = children.begin(); i != children.end(); ++i ) { if ( *i != m_labelWin ) - (*i)->NotifyWindowOnEnableChange(enable); + (*i)->Enable(enable); } - m_isEnabled = enable; - return true; } #endif // wxHAS_WINDOW_LABEL_IN_STATIC_BOX From 9981a0216f841d02d306afaa01990c5ae749cf51 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sat, 20 Jan 2018 19:54:36 +0100 Subject: [PATCH 28/28] Add "windowlabel" element of wxStaticBoxSizer to the schema This was forgotten in the commit updating the sizer XRC handler. --- misc/schema/xrc_schema.rnc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/misc/schema/xrc_schema.rnc b/misc/schema/xrc_schema.rnc index b298088e45..ba9a688340 100644 --- a/misc/schema/xrc_schema.rnc +++ b/misc/schema/xrc_schema.rnc @@ -1864,6 +1864,7 @@ wxStaticBoxSizer_horz = stdObjectNodeAttributes & stdSizerProperties & [xrc:p="important"] element label {_, t_text }* & + element windowlabel {windowNode}* & [xrc:p="o"] element orient {_, "wxHORIZONTAL" }* & (wxBoxSizer_horz_item | objectRef)* } @@ -1874,6 +1875,7 @@ wxStaticBoxSizer_vert = stdObjectNodeAttributes & stdSizerProperties & [xrc:p="important"] element label {_, t_text }* & + element windowlabel {windowNode}* & element orient {_, "wxVERTICAL" } & (wxBoxSizer_vert_item | objectRef)* }