-
Notifications
You must be signed in to change notification settings - Fork 1.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Move onboarding state to new persistence + reducer context #1835
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dig this pattern
@@ -9,6 +9,7 @@ import {cleanError} from 'lib/strings/errors' | |||
import {getAge} from 'lib/strings/time' | |||
import {track} from 'lib/analytics/analytics' | |||
import {logger} from '#/logger' | |||
import {DispatchContext as OnboardingDispatchContext} from '#/state/shell/onboarding' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Appreciate the rename 🙌
handle: z.string().optional(), | ||
displayName: z.string().optional(), | ||
aviUrl: z.string().optional(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If our plan is to fetch this data to render screens that need it, should we just remove these entirely and only store the bare minimum in storage?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that might be better yeah
src/state/shell/onboarding.tsx
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When we update onboarding, we should be able to localize this state a little more since the rest of the app doesn't really need to be aware of onboarding state, aside from the kinda boolean "is onboarding complete/active or not" we may need elsewhere
987614f
to
3893acc
Compare
…-state-into-separate-statecontext * origin: Move invite-state to new persistence + context and replace the notifications with just showing uses in the modal (#1840) Move muted threads to new persistence + context (#1838) Move onboarding state to new persistence + reducer context (#1835) Move require alt-text to new persistence + context (#1839) Move reminders to new persisted state layer (#1834) Add persistent state provider (#1830)
Refactors onboarding state away from the mobx model and into reducer contexts which use the new persistence layer