Skip to content

Introduce Wizard control #106

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

Merged
merged 1 commit into from
May 28, 2022
Merged

Conversation

jarolrod
Copy link
Member

@jarolrod jarolrod commented Jan 5, 2022

Introduces the wizard control which will hold all of the onboarding views. A convenient demo can be found here: onboarding-wizard-demo

The demo shows the following views:
onboarding-flow

Windows
macOS
Android

@jarolrod jarolrod marked this pull request as draft January 5, 2022 15:45
@jarolrod jarolrod changed the title WIP: introduce and demo Wizard control Introduce and demo Wizard control Jan 6, 2022
@jarolrod jarolrod marked this pull request as ready for review January 6, 2022 03:13
@jarolrod
Copy link
Member Author

jarolrod commented Jan 6, 2022

updated from 35fad87 -> 3ea53bd

changes:

  • rebased over master
  • Update imports
  • Various tidy ups
  • Refactor finished() signal to property finished, check onFinishedChanged to see if we should push the node component

@hebasto
Copy link
Member

hebasto commented Jan 6, 2022

#103 has been merged, please rebase.

@jarolrod
Copy link
Member Author

jarolrod commented Jan 6, 2022

updated from 3ea53bd -> b93bb21

Changes:

  • rebased
  • included progress indicator in demo

@GBKS
Copy link
Contributor

GBKS commented Jan 10, 2022

This is great. Do you want feedback on this at the moment?

@jarolrod
Copy link
Member Author

updated from b93bb21 -> caf2ba2

Changes:

