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

Introduce Display settings #311

Merged
merged 12 commits into from
May 31, 2023
Merged

Conversation

jarolrod
Copy link
Member

@jarolrod jarolrod commented Mar 31, 2023

This moves "Display" related settings into a new "Display" page. Also sets up QSettings wiring for options.

Display Settings Theme Block clock
Screen Shot 2023-05-09 at 9 48 38 PM Screen Shot 2023-04-23 at 2 13 16 AM Screen Shot 2023-04-23 at 2 13 19 AM

Windows
Intel macOS
Apple Silicon macOS
ARM64 Android

@jarolrod jarolrod changed the title Display setting Introduce Display settings Mar 31, 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.

Concept ACK. I like the functionality here. Noticed some minor things when reviewing.

src/qml/components/ThemeSettings.qml Outdated Show resolved Hide resolved
src/qml/components/BlockClockSizeSettings.qml Outdated Show resolved Hide resolved
src/qml/components/BlockClockSizeSettings.qml Outdated Show resolved Hide resolved
src/qml/components/ThemeSettings.qml Outdated Show resolved Hide resolved
src/qml/components/blockclockdial.cpp Show resolved Hide resolved
src/qml/pages/settings/SettingsBlockClockSize.qml Outdated Show resolved Hide resolved
src/qml/pages/settings/SettingsDisplay.qml Outdated Show resolved Hide resolved
@GBKS
Copy link
Contributor

GBKS commented Apr 12, 2023

I have to say, this doesn't feel right. Having to go several screens into settings to adjust the size only having to go back to see what it looks like is just not right. And the big one is so huge on desktop that it looks a bit crazy.

Could we also toggle through the sizes via a simple double-tap on the clock? It does require that the UI can differentiate between single and double-taps, not sure if that's built-on or some custom functionality.

Here's a quick mock-up with some ideas (icons are placeholders). I reframed this as two display modes. One is for active use, the other is called "Showcase". Each has an explainer. For "Showcase", there might be other things we may want to tweak, like hiding the Settings button until you interact with the screen. It could also be cool if the big one renders with more detail, more animations or other unique treatments rather than just being bigger.

image

@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
@jarolrod
Copy link
Member Author

jarolrod commented Apr 18, 2023

Updated from 7626d1f to fd00969, compare

Changes: addressed review feedback

@GBKS I've integrated your suggestion on the "Block Clock display mode" page. But, I've left out the icon until we get an official svg for it (see pictures in PR description). I've also left out the text that you can double tap on the clock; that will be introduced in the followup that implements double tap detection. I've also left out the last sentence in the Showcase option until that functionality is actually introduced.

The values for default and showtime could use review.

@jarolrod
Copy link
Member Author

Updated from from fd00969 to 753d052

Changes: default is now 1/3 as declared by the based on PR, and display is now 1/2. We can fine tune these values later

@johnny9
Copy link
Contributor

johnny9 commented Apr 21, 2023

ACK 753d052

@GBKS
Copy link
Contributor

GBKS commented Apr 21, 2023

Tested and looks good. I slightly tweaked the icons and made them export ready, find them in Figma here.

image

What do you think of changing "Default" to "Compact"?

Could you also please put the page title in the title bar rather than in the page itself?

And another question about casing. I always use sentence case in design, but I see a good amount of title case in the application. Let's decide on something (obviously I'd go for sentence case). What's your take?

1/2 still feels a tad OP, but let's run with it and see if we want to tweak it after a bit of usage.

@jarolrod
Copy link
Member Author

Updated from from 753d052 to b59654c, compare

Changes:

  • update to new centered bitcoin icon
  • introduced and use block clock display mode icons

@jarolrod
Copy link
Member Author

@GBKS as shown in the new PR description images, should the Display settings page itself have a title bar?

@GBKS
Copy link
Contributor

GBKS commented Apr 28, 2023

Yes sir, just "Settings".

@jarolrod jarolrod force-pushed the display-setting branch 2 times, most recently from b59654c to 410b135 Compare May 10, 2023 01:00
@jarolrod
Copy link
Member Author

Updated b59654c to 410b135

changes: rebased over main

@jarolrod
Copy link
Member Author

Updated from 410b135 to b0f7265

changes: added "Settings" title to the display settings page

@jarolrod
Copy link
Member Author

cc @GBKS @mouxdesign CI artifacts are built and this can be tested now

@jarolrod jarolrod added this to the v1.0 milestone May 29, 2023
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.
@jarolrod
Copy link
Member Author

Updated from b0f7265 to af0ddf6, rebased over main

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 af0ddf6

Updates look good, no qml errors.

@hebasto hebasto merged commit 80ee578 into bitcoin-core:main May 31, 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.

None yet

4 participants