Skip to content

Conversation

Sven-vh
Copy link

@Sven-vh Sven-vh commented Oct 8, 2025

As the title says, this PR adds an overload to BeginPopup and takes in an ImGuiID.

OpenPopup already has an ImGuiID overload, so I was a bit confused why BeginPopup didn't have this.
Related to: #3993

This is not a huge issue since there are some workarounds, but they either make you add a new variable or include imgui_internal.h

Workaround 1:

Add name variable to use in GetID and BeginPopup

const char* name = "MyPopUp";
ImGuiID id = ImGui::GetID(name);

if (ImGui::Button("Open PopUp")) {
	ImGui::OpenPopup(id);
}

if (ImGui::BeginPopup(name)) {
	ImGui::Text("Hello from popup!");
	if (ImGui::Button("Close"))
		ImGui::CloseCurrentPopup();
	ImGui::EndPopup();
}

Workaround 2:

Include imgui_internal.h and use BeginPopupEx

Edit: This also removed all of the default flags, so you need to manually add those again.

#include "imgui_internal.h"

ImGuiID id = ImGui::GetID("MyPopUp");

if (ImGui::Button("Open PopUp")) {
	ImGui::OpenPopup(id);
}

if (ImGui::BeginPopupEx(id, ImGuiWindowFlags_AlwaysAutoResize | ImGuiWindowFlags_NoTitleBar | ImGuiWindowFlags_NoSavedSettings)) {
	ImGui::Text("Hello from popup!");
	if (ImGui::Button("Close"))
		ImGui::CloseCurrentPopup();
	ImGui::EndPopup();
}

PR fix:

Added an overload for BeginPopup to take in an ImGuiID

ImGuiID id = ImGui::GetID("MyPopUp");

if (ImGui::Button("Open PopUp")) {
	ImGui::OpenPopup(id);
}

if (ImGui::BeginPopup(id)) {
	ImGui::Text("Hello from popup!");
	if (ImGui::Button("Close"))
		ImGui::CloseCurrentPopup();
	ImGui::EndPopup();
}

Most of the time, I call OpenPopup in nested IDs so I almost always use ImGuiID for pop-ups.

This is my first PR, so I'm sorry if I missed a rule/guideline.

@ocornut ocornut added popups label/id and id stack implicit identifiers, pushid(), id stack labels Oct 8, 2025
@ocornut
Copy link
Owner

ocornut commented Oct 8, 2025

In principle this is correct, but merging #3993 was meant to solve a specific use cases which was not possible to use without the public API.

I realize that the presence of OpenPopup(ImGuiID id) alone may be confusing.
However my issue with adding BeginPopup(ImGuiID id) in public API right now is that technically we would also need to add BeginPopupModal(ImGuiID id, ...);. I am not thrilled about adding too many functions with same name and I would prefer to temporize this and hope for a wider refactor which would make the ->GetID("") approach less required.

(I realize that I promised the "path" thing described above a while ago, we've been happily using in the TestEngine, see "named references", it's mostly another API design issue that it's not there yet in popups).

IMHO for now I would prefer to snooze this until I can spend time to think about this more thoroughly, and suggest you use "Workaround 1" which is not really a problem at all, just call ImGui::OpenPopup(ImGui::GetID(name)).

This is my first PR, so I'm sorry if I missed a rule/guideline.

Your PR is good, thank you!

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

Successfully merging this pull request may close these issues.

2 participants