Repeater {
model: views.length
Loader {
source: "../pages/" + views[index]
Copy link
Contributor

Choose a reason for hiding this comment

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

2d84aa6

nit, bad indentation.

FWIW I don't recommend this approach of loading. It sounds very simple but has some drawbacks:

  • say page X is faulty, then those errors will only show when you reach X
  • the component instance will use the loader context when resolving property values, this prevents you from using required properties on each page
  • You have to stick to the filename convention, and later you have to rename some in order to add a new page in the middle

Instead, I suggest using loader sourceComponent property.

However, I think we can avoid the Loader, like this:

// Wizard.qml
Page {
    default property alias pages: swipeView.data
   
    contentItem: SwipeView {
    }
}

Then to implement some wizard:

Wizard {
    Page1 {
        someRequiredProperty: someWildExpression
    }
    Page2 {
    }
}

Feel free to discuss or ask for help in slack.

Copy link
Member Author

Choose a reason for hiding this comment

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

will leave this as a follow-up for now because, with this suggestion, the file in which we implement all the views will be huge. We probably need a template for each page of the onboarding views in order to keep it tight

active: wizard.currentIndex > 0 ? true : false
visible: active
sourceComponent: TextButton {
text: "‹ Back"
Copy link
Contributor

Choose a reason for hiding this comment

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

2d84aa6

add translation. The arrow should be an icon?

Copy link
Member Author

Choose a reason for hiding this comment

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

will keep for a follow-up until I plot out all the necessary types of buttons, and determine if this should be a custom component or not

@jarolrod jarolrod marked this pull request as draft February 6, 2022 09:29
hebasto added a commit that referenced this pull request Feb 9, 2022
563ee68 qml: introduce PageIndicator control (jarolrod)

Pull request description:

  Broken off from #106

  Introduces a PageIndicator control which lets the user know where they are within a Wizard or similar flow.

  Note: There is no current design file documentation of the specifics for the PageIndicator; therefore, the implementation provided here could be subject to change.

  A demo of the control can be found here: [pageindicator-demo](https://github.com/jarolrod/gui-qml/tree/pageindicator-demo)

  | Screenshot of PageIndicator |
  |-----------------------------|
  | <img width="95" alt="Screen Shot 2022-02-06 at 4 28 25 AM" src="https://user-images.githubusercontent.com/23396902/152674844-54ad0116-3e07-47ac-82a9-07a932f6513b.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/113)
  [![macOS](https://img.shields.io/badge/OS-macOS-green)](https://api.cirrus-ci.com/v1/artifact/github/bitcoin-core/gui-qml/macos/insecure_mac_gui.zip?branch=pull/113)
  [![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/113)

ACKs for top commit:
  hebasto:
    ACK 563ee68

Tree-SHA512: 2f9a16badd496a9c852a5fd8e930751b4e4bea7a7efba1aad15ba9f47d117616dc0d48b92adfafb6722be42cb5ab6e59facd2e79797f9ae85445f0c5f1af0abb
@jarolrod jarolrod force-pushed the onboarding-wizard branch from caf2ba2 to 50b9573 Compare May 25, 2022 03:33
@jarolrod jarolrod marked this pull request as ready for review May 25, 2022 03:34
@jarolrod
Copy link
Member Author

updated from caf2ba2 to 50b9573

changes:

  • incorporate most of @promag suggestions
  • split out views from this change, to be opened in a future PR

@jarolrod jarolrod changed the title Introduce and demo Wizard control Introduce Wizard control May 25, 2022
@hebasto
Copy link
Member

hebasto commented May 28, 2022

This PR needs to be rebased.

Co-authored-by: João Barbosa <joao.paulo.barbosa@gmail.com>
@jarolrod jarolrod force-pushed the onboarding-wizard branch from 50b9573 to dffb73e Compare May 28, 2022 17:19
@jarolrod
Copy link
Member Author

Updated from 50b9573 to dffb73e, also updated the onboarding-wizard-demo for convenient testing

Changes:

  • rebased over master

@shaavan
Copy link
Contributor

shaavan commented May 28, 2022

Amazing work, @jarolrod!
I shall be posting my review soon.

For now, I would like to bring attention to the problem I noticed, which needs to be addressed.

I experimented with different window sizes, and I noticed that the text from the onboarding 03a page is overflowing onto the 03b page for windows with small vertical height.

onboarding 03b page
Screenshot from 2022-05-28 23-38-31

I thought its best to bring it to the attention before moving forward with reviewing.

@jarolrod
Copy link
Member Author

jarolrod commented May 28, 2022

@shaavan that's just the demo, the window sizes aren't complete on the rudimentary demo and those pages aren't perfect. Just meant to show that the wizard control works and works for all the use cases required, like subpages.

@shaavan
Copy link
Contributor

shaavan commented May 28, 2022

Got it! I reckoned that this might be the case, as the pages are in the prototype phase. But I still thought to bring it to the notice, just in case this was an unintended issue.

Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

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

ACK dffb73e

@hebasto hebasto merged commit 2a7c1b1 into bitcoin-core:main May 28, 2022
hebasto added a commit that referenced this pull request Dec 14, 2022
…ow have descriptive names

6c29ddf qml: remove wizard control (Jarol Rodriguez)
8619e50 qml: do not load onboarding pages from url (Jarol Rodriguez)
5120735 qml: remove unused inSubPage bool (Jarol Rodriguez)
9cc991f qml: give onboarding views descriptive names (Jarol Rodriguez)

Pull request description:

  This is addressing the [desire for descriptive names](#149 (comment)) for the onboarding views as well as (partially) addressing an old piece of feedback from **promag** [about loading pages](#106 (comment)).

  In regards to the second issue this is addressing, the current state of the PR doesn't perfectly address this as there doesn't seem to be a way of given a SwipeView children after it's already been declared. So this PR just doesn't use the Wizard control anymore (which already has limited value and is vulnerable to removal), and deletes it as well.

  ## Desktop

  ### Master

  | master-1 | master-2 | master-3 | master-4 | master-5 |
  | -------- | -------- | -------- | -------- | -------- |
  | <img width="708" alt="master-1" src="https://user-images.githubusercontent.com/23396902/206313556-a2f9b720-368b-41c5-bc13-bb695c83c807.png"> | <img width="752" alt="master-2" src="https://user-images.githubusercontent.com/23396902/206313569-9a90d545-a243-484e-975d-87b5192e7637.png"> | <img width="752" alt="master-3" src="https://user-images.githubusercontent.com/23396902/206313586-fa967658-cb38-47cb-8c25-1d1cc79e244b.png"> | <img width="752" alt="master-4" src="https://user-images.githubusercontent.com/23396902/206313603-da9849eb-8f40-4bdd-a231-de7843e8bd6f.png"> | <img width="752" alt="master-5" src="https://user-images.githubusercontent.com/23396902/206313630-7429fc73-4505-4888-9a00-f03ac7b8a66f.png"> |

  | master-6 | master-7 | master-8 | master-9 | master-10 |
  | -------- | -------- | -------- | -------- | --------- |
  | <img width="752" alt="master-6" src="https://user-images.githubusercontent.com/23396902/206313704-99e33b2a-a7b5-498b-9ae1-b4b77aa343a4.png"> | <img width="752" alt="master-7" src="https://user-images.githubusercontent.com/23396902/206313719-2f5efacf-85c1-436a-8782-0fad0c9d7193.png"> | <img width="752" alt="master-8" src="https://user-images.githubusercontent.com/23396902/206313739-1f02173a-0fac-45ba-af37-338c18e765f7.png"> | <img width="752" alt="master-9" src="https://user-images.githubusercontent.com/23396902/206313756-79072fc9-5af1-43be-b6f3-99dcb402f98f.png"> | <img width="752" alt="master-10" src="https://user-images.githubusercontent.com/23396902/206313767-33b8a2ad-eafc-47b3-a0eb-8a578a55597f.png"> |

  ### PR

  | pr-1 | pr-2 | pr-3 | pr-4 | pr-5 |
  | ---- | ---- | ---- | ---- | ---- |
  | <img width="708" alt="pr-1" src="https://user-images.githubusercontent.com/23396902/206323540-7ad151f7-574c-4ed5-ae1e-8668ded70c50.png"> | <img width="752" alt="pr-2" src="https://user-images.githubusercontent.com/23396902/206323554-6d39bdda-5b30-4653-914c-59b3d03d2e2e.png"> | <img width="752" alt="pr-3" src="https://user-images.githubusercontent.com/23396902/206323568-b80db1f1-8ee0-4df2-b86a-b45fa1300c86.png"> | <img width="752" alt="pr-4" src="https://user-images.githubusercontent.com/23396902/206323585-175a8a5e-814a-4260-8ce3-63af9da4dbbb.png"> | <img width="752" alt="pr-5" src="https://user-images.githubusercontent.com/23396902/206323601-43606a1f-83c5-44d6-b891-231951bba47b.png"> |

  | pr-6 | pr-7 | pr-8 | pr-9 | pr-10 |
  | ---- | ---- | ---- | ---- | ----- |
  | <img width="752" alt="pr-6" src="https://user-images.githubusercontent.com/23396902/206323654-d96852e3-0879-40aa-92a3-8a324107d1b5.png"> | <img width="752" alt="pr-7" src="https://user-images.githubusercontent.com/23396902/206323666-4aaa68d9-2e60-45d6-98ae-e580125c55b3.png"> | <img width="752" alt="pr-8" src="https://user-images.githubusercontent.com/23396902/206323683-26bbfdca-a692-41cd-869e-1b95654ff80f.png"> | <img width="752" alt="pr-9" src="https://user-images.githubusercontent.com/23396902/206323695-c2b4bc04-1d4a-4096-9081-1392b923458e.png"> | <img width="752" alt="pr-10" src="https://user-images.githubusercontent.com/23396902/206323710-8ace360d-0d62-4597-9963-6e1df3ca6216.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/198)
  [![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/198)
  [![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/198)
  [![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/198)

ACKs for top commit:
  johnny9:
    ACK 6c29ddf

Tree-SHA512: bacd3419deb0c68652eb0c470fd72f61e424c6872ff17f3bf7adf2848191b42e69627e79b75734b11fa917c3e2fd79288b764bfd4cf318f4711ebaea317b8734
hebasto pushed a commit to hebasto/gui-qml that referenced this pull request Jun 7, 2025
Github-Pull: bitcoin-core#106
Rebased-From: dffb73e

Co-authored-by: João Barbosa <joao.paulo.barbosa@gmail.com>
hebasto pushed a commit to hebasto/gui-qml that referenced this pull request Jun 8, 2025
Github-Pull: bitcoin-core#106
Rebased-From: dffb73e

Co-authored-by: João Barbosa <joao.paulo.barbosa@gmail.com>
hebasto pushed a commit to hebasto/gui-qml that referenced this pull request Jun 8, 2025
Github-Pull: bitcoin-core#106
Rebased-From: dffb73e

Co-authored-by: João Barbosa <joao.paulo.barbosa@gmail.com>
hebasto pushed a commit to hebasto/gui-qml that referenced this pull request Jun 9, 2025
Github-Pull: bitcoin-core#106
Rebased-From: dffb73e

Co-authored-by: João Barbosa <joao.paulo.barbosa@gmail.com>
hebasto pushed a commit to hebasto/gui-qml that referenced this pull request Jun 9, 2025
Github-Pull: bitcoin-core#106
Rebased-From: dffb73e

Co-authored-by: João Barbosa <joao.paulo.barbosa@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants