Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Menus API work #126

Closed
ocornut opened this issue Feb 9, 2015 · 68 comments
Closed

Menus API work #126

ocornut opened this issue Feb 9, 2015 · 68 comments
Labels
enhancement menus menu bars, menu items

Comments

@ocornut
Copy link
Owner

ocornut commented Feb 9, 2015

Discussion for an upcoming menu api (still being designed).
If you have ideas or references of menu being implemented in imgui systems please post here!

Some thoughts:

  • Menu items typically contains a label, an optional checkmark, an optional local shortcut, an optional global shorcut, an optional link to a submenu, an optional Icon. Menus will be created and developed in a way analogous to the existing patterns in ImGui. API needs to be terse for the common case.
  • We should be able to use most types of widgets within a menu. Menu specialize for "menu items" but other widgets should be usable. We should be able to pack sliders or images into a menu.
  • We need "menu bars" as the common way to layout menus. Menu bars can be inside individual windows but we need to provide an easy way to have a "top of the screen" menu bar. Also provide easy way to hide menus.
  • We want popups menu. They are likely spawned from an event (e.g. clicking a button). This is posing a small problem: if a menu appears upon reacting to an event, we need to design a coding pattern that will allow the menu to stay on after the event has happened.
  • Support local keyboard shortcuts, We can use Windows syntax of using & (e.g. "&Save") for convenience.
  • Support general "global" shortcuts (e.g. "CTRL+S"). As a design goal of ImGui we want to avoid code and state duplication, so I'd like the ImGui system to handle shortcuts for the user. It will be optional but likely available by default. So the program can have a single entry point for "Save" whether it is activated via clicking in the menu or via pressing the shortcut. The way it would work is that when a global shortcut scheme is activated, the menu functions always notify the user code to develop its content so ImGui can parse and execute the shortcuts as they are declared, but the actual menu is not layed out nor rendered. The shortcut scheme can be disabled on a per-menu/window/global basis. In particular, procedurally generated menus that may have infinite depth will need to be able to disable the global shortcut scheme. In its "closed" state, the system has to be as lightweight as if the user were testing a bunch of shortcuts themselves. The scope of shortcuts can be dependent on factor such as if the parent window is focused so they aren't always "global". The user should also be able to display the label for a shortcut in the menu without letting ImGui handle the shortcut itself.
  • Menu navigation may requires ImGui to support more thorough keyboard navigation (currently we only handle TAB and Shift+TAB for navigation).
  • Menus needs to scroll if they can't fit in screen.
  • Menus needs to position themselves nicely when opened from a parent menu.
  • Search in menus like OSX does?
@ocornut
Copy link
Owner Author

ocornut commented Feb 9, 2015

To mimic similar existing api, the "default" could be something like this:

IMGUI_API bool MenuBegin(const char* label, flags);
IMGUI_API bool Item(const char* label, const char* shortcut = "", bool enabled = true, bool selected = false);
IMGUI_API bool Item(const char* label, const char* shortcut, bool enabled, bool* p_selected);

Using overload of Item(), Not considering extra parameters to MenuBegin yet.

