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

Fixed horizontal navigation issue for NavClampRectToVisibleAreaForMoveDir #2221

Conversation

zzzyap
Copy link

@zzzyap zzzyap commented Nov 30, 2018

Issue:

  • If the value of the column is unclipped and long (such that the center of its rect is past the item on the right column).
    Trying to move left into this column from the column on its right, would ignore this column.
    (Do yell out if this ^ doesn't make any sense)

Changes:

  • If the window has columns, clip the rect's horizontal bounds.

zzzyap and others added 2 commits November 30, 2018 09:13
When the focused window become inactive don't restore focus to a window with the ImGuiWindowFlags_NoInputs flag. (ocornut#2213)
… unclipped name in the left column is long enough to make its center pass the left-hand-side of the entry on the right, trying to go left would ignore that entry. This fixes it.
@ocornut ocornut added the nav keyboard/gamepad navigation label Nov 30, 2018
@ocornut
Copy link
Owner

ocornut commented Dec 3, 2018

Please provide a screenshot with your problematic layout for reference.
I need to look at this in details as it this may possibly be causing more problems.

(We might also want to bake this info somehow because NavScoreItem() > NavClampRectToVisibleAreaForMoveDir() can be called a lot, we can consider this once the right solution is found).

@zzzyap
Copy link
Author

zzzyap commented Dec 3, 2018

For reference, the code to generate the widget in the following examples:

ImGui::Begin("NavClamp Example");
ImGui::Columns(2);
ImGui::SmallButton("Really really really really really really long text");
ImGui::NextColumn();
ImGui::SmallButton("Short text");
ImGui::NextColumn();
ImGui::End();

nofix
Using nav with keyboard (Left arrow) without fix, can't move into left column.


fixed
Using nav with keyboard (Left arrow) with fix, can move into left column.


streched-out
The widget stretched out for reference.


45kxug85ou
Using nav with keyboard (Left arrow) without fix, to demonstrate:

the center of its rect is past the item on the right column

@ocornut ocornut force-pushed the master branch 2 times, most recently from 0c1e5bd to bb6a60b Compare August 27, 2021 19:10
@ocornut ocornut force-pushed the master branch 2 times, most recently from 8b83e0a to d735066 Compare December 13, 2021 11:31
@ocornut ocornut force-pushed the master branch 2 times, most recently from b3b85d8 to 0755767 Compare January 17, 2022 14:21
@ocornut ocornut force-pushed the master branch 3 times, most recently from c817acb to 8d39063 Compare February 15, 2022 16:25
@ocornut ocornut added the bug label Apr 20, 2023
ocornut added a commit that referenced this pull request Apr 20, 2023
@ocornut
Copy link
Owner

ocornut commented Apr 20, 2023

I thoroughly apologize for not getting on this earlier... I am currently doing a pass at navigation issue and stumbled on this.
I suspect in practice it didn't happen "so much" because code layout clickable contents in table/columns would probably have a tendency to right-align it.

Pushed fix 00d3f92 + Pushed regression test ocornut/imgui_test_engine@c953209

The fix I pushed is slightly different:

  • The thing was slightly abstract in term of a "NavIsScrollPushableX" concept which could apply to other items such as a PushWorkRect() API I intend to provide later. I also cached the result and stored it in a hot location as this is a pre-clipping check when a navigation query is running.
  • I am doing the clamp earlier (in NavProcessItem()), one of the reason which is not immediately obvious it that the clamp also needs to be done on the "source" scoring rect (generally == current nav location). This isn't making a difference in current version because we currently do scoring_rect.Max.x = scoring_rect.Min.x; in NavUpdateCreateMoveRequest() so the scoring rect is currently a line instead of a rect, but as soon as you revert it to be a rect you need this clamping to be done the same way, otherwise the same bug manifest but with the source item instead of the target candidate item.

Thank you for your patience 🙇

@ocornut ocornut closed this Apr 20, 2023
ocornut added a commit that referenced this pull request Apr 24, 2023
…ped. (#2221)

Amend 00d3f92 + older f2d1472, 0cc20fc
+ Add ImGuiNavMoveFlags_WrapMask_ for good measure.
@ocornut
Copy link
Owner

ocornut commented Apr 24, 2023

Amended with eed7b0e : the code in NavClampRectToVisibleAreaForMoveDir() now seems unnecessary.

kjblanchard pushed a commit to kjblanchard/imgui that referenced this pull request May 5, 2023
kjblanchard pushed a commit to kjblanchard/imgui that referenced this pull request May 5, 2023
…ped. (ocornut#2221)

Amend 00d3f92 + older f2d1472, 0cc20fc
+ Add ImGuiNavMoveFlags_WrapMask_ for good measure.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug nav keyboard/gamepad navigation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants