[Old PR libui#384] add submenus api for all platforms

[mohad12211]

based on andlabs/libui#384.
notes:

• does it make sense to have an onclick for an item with a submenu? should it be userbug to do so?
• unix code in the original PR has a bug with the free function, I fixed the bug but the code looks kinda messy, no idea how to make it cleaner.
• windows will crash on calling uiMenuItemDisable on an item with submenu.
• document the api and add a note for setting an onclick callback.

[cody271]

• does it make sense to have an onclick for an item with a submenu? should it be userbug to do so?

Probably not. Any non-default callback assigned to a submenu item onclick should be userbug for now yes.

• unix code in the original PR has a bug with the free function, I fixed the bug but the code looks kinda messy, no idea how to make it cleaner.

The uiMenuAppendSubmenu() API addition itself is straightforward and obvious enough, but the unix code here will really have to be cleaned up to merge this. Its very convoluted and hard to follow.. Especially concerning when compared next to the darwin code, which is just a few lines!

[mohad12211]

Especially concerning when compared next to the darwin code, which is just a few lines!

unlike macos, on windows and linux, each window has it's own menu, which is the reason for all of these problems...

I refactored the unix code, not the best, but I think it's better, especially the FreeMenubar function which has the most problems.
each uiMenuItem has an array of the menu items on each window, when a window is closed, we remove that menu item from the array, so to find the index of the item to remove, I do a search on the first uiMenuItem (on the first uiMenu in the unix code, and on the first non-submenu uiMenu in the windows code) so I'm assuming that those menus have at least one item, is that safe to assume? should I add a userbug if a uiMenu has no items? or should I change the code and allow empty menus (although it doesn't make sense)

[mohad12211]

A way to test this is to change page2.c like this

diff --git a/test/page2.c b/test/page2.c
index abb06489..98b342de 100644
--- a/test/page2.c
+++ b/test/page2.c
@@ -39,6 +39,11 @@ static void movePage1(uiButton *b, void *data)
        moveBack = 1;
 }
 
+int closeWindow(uiWindow *w, void *data)
+{
+       return 1;
+}
+
 static void openAnotherWindow(uiButton *bb, void *data)
 {
        uiWindow *w;
@@ -54,6 +59,7 @@ static void openAnotherWindow(uiButton *bb, void *data)
        } else
                uiWindowSetChild(w, uiControl(makePage6()));
        uiWindowSetMargined(w, 1);
+       uiWindowOnClosing(w, closeWindow, NULL);
        uiControlShow(uiControl(w));
 }

now you can open and close menued and menuless windows to verify that the free function works.

as to uiprivImplBug("menu item %p (%s) still has uiWindows attached...);
should this be a userbug and require the user to destroy all windows before calling uiUninit(), or should it be implementation bug and we should destroy all windows at when calling uiQuit() or uiUninit()?

[cblc]

What does this PR need for being accepted? I believe submenus are a must-have.

[cody271]

What does this PR need for being accepted? I believe submenus are a must-have.

This PR needs more rigorous testing to be merged. Ideally a new submenu section added to the uiMenu unit tests, to better prove that it doesn't break the existing functionality. There is also a merge conflict present according to GitHub, that would have to be resolved.

[cblc]

I could try to do it myself, but I would need to learn how the unit tests work in this repository, because at first glance my impression is that user review of the results would be necessary for UI-related tests. I will take a look at how the tests work and I will try to add the tests if possible.

[cody271]

I could try to do it myself, but I would need to learn how the unit tests work in this repository, because at first glance my impression is that user review of the results would be necessary for UI-related tests. I will take a look at how the tests work and I will try to add the tests if possible.

There are actually three different sets of tests:

qa is a manual test suite requiring user review of each step performed.
tester is the original, and a somewhat random collection of tests at this point.
unit is a fully automated unit test suite built with cmocka.