Skip to content

Commit c2e5287

Browse files
authored
fix: scrollBehavior should not affect all scroll behaviors (#1003)
Currently, scrollBehavior is being applied to the entire scroll area. This can result in an undesirable user experience when loading messages or updating message heights, leading to a bad UX where the message scroll jumps from end to end. We have revised it to avoid setting scrollBehavior to the entire scroll area and improved it by explicitly specifying the animated value for parts that should not be smooth. Browser: Safari AS-IS **[00:00 ~ 00:10]** TO-BE **[00:36 ~ 00:53]** <video src="https://github.com/sendbird/sendbird-uikit-react/assets/26326015/3a3f4d68-a376-444e-b40d-cc9b947a8016" /> from: https://sendbird.slack.com/archives/C0585965FFA/p1709753211213979
1 parent 6bd3822 commit c2e5287

File tree

8 files changed

+83
-116
lines changed

8 files changed

+83
-116
lines changed

src/modules/GroupChannel/components/Message/MessageView.tsx

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ export interface MessageViewProps extends MessageProps {
9090
setHighLightedMessageId?: React.Dispatch<React.SetStateAction<number>>;
9191
/** @deprecated * */
9292
onMessageHighlighted?: () => void;
93+
usedInLegacy?: boolean;
9394
}
9495

9596
// TODO: Refactor this component, is too complex now
@@ -138,6 +139,7 @@ const MessageView = (props: MessageViewProps) => {
138139
setAnimatedMessageId,
139140
animatedMessageId,
140141
onMessageAnimated,
142+
usedInLegacy = true,
141143
} = props;
142144

143145
const { dateLocale, stringSet } = useLocalization();
@@ -192,8 +194,8 @@ const MessageView = (props: MessageViewProps) => {
192194
}, [message?.updatedAt, (message as UserMessage)?.message]);
193195

194196
useLayoutEffect(() => {
195-
// Keep the scrollBottom value after fetching new message list
196-
handleScroll?.(true);
197+
// Keep the scrollBottom value after fetching new message list (but GroupChannel module is not needed.)
198+
if (usedInLegacy) handleScroll?.(true);
197199
}, []);
198200

