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.

idbrii added a commit to idbrii/cpp-imgui that referenced this issue Mar 24, 2025
Includes my merged PRs and everything in my dev branch. Haven't tested
with it yet.

Changelog:
Test case for clip rect
HACK: more recent Windows SDK and VS2017; disable graph
Set size to amount of space required
Merge pull request ocornut#349 from maksw2/master
Merge pull request ocornut#347 from mgerhardy/341
Merge pull request ocornut#348 from mgerhardy/fixed-warning
Merge pull request ocornut#346 from mgerhardy/280
Merge pull request ocornut#345 from mgerhardy/322
Merge pull request ocornut#344 from rherilier/fix-gcc-warnings
Merge pull request ocornut#336 from rherilier/add-isusingviewmanipulate
Merge pull request ocornut#335 from RedSkittleFox/alternative_window
Merge pull request ocornut#334 from ocornut/fix-beginchildframe
dear imgui update and small fixes
Merge pull request ocornut#316 from Batres3/2DSupport
Merge pull request ocornut#326 from Sayama3/use-push-pop-id
Merge pull request ocornut#328 from georgeto/master
Merge pull request ocornut#330 from maritim/master
Merge pull request ocornut#331 from GiovanyH/patch-1
Merge pull request ocornut#318 from dougbinks/imgui_math_operators
Merge pull request ocornut#312 from kimidaisuki22/master
div 0 fixed
Merge pull request ocornut#301 from ZingBallyhoo/using-any
Merge pull request ocornut#300 from Clog41200/Configurable-limits
Merge pull request ocornut#298 from xDUDSSx/fix/rotation_circles
Merge pull request ocornut#297 from xDUDSSx/fix/vertical-aspect-scaling
Merge pull request ocornut#289 from ComputationalBiomechanicsLab/fix_isusing-ignores-setid
Merge pull request ocornut#291 from ocornut/fix_math_operators_include
Merge pull request ocornut#282 from MohitSethi99/master
Merge pull request ocornut#276 from pthom/virtual_destructors
Merge pull request ocornut#271 from idbrii/clip-parent
Merge pull request ocornut#270 from idbrii/btn-behaviour
Merge pull request ocornut#265 from mgerhardy/pr/fix-minor-formatting
Merge pull request ocornut#264 from mgerhardy/pr/div0
Merge pull request ocornut#269 from peter1745/hatched-line-thickness-enhancement
Merge pull request ocornut#259 from miyanyan/master
Merge branch 'master' of https://github.com/CedricGuillemet/ImGuizmo
update dear imgui
Merge pull request ocornut#256 from Aidiakapi/patch-1
Merge pull request ocornut#252 from aaronkirkham/master
Merge pull request ocornut#249 from rokups/rk/mouse-capture
Merge pull request ocornut#246 from mgerhardy/pr/viewmanipulate
removed commented code
fix click view cube
Merge pull request ocornut#231 from mgerhardy/master
Merge pull request ocornut#230 from mgerhardy/master
Merge pull request ocornut#228 from longod/master
Merge pull request ocornut#227 from madeso/master
AddBezierCubic
Merge pull request ocornut#226 from sherief/master
revert culling test commit
Merge pull request ocornut#203 from rokups/rk/misc-fixes
Merge pull request ocornut#212 from zhaijialong/fix-behind-camera-cull
Merge pull request ocornut#209 from VictorFouquet/fix_normalize
scale is always local
Merge pull request ocornut#202 from pezy/master
imguizmo namespace
Merge pull request ocornut#194 from JonathanHiggs/vcpkg-example
1.84 WIP
Merge pull request ocornut#197 from idbrii/seq-btn-color
Merge pull request ocornut#196 from idbrii/seq-big-handles
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