splash: New layout by RandyMcMillan · Pull Request #147 · bitcoin-core/gui

RandyMcMillan · 2020-12-09T01:52:55Z

Before:

After:

jonasschnelli · 2020-12-09T07:56:12Z

Usually one doesn't mix center alignment (the Verifing blocks...-part) with left/right alignment (the rest).
weak-NACK on removing "Core" just here. I suggest keeping it. If you want to propose a renaming, it should be an independent PR that covers a complete re-name.

RandyMcMillan · 2020-12-09T16:57:04Z

@jonatack
@jonasschnelli
@Bosch-0

I thought the

The Bitcoin Core developers as a group was a strong point and I am
implementing changes based on this idea.

Can you come to some decision on this?
This is past the point of absurdity.

I agree with not emphasizing "Core". Let's not institutionalize that emphasis further. Please keep them equal or put more weight on "Bitcoin".

One of my pet peeves is people writing "Core" instead of "Bitcoin Core" ("Core devs", "running Core", and so on).

See the Bitcoin Optech style guide:
Forbidden abbreviations:

Core (use Bitcoin Core)
100% agree with our Bitcoin Optech colleagues about this.

I've already explained my rationale many times above.

I'm still of the thought that:

Bitcoin Core is a group of developers who work on Bitcoin.

Bitcoin is a network that everyone is apart of.

Electrum and BlueWallet are part of the Bitcoin ecosystem as is the GUI developed by the Core developers.

promag · 2020-12-09T17:08:45Z

NACK, beside some alignments current splash LGTM.

As suggested by @jonasschnelli try to submit individual pull requests, in this case drop-core-from-splash and change-splash-layout.

RandyMcMillan · 2020-12-09T18:34:22Z

@promag @jonasschnelli @laanwj - I find it strange that there are some many radical changes going on "under the hood" but changes to the Gui need to be split into "incremental" changes that will drag out Gui dev ad infinitum...

promag · 2020-12-09T18:49:18Z

You are trying to change different things in one shot. Also these changes have no motivation, doesn't fix anything, doesn't improve anything. I've suggested to split in 2 pull requests, or if you prefer, you can split in 2 commits, so reviewers can focus in what's more relevant to them. You can see from git log that there's an effort in having logical commits. I've already suggested this approach in #135 (review) but didn't got a response, just a 👀 reaction.

I find it strange that there are some many radical changes going on "under the hood"

Please point what changes you refer that go without review and approval.

Bosch-0 · 2020-12-11T08:09:51Z

Here is a WIP design I did for this current splash screen - just my 2c. Planning on returning to this design work soon.

jonasschnelli · 2020-12-11T08:22:24Z

I like @Bosch-0's design. Looks nice and clean.

Rspigler · 2020-12-12T21:07:15Z

