Get sign in/out working end to end#1739
Conversation
|
Preview this change https://demo.audius.co/rj-signinout |
1 similar comment
|
Preview this change https://demo.audius.co/rj-signinout |
sliptype
left a comment
There was a problem hiding this comment.
Looks good! your thinking makes sense, we are gonna have to have the "mirroring" until we drop the webview entirely and then can get rid of it
Also I'm on board with keeping the sagas enabled, but let's keep an eye on perf
| const sagaMiddleware = createSagaMiddleware({ context: storeContext }) | ||
| const middlewares = applyMiddleware(sagaMiddleware) | ||
| const composeEnhancers = composeWithDevTools({ trace: true, traceLimit: 25 }) | ||
| const composeEnhancers = composeWithDevTools({ trace: true, traceLimit: 250 }) |
|
|
||
| const signedIn = signOnStatus === 'success' | ||
| const isSigninError = passwordFieldValue.error | ||
| const emailIsAvailable = emailFieldValue.error !== 'inUse' |
There was a problem hiding this comment.
Is there a reason we aren't using selectors for these anymore? If so can we delete the selectors?
There was a problem hiding this comment.
these are derived values from the result of the selector
| // config | ||
| ...backendSagas(), | ||
| // ...accountSagas(), | ||
| ...accountSagas(), |
There was a problem hiding this comment.
Did you notice any worse perf with everything enabled like this?
There was a problem hiding this comment.
Across the app no. There's a slight hiccup when pressing the sign in button, but i think fix-forward makes sense for it. Can disable if it's a bad call once it's on the staging app
dylanjeffers
left a comment
There was a problem hiding this comment.
awesome changes man, this is happening
|
|
||
| const handleSignOut = useCallback(() => { | ||
| dispatchWeb(signOut) | ||
| dispatch(signOut({})) |
There was a problem hiding this comment.
just wondering why we need empty args on this one
There was a problem hiding this comment.
ts was complaining b/c it's an action creator vs. some js garbo actions. both technically work, but this is more "correct" and I didn't want to change the other
| const sagaMiddleware = createSagaMiddleware({ context: storeContext }) | ||
| const middlewares = applyMiddleware(sagaMiddleware) | ||
| const composeEnhancers = composeWithDevTools({ trace: true, traceLimit: 25 }) | ||
| const composeEnhancers = composeWithDevTools({ trace: true, traceLimit: 250 }) |
| ) => {}, | ||
| showPushNotificationConfirmation: () => {} | ||
| showPushNotificationConfirmation: () => {}, | ||
| resetAccount: () => { |
| const audiusBackendInstance = yield* getContext('audiusBackendInstance') | ||
| const localStorage = yield* getContext('localStorage') | ||
| yield takeLatest(signOutAction.type, function* () { | ||
| yield put(resetAccount()) |
There was a problem hiding this comment.
was this just a bug you were seeing overall?
There was a problem hiding this comment.
yeah, present on web too, but less impactful
a51ed9b to
5ced15b
Compare
|
Preview this change https://demo.audius.co/rj-signinout |
[8caef33] [PAY-564] Enable LayoutAnimation on android for profile screen (#1754) Reed [03c2e3d] [C-860] Fix failing mac ci build due to peer deps (#1749) Sebastian Klingler [a88f8c0] Move Coinbase button into its own component (#1741) Marcus Pasell [6c204c6] [C-859] Omit `is_verified` from metadata that gets stored in user cache from fetchCID (#1750) Sebastian Klingler [03eba06] Add bounds to $AUDIO purchase (#1744) Marcus Pasell [885202d] Bump iOS build (#1748) Michael Piazza [d035b6d] v1.3.0 (#1740) Sebastian Klingler [53128be] Update @audius/sdk to 0.0.37 (#1747) Sebastian Klingler [992d243] Fix twitter share copy (#1746) Michael Piazza [f783bf6] [PAY-540] Add analytics to buy audio flow (#1673) Marcus Pasell [898fc0a] Get sign in/out working end to end (#1739) Raymond Jacobson [94686ef] Bump ios version (#1742) Sebastian Klingler
Description
Ended up struggling to test sign up stuff without sign out, so I went ahead in that direction. Hopefully didn't steal too much.
Curious for thoughts here. I believe this is "shippable" with the sagas on mobile enabled at this point. That might save us some pain of having everything be off & then on while we're working.
This is all a good learning exercise for me. I'm starting to get the feeling that we're doing an "in then back out" approach to things.
Maybe this was already obvious to folks...
Simulator.Screen.Recording.-.iPhone.13.-.2022-08-18.at.00.29.13.mp4
Dragons
Is there anything the reviewer should be on the lookout for? Are there any dangerous changes?
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide repro instructions & any configuration.
Local simulator
How will this change be monitored?
For features that are critical or could fail silently please describe the monitoring/alerting being added.
Feature Flags
Are all new features properly feature flagged? Describe added feature flags.