Improve account switcher pending state (#3827)

* Protect against races

* Reduce UI jank when switching accounts

* Add pending state to selected account

* Disable presses while pending
zio/stable
dan 2024-05-02 21:55:50 +01:00 committed by GitHub
parent 8ba1b10ce0
commit b86c3b486f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 105 additions and 79 deletions

View File

@ -16,12 +16,12 @@ export function AccountList({
onSelectAccount,
onSelectOther,
otherLabel,
isSwitchingAccounts,
pendingDid,
}: {
onSelectAccount: (account: SessionAccount) => void
onSelectOther: () => void
otherLabel?: string
isSwitchingAccounts: boolean
pendingDid: string | null
}) {
const {currentAccount, accounts} = useSession()
const t = useTheme()
@ -33,6 +33,7 @@ export function AccountList({
return (
<View
pointerEvents={pendingDid ? 'none' : 'auto'}
style={[
a.rounded_md,
a.overflow_hidden,
@ -45,6 +46,7 @@ export function AccountList({
account={account}
onSelect={onSelectAccount}
isCurrentAccount={account.did === currentAccount?.did}
isPendingAccount={account.did === pendingDid}
/>
<View style={[a.border_b, t.atoms.border_contrast_low]} />
</React.Fragment>
@ -52,7 +54,7 @@ export function AccountList({
<Button
testID="chooseAddAccountBtn"
style={[a.flex_1]}
onPress={isSwitchingAccounts ? undefined : onPressAddAccount}
onPress={pendingDid ? undefined : onPressAddAccount}
label={_(msg`Login to account that is not listed`)}>
{({hovered, pressed}) => (
<View
@ -61,8 +63,7 @@ export function AccountList({
a.flex_row,
a.align_center,
{height: 48},
(hovered || pressed || isSwitchingAccounts) &&
t.atoms.bg_contrast_25,
(hovered || pressed) && t.atoms.bg_contrast_25,
]}>
<Text
style={[
@ -86,10 +87,12 @@ function AccountItem({
account,
onSelect,
isCurrentAccount,
isPendingAccount,
}: {
account: SessionAccount
onSelect: (account: SessionAccount) => void
isCurrentAccount: boolean
isPendingAccount: boolean
}) {
const t = useTheme()
const {_} = useLingui()
@ -117,7 +120,7 @@ function AccountItem({
a.flex_row,
a.align_center,
{height: 48},
(hovered || pressed) && t.atoms.bg_contrast_25,
(hovered || pressed || isPendingAccount) && t.atoms.bg_contrast_25,
]}>
<View style={a.p_md}>
<UserAvatar avatar={profile?.avatar} size={24} />

View File

@ -18,7 +18,7 @@ export function SwitchAccountDialog({
}) {
const {_} = useLingui()
const {currentAccount} = useSession()
const {onPressSwitchAccount, isSwitchingAccounts} = useAccountSwitcher()
const {onPressSwitchAccount, pendingDid} = useAccountSwitcher()
const {setShowLoggedOut} = useLoggedOutViewControls()
const onSelectAccount = useCallback(
@ -54,7 +54,7 @@ export function SwitchAccountDialog({
onSelectAccount={onSelectAccount}
onSelectOther={onPressAddAccount}
otherLabel={_(msg`Add account`)}
isSwitchingAccounts={isSwitchingAccounts}
pendingDid={pendingDid}
/>
</View>
</Dialog.ScrollableInner>

View File

@ -12,7 +12,7 @@ import {logEvent} from '../statsig/statsig'
import {LogEvents} from '../statsig/statsig'
export function useAccountSwitcher() {
const [isSwitchingAccounts, setIsSwitchingAccounts] = useState(false)
const [pendingDid, setPendingDid] = useState<string | null>(null)
const {_} = useLingui()
const {track} = useAnalytics()
const {initSession, clearCurrentAccount} = useSessionApi()
@ -24,9 +24,12 @@ export function useAccountSwitcher() {
logContext: LogEvents['account:loggedIn']['logContext'],
) => {
track('Settings:SwitchAccountButtonClicked')
if (pendingDid) {
// The session API isn't resilient to race conditions so let's just ignore this.
return
}
try {
setIsSwitchingAccounts(true)
setPendingDid(account.did)
if (account.accessJwt) {
if (isWeb) {
// We're switching accounts, which remounts the entire app.
@ -57,11 +60,18 @@ export function useAccountSwitcher() {
Toast.show(_(msg`Sorry! We need you to enter your password.`))
}, 100)
} finally {
setIsSwitchingAccounts(false)
setPendingDid(null)
}
},
[_, track, clearCurrentAccount, initSession, requestSwitchToAccount],
[
_,
track,
clearCurrentAccount,
initSession,
requestSwitchToAccount,
pendingDid,
],
)
return {onPressSwitchAccount, isSwitchingAccounts}
return {onPressSwitchAccount, pendingDid}
}

View File

@ -22,7 +22,7 @@ export const ChooseAccountForm = ({
onSelectAccount: (account?: SessionAccount) => void
onPressBack: () => void
}) => {
const [isSwitchingAccounts, setIsSwitchingAccounts] = React.useState(false)
const [pendingDid, setPendingDid] = React.useState<string | null>(null)
const {track, screen} = useAnalytics()
const {_} = useLingui()
const {currentAccount} = useSession()
@ -35,13 +35,17 @@ export const ChooseAccountForm = ({
const onSelect = React.useCallback(
async (account: SessionAccount) => {
if (pendingDid) {
// The session API isn't resilient to race conditions so let's just ignore this.
return
}
if (account.accessJwt) {
if (account.did === currentAccount?.did) {
setShowLoggedOut(false)
Toast.show(_(msg`Already signed in as @${account.handle}`))
} else {
try {
setIsSwitchingAccounts(true)
setPendingDid(account.did)
await initSession(account)
logEvent('account:loggedIn', {
logContext: 'ChooseAccountForm',
@ -57,14 +61,22 @@ export const ChooseAccountForm = ({
})
onSelectAccount(account)
} finally {
setIsSwitchingAccounts(false)
setPendingDid(null)
}
}
} else {
onSelectAccount(account)
}
},
[currentAccount, track, initSession, onSelectAccount, setShowLoggedOut, _],
[
currentAccount,
track,
initSession,
pendingDid,
onSelectAccount,
setShowLoggedOut,
_,
],
)
return (
@ -78,7 +90,7 @@ export const ChooseAccountForm = ({
<AccountList
onSelectAccount={onSelect}
onSelectOther={() => onSelectAccount()}
isSwitchingAccounts={isSwitchingAccounts}
pendingDid={pendingDid}
/>
</View>
<View style={[a.flex_row]}>

View File

@ -1,6 +1,5 @@
import React from 'react'
import {
ActivityIndicator,
Linking,
Platform,
Pressable,
@ -63,6 +62,7 @@ import * as Toast from 'view/com/util/Toast'
import {UserAvatar} from 'view/com/util/UserAvatar'
import {ScrollView} from 'view/com/util/Views'
import {useDmServiceUrlStorage} from '#/screens/Messages/Temp/useDmServiceUrlStorage'
import {useTheme} from '#/alf'
import {useDialogControl} from '#/components/Dialog'
import {BirthDateSettingsDialog} from '#/components/dialogs/BirthDateSettings'
import * as TextField from '#/components/forms/TextField'
@ -72,11 +72,11 @@ import {ExportCarDialog} from './ExportCarDialog'
function SettingsAccountCard({
account,
isSwitchingAccounts,
pendingDid,
onPressSwitchAccount,
}: {
account: SessionAccount
isSwitchingAccounts: boolean
pendingDid: string | null
onPressSwitchAccount: (
account: SessionAccount,
logContext: 'Settings',
@ -84,13 +84,19 @@ function SettingsAccountCard({
}) {
const pal = usePalette('default')
const {_} = useLingui()
const t = useTheme()
const {currentAccount} = useSession()
const {logout} = useSessionApi()
const {data: profile} = useProfileQuery({did: account.did})
const isCurrentAccount = account.did === currentAccount?.did
const contents = (
<View style={[pal.view, styles.linkCard]}>
<View
style={[
pal.view,
styles.linkCard,
account.did === pendingDid && t.atoms.bg_contrast_25,
]}>
<View style={styles.avi}>
<UserAvatar
size={40}
@ -122,7 +128,8 @@ function SettingsAccountCard({
}}
accessibilityRole="button"
accessibilityLabel={_(msg`Sign out`)}
accessibilityHint={`Signs ${profile?.displayName} out of Bluesky`}>
accessibilityHint={`Signs ${profile?.displayName} out of Bluesky`}
activeOpacity={0.8}>
<Text type="lg" style={pal.link}>
<Trans>Sign out</Trans>
</Text>
@ -148,13 +155,12 @@ function SettingsAccountCard({
testID={`switchToAccountBtn-${account.handle}`}
key={account.did}
onPress={
isSwitchingAccounts
? undefined
: () => onPressSwitchAccount(account, 'Settings')
pendingDid ? undefined : () => onPressSwitchAccount(account, 'Settings')
}
accessibilityRole="button"
accessibilityLabel={_(msg`Switch to ${account.handle}`)}
accessibilityHint={_(msg`Switches the account you are logged in to`)}>
accessibilityHint={_(msg`Switches the account you are logged in to`)}
activeOpacity={0.8}>
{contents}
</TouchableOpacity>
)
@ -181,7 +187,8 @@ export function SettingsScreen({}: Props) {
const closeAllActiveElements = useCloseAllActiveElements()
const exportCarControl = useDialogControl()
const birthdayControl = useDialogControl()
const {isSwitchingAccounts, onPressSwitchAccount} = useAccountSwitcher()
const {pendingDid, onPressSwitchAccount} = useAccountSwitcher()
const isSwitchingAccounts = !!pendingDid
// TODO: TEMP REMOVE WHEN CLOPS ARE RELEASED
const gate = useGate()
@ -382,60 +389,54 @@ export function SettingsScreen({}: Props) {
<View style={styles.spacer20} />
{!currentAccount.emailConfirmed && <EmailConfirmationNotice />}
<View style={[s.flexRow, styles.heading]}>
<Text type="xl-bold" style={pal.text}>
<Trans>Signed in as</Trans>
</Text>
<View style={s.flex1} />
</View>
<View pointerEvents={pendingDid ? 'none' : 'auto'}>
<SettingsAccountCard
account={currentAccount}
onPressSwitchAccount={onPressSwitchAccount}
pendingDid={pendingDid}
/>
</View>
</>
) : null}
<View style={[s.flexRow, styles.heading]}>
<Text type="xl-bold" style={pal.text}>
<Trans>Signed in as</Trans>
</Text>
<View style={s.flex1} />
<View pointerEvents={pendingDid ? 'none' : 'auto'}>
{accounts
.filter(a => a.did !== currentAccount?.did)
.map(account => (
<SettingsAccountCard
key={account.did}
account={account}
onPressSwitchAccount={onPressSwitchAccount}
pendingDid={pendingDid}
/>
))}
<TouchableOpacity
testID="switchToNewAccountBtn"
style={[styles.linkCard, pal.view]}
onPress={isSwitchingAccounts ? undefined : onPressAddAccount}
accessibilityRole="button"
accessibilityLabel={_(msg`Add account`)}
accessibilityHint={_(msg`Create a new Bluesky account`)}>
<View style={[styles.iconContainer, pal.btn]}>
<FontAwesomeIcon
icon="plus"
style={pal.text as FontAwesomeIconStyle}
/>
</View>
<Text type="lg" style={pal.text}>
<Trans>Add account</Trans>
</Text>
</TouchableOpacity>
</View>
{isSwitchingAccounts ? (
<View style={[pal.view, styles.linkCard]}>
<ActivityIndicator />
</View>
) : (
<SettingsAccountCard
account={currentAccount!}
onPressSwitchAccount={onPressSwitchAccount}
isSwitchingAccounts={isSwitchingAccounts}
/>
)}
{accounts
.filter(a => a.did !== currentAccount?.did)
.map(account => (
<SettingsAccountCard
key={account.did}
account={account}
onPressSwitchAccount={onPressSwitchAccount}
isSwitchingAccounts={isSwitchingAccounts}
/>
))}
<TouchableOpacity
testID="switchToNewAccountBtn"
style={[
styles.linkCard,
pal.view,
isSwitchingAccounts && styles.dimmed,
]}
onPress={isSwitchingAccounts ? undefined : onPressAddAccount}
accessibilityRole="button"
accessibilityLabel={_(msg`Add account`)}
accessibilityHint={_(msg`Create a new Bluesky account`)}>
<View style={[styles.iconContainer, pal.btn]}>
<FontAwesomeIcon
icon="plus"
style={pal.text as FontAwesomeIconStyle}
/>
</View>
<Text type="lg" style={pal.text}>
<Trans>Add account</Trans>
</Text>
</TouchableOpacity>
<View style={styles.spacer20} />
<Text type="xl-bold" style={[pal.text, styles.heading]}>