From: Vadim Zeitlin Date: Tue, 16 Jul 2013 14:10:39 +0000 (+0000) Subject: Fix insertion of radio menu items in wxOSX wxMenu. X-Git-Url: https://git.saurik.com/wxWidgets.git/commitdiff_plain/27d79a5027bee4f46e57c813d072422065cb1592 Fix insertion of radio menu items in wxOSX wxMenu. Deal correctly with updating the indices when a radio item is inserted into an existing radio group (which wasn't done previously and resulted in a unit test failure in MenuTestCase::RadioItems()) and also with inserting the normal items before an existing radio group as the stored indices were not updated correctly. The code is still ugly and it probably wouldn't be a bad idea to reuse wxMenuRadioItemsData used in wxMSW for similar purposes, but at least the unit tests pass now. git-svn-id: https://svn.wxwidgets.org/svn/wx/wxWidgets/trunk@74548 c3d73ce0-8a6f-49c7-b76d-6d57e0e08775 --- diff --git a/docs/changes.txt b/docs/changes.txt index 0e824dccee..0eebedff24 100644 --- a/docs/changes.txt +++ b/docs/changes.txt @@ -726,6 +726,7 @@ wxOSX/Cocoa: - Implement image support in wxNotebook (Malcolm MacLeod). - Add support for button mnemonics (joostn). - Implemented wxTextCtrl::SetDefaultStyle(). +- Fix insertion and removal of radio items in wxMenu. 2.9.4: (released 2012-07-09) diff --git a/src/osx/menu_osx.cpp b/src/osx/menu_osx.cpp index e7199755d3..02be1abc43 100644 --- a/src/osx/menu_osx.cpp +++ b/src/osx/menu_osx.cpp @@ -105,6 +105,8 @@ 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 ); + bool check = false; + if ( item->IsSeparator() ) { // nothing to do here @@ -119,69 +121,131 @@ bool wxMenu::DoInsertOrAppend(wxMenuItem *item, size_t pos) pSubMenu->DoRearrange(); } - else + else if ( item->IsRadio() ) { - if ( item->GetId() == idMenuTitle ) - item->GetMenu()->Enable( idMenuTitle, false ); - } - } + // 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(); - // if we're already attached to the menubar, we must update it - if ( IsAttached() && GetMenuBar()->IsAttached() ) - GetMenuBar()->Refresh(); + size_t const + posItem = pos == (size_t)-1 ? items.GetCount() - 1 : pos; - return true ; -} + wxMenuItemList::compatibility_iterator node = items.Item(posItem); + wxCHECK_MSG( node, false, wxS("New item must have been inserted") ); -wxMenuItem* wxMenu::DoAppend(wxMenuItem *item) -{ - wxCHECK_MSG( item, NULL, wxT("NULL item in wxMenu::DoAppend") ); + bool foundGroup = false; + if ( node->GetPrevious() ) + { + wxMenuItem* const prev = node->GetPrevious()->GetData(); - bool check = false; + 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 ( item->IsRadio() ) - { - int count = GetMenuItemCount(); + if ( !foundGroup && node->GetNext() ) + { + wxMenuItem* const next = node->GetNext()->GetData(); - if ( !count || !(*GetMenuItems().rbegin())->IsRadio() ) - { - // start a new radio group - item->SetAsRadioGroupStart(); - item->SetRadioGroupEnd(count); + 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?") ); - // ensure that we have a checked item in the radio group - check = true; - } - else // extend the current radio group - { - // we need to update its end item - wxMenuItem* const last = *GetMenuItems().rbegin(); - const int groupStart = last->IsRadioGroupStart() - ? count - : last->GetRadioGroupStart(); + // 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); - item->SetRadioGroupStart(groupStart); - wxMenuItemList::compatibility_iterator node = GetMenuItems().Item(groupStart); + // And the previous start item is not one any longer. + next->SetAsRadioGroupStart(false); + + foundGroup = true; + } + } - if ( node ) + if ( !foundGroup ) { - node->GetData()->SetRadioGroupEnd(count); + // start a new radio group + item->SetAsRadioGroupStart(); + item->SetRadioGroupEnd(posItem); + + // ensure that we have a checked item in the radio group + check = true; } - else + } + 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() ) { - wxFAIL_MSG( wxT("where is the radio group start item?") ); + 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); + } } } } - if ( !wxMenuBase::DoAppend(item) || !DoInsertOrAppend(item) ) - return NULL; + // if we're already attached to the menubar, we must update it + if ( IsAttached() && GetMenuBar()->IsAttached() ) + GetMenuBar()->Refresh(); if ( check ) - // check the item initially item->Check(true); - return item; + return true ; +} + +wxMenuItem* wxMenu::DoAppend(wxMenuItem *item) +{ + if (wxMenuBase::DoAppend(item) && DoInsertOrAppend(item) ) + return item; + + return NULL; } wxMenuItem* wxMenu::DoInsert(size_t pos, wxMenuItem *item)