Choose monospaced font in C++ code rather in *.ui file #87

1. 
   hebasto commented at 4:16 pm on September 10, 2020: member

   This change makes macOS choose the correct monospaced font.

   fanquake noted (https://github.com/bitcoin/bitcoin/pull/16432#issuecomment-514486077):

   the monospace font looks a bit weird on macOS

   … because it is not monospaced.

   This is an alternative to #79 as that PR has a Concept NACK from luke-jr.

   I’m personally still lean to embedding font as it makes the GUI more stable.

2. 
   hebasto commented at 4:16 pm on September 10, 2020: member

   cc @fanquake @luke-jr
3. 
   hebasto commented at 10:31 am on October 23, 2020: member

   @jonasschnelli Should this be considered a bugfix?
4. 
   jonasschnelli commented at 10:34 am on October 23, 2020: contributor

   Unsure about this. IMO this is a MVC violation (which we have plenty but unsure to add more).
5. 
   hebasto commented at 10:38 am on October 23, 2020: member

   IMO this is a MVC violation (which we have plenty but unsure to add more).

   How is it so? I think both qt/overviewpage.cpp and qt/forms/overviewpage.ui belong to “View” in MVC, no?

6. 
   jonasschnelli commented at 10:44 am on October 23, 2020: contributor

   How is it so? I think both qt/overviewpage.cpp and qt/forms/overviewpage.ui belong to “View” in MVC, no?

   I’d say *.ui is view, *.cpp is controller (the non-automatic generated ones).

7. 
   hebasto commented at 10:47 am on October 23, 2020: member

   How is it so? I think both qt/overviewpage.cpp and qt/forms/overviewpage.ui belong to “View” in MVC, no?

   I’d say *.ui is view, *.cpp is controller (the non-automatic generated ones).

   Right. But Qt programming usually assumes “the view and the controller objects are combined” (https://doc.qt.io/qt-5/model-view-programming.html).

8. in src/qt/overviewpage.cpp:147 in a44ba74cec outdated

   140@@ -141,6 +141,18 @@ OverviewPage::OverviewPage(const PlatformStyle *platformStyle, QWidget *parent)
   141     showOutOfSyncWarning(true);
   142     connect(ui->labelWalletStatus, &QPushButton::clicked, this, &OverviewPage::handleOutOfSyncWarningClicks);
   143     connect(ui->labelTransactionsStatus, &QPushButton::clicked, this, &OverviewPage::handleOutOfSyncWarningClicks);
   144+
   145+    QFont f = GUIUtil::fixedPitchFont();
   146+    f.setWeight(QFont::Bold);
   147+    f.setPointSize(10);
   

   ---

   ---

   

   luke-jr commented at 7:58 pm on October 24, 2020:

   Let’s not override this for no reason?

   ---

   

   hebasto commented at 9:01 am on October 25, 2020:

   The only reason is to make things explicitly rather than implicitly. Does setPointSize call has any downsides?

   ---

   

   luke-jr commented at 1:53 pm on October 25, 2020:

   Yes, the downside is that it is explicit and overrides the user’s preference (which may not be 10pt).

   ---

   

   hebasto commented at 3:09 pm on October 25, 2020:

   Thanks! Updated.

   Now I’m able to change the font size with the qt5ct utility on Linux Mint 20 :)

9. luke-jr changes_requested
10. 
    luke-jr commented at 7:59 pm on October 24, 2020: member

    Looks good to me, one change requested
11. qt: Choose monospaced font in C++ code rather in `*.ui` file

    This change makes macOS choose the correct monospaced font.

    2e386cd3dd
12. hebasto force-pushed on Oct 25, 2020
13. 
    hebasto commented at 3:07 pm on October 25, 2020: member

    Updated a44ba74ceccf6f61c65972e2ae37327de0db4c79 -> 2e386cd3dd3fea5a22dc8ae871b80a80f8cdb3f7 (pr87.01 -> pr87.02, diff):

    • addressed @luke-jr’s comment:

    Yes, the downside is that it is explicit and overrides the user’s preference (which may not be 10pt).

14. luke-jr approved
15. 
    luke-jr commented at 3:24 pm on October 25, 2020: member

    utACK
16. jarolrod approved
17. 
    jarolrod commented at 9:56 pm on November 10, 2020: member

    Tested ACK on macOS 10.15.7

    Master

    PR

18. 
    jonasschnelli commented at 9:27 am on January 28, 2021: contributor

    Master:

    This PR:

    I think the correct monospace font should be bigger and eventually bold.

19. 
    jonasschnelli commented at 10:03 am on January 28, 2021: contributor

    Adding f.setPointSize(12); makes it look much better (on macOS, unsure about windows/linux):
20. 
    hebasto commented at 10:12 am on January 28, 2021: member

    Adding f.setPointSize(12); makes it look much better (on macOS, unsure about windows/linux):

    Right. But @luke-jr’s opinion is opposite.

21. 
    RandyMcMillan commented at 3:10 am on January 29, 2021: contributor

    It would be great to add a font size selector - similar to the console interface.

    

22. 
    hebasto commented at 12:17 pm on February 1, 2021: member

    Closed in favor of #207 which should be less controversial.
23. hebasto closed this on Feb 1, 2021

    ---

24. hebasto deleted the branch on Feb 22, 2021
25. bitcoin-core locked this on Aug 16, 2022

Contributors
hebasto jonasschnelli luke-jr jarolrod RandyMcMillan