Use KMainWindow instead of QMainWindow
ClosedPublic
Actions

Authored by mschiller on Sep 15 2018, 2:38 PM.

Details

Reviewers

hein
alexeymin

Group Reviewers

KDE Applications

Commits

R369:0c0eb086bdc0: Use KMainWindow instead of QMainWindow

Summary

MainWindow should inherit from KMainWindow and not from QMainWindow
because QMainWindow does not call the queryClose function upon closing.
This disabled the "really quit?" dialog which should come up if more than one session
is currently open.
Furthermore makes Yakuake properly quit when requested via the "Quit" action or the dialog by
making the QApplication quit if the last Window Closes.

BUG: 398425

Test Plan

open yakuake
press ctrl + shift + q

without the patch yakuake just hides again (press F12 to confirm) with the patch it properly quits.

open yakuake
open multiple (>1) sessions (bottom left "+" symbol)
press ctrl + shift + q

without the patch yakuake just hides with the patch a dialog comes up and asks "really quit?"
on pressing "quit" yakuake properly quits.

Diff Detail

Repository

R369 Yakuake

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

mschiller requested review of this revision.Sep 15 2018, 2:38 PM

mschiller created this revision.

mschiller edited the test plan for this revision. (Show Details)

Looks good. Do you have a dev account or do we need someone to land this for you?

This revision is now accepted and ready to land.Sep 16 2018, 6:48 AM

Thanks. I don't have a dev account (yet ;) ).

Sorry, one more lap: Can you verify that the bug addressed in https://commits.kde.org/yakuake/e2b603d16955561540b9c194c0844ec320fc2694 doesn't happen with your patch applied?

tcanabrava added a subscriber: tcanabrava.Sep 17 2018, 7:45 AM

tcanabrava added inline comments.

app/mainwindow.cpp
227–230	return result != KMessageBox::Cancel;
app/mainwindow.h
97	use override; Q_DECL_OVERRIDE was needed when none of the compilers actually accepted c++11.

Replace Q_DECL_OVERRIDE with override.
Make the Quit button respect the proper quitting method.
Quit the application if the close event has been accepted by the main window
instead of closing when the last window is closed.

mschiller marked 2 inline comments as done.Sep 17 2018, 9:38 AM

mschiller requested review of this revision.Sep 23 2018, 2:05 PM

bump ;)

shubham added a subscriber: shubham.Oct 9 2018, 2:19 PM

shubham added inline comments.

app/mainwindow.h
28	remove #include <QMainWindow>

shubham removed a subscriber: shubham.Oct 9 2018, 2:20 PM

Remove QMainWindow include

Harbormaster completed remote builds in B3689: Diff 43222.Oct 9 2018, 2:24 PM

mschiller marked an inline comment as done.Oct 9 2018, 2:24 PM

hein accepted this revision.Oct 10 2018, 6:03 AM

This revision is now accepted and ready to land.Oct 10 2018, 6:03 AM

Sorry for being incredibly slow on this, life has been so busy :/

Do you mind if I push this with your realname (I know it from the dev account application)? We usually don't do nicknames in contributions.

No worries nobody dies without this patch ;-)
And of course you can use my real name.
Thanks for the reviewing :)

Closed by commit R369:0c0eb086bdc0: Use KMainWindow instead of QMainWindow (authored by mschiller, committed by hein). · Explain WhyOct 22 2018, 5:22 AM

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

			Path	Packages
M			app/mainwindow.h (15 lines)
M			app/mainwindow.cpp (25 lines)
M			app/tabbar.h (28 lines)
M			app/terminal.h (2 lines)
M			app/titlebar.h (4 lines)
M			app/titlebar.cpp (2 lines)
M			app/visualeventoverlay.h (6 lines)

Diff	ID	Base	Description	Created	Lint	Unit
Base			Base
Diff 1	41700			Sep 15 2018, 2:14 PM	★	★
Diff 2	41803			Sep 17 2018, 9:34 AM	★	★
Diff 3	43222	e648c25	Remove QMainWindow include	Oct 9 2018, 2:24 PM	★	★
Diff 4	44058	8adea8a	R369:0c0eb086bdc0a726874a56b46de29fcc73fb792b	Oct 22 2018, 5:20 AM	★	★