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

Closing windows restoring z-ordered focus / how to implement menu command for top-most window #727

Open
pdoane opened this issue Jul 6, 2016 · 12 comments
Labels
focus nav keyboard/gamepad navigation

Comments

@pdoane
Copy link

pdoane commented Jul 6, 2016

I am trying to implement a generic command (e.g. File/Close) that should apply to the top-most window. My assumption is that I should write something like this:

    static bool closeWindow;

    closeWindow = false;
    if (ImGui::BeginMainMenuBar())
    {
        if (ImGui::BeginMenu("File"))
        {
            if (ImGui::MenuItem("Close", "Ctrl+W")) { closeWindow = true };
        }
    }

    for (auto it = documents.begin(); it != documents.end(); )
    {
        bool opened = true;
        if (ImGui::Begin(it->name), &opened)
        {
            if (<Is Top Level Window> && closeWindow)
                opened = false;
        }

        if (!opened)
            it = documents.erase(it);
        else
            ++it;
    }

I was thinking I could implement by using IsWindowFocused() but this doesn't work because the menu bar claims focus.

Does this seem like the right strategy or is there another approach that should be used?

@ocornut
Copy link
Owner

ocornut commented Jul 6, 2016

You probably need to keep track of your last-focused main document then.

As soon as you start introducing any other window than your "document" window you'll run into the same problem.

@pdoane
Copy link
Author

pdoane commented Jul 6, 2016

I tried that approach as well - but it didn't work out, e.g.:

    static Document* s_activeDocument;
    for (auto it = documents.begin(); it != documents.end(); )
    {
        if (ImGui::Begin(it->name))
        {
            if (ImGui::IsWindowFocused())
                s_activeDocument = &*it;
        }
    }

    if (ImGui::BeginMainMenuBar())
    {
        if (ImGui::BeginMenu("File"))
        {
            if (ImGui::MenuItem("Close", "Ctrl+W"))
            { /* erase s_activeDocument from documents };
        }
    }

Now the first Close operation works but the menu bar is still the focus so there is no active document anymore and the second Close operation doesn't do anything. I could require that the user click in the window again to get focus but that's confusing compared to how GUIs typically work.

It might be possible to extend this to keeping track of all documents in focus order. E.g.:

    static vector<Document>* s_documentsInFocusOrder;
    for (auto it = documents.begin(); it != documents.end(); )
    {
        if (ImGui::Begin(it->name))
        {
            if (ImGui::IsWindowFocused())
            {
                /* remove document from s_documentsInFocusOrder and put it at the head */
            }
        }
    }

@ocornut
Copy link
Owner

ocornut commented Jul 6, 2016

Well the whole point of tracking the last active document is that when a menu is open you don't lose this information of which one is active.
And the concept of closing a document is completely your code, so there is clearly a bug or misunderstanding on your side.

Sent from my fax machine

On 7 Jul 2016, at 06:05, pdoane [email protected] wrote:

I tried that approach as well - but it didn't work out, e.g.:

static Document* s_activeDocument;
for (auto it = documents.begin(); it != documents.end(); )
{
    if (ImGui::Begin(it->name))
    {
        if (ImGui::IsWindowFocused())
            s_activeDocument = &*it;
    }
}

if (ImGui::BeginMainMenuBar())
{
    if (ImGui::BeginMenu("File"))
    {
        if (ImGui::MenuItem("Close", "Ctrl+W"))
        { /* erase s_activeDocument from documents };
    }
}

Now the first Close operation works but the menu bar is still the focus so there is no active document anymore and the second Close operation doesn't do anything. I could require that the user click in the window again to get focus but that's confusing compared to how GUIs typically work.

It might be possible to extend this to keeping track of all documents in focus order. E.g.:

static vector<Document>* s_documentsInFocusOrder;
for (auto it = documents.begin(); it != documents.end(); )
{
    if (ImGui::Begin(it->name))
    {
        if (ImGui::IsWindowFocused())
        {
            /* remove document from s_documentsInFocusOrder and put it at the head */
        }
    }
}


You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.

@pdoane
Copy link
Author

pdoane commented Jul 6, 2016

Maybe I'm not following how to track the active document state. How can I do this without querying the window focus value?

It's also possible that things are behaving as you would expect, but it's pretty different than typical desktop applications which I think will be confusing.

@pdoane
Copy link
Author

pdoane commented Jul 7, 2016

