Replace “Hide tray icon” option with positive “Show tray icon” one #115

1. 
   hebasto commented at 10:25 pm on October 24, 2020: member

   This change makes easier both (1) using this option, and (2) reasoning about the code.
2. in src/qt/optionsmodel.cpp:57 in 87e38a6512 outdated

   53@@ -54,14 +54,15 @@ void OptionsModel::Init(bool resetSettings)
   54     // These are Qt-only settings:
   55 
   56     // Window
   57-    if (!settings.contains("fHideTrayIcon"))
   58-        settings.setValue("fHideTrayIcon", false);
   59-    fHideTrayIcon = settings.value("fHideTrayIcon").toBool();
   60-    Q_EMIT hideTrayIconChanged(fHideTrayIcon);
   61+    if (!settings.contains("bShowTrayIcon")) {
   

   ---

   ---

   

   promag commented at 2:05 pm on October 28, 2020:

   nit, is there a reason to change setting key? No big deal in this case, but ideally “old” key would be migrated - if you wish to rename the key.

   ---

   

   hebasto commented at 2:17 pm on October 28, 2020:

   It is not just renaming here. fHideTrayIcon and bShowTrayIcon are different semantically. As the Bitcoin-Qt.conf is written in the human readable format, I’d prefer to introduce a new setting key to avoid any possible confusing.

   An old key migration sounds good to me. But is it worth to add more code for such a non-critical key?

   ---

   

   promag commented at 2:19 pm on October 28, 2020:

   It is not just renaming here. fHideTrayIcon and bShowTrayIcon are different semantically.

   Of course, you would have to take the !fHideTrayIcon.

   An old key migration sounds good to me. But is it worth to add more code for such a non-critical key?

   I’ve started with “nit” 😅

   ---

   

   luke-jr commented at 6:57 pm on November 13, 2020:

   It is not just renaming here. fHideTrayIcon and bShowTrayIcon are different semantically.

   How?

   As the Bitcoin-Qt.conf is written in the human readable format, I’d prefer to introduce a new setting key to avoid any possible confusing.

   That’s not a good reason IMO

   ---

   

   hebasto commented at 7:42 am on November 15, 2020:

   It is not just renaming here. fHideTrayIcon and bShowTrayIcon are different semantically.

   How?

   The meaning of fHideTrayIcon is opposite to bShowTrayIcon, no?

   As the Bitcoin-Qt.conf is written in the human readable format, I’d prefer to introduce a new setting key to avoid any possible confusing.

   That’s not a good reason IMO

   What do you suggest?

   ---

   

   luke-jr commented at 4:52 pm on November 15, 2020:

   The meaning of fHideTrayIcon is opposite to bShowTrayIcon, no?

   It’s a strict opposite.

   What do you suggest?

   Leave fHideTrayIcon alone and just change the UI presentation.

   ---

   

   hebasto commented at 5:25 pm on November 15, 2020:

   Updated.
3. 
   promag commented at 2:07 pm on October 28, 2020: contributor

   Code review ACK a124ee763e294a75871823c642f55f46633cdb31.
4. jonasschnelli added this to the milestone 0.22.0 on Nov 11, 2020
5. luke-jr changes_requested
6. gui: Replace "Hide tray icon" option with positive "Show tray icon" one

   This change makes easier both (1) using this option, and (2) reasoning
   about the code.

   17174f8328
7. qt: Remove redundant BitcoinGUI::setTrayIconVisible

   The removed BitcoinGUI::setTrayIconVisible just duplicates
   QSystemTrayIcon::setVisible.

   03edb52eee
8. hebasto force-pushed on Nov 15, 2020
9. 
   hebasto commented at 5:25 pm on November 15, 2020: member

   Updated a124ee763e294a75871823c642f55f46633cdb31 -> 03edb52eee5a87af16161c23bdc6cde91a2e5b8b (pr115.01 -> pr115.02, diff):

   • addressed @luke-jr’s comment:

   Leave fHideTrayIcon alone and just change the UI presentation.

10. luke-jr approved
11. 
    luke-jr commented at 6:45 pm on November 15, 2020: member

    utACK

    nit: I would split it into more commits. (eg, one changing GUI, one changing internals)

12. jonasschnelli approved
13. 
    jonasschnelli commented at 8:27 am on December 15, 2020: contributor

    utACK 03edb52eee5a87af16161c23bdc6cde91a2e5b8b
14. jonasschnelli merged this on Dec 15, 2020
15. jonasschnelli closed this on Dec 15, 2020

    ---

16. sidhujag referenced this in commit 277ed8f600 on Dec 15, 2020
17. hebasto deleted the branch on Dec 15, 2020
18. bitcoin-core locked this on Feb 15, 2022

Contributors
hebasto promag luke-jr jonasschnelli

Milestone
22.0