fix: allows OTP to be approved from a cold start #1256
Conversation
|
Preview available at https://expo.dev/accounts/nearform/projects/optic-expo/updates/4648bf54-ef79-47cc-9504-970895943719 |
|
This is currently not working on IOS |
There was a problem hiding this comment.
The gut feeling when looking at this change is that there should be a more straightforward way to achieve what we want. The assumption is that we're not doing anything fancy with push notifications, we just need to open the app when we get one, open the app in a specific "state".
Hence, I have a feeling that if it requires a solution like this one, which is moderately complex and error prone, there's a good chance we're doing something wrong.
On the other hand I don't have a solution either, but I would try to spend some time trying to understand what the root problem is rather than trying patching the issue.
|
I have done some more research for alternative solutions since learning that this approach is possibly non-viable for IOS, I have a lead for a solution that feels less hacky, by using the 
|
|
Good lead Will, worth investigating |
|
The problem currently is that the notification is consumed to open the app, before the listeners are active, hopefully the |
|
I am not terribly familiar with how this issue manifests itself, but in the app we have a notifications section. If we managed to go in there and keep the notification visible in the list, it may be an acceptable compromise. But I'm just making this up now, let's definitely investigate what seems to be a more appropriate solution |
|
In the case of both adding a notification to the list and displaying the 2FA challenge, there is the same difficulty of having to 'catch' the notification before the app starts up, or it gets lost. My feeling is that such a compromise likely won't save much effort. I will see how this listener behaves before jumping to conclusions |
|
Upon investigating, it seems like this implementation is the same as what we currently have. The docs would suggest that this implementation should work for our needs but it seems like this is not the case, see: |
src/context/InitalLoadingContext.tsx
Outdated
|
|
||
| return ( | ||
| <InitialLoadingContext.Provider | ||
| value={{ initialLoadingComplete, markInitialLoadingComplete }} |
There was a problem hiding this comment.
prefer memoizing this object, otherwise each render of this component will create a new object, which will cause a rerender of all users of this context, which is unnecessary
There was a problem hiding this comment.
resolved in 9a2f856
Adds a conditional to the notification listener useEffect to use the necessary callbacks when a OTP request is pressed whilst the app is closed.
closes: #1155