Skip to content

Commit

Permalink
fix: improve __jump_to_message reliability
Browse files Browse the repository at this point in the history
Follow-up to 6a656f9
(#4510).
Partially addresses #4508.
Why partially? Because the bug appears to still happen sometimes.
But this commit should make it more reliable, or at least be a refactor.

This should also improve performance in some cases,
thanks to the removal of `setTimeout()`.
  • Loading branch information
WofWca committed Jan 25, 2025
1 parent 09d6325 commit 321ef27
Show file tree
Hide file tree
Showing 6 changed files with 75 additions and 33 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
- message list being empty when opening a chat in some cases #4555
- numpad "Enter" not working as regular "Enter" #4546
- improve performance a little
- fix clicking on message search result or "Reply Privately" quote not jumping to the message on first click sometimes, again #4554

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

Expand Down
19 changes: 13 additions & 6 deletions packages/frontend/src/components/RuntimeAdapter.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,19 @@ export default function RuntimeAdapter({
clearNotificationsForChat(notificationAccountId, chatId)
}
if (msgId) {
window.__internal_jump_to_message?.({
msgId,
scrollIntoViewArg: { block: 'center' },
// We probably want the composer to be focused.
focus: false,
})
window.__internal_jump_to_message_asap = {
accountId: notificationAccountId,
chatId,
jumpToMessageArgs: [
{
msgId,
scrollIntoViewArg: { block: 'center' },
// We probably want the composer to be focused.
focus: false,
},
],
}
window.__internal_check_jump_to_message?.()
}
}
)
Expand Down
12 changes: 11 additions & 1 deletion packages/frontend/src/components/message/MessageList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -222,9 +222,19 @@ export default function MessageList({ accountId, chat, refComposer }: Props) {
}
}, [])

const maybeJumpToMessageHack = () => {
if (
window.__internal_jump_to_message_asap?.accountId === accountId &&
window.__internal_jump_to_message_asap.chatId === chat.id
) {
jumpToMessage(...window.__internal_jump_to_message_asap.jumpToMessageArgs)
window.__internal_jump_to_message_asap = undefined
}
}
maybeJumpToMessageHack()
// TODO perf: to save memory, maybe set to `undefined` when unmounting,
// but be sure not to unset it if the new component render already set it.
window.__internal_jump_to_message = jumpToMessage
window.__internal_check_jump_to_message = maybeJumpToMessageHack

const pendingProgrammaticSmoothScrollTo = useRef<null | number>(null)
const pendingProgrammaticSmoothScrollTimeout = useRef<number>(-1)
Expand Down
23 changes: 15 additions & 8 deletions packages/frontend/src/contexts/ChatContext.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -75,14 +75,21 @@ export const ChatProvider = ({
// 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) {
window.__internal_jump_to_message?.({
msgId: undefined,
highlight: false,
focus: false,
addMessageIdToStack: undefined,
// `scrollIntoViewArg:` doesn't really have effect when
// jumping to the last message.
})
window.__internal_jump_to_message_asap = {
accountId,
chatId: nextChatId,
jumpToMessageArgs: [
{
msgId: undefined,
highlight: false,
focus: false,
addMessageIdToStack: undefined,
// `scrollIntoViewArg:` doesn't really have effect when
// jumping to the last message.
},
],
}
window.__internal_check_jump_to_message?.()
}

// Already set known state
Expand Down
30 changes: 21 additions & 9 deletions packages/frontend/src/global.d.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { userFeedback, Screens } from './ScreenController'

import '@deltachat-desktop/shared/global.d.ts'
import type { useMessageList } from './stores/messagelist'

declare global {
interface Window {
Expand All @@ -21,15 +22,26 @@ declare global {
| ((searchTerm: string, chatId: number | null) => void)
| undefined
__refetchChatlist: undefined | (() => void)
__internal_jump_to_message:
| undefined
| ((params: {
msgId: number | undefined
focus: boolean
highlight?: boolean
addMessageIdToStack?: undefined | number
scrollIntoViewArg?: Parameters<HTMLElement['scrollIntoView']>[0]
}) => Promise<void>)
/**
* Settings this will make the MessageList component `jumpToMessage`
* as soon as it loads the chat for `accountId` and `chatId`,
* after which it will reset `__internal_jump_to_message_asap`
* to `undefined`.
*/
__internal_jump_to_message_asap?: {
accountId: number
chatId: number
jumpToMessageArgs: Parameters<
ReturnType<typeof useMessageList>['store']['effect']['jumpToMessage']
>
}
/**
* Make the MessageList component check
* `__internal_jump_to_message_asap`, without waiting for a render.
* Useful in case the component is already rendered with the specified
* `accountId` and `chatId`.
*/
__internal_check_jump_to_message?: () => void
__updateAccountListSidebar: (() => void) | undefined
}
}
23 changes: 14 additions & 9 deletions packages/frontend/src/hooks/chat/useMessage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,15 +98,20 @@ export default function useMessage() {
setChatView(ChatView.MessageList)

// Workaround to actual jump to message in regarding mounted component view
setTimeout(() => {
window.__internal_jump_to_message?.({
msgId,
highlight,
focus,
addMessageIdToStack: msgParentId,
scrollIntoViewArg,
})
}, 0)
window.__internal_jump_to_message_asap = {
accountId,
chatId: msgChatId,
jumpToMessageArgs: [
{
msgId,
highlight,
focus,
addMessageIdToStack: msgParentId,
scrollIntoViewArg,
},
],
}
window.__internal_check_jump_to_message?.()
},
[chatId, selectChat, setChatView]
)
Expand Down

0 comments on commit 321ef27

Please sign in to comment.