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

Qt: Add option to disable the system tray icon #8006

Merged
merged 1 commit into from May 12, 2016

Conversation

Projects
None yet
3 participants
@Tyler-Hardin
Contributor

Tyler-Hardin commented May 5, 2016

My changes leave all tray icon and menu creation/initialization logic untouched. It only shows or hides the icon according to the setting.

A new checkbox was added to the OptionsDialog under the Window tab. A bool option named "disableTray" was added to OptionsModel. This checkbox was mapped like other all options to the OptionsModel.

A signal was added to the OptionsModel for broadcasting changes the the
disableTray option. This signal was connected to a new slot added to
BitcoinGUI named updateDisableTray(bool). The slot simply hides or shows
the trayIcon in BitcoinGUI according to the parameter recieved.

The negative wording of "disableTray" instead of a positive wording
("enableTray") because enabled is the default. Perhaps "hideTray" would
be better than disable.

This pull request intends to resolve this issue: #7160

@Tyler-Hardin

This comment has been minimized.

Show comment
Hide comment
@Tyler-Hardin

Tyler-Hardin May 5, 2016

Contributor

Btw, this is my first pull request. Don't hurt me.

Contributor

Tyler-Hardin commented May 5, 2016

Btw, this is my first pull request. Don't hurt me.

@jonasschnelli jonasschnelli added the GUI label May 5, 2016

@jonasschnelli

View changes

Show outdated Hide outdated src/qt/forms/optionsdialog.ui
@Tyler-Hardin

This comment has been minimized.

Show comment
Hide comment
@Tyler-Hardin

Tyler-Hardin May 5, 2016

Contributor

@jonasschnelli, yes, it was. I'll edit it back to zero.

Contributor

Tyler-Hardin commented May 5, 2016

@jonasschnelli, yes, it was. I'll edit it back to zero.

@jonasschnelli

This comment has been minimized.

Show comment
Hide comment
@jonasschnelli

jonasschnelli May 5, 2016

Member

Nice work!
Concept ACK will test in detail soon.

Member

jonasschnelli commented May 5, 2016

Nice work!
Concept ACK will test in detail soon.

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj May 5, 2016

Member

Concept ACK.

Perhaps "hideTray" would be better than disable.

I'd prefer "setTrayVisible(bool)" - this is closer to Qt's own usage. I agree on the positive versus negative wording in principle, but on the other hand the default could change later, so may as well make it general.

Member

laanwj commented May 5, 2016

Concept ACK.

Perhaps "hideTray" would be better than disable.

I'd prefer "setTrayVisible(bool)" - this is closer to Qt's own usage. I agree on the positive versus negative wording in principle, but on the other hand the default could change later, so may as well make it general.

@Tyler-Hardin

This comment has been minimized.

Show comment
Hide comment
@Tyler-Hardin

Tyler-Hardin May 5, 2016

Contributor

@laanwj, since we're getting verbose anyway, would "setTrayIconVisible(bool)" be even better? Because, technically, I'm setting the visibility on the TrayIcon.

Btw, thanks for reminding me about the setVisible function. I had used an if(flag) tray->hide(); else tray->show() logic that I can simplify to a single line with setVisible.

Contributor

Tyler-Hardin commented May 5, 2016

@laanwj, since we're getting verbose anyway, would "setTrayIconVisible(bool)" be even better? Because, technically, I'm setting the visibility on the TrayIcon.

Btw, thanks for reminding me about the setVisible function. I had used an if(flag) tray->hide(); else tray->show() logic that I can simplify to a single line with setVisible.

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj May 5, 2016

Member

setTrayIconVisible(bool)

Sure, even clearer.

Member

laanwj commented May 5, 2016

setTrayIconVisible(bool)

Sure, even clearer.

@Tyler-Hardin

This comment has been minimized.

Show comment
Hide comment
@Tyler-Hardin

Tyler-Hardin May 5, 2016

Contributor

@laanwj, fixed.

Contributor

Tyler-Hardin commented May 5, 2016

@laanwj, fixed.

@jonasschnelli

This comment has been minimized.

Show comment
Hide comment
@jonasschnelli

jonasschnelli May 5, 2016

Member

This currently results in a startup crash on OSX because trayIcon is NULL on OSX (see void BitcoinGUI::createTrayIcon(const NetworkStyle *networkStyle)).

IMO this PR should remove the #ifndef Q_OS_MAC for the tray-icon creation (so it also gets created on OSX) and disabled it by default on OSX (over platformstyle class).

Member

jonasschnelli commented May 5, 2016

This currently results in a startup crash on OSX because trayIcon is NULL on OSX (see void BitcoinGUI::createTrayIcon(const NetworkStyle *networkStyle)).

IMO this PR should remove the #ifndef Q_OS_MAC for the tray-icon creation (so it also gets created on OSX) and disabled it by default on OSX (over platformstyle class).

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj May 5, 2016

Member

@jonasschnelli I think the reason it's not created on OSX is because the menu items are added to some other menu (which effectively acts as a tray icon). It was non-trivial to get rid of the #ifdeffing there, at least it seemed back in the day when I implmemented platformstyle.

