From 6b7b43d1fef7e27700ca24d534cc25c0e066ea68 Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Wed, 21 Nov 2018 12:58:23 +0000 Subject: [PATCH 1/6] Get WxQt menu titles passing GUI tests --- include/wx/qt/menu.h | 1 + src/qt/menu.cpp | 8 ++++++++ 2 files changed, 9 insertions(+) diff --git a/include/wx/qt/menu.h b/include/wx/qt/menu.h index b8d5abca52..f6d52bb3e2 100644 --- a/include/wx/qt/menu.h +++ b/include/wx/qt/menu.h @@ -44,6 +44,7 @@ public: virtual wxMenu *Remove(size_t pos); virtual void EnableTop(size_t pos, bool enable); + virtual bool IsEnabledTop(size_t pos) const wxOVERRIDE; virtual void SetMenuLabel(size_t pos, const wxString& label); virtual wxString GetMenuLabel(size_t pos) const; diff --git a/src/qt/menu.cpp b/src/qt/menu.cpp index 61d27b7976..e0f7d8c5fc 100644 --- a/src/qt/menu.cpp +++ b/src/qt/menu.cpp @@ -184,6 +184,8 @@ wxMenuBar::wxMenuBar(size_t count, wxMenu *menus[], const wxString titles[], lon static QMenu *SetTitle( wxMenu *menu, const wxString &title ) { + menu->SetTitle(title); + QMenu *qtMenu = menu->GetHandle(); qtMenu->setTitle( wxQtConvertString( title )); @@ -243,6 +245,12 @@ void wxMenuBar::EnableTop(size_t pos, bool enable) qtAction->setEnabled( enable ); } +bool wxMenuBar::IsEnabledTop(size_t pos) const +{ + QAction *qtAction = GetActionAt( m_qtMenuBar, pos ); + return qtAction->isEnabled(); +} + void wxMenuBar::SetMenuLabel(size_t pos, const wxString& label) { From bfad2a5425cc1e97362081180c0a64b85daae3ff Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Wed, 21 Nov 2018 16:02:33 +0000 Subject: [PATCH 2/6] Fix menu first radio button state and synchronise state changes. --- src/qt/menu.cpp | 3 ++- src/qt/menuitem.cpp | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/qt/menu.cpp b/src/qt/menu.cpp index e0f7d8c5fc..1b810cecd9 100644 --- a/src/qt/menu.cpp +++ b/src/qt/menu.cpp @@ -53,7 +53,7 @@ static wxMenuItem *GetMenuItemAt( const wxMenu *menu, size_t position ) static void InsertMenuItemAction( const wxMenu *menu, const wxMenuItem *previousItem, - const wxMenuItem *item, const wxMenuItem *successiveItem ) + wxMenuItem *item, const wxMenuItem *successiveItem ) { QMenu *qtMenu = menu->GetHandle(); QAction *itemAction = item->GetHandle(); @@ -74,6 +74,7 @@ static void InsertMenuItemAction( const wxMenu *menu, const wxMenuItem *previous { QActionGroup *actionGroup = new QActionGroup( qtMenu ); actionGroup->addAction( itemAction ); + item->Check(); wxASSERT_MSG( itemAction->actionGroup() == actionGroup, "Must be the same action group" ); } break; diff --git a/src/qt/menuitem.cpp b/src/qt/menuitem.cpp index 6ce89dc5be..328e65c892 100644 --- a/src/qt/menuitem.cpp +++ b/src/qt/menuitem.cpp @@ -165,5 +165,6 @@ void wxQtAction::onActionTriggered( bool checked ) { wxMenuItem *handler = GetHandler(); wxMenu *menu = handler->GetMenu(); + if(handler->IsCheckable()) handler->Check(checked); menu->SendEvent( handler->GetId(), handler->IsCheckable() ? checked : -1 ); } From 82f8fad24052f48b481c1a926b752c42e7d4456b Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Thu, 22 Nov 2018 10:22:39 +0000 Subject: [PATCH 3/6] Split out test which will not work on QT, and fix insertion behaviour of radio menu items. --- src/qt/menu.cpp | 7 ++++++ tests/menu/menu.cpp | 55 +++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 58 insertions(+), 4 deletions(-) diff --git a/src/qt/menu.cpp b/src/qt/menu.cpp index 1b810cecd9..c346a1641c 100644 --- a/src/qt/menu.cpp +++ b/src/qt/menu.cpp @@ -70,6 +70,13 @@ static void InsertMenuItemAction( const wxMenu *menu, const wxMenuItem *previous wxASSERT_MSG( previousItemActionGroup != NULL, "An action group should have been setup" ); previousItemActionGroup->addAction( itemAction ); } + else if( successiveItem != NULL && successiveItem->GetKind() == wxITEM_RADIO ) + { + QAction *successiveItemAction = successiveItem->GetHandle(); + QActionGroup *successiveItemActionGroup = successiveItemAction->actionGroup(); + wxASSERT_MSG( successiveItemActionGroup != NULL, "An action group should have been setup" ); + successiveItemActionGroup->addAction( itemAction ); + } else { QActionGroup *actionGroup = new QActionGroup( qtMenu ); diff --git a/tests/menu/menu.cpp b/tests/menu/menu.cpp index dbda30cb74..8c0547b78c 100644 --- a/tests/menu/menu.cpp +++ b/tests/menu/menu.cpp @@ -90,7 +90,11 @@ private: #if wxUSE_INTL CPPUNIT_TEST( TranslatedMnemonics ); #endif // wxUSE_INTL +#ifndef __WXQT__ CPPUNIT_TEST( RadioItems ); +#else + CPPUNIT_TEST( RadioItemsWithoutMutualExclusion ); +#endif CPPUNIT_TEST( RemoveAdd ); CPPUNIT_TEST( ChangeBitmap ); WXUISIM_TEST( Events ); @@ -107,6 +111,7 @@ private: void TranslatedMnemonics(); #endif // wxUSE_INTL void RadioItems(); + void RadioItemsWithoutMutualExclusion(); void RemoveAdd(); void ChangeBitmap(); void Events(); @@ -425,7 +430,6 @@ void MenuTestCase::RadioItems() 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) ); @@ -433,13 +437,56 @@ void MenuTestCase::RadioItems() 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"); + + // Items should not be checked, as they are part of an existing group. + CPPUNIT_ASSERT( !menu->IsChecked(MenuTestCase_First + 6) ); + CPPUNIT_ASSERT( !menu->IsChecked(MenuTestCase_First + 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) ); +} + +void MenuTestCase::RadioItemsWithoutMutualExclusion() +{ + 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) ); + + // Subsequent items in a group are not checked. + 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) ); + CPPUNIT_ASSERT( menu->IsChecked(MenuTestCase_First + 2) ); + + // 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 + 2) ); // 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) ); - + CPPUNIT_ASSERT( !menu->IsChecked(MenuTestCase_First + 6) ); + CPPUNIT_ASSERT( !menu->IsChecked(MenuTestCase_First + 7) ); // Check that the last radio group still works as expected. menu->Check(MenuTestCase_First + 4, true); From 902ce48189e49f95977435a587683b46dc36b51b Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Thu, 22 Nov 2018 10:35:40 +0000 Subject: [PATCH 4/6] Remove duplicated code. --- src/qt/menu.cpp | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/qt/menu.cpp b/src/qt/menu.cpp index c346a1641c..e81e237f96 100644 --- a/src/qt/menu.cpp +++ b/src/qt/menu.cpp @@ -51,6 +51,13 @@ static wxMenuItem *GetMenuItemAt( const wxMenu *menu, size_t position ) return NULL; } +static void AddItemActionToGroup( const wxMenuItem *groupItem, QAction *itemAction ) +{ + QAction *groupItemAction = groupItem->GetHandle(); + QActionGroup *itemActionGroup = groupItemAction->actionGroup(); + wxASSERT_MSG( itemActionGroup != NULL, "An action group should have been setup" ); + itemActionGroup->addAction( itemAction ); +} static void InsertMenuItemAction( const wxMenu *menu, const wxMenuItem *previousItem, wxMenuItem *item, const wxMenuItem *successiveItem ) @@ -60,22 +67,16 @@ static void InsertMenuItemAction( const wxMenu *menu, const wxMenuItem *previous switch ( item->GetKind() ) { case wxITEM_RADIO: - // If the previous menu item is a radio item then add this item to the + // If a neighbouring menu item is a radio item then add this item to the // same action group, otherwise start a new group: if ( previousItem != NULL && previousItem->GetKind() == wxITEM_RADIO ) { - QAction *previousItemAction = previousItem->GetHandle(); - QActionGroup *previousItemActionGroup = previousItemAction->actionGroup(); - wxASSERT_MSG( previousItemActionGroup != NULL, "An action group should have been setup" ); - previousItemActionGroup->addAction( itemAction ); + AddItemActionToGroup( previousItem, itemAction ); } else if( successiveItem != NULL && successiveItem->GetKind() == wxITEM_RADIO ) { - QAction *successiveItemAction = successiveItem->GetHandle(); - QActionGroup *successiveItemActionGroup = successiveItemAction->actionGroup(); - wxASSERT_MSG( successiveItemActionGroup != NULL, "An action group should have been setup" ); - successiveItemActionGroup->addAction( itemAction ); + AddItemActionToGroup( successiveItem, itemAction ); } else { From 74f60d90a6d67f1e167bd37d8d09ec8f25dbf7ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C4=83t=C4=83lin=20R=C4=83ceanu?= Date: Thu, 22 Nov 2018 12:38:29 +0000 Subject: [PATCH 5/6] Update src/qt/menuitem.cpp Co-Authored-By: evileye-uk --- src/qt/menuitem.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/qt/menuitem.cpp b/src/qt/menuitem.cpp index 328e65c892..fa729e33de 100644 --- a/src/qt/menuitem.cpp +++ b/src/qt/menuitem.cpp @@ -165,6 +165,7 @@ void wxQtAction::onActionTriggered( bool checked ) { wxMenuItem *handler = GetHandler(); wxMenu *menu = handler->GetMenu(); - if(handler->IsCheckable()) handler->Check(checked); + if ( handler->IsCheckable() ) + handler->Check(checked); menu->SendEvent( handler->GetId(), handler->IsCheckable() ? checked : -1 ); } From 69c19f29cae0ecbce53f3e495dac93a6b0466660 Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Thu, 22 Nov 2018 22:32:08 +0000 Subject: [PATCH 6/6] Restore menu tests, excluding QT where appropriate. --- tests/menu/menu.cpp | 78 +++++++++++++++++---------------------------- 1 file changed, 29 insertions(+), 49 deletions(-) diff --git a/tests/menu/menu.cpp b/tests/menu/menu.cpp index 8c0547b78c..6e6cd6f36b 100644 --- a/tests/menu/menu.cpp +++ b/tests/menu/menu.cpp @@ -90,11 +90,7 @@ private: #if wxUSE_INTL CPPUNIT_TEST( TranslatedMnemonics ); #endif // wxUSE_INTL -#ifndef __WXQT__ CPPUNIT_TEST( RadioItems ); -#else - CPPUNIT_TEST( RadioItemsWithoutMutualExclusion ); -#endif CPPUNIT_TEST( RemoveAdd ); CPPUNIT_TEST( ChangeBitmap ); WXUISIM_TEST( Events ); @@ -111,7 +107,6 @@ private: void TranslatedMnemonics(); #endif // wxUSE_INTL void RadioItems(); - void RadioItemsWithoutMutualExclusion(); void RemoveAdd(); void ChangeBitmap(); void Events(); @@ -411,10 +406,18 @@ void MenuTestCase::RadioItems() // First item of a radio group is checked by default. CPPUNIT_ASSERT( menu->IsChecked(MenuTestCase_First) ); + // Subsequent items in a group are not checked. + CPPUNIT_ASSERT( !menu->IsChecked(MenuTestCase_First + 1) ); + +#ifdef __WXQT__ + WARN("Radio check test does not work under Qt"); +#else // 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) ); + menu->Check(MenuTestCase_First, true); +#endif // Adding more radio items after a separator creates another radio group... menu->AppendSeparator(); @@ -423,74 +426,51 @@ void MenuTestCase::RadioItems() menu->AppendRadioItem(MenuTestCase_First + 4, "Radio 4"); // ... which is independent from the first one. + CPPUNIT_ASSERT( menu->IsChecked(MenuTestCase_First) ); CPPUNIT_ASSERT( menu->IsChecked(MenuTestCase_First + 2) ); +#ifdef __WXQT__ + WARN("Radio check test does not work under Qt"); +#else 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) ); + + CPPUNIT_ASSERT( menu->IsChecked(MenuTestCase_First) ); + menu->Check(MenuTestCase_First + 2, true); +#endif // 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) ); + CPPUNIT_ASSERT( menu->IsChecked(MenuTestCase_First + 2) ); + CPPUNIT_ASSERT( !menu->IsChecked(MenuTestCase_First + 5) ); +#ifdef __WXQT__ + WARN("Radio check test does not work under Qt"); +#else menu->Check( MenuTestCase_First + 5, true ); CPPUNIT_ASSERT( !menu->IsChecked(MenuTestCase_First + 3) ); + menu->Check( MenuTestCase_First + 3, true ); +#endif + // Prepend a couple of items before the first group. menu->PrependRadioItem(MenuTestCase_First + 6, "Radio 6"); menu->PrependRadioItem(MenuTestCase_First + 7, "Radio 7"); - - // Items should not be checked, as they are part of an existing group. CPPUNIT_ASSERT( !menu->IsChecked(MenuTestCase_First + 6) ); CPPUNIT_ASSERT( !menu->IsChecked(MenuTestCase_First + 7) ); + +#ifdef __WXQT__ + WARN("Radio check test does not work under Qt"); +#else 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) ); -} - -void MenuTestCase::RadioItemsWithoutMutualExclusion() -{ - 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) ); - - // Subsequent items in a group are not checked. - 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) ); - CPPUNIT_ASSERT( menu->IsChecked(MenuTestCase_First + 2) ); - - // 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 + 2) ); - - // Prepend a couple of items before the first group. - menu->PrependRadioItem(MenuTestCase_First + 6, "Radio 6"); - menu->PrependRadioItem(MenuTestCase_First + 7, "Radio 7"); - CPPUNIT_ASSERT( !menu->IsChecked(MenuTestCase_First + 6) ); - CPPUNIT_ASSERT( !menu->IsChecked(MenuTestCase_First + 7) ); // Check that the last radio group still works as expected. menu->Check(MenuTestCase_First + 4, true); CPPUNIT_ASSERT( !menu->IsChecked(MenuTestCase_First + 5) ); +#endif } void MenuTestCase::RemoveAdd()