fix: deep links #1327
fix: deep links #1327
Conversation
Signed-off-by: Bryce McMath <[email protected]>
Signed-off-by: Bryce McMath <[email protected]>
Signed-off-by: Bryce McMath <[email protected]>
|
| @@ -171,14 +171,13 @@ export class MainContainer implements Container { | |||
| loadState<StoreOnboardingState>(LocalStorageKeys.Onboarding, (val) => (onboarding = val)), | |||
| ]) | |||
|
|
|||
| const state: State = { | |||
| ...defaultState, | |||
There was a problem hiding this comment.
Having the defaultState spread on state load was sometimes causing the newest values to be overwritten if it got called after state had changed. ie. didAuthenticate gets set to true and then loadState gets called and it gets overwritten with false.
| loginAttempt: { ...defaultState.loginAttempt, ...loginAttempt }, | ||
| preferences: { ...defaultState.preferences, ...preferences }, | ||
| migration: { ...defaultState.migration, ...migration }, | ||
| tours: { ...defaultState.tours, ...tours }, | ||
| onboarding: { ...defaultState.onboarding, ...onboarding }, | ||
| } | ||
| } as State |
There was a problem hiding this comment.
The typing change here takes it from being "must be a complete state object" to "must be a subset of state type"
| @@ -67,7 +67,6 @@ export const defaultState: State = { | |||
| seenCredentialOfferTour: false, | |||
| seenProofRequestTour: false, | |||
| }, | |||
| loading: false, | |||
There was a problem hiding this comment.
this was leftover unused code
| const ready = useMemo( | ||
| () => store.authentication.didAuthenticate && ['active', 'inactive'].includes(appStateStatus), | ||
| [store.authentication.didAuthenticate, appStateStatus] | ||
| ) |
There was a problem hiding this comment.
basically checking "are you logged in and in the foreground" before we attempt to handle a deeplink
| if (agent) { | ||
| await restartExistingAgent() | ||
| return agent | ||
| } | ||
|
|
There was a problem hiding this comment.
Starting a new agent every time we log in fixed some issues with improper teardown/restart. My opinion is that until agent.shutdown can reliably finish completely, we shouldn't attempt to restart the agent between logins.
| // handle deeplink events | ||
| useEffect(() => { | ||
| async function handleDeepLink(deepLink: string) { |
There was a problem hiding this comment.
Handling the deeplinks from the TabStack rather than the Rootstack prevented attempts to navigate to the Connection screen before the current stack and navigation is ready
| if (currentState === 'background') { | ||
| agent.mediationRecipient | ||
| .stopMessagePickup() | ||
| .then(() => { | ||
| logger.info('Stopped agent message pickup') | ||
| }) | ||
| .catch((err) => { | ||
| logger.error(`Error stopping agent message pickup, ${err}`) | ||
| }) | ||
|
|
||
| return | ||
| } | ||
|
|
||
| if (currentState === 'active') { | ||
| agent.mediationRecipient | ||
| .initiateMessagePickup() | ||
| .then(() => { | ||
| logger.info('Resuming agent message pickup') | ||
| }) | ||
| .catch((err) => { | ||
| logger.error(`Error resuming agent message pickup, ${err}`) | ||
| }) |
There was a problem hiding this comment.
Centralized this logic by moving it to the ActivityContext so we're only using AppState listeners in one place
| // stop the tour when the screen unmounts | ||
| useEffect(() => { | ||
| return stop | ||
| }, [stop]) |
There was a problem hiding this comment.
These cleanups are for if the deeplink gets handled when a tour is active, we stop the tour when we navigate away to the connection screen.
| @@ -11,7 +11,7 @@ export const AttestationEventTypes = { | |||
| export interface AttestationMonitor { | |||
| readonly attestationWorkflowInProgress: boolean | |||
| shouldHandleProofRequestAutomatically: boolean | |||
| start(agent: Agent): Promise<void> | |||
There was a problem hiding this comment.
Was unnecessarily a promise (had no asynchronous calls) so I just cleaned this up a bit
| @@ -68,7 +68,6 @@ export interface State { | |||
| tours: Tours | |||
| deepLink?: string | |||
| migration: Migration | |||
| loading: boolean | |||
There was a problem hiding this comment.
Again, leftover unused code
| @@ -86,120 +76,6 @@ const RootStack: React.FC = () => { | |||
| }) | |||
| }, [dispatch, loadState, t]) | |||
|
|
|||
| // handle deeplink events | |||
Summary of Changes
This PR will fix the deeplink issues (and may also fix the one-message-at-a-time after backgrounding issue)
Related Issues
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.
Signed-off-byline (we use the DCO GitHub app to enforce this);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 🤓
PR template adapted from the Python attrs project.