Skip to content

Constrain Options views to 450 pixels plus padding #185

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
Nov 8, 2022
Merged

Constrain Options views to 450 pixels plus padding #185

merged 1 commit into from
Nov 8, 2022

Conversation

johnny9
Copy link
Collaborator

@johnny9 johnny9 commented Oct 28, 2022

No description provided.

@johnny9
Copy link
Collaborator Author

johnny9 commented Oct 28, 2022 •
edited
Loading

PR

aboutoptions connectionsettings developer_options storagesettings

jarolrod
jarolrod reviewed Nov 8, 2022
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.

Master

Desktop

About Developer Options Storage Settings Connection Settings
Screen Shot 2022-11-07 at 7 25 46 PM Screen Shot 2022-11-07 at 7 25 54 PM Screen Shot 2022-11-07 at 7 26 07 PM Screen Shot 2022-11-07 at 7 26 15 PM

Android Phone

Portrait
About Developer Options Storage Settings Connection Settings
Screenshot_20221107-170729 Screenshot_20221107-170733 Screenshot_20221107-170752 Screenshot_20221107-170922
Horizontal
About Developer Options
Screenshot_20221107-170837 Screenshot_20221107-170846
Storage Settings Connection Settings
Screenshot_20221107-170819 Screenshot_20221107-170931

Android Tablet

Portrait
About Developer Options Storage Settings Connection Settings
Screenshot_20221107_173553 Screenshot_20221107_173614 Screenshot_20221107_173656 Screenshot_20221107_173718
Horizontal
About Developer Options
Screenshot_20221107_173606 Screenshot_20221107_173624
Storage Settings Connection Settings
Screenshot_20221107_173704 Screenshot_20221107_173726

Pull Request

Desktop

About Developer Options Storage Settings Connection Settings
Screen Shot 2022-11-07 at 7 31 48 PM Screen Shot 2022-11-07 at 7 32 05 PM Screen Shot 2022-11-07 at 7 32 15 PM Screen Shot 2022-11-07 at 7 32 21 PM

Android Phone

Portrait
About Developer Options Storage Settings Connection Settings
Screenshot_20221107-171809 Screenshot_20221107-171829 Screenshot_20221107-171902 Screenshot_20221107-171920
Horizontal
About Developer Options
Screenshot_20221107-171818 Screenshot_20221107-171836
Storage Settings Connection Settings
Screenshot_20221107-171909 Screenshot_20221107-171927

Android Tablet

Portrait
About Developer Options Storage Settings Connection Settings
Screenshot_20221107_173934 Screenshot_20221107_173953 Screenshot_20221107_174030 Screenshot_20221107_174038
Horizontal
About Developer Options
Screenshot_20221107_173944 Screenshot_20221107_174000
Storage Settings Connection Settings
Screenshot_20221107_174022 Screenshot_20221107_174045

jarolrod
jarolrod approved these changes Nov 8, 2022
View reviewed changes
Copy link
Member

@jarolrod jarolrod left a comment

Choose a reason for hiding this comment

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

ACK 595a57d

Confirming this fixes the issue, this is a nice simple fix.

@hebasto hebasto merged commit 2067d94 into bitcoin-core:main Nov 8, 2022
@jarolrod jarolrod mentioned this pull request Nov 23, 2022
Merged
hebasto added a commit that referenced this pull request Dec 8, 2022
@hebasto
Merge #192: Use OnboardingInfo for all onboarding pages
7519f7f 
b954e9f qml: use the OnboardingInfo control for onboarding settings views (jarolrod)
325099d qml: make continueButton text optional in the OnboardingInfo control (jarolrod)

