Skip to content

Do not dynamically load OnboardingViews from url, which now have descriptive names#198

Merged
hebasto merged 4 commits into
bitcoin-core:mainfrom
jarolrod:wizard-page-instantiation
Dec 14, 2022
Merged

Do not dynamically load OnboardingViews from url, which now have descriptive names#198
hebasto merged 4 commits into
bitcoin-core:mainfrom
jarolrod:wizard-page-instantiation

Conversation

@jarolrod
Copy link
Copy Markdown
Contributor

@jarolrod jarolrod commented Dec 7, 2022 •
edited
Loading

This is addressing the desire for descriptive names for the onboarding views as well as (partially) addressing an old piece of feedback from promag about loading pages.

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
master-1 master-2 master-3 master-4 master-5
master-6 master-7 master-8 master-9 master-10
master-6 master-7 master-8 master-9 master-10

PR

pr-1 pr-2 pr-3 pr-4 pr-5
pr-1 pr-2 pr-3 pr-4 pr-5
pr-6 pr-7 pr-8 pr-9 pr-10
pr-6 pr-7 pr-8 pr-9 pr-10

Windows
Intel macOS
Apple Silicon macOS
ARM64 Android

This was referenced Dec 7, 2022
Closed
Merged
@jarolrod jarolrod force-pushed the wizard-page-instantiation branch from bfc2bbc to 15012e7 Compare December 8, 2022 18:14
@jarolrod
Copy link
Copy Markdown
Contributor Author

jarolrod commented Dec 8, 2022

updated from bfc2bbc to 15012e7

changes: rebased over master

@hebasto
Copy link
Copy Markdown
Member

hebasto commented Dec 9, 2022

based on #197

It has just been merged.

changes: rebased over master

One more time? 😄

@jarolrod jarolrod force-pushed the wizard-page-instantiation branch from 15012e7 to 3d3b62e Compare December 9, 2022 17:18
@jarolrod
Copy link
Copy Markdown
Contributor Author

jarolrod commented Dec 9, 2022

updated from 15012e7 to 3d3b62e

changes: rebase over master

@jarolrod jarolrod mentioned this pull request Dec 10, 2022
Merged
@hebasto
Copy link
Copy Markdown
Member

hebasto commented Dec 10, 2022

Needs rebase again.

@jarolrod
qml: give onboarding views descriptive names
9cc991f
@jarolrod
qml: remove unused inSubPage bool
5120735 
This is not used at all. This is a remnant from a time where the idea
was to have the navbar encapsulated within the Wizard control, and this
bool would dictate what navbutton to display. We have since moved to
each page provides its navbar and its navbuttons.
@jarolrod jarolrod force-pushed the wizard-page-instantiation branch from 3d3b62e to cc8c7e7 Compare December 10, 2022 16:46
@jarolrod
Copy link
Copy Markdown
Contributor Author

jarolrod commented Dec 10, 2022

updated from 3d3b62e to cc8c7e7

changes: rebased over master

johnny9
johnny9 suggested changes Dec 11, 2022
View reviewed changes
Comment thread src/qml/pages/main.qml Outdated
@johnny9
Copy link
Copy Markdown
Collaborator

johnny9 commented Dec 12, 2022

Looks like controls/Wizard.qml can be removed.

@jarolrod
qml: do not load onboarding pages from url
8619e50
@jarolrod
qml: remove wizard control
6c29ddf 
This is no longer providing any value and is not needed.
@jarolrod jarolrod force-pushed the wizard-page-instantiation branch from cc8c7e7 to 6c29ddf Compare December 13, 2022 19:25
@jarolrod
Copy link
Copy Markdown
Contributor Author

jarolrod commented Dec 13, 2022

updated from cc8c7e7 to 6c29ddf (pr198.04 -> pr198.05, diff)

changes: incorporated @johnny9 suggestions, and added a commit to finally remove the wizard control; as it is no longer needed.

johnny9
johnny9 approved these changes Dec 14, 2022
View reviewed changes
Copy link
Copy Markdown
Collaborator

@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 6c29ddf

Good changes. Tested mobile and desktop and no QML errors and flow is the same.

@jarolrod jarolrod mentioned this pull request Dec 14, 2022
Merged
@hebasto hebasto merged commit 18dfb2c into bitcoin-core:main Dec 14, 2022
@jarolrod jarolrod deleted the wizard-page-instantiation branch December 14, 2022 08:10
hebasto pushed a commit to hebasto/gui-qml that referenced this pull request Jun 8, 2025
@jarolrod @hebasto
qml: give onboarding views descriptive names
0ca26c8 
Github-Pull: bitcoin-core#198
Rebased-From: 9cc991f
hebasto pushed a commit to hebasto/gui-qml that referenced this pull request Jun 8, 2025
@jarolrod @hebasto
qml: remove unused inSubPage bool
8620c0f 
This is not used at all. This is a remnant from a time where the idea
was to have the navbar encapsulated within the Wizard control, and this
bool would dictate what navbutton to display. We have since moved to
each page provides its navbar and its navbuttons.

Github-Pull: bitcoin-core#198
Rebased-From: 5120735
hebasto pushed a commit to hebasto/gui-qml that referenced this pull request Jun 8, 2025
@jarolrod @hebasto
qml: do not load onboarding pages from url
4cad8e6 
Github-Pull: bitcoin-core#198
Rebased-From: 8619e50
hebasto pushed a commit to hebasto/gui-qml that referenced this pull request Jun 8, 2025
@jarolrod @hebasto
qml: remove wizard control
c3b49c1 
This is no longer providing any value and is not needed.

Github-Pull: bitcoin-core#198
Rebased-From: 6c29ddf
hebasto pushed a commit to hebasto/gui-qml that referenced this pull request Jun 9, 2025
@jarolrod @hebasto
qml: give onboarding views descriptive names
a8cc5b1 
Github-Pull: bitcoin-core#198
Rebased-From: 9cc991f
hebasto pushed a commit to hebasto/gui-qml that referenced this pull request Jun 9, 2025
@jarolrod @hebasto
qml: remove unused inSubPage bool
3cac003 
This is not used at all. This is a remnant from a time where the idea
was to have the navbar encapsulated within the Wizard control, and this
bool would dictate what navbutton to display. We have since moved to
each page provides its navbar and its navbuttons.

Github-Pull: bitcoin-core#198
Rebased-From: 5120735
hebasto pushed a commit to hebasto/gui-qml that referenced this pull request Jun 9, 2025
@jarolrod @hebasto
qml: do not load onboarding pages from url
0954bb2 
Github-Pull: bitcoin-core#198
Rebased-From: 8619e50
hebasto pushed a commit to hebasto/gui-qml that referenced this pull request Jun 9, 2025
@jarolrod @hebasto
qml: remove wizard control
5087469 
This is no longer providing any value and is not needed.

Github-Pull: bitcoin-core#198
Rebased-From: 6c29ddf
hebasto pushed a commit to hebasto/gui-qml that referenced this pull request Jun 9, 2025
@jarolrod @hebasto
qml: give onboarding views descriptive names
d473b77 
Github-Pull: bitcoin-core#198
Rebased-From: 9cc991f
hebasto pushed a commit to hebasto/gui-qml that referenced this pull request Jun 9, 2025
@jarolrod @hebasto
qml: remove unused inSubPage bool
9a9940e 
This is not used at all. This is a remnant from a time where the idea
was to have the navbar encapsulated within the Wizard control, and this
bool would dictate what navbutton to display. We have since moved to
each page provides its navbar and its navbuttons.

Github-Pull: bitcoin-core#198
Rebased-From: 5120735
hebasto pushed a commit to hebasto/gui-qml that referenced this pull request Jun 9, 2025
@jarolrod @hebasto
qml: do not load onboarding pages from url
e85d29f 
Github-Pull: bitcoin-core#198
Rebased-From: 8619e50
hebasto pushed a commit to hebasto/gui-qml that referenced this pull request Jun 9, 2025
@jarolrod @hebasto
qml: remove wizard control
44f95d9 
This is no longer providing any value and is not needed.

Github-Pull: bitcoin-core#198
Rebased-From: 6c29ddf
johnny9 pushed a commit to johnny9/bitcoin-core-app that referenced this pull request Jul 4, 2025
@hebasto
Merge bitcoin-core/gui-qml#198: Do not dynamically load OnboardingVie…
ec2ad1b 
…ws from url, which now have descriptive names

d34c53b qml: remove wizard control (Jarol Rodriguez)
7752257 qml: do not load onboarding pages from url (Jarol Rodriguez)
68be603 qml: remove unused inSubPage bool (Jarol Rodriguez)
7f05903 qml: give onboarding views descriptive names (Jarol Rodriguez)

Pull request description:

  This is addressing the [desire for descriptive names](bitcoin-core/gui-qml#149 (comment)) for the onboarding views as well as (partially) addressing an old piece of feedback from **promag** [about loading pages](bitcoin-core/gui-qml#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 d34c53b

Tree-SHA512: bacd3419deb0c68652eb0c470fd72f61e424c6872ff17f3bf7adf2848191b42e69627e79b75734b11fa917c3e2fd79288b764bfd4cf318f4711ebaea317b8734
tx-signer450 added a commit to tx-signer450/gui-qml that referenced this pull request Oct 20, 2025
@tx-signer450
Merge bitcoin-core/gui-qml#198: Do not dynamically load OnboardingVie…
76e19d7 
…ws from url, which now have descriptive names

d34c53bb4abafb380478eee86d3b5a7eac90b242 qml: remove wizard control (Jarol Rodriguez)
7752257b8071142c3868378dae923397216e30c9 qml: do not load onboarding pages from url (Jarol Rodriguez)
68be60364a7df535c6cbe420bfb1dd4400b54727 qml: remove unused inSubPage bool (Jarol Rodriguez)
7f05903487cc828180c3e4d467963cb88a470a20 qml: give onboarding views descriptive names (Jarol Rodriguez)

Pull request description:

  This is addressing the [desire for descriptive names](bitcoin-core/gui-qml#149 (comment)) for the onboarding views as well as (partially) addressing an old piece of feedback from **promag** [about loading pages](bitcoin-core/gui-qml#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 d34c53bb4abafb380478eee86d3b5a7eac90b242

Tree-SHA512: bacd3419deb0c68652eb0c470fd72f61e424c6872ff17f3bf7adf2848191b42e69627e79b75734b11fa917c3e2fd79288b764bfd4cf318f4711ebaea317b8734
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@johnny9 johnny9 johnny9 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@jarolrod @hebasto @johnny9