Introduce Signal Based Navigation Model to have Self-Contained Pages#427
Merged
Conversation
Pages exist within a view, in this page <-> view dependency, somewhere
inside of there, at least for views that are 'flows' or 'wizards,' there
needs to be logic on:
a) when to move to a new page
b) what page to move to.
We are currently handling this navigation logic primarily inside the
definition of a page itself. Inside of the page, we assume a hard-coded
ID for a parent view and call navigation function directly to the view
from within the page itself telling it to:
a) to pop or push
b) the id of the page we want to push.
This is bad because:
1. It creates a dependency between page declarations that does not
need to exist. As in, PageB.qml doesn't really exist unless
PageA.qml includes code to go to it.
2. This prevents us from being able to 'statically' define and
clearly contain pages. Clearly containing pages makes it so that
when we want to reason about PageA.qml, we only have to look
inside of PageA.qml and the components that are used within that
file; we don't have to go on a chase for interlinked dependencies
on id's or properties that are hidden or at least not easy to
find in other pages.
3. We should never assume outside id's from within a page, as these
can change, and when they change, the page that assumes said
outside id is now broken, and debugging can be annoying.
All of this is unnecessary.
In this commit, we address the issue of the logic for navigation being
contained within the page definitions. This proposes a new navigation
model based on signals and first applies it to the onboarding flow.
Pages within our onboarding flow now:
1. Emit a signal when it's time to go to a new page
2. The parent View itself handles the signals and contains the logic
for navigation.
This removes the dependence on having a hardcoded outside ID within a
a page, and is one step to moving us to pages with no dependency on
other pages.
Here, pages are clearly contained, but the logic of moving to SettingsDeveloper from within SettingsAbout and SettingsProxy from within SettingsConnection is still contained here within the definitions of SettingsAbout and SettingsConnection. While all pages are independently still usable, we do want to iron out this one kink in a follow-up.
Makes our CreateWallet flow into a wizard, as that is what it should be. The CreateWalletWizard is introduced as a StackView with the associated pages implementing signal based navigation. Previously, there was an interlinked dependency between CreateName and CreatePassword in that CreateName would pass a string for walletName to CreatePassword. This dependency is removed by having CreateName set the string in a property contained in the CreateWalletWizard StackView, and CreatePassword pulling in this property to satisfy its requirement for walletName. This is a temporary workaround so that we can still have clearly contained pages here. A follow-up should move this into a more appropriate backend object
Merged
MarnixCroes
reviewed
Oct 25, 2024
Contributor
MarnixCroes
left a comment
MarnixCroes
left a comment
There was a problem hiding this comment.
cACK
I went through all the pages, including onboarding and it work ok.
related lint failure should be fixable
| onNext: aboutSwipe.incrementCurrentIndex() | ||
| } | ||
|
|
||
| states : [ |
Contributor
There was a problem hiding this comment.
nit
Suggested change
| states : [ | |
| states: [ |
Contributor
Author
There was a problem hiding this comment.
we should set a style guide actually
Collaborator
|
Concept ACK No qml errors when going through onboard. Looks like there are just some whitespace issues to fix to make the lint job happy. |
In containing pages, everything a page needs should be defined where the page is declared. Allowing to alias nav elements into a page breaks the idea of pages being statically defined. The action of aliasing down nav elements means that the real declaration of a page is not in its 'filename.qml', but instead where the page is used, meaning a page can be defined an infinite amount of times in an infinite amount of ways; we don't want this. The only reason that we were aliasing down nav elements is that we have reusable pages for settings and what elements the nav bar will contain is different depending on whether we are onboarding or not. So, we should treat that as the navbar for the page has two states, so we should have logic for these states instead of defining the page in different locations in different ways. This state logic is handled by setting a property of onboarding or not, a follow-up can potentially move this into an appropriate backend object such as AppMode.
Renaming for consistency with other pages that 'back' signal and handles the 'back' signal. No reason to have some pages call this differently.
jarolrod
force-pushed
the
contain-pages
branch
from
59aeba2 to
3f94240
Compare
Contributor
Author
johnny9
approved these changes
Oct 27, 2024
Contributor
pablomartin4btc
left a comment
pablomartin4btc
left a comment
There was a problem hiding this comment.
post-merge tACK 3f94240
we should set a style guide actually
As discussed offline, we should do that + some specific dev notes for this repo.
hebasto
added a commit
that referenced
this pull request
Nov 24, 2024
c52566d qml: Swap existing SwipeViews for PageStack (jarolrod) f0e5951 qml: Swap existing StackViews for PageStack (jarolrod) 7ba2f62 qml: declare OnboardingWizard file, reintroduce as PageStack (jarolrod) d49b888 qml: Introduce PageStack control (jarolrod) Pull request description: After fixing our sins in #427, migrating to A StackView based GUI is quite simple. Here we use a custom StackView control, PageStack, that has a property `vertical` used to declare if we want vertical or horizontal animations, and additionally implements the [custom animation](#422) we want. Closes #422 Closes #219 []() ACKs for top commit: pablomartin4btc: re-ACK c52566d MarnixCroes: tACK c52566d D33r-Gee: tACK [c52566d](c52566d) Works as expected on WSL Ubuntu 22.04 johnny9: ACK c52566d Tree-SHA512: f9a025944db24a46e1f78f4ad8a9b9aca4e1f3ff4dd5927eebfa2a7fb28ed390a17da79e23c6248c3c0e82b361ff1b2dedbf4df9df2a1d0677b05bacb7763bcb
johnny9
pushed a commit
to johnny9/bitcoin-core-app
that referenced
this pull request
Jul 4, 2025
…el to have Self-Contained Pages f922f30 qml: rename backClicked signal to back (jarolrod) cc73417 qml: Use signal based navigation in NodeSettings, don't alias nav elems (jarolrod) f3d3a4a qml: use signal based navigation in CreateWallet pages, introduce wizard (jarolrod) 3724392 qml: use signal based navigation in settings with subpages (jarolrod) bdbf761 qml: signal based navigation in onboarding (jarolrod) Pull request description: Here we introduce a Signal Based navigation model, so that we can have self-contained pages. Self-contained pages include everything they need where they are declared, and do not have more than one definition. This makes pages modular, easier to reason about, but also easier to maintain by eliminating dependencies on hard-coded IDs, direct calls to parent views, and interlinked properties between pages. When we want to reason about `PageA.qml`, we only have to look in there and the child components it uses, and not have to use a search function to ensure we have the entire scope of `PageA.qml`. To Achieve this, this PR implements the following main ideas: 1. Ensure that pages are not hardcoding and reaching into an outside view. 2. Ensure we don't alias down any components into pages, and instead have pages define all their components, but react to state events 3. Ensure we don't have interlinked properties between pages 4. Move navigation and page linking logic from the pages themselves and into the declaration of the view itself. Additionally, while here we structure the CreateWallet flow into a Wizard. **Follow-Ups:** Consider moving onboarding and walletName into more appropriate backend objects. []() ACKs for top commit: johnny9: ACK f922f30 MarnixCroes: ACK f922f30 Tree-SHA512: 134ba2ee3a1b4aec77601ca065cfc690098e6b3695a9bcf8c65498a80e41857fa6f2cf218e9269e9b31c51babb3faa70fd1db35e148dd3dbd261c9359c3212b4
johnny9
pushed a commit
to johnny9/bitcoin-core-app
that referenced
this pull request
Jul 4, 2025
edc44e1 qml: Swap existing SwipeViews for PageStack (jarolrod) 825b883 qml: Swap existing StackViews for PageStack (jarolrod) 9c02a20 qml: declare OnboardingWizard file, reintroduce as PageStack (jarolrod) 51b1273 qml: Introduce PageStack control (jarolrod) Pull request description: After fixing our sins in bitcoin-core/gui-qml#427, migrating to A StackView based GUI is quite simple. Here we use a custom StackView control, PageStack, that has a property `vertical` used to declare if we want vertical or horizontal animations, and additionally implements the [custom animation](bitcoin-core/gui-qml#422) we want. Closes bitcoin-core/gui-qml#422 Closes bitcoin-core/gui-qml#219 []() ACKs for top commit: pablomartin4btc: re-ACK edc44e1 MarnixCroes: tACK edc44e1 D33r-Gee: tACK [edc44e1](bitcoin-core/gui-qml@edc44e1) Works as expected on WSL Ubuntu 22.04 johnny9: ACK edc44e1 Tree-SHA512: f9a025944db24a46e1f78f4ad8a9b9aca4e1f3ff4dd5927eebfa2a7fb28ed390a17da79e23c6248c3c0e82b361ff1b2dedbf4df9df2a1d0677b05bacb7763bcb
tx-signer450
added a commit
to tx-signer450/gui-qml
that referenced
this pull request
Oct 20, 2025
…el to have Self-Contained Pages f922f30f2efe30bd70e9c5f1870f01716a821d11 qml: rename backClicked signal to back (jarolrod) cc7341758112a5dbf748e80178e70cad1f8d32bf qml: Use signal based navigation in NodeSettings, don't alias nav elems (jarolrod) f3d3a4acb6016143d6b6fdf400f8d3c71d8c7d24 qml: use signal based navigation in CreateWallet pages, introduce wizard (jarolrod) 3724392b3f299c08a09fd477a9a5ff184d15cf52 qml: use signal based navigation in settings with subpages (jarolrod) bdbf7616026b672f732005d2ecc7133e7a514605 qml: signal based navigation in onboarding (jarolrod) Pull request description: Here we introduce a Signal Based navigation model, so that we can have self-contained pages. Self-contained pages include everything they need where they are declared, and do not have more than one definition. This makes pages modular, easier to reason about, but also easier to maintain by eliminating dependencies on hard-coded IDs, direct calls to parent views, and interlinked properties between pages. When we want to reason about `PageA.qml`, we only have to look in there and the child components it uses, and not have to use a search function to ensure we have the entire scope of `PageA.qml`. To Achieve this, this PR implements the following main ideas: 1. Ensure that pages are not hardcoding and reaching into an outside view. 2. Ensure we don't alias down any components into pages, and instead have pages define all their components, but react to state events 3. Ensure we don't have interlinked properties between pages 4. Move navigation and page linking logic from the pages themselves and into the declaration of the view itself. Additionally, while here we structure the CreateWallet flow into a Wizard. **Follow-Ups:** Consider moving onboarding and walletName into more appropriate backend objects. []() ACKs for top commit: johnny9: ACK f922f30f2efe30bd70e9c5f1870f01716a821d11 MarnixCroes: ACK f922f30f2efe30bd70e9c5f1870f01716a821d11 Tree-SHA512: 134ba2ee3a1b4aec77601ca065cfc690098e6b3695a9bcf8c65498a80e41857fa6f2cf218e9269e9b31c51babb3faa70fd1db35e148dd3dbd261c9359c3212b4
tx-signer450
added a commit
to tx-signer450/gui-qml
that referenced
this pull request
Oct 20, 2025
edc44e1ef1becfb7ee753bd8dd8b5663d39ecf41 qml: Swap existing SwipeViews for PageStack (jarolrod) 825b883269ae3f05a59977010c81af6ecad8f4e8 qml: Swap existing StackViews for PageStack (jarolrod) 9c02a20ba1b13e4874be5009077e6513db0790db qml: declare OnboardingWizard file, reintroduce as PageStack (jarolrod) 51b1273ba151ce7ac2eb00a94b4e3dc7528c3e17 qml: Introduce PageStack control (jarolrod) Pull request description: After fixing our sins in bitcoin-core/gui-qml#427, migrating to A StackView based GUI is quite simple. Here we use a custom StackView control, PageStack, that has a property `vertical` used to declare if we want vertical or horizontal animations, and additionally implements the [custom animation](bitcoin-core/gui-qml#422) we want. Closes bitcoin-core/gui-qml#422 Closes bitcoin-core/gui-qml#219 []() ACKs for top commit: pablomartin4btc: re-ACK edc44e1ef1becfb7ee753bd8dd8b5663d39ecf41 MarnixCroes: tACK edc44e1ef1becfb7ee753bd8dd8b5663d39ecf41 D33r-Gee: tACK [edc44e1](bitcoin-core/gui-qml@edc44e1) Works as expected on WSL Ubuntu 22.04 johnny9: ACK edc44e1ef1becfb7ee753bd8dd8b5663d39ecf41 Tree-SHA512: f9a025944db24a46e1f78f4ad8a9b9aca4e1f3ff4dd5927eebfa2a7fb28ed390a17da79e23c6248c3c0e82b361ff1b2dedbf4df9df2a1d0677b05bacb7763bcb
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Here we introduce a Signal Based navigation model, so that we can have self-contained pages. Self-contained pages include everything they need where they are declared, and do not have more than one definition. This makes pages modular, easier to reason about, but also easier to maintain by eliminating dependencies on hard-coded IDs, direct calls to parent views, and interlinked properties between pages.
When we want to reason about
PageA.qml, we only have to look in there and the child components it uses, and not have to use a search function to ensure we have the entire scope ofPageA.qml.To Achieve this, this PR implements the following main ideas:
Additionally, while here we structure the CreateWallet flow into a Wizard.
Follow-Ups:
Consider moving onboarding and walletName into more appropriate backend objects.