Pull request description:

  This PR reworks the `OnboardingInfo` control so that we can use it for *all* onboarding pages.

  The `OnboardingInfo` control doesn't work for the onboarding pages which represent the advanced settings views because the `OnboardingInfo` control always shows the continue button, which the advanced settings views do not use. We get around this here by making the buttonText value optional, making it so that the ContinueButton is not visible or enabled when the continueText value is not specified.

  Additionally, this now changes the way that we fix the issue shown in #185. Instead of setting a maxwidth or 490 for the content, we give the `Setting` control an implicitWidth of 450. This is necessary in order to accommodate this change as we will no longer have an independent Header control which stretches out the width.

  ## Desktop
  ### master

  | a | b | c | d |
  | - | - | - | - |
  | <img width="752" alt="Screen Shot 2022-11-23 at 1 40 14 AM" src="https://user-images.githubusercontent.com/23396902/203560540-8766703f-f214-4d0c-bc07-27ae1fd244c8.png"> | <img width="752" alt="Screen Shot 2022-11-23 at 1 40 26 AM" src="https://user-images.githubusercontent.com/23396902/203560559-a58ff346-11be-4da4-8dbd-8f08adae7a7b.png"> | <img width="752" alt="Screen Shot 2022-11-23 at 1 40 40 AM" src="https://user-images.githubusercontent.com/23396902/203560582-582f404f-2c47-4609-9ac8-9b71c44567ef.png"> | <img width="752" alt="Screen Shot 2022-11-23 at 1 40 52 AM" src="https://user-images.githubusercontent.com/23396902/203560606-b04738c7-3e1a-4e77-a612-524bf85e58c1.png"> |

  ### pr

  | a | b | c | d |
  | - | - | - | - |
  | <img width="752" alt="Screen Shot 2022-11-23 at 2 56 48 AM" src="https://user-images.githubusercontent.com/23396902/203560246-fd2137f3-8172-4fdb-9df9-fc7278777303.png"> | <img width="752" alt="Screen Shot 2022-11-23 at 2 56 55 AM" src="https://user-images.githubusercontent.com/23396902/203560306-3c4b064a-0054-4e0f-bfab-bddba5496a27.png"> | <img width="752" alt="Screen Shot 2022-11-23 at 2 57 10 AM" src="https://user-images.githubusercontent.com/23396902/203560333-0c962a39-9a52-4388-acdd-95ee517401db.png"> | <img width="752" alt="Screen Shot 2022-11-23 at 2 57 21 AM" src="https://user-images.githubusercontent.com/23396902/203560402-f54cb352-89df-4f33-8bc4-46021efcc52c.png"> |

  [![Windows](https://img.shields.io/badge/OS-Windows-green)](https://api.cirrus-ci.com/v1/artifact/github/bitcoin-core/gui-qml/win64/insecure_win_gui.zip?branch=pull/192)
  [![Intel macOS](https://img.shields.io/badge/OS-Intel%20macOS-green)](https://api.cirrus-ci.com/v1/artifact/github/bitcoin-core/gui-qml/macos/insecure_mac_gui.zip?branch=pull/192)
  [![Apple Silicon macOS](https://img.shields.io/badge/OS-Apple%20Silicon%20macOS-green)](https://api.cirrus-ci.com/v1/artifact/github/bitcoin-core/gui-qml/macos_arm64/insecure_mac_arm64_gui.zip?branch=pull/192)
  [![ARM64 Android](https://img.shields.io/badge/OS-Android-green)](https://api.cirrus-ci.com/v1/artifact/github/bitcoin-core/gui-qml/android/insecure_android_apk.zip?branch=pull/192)

ACKs for top commit:
  johnny9:
    ACK b954e9f

Tree-SHA512: d82bb4f20bfe65cfb45fb19b0cbbcd16f31500df0ca3a132c473245677a6b66264a21a756abbc89589cac610fd7e075813637ffa6a659b8729735d341d5805bd
hebasto pushed a commit to hebasto/gui-qml that referenced this pull request Jun 8, 2025
@hebasto
qml: constrain Options views to 450 pixels plus padding
ecadba7 
Github-Pull: bitcoin-core#185
Rebased-From: 595a57d
hebasto pushed a commit to hebasto/gui-qml that referenced this pull request Jun 9, 2025
@hebasto
qml: constrain Options views to 450 pixels plus padding
b8e5148 
Github-Pull: bitcoin-core#185
Rebased-From: 595a57d
hebasto pushed a commit to hebasto/gui-qml that referenced this pull request Jun 9, 2025
@hebasto
qml: constrain Options views to 450 pixels plus padding
b187cc4 
Github-Pull: bitcoin-core#185
Rebased-From: 595a57d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jarolrod jarolrod jarolrod approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@johnny9 @jarolrod @hebasto