improvement: better scroll for jumpToMessage

Add `scrollIntoViewArg` parameter to `jumpToMessage`
and specify different values for it depending on context.

More specifically, prefer scrolling the target message
to the center, e.g. when showing the first unread message.
Prior to this commit, we'd scroll the first unread message
such that it's at the bottom of the message list,
which can be considered a regression, which was introduced in
5f0efe1.

Closes #4284.

CHANGELOG.md

@@ -6,6 +6,7 @@
 - style: avoid scrolling to account list items such that they're at the very edge of the list #4252
 - Update local help (2024-10-25)
 - Limit options for "Delete Messages from Server" for chatmail accounts #4276
+- when jumping to a message (e.g. when showing the first unread message, or when jumping to a message through "show in chat"), position it more appropriately in the scrollable area #4286
 
 ## Fixed
 - image thumbnails not showing in chat list #4247

packages/frontend/src/components/RuntimeAdapter.tsx

@@ -55,7 +55,10 @@ export default function RuntimeAdapter({
           clearNotificationsForChat(notificationAccountId, chatId)
         }
         if (msgId) {
-          window.__internal_jump_to_message?.({ msgId })
+          window.__internal_jump_to_message?.({
+            msgId,
+            scrollIntoViewArg: { block: 'center' },
+          })
         }
       }
     )

packages/frontend/src/components/attachment/mediaAttachment.tsx

@@ -78,6 +78,7 @@ const contextMenuFactory = (
           accountId,
           msgId: message.id,
           msgChatId: message.chatId,
+          scrollIntoViewArg: { block: 'center' },
         }),
     },
     {

packages/frontend/src/components/chat/ChatListItemRow.tsx

@@ -122,7 +122,11 @@ export const ChatListItemRowMessage = React.memo<{
           queryStr={queryStr || ''}
           msr={messageSearchResult}
           onClick={() => {
-            jumpToMessage({ accountId, msgId: msrId })
+            jumpToMessage({
+              accountId,
+              msgId: msrId,
+              scrollIntoViewArg: { block: 'center' },
+            })
           }}
         />
       ) : (

packages/frontend/src/components/composer/Composer.tsx

@@ -633,6 +633,9 @@ export function useDraft(
         msgId: messageId,
         msgChatId: chatId,
         highlight: true,
+        // The message is usually already in view,
+        // so let's not scroll at all if so.
+        scrollIntoViewArg: { block: 'nearest' },
       })
     }
     // TODO perf: I imagine this is pretty slow, given IPC and some chats

packages/frontend/src/components/dialogs/ChatAuditLogDialog.tsx

@@ -50,6 +50,7 @@ function buildContextMenu(
             msgId: message.id,
             msgChatId: message.chatId,
             highlight: true,
+            scrollIntoViewArg: { block: 'center' },
           })
         )
       },
@@ -68,6 +69,7 @@ function buildContextMenu(
             // but let's not pass `chatId` here, for future-proofing.
             msgChatId: undefined,
             highlight: true,
+            scrollIntoViewArg: { block: 'center' },
           })
         }
       },

packages/frontend/src/components/dialogs/FullscreenMedia.tsx

@@ -115,7 +115,12 @@ export default function FullscreenMedia(props: Props & DialogProps) {
     {
       label: tx('show_in_chat'),
       action: () => {
-        jumpToMessage({ accountId, msgId: msg.id, msgChatId: msg.chatId })
+        jumpToMessage({
+          accountId,
+          msgId: msg.id,
+          msgChatId: msg.chatId,
+          scrollIntoViewArg: { block: 'center' },
+        })
         onClose()
       },
     },

packages/frontend/src/components/message/Message.tsx

@@ -455,6 +455,7 @@ export default function Message(props: {
             msgChatId: undefined,
             highlight: true,
             msgParentId: message.id,
+            scrollIntoViewArg: { block: 'center' },
           })
         } else if (isProtectionBrokenMsg) {
           const { name } = await BackendRemote.rpc.getBasicChatInfo(
@@ -749,6 +750,9 @@ export const Quote = ({
             msgChatId: undefined,
             highlight: true,
             msgParentId,
+            // Often times the quoted message is already in view,
+            // so let's not scroll at all if so.
+            scrollIntoViewArg: { block: 'nearest' },
           })
       }}
     >

packages/frontend/src/components/message/MessageList.tsx

@@ -333,17 +333,7 @@ export default function MessageList({ accountId, chat, refComposer }: Props) {
         )
       }
 
-      domElement.scrollIntoView({
-        // "nearest" so as to not scroll if the message is already in view.
-        // Otherwise we'd try to scroll in such a way that the message
-        // is at the very top of the messages list.
-        // This would not be nice for the Ctrl + Down shortcut
-        // (when quoting a message that a bit far up),
-        // or when highlighting the reply that is already in view.
-        block: 'nearest',
-        inline: 'nearest',
-        // behavior:
-      })
+      domElement.scrollIntoView(scrollTo.scrollIntoViewArg)
 
       if (scrollTo.highlight === true) {
         // Trigger highlight animation
@@ -800,6 +790,7 @@ function JumpDownButton({
     msgId: number | undefined
     highlight?: boolean
     addMessageIdToStack?: undefined | number
+    scrollIntoViewArg?: Parameters<HTMLElement['scrollIntoView']>[0]
   }) => Promise<void>
   jumpToMessageStack: number[]
 }) {
@@ -823,7 +814,14 @@ function JumpDownButton({
         <div
           className='button'
           onClick={() => {
-            jumpToMessage({ msgId: undefined, highlight: true })
+            jumpToMessage({
+              msgId: undefined,
+              highlight: true,
+              // 'center' is for when we're jumping the message stack.
+              // When the stack is empty, we'll jump to last message,
+              // and 'center' will make the chat scroll down all the way.
+              scrollIntoViewArg: { block: 'center' },
+            })
           }}
         >
           <div

packages/frontend/src/contexts/ChatContext.tsx

@@ -79,6 +79,8 @@ export const ChatProvider = ({
           msgId: undefined,
           highlight: false,
           addMessageIdToStack: undefined,
+          // `scrollIntoViewArg:` doesn't really have effect when
+          // jumping to the last message.
         })
       }
 

packages/frontend/src/global.d.ts

@@ -27,6 +27,7 @@ declare global {
           msgId: number | undefined
           highlight?: boolean
           addMessageIdToStack?: undefined | number
+          scrollIntoViewArg?: Parameters<HTMLElement['scrollIntoView']>[0]
         }) => Promise<void>)
     __updateAccountListSidebar: (() => void) | undefined
   }

packages/frontend/src/hooks/chat/useMessage.ts

@@ -19,6 +19,13 @@ export type JumpToMessage = (params: {
   msgChatId?: number
   highlight?: boolean
   msgParentId?: number
+  /**
+   * `behavior: 'smooth'` should not be used due to "scroll locking":
+   * they don't behave well together currently.
+   * `inline` also isn't supposed to have effect because
+   * the messages list should not be horizontally scrollable.
+   */
+  scrollIntoViewArg?: Parameters<HTMLElement['scrollIntoView']>[0]
 }) => Promise<void>
 
 export type SendMessage = (
@@ -55,7 +62,14 @@ export default function useMessage() {
   const { chatId, setChatView, selectChat } = useChat()
 
   const jumpToMessage = useCallback<JumpToMessage>(
-    async ({ accountId, msgId, msgChatId, highlight = true, msgParentId }) => {
+    async ({
+      accountId,
+      msgId,
+      msgChatId,
+      highlight = true,
+      msgParentId,
+      scrollIntoViewArg,
+    }) => {
       log.debug(`jumpToMessage with messageId: ${msgId}`)
 
       if (msgChatId == undefined) {
@@ -74,6 +88,7 @@ export default function useMessage() {
           msgId,
           highlight,
           addMessageIdToStack: msgParentId,
+          scrollIntoViewArg,
         })
       }, 0)
     },

packages/frontend/src/stores/chat/chat_view_reducer.ts

@@ -8,6 +8,7 @@ type ScrollTo =
 interface ScrollToMessage {
   type: 'scrollToMessage'
   msgId: number
+  scrollIntoViewArg?: Parameters<HTMLElement['scrollIntoView']>[0]
   highlight: boolean
 }
 
@@ -140,14 +141,16 @@ export class ChatViewReducer {
   static jumpToMessage(
     prevState: ChatViewState,
     jumpToMessageId: number,
-    highlight: boolean
+    highlight: boolean,
+    scrollIntoViewArg: ScrollToMessage['scrollIntoViewArg']
   ): ChatViewState {
     return {
       ...prevState,
       scrollTo: {
         type: 'scrollToMessage',
         msgId: jumpToMessageId,
         highlight,
+        scrollIntoViewArg,
       },
     }
   }

packages/frontend/src/stores/messagelist.ts

@@ -349,6 +349,9 @@ class MessageListStore extends Store<MessageListState> {
             this.effect.jumpToMessage({
               msgId: firstUnreadMsgId,
               highlight: false,
+              // 'center' so that old messages are also shown, for context.
+              // See https://github.com/deltachat/deltachat-desktop/issues/4284
+              scrollIntoViewArg: { block: 'center' },
             })
             ActionEmitter.emitAction(
               chat.archived
@@ -410,10 +413,12 @@ class MessageListStore extends Store<MessageListState> {
         msgId: jumpToMessageId,
         highlight = true,
         addMessageIdToStack,
+        scrollIntoViewArg,
       }: {
         msgId: number | undefined
         highlight?: boolean
         addMessageIdToStack?: undefined | number
+        scrollIntoViewArg?: Parameters<HTMLElement['scrollIntoView']>[0]
       }) => {
         const startTime = performance.now()
 
@@ -621,7 +626,8 @@ class MessageListStore extends Store<MessageListState> {
           viewState: ChatViewReducer.jumpToMessage(
             this.state.viewState,
             jumpToMessageId,
-            highlight
+            highlight,
+            scrollIntoViewArg
           ),
           jumpToMessageStack,
         })