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

Can not open modal popup from menu / Issue with ID stack #331

Open
nem0 opened this issue Sep 14, 2015 · 22 comments
Open

Can not open modal popup from menu / Issue with ID stack #331

nem0 opened this issue Sep 14, 2015 · 22 comments
Labels
label/id and id stack implicit identifiers, pushid(), id stack popups

Comments

@nem0
Copy link
Contributor

nem0 commented Sep 14, 2015

This works:

 if (ImGui::Button("Import asset"))
 {
    ImGui::OpenPopup("ImportAssetDialog");
 }
 if (ImGui::BeginPopupModal("ImportAssetDialog"))
 {
    ImGui::Text("test");
 }

This does not:

 if (ImGui::MenuItem("Import asset"))
 {
    ImGui::OpenPopup("ImportAssetDialog");
 }
 if (ImGui::BeginPopupModal("ImportAssetDialog"))
 {
    ImGui::Text("test");
 }

This happens

imgui_popup_error

@ocornut
Copy link
Owner

ocornut commented Sep 14, 2015

Is that exactly the code you are actually using/testing with ?

Popup Identifiers have to be part of the ID stack to clear ambiguity and allows a same popup to work from different items sources. So your calls OpenPopup() and BeginPopupModal() have to be in the same level of the ID stack. I imagine your MenuItem() is inside a BeginMenu/EndMenu pair in your code and that adds to the popup stack?

Now there is another problem here is that MenuItem() will close your current popup as well.

