From 63291aea92ea6cf91f44639ee8668a45bb3f8a0e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Matthias=20K=C3=B6fferlein?= <matthias@koefferlein.de>
Date: Fri, 13 Nov 2020 01:52:53 +0100
Subject: [PATCH] Fixed (hopefully) #658 - needs some testing (#665)

This patch introduces a slightly more elaborate scheme for building
menus on MacOS. Under normal circumstances, this scheme should maintain
the menu item order as far as possible when Qt is trying to sync with
Mac's menu bar.

Co-authored-by: Matthias <klayoutmatthias>
---
 src/laybasic/laybasic/layAbstractMenu.cc | 50 +++++++++++++++++++-----
 1 file changed, 40 insertions(+), 10 deletions(-)

diff --git a/src/laybasic/laybasic/layAbstractMenu.cc b/src/laybasic/laybasic/layAbstractMenu.cc
index cad1e30a6..2247ce1f3 100644
--- a/src/laybasic/laybasic/layAbstractMenu.cc
+++ b/src/laybasic/laybasic/layAbstractMenu.cc
@@ -1126,6 +1126,26 @@ AbstractMenu::build_detached (const std::string &name, QFrame *mbar)
   menu_layout->addStretch (1);
 }
 
+static QAction *insert_action_after (QWidget *widget, QAction *after, QAction *action)
+{
+  QList<QAction *> actions = widget->actions ();
+
+  QAction *before = 0;
+  if (after == 0) {
+    if (! actions.isEmpty ()) {
+      before = actions.front ();
+    }
+  } else {
+    int index = actions.indexOf (after);
+    if (index >= 0 && index + 1 < actions.size ()) {
+      before = actions [index + 1];
+    }
+  }
+  widget->insertAction (before, action);
+
+  return action;
+}
+
 void
 AbstractMenu::build (QMenuBar *mbar, QToolBar *tbar)
 {
@@ -1140,6 +1160,8 @@ AbstractMenu::build (QMenuBar *mbar, QToolBar *tbar)
     present_actions.insert (std::make_pair (id_from_action (*i), *i));
   }
 
+  QAction *prev_action = 0;
+
   for (std::list<AbstractMenuItem>::iterator c = m_root.children.begin (); c != m_root.children.end (); ++c) {
 
     if (c->has_submenu ()) {
@@ -1172,19 +1194,20 @@ AbstractMenu::build (QMenuBar *mbar, QToolBar *tbar)
         if (c->menu () == 0) {
           QMenu *menu = new QMenu ();
           menu->setTitle (tl::to_qstring (c->action ().get_title ()));
-          mbar->addMenu (menu);
           c->set_action (Action (new ActionHandle (menu)), true);
+          prev_action = insert_action_after (mbar, prev_action, menu->menuAction ());
         } else {
           //  Move the action to the end if present in the menu already
           std::set<std::pair<size_t, QAction *> >::iterator a = present_actions.find (std::make_pair (id_from_action (c->menu ()->menuAction ()), c->menu ()->menuAction ()));
           if (a != present_actions.end ()) {
             if (s_can_move_menu) {
               mbar->removeAction (a->second);
-              mbar->addMenu (c->menu ());
+              insert_action_after (mbar, prev_action, a->second);
             }
+            prev_action = a->second;
             present_actions.erase (*a);
           } else {
-            mbar->addMenu (c->menu ());
+            prev_action = insert_action_after (mbar, prev_action, c->menu ()->menuAction ());
           }
         }
 
@@ -1193,17 +1216,20 @@ AbstractMenu::build (QMenuBar *mbar, QToolBar *tbar)
       }
 
     } else {
+
       //  Move the action to the end if present in the menu already
       std::set<std::pair<size_t, QAction *> >::iterator a = present_actions.find (std::make_pair (id_from_action (c->action ().qaction ()), c->action ().qaction ()));
       if (a != present_actions.end ()) {
         if (s_can_move_menu) {
           mbar->removeAction (a->second);
-          mbar->addAction (c->action ().qaction ());
+          insert_action_after (mbar, prev_action, a->second);
         }
+        prev_action = a->second;
         present_actions.erase (*a);
       } else {
-        mbar->addAction (c->action ().qaction ());
+        prev_action = insert_action_after (mbar, prev_action, c->action ().qaction ());
       }
+
     }
 
   }
@@ -1223,6 +1249,8 @@ AbstractMenu::build (QMenu *m, std::list<AbstractMenuItem> &items)
     present_actions.insert (std::make_pair (id_from_action (*i), *i));
   }
 
+  QAction *prev_action = 0;
+
   for (std::list<AbstractMenuItem>::iterator c = items.begin (); c != items.end (); ++c) {
 
     if (c->has_submenu ()) {
@@ -1232,19 +1260,20 @@ AbstractMenu::build (QMenu *m, std::list<AbstractMenuItem> &items)
         //  menu with a given action. The action is provided by addMenu instead.
         QMenu *menu = new QMenu ();
         menu->setTitle (tl::to_qstring (c->action ().get_title ()));
-        m->addMenu (menu);
         c->set_action (Action (new ActionHandle (menu)), true);
+        prev_action = insert_action_after (m, prev_action, menu->menuAction ());
       } else {
         //  Move the action to the end if present in the menu already
         std::set<std::pair<size_t, QAction *> >::iterator a = present_actions.find (std::make_pair (id_from_action (c->menu ()->menuAction ()), c->menu ()->menuAction ()));
         if (a != present_actions.end ()) {
           if (s_can_move_menu) {
             m->removeAction (a->second);
-            m->addMenu (c->menu ());
+            insert_action_after (m, prev_action, a->second);
           }
+          prev_action = a->second;
           present_actions.erase (*a);
         } else {
-          m->addMenu (c->menu ());
+          prev_action = insert_action_after (m, prev_action, c->menu ()->menuAction ());
         }
       }
 
@@ -1257,11 +1286,12 @@ AbstractMenu::build (QMenu *m, std::list<AbstractMenuItem> &items)
       if (a != present_actions.end ()) {
         if (s_can_move_menu) {
           m->removeAction (a->second);
-          m->addAction (c->action ().qaction ());
+          insert_action_after (m, prev_action, a->second);
         }
+        prev_action = a->second;
         present_actions.erase (*a);
       } else {
-        m->addAction (c->action ().qaction ());
+        prev_action = insert_action_after (m, prev_action, c->action ().qaction ());
       }
 
     }