Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent re-rendering of account sidebar when switching account #3789

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?.()

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does this mean here? Looks weird to me :)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function is set from inside ChatContext. Read it as chatContext.unselectChat()

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An alternative to doing it like that is to introduce the unselectChat as a prop to the classic class-style React component:

https://github.com/deltachat/deltachat-desktop/blob/adz/i3776-fix-rerendering/src/renderer/ScreenController.tsx#L330-L337

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
Loading