Skip to content

Commit

Permalink
Prevent re-rendering of account sidebar when switching account (#3789)
Browse files Browse the repository at this point in the history
* Prevent re-rendering of account sidebar when switching account

* changelog entry

* unselect chat without moving chatstore up in the hierarchy

* fix typing and eslint issues

* close dialogs when switching account from multiaccount notification

* fix test

* remove unused cached key from SettingsStoreState

* add accountId as argument to selectChat

Fixes problem where changing account set chat id of previous account to new account as last selected chat id, which selected a random chat in the account you switched to because the selected id came from previous account.

* fix fmt

* fix after rebase
  • Loading branch information
Simon-Laux committed Apr 30, 2024
1 parent 2f9e829 commit e7fc9e2
Show file tree
Hide file tree
Showing 24 changed files with 76 additions and 44 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,12 @@
- Unmount QR scanner and disable camera correctly on abort or exit #3762
- Close reactions bar on emoji selection #3788
- fix Clicking notification does not bring Delta Chat to foreground on Windows #3793
- Prevent re-rendering of account sidebar when switching account #3789

### Removed
- remove disabled composer reason, now composer is just always hidden when `chat.canSend` is `false` #3791


<a id="1_44_1"></a>

## [1.44.1] - 2024-03-09
Expand Down
23 changes: 13 additions & 10 deletions src/renderer/ScreenController.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React from 'react'
import React, { createRef } from 'react'
import { Component } from 'react'
import { DcEventType } from '@deltachat/jsonrpc-client'
import { debounce } from 'debounce'
Expand All @@ -19,7 +19,7 @@ import SettingsStoreInstance from './stores/settings'
import { NoAccountSelectedScreen } from './components/screens/NoAccountSelectedScreen/NoAccountSelectedScreen'
import AccountDeletionScreen from './components/screens/AccountDeletionScreen/AccountDeletionScreen'
import RuntimeAdapter from './components/RuntimeAdapter'
import { ChatProvider } from './contexts/ChatContext'
import { ChatProvider, UnselectChat } from './contexts/ChatContext'
import { ContextMenuProvider } from './contexts/ContextMenuContext'
import { InstantOnboardingProvider } from './contexts/InstantOnboardingContext'

Expand All @@ -43,6 +43,7 @@ export default class ScreenController extends Component {
state: { message: userFeedback | false; screen: Screens }
selectedAccountId: number | undefined
lastAccountBeforeAddingNewAccount: number | null = null
unselectChatRef = createRef<UnselectChat>()

constructor(public props: {}) {
super(props)
Expand Down Expand Up @@ -130,6 +131,9 @@ export default class ScreenController extends Component {
if (this.selectedAccountId === undefined) {
return
}

this.unselectChatRef.current?.()

const previousAccountId = this.selectedAccountId

SettingsStoreInstance.effect.clear()
Expand Down Expand Up @@ -231,10 +235,10 @@ export default class ScreenController extends Component {
this.userFeedback({ type: 'success', text })
}

renderScreen() {
renderScreen(key: React.Key | null | undefined) {
switch (this.state.screen) {
case Screens.Main:
return <MainScreen accountId={this.selectedAccountId} />
return <MainScreen accountId={this.selectedAccountId} key={key} />
case Screens.Login:
if (this.selectedAccountId === undefined) {
throw new Error('Selected account not defined')
Expand All @@ -243,6 +247,7 @@ export default class ScreenController extends Component {
<AccountSetupScreen
selectAccount={this.selectAccount}
accountId={this.selectedAccountId}
key={key}
/>
)
case Screens.Welcome:
Expand All @@ -254,6 +259,7 @@ export default class ScreenController extends Component {
selectedAccountId={this.selectedAccountId}
onUnSelectAccount={this.unSelectAccount}
onExitWelcomeScreen={this.onExitWelcomeScreen}
key={key}
/>
)
case Screens.DeleteAccount:
Expand All @@ -264,6 +270,7 @@ export default class ScreenController extends Component {
<AccountDeletionScreen
selectedAccountId={this.selectedAccountId}
onDeleteAccount={this.onDeleteAccount.bind(this)}
key={key}
/>
)
case Screens.NoAccountSelected:
Expand Down Expand Up @@ -292,13 +299,9 @@ export default class ScreenController extends Component {
}}
>
<InstantOnboardingProvider>
{/*
The key attribute here forces a clean re-rendering when the
account changes. We needs this to reset the chat context state.
*/}
<ChatProvider
key={this.selectedAccountId}
accountId={this.selectedAccountId}
unselectChatRef={this.unselectChatRef}
>
<ContextMenuProvider>
<DialogContextProvider>
Expand All @@ -313,7 +316,7 @@ export default class ScreenController extends Component {
this
)}
/>
{this.renderScreen()}
{this.renderScreen(this.selectedAccountId)}
</div>
</KeybindingsContextProvider>
</DialogContextProvider>
Expand Down
9 changes: 5 additions & 4 deletions src/renderer/components/RuntimeAdapter.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ export default function RuntimeAdapter({
const processQr = useProcessQr()
const { jumpToMessage } = useMessage()

const { closeDialog, openDialog, closeAllDialogs } = useDialog()
const openSendToDialogId = useRef<string | undefined>(undefined)

useEffect(() => {
runtime.onOpenQrUrl = (url: string) => {
if (!accountId) {
Expand All @@ -38,6 +41,7 @@ export default function RuntimeAdapter({
runtime.setNotificationCallback(
async ({ accountId: notificationAccountId, msgId, chatId }) => {
if (accountId !== notificationAccountId) {
closeAllDialogs()
await window.__selectAccount(notificationAccountId)
}

Expand All @@ -60,10 +64,7 @@ export default function RuntimeAdapter({
ActionEmitter.emitAction(KeybindAction.Settings_Open)
}
}
}, [accountId, jumpToMessage, processQr])

const { closeDialog, openDialog } = useDialog()
const openSendToDialogId = useRef<string | undefined>(undefined)
}, [accountId, jumpToMessage, processQr, closeAllDialogs])

useEffect(() => {
runtime.onWebxdcSendToChat = (file, text) => {
Expand Down
6 changes: 3 additions & 3 deletions src/renderer/components/chat/ChatList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ export default function ChatList(props: {
showArchivedChats,
])

const selectFirstChat = () => selectChat(chatListIds[0])
const selectFirstChat = () => selectChat(accountId, chatListIds[0])

// KeyboardShortcuts ---------
useKeyBindingAction(KeybindAction.ChatList_ScrollToSelectedChat, () =>
Expand All @@ -242,7 +242,7 @@ export default function ChatList(props: {
)
const newChatId = chatListIds[selectedChatIndex + 1]
if (newChatId && newChatId !== C.DC_CHAT_ID_ARCHIVED_LINK) {
selectChat(newChatId)
selectChat(accountId, newChatId)
}
})

Expand All @@ -253,7 +253,7 @@ export default function ChatList(props: {
)
const newChatId = chatListIds[selectedChatIndex - 1]
if (newChatId && newChatId !== C.DC_CHAT_ID_ARCHIVED_LINK) {
selectChat(newChatId)
selectChat(accountId, newChatId)
}
})

Expand Down
4 changes: 2 additions & 2 deletions src/renderer/components/dialogs/CreateChat.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -809,7 +809,7 @@ const useCreateGroup = (

const chatId = await createGroup()
onClose()
selectChat(chatId)
selectChat(accountId, chatId)
}
}

Expand Down Expand Up @@ -838,7 +838,7 @@ const useCreateBroadcast = (
return async () => {
const chatId = await createBroadcastList()
onClose()
selectChat(chatId)
selectChat(accountId, chatId)
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/renderer/components/dialogs/ForwardMessage/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -45,15 +45,15 @@ export default function ForwardMessage(props: Props) {
const chat = await BackendRemote.rpc.getFullChatById(accountId, chatId)
onClose()
if (!chat.isSelfTalk) {
selectChat(chat.id)
selectChat(accountId, chat.id)
const yes = await confirmForwardMessage(
openDialog,
accountId,
message,
chat
)
if (!yes) {
selectChat(message.chatId)
selectChat(accountId, message.chatId)
}
} else {
await forwardMessage(accountId, message.id, chat.id)
Expand Down
4 changes: 2 additions & 2 deletions src/renderer/components/dialogs/QrCode.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -216,10 +216,10 @@ export function QrCodeScanQrInner({

const handleScanResult = useCallback(
(chatId: number | null = null) => {
chatId && selectChat(chatId)
chatId && selectChat(accountId, chatId)
onDone()
},
[onDone, selectChat]
[onDone, selectChat, accountId]
)

const handleScan = useCallback(
Expand Down
2 changes: 1 addition & 1 deletion src/renderer/components/dialogs/ViewGroup.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ function ViewGroupInner(
const [profileContact, setProfileContact] = useState<T.Contact | null>(null)

const onChatClick = (chatId: number) => {
selectChat(chatId)
selectChat(accountId, chatId)
onClose()
}

Expand Down
2 changes: 1 addition & 1 deletion src/renderer/components/dialogs/ViewProfile/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ export function ViewProfileInner({
const isSelfChat = contact.id === C.DC_CONTACT_ID_SELF

const onChatClick = (chatId: number) => {
selectChat(chatId)
selectChat(accountId, chatId)
onClose()
}
const onSendMessage = async () => {
Expand Down
2 changes: 1 addition & 1 deletion src/renderer/components/dialogs/WebxdcSendToChat/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ function useSendToChatAction() {
const chat = await BackendRemote.rpc.getBasicChatInfo(accountId, chatId)
const draft = await BackendRemote.rpc.getDraft(accountId, chatId)

selectChat(chatId)
selectChat(accountId, chatId)

if (draft) {
// ask if the draft should be replaced
Expand Down
6 changes: 3 additions & 3 deletions src/renderer/components/message/MessageMarkdown.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ function EmailLink({ email }: { email: string }): JSX.Element {
const handleClick = async () => {
const chatId = await createChatByEmail(accountId, email)
if (chatId) {
selectChat(chatId)
selectChat(accountId, chatId)
}
}

Expand Down Expand Up @@ -221,12 +221,12 @@ function BotCommandSuggestion({ suggestion }: { suggestion: string }) {
messageDisplay.contact_id
)
// also select the chat and close the profile window if this is the case
selectChat(chatId)
selectChat(accountId, chatId)
messageDisplay.closeProfileDialog()
} else if (messageDisplay.context == 'chat_map') {
chatId = messageDisplay.chatId
// go back to chat view
selectChat(chatId)
selectChat(accountId, chatId)
setChatView(ChatView.MessageList)
} else if (messageDisplay.context == 'chat_messagelist') {
// nothing special to do
Expand Down
2 changes: 1 addition & 1 deletion src/renderer/components/screens/MainScreen.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ export default function MainScreen({ accountId }: Props) {
return
}

selectChat(chatId)
accountId && selectChat(accountId, chatId)
}

const searchChats = (queryStr: string, chatId: number | null = null) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ export default function InstantOnboardingScreen({
// If the user scanned a QR code to join a contact or group, then
// we redirect to the created chat after instant onboarding
if (chatId !== null) {
selectChat(chatId)
selectChat(selectedAccountId, chatId)
}
} catch (error: any) {
await openAlertDialog({
Expand Down
23 changes: 20 additions & 3 deletions src/renderer/contexts/ChatContext.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { ActionEmitter, KeybindAction } from '../keybindings'
import { markChatAsSeen, saveLastChatId } from '../backend/chat'
import { BackendRemote } from '../backend-com'

import type { PropsWithChildren } from 'react'
import type { MutableRefObject, PropsWithChildren } from 'react'
import type { T } from '@deltachat/jsonrpc-client'

export enum ChatView {
Expand All @@ -15,7 +15,10 @@ export enum ChatView {

export type SetView = (nextView: ChatView) => void

export type SelectChat = (chatId: number) => Promise<void>
export type SelectChat = (
nextAccountId: number,
chatId: number
) => Promise<void>

export type UnselectChat = () => void

Expand All @@ -30,13 +33,19 @@ export type ChatContextValue = {

type Props = {
accountId?: number
/**
* the ref gives us a handle to reset the component without moving it up in the hierarchy.
* a class component would give us the option to call methods on the component,
* but we are using a functional component here so we need to pass this as a property instead*/
unselectChatRef: MutableRefObject<UnselectChat | null>
}

export const ChatContext = React.createContext<ChatContextValue | null>(null)

export const ChatProvider = ({
children,
accountId,
unselectChatRef,
}: PropsWithChildren<Props>) => {
const [activeView, setActiveView] = useState(ChatView.MessageList)
const [chat, setChat] = useState<T.FullChat | undefined>()
Expand All @@ -47,11 +56,17 @@ export const ChatProvider = ({
}, [])

const selectChat = useCallback<SelectChat>(
async (nextChatId: number) => {
async (nextAccountId: number, nextChatId: number) => {
if (!accountId) {
throw new Error('can not select chat when no `accountId` is given')
}

if (accountId !== nextAccountId) {
throw new Error(
'accountid of ChatProvider context is not equal to nextAccountId'
)
}

// Jump to last message if user clicked chat twice
// @TODO: We probably want this to be part of the UI logic instead
if (nextChatId === chatId) {
Expand Down Expand Up @@ -100,6 +115,8 @@ export const ChatProvider = ({
setChat(undefined)
}, [])

unselectChatRef.current = unselectChat

// Subscribe to events coming from the core
useEffect(() => {
const onChatModified = (
Expand Down
9 changes: 9 additions & 0 deletions src/renderer/contexts/DialogContext.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,20 @@ export type OpenDialog = <T extends { [key: string]: any }>(

export type CloseDialog = (id: DialogId) => void

export type CloseAllDialogs = () => void

type DialogContextValue = {
hasOpenDialogs: boolean
openDialog: OpenDialog
closeDialog: CloseDialog
closeAllDialogs: CloseAllDialogs
}

const initialValues: DialogContextValue = {
hasOpenDialogs: false,
openDialog: _ => '',
closeDialog: _ => {},
closeAllDialogs: () => {},
}

export const DialogContext =
Expand All @@ -41,6 +45,10 @@ export const DialogContextProvider = ({ children }: PropsWithChildren<{}>) => {
setDialogs(({ [id]: _, ...rest }) => rest)
}, [])

const closeAllDialogs: CloseAllDialogs = useCallback(() => {
setDialogs({})
}, [])

const openDialog = useCallback<OpenDialog>(
(dialogElement, additionalProps) => {
const newDialogId = generateRandomUUID()
Expand Down Expand Up @@ -77,6 +85,7 @@ export const DialogContextProvider = ({ children }: PropsWithChildren<{}>) => {
hasOpenDialogs,
openDialog,
closeDialog,
closeAllDialogs,
}

return (
Expand Down
2 changes: 1 addition & 1 deletion src/renderer/hooks/chat/useChatDialog.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ export default function useChatDialog() {
// messages by unloading the chat first.
unselectChat()
await BackendRemote.rpc.deleteMessages(accountId, messagesToDelete)
selectChat(chatId)
selectChat(accountId, chatId)
}
},
[openConfirmationDialog, selectChat, tx, unselectChat]
Expand Down
Loading

0 comments on commit e7fc9e2

Please sign in to comment.