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

BeginGroup causing popups to trigger BeginDragDropSource #2840

Open
Unit2Ed opened this issue Oct 9, 2019 · 3 comments
Open

BeginGroup causing popups to trigger BeginDragDropSource #2840

Unit2Ed opened this issue Oct 9, 2019 · 3 comments
Labels
drag drop drag and drop

Comments

@Unit2Ed
Copy link

Unit2Ed commented Oct 9, 2019

(you may also go to Demo>About Window, and click "Config/Build Information" to obtain a bunch of detailed information that you can paste here)

Version/Branch of Dear ImGui:

Version: 1.72 WIP
Branch: docking

Back-end/Renderer/Compiler/OS

Back-ends: imgui_impl_win32.cpp + imgui_impl_dx11.cpp
Compiler: MSVC 1916
Operating System: Windows 10

My Issue/Question:

We have a combo widget that's a drag drop source. When the widget is surrounded with BeginGroup/EndGroup, dragging on the scrollbar of the combo's popup (or any other widget within the popup) will incorrectly cause BeginDragDropSource to return true. Without BeginGroup/EndGroup, it doesn't start a drag.

Screenshots/Video

First combo shows expected behaviour, second shows it broken - notice the little square of the colour drag next to the cursor
BeginGroupComboDrag

// Dragging the scroll bar on this combo will NOT start a drag-drop
if (ImGui::BeginCombo("Fine", "Preview")) 
{
	for (int i = 0; i < 10; ++i)
	{
		ImGui::PushID(i);
		ImGui::Selectable("Selectable");
		ImGui::PopID();
	}

	ImGui::EndCombo();
}

if (ImGui::BeginDragDropSource())
{
	ImVec4 col_rgb;
	ImGui::SetDragDropPayload(IMGUI_PAYLOAD_TYPE_COLOR_4F, &col_rgb, sizeof(float) * 4, ImGuiCond_Once);
	ImGui::EndDragDropSource();
}

// Dragging the scroll bar on this combo WILL start a drag-drop
ImGui::BeginGroup();
if (ImGui::BeginCombo("Broken", "Preview")) 
{
	for (int i = 0; i < 10; ++i)
	{
		ImGui::PushID(i);
		ImGui::Selectable("Selectable");
		ImGui::PopID();
	}

	ImGui::EndCombo();
}
ImGui::EndGroup();

if (ImGui::BeginDragDropSource(ImGuiDragDropFlags_SourceAllowNullID))
{
	ImVec4 col_rgb;
	ImGui::SetDragDropPayload(IMGUI_PAYLOAD_TYPE_COLOR_4F, &col_rgb, sizeof(float) * 4, ImGuiCond_Once);
	ImGui::EndDragDropSource();
}
@ocornut
Copy link
Owner

ocornut commented Oct 9, 2019 via email

@ocornut ocornut added the drag drop drag and drop label Oct 10, 2019
@Unit2Ed
Copy link
Author

Unit2Ed commented Oct 10, 2019

Hi Omar,
I was trying to be concise in the report, so the example is much simplified from the actual situation we're seeing it in, which is a fairly typical property editor (not unlike you'd find in Unreal, Unity or Visual Studio).

Adding a drag-source button/icon is something we've considered and we may still do, though there are screen-space concerns and aesthetic reasons why we don't want to do it if possible. It's also not suitable for all situations where we have a draggable element that may be in a group and could cause a popup can be generated. For example grid views that have a context popup, which can contain menu items (which unexpectedly become drag sources), or other elements on which you would naturally perform drag-like actions (eg number/text inputs).

We could find a way to not use groups to accomplish this, but they're obviously a very useful solution for bundling a few widgets together and treating them as one.

@ocornut
Copy link
Owner

ocornut commented Oct 10, 2019

Hello,

To clarify, in your Combo box, where do you expect to be dragging from?
Mouse dragging after clicking on the closed combo box, or mouse dragging from individual elements?

It's also not suitable for all situations where we have a draggable element that may be in a group and could cause a popup can be generated. For example grid views that have a context popup, which can contain menu items (which unexpectedly become drag sources), or other elements on which you would naturally perform drag-like actions (eg number/text inputs).

If you have time to submit a little more details (e.g. gif/screens, no need for working repro) to understand the fuller context more in details it would generally help. Not absolutely necessary but it tends to facilitate designing better solutions.

At the moment the code in EndGroup() decide if the group is active based on wether the active item was submitted between BeginGroup() and EndGroup(). calls It's a little bit of a wonky magic trick we are performing here.

In EndGroup() we could decide to check if the active id was submitted within the same window: In the line of EndGroup() that does

const bool group_contains_curr_active_id = (group_data.BackupActiveIdIsAlive != g.ActiveId) && (g.ActiveIdIsAlive == g.ActiveId) && g.ActiveId;

By adding

&& g.ActiveIdWindow == window

But this is also why I asked my first question above.
If you added this filter you would be able to drag from the closed combo box, but not from individual selectable elements. If you want to drag from the individual selectable elements then it feels like it makes much more sense to make each of those Selectable a drag source rather than the combo box.

Or, we could add more semantic/data along with g.ActiveId (e.g. g.ActiveIdWidgetTag, I don't know).

In either case we could have to clarify what groups are, perhaps add a bunch of flags to them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
drag drop drag and drop
Projects
None yet
Development

No branches or pull requests

2 participants