Skip to content
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.

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

Allow BlockClock to scale and maintain 1/3 of the available window real estate #306

Closed
wants to merge 5 commits into from

Conversation

jarolrod
Copy link
Member

@jarolrod jarolrod commented Mar 29, 2023

Follow up can allow to customize this scaling in settings.

small window size

master pr
master-1 Screen Shot 2023-04-20 at 2 55 23 PM

large window size

master pr
master-2 Screen Shot 2023-04-20 at 2 55 28 PM

Windows
Intel macOS
Apple Silicon macOS
ARM64 Android

@@ -28,7 +30,9 @@ Item {

BlockClockDial {
id: dial
anchors.fill: parent
width: Math.min((root.parentWidth / 3), (root.parentHeight / 3))
Copy link
Member Author

Choose a reason for hiding this comment

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

Allowing for different scale factors is as easy as changing the number 3 here

@johnny9
Copy link
Contributor

johnny9 commented Mar 29, 2023

Concept ACK

This is great, glad to see that just increasing the icon size resolves the icon scaling problem. My preference would be 2/3 of the width/height but i'll leave that to the designers to decide on a default.

Copy link
Contributor

@johnny9 johnny9 left a comment

Choose a reason for hiding this comment

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

I think the scaling logic for the dial looks great. I'm not sure about the network indicator scaling and having the two scale off of the their parent's parent width/height. It breaks the encapsulation.

@@ -12,9 +12,11 @@ import "../controls"

Item {
id: root
property real parentWidth: 600
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think these properties are needed. You can reference the parent with root.parent.width and root.parent.height.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think its better for clarity, and this in a small way documents what the proportions are based on by default - coming from the design file

id: networkIndicator
property int topMargin: dial.width * (3/20)
anchors.top: dial.bottom
textSize: dial.width * (3/40)
Copy link
Contributor

Choose a reason for hiding this comment

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

I understand why you did it here to match the original design, but I think making the size of this component depend on the size of the dial creates the wierd logic with having to know more about the parents layout than it should.

Greetings. I am pleased to inform you that the BlockClock component and
the Network Indicator component are merging in a most harmonious and
efficient manner. This event brings me great joy and excitement, as it
represents a significant milestone in the world of Bitcoin.

BlockClock, you have been diligent in displaying the last 12 hours of
blocks mined on the Bitcoin blockchain, consistently providing valuable
information to users. Network Indicator, your expertise in indicating
the current network - main, test, or signet - has been indispensable.

As I observe this union within the BlockClock.qml file, I cannot help
but feel a sense of pride and satisfaction. This integration will
undoubtedly lead to enhanced functionality and streamlined user
experience.

I offer my sincerest congratulations to BlockClock and
Network Indicator on their momentous union. May your combined efforts
bring about continued success and increased stability in the Bitcoin
ecosystem.

To the moon, my fellow components. To the moon.
This fixes an issue with the icon on very HDPI screens when resizing
the block clock.
@jarolrod jarolrod changed the title Allow BlockClock to scale and maintain 1/3 of the available window real estate Allow BlockClock to scale and maintain 1/2 of the available window real estate Mar 31, 2023
@jarolrod
Copy link
Member Author

Updated from 87b9f59 to 71a1d99, compare

Changes:

  • block clock takes up half the space
  • Network Indicator doesn't scale, stays fixed size

@jarolrod
Copy link
Member Author

Updated from 71a1d99 to d138d1c, compare

Changes: block clock is now 5/12 in size by default, this is halfway between 1/2 and 1/3.

@jarolrod jarolrod changed the title Allow BlockClock to scale and maintain 1/2 of the available window real estate Allow BlockClock to scale and maintain 5/12 of the available window real estate Mar 31, 2023
@hebasto hebasto added the UX Designers' opinions are required label Apr 17, 2023
@jarolrod jarolrod added Needs design review Designer's review needed Design suggestion Makes a suggestion on design labels Apr 17, 2023
Copy link
Contributor

@johnny9 johnny9 left a comment

Choose a reason for hiding this comment

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

ACK d138d1c

@jarolrod jarolrod changed the title Allow BlockClock to scale and maintain 5/12 of the available window real estate Allow BlockClock to scale and maintain 1/3 of the available window real estate Apr 20, 2023
@jarolrod
Copy link
Member Author

Updated from d138d1c to cf6e327, compare

Changes: reverted back to using a scale factor of 1/3. We can fine tune this later

@thebagmaster
Copy link

thebagmaster commented Apr 20, 2023

ACK cf6e327
seems to scale fine:
1
2

@jarolrod
Copy link
Member Author

Closing this in favor of keeping everything in #311

@jarolrod jarolrod closed this Apr 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Design suggestion Makes a suggestion on design Needs design review Designer's review needed UX Designers' opinions are required
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants