refactor: onboarding mechanics and navigation #1464

jleach · 2025-03-10T18:05:48Z

Summary of Changes

Improving Onboarding Flexibility

Currently, adding or reorganizing onboarding screens is cumbersome due to hardcoded navigation logic. This PR simplifies the process by relying more on application state and less on manual navigation rules.

Key Changes:

Onboarding screens are now managed as an array, making it easier to modify the order of screens.
The OnboardingStack automatically displays the first incomplete screen.
Once all screens are completed, an event is emitted to signal that onboarding is finished, prompting the root stack to transition to MainStack.
The only required screens in this process are CreatePIN and/or EnterPIN.

Screenshots, videos, or gifs

Replace this text with embedded media for UI changes if they are included in this PR. If there are none, simply enter N/A

Breaking change guide

Replace this text with any breaking changes included in this PR along with how to address them in downstream projects. If there are none, simply enter N/A

Related Issues

Replace this text with issue #'s that are relevant to this PR. If there are none, simply enter N/A

Pull Request Checklist

Tick all boxes below to demonstrate that you have completed the respective task. If the item does not apply to your this PR check it anyway to make it apparent that there's nothing to do.

All commits contain a DCO Signed-off-by line (we use the DCO GitHub app to enforce this)
If applicable, screenshots, gifs, or video are included for UI changes
If applicable, breaking changes are described above along with how to address them
Updated documentation as needed for changed code and new or modified features
Added sufficient tests so that overall code coverage is not reduced

If you have any questions to any of the points above, just submit and ask! This checklist is here to help you, not to deter you from contributing!

Pro Tip 🤓

Read our contribution guide at least once; it will save you a few review cycles!
Your PR will likely not be reviewed until all the above boxes are checked and all automated checks have passed

packages/legacy/core/App/navigators/OnboardingStack.tsx

packages/legacy/core/App/container-impl.ts

packages/legacy/core/App/navigators/OnboardingScreens.ts

fc-santos · 2025-03-12T18:45:50Z

packages/legacy/core/App/navigators/OnboardingStack.tsx

@@ -15,36 +15,37 @@ import { createCarouselStyle } from '../screens/OnboardingPages'
 import PINCreate from '../screens/PINCreate'
 import PINEnter from '../screens/PINEnter'
 import PushNotification from '../screens/PushNotification'
+// import UpdateAvailable from '../screens/UpdateAvailable'


Any reason to keep this comment?

packages/legacy/core/App/navigators/OnboardingStack.tsx

al-rosenthal · 2025-03-13T21:50:35Z

packages/legacy/core/__tests__/screens/Splash.test.tsx

@@ -105,6 +105,6 @@ describe('Splash Screen', () => {
      jest.runAllTimers()
    })

-    expect(navigation.dispatch).toHaveBeenCalledTimes(1)
+    expect(navigation.dispatch).toHaveBeenCalledTimes(0)


nit: what are we testing for if we don't expect this to be called?

packages/legacy/core/App/screens/Terms.tsx

bryce-mcmath

Few small non-blocking suggestions and some questions

bryce-mcmath · 2025-03-13T21:39:03Z

packages/legacy/core/App/navigators/OnboardingScreens.ts

+  PushNotification: React.FC<StackScreenProps<ParamListBase, Screens.UsePushNotifications>>
+  Developer: React.FC<StackScreenProps<ParamListBase>>
+  AttemptLockout: React.FC<StackScreenProps<ParamListBase>>
+  OnBoardingScreen: React.FC


Should we take the opportunity to change this while we're touching this code, so it's the same casing as it is everywhere else?

Suggested change

OnBoardingScreen: React.FC

OnboardingScreen: React.FC

bryce-mcmath · 2025-03-13T21:39:47Z

packages/legacy/core/App/navigators/OnboardingStack.tsx

@@ -73,7 +81,7 @@ const OnboardingStack: React.FC = () => {
    [dispatch]
  )