If you understand those things you should be able to come up with a workaround very easily, it is likely that your real code is misplaced (you aren't pasting the full code here).

But I still would like to improve the situation here.

  • Clarify or improve the situation with MenuItem/Selectable closing their parent popup and how it affect ID and popups created there.
  • Perhaps be able to sort of "clear" the value at the top of the ID stack so those locations can refer to the same identifier. This would also be useful in the future to programmatically refer to widgets from outside their location.

@nem0
Copy link
Contributor Author

nem0 commented Sep 14, 2015

The exact code I use is complicated, therefore I did not paste it here. However I tried to find the simplest code to reproduce the problem, here it is:

        ImGui::NewFrame();

        if (ImGui::BeginMainMenuBar())
        {
            if (ImGui::BeginMenu("menu"))
            {
                if (ImGui::MenuItem("menu item"))
                {
                    ImGui::OpenPopup("popup");
                }
                if (ImGui::BeginPopupModal("popup"))
                {
                    ImGui::Text("Lorem ipsum");
                }
                ImGui::EndMenu();
            }
            ImGui::EndMainMenuBar();
        }

        ImGui::Render();

Just to be sure I tried to place BeginPopupModal from the example in every possible place, nothing works.

The only solution I found is this, however I hope there is something better:

        ImGui::NewFrame();

        bool b = false;
        if (ImGui::BeginMainMenuBar())
        {
            if (ImGui::BeginMenu("menu"))
            {
                if (ImGui::MenuItem("menu item"))
                {
                    b = true;
                }
                ImGui::EndMenu();
            }
            ImGui::EndMainMenuBar();
        }

        if (b)
        {
            ImGui::OpenPopup("popup");
        }

        if (ImGui::BeginPopupModal("popup"))
        {
            ImGui::Text("Lorem ipsum");
            ImGui::EndPopup();
        }

        ImGui::Render();

I've pulled the source code of ImGui yesterday.

@ocornut
Copy link
Owner

ocornut commented Sep 14, 2015

Your first example can't work anyway because the if (ImGui::BeginPopupModal("popup")) block is only called when the menu is being browsed. If it worked it would assert anyway because you aren't calling `EndPopup' anywhere.

The second example is correct and exactly what I have mentioned, the ID will match.

Now I think what would be desirable is to make this example work (currently it won't because the opened popup identifier will be "mainmenubar+popup" and the begin is on "popup", this why I was suggesting a way to alter how the popup is using - or not - the id stack.

        ImGui::NewFrame();

        bool b = false;
        if (ImGui::BeginMainMenuBar())
        {
            if (ImGui::BeginMenu("menu"))
            {
                if (ImGui::MenuItem("menu item"))
                   ImGui::OpenPopup("popup");
                ImGui::EndMenu();
            }
            ImGui::EndMainMenuBar();
        }

        if (ImGui::BeginPopupModal("popup"))
        {
            ImGui::Text("Lorem ipsum");
            ImGui::EndPopup();
        }

        ImGui::Render();

@nem0
Copy link
Contributor Author

nem0 commented Sep 14, 2015

Now I understand why the first example does not work. Can there be some special way/flag in OpenPopup so that your example works?

For example:

 ImGui::OpenPopup("some_id", FlagPutInRoot)

 // somewhere else in root 
 if (ImGui::BeginPopupModal("popup"))
 {

@ocornut
Copy link
Owner

ocornut commented Sep 14, 2015

Yes I would like to solve that but it would need to be solved in a generic manner not only for popup. I don't know how and when yet (considering there is a workaround for popups it isn't a top priority).

@nem0
Copy link
Contributor Author

nem0 commented Sep 14, 2015

Ok, I will use the workaround until it's solved. Thank you.

@janwaltl
Copy link

Hello, I also encoutered this "problem". I have a humble idea, what if when label contains for example 4# at the begining like:
ImGui::Button("####I am a global label");
Then when generating ID it would just simply ignore the stack and use only inputed string for hashing. Of course that would mean that user must be more careful with them because of collisions. But it seems like relatively easy solution to me.
Of course I might be wrong, because of some internal structure or something....

@ocornut
Copy link
Owner

ocornut commented Sep 27, 2015

This is probably a good solution to solve this specific problem (aside from the fact that the amount of # characters may start to be a little worrying). My concern is that it doesn't provide a clear solution to identifying any widget without relying on global id, so it may be good for popup but not for other things. At this point my tendency is to sit on ideas until I can tackle a wider set of related problems all-together to avoid making too many short-term mistakes. So I'd be happy if you want to think about or discuss about identifiers in general and see where it can lead us.

Interestingly, for an hypothetical ActivateWidget() function, appending strings without any separator would work (syntactically we could allow a separator such as '/' to make them more readable) but we might still need a way to pass on chain of identifiers involving integers/pointers.

@r-lyeh-archived
Copy link

Aha, just wanted to say that took me a while to understand why the popups didnt work after a MenuItem() as well.
Got puzzled exactly like nem0 and found the same "solution".

@q-depot
Copy link

q-depot commented May 16, 2017

I stumbled upon the same issue, I kind of understand why this is happening and I was wondering if we could find a temporary solution.

Would it be possible to open a modal popup using a variable?
I thought I could use the p_open argument in BeginPopupModal, but it doesn't work and neither using the variable as a condition to create the modal itself:


var openModal = false;

if ( ImGui::Button("open") )
	openModal = true;

if ( ImGui::BeginPopupModal("foo", &openModal) )
{
    ImGui::Text("Hello!");    
    if ( ImGui::Button("Ok") ) 
    { 
    	ImGui::CloseCurrentPopup(); 
    	openModal = false;
    }
    ImGui::EndPopup();
}

if ( openModal )
{
	ImGui::BeginPopupModal("foo");
    ImGui::Text("Hello!");    
    if ( ImGui::Button("Ok") ) 
    { 
    	ImGui::CloseCurrentPopup(); 
    	openModal = false;
    }
    ImGui::EndPopup();
}

I ended up going back using a Button which kind of works(parent popup stays open), however I can't close the parent popup when I close the modal, ideally I'd like to call ImGui::ClosePopup("parent-popup") but it's not accessible.

if ( ImGui::BeginPopupModal("foo") )
{
    ImGui::Text("Hello!");    
    if ( ImGui::Button("Ok") ) 
    { 
    	ImGui::CloseCurrentPopup();                 // close modal
        ImGui::ClosePopup("parent-popup");   // close parent popup
    }
    ImGui::EndPopup();
}

@NPatch
Copy link

NPatch commented Oct 26, 2018

I had a similar situation trying to write a Save File Menu Item. It opens up a modal popup for you to write the full path for the file to be saved(temporary until I figure out a good way to incorporate proper File Dialogs) and that ,if the file already exists ,in turn has to open up a stacked modal popup that asks for permission to overwrite.
The first modal is easy. The solution above with the bool flag toggle inside the MenuItem works fine. But that got a bit messy with bool flags too quickly due to dependencies. The first modal upon clicking Save has to either complete the action or wait for positive feedback from the second stacked modal.

Instead an enum with the states of this situation worked on the first try. Much cleaner and more straightforward.

@NPatch
Copy link

NPatch commented Oct 26, 2018

In the case of MenuItems...if you've got many modals and possibly stacked as well, having an enum with the full flow in states for each different operation makes it easier to handle multiple modals code in the same scope with minimal variables. You basically have one variable for all the modals and the menu item they get triggered by for each operation without affecting the others.

@flamendless
Copy link

I was about to open an issue regarding the same problem, good thing I've searched first.

@AnimatorJeroen
Copy link

AnimatorJeroen commented Feb 19, 2020

My solution to this is to use a lambda. A function pointer can be set at any state (even inside a menu click) and in the end of your code you simply need to call it.

Not sure if this has perfomance issues I may be overlooking though, otherwise its pretty handy:

{

ImGui::NewFrame();

static void(*ShowPopup)() = []() {};

if (ImGui::BeginMainMenuBar())
{
	if (ImGui::BeginMenu("menu"))
	{
		if (ImGui::MenuItem("menu item"))
		{
			ShowPopup = []()
			{
				if (!ImGui::IsPopupOpen("popup"))
					ImGui::OpenPopup("popup");

				if (ImGui::BeginPopupModal("popup"))
				{
					ImGui::Text("Lorem ipsum");

					if (ImGui::Button("Close", ImVec2(80, 0)))
					{
						ImGui::CloseCurrentPopup();
						ShowPopup = []() {};
					}
					ImGui::EndPopup();
				}
			};
		}
		ImGui::EndMenu();
	}
	ImGui::EndMainMenuBar();
}

ShowPopup();

ImGui::Render();

}

@ocornut ocornut added the label/id and id stack implicit identifiers, pushid(), id stack label Apr 6, 2020
@ppekko
Copy link

ppekko commented Dec 26, 2020

I found a temporary solution to this bug for anyone waiting for a fix. Hopefully ocornut fixes it soon.

bool openpopuptemp = false;

//in render loop
if (ImGui::BeginMenu("Menu")){
		if (ImGui::MenuItem("Open Popup", NULL)) { 
			openpopuptemp = true;
		}
		ImGui::EndMenu();
}
if (openpopuptemp == true) {
		ImGui::OpenPopup("popup");
		openpopuptemp = false;
}
if (ImGui::BeginPopupModal("popup")){
            ImGui::Text("Lorem ipsum");
            ImGui::EndPopup();
}

@decitrig
Copy link

A tricky bit with the solution in #331 (comment) is that if you want to set size/position on the popup with SetNextWindow*, you need to make sure the calls to SetNextWindow are right nearby the call to OpenPopup and that no other windows sneak in while you're not looking:

		if imgui.IsKeyDown(sdl.SCANCODE_ESCAPE) {
			imgui.OpenPopup("Menu")
			imgui.SetNextWindowSize(imgui.Vec2{X: 400, Y: 0})
			imgui.SetNextWindowPosV(imgui.Vec2{X: float32(wd) / 2, Y: float32(ht) / 2}, 0, imgui.Vec2{X: .5, Y: .5})
		}
                // --------- No windows in here, or else! -----------
		if imgui.BeginPopupModalV("Menu", nil, imgui.WindowFlagsNoScrollbar|imgui.WindowFlagsNoResize) {
			if imgui.ButtonV("Quit", imgui.Vec2{X: -1, Y: 0}) {
				running = false
			}
			if imgui.ButtonV("Return", imgui.Vec2{X: -1, Y: 0}) {
				imgui.CloseCurrentPopup()
			}
			imgui.EndPopup()
		}

(this is go, but it maps pretty directly to the c++)

Or am I missing a trick somewhere?

@w0utert
Copy link

w0utert commented Sep 5, 2022

Just came here to say I also ran into this problem, and experienced the current behavior when opening a popup from a MenuItem to be very confusing and non-discoverable from the demo application.

I'm using the workaround proposed by @nem0 in 2015, which works, but it kind of ruins the nice immediate property of the rest of the ImGui API by having to explicitly track menu activation state using a boolean and defer the logic of the modal itself to some point after where it is triggered (which could be somewhere way down the source code if you have a long menu).

In my case I just want to open a 'confirm quit yes/no?' dialog triggered from the 'Quit..' menu item in the main menu bar, which seems like a very common use case. Would be nice if there would be a better solution.

@ocornut
Copy link
Owner

ocornut commented Sep 5, 2022 via email

@w0utert
Copy link

w0utert commented Sep 5, 2022

That sounds like a nice solution, I will keep watching this issue so I can adapt my code when this is available, thanks!

@katemonster33
Copy link

katemonster33 commented May 11, 2023

Hi, I came up with a different work-around to this after struggling like OP has. I use PushOverrideID from the internal api:

ImGuiID popup_id = ImHashStr( "POPUP" );
ImGui::PushOverrideID( popup_id );
ImGui::OpenPopup( "POPUP" );
ImGui::PopID();
[...]
ImGui::PushOverrideID( popup_id );
 if( ImGui::BeginPopup( "POPUP" ) ) {
    [...]
    ImGui::EndPopup();
}
ImGui::PopID();

This will work in any context :) hope this helps someone.

@aymen157
Copy link

aymen157 commented Sep 23, 2024

does this still not work still after almost 10 years or am i missing some new api?
the following code still fails:

if (ImGui.BeginPopupContextItem("generate"))
{
	if (ImGui.MenuItem("events"))
		ImGui.OpenPopup("confirm");
	ImGui.EndPopup();
}
if (ImGui.BeginPopupModal("confirm"))
{
	ImGui.Text("hello")
	ImGui.EndPopup();
}

and honestly the approach above works but is silly

@ocornut
Copy link
Owner

ocornut commented Sep 23, 2024

The issue is open because the status quo hasn't changed yet.
My plan is to add support for "/" "../" prefixes as mentioned, and to be in line with what imgui_test_engine support, but for various reasons it requires specific design for imgui which I have not done yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
label/id and id stack implicit identifiers, pushid(), id stack popups
Projects
None yet
Development

No branches or pull requests