diff --git a/include/wx/msw/menu.h b/include/wx/msw/menu.h index d80e85cca1..4c5e95f5ad 100644 --- a/include/wx/msw/menu.h +++ b/include/wx/msw/menu.h @@ -25,6 +25,7 @@ class WXDLLIMPEXP_FWD_CORE wxFrame; class WXDLLIMPEXP_FWD_CORE wxToolBar; #endif +class wxMenuRadioItemsData; // Not using a combined wxToolBar/wxMenuBar? then use // a commandbar in WinCE .NET to implement the @@ -63,13 +64,16 @@ public: // implementation only from now on // ------------------------------- - virtual void Attach(wxMenuBarBase *menubar); - bool MSWCommand(WXUINT param, WXWORD id); // get the native menu handle WXHMENU GetHMenu() const { return m_hMenu; } + // Return the start and end position of the radio group to which the item + // at the given position belongs. Returns false if there is no radio group + // containing this position. + bool MSWGetRadioGroupRange(int pos, int *start, int *end) const; + #if wxUSE_ACCEL // called by wxMenuBar to build its accel table from the accels of all menus bool HasAccels() const { return !m_accels.empty(); } @@ -122,15 +126,17 @@ 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(); + + // This variable contains the description of the radio item groups and + // allows to find whether an item at the given position is part of the + // group and also where its group starts and ends. + // + // It is initially NULL and only allocated if we have any radio items. + wxMenuRadioItemsData *m_radioData; // if true, insert a breal before appending the next item bool m_doBreak; - // the position of the first item in the current radio group or -1 - int m_startRadioGroup; - // the menu handle of this menu WXHMENU m_hMenu; diff --git a/include/wx/msw/menuitem.h b/include/wx/msw/menuitem.h index 21be80c38f..1fe684971b 100644 --- a/include/wx/msw/menuitem.h +++ b/include/wx/msw/menuitem.h @@ -61,11 +61,6 @@ public: // Win32 API WXWPARAM GetMSWId() const; - // mark item as belonging to the given radio group - void SetAsRadioGroupStart(); - void SetRadioGroupStart(int start); - void SetRadioGroupEnd(int end); - #if WXWIN_COMPATIBILITY_2_8 // compatibility only, don't use in new code wxDEPRECATED( @@ -130,18 +125,6 @@ private: // common part of all ctors void Init(); - // 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; #if wxUSE_OWNER_DRAWN // item bitmaps diff --git a/src/msw/menu.cpp b/src/msw/menu.cpp index 3901d885fe..c1accd90e2 100644 --- a/src/msw/menu.cpp +++ b/src/msw/menu.cpp @@ -41,6 +41,7 @@ #endif #include "wx/scopedarray.h" +#include "wx/vector.h" #include "wx/msw/private.h" #include "wx/msw/wrapcctl.h" // include "properly" @@ -85,9 +86,102 @@ static const int idMenuTitle = wxID_NONE; // ---------------------------------------------------------------------------- -// private functions +// private helper classes and functions // ---------------------------------------------------------------------------- +// Contains the data about the radio items groups in the given menu. +class wxMenuRadioItemsData +{ +public: + wxMenuRadioItemsData() { } + + // Default copy ctor, assignment operator and dtor are all ok. + + // Find the start and end of the group containing the given position or + // return false if it's not inside any range. + bool GetGroupRange(int pos, int *start, int *end) const + { + // We use a simple linear search here because there are not that many + // items in a menu and hence even fewer radio items ranges anyhow, so + // normally there is no need to do anything fancy (like keeping the + // array sorted and using binary search). + for ( Ranges::const_iterator it = m_ranges.begin(); + it != m_ranges.end(); + ++it ) + { + const Range& r = *it; + + if ( r.start <= pos && pos <= r.end ) + { + if ( start ) + *start = r.start; + if ( end ) + *end = r.end; + + return true; + } + } + + return false; + } + + // Take into account the new radio item about to be added at the given + // position. + // + // Returns true if this item starts a new radio group, false if it extends + // an existing one. + bool UpdateOnInsert(int pos) + { + bool inExistingGroup = false; + + for ( Ranges::iterator it = m_ranges.begin(); + it != m_ranges.end(); + ++it ) + { + Range& r = *it; + + if ( pos < r.start ) + { + // Item is inserted before this range, update its indices. + r.start++; + r.end++; + } + else if ( pos <= r.end + 1 ) + { + // Item is inserted in the middle of this range or immediately + // after it in which case it extends this range so make it span + // one more item in any case. + r.end++; + + inExistingGroup = true; + } + //else: Item is inserted after this range, nothing to do for it. + } + + if ( inExistingGroup ) + return false; + + // Make a new range for the group this item will belong to. + Range r; + r.start = pos; + r.end = pos; + m_ranges.push_back(r); + + return true; + } + +private: + // Contains the inclusive positions of the range start and end. + struct Range + { + int start; + int end; + }; + + typedef wxVector Ranges; + Ranges m_ranges; +}; + namespace { @@ -168,8 +262,8 @@ inline bool IsGreaterThanStdSize(const wxBitmap& bmp) // Construct a menu with optional title (then use append) void wxMenu::Init() { + m_radioData = NULL; m_doBreak = false; - m_startRadioGroup = -1; #if wxUSE_OWNER_DRAWN m_ownerDrawn = false; @@ -211,6 +305,8 @@ wxMenu::~wxMenu() // delete accels WX_CLEAR_ARRAY(m_accels); #endif // wxUSE_ACCEL + + delete m_radioData; } void wxMenu::Break() @@ -219,13 +315,6 @@ void wxMenu::Break() m_doBreak = true; } -void wxMenu::Attach(wxMenuBarBase *menubar) -{ - wxMenuBase::Attach(menubar); - - EndRadioGroup(); -} - #if wxUSE_ACCEL int wxMenu::FindAccel(int id) const @@ -348,6 +437,11 @@ HBITMAP GetHBitmapForMenu(wxMenuItem *pItem, bool checked = true) } // anonymous namespace +bool wxMenu::MSWGetRadioGroupRange(int pos, int *start, int *end) const +{ + return m_radioData && m_radioData->GetGroupRange(pos, start, end); +} + // append a new item or submenu to the menu bool wxMenu::DoInsertOrAppend(wxMenuItem *pItem, size_t pos) { @@ -397,6 +491,21 @@ bool wxMenu::DoInsertOrAppend(wxMenuItem *pItem, size_t pos) pos = GetMenuItemCount() - 1; } + // Update radio groups data if we're inserting a new radio item. + // + // NB: If we supported inserting non-radio items in the middle of existing + // radio groups to break them into two subgroups, we'd need to update + // m_radioData in this case too but currently this is not supported. + bool checkInitially = false; + if ( pItem->GetKind() == wxITEM_RADIO ) + { + if ( !m_radioData ) + m_radioData = new wxMenuRadioItemsData; + + if ( m_radioData->UpdateOnInsert(pos) ) + checkInitially = true; + } + // adjust position to account for the title of a popup menu, if any if ( !GetMenuBar() && !m_title.empty() ) pos += 2; // for the title itself and its separator @@ -600,6 +709,10 @@ bool wxMenu::DoInsertOrAppend(wxMenuItem *pItem, size_t pos) } + // Check the item if it should be initially checked. + if ( checkInitially ) + pItem->Check(true); + // if we just appended the title, highlight it if ( id == (UINT_PTR)idMenuTitle ) { @@ -616,67 +729,9 @@ 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 ) - { - int count = GetMenuItemCount(); - - if ( m_startRadioGroup == -1 ) - { - // start a new radio group - m_startRadioGroup = count; - - // for now it has just one element - item->SetAsRadioGroupStart(); - item->SetRadioGroupEnd(m_startRadioGroup); - - // 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 - item->SetRadioGroupStart(m_startRadioGroup); - wxMenuItemList::compatibility_iterator node = GetMenuItems().Item(m_startRadioGroup); - - if ( node ) - { - node->GetData()->SetRadioGroupEnd(count); - } - else - { - wxFAIL_MSG( wxT("where is the radio group start item?") ); - } - } - } - else // not a radio item - { - EndRadioGroup(); - } - - if ( !wxMenuBase::DoAppend(item) || !DoInsertOrAppend(item) ) - { - return NULL; - } - - if ( check ) - { - // check the item initially - item->Check(true); - } - - return item; + return wxMenuBase::DoAppend(item) && DoInsertOrAppend(item) ? item : NULL; } wxMenuItem* wxMenu::DoInsert(size_t pos, wxMenuItem *item) diff --git a/src/msw/menuitem.cpp b/src/msw/menuitem.cpp index f4f61e1321..94d963d4ac 100644 --- a/src/msw/menuitem.cpp +++ b/src/msw/menuitem.cpp @@ -494,9 +494,6 @@ wxMenuItem::wxMenuItem(wxMenu *parentMenu, void wxMenuItem::Init() { - m_radioGroup.start = -1; - m_isRadioGroupStart = false; - #if wxUSE_OWNER_DRAWN // when the color is not valid, wxOwnerDraw takes the default ones. @@ -557,30 +554,6 @@ bool wxMenuItem::IsChecked() const return (flag & MF_CHECKED) != 0; } -// radio group stuff -// ----------------- - -void wxMenuItem::SetAsRadioGroupStart() -{ - m_isRadioGroupStart = true; -} - -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; -} - // change item state // ----------------- @@ -634,17 +607,10 @@ void wxMenuItem::Check(bool check) int start, end; - if ( m_isRadioGroupStart ) + if ( !m_parentMenu->MSWGetRadioGroupRange(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( wxT("Menu radio item not part of radio group?") ); + return; } #ifdef __WIN32__ diff --git a/tests/menu/menu.cpp b/tests/menu/menu.cpp index 6d8fd784de..3424af6b50 100644 --- a/tests/menu/menu.cpp +++ b/tests/menu/menu.cpp @@ -84,6 +84,7 @@ private: CPPUNIT_TEST( FindInMenu ); CPPUNIT_TEST( Count ); CPPUNIT_TEST( Labels ); + CPPUNIT_TEST( RadioItems ); CPPUNIT_TEST_SUITE_END(); void CreateFrame(); @@ -92,6 +93,7 @@ private: void FindInMenu(); void Count(); void Labels(); + void RadioItems(); wxFrame* m_frame; @@ -304,3 +306,56 @@ void MenuTestCase::Labels() CPPUNIT_ASSERT_EQUAL( "Foo", itemFoo->GetItemLabelText() ); CPPUNIT_ASSERT_EQUAL( "Foo", wxMenuItem::GetLabelText("&Foo\tCtrl-F") ); } + +void MenuTestCase::RadioItems() +{ + wxMenuBar * const bar = m_frame->GetMenuBar(); + wxMenu * const menu = new wxMenu; + bar->Append(menu, "&Radio"); + + // Adding consecutive radio items creates a radio group. + menu->AppendRadioItem(MenuTestCase_First, "Radio 0"); + menu->AppendRadioItem(MenuTestCase_First + 1, "Radio 1"); + + // First item of a radio group is checked by default. + CPPUNIT_ASSERT( menu->IsChecked(MenuTestCase_First) ); + + // Checking the second one make the first one unchecked however. + menu->Check(MenuTestCase_First + 1, true); + CPPUNIT_ASSERT( !menu->IsChecked(MenuTestCase_First) ); + CPPUNIT_ASSERT( menu->IsChecked(MenuTestCase_First + 1) ); + + // Adding more radio items after a separator creates another radio group... + menu->AppendSeparator(); + menu->AppendRadioItem(MenuTestCase_First + 2, "Radio 2"); + menu->AppendRadioItem(MenuTestCase_First + 3, "Radio 3"); + menu->AppendRadioItem(MenuTestCase_First + 4, "Radio 4"); + + // ... which is independent from the first one. + CPPUNIT_ASSERT( menu->IsChecked(MenuTestCase_First + 2) ); + + menu->Check(MenuTestCase_First + 3, true); + CPPUNIT_ASSERT( menu->IsChecked(MenuTestCase_First + 3) ); + CPPUNIT_ASSERT( !menu->IsChecked(MenuTestCase_First + 2) ); + CPPUNIT_ASSERT( menu->IsChecked(MenuTestCase_First + 1) ); + + + // Insert an item in the middle of an existing radio group. + menu->InsertRadioItem(4, MenuTestCase_First + 5, "Radio 5"); + CPPUNIT_ASSERT( menu->IsChecked(MenuTestCase_First + 3) ); + + menu->Check( MenuTestCase_First + 5, true ); + CPPUNIT_ASSERT( !menu->IsChecked(MenuTestCase_First + 3) ); + + + // Prepend a couple of items before the first group. + menu->PrependRadioItem(MenuTestCase_First + 6, "Radio 6"); + menu->PrependRadioItem(MenuTestCase_First + 7, "Radio 7"); + menu->Check(MenuTestCase_First + 7, true); + CPPUNIT_ASSERT( !menu->IsChecked(MenuTestCase_First + 1) ); + + + // Check that the last radio group still works as expected. + menu->Check(MenuTestCase_First + 4, true); + CPPUNIT_ASSERT( !menu->IsChecked(MenuTestCase_First + 5) ); +}