qt: Cleanup SplashScreen class #14854

1. 
   hebasto commented at 11:40 pm on December 1, 2018: member

   Cleaning up after replacing the QSplashScreen base class with the QWidget class (#4941 by @laanwj).

   cc @jonasschnelli

2. Cleanup SplashScreen class

   Cleaning up after replacing the QSplashScreen base class with the
   QWidget class.

   7d1b60ce93
3. fanquake added the label GUI on Dec 2, 2018
4. in src/qt/bitcoin.cpp:362 in 7d1b60ce93

   356@@ -357,9 +357,9 @@ void BitcoinApplication::createSplashScreen(const NetworkStyle *networkStyle)
   357 {
   358     SplashScreen *splash = new SplashScreen(m_node, 0, networkStyle);
   359     // We don't hold a direct pointer to the splash screen after creation, but the splash
   360-    // screen will take care of deleting itself when slotFinish happens.
   361+    // screen will take care of deleting itself when finish() happens.
   362     splash->show();
   363-    connect(this, &BitcoinApplication::splashFinished, splash, &SplashScreen::slotFinish);
   364+    connect(this, &BitcoinApplication::splashFinished, splash, &SplashScreen::finish);
   

   ---

   ---

   

   promag commented at 3:52 pm on December 3, 2018:

   Could just

   0connect(this, &BitcoinApplication::splashFinished, splash, &SplashScreen::deleteLater);
   

   or is there a good reason to call hide()?

   ---

   

   promag commented at 3:55 pm on December 3, 2018:

   When #8231 was merged there was no deleteLater() call.

   ---

   

   hebasto commented at 8:32 pm on December 3, 2018:

   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.

   ---

   

   hebasto commented at 9:17 pm on December 3, 2018:

   @promag Moreover, IIUC, 34ebceb25a40a2160349656265951cbe9a5b0f34 commit has been effectively disabled by e4f126a7ba66e7317718c30276dff6db92dc1986. What do you think about this?

   ---

   

   promag commented at 11:08 pm on December 3, 2018:

   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?

   ---

   

   hebasto commented at 7:43 am on December 4, 2018:

   @promag

   What’s your Qt version?

   Qt 5.11.2

   ---

   

   promag commented at 11:34 pm on December 4, 2018:

   @hebasto with my above suggestion I can’t reproduce #7718 in my system. Maybe this is was macOS issue?

   ---

   

   hebasto commented at 7:50 am on December 5, 2018:

   @promag

   Maybe this is was macOS issue?

   Agree. As there are systems subject to #7718 it is better to not touch the patch from #8231, IMO.

5. 
   DrahtBot commented at 5:05 pm on December 3, 2018: member

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #14250 (qt: Remove redundant stopThread() and stopExecutor() signals by hebasto)
   • #13880 (gui: Try shutdown also on failure by MarcoFalke)

   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.

6. 
   jonasschnelli commented at 3:36 pm on December 7, 2018: contributor

   Can’t remember why the unused variable ended up there.

   utACK 7d1b60ce931d5944576fc726b0ef6b9a10ef8793

7. 
   laanwj commented at 4:12 pm on December 7, 2018: member

   utACK 7d1b60ce931d5944576fc726b0ef6b9a10ef8793
8. pull[bot] referenced this in commit 2b12268095 on Dec 7, 2018
9. laanwj merged this on Dec 7, 2018
10. laanwj closed this on Dec 7, 2018

    ---

11. hebasto deleted the branch on Dec 7, 2018
12. jasonbcox referenced this in commit 75732ddc22 on Oct 9, 2020
13. DrahtBot locked this on Sep 8, 2021

Contributors
hebasto promag DrahtBot jonasschnelli laanwj

Labels
GUI