Update about logo icon (colour) to denote the chain type of the QT instance in About/ Help Message Window/ Dialog #762

pull pablomartin4btc wants to merge 1 commits into bitcoin-core:master from pablomartin4btc:gui-update-about-logo-icon changing 4 files +34 −7
  1. pablomartin4btc commented at 0:03 am on September 25, 2023: contributor

    It fixes #761.

    About logo icon is updated now with the correct colour indicating the chain type of the QT instance that user is running (as the splash screen), plus the title of the about window also is updated with the chain type string as in the main window.

    image

    image

    image

    Tested it also with:

    • mainnet (no command line args after bitcoin-qt or bitcoin-qt -chain=main),
    • -regtest and
    • -testnet.
  2. DrahtBot commented at 0:03 am on September 25, 2023: contributor

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

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    Concept ACK hernanmarino
    Stale ACK BrandonOdiwuor

    If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

    Conflicts

    Reviewers, 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.

  3. DrahtBot added the label CI failed on Sep 25, 2023
  4. DrahtBot removed the label CI failed on Sep 25, 2023
  5. pablomartin4btc force-pushed on Sep 25, 2023
  6. in src/qt/utilitydialog.cpp:142 in 30c235b2e6 outdated
    136@@ -135,6 +137,19 @@ void HelpMessageDialog::on_okButton_accepted()
    137     close();
    138 }
    139 
    140+void HelpMessageDialog::setAboutWindowTitle(const NetworkStyle *networkStyle)
    141+{
    142+    QString aboutTitle = tr("About %1").arg(PACKAGE_NAME);
    143+    if ((networkStyle) && (Params().GetChainType() != ChainType::MAIN)) aboutTitle.append(" " + networkStyle->getTitleAddText());
    


    maflcko commented at 2:03 pm on October 5, 2023:
    Why would this be nullptr? Seems easier to use a reference that is never null?

    pablomartin4btc commented at 3:38 pm on October 5, 2023:

    I didn’t want to touch src/qt/bitcoin.cpp:

    https://github.com/bitcoin-core/gui/blob/db19a7e89d19d97094bcf8753b37619a16b7f033/src/qt/bitcoin.cpp#L568

    And I also needed to move where the networkStyle is initialised/ set before the HelpMessageDialog constructor:

    https://github.com/bitcoin-core/gui/blob/db19a7e89d19d97094bcf8753b37619a16b7f033/src/qt/bitcoin.cpp#L607

    But I can do it if that’s better.

  7. maflcko commented at 2:03 pm on October 5, 2023: contributor
    It may help to show screenshots before and after
  8. pablomartin4btc commented at 2:38 pm on October 5, 2023: contributor

    It may help to show screenshots before and after

    Thanks for your review @MarcoFalke, the before screenshot is on the issue #761 but I can bring it here if that makes it clearer.

  9. hernanmarino approved
  10. hernanmarino commented at 9:27 pm on October 6, 2023: contributor
    tested ACK
  11. pablomartin4btc commented at 7:34 pm on October 7, 2023: contributor

    It may help to show screenshots before and after @maflcko I’ve updated the description with your suggestion (copied @furszy’s style from his PR #764, I find it less invasive for the reviewer)

  12. MarnixCroes approved
  13. MarnixCroes commented at 2:25 pm on November 2, 2023: contributor
    tack 30c235b2e6fd9b7e3634d795cedc171d4d8d0e27
  14. in src/qt/utilitydialog.cpp:149 in 30c235b2e6 outdated
    144+    setWindowTitle(aboutTitle);
    145+}
    146+
    147+void HelpMessageDialog::setChainTypeIconOnAboutLogo(const NetworkStyle *networkStyle)
    148+{
    149+    const QSize requiredSize(1024,1024);
    


    kristapsk commented at 6:20 pm on December 27, 2023:

    nit

    0    const QSize requiredSize(1024, 1024);
    

    pablomartin4btc commented at 1:58 pm on December 28, 2023:
    Thanks for reviewing! I’ll change it if I have to re-touch it.

    pablomartin4btc commented at 7:11 pm on May 19, 2024:
    done!
  15. BrandonOdiwuor commented at 11:06 am on December 28, 2023: contributor
    Concept ACK
  16. BrandonOdiwuor approved
  17. BrandonOdiwuor commented at 1:38 pm on December 28, 2023: contributor

    Tested ACK 30c235b2e6fd9b7e3634d795cedc171d4d8d0e27

    Screenshot from 2023-12-28 16-34-43

  18. DrahtBot requested review from hernanmarino on Dec 28, 2023
  19. DrahtBot removed review request from hernanmarino on Dec 28, 2023
  20. DrahtBot requested review from hernanmarino on Dec 28, 2023
  21. DrahtBot added the label CI failed on Jan 14, 2024
  22. hebasto commented at 2:00 pm on February 12, 2024: member
    @GBKS What do you think from a designer’s point of view?
  23. DrahtBot removed review request from hernanmarino on Feb 12, 2024
  24. DrahtBot requested review from hernanmarino on Feb 12, 2024
  25. maflcko commented at 2:13 pm on February 12, 2024: contributor
    Not sure. All info displayed on the About page is fixed details about the software itself, not about the current (test)chain. So modulating the icon or title bar seems confusing, but no strong opinion.
  26. DrahtBot removed review request from hernanmarino on Feb 12, 2024
  27. DrahtBot requested review from hernanmarino on Feb 12, 2024
  28. GBKS commented at 12:30 pm on February 15, 2024: none
    Definitely more consistent this way.
  29. DrahtBot removed review request from hernanmarino on Feb 15, 2024
  30. DrahtBot requested review from hernanmarino on Feb 15, 2024
  31. hebasto commented at 1:22 pm on February 15, 2024: member

    Not sure. All info displayed on the About page is fixed details about the software itself, not about the current (test)chain.

    In that case, shouldn’t the logo on the About page be black-toned?

  32. DrahtBot removed review request from hernanmarino on Feb 15, 2024
  33. DrahtBot requested review from hernanmarino on Feb 15, 2024
  34. pablomartin4btc commented at 0:23 am on February 28, 2024: contributor

    Not sure. All info displayed on the About page is fixed details about the software itself, not about the current (test)chain.

    In that case, shouldn’t the logo on the About page be black-toned?

    I’m less sure about black-toned icons, in that specific case, it’s more common usage I guess, users are already used to the orange one.

    I think the most notable use case is when you have different instances open and you want to make sure you are standing on the correct one but more importantly the consistency regarding other components such as the splash screen and task-bar icons as pointed by @GBKS.

  35. DrahtBot removed review request from hernanmarino on Feb 28, 2024
  36. DrahtBot requested review from hernanmarino on Feb 28, 2024
  37. hebasto commented at 5:56 pm on May 15, 2024: member

    #762 (comment):

    Definitely more consistent this way.

    Thank you @GBKS for your designer’s opinion!

  38. in src/qt/utilitydialog.h:40 in 30c235b2e6 outdated
    35@@ -34,6 +36,8 @@ class HelpMessageDialog : public QDialog
    36 
    37 private Q_SLOTS:
    38     void on_okButton_accepted();
    39+    void setAboutWindowTitle(const NetworkStyle *networkStyle = nullptr);
    40+    void setChainTypeIconOnAboutLogo(const NetworkStyle *networkStyle = nullptr);
    


    hebasto commented at 6:07 pm on May 15, 2024:
    Why these two member functions are declared as slots?

    pablomartin4btc commented at 7:34 pm on May 15, 2024:
    Sorry, that’s wrong, I meant to put them just on the private section. I’ll fix it soon. Thanks!

    pablomartin4btc commented at 7:10 pm on May 19, 2024:
    done!
  39. hebasto commented at 6:10 pm on May 15, 2024: member
    If you will touch this branch, mind applying clang-format-diff.py?
  40. pablomartin4btc force-pushed on May 19, 2024
  41. pablomartin4btc commented at 7:33 pm on May 19, 2024: contributor

    Updates:

    0git diff -U0 HEAD~1.. | ./contrib/devtools/clang-format-diff.py -p1 -i -v
    1Formatting src/qt/bitcoingui.cpp
    2Formatting src/qt/utilitydialog.cpp
    3Formatting src/qt/utilitydialog.h
    
  42. in src/qt/utilitydialog.cpp:35 in 577d500cb1 outdated
    30@@ -29,18 +31,17 @@
    31 #include <QVBoxLayout>
    32 
    33 /** "Help message" or "About" dialog box */
    34-HelpMessageDialog::HelpMessageDialog(QWidget *parent, bool about) :
    35-    QDialog(parent, GUIUtil::dialog_flags),
    36-    ui(new Ui::HelpMessageDialog)
    37+HelpMessageDialog::HelpMessageDialog(QWidget* parent, bool about, const NetworkStyle* networkStyle) : QDialog(parent, GUIUtil::dialog_flags),
    38+                                                                                                      ui(new Ui::HelpMessageDialog)
    


    hebasto commented at 3:20 pm on August 5, 2024:

    Break the long line for better readability:

    0HelpMessageDialog::HelpMessageDialog(QWidget* parent, bool about, const NetworkStyle* networkStyle)
    1    : QDialog(parent, GUIUtil::dialog_flags),
    2      ui(new Ui::HelpMessageDialog)
    

    ?


    pablomartin4btc commented at 3:56 pm on August 5, 2024:
    Ok, will do.
  43. in src/qt/utilitydialog.cpp:142 in 577d500cb1 outdated
    135@@ -135,6 +136,19 @@ void HelpMessageDialog::on_okButton_accepted()
    136     close();
    137 }
    138 
    139+void HelpMessageDialog::setAboutWindowTitle(const NetworkStyle* networkStyle)
    140+{
    141+    QString aboutTitle = tr("About %1").arg(PACKAGE_NAME);
    142+    if ((networkStyle) && (Params().GetChainType() != ChainType::MAIN)) aboutTitle.append(" " + networkStyle->getTitleAddText());
    


    hebasto commented at 3:39 pm on August 5, 2024:

    Putting a non-trivial expression into a separate line can facilitate debugging in the future:

    0    if (networkStyle && Params().GetChainType() != ChainType::MAIN) {
    1        aboutTitle.append(" " + networkStyle->getTitleAddText());
    2    }
    

    pablomartin4btc commented at 3:55 pm on August 5, 2024:
    Ok, I’ll update it, thanks!
  44. hebasto commented at 3:45 pm on August 5, 2024: member

    Tested 577d500cb1ac4122fba3202c289454dcbe31a194.

    On Windows, there remains some inconsistency. Running bitcoin-qt.exe -signet -about ends with a dialog with an orange-coloured icon.

  45. pablomartin4btc commented at 3:56 pm on August 5, 2024: contributor

    On Windows, there remains some inconsistency. Running bitcoin-qt.exe -signet -about ends with a dialog with an orange-coloured icon.

    Let me take a look…

  46. pablomartin4btc commented at 4:51 pm on August 8, 2024: contributor

    On Windows, there remains some inconsistency. Running bitcoin-qt.exe -signet -about ends with a dialog with an orange-coloured icon.

    Let me take a look…

    image

    Good catch… trying to get it sorted…

  47. gui: Update about logo icon to denote chain type
    Adding the networkStyle parameter to the HelpMessageDialog
    creator on utilitydialog, updating all calls where its instance
    is being created from bitcoingui.cpp. In the object itself,
    use this new parameter object to build the about window title and
    set the icon of the about logo widget.
    
    Also, set th correct chain type icon at the very beginning of
    GuiMain() where command line arguments were validated and most of
    the app settings were not initialised yet.
    5556860851
  48. pablomartin4btc force-pushed on Aug 25, 2024
  49. pablomartin4btc commented at 7:16 pm on August 25, 2024: contributor

    Updates:

    image

  50. DrahtBot removed the label CI failed on Aug 25, 2024
  51. DrahtBot commented at 5:50 pm on October 28, 2024: contributor

    🐙 This pull request conflicts with the target branch and needs rebase.

  52. DrahtBot added the label Needs rebase on Oct 28, 2024

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin-core/gui. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-10-31 22:20 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me