199201
useLayoutEffect(() => {

src/modules/GroupChannel/components/Message/index.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ export const Message = (props: MessageProps): React.ReactElement => {
8181
onMessageAnimated={onMessageAnimated}
8282
renderFileViewer={(props) => <FileViewer {...props} />}
8383
renderRemoveMessageModal={(props) => <RemoveMessageModal {...props} />}
84+
usedInLegacy={false}
8485
/>
8586
);
8687
};

src/modules/GroupChannel/components/MessageList/hooks/__test__/useScrollBehavior.spec.ts

Lines changed: 0 additions & 35 deletions
This file was deleted.

src/modules/GroupChannel/components/MessageList/hooks/useScrollBehavior.ts

Lines changed: 0 additions & 17 deletions
This file was deleted.

src/modules/GroupChannel/components/MessageList/index.tsx

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ import FrozenNotification from '../FrozenNotification';
1515
import { SCROLL_BUFFER } from '../../../../utils/consts';
1616
import useSendbirdStateContext from '../../../../hooks/useSendbirdStateContext';
1717
import { MessageProvider } from '../../../Message/context/MessageProvider';
18-
import { useScrollBehavior } from './hooks/useScrollBehavior';
1918
import TypingIndicatorBubble from '../../../../ui/TypingIndicatorBubble';
2019
import { useGroupChannelContext } from '../../context/GroupChannelProvider';
2120
import { getComponentKeyFromMessage } from '../../context/utils';
@@ -82,7 +81,6 @@ export const MessageList = ({
8281

8382
const [unreadSinceDate, setUnreadSinceDate] = useState<Date>();
8483

85-
useScrollBehavior();
8684
useEffect(() => {
8785
if (isScrollBottomReached) {
8886
setUnreadSinceDate(undefined);
@@ -104,7 +102,7 @@ export const MessageList = ({
104102
if (latestDistance < currentDistance && (!isBottomMessageAffected || latestDistance < SCROLL_BUFFER)) {
105103
const diff = currentDistance - latestDistance;
106104
// Move the scroll as much as the height of the message has changed
107-
scrollPubSub.publish('scroll', { top: elem.scrollTop + diff, lazy: false });
105+
scrollPubSub.publish('scroll', { top: elem.scrollTop + diff, lazy: false, animated: false });
108106
}
109107
}
110108
};
@@ -121,7 +119,7 @@ export const MessageList = ({
121119
className="sendbird-conversation__messages__notification"
122120
count={newMessages.length}
123121
lastReadAt={unreadSinceDate}
124-
onClick={scrollToBottom}
122+
onClick={() => scrollToBottom()}
125123
/>
126124
);
127125
},
@@ -131,8 +129,8 @@ export const MessageList = ({
131129
return (
132130
<div
133131
className="sendbird-conversation__scroll-bottom-button"
134-
onClick={scrollToBottom}
135-
onKeyDown={scrollToBottom}
132+
onClick={() => scrollToBottom()}
133+
onKeyDown={() => scrollToBottom()}
136134
tabIndex={0}
137135
role="button"
138136
>

src/modules/GroupChannel/context/GroupChannelProvider.tsx

Lines changed: 50 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ export interface GroupChannelContextType extends ContextBaseType, MessageListDat
9191
isScrollBottomReached: boolean;
9292
setIsScrollBottomReached: React.Dispatch<React.SetStateAction<boolean>>;
9393

94-
scrollToBottom: () => void;
94+
scrollToBottom: (animated?: boolean) => void;
9595
scrollToMessage: (createdAt: number, messageId: number) => void;
9696
toggleReaction(message: SendableMessageType, emojiKey: string, isReacted: boolean): void;
9797
}
@@ -145,7 +145,7 @@ export const GroupChannelProvider = (props: GroupChannelProviderProps) => {
145145
const [fetchChannelError, setFetchChannelError] = useState<SendbirdError>(null);
146146

147147
// Ref
148-
const { scrollRef, scrollPubSub, scrollDistanceFromBottomRef, isScrollBottomReached, setIsScrollBottomReached } = useMessageListScroll();
148+
const { scrollRef, scrollPubSub, scrollDistanceFromBottomRef, isScrollBottomReached, setIsScrollBottomReached } = useMessageListScroll(scrollBehavior);
149149
const messageInputRef = useRef(null);
150150

151151
const toggleReaction = useToggleReactionCallback(currentChannel, config.logger);
@@ -179,7 +179,7 @@ export const GroupChannelProvider = (props: GroupChannelProviderProps) => {
179179
},
180180
onMessagesReceived: () => {
181181
if (isScrollBottomReached && isContextMenuClosed()) {
182-
scrollPubSub.publish('scrollToBottom', null);
182+
scrollPubSub.publish('scrollToBottom', {});
183183
}
184184
},
185185
onChannelDeleted: () => {
@@ -211,7 +211,7 @@ export const GroupChannelProvider = (props: GroupChannelProviderProps) => {
211211

212212
if (viewUpdated) {
213213
const bottomOffset = prevViewInfo.scrollHeight - prevViewInfo.scrollTop;
214-
scrollPubSub.publish('scroll', { top: nextViewInfo.scrollHeight - bottomOffset, lazy: false });
214+
scrollPubSub.publish('scroll', { top: nextViewInfo.scrollHeight - bottomOffset, lazy: false, animated: false });
215215
}
216216
});
217217
});
@@ -232,7 +232,7 @@ export const GroupChannelProvider = (props: GroupChannelProviderProps) => {
232232
const viewUpdated = prevViewInfo.scrollHeight < nextViewInfo.scrollHeight;
233233

234234
if (viewUpdated) {
235-
scrollPubSub.publish('scroll', { top: prevViewInfo.scrollTop, lazy: false });
235+
scrollPubSub.publish('scroll', { top: prevViewInfo.scrollTop, lazy: false, animated: false });
236236
}
237237
});
238238
});
@@ -273,22 +273,22 @@ export const GroupChannelProvider = (props: GroupChannelProviderProps) => {
273273
preventDuplicateRequest.lock();
274274
await preventDuplicateRequest.run(() => {
275275
return new Promise<void>((resolve) => {
276-
scrollPubSub.publish('scrollToBottom', resolve);
276+
scrollPubSub.publish('scrollToBottom', { resolve, animated: false });
277277
});
278278
});
279279
preventDuplicateRequest.release();
280280
}
281281

282282
const onSentMessageFromOtherModule = (data: PubSubSendMessagePayload) => {
283-
if (data.channel.url === channelUrl) scrollPubSub.publish('scrollToBottom', null);
283+
if (data.channel.url === channelUrl) scrollPubSub.publish('scrollToBottom', {});
284284
};
285285
const subscribes = [
286286
config.pubSub.subscribe(PUBSUB_TOPICS.SEND_USER_MESSAGE, onSentMessageFromOtherModule),
287287
config.pubSub.subscribe(PUBSUB_TOPICS.SEND_FILE_MESSAGE, onSentMessageFromOtherModule),
288288
];
289289
return () => {
290290
subscribes.forEach((subscribe) => subscribe.remove());
291-
scrollPubSub.publish('scrollToBottom', null);
291+
scrollPubSub.publish('scrollToBottom', { animated: false });
292292
};
293293
}, [messageDataSource.initialized, channelUrl]);
294294

@@ -297,7 +297,7 @@ export const GroupChannelProvider = (props: GroupChannelProviderProps) => {
297297
if (typeof startingPoint === 'number') {
298298
// We do not handle animation for message search here.
299299
// Please update the animatedMessageId prop to trigger the animation.
300-
scrollToMessage(startingPoint, 0, false);
300+
scrollToMessage(startingPoint, 0, false, false);
301301
}
302302
}, [startingPoint]);
303303

@@ -306,17 +306,17 @@ export const GroupChannelProvider = (props: GroupChannelProviderProps) => {
306306
if (_animatedMessageId) setAnimatedMessageId(_animatedMessageId);
307307
}, [_animatedMessageId]);
308308

