Skip to content
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

feat(OnboardingLayout): Decompose into smaller, pure ui sub-flows #16977

Open
wants to merge 3 commits into
base: feat/onboarding-rework-1
Choose a base branch
from

Conversation

micieslak
Copy link
Member

@micieslak micieslak commented Dec 17, 2024

What does the PR do

Decomposes OnboardingLayout into following elements:

  • OnboardingFlow orchestrating all on-boarding flows on the stack
  • OnboardingStackView which is specifically configured StackView for handling all on-boarding flows
  • OnboardingLayout itself integrating OboardingFlow with stores and OnboardingStackView

Moreover OnboardingFlow consists of sub-flows:

  • CreateNewProfileFlow
  • KeycardCreateProfileFlow
  • RecoveryPhraseCreateProfileFlow
  • LoginBySyncingFlow
  • LoginWithKeycardFlow
  • UseRecoveryPhraseFlow

Flows don't operate on own StackView, which must be provided from outside. This allows to compose complex flows from smaller ones within one StackView.

Other:

  • little change in syncing flow, retry causes instantaneous retry with the previously provided connection string, user can change the connection string by pressing back button
  • changed approach regarding back button control. It's no longer needed to have single element on the stack to hide back button. Every page may define backAvailableHint to indicate if back button should be available or not. It brings two benefits:
    • back button can be easily hidden when some operation is in progress, like syncing (aligned to the design in this pr)
    • no need to artificially clear the whole stack and push multiple elements later to compensate clearing
    • works seamlessly with the sub-flows because sub-flows shouldn't clear the whole stack on their own, affecting parent flow
  • minor wording fix in KeycardEnterPinPage
  • fixes broken unitests (last commit to onboarding-rework-1 broke them)

Further work

  • primaryFlow/secondaryFlow enums should be replaced by one enum, probably can be kept in the QML file
  • the flow type exposed via OnboardingFlow::finish is not collected in an elegant way. Maybe there is better option for that.
  • reconsider how jumping from one flow to another should be handled. In case of keycard flow it's possible to push pages on the stack infinitely.
  • introducing minimal times for some "in progress" pages. E.g. Sync in progress should be notified for some time even if the underlying operation was very quick (no matter if error or success). This gives the user the opportunity to familiarize themselves with the message and prevents from "blinking" when things happen quickly.
  • property timeoutInterval is probably not needed in SyncProgressPage and KeycardAddKeyPairPage. It should be decided on the backend level what's the actual timeout.
  • add detailed tests for particular flows, maybe also for particular pages
  • consider adding storybook pages for so far not covered pages and flows
  • add some additional utilities for existing page for OnboardingLayout
    • automatic fill on seedphrase confirmation page
    • showing full content of the stack, not only the current page
    • expose states for syncing and adding key pair
  • document backAvailableHint as it's implicit way of communication between flows and provided stack view
  • localAppSettings id used from outer scope, ideally whole settings handling should be externalized
  • would be nice to have text in popups selectable/copyable
  • modals open with a freeze delay, ideally text should be read async from the file

Closes: #16947

Affected areas

OnboardingLayout and related components

Architecture compliance

Screenshot of functionality (including design for comparison)

  • I've checked the design and this PR matches it

@micieslak micieslak requested review from caybro, alexjba and a team as code owners December 17, 2024 13:31
@status-im-auto
Copy link
Member

status-im-auto commented Dec 17, 2024

Jenkins Builds

Click to see older builds (15)
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 84fee43 #1 2024-12-17 13:38:44 ~6 min macos/aarch64 🍎dmg
✔️ 84fee43 #1 2024-12-17 13:39:18 ~7 min tests/nim 📄log
84fee43 #1 2024-12-17 13:43:41 ~11 min tests/ui 📄log
✔️ 84fee43 #1 2024-12-17 13:48:40 ~17 min linux/x86_64 📦tgz
✔️ 84fee43 #1 2024-12-17 13:50:09 ~18 min macos/x86_64 🍎dmg
✔️ 84fee43 #1 2024-12-17 13:52:31 ~20 min linux-nix/x86_64 📦tgz
✔️ 84fee43 #1 2024-12-17 13:55:41 ~23 min windows/x86_64 💿exe
✔️ 84fee43 #2 2024-12-17 15:10:20 ~11 min tests/ui 📄log
✔️ e1c752b #2 2024-12-17 16:05:53 ~4 min macos/aarch64 🍎dmg
✔️ e1c752b #2 2024-12-17 16:09:16 ~7 min tests/nim 📄log
✔️ e1c752b #3 2024-12-17 16:13:24 ~11 min tests/ui 📄log
✔️ e1c752b #2 2024-12-17 16:15:48 ~14 min macos/x86_64 🍎dmg
✔️ e1c752b #2 2024-12-17 16:16:58 ~15 min linux/x86_64 📦tgz
✔️ e1c752b #2 2024-12-17 16:18:50 ~17 min linux-nix/x86_64 📦tgz
✔️ e1c752b #2 2024-12-17 16:23:29 ~21 min windows/x86_64 💿exe
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 30c6aba #3 2024-12-17 16:52:32 ~4 min macos/aarch64 🍎dmg
✔️ 30c6aba #3 2024-12-17 16:56:14 ~8 min tests/nim 📄log
✔️ 30c6aba #4 2024-12-17 17:01:14 ~13 min tests/ui 📄log
✔️ 30c6aba #3 2024-12-17 17:03:46 ~15 min macos/x86_64 🍎dmg
✔️ 30c6aba #3 2024-12-17 17:05:45 ~17 min linux-nix/x86_64 📦tgz
✔️ 30c6aba #3 2024-12-17 17:08:38 ~20 min linux/x86_64 📦tgz
✔️ 30c6aba #3 2024-12-17 17:08:55 ~20 min windows/x86_64 💿exe
✔️ c9f2283 #4 2024-12-17 20:18:47 ~4 min macos/aarch64 🍎dmg
✔️ c9f2283 #4 2024-12-17 20:21:52 ~7 min tests/nim 📄log
✔️ c9f2283 #5 2024-12-17 20:25:56 ~11 min tests/ui 📄log
✔️ c9f2283 #4 2024-12-17 20:30:38 ~16 min macos/x86_64 🍎dmg
✔️ c9f2283 #4 2024-12-17 20:30:58 ~16 min linux-nix/x86_64 📦tgz
✔️ c9f2283 #4 2024-12-17 20:33:26 ~19 min windows/x86_64 💿exe
✔️ c9f2283 #4 2024-12-17 20:35:44 ~21 min linux/x86_64 📦tgz

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.

2 participants