qt: Cleanup SplashScreen class #14854
Conversation
Cleaning up after replacing the QSplashScreen base class with the QWidget class.
| // screen will take care of deleting itself when finish() happens. | ||
| splash->show(); | ||
| connect(this, &BitcoinApplication::splashFinished, splash, &SplashScreen::slotFinish); | ||
| connect(this, &BitcoinApplication::splashFinished, splash, &SplashScreen::finish); |
There was a problem hiding this comment.
Could just
connect(this, &BitcoinApplication::splashFinished, splash, &SplashScreen::deleteLater);
or is there a good reason to call hide()?
There was a problem hiding this comment.
When #8231 was merged there was no deleteLater() call.
There was a problem hiding this comment.
Tested on macOS 10.13.6: if the splash screen is minimized, neither deleteLater() nor hide(); deleteLater() can remove it from the Dock.
It seems the simplest way is to not touch the patch from #8231.
There was a problem hiding this comment.
Just tested my suggestion above with Qt 5.11 on macOS 10.14.1 and the following are good:
- closing the app while splash is open
- splash goes away from the dock even if it's minimized.
What's your Qt version?
There was a problem hiding this comment.
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
|
Can't remember why the unused variable ended up there. utACK 7d1b60c |
|
utACK 7d1b60c |
Summary: Cleaning up after replacing the QSplashScreen base class with the QWidget class ([[bitcoin/bitcoin#4941 | PR4941]]). Backport of Core [[bitcoin/bitcoin#14854 | PR14854]] Test Plan: ``` ninja && ninja check src/qt/bitcoin-qt ``` Reviewers: O1 Bitcoin ABC, #bitcoin_abc, Fabien Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, Fabien Differential Revision: https://reviews.bitcoinabc.org/D7840
6 participants
Cleaning up after replacing the
QSplashScreenbase class with theQWidgetclass (#4941 by @laanwj).cc @jonasschnelli