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

Context menus don't work on ColorEdit4 #4167

Open
CobaltXII opened this issue May 24, 2021 · 5 comments
Open

Context menus don't work on ColorEdit4 #4167

CobaltXII opened this issue May 24, 2021 · 5 comments

Comments

@CobaltXII
Copy link

Version/Branch of Dear ImGui:

Version: dear imgui, v1.79 WIP
Branch: master

Back-end/Renderer/Compiler/OS

Back-ends: imgui_impl_opengl3.cpp + imgui_impl_sdl2.cpp
Operating System: macOS

My Issue/Question:

Context menus don't work on combo boxes but they work on other widgets. In the following code, if I right-click on Combo, all of ImGui freezes (but not the program itself). If I right-click on Slider, it works fine.

Standalone, minimal, complete and verifiable example:

// window
ImGui::Begin("Test", NULL, ImGuiWindowFlags_NoSavedSettings);

// combo
static int a = 0;
if (ImGui::BeginCombo("Combo", std::to_string(a).c_str())) {
	for (int i = 0; i < 10; i++) {
		bool selected = i == a;
		if (ImGui::Selectable(std::to_string(i).c_str(), selected)) a = i;
		if (selected) ImGui::SetItemDefaultFocus();
	}
	ImGui::EndCombo();
}

// context for combo
if (ImGui::BeginPopupContextItem("Combo")) {
	if (ImGui::MenuItem("Reset")) a = 0;
	ImGui::EndPopup();
}

// slider
static int b = 0;
ImGui::SliderInt("Slider", &b, 0, 10);

// context for slider
if (ImGui::BeginPopupContextItem("Slider")) {
	if (ImGui::MenuItem("Reset")) b = 0;
	ImGui::EndPopup();
}

ImGui::End();
@ocornut
Copy link
Owner

ocornut commented May 24, 2021

Hello,
Thanks for the report.

That's quite interesting.
A few things:

  • The string passed to BeginPopupContextItem() is the id of the popup and doesn't have to be the same as the widget above.
  • BeginPopupContextItem() by default can be called with no parameter and will use the last item id, as convenience. In your example if you emitted both parameters to BeginPopupContextItem() it would have the same result.

There is indeed an issue with the fact that the Combo popup will share the same ID as the Context Item Popups and they will keep overriding each others.

The immediate/easy workaround is simply to use another ID when caling BeginPopupContextItem(). This will fix your issue.

However from my POV we are trying to encourage people from not passing an ID override when calling BeginPopupContextItem() and just use the last item ID, so this is a problem that will need fixing on our end.

ocornut added a commit that referenced this issue May 24, 2021
@ocornut
Copy link
Owner

ocornut commented May 24, 2021

Pushed a fix: 029c83c

@ocornut ocornut closed this as completed May 24, 2021
@CobaltXII
Copy link
Author

Thanks so much for the quick response. Combo boxes work well with the fix. I changed all my code to call BeginPopupContextItem with no arguments. This works fine for all the widgets I am using except for ColorEdit4. However, it continues to work fine if I call BeginPopupContextItem with the ID of the color editor as I was doing before. Here is the code I am using:

ImGui::ColorEdit4(name, &color->x, ImGuiColorEditFlags_Float);
// if (ImGui::BeginPopupContextItem(name)) { <-- this works!
if (ImGui::BeginPopupContextItem()) {
	if (ImGui::MenuItem("Reset")) (void*)0;
	ImGui::EndPopup();
}

And this is the assertion that is failing:

Assertion failed: (id != 0), function BeginPopupContextItem, file lib/imgui/imgui.cpp, line 7931.

I will open a separate issue for this if you want. Thanks in advance.

@ocornut ocornut reopened this May 24, 2021
@ocornut
Copy link
Owner

ocornut commented May 24, 2021

Thanks! I'll look at this from pov of view of ColorEdit later (it uses a group + popup so may be different)

@ocornut ocornut changed the title Context menus don't work on combo boxes Context menus don't work on combo boxes / coloredit May 24, 2021
@ocornut
Copy link
Owner

ocornut commented May 25, 2021

All widgets using groups such as SliderFloat3(), ColorEdit4() will have the same issue.

What I would do right now is to use
if (ImGui::BeginPopupContextItem("Context")) { }
Or if you have multiple of them in same id stack level:
if (ImGui::BeginPopupContextItem("mywidget##context")) {}

The fix is not presently trivial, relates to #840, #2840, #2550, #1875

I think we may have to investigate the idea presented in #840 (which would mean that last item id would change when activating a sub-component, may be fine).

@ocornut ocornut changed the title Context menus don't work on combo boxes / coloredit Context menus don't work on coloredit4 May 27, 2021
@ocornut ocornut changed the title Context menus don't work on coloredit4 Context menus don't work on ColorEdit4 Jun 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants