Skip to content

Conversation

@jarolrod
Copy link
Contributor

@jarolrod jarolrod commented Oct 25, 2024

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 we want.

Closes #422
Closes #219

Build Artifacts

@jarolrod jarolrod changed the title Migrate to PageStack's Migrate to PageStacks Oct 25, 2024
@jarolrod jarolrod force-pushed the pagestack-migration branch from 0d7c1ab to 3788060 Compare October 26, 2024 23:35
@jarolrod
Copy link
Contributor Author

Updated from 0d7c1ab to 3788060, compare

changes: rebased over changes in #427, and fixed lint errors

Copy link
Contributor

@pablomartin4btc pablomartin4btc left a comment

Choose a reason for hiding this comment

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

tACK 3788060

Nice job! Removing dependencies with ids outside a page was almost mandatory, this avoids unexpected crashes when things change outside the page and can't be noticed. We need to add a hook or a test that evaluates the existence of views we are moving to onNext: and warns if there are components that are not/ (or left un-) referenced.

Comment on lines 131 to 133
onBack: {
nodeSettingsView.pop()
}
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: as in other places in the same file...

Suggested change
onBack: {
nodeSettingsView.pop()
}
onBack: nodeSettingsView.pop()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

a note for the style guide

Comment on lines 163 to 165
onBack: {
nodeSettingsView.pop()
}
Copy link
Contributor

Choose a reason for hiding this comment

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

same as above.

Introduces a custom PageStack control that is a StackView adapted for
our use in terms of the custom animations we demand, and the ability
to switch between vertical and horizontal push() and pop().

This can later be further extended for our use cases.
This should be encapsulated so it can be reasoned about independently
and in a well defined location. Additionally, swap from SwipeView to
PageStack.
Vertical PageStacks used to replace vertical SwipeViews.
@jarolrod jarolrod force-pushed the pagestack-migration branch from 3788060 to c52566d Compare November 11, 2024 20:58
@jarolrod
Copy link
Contributor Author

Updated from 3788060 to c52566d

Changes: rebased over main

Copy link
Contributor

@pablomartin4btc pablomartin4btc left a comment

Choose a reason for hiding this comment

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

re-ACK c52566d

Copy link
Contributor

@MarnixCroes MarnixCroes left a comment

Choose a reason for hiding this comment

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

tACK c52566d
on Ubuntu 24.04.1 LTS

}
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: there are extra blank lines

Copy link
Contributor

@D33r-Gee D33r-Gee left a comment

Choose a reason for hiding this comment

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

tACK c52566d Works as expected on WSL Ubuntu 22.04

Copy link
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 c52566d

@hebasto hebasto merged commit 574817b into bitcoin-core:main Nov 24, 2024
9 checks passed
@GBKS GBKS mentioned this pull request May 12, 2025
1 task
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

  [![Build Artifacts](https://img.shields.io/badge/Build%20Artifacts-green
  )]()

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
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

  [![Build Artifacts](https://img.shields.io/badge/Build%20Artifacts-green
  )]()

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Page Animations should be 400ms with cubic-bezier Brainstorm: On the loading of pages within a view container and focus policies

6 participants