Also, just to clearly spell out the user experience:

  • There is a global menu bar and several document windows. No other windows exist.
  • The global menu bar has commands which should be applied to active window. File/Close is a particular one that is problematic.
  • The user chooses File/Close multiple times in succession and expects the documents to be closed according to their current z-order of display.

I can track the last active document by querying the focus state of the window as I iterate the application state for documents. This provides a solution for most commands that need to route to the active document. File/Close is different though because it changes which document is active. If the mechanism to do this is querying the focus state, that has been transferred to the menu bar so there is no longer any active document until the users clicks on a window.

@ocornut
Copy link
Owner

ocornut commented Jul 7, 2016

You were doing the right thing.
Just only set the value for your "document" window that's focused, and keep the value unmodified otherwise, aka save it locally.

On 7 Jul 2016, at 08:42, pdoane [email protected] wrote:

Maybe I'm not following how to track the active document state. How can I do this without querying the window focus value?

It's also possible that things are behaving as you would expect, but it's pretty different than typical desktop applications which I think will be confusing.


You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.

@pdoane
Copy link
Author

pdoane commented Jul 7, 2016

But again that doesn't work (at least with a single state value). The sequence looks like this:

Initial state:

  • Documents are traversed, windows are submitted, one of them is the focus and the active document value is latched.

1st Close Command:

  • Menu bar processing begins and focus is transferred to it
  • Close command is executed and dispatched to the active document
  • Focus is still the menu bar

Next frame:

  • Documents are traversed to build the window. None of them are the focus so the active document remains null.

2nd close command:

  • Focus is still the menu bar
  • Close is chosen by the user but it doesn't know where to route it.

If the internal ImGui z-order structure is mirrored application side, then this can be made to work but it doesn't seem like you are suggesting that.

@johanwendin
Copy link

Why not keep a (unique) stack of focused windows locally then?

After each close command, just close the .top(), pop the stack and set focus to the new .top() ?

@pdoane
Copy link
Author

pdoane commented Jul 7, 2016

Yes that's possible to do and I've suggested it a couple of times in the thread.

What I'd like to establish is how we expect the problem to be solved. My first thought was exactly the same as Omar - just track the last active document, and that worked great until I hit the case for closing the window.

So if the thinking is that maintaining an application side copy of the ImGui window z-order for command processing is the right way to solve the problem, then we're done.

But I think there is a bigger issue with menu bar focus behavior. It was a minor issue discussed in #622 but this case creates user confusion without a workaround applied.

@ocornut
Copy link
Owner

ocornut commented Jul 12, 2016

OK. I see what you are saying. In that sort of case you expect the menu-bar so give back focus to highest remaining window in the z-order, and the case discussed in #622 also would need to be honored. Actually as part of #323 I am already doing something along the line of #622 but it is still wip.

Will get back to this later.

You might try to add ImGuiWindowFlags_NoFocusOnAppearing or ImGuiWindowFlags_NoBringToFrontOnFocus flags in BeginMainMenuBar() and see if it slightly changes the behavior in a way that's useful for you.

Otherwise as johanwendin pointed out you can easily maintain a stack locally. That's not how I'd expect the problem to be solved for end-users but would be a easy to implement workaround in the meanwhile.

@ocornut ocornut changed the title How to implement menu command for top-most window Closing windows restoring z-ordered focus / how to implement menu command for top-most window Jul 17, 2016
ocornut added a commit that referenced this issue Jul 18, 2016
@ocornut
Copy link
Owner

ocornut commented Jul 18, 2016

FYI I have committed a small fix to restore focus when closing a window but this doesn't fix your issue here because the mainmenubar window is still taking focus at the moment.

@ocornut
Copy link
Owner

ocornut commented Nov 17, 2017

Forgot to link it in this topic, but as part of #787 work, on October 23 I have pushed a change that makes the Main Menu Bar restore focus after being used:
2098377

It doesn't fully solve the discussion here (which can presumably currently be solved by tracking z-order of documents on the user side) but relates somehow.

@ocornut ocornut added nav keyboard/gamepad navigation focus and removed in progress labels Jan 31, 2018
ocornut added a commit that referenced this issue Jan 31, 2018
…efocusing. This is a little experimental and potentially error-prone right now. (#787, vaguely relate to ~#727) Ideally we should maintain a non-sorted last-focused list that include childs windows.
ocornut added a commit that referenced this issue Feb 1, 2018
…#727) One visible issue was pressing Left to leave a child menu.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
focus nav keyboard/gamepad navigation
Projects
None yet
Development

No branches or pull requests

3 participants