Skip to content
This repository has been archived by the owner on Oct 20, 2023. It is now read-only.

Commit

Permalink
Comments virtualized list bugs (#146)
Browse files Browse the repository at this point in the history 
* fix bugs: virtuoso items with margin, circular dependency in useEffect

* refactored names in Comments, consolidated render function

* Update add-delete-comment test to address failure.

* merge Comments useQuery with useEffect to prevent render from being one step behind

* remove store.campaign.commentsList ?

* setting up a merge of Comments.tsx useQuery

* merge Comments data calls into a single useQuery, merged isLoading too

* flex CommentGroup css

* change render conditions for Comments ProgressBar vs 'No Comments' message

---------

Co-authored-by: James Bradford <j.bradford@pnnl.gov>
Co-authored-by: Courtney Carpenter <ccarpenter28@gmail.com>
  • Loading branch information
@arniebradfo @ccarpenter28
3 people authored Jun 13, 2023
1 parent 095d3d3 commit d4a0091
Show file tree
Hide file tree
Showing 7 changed files with 84 additions and 156 deletions.
1 change: 0 additions & 1 deletion applications/client/src/store/campaign/campaign.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ export class CampaignStore extends ExtendedModel(() => ({
hiddenCount: number;
}>(() => ({ groupSelect: false, selectedBeacons: [], hiddenCount: 0 })).withSetter(),
bulkSelectCantHideEntityIds: prop<string[]>(() => []).withSetter(),
commentsList: prop<{ commandGroupIds: string[] }>(() => ({ commandGroupIds: [] })).withSetter(),
},
})) {
@observable sortMemory: {
Expand Down
48 changes: 17 additions & 31 deletions applications/client/src/views/Campaign/Explore/Panels/Comment/CommentGroup.tsx
View file
Original file line number Diff line number Diff line change
@@ -1,17 +1,14 @@
import { css } from '@emotion/react';
import type { AnnotationModel, CommandGroupModel } from '@redeye/client/store';
import type { AnnotationModel } from '@redeye/client/store';
import { useStore } from '@redeye/client/store';
import type { UUID } from '@redeye/client/types/uuid';
import { CommandContainer, CommentBox, NavBreadcrumbs } from '@redeye/client/views';
import { CoreTokens, ThemeClasses, Flex } from '@redeye/ui-styles';
import type { Ref } from 'mobx-keystone';
import { observer } from 'mobx-react-lite';
import type { ComponentProps } from 'react';
import { useEffect, useState } from 'react';

export type CommentGroupProps = ComponentProps<'div'> & {
commandGroup?: CommandGroupModel;
commandGroupId?: string | null;
commandGroupId: string;
toggleNewComment: (id?: string) => void;
newComment: string | undefined;
measure?: any;
Expand All @@ -22,7 +19,6 @@ export type CommentGroupProps = ComponentProps<'div'> & {
};
export const CommentGroup = observer<CommentGroupProps>(
({
commandGroup,
commandGroupId,
toggleNewComment,
newComment,
Expand All @@ -33,50 +29,40 @@ export const CommentGroup = observer<CommentGroupProps>(
...props
}) => {
const store = useStore();
const [commandGroupId1, setCommandGroupId1] = useState<string | null | undefined>(commandGroupId);
const [commandGroup1, setCommandGroup1] = useState<CommandGroupModel | undefined>(commandGroup);
const firstCommandId = commandGroup1?.commandIds?.[0];
const commandGroup = store.graphqlStore.commandGroups.get(commandGroupId);
const firstCommandId = commandGroup?.commandIds?.[0];
const firstCommand = firstCommandId && store.graphqlStore.commands.get(firstCommandId);
useEffect(() => {
if (commandGroupId) {
// For Comments Tab
setCommandGroupId1(commandGroupId as UUID);
setCommandGroup1(store.graphqlStore.commandGroups.get(commandGroupId));
}
if (commandGroup) {
// For Presentation Tab
setCommandGroupId1(commandGroup?.id as UUID);
setCommandGroup1(commandGroup);
}
}, [commandGroupId, commandGroup]);

return (
<div
css={[
css`
display: flex;
flex-direction: column;
width: 100%; // For host meta tab;
padding: 1px 0; // to force impossibility of block layout margin
`,
!hideCommands &&
css`
border-bottom: 1px solid ${CoreTokens.BorderMuted}; //ask ryan how to remove this
border-bottom: 1px solid ${CoreTokens.BorderMuted};
`,
]}
{...props}
>
<Flex column css={{ margin: 16 }}>
{commandGroup1?.annotations?.map((annotation: Ref<AnnotationModel>) => (
<Flex column css={{ padding: 16 }}>
{!commandGroup && <div css={[commentBoxStyle, { height: 300 }]} />}
{commandGroup?.annotations?.map((annotation: Ref<AnnotationModel>) => (
<CommentBox
key={annotation.id}
css={commentBoxStyle}
reply={() => toggleNewComment(commandGroup1?.id)}
reply={() => toggleNewComment(commandGroup?.id)}
annotation={annotation?.maybeCurrent}
commandGroup={commandGroup1}
commandGroup={commandGroup}
isFullList
/>
))}
{newComment === commandGroup1?.id && (
<CommentBox newComment commandGroupId={commandGroupId1} cancel={toggleNewComment} css={commentBoxStyle} />
{newComment === commandGroup?.id && (
<CommentBox newComment commandGroupId={commandGroupId} cancel={toggleNewComment} css={commentBoxStyle} />
)}
</Flex>
<Flex column>
Expand All @@ -92,14 +78,14 @@ export const CommentGroup = observer<CommentGroupProps>(
/>
)}
{!hideCommands &&
commandGroup1?.commandIds?.map((commandId) => (
commandGroup?.commandIds?.map((commandId) => (
<CommandContainer
commandGroupId={commandGroupId1}
commandGroupId={commandGroupId}
commandId={commandId}
css={css`
border-bottom: none !important;
`}
key={`${commandGroup1?.id}${commandId}`}
key={`${commandGroup?.id}${commandId}`}
hideCommentButton
showPath={!showPath} // configurable
expandedCommandIDs={expandedCommandIDs}
Expand Down
162 changes: 47 additions & 115 deletions applications/client/src/views/Campaign/Explore/Panels/Comment/Comments.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
useStore,
commandGroupCommentsQuery,
} from '@redeye/client/store';
import type { UUID } from '@redeye/client/types';
import { CommentGroup, MessageRow } from '@redeye/client/views';
import { useQuery } from '@tanstack/react-query';
import { observable } from 'mobx';
Expand Down Expand Up @@ -38,7 +39,7 @@ export const Comments = observer<CommentsProps>(({ sort }) => {
},
visibleRange: {
startIndex: 0,
endIndex: 0,
endIndex: pageSize,
},
expandedCommandIDs: store.router.params.activeItemId
? observable.array([store.router.params.activeItemId])
Expand All @@ -47,7 +48,8 @@ export const Comments = observer<CommentsProps>(({ sort }) => {
this.expandedCommandIDs.remove(commandId);
},
});
const { data } = useQuery(

const { data: commandGroupIdData } = useQuery(
[
'commandGroups',
store.campaign?.id,
Expand All @@ -59,102 +61,59 @@ export const Comments = observer<CommentsProps>(({ sort }) => {
store.router.params.currentItem,
store.router.params.currentItemId,
],
async () =>
await store.graphqlStore.queryCommandGroupIds({
campaignId: store.campaign?.id!,
beaconId: store.campaign?.interactionState.selectedBeacon?.id as string,
hostId: store.campaign?.interactionState.selectedHost?.id as string,
operatorId: store.campaign?.interactionState.selectedOperator?.id!,
commandType: store.campaign?.interactionState.selectedCommandType?.id!,
hidden: store.settings.showHidden,
sort: {
...sort,
// @ts-ignore
sortBy: sort.sortBy === 'minTime' ? SortOptionComments.time : sort.sortBy,
},
})
);

const { isLoading } = useQuery(
[
'commandGroupsById',
'commandGroups',
store.campaign.id,
data?.commandGroupIds,
Math.trunc(state.visibleRange.endIndex / pageSize),
store.router.params.currentItem,
store.router.params.currentItemId,
],
async () => {
if (data?.commandGroupIds?.length) {
const index = Math.trunc(state.visibleRange.endIndex / pageSize);
const start = index * pageSize;
const ids = data.commandGroupIds.slice(start, start + pageSize);
// query commands as temp solution
const commandGroupsQuery = await store.graphqlStore.queryCommandGroups(
if (store.router.params.currentItem === 'comments_list' && store.router.params.currentItemId) {
// Fetch presentationItemsData again when refreshing browser @comments_list-[currentItemId] and changing sort
const presentationItemsData = await store.graphqlStore.queryPresentationItems(
{
campaignId: store.campaign?.id!,
commandGroupIds: ids,
campaignId: store.campaign.id!,
hidden: store.settings.showHidden,
forOverviewComments: true,
commentsTabSort: sort as SortTypeCommentsTab,
},
commandGroupCommentsQuery // command cache issue?
presentationItemModelPrimitives.commandGroups(presentationCommandGroupModelPrimitives).toString(),
undefined,
true
);

// query commands as temp solution
const commandIds = commandGroupsQuery?.commandGroups.flatMap((cg) => cg.commandIds).filter<string>(isDefined);
return store.graphqlStore.queryCommands(
{
campaignId: store.campaign?.id!,
commandIds,
hidden: store.settings.showHidden,
},
commandQuery
const currentPresentationItem = presentationItemsData?.presentationItems.find(
(item) => item.id === store.router.params.currentItemId
);
// query commands as temp solution
return { commandGroupIds: Array.from(currentPresentationItem?.commandGroupIds || []) } as CommandGroupIdData;
} else {
return (await store.graphqlStore.queryCommandGroupIds({
campaignId: store.campaign?.id!,
beaconId: store.campaign?.interactionState.selectedBeacon?.id as string,
hostId: store.campaign?.interactionState.selectedHost?.id as string,
operatorId: store.campaign?.interactionState.selectedOperator?.id!,
commandType: store.campaign?.interactionState.selectedCommandType?.id!,
hidden: store.settings.showHidden,
sort: {
...sort,
sortBy: sort.sortBy === 'minTime' ? SortOptionComments.time : (sort.sortBy as SortOptionComments),
},
})) as CommandGroupIdData;
}
},
{
enabled: !!data?.commandGroupIds?.length,
}
);

// Fetch presentationData again when refreshing browser @comments_list-[currentItemId] and changing sort
const { data: commentsData } = useQuery(
[
'overview-comments-items',
store.campaign.id,
store.campaign.sortMemory.comments_list,
store.campaign.sortMemory.comments,
],
async () =>
await store.graphqlStore.queryPresentationItems(
{
campaignId: store.campaign.id!,
hidden: store.settings.showHidden,
forOverviewComments: true,
commentsTabSort: sort as SortTypeCommentsTab,
},
presentationItemModelPrimitives.commandGroups(presentationCommandGroupModelPrimitives).toString(),
undefined,
true
)
);

const { isLoading: isCommentsDataLoading } = useQuery(
// const { isLoading: isLoadingMoreComments } = useQuery(
useQuery(
[
'commandGroupsById',
'commandGroups',
store.campaign.id,
store.campaign.commentsList.commandGroupIds,
commandGroupIdData?.commandGroupIds,
Math.trunc(state.visibleRange.endIndex / pageSize),
store.router.params.currentItem,
store.router.params.currentItemId,
],
async () => {
if (store.campaign.commentsList.commandGroupIds.length) {
if (commandGroupIdData?.commandGroupIds?.length) {
const index = Math.trunc(state.visibleRange.endIndex / pageSize);
const start = index * pageSize;
const ids = store.campaign.commentsList.commandGroupIds.slice(start, start + pageSize);
const end = start + pageSize;
const ids = commandGroupIdData.commandGroupIds.slice(start, end);

// query commands as temp solution
const commandGroupsQuery = await store.graphqlStore.queryCommandGroups(
{
Expand All @@ -179,27 +138,16 @@ export const Comments = observer<CommentsProps>(({ sort }) => {
}
},
{
enabled: !!store.campaign.commentsList.commandGroupIds.length,
enabled: !!commandGroupIdData?.commandGroupIds?.length,
}
);

useEffect(() => {
if (store.router.params.currentItem === 'comments_list' && store.router.params.currentItemId) {
const filteredData = commentsData?.presentationItems.find(
(item) => item.id === store.router.params.currentItemId
);
store.campaign?.setCommentsList({
commandGroupIds: Array.from(filteredData?.commandGroupIds || []),
});
}
}, [store.campaign.commentsList.commandGroupIds, store.router.params.currentItem, store.router.params.currentItemId]);

useEffect(() => {
if (store.campaign?.commentStore.scrollToComment) {
if (store.campaign?.commentStore.scrollToComment && commandGroupIdData) {
state.update(
'scrollToIndex',
Object.values(store.graphqlStore.commandGroups?.items).findIndex(
(commandGroup) => commandGroup.id === store.campaign?.commentStore.scrollToComment
Object.values(commandGroupIdData?.commandGroupIds).findIndex(
(commandGroupId) => commandGroupId === store.campaign?.commentStore.scrollToComment
)
);
if (state.scrollToIndex !== -1) {
Expand All @@ -216,38 +164,20 @@ export const Comments = observer<CommentsProps>(({ sort }) => {
}, 250);
}
}
}, [data, commentsData]);
}, [commandGroupIdData]);

return (
<VirtualizedList
rangeChanged={(visibleRange) => state.update('visibleRange', visibleRange)}
listRef={listRef}
cy-test="comments-view"
>
{store.router.params.currentItem === 'comments_list' ? (
store.campaign.commentsList.commandGroupIds.length === 0 ? (
<MessageRow>No comments</MessageRow>
) : isCommentsDataLoading ? (
<ProgressBar intent={Intent.PRIMARY} />
) : (
store.campaign.commentsList.commandGroupIds.map((commandGroupId) => (
<CommentGroup
cy-test="comment-group"
key={commandGroupId}
commandGroupId={commandGroupId}
toggleNewComment={state.toggleNewComment}
newComment={state.newComment}
expandedCommandIDs={state.expandedCommandIDs}
removeExpandedCommandID={state.removeExpandedCommandID}
/>
))
)
) : data?.commandGroupIds.length === 0 ? (
<MessageRow>No comments</MessageRow>
) : isLoading ? (
{!commandGroupIdData?.commandGroupIds ? (
<ProgressBar intent={Intent.PRIMARY} />
) : commandGroupIdData?.commandGroupIds?.length === 0 ? (
<MessageRow>No comments</MessageRow>
) : (
data?.commandGroupIds.map((commandGroupId) => (
commandGroupIdData?.commandGroupIds?.map((commandGroupId) => (
<CommentGroup
cy-test="comment-group"
key={commandGroupId}
Expand All @@ -262,3 +192,5 @@ export const Comments = observer<CommentsProps>(({ sort }) => {
</VirtualizedList>
);
});

type CommandGroupIdData = { commandGroupIds: UUID[] };
17 changes: 12 additions & 5 deletions applications/client/src/views/Campaign/Explore/Panels/Comment/CommentsList.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,6 @@ export const CommentsList = observer<CommentsListProps>(({ sort }) => {

// For presentationItem.id by User or Tag, make sure use a prefix in case it's same to other general types.
const handleClickType = useCallback((presentationItem) => {
store.campaign?.setCommentsList({
commandGroupIds: Array.from(presentationItem.commandGroupIds),
});
store.router.updateRoute({
path: routes[CampaignViews.EXPLORE],
params: {
Expand Down Expand Up @@ -95,8 +92,18 @@ export const CommentsList = observer<CommentsListProps>(({ sort }) => {
{rowTitle(presentationItem)}
</RowTitle>
<FlexSplitter />
<IconLabel title="Commands" value={presentationItem.commandCount} icon={semanticIcons.commands} />
<IconLabel title="comments" value={presentationItem.count} icon={semanticIcons.comment} />
<IconLabel
cy-test="command-count"
title="Commands"
value={presentationItem.commandCount}
icon={semanticIcons.commands}
/>
<IconLabel
cy-test="comment-count"
title="comments"
value={presentationItem.count}
icon={semanticIcons.comment}
/>
</InfoRow>
))}
</VirtualizedList>
Expand Down
Loading

0 comments on commit d4a0091

Please sign in to comment.