Introduce the StorageOptions component and TextButton control #102

jarolrod · 2022-01-04T17:29:41Z

This introduces the StorageOptions component which represents the storage options page as designed in the main design file.

This component reuses the OptionButton control by making modifications to it. We refactor the OptionButton control to reuse the Header control instead of specifying separate labels. This simplifies the control itself.

The demo of this component can be found here: storageoptions-demo. In the demo, theStorageOptions component is added to our stack. Now that there are several elements in the stack, we need some navigation. Therefore, this also introduces the TextButton control which represents text based buttons such as the "back" navigation button and the "detailed settings" button. This added TextButton control is then given the text "< Back" and added to the header of our ApplicationWindow. When clicked, this button will rewind the stack. Only visible when the currentIndex of the stack is < 0.

Images of added elements

StorageOptions	Back Button

promag · 2022-01-05T01:14:32Z

@jarolrod please take a look at #103 for an alternative to the option button change.

jarolrod · 2022-01-06T02:04:33Z

updated from 4c1a5b8 -> e98b0e5

Changes:

Use inline item change from Support detail item inside option button #103 instead of altering it here
various tidy-ups

jarolrod · 2022-01-06T15:39:59Z

Updated from e98b0e5 -> a57fbb0:

Changes:

rebased over main

GBKS · 2022-01-10T13:32:11Z

Looks great. Could we also introduce an "Icon text button", so we don't need to rely on text characters for the caret on the Back button?

This simplifies the OptionButton control.

jarolrod · 2022-01-11T04:39:44Z

Updated from a57fbb0 -> f440d82

@GBKS

Looks great. Could we also introduce an "Icon text button", so we don't need to rely on text characters for the caret on the Back button?

Sure! We'll leave this for a follow-up.

Changes:

rebased over main to drop dependency on Add and demo ConnectionSettings and ContinueButton components #101
moved demo to: storageoptions-demo

hebasto · 2022-01-11T11:37:45Z

What workflow is supposed for a user to interact with StorageOptions?
Why description and progress are hardcoded?

shaavan

ACK f440d82

Going commit wise:

The first commit uses the Header component introduced in Add Header Control #100 in the OptionButton control. It makes sense because we want to reuse components efficiently and avoid redundancy.
The second commit introduces the StorageOptions component, which uses two OptionButton (grouped) to display the contents of its page.
The application of the OptionButton LGTM. However, we need to eliminate the use of hardcoded ProgressIndicator. We can experiment with using GetFreeDiskSpace (see this article). Still, we can do so in a c++ implementation-specific follow-up PR.
The third commit introduces TextButton controls, which model a reusable component based on the Back button. The implementation looks good to me.
Taking @GBKS this suggestion, I think we can modify this control file to incorporate an optional icon loader that displays the icon if its location is provided. I think I can give it a shot as a follow-up to this PR.

shaavan

I realize this is the early stage for this page and the repository. Still, I would like to give my humble review on the design side of the StorageOption component. I would request @GBKS to take a look at this, and help improve this review and correct me where my analysis is erroneous.

Tested using @Jarol storage-options-demo branch on a 13 inch screen with 16:9 aspect ratio, 1920*1080 resolution.

Let us go component-wise:

Back Button:
- The button looks a little too bold and heavy and captures a lot more attention than needed. We can experiment with decreasing its font heaviness and maybe the size.
Bitcoin Logo:
- Two small compared to the text. We have to increase its size.
- Logo and main heading are pretty far apart. We have to decrease it so that it looks more natural.
Heading description:
- It is too large and has a confusing visual hierarchy with the main heading. We can experiment with decreasing its size as well as weight.
Subtext:
- It is too small in size. I had to strain my eyes a bit to read it. It can use a size increase.
Progress bar:
- The text and bar should be closer and a little farther from the button below them. This allows for proper visual grouping and hence creates a better viewing experience.
Button:
- The text in the button, especially the one with the recommended text, looks a bit cramped up. We can experiment with reducing the size of the text in the button.
Page indicator (the three dots at the bottom of page):
- The color is a little too transparent. Personally, I think they should be a little brighter and more opaque to make them peripherally visible to the user while also not getting too obvious and visually demanding.

I hope my observation will help make this PR better. If any of my observations are erroneous, I humbly welcome you to correct me. Thank you.

jarolrod · 2022-01-11T18:29:45Z

@shaavan
In regards to subtext, heading description and recommended button: They are implemented as specified in the main design file. Any suggestions for designs should be opened as an issue in the BitcoinDesign/Bitcoin-Core-App repo. In this repo we aim to implement the designs.

In regards to the bitcoin logo, progress bar, and page indicator: What is shown in the demo branch is just that; a demo of the components. In introducing a StorageOptions component there's no reason for me to change the position and size of unrelated components in the stub file.

Take a look at #106 where a lot of your concerns would be addressed. The demo here is just to demo the StorageOptions and TextButton elements, nothing else.

shaavan · 2022-01-11T19:09:03Z

Any suggestions for designs should be opened as an issue in the BitcoinDesign/Bitcoin-Core-App repo. In this repo we aim to implement the designs.

Got it! I shall take care of this for my future reviews.

Take a look at #106 where a lot of your concerns would be addressed.

I shall take a look at #106.

GBKS · 2022-01-13T15:49:25Z

@shaavan thanks for the detailed feedback. Are you comfortable using Figma? If so, you are very welcome to duplicate the Figma file and apply your recommendations. That approach would require less interpretation and back and forth. But let me know if that's something you do not want to get into and we'll find a different way. Also, feel free to leave comments directly in Figma.

Generally, we try to finalize everything in design (Figma) first, and then replicate it 1:1 in code (as much as possible). It can happen very easily that design and code get out of sync, which makes it really hard for any non-developer to fully understand what the user experience is like, comment on certain areas, propose improvements, etc. Design is a great planning tool and can save a lot of front-end implementation time.

Also, one more thing to consider in design reviews. These designs are optimized for mobile. Most likely, buttons and text will very slightly bigger than normal desktop interfaces (fingers are less precise than a mouse cursor, requiring larger tap areas).

Keep it coming.

shaavan · 2022-01-14T08:48:47Z

@GBKS

I am pretty comfortable using Figma. Let me experiment with some design changes to make it look good on a Desktop screen.

Generally, we try to finalize everything in design (Figma) first, and then replicate it 1:1 in code (as much as possible).

I agree. It's better to plan the architecture before starting putting bricks.

These designs are optimized for mobile. Most likely, buttons and text will very slightly bigger than normal desktop interfaces (fingers are less precise than a mouse cursor, requiring larger tap areas).

Makes sense. However, it would be awesome if we could go with a responsive design for the app that adjusts the design based on the screen resolution and size. But that could come later in the project's journey.