Review for load more comments in profile

- compose everything inside fetchComments
- put skip counter in ref and prevent unnecessary rerenders
- got rid of additional handlers for loading
- add spinner as loading indicator for loading of additional comments

frontend/app/components/profile/profile.module.css

@@ -193,3 +193,11 @@
   margin: auto;
   color: var(--color13);
 }
+
+.loadMoreWrapper {
+  display: flex;
+  justify-content: center;
+  align-items: center;
+  height: 35px;
+  flex-shrink: 0;
+}

frontend/app/components/profile/profile.spec.tsx

@@ -1,5 +1,7 @@
 import { h } from 'preact';
 import '@testing-library/jest-dom';
+import { waitFor } from '@testing-library/preact';
+import { fireEvent } from '@testing-library/dom';
 
 import { render } from 'tests/utils';
 import * as api from 'common/api';
@@ -7,7 +9,6 @@ import * as pq from 'utils/parse-query';
 import type { Comment, User } from 'common/types';
 
 import { Profile } from './profile';
-import { fireEvent } from '@testing-library/dom';
 
 const userParamsStub = {
   id: '1',
@@ -90,25 +91,24 @@ describe('<Profile />', () => {
       .spyOn(api, 'getUserComments')
       .mockImplementation(async () => ({ comments: commentsStub, count: commentsStub.length }));
 
-    const { findByText, queryByLabelText, queryByRole, queryByTestId } = render(<Profile />);
+    const { findByText, queryByRole, queryByTestId } = render(<Profile />);
 
     expect(await findByText('Comments')).toBeInTheDocument();
     expect(queryByTestId('comments-counter')).toHaveTextContent(commentsStub.length.toString());
-    expect(queryByLabelText('Loading...')).not.toBeInTheDocument();
     expect(queryByRole('button', { name: /retry/i })).not.toBeInTheDocument();
     expect(queryByRole('button', { name: /load more/i })).not.toBeInTheDocument();
   });
 
   it('should render user with comments with load more button', async () => {
-    const comments = new Array(15).fill(commentStub);
     jest.spyOn(pq, 'parseQuery').mockImplementation(() => userParamsStub);
-    jest.spyOn(api, 'getUserComments').mockImplementation(async () => ({ comments, count: comments.length }));
+    jest
+      .spyOn(api, 'getUserComments')
+      .mockImplementation(async () => ({ comments: new Array(10).fill(commentStub), count: 15 }));
 
-    const { findByText, queryByLabelText, queryByRole, queryByTestId } = render(<Profile />);
+    const { findByText, queryByRole, queryByTestId } = render(<Profile />);
 
     expect(await findByText('Comments')).toBeInTheDocument();
-    expect(queryByTestId('comments-counter')).toHaveTextContent(comments.length.toString());
-    expect(queryByLabelText('Loading...')).not.toBeInTheDocument();
+    expect(queryByTestId('comments-counter')).toHaveTextContent('15');
     expect(queryByRole('button', { name: /retry/i })).not.toBeInTheDocument();
     expect(queryByRole('button', { name: /load more/i })).toBeInTheDocument();
   });
@@ -167,13 +167,16 @@ describe('<Profile />', () => {
   });
 
   it('load more button should dissapear if there no more comments to fetch', async () => {
-    const comments = Array(20).fill(commentStub);
-    jest.spyOn(api, 'getUserComments').mockImplementation(async () => ({ comments, count: comments.length }));
+    jest
+      .spyOn(api, 'getUserComments')
+      .mockImplementation(async () => ({ comments: new Array(10).fill(commentStub), count: 15 }));
 
-    const { findByRole, queryByRole } = render(<Profile />);
+    const { findByRole, getByRole, queryByRole } = render(<Profile />);
 
     expect(await findByRole('button', { name: /load more/i })).toBeInTheDocument();
-    fireEvent.click(await findByRole('button', { name: /load more/i }));
+    fireEvent.click(getByRole('button', { name: /load more/i }));
+    expect(getByRole('presentation')).toHaveClass('spinner');
+    await waitFor(() => expect(queryByRole('presentation')).not.toBeInTheDocument());
     expect(queryByRole('button', { name: /load more/i })).not.toBeInTheDocument();
   });
 });

frontend/app/components/profile/profile.tsx

