splash: New layout #147

pull RandyMcMillan wants to merge 1 commits into bitcoin-core:master from RandyMcMillan:splash changing 1 files +42 −36
  1. RandyMcMillan commented at 1:52 am on December 9, 2020: contributor

    Before: Screen Shot 2020-12-08 at 8 51 03 PM

    After: Screen Shot 2020-12-08 at 8 45 31 PM

  2. jonasschnelli commented at 7:56 am on December 9, 2020: contributor
    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.
  3. RandyMcMillan commented at 4:57 pm on December 9, 2020: contributor

    @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:

    0Forbidden abbreviations:
    1
    2Core (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.

  4. promag commented at 5:08 pm on December 9, 2020: contributor

    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.

  5. RandyMcMillan commented at 6:34 pm on December 9, 2020: contributor
    @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…
  6. promag commented at 6:49 pm on December 9, 2020: contributor

    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#pullrequestreview-543049890 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.

  7. Bosch-0 commented at 8:09 am on December 11, 2020: none

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

    image

  8. jonasschnelli commented at 8:22 am on December 11, 2020: contributor
    I like @Bosch-0’s design. Looks nice and clean.
  9. Rspigler commented at 9:07 pm on December 12, 2020: contributor

    NACK. I agree with the other commentators. As discussed here (https://github.com/bitcoin-core/gui/pull/140#issuecomment-743607254) 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.

  10. luke-jr commented at 8:43 pm on December 14, 2020: member

    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.

  11. RandyMcMillan marked this as a draft on Dec 15, 2020
  12. RandyMcMillan commented at 1:40 am on December 25, 2020: contributor
    @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.
  13. RandyMcMillan commented at 2:05 am on December 25, 2020: contributor
    @luke-jr - had you been a better custodian - maybe controversial issues like this wouldn’t even exist?
  14. Bosch-0 commented at 7:43 am on December 30, 2020: none

    plus it bypasses the whole “Bitcoin” vs “BitcoinCore” issue…

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

  15. RandyMcMillan commented at 3:55 pm on December 30, 2020: contributor
  16. hebasto commented at 4:11 pm on December 30, 2020: member

    I am not sure why the linting fails…

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

    0src/qt/splashscreen.cpp: Expected 1 argument(s) after format string but found 0 argument(s): strprintf("%")
    1src/qt/splashscreen.cpp: Expected 1 argument(s) after format string but found 0 argument(s): strprintf("%")
    2^---- failure generated from test/lint/lint-format-strings.s
    
  17. Bosch-0 commented at 5:06 am on January 9, 2021: none

    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.

  18. RandyMcMillan commented at 5:31 pm on January 26, 2021: contributor
    @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! :)
  19. Rspigler commented at 7:11 pm on January 26, 2021: contributor
    I think the PR should contain the proper icon before merge
  20. RandyMcMillan commented at 7:54 pm on January 26, 2021: contributor

    @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: Screen Shot 2021-01-26 at 2 36 34 PM

    Updated: Screen Shot 2021-01-26 at 2 42 14 PM

  21. RandyMcMillan renamed this:
    splash:bitcoin branding and cleanup
    splash: New layout
    on Jan 26, 2021
  22. RandyMcMillan marked this as ready for review on Jan 26, 2021
  23. jarolrod commented at 11:44 pm on January 26, 2021: member

    Concept ACK

    TheVerifying Blocks prompt glitches on macOS 11.1 with Qt 5.15.2. Screen Shot 2021-01-26 at 6 37 34 PM

    Some additional thoughts:

    • Update the OP screenshot. The screenshot currently shown is not what this PR does
    • The current layout as of 009b1b68d6a59057ed16667385d3d0599bd09772 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.
  24. in src/qt/splashscreen.cpp:45 in 009b1b68d6 outdated
    49-    QString versionText     = QString("Version %1").arg(QString::fromStdString(FormatFullVersion()));
    50-    QString copyrightText   = QString::fromUtf8(CopyrightHolders(strprintf("\xc2\xA9 %u-%u ", 2009, COPYRIGHT_YEAR)).c_str());
    51-    QString titleAddText    = networkStyle->getTitleAddText();
    52+    QString titleText           = PACKAGE_NAME;
    53+    QString packageNameSubStr   = titleText.mid(0,7);
    54+    QString splashText          = QString("Connecting to The %1 Network").arg(packageNameSubStr);
    


    jarolrod commented at 11:51 pm on January 26, 2021:
    0    QString splashText          = QString("Connecting to the %1 network").arg(packageNameSubStr);
    

    the and network should not be capitalized


    RandyMcMillan commented at 0:45 am on January 27, 2021:
    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…
  25. jarolrod commented at 11:52 pm on January 26, 2021: member
    Small change on wording
  26. laanwj commented at 3:27 am on January 27, 2021: member

    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.

  27. Bosch-0 commented at 5:02 am on January 27, 2021: none

    @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!

  28. hebasto commented at 8:14 pm on February 22, 2021: member

    Tested da1ac0f0698b0d9bc6ec00774c5882d7e4ab244c on Linux Mint 20.1 (Qt 5.12.8): DeepinScreenshot_select-area_20210222220848

    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.

  29. RandyMcMillan commented at 1:42 am on February 26, 2021: contributor

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

  30. splash: New layout 03d3677de3
  31. RandyMcMillan commented at 3:05 am on February 28, 2021: contributor
  32. RandyMcMillan marked this as a draft on Feb 28, 2021
  33. RandyMcMillan closed this on Apr 20, 2021

  34. bitcoin-core locked this on Aug 16, 2022

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-23 02:20 UTC

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