#5127: Improve PopupMenu::show method to actually detach hidden items…

… from and attach visible ones to the menu before it is shown

libs/wxutil/menu/PopupMenu.cpp

@@ -12,8 +12,7 @@ PopupMenu::PopupMenu() :
 }
 
 PopupMenu::~PopupMenu()
-{
-}
+{}
 
 // Add a named menu item
 void PopupMenu::addItem(wxMenuItem* widget,
@@ -40,24 +39,77 @@ void PopupMenu::addSeparator()
     );
 }
 
+void PopupMenu::attachItem(const ui::IMenuItemPtr& item, int position)
+{
+    if (item->getMenuItem()->GetMenu() == nullptr)
+    {
+        Insert(position, item->getMenuItem());
+    }
+}
+
+void PopupMenu::detachItem(const ui::IMenuItemPtr& item)
+{
+    if (item->getMenuItem()->GetMenu() != nullptr)
+    {
+        Remove(item->getMenuItem());
+    }
+}
+
+bool PopupMenu::itemIsVisible(int index, int menuPosition)
+{
+    const auto& item = _menuItems[index];
+
+    // Call the user visibility check
+    bool visible = item->isVisible();
+
+    if (!visible)
+    {
+        // Item is hidden
+        return false;
+    }
+
+    // Special treatment for separators
+    if (item->getMenuItem()->IsSeparator())
+    {
+        // Trim separators from the beginning or the end of the menu
+        if (menuPosition == 0 || index + 1 >= _menuItems.size())
+        {
+            return false;
+        }
+
+        // Skip all consecutive seprators except for the last
+        if (index + 1 < _menuItems.size() && _menuItems[index + 1]->getMenuItem()->IsSeparator())
+        {
+            return false;
+        }
+    }
+
+    return true;
+}
+
 void PopupMenu::show(wxWindow* parent)
 {
-	// Iterate through the list of MenuItems, enabling or disabling each widget
-	// based on its SensitivityTest
-	for (auto item : _menuItems)
+	// Iterate through the list of MenuItems, showing/hiding and enabling/disabling each widget
+	// based on its test functions
+    int position = 0;
+
+	for (auto i = 0; i < _menuItems.size(); ++i)
 	{
-		bool visible = item->isVisible();
+        const auto& item = _menuItems[i];
 
-		if (visible)
-		{
-			// Visibility check passed
-			item->getMenuItem()->Enable(item->isSensitive());
-		}
-		else
-		{
-			// Visibility check failed, skip sensitivity check
-			item->getMenuItem()->Enable(false);
-		}
+        if (!itemIsVisible(i, position))
+        {
+            // Visibility check failed, detach and skip sensitivity check
+            // Don't increase the position counter
+            detachItem(item);
+            continue;
+        }
+
+		// Make sure visible items are attached, increase position
+        attachItem(item, position++);
+
+        // Perform the sensitivity check on visible items
+		item->getMenuItem()->Enable(item->isSensitive());
 	}
 
 	parent->PopupMenu(this);

libs/wxutil/menu/PopupMenu.h

@@ -5,7 +5,7 @@
 #include <functional>
 #include <memory>
 #include "util/Noncopyable.h"
-#include <list>
+#include <vector>
 
 #include <wx/menu.h>
 
@@ -24,7 +24,7 @@ class PopupMenu :
 {
 private:
 	// List of menu items
-	typedef std::list<ui::IMenuItemPtr> MenuItemList;
+	typedef std::vector<ui::IMenuItemPtr> MenuItemList;
 	MenuItemList _menuItems;
 
 private:
@@ -80,6 +80,13 @@ class PopupMenu :
 
 protected:
 	virtual void foreachMenuItem(const std::function<void(const ui::IMenuItemPtr&)>& functor);
+
+    // Returns true if the given item is visible for the projected menuPosition
+    virtual bool itemIsVisible(int index, int menuPosition);
+
+private:
+    void attachItem(const ui::IMenuItemPtr& item, int position);
+    void detachItem(const ui::IMenuItemPtr& item);
 };
 typedef std::shared_ptr<PopupMenu> PopupMenuPtr;