@@ -38,52 +38,31 @@ export function Profile() {
   const [isCommentsLoading, setIsCommentsLoading] = useState(false);
   const [error, setError] = useState(false);
   const [comments, setComments] = useState<CommentType[] | null>(null);
-  const [commentsAmount, setCommentsAmount] = useState<number | null>(null);
-  const [commentsSkipCounts, setCommentsSkipCounts] = useState<number>(0);
+  const [commentsAmount, setCommentsAmount] = useState(0);
+  // store skip count in ref because it don't affect the view
+  const commentsSkipCountsRef = useRef(0);
   const [isSigningOut, setSigningOut] = useState(false);
-  const isLoadMoreVisible = commentsAmount && commentsAmount > commentsSkipCounts + COMMENTS_LIMIT;
 
-  const fetchUserComments = useCallback(
-    async (skip: number = 0) => {
-      const { comments, count } = await getUserComments(user.id, { skip, limit: COMMENTS_LIMIT });
-      return { comments, count };
-    },
-    [user.id]
-  );
-
-  const fetchUserCommentsOnMount = useCallback(async () => {
+  const fetchComments = useCallback(async () => {
     setIsCommentsLoading(true);
     setError(false);
-    setComments(null);
-    setCommentsAmount(null);
 
     try {
-      const { comments, count } = await fetchUserComments();
+      const { comments, count } = await getUserComments(user.id, {
+        skip: commentsSkipCountsRef.current,
+        limit: COMMENTS_LIMIT,
+      });
 
-      setComments(comments);
+      // update skip count after successful fetch before rendering
+      commentsSkipCountsRef.current += COMMENTS_LIMIT;
+      setComments((c) => [...(c || []), ...comments]);
       setCommentsAmount(count);
     } catch (err) {
       setError(true);
     } finally {
       setIsCommentsLoading(false);
     }
-  }, [fetchUserComments]);
-
-  const fetchMoreUserComments = useCallback(
-    async (skipCounts: number) => {
-      setError(false);
-
-      try {
-        const { comments: nextComments, count } = await fetchUserComments(skipCounts);
-
-        setComments([...(comments || []), ...nextComments]);
-        setCommentsAmount(count);
-      } catch (err) {
-        setError(true);
-      }
-    },
-    [comments, fetchUserComments]
-  );
+  }, []);
 
   function handleClickClose() {
     const rootElement = rootRef.current;
@@ -107,20 +86,9 @@ export function Profile() {
     await signout();
   }
 
-  function handleLoadMore() {
-    const nextSkipCounts = commentsSkipCounts + COMMENTS_LIMIT;
-
-    setCommentsSkipCounts(nextSkipCounts);
-    fetchMoreUserComments(nextSkipCounts);
-  }
-
-  function handleClickRetryCommentsRequest() {
-    fetchMoreUserComments(commentsSkipCounts);
-  }
-
   useEffect(() => {
-    fetchUserCommentsOnMount();
-  }, [fetchUserCommentsOnMount]);
+    fetchComments();
+  }, [fetchComments]);
 
   useEffect(() => {
     const styles = { height: '100%', padding: 0 };
@@ -151,6 +119,7 @@ export function Profile() {
     return null;
   }
 
+  const isLoadMoreVisible = commentsAmount > commentsSkipCountsRef.current;
   const isCurrent = user.current === '1';
   const commentsJSX = comments?.length ? (
     <>
@@ -162,7 +131,7 @@ export function Profile() {
             <FormattedMessage key="user.comments" id="user.comments" defaultMessage="Comments" />
           )}
         </h3>
-        {!!commentsAmount && (
+        {commentsAmount > 0 && (
           <div className={styles.counterWrapper}>
             <Counter>{commentsAmount}</Counter>
           </div>
@@ -181,9 +150,15 @@ export function Profile() {
         />
       ))}
       {isLoadMoreVisible && (
-        <Button kind="link" size="sm" onClick={handleLoadMore}>
-          <FormattedMessage id="user.load-more" defaultMessage="Load more" />
-        </Button>
+        <div className={styles.loadMoreWrapper}>
+          {isCommentsLoading ? (
+            <Spinner color="gray" />
+          ) : (
+            <Button kind="link" size="sm" onClick={fetchComments}>
+              <FormattedMessage id="user.load-more" defaultMessage="Load more" />
+            </Button>
+          )}
+        </div>
       )}
     </>
   ) : (
@@ -228,7 +203,7 @@ export function Profile() {
               <p className={clsx('profile-error', styles.error)}>
                 <FormattedMessage id="errors.0" defaultMessage="Something went wrong. Please try again a bit later." />
               </p>
-              <Button kind="link" size="sm" onClick={handleClickRetryCommentsRequest}>
+              <Button kind="link" size="sm" onClick={fetchComments}>
                 <FormattedMessage id="retry" defaultMessage="Retry" />
               </Button>
             </div>

frontend/app/components/spinner/spinner.module.css

@@ -7,6 +7,16 @@
   animation: spin 1s linear infinite;
 }
 
+.dark {
+  border: 2px solid var(--color46);
+  border-right-color: var(--color37);
+}
+
+:global(.dark) & {
+  border: 2px solid rgba(var(--white-color), 0.2);
+  border-right-color: rgb(var(--white-color));
+}
+
 @keyframes spin {
   0% {
     transform: rotate(0deg);

frontend/app/components/spinner/spinner.tsx

@@ -5,12 +5,16 @@ import { useIntl } from 'react-intl';
 
 import styles from './spinner.module.css';
 
-export function Spinner() {
+type Props = {
+  color?: 'white' | 'gray';
+};
+
+export function Spinner({ color }: Props) {
   const intl = useIntl();
 
   return (
     <div
-      className={clsx('spinner', styles.root)}
+      className={clsx('spinner', styles.root, { [styles.dark]: color === 'gray' })}
       role="presentation"
       aria-label={intl.formatMessage(messages.loading)}
     />