-
Notifications
You must be signed in to change notification settings - Fork 50
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
Conversation
35fad87
to
3ea53bd
Compare
#103 has been merged, please rebase. |
3ea53bd
to
b93bb21
Compare
This is great. Do you want feedback on this at the moment? |
b93bb21
to
caf2ba2
Compare
updated from b93bb21 -> caf2ba2 Changes:
|
src/qml/controls/Wizard.qml
Outdated
Repeater { | ||
model: views.length | ||
Loader { | ||
source: "../pages/" + views[index] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
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" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add translation. The arrow should be an icon?
There was a problem hiding this comment.
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
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"> | [](https://api.cirrus-ci.com/v1/artifact/github/bitcoin-core/gui-qml/win64/insecure_win_gui.zip?branch=pull/113) [](https://api.cirrus-ci.com/v1/artifact/github/bitcoin-core/gui-qml/macos/insecure_mac_gui.zip?branch=pull/113) [](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
caf2ba2
to
50b9573
Compare
This PR needs to be rebased. |
Co-authored-by: João Barbosa <joao.paulo.barbosa@gmail.com>
50b9573
to
dffb73e
Compare
Updated from 50b9573 to dffb73e, also updated the onboarding-wizard-demo for convenient testing Changes:
|
Amazing work, @jarolrod! 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. I thought its best to bring it to the attention before moving forward with reviewing. |
@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. |
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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK dffb73e
…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"> | [](https://api.cirrus-ci.com/v1/artifact/github/bitcoin-core/gui-qml/win64/insecure_win_gui.zip?branch=pull/198) [](https://api.cirrus-ci.com/v1/artifact/github/bitcoin-core/gui-qml/macos/insecure_mac_gui.zip?branch=pull/198) [](https://api.cirrus-ci.com/v1/artifact/github/bitcoin-core/gui-qml/macos_arm64/insecure_mac_arm64_gui.zip?branch=pull/198) [](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
Github-Pull: bitcoin-core#106 Rebased-From: dffb73e Co-authored-by: João Barbosa <joao.paulo.barbosa@gmail.com>
Github-Pull: bitcoin-core#106 Rebased-From: dffb73e Co-authored-by: João Barbosa <joao.paulo.barbosa@gmail.com>
Github-Pull: bitcoin-core#106 Rebased-From: dffb73e Co-authored-by: João Barbosa <joao.paulo.barbosa@gmail.com>
Github-Pull: bitcoin-core#106 Rebased-From: dffb73e Co-authored-by: João Barbosa <joao.paulo.barbosa@gmail.com>
Github-Pull: bitcoin-core#106 Rebased-From: dffb73e Co-authored-by: João Barbosa <joao.paulo.barbosa@gmail.com>
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:
