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

Initialize keyboard navigation with a highlighted, but inactive item #7237

Closed
GamingMinds-DanielC opened this issue Jan 18, 2024 · 11 comments
Closed
Labels
inputs nav keyboard/gamepad navigation

Comments

@GamingMinds-DanielC
Copy link
Contributor

Version/Branch of Dear ImGui:

Version 1.90.1 WIP (19002), Branch: docking

Back-ends:

imgui_impl_win32.cpp + imgui_impl_dx11/dx12.cpp

Compiler, OS:

Windows 10/11 + MSVC 2019/2022

Full config/build information:

Dear ImGui 1.90.1 WIP (19002)
--------------------------------
sizeof(size_t): 8, sizeof(ImDrawIdx): 2, sizeof(ImDrawVert): 20
define: __cplusplus=199711
define: _WIN32
define: _WIN64
define: _MSC_VER=1929
define: _MSVC_LANG=201703
define: IMGUI_HAS_VIEWPORT
define: IMGUI_HAS_DOCK
--------------------------------
io.BackendPlatformName: imgui_impl_win32
io.BackendRendererName: imgui_impl_dx11
io.ConfigFlags: 0x0000C441
 NavEnableKeyboard
 DockingEnable
 ViewportsEnable
 DpiEnableScaleViewports
 DpiEnableScaleFonts
io.ConfigViewportsNoDecoration
io.ConfigInputTextCursorBlink
io.ConfigWindowsResizeFromEdges
io.ConfigMemoryCompactTimer = 60.0
io.BackendFlags: 0x00001C0E
 HasMouseCursors
 HasSetMousePos
 PlatformHasViewports
 HasMouseHoveredViewport
 RendererHasVtxOffset
 RendererHasViewports
--------------------------------
io.Fonts: 1 fonts, Flags: 0x00000000, TexSize: 512,64
io.DisplaySize: 1264.00,761.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:

I want to implement custom keyboard navigation for optimized workflow of the common use cases in a modal dialog, while still providing a fallback to default keyboard navigation for more complex usage without needing to touch the mouse. The custom navigation is no problem, but I can't seem to get the switch to the default navigation just right. The example code has a modal popup with basically no functionality, it is stripped down to just demonstrate the issue.

When I press and release the left Alt key, I want to switch to the default keyboard navigation. I want the filter input to be visibly highlighted, but not active, so that I could f.e. navigate to the button right of it by pressing right. That almost works (with setting a few internals), except for the deactivation of the widget. If I clear the active ID, the text input stays active, same if I defer that clear to the next frame. And because of being active, the text input consumes both left and right arrow keys. Calling NavMoveRequestCancel() will deactivate the item, but gets rid of the highlight as well, which is also undesirable. I also tried setting the nav item, window and so on directly to the state that the metrics window shows, but without success. I also added Escape key down and up events to the input queue to emulate an initial "manual" deactivation, didn't help either. Is there a way I could fiddle with the internals to achieve what I want?

There is also a related issue reproducible with the example code, one that I think is an actual bug:
When hitting Escape after pressing Alt, the items gets deactivated (the state I want after pressing Alt). When hitting Escape again, the navigation moves up a level to the child window. In a non-flattened window, the entire child window would then be highlighted, as is the case when navigating into the item list and back out again. Hitting Escape again would then disable the highlight display, triggering my return to custom navigation. But the "Filters" child window has flattened navigation. When hitting Escape, the highlight should be moved further up and therefore properly deactivated, but it won't be. Instead, it stands invisibly on the child window with flattened navigation while being marked as visible (GetIO().NavVisible is true), as can be seen in the metrics window.

Screenshots/Video:

No response

Minimal, Complete and Verifiable Example code:

// call once per frame in the update loop, needs access to imgui_internal.h
void testMixedNav()
{
	static bool customNavActive        = true;
	static bool defaultNavVisible      = false;
	static bool setNavTargetNextFrame  = false;
	static bool clearActiveIdNextFrame = false;

	ImGuiContext& ctx = *ImGui::GetCurrentContext();
	ImGuiIO&      io  = ImGui::GetIO();

	bool focusFilter  = false;
	bool customNav    = customNavActive; // delay by one frame, don't use custom handling for escape when using that to end default keyboard navigation
	bool setNavTarget = setNavTargetNextFrame;

	setNavTargetNextFrame = false;

	if ( clearActiveIdNextFrame )
	{
		ImGui::ClearActiveID();
		clearActiveIdNextFrame = false;
	}

	if ( !customNavActive && defaultNavVisible && !io.NavVisible ) // effectively hitting escape until the highlight disappears
	{
		customNavActive = true;
		focusFilter     = true;
	}

	defaultNavVisible = io.NavVisible;

	if ( customNavActive && ImGui::IsKeyReleased( ImGuiKey_LeftAlt ) )
	{
		customNavActive = false;
		customNav       = false;

		//setNavTarget          = true;
		setNavTargetNextFrame = true; // deferring by 1 frame prevents default navigation from seeing a release of ImGuiKey_LeftAlt
	}

	if ( setNavTarget )
	{
		// according to the metrics/debugger window, these are the values in the desired state
		io.NavActive  = true;
		io.NavVisible = true;

		ctx.NavDisableHighlight  = false;
		ctx.NavDisableMouseHover = true;

		focusFilter = true; // will supply the correct nav item, window and rect, but activates the item as well
	}

	static bool openPopup = true;

	// Ctrl+M to re-open the modal popup, useful f.e. to temporarily close it and open the metrics window
	if ( !openPopup && !ImGui::IsPopupOpen( "MixedNav" ) && ImGui::IsKeyChordPressed( ImGuiKey_M | ImGuiMod_Ctrl ) )
		openPopup = true;

	if ( openPopup )
	{
		ImGui::OpenPopup( "MixedNav" );
		openPopup = false;
	}

	const ImGuiWindowFlags windowNavFlags = customNavActive ? ImGuiWindowFlags_NoNavInputs : ImGuiWindowFlags_None;

	if ( ImGui::BeginPopupModal( "MixedNav", nullptr, ImGuiWindowFlags_None | windowNavFlags ) )
	{
		if ( ImGui::IsWindowAppearing() )
			focusFilter = true;

		if ( ImGui::BeginChild( "Filters", ImVec2( 0.0f, ImGui::GetFrameHeight() ), ImGuiChildFlags_None, ImGuiWindowFlags_NavFlattened | windowNavFlags ) )
		{
			const float buttonWidth = ImGui::CalcTextSize( "Clr" ).x + 2.0f * ImGui::GetStyle().FramePadding.x;

			if ( focusFilter )
				ImGui::SetKeyboardFocusHere();

			static char pattern[ 256 ] = "*";

			ImGui::SetNextItemWidth( -buttonWidth - ImGui::GetStyle().ItemSpacing.x );
			ImGui::InputText( "##Pattern", pattern, 256 );

			if ( setNavTarget )
			{
				//ImGui::ClearActiveID(); // doesn't deactivate the item :(
				//ImGui::NavMoveRequestCancel(); // cancles the highlight display as well :(
				clearActiveIdNextFrame = true; // deferring by one frame doesn't deactivate the item either :(
			}

			ImGui::SameLine();
			ImGui::Button( "Clr" );
		}
		ImGui::EndChild();

		if ( ImGui::BeginChild( "Items", ImVec2( 0.0f, -ImGui::GetFrameHeightWithSpacing() ), ImGuiChildFlags_None, ImGuiWindowFlags_None | windowNavFlags ) )
		{
			static bool selected[ 10 ] = {};

			for ( int i = 0; i < 10; ++i )
			{
				char name[ 32 ];
				sprintf_s( name, "Item #%i", i );

				ImGui::Selectable( name, &selected[ i ] );
			}
		}
		ImGui::EndChild();

		if ( ImGui::Button( "OK" ) || ( customNav && ImGui::IsKeyPressed( ImGuiKey_Enter ) ) )
			ImGui::CloseCurrentPopup();

		ImGui::SameLine();

		if ( ImGui::Button( "Cancel" ) || ( customNav && ImGui::IsKeyPressed( ImGuiKey_Escape ) ) )
			ImGui::CloseCurrentPopup();

		ImGui::EndPopup();
	}
}
@ocornut ocornut added the nav keyboard/gamepad navigation label Jan 19, 2024
@GamingMinds-DanielC
Copy link
Contributor Author