if (ImGui::BeginMenu("&File"))
{
    if (ImGui::Item("&New", "CTRL+N")) {}
    if (ImGui::Item("&Open..", "CTRL+O")) {}
    if (ImGui::Item("&Save", "CTRL+S") {}
    if (ImGui::Item("Save &As..")) {}
    ImGui::Separator();
    if (ImGui::BeginMenu("Recent Files"))
    {
        // iterate items..
        // e.g. if (ImGui::Item("&1. filename.txt")) {}
        ImGui::EndMenu();
    }
    ImGui::Separator();
    if (ImGui::Item("Quit", "CTRL+W")) {}
    ImGui::EndMenu();
}

But something along those lines.
We could potentially store the activated item and have an api to query it back before the end of a menu or later (it would match the char* the user passed), which would allow to write the "execution" code later. If this ends up a useful pattern we can pass around a parameter (id, pointer, etc.) to make the execution code shorter to write. But it make sense to focus on immediate execution as the common case since this is analogous to everything else in ImGui.

The order of parameters makes it a bit tricky if we want to add a printf-format style version of the function, but I imagine this is rather rare (e.g. for filenames in a Recent Files you may want to embed a local shortcut). I don't have a solution I am happy with for that.

@emoon
Copy link
Contributor

emoon commented Feb 9, 2015

I would imagine that Popup menu can work in a similar way:

ImGui::BeginPopupMenu();
if (ImGui::Item("Foo")) {}
ImGui::EndMenu();

@ocornut
Copy link
Owner Author

ocornut commented Feb 9, 2015

Yes, but you'd need to keep track of the menu lifetime somehow. Pseudo-code:

static bool popup_is_active = false;
if (ImGui::Button("click me"))
    popup_is_active = true;

if (ImGui::BeginPopupMenu(&popup_is_active))
{
    if (ImGui::Item("Foo")) {}
    ImGui::EndMenu();
}

ImGui would keep track of the menu being active. Clicking outside the menu or pressing Escape would deactivate it.

Or better?

if (ImGui::Button("clickme"))
   ImGui::OpenPopupMenu("popup_id");

if (ImGui::BeginPopupMenu("popup_id"))
{
    if (ImGui::Item("Foo")) {}
    ImGui::EndMenu();
}

Except ID can be string / idx / pointer the same way ImGui::TreeNode() allows for all three. Then within a tree we can test for right-click over a node and start a popup that would run for this specific node.

@emoon
Copy link
Contributor

emoon commented Feb 9, 2015

How would it work for mouse? Something like this?

if (ImGui::IsRmbDown())
{
  if (ImGui::BeginPopupMenu("popup_id"))
  {
    if (ImGui::Item("Foo")) {}
    ImGui::EndMenu();
  }
}

I think this is quite common because you may not press an actual control to bring up a Popup menu.

@emoon
Copy link
Contributor

emoon commented Feb 9, 2015

Hum.. I think I like approach 1 better even if I don't have any strong opinions about it.

@ocornut
Copy link
Owner Author

ocornut commented Feb 9, 2015

Under the above scheme it would be somethng like

if (!ImGui::IsAnyItemActive() && ImGui::IsMouseClicked(2))
{
    ImGui::OpenPopup("foobar");
}
if (ImGui::BeginPopup("foobar"))
{
   ...
}

Which leaves a chance for another item to be active or another more specialized menu to run. Note how the Begin() test is outside of the mouse test block.

With the Open/Begin scheme maybe Begin is a misleading word. It is more analogous to TreeNode() which tests a state (is the tree node open). Or if we keep the bool on the user side as in the first example of my previous post, then we become more analogous to Begin() so it is clearer but the common case requires a little more work. So to reiterate:

static bool menu_opened = false;
if (!ImGui::IsAnyItemActive() && ImGui::IsMouseClicked(2))
{
    menu_opened = true;
}
if (ImGui::BeginPopup(&menu_opened))
{
   ...
}

@emoon
Copy link
Contributor

emoon commented Feb 9, 2015

Alright. That makes sense.

@ocornut
Copy link
Owner Author

ocornut commented Feb 9, 2015

Add to that: as a user I would like to be able to right-click on any tree node and have popup with a "Copy" button that allows copying the entire node contents to the clipboard (the logging api allows to do that easily). I think it'd be a sensible default in many cases.

Now, ideally I don't want to user to have to add the popup code after every TreeNode call. The user can create their own replacement helper for TreeNode() that does both. Or we can have a dedicated functionality just for this case were TreeNode can automatically spawn a popup with a "Copy" function when right-clicking. Or we can have a system where the user can push a callback to provide a default popup menu. Either way it would need to allow the user code to ignore that menu and create their own. Not sure how to approach that exactly but it is solvable. I guess this is more of a feature of the general/tree API rather than specific to menus.

@ocornut
Copy link
Owner Author

ocornut commented Mar 26, 2015

FYI i tested and added a demo of the idea of creating popup menus with the existing API.

popup

static bool popup_open = false;
static int selected_fish = -1;
const char* fishes[] = { "Bream", "Mackerel", "Pollock", "Tilefish" };
if (ImGui::Button("Select.."))
{
    popup_open = true;
    ImGui::SetNextWindowPos(ImGui::GetMousePos());
}
ImGui::SameLine();
ImGui::Text(selected_fish == -1 ? "<None>" : fishes[selected_fish]);
if (popup_open)
{
    ImGui::PushStyleVar(ImGuiStyleVar_WindowRounding, 0.0f);
    ImGui::Begin("##Popup", &popup_open, ImGuiWindowFlags_NoMove|ImGuiWindowFlags_NoTitleBar|ImGuiWindowFlags_NoSavedSettings|ImGuiWindowFlags_AlwaysAutoResize);
    if (!ImGui::IsWindowFocused())
        popup_open = false;
    for (size_t i = 0; i < IM_ARRAYSIZE(fishes); i++)
        if (ImGui::Selectable(fishes[i], false))
        {
            selected_fish = i;
            popup_open = false;
        }
    ImGui::End();
    ImGui::PopStyleVar();
}

There's a few problems but it works. I hope by the next version I'll be able to turn that into a 2nd class citizen (with shorter API, automatic positioning, clipping within screen, perhaps ALT+letter shortcuts).

Some of the most immediate problems with this:

  • The Begin() call is long.
  • Won't position elegantly if size is too big.
  • Creating a tooltip (within the popup window or elsewhere, e.g. hovering another window that has code to create a tooltip) will unfocus and remove the popup.

@ocornut
Copy link
Owner Author

ocornut commented Mar 26, 2015

I added a simple BeginPopup() / EndPopup() API to wrap the pattern above.

static bool popup_open = false;
static int selected_fish = -1;
const char* fishes[] = { "Bream", "Mackerel", "Pollock", "Tilefish" };
if (ImGui::Button("Select.."))
    popup_open = true;
ImGui::SameLine();
ImGui::Text(selected_fish == -1 ? "<None>" : fishes[selected_fish]);

if (popup_open)
{
    ImGui::BeginPopup(&popup_open))
    for (size_t i = 0; i < IM_ARRAYSIZE(fishes); i++)
        if (ImGui::Selectable(fishes[i], false))
        {
            selected_fish = i;
            popup_open = false;
        }
    ImGui::EndPopup();
}

@ocornut
Copy link
Owner Author

ocornut commented Mar 26, 2015

I tweaked that enough that BeginPopup() / EndPopup() is pretty much usable now.
You can fill the menu with
if (Selectable("Item.."))

You won't get icons/shortcuts/submenus expected in a normal menu yet.
EDIT Of course you can repro those with custom code, but may not be worth it, better add the features in core ImGui.

@emoon
Copy link
Contributor

emoon commented Mar 26, 2015

Nice! Will check this out as soon as I have time.

@xythobuz
Copy link
Contributor

I think there is a bug here 😄
bug

@ocornut
Copy link
Owner Author

ocornut commented Mar 26, 2015

Hmm strange is your style modified?

@ocornut
Copy link
Owner Author

ocornut commented Mar 26, 2015

OK yeah it happens if WindowPadding.x < AutoFitPadding.x
EDIT fixed

@xythobuz
Copy link
Contributor

Oh, okay. I just wanted to post all my style settings. These are the two you mentioned:

style.WindowPadding                         = ImVec2(2, 2);
style.FramePadding                          = ImVec2(2, 1);

@ocornut
Copy link
Owner Author

ocornut commented Mar 26, 2015

It's not actually new it's a bug in Selectable() but the auto-resizing windows shows it clearly.
I'll patch the new release before it gets noticed :)

The repro is:

ImGuI::GetStyle().WindowPadding.x = 7;
ImGuI::GetStyle().AutoFitPadding.x = 8;
        ImGui::Begin("Test", NULL, ImGuiWindowFlags_AlwaysAutoResize);
        ImGui::Selectable("Hello");
        ImGui::End();

@ocornut
Copy link
Owner Author

ocornut commented Mar 26, 2015

Pushed a fix! Let me know if that works for you :) I deleted 1.37 and will republish it.

@emoon
Copy link
Contributor

emoon commented Apr 13, 2015

I haven't investigated this but is it possible to use submenus with this approach also?

@ocornut
Copy link
Owner Author

ocornut commented Apr 13, 2015

Not really yet, it's not really a proper menu API. You can do sub popups on click but that wouldn't be super user friendly. We want sub-menu hover, automatic positioning and alignment, etc.
(Coincidentally I was just working using popups right now!)

@emoon
Copy link
Contributor

emoon commented Apr 13, 2015

Cool :) I will wait a bit then with removing my native (as in platform specific) popups until you have it in.

@thevaber
Copy link
Contributor

I've been testing this for last few days and so far everything works perfectly, great job! Something that would be useful is a way to know whether there is any menu or popup currently open to allow ignoring mouse clicks in the void area which are only meant to close all currently open menu/popups - right now such clicks can (depending on the application) cause unwanted non-gui interactions because WantCaptureMouse is false

