From b6e515c664d51ffe357c3562fd514301805ade8c Mon Sep 17 00:00:00 2001 From: dan Date: Thu, 15 Aug 2024 20:58:13 +0100 Subject: [PATCH] Move global "Sign out" out of the current account row (#4941) * Rename logout to logoutEveryAccount * Add logoutCurrentAccount() * Make all "Log out" buttons refer to current account Each of these usages is completely contextual and refers to a specific account. * Add Sign out of all accounts to Settings * Move single account Sign Out below as well * Prompt on account removal * Add Other Accounts header to reduce ambiguity * Spacing fix --------- Co-authored-by: Paul Frazee --- src/lib/statsig/events.ts | 1 + src/screens/Deactivated.tsx | 6 +- .../components/DeactivateAccountDialog.tsx | 6 +- src/screens/SignupQueued.tsx | 6 +- src/state/session/__tests__/session-test.ts | 103 +++++++++++++++++- src/state/session/index.tsx | 38 ++++++- src/state/session/reducer.ts | 23 +++- src/state/session/types.ts | 12 +- src/view/com/testing/TestCtrls.e2e.tsx | 4 +- src/view/com/util/AccountDropdownBtn.tsx | 60 ++++++---- src/view/screens/Settings/index.tsx | 65 ++++++----- 11 files changed, 247 insertions(+), 77 deletions(-) diff --git a/src/lib/statsig/events.ts b/src/lib/statsig/events.ts index 9a427ad4..7ef0c9e2 100644 --- a/src/lib/statsig/events.ts +++ b/src/lib/statsig/events.ts @@ -14,6 +14,7 @@ export type LogEvents = { } 'account:loggedOut': { logContext: 'SwitchAccount' | 'Settings' | 'SignupQueued' | 'Deactivated' + scope: 'current' | 'every' } 'notifications:openApp': {} 'notifications:request': { diff --git a/src/screens/Deactivated.tsx b/src/screens/Deactivated.tsx index add550f9..997fe419 100644 --- a/src/screens/Deactivated.tsx +++ b/src/screens/Deactivated.tsx @@ -38,7 +38,7 @@ export function Deactivated() { const {setShowLoggedOut} = useLoggedOutViewControls() const hasOtherAccounts = accounts.length > 1 const setMinimalShellMode = useSetMinimalShellMode() - const {logout} = useSessionApi() + const {logoutCurrentAccount} = useSessionApi() const agent = useAgent() const [pending, setPending] = React.useState(false) const [error, setError] = React.useState() @@ -72,8 +72,8 @@ export function Deactivated() { // So we change the URL ourselves. The navigator will pick it up on remount. history.pushState(null, '', '/') } - logout('Deactivated') - }, [logout]) + logoutCurrentAccount('Deactivated') + }, [logoutCurrentAccount]) const handleActivate = React.useCallback(async () => { try { diff --git a/src/screens/Settings/components/DeactivateAccountDialog.tsx b/src/screens/Settings/components/DeactivateAccountDialog.tsx index 99999d06..2be42d13 100644 --- a/src/screens/Settings/components/DeactivateAccountDialog.tsx +++ b/src/screens/Settings/components/DeactivateAccountDialog.tsx @@ -35,7 +35,7 @@ function DeactivateAccountDialogInner({ const {gtMobile} = useBreakpoints() const {_} = useLingui() const agent = useAgent() - const {logout} = useSessionApi() + const {logoutCurrentAccount} = useSessionApi() const [pending, setPending] = React.useState(false) const [error, setError] = React.useState() @@ -44,7 +44,7 @@ function DeactivateAccountDialogInner({ setPending(true) await agent.com.atproto.server.deactivateAccount({}) control.close(() => { - logout('Deactivated') + logoutCurrentAccount('Deactivated') }) } catch (e: any) { switch (e.message) { @@ -66,7 +66,7 @@ function DeactivateAccountDialogInner({ } finally { setPending(false) } - }, [agent, control, logout, _, setPending]) + }, [agent, control, logoutCurrentAccount, _, setPending]) return ( <> diff --git a/src/screens/SignupQueued.tsx b/src/screens/SignupQueued.tsx index 69ef9361..e7336569 100644 --- a/src/screens/SignupQueued.tsx +++ b/src/screens/SignupQueued.tsx @@ -23,7 +23,7 @@ export function SignupQueued() { const insets = useSafeAreaInsets() const {gtMobile} = useBreakpoints() const onboardingDispatch = useOnboardingDispatch() - const {logout} = useSessionApi() + const {logoutCurrentAccount} = useSessionApi() const agent = useAgent() const [isProcessing, setProcessing] = React.useState(false) @@ -153,7 +153,7 @@ export function SignupQueued() { variant="ghost" size="large" label={_(msg`Log out`)} - onPress={() => logout('SignupQueued')}> + onPress={() => logoutCurrentAccount('SignupQueued')}> Log out @@ -182,7 +182,7 @@ export function SignupQueued() { variant="ghost" size="large" label={_(msg`Log out`)} - onPress={() => logout('SignupQueued')}> + onPress={() => logoutCurrentAccount('SignupQueued')}> Log out diff --git a/src/state/session/__tests__/session-test.ts b/src/state/session/__tests__/session-test.ts index cb4c6a35..3e22c262 100644 --- a/src/state/session/__tests__/session-test.ts +++ b/src/state/session/__tests__/session-test.ts @@ -76,7 +76,7 @@ describe('session', () => { state = run(state, [ { - type: 'logged-out', + type: 'logged-out-every-account', }, ]) // Should keep the account but clear out the tokens. @@ -372,7 +372,7 @@ describe('session', () => { state = run(state, [ { // Log everyone out. - type: 'logged-out', + type: 'logged-out-every-account', }, ]) expect(state.accounts.length).toBe(3) @@ -466,7 +466,7 @@ describe('session', () => { state = run(state, [ { - type: 'logged-out', + type: 'logged-out-every-account', }, ]) expect(state.accounts.length).toBe(1) @@ -674,6 +674,103 @@ describe('session', () => { expect(state.currentAgentState.did).toBe(undefined) }) + it('can log out of the current account', () => { + let state = getInitialState([]) + + const agent1 = new BskyAgent({service: 'https://alice.com'}) + agent1.sessionManager.session = { + active: true, + did: 'alice-did', + handle: 'alice.test', + accessJwt: 'alice-access-jwt-1', + refreshJwt: 'alice-refresh-jwt-1', + } + state = run(state, [ + { + type: 'switched-to-account', + newAgent: agent1, + newAccount: agentToSessionAccountOrThrow(agent1), + }, + ]) + expect(state.accounts.length).toBe(1) + expect(state.accounts[0].accessJwt).toBe('alice-access-jwt-1') + expect(state.accounts[0].refreshJwt).toBe('alice-refresh-jwt-1') + expect(state.currentAgentState.did).toBe('alice-did') + + const agent2 = new BskyAgent({service: 'https://bob.com'}) + agent2.sessionManager.session = { + active: true, + did: 'bob-did', + handle: 'bob.test', + accessJwt: 'bob-access-jwt-1', + refreshJwt: 'bob-refresh-jwt-1', + } + state = run(state, [ + { + type: 'switched-to-account', + newAgent: agent2, + newAccount: agentToSessionAccountOrThrow(agent2), + }, + ]) + expect(state.accounts.length).toBe(2) + expect(state.accounts[0].accessJwt).toBe('bob-access-jwt-1') + expect(state.accounts[0].refreshJwt).toBe('bob-refresh-jwt-1') + expect(state.currentAgentState.did).toBe('bob-did') + + state = run(state, [ + { + type: 'logged-out-current-account', + }, + ]) + expect(state.accounts.length).toBe(2) + expect(state.accounts[0].accessJwt).toBe(undefined) + expect(state.accounts[0].refreshJwt).toBe(undefined) + expect(state.accounts[1].accessJwt).toBe('alice-access-jwt-1') + expect(state.accounts[1].refreshJwt).toBe('alice-refresh-jwt-1') + expect(state.currentAgentState.did).toBe(undefined) + expect(printState(state)).toMatchInlineSnapshot(` + { + "accounts": [ + { + "accessJwt": undefined, + "active": true, + "did": "bob-did", + "email": undefined, + "emailAuthFactor": false, + "emailConfirmed": false, + "handle": "bob.test", + "pdsUrl": undefined, + "refreshJwt": undefined, + "service": "https://bob.com/", + "signupQueued": false, + "status": undefined, + }, + { + "accessJwt": "alice-access-jwt-1", + "active": true, + "did": "alice-did", + "email": undefined, + "emailAuthFactor": false, + "emailConfirmed": false, + "handle": "alice.test", + "pdsUrl": undefined, + "refreshJwt": "alice-refresh-jwt-1", + "service": "https://alice.com/", + "signupQueued": false, + "status": undefined, + }, + ], + "currentAgentState": { + "agent": { + "service": "https://public.api.bsky.app/", + }, + "did": undefined, + }, + "needsPersist": true, + } + `) + }) + it('updates stored account with refreshed tokens', () => { let state = getInitialState([]) diff --git a/src/state/session/index.tsx b/src/state/session/index.tsx index ba12f4ea..21fe7f75 100644 --- a/src/state/session/index.tsx +++ b/src/state/session/index.tsx @@ -35,7 +35,8 @@ const AgentContext = React.createContext(null) const ApiContext = React.createContext({ createAccount: async () => {}, login: async () => {}, - logout: async () => {}, + logoutCurrentAccount: async () => {}, + logoutEveryAccount: async () => {}, resumeSession: async () => {}, removeAccount: () => {}, }) @@ -115,14 +116,31 @@ export function Provider({children}: React.PropsWithChildren<{}>) { [onAgentSessionChange, cancelPendingTask], ) - const logout = React.useCallback( + const logoutCurrentAccount = React.useCallback< + SessionApiContext['logoutEveryAccount'] + >( logContext => { addSessionDebugLog({type: 'method:start', method: 'logout'}) cancelPendingTask() dispatch({ - type: 'logged-out', + type: 'logged-out-current-account', }) - logEvent('account:loggedOut', {logContext}) + logEvent('account:loggedOut', {logContext, scope: 'current'}) + addSessionDebugLog({type: 'method:end', method: 'logout'}) + }, + [cancelPendingTask], + ) + + const logoutEveryAccount = React.useCallback< + SessionApiContext['logoutEveryAccount'] + >( + logContext => { + addSessionDebugLog({type: 'method:start', method: 'logout'}) + cancelPendingTask() + dispatch({ + type: 'logged-out-every-account', + }) + logEvent('account:loggedOut', {logContext, scope: 'every'}) addSessionDebugLog({type: 'method:end', method: 'logout'}) }, [cancelPendingTask], @@ -230,11 +248,19 @@ export function Provider({children}: React.PropsWithChildren<{}>) { () => ({ createAccount, login, - logout, + logoutCurrentAccount, + logoutEveryAccount, resumeSession, removeAccount, }), - [createAccount, login, logout, resumeSession, removeAccount], + [ + createAccount, + login, + logoutCurrentAccount, + logoutEveryAccount, + resumeSession, + removeAccount, + ], ) // @ts-ignore diff --git a/src/state/session/reducer.ts b/src/state/session/reducer.ts index b4919851..22ba4716 100644 --- a/src/state/session/reducer.ts +++ b/src/state/session/reducer.ts @@ -42,7 +42,10 @@ export type Action = accountDid: string } | { - type: 'logged-out' + type: 'logged-out-current-account' + } + | { + type: 'logged-out-every-account' } | { type: 'synced-accounts' @@ -138,7 +141,23 @@ let reducer = (state: State, action: Action): State => { needsPersist: true, } } - case 'logged-out': { + case 'logged-out-current-account': { + const {currentAgentState} = state + return { + accounts: state.accounts.map(a => + a.did === currentAgentState.did + ? { + ...a, + refreshJwt: undefined, + accessJwt: undefined, + } + : a, + ), + currentAgentState: createPublicAgentState(), + needsPersist: true, + } + } + case 'logged-out-every-account': { return { accounts: state.accounts.map(a => ({ ...a, diff --git a/src/state/session/types.ts b/src/state/session/types.ts index d43b57cc..d32259de 100644 --- a/src/state/session/types.ts +++ b/src/state/session/types.ts @@ -29,12 +29,12 @@ export type SessionApiContext = { }, logContext: LogEvents['account:loggedIn']['logContext'], ) => Promise - /** - * A full logout. Clears the `currentAccount` from session, AND removes - * access tokens from all accounts, so that returning as any user will - * require a full login. - */ - logout: (logContext: LogEvents['account:loggedOut']['logContext']) => void + logoutCurrentAccount: ( + logContext: LogEvents['account:loggedOut']['logContext'], + ) => void + logoutEveryAccount: ( + logContext: LogEvents['account:loggedOut']['logContext'], + ) => void resumeSession: (account: SessionAccount) => Promise removeAccount: (account: SessionAccount) => void } diff --git a/src/view/com/testing/TestCtrls.e2e.tsx b/src/view/com/testing/TestCtrls.e2e.tsx index 82750959..83c79ab7 100644 --- a/src/view/com/testing/TestCtrls.e2e.tsx +++ b/src/view/com/testing/TestCtrls.e2e.tsx @@ -20,7 +20,7 @@ const BTN = {height: 1, width: 1, backgroundColor: 'red'} export function TestCtrls() { const queryClient = useQueryClient() - const {logout, login} = useSessionApi() + const {logoutEveryAccount, login} = useSessionApi() const {openModal} = useModalControls() const onboardingDispatch = useOnboardingDispatch() const {setShowLoggedOut} = useLoggedOutViewControls() @@ -60,7 +60,7 @@ export function TestCtrls() { /> logout('Settings')} + onPress={() => logoutEveryAccount('Settings')} accessibilityRole="button" style={BTN} /> diff --git a/src/view/com/util/AccountDropdownBtn.tsx b/src/view/com/util/AccountDropdownBtn.tsx index 221879df..fa2553d3 100644 --- a/src/view/com/util/AccountDropdownBtn.tsx +++ b/src/view/com/util/AccountDropdownBtn.tsx @@ -4,26 +4,27 @@ import { FontAwesomeIcon, FontAwesomeIconStyle, } from '@fortawesome/react-native-fontawesome' -import {s} from 'lib/styles' -import {usePalette} from 'lib/hooks/usePalette' -import {DropdownItem, NativeDropdown} from './forms/NativeDropdown' -import * as Toast from '../../com/util/Toast' -import {useSessionApi, SessionAccount} from '#/state/session' -import {useLingui} from '@lingui/react' import {msg} from '@lingui/macro' +import {useLingui} from '@lingui/react' + +import {SessionAccount, useSessionApi} from '#/state/session' +import {usePalette} from 'lib/hooks/usePalette' +import {s} from 'lib/styles' +import {useDialogControl} from '#/components/Dialog' +import * as Prompt from '#/components/Prompt' +import * as Toast from '../../com/util/Toast' +import {DropdownItem, NativeDropdown} from './forms/NativeDropdown' export function AccountDropdownBtn({account}: {account: SessionAccount}) { const pal = usePalette('default') const {removeAccount} = useSessionApi() + const removePromptControl = useDialogControl() const {_} = useLingui() const items: DropdownItem[] = [ { label: _(msg`Remove account`), - onPress: () => { - removeAccount(account) - Toast.show(_(msg`Account removed from quick access`)) - }, + onPress: removePromptControl.open, icon: { ios: { name: 'trash', @@ -34,17 +35,32 @@ export function AccountDropdownBtn({account}: {account: SessionAccount}) { }, ] return ( - - - - - + <> + + + + + + { + removeAccount(account) + Toast.show(_(msg`Account removed from quick access`)) + }} + confirmButtonCta={_(msg`Remove`)} + confirmButtonColor="negative" + /> + ) } diff --git a/src/view/screens/Settings/index.tsx b/src/view/screens/Settings/index.tsx index 521c2019..fe449fcd 100644 --- a/src/view/screens/Settings/index.tsx +++ b/src/view/screens/Settings/index.tsx @@ -57,7 +57,6 @@ import {DeactivateAccountDialog} from '#/screens/Settings/components/DeactivateA import {atoms as a, useTheme} from '#/alf' import {useDialogControl} from '#/components/Dialog' import {BirthDateSettingsDialog} from '#/components/dialogs/BirthDateSettings' -import {navigate, resetToTab} from '#/Navigation' import {Email2FAToggle} from './Email2FAToggle' import {ExportCarDialog} from './ExportCarDialog' @@ -77,7 +76,6 @@ function SettingsAccountCard({ const {_} = useLingui() const t = useTheme() const {currentAccount} = useSession() - const {logout} = useSessionApi() const {data: profile} = useProfileQuery({did: account.did}) const isCurrentAccount = account.did === currentAccount?.did @@ -103,31 +101,7 @@ function SettingsAccountCard({ {account.handle} - - {isCurrentAccount ? ( - { - if (isNative) { - logout('Settings') - resetToTab('HomeTab') - } else { - navigate('Home').then(() => { - logout('Settings') - }) - } - }} - accessibilityRole="button" - accessibilityLabel={_(msg`Sign out`)} - accessibilityHint={`Signs ${profile?.displayName} out of Bluesky`} - activeOpacity={0.8}> - - Sign out - - - ) : ( - - )} + ) @@ -173,6 +147,7 @@ export function SettingsScreen({}: Props) { const {accounts, currentAccount} = useSession() const {mutate: clearPreferences} = useClearPreferencesMutation() const {setShowLoggedOut} = useLoggedOutViewControls() + const {logoutEveryAccount} = useSessionApi() const closeAllActiveElements = useCloseAllActiveElements() const exportCarControl = useDialogControl() const birthdayControl = useDialogControl() @@ -237,6 +212,10 @@ export function SettingsScreen({}: Props) { openModal({name: 'delete-account'}) }, [openModal]) + const onPressLogoutEveryAccount = React.useCallback(() => { + logoutEveryAccount('Settings') + }, [logoutEveryAccount]) + const onPressResetPreferences = React.useCallback(async () => { clearPreferences() }, [clearPreferences]) @@ -394,6 +373,15 @@ export function SettingsScreen({}: Props) { ) : null} + {accounts.length > 1 && ( + + + Other accounts + + + + )} + {accounts .filter(a => a.did !== currentAccount?.did) .map(account => ( @@ -422,6 +410,29 @@ export function SettingsScreen({}: Props) { Add account + + + + + + + {accounts.length > 1 ? ( + Sign out of all accounts + ) : ( + Sign out + )} + +