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

Don't bring appearing host window to front unless any of the contained… #7116

Open
wants to merge 2 commits into
base: docking
Choose a base branch
from

Conversation

GamingMinds-DanielC
Copy link
Contributor

… windows require initial focus

I took a closer look at how to implement the "officially" suggested fix for one of the problems in #7008, specifically this one:

About Daniel's issue: seems like there's two steps for this. Making the dock node using _NoFocusOnAppearing if all just appearing windows share this flag, I think is a reasonable first move and would fix the biggest problem you have (focus stolen away from current menu), ...

The fix was surprisingly easy since all of the needed information is available right there. If the host window is appearing, instead of just bringing it to the front, it now checks its contained appearing windows first and is brought to the front only if one (or more) of the contained appearing windows has ImGuiWindowFlags_NoFocusOnAppearing not set (thus requiring initial focus).

This fix makes the behavior of appearing windows consistent regardless of how they may be docked. Here are some GIFs of the before and after behavior. Ctrl+Click is custom functionality through wrappers (no library change) for "sticky" menu items. They keep the menu open while appearing windows get the ImGuiWindowFlags_NoFocusOnAppearing flag.

Normal click closes the menu and opens the window without the flag, the behavior is not changed by the fix:
no_focus_fix_default

Ctrl+Click keeps the menu open, window gets the flag, but host window gets brought to the front anyway without the fix:
no_focus_fix_error

Ctrl+Click with the fix results in the desired behavior:
no_focus_fix_wanted

@ocornut
Copy link
Owner

ocornut commented Dec 11, 2023

Thanks for the PR!
Considering the general often-backfiring complexity of those stuff it would be good to consider writing an exhaustive imgui_test_suite test for it. It's also a good habit to try to consider any other side effects of the change, and while doing so devise some additional test for unchanged or changed behavior.

@GamingMinds-DanielC
Copy link
Contributor Author

I haven't familiarized myself with the test suite yet, but yes, exhaustive automatic testing would be good. I will give it a try when possible, but can't promise anything soon.

I thought I did pretty exhaustive manual testing, but it turns out I missed a case. Or a few cases when docking already existing windows together. Then there are no appearing windows except for the host window, which doesn't get brought to the display front. In most cases this looks fine, but technically the order is wrong. And in the case where window A is partially overlapped by B, then C gets manually docked into A, then C has focus but is still overlapped by B. I could just bring the host window to the front if none of the windows are appearing, but that would exclude complex cases where windows are being docked while new ones are appearing. And I like the idea of not loosing the current topmost window if two other non-topmost windows get docked together programmatically. So I just tested the combined criteria of any appearing window wanting focus or (that's the addition) any contained window having been the previous topmost window, then the host window should be brought to the front. So far this seems to work as desired, but I need to do a bit more testing to be sure. I will update tomorrow.

@GamingMinds-DanielC
Copy link
Contributor Author

I pushed an update after quite extensive testing. It is still rather simple, but handles all of the cases now:

  • The appearing host window gets brought to the front not only if any of the appearing windows requires focus, but also if any of the previously existing windows currently has focus. This handles all cases of manual docking as well since dragging either a single window or another host with multiple windows into a window to create a new host guarantees that the dragged window (or one of them if a host is dragged) has focus.
  • Remaining cases are any where one or more windows get docked programmatically into a new host without any of them having focus. For this case the new host gets brought under the reference window (first window that defines the position and size of the new host). This way the display order of the reference window is preserved after moving into the appearing host window. Other windows that are not part of the dock operation will remain in front of or behind the reference window just as they were before.
  • There is one purely theoretical case where a host window is appearing without any contained windows, thus having no reference window. In that case there is no display order to retain and the host window is not brought to the front either, but that case should never happen anyway (possibly triggered by erroneous code?). Just in case, the presence of a reference window is still checked so that it won't crash.

Now this improved solution handles all of the cases in a neat way. No fully automatic test case yet, but I did a lot of manual testing (appearing windows, docking entirely manual, docking with code in all variations). And the complexity of the change is so low that all possible paths can be verified.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants