diff --git a/modules/juce_gui_basics/menus/juce_PopupMenu.cpp b/modules/juce_gui_basics/menus/juce_PopupMenu.cpp index 276f1203af..5b369852b8 100644 --- a/modules/juce_gui_basics/menus/juce_PopupMenu.cpp +++ b/modules/juce_gui_basics/menus/juce_PopupMenu.cpp @@ -41,8 +41,20 @@ struct PopupMenu::HelperClasses class MouseSourceState; struct MenuWindow; -static bool canBeTriggered (const PopupMenu::Item& item) noexcept { return item.isEnabled && item.itemID != 0 && ! item.isSectionHeader; } -static bool hasActiveSubMenu (const PopupMenu::Item& item) noexcept { return item.isEnabled && item.subMenu != nullptr && item.subMenu->items.size() > 0; } +static bool canBeTriggered (const PopupMenu::Item& item) noexcept +{ + return item.isEnabled + && item.itemID != 0 + && ! item.isSectionHeader + && (item.customComponent == nullptr || item.customComponent->isTriggeredAutomatically()); +} + +static bool hasActiveSubMenu (const PopupMenu::Item& item) noexcept +{ + return item.isEnabled + && item.subMenu != nullptr + && item.subMenu->items.size() > 0; +} //============================================================================== struct HeaderItemComponent : public PopupMenu::CustomComponent @@ -73,6 +85,11 @@ struct HeaderItemComponent : public PopupMenu::CustomComponent idealWidth += idealWidth / 4; } + std::unique_ptr createAccessibilityHandler() override + { + return nullptr; + } + const Options& options; JUCE_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR (HeaderItemComponent) @@ -164,6 +181,11 @@ struct ItemComponent : public Component } } + static bool isAccessibilityHandlerRequired (const PopupMenu::Item& item) + { + return item.isSectionHeader || hasActiveSubMenu (item) || canBeTriggered (item); + } + PopupMenu::Item item; private: @@ -173,7 +195,8 @@ private: public: explicit ItemAccessibilityHandler (ItemComponent& itemComponentToWrap) : AccessibilityHandler (itemComponentToWrap, - AccessibilityRole::menuItem, + isAccessibilityHandlerRequired (itemComponentToWrap.item) ? AccessibilityRole::menuItem + : AccessibilityRole::ignored, getAccessibilityActions (*this, itemComponentToWrap)), itemComponent (itemComponentToWrap) { @@ -209,12 +232,6 @@ private: item.parentWindow.setCurrentlyHighlightedChild (&item); }; - auto onPress = [&item] - { - item.parentWindow.setCurrentlyHighlightedChild (&item); - item.parentWindow.triggerCurrentlyHighlightedItem(); - }; - auto onToggle = [&handler, &item, onFocus] { if (handler.getCurrentState().isSelected()) @@ -224,9 +241,17 @@ private: }; auto actions = AccessibilityActions().addAction (AccessibilityActionType::focus, std::move (onFocus)) - .addAction (AccessibilityActionType::press, std::move (onPress)) .addAction (AccessibilityActionType::toggle, std::move (onToggle)); + if (canBeTriggered (item.item)) + { + actions.addAction (AccessibilityActionType::press, [&item] + { + item.parentWindow.setCurrentlyHighlightedChild (&item); + item.parentWindow.triggerCurrentlyHighlightedItem(); + }); + } + if (hasActiveSubMenu (item.item)) { auto showSubMenu = [&item] @@ -1176,10 +1201,7 @@ struct MenuWindow : public Component void triggerCurrentlyHighlightedItem() { - if (currentChild != nullptr - && canBeTriggered (currentChild->item) - && (currentChild->item.customComponent == nullptr - || currentChild->item.customComponent->isTriggeredAutomatically())) + if (currentChild != nullptr && canBeTriggered (currentChild->item)) { dismissMenu (¤tChild->item); } @@ -1720,7 +1742,7 @@ PopupMenu::Item&& PopupMenu::Item::setImage (std::unique_ptr newImage) void PopupMenu::addItem (Item newItem) { // An ID of 0 is used as a return value to indicate that the user - // didn't pick anything, so you shouldn't use it as the ID for an item.. + // didn't pick anything, so you shouldn't use it as the ID for an item. jassert (newItem.itemID != 0 || newItem.isSeparator || newItem.isSectionHeader || newItem.subMenu != nullptr); @@ -1828,12 +1850,23 @@ void PopupMenu::addColouredItem (int itemResultID, String itemText, Colour itemT void PopupMenu::addCustomItem (int itemResultID, std::unique_ptr cc, - std::unique_ptr subMenu) + std::unique_ptr subMenu, + const String& itemTitle) { Item i; + i.text = itemTitle; i.itemID = itemResultID; i.customComponent = cc.release(); i.subMenu.reset (createCopyIfNotNull (subMenu.get())); + + // If this assertion is hit, this item will be visible to screen readers but with + // no name, which may be confusing to users. + // It's probably a good idea to add a title for this menu item that describes + // the meaning of the item, or the contents of the submenu, as appropriate. + // If you don't want this menu item to be press-able directly, pass "false" to the + // constructor of the CustomComponent. + jassert (! (HelperClasses::ItemComponent::isAccessibilityHandlerRequired (i) && itemTitle.isEmpty())); + addItem (std::move (i)); } @@ -1841,11 +1874,12 @@ void PopupMenu::addCustomItem (int itemResultID, Component& customComponent, int idealWidth, int idealHeight, bool triggerMenuItemAutomaticallyWhenClicked, - std::unique_ptr subMenu) + std::unique_ptr subMenu, + const String& itemTitle) { auto comp = std::make_unique (customComponent, idealWidth, idealHeight, triggerMenuItemAutomaticallyWhenClicked); - addCustomItem (itemResultID, std::move (comp), std::move (subMenu)); + addCustomItem (itemResultID, std::move (comp), std::move (subMenu), itemTitle); } void PopupMenu::addSubMenu (String subMenuName, PopupMenu subMenu, bool isActive) @@ -2211,15 +2245,13 @@ void PopupMenu::setItem (CustomComponent& c, const Item* itemToUse) } //============================================================================== +PopupMenu::CustomComponent::CustomComponent() : CustomComponent (true) {} + PopupMenu::CustomComponent::CustomComponent (bool autoTrigger) : triggeredAutomatically (autoTrigger) { } -PopupMenu::CustomComponent::~CustomComponent() -{ -} - void PopupMenu::CustomComponent::setHighlighted (bool shouldBeHighlighted) { isHighlighted = shouldBeHighlighted; diff --git a/modules/juce_gui_basics/menus/juce_PopupMenu.h b/modules/juce_gui_basics/menus/juce_PopupMenu.h index c1a210b792..95677219b9 100644 --- a/modules/juce_gui_basics/menus/juce_PopupMenu.h +++ b/modules/juce_gui_basics/menus/juce_PopupMenu.h @@ -333,11 +333,15 @@ public: Note that native macOS menus do not support custom components. + itemTitle will be used as the fallback text for this item, and will + be exposed to screen reader clients. + @see CustomComponent */ void addCustomItem (int itemResultID, std::unique_ptr customComponent, - std::unique_ptr optionalSubMenu = nullptr); + std::unique_ptr optionalSubMenu = nullptr, + const String& itemTitle = {}); /** Appends a custom menu item that can't be used to trigger a result. @@ -350,6 +354,9 @@ public: menu ID specified in itemResultID. If this is false, the menu item can't be triggered, so itemResultID is not used. + itemTitle will be used as the fallback text for this item, and will + be exposed to screen reader clients. + Note that native macOS menus do not support custom components. */ void addCustomItem (int itemResultID, @@ -357,7 +364,8 @@ public: int idealWidth, int idealHeight, bool triggerMenuItemAutomaticallyWhenClicked, - std::unique_ptr optionalSubMenu = nullptr); + std::unique_ptr optionalSubMenu = nullptr, + const String& itemTitle = {}); /** Appends a sub-menu. @@ -820,15 +828,22 @@ public: public SingleThreadedReferenceCountedObject { public: + /** Creates a custom item that is triggered automatically. */ + CustomComponent(); + /** Creates a custom item. + If isTriggeredAutomatically is true, then the menu will automatically detect a mouse-click on this component and use that to invoke the menu item. If it's false, then it's up to your class to manually trigger the item when it wants to. - */ - CustomComponent (bool isTriggeredAutomatically = true); - /** Destructor. */ - ~CustomComponent() override; + If isTriggeredAutomatically is true, then an accessibility handler 'wrapper' + will be created for the item that allows pressing, focusing, and toggling. + If isTriggeredAutomatically is false, and the item has no submenu, then + no accessibility wrapper will be created and your component must be + independently accessible. + */ + explicit CustomComponent (bool isTriggeredAutomatically); /** Returns a rectangle with the size that this component would like to have.