GamingMinds-DanielC commented Jan 19, 2024

Update: I managed to get the first part of the issue working with this adjustment:

		if ( ImGui::BeginChild( "Filters", ImVec2( 0.0f, ImGui::GetFrameHeight() ), ImGuiChildFlags_None, ImGuiWindowFlags_NavFlattened | windowNavFlags ) )
		{
			// ...

			ImGui::SetNextItemWidth( -buttonWidth - ImGui::GetStyle().ItemSpacing.x );
			ImGui::InputText( "##Pattern", pattern, 256 );

			if ( setNavTarget )
			{
				// requesting a move to the right will deactivate the item without moving the highlight to the right,
				// feels kinda dirty but works :)
				ImGui::NavMoveRequestForward( ImGuiDir_Right, ImGuiDir_Right, ImGuiNavMoveFlags_None, ImGuiScrollFlags_None );
			}

			ImGui::SameLine();
			ImGui::Button( "Clr" );
		}
		ImGui::EndChild();

I can detect the second part of the issue (nav id references a child with flattened navigation) and reenable my custom navigation, but I still think it is worth a look. It doesn't feel right that the flattened child window is the current target and reports a visible highlight without having one.

@ocornut
Copy link
Owner

ocornut commented Jan 19, 2024

It would take a while to parse this issue and I only could skim through but I am not sure I understand what you aim or mean by "mixed" mode. You can leave nav system alone and query enter/escape keys fine?
Note sure I understand why using Alt toggle mode and how that's natural/standard UX.
But again I've skimmed very briefly through this.

@ocornut
Copy link
Owner

ocornut commented Jan 19, 2024

I want to implement custom keyboard navigation for optimized workflow of the common use cases in a modal dialog,

Your explanation and code seems very complicated and XY-problem-ey could you clarify exactly what you are trying to achieve in the first place?

@ocornut ocornut added the inputs label Jan 19, 2024
@GamingMinds-DanielC
Copy link
Contributor Author

So, first about the why, the explanation of what I'm trying to achieve:

I have a modal popup that is used to spawn assets. That fast way of using it is to open it with a keyboard shortcut, type a bit to filter from the available assets, select the desired one with arrow keys and then press enter to close the popup and place the asset (or escape to close and cancel). While the user is selecting the asset with up/down arrows in the asset list, the keyboard focus stays on the filter so that any typing can be used to further refine the filter directly. Pretty efficient usage.

I can't use the default keyboard navigation for this, while selecting the items the filter would loose focus. Then again, I have advanced options next to the filter and potentially additional filters that I want to make accessible with keyboard navigation as well. Those are not accessible with the quick custom navigation, but the default keyboard navigation works pretty well for this, Therefore I made a "mixed mode", a way to toggle between different navigation modes. The alt key is certainly no industry standard for this, but it felt natural since it is used to trigger an alternate mode, that's what the key is usually meant for.

Now the technical issues in short form:

  1. No clean way to toggle keyboard navigation with highlight visible on an inactive item. Got it working in my update to the issue, but it's more of a hack that could stop working in any future version of the library.

  2. No simple way to detect if keyboard navigation currently has a visible highlight. The visibility is incorrectly reported in the internals in case of navigating up (with the escape key) from within a child window with flattened navigation.

ocornut added a commit that referenced this issue Jan 19, 2024
…f Enter key as it doesn't go through Shortcut

InputText: no need to call SetShortcutRouting() directly.
Tangential to experiments for #7237
ocornut added a commit that referenced this issue Jan 19, 2024
- It doesn't sense to test route without ownership (which may be overrided by code not using routing)
- It also wouldn't be possible to call Shortcut() with _None anyway, since successful routing sets ownership.
Tangential to experiments for #7237
@ocornut
Copy link
Owner

ocornut commented Jan 19, 2024

I don't think your "technical issues 1 & 2" are things you should focus at the moment, below I'll give you pointers to concepts which should help solving this better. There might be remaining issues related to highlight we can discuss afterwards but I don't think it matter as much as you think it does.

