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

Nav Branch: Pressing "right" in tree causes you to jump out of tree #1079

Open
uglycoyote opened this issue Mar 24, 2017 · 6 comments
Open
Labels
nav keyboard/gamepad navigation tree tree nodes

Comments

@uglycoyote
Copy link

I commented on this #787 but perhaps it's worthy of its own bug report with some sample code and an image of what's going on.

One issue that I believe is making the gamepad stuff hard to use is that pressing right frequently jumps you out of a Tree. Navigating a tree structure with a game pad is fairly slow, so there's a lot of investment one puts in to getting the nav-highlight to the right spot in the tree, so if the focus jumps someplace else it is frustrating because you have to "redo your work" to get back to the same place.

I realize that the navigation is driven off a model where a directional press to the right triggers a search for "most relevant item to the right of the current one" but IMHO when inside a tree the pad navigation should enter a more restricted mode where it's not possible to just jump around arbitrarily, and you have to very explicitly "exit the tree" before you can navigate to anything outside of it.

Here's an example of a tree that I constructed and I'm navigating (with the keyboard in this case, but the gamepad behaviour is the same). You can see that at several points the nav-focus jumps up to the button above the tree. This is when I press to the right. Other times the focus jumps from a lower tree node to a higher one, this is also from pressing "right"

treerightpress

I'm used to other systems where pressing right opens a tree node, and also when you have a slider highlighted it seems natural that pressing right would increment the slider. I keep forgetting that there's a fancy "hold down button and press right" thing for sliders, and so I press right and end up getting teleported away to the far reaches of the universe.

Here's the code that I'm using to create this test case. I have omitted the MenuItem class for brevity here, it's the same one that I pasted into issue #1074.

static float x = 0.0f;

void DisplayTree(MenuItem* menuItem)
{

	if ( menuItem->children.size() == 0 )
	{
		ImGui::SliderFloat( menuItem->text.c_str(), &x, 0.0f, 1.0f );
	}
	else
	{
        if (ImGui::TreeNode(menuItem->text.c_str()))
        {
			for ( MenuItem* child : menuItem->children )
			{
				DisplayTree( child );
			}
            ImGui::TreePop();
        }
	}


}

void MBCTreeTest()
{
	MenuItem* menu = GetMainMenu();

	bool showMBCTest;
    ImGui::SetNextWindowSize(ImVec2(200,100), ImGuiSetCond_FirstUseEver);
    ImGui::Begin("MBCTest", &showMBCTest);
	ImGui::Button( "Hello" ); ImGui::SameLine(  );
	ImGui::Button( "My" ); ImGui::SameLine(  );
	ImGui::Button( "Pretty" ); ImGui::SameLine(  );
	ImGui::Button( "Little" ); ImGui::SameLine(  );
	ImGui::Button( "World" ); ImGui::SameLine(  );

	ImGui::Separator();


	DisplayTree(menu);

	// awkward, but needed to make an item be highlighted off the start
	GImGui->NavDisableHighlight = false;


	ImGui::End();

}

@ocornut
Copy link
Owner

ocornut commented Mar 25, 2017

Thanks for those two posts and the repro. Would you mind copying your post from the other thread here as well, for reference? I agree on the principle that this is desirable and will look into it (unfortunately no ETA, but in a few weeks I hope to start giving time back to imgui). There's probably a solution that would solve it while still allowing multiple items around tree nodes.

Any extra repro/edge case welcome, but I presume we can start looking at the example you provided.

@uglycoyote
Copy link
Author

Here's the feedback I left on #787:

Thanks so much for the progress on getting gamepad navigation working.

We have an existing custom (non-imgui) tree-based debug menu in our game. I created an alternate implementation in ImGui and am considering deprecating the old one eventually. It mostly depends on getting the game-pad support right as we don't always have the ability to use mouse on our game builds.

The menu is essentially just a big tree containing mostly booleans, ints and floats and "triggers", so in the imgui implementation they are mostly checkboxes, sliders, and buttons.

I'm trying out the latest 2016-07-Navigation branch. At the moment I'm finding the dpad left/right behaviour particularly frustrating.

In my case the thing I'm trying to replace something that users have been using for about 10 years and the controls are ingrained in everyone's muscle memory so it would be nice to be able to configure the controls in ImGui to work as similar as possible to how the tree navigation works under that system.

