From 296bd7d64ef57f271398016288871563b94b1a1c Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Mon, 21 Aug 2017 00:36:29 +0200 Subject: [PATCH] Simplify TAB order code in wxStdDialogButtonSizer::Realize() No real changes, just add a helper class to avoid repeating the same sequence of lines many times in this function. This aims to slightly improve 42e9eb7ce89056a9a16884796265c11879420f97. See #17940. Closes https://github.com/wxWidgets/wxWidgets/pull/539 --- src/common/sizer.cpp | 145 ++++++++++++++----------------------------- 1 file changed, 48 insertions(+), 97 deletions(-) diff --git a/src/common/sizer.cpp b/src/common/sizer.cpp index 92cd35b9f9..ce59f88a1e 100644 --- a/src/common/sizer.cpp +++ b/src/common/sizer.cpp @@ -2764,25 +2764,47 @@ void wxStdDialogButtonSizer::SetCancelButton( wxButton *button ) void wxStdDialogButtonSizer::Realize() { - wxButton* lastAdded = NULL; + // Helper used to move the most recently added button after the previously + // added one in TAB order. + class TabOrderUpdater + { + public: + TabOrderUpdater() + : m_lastAdded(NULL) + { + } + + void Add(wxButton* btn) + { + if ( m_lastAdded ) + { + // Button should follow previous one in the keyboard navigation + // order. + btn->MoveAfterInTabOrder(m_lastAdded); + } + + m_lastAdded = btn; + } + + private: + wxButton* m_lastAdded; + }; + + TabOrderUpdater tabOrder; + #ifdef __WXMAC__ Add(0, 0, 0, wxLEFT, 6); if (m_buttonHelp) { Add((wxWindow*)m_buttonHelp, 0, wxALIGN_CENTRE | wxLEFT | wxRIGHT, 6); - lastAdded = m_buttonHelp; + tabOrder.Add(m_buttonHelp); } if (m_buttonNegative){ // HIG POLICE BULLETIN - destructive buttons need extra padding // 24 pixels on either side Add((wxWindow*)m_buttonNegative, 0, wxALIGN_CENTRE | wxLEFT | wxRIGHT, 12); - // Button should follow previous one in the keyboard navigation order. - if ( lastAdded ) - { - m_buttonNegative->MoveAfterInTabOrder(lastAdded); - } - lastAdded = m_buttonNegative; + tabOrder.Add(m_buttonNegative); } // extra whitespace between help/negative and cancel/ok buttons @@ -2793,12 +2815,7 @@ void wxStdDialogButtonSizer::Realize() // Cancel or help should be default // m_buttonCancel->SetDefaultButton(); - // Button should follow previous one in the keyboard navigation order. - if ( lastAdded ) - { - m_buttonCancel->MoveAfterInTabOrder(lastAdded); - } - lastAdded = m_buttonCancel; + tabOrder.Add(m_buttonCancel); } // Ugh, Mac doesn't really have apply dialogs, so I'll just @@ -2806,12 +2823,7 @@ void wxStdDialogButtonSizer::Realize() if (m_buttonApply) { Add((wxWindow*)m_buttonApply, 0, wxALIGN_CENTRE | wxLEFT | wxRIGHT, 6); - // Button should follow previous one in the keyboard navigation order. - if ( lastAdded ) - { - m_buttonApply->MoveAfterInTabOrder(lastAdded); - } - lastAdded = m_buttonApply; + tabOrder.Add(m_buttonApply); } if (m_buttonAffirmative){ @@ -2823,11 +2835,7 @@ void wxStdDialogButtonSizer::Realize() if (m_buttonNegative) m_buttonNegative->SetLabel(_("Don't Save")); } - // Button should follow previous one in the keyboard navigation order. - if ( lastAdded ) - { - m_buttonAffirmative->MoveAfterInTabOrder(lastAdded); - } + tabOrder.Add(m_buttonAffirmative); } // Extra space around and at the right @@ -2848,7 +2856,7 @@ void wxStdDialogButtonSizer::Realize() if (m_buttonHelp) { Add(m_buttonHelp, flagsBtn); - lastAdded = m_buttonHelp; + tabOrder.Add(m_buttonHelp); } // Align the rest of the buttons to the right. @@ -2857,44 +2865,25 @@ void wxStdDialogButtonSizer::Realize() if (m_buttonNegative) { Add(m_buttonNegative, flagsBtn); - // Button should follow previous one in the keyboard navigation order. - if ( lastAdded ) - { - m_buttonNegative->MoveAfterInTabOrder(lastAdded); - } - lastAdded = m_buttonNegative; + tabOrder.Add(m_buttonNegative); } if (m_buttonApply) { Add(m_buttonApply, flagsBtn); - // Button should follow previous one in the keyboard navigation order. - if ( lastAdded ) - { - m_buttonApply->MoveAfterInTabOrder(lastAdded); - } - lastAdded = m_buttonApply; + tabOrder.Add(m_buttonApply); } if (m_buttonCancel) { Add(m_buttonCancel, flagsBtn); - // Button should follow previous one in the keyboard navigation order. - if ( lastAdded ) - { - m_buttonCancel->MoveAfterInTabOrder(lastAdded); - } - lastAdded = m_buttonCancel; + tabOrder.Add(m_buttonCancel); } if (m_buttonAffirmative) { Add(m_buttonAffirmative, flagsBtn); - // Button should follow previous one in the keyboard navigation order. - if ( lastAdded ) - { - m_buttonAffirmative->MoveAfterInTabOrder(lastAdded); - } + tabOrder.Add(m_buttonAffirmative); } // Ensure that the right margin is 12 as well. @@ -2907,48 +2896,29 @@ void wxStdDialogButtonSizer::Realize() if (m_buttonAffirmative){ Add((wxWindow*)m_buttonAffirmative, 0, wxALIGN_CENTRE | wxLEFT | wxRIGHT, m_buttonAffirmative->ConvertDialogToPixels(wxSize(2, 0)).x); - lastAdded = m_buttonAffirmative; + tabOrder.Add(m_buttonAffirmative); } if (m_buttonNegative){ Add((wxWindow*)m_buttonNegative, 0, wxALIGN_CENTRE | wxLEFT | wxRIGHT, m_buttonNegative->ConvertDialogToPixels(wxSize(2, 0)).x); - // Button should follow previous one in the keyboard navigation order. - if ( lastAdded ) - { - m_buttonNegative->MoveAfterInTabOrder(lastAdded); - } - lastAdded = m_buttonNegative; + tabOrder.Add(m_buttonNegative); } if (m_buttonCancel){ Add((wxWindow*)m_buttonCancel, 0, wxALIGN_CENTRE | wxLEFT | wxRIGHT, m_buttonCancel->ConvertDialogToPixels(wxSize(2, 0)).x); - // Button should follow previous one in the keyboard navigation order. - if ( lastAdded ) - { - m_buttonCancel->MoveAfterInTabOrder(lastAdded); - } - lastAdded = m_buttonCancel; + tabOrder.Add(m_buttonCancel); } if (m_buttonApply) { Add((wxWindow*)m_buttonApply, 0, wxALIGN_CENTRE | wxLEFT | wxRIGHT, m_buttonApply->ConvertDialogToPixels(wxSize(2, 0)).x); - // Button should follow previous one in the keyboard navigation order. - if ( lastAdded ) - { - m_buttonApply->MoveAfterInTabOrder(lastAdded); - } - lastAdded = m_buttonApply; + tabOrder.Add(m_buttonApply); } if (m_buttonHelp) { Add((wxWindow*)m_buttonHelp, 0, wxALIGN_CENTRE | wxLEFT | wxRIGHT, m_buttonHelp->ConvertDialogToPixels(wxSize(2, 0)).x); - // Button should follow previous one in the keyboard navigation order. - if ( lastAdded ) - { - m_buttonHelp->MoveAfterInTabOrder(lastAdded); - } + tabOrder.Add(m_buttonHelp); } #else // GTK+1 and any other platform @@ -2957,7 +2927,7 @@ void wxStdDialogButtonSizer::Realize() if (m_buttonHelp) { Add((wxWindow*)m_buttonHelp, 0, wxALIGN_CENTRE | wxLEFT | wxRIGHT, m_buttonHelp->ConvertDialogToPixels(wxSize(4, 0)).x); - lastAdded = m_buttonHelp; + tabOrder.Add(m_buttonHelp); } // extra whitespace between help and cancel/ok buttons @@ -2966,43 +2936,24 @@ void wxStdDialogButtonSizer::Realize() if (m_buttonApply) { Add((wxWindow*)m_buttonApply, 0, wxALIGN_CENTRE | wxLEFT | wxRIGHT, m_buttonApply->ConvertDialogToPixels(wxSize(4, 0)).x); - // Button should follow previous one in the keyboard navigation order. - if ( lastAdded ) - { - m_buttonApply->MoveAfterInTabOrder(lastAdded); - } - lastAdded = m_buttonApply; + tabOrder.Add(m_buttonApply); } if (m_buttonAffirmative){ Add((wxWindow*)m_buttonAffirmative, 0, wxALIGN_CENTRE | wxLEFT | wxRIGHT, m_buttonAffirmative->ConvertDialogToPixels(wxSize(4, 0)).x); - // Button should follow previous one in the keyboard navigation order. - if ( lastAdded ) - { - m_buttonAffirmative->MoveAfterInTabOrder(lastAdded); - } - lastAdded = m_buttonAffirmative; + tabOrder.Add(m_buttonAffirmative); } if (m_buttonNegative){ Add((wxWindow*)m_buttonNegative, 0, wxALIGN_CENTRE | wxLEFT | wxRIGHT, m_buttonNegative->ConvertDialogToPixels(wxSize(4, 0)).x); - // Button should follow previous one in the keyboard navigation order. - if ( lastAdded ) - { - m_buttonNegative->MoveAfterInTabOrder(lastAdded); - } - lastAdded = m_buttonNegative; + tabOrder.Add(m_buttonNegative); } if (m_buttonCancel){ Add((wxWindow*)m_buttonCancel, 0, wxALIGN_CENTRE | wxLEFT | wxRIGHT, m_buttonCancel->ConvertDialogToPixels(wxSize(4, 0)).x); // Cancel or help should be default // m_buttonCancel->SetDefaultButton(); - // Button should follow previous one in the keyboard navigation order. - if ( lastAdded ) - { - m_buttonCancel->MoveAfterInTabOrder(lastAdded); - } + tabOrder.Add(m_buttonCancel); } #endif