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 24, 2025
1 parent 4f7e52c commit 4832061
Show file tree
Hide file tree
Showing 6 changed files with 67 additions and 28 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
### Changed

### Fixed
- fix clicking on message search result or "reply privately" quote not jumping to the message on first click sometimes, again

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

Expand Down
15 changes: 11 additions & 4 deletions packages/frontend/src/components/RuntimeAdapter.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,17 @@ export default function RuntimeAdapter({
clearNotificationsForChat(notificationAccountId, chatId)
}
if (msgId) {
window.__internal_jump_to_message?.({
msgId,
scrollIntoViewArg: { block: 'center' },
})
window.__internal_jump_to_message_on_render = {
accountId: notificationAccountId,
chatId,
jumpToMessageArgs: [
{
msgId,
scrollIntoViewArg: { block: 'center' },
},
],
}
window.__internal_check_jump_to_message?.()
}
}
)
Expand Down
14 changes: 13 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,21 @@ export default function MessageList({ accountId, chat, refComposer }: Props) {
}
}, [])

const maybeJumpToMessageHack = () => {
if (
window.__internal_jump_to_message_on_render?.accountId === accountId &&
window.__internal_jump_to_message_on_render.chatId === chat.id
) {
jumpToMessage(
...window.__internal_jump_to_message_on_render.jumpToMessageArgs
)
window.__internal_jump_to_message_on_render = 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
21 changes: 14 additions & 7 deletions packages/frontend/src/contexts/ChatContext.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -75,13 +75,20 @@ 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,
addMessageIdToStack: undefined,
// `scrollIntoViewArg:` doesn't really have effect when
// jumping to the last message.
})
window.__internal_jump_to_message_on_render = {
accountId,
chatId: nextChatId,
jumpToMessageArgs: [
{
msgId: undefined,
highlight: 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
23 changes: 15 additions & 8 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,14 +22,20 @@ declare global {
| ((searchTerm: string, chatId: number | null) => void)
| undefined
__refetchChatlist: undefined | (() => void)
__internal_jump_to_message:
| undefined
| ((params: {
msgId: number | undefined
highlight?: boolean
addMessageIdToStack?: undefined | number
scrollIntoViewArg?: Parameters<HTMLElement['scrollIntoView']>[0]
}) => Promise<void>)
__internal_jump_to_message_on_render?: {
accountId: number
chatId: number
jumpToMessageArgs: Parameters<
ReturnType<typeof useMessageList>['store']['effect']['jumpToMessage']
>
}
/**
* Make the MessageList component check
* __internal_jump_to_message_on_render, 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
}
}
21 changes: 13 additions & 8 deletions packages/frontend/src/hooks/chat/useMessage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,14 +96,19 @@ 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,
addMessageIdToStack: msgParentId,
scrollIntoViewArg,
})
}, 0)
window.__internal_jump_to_message_on_render = {
accountId,
chatId: msgChatId,
jumpToMessageArgs: [
{
msgId,
highlight,
addMessageIdToStack: msgParentId,
scrollIntoViewArg,
},
],
}
window.__internal_check_jump_to_message?.()
},
[chatId, selectChat, setChatView]
)
Expand Down

0 comments on commit 4832061

Please sign in to comment.