NACK. I agree with the other commentators. As discussed here (#140 (comment)) you can't slip in a name change, especially in multiple unrelated PRs, creating more confusion/inconsistency.

I like @Bosch-0 's concept, I think it would be a good middle ground for those who have expressed concern that there isn't enough showing that 'Bitcoin Core' connects to the 'Bitcoin' network.

luke-jr · 2020-12-14T20:43:34Z

I don't know where those quotes are from, but this project/software is not "Bitcoin" - it is "Bitcoin Core".

Fine with considering a name change (to a reasonable name, not "Bitcoin"), but that should be a dedicated PR as others have said.

Personally, I like the current splash screen.

RandyMcMillan · 2020-12-25T01:40:42Z

@Bosch-0 - I like the simplicity of your layout - plus it bypasses the whole "Bitcoin" vs "BitcoinCore" issue...
I am going to implement your layout in this PR.

RandyMcMillan · 2020-12-25T02:05:47Z

@luke-jr - had you been a better custodian - maybe controversial issues like this wouldn't even exist?

Bosch-0 · 2020-12-30T07:43:35Z

plus it bypasses the whole "Bitcoin" vs "BitcoinCore" issue...

Yes this was my main intention with this, makes both camps happy

RandyMcMillan · 2020-12-30T15:55:24Z

@Bosch-0 @jonasschnelli

WIP: Current presentation...

I am not sure why the linting fails...
be22124#diff-f63c3d5094d55f88dbd1967774f85838b3aee5a40540b8c82b924574bca772a0R187

NOTE: The curMessage needs more work.

hebasto · 2020-12-30T16:11:27Z

I am not sure why the linting fails...

https://cirrus-ci.com/task/4754423266148352?command=lint#L884:

src/qt/splashscreen.cpp: Expected 1 argument(s) after format string but found 0 argument(s): strprintf("%")
src/qt/splashscreen.cpp: Expected 1 argument(s) after format string but found 0 argument(s): strprintf("%")
^---- failure generated from test/lint/lint-format-strings.s

Bosch-0 · 2021-01-09T05:06:52Z

What's the reason the icon looks off? If you need a new icon the one I included in the above design is here > https://www.figma.com/file/0oqnohjahRtprjRyaetDOL/Bitcoin-Core-Design-System?node-id=1851%3A135

To export just click the icon > go to the menu on the right hand side > go to the export tab > export as PNG.

RandyMcMillan · 2021-01-26T17:31:24Z

@Bosch-0 - I would prefer to limit the scope of this PR to the layout implementation only.
The art work itself - I believe - can be its own PR.
BTW - the work looks excellent! Definitely moving in the right direction! :)

Rspigler · 2021-01-26T19:11:26Z

I think the PR should contain the proper icon before merge

RandyMcMillan · 2021-01-26T19:54:46Z

@Bosch-0 @Rspigler

the icon needs to be 1024x1024 RGB Alpha:Yes to match the existing artwork.
This ensures proper resolution on high res displays.

The QPainter expects a certain geometry when inserting graphics into the UI.
This requires its own testing.

I believe the art work needs its own PR (imo)

Original:

Updated:

jarolrod

Concept ACK

TheVerifying Blocks prompt glitches on macOS 11.1 with Qt 5.15.2.

Some additional thoughts:

Update the OP screenshot. The screenshot currently shown is not what this PR does
The current layout as of 009b1b6 doesn't exactly abide to the layout designed by @Bosch-0, but it's close. Perhaps we should get it as close as possible to his design before merge.

jarolrod

Small change on wording

jarolrod · 2021-01-26T23:51:20Z

-    QString titleAddText    = networkStyle->getTitleAddText();
+    QString titleText           = PACKAGE_NAME;
+    QString packageNameSubStr   = titleText.mid(0,7);
+    QString splashText          = QString("Connecting to The %1 Network").arg(packageNameSubStr);


Suggested change

QString splashText = QString("Connecting to The %1 Network").arg(packageNameSubStr);

QString splashText = QString("Connecting to the %1 network").arg(packageNameSubStr);

the and network should not be capitalized

I just kinda like "title case" better - since "The Bitcoin Network" is an entity unto itself. :)
You can define it as a collection of sovereign individuals - and in this way - I believe title case is appropriate.
But I will change it...

laanwj · 2021-01-27T03:27:16Z

Not sure why I'm tagged here, but if you want my opinion: NACK on renaming the software from Bitcoin Core, ACK on graphically more attractive splash screen.

Ideally though, in the long run, would be to get rid of the splash screen completely. To do everything but basic program initialization in the background while some kind of UI is already visible and usable.

Bosch-0 · 2021-01-27T05:02:43Z

@Bosch-0 - I would prefer to limit the scope of this PR to the layout implementation only.
The art work itself - I believe - can be its own PR.

I'll get on to this, concept ACK though looks good!

hebasto

Tested da1ac0f on Linux Mint 20.1 (Qt 5.12.8):

Hmm. GH shows 2 commits, but the fetched pr branch shows 1 commit on top of the master branch locally.

