diff --git a/include/wx/qt/menuitem.h b/include/wx/qt/menuitem.h index 92ca51f34a..c173d4c6b1 100644 --- a/include/wx/qt/menuitem.h +++ b/include/wx/qt/menuitem.h @@ -40,6 +40,12 @@ public: virtual QAction *GetHandle() const; virtual void SetFont(const wxFont& font); + +#if wxUSE_ACCEL + virtual void AddExtraAccel(const wxAcceleratorEntry& accel) override; + virtual void ClearExtraAccels() override; +#endif // wxUSE_ACCEL + private: // Qt is using an action instead of a menu item. wxQtAction *m_qtAction; diff --git a/interface/wx/menuitem.h b/interface/wx/menuitem.h index 228ffd3f0f..b83f26515e 100644 --- a/interface/wx/menuitem.h +++ b/interface/wx/menuitem.h @@ -439,7 +439,10 @@ public: \\t followed by a valid key combination (e.g. CTRL+V). Its general syntax is any combination of @c "CTRL", @c "RAWCTRL", @c "ALT" and @c "SHIFT" strings (case doesn't matter) separated by either - @c '-' or @c '+' characters and followed by the accelerator itself. + @c '-' or @c '+' characters and followed by the accelerator itself. Note that + the displayed accelerator can differ from the string specified here though, + e.g. Ctrl+X could be actually shown on the screen when Ctrl-X + is used. Notice that @c CTRL corresponds to the "Ctrl" key on most platforms but not under macOS where it is mapped to "Cmd" key on Mac keyboard. Usually this is exactly what you want in portable code but if you diff --git a/src/qt/menu.cpp b/src/qt/menu.cpp index be0a12a0b4..8907faed67 100644 --- a/src/qt/menu.cpp +++ b/src/qt/menu.cpp @@ -11,6 +11,7 @@ #include "wx/menu.h" #include "wx/qt/private/utils.h" #include "wx/qt/private/converter.h" +#include "wx/qt/private/winevent.h" #include "wx/stockitem.h" #include @@ -22,6 +23,36 @@ static void ApplyStyle( QMenu *qtMenu, long style ) qtMenu->setTearOffEnabled( true ); } +// wxQtActionGroup: an exclusive group which synchronizes QActions in +// QActionGroup with their wx wrappers. +class wxQtActionGroup : public QActionGroup, public wxQtSignalHandler +{ + +public: + explicit wxQtActionGroup( wxMenu* handler ) + : QActionGroup( handler->GetHandle() ), + wxQtSignalHandler( handler ) + { + setExclusive(true); + + connect( this, &QActionGroup::triggered, this, &wxQtActionGroup::triggered ); + } + + void AddAction( QAction* action ) + { + m_activeAction = QActionGroup::addAction(action); + } + +private: + void triggered ( QAction* action ); + + QAction* m_activeAction; +}; + +//----------------------------------------------------------------------------- +// wxMenu implementation +//----------------------------------------------------------------------------- + wxMenu::wxMenu(long style) : wxMenuBase( style ) { @@ -80,8 +111,8 @@ static void InsertMenuItemAction( const wxMenu *menu, const wxMenuItem *previous } else { - QActionGroup *actionGroup = new QActionGroup( qtMenu ); - actionGroup->addAction( itemAction ); + auto actionGroup = new wxQtActionGroup( const_cast(menu) ); + actionGroup->AddAction( itemAction ); item->Check(); wxASSERT_MSG( itemAction->actionGroup() == actionGroup, "Must be the same action group" ); } @@ -309,3 +340,14 @@ QWidget *wxMenuBar::GetHandle() const { return m_qtMenuBar; } + +void wxQtActionGroup::triggered( QAction* action ) +{ + if ( action != m_activeAction ) + { + if ( m_activeAction->isCheckable() ) + m_activeAction->setChecked(false); + + m_activeAction = action; + } +} diff --git a/src/qt/menuitem.cpp b/src/qt/menuitem.cpp index 506b7f1408..3aa2fde730 100644 --- a/src/qt/menuitem.cpp +++ b/src/qt/menuitem.cpp @@ -26,7 +26,10 @@ public: wxItemKind kind, wxMenu *subMenu, wxMenuItem *menuItem ); // Set the action shortcut to correspond to the accelerator specified by - // the given label. + // the given label. They set the primary shortcut the first time they are + // called, and set additional shortcuts/accels for subsequent calls. if + // text is empty, any axtra accelerators will be removed. + void UpdateShortcuts(const wxString& text); void UpdateShortcutsFromLabel(const wxString& text); wxMenu* GetMenu() const @@ -34,7 +37,16 @@ public: return static_cast(wxQtSignalHandler::GetHandler()); } + // Convert hyphenated shortcuts to use the plus sign (+) which Qt understands. + static wxString Normalize(const wxString& text) + { + QString normalized = wxQtConvertString( text ); + normalized.replace(QRegExp("([^+-])[-]"), "\\1+"); + return wxQtConvertString( normalized ); + } + private: + void onActionToggled( bool checked ); void onActionTriggered( bool checked ); const wxWindowID m_mitemId; @@ -54,18 +66,22 @@ wxMenuItem::wxMenuItem(wxMenu *parentMenu, int id, const wxString& text, const wxString& help, wxItemKind kind, wxMenu *subMenu) : wxMenuItemBase( parentMenu, id, text, help, kind, subMenu ) { - m_qtAction = new wxQtAction( parentMenu, id, text, help, kind, subMenu, this ); + m_qtAction = new wxQtAction( parentMenu, id, + wxQtAction::Normalize( text ), + help, kind, subMenu, this ); } void wxMenuItem::SetItemLabel( const wxString &label ) { - wxMenuItemBase::SetItemLabel( label ); + const wxString qtlabel = wxQtAction::Normalize( label ); - m_qtAction->UpdateShortcutsFromLabel( label ); + wxMenuItemBase::SetItemLabel( qtlabel ); - m_qtAction->setText( wxQtConvertString( label )); + m_qtAction->UpdateShortcutsFromLabel( qtlabel ); + + m_qtAction->setText( wxQtConvertString( qtlabel )); } @@ -146,6 +162,22 @@ QAction *wxMenuItem::GetHandle() const return m_qtAction; } +#if wxUSE_ACCEL +void wxMenuItem::AddExtraAccel(const wxAcceleratorEntry& accel) +{ + wxMenuItemBase::AddExtraAccel(accel); + + m_qtAction->UpdateShortcuts( accel.ToRawString() ); +} + +void wxMenuItem::ClearExtraAccels() +{ + wxMenuItemBase::ClearExtraAccels(); + + m_qtAction->UpdateShortcuts( wxString() ); +} +#endif // wxUSE_ACCEL + //============================================================================= wxQtAction::wxQtAction( wxMenu *handler, int id, const wxString &text, const wxString &help, @@ -180,6 +212,7 @@ wxQtAction::wxQtAction( wxMenu *handler, int id, const wxString &text, const wxS break; } + connect( this, &QAction::toggled, this, &wxQtAction::onActionToggled ); connect( this, &QAction::triggered, this, &wxQtAction::onActionTriggered ); UpdateShortcutsFromLabel( text ); @@ -191,13 +224,47 @@ void wxQtAction::UpdateShortcutsFromLabel(const wxString& text) const wxString accelStr = text.AfterFirst('\t'); if ( !accelStr.empty() ) { - setShortcut( QKeySequence( wxQtConvertString(accelStr) ) ); + UpdateShortcuts(accelStr); } +#else + wxUnusedVar(text); #endif // wxUSE_ACCEL } +void wxQtAction::UpdateShortcuts(const wxString& text) +{ +#if wxUSE_ACCEL + QList shortcuts = this->shortcuts(); + + if ( text.empty() ) + { + if ( shortcuts.size() > 1 ) + { + // Keep the primary shortcut only and get rid of the rest + setShortcut( shortcuts.first() ); + } + } + else + { + QKeySequence keySequence = QKeySequence( wxQtConvertString( text ) ); + + if ( !shortcuts.contains(keySequence) ) + { + shortcuts.push_back(keySequence); + setShortcuts( shortcuts ); + } + } +#else + wxUnusedVar(text); +#endif // wxUSE_ACCEL +} + +void wxQtAction::onActionToggled( bool checked ) +{ + GetMenu()->Check(m_mitemId, checked); +} + void wxQtAction::onActionTriggered( bool checked ) { - GetMenu()->Check(m_mitemId, checked); GetMenu()->SendEvent(m_mitemId, m_isCheckable ? checked : -1 ); } diff --git a/tests/menu/menu.cpp b/tests/menu/menu.cpp index d97e15f209..78af3ceb25 100644 --- a/tests/menu/menu.cpp +++ b/tests/menu/menu.cpp @@ -444,15 +444,11 @@ void MenuTestCase::RadioItems() // 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(); @@ -464,30 +460,22 @@ void MenuTestCase::RadioItems() 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) ); 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 + 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"); @@ -495,9 +483,6 @@ void MenuTestCase::RadioItems() 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) ); @@ -505,7 +490,6 @@ void MenuTestCase::RadioItems() // 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()