fix: chat "scrolling up" upon reaction

Closes #3753

This removes `scrollToBottom` JS on each new message,
which is now unnecessary. However, the old code used `ifClose: true`,
which would still scroll the chat if it was scrolled up less than
7 pixels up. The new implementation still has similar behavior as
the new scroll anchor is not at the very bottom
of the scrollable area, so the chat will still show the new message
as long as the bottom edge of the previous oldest message
was still visible when the new one arrived.

CHANGELOG.md

@@ -26,6 +26,7 @@
 - Fix the problem of Quit menu item on WebXDC apps closes the whole DC app #3995
 - minor performance improvements #3981
 - fix chat list items (e.g. Archive) and contacts not showing up sometimes #4004
+- fix chat "scrolling up" when someone adds a reaction #4012
 
 <a id="1_46_1"></a>
 

scss/message/_message-list.scss

@@ -91,6 +91,60 @@
     width: 100%;
     padding: 0 0.5em;
 
+    // The following code block is to prevent
+    // https://github.com/deltachat/deltachat-desktop/issues/3753
+    // which is 'Adding a reaction makes the chat "scroll up"
+    // (then you have to scroll to see new messages)'.
+    // Signal messenger implements the same measure:
+    // https://github.com/signalapp/Signal-Desktop/blob/29c404266863491a3b26a73b24602d36d7a3ac46/stylesheets/_modules.scss#L5573-L5587
+    // which they have looked up here:
+    // https://css-tricks.com/books/greatest-css-tricks/pin-scrolling-to-bottom/
+    // Spec:
+    // https://drafts.csswg.org/css-scroll-anchoring
+    //
+    // TODO there is a concern: whether this makes the chat list jumpy
+    // when it is scrolled up, making the scroll anchor
+    // (which is at the bottom of chat list) go way off screen.
+    // This doesn't appear to be the case.
+    // Also whether we're gonna jump to bottom if the user is reading old
+    // messages as is waiting for the newer ones to load
+    // while scrolled to bottom. This would be unexpected.
+    // I suppose this is why Signal has `--have-newest`.
+    // But I also haven't managed to see this happen.
+    // But, for example, if you scroll up a bit, and then someone
+    // adds a reaction to a message that is rendered, but is above the messages
+    // that are currently in view, the view will shift down.
+    //
+    // Perhaps a more reasonable approch would be to set the last message
+    // _in view_ as the anchor, and only explicitly scroll to latest message
+    // when it is freshly received,
+    // but this will (probably?) require quite some JS.
+    // Or simply only alternate scroll anchoring behavior when
+    // we're scrolled to bottom.
+    // Ah, if only there was a way to "flip" overflow anchor selection
+    // algorithm to just select bottom-most visible element
+    // and not the top-most.
+    // Maybe we could though? E.g. with `order` CSS property?
+    //
+    // Anyways, it's not often that already sent messages change in Delta Chat,
+    // because there is no "delete for everyone" and no "Edit".
+    & > ul {
+      // But the scrollable element is not `<ul>` but `#message-list` -
+      // its parent?? This appears to still work.
+      & > *:not(.overflow-anchor) {
+        overflow-anchor: none;
+      }
+      &::after {
+        content: '';
+        height: 1px;
+        display: block;
+        overflow-anchor: auto;
+      }
+      & > :last-child {
+        margin-bottom: 0;
+      }
+    }
+
     &::-webkit-scrollbar-track {
       background: transparent;
     }

src/renderer/components/ReactionsBar/ReactionsBarContext.tsx

@@ -17,6 +17,8 @@ export type ReactionsBarValue = {
   showReactionsBar: (args: ShowReactionBar) => void
   hideReactionsBar: () => void
   isReactionsBarShown: boolean
+  /** `undefined` when `!isReactionsBarShown` */
+  reactionBarShownForMessageId: number | undefined
 }
 
 export const ReactionsBarContext =
@@ -37,6 +39,7 @@ export const ReactionsBarProvider = ({ children }: PropsWithChildren<{}>) => {
     showReactionsBar,
     hideReactionsBar,
     isReactionsBarShown: barArgs !== null,
+    reactionBarShownForMessageId: barArgs?.messageId,
   }
 
   return (

src/renderer/components/ReactionsBar/useReactionsBar.ts

@@ -8,7 +8,7 @@ import type { T } from '@deltachat/jsonrpc-client'
 
 type UseReactionsBar = Pick<
   ReactionsBarValue,
-  'hideReactionsBar' | 'isReactionsBarShown'
+  'hideReactionsBar' | 'isReactionsBarShown' | 'reactionBarShownForMessageId'
 > & {
   showReactionsBar: (
     args: Pick<ShowReactionBar, 'messageId' | 'x' | 'y'> & {

src/renderer/components/message/Message.tsx

@@ -36,7 +36,7 @@ import useOpenViewProfileDialog from '../../hooks/dialog/useOpenViewProfileDialo
 import usePrivateReply from '../../hooks/chat/usePrivateReply'
 import useTranslationFunction from '../../hooks/useTranslationFunction'
 import useVideoChat from '../../hooks/useVideoChat'
-import { useReactionsBar, showReactionsUi } from '../ReactionsBar'
+import { type useReactionsBar, showReactionsUi } from '../ReactionsBar'
 import EnterAutocryptSetupMessage from '../dialogs/EnterAutocryptSetupMessage'
 import { ContextMenuContext } from '../../contexts/ContextMenuContext'
 import Reactions from '../Reactions'
@@ -342,16 +342,16 @@ export default function Message(props: {
   chat: T.FullChat
   message: T.Message
   conversationType: ConversationType
+  showReactionsBar: ReturnType<typeof useReactionsBar>['showReactionsBar']
 }) {
-  const { message, conversationType } = props
+  const { message, conversationType, showReactionsBar } = props
   const { id, viewType, text, hasLocation, isSetupmessage, hasHtml } = message
   const direction = getDirection(message)
   const status = mapCoreMsgStatus2String(message.state)
 
   const tx = useTranslationFunction()
   const accountId = selectedAccountId()
 
-  const { showReactionsBar } = useReactionsBar()
   const { openDialog } = useDialog()
   const privateReply = usePrivateReply()
   const { openContextMenu } = useContext(ContextMenuContext)

src/renderer/components/message/MessageList.tsx

@@ -203,6 +203,31 @@ export default function MessageList({ accountId, chat, refComposer }: Props) {
     }
   }, [jumpToMessage])
 
+  const getDistanceToBottom = (el: HTMLElement) =>
+    el.scrollHeight - el.scrollTop - el.clientHeight
+
+  const needToShowJumpToBottom: () => boolean = useCallback(() => {
+    if (!messageListRef.current) {
+      return false
+    }
+    const distanceToBottom = getDistanceToBottom(messageListRef.current)
+
+    const isNewestMessageLoaded =
+      newestFetchedMessageListItemIndex === messageListItems.length - 1
+
+    return (
+      !isNewestMessageLoaded ||
+      distanceToBottom >= 10 /* 10 is close enough to 0 */
+    )
+  }, [messageListItems.length, newestFetchedMessageListItemIndex])
+
+  useLayoutEffect(() => {
+    const newShowJumpDownButton = needToShowJumpToBottom()
+    if (newShowJumpDownButton !== showJumpDownButton) {
+      setShowJumpDownButton(newShowJumpDownButton)
+    }
+  }, [needToShowJumpToBottom, showJumpDownButton])
+
   const onScroll = useCallback(
     (ev: React.UIEvent<HTMLDivElement> | null) => {
       if (!messageListRef.current) {
@@ -215,16 +240,9 @@ export default function MessageList({ accountId, chat, refComposer }: Props) {
       if (ev) hideReactionsBar()
 
       const distanceToTop = messageListRef.current.scrollTop
-      const distanceToBottom =
-        messageListRef.current.scrollHeight -
-        messageListRef.current.scrollTop -
-        messageListRef.current.clientHeight
-
-      const isNewestMessageLoaded =
-        newestFetchedMessageListItemIndex === messageListItems.length - 1
-      const newShowJumpDownButton =
-        !isNewestMessageLoaded ||
-        distanceToBottom >= 10 /* 10 is close enough to 0 */
+      const distanceToBottom = getDistanceToBottom(messageListRef.current)
+
+      const newShowJumpDownButton = needToShowJumpToBottom()
       if (newShowJumpDownButton != showJumpDownButton) {
         setShowJumpDownButton(newShowJumpDownButton)
       }
@@ -258,10 +276,8 @@ export default function MessageList({ accountId, chat, refComposer }: Props) {
       fetchMoreBottom,
       fetchMoreTop,
       hideReactionsBar,
-      messageListItems.length,
-      newestFetchedMessageListItemIndex,
+      needToShowJumpToBottom,
       scheduler,
-      setShowJumpDownButton,
       showJumpDownButton,
     ]
   )

src/renderer/components/message/MessageWrapper.tsx

@@ -6,6 +6,8 @@ import { ConversationType } from './MessageList'
 import { getLogger } from '../../../shared/logger'
 
 import type { T } from '@deltachat/jsonrpc-client'
+import classNames from 'classnames'
+import { useReactionsBar } from '../ReactionsBar'
 
 type RenderMessageProps = {
   key2: string
@@ -22,6 +24,8 @@ export function MessageWrapper(props: RenderMessageProps) {
   const shouldInViewObserve =
     state === C.DC_STATE_IN_FRESH || state === C.DC_STATE_IN_NOTICED
 
+  const { showReactionsBar, reactionBarShownForMessageId } = useReactionsBar()
+
   // Add this message to unreadMessageInViewIntersectionObserver to mark this message
   // as read if it's displayed on the screen
   useLayoutEffect(() => {
@@ -64,8 +68,13 @@ export function MessageWrapper(props: RenderMessageProps) {
   ])
 
   return (
-    <li id={props.key2} className='message-wrapper'>
-      <Message {...props} />
+    <li
+      id={props.key2}
+      className={classNames('message-wrapper', {
+        'overflow-anchor': reactionBarShownForMessageId === props.message.id,
+      })}
+    >
+      <Message showReactionsBar={showReactionsBar} {...props} />
       <div className='message-observer-bottom' id={'bottom-' + props.key2} />
     </li>
   )

src/renderer/stores/chat/chat_view_reducer.ts

@@ -84,22 +84,6 @@ export class ChatViewReducer {
     }
   }
 
-  static fetchedIncomingMessages(prevState: ChatViewState): ChatViewState {
-    const {
-      lastKnownScrollHeight,
-      // lastKnownScrollTop,
-    } = getLastKnownScrollPosition()
-
-    return {
-      ...prevState,
-      scrollTo: {
-        type: 'scrollToBottom',
-        ifClose: true,
-      },
-      lastKnownScrollHeight,
-    }
-  }
-
   static unlockScroll(prevState: ChatViewState): ChatViewState {
     return {
       ...prevState,

src/renderer/stores/messagelist.ts

@@ -233,7 +233,6 @@ class MessageListStore extends Store<MessageListState> {
             ...payload.newMessageCacheItems,
           },
           newestFetchedMessageListItemIndex: payload.newestFetchedMessageIndex,
-          viewState: ChatViewReducer.fetchedIncomingMessages(state.viewState),
         }
         return modifiedState
       }, 'fetchedIncomingMessages')