Member

laanwj commented May 5, 2016

@jonasschnelli I think the reason it's not created on OSX is because the menu items are added to some other menu (which effectively acts as a tray icon). It was non-trivial to get rid of the #ifdeffing there, at least it seemed back in the day when I implmemented platformstyle.

@jonasschnelli

This comment has been minimized.

Show comment
Hide comment
@jonasschnelli

jonasschnelli May 6, 2016

Member

utACK (testes a bit but will test more in detail).
I can fix the OSX tray-icon support later (if even possible).

Member

jonasschnelli commented May 6, 2016

utACK (testes a bit but will test more in detail).
I can fix the OSX tray-icon support later (if even possible).

@jonasschnelli

This comment has been minimized.

Show comment
Hide comment
@jonasschnelli

jonasschnelli May 6, 2016

Member

Needs squashing.

Member

jonasschnelli commented May 6, 2016

Needs squashing.

@Tyler-Hardin

This comment has been minimized.

Show comment
Hide comment
@Tyler-Hardin

Tyler-Hardin May 6, 2016

Contributor

@jonasschnelli, squashed.

Contributor

Tyler-Hardin commented May 6, 2016

@jonasschnelli, squashed.

@jonasschnelli

This comment has been minimized.

Show comment
Hide comment
@jonasschnelli

jonasschnelli May 11, 2016

Member

Tested ACK a8abd45ec7ddb16d09f9abfd90b6eea722c55103

A little nit:
If you have your tray icon disabled, it shortly flickers up during startup of Bitcoin-Qt.

A simple change can fix this:

--- a/src/qt/bitcoingui.cpp
+++ b/src/qt/bitcoingui.cpp
@@ -545,7 +545,7 @@ void BitcoinGUI::createTrayIcon(const NetworkStyle *networkStyle)
     QString toolTip = tr("%1 client").arg(tr(PACKAGE_NAME)) + " " + networkStyle->getTitleAddText();
     trayIcon->setToolTip(toolTip);
     trayIcon->setIcon(networkStyle->getTrayAndWindowIcon());
-    trayIcon->show();
+    trayIcon->hide();
 #endif

     notificator = new Notificator(QApplication::applicationName(), trayIcon, this);
Member

jonasschnelli commented May 11, 2016

Tested ACK a8abd45ec7ddb16d09f9abfd90b6eea722c55103

A little nit:
If you have your tray icon disabled, it shortly flickers up during startup of Bitcoin-Qt.

A simple change can fix this:

--- a/src/qt/bitcoingui.cpp
+++ b/src/qt/bitcoingui.cpp
@@ -545,7 +545,7 @@ void BitcoinGUI::createTrayIcon(const NetworkStyle *networkStyle)
     QString toolTip = tr("%1 client").arg(tr(PACKAGE_NAME)) + " " + networkStyle->getTitleAddText();
     trayIcon->setToolTip(toolTip);
     trayIcon->setIcon(networkStyle->getTrayAndWindowIcon());
-    trayIcon->show();
+    trayIcon->hide();
 #endif

     notificator = new Notificator(QApplication::applicationName(), trayIcon, this);
Qt: Add option to hide the system tray icon
My changes leave all tray icon and menu creation/initialization logic
untouched. It only shows or hides the icon according to the setting.

A new checkbox was added to the OptionsDialog under the Window tab. A
bool option named "hideTrayIcon" was added to OptionsModel. This
checkbox was mapped like other all options to the OptionsModel.

A signal was added to the OptionsModel for broadcasting changes the the
hideTrayIcon option. This signal was connected to a new slot added to
BitcoinGUI named setTrayIconVisible(bool). The slot simply hides or
shows the trayIcon in BitcoinGUI according to the parameter recieved.
@Tyler-Hardin

This comment has been minimized.

Show comment
Hide comment
@Tyler-Hardin

Tyler-Hardin May 12, 2016

Contributor

@jonasschnelli, good catch. I added that edit.

Contributor

Tyler-Hardin commented May 12, 2016

@jonasschnelli, good catch. I added that edit.

@jonasschnelli

This comment has been minimized.

Show comment
Hide comment
@jonasschnelli

jonasschnelli May 12, 2016

Member

Tested ACK 8b0e497

Member

jonasschnelli commented May 12, 2016

Tested ACK 8b0e497

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj May 12, 2016

Member

Tested ACK 8b0e497

Member

laanwj commented May 12, 2016

Tested ACK 8b0e497

@laanwj laanwj merged commit 8b0e497 into bitcoin:master May 12, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

laanwj added a commit that referenced this pull request May 12, 2016

Merge #8006: Qt: Add option to disable the system tray icon
8b0e497 Qt: Add option to hide the system tray icon (Tyler Hardin)

codablock added a commit to codablock/dash that referenced this pull request Sep 7, 2017

Merge #8006: Qt: Add option to disable the system tray icon
8b0e497 Qt: Add option to hide the system tray icon (Tyler Hardin)

