diff --git a/docs/changes.txt b/docs/changes.txt index bb933010e2..cc8d7a83f7 100644 --- a/docs/changes.txt +++ b/docs/changes.txt @@ -230,6 +230,7 @@ wxOSX: - Add support for wxTE_NO_VSCROLL style to wxTextCtrl. - Add support for wxTE_CHARWRAP style to wxTextCtrl. - Fix selecting RGB bitmaps (with no alpha channel) into wxMemoryDC. +- Fix updating radio groups when menu item is inserted/removed from wxMenu. Unix: diff --git a/include/wx/osx/menu.h b/include/wx/osx/menu.h index 19886d2b66..d9808e5f1d 100644 --- a/include/wx/osx/menu.h +++ b/include/wx/osx/menu.h @@ -15,6 +15,8 @@ class WXDLLIMPEXP_FWD_CORE wxFrame; #include "wx/arrstr.h" +class wxMenuRadioItemsData; + // ---------------------------------------------------------------------------- // Menu // ---------------------------------------------------------------------------- @@ -60,6 +62,12 @@ public: // we don't want native menu events being triggered void SetNoEventsMode( bool noEvents ); bool GetNoEventsMode() const { return m_noEventsMode; } + + // Returns the start and end position of the radio group to which the item + // at given position belongs. Return false if there is no radio group + // containing this position. + bool OSXGetRadioGroupRange(int pos, int *start, int *end) const; + protected: // hide special menu items like exit, preferences etc // that are expected in the app menu @@ -89,6 +97,8 @@ private: // don't trigger native events bool m_noEventsMode; + wxMenuRadioItemsData* m_radioData; + wxMenuImpl* m_peer; wxDECLARE_DYNAMIC_CLASS(wxMenu); diff --git a/include/wx/osx/menuitem.h b/include/wx/osx/menuitem.h index b5e0000876..371b790b40 100644 --- a/include/wx/osx/menuitem.h +++ b/include/wx/osx/menuitem.h @@ -53,41 +53,10 @@ public: void UpdateItemText() ; void UpdateItemStatus() ; - // mark item as belonging to the given radio group - void SetAsRadioGroupStart(bool start = true); - 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() ; - // the positions of the first and last items of the radio group this item - // belongs to or -1: start is the radio group start and is valid for all - // but first radio group items (m_isRadioGroupStart == FALSE), end is valid - // only for the first one - union - { - int start; - int end; - } m_radioGroup; - - // does this item start a radio group? - bool m_isRadioGroupStart; - wxBitmap m_bitmap; // Bitmap for menuitem, if any wxMenuItemImpl* m_peer; diff --git a/src/osx/menu_osx.cpp b/src/osx/menu_osx.cpp index 0e778bc604..86f9d6c299 100644 --- a/src/osx/menu_osx.cpp +++ b/src/osx/menu_osx.cpp @@ -32,6 +32,7 @@ #include "wx/osx/private.h" #include "wx/scopedptr.h" +#include "wx/private/menuradio.h" // for wxMenuRadioItemsData // other standard headers // ---------------------- @@ -61,6 +62,7 @@ void wxMenu::Init() m_doBreak = false; m_allowRearrange = true; m_noEventsMode = false; + m_radioData = NULL; m_peer = wxMenuImpl::Create( this, wxStripMenuCodes(m_title) ); @@ -75,6 +77,7 @@ void wxMenu::Init() wxMenu::~wxMenu() { + delete m_radioData; delete m_peer; } @@ -100,6 +103,11 @@ void wxMenu::SetNoEventsMode( bool noEvents ) m_noEventsMode = noEvents; } +bool wxMenu::OSXGetRadioGroupRange(int pos, int *start, int *end) const +{ + return m_radioData && m_radioData->GetGroupRange(pos, start, end); +} + // function appends a new item or submenu to the menu // append a new item or submenu to the menu bool wxMenu::DoInsertOrAppend(wxMenuItem *item, size_t pos) @@ -107,129 +115,43 @@ bool wxMenu::DoInsertOrAppend(wxMenuItem *item, size_t pos) wxASSERT_MSG( item != NULL, wxT("can't append NULL item to the menu") ); GetPeer()->InsertOrAppend( item, pos ); + wxMenu *pSubMenu = item->GetSubMenu() ; + if ( pSubMenu != NULL ) + { + wxASSERT_MSG( pSubMenu->GetHMenu() != NULL , wxT("invalid submenu added")); + pSubMenu->m_menuParent = this ; + + pSubMenu->DoRearrange(); + } + else if ( item->GetId() == idMenuTitle ) + { + item->GetMenu()->Enable( idMenuTitle, false ); + } + + if ( pos == (size_t)-1 ) + { + pos = GetMenuItemCount() - 1; + } + + // Update radio groups if we're inserting a new menu item. + // Inserting radio and non-radio item has a different impact + // on radio groups, so we have to handle each case separately. + // (Inserting a radio item in the middle of exisiting groups extends this group, + // but inserting a non-radio item breaks it into two subgroups.) bool check = false; - - if ( item->IsSeparator() ) + if ( item->IsRadio() ) { - // nothing to do here + if ( !m_radioData ) + m_radioData = new wxMenuRadioItemsData; + + if ( m_radioData->UpdateOnInsertRadio(pos) ) + check = true; // ensure that we have a checked item in the radio group } - else + else if ( m_radioData ) { - wxMenu *pSubMenu = item->GetSubMenu() ; - if ( pSubMenu != NULL ) - { - wxASSERT_MSG( pSubMenu->GetHMenu() != NULL , wxT("invalid submenu added")); - pSubMenu->m_menuParent = this ; - - pSubMenu->DoRearrange(); - } - else if ( item->IsRadio() ) - { - // If a previous or next item is a radio button, add this radio - // button to the existing radio group. Otherwise start a new one - // for it. - wxMenuItemList& items = GetMenuItems(); - - size_t const - posItem = pos == (size_t)-1 ? items.GetCount() - 1 : pos; - - wxMenuItemList::compatibility_iterator node = items.Item(posItem); - wxCHECK_MSG( node, false, wxS("New item must have been inserted") ); - - bool foundGroup = false; - if ( node->GetPrevious() ) - { - wxMenuItem* const prev = node->GetPrevious()->GetData(); - - if ( prev->IsRadio() ) - { - // This item is in the same group as the preceding one so - // we should use the same starting item, but getting it is - // a bit difficult as we can't query the start radio group - // item for it. - const int groupStart = prev->IsRadioGroupStart() - ? posItem - 1 - : prev->GetRadioGroupStart(); - item->SetRadioGroupStart(groupStart); - - // We must also account for the new item by incrementing - // the index of the last item in this group. - wxMenuItem* const first = items.Item(groupStart)->GetData(); - first->SetRadioGroupEnd(first->GetRadioGroupEnd() + 1); - - foundGroup = true; - } - } - - if ( !foundGroup && node->GetNext() ) - { - wxMenuItem* const next = node->GetNext()->GetData(); - - if ( next->IsRadio() ) - { - // This item is the new starting item of this group as the - // previous item is not a radio item. - wxASSERT_MSG( next->IsRadioGroupStart(), - wxS("Where is the start of this group?") ); - - // The index of the last item of the radio group must be - // incremented to account for the new item. - item->SetAsRadioGroupStart(); - item->SetRadioGroupEnd(next->GetRadioGroupEnd() + 1); - - // And the previous start item is not one any longer. - next->SetAsRadioGroupStart(false); - - foundGroup = true; - } - } - - if ( !foundGroup ) - { - // start a new radio group - item->SetAsRadioGroupStart(); - item->SetRadioGroupEnd(posItem); - - // ensure that we have a checked item in the radio group - check = true; - } - } - else - { - if ( item->GetId() == idMenuTitle ) - item->GetMenu()->Enable( idMenuTitle, false ); - } - } - - // We also need to update the indices of radio group start and end we store - // in any existing radio items after this item. - if ( pos < GetMenuItemCount() - 1 ) // takes into account pos == -1 case - { - for ( wxMenuItemList::compatibility_iterator - node = GetMenuItems().Item(pos + 1); - node; - node = node->GetNext() ) - { - wxMenuItem* const item = node->GetData(); - if ( item->IsRadio() ) - { - if ( item->IsRadioGroupStart() ) - { - // If the starting item is after the just inserted one, - // then the end one must be after it too and needs to be - // updated. - item->SetRadioGroupEnd(item->GetRadioGroupEnd() + 1); - } - else // Not the first radio group item. - { - // We need to update the start item index only if it is - // after the just inserted item. - const int groupStart = item->GetRadioGroupStart(); - if ( (size_t)groupStart > pos ) - item->SetRadioGroupStart(groupStart + 1); - } - } - } + bool groupWasSplit = m_radioData->UpdateOnInsertNonRadio(pos); + wxASSERT_MSG( !groupWasSplit, + wxS("Inserting non-radio item inside a radio group?") ); } // if we're already attached to the menubar, we must update it @@ -260,34 +182,15 @@ wxMenuItem* wxMenu::DoInsert(size_t pos, wxMenuItem *item) wxMenuItem *wxMenu::DoRemove(wxMenuItem *item) { - if ( item->IsRadio() ) + // Update indices of radio groups. + if ( m_radioData ) { - // Check if we're removing the item starting the radio group - if ( item->IsRadioGroupStart() ) + int pos = GetMenuItems().IndexOf(item); + if ( m_radioData->UpdateOnRemoveItem(pos) ) { - // 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; - } + wxASSERT_MSG( item->IsRadio(), wxS("Removing non radio button from radio group?") ); } + //else: item being removed is not in a radio group } /* diff --git a/src/osx/menuitem_osx.cpp b/src/osx/menuitem_osx.cpp index 0d6ba706ba..bac59e73ac 100644 --- a/src/osx/menuitem_osx.cpp +++ b/src/osx/menuitem_osx.cpp @@ -44,9 +44,6 @@ wxMenuItem::wxMenuItem(wxMenu *pParentMenu, if ( wxStripMenuCodes(m_text).Upper() == wxT("EXIT") ) m_text = wxT("Quit\tCtrl+Q") ; - m_radioGroup.start = -1; - m_isRadioGroupStart = false; - wxString text = wxStripMenuCodes(m_text, (pParentMenu != NULL && pParentMenu->GetNoEventsMode()) ? wxStrip_Accel : wxStrip_All); if (text.IsEmpty() && !IsSeparator()) { @@ -122,18 +119,9 @@ void wxMenuItem::Check(bool bDoCheck) // get the radio group range int start, end; - - if ( m_isRadioGroupStart ) + if ( !m_parentMenu->OSXGetRadioGroupRange(pos, &start, &end) ) { - // we already have all information we need - start = pos; - end = m_radioGroup.end; - } - else // next radio group item - { - // get the radio group end from the start item - start = m_radioGroup.start; - end = items.Item(start)->GetData()->m_radioGroup.end; + wxFAIL_MSG( wxS("Menu radio item not part of radio group?") ); } // also uncheck all the other items in this radio group @@ -211,51 +199,6 @@ void wxMenuItem::UpdateItemText() delete entry ; } -// radio group stuff -// ----------------- - -void wxMenuItem::SetAsRadioGroupStart(bool start) -{ - m_isRadioGroupStart = start; -} - -void wxMenuItem::SetRadioGroupStart(int start) -{ - wxASSERT_MSG( !m_isRadioGroupStart, - wxT("should only be called for the next radio items") ); - - m_radioGroup.start = start; -} - -void wxMenuItem::SetRadioGroupEnd(int end) -{ - wxASSERT_MSG( m_isRadioGroupStart, - wxT("should only be called for the first radio item") ); - - 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 // ----------------------------------------------------------------------------