This is how our existing menu system works and how I hoped ImGui would be able to configured to work:

When focused on a folder in the tree:

  • Press PadRight or "A" (activate) to expand a folder in the tree
  • Press PadLeft or "B" (cancel) to collapse the folder

When focused on a leaf node in the tree

  • Press PadRight or "A" to toggle a checkbox or activate a button
  • Press PadRight or PadLeft to adjust a slider
  • Press B to collapse the parent folder and set parent as the focused item

In ImGui Pressing PadRight takes the focus away from wherever you are in the tree and off to never-never land. From looking at the code I understand that it's using some kind of scoring system for determining where the focus goes, but at least in my use case it feels wrong and unpredictable. It either focuses another item higher in the tree (presumably because that item is indented more to the right) or it jumps the focus to a "Clear" button that I have on the filter box at the top of the tree. Either way it's taking the input focus away from something that you just pressed the d-pad a few dozen times to get to, so it's frustrating in that regard.

I know others on this thread and the previous one have said similar things, but the rebuttal was that the dpad navigation needs to be able to work with multiple columns and with tree nodes which have multiple things in them that you can focus on. But in my case I only have one column and each tree node only has one focusable element, (basically the simplest kind of tree) so it would be nice if it worked well in this situation.

The scoring system leads to unpredictable jumping of focus which takes the user a long time to recover from. IMHO the scoring system should only be used when there's not some simpler more direct context-specific interpretation of the arrow keys. Once you enter the "tree navigation" context it should not use the scoring system anymore but a more custom logic for interpreting directional inputs such as the rules I listed above.

@ocornut ocornut added the nav keyboard/gamepad navigation label May 1, 2017
@uglycoyote
Copy link
Author

Hi Omar,

Thanks so much for looking in to some of the issues with tree navigation on the Nav Branch that I mentioned. I grabbed your latest code from the nav branch and tested my use case above, and it worked better (the focus no longer jumped up to the buttons at the top when pressing right from within the tree).

However I did run into a similar case where the focus still annoyingly jumps out of the tree. This time, my tree is embedded in a menu, and pressing right from a leaf of the tree causes it to jump out completely out of the tree, out of the menu, and into the next menu to the right. I have a short video capture of this.

jumpoutofinterestingmenu

In case it's not clear, i'm pressing "right" when the slider "Folder 2 Subitem 3" is highlighted, and it's jumping out of the "interesting menu" and into "boring menu 2"

You could argue that this is working "as designed", giving the person a quick way of jumping to the different menus, but my arguments against it would be ones that I have probably mentioned before

  • In a deep tree like the one in our application, it takes a long time to navigate, and if you jump out of the tree accidentally, the work you have done needs to be repeated
  • The slider having a big blue highlight around it is just begging the user to press left and right to adjust it, and it's not obvious (at least to a casual user) that you should need to hold down the special modifier button while pressing right

I think that it would be nice if the sliders responded to left and right without having to "activate" them. The blue "nav focus" highlight would imply to most people that that control is the active input focus and it seems a bit strange that there's two different levels of focusedness.

Assuming that you didn't want to change things to allow editing sliders without holding down the activate button, then it would better if pressing right in this situation did nothing (instead of jumping you from the tree), then at least the user would realize that they needed to do something else besides just press right to adjust the slider, and might discover the right thing to do through trial and error.

My recommendation would be that pressing cancel is the only way to exit from the tree, and that each time you press cancel it should only close up the current level of the tree, so that you can sort of do "cd .." (to give a command line analogy) one folder at a time if you go into the wrong place. Currently cancel also closes the entire tree, but it would be nice if it it selected "currentNode->Parent" and collapsed the children of that parent.

If the user really wants to leave a big deep tree and go to a completely other menu, pressing cancel several times in a row doesn't seem like a bit thing to ask the user to do, i think if they have invested the time to explore a deep structure they would not mind having to explicitly cancel out of each layer of the onion.

Here's some sample code for this use case:


#include "MBCTest.h"
#include "imgui.h"
#include "imgui_internal.h"

