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.
Tested it also with:
bitcoin-qt or bitcoin-qt -chain=main),-regtest and-testnet.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
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.
Reviewers, this pull request conflicts with the following ones:
PACKAGE_* variables to CLIENT_* by hebasto)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.
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());
I didn’t want to touch src/qt/bitcoin.cpp:
And I also needed to move where the networkStyle is initialised/ set before the HelpMessageDialog constructor:
But I can do it if that’s better.
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.
144+ setWindowTitle(aboutTitle);
145+}
146+
147+void HelpMessageDialog::setChainTypeIconOnAboutLogo(const NetworkStyle *networkStyle)
148+{
149+ const QSize requiredSize(1024,1024);
nit
0 const QSize requiredSize(1024, 1024);
Tested ACK 30c235b2e6fd9b7e3634d795cedc171d4d8d0e27
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?
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@@ -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);
clang-format-diff.py?
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
private section (no in slots as mistaken).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)
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)
?
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());
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 }
Tested 577d500cb1ac4122fba3202c289454dcbe31a194.
On Windows, there remains some inconsistency. Running bitcoin-qt.exe -signet -about ends with a dialog with an orange-coloured icon.
On Windows, there remains some inconsistency. Running
bitcoin-qt.exe -signet -aboutends with a dialog with an orange-coloured icon.
Let me take a look…
On Windows, there remains some inconsistency. Running
bitcoin-qt.exe -signet -aboutends with a dialog with an orange-coloured icon.Let me take a look…
Good catch… trying to get it sorted…
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.
Updates:
🐙 This pull request conflicts with the target branch and needs rebase.