From 700649d86cef3dac456fa3372db692b07ee4db85 Mon Sep 17 00:00:00 2001 From: Carson Full Date: Wed, 9 Oct 2024 17:21:15 -0500 Subject: [PATCH 01/10] Fix I think a mistake in key args skipping single arg --- src/api/schema/typePolicies/lists/page-limit-pagination.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/api/schema/typePolicies/lists/page-limit-pagination.ts b/src/api/schema/typePolicies/lists/page-limit-pagination.ts index f9f8cc984..b377006c0 100644 --- a/src/api/schema/typePolicies/lists/page-limit-pagination.ts +++ b/src/api/schema/typePolicies/lists/page-limit-pagination.ts @@ -226,7 +226,7 @@ const reverse = (list: readonly T[]): readonly T[] => list.slice().reverse(); // left out of the key specifier const objectToKeyArgs = (obj: Record): KeyArgs => { const keys = objectToKeyArgsRecurse(cleanEmptyObjects(obj)); - return keys.length > 1 ? keys : false; + return keys.length > 0 ? keys : false; }; const objectToKeyArgsRecurse = (obj: Record): KeySpecifier => Object.entries(obj).reduce( From 4b339e94127831fd580e26295b83796c5e7588b2 Mon Sep 17 00:00:00 2001 From: Carson Full Date: Wed, 9 Oct 2024 17:55:36 -0500 Subject: [PATCH 02/10] Avoid `changeset: null` in Apollo cache This worked out fine so far because we have the `redirectToTypeById` lookups which stripped falsy value from the key args. But it still looks a little weird in cache, and maybe there's a situation where we don't have redirectToTypeById configured. --- src/components/Changeset/ChangesetBanner.tsx | 2 +- src/components/Changeset/useChangesetAwareIdFromUrl.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/components/Changeset/ChangesetBanner.tsx b/src/components/Changeset/ChangesetBanner.tsx index d7c8d52e1..230cace42 100644 --- a/src/components/Changeset/ChangesetBanner.tsx +++ b/src/components/Changeset/ChangesetBanner.tsx @@ -15,7 +15,7 @@ const useStyles = makeStyles()(({ spacing, breakpoints }) => ({ })); interface Props { - changesetId: string | null; + changesetId: string | undefined; onEdit?: () => void; onClose?: () => void; } diff --git a/src/components/Changeset/useChangesetAwareIdFromUrl.ts b/src/components/Changeset/useChangesetAwareIdFromUrl.ts index 6f0b4849a..8ea37957c 100644 --- a/src/components/Changeset/useChangesetAwareIdFromUrl.ts +++ b/src/components/Changeset/useChangesetAwareIdFromUrl.ts @@ -9,10 +9,10 @@ export const useChangesetAwareIdFromUrl = (paramName: string) => { const navigate = useNavigate(); let { [paramName]: objAndChangesetId = '' } = useParams(); // eslint-disable-next-line prefer-const -- false positive - let [id = '', changesetId = null] = objAndChangesetId.split('~'); + let [id = '', changesetId] = objAndChangesetId.split('~'); if (!useBetaFeatures().has('projectChangeRequests')) { objAndChangesetId = id; - changesetId = null; + changesetId = undefined; } return { mergedId: objAndChangesetId, From 14b3396938fdbfe7d0e2d1aed689e44a3d994cfc Mon Sep 17 00:00:00 2001 From: Carson Full Date: Wed, 9 Oct 2024 18:09:00 -0500 Subject: [PATCH 03/10] Change the comment badge to only be thread count Per a conversation with product we think this makes more sense --- src/components/Comments/CommentsThreadList.tsx | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/components/Comments/CommentsThreadList.tsx b/src/components/Comments/CommentsThreadList.tsx index cc69271a3..612686104 100644 --- a/src/components/Comments/CommentsThreadList.tsx +++ b/src/components/Comments/CommentsThreadList.tsx @@ -34,11 +34,7 @@ export const CommentsThreadList = ({ resourceId }: CommentThreadListProps) => { const { data, error, loadMore, networkStatus } = list; useEffect(() => { - const totalComments = (data?.items ?? []) - .flatMap((thread) => thread.comments.total) - .reduce((prev, total) => prev + total, 0); - - setResourceCommentsTotal(totalComments); + setResourceCommentsTotal(data?.total ?? 0); }, [data, setResourceCommentsTotal]); if (error) { From 6966286a52a63ba07c585a9a58006c09faeabd54 Mon Sep 17 00:00:00 2001 From: Carson Full Date: Wed, 9 Oct 2024 18:09:41 -0500 Subject: [PATCH 04/10] Change the comment badge color to primary Per a conversation with product we think this makes more sense --- src/components/Comments/ToggleCommentButton.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/Comments/ToggleCommentButton.tsx b/src/components/Comments/ToggleCommentButton.tsx index 5d571159f..da77fd61d 100644 --- a/src/components/Comments/ToggleCommentButton.tsx +++ b/src/components/Comments/ToggleCommentButton.tsx @@ -15,7 +15,7 @@ export const ToggleCommentsButton = ({ return ( toggleCommentsBar()} {...rest}> - + From 0d05c3fdc9412fbc05ddbef0956f758f20805796 Mon Sep 17 00:00:00 2001 From: Carson Full Date: Wed, 9 Oct 2024 18:14:08 -0500 Subject: [PATCH 05/10] Move fetching thread total into button This simplifies code. But we need to work handling fetching next. --- src/components/Comments/CommentsContext.tsx | 7 ------- src/components/Comments/CommentsThreadList.tsx | 8 -------- src/components/Comments/ThreadCount.graphql | 5 +++++ src/components/Comments/ToggleCommentButton.tsx | 12 ++++++++++-- 4 files changed, 15 insertions(+), 17 deletions(-) create mode 100644 src/components/Comments/ThreadCount.graphql diff --git a/src/components/Comments/CommentsContext.tsx b/src/components/Comments/CommentsContext.tsx index 8c1005f03..44a679353 100644 --- a/src/components/Comments/CommentsContext.tsx +++ b/src/components/Comments/CommentsContext.tsx @@ -19,8 +19,6 @@ const initialCommentsBarContext = { toggleCommentsBar: noop as (state?: boolean) => void, isCommentsBarOpen: false, expandedThreads: {} as unknown as ExpandedThreads, - resourceCommentsTotal: 0, - setResourceCommentsTotal: noop, resourceId: undefined as string | undefined, setResourceId: noop as (resourceId: string | undefined) => void, }; @@ -33,8 +31,6 @@ export const CommentsProvider = ({ children }: ChildrenProp) => { const [resourceId, setResourceId] = useState(undefined); - const [resourceCommentsTotal, setResourceCommentsTotal] = useState(0); - const [currentExpandedThreads, setExpandedThreads] = useSet(); const expandedThreads = useMemo( (): ExpandedThreads => ({ @@ -60,8 +56,6 @@ export const CommentsProvider = ({ children }: ChildrenProp) => { toggleCommentsBar, isCommentsBarOpen, expandedThreads, - resourceCommentsTotal, - setResourceCommentsTotal, resourceId, setResourceId, }), @@ -69,7 +63,6 @@ export const CommentsProvider = ({ children }: ChildrenProp) => { toggleCommentsBar, isCommentsBarOpen, expandedThreads, - resourceCommentsTotal, resourceId, setResourceId, ] diff --git a/src/components/Comments/CommentsThreadList.tsx b/src/components/Comments/CommentsThreadList.tsx index 612686104..b57b67e7f 100644 --- a/src/components/Comments/CommentsThreadList.tsx +++ b/src/components/Comments/CommentsThreadList.tsx @@ -5,13 +5,11 @@ import { Stack, Typography, } from '@mui/material'; -import { useEffect } from 'react'; import { isNetworkRequestInFlight } from '../../api'; import { renderError } from '../Error/error-handling'; import { useListQuery } from '../List'; import { ProgressButton } from '../ProgressButton'; import { CreateComment } from './CommentForm/CreateComment'; -import { useCommentsContext } from './CommentsContext'; import { CommentThreadsListDocument } from './CommentsThreadList.graphql'; import { CommentThread } from './CommentThread'; @@ -20,8 +18,6 @@ interface CommentThreadListProps { } export const CommentsThreadList = ({ resourceId }: CommentThreadListProps) => { - const { setResourceCommentsTotal } = useCommentsContext(); - const list = useListQuery(CommentThreadsListDocument, { listAt: (data) => data.commentThreads, variables: { @@ -33,10 +29,6 @@ export const CommentsThreadList = ({ resourceId }: CommentThreadListProps) => { }); const { data, error, loadMore, networkStatus } = list; - useEffect(() => { - setResourceCommentsTotal(data?.total ?? 0); - }, [data, setResourceCommentsTotal]); - if (error) { const renderedError = renderError(error, { Unauthorized: (ex) => ex.message, diff --git a/src/components/Comments/ThreadCount.graphql b/src/components/Comments/ThreadCount.graphql new file mode 100644 index 000000000..59ddaf62f --- /dev/null +++ b/src/components/Comments/ThreadCount.graphql @@ -0,0 +1,5 @@ +query ThreadCount($id: ID!) { + commentThreads(resource: $id) { + total + } +} diff --git a/src/components/Comments/ToggleCommentButton.tsx b/src/components/Comments/ToggleCommentButton.tsx index da77fd61d..cc4d023fc 100644 --- a/src/components/Comments/ToggleCommentButton.tsx +++ b/src/components/Comments/ToggleCommentButton.tsx @@ -1,21 +1,29 @@ +import { useQuery } from '@apollo/client'; import { Comment } from '@mui/icons-material'; import { Badge } from '@mui/material'; import { Except } from 'type-fest'; import { Feature } from '../Feature'; import { IconButton, IconButtonProps } from '../IconButton'; import { useCommentsContext } from './CommentsContext'; +import { ThreadCountDocument } from './ThreadCount.graphql'; export type ToggleCommentsButtonProps = Except; export const ToggleCommentsButton = ({ ...rest }: ToggleCommentsButtonProps) => { - const { toggleCommentsBar, resourceCommentsTotal } = useCommentsContext(); + const { resourceId, toggleCommentsBar } = useCommentsContext(); + + const { data } = useQuery(ThreadCountDocument, { + variables: { id: resourceId! }, + skip: !resourceId, + }); + const total = data?.commentThreads.total ?? 0; return ( toggleCommentsBar()} {...rest}> - + From b48b63e53776e8bacd6fc87cd2291a61f8062718 Mon Sep 17 00:00:00 2001 From: Carson Full Date: Wed, 9 Oct 2024 18:24:46 -0500 Subject: [PATCH 06/10] Change comment threads to be sorted desc --- src/components/Comments/CommentsThreadList.tsx | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/components/Comments/CommentsThreadList.tsx b/src/components/Comments/CommentsThreadList.tsx index b57b67e7f..edf3a8ffd 100644 --- a/src/components/Comments/CommentsThreadList.tsx +++ b/src/components/Comments/CommentsThreadList.tsx @@ -22,9 +22,6 @@ export const CommentsThreadList = ({ resourceId }: CommentThreadListProps) => { listAt: (data) => data.commentThreads, variables: { resourceId, - input: { - order: 'ASC', - }, }, }); const { data, error, loadMore, networkStatus } = list; From 80aedb94dfa5276d744ccd3a9674e352faba061c Mon Sep 17 00:00:00 2001 From: Carson Full Date: Wed, 9 Oct 2024 18:39:41 -0500 Subject: [PATCH 07/10] GQL commentThreads -> Commentable.commentThreads This is more idiomatic to how we view threads - as under resources. This changes how it's stored in cache, and will allow other queries to fetch/use this cached data. --- src/components/Comments/CommentsThreadList.graphql | 13 ++++++++----- src/components/Comments/CommentsThreadList.tsx | 2 +- src/components/Comments/ThreadCount.graphql | 7 +++++-- src/components/Comments/ToggleCommentButton.tsx | 2 +- 4 files changed, 15 insertions(+), 9 deletions(-) diff --git a/src/components/Comments/CommentsThreadList.graphql b/src/components/Comments/CommentsThreadList.graphql index e479d2e8f..226ac3a6a 100644 --- a/src/components/Comments/CommentsThreadList.graphql +++ b/src/components/Comments/CommentsThreadList.graphql @@ -1,9 +1,12 @@ query CommentThreadsList($resourceId: ID!, $input: CommentThreadListInput) { - commentThreads(resource: $resourceId, input: $input) { - canCreate - items { - ...commentThread + commentable(resource: $resourceId) { + id + commentThreads(input: $input) { + canCreate + items { + ...commentThread + } + ...Pagination } - ...Pagination } } diff --git a/src/components/Comments/CommentsThreadList.tsx b/src/components/Comments/CommentsThreadList.tsx index edf3a8ffd..4e1567f5b 100644 --- a/src/components/Comments/CommentsThreadList.tsx +++ b/src/components/Comments/CommentsThreadList.tsx @@ -19,7 +19,7 @@ interface CommentThreadListProps { export const CommentsThreadList = ({ resourceId }: CommentThreadListProps) => { const list = useListQuery(CommentThreadsListDocument, { - listAt: (data) => data.commentThreads, + listAt: (data) => data.commentable.commentThreads, variables: { resourceId, }, diff --git a/src/components/Comments/ThreadCount.graphql b/src/components/Comments/ThreadCount.graphql index 59ddaf62f..ccecaea25 100644 --- a/src/components/Comments/ThreadCount.graphql +++ b/src/components/Comments/ThreadCount.graphql @@ -1,5 +1,8 @@ query ThreadCount($id: ID!) { - commentThreads(resource: $id) { - total + commentable(resource: $id) { + id + commentThreads { + total + } } } diff --git a/src/components/Comments/ToggleCommentButton.tsx b/src/components/Comments/ToggleCommentButton.tsx index cc4d023fc..9ee992118 100644 --- a/src/components/Comments/ToggleCommentButton.tsx +++ b/src/components/Comments/ToggleCommentButton.tsx @@ -18,7 +18,7 @@ export const ToggleCommentsButton = ({ variables: { id: resourceId! }, skip: !resourceId, }); - const total = data?.commentThreads.total ?? 0; + const total = data?.commentable.commentThreads.total ?? 0; return ( From c413b0b4b1e81bcc473fc1cc0c44050f17dad4ef Mon Sep 17 00:00:00 2001 From: Carson Full Date: Wed, 9 Oct 2024 18:42:00 -0500 Subject: [PATCH 08/10] Only load comments if comments sidebar is open --- src/components/Comments/CommentsThreadList.tsx | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/components/Comments/CommentsThreadList.tsx b/src/components/Comments/CommentsThreadList.tsx index 4e1567f5b..e95baa0fa 100644 --- a/src/components/Comments/CommentsThreadList.tsx +++ b/src/components/Comments/CommentsThreadList.tsx @@ -10,6 +10,7 @@ import { renderError } from '../Error/error-handling'; import { useListQuery } from '../List'; import { ProgressButton } from '../ProgressButton'; import { CreateComment } from './CommentForm/CreateComment'; +import { useCommentsContext } from './CommentsContext'; import { CommentThreadsListDocument } from './CommentsThreadList.graphql'; import { CommentThread } from './CommentThread'; @@ -18,11 +19,14 @@ interface CommentThreadListProps { } export const CommentsThreadList = ({ resourceId }: CommentThreadListProps) => { + const { isCommentsBarOpen } = useCommentsContext(); + const list = useListQuery(CommentThreadsListDocument, { listAt: (data) => data.commentable.commentThreads, variables: { resourceId, }, + skip: !isCommentsBarOpen, }); const { data, error, loadMore, networkStatus } = list; From 0b4310b034eb8d793d9d761c5ae08aa3ac92ca26 Mon Sep 17 00:00:00 2001 From: Carson Full Date: Wed, 9 Oct 2024 18:57:04 -0500 Subject: [PATCH 09/10] Only go to network for thread count if sidebar is closed This way there are not two requests in flight for the same info --- src/components/Comments/ToggleCommentButton.tsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/components/Comments/ToggleCommentButton.tsx b/src/components/Comments/ToggleCommentButton.tsx index 9ee992118..0d159c77d 100644 --- a/src/components/Comments/ToggleCommentButton.tsx +++ b/src/components/Comments/ToggleCommentButton.tsx @@ -12,11 +12,13 @@ export type ToggleCommentsButtonProps = Except; export const ToggleCommentsButton = ({ ...rest }: ToggleCommentsButtonProps) => { - const { resourceId, toggleCommentsBar } = useCommentsContext(); + const { resourceId, isCommentsBarOpen, toggleCommentsBar } = + useCommentsContext(); const { data } = useQuery(ThreadCountDocument, { variables: { id: resourceId! }, skip: !resourceId, + fetchPolicy: isCommentsBarOpen ? 'cache-only' : 'cache-first', }); const total = data?.commentable.commentThreads.total ?? 0; From 347ead7b36606cb0c7812bcb5b3f2bda1eb03b1f Mon Sep 17 00:00:00 2001 From: Carson Full Date: Thu, 10 Oct 2024 10:44:08 -0500 Subject: [PATCH 10/10] Add tooltip for comment toggle --- src/components/Comments/ToggleCommentButton.tsx | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/components/Comments/ToggleCommentButton.tsx b/src/components/Comments/ToggleCommentButton.tsx index 0d159c77d..92be495ac 100644 --- a/src/components/Comments/ToggleCommentButton.tsx +++ b/src/components/Comments/ToggleCommentButton.tsx @@ -1,6 +1,6 @@ import { useQuery } from '@apollo/client'; import { Comment } from '@mui/icons-material'; -import { Badge } from '@mui/material'; +import { Badge, Tooltip } from '@mui/material'; import { Except } from 'type-fest'; import { Feature } from '../Feature'; import { IconButton, IconButtonProps } from '../IconButton'; @@ -24,11 +24,13 @@ export const ToggleCommentsButton = ({ return ( - toggleCommentsBar()} {...rest}> - - - - + + toggleCommentsBar()} {...rest}> + + + + + ); };