While the user is selecting the asset with up/down arrows in the asset list, the keyboard focus stays on the filter so that any typing can be used to further refine the filter directly. Pretty efficient usage.
I can't use the default keyboard navigation for this, while selecting the items the filter would loose focus.

I have among my upcoming task list to look into #2057 and #718 and provide better idioms on how to do that, TL;DR; there are many layers to it and its not simple to do ideally but I think you can approach already.

I tried to implement something and found some issues specifically related to Enter key that I fixed now (2 commits linked above),

Some key concepts:

  • When you claim ownership of a key, Navigation won't use it.
  • Shortcut() will register an input routing request, claim key ownership when Mods match, and succeed when scoring passes.

In this attempt, notice how Shortcut() can be leveraged both to open the popup, handle up/down while stealing it away from navigation, and handle enter/escape in a way compatible with using the InputText(). It's also less code.

void testMixedNav2()
{
    // Ctrl+M to re-open the modal popup, useful f.e. to temporarily close it and open the metrics window
    //if (!ImGui::IsPopupOpen("MixedNav") && ImGui::IsKeyChordPressed(ImGuiKey_M | ImGuiMod_Ctrl))
    if (ImGui::Shortcut(ImGuiKey_M | ImGuiMod_Ctrl, 0, ImGuiInputFlags_RouteGlobalLow))
        ImGui::OpenPopup("MixedNav");

    if (ImGui::BeginPopupModal("MixedNav"))
    {
        const bool focusFilter = ImGui::IsWindowAppearing();

        // Programmatically decide of a condition to decide whether Enter to close will be enabled.
        // May base it based on which child is focused?
        bool disableEnterToClose = false;

        if (ImGui::BeginChild("Filters", ImVec2(0.0f, ImGui::GetFrameHeight()), ImGuiChildFlags_None, ImGuiWindowFlags_NavFlattened))
        {
            const float buttonWidth = ImGui::CalcTextSize("Clr").x + ImGui::GetStyle().FramePadding.x * 2.0f;

            if (focusFilter)
                ImGui::SetKeyboardFocusHere();

            static char pattern[256] = "*";

            ImGui::SetNextItemWidth(-buttonWidth - ImGui::GetStyle().ItemSpacing.x);
            ImGui::InputText("##Pattern", pattern, 256);
            disableEnterToClose |= ImGui::IsItemFocused();
            if (ImGui::IsItemActive())
            {
                if (ImGui::Shortcut(ImGuiKey_UpArrow))
                    printf("Up!\n"); // todo: update your selection data
                if (ImGui::Shortcut(ImGuiKey_DownArrow))
                    printf("Down!\n"); // todo: update your selection data
            }

            ImGui::SameLine();
            ImGui::Button("Clr");
        }
        ImGui::EndChild();

        if (ImGui::BeginChild("Items", ImVec2(0.0f, -ImGui::GetFrameHeightWithSpacing())))
        {
            static bool selected[10] = {};
            for (int i = 0; i < 10; ++i)
            {
                char name[32];
                sprintf_s(name, "Item #%i", i);
                ImGui::Selectable(name, &selected[i]);
            }
        }
        ImGui::EndChild();

        if (ImGui::Button("OK") || (!disableEnterToClose && ImGui::Shortcut(ImGuiKey_Enter)))
            ImGui::CloseCurrentPopup();

        ImGui::SameLine();

        if (ImGui::Button("Cancel") || (ImGui::Shortcut(ImGuiKey_Escape)))
            ImGui::CloseCurrentPopup();

        ImGui::EndPopup();
    }
}

Try to toy with it and let me know.

What remains IMHO:

  • I dislike how you would specifically need to handle the Up/Down arrows. While it is probably easy in this case to maintain a last-selected-item and use that to control focus, I would like to make it possible to simulate an actual navigation in that remote scope, so we should later benefit from more features such as automatically supporting Shift-select and Ctrl-select.
  • It also means you likely need to overwrite active InputText() contents which itself is a known mess.

@ocornut
Copy link
Owner

ocornut commented Jan 19, 2024

About this part:

