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

pablomartin4btc commented at 12: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.

<details> <summary>Before PR changes screenshot</summary>

</details>

<details> <summary>After PR changes screenshot</summary>

</details>

<details> <summary>Current splash screen screenshot</summary> (this hasn't been touched, just to show consistency with this PR changes)

</details>

Tested it also with:

mainnet (no command line args after bitcoin-qt or bitcoin-qt -chain=main),
-regtest and
-testnet.

DrahtBot commented at 12: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, laanwj
Stale ACK	BrandonOdiwuor

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#933 ([DRAFT] Introduce Qt test automation bridge and gui functional tests by johnny9)

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.

LLM Linter (✨ experimental)

Possible typos and grammar issues:

"Create a message box, because the gui has neither been created nor has subscribed to core signals" -> "Create a message box, because the gui has neither been created nor has it subscribed to core signals." [Missing pronoun "it" makes the second clause ungrammatical; adding "it" fixes the subject for "has subscribed."]

No other typographic or grammatical errors affecting comprehension were found.

2025-12-03

DrahtBot added the label CI failed on Sep 25, 2023

DrahtBot removed the label CI failed on Sep 25, 2023

pablomartin4btc force-pushed on Sep 25, 2023

in src/qt/utilitydialog.cpp:143 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.

maflcko commented at 2:03 PM on October 5, 2023: contributor

It may help to show screenshots before and after

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.

hernanmarino approved

hernanmarino commented at 9:27 PM on October 6, 2023: contributor

tested ACK

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)

MarnixCroes approved

MarnixCroes commented at 2:25 PM on November 2, 2023: contributor

tack 30c235b2e6fd9b7e3634d795cedc171d4d8d0e27

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

    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!

BrandonOdiwuor commented at 11:06 AM on December 28, 2023: contributor

Concept ACK

BrandonOdiwuor approved

BrandonOdiwuor commented at 1:38 PM on December 28, 2023: contributor

Tested ACK 30c235b2e6fd9b7e3634d795cedc171d4d8d0e27

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

DrahtBot requested review from hernanmarino on Dec 28, 2023

DrahtBot removed review request from hernanmarino on Dec 28, 2023

DrahtBot requested review from hernanmarino on Dec 28, 2023

DrahtBot added the label CI failed on Jan 14, 2024

hebasto commented at 2:00 PM on February 12, 2024: member

@GBKS What do you think from a designer's point of view?

DrahtBot removed review request from hernanmarino on Feb 12, 2024

DrahtBot requested review from hernanmarino on Feb 12, 2024

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.

DrahtBot removed review request from hernanmarino on Feb 12, 2024

DrahtBot requested review from hernanmarino on Feb 12, 2024

GBKS commented at 12:30 PM on February 15, 2024: none

Definitely more consistent this way.

DrahtBot removed review request from hernanmarino on Feb 15, 2024

DrahtBot requested review from hernanmarino on Feb 15, 2024

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?

DrahtBot removed review request from hernanmarino on Feb 15, 2024

DrahtBot requested review from hernanmarino on Feb 15, 2024

pablomartin4btc commented at 12: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.

DrahtBot removed review request from hernanmarino on Feb 28, 2024

DrahtBot requested review from hernanmarino on Feb 28, 2024

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!

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!

hebasto commented at 6:10 PM on May 15, 2024: member

If you will touch this branch, mind applying clang-format-diff.py?

pablomartin4btc force-pushed on May 19, 2024

pablomartin4btc commented at 7:33 PM on May 19, 2024: contributor

Updates:

<ul><li><details> <summary>Addressed [@hebasto](/bitcoin-core-gui/contributor/hebasto/)'s <a href="https://github.com/bitcoin-core/gui/pull/762#pullrequestreview-2058694015">feedback</a>.</summary>

git diff -U0 HEAD~1.. | ./contrib/devtools/clang-format-diff.py -p1 -i -v
Formatting src/qt/bitcoingui.cpp
Formatting src/qt/utilitydialog.cpp
Formatting src/qt/utilitydialog.h

</details></li></ul>

As part of the above, functions were re-placed in the private section (no in slots as mistaken).
As a consequence of applying the clang-formatting, @kristapsk's feedback got addressed as well.

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:

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

?

pablomartin4btc commented at 3:56 PM on August 5, 2024:

Ok, will do.

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:

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

pablomartin4btc commented at 3:55 PM on August 5, 2024:

Ok, I'll update it, thanks!

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.

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

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

Good catch... trying to get it sorted...

pablomartin4btc force-pushed on Aug 25, 2024

pablomartin4btc commented at 7:16 PM on August 25, 2024: contributor

Updates:

Addressed @hebasto's feedback.
Fixed the error message window inconsistency detected by @hebasto above.

<details> <summary>Now running <code>qt</code> from the command line with <code>bitcoin-qt.exe -signet -about</code> (or similar command with an invalid argument) will show the correct chain type icon at the top left of the title bar (this is only happening in Windows at the moment).</summary>

</details>

DrahtBot removed the label CI failed on Aug 25, 2024

DrahtBot added the label Needs rebase on Oct 28, 2024

pablomartin4btc force-pushed on Jan 31, 2025

DrahtBot removed the label Needs rebase on Jan 31, 2025

pablomartin4btc commented at 2:50 PM on January 31, 2025: contributor

Rebased.

achow101 removed review request from hernanmarino on Oct 22, 2025

achow101 requested review from davidgumberg on Oct 22, 2025

achow101 requested review from laanwj on Oct 22, 2025

laanwj commented at 3:00 PM on October 23, 2025: member