309-
const scrollToBottom = usePreservedCallback(async () => {
309+
const scrollToBottom = usePreservedCallback(async (animated?: boolean) => {
310310
if (!scrollRef.current) return;
311311

312312
setAnimatedMessageId(null);
313313
setIsScrollBottomReached(true);
314314

315315
if (config.isOnline && messageDataSource.hasNext()) {
316316
await messageDataSource.resetWithStartingPoint(Number.MAX_SAFE_INTEGER);
317-
scrollPubSub.publish('scrollToBottom', null);
317+
scrollPubSub.publish('scrollToBottom', { animated });
318318
} else {
319-
scrollPubSub.publish('scrollToBottom', null);
319+
scrollPubSub.publish('scrollToBottom', { animated });
320320
}
321321

322322
if (currentChannel && !messageDataSource.hasNext()) {
@@ -325,43 +325,45 @@ export const GroupChannelProvider = (props: GroupChannelProviderProps) => {
325325
}
326326
});
327327

328-
const scrollToMessage = usePreservedCallback(async (createdAt: number, messageId: number, animated = true) => {
329-
// NOTE: To prevent multiple clicks on the message in the channel while scrolling
330-
// Check if it can be replaced with event.stopPropagation()
331-
const element = scrollRef.current;
332-
const parentNode = element?.parentNode as HTMLDivElement;
333-
const clickHandler = {
334-
activate() {
335-
if (!element || !parentNode) return;
336-
element.style.pointerEvents = 'auto';
337-
parentNode.style.cursor = 'auto';
338-
},
339-
deactivate() {
340-
if (!element || !parentNode) return;
341-
element.style.pointerEvents = 'none';
342-
parentNode.style.cursor = 'wait';
343-
},
344-
};
345-
346-
clickHandler.deactivate();
347-
348-
setAnimatedMessageId(null);
349-
const message = messageDataSource.messages.find((it) => it.messageId === messageId || it.createdAt === createdAt);
350-
if (message) {
351-
const topOffset = getMessageTopOffset(message.createdAt);
352-
if (topOffset) scrollPubSub.publish('scroll', { top: topOffset });
353-
if (animated) setAnimatedMessageId(messageId);
354-
} else {
355-
await messageDataSource.resetWithStartingPoint(createdAt);
356-
setTimeout(() => {
357-
const topOffset = getMessageTopOffset(createdAt);
358-
if (topOffset) scrollPubSub.publish('scroll', { top: topOffset, lazy: false });
359-
if (animated) setAnimatedMessageId(messageId);
360-
});
361-
}
328+
const scrollToMessage = usePreservedCallback(
329+
async (createdAt: number, messageId: number, messageFocusAnimated?: boolean, scrollAnimated?: boolean) => {
330+
// NOTE: To prevent multiple clicks on the message in the channel while scrolling
331+
// Check if it can be replaced with event.stopPropagation()
332+
const element = scrollRef.current;
333+
const parentNode = element?.parentNode as HTMLDivElement;
334+
const clickHandler = {
335+
activate() {
336+
if (!element || !parentNode) return;
337+
element.style.pointerEvents = 'auto';
338+
parentNode.style.cursor = 'auto';
339+
},
340+
deactivate() {
341+
if (!element || !parentNode) return;
342+
element.style.pointerEvents = 'none';
343+
parentNode.style.cursor = 'wait';
344+
},
345+
};
346+
347+
clickHandler.deactivate();
348+
349+
setAnimatedMessageId(null);
350+
const message = messageDataSource.messages.find((it) => it.messageId === messageId || it.createdAt === createdAt);
351+
if (message) {
352+
const topOffset = getMessageTopOffset(message.createdAt);
353+
if (topOffset) scrollPubSub.publish('scroll', { top: topOffset, animated: scrollAnimated });
354+
if (messageFocusAnimated ?? true) setAnimatedMessageId(messageId);
355+
} else {
356+
await messageDataSource.resetWithStartingPoint(createdAt);
357+
setTimeout(() => {
358+
const topOffset = getMessageTopOffset(createdAt);
359+
if (topOffset) scrollPubSub.publish('scroll', { top: topOffset, lazy: false, animated: scrollAnimated });
360+
if (messageFocusAnimated ?? true) setAnimatedMessageId(messageId);
361+
});
362+
}
362363

363-
clickHandler.activate();
364-
});
364+
clickHandler.activate();
365+
},
366+
);
365367

366368
const messageActions = useMessageActions({ ...props, ...messageDataSource, scrollToBottom, quoteMessage, replyType });
367369

src/modules/GroupChannel/context/hooks/useMessageActions.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ type MessageActions = {
3333
};
3434

3535
interface Params extends GroupChannelProviderProps, MessageListDataSource {
36-
scrollToBottom(): Promise<void>;
36+
scrollToBottom(animated?: boolean): Promise<void>;
3737
quoteMessage?: SendableMessageType;
3838
replyType: ReplyType;
3939
}

src/modules/GroupChannel/context/hooks/useMessageListScroll.ts

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,10 @@ export type ScrollTopics = 'scrollToBottom' | 'scroll';
1010
export type ScrollTopicUnion =
1111
| {
1212
topic: 'scrollToBottom';
13-
payload: undefined | null | PromiseResolver;
13+
payload: {
14+
animated?: boolean;
15+
resolve?: PromiseResolver;
16+
};
1417
}
1518
| {
1619
topic: 'scroll';
@@ -32,7 +35,12 @@ function runCallback(callback: () => void, lazy = true) {
3235
}
3336
}
3437

35-
export function useMessageListScroll() {
38+
function getScrollBehavior(behavior: 'smooth' | 'auto', animated?: boolean) {
39+
if (typeof animated === 'boolean') return animated ? 'smooth' : 'auto';
40+
return behavior;
41+
}
42+
43+
export function useMessageListScroll(behavior: 'smooth' | 'auto') {
3644
const scrollRef = useRef<HTMLDivElement>(null);
3745
const scrollDistanceFromBottomRef = useRef(0);
3846

@@ -43,11 +51,15 @@ export function useMessageListScroll() {
4351
const unsubscribes: { remove(): void }[] = [];
4452

4553
unsubscribes.push(
46-
scrollPubSub.subscribe('scrollToBottom', (resolve) => {
54+
scrollPubSub.subscribe('scrollToBottom', ({ resolve, animated }) => {
4755
runCallback(() => {
4856
if (!scrollRef.current) return;
4957

50-
scrollRef.current.scrollTop = scrollRef.current.scrollHeight;
58+
if (scrollRef.current.scroll) {
59+
scrollRef.current.scroll({ top: scrollRef.current.scrollHeight, behavior: getScrollBehavior(behavior, animated) });
60+
} else {
61+
scrollRef.current.scrollTop = scrollRef.current.scrollHeight;
62+
}
5163

5264
// Update data by manual update
5365
scrollDistanceFromBottomRef.current = 0;
@@ -59,12 +71,16 @@ export function useMessageListScroll() {
5971
);
6072

6173
unsubscribes.push(
62-
scrollPubSub.subscribe('scroll', ({ top, animated = false, lazy, resolve }) => {
74+
scrollPubSub.subscribe('scroll', ({ top, animated, lazy, resolve }) => {
6375
runCallback(() => {
6476
if (!scrollRef.current) return;
6577
const { scrollTop, scrollHeight, clientHeight } = scrollRef.current;
6678

67-
scrollRef.current.scroll({ top, behavior: animated ? 'smooth' : 'auto' });
79+
if (scrollRef.current.scroll) {
80+
scrollRef.current.scroll({ top, behavior: getScrollBehavior(behavior, animated) });
81+
} else {
82+
scrollRef.current.scrollTop = top;
83+
}
6884

6985
// Update data by manual update
7086
scrollDistanceFromBottomRef.current = Math.max(0, scrollHeight - scrollTop - clientHeight);
@@ -78,7 +94,7 @@ export function useMessageListScroll() {
7894
return () => {
7995
unsubscribes.forEach(({ remove }) => remove());
8096
};
81-
}, []);
97+
}, [behavior]);
8298

8399
// Update data by scroll events
84100
useOnScrollPositionChangeDetectorWithRef(scrollRef, {

0 commit comments

Comments
 (0)