Reimplement adding items to radio groups in wxMenu (wxOSX)

Rewrote wxOSX radio groups-related code reusing the code which works on
wxMSW and which seems to provide more rich functionality (supports adding
radio as well as no-radio items) and to have known bugs fixed.
The ranges of all radio groups are stored in wxMenu itself instead of
storing the information about the radio group an item belongs to in
the item itself - see 89511b4268.

Closes #14213.
Closes #17568.
This commit is contained in:
Artur Wieczorek
2017-08-13 01:11:35 +02:00
parent 4bc1c6fb7c
commit b6e4bdce3d
5 changed files with 60 additions and 234 deletions

View File

@@ -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:

View File

@@ -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);

View File

@@ -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;

View File

@@ -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
}
/*

View File

@@ -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
// ----------------------------------------------------------------------------