From 3481987d6d1230905b005f8893386b92c58b41bc Mon Sep 17 00:00:00 2001 From: Herman Wikner Date: Mon, 9 Oct 2023 12:20:00 +0200 Subject: [PATCH] feat: improve perf by memoizing the comments list component --- .../components/list/CommentThreadLayout.tsx | 10 - .../comments/components/list/CommentsList.tsx | 239 +++++++++--------- .../components/list/CreateNewThreadInput.tsx | 2 +- 3 files changed, 121 insertions(+), 130 deletions(-) diff --git a/packages/sanity/src/core/comments/components/list/CommentThreadLayout.tsx b/packages/sanity/src/core/comments/components/list/CommentThreadLayout.tsx index 040f0b781c6..8ec604f521c 100644 --- a/packages/sanity/src/core/comments/components/list/CommentThreadLayout.tsx +++ b/packages/sanity/src/core/comments/components/list/CommentThreadLayout.tsx @@ -49,16 +49,6 @@ export function CommentThreadLayout(props: CommentThreadLayoutProps) { const onCreateNewThreadClick = useCallback(() => { setDisplayNewThreadInput(true) - - // We need to wait for the next tick to focus the input - const raf = requestAnimationFrame(() => { - createNewThreadInputRef.current?.focus() - createNewThreadInputRef.current?.scrollTo() - }) - - return () => { - cancelAnimationFrame(raf) - } }, []) const handleNewThreadCreateDiscard = useCallback(() => { diff --git a/packages/sanity/src/core/comments/components/list/CommentsList.tsx b/packages/sanity/src/core/comments/components/list/CommentsList.tsx index c9537d49b4c..1b49fea9431 100644 --- a/packages/sanity/src/core/comments/components/list/CommentsList.tsx +++ b/packages/sanity/src/core/comments/components/list/CommentsList.tsx @@ -1,5 +1,5 @@ import React, {forwardRef, useCallback, useImperativeHandle, useMemo, useState} from 'react' -import {BoundaryElementProvider, Container, Flex, Spinner, Stack, Text} from '@sanity/ui' +import {Container, Flex, Spinner, Stack, Text} from '@sanity/ui' import {CurrentUser, Path} from '@sanity/types' import * as PathUtils from '@sanity/util/paths' import { @@ -69,131 +69,129 @@ export interface CommentsListHandle { * @beta * @hidden */ -export const CommentsList = forwardRef(function CommentsList( - props: CommentsListProps, - ref, -) { - const { - comments, - currentUser, - error, - loading, - mentionOptions, - onCreateRetry, - onDelete, - onEdit, - onNewThreadCreate, - onPathFocus, - onReply, - onStatusChange, - status, - } = props - const [boundaryElement, setBoundaryElement] = useState(null) +export const CommentsListInner = forwardRef( + function CommentsListInner(props: CommentsListProps, ref) { + const { + comments, + currentUser, + error, + loading, + mentionOptions, + onCreateRetry, + onDelete, + onEdit, + onNewThreadCreate, + onPathFocus, + onReply, + onStatusChange, + status, + } = props + const [boundaryElement, setBoundaryElement] = useState(null) - const scrollToComment = useCallback( - (id: string) => { - const commentElement = boundaryElement?.querySelector(`[data-comment-id="${id}"]`) - commentElement?.scrollIntoView(SCROLL_INTO_VIEW_OPTIONS) - }, - [boundaryElement], - ) + const scrollToComment = useCallback( + (id: string) => { + const commentElement = boundaryElement?.querySelector(`[data-comment-id="${id}"]`) + commentElement?.scrollIntoView(SCROLL_INTO_VIEW_OPTIONS) + }, + [boundaryElement], + ) - useImperativeHandle( - ref, - () => { - return { - scrollToComment, - } - }, - [scrollToComment], - ) + useImperativeHandle( + ref, + () => { + return { + scrollToComment, + } + }, + [scrollToComment], + ) - const showComments = !loading && !error && comments.length > 0 - const showEmptyState = !loading && !error && comments.length === 0 - const showError = error - const showLoading = loading && !error + const showComments = !loading && !error && comments.length > 0 + const showEmptyState = !loading && !error && comments.length === 0 + const showError = error + const showLoading = loading && !error - // We group the threads so that they can be rendered together under the - // same breadcrumbs. This is to avoid having the same breadcrumbs repeated - // for every single comment thread. Also, we don't want to have threads pointing - // to the same field to be rendered separately in the list since that makes it - // harder to get an overview of the comments about a specific field. - const groupedThreads = useMemo(() => { - const entries = Object.entries(groupThreads(comments)) + // We group the threads so that they can be rendered together under the + // same breadcrumbs. This is to avoid having the same breadcrumbs repeated + // for every single comment thread. Also, we don't want to have threads pointing + // to the same field to be rendered separately in the list since that makes it + // harder to get an overview of the comments about a specific field. + const groupedThreads = useMemo(() => { + const entries = Object.entries(groupThreads(comments)) - // Sort groupedThreads in descending order (newest first) base on the date of the - // last comment in the thread. - // This is to make sure that, when a new thread is added to the group, the group - // is not moved to the top of the list. - return entries.sort((a, b) => { - const [, threadA] = a - const [, threadB] = b + // Sort groupedThreads in descending order (newest first) base on the date of the + // last comment in the thread. + // This is to make sure that, when a new thread is added to the group, the group + // is not moved to the top of the list. + return entries.sort((a, b) => { + const [, threadA] = a + const [, threadB] = b - const lastCommentA = threadA[threadA.length - 1] - const lastCommentB = threadB[threadB.length - 1] + const lastCommentA = threadA[threadA.length - 1] + const lastCommentB = threadB[threadB.length - 1] - return lastCommentB.parentComment._createdAt.localeCompare( - lastCommentA.parentComment._createdAt, - ) - }) - }, [comments]) + return lastCommentB.parentComment._createdAt.localeCompare( + lastCommentA.parentComment._createdAt, + ) + }) + }, [comments]) - return ( - - {showEmptyState && ( - - - - - {EMPTY_STATE_MESSAGES[status].title} - - - {EMPTY_STATE_MESSAGES[status].message} - - - - - )} + return ( + + {showEmptyState && ( + + + + + {EMPTY_STATE_MESSAGES[status].title} + + + {EMPTY_STATE_MESSAGES[status].message} + + + + + )} - {showLoading && ( - - - - - Loading comments... - + {showLoading && ( + + + + + Loading comments... + + - - )} + )} - {showError && ( - - - - Something went wrong - + {showError && ( + + + + Something went wrong + + - - )} + )} - {showComments && ( - - + {showComments && ( + + {/* */} {groupedThreads?.map(([fieldPath, group]) => { // Since all threads in the group point to the same field, the breadcrumbs will be // the same for all of them. Therefore, we can just pick the first one. @@ -248,9 +246,12 @@ export const CommentsList = forwardRef(fu ) })} - - - )} - - ) -}) + {/* */} + + )} + + ) + }, +) + +export const CommentsList = React.memo(CommentsListInner) diff --git a/packages/sanity/src/core/comments/components/list/CreateNewThreadInput.tsx b/packages/sanity/src/core/comments/components/list/CreateNewThreadInput.tsx index c6ff1510a35..010cd4f420c 100644 --- a/packages/sanity/src/core/comments/components/list/CreateNewThreadInput.tsx +++ b/packages/sanity/src/core/comments/components/list/CreateNewThreadInput.tsx @@ -32,13 +32,13 @@ export const CreateNewThreadInput = forwardRef(function CreateNewThreadInput( return ( ) })