[Clipclops] Refactor message list (#3832)

* rework the list for accessibility * Reverse reverse * progress * good to start testing * memo `MessageItem` * small hack * use our custom `List` impl * use `ScrollProvider` for `onScroll` event * remove use of `runOnJS` * actually, let's keep it * add some comments --------- Co-authored-by: Eric Bailey <[email protected]>
bluesky-social · May 3, 2024 · 8768166 · 8768166
1 parent 6a4199f
commit 8768166
Show file tree

Hide file tree

Showing 7 changed files with 158 additions and 95 deletions.
diff --git a/src/components/dms/MessageItem.tsx b/src/components/dms/MessageItem.tsx
@@ -10,7 +10,7 @@ import {atoms as a, useTheme} from '#/alf'
 import {ActionsWrapper} from '#/components/dms/ActionsWrapper'
 import {Text} from '#/components/Typography'
 
-export function MessageItem({
+export let MessageItem = ({
   item,
   next,
   pending,
@@ -21,7 +21,7 @@ export function MessageItem({
     | ChatBskyConvoDefs.DeletedMessageView
     | null
   pending?: boolean
-}) {
+}): React.ReactNode => {
   const t = useTheme()
   const {currentAccount} = useSession()
 
@@ -97,15 +97,17 @@ export function MessageItem({
   )
 }
 
-export function MessageItemMetadata({
+MessageItem = React.memo(MessageItem)
+
+let MessageItemMetadata = ({
   message,
   isLastInGroup,
   style,
 }: {
   message: ChatBskyConvoDefs.MessageView
   isLastInGroup: boolean
   style: StyleProp<TextStyle>
-}) {
+}): React.ReactNode => {
   const t = useTheme()
   const {_} = useLingui()
 
@@ -174,6 +176,8 @@ export function MessageItemMetadata({
   )
 }
 
+MessageItemMetadata = React.memo(MessageItemMetadata)
+
 function localDateString(date: Date) {
   // can't use toISOString because it should be in local time
   const mm = date.getMonth()

diff --git a/src/screens/Messages/Conversation/MessageInput.tsx b/src/screens/Messages/Conversation/MessageInput.tsx
@@ -19,11 +19,9 @@ import {PaperPlane_Stroke2_Corner0_Rounded as PaperPlane} from '#/components/ico
 export function MessageInput({
   onSendMessage,
   onFocus,
-  onBlur,
 }: {
   onSendMessage: (message: string) => void
-  onFocus: () => void
-  onBlur: () => void
+  onFocus?: () => void
 }) {
   const {_} = useLingui()
   const t = useTheme()
@@ -85,7 +83,6 @@ export function MessageInput({
           scrollEnabled={isInputScrollable}
           blurOnSubmit={false}
           onFocus={onFocus}
-          onBlur={onBlur}
           onContentSizeChange={onInputLayout}
           ref={inputRef}
         />

diff --git a/src/screens/Messages/Conversation/MessageInput.web.tsx b/src/screens/Messages/Conversation/MessageInput.web.tsx
@@ -12,7 +12,6 @@ export function MessageInput({
 }: {
   onSendMessage: (message: string) => void
   onFocus: () => void
-  onBlur: () => void
 }) {
   const {_} = useLingui()
   const t = useTheme()

diff --git a/src/screens/Messages/Conversation/MessagesList.tsx b/src/screens/Messages/Conversation/MessagesList.tsx
@@ -1,21 +1,21 @@
 import React, {useCallback, useRef} from 'react'
-import {
-  FlatList,
-  NativeScrollEvent,
-  NativeSyntheticEvent,
-  Platform,
-  View,
-} from 'react-native'
+import {FlatList, Platform, View} from 'react-native'
 import {KeyboardAvoidingView} from 'react-native-keyboard-controller'
+import {runOnJS, useSharedValue} from 'react-native-reanimated'
+import {ReanimatedScrollEvent} from 'react-native-reanimated/lib/typescript/reanimated2/hook/commonTypes'
 import {useSafeAreaInsets} from 'react-native-safe-area-context'
 import {msg, Trans} from '@lingui/macro'
 import {useLingui} from '@lingui/react'
 
 import {isIOS} from '#/platform/detection'
 import {useChat} from '#/state/messages'
 import {ConvoItem, ConvoStatus} from '#/state/messages/convo'
+import {ScrollProvider} from 'lib/ScrollContext'
+import {isWeb} from 'platform/detection'
+import {List} from 'view/com/util/List'
 import {MessageInput} from '#/screens/Messages/Conversation/MessageInput'
 import {MessageListError} from '#/screens/Messages/Conversation/MessageListError'
+import {useScrollToEndOnFocus} from '#/screens/Messages/Conversation/useScrollToEndOnFocus'
 import {atoms as a, useBreakpoints} from '#/alf'
 import {Button, ButtonText} from '#/components/Button'
 import {MessageItem} from '#/components/dms/MessageItem'
@@ -79,36 +79,64 @@ function keyExtractor(item: ConvoItem) {
   return item.key
 }
 
-function onScrollToEndFailed() {
+function onScrollToIndexFailed() {
   // Placeholder function. You have to give FlatList something or else it will error.
 }
 
 export function MessagesList() {
   const chat = useChat()
   const flatListRef = useRef<FlatList>(null)
-  // We use this to know if we should scroll after a new clop is added to the list
-  const isAtBottom = useRef(false)
-  const currentOffset = React.useRef(0)
 
-  const onContentSizeChange = useCallback(() => {
-    if (currentOffset.current <= 100) {
-      flatListRef.current?.scrollToOffset({offset: 0, animated: true})
-    }
-  }, [])
+  // We need to keep track of when the scroll offset is at the bottom of the list to know when to scroll as new items
+  // are added to the list. For example, if the user is scrolled up to 1iew older messages, we don't want to scroll to
+  // the bottom.
+  const isAtBottom = useSharedValue(true)
+
+  // Used to keep track of the current content height. We'll need this in `onScroll` so we know when to start allowing
+  // onStartReached to fire.
+  const contentHeight = useSharedValue(0)
+
+  const [hasInitiallyScrolled, setHasInitiallyScrolled] = React.useState(false)
+
+  // This is only used on native because `Keyboard` can't be imported on web. On web, an input focus will immediately
+  // trigger scrolling to the bottom. On native however, we need to wait for the keyboard to present before scrolling,
+  // which is what this hook listens for
+  useScrollToEndOnFocus(flatListRef)
+
+  // Every time the content size changes, that means one of two things is happening:
+  // 1. New messages are being added from the log or from a message you have sent
+  // 2. Old messages are being prepended to the top
+  //
+  // The first time that the content size changes is when the initial items are rendered. Because we cannot rely on
+  // `initialScrollIndex`, we need to immediately scroll to the bottom of the list. That scroll will not be animated.
+  //
+  // Subsequent resizes will only scroll to the bottom if the user is at the bottom of the list (within 100 pixels of
+  // the bottom). Therefore, any new messages that come in or are sent will result in an animated scroll to end. However
+  // we will not scroll whenever new items get prepended to the top.
+  const onContentSizeChange = useCallback(
+    (_: number, height: number) => {
+      contentHeight.value = height
+
+      // This number _must_ be the height of the MaybeLoader component
+      if (height <= 50 || !isAtBottom.value) {
+        return
+      }
 
-  const onEndReached = useCallback(() => {
-    if (chat.status === ConvoStatus.Ready) {
-      chat.fetchMessageHistory()
-    }
-  }, [chat])
+      flatListRef.current?.scrollToOffset({
+        animated: hasInitiallyScrolled,
+        offset: height,
+      })
+    },
+    [contentHeight, hasInitiallyScrolled, isAtBottom.value],
+  )
 
-  const onInputFocus = useCallback(() => {
-    if (!isAtBottom.current) {
-      flatListRef.current?.scrollToOffset({offset: 0, animated: true})
+  // The check for `hasInitiallyScrolled` prevents an initial fetch on mount. FlatList triggers `onStartReached`
+  // immediately on mount, since we are in fact at an offset of zero, so we have to ignore those initial calls.
+  const onStartReached = useCallback(() => {
+    if (chat.status === ConvoStatus.Ready && hasInitiallyScrolled) {
+      chat.fetchMessageHistory()
     }
-  }, [])
-
-  const onInputBlur = useCallback(() => {}, [])
+  }, [chat, hasInitiallyScrolled])
 
   const onSendMessage = useCallback(
     (text: string) => {
@@ -122,12 +150,28 @@ export function MessagesList() {
   )
 
   const onScroll = React.useCallback(
-    (e: NativeSyntheticEvent<NativeScrollEvent>) => {
-      currentOffset.current = e.nativeEvent.contentOffset.y
+    (e: ReanimatedScrollEvent) => {
+      'worklet'
+      const bottomOffset = e.contentOffset.y + e.layoutMeasurement.height
+
+      // Most apps have a little bit of space the user can scroll past while still automatically scrolling ot the bottom
+      // when a new message is added, hence the 100 pixel offset
+      isAtBottom.value = e.contentSize.height - 100 < bottomOffset
+
+      // This number _must_ be the height of the MaybeLoader component.
+      // We don't check for zero, because the `MaybeLoader` component is always present, even when not visible, which
+      // adds a 50 pixel offset.
+      if (contentHeight.value > 50 && !hasInitiallyScrolled) {
+        runOnJS(setHasInitiallyScrolled)(true)
+      }
     },
-    [],
+    [contentHeight.value, hasInitiallyScrolled, isAtBottom],
   )
 
+  const onInputFocus = React.useCallback(() => {
+    flatListRef.current?.scrollToEnd({animated: true})
+  }, [flatListRef])
+
   const {bottom: bottomInset} = useSafeAreaInsets()
   const {gtMobile} = useBreakpoints()
   const bottomBarHeight = gtMobile ? 0 : isIOS ? 40 : 60
@@ -139,42 +183,41 @@ export function MessagesList() {
       keyboardVerticalOffset={keyboardVerticalOffset}
       behavior="padding"
       contentContainerStyle={a.flex_1}>
-      <FlatList
-        ref={flatListRef}
-        data={chat.status === ConvoStatus.Ready ? chat.items : undefined}
-        keyExtractor={keyExtractor}
-        renderItem={renderItem}
-        contentContainerStyle={{paddingHorizontal: 10}}
-        // In the future, we might want to adjust this value. Not very concerning right now as long as we are only
-        // dealing with text. But whenever we have images or other media and things are taller, we will want to lower
-        // this...probably.
-        initialNumToRender={20}
-        // Same with the max to render per batch. Let's be safe for now though.
-        maxToRenderPerBatch={25}
-        inverted={true}
-        onEndReached={onEndReached}
-        onScrollToIndexFailed={onScrollToEndFailed}
-        onContentSizeChange={onContentSizeChange}
-        onScroll={onScroll}
-        // We don't really need to call this much since there are not any animations that rely on this
-        scrollEventThrottle={100}
-        maintainVisibleContentPosition={{
-          minIndexForVisible: 1,
-        }}
-        ListFooterComponent={
-          <MaybeLoader
-            isLoading={
-              chat.status === ConvoStatus.Ready && chat.isFetchingHistory
+      {/* This view keeps the scroll bar and content within the CenterView on web, otherwise the entire window would scroll */}
+      {/* @ts-expect-error web only */}
+      <View style={[{flex: 1}, isWeb && {'overflow-y': 'scroll'}]}>
+        {/* Custom scroll provider so we can use the `onScroll` event in our custom List implementation */}
+        <ScrollProvider onScroll={onScroll}>
+          <List
+            ref={flatListRef}
+            data={chat.status === ConvoStatus.Ready ? chat.items : undefined}
+            renderItem={renderItem}
+            keyExtractor={keyExtractor}
+            disableVirtualization={true}
+            initialNumToRender={isWeb ? 50 : 25}
+            maxToRenderPerBatch={isWeb ? 50 : 25}
+            keyboardDismissMode="on-drag"
+            maintainVisibleContentPosition={{
+              minIndexForVisible: 1,
+            }}
+            removeClippedSubviews={false}
+            onContentSizeChange={onContentSizeChange}
+            onStartReached={onStartReached}
+            onScrollToIndexFailed={onScrollToIndexFailed}
+            scrollEventThrottle={100}
+            ListHeaderComponent={
+              <MaybeLoader
+                isLoading={
+                  chat.status === ConvoStatus.Ready && chat.isFetchingHistory
+                }
+              />
             }
           />
-        }
-        removeClippedSubviews={true}
-        keyboardDismissMode="on-drag"
-      />
+        </ScrollProvider>
+      </View>
       <MessageInput
         onSendMessage={onSendMessage}
-        onFocus={onInputFocus}
-        onBlur={onInputBlur}
+        onFocus={isWeb ? onInputFocus : undefined}
       />
     </KeyboardAvoidingView>
   )

diff --git a/src/screens/Messages/Conversation/useScrollToEndOnFocus.ts b/src/screens/Messages/Conversation/useScrollToEndOnFocus.ts
@@ -0,0 +1,16 @@
+import React from 'react'
+import {FlatList, Keyboard} from 'react-native'
+
+export function useScrollToEndOnFocus(flatListRef: React.RefObject<FlatList>) {
+  React.useEffect(() => {
+    const listener = Keyboard.addListener('keyboardDidShow', () => {
+      requestAnimationFrame(() => {
+        flatListRef.current?.scrollToEnd({animated: true})
+      })
+    })
+
+    return () => {
+      listener.remove()
+    }
+  }, [flatListRef])
+}
diff --git a/src/screens/Messages/Conversation/useScrollToEndOnFocus.web.ts b/src/screens/Messages/Conversation/useScrollToEndOnFocus.web.ts
@@ -0,0 +1,6 @@
+import React from 'react'
+import {FlatList} from 'react-native'
+
+// Stub for web
+// eslint-disable-next-line @typescript-eslint/no-unused-vars
+export function useScrollToEndOnFocus(flatListRef: React.RefObject<FlatList>) {}
diff --git a/src/state/messages/convo.ts b/src/state/messages/convo.ts
@@ -710,8 +710,11 @@ export class Convo {
   getItems(): ConvoItem[] {
     const items: ConvoItem[] = []
 
-    // `newMessages` is in insertion order, unshift to reverse
-    this.newMessages.forEach(m => {
+    this.headerItems.forEach(item => {
+      items.push(item)
+    })
+
+    this.pastMessages.forEach(m => {
       if (ChatBskyConvoDefs.isMessageView(m)) {
         items.unshift({
           type: 'message',
@@ -729,27 +732,7 @@ export class Convo {
       }
     })
 
-    // `newMessages` is in insertion order, unshift to reverse
-    this.pendingMessages.forEach(m => {
-      items.unshift({
-        type: 'pending-message',
-        key: m.id,
-        message: {
-          ...m.message,
-          id: nanoid(),
-          rev: '__fake__',
-          sentAt: new Date().toISOString(),
-          sender: this.sender,
-        },
-        nextMessage: null,
-      })
-    })
-
-    this.footerItems.forEach(item => {
-      items.unshift(item)
-    })
-
-    this.pastMessages.forEach(m => {
+    this.newMessages.forEach(m => {
       if (ChatBskyConvoDefs.isMessageView(m)) {
         items.push({
           type: 'message',
@@ -767,7 +750,22 @@ export class Convo {
       }
     })
 
-    this.headerItems.forEach(item => {
+    this.pendingMessages.forEach(m => {
+      items.push({
+        type: 'pending-message',
+        key: m.id,
+        message: {
+          ...m.message,
+          id: nanoid(),
+          rev: '__fake__',
+          sentAt: new Date().toISOString(),
+          sender: this.sender,
+        },
+        nextMessage: null,
+      })
+    })
+
+    this.footerItems.forEach(item => {
       items.push(item)
     })