void DisplayTree(MenuItem* menuItem)
{
	ImGui::PushID(menuItem);

	std::string::const_pointer label = menuItem->text.c_str();
	if ( menuItem->children.size() == 0 )
	{
		ImGui::SliderFloat( label, &(menuItem->x), 0.0f, 1.0f );
	}
	else
	{
		bool opened = ImGui::TreeNode(label);  ImGui::NextColumn(); ImGui::NextColumn();
		if (opened)
		{
			for ( MenuItem* child : menuItem->children )
			{
				DisplayTree( child );
			}
			ImGui::TreePop();
		}
	}

	ImGui::PopID();


}

void MBCTreeTest()
{
	MenuItem* menu = GetMainMenu();

	if ( ImGui::BeginMainMenuBar() )
	{

		if(  ImGui::BeginMenu( "Boring Menu 1" ) )
		{
			ImGui::Text("Not very interesting in here");		
			ImGui::Button("yum");		
			ImGui::EndMenu();
		}

		if ( ImGui::BeginMenu( "Interesting Menu" ) )
		{
			DisplayTree(menu);
			ImGui::EndMenu();
		}

		if(  ImGui::BeginMenu( "Boring Menu 2" ) )
		{
			ImGui::Text("Even less interesting in here");		
			ImGui::EndMenu();
		}

		ImGui::EndMainMenuBar();
		
	}

	// awkward, but needed to make an item be highlighted off the start
	GImGui->NavDisableHighlight = false;


}

ocornut added a commit that referenced this issue Feb 2, 2018
… closing a TreeNode() from any of child. The explicit flag is not great, perhaps allowing some form of inheritance would help. (#787, #1079)
@ocornut
Copy link
Owner

ocornut commented Feb 2, 2018

@uglycoyote : I added a test flag called ImGuiTreeNodeFlags_NavCloseFromChild to allow using the Left Arrow to close a tree node from any of its descendent.

I don't think it is good enough that the flag is explicit because all your TreeNode needs it. We could decide on a mechanism to automatically inherit it within a tree so only one root node could enable it and it would affect the entire tree (perhaps unless explicitly disabled by one of its child node), so perhaps I am going to turn that into a duo of flags NavCloseFromChild and NoNavClosefromChild and within a tree hierarchy the latest one is an override.

Unfortunately it's really tricky to enable that behavior by default. As with a lots of your concerns and frustration, the proposed changes work in your specific usage scenario but would be detrimental to other common uses. Pretty much anything can be "inside a tree node" and the same way you don't want your Left-Right inputs to be caught by the parent menu, not everybody wants a Left input to close a bunch of contents.

I think we could add a menu/menubar flag to disable the behavior of catching and turning un-handled Left/Right move requests into a menu change. You probably shouldn't store elaborate hierarchy instead a menubar/menu, but if you do so then I suppose it's ok to add that flag to BeginMenu(). The problem is either you set this flag everywhere and make general menu navigation less confortable, either you set it selectively and your controls are inconsistent.

While the current system is not perfect and we should strive to answer most concerns and improve it, a lot of your issues stem from the fact that you are trying to transition away from existing habits.

ocornut added a commit that referenced this issue Feb 2, 2018
…moves you to parent node instead of closing it immediately. More standard. (#787, #1079)
@uglycoyote
Copy link
Author

Thanks Omar! I am currently away from my work when I am in the office next I'll try to experiment with this new flag.

@ocornut
Copy link
Owner

ocornut commented Feb 5, 2018

@uglycoyote Among recent changes, I have changed the Slider and Drag widget to be tweakable by toggling NavActivate (cross, space) instead of holding it, which I believe it easier to understand.

Also made this control sheet (tell me if you want the PSD)

imgui controls v4

ocornut added a commit that referenced this issue Feb 25, 2018
@ocornut ocornut added the tree tree nodes label Jun 15, 2018
ocornut added a commit that referenced this issue Aug 8, 2023
…Here now goes through proper navigation logic: honor scrolling and selection. (#1079, #1131)

Added a stack for this purpose which other features might build on (e.g. #2920). However this is currently gated by many tests and not a performance concern, but making stack happen all the time may be undesirable.
Ajblast pushed a commit to Ajblast/imgui that referenced this issue Aug 9, 2023
…Here now goes through proper navigation logic: honor scrolling and selection. (ocornut#1079, ocornut#1131)

Added a stack for this purpose which other features might build on (e.g. ocornut#2920). However this is currently gated by many tests and not a performance concern, but making stack happen all the time may be undesirable.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
nav keyboard/gamepad navigation tree tree nodes
Projects
None yet
Development

No branches or pull requests

2 participants