-  const OnBoardingScreen = () => {
+  const OnBoardingScreen = useCallback(() => {


Suggested change

const OnBoardingScreen = useCallback(() => {

const OnboardingScreen = useCallback(() => {

bryce-mcmath · 2025-03-14T20:22:57Z

packages/legacy/core/App/navigators/RootStack.tsx

-    () => store.authentication.didAuthenticate && store.onboarding.postAuthScreens.length === 0,
-    [store.authentication.didAuthenticate, store.onboarding.postAuthScreens]
-  )
+  const isOnboardingComplete = useMemo(() => onboardingComplete, [onboardingComplete])


I don't think this useMemo will have any effect. I think primitive values already only cause re-renders when they change. If they are reassigned to the same value they don't cause re-renders (afaik)

bryce-mcmath · 2025-03-14T20:25:29Z

packages/legacy/core/App/navigators/OnboardingStack.tsx

  const { t } = useTranslation()
  const Stack = createStackNavigator()
  const theme = useTheme()
  const OnboardingTheme = theme.OnboardingTheme
  const carousel = createCarouselStyle(OnboardingTheme)
  const [
-    splash,
+    config,
+    Splash,
    pages,
    useBiometry,


Since this is a screen (JSX) and not a hook it should probably be capitalized. Not something we have to do in this PR, just at some point

You mean "useBiometry" if so I created a ticket with some issues I noticed along the way and this was one. I think it should be renamed to not use use in the name for clarity.

bryce-mcmath · 2025-03-14T20:27:37Z

packages/legacy/core/App/screens/NameWallet.tsx

-      if (store.onboarding.didCompleteOnboarding) {
-        navigation.goBack()
-      } else {


I think this screen can be accessed from the settings menu, not just onboarding. So that call to navigation.goBack() might still be needed in that case

bryce-mcmath · 2025-03-14T20:30:46Z

packages/legacy/core/App/screens/Splash.tsx

-    setMounted(true)
-  }, [])
-
-  useEffect(() => {


With this change, what does the user see while state is being loaded for the first time on a cold start?

I think I may need to add this back then. It's a bit confusing but we have a splash screen imported from a NPM package and our Splash screen. I assumed there was a bug in the NPM version that is meant to show the app logo screen until the "state" is loaded at which point the app can then continue. Maybe I need to update Splash to show in two different ways, or fix the npm package to work.

Signed-off-by: Jason C. Leach <[email protected]>

sonarqubecloud · 2025-03-17T21:44:07Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.2% Duplication on New Code

See analysis details on SonarQube Cloud

fc-santos reviewed Mar 11, 2025

View reviewed changes

packages/legacy/core/App/navigators/OnboardingStack.tsx Outdated Show resolved Hide resolved

fc-santos reviewed Mar 11, 2025

View reviewed changes

packages/legacy/core/App/container-impl.ts Outdated Show resolved Hide resolved

jleach force-pushed the feat/refactor-onboarding-nav branch from 9d17b8b to db9bc97 Compare March 12, 2025 17:59

jleach marked this pull request as ready for review March 12, 2025 18:00

jleach requested a review from a team as a code owner March 12, 2025 18:00

jleach requested a review from fc-santos March 12, 2025 18:07

fc-santos reviewed Mar 12, 2025

View reviewed changes

packages/legacy/core/App/navigators/OnboardingScreens.ts Show resolved Hide resolved

fc-santos reviewed Mar 12, 2025

View reviewed changes

packages/legacy/core/App/navigators/OnboardingScreens.ts Outdated Show resolved Hide resolved

fc-santos reviewed Mar 12, 2025

View reviewed changes

jleach mentioned this pull request Mar 12, 2025

refactor: use new onboarding navigation mechanics bcgov/bc-wallet-mobile#2404

Closed

fc-santos reviewed Mar 13, 2025

View reviewed changes

packages/legacy/core/App/navigators/OnboardingStack.tsx Show resolved Hide resolved

jleach requested a review from fc-santos March 13, 2025 16:32

This was referenced Mar 13, 2025

refactor: bring in changes to onboarding bcgov/bc-wallet-mobile#2405

Closed

refactor: bring in new onboarding mechanics bcgov/bc-wallet-mobile#2406

Draft

fc-santos previously approved these changes Mar 13, 2025

View reviewed changes

al-rosenthal reviewed Mar 13, 2025

View reviewed changes

bryce-mcmath reviewed Mar 14, 2025

View reviewed changes

jleach dismissed fc-santos’s stale review via afdf89e March 17, 2025 20:13

jleach added 12 commits March 17, 2025 14:36

refactor: onboarding mechanics

95e63d5

Signed-off-by: Jason C. Leach <[email protected]>

refactor: onboarding mechanics

3adf3e5

Signed-off-by: Jason C. Leach <[email protected]>

fix: cleanup events

62174da

Signed-off-by: Jason C. Leach <[email protected]>

fix: code cleanup

3f8c5bc

Signed-off-by: Jason C. Leach <[email protected]>

fefactor: onbording logic

a8bcb99

Signed-off-by: Jason C. Leach <[email protected]>

fix: integration

539ee7e

Signed-off-by: Jason C. Leach <[email protected]>

fix: typescript errors

2906ded

Signed-off-by: Jason C. Leach <[email protected]>

fix: lint errors

bf2a573

Signed-off-by: Jason C. Leach <[email protected]>

fix: lint errors

cabc49e

Signed-off-by: Jason C. Leach <[email protected]>

fix: lint errors

3def348

Signed-off-by: Jason C. Leach <[email protected]>

fix: unit tests

7bec952

Signed-off-by: Jason C. Leach <[email protected]>

fix: code quality issues

448c9aa

Signed-off-by: Jason C. Leach <[email protected]>

jleach added 9 commits March 17, 2025 14:36

fix: code quality issues

043ea5a

Signed-off-by: Jason C. Leach <[email protected]>

fix: code quality issues

97d5ea5

Signed-off-by: Jason C. Leach <[email protected]>

fix: code quality issues

12e6da4

Signed-off-by: Jason C. Leach <[email protected]>

fix: remove dead code

75c46a6

Signed-off-by: Jason C. Leach <[email protected]>

fix: remove dead code

021b3f0

Signed-off-by: Jason C. Leach <[email protected]>

fix: remove dead code

bb3bd68

Signed-off-by: Jason C. Leach <[email protected]>

fix: update exports

2a018a7

Signed-off-by: Jason C. Leach <[email protected]>

fix: tests

d06aeda

Signed-off-by: Jason C. Leach <[email protected]>

fix: screen usage

ad95120

Signed-off-by: Jason C. Leach <[email protected]>

jleach force-pushed the feat/refactor-onboarding-nav branch from 992421f to ad95120 Compare March 17, 2025 21:36

jleach added 3 commits March 17, 2025 14:39

fix: naming

fe2184f

Signed-off-by: Jason C. Leach <[email protected]>

fix: naming

323df8f

Signed-off-by: Jason C. Leach <[email protected]>

fix: tests

2745040

Signed-off-by: Jason C. Leach <[email protected]>

jleach enabled auto-merge (squash) March 17, 2025 23:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: onboarding mechanics and navigation #1464

refactor: onboarding mechanics and navigation #1464

jleach commented Mar 10, 2025 •

edited

Loading

fc-santos Mar 12, 2025

jleach Mar 12, 2025

al-rosenthal Mar 13, 2025

bryce-mcmath left a comment

bryce-mcmath Mar 13, 2025

jleach Mar 17, 2025

bryce-mcmath Mar 13, 2025

jleach Mar 17, 2025

bryce-mcmath Mar 14, 2025

jleach Mar 17, 2025

bryce-mcmath Mar 14, 2025

jleach Mar 17, 2025

bryce-mcmath Mar 14, 2025

jleach Mar 17, 2025

bryce-mcmath Mar 14, 2025

jleach Mar 17, 2025 •

edited

Loading

sonarqubecloud bot commented Mar 17, 2025

	const OnBoardingScreen = useCallback(() => {
	const OnboardingScreen = useCallback(() => {

refactor: onboarding mechanics and navigation #1464

Are you sure you want to change the base?

refactor: onboarding mechanics and navigation #1464

Conversation

jleach commented Mar 10, 2025 • edited Loading

Summary of Changes

Improving Onboarding Flexibility

Key Changes:

Screenshots, videos, or gifs

Breaking change guide

Related Issues

Pull Request Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bryce-mcmath left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jleach Mar 17, 2025 • edited Loading

Choose a reason for hiding this comment

sonarqubecloud bot commented Mar 17, 2025

Quality Gate passed

jleach commented Mar 10, 2025 •

edited

Loading

jleach Mar 17, 2025 •

edited

Loading