diff --git a/dotcom-rendering/src/components/Discussion.tsx b/dotcom-rendering/src/components/Discussion.tsx index 86808242802..5cab7ca56ff 100644 --- a/dotcom-rendering/src/components/Discussion.tsx +++ b/dotcom-rendering/src/components/Discussion.tsx @@ -1,15 +1,17 @@ import { css } from '@emotion/react'; -import { joinUrl, storage } from '@guardian/libs'; +import { storage } from '@guardian/libs'; import { palette, space } from '@guardian/source-foundations'; import { Button, SvgPlus } from '@guardian/source-react-components'; import { useEffect, useState } from 'react'; +import { getDiscussion } from '../lib/discussionApi'; import { getCommentContext, initFiltersFromLocalStorage, } from '../lib/getCommentContext'; -import { useDiscussion } from '../lib/useDiscussion'; +import { useCommentCount } from '../lib/useCommentCount'; import { palette as themePalette } from '../palette'; import type { + CommentType, FilterOptions, SignedInUser, ThreadsType, @@ -90,22 +92,38 @@ export const Discussion = ({ user, idApiUrl, }: Props) => { - const [commentPage, setCommentPage] = useState(1); + const [commentPage, setCommentPage] = useState(1); const [commentPageSize, setCommentPageSize] = useState<25 | 50 | 100>(); const [commentOrderBy, setCommentOrderBy] = useState< 'newest' | 'oldest' | 'recommendations' >(); - const [isExpanded, setIsExpanded] = useState(false); + const [comments, setComments] = useState([]); + const [isClosedForComments, setIsClosedForComments] = useState(false); + const [isExpanded, setIsExpanded] = useState(false); const [hashCommentId, setHashCommentId] = useState( commentIdFromUrl(), ); const [filters, setFilters] = useState( initFiltersFromLocalStorage(), ); + const [loading, setLoading] = useState(true); + const [totalPages, setTotalPages] = useState(0); - const { commentCount, isClosedForComments } = useDiscussion( - joinUrl(discussionApiUrl, 'discussion', shortUrlId), - ); + const commentCount = useCommentCount(discussionApiUrl, shortUrlId); + + useEffect(() => { + setLoading(true); + void getDiscussion(shortUrlId, { ...filters, page: commentPage }) + .then((json) => { + setLoading(false); + if (json && json.status !== 'error') { + setComments(json.discussion.comments); + setIsClosedForComments(json.discussion.isClosedForComments); + } + if (json?.pages != null) setTotalPages(json.pages); + }) + .catch((e) => console.error(`getDiscussion - error: ${String(e)}`)); + }, [filters, commentPage, shortUrlId]); useEffect(() => { const orderByClosed = isClosedForComments ? 'oldest' : undefined; @@ -168,6 +186,19 @@ export const Discussion = ({ } }, [commentCount]); + useEffect(() => { + if (window.location.hash === '#comments') { + setIsExpanded(true); + } + }, []); + + useEffect(() => { + // There's no point showing the view more button if there isn't much more to view + if (commentCount === 0 || commentCount === 1 || commentCount === 2) { + setIsExpanded(true); + } + }, [commentCount]); + return ( <>
@@ -209,6 +240,11 @@ export const Discussion = ({ setPage={setCommentPage} filters={filters} setFilters={setFilters} + commentCount={commentCount ?? 0} + loading={loading} + totalPages={totalPages} + comments={comments} + setComments={setComments} /> {!isExpanded && (
diff --git a/dotcom-rendering/src/components/Discussion/Comments.stories.tsx b/dotcom-rendering/src/components/Discussion/Comments.stories.tsx index a727f526afa..224330e1341 100644 --- a/dotcom-rendering/src/components/Discussion/Comments.stories.tsx +++ b/dotcom-rendering/src/components/Discussion/Comments.stories.tsx @@ -2,6 +2,9 @@ import { css } from '@emotion/react'; import { ArticleDesign, ArticleDisplay, Pillar } from '@guardian/libs'; import { splitTheme } from '../../../.storybook/decorators/splitThemeDecorator'; import { lightDecorator } from '../../../.storybook/decorators/themeDecorator'; +import { discussion as discussionMock } from '../../../fixtures/manual/discussion'; +import { discussionWithTwoComments } from '../../../fixtures/manual/discussionWithTwoComments'; +import { legacyDiscussionWithoutThreading } from '../../../fixtures/manual/legacyDiscussionWithoutThreading'; import type { FilterOptions, SignedInUser } from '../../types/discussion'; import { Comments } from './Comments'; @@ -45,7 +48,7 @@ export const LoggedOutHiddenPicks = () => ( `} > ( setPage={() => {}} filters={filters} setFilters={() => {}} + commentCount={discussionMock.discussion.commentCount} + loading={false} + totalPages={discussionMock.pages} + comments={discussionMock.discussion.comments} + setComments={() => {}} />
); @@ -82,7 +90,7 @@ export const InitialPage = () => ( `} > ( onExpand={() => {}} apiKey="" idApiUrl="https://idapi.theguardian.com" - page={3} + page={1} setPage={() => {}} filters={filters} setFilters={() => {}} + commentCount={discussionMock.discussion.commentCount} + loading={false} + totalPages={discussionMock.pages} + comments={discussionMock.discussion.comments} + setComments={() => {}} />
); -InitialPage.storyName = 'with initial page set to 3'; +InitialPage.storyName = 'with initial page set to 1'; InitialPage.decorators = [ splitTheme([ { @@ -136,6 +149,11 @@ export const LoggedInHiddenNoPicks = () => ( setPage={() => {}} filters={filters} setFilters={() => {}} + commentCount={discussionMock.discussion.commentCount} + loading={false} + totalPages={discussionMock.pages} + comments={discussionMock.discussion.comments} + setComments={() => {}} /> ); @@ -168,6 +186,11 @@ export const LoggedIn = () => ( setPage={() => {}} filters={filters} setFilters={() => {}} + commentCount={discussionMock.discussion.commentCount} + loading={false} + totalPages={discussionMock.pages} + comments={discussionMock.discussion.comments} + setComments={() => {}} /> ); @@ -182,7 +205,7 @@ export const LoggedInShortDiscussion = () => ( `} > ( setPage={() => {}} filters={filters} setFilters={() => {}} + commentCount={discussionWithTwoComments.discussion.commentCount} + loading={false} + totalPages={discussionWithTwoComments.pages} + comments={discussionWithTwoComments.discussion.comments} + setComments={() => {}} /> ); @@ -229,6 +257,11 @@ export const LoggedOutHiddenNoPicks = () => ( setPage={() => {}} filters={filters} setFilters={() => {}} + commentCount={discussionMock.discussion.commentCount} + loading={false} + totalPages={0} + comments={discussionMock.discussion.comments} + setComments={() => {}} /> ); @@ -251,7 +284,7 @@ export const Closed = () => ( `} > ( setPage={() => {}} filters={filters} setFilters={() => {}} + commentCount={discussionMock.discussion.commentCount} + loading={false} + totalPages={discussionMock.pages} + comments={discussionMock.discussion.comments} + setComments={() => {}} /> ); @@ -305,6 +343,11 @@ export const NoComments = () => ( setPage={() => {}} filters={filters} setFilters={() => {}} + commentCount={0} + loading={false} + totalPages={0} + comments={[]} + setComments={() => {}} /> ); @@ -326,7 +369,7 @@ export const LegacyDiscussion = () => ( `} > ( setPage={() => {}} filters={filters} setFilters={() => {}} + commentCount={ + legacyDiscussionWithoutThreading.discussion.commentCount + } + loading={false} + totalPages={legacyDiscussionWithoutThreading.pages} + comments={legacyDiscussionWithoutThreading.discussion.comments} + setComments={() => {}} /> ); diff --git a/dotcom-rendering/src/components/Discussion/Comments.tsx b/dotcom-rendering/src/components/Discussion/Comments.tsx index b56aa950ef2..d3ead48b815 100644 --- a/dotcom-rendering/src/components/Discussion/Comments.tsx +++ b/dotcom-rendering/src/components/Discussion/Comments.tsx @@ -6,11 +6,7 @@ import { textSans, } from '@guardian/source-foundations'; import { useEffect, useState } from 'react'; -import { - getDiscussion, - getPicks, - initialiseApi, -} from '../../lib/discussionApi'; +import { getPicks, initialiseApi } from '../../lib/discussionApi'; import type { AdditionalHeadersType, CommentResponse, @@ -49,6 +45,11 @@ type Props = { setPage: (page: number) => void; filters: FilterOptions; setFilters: (filters: FilterOptions) => void; + commentCount: number; + loading: boolean; + totalPages: number; + comments: CommentType[]; + setComments: (comments: CommentType[]) => void; }; const footerStyles = css` @@ -121,16 +122,16 @@ export const Comments = ({ setPage, filters, setFilters, + commentCount, + loading, + totalPages, + comments, + setComments, }: Props) => { - const [loading, setLoading] = useState(true); - const [totalPages, setTotalPages] = useState(0); const [picks, setPicks] = useState([]); const [commentBeingRepliedTo, setCommentBeingRepliedTo] = useState(); - const [comments, setComments] = useState([]); - const [numberOfCommentsToShow, setNumberOfCommentsToShow] = - useState(10); - const [commentCount, setCommentCount] = useState(0); + const [numberOfCommentsToShow, setNumberOfCommentsToShow] = useState(10); const [mutes, setMutes] = useState(readMutes()); const loadingMore = !loading && comments.length !== numberOfCommentsToShow; @@ -149,20 +150,6 @@ export const Comments = ({ } else return; }, [expanded, comments.length]); - useEffect(() => { - setLoading(true); - //todo: come back and handle the error case - void getDiscussion(shortUrl, { ...filters, page }).then((json) => { - setLoading(false); - if (json && json.status !== 'error') { - setComments(json.discussion.comments); - setCommentCount(json.discussion.topLevelCommentCount); - } - //todo: come back and parse this properly (apologies for the horribleness) - setTotalPages(json?.pages as number); - }); - }, [filters, page, shortUrl]); - //todo: parse this properly useEffect(() => { const fetchPicks = async () => { diff --git a/dotcom-rendering/src/components/Discussion/Comments.test.tsx b/dotcom-rendering/src/components/Discussion/Discussion.test.tsx similarity index 53% rename from dotcom-rendering/src/components/Discussion/Comments.test.tsx rename to dotcom-rendering/src/components/Discussion/Discussion.test.tsx index 47cf16e972c..06a4a41f855 100644 --- a/dotcom-rendering/src/components/Discussion/Comments.test.tsx +++ b/dotcom-rendering/src/components/Discussion/Discussion.test.tsx @@ -5,36 +5,21 @@ import { waitForElementToBeRemoved, } from '@testing-library/react'; import { mockRESTCalls } from '../../lib/mockRESTCalls'; -import { Comments } from './Comments'; +import { Discussion } from '../Discussion'; mockRESTCalls(); describe('App', () => { it('should not render the comment form if user is logged out', async () => { render( - {}} - onExpand={() => { - // do nothing - }} - apiKey="" + {}} - filters={{ - threads: 'collapsed', - pageSize: 25, - orderBy: 'newest', - }} - setFilters={() => {}} />, ); diff --git a/dotcom-rendering/src/components/DiscussionMeta.importable.tsx b/dotcom-rendering/src/components/DiscussionMeta.importable.tsx index 667be17768e..d48c547cb52 100644 --- a/dotcom-rendering/src/components/DiscussionMeta.importable.tsx +++ b/dotcom-rendering/src/components/DiscussionMeta.importable.tsx @@ -2,7 +2,8 @@ import { joinUrl } from '@guardian/libs'; import { getOptionsHeadersWithOkta } from '../lib/identity'; import { useApi } from '../lib/useApi'; import { useAuthStatus } from '../lib/useAuthStatus'; -import { useDiscussion } from '../lib/useDiscussion'; +import { useCommentCount } from '../lib/useCommentCount'; +import type { DiscussionResponse } from '../types/discussion'; import { SignedInAs } from './SignedInAs'; type Props = { @@ -17,12 +18,18 @@ export const DiscussionMeta = ({ enableDiscussionSwitch, }: Props) => { const authStatus = useAuthStatus(); + const commentCount = useCommentCount(discussionApiUrl, shortUrlId); - const { commentCount, isClosedForComments } = useDiscussion( + const { data: discussionData } = useApi( joinUrl(discussionApiUrl, 'discussion', shortUrlId), + { + // The default for dedupingInterval is 2 seconds but we want to wait longer here because the cache time + // for a discussion is at least 15 seconds + dedupingInterval: 8000, + }, ); - const { data } = useApi<{ userProfile: UserProfile }>( + const { data: userData } = useApi<{ userProfile: UserProfile }>( authStatus.kind === 'SignedInWithOkta' || authStatus.kind === 'SignedInWithCookies' ? joinUrl( @@ -40,9 +47,9 @@ export const DiscussionMeta = ({ return ( ); }; diff --git a/dotcom-rendering/src/lib/useDiscussion.ts b/dotcom-rendering/src/lib/useDiscussion.ts deleted file mode 100644 index 97ed4cb38c4..00000000000 --- a/dotcom-rendering/src/lib/useDiscussion.ts +++ /dev/null @@ -1,66 +0,0 @@ -import { useApi } from './useApi'; - -type DiscussionResponse = { - status: string; - page: number; - pages: number; - pageSize: number; - orderBy: string; - discussion: { - key: string; - webUrl: string; - apiUrl: string; - commentCount: number; - topLevelCommentCount: number; - isClosedForComments: boolean; - isClosedForRecommendation: boolean; - isThreaded: boolean; - title: string; - comments: CommentType[]; - }; -}; - -type CommentType = { - id: number; - body: string; - date: string; - isoDateTime: string; - status: string; - webUrl: string; - apiUrl: string; - numResponses?: number; - numRecommends: number; - isHighlighted: boolean; - userProfile: UserProfile; - responseTo?: { - displayName: string; - commentApiUrl: string; - isoDateTime: string; - date: string; - commentId: number; - commentWebUrl: string; - }; - responses?: CommentType[]; - metaData?: { - commentCount: number; - staffCommenterCount: number; - editorsPickCount: number; - blockedCount: number; - responseCount: number; - }; -}; - -export const useDiscussion = ( - url: string, -): { commentCount?: number; isClosedForComments?: boolean } => { - const { data } = useApi(url, { - // The default for dedupingInterval is 2 seconds but we want to wait longer here because the cache time - // for a discussion is at least 15 seconds - dedupingInterval: 8000, - }); - - return { - commentCount: data?.discussion.commentCount, - isClosedForComments: data?.discussion.isClosedForComments, - }; -};