Skip to content

Commit

Permalink
fix(Pagination): display correct thread count
Browse files Browse the repository at this point in the history
remove unused total page count from state,
as it can be inferred from the filters
  • Loading branch information
mxdvl committed Feb 5, 2024
1 parent b749afd commit a3bbc62
Show file tree
Hide file tree
Showing 7 changed files with 91 additions and 76 deletions.
6 changes: 3 additions & 3 deletions dotcom-rendering/fixtures/manual/discussionWithTwoComments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,15 @@ import type { GetDiscussionSuccess } from '../../src/types/discussion';
export const discussionWithTwoComments = {
status: 'ok',
currentPage: 1,
pages: 0,
pages: 1,
pageSize: 100,
orderBy: 'oldest',
discussion: {
key: '/p/39f5x',
webUrl: 'https://www.theguardian.com/science/grrlscientist/2012/aug/07/3',
apiUrl: 'https://discussion.guardianapis.com/discussion-api/discussion//p/39f5x',
commentCount: 0,
topLevelCommentCount: 0,
commentCount: 2,
topLevelCommentCount: 2,
isClosedForComments: false,
isClosedForRecommendation: false,
isThreaded: true,
Expand Down
27 changes: 15 additions & 12 deletions dotcom-rendering/src/components/Discussion.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import {
getCommentContext,
initFiltersFromLocalStorage,
} from '../lib/getCommentContext';
import { useCommentCount } from '../lib/useCommentCount';
import { palette as themePalette } from '../palette';
import type {
CommentForm,
Expand Down Expand Up @@ -96,9 +95,10 @@ type State = {
isClosedForComments: boolean;
isExpanded: boolean;
commentPage: number;
commentCount: number;
topLevelCommentCount: number;
filters: FilterOptions;
hashCommentId: number | undefined;
totalPages: number;
loading: boolean;
topForm: CommentForm;
replyForm: CommentForm;
Expand All @@ -115,9 +115,10 @@ const initialState: State = {
isClosedForComments: false,
isExpanded: false,
commentPage: 1,
commentCount: 0,
topLevelCommentCount: 0,
filters: initFiltersFromLocalStorage(),
hashCommentId: undefined,
totalPages: 0,
loading: true,
topForm: initialCommentFormState,
replyForm: initialCommentFormState,
Expand All @@ -129,7 +130,8 @@ type Action =
type: 'commentsLoaded';
comments: CommentType[];
isClosedForComments: boolean;
totalPages: number;
commentCount: number;
topLevelCommentCount: number;
}
| { type: 'expandComments' }
| { type: 'addComment'; comment: CommentType }
Expand All @@ -151,7 +153,8 @@ const reducer = (state: State, action: Action): State => {
...state,
comments: action.comments,
isClosedForComments: action.isClosedForComments,
totalPages: action.totalPages,
commentCount: action.commentCount,
topLevelCommentCount: action.topLevelCommentCount,
loading: false,
};
case 'addComment':
Expand Down Expand Up @@ -261,17 +264,16 @@ export const Discussion = ({
commentPage,
filters,
hashCommentId,
totalPages,
loading,
topForm,
replyForm,
bottomForm,
topLevelCommentCount,
commentCount,
},
dispatch,
] = useReducer(reducer, initialState);

const commentCount = useCommentCount(discussionApiUrl, shortUrlId);

useEffect(() => {
const newHashCommentId = commentIdFromUrl();
if (newHashCommentId !== undefined) {
Expand All @@ -291,13 +293,14 @@ export const Discussion = ({
return;
}

const { pages, discussion } = result.value;
const { discussion } = result.value;

dispatch({
type: 'commentsLoaded',
comments: discussion.comments,
isClosedForComments: discussion.isClosedForComments,
totalPages: pages,
topLevelCommentCount: discussion.topLevelCommentCount,
commentCount: discussion.commentCount,
});
})
.catch(() => {
Expand Down Expand Up @@ -400,9 +403,9 @@ export const Discussion = ({
});
}}
filters={validFilters}
commentCount={commentCount ?? 0}
commentCount={commentCount}
topLevelCommentCount={topLevelCommentCount}
loading={loading}
totalPages={totalPages}
comments={comments}
setComment={(comment: CommentType) => {
dispatch({ type: 'addComment', comment });
Expand Down
34 changes: 25 additions & 9 deletions dotcom-rendering/src/components/Discussion/Comments.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,10 @@ export const LoggedOutHiddenPicks = () => (
setPage={() => {}}
filters={filters}
commentCount={discussionMock.discussion.commentCount}
topLevelCommentCount={
discussionMock.discussion.topLevelCommentCount
}
loading={false}
totalPages={discussionMock.pages}
comments={discussionMock.discussion.comments}
setComment={() => {}}
handleFilterChange={() => {}}
Expand Down Expand Up @@ -124,8 +126,10 @@ export const InitialPage = () => (
setPage={() => {}}
filters={filters}
commentCount={discussionMock.discussion.commentCount}
topLevelCommentCount={
discussionMock.discussion.topLevelCommentCount
}
loading={false}
totalPages={discussionMock.pages}
comments={discussionMock.discussion.comments}
setComment={() => {}}
handleFilterChange={() => {}}
Expand Down Expand Up @@ -178,8 +182,10 @@ export const LoggedInHiddenNoPicks = () => {
setPage={() => {}}
filters={filters}
commentCount={discussionMock.discussion.commentCount}
topLevelCommentCount={
discussionMock.discussion.topLevelCommentCount
}
loading={false}
totalPages={discussionMock.pages}
comments={discussionMock.discussion.comments}
setComment={() => {}}
handleFilterChange={() => {}}
Expand Down Expand Up @@ -228,8 +234,10 @@ export const LoggedIn = () => {
setPage={() => {}}
filters={filters}
commentCount={discussionMock.discussion.commentCount}
topLevelCommentCount={
discussionMock.discussion.topLevelCommentCount
}
loading={false}
totalPages={discussionMock.pages}
comments={discussionMock.discussion.comments}
setComment={() => {}}
handleFilterChange={() => {}}
Expand Down Expand Up @@ -283,8 +291,10 @@ export const LoggedInShortDiscussion = () => {
setPage={() => {}}
filters={filters}
commentCount={discussionWithTwoComments.discussion.commentCount}
topLevelCommentCount={
discussionWithTwoComments.discussion.topLevelCommentCount
}
loading={false}
totalPages={discussionWithTwoComments.pages}
comments={discussionWithTwoComments.discussion.comments}
setComment={() => {}}
handleFilterChange={() => {}}
Expand Down Expand Up @@ -329,8 +339,10 @@ export const LoggedOutHiddenNoPicks = () => (
setPage={() => {}}
filters={filters}
commentCount={discussionMock.discussion.commentCount}
topLevelCommentCount={
discussionMock.discussion.topLevelCommentCount
}
loading={false}
totalPages={0}
comments={discussionMock.discussion.comments}
setComment={() => {}}
handleFilterChange={() => {}}
Expand Down Expand Up @@ -381,8 +393,10 @@ export const Closed = () => (
setPage={() => {}}
filters={filters}
commentCount={discussionMock.discussion.commentCount}
topLevelCommentCount={
discussionMock.discussion.topLevelCommentCount
}
loading={false}
totalPages={discussionMock.pages}
comments={discussionMock.discussion.comments}
setComment={() => {}}
handleFilterChange={() => {}}
Expand Down Expand Up @@ -431,8 +445,8 @@ export const NoComments = () => (
setPage={() => {}}
filters={filters}
commentCount={0}
topLevelCommentCount={0}
loading={false}
totalPages={0}
comments={[]}
setComment={() => {}}
handleFilterChange={() => {}}
Expand Down Expand Up @@ -483,8 +497,10 @@ export const LegacyDiscussion = () => (
commentCount={
legacyDiscussionWithoutThreading.discussion.commentCount
}
topLevelCommentCount={
legacyDiscussionWithoutThreading.discussion.topLevelCommentCount
}
loading={false}
totalPages={legacyDiscussionWithoutThreading.pages}
comments={legacyDiscussionWithoutThreading.discussion.comments}
setComment={() => {}}
handleFilterChange={() => {}}
Expand Down
17 changes: 8 additions & 9 deletions dotcom-rendering/src/components/Discussion/Comments.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ type Props = {
setPage: (page: number, shouldExpand: boolean) => void;
filters: FilterOptions;
commentCount: number;
topLevelCommentCount: number;
loading: boolean;
totalPages: number;
comments: CommentType[];
setComment: (comment: CommentType) => void;
handleFilterChange: (newFilters: FilterOptions, page?: number) => void;
Expand Down Expand Up @@ -122,8 +122,8 @@ export const Comments = ({
setPage,
filters,
commentCount,
topLevelCommentCount,
loading,
totalPages,
comments,
setComment,
handleFilterChange,
Expand Down Expand Up @@ -236,7 +236,9 @@ export const Comments = ({

initialiseApi({ additionalHeaders, baseUrl, apiKey, idApiUrl });

const showPagination = totalPages > 1;
const totalCount =
filters.threads === 'unthreaded' ? commentCount : topLevelCommentCount;
const showPagination = totalCount > filters.pageSize;

if (!expanded && loading) {
return <span data-testid="loading-comments"></span>;
Expand All @@ -262,10 +264,9 @@ export const Comments = ({
/>
{showPagination && (
<Pagination
totalPages={totalPages}
currentPage={page}
setCurrentPage={onPageChange}
commentCount={commentCount}
totalCount={totalCount}
filters={filters}
/>
)}
Expand Down Expand Up @@ -355,10 +356,9 @@ export const Comments = ({
/>
{showPagination && (
<Pagination
totalPages={totalPages}
currentPage={page}
setCurrentPage={onPageChange}
commentCount={commentCount}
totalCount={totalCount}
filters={filters}
/>
)}
Expand Down Expand Up @@ -407,10 +407,9 @@ export const Comments = ({
{showPagination && (
<footer css={footerStyles}>
<Pagination
totalPages={totalPages}
currentPage={page}
setCurrentPage={onPageChange}
commentCount={commentCount}
totalCount={totalCount}
filters={filters}
/>
</footer>
Expand Down
Loading

0 comments on commit a3bbc62

Please sign in to comment.