From b86c3b486f2e126fe887733c9e8ef6856cd67e91 Mon Sep 17 00:00:00 2001
From: dan <dan.abramov@gmail.com>
Date: Thu, 2 May 2024 21:55:50 +0100
Subject: [PATCH] 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
---
 src/components/AccountList.tsx           |  15 +--
 src/components/dialogs/SwitchAccount.tsx |   4 +-
 src/lib/hooks/useAccountSwitcher.ts      |  22 +++--
 src/screens/Login/ChooseAccountForm.tsx  |  22 ++++-
 src/view/screens/Settings/index.tsx      | 121 ++++++++++++-----------
 5 files changed, 105 insertions(+), 79 deletions(-)

diff --git a/src/components/AccountList.tsx b/src/components/AccountList.tsx
index 76596df3..7d696801 100644
--- a/src/components/AccountList.tsx
+++ b/src/components/AccountList.tsx
@@ -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} />
diff --git a/src/components/dialogs/SwitchAccount.tsx b/src/components/dialogs/SwitchAccount.tsx
index 18fc55aa..0bd4bcb8 100644
--- a/src/components/dialogs/SwitchAccount.tsx
+++ b/src/components/dialogs/SwitchAccount.tsx
@@ -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>
diff --git a/src/lib/hooks/useAccountSwitcher.ts b/src/lib/hooks/useAccountSwitcher.ts
index d4e02695..de50e533 100644
--- a/src/lib/hooks/useAccountSwitcher.ts
+++ b/src/lib/hooks/useAccountSwitcher.ts
@@ -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}
 }
diff --git a/src/screens/Login/ChooseAccountForm.tsx b/src/screens/Login/ChooseAccountForm.tsx
index 8a58ac03..e097e0db 100644
--- a/src/screens/Login/ChooseAccountForm.tsx
+++ b/src/screens/Login/ChooseAccountForm.tsx
@@ -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]}>
diff --git a/src/view/screens/Settings/index.tsx b/src/view/screens/Settings/index.tsx
index b3313f60..2e4e8c10 100644
--- a/src/view/screens/Settings/index.tsx
+++ b/src/view/screens/Settings/index.tsx
@@ -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]}>