feat(composer): add an account switcher #9050
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -99,7 +99,7 @@ import {STALE} from '#/state/queries' | |
| import {usePreferencesQuery} from '#/state/queries/preferences' | ||
| import {useProfilesQuery} from '#/state/queries/profile' | ||
| import {type Gif} from '#/state/queries/tenor' | ||
| import {useAgent, useSession} from '#/state/session' | ||
| import {useComposerControls} from '#/state/shell/composer' | ||
| import {type ComposerOpts, type OnPostSuccessData} from '#/state/shell/composer' | ||
| import {useLoggedOutViewControls} from '#/state/shell/logged-out' | ||
|
|
@@ -182,24 +182,46 @@ export const ComposePost = ({ | |
| }) => { | ||
| const {currentAccount, accounts} = useSession() | ||
| const defaultAgent = useAgent() | ||
| const [selectedAccountDid, setSelectedAccountDid] = useState( | ||
| currentAccount!.did, | ||
| ) | ||
|
|
||
| const {data: agent = defaultAgent} = useQuery({ | ||
| queryKey: [ | ||
| 'composer-agent', | ||
| currentAccount?.did, | ||
| currentAccount?.service, | ||
| currentAccount?.active, | ||
| selectedAccountDid, | ||
| // include account data in the query key to invalidate when tokens change | ||
| // hmm we don't want a nested array i think so spreading seems like the way to go(?) | ||
| ...(() => { | ||
| const selectedAccount = accounts.find( | ||
| acc => acc.did === selectedAccountDid, | ||
| ) | ||
| return selectedAccount | ||
| ? [ | ||
| selectedAccount.service, | ||
| selectedAccount.accessJwt, | ||
| selectedAccount.refreshJwt, | ||
| selectedAccount.active, | ||
| ] | ||
| : [] | ||
| })(), | ||
| ], | ||
| queryFn: async () => { | ||
| if (selectedAccountDid === currentAccount!.did) { | ||
| return defaultAgent | ||
| } | ||
|
|
||
| // get fresh account data from the session store | ||
| const selectedAccount = accounts.find( | ||
| acc => acc.did === selectedAccountDid, | ||
| ) | ||
| if (!selectedAccount) { | ||
| throw new Error(`Account with DID ${selectedAccountDid} not found`) | ||
| } | ||
|
|
||
| const session = new CredentialSession(new URL(selectedAccount.service)) | ||
| if ( | ||
| selectedAccount.refreshJwt && | ||
|
|
@@ -219,7 +241,7 @@ export const ComposePost = ({ | |
| }) | ||
|
|
||
| const queryClient = useQueryClient() | ||
| const currentDid = selectedAccountDid | ||
| const {closeComposer} = useComposerControls() | ||
| const {requestSwitchToAccount} = useLoggedOutViewControls() | ||
| const {_} = useLingui() | ||
|
|
@@ -254,8 +276,15 @@ export const ComposePost = ({ | |
| ) | ||
|
|
||
| const onSelectAccount = React.useCallback( | ||
|
There was a problem hiding this comment. make sure to check what happens if the async code below is slowed down to trigger a race condition of resolving in the opposite order There was a problem hiding this comment. I quickly switched from account A to B to C, then back to A. Switching accounts has a (close to) 1 second delay (might be quicker with your connection, it varied for me), with subsequent switches being a lot faster I also decided to add a 1 second setTimeout promise delay just to check if the rapid fire switch requests would be out of order. I haven't noticed that being the case There was a problem hiding this comment. Though I could probably add a condition guard or something just in case... hm There was a problem hiding this comment. to clarify, i mean that race conditions are always possible — so it’s good to find a way to trigger one reliably (eg by simulating alternating delays) and then to write the code in a way that it’s resilient to the race |
||
| async (accountDid: string) => { | ||
| if (accountDid === selectedAccountDid) { | ||
| return | ||
| } | ||
|
|
||
| // get fresh account data from session store | ||
| const account = accounts.find(acc => acc.did === accountDid) | ||
| if (!account) { | ||
| setError('Account not found') | ||
|
There was a problem hiding this comment. i'm not 100% sure about the setError statement here tbh since the codebase only uses setError with either translated strings using |
||
| return | ||
| } | ||
|
|
||
|
|
@@ -281,7 +310,7 @@ export const ComposePost = ({ | |
| currentAccount?.did, | ||
| currentAccount?.service, | ||
| currentAccount?.active, | ||
| accountDid, | ||
| account.service, | ||
| account.accessJwt, | ||
| account.refreshJwt, | ||
|
|
@@ -290,7 +319,7 @@ export const ComposePost = ({ | |
| tempAgent, | ||
| ) | ||
| // if it succeeds, update the selected account | ||
| setSelectedAccountDid(accountDid) | ||
| } catch (e: any) { | ||
| if ( | ||
| String(e.message).toLowerCase().includes('token has expired') || | ||
|
|
@@ -308,7 +337,8 @@ export const ComposePost = ({ | |
| } | ||
| }, | ||
| [ | ||
| selectedAccountDid, | ||
| accounts, | ||
| closeComposer, | ||
| requestSwitchToAccount, | ||
| _, | ||
|
|
@@ -854,7 +884,7 @@ export const ComposePost = ({ | |
| onClearVideo={clearVideo} | ||
| onPublish={onComposerPostPublish} | ||
| onError={setError} | ||
| selectedAccountDid={selectedAccountDid} | ||
| onSelectAccount={onSelectAccount} | ||
| profiles={profiles?.profiles} | ||
| /> | ||
|
|
@@ -895,7 +925,7 @@ let ComposerPost = React.memo(function ComposerPost({ | |
| onSelectVideo, | ||
| onError, | ||
| onPublish, | ||
| selectedAccountDid, | ||
| onSelectAccount, | ||
| profiles, | ||
| }: { | ||
|
|
@@ -913,8 +943,8 @@ let ComposerPost = React.memo(function ComposerPost({ | |
| onSelectVideo: (postId: string, asset: ImagePickerAsset) => void | ||
| onError: (error: string) => void | ||
| onPublish: (richtext: RichText) => void | ||
| selectedAccountDid: string | ||
| onSelectAccount: (accountDid: string) => void | ||
| profiles: AppBskyActorDefs.ProfileViewDetailed[] | undefined | ||
| }) { | ||
| const {_} = useLingui() | ||
|
|
@@ -996,7 +1026,7 @@ let ComposerPost = React.memo(function ComposerPost({ | |
| ]}> | ||
| <View style={[a.flex_row, a.align_start, isNative && a.flex_1]}> | ||
| <AccountSwitcher | ||
| selectedAccountDid={selectedAccountDid} | ||
| onSelectAccount={onSelectAccount} | ||
| profiles={profiles} | ||
| /> | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
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.
shouldn't this have a listener for session updates, any session expiry here is going to result in the session getting marked as invalid (due to stale refresh token) when you do another post (with that account), or actually switch to that account later
selectedAccount should probably be a DID, instead of passing the entire Account object in
Uh oh!
There was an error while loading. Please reload this page.
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.
ya good point, i just quickly put things together to get the switcher working in the composer but now that i look at it its redundant
Uh oh!
There was an error while loading. Please reload this page.
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.
@mary-ext done. you can check out commits 1ccdbcc and 06aec44