ocornut added a commit that referenced this issue May 28, 2015
…child (#126)

It think it makes more sense? Maybe?
Note that calling OpenPopup() every frame probably doesn't make sense.
@ocornut
Copy link
Owner Author

ocornut commented May 28, 2015

Good to hear it's working well for you. Note that I may still change the name of the 3 functions I added yesterday (currently BeginPopupContextItem / BeginPopupContextWindow / BeginPopupContextVoid).

I'll look into what you're mentioning. I think the bug should be that WantCaptureMouse should always be true automatically when a popup is active, so you don't have to check that yourself.

ocornut added a commit that referenced this issue May 28, 2015
ocornut added a commit that referenced this issue May 29, 2015
…racked mouse button ownership when mouse down (#126)
@ocornut
Copy link
Owner Author

ocornut commented May 29, 2015

@thevaber : should be fixed now. When you close a popup or menu by clicking in void WantCaptureMouse will be kept high as long as the button isn't released.

I am now seeing that the input inhibition performed by popups/menus doesn't match what Windows is doing with menus. Windows only inhibits inputs for things under the popup for the same window, but other windows aren't affected. I wonder if I should work toward that.

The reason it relates to WantCaptureMouse is that with the fix above if you have a menu open it might feels unreactive if you click on the void to, say, move your game 3d camera and it doesn't work on the first click, you need to release and click. However having popups/menus open and trying to close them isn't such a common thing and it should be understandable to the user why they need to click once to close the popups/menus before they can interact with your game.

The difference between an operating system with a desktop and imgui "void" in your game app is that you never actually have void, that space is likely used by the rest of your app and if you are using inputs to interact with that space then we are left with nowhere to click on to close a popup without triggering something.

@thevaber
Copy link
Contributor

Thanks for the fix! Indeed, for some applications this behavior might not be desirable, while for some it is nearly essential.

My use case is a painting application - think photoshop, you have a brush tool selected and you start doing something in menus - then you change your mind, and click outside the menu to get rid of it, likely on the canvas - and you don't want that to actually paint anything. In photoshop (and other similar software I tried) this is the case, no painting is done on such mouse clicks (until the mouse button is released and clicked again), but clicking anywhere other than the canvas works exactly as Windows OS does it, like you described (ignoring clicks to the same window). But for some other applications, even clicking the "void" (well, non-imgui area) should actually do something.

I see some applications solving this by having a rather short timer for closing the menu when it's not hovered, thus not requiring a mouse click to close the menu at all, though that seems not exactly ideal either, it feels quite cumbersome.. So I'm guessing that this void click behavior will probably need to be configurable, or perhaps use another WantCaptureMouse-like variable (something like WantCaptureMouseVoid, which anyone can choose to ignore or OR it with WantCaptureMouse)?

@ocornut
Copy link
Owner Author

ocornut commented May 29, 2015

Yes that would make sense. I'll wait until actual feedback come to get the "other" behavior before adding something. Menus are still new. Also don't hesitate to post screenshots if you're done something cool that you can share!

@extrawurst
Copy link
Contributor

for me clicking in void to close the popup and ignoring that click WantCaptureMouse=true feels natural. +1 for keeping it that way.

ocornut added a commit that referenced this issue May 31, 2015
…)+ comments

Sorry if you used this parameter already.
@ocornut
Copy link
Owner Author

ocornut commented May 31, 2015

I am going to release that as 1.40 (skipping 1.39 because the changelist is very huge already) in a few hours. Hopefully we won't find a bug that would require to change all the API the next day.

@ocornut
Copy link
Owner Author

ocornut commented Jun 20, 2015

Closing this topic for now. Bug fixes and additions such as shortcuts, e.g. can be handled in a separate topic.

@ocornut ocornut closed this as completed Jun 20, 2015
@ocornut ocornut mentioned this issue Dec 25, 2015
7 tasks
ocornut added a commit that referenced this issue May 28, 2016
ocornut added a commit that referenced this issue Aug 7, 2016
…re standard (#126, #245, #323)

Apparently menu items started with OnClick (vs OnClickHoldRelease) when
doing #126. Hope to not break anything.
Also allows using xxx_DontClosePopup flags.
@ocornut ocornut added the menus menu bars, menu items label Sep 26, 2017
ocornut added a commit that referenced this issue Sep 29, 2017
…out in a menu bar. Makes MenuItem() in a regular window behave more consistently, and this will be also needed by upcoming menu-navigation changes in the nav branch. (#126, #787)
ocornut added a commit that referenced this issue Sep 29, 2017
…ting inside one of their child. If a Left<>Right navigation request fails to find a match we forward the request to the root menu. (#787, #126)

Currently the sibling menu is isn't automatically opened, that's still left to it (and even that can be anoying in Windows when the first menu-item is a child menu)
ocornut added a commit that referenced this issue Sep 29, 2017
…out in a menu bar. Makes MenuItem() in a regular window behave more consistently, and this will be also needed by upcoming menu-navigation changes in the nav branch. (#126, #787)
ocornut added a commit that referenced this issue Jul 8, 2021
…e via internals) + fix menus being affected by style.SelectableTextAlign (#126)
ocornut added a commit that referenced this issue Jul 8, 2021
…e via internals) + fix menus being affected by style.SelectableTextAlign (#126)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement menus menu bars, menu items
Projects
None yet
Development

No branches or pull requests

7 participants