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

SetItemDefaultFocus scroll improvement suggestions #2812

Closed
DomGries opened this issue Sep 28, 2019 · 4 comments
Closed

SetItemDefaultFocus scroll improvement suggestions #2812

DomGries opened this issue Sep 28, 2019 · 4 comments
Labels
nav keyboard/gamepad navigation scrolling

Comments

@DomGries
Copy link
Contributor

Hey, I think SetItemDefaultFocus scrolling could be improved due to the following minor issues:

  • An item which is barely visible (1px) is not being visibly focused
  • Scrolling to an invisible item could be improved to fix the jump between barely visible and completely centered

I think the best behavior would be to copy what the .NET ListView EnsureVisible method does: https://docs.microsoft.com/en-us/dotnet/api/system.windows.forms.listview.ensurevisible?view=netframework-4.8

If the item that you want to ensure is visible is located above the viewable region, calling the EnsureVisible method will scroll the contents until it is the first item in the viewable area. If the item is below the viewable region, calling the EnsureVisible method will scroll the contents until the item is the last item in the viewable area.

Dear ImGui 1.73

@ocornut
Copy link
Owner

ocornut commented Sep 30, 2019

Hello,

Could you provide concrete example/screenshots of both of your case?

If SetItemDefaultFocus() is called on a frame where the window is appearing, I can think of cases where it would be preferable that the be centered. e.g. for a list of items in a combo box.
Perhaps the behavior should differ depending on whether the window is appearing? Would need some more thought..

@DomGries
Copy link
Contributor Author

Case 1

Popup window does not visibly select 1280x720 button because 1px is already visible (the black line at bottom under 1024x768) when calling SetItemDefaultFocus(). I think the logic should be changed so that it should also scroll when the item is not fully visible:

Capture

Case 2

Only centering the view on the item when it is not visible is weird especially combined with suggested fix of case 1.

Assume that the focused item is just barely fully visible then it would be at the very bottom (or top). But if the focused item is just the one below that then it results in it being completely centered which is a jump in behaviour.

I just tested default comboboxes on Windows and they actually simply try to scroll so that the item is at the top or as much as possible at the top. So in SetItemDefaultFocus that would be SetScrollHereY(0.f) without the IsItemVisible() check.

@ocornut
Copy link
Owner

ocornut commented Sep 30, 2019

Case 1: Totally agreeing.

Case 2: I don't think your answer refers to the same idea that I suggested? I suggested using the "IsWindowAppearing" test. Typically behave different on the frame a window is appearing vs if function is called while the window is already up. This is unrelated to the visibility of items within a given window.

Scrolling to the center or to the top is another decision to do. I agree that the behavior of ScrollToBringRectIntoView() is more desirable in some situation. This is also what I asked for a usage scenario here where that behavior is the most desirable (scrolling to make item appear at edge vs centering).

@ocornut ocornut added the nav keyboard/gamepad navigation label Oct 2, 2019
ocornut added a commit that referenced this issue Sep 29, 2021
ocornut added a commit that referenced this issue Sep 29, 2021
ocornut added a commit that referenced this issue Nov 2, 2021
…ed (fix nav in one axis scrolling back and forth between axises when space is tight by just < ItemSpacing*2) (#3692, #2812, #4242, #2900)

Amend 8f495e5
actondev pushed a commit to actondev/imgui that referenced this issue Nov 26, 2021
…ed (fix nav in one axis scrolling back and forth between axises when space is tight by just < ItemSpacing*2) (ocornut#3692, ocornut#2812, ocornut#4242, ocornut#2900)

Amend 8f495e5
ocornut added a commit that referenced this issue Nov 24, 2022
…ocus()/ScrollToRectEx() during an appearing form not centering item. (#5902, #2812, #4242, #2900)

Amend 44f8011 and 8f495e5
ocornut pushed a commit that referenced this issue Dec 20, 2022
…tered element that is not visible but could be would take the item's Y coordinate into account.

Neither behavior were used in the codebase for this axis.
Amend 27c58c3 (#5902, #2812, #4242, #2900)
Signed-off-by: Neil Bickford <nbickford@nvidia.com>
@ocornut
Copy link
Owner

ocornut commented Mar 10, 2023

Closing as per #2814, both cases should be solved. Thanks for your patience!

@ocornut ocornut closed this as completed Mar 10, 2023
kjblanchard pushed a commit to kjblanchard/imgui that referenced this issue May 5, 2023
…ocus()/ScrollToRectEx() during an appearing form not centering item. (ocornut#5902, ocornut#2812, ocornut#4242, ocornut#2900)

Amend 44f8011 and 8f495e5
kjblanchard pushed a commit to kjblanchard/imgui that referenced this issue May 5, 2023
…tered element that is not visible but could be would take the item's Y coordinate into account.

Neither behavior were used in the codebase for this axis.
Amend 27c58c3 (ocornut#5902, ocornut#2812, ocornut#4242, ocornut#2900)
Signed-off-by: Neil Bickford <nbickford@nvidia.com>
kjblanchard pushed a commit to kjblanchard/imgui that referenced this issue May 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
nav keyboard/gamepad navigation scrolling
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants