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

Fix Issue #7412 (After closing Song Editor new project loaded ends with empty song editor window) by redoing PR #7035 #7515

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

Conversation

firewall1110
Copy link
Contributor

(1) Bug affects not only Song Editor window, but also Piano Roll, Pattern Editor, Automation Editor windows (all are extending Editor class). Using MainWindow::refocus() makes closing event handling close to closing by shortcut (from MainWindow code)
(2) This is BugFix PR so I do not change coding convention violation ( variable _ce ).

@firewall1110
Copy link
Contributor Author

@michaelgregorius wrote

... with an explanation of why you think this fixes the bug. ...
ensure that the solution does not break anything else what worked before

(1) I think that it fix the bug because I test my code.

(2) Bug is result of not proper another bug solution done by PR #7035

So I add another line
getGUI()->mainWindow()->refocus();

but restore

_ce->ignore();

as it was before.

I tested it against PR #7035 fixed bug steps:

1: place notes on the grid.
2: reduce the grid until at least one of the notes doesn't line up.
3: play the loop.
4: close the piano roll editor.
5: hit space to pause.

so line
getGUI()->mainWindow()->refocus();
is proper solution (and should be added in PR #7035 instead to change _ce->ignore();).

And this code is called, than windows are closed by Ctrl+1 , Ctrl+2, Ctrl+3, Ctrl+4 .
So calling it from here can not harm more than calling in Ctrl+1 , Ctrl+2, Ctrl+3, Ctrl+4 context.

(3)

Using _ce->ignore(); in such situation is also in Qt 5.12 documentation:

"The event handler QWidget::closeEvent() receives close events. The default implementation of this event handler accepts the close event. If you do not want your widget to be hidden, or want some special handling, you should reimplement the event handler and ignore() the event."

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.

1 participant