From 3b28ca6503fcd460d7de157ecf8b485590252156 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Tue, 16 Jul 2013 14:10:26 +0000 Subject: [PATCH] Get rid of wxMenu::m_startRadioGroup in wxOSX. This code was probably copied from wxUniv but was wrong as we can't rely on the items being always inserted in order. This commit on its own fixes removing the first radio group menu item but it also makes possible to properly implement the insertion of new items in the middle of an existing radio group which couldn't be done with m_startRadioGroup approach at all. git-svn-id: https://svn.wxwidgets.org/svn/wx/wxWidgets/trunk@74544 c3d73ce0-8a6f-49c7-b76d-6d57e0e08775 --- include/wx/osx/menu.h | 8 ----- include/wx/osx/menuitem.h | 16 ++++++++++ src/osx/menu_osx.cpp | 65 ++++++++++++++++++++------------------- src/osx/menuitem_osx.cpp | 21 +++++++++++++ 4 files changed, 71 insertions(+), 39 deletions(-) diff --git a/include/wx/osx/menu.h b/include/wx/osx/menu.h index 17f351efd6..ceea341234 100644 --- a/include/wx/osx/menu.h +++ b/include/wx/osx/menu.h @@ -33,8 +33,6 @@ public: virtual ~wxMenu(); - virtual void Attach(wxMenuBarBase *menubar) ; - virtual void Break(); virtual void SetTitle(const wxString& title); @@ -80,9 +78,6 @@ private: // common part of Append/Insert (behaves as Append is pos == (size_t)-1) bool DoInsertOrAppend(wxMenuItem *item, size_t pos = (size_t)-1); - // terminate the current radio group, if any - void EndRadioGroup(); - // Common part of HandleMenu{Opened,Closed}(). void DoHandleMenuOpenedOrClosed(wxEventType evtType); @@ -96,9 +91,6 @@ private: // don't trigger native events bool m_noEventsMode; - // the position of the first item in the current radio group or -1 - int m_startRadioGroup; - wxMenuImpl* m_peer; DECLARE_DYNAMIC_CLASS(wxMenu) diff --git a/include/wx/osx/menuitem.h b/include/wx/osx/menuitem.h index e4f53c86e8..31decc1e33 100644 --- a/include/wx/osx/menuitem.h +++ b/include/wx/osx/menuitem.h @@ -46,6 +46,9 @@ public: virtual void SetBitmap(const wxBitmap& bitmap) ; virtual const wxBitmap& GetBitmap() const { return m_bitmap; } + + // Implementation only from now on. + // update the os specific representation void UpdateItemBitmap() ; void UpdateItemText() ; @@ -56,6 +59,19 @@ public: void SetRadioGroupStart(int start); void SetRadioGroupEnd(int end); + // return true if this is the starting item of a radio group + bool IsRadioGroupStart() const; + + // get the start of the radio group this item belongs to: should not be + // called for the starting radio group item itself because it doesn't have + // this information + int GetRadioGroupStart() const; + + // get the end of the radio group this item belongs to: should be only + // called for the starting radio group item, i.e. if IsRadioGroupStart() is + // true + int GetRadioGroupEnd() const; + wxMenuItemImpl* GetPeer() { return m_peer; } private: void UncheckRadio() ; diff --git a/src/osx/menu_osx.cpp b/src/osx/menu_osx.cpp index 4ad6fa1fdf..8a795e67ce 100644 --- a/src/osx/menu_osx.cpp +++ b/src/osx/menu_osx.cpp @@ -57,7 +57,6 @@ static const int idMenuTitle = -3; void wxMenu::Init() { m_doBreak = false; - m_startRadioGroup = -1; m_allowRearrange = true; m_noEventsMode = false; @@ -89,13 +88,6 @@ void wxMenu::Break() // not available on the mac platform } -void wxMenu::Attach(wxMenuBarBase *menubar) -{ - wxMenuBase::Attach(menubar); - - EndRadioGroup(); -} - void wxMenu::SetAllowRearrange( bool allow ) { m_allowRearrange = allow; @@ -141,30 +133,21 @@ bool wxMenu::DoInsertOrAppend(wxMenuItem *pItem, size_t pos) return true ; } -void wxMenu::EndRadioGroup() -{ - // we're not inside a radio group any longer - m_startRadioGroup = -1; -} - wxMenuItem* wxMenu::DoAppend(wxMenuItem *item) { wxCHECK_MSG( item, NULL, wxT("NULL item in wxMenu::DoAppend") ); bool check = false; - if ( item->GetKind() == wxITEM_RADIO ) + if ( item->IsRadio() ) { int count = GetMenuItemCount(); - if ( m_startRadioGroup == -1 ) + if ( !count || !(*GetMenuItems().rbegin())->IsRadio() ) { // start a new radio group - m_startRadioGroup = count; - - // for now it has just one element item->SetAsRadioGroupStart(); - item->SetRadioGroupEnd(m_startRadioGroup); + item->SetRadioGroupEnd(count); // ensure that we have a checked item in the radio group check = true; @@ -172,8 +155,13 @@ wxMenuItem* wxMenu::DoAppend(wxMenuItem *item) else // extend the current radio group { // we need to update its end item - item->SetRadioGroupStart(m_startRadioGroup); - wxMenuItemList::compatibility_iterator node = GetMenuItems().Item(m_startRadioGroup); + wxMenuItem* const last = *GetMenuItems().rbegin(); + const int groupStart = last->IsRadioGroupStart() + ? count + : last->GetRadioGroupStart(); + + item->SetRadioGroupStart(groupStart); + wxMenuItemList::compatibility_iterator node = GetMenuItems().Item(groupStart); if ( node ) { @@ -185,10 +173,6 @@ wxMenuItem* wxMenu::DoAppend(wxMenuItem *item) } } } - else // not a radio item - { - EndRadioGroup(); - } if ( !wxMenuBase::DoAppend(item) || !DoInsertOrAppend(item) ) return NULL; @@ -210,14 +194,33 @@ wxMenuItem* wxMenu::DoInsert(size_t pos, wxMenuItem *item) wxMenuItem *wxMenu::DoRemove(wxMenuItem *item) { - if ( m_startRadioGroup != -1 ) + if ( item->IsRadio() ) { // Check if we're removing the item starting the radio group - if ( GetMenuItems().Item(m_startRadioGroup)->GetData() == item ) + if ( item->IsRadioGroupStart() ) { - // Yes, we do, so reset its index as the next item added shouldn't - // count as part of the same radio group anyhow. - m_startRadioGroup = -1; + // Yes, we do, update the next radio group item, if any, to be the + // start one now. + const int endGroup = item->GetRadioGroupEnd(); + + wxMenuItemList::compatibility_iterator + node = GetMenuItems().Item(endGroup); + wxASSERT_MSG( node, wxS("Should have valid radio group end") ); + + while ( node->GetData() != item ) + { + const wxMenuItemList::compatibility_iterator + prevNode = node->GetPrevious(); + wxMenuItem* const prevItem = prevNode->GetData(); + if ( prevItem == item ) + { + prevItem->SetAsRadioGroupStart(); + prevItem->SetRadioGroupEnd(endGroup); + break; + } + + node = prevNode; + } } } diff --git a/src/osx/menuitem_osx.cpp b/src/osx/menuitem_osx.cpp index 93ef505286..096d46c583 100644 --- a/src/osx/menuitem_osx.cpp +++ b/src/osx/menuitem_osx.cpp @@ -239,6 +239,27 @@ void wxMenuItem::SetRadioGroupEnd(int end) m_radioGroup.end = end; } +bool wxMenuItem::IsRadioGroupStart() const +{ + return m_isRadioGroupStart; +} + +int wxMenuItem::GetRadioGroupStart() const +{ + wxASSERT_MSG( !m_isRadioGroupStart, + wxS("shouldn't be called for the first radio item") ); + + return m_radioGroup.start; +} + +int wxMenuItem::GetRadioGroupEnd() const +{ + wxASSERT_MSG( m_isRadioGroupStart, + wxS("shouldn't be called for the first radio item") ); + + return m_radioGroup.end; +} + // ---------------------------------------------------------------------------- // wxMenuItemBase // ---------------------------------------------------------------------------- -- 2.45.2