Concept ACK on @Bosch-0's design.

While this pr is a layout revamping, why do not use a Qt Designer's UI file?
It will distinguish design from coding.

RandyMcMillan · 2021-02-26T01:42:33Z

@hebasto - I will have to search for the comment (not in src/qt/splashscreen.cpp) that stated that the splashscreen was implemented this way to work around an issue with Qt. It may have been a timing issue with launching other services simultaneously if memory serves me.

I agree - it would be optimal to use a .ui file - to eliminate a lot of layout code.

RandyMcMillan · 2021-02-28T03:05:56Z

@Bosch-0 - checkout https://github.com/RandyMcMillan/gui/tree/new-icon for testing.

Bosch-0 mentioned this pull request Dec 12, 2020

[Discussion] Bitcoin Core logo #89

Open

RandyMcMillan marked this pull request as draft December 15, 2020 14:44

RandyMcMillan changed the title ~~splash:bitcoin branding and cleanup~~ splash: New layout Jan 26, 2021

RandyMcMillan marked this pull request as ready for review January 26, 2021 19:56

jarolrod reviewed Jan 26, 2021

View reviewed changes

Bosch-0 mentioned this pull request Jan 27, 2021

Added updated bitcoin PNG / SVG icon #199

Closed

hebasto reviewed Feb 22, 2021

View reviewed changes

splash: New layout

03d3677

RandyMcMillan marked this pull request as draft February 28, 2021 03:14

RandyMcMillan mentioned this pull request Feb 28, 2021

splash: new layout and new icon #231

Closed

RandyMcMillan closed this Apr 20, 2021

bitcoin-core locked as resolved and limited conversation to collaborators Aug 16, 2022

	QString splashText = QString("Connecting to The %1 Network").arg(packageNameSubStr);
	QString splashText = QString("Connecting to the %1 network").arg(packageNameSubStr);

Conversation

RandyMcMillan commented Dec 9, 2020

Uh oh!

jonasschnelli commented Dec 9, 2020

Uh oh!

RandyMcMillan commented Dec 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

promag commented Dec 9, 2020

Uh oh!

RandyMcMillan commented Dec 9, 2020

Uh oh!

promag commented Dec 9, 2020

Uh oh!

Bosch-0 commented Dec 11, 2020

Uh oh!

jonasschnelli commented Dec 11, 2020

Uh oh!

Rspigler commented Dec 12, 2020

Uh oh!

luke-jr commented Dec 14, 2020

Uh oh!

RandyMcMillan commented Dec 25, 2020

Uh oh!

RandyMcMillan commented Dec 25, 2020

Uh oh!

Bosch-0 commented Dec 30, 2020

Uh oh!

RandyMcMillan commented Dec 30, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hebasto commented Dec 30, 2020

Uh oh!

Bosch-0 commented Jan 9, 2021

Uh oh!

RandyMcMillan commented Jan 26, 2021

Uh oh!

Rspigler commented Jan 26, 2021

Uh oh!

RandyMcMillan commented Jan 26, 2021

I believe the art work needs its own PR (imo)

Uh oh!

jarolrod left a comment

Choose a reason for hiding this comment

Uh oh!

jarolrod left a comment

Choose a reason for hiding this comment

Uh oh!

jarolrod Jan 26, 2021

Choose a reason for hiding this comment

Uh oh!

RandyMcMillan Jan 27, 2021

Choose a reason for hiding this comment

Uh oh!

laanwj commented Jan 27, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Bosch-0 commented Jan 27, 2021

Uh oh!

hebasto left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RandyMcMillan commented Feb 26, 2021

Uh oh!

RandyMcMillan commented Feb 28, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

RandyMcMillan commented Dec 9, 2020 •

edited

Loading

RandyMcMillan commented Dec 30, 2020 •

edited

Loading

laanwj commented Jan 27, 2021 •

edited

Loading

hebasto left a comment •

edited

Loading