Skip to content

Use layout manager for Create Wallet dialog #171

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Jan 21, 2021
Merged

Use layout manager for Create Wallet dialog #171

merged 1 commit into from
Jan 21, 2021

Conversation

hebasto
Copy link
Member

@hebasto hebasto commented Jan 1, 2021 •
edited
Loading

On master (e75f91e) not using layout manager causes problems with resizing:

Screenshot from 2021-01-01 13-03-13
Screenshot from 2021-01-01 13-03-26

Also text labels are not resized properly on some window managers (bitcoin/bitcoin#20777), or if their lengths are changed (after translation).

This PR introduces a standard layout manager for the "Create Wallet" dialog that fixes all layout issues (actually, the createwalletdialog.ui has been re-written from scratch):

Screenshot from 2021-01-01 13-10-03
Screenshot from 2021-01-06 23-50-36

Additional visual changes:

Fix bitcoin/bitcoin#20777

@hebasto hebasto mentioned this pull request Jan 1, 2021
Closed
@Sjors
Copy link
Member

Sjors commented Jan 1, 2021 •
edited
Loading

tACK 3cbcb25 on macOS 11.1 dark mode
Schermafbeelding 2021-01-01 om 20 17 49

@dooglus
Copy link
Contributor

dooglus commented Jan 2, 2021

Looks great now:

Screenshot_2021-01-01_14-05-35

When I resize the window it redraws like this:

Screenshot_2021-01-01_14-05-52

@maflcko maflcko added this to the 0.21.1 milestone Jan 2, 2021
@maflcko
Copy link
Contributor

maflcko commented Jan 4, 2021

Tagged for 0.21.1

jarolrod
jarolrod reviewed Jan 6, 2021
View reviewed changes
Copy link
Member

@jarolrod jarolrod left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Testing on macOS Big Sur 11.1 with Qt 5.15.2 I noticed some abnormalities concerning the resizing behavior and the text. Below are some screenshots comparing behavior of Master and PR. Looking at the PR screenshots you will notice:

  1. The Wallet Input Form does not stay in the top left corner and it does not resize. It just stays in the center of the window.
  2. The Advance Options Text is no longer Bold
  3. The Advanced Options and Wallet Name text are a different size

Master
master-normal

master-resize

PR
pr-normal

pr-resize

@jonatack
Copy link
Member

jonatack commented Jan 6, 2021

Concept ACK

@luke-jr
Copy link
Member

luke-jr commented Jan 6, 2021

I would think the empty space belongs below the advanced-options group box...

@hebasto
Copy link
Member Author

hebasto commented Jan 6, 2021

Updated 3cbcb25 -> d4feb68 (pr171.01 -> pr171.02, diff):

The Wallet Input Form does not stay in the top left corner and it does not resize. It just stays in the center of the window.

I would think the empty space belongs below the advanced-options group box...

@jarolrod

  1. The Advance Options Text is no longer Bold

  2. The Advanced Options and Wallet Name text are a different size

It is intended to left default parameters for the QGroupBox widget in order to make look-and-feel standard for a particular platform. We already use the same approach, see screenshots from macOS:
Screenshot from 2021-01-06 23-48-20
Screenshot from 2021-01-06 23-47-48

OP screenshots updated.

@jarolrod
Copy link
Member

jarolrod commented Jan 6, 2021 •
edited
Loading

ACK d4feb68

Fixed the resizing issue on macOS for me.
Screen Shot 2021-01-06 at 5 16 04 PM

@Sjors
Copy link
Member

Sjors commented Jan 8, 2021

re-tACK d4feb68

@promag
Copy link
Contributor

promag commented Jan 8, 2021

Why not prevent vertical resize? Is there a guideline to support unnecessary vertical resize?

@hebasto
Copy link
Member Author

hebasto commented Jan 8, 2021 via email

@Sjors Sjors mentioned this pull request Jan 12, 2021
Merged
2 tasks
@laanwj
Copy link
Member

laanwj commented Jan 12, 2021

Concept ACK

promag
promag reviewed Jan 14, 2021
View reviewed changes
Copy link
Contributor

@promag promag left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested ACK d4feb68 on macos.

@maflcko maflcko merged commit 53bbbe5 into bitcoin-core:master Jan 21, 2021
maflcko pushed a commit to maflcko/bitcoin-core that referenced this pull request Jan 21, 2021
@hebasto
qt: Use layout manager for Create Wallet dialog
0dba346 
Github-Pull: bitcoin-core/gui#171
Rebased-From: d4feb68
@hebasto hebasto deleted the 210101-wallet branch January 21, 2021 18:00
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Jan 21, 2021
Merge bitcoin-core/gui#171: Use layout manager for Create Wallet dialog
a271d8c 
d4feb68 qt: Use layout manager for Create Wallet dialog (Hennadii Stepanov)

Pull request description:

  On master (e75f91e) not using layout manager causes problems with resizing:

  ![Screenshot from 2021-01-01 13-03-13](https://user-images.githubusercontent.com/32963518/103437728-ce1d4580-4c33-11eb-8915-1e9482775653.png)
  ![Screenshot from 2021-01-01 13-03-26](https://user-images.githubusercontent.com/32963518/103437730-d6758080-4c33-11eb-9e0f-87d0dd487fcb.png)

  Also text labels are not resized properly on some window managers (bitcoin#20777), or if their lengths are changed (after translation).

  This PR introduces a standard layout manager for the "Create Wallet" dialog that fixes all layout issues (actually, the `createwalletdialog.ui` has been re-written from scratch):

  ![Screenshot from 2021-01-01 13-10-03](https://user-images.githubusercontent.com/32963518/103437822-d0cc6a80-4c34-11eb-84fd-fcb10a16d9ef.png)
  ![Screenshot from 2021-01-06 23-50-36](https://user-images.githubusercontent.com/32963518/103823090-0b416780-507a-11eb-89dd-3f48a358e168.png)

  Additional visual changes:
  - advanced options are grouped in `QGroupBox` (bitcoin-core/gui#96 (comment))
  - enabled the [size grip](https://doc.qt.io/qt-5/qsizegrip.html#details)

  Fix bitcoin#20777

ACKs for top commit:
  jarolrod:
    ACK d4feb68
  Sjors:
    re-tACK d4feb68
  promag:
    Tested ACK d4feb68 on macos.

Tree-SHA512: 4c055962e49f88624900b880b33a866976d224628784593428b712d2e94563d77ddefddea3397134d20e72f738a8cf9aa885c1272fd9ffc90213c104435fb9f4
apoelstra pushed a commit to apoelstra/elements that referenced this pull request Sep 21, 2021
@hebasto @apoelstra
qt: Use layout manager for Create Wallet dialog
1721495 
Github-Pull: bitcoin-core/gui#171
Rebased-From: d4feb68
(cherry picked from commit 0dba346)
apoelstra pushed a commit to apoelstra/elements that referenced this pull request Sep 21, 2021
@hebasto @apoelstra
qt: Use layout manager for Create Wallet dialog
9a729f8 
Github-Pull: bitcoin-core/gui#171
Rebased-From: d4feb68
(cherry picked from commit 0dba346)
apoelstra pushed a commit to apoelstra/elements that referenced this pull request Sep 22, 2021
@hebasto @apoelstra
qt: Use layout manager for Create Wallet dialog
c89caab 
Github-Pull: bitcoin-core/gui#171
Rebased-From: d4feb68
(cherry picked from commit 0dba346)
@bitcoin-core bitcoin-core locked as resolved and limited conversation to collaborators Aug 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
2 more reviewers

@promag promag promag left review comments

@jarolrod jarolrod jarolrod left review comments

Reviewers whose approvals may not affect merge requirements
Assignees

@jonasschnelli jonasschnelli

Labels
None yet
Projects
None yet
Milestone
0.21.1
Development

Successfully merging this pull request may close these issues.

gui: file>create wallet dialog is unreadable
10 participants
@hebasto @Sjors @dooglus @maflcko @jonatack @luke-jr @jarolrod @promag @laanwj @jonasschnelli