Concept ACK

in src/qt/utilitydialog.cpp:32 in 036f070e3b

  28 | @@ -27,7 +29,7 @@
  29 |  #include <QVBoxLayout>
  30 |  
  31 |  /** "Help message" or "About" dialog box */
  32 | -HelpMessageDialog::HelpMessageDialog(QWidget *parent, bool about) :
  33 | +HelpMessageDialog::HelpMessageDialog(QWidget* parent, bool about, const NetworkStyle* networkStyle) :

laanwj commented at 3:13 PM on October 23, 2025:

Please use network_style naming for new parameters.

pablomartin4btc commented at 1:44 AM on December 3, 2025:

Done.

in src/qt/bitcoingui.cpp:108 in 036f070e3b

 104 | @@ -105,7 +105,7 @@ BitcoinGUI::BitcoinGUI(interfaces::Node& node, const PlatformStyle *_platformSty
 105 |      updateWindowTitle();
 106 |  
 107 |      rpcConsole = new RPCConsole(node, _platformStyle, nullptr);
 108 | -    helpMessageDialog = new HelpMessageDialog(this, false);
 109 | +    helpMessageDialog = new HelpMessageDialog(this, false, m_network_style);

laanwj commented at 3:14 PM on October 23, 2025:

Try to be consistent about using named arguments here and below. e.g. either add for both

helpMessageDialog = new HelpMessageDialog(/*parent=*/this, /*about=*/false, /*network_style=*/m_network_style);

Or remove them (but i think adding is better).

pablomartin4btc commented at 1:44 AM on December 3, 2025:

Done.

in src/qt/bitcoin.cpp:539 in 036f070e3b outdated

 535 | -        QMessageBox::critical(nullptr, CLIENT_NAME,
 536 | +        QMessageBox mBox(static_cast<QMessageBox::Icon>(nMBoxIcon), CLIENT_NAME,
 537 |              // message cannot be translated because translations have not been initialized
 538 |              QString::fromStdString("Error parsing command line arguments: %1.").arg(QString::fromStdString(error)));
 539 | +        mBox.setTextFormat(Qt::PlainText);
 540 | +        if (gArgs.GetChainTypeString() != "main") {

laanwj commented at 3:18 PM on October 23, 2025:

What is the reason to skip this for main? It should always work to get the right color scheme, right?

pablomartin4btc commented at 5:43 PM on November 30, 2025:

<details> <summary>Yeah, that's wrong because when there's no chain type specified or it's <code>-chain=main</code>, won't be any icon shown on the message box title bar.</summary>

</details>

At the time of writing it, "maybe" I meant something like this if (!gArgs.GetChainTypeString().empty()) {, which is what I'm going to use as a fix.

But I see other places, mostly within the same function (int GuiMain(int argc, char* argv[])), where this is happening (no icon on critical message box -> check the first paragraph collapsed section's picture). So I'll fix them all on a separate commit.

pablomartin4btc commented at 4:08 AM on December 3, 2025:

Done in 2nd. commit: "gui: Update QMessageBox::Icon with chaintype image".

pablomartin4btc force-pushed on Dec 3, 2025

DrahtBot added the label CI failed on Dec 3, 2025

pablomartin4btc force-pushed on Dec 3, 2025

DrahtBot removed the label CI failed on Dec 3, 2025

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.

8f5949e4ff

gui: Update QMessageBox::Icon with chaintype image

Introducing a new helper GUIUtil::ShowMessageBox() in order to
incorporate the chaintype image in the message box window icon.

da44744ee4

pablomartin4btc force-pushed on Dec 3, 2025

pablomartin4btc commented at 3:16 PM on December 3, 2025: contributor

<ins>Updates:</ins>

Addressed @laanwj's feedback.

Reworked the error message window inconsistency fix detected by @hebasto above, applying it to most QMessageBox possible instances in bitcoin.cpp and separating it in the 2nd commit for clarity and in case it needs to be dropped because it's getting out of scope or extended to other files (perhaps a follow-up in a separate PR).

<details> <summary>Message-box window icon behavior is inconsistent across platforms: some X11 window managers and many Wayland compositors ignore message-box icons entirely. With the new <code>GUIUtil::ShowMessageBox()</code> helper, we can instead explicitly set <code>QIcon()</code> (no icon) and avoid constructing a <code>NetworkStyle</code> just for dialogs or checking whether a <code>ChainType</code> was passed. (Windows behavior still needs to be re-checked under Qt 6 — to confirm whether the no-icon option is effectively applied.)</summary>

--- a/src/qt/guiutil.cpp
+++ b/src/qt/guiutil.cpp
@@ -1012,7 +1012,6 @@ void ShowModalDialogAsynchronously(QDialog* dialog)

 void ShowMessageBox(const QString& message,
                             QMessageBox::Icon box_icon,
-                            const NetworkStyle* network_style,
                             const QString& title)
 {
     QString qTitle = CLIENT_NAME;
@@ -1021,14 +1020,7 @@ void ShowMessageBox(const QString& message,

     mBox.setTextFormat(Qt::PlainText);

-    if (network_style) {
-        mBox.setWindowIcon(network_style->getTrayAndWindowIcon());
-    } else if (!gArgs.GetChainTypeString().empty()) {
-        std::unique_ptr<const NetworkStyle> fallback(NetworkStyle::instantiate(gArgs.GetChainType()));
-        if (fallback) {
-            mBox.setWindowIcon(fallback->getTrayAndWindowIcon());
-        }
-    }
+    mBox.setWindowIcon(QIcon());

     mBox.exec();
 }

</details>