// Programmatically decide of a condition to decide whether Enter to close will be enabled.
// May base it based on which child is focused?
bool disableEnterToClose = false;
[...]
ImGui::InputText("##Pattern", pattern, 256);
disableEnterToClose |= ImGui::IsItemFocused();
[...]
if (!disableEnterToClose && ImGui::Shortcut(ImGuiKey_Enter))
    ImGui::CloseCurrentPopup();

I added the filter specifically to allow pressing Enter to REACTIVATE the InputText() when focusing.
I will need to explore this in more details, but I imagine we could come up with more standardized ways to handle this.

@GamingMinds-DanielC
Copy link
Contributor Author

Thank you. I will try out the Shortcut() way, but won't be able to get around to it before next week.

I also took a look at NavVisible since that doesn't accurately report (see issue part 2). Besides for debug displays, this value is never used in the library itself, so making it accurate is no risk. And making it accurate would even be simpler code. Instead of the current calculation that combines quite a few values...

  • add a bool NavHighlightRendered to ImGuiContext, initialized to false
  • in RenderNavHighlight(), set NavHighlightRendered to true once the early outs have been survived
  • in NavUpdate(), set ImGuiIO::NavVisible to ImGuiContext::NavHighlightRendered, then reset that to false

That would then reflect the actual visibility exactly and setting the value won't have to reconstruct the conditions for display based on other values.

@ocornut
Copy link
Owner

ocornut commented Jan 19, 2024

The visibility is incorrectly reported in the internals in case of navigating up (with the escape key) from within a child window with flattened navigation.

I will look into fixing it first.

At that point I'm not even totally sure of the purpose of io.NavVisible, once we tackle move Nav issues we can revisit it.

@ocornut
Copy link
Owner

ocornut commented Jan 19, 2024

The visibility is incorrectly reported in the internals in case of navigating up (with the escape key) from within a child window with flattened navigation.

Fixed with 763100b

@GamingMinds-DanielC
Copy link
Contributor Author

Fixed with 763100b

Thank you. I have tried it in a test application and can confirm that flattened navigation now works as expected.

ocornut added a commit that referenced this issue Oct 18, 2024
ocornut added a commit that referenced this issue Oct 18, 2024
…(), ImGuiNavHighlightFlags to ImGuiNavRenderCursorFlags. (#1074, #2048, #7237, #8059, #1712, #7370, #787)

+ referenced in #8057, #3882, #3411, #2155, #3351, #4722, #1658, #4050.
ocornut added a commit that referenced this issue Oct 18, 2024
+ Further internal renaming for consistency.
ocornut added a commit that referenced this issue Oct 18, 2024
…ways. (#1074, #2048, #7237, #8059, #3200, #787)

Note: the NavCursorHideFrames addition is to support 88a3545 even though ConfigNavCursorVisibleAlways is set.
@ocornut
Copy link
Owner

ocornut commented Oct 18, 2024

I have added a few things related to those requests:

  • Nav: added io.ConfigNavCursorVisibleAuto and io.ConfigNavCursorVisibleAlways config options (ab9ce2a).
  • Nav: added SetNavCursorVisible() to override current state (634a7ed).

Technically I think the config options are sufficient for most people.

Other tangentially related options:

  • Nav: added io.ConfigNavEscapeClearFocusItem (7a56b41).
  • Nav: added io.ConfigNavEscapeClearFocusWindow (b001038).
  • And a couple of related fixes for all the above.

Technically this # also pertain to use SetKeyboardFocusHere() and other functions which are still be redesigned/improved.
This issue was also a little bit multi-faceted, and addressed other things (e.g. NavFlattened) so I'd rather close it but PLEASE let me know is one aspect of it isn't solved (and we could reopen a new issue).

(

Also note, if you have mods using internals, that those two fields have been renamed internally:

  • Terminology has been changed to call this the "nav cursor" more consistently.
  • bool NavDisableHighlight -> became bool NavCursorVisible (with opposite value).
  • bool NavDisableMouseHover -> became bool NavDisableMouseHover

)

@ocornut ocornut closed this as completed Oct 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
inputs nav keyboard/gamepad navigation
Projects
None yet
Development

No branches or pull requests

2 participants