UdjinM6 added a commit to dashpay/dash that referenced this pull request Sep 9, 2017

Backport Bitcoin Qt/Gui changes up to 0.14.x part 2 (#1615)
* Merge #7506: Use CCoinControl selection in CWallet::FundTransaction

d6cc6a1 Use CCoinControl selection in CWallet::FundTransaction (João Barbosa)

* Merge #7732: [Qt] Debug window: replace "Build date" with "Datadir"

fc737d1 [Qt] remove unused formatBuildDate method (Jonas Schnelli)
4856f1d [Qt] Debug window: replace "Build date" with "Datadir" (Jonas Schnelli)

* Merge #7707: [RPC][QT] UI support for abandoned transactions

8efed3b [Qt] Support for abandoned/abandoning transactions (Jonas Schnelli)

* Merge #7688: List solvability in listunspent output and improve help

c3932b3 List solvability in listunspent output and improve help (Pieter Wuille)

* Merge #8006: Qt: Add option to disable the system tray icon

8b0e497 Qt: Add option to hide the system tray icon (Tyler Hardin)

* Merge #8073: qt: askpassphrasedialog: Clear pass fields on accept

02ce2a3 qt: askpassphrasedialog: Clear pass fields on accept (Pavel Vasin)

* Merge #8231: [Qt] fix a bug where the SplashScreen will not be hidden during startup

b3e1348 [Qt] fix a bug where the SplashScreen will not be hidden during startup (Jonas Schnelli)

* Merge #8257: Do not ask a UI question from bitcoind

1acf1db Do not ask a UI question from bitcoind (Pieter Wuille)

* Merge #8463: [qt] Remove Priority from coincontrol dialog

fa8dd78 [qt] Remove Priority from coincontrol dialog (MarcoFalke)

* Merge #8678: [Qt][CoinControl] fix UI bug that could result in paying unexpected fee

0480293 [Qt][CoinControl] fix UI bug that could result in paying unexpected fee (Jonas Schnelli)

* Merge #8672: Qt: Show transaction size in transaction details window

c015634 qt: Adding transaction size to transaction details window (Hampus Sjöberg)
 \-- merge fix for s/size/total size/
fdf82fb Adding method GetTotalSize() to CTransaction (Hampus Sjöberg)

* Merge #8371: [Qt] Add out-of-sync modal info layer

08827df [Qt] modalinfolayer: removed unused comments, renamed signal, code style overhaul (Jonas Schnelli)
d8b062e [Qt] only update "amount of blocks left" when the header chain is in-sync (Jonas Schnelli)
e3245b4 [Qt] add out-of-sync modal info layer (Jonas Schnelli)
e47052f [Qt] ClientModel add method to get the height of the header chain (Jonas Schnelli)
a001f18 [Qt] Always pass the numBlocksChanged signal for headers tip changed (Jonas Schnelli)
bd44a04 [Qt] make Out-Of-Sync warning icon clickable (Jonas Schnelli)
0904c3c [Refactor] refactor function that forms human readable text out of a timeoffset (Jonas Schnelli)

* Merge #8805: Trivial: Grammar and capitalization

c9ce17b Trivial: Grammar and capitalization (Derek Miller)

* Merge #8885: gui: fix ban from qt console

cb78c60 gui: fix ban from qt console (Cory Fields)

* Merge #8821: [qt] sync-overlay: Don't block during reindex

fa85e86 [qt] sync-overlay: Don't show estimated number of headers left (MarcoFalke)
faa4de2 [qt] sync-overlay: Don't block during reindex (MarcoFalke)

* Support themes for new transaction_abandoned icon

* Fix constructor call to COutput

* Merge #7842: RPC: do not print minping time in getpeerinfo when no ping received yet

62a6486 RPC: do not print ping info in getpeerinfo when no ping received yet, fix help (Pavel Janík)

* Merge #8918: Qt: Add "Copy URI" to payment request context menu

21f5a63 Qt: Add "Copy URI" to payment request context menu (Luke Dashjr)

* Merge #8925: qt: Display minimum ping in debug window.

1724a40 Display minimum ping in debug window. (R E Broadley)

* Merge #8972: [Qt] make warnings label selectable (jonasschnelli)

ef0c9ee [Qt] make warnings label selectable (Jonas Schnelli)

* Make background of warning icon transparent in modaloverlay

* Merge #9088: Reduce ambiguity of warning message

77cbbd9 Make warning message about wallet balance possibly being incorrect less ambiguous. (R E Broadley)

* Replace Bitcoin with Dash in modal overlay

* Remove clicked signals from labelWalletStatus and labelTransactionsStatus

As both are really just labels, clicking on those is not possible.
This is different in Bitcoin, where these labels are actually buttons.

* Pull out modaloverlay show/hide into it's own if/else block and switch to time based check

Also don't use masternodeSync.IsBlockchainSynced() for now as it won't
report the blockchain being synced before the first block (or other MN
data?) arrives. This would otherwise give the impression that sync is
being stuck.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment