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

Removing focus from child windows upon hiding them. #1406

Merged
merged 2 commits into from
Dec 6, 2014
Merged

Removing focus from child windows upon hiding them. #1406

merged 2 commits into from
Dec 6, 2014

Conversation

csimons
Copy link
Contributor

@csimons csimons commented Dec 5, 2014

When any of the Song Editor, BB Editor, Automation Editor, or Piano Roll windows are hidden or toggled off, they retain focus and capture key events. The problem here is that the user is unwittingly sending keys to the apparently-closed window rather than only to the main application window.

The problem can be demonstrated by the following steps:

  1. Load a song/project.
  2. Close all child windows other than the Song Editor.
  3. Give focus to the Song Editor window by clicking its title bar.
  4. Hit the [space] key to start playback.
  5. Toggle off the Song Editor by using F5 or clicking the Song Editor toolbar button.
  6. Press [space] again while no child windows are open.

It is then observed that the key events continue to be handled by the (apparently closed) child window, toggling playback.

This happens with any child window that can be toggled and that handles key events; whichever child window was hidden last will continue receiving key events until another child window is explicitly focused by toggling it on or clicking it.

The proposed change restores focus to the parent window when a child window is hidden.

@csimons csimons changed the title Removing focus from child windows upon closing them. Removing focus from child windows upon hiding them. Dec 5, 2014
@tresf
Copy link
Member

tresf commented Dec 5, 2014

Hmm... this is such a small one, we should cherry-pick it to stable-1.1 as well.

@tresf
Copy link
Member

tresf commented Dec 5, 2014

@csimons, I assume you have tested this? I can't seem to set focus to the main window manually, so I'm not sure this is a good approach. Could we instead grab the first open mdi window? I'm not sure how we'd set focus to MainWindow programatically when we can't do it interactively.

@tresf
Copy link
Member

tresf commented Dec 5, 2014

Ok, I managed to get MainWindow focus via the Pitch/Master Volume sliders. Those seem to be the only controls interactively that can steal focus (but at least it is possible?) I'd like to hear @lukas-w and @diizy's thoughts on this.

My first instinct is to search for the Song-Editor and give that focus when its active and a non-Song-Editor window is open. However, ideally, we'd set focus to the Last active window (assuming it's still open) but if the MainWindow is a good catch-all for this behavior, perhaps it is the best?

@csimons
Copy link
Contributor Author

csimons commented Dec 5, 2014

I assume you have tested this?

Yes, using the steps in the initial PR comment. Without 5675b1a, after step 6 the keyboard controls for the SongEditor still work, even though the window is apparently closed. With the change, the SongEditor controls no longer work, but MainWindow events are still handled (Ctrl+O for Open, etc.).

Could we instead grab the first open mdi window? ... My first instinct is to search for the Song-Editor and give that focus ...

I think this is a good suggestion; if there are other open/visible windows, taking focus away from all of them seems suboptimal. My proposal would be the following:

(1) If the SongEditor window is open and visible, set focus to it; else,
(2) if any non-hidden child window is found, set focus to it; else,
(3) set focus to the main window.

Thoughts?

@tresf
Copy link
Member

tresf commented Dec 5, 2014

(1) If the SongEditor window is open and visible, set focus to it; else,
(2) If any non-hidden child window is found, set focus to it; else,
(3) set focus to the main window.

👍 From this guy :), unless of course QT has a stack of recent windows already in-order and you can pop it off the top.

…any other visible editors, or finally the parent window if all editors are hidden.
@csimons
Copy link
Contributor Author

csimons commented Dec 6, 2014

The latest commit in this PR, ca973b9, implements the logic discussed above.

diizy added a commit that referenced this pull request Dec 6, 2014
Removing focus from child windows upon hiding them.
@diizy diizy merged commit fd6e49c into LMMS:master Dec 6, 2014
@csimons csimons deleted the unbind-keys branch December 6, 2014 15:45
@tresf tresf mentioned this pull request Mar 31, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants