feat: make tour provider tour steps dynamic #1428

fc-santos · 2025-02-04T21:08:29Z

Summary of Changes

This PR makes the tour steps more dynamic. The base tours still exist (homeTourSteps, credentialsTourSteps, etc), but a wallet will be able to add other tours if needed it.

Breaking change: The TourProvider will accept a tours prop instead of the homeTourSteps, credentialsTourSteps.

Here is the type definition of Tours:

type TourID = `${string}TourSteps` 

type Tours = {
  [key: TourID]: TourStep[]
}

/* 
const tours: Tours = {
   homeTourSteps: myWalletHomeTourSteps,
  credentialsTourSteps: myWalletCredentialsTourSteps,
  ....,
  // Let's say a wallet has another screen where they want an app tour
  someScreenTourSteps: myWalletScreenTourSteps
}
*/

Screenshots, videos, or gifs

N/A

Breaking change guide

The Tour Provider will be defined as following in App.tsx:

import { homeTourSteps } from './components/tour/HomeTourSteps'
import { credentialsTourSteps } from './components/tour/CredentialsTourSteps'
import { credentialOfferTourSteps } from './components/tour/CredentialOfferTourSteps'
import { proofRequestTourSteps } from './components/tour/ProofRequestTourSteps'
.
.
.
const tours: Tours = {
  homeTourSteps,
  credentialsTourSteps,
  credentialOfferTourSteps,
  proofRequestTourSteps
}

<TourProvider tours={tours} overlayColor={'gray'} overlayOpacity={0.7}>
      <RootStack />
</TourProvider>

And to use the AttachTourStep component:

Example in TabStack.tsx
<AttachTourStep tourID={TourIDKeys.homeTourSteps} index={1}></AttachTourStep>

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

Signed-off-by: fc-santos <[email protected]>

fc-santos · 2025-02-04T21:20:33Z

@bryce-mcmath, I see you're the person who worked in the TourProvider implementation. If you could take a lot at this, it'd be greatly appreciated. To explain a little bit more why I have done this, in Quebec, we have other screens where we would like to have the tour guide, but the current implementation won't allow this. This PR will allows us to add a guided tour for the Activities screen and the More screen here:

Signed-off-by: fc-santos <[email protected]>

bryce-mcmath · 2025-02-04T21:45:35Z

@bryce-mcmath, I see you're the person who worked in the TourProvider implementation. If you could take a lot at this, it'd be greatly appreciated. To explain a little bit more why I have done this, in Quebec, we have other screens where we would like to have the tour guide, but the current implementation won't allow this. This PR will allows us to add a guided tour for the Activities screen and the More screen

@fc-santos I'm still reading through it, but so far I love it, nice work! It's much more extensible this way (and I'm glad someone was able to understand the code for this feature.)

This is a small thing, but could we follow the SonarCloud optional chaining suggestion? The other two SonarCloud issues are fine as is, but the optional chaining one is a good and easy change I think.

bryce-mcmath

Mostly small things related to constants. There are some great improvements here though, good work so far

packages/legacy/core/App/constants.ts

bryce-mcmath · 2025-02-04T22:36:04Z

packages/legacy/core/App/contexts/tour/tour-context.tsx

@@ -137,17 +132,14 @@ export const ORIGIN_SPOT: LayoutRectangle = {
 }

 export const TourContext = createContext<TourCtx>({
-  currentTour: TourID.HomeTour,
+  currentTour: 'homeTourSteps',


Can we use constants for these?

bryce-mcmath · 2025-02-04T22:36:28Z

packages/legacy/core/App/contexts/tour/tour-provider.tsx

    nativeDriver = false,
  } = props

-  const [currentTour, setCurrentTour] = useState<TourID>(TourID.HomeTour)
+  const [currentTour, setCurrentTour] = useState<TourID>('homeTourSteps')


same as above

bryce-mcmath · 2025-02-04T22:37:16Z

packages/legacy/core/App/contexts/tour/tour-provider.tsx

-        (currentTour === TourID.CredentialOfferTour && credentialOfferTourSteps[index] !== undefined) ||
-        (currentTour === TourID.ProofRequestTour && proofRequestTourSteps[index] !== undefined)
-      ) {
+      if (currentTour && tours[currentTour] && tours[currentTour][index] !== undefined) {


Nice simplification! We could also add the suggested optional chaining here too to clean it up even further

bryce-mcmath · 2025-02-04T22:39:59Z

packages/legacy/core/App/contexts/tour/tour-provider.tsx

+    if (currentTour && currentStep !== undefined && tours[currentTour]) {
+      return tours[currentTour][currentStep] ?? { Render: () => <></> }
    }
-
-    stepToRender = currentStep !== undefined ? steps[currentStep] : undefined
-    return stepToRender ?? { Render: () => <></> }
-  }, [homeTourSteps, currentTour, credentialsTourSteps, credentialOfferTourSteps, proofRequestTourSteps, currentStep])
+    return { Render: () => <></> }
+  }, [currentTour, currentStep, tours])


I think with this refactoring we could simplify it further to just one conditional and won't need both returns (won't need two { Render: () => ... })

bryce-mcmath · 2025-02-04T22:42:41Z

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

@@ -203,7 +202,7 @@ const ProofRequest: React.FC<ProofRequestProps> = ({ navigation, proofId }) => {
    const shouldShowTour = enableToursConfig && store.tours.enableTours && !store.tours.seenProofRequestTour

    if (shouldShowTour && screenIsFocused) {
-      start(TourID.ProofRequestTour)
+      start('proofRequestTourSteps')


bryce-mcmath · 2025-02-04T22:44:48Z

packages/legacy/core/App/types/tour.ts

-  CredentialOfferTour,
-  ProofRequestTour,
-}
+export type TourID = `${string}TourSteps`


Can we make this an extensible type so that we can use it for constants throughout Bifold as well as extensions (like the Quebec or BC apps?)

I'm not sure what you mean by making an extensible type. Could you please explain ? Thanks!

P.S: All other comments should have been resolved, if you can review it again.

I was thinking something like this:

// in Bifold export enum BifoldTourIds { HomeTour = "homeTourSteps", ProofRequestTour = "proofRequestTourSteps" ... } // in Quebec Wallet enum QuebecTourIds { QuebecSpecificATour = "quebecSpecificATourSteps", QuebecSpecificBTour = "quebecSpecificBTourSteps" ... } // in Quebec Wallet type TourIds = BifoldTourIds | QuebecTourIds // in Quebec Wallet when you want to start a tour for a screen that's unique to Quebec start(TourIds.QuebecTourA) // in Quebec Wallet when you want to start a tour for a Bifold screen that Quebec is injecting their own custom version start(TourIds.HomeTour)

And we could then use those TourIds where you are using TourIDKeys now (I'm not picky about the naming as long as it's clear. Whether we use as const or enum doesn't really matter to me either but we should export some kind of BifoldTourIds that we can extend so we can use that constant / enum everywhere in both Bifold and Quebec Wallet (or any other Bifold based wallet.) No plain strings like tourID={'homeTourSteps'}

bryce-mcmath · 2025-02-04T22:45:04Z

packages/legacy/core/__tests__/components/AttachTourStep.test.tsx

-      >
-        <AttachTourStep index={0} tourID={TourID.HomeTour}>
+      <TourProvider tours={tours} overlayColor={'gray'} overlayOpacity={0.7}>
+        <AttachTourStep index={0} tourID={'homeTourSteps'}>


Use constant as mentioned above

bryce-mcmath · 2025-02-04T22:45:23Z

packages/legacy/core/__tests__/components/TourBox.test.tsx

        <TourBox
          title={'Title'}
          leftText={'Left'}
          rightText={'Right'}
          onLeft={previous}
          onRight={next}
-          currentTour={TourID.HomeTour}
+          currentTour={'homeTourSteps'}


bryce-mcmath · 2025-02-04T22:45:32Z

packages/legacy/core/__tests__/components/TourOverlay.test.tsx

        <TourOverlay
          color={'grey'}
          currentStep={0}
-          currentTour={TourID.HomeTour}
+          currentTour={'homeTourSteps'}


Signed-off-by: fc-santos <[email protected]>

sonarqubecloud · 2025-02-07T14:34:19Z

Quality Gate passed

Issues
1 New issue
0 Accepted issues

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

See analysis details on SonarQube Cloud

bryce-mcmath

Good improvements, added a comment above explaining the small change requested. Wasn't very clear the first time, sorry about that

#1428 (comment)

fc-santos added 2 commits February 3, 2025 08:19

feat: make tour provider tour steps dynamic

5fbea3b

Signed-off-by: fc-santos <[email protected]>

rebase with main

bc36a55

Signed-off-by: fc-santos <[email protected]>

fc-santos requested a review from a team as a code owner February 4, 2025 21:08

refactor: delete comment and update tours constant

0d1e166

Signed-off-by: fc-santos <[email protected]>

fc-santos requested a review from bryce-mcmath February 4, 2025 21:11

resolve sonar cloud issue

0f51b14

Signed-off-by: fc-santos <[email protected]>

bryce-mcmath requested changes Feb 4, 2025

View reviewed changes

fc-santos added 2 commits February 4, 2025 18:40

refactor: add TourIDKeys

f38ad45

Signed-off-by: fc-santos <[email protected]>

rebase with main

564de63

Signed-off-by: fc-santos <[email protected]>

fc-santos requested a review from bryce-mcmath February 5, 2025 15:36

rebase with main

20d77d3

Signed-off-by: fc-santos <[email protected]>

bryce-mcmath requested changes Feb 7, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: make tour provider tour steps dynamic #1428

feat: make tour provider tour steps dynamic #1428

fc-santos commented Feb 4, 2025 •

edited

Loading

fc-santos commented Feb 4, 2025

bryce-mcmath commented Feb 4, 2025

bryce-mcmath left a comment

bryce-mcmath Feb 4, 2025

fc-santos Feb 4, 2025

bryce-mcmath Feb 4, 2025

fc-santos Feb 4, 2025

bryce-mcmath Feb 4, 2025

fc-santos Feb 4, 2025

bryce-mcmath Feb 4, 2025

fc-santos Feb 4, 2025

bryce-mcmath Feb 4, 2025

bryce-mcmath Feb 4, 2025

fc-santos Feb 5, 2025

bryce-mcmath Feb 7, 2025

bryce-mcmath Feb 4, 2025

bryce-mcmath Feb 4, 2025

bryce-mcmath Feb 4, 2025

sonarqubecloud bot commented Feb 7, 2025

bryce-mcmath left a comment •

edited

Loading

feat: make tour provider tour steps dynamic #1428

Are you sure you want to change the base?

feat: make tour provider tour steps dynamic #1428

Conversation

fc-santos commented Feb 4, 2025 • edited Loading

Summary of Changes

Screenshots, videos, or gifs

Breaking change guide

Related Issues

Pull Request Checklist

fc-santos commented Feb 4, 2025

bryce-mcmath commented Feb 4, 2025

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

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

sonarqubecloud bot commented Feb 7, 2025

Quality Gate passed

bryce-mcmath left a comment • edited Loading

Choose a reason for hiding this comment

fc-santos commented Feb 4, 2025 •

edited

Loading

bryce-mcmath left a comment •

edited

Loading