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

MultiSelect + DragDrop + TreeNode: Hovering over an open TreeNode with DragDrop active causes MultiSelect selection to be cleared and replaced by only the hovered TreeNode #7850

Closed
bratpilz opened this issue Aug 1, 2024 · 3 comments
Labels
bug drag drop drag and drop selection tree tree nodes

Comments

@bratpilz
Copy link

bratpilz commented Aug 1, 2024

Version/Branch of Dear ImGui:

Version 1.91.1 WIP, Branch: master

Back-ends:

imgui_impl_glfw.cpp + imgui_impl_opengl3.cpp

Compiler, OS:

Linux + GCC

Full config/build information:

Dear ImGui 1.91.1 WIP (19101)
--------------------------------
sizeof(size_t): 8, sizeof(ImDrawIdx): 2, sizeof(ImDrawVert): 20
define: __cplusplus=201103
define: __linux__
define: __GNUC__=12
--------------------------------
io.BackendPlatformName: imgui_impl_glfw
io.BackendRendererName: imgui_impl_opengl3
io.ConfigFlags: 0x00000003
 NavEnableKeyboard
 NavEnableGamepad
io.ConfigInputTextCursorBlink
io.ConfigWindowsResizeFromEdges
io.ConfigMemoryCompactTimer = 60.0
io.BackendFlags: 0x0000000E
 HasMouseCursors
 HasSetMousePos
 RendererHasVtxOffset
--------------------------------
io.Fonts: 1 fonts, Flags: 0x00000000, TexSize: 512,64
io.DisplaySize: 1280.00,720.00
io.DisplayFramebufferScale: 1.00,1.00
--------------------------------
style.WindowPadding: 8.00,8.00
style.WindowBorderSize: 1.00
style.FramePadding: 4.00,3.00
style.FrameRounding: 0.00
style.FrameBorderSize: 0.00
style.ItemSpacing: 8.00,4.00
style.ItemInnerSpacing: 4.00,4.00

Details:

My Issue/Question:

Hello again! I know MultiSelect in combination with trees is still experimental, but I tried it anyway and ran into the issue described in the title.
Using the various MultiSelect features like Ctrl/Shift clicking works fine on TreeNodes. But now I wanted to be able to drag all selected nodes at once. For this, I call Begin(/End)DragDropSource after each TreeNode, which also works well. But when I have this DragDrop active and hover over a TreeNode that's currently open, the selection gets overwritten by just that single node I'm hovering. I'm guessing this has something to do with the fact that TreeNodes get automatically opened on hover while DragDrop is active. Relatedly, passing ImGuiDragDropFlags_SourceNoHoldToOpenOthers to BeginDragDropSource prevents this issue from occurring.

Screenshots/Video:

multi-select-treenode-open-issue.mp4

Minimal, Complete and Verifiable Example code:

// Here's some code anyone can copy and paste to reproduce your issue
	{
	  using namespace ImGui;
	  SetNextWindowSize (ImVec2 (400.0f, 300.0f), ImGuiCond_Appearing);
	  Begin ("Test");
	  static ImGuiSelectionBasicStorage selection;
	  static unsigned const N = 3;
	  auto ms = BeginMultiSelect (ImGuiMultiSelectFlags_BoxSelect1d, selection.Size, N * (N+1));
	  selection.ApplyRequests (ms);
	  auto dnd_source = [&]() {
	    if (!BeginDragDropSource()) // Workaround: Pass ImGuiDragDropFlags_SourceNoHoldToOpenOthers here to prevent the issue from occurring.
	      return;
	    SetDragDropPayload ("test", 0, 0);
	    Text ("Dragging %u selected node%s:", selection.Size, selection.Size == 1 ? "" : "s");
	    void *it = 0;
	    ImGuiID id;
	    while (selection.GetNextSelectedItem (&it, &id))
	      {
		unsigned id_i = id / (N+1);
		unsigned rest = id - id_i * (N+1);
		if (rest == 0)
		  Text ("- Top level %u", id_i);
		else
		  Text ("- Top level %u / Leaf %u", id_i, rest-1);
	      }
	    EndDragDropSource(); };
	  for (unsigned i = 0; i < N; ++i)
	    {
	      char label[32];
	      snprintf (label, sizeof label, "Top level %u", i);
	      int base_flags = ImGuiTreeNodeFlags_SpanFullWidth;
	      int flags = base_flags;
	      unsigned base_id = i * (N+1);
	      if (selection.Contains (base_id))
		flags |= ImGuiTreeNodeFlags_Selected;
	      SetNextItemSelectionUserData (base_id);
	      bool top_level_open = TreeNodeEx (label, flags);
	      dnd_source();
	      if (!top_level_open)
		continue;
	      for (unsigned j = 0; j < N; ++j)
		{
		  int flags = base_flags | ImGuiTreeNodeFlags_Leaf;
		  unsigned leaf_id = base_id + 1 + j;
		  if (selection.Contains (leaf_id))
		    flags |= ImGuiTreeNodeFlags_Selected;
		  snprintf (label, sizeof label, "Leaf %u", j);
		  SetNextItemSelectionUserData (leaf_id);
		  bool leaf_open = TreeNodeEx (label, flags);
		  dnd_source();
		  if (leaf_open)
		    TreePop();
		}
	      TreePop();
	    }
	  ms = EndMultiSelect();
	  selection.ApplyRequests (ms);
	  End();
	}
@ocornut ocornut added drag drop drag and drop tree tree nodes selection bug labels Aug 1, 2024
ocornut added a commit that referenced this issue Aug 1, 2024
… drag and drop payload over an already open tree node would select it. (#7850)
@ocornut
Copy link
Owner

ocornut commented Aug 1, 2024

Thank for the detailed bug report, very much appreciated.
I have now pushed a fix for this: 2981a10, turns out it was pretty simple.

Note that the multi-select demos include an idea to traverse a tree linearly.

@ocornut
Copy link
Owner

ocornut commented Aug 1, 2024

While looking at your test case I was initially frustrated that double-clicking wouldn't open tree nodes.

Normal tree node open on simple click (could potentially be reevaluated) but when using multi-select we are forced to enable ImGuiTreeNodeFlags_OpenOnArrow. However I think we could come up with the change that if none of the _OpenOnXXX behavior is explicit, then the default for multi-selectable tree node would be ImGuiTreeNodeFlags_OpenOnArrow | ImGuiTreeNodeFlags_OpenOnDoubleClick, aka a more sensible default. Going to make that change now.

ocornut added a commit that referenced this issue Aug 1, 2024
…enOnArrow when used in a multi-select context without any OpenOnXXX flags set. (#7850)
@ocornut ocornut closed this as completed Aug 1, 2024
@bratpilz
Copy link
Author

bratpilz commented Aug 1, 2024

Thanks for another very fast fix! This new default open behavior sounds good too. During my testing I had also noticed that I had to press on the arrow to open the nodes now, which wasn't super convenient, so this makes it easier.

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

No branches or pull requests

2 participants