From 876816675e84d4175072950f36af5e19d412ce9b Mon Sep 17 00:00:00 2001 From: Hailey Date: Fri, 3 May 2024 14:18:01 -0700 Subject: [PATCH] [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 --- src/components/dms/MessageItem.tsx | 12 +- .../Messages/Conversation/MessageInput.tsx | 5 +- .../Conversation/MessageInput.web.tsx | 1 - .../Messages/Conversation/MessagesList.tsx | 167 +++++++++++------- .../Conversation/useScrollToEndOnFocus.ts | 16 ++ .../Conversation/useScrollToEndOnFocus.web.ts | 6 + src/state/messages/convo.ts | 50 +++--- 7 files changed, 160 insertions(+), 97 deletions(-) create mode 100644 src/screens/Messages/Conversation/useScrollToEndOnFocus.ts create mode 100644 src/screens/Messages/Conversation/useScrollToEndOnFocus.web.ts diff --git a/src/components/dms/MessageItem.tsx b/src/components/dms/MessageItem.tsx index a8393c74..ba90dd14 100644 --- 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,7 +97,9 @@ export function MessageItem({ ) } -export function MessageItemMetadata({ +MessageItem = React.memo(MessageItem) + +let MessageItemMetadata = ({ message, isLastInGroup, style, @@ -105,7 +107,7 @@ export function MessageItemMetadata({ message: ChatBskyConvoDefs.MessageView isLastInGroup: boolean style: StyleProp -}) { +}): 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 index d450578f..e94a295e 100644 --- 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 index 48e815a2..fd13dd85 100644 --- 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 index 28cc4877..bc64d2b1 100644 --- a/src/screens/Messages/Conversation/MessagesList.tsx +++ b/src/screens/Messages/Conversation/MessagesList.tsx @@ -1,12 +1,8 @@ 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' @@ -14,8 +10,12 @@ 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(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) - const onEndReached = useCallback(() => { - if (chat.status === ConvoStatus.Ready) { + // 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 + } + + flatListRef.current?.scrollToOffset({ + animated: hasInitiallyScrolled, + offset: height, + }) + }, + [contentHeight, hasInitiallyScrolled, isAtBottom.value], + ) + + // 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() } - }, [chat]) - - const onInputFocus = useCallback(() => { - if (!isAtBottom.current) { - flatListRef.current?.scrollToOffset({offset: 0, animated: true}) - } - }, []) - - const onInputBlur = useCallback(() => {}, []) + }, [chat, hasInitiallyScrolled]) const onSendMessage = useCallback( (text: string) => { @@ -122,12 +150,28 @@ export function MessagesList() { ) const onScroll = React.useCallback( - (e: NativeSyntheticEvent) => { - 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}> - + {/* Custom scroll provider so we can use the `onScroll` event in our custom List implementation */} + + } /> - } - removeClippedSubviews={true} - keyboardDismissMode="on-drag" - /> + + ) diff --git a/src/screens/Messages/Conversation/useScrollToEndOnFocus.ts b/src/screens/Messages/Conversation/useScrollToEndOnFocus.ts new file mode 100644 index 00000000..e6e04c0b --- /dev/null +++ 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) { + 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 new file mode 100644 index 00000000..8ee30185 --- /dev/null +++ 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) {} diff --git a/src/state/messages/convo.ts b/src/state/messages/convo.ts index fe2095c4..a65e0c48 100644 --- 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,9 +732,26 @@ export class Convo { } }) - // `newMessages` is in insertion order, unshift to reverse + this.newMessages.forEach(m => { + if (ChatBskyConvoDefs.isMessageView(m)) { + items.push({ + type: 'message', + key: m.id, + message: m, + nextMessage: null, + }) + } else if (ChatBskyConvoDefs.isDeletedMessageView(m)) { + items.push({ + type: 'deleted-message', + key: m.id, + message: m, + nextMessage: null, + }) + } + }) + this.pendingMessages.forEach(m => { - items.unshift({ + items.push({ type: 'pending-message', key: m.id, message: { @@ -746,28 +766,6 @@ export class Convo { }) this.footerItems.forEach(item => { - items.unshift(item) - }) - - this.pastMessages.forEach(m => { - if (ChatBskyConvoDefs.isMessageView(m)) { - items.push({ - type: 'message', - key: m.id, - message: m, - nextMessage: null, - }) - } else if (ChatBskyConvoDefs.isDeletedMessageView(m)) { - items.push({ - type: 'deleted-message', - key: m.id, - message: m, - nextMessage: null, - }) - } - }) - - this.headerItems.forEach(item => { items.push(item) })