From e2aa4bfa0cd1192f94913232326ce7b31d8d39ef Mon Sep 17 00:00:00 2001 From: Amy Benson Date: Thu, 9 Nov 2023 10:17:26 +0000 Subject: [PATCH] EES-4544 comments on data and embed blocks --- .../src/components/comments/Comment.tsx | 237 ++++---- .../comments/CommentAddForm.module.scss | 14 +- .../components/comments/CommentAddForm.tsx | 18 +- .../components/comments/CommentEditForm.tsx | 11 +- .../src/components/comments/CommentsList.tsx | 47 +- .../comments/CommentsWrapper.module.scss | 43 ++ .../components/comments/CommentsWrapper.tsx | 104 ++++ .../comments/__tests__/Comment.test.tsx | 281 +++++++--- .../__tests__/CommentAddForm.test.tsx | 4 +- .../__tests__/CommentEditForm.test.tsx | 2 +- .../comments/__tests__/CommentsList.test.tsx | 16 +- .../__tests__/CommentsWrapper.test.tsx | 332 ++++++++++++ .../editable/EditableContentBlock.module.scss | 10 - .../editable/EditableContentBlock.tsx | 41 +- .../editable/EditableContentForm.module.scss | 24 - .../editable/EditableContentForm.tsx | 38 +- .../__tests__/EditableContentBlock.test.tsx | 8 +- .../__tests__/EditableContentForm.test.tsx | 34 +- .../src/contexts/CommentsContext.tsx | 21 +- .../content/ReleaseContentPage.module.scss | 9 + .../release/content/ReleaseContentPage.tsx | 3 +- .../components/ReleaseEditableBlock.tsx | 130 +++-- .../__tests__/ReleaseEditableBlock.test.tsx | 511 +++++++++++++++++- .../src/styles/_variables.scss | 1 + .../admin/bau/release_content_authoring.robot | 102 +++- .../robot-tests/tests/libs/admin-common.robot | 8 +- 26 files changed, 1635 insertions(+), 414 deletions(-) create mode 100644 src/explore-education-statistics-admin/src/components/comments/CommentsWrapper.module.scss create mode 100644 src/explore-education-statistics-admin/src/components/comments/CommentsWrapper.tsx create mode 100644 src/explore-education-statistics-admin/src/components/comments/__tests__/CommentsWrapper.test.tsx create mode 100644 src/explore-education-statistics-admin/src/pages/release/content/ReleaseContentPage.module.scss diff --git a/src/explore-education-statistics-admin/src/components/comments/Comment.tsx b/src/explore-education-statistics-admin/src/components/comments/Comment.tsx index d748846ae28..41932447dbb 100644 --- a/src/explore-education-statistics-admin/src/components/comments/Comment.tsx +++ b/src/explore-education-statistics-admin/src/components/comments/Comment.tsx @@ -1,7 +1,7 @@ import styles from '@admin/components/comments/Comment.module.scss'; import CommentEditForm from '@admin/components/comments/CommentEditForm'; import { useCommentsContext } from '@admin/contexts/CommentsContext'; -import { Comment as CommentType } from '@admin/services/types/content'; +import { Comment as CommentData } from '@admin/services/types/content'; import FormattedDate from '@common/components/FormattedDate'; import { useAuthContext } from '@admin/contexts/AuthContext'; import classNames from 'classnames'; @@ -9,15 +9,24 @@ import Button from '@common/components/Button'; import ButtonGroup from '@common/components/ButtonGroup'; import ButtonText from '@common/components/ButtonText'; import useToggle from '@common/hooks/useToggle'; -import React, { useEffect, useRef } from 'react'; +import React, { ReactNode, useEffect, useRef } from 'react'; + +export type CommentType = 'inline' | 'block'; interface Props { - comment: CommentType; + comment: CommentData; + type: CommentType; } -const Comment = ({ comment }: Props) => { - const { content, created, createdBy, id, resolved, resolvedBy, updated } = - comment; +export default function Comment({ comment, type }: Props) { + return type === 'inline' ? ( + + ) : ( + + ); +} + +function InlineComment({ comment }: { comment: CommentData }) { const { selectedComment, removeComment, @@ -25,23 +34,21 @@ const Comment = ({ comment }: Props) => { unresolveComment, setSelectedComment, } = useCommentsContext(); - const [isEditingComment, toggleIsEditingComment] = useToggle(false); const [isFocused, toggleIsFocused] = useToggle(false); const ref = useRef(null); - const { user } = useAuthContext(); useEffect(() => { - if (selectedComment.id === id) { + if (selectedComment.id === comment.id) { ref.current?.scrollIntoView({ behavior: 'smooth', block: 'nearest', inline: 'start', }); } - }, [id, selectedComment]); + }, [comment.id, selectedComment]); const handleCommentSelection = () => { - if (!resolved) { + if (!comment.resolved) { setSelectedComment({ id: comment.id }); } }; @@ -51,57 +58,15 @@ const Comment = ({ comment }: Props) => { {/* eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/no-static-element-interactions */}
-

- - {`${createdBy.firstName} ${createdBy.lastName} `} - - commented on -
- {created} - {updated && ( - <> -
- (Updated{' '} - - {updated} - - ) - - )} -

- {isEditingComment ? ( -
- -
- ) : ( -
- {content} -
- )} - - {resolved && ( -

- Resolved by {resolvedBy?.firstName} {resolvedBy?.lastName} on{' '} - {resolved} -

- )} - - {!isEditingComment && ( - <> + { > Highlight comment in content - {resolved ? ( - { - await unresolveComment.current(comment.id, true); - }} - > - Unresolve - - ) : ( - - - {user?.id === createdBy.id && ( - <> - - Edit - + } + onRemove={() => { + removeComment.current(comment.id); + }} + onResolve={async () => { + await resolveComment.current(comment.id, true); + }} + onUnresolve={async () => { + await unresolveComment.current(comment.id, true); + }} + /> +
+ + ); +} - { - removeComment.current(comment.id); - }} - > - Delete - - - )} - - )} - - )} +function BlockComment({ comment }: { comment: CommentData }) { + const { deleteComment, resolveComment, unresolveComment } = + useCommentsContext(); + + return ( +
  • +
    + { + await deleteComment(comment.id); + }} + onResolve={async () => { + await resolveComment.current(comment.id, true); + }} + onUnresolve={async () => { + await unresolveComment.current(comment.id, true); + }} + />
    • ); -}; +} + +interface BaseCommentProps { + comment: CommentData; + highlightButton?: ReactNode; + onRemove: () => void; + onResolve: () => void; + onUnresolve: () => void; +} + +function BaseComment({ + comment, + highlightButton, + onRemove, + onResolve, + onUnresolve, +}: BaseCommentProps) { + const { content, created, createdBy, id, resolved, resolvedBy, updated } = + comment; + + const { user } = useAuthContext(); + + const [isEditingComment, toggleIsEditingComment] = useToggle(false); -export default Comment; + return ( + <> +

    + + {`${createdBy.firstName} ${createdBy.lastName} `} + + commented on +
    + {created} + {updated && ( + <> +
    + (Updated{' '} + {updated}) + + )} +

    + {isEditingComment ? ( +
    + +
    + ) : ( +
    + {content} +
    + )} + {resolved && ( +

    + Resolved by {resolvedBy?.firstName} {resolvedBy?.lastName} on{' '} + {resolved} +

    + )} + {!isEditingComment && ( + <> + {highlightButton} + {resolved ? ( + Unresolve + ) : ( + + + {user?.id === createdBy.id && ( + <> + + Edit + + + Delete + + )} + + )} + + )} + + ); +} diff --git a/src/explore-education-statistics-admin/src/components/comments/CommentAddForm.module.scss b/src/explore-education-statistics-admin/src/components/comments/CommentAddForm.module.scss index 3ce32867b36..1c5ab0a0639 100644 --- a/src/explore-education-statistics-admin/src/components/comments/CommentAddForm.module.scss +++ b/src/explore-education-statistics-admin/src/components/comments/CommentAddForm.module.scss @@ -4,10 +4,14 @@ .container { background: govuk-colour('white'); border: 1px solid govuk-colour('dark-grey'); - margin-left: -$dfe-comments-gutter/2; + margin-bottom: govuk-spacing(2); padding: govuk-spacing(2); - position: absolute; - top: 0; - width: $dfe-comments-width; - z-index: 9; + + @include govuk-media-query($from: desktop) { + margin-bottom: 0; + position: absolute; + top: 0; + z-index: 9; + width: $dfe-comments-width - $dfe-comments-gutter; + } } diff --git a/src/explore-education-statistics-admin/src/components/comments/CommentAddForm.tsx b/src/explore-education-statistics-admin/src/components/comments/CommentAddForm.tsx index 285946ba5d7..a1f8c6758b1 100644 --- a/src/explore-education-statistics-admin/src/components/comments/CommentAddForm.tsx +++ b/src/explore-education-statistics-admin/src/components/comments/CommentAddForm.tsx @@ -22,7 +22,12 @@ interface Props { onSave: () => void; } -const CommentAddForm = ({ baseId, containerRef, onCancel, onSave }: Props) => { +export default function CommentAddForm({ + baseId, + containerRef, + onCancel, + onSave, +}: Props) { const { addComment, setCurrentInteraction } = useCommentsContext(); const ref = useRef(null); @@ -46,7 +51,11 @@ const CommentAddForm = ({ baseId, containerRef, onCancel, onSave }: Props) => { }; return ( -
    +
    { + { setCurrentInteraction?.({ @@ -91,6 +101,4 @@ const CommentAddForm = ({ baseId, containerRef, onCancel, onSave }: Props) => {
    ); -}; - -export default CommentAddForm; +} diff --git a/src/explore-education-statistics-admin/src/components/comments/CommentEditForm.tsx b/src/explore-education-statistics-admin/src/components/comments/CommentEditForm.tsx index 73d6dc7d00a..d92cb9059e1 100644 --- a/src/explore-education-statistics-admin/src/components/comments/CommentEditForm.tsx +++ b/src/explore-education-statistics-admin/src/components/comments/CommentEditForm.tsx @@ -21,7 +21,12 @@ interface Props { onSubmit: () => void; } -const CommentEditForm = ({ comment, id, onCancel, onSubmit }: Props) => { +export default function CommentEditForm({ + comment, + id, + onCancel, + onSubmit, +}: Props) { const { content } = comment; const { updateComment } = useCommentsContext(); @@ -74,6 +79,4 @@ const CommentEditForm = ({ comment, id, onCancel, onSubmit }: Props) => { }} ); -}; - -export default CommentEditForm; +} diff --git a/src/explore-education-statistics-admin/src/components/comments/CommentsList.tsx b/src/explore-education-statistics-admin/src/components/comments/CommentsList.tsx index f4fb72213d6..a230a45696d 100644 --- a/src/explore-education-statistics-admin/src/components/comments/CommentsList.tsx +++ b/src/explore-education-statistics-admin/src/components/comments/CommentsList.tsx @@ -1,17 +1,18 @@ -import Comment from '@admin/components/comments/Comment'; +import Comment, { CommentType } from '@admin/components/comments/Comment'; import styles from '@admin/components/comments/CommentsList.module.scss'; import { useCommentsContext } from '@admin/contexts/CommentsContext'; import Details from '@common/components/Details'; +import { useDesktopMedia } from '@common/hooks/useMedia'; import sortBy from 'lodash/sortBy'; -import React from 'react'; +import React, { ReactNode } from 'react'; interface Props { className?: string; + type: CommentType; } -const CommentsList = ({ className }: Props) => { +export default function CommentsList({ className, type }: Props) { const { comments, markersOrder } = useCommentsContext(); - const resolvedComments = comments.filter(comment => comment.resolved); const unresolvedComments = sortBy( comments.filter(comment => !comment.resolved), @@ -21,24 +22,44 @@ const CommentsList = ({ className }: Props) => { return (
    {unresolvedComments.length > 0 && ( -
      - {unresolvedComments.map(comment => ( - - ))} -
    + +
      + {unresolvedComments.map(comment => ( + + ))} +
    +     )} {resolvedComments.length > 0 && (
    -
      +
        {resolvedComments.map(comment => ( - + ))}
    )}
    ); -}; +} + +function UnresolvedCommentsWrapper({ + children, + total, +}: { + children: ReactNode; + total: number; +}) { + const { isMedia: isDesktopMedia } = useDesktopMedia(); -export default CommentsList; + if (isDesktopMedia) { + return <>{children}; + } + + return ( +
    + {children} +
    + ); +} diff --git a/src/explore-education-statistics-admin/src/components/comments/CommentsWrapper.module.scss b/src/explore-education-statistics-admin/src/components/comments/CommentsWrapper.module.scss new file mode 100644 index 00000000000..1ca9eead1b1 --- /dev/null +++ b/src/explore-education-statistics-admin/src/components/comments/CommentsWrapper.module.scss @@ -0,0 +1,43 @@ +@import '~govuk-frontend/govuk/base'; +@import '@admin/styles/variables'; + +@include govuk-media-query($from: desktop) { + .container { + display: flex; + margin-left: -$dfe-comments-width; + position: relative; + } + + .sidebar { + bottom: 0; + margin-right: $dfe-comments-gutter; + padding: 2px; + position: absolute; + top: 0; + width: $dfe-comments-width - $dfe-comments-gutter; + } + + .list { + max-height: calc(100% - 46px); + overflow-y: auto; + + &.inline { + max-height: 100%; + } + + &.formOpen { + margin-top: $dfe-comments-form-height; + max-height: calc(100% - #{$dfe-comments-form-height}); + } + } + + .block { + margin-left: $dfe-comments-width; + max-width: calc(100% - #{$dfe-comments-width}); + width: 100%; + } + + .button { + width: 100%; + } +} diff --git a/src/explore-education-statistics-admin/src/components/comments/CommentsWrapper.tsx b/src/explore-education-statistics-admin/src/components/comments/CommentsWrapper.tsx new file mode 100644 index 00000000000..bdd9bbd5b05 --- /dev/null +++ b/src/explore-education-statistics-admin/src/components/comments/CommentsWrapper.tsx @@ -0,0 +1,104 @@ +import styles from '@admin/components/comments/CommentsWrapper.module.scss'; +import { CommentType } from '@admin/components/comments/Comment'; +import CommentAddForm from '@admin/components/comments/CommentAddForm'; +import CommentsList from '@admin/components/comments/CommentsList'; +import { useCommentsContext } from '@admin/contexts/CommentsContext'; +import React, { ReactNode, useRef } from 'react'; +import classNames from 'classnames'; +import Button from '@common/components/Button'; +import Tooltip from '@common/components/Tooltip'; + +interface Props { + allowComments?: boolean; + blockLockedMessage?: string; + children: ReactNode; + commentType: CommentType; + id: string; + showCommentAddForm?: boolean; + showCommentsList?: boolean; + testId?: string; + onAdd?: () => void; + onAddCancel?: () => void; + onAddSave?: () => void; + onViewComments?: () => void; +} + +export default function CommentsWrapper({ + allowComments = true, + blockLockedMessage, + children, + id, + commentType, + showCommentAddForm, + showCommentsList = true, + testId, + onAdd, + onAddCancel, + onAddSave, + onViewComments, +}: Props) { + const containerRef = useRef(null); + const { comments } = useCommentsContext(); + const isInline = commentType === 'inline'; + + return ( +
    + {allowComments && ( +
    + {showCommentAddForm && onAddCancel && onAddSave && ( + + )} + {!showCommentAddForm && !isInline && ( + + )} + + {!showCommentsList && comments.length > 0 && ( + + {({ ref }) => ( + + )} + + )} + + {showCommentsList && comments.length > 0 && ( + + )} +
    + )} +
    {children}
    +
    + ); +} diff --git a/src/explore-education-statistics-admin/src/components/comments/__tests__/Comment.test.tsx b/src/explore-education-statistics-admin/src/components/comments/__tests__/Comment.test.tsx index 7fe9e0ab63d..791f0aeac46 100644 --- a/src/explore-education-statistics-admin/src/components/comments/__tests__/Comment.test.tsx +++ b/src/explore-education-statistics-admin/src/components/comments/__tests__/Comment.test.tsx @@ -21,98 +21,203 @@ describe('Comment', () => { permissions: {} as GlobalPermissions, }; - test('renders an unresolved comment correctly', () => { - render(); - - const comment = screen.getByTestId('comment'); - expect(comment).toHaveTextContent('Comment 3'); - expect(comment).toHaveTextContent('User Two'); - expect(comment).toHaveTextContent('30 Nov 2021, 13:55'); - - expect(screen.getByRole('button', { name: 'Resolve' })).toBeInTheDocument(); - expect( - screen.queryByRole('button', { name: 'Unresolve' }), - ).not.toBeInTheDocument(); - }); + describe('inline comments', () => { + test('renders an unresolved comment correctly', () => { + render(); - test('renders the edit and delete buttons if the user created the comment', () => { - render( - - , - , - ); - - expect(screen.getByRole('button', { name: 'Edit' })).toBeInTheDocument(); - expect(screen.getByRole('button', { name: 'Delete' })).toBeInTheDocument(); - }); + const comment = screen.getByTestId('comment'); + expect(comment).toHaveTextContent('Comment 3'); + expect(comment).toHaveTextContent('User Two'); + expect(comment).toHaveTextContent('30 Nov 2021, 13:55'); - test('does not render the edit and delete buttons if the user did not create the comment', () => { - render( - - , - , - ); - - expect( - screen.queryByRole('button', { name: 'Edit' }), - ).not.toBeInTheDocument(); - expect( - screen.queryByRole('button', { name: 'Delete' }), - ).not.toBeInTheDocument(); - }); + expect( + screen.getByRole('button', { name: 'Resolve' }), + ).toBeInTheDocument(); + expect( + screen.queryByRole('button', { name: 'Unresolve' }), + ).not.toBeInTheDocument(); + }); + + test('renders the edit and delete buttons if the user created the comment', () => { + render( + + , + , + ); + + expect(screen.getByRole('button', { name: 'Edit' })).toBeInTheDocument(); + expect( + screen.getByRole('button', { name: 'Delete' }), + ).toBeInTheDocument(); + }); + + test('does not render the edit and delete buttons if the user did not create the comment', () => { + render( + + , + , + ); + + expect( + screen.queryByRole('button', { name: 'Edit' }), + ).not.toBeInTheDocument(); + expect( + screen.queryByRole('button', { name: 'Delete' }), + ).not.toBeInTheDocument(); + }); + + test('renders an updated comment correctly', () => { + const updatedComment = { + ...testComments[2], + updated: '2021-11-30T14:00', + }; + render(); + + const comment = screen.getByTestId('comment'); + expect(comment).toHaveTextContent('Comment 3'); + expect(comment).toHaveTextContent('30 Nov 2021, 13:55'); + expect(comment).toHaveTextContent('(Updated 30 Nov 2021, 14:00)'); + }); + + test('renders a resolved comment correctly', () => { + render( + + + , + ); + + const comment = screen.getByTestId('comment'); + expect(comment).toHaveTextContent('Comment 1'); + expect(comment).toHaveTextContent('User One'); + expect(comment).toHaveTextContent('29 Nov 2021, 13:55'); + expect(comment).toHaveTextContent( + 'Resolved by User Two on 30 Nov 2021, 13:55', + ); - test('renders an updated comment correctly', () => { - const updatedComment = { - ...testComments[2], - updated: '2021-11-30T14:00', - }; - render(); - - const comment = screen.getByTestId('comment'); - expect(comment).toHaveTextContent('Comment 3'); - expect(comment).toHaveTextContent( - '30 Nov 2021, 13:55(Updated 30 Nov 2021, 14:00)', - ); + expect( + screen.getByRole('button', { name: 'Unresolve' }), + ).toBeInTheDocument(); + expect( + screen.queryByRole('button', { name: 'Resolve' }), + ).not.toBeInTheDocument(); + expect( + screen.queryByRole('button', { name: 'Edit' }), + ).not.toBeInTheDocument(); + expect( + screen.queryByRole('button', { name: 'Delete' }), + ).not.toBeInTheDocument(); + }); }); - test('renders a resolved comment correctly', () => { - render( - - - , - ); - - const comment = screen.getByTestId('comment'); - expect(comment).toHaveTextContent('Comment 1'); - expect(comment).toHaveTextContent('User One'); - expect(comment).toHaveTextContent('29 Nov 2021, 13:55'); - expect(comment).toHaveTextContent( - 'Resolved by User Two on 30 Nov 2021, 13:55', - ); - - expect( - screen.getByRole('button', { name: 'Unresolve' }), - ).toBeInTheDocument(); - expect( - screen.queryByRole('button', { name: 'Resolve' }), - ).not.toBeInTheDocument(); - expect( - screen.queryByRole('button', { name: 'Edit' }), - ).not.toBeInTheDocument(); - expect( - screen.queryByRole('button', { name: 'Delete' }), - ).not.toBeInTheDocument(); + describe('block comments', () => { + test('renders an unresolved comment correctly', () => { + render(); + + const comment = screen.getByTestId('comment'); + expect(comment).toHaveTextContent('Comment 3'); + expect(comment).toHaveTextContent('User Two'); + expect(comment).toHaveTextContent('30 Nov 2021, 13:55'); + + expect( + screen.getByRole('button', { name: 'Resolve' }), + ).toBeInTheDocument(); + expect( + screen.queryByRole('button', { name: 'Unresolve' }), + ).not.toBeInTheDocument(); + }); + + test('renders the edit and delete buttons if the user created the comment', () => { + render( + + , + , + ); + + expect(screen.getByRole('button', { name: 'Edit' })).toBeInTheDocument(); + expect( + screen.getByRole('button', { name: 'Delete' }), + ).toBeInTheDocument(); + }); + + test('does not render the edit and delete buttons if the user did not create the comment', () => { + render( + + , + , + ); + + expect( + screen.queryByRole('button', { name: 'Edit' }), + ).not.toBeInTheDocument(); + expect( + screen.queryByRole('button', { name: 'Delete' }), + ).not.toBeInTheDocument(); + }); + + test('renders an updated comment correctly', () => { + const updatedComment = { + ...testComments[2], + updated: '2021-11-30T14:00', + }; + render(); + + const comment = screen.getByTestId('comment'); + expect(comment).toHaveTextContent('Comment 3'); + expect(comment).toHaveTextContent('30 Nov 2021, 13:55'); + expect(comment).toHaveTextContent('(Updated 30 Nov 2021, 14:00)'); + }); + + test('renders a resolved comment correctly', () => { + render( + + + , + ); + + const comment = screen.getByTestId('comment'); + expect(comment).toHaveTextContent('Comment 1'); + expect(comment).toHaveTextContent('User One'); + expect(comment).toHaveTextContent('29 Nov 2021, 13:55'); + expect(comment).toHaveTextContent( + 'Resolved by User Two on 30 Nov 2021, 13:55', + ); + + expect( + screen.getByRole('button', { name: 'Unresolve' }), + ).toBeInTheDocument(); + expect( + screen.queryByRole('button', { name: 'Resolve' }), + ).not.toBeInTheDocument(); + expect( + screen.queryByRole('button', { name: 'Edit' }), + ).not.toBeInTheDocument(); + expect( + screen.queryByRole('button', { name: 'Delete' }), + ).not.toBeInTheDocument(); + }); }); }); diff --git a/src/explore-education-statistics-admin/src/components/comments/__tests__/CommentAddForm.test.tsx b/src/explore-education-statistics-admin/src/components/comments/__tests__/CommentAddForm.test.tsx index 60ef5028a1e..5a4895bcf08 100644 --- a/src/explore-education-statistics-admin/src/components/comments/__tests__/CommentAddForm.test.tsx +++ b/src/explore-education-statistics-admin/src/components/comments/__tests__/CommentAddForm.test.tsx @@ -47,9 +47,9 @@ describe('CommentAddForm', () => { render( Promise.resolve()} onCreate={handleSaveComment} - onUpdate={noop} + onUpdate={() => Promise.resolve()} onPendingDelete={noop} onPendingDeleteUndo={noop} > diff --git a/src/explore-education-statistics-admin/src/components/comments/__tests__/CommentEditForm.test.tsx b/src/explore-education-statistics-admin/src/components/comments/__tests__/CommentEditForm.test.tsx index 47c94f816bd..981863bfe2a 100644 --- a/src/explore-education-statistics-admin/src/components/comments/__tests__/CommentEditForm.test.tsx +++ b/src/explore-education-statistics-admin/src/components/comments/__tests__/CommentEditForm.test.tsx @@ -107,7 +107,7 @@ describe('CommentEditForm', () => { render( Promise.resolve()} onCreate={jest.fn()} onUpdate={handleUpdateComment} onPendingDelete={noop} diff --git a/src/explore-education-statistics-admin/src/components/comments/__tests__/CommentsList.test.tsx b/src/explore-education-statistics-admin/src/components/comments/__tests__/CommentsList.test.tsx index b27a73587ac..a8fa8934ef4 100644 --- a/src/explore-education-statistics-admin/src/components/comments/__tests__/CommentsList.test.tsx +++ b/src/explore-education-statistics-admin/src/components/comments/__tests__/CommentsList.test.tsx @@ -14,18 +14,18 @@ describe('CommentsList', () => { Promise.resolve()} onCreate={jest.fn()} - onUpdate={noop} + onUpdate={() => Promise.resolve()} onPendingDelete={noop} onPendingDeleteUndo={noop} > - + , ); const unresolvedComments = within( - screen.getByTestId('unresolvedComments'), + screen.getByTestId('comments-unresolved'), ).getAllByTestId('comment'); expect(unresolvedComments).toHaveLength(3); @@ -39,13 +39,13 @@ describe('CommentsList', () => { Promise.resolve()} onCreate={jest.fn()} - onUpdate={noop} + onUpdate={() => Promise.resolve()} onPendingDelete={noop} onPendingDeleteUndo={noop} > - + , ); @@ -60,7 +60,7 @@ describe('CommentsList', () => { ).toBeInTheDocument(); const resolvedComments = within( - screen.getByTestId('resolvedComments'), + screen.getByTestId('comments-resolved'), ).getAllByTestId('comment'); expect(resolvedComments).toHaveLength(2); diff --git a/src/explore-education-statistics-admin/src/components/comments/__tests__/CommentsWrapper.test.tsx b/src/explore-education-statistics-admin/src/components/comments/__tests__/CommentsWrapper.test.tsx new file mode 100644 index 00000000000..78680c14661 --- /dev/null +++ b/src/explore-education-statistics-admin/src/components/comments/__tests__/CommentsWrapper.test.tsx @@ -0,0 +1,332 @@ +import CommentsWrapper from '@admin/components/comments/CommentsWrapper'; +import { testComments } from '@admin/components/comments/__data__/testComments'; +import { CommentsContextProvider } from '@admin/contexts/CommentsContext'; +import { render, screen } from '@testing-library/react'; +import noop from 'lodash/noop'; +import React from 'react'; + +describe('CommentsWrapper', () => { + test('renders the child element', () => { + render( + Promise.resolve()} + onCreate={jest.fn()} + onUpdate={() => Promise.resolve()} + onPendingDelete={noop} + onPendingDeleteUndo={noop} + > + +

    child block

    +     +     , + ); + + expect(screen.getByText('child block')).toBeInTheDocument(); + }); + + test('renders the add comment form when `showCommentAddForm` is true', () => { + render( + Promise.resolve()} + onCreate={jest.fn()} + onUpdate={() => Promise.resolve()} + onPendingDelete={noop} + onPendingDeleteUndo={noop} + > + +

    child block

    +     +     , + ); + + expect(screen.getByTestId('comment-add-form')).toBeInTheDocument(); + }); + + test('does not render the add comment form when `showCommentAddForm` is false', () => { + render( + Promise.resolve()} + onCreate={jest.fn()} + onUpdate={() => Promise.resolve()} + onPendingDelete={noop} + onPendingDeleteUndo={noop} + > + +

    child block

    +     +     , + ); + + expect(screen.queryByTestId('comment-add-form')).not.toBeInTheDocument(); + }); + + test('renders the comments list when there are comments and `showCommentsList` is true', () => { + render( + Promise.resolve()} + onCreate={jest.fn()} + onUpdate={() => Promise.resolve()} + onPendingDelete={noop} + onPendingDeleteUndo={noop} + > + +

    child block

    +     +     , + ); + + expect(screen.getByTestId('comments-unresolved')).toBeInTheDocument(); + }); + + test('does not render the comments list when there are no comments and `showCommentsList` is true', () => { + render( + Promise.resolve()} + onCreate={jest.fn()} + onUpdate={() => Promise.resolve()} + onPendingDelete={noop} + onPendingDeleteUndo={noop} + > + +

    child block

    +     +     , + ); + + expect(screen.queryByTestId('comments-unresolved')).not.toBeInTheDocument(); + }); + + test('does not render the comments list when there are comments and `showCommentsList` is false', () => { + render( + Promise.resolve()} + onCreate={jest.fn()} + onUpdate={() => Promise.resolve()} + onPendingDelete={noop} + onPendingDeleteUndo={noop} + > + +

    child block

    +     +     , + ); + + expect(screen.queryByTestId('comments-unresolved')).not.toBeInTheDocument(); + }); + + test('does not render the sidebar if allow comments is false', () => { + render( + Promise.resolve()} + onCreate={jest.fn()} + onUpdate={() => Promise.resolve()} + onPendingDelete={noop} + onPendingDeleteUndo={noop} + > + +

    child block

    +     +     , + ); + + expect(screen.queryByTestId('comments-sidebar')).not.toBeInTheDocument(); + }); + + describe('inline comments', () => { + test('renders the view button when there are comments and `showCommentsList` is false', () => { + render( + Promise.resolve()} + onCreate={jest.fn()} + onUpdate={() => Promise.resolve()} + onPendingDelete={noop} + onPendingDeleteUndo={noop} + > + +

    child block

    +     +     , + ); + + expect( + screen.getByRole('button', { name: 'View comments (3 unresolved)' }), + ).toBeInTheDocument(); + }); + + test('does not render the view button when there are comments and `showCommentsList` is true', () => { + render( + Promise.resolve()} + onCreate={jest.fn()} + onUpdate={() => Promise.resolve()} + onPendingDelete={noop} + onPendingDeleteUndo={noop} + > + +

    child block

    +     +     , + ); + + expect( + screen.queryByRole('button', { name: 'View comments (3 unresolved)' }), + ).not.toBeInTheDocument(); + }); + + test('does not render the view button when there are no comments and `showCommentsList` is false', () => { + render( + Promise.resolve()} + onCreate={jest.fn()} + onUpdate={() => Promise.resolve()} + onPendingDelete={noop} + onPendingDeleteUndo={noop} + > + +

    child block

    +     +     , + ); + + expect( + screen.queryByRole('button', { name: 'View comments (3 unresolved)' }), + ).not.toBeInTheDocument(); + }); + }); + + describe('block comments', () => { + test('renders the add comment button when `showCommentAddForm` is false', () => { + render( + Promise.resolve()} + onCreate={jest.fn()} + onUpdate={() => Promise.resolve()} + onPendingDelete={noop} + onPendingDeleteUndo={noop} + > + +

    child block

    +     +     , + ); + + expect(screen.getByTestId('comment-add-button')).toBeInTheDocument(); + }); + + test('does not render the add comment button when `showCommentAddForm` is true', () => { + render( + Promise.resolve()} + onCreate={jest.fn()} + onUpdate={() => Promise.resolve()} + onPendingDelete={noop} + onPendingDeleteUndo={noop} + > + +

    child block

    +     +     , + ); + + expect( + screen.queryByTestId('comment-add-button'), + ).not.toBeInTheDocument(); + }); + }); +}); diff --git a/src/explore-education-statistics-admin/src/components/editable/EditableContentBlock.module.scss b/src/explore-education-statistics-admin/src/components/editable/EditableContentBlock.module.scss index 2296a6f4e1f..1fca28023c2 100644 --- a/src/explore-education-statistics-admin/src/components/editable/EditableContentBlock.module.scss +++ b/src/explore-education-statistics-admin/src/components/editable/EditableContentBlock.module.scss @@ -45,13 +45,3 @@ $locked-border-width: 4px; position: absolute; right: -#{$locked-border-width}; } - -.commentsButtonContainer { - margin-left: -$dfe-comments-width; - position: absolute; - width: $dfe-comments-width - $dfe-comments-gutter; - - button { - width: 100%; - } -} diff --git a/src/explore-education-statistics-admin/src/components/editable/EditableContentBlock.tsx b/src/explore-education-statistics-admin/src/components/editable/EditableContentBlock.tsx index 714524c7590..5b9d36dc8b3 100644 --- a/src/explore-education-statistics-admin/src/components/editable/EditableContentBlock.tsx +++ b/src/explore-education-statistics-admin/src/components/editable/EditableContentBlock.tsx @@ -1,5 +1,6 @@ import EditableBlockLockedMessage from '@admin/components/editable/EditableBlockLockedMessage'; import { useCommentsContext } from '@admin/contexts/CommentsContext'; +import CommentsWrapper from '@admin/components/comments/CommentsWrapper'; import EditableBlockWrapper from '@admin/components/editable/EditableBlockWrapper'; import EditableContentForm, { AltTextWarningMessage, @@ -12,7 +13,6 @@ import { ImageUploadHandler, } from '@admin/utils/ckeditor/CustomUploadAdapter'; import toHtml from '@admin/utils/markdown/toHtml'; -import Button from '@common/components/Button'; import ContentHtml from '@common/components/ContentHtml'; import Tooltip from '@common/components/Tooltip'; import useToggle from '@common/hooks/useToggle'; @@ -160,36 +160,19 @@ const EditableContentBlock = ({ const isEditable = editable && !isLoading && !lockedBy; - const disabledTooltip = lockedBy + const blockLockedMessage = lockedBy ? `This block is being edited by ${lockedBy?.displayName}` : undefined; return ( - <> - {allowComments && comments.length > 0 && ( -
    - - {({ ref }) => ( - - )} - -
    - )} - + {showAltTextMessage && } {locked && lockedBy && ( @@ -202,7 +185,7 @@ const EditableContentBlock = ({ onEdit={editable ? onEditing : undefined} onDelete={editable ? onDelete : undefined} > - + {({ ref }) => ( // eslint-disable-next-line jsx-a11y/click-events-have-key-events,jsx-a11y/no-static-element-interactions
    - + ); }; diff --git a/src/explore-education-statistics-admin/src/components/editable/EditableContentForm.module.scss b/src/explore-education-statistics-admin/src/components/editable/EditableContentForm.module.scss index 7d3df5b54f4..ad035aa9b5d 100644 --- a/src/explore-education-statistics-admin/src/components/editable/EditableContentForm.module.scss +++ b/src/explore-education-statistics-admin/src/components/editable/EditableContentForm.module.scss @@ -1,31 +1,7 @@ @import '~govuk-frontend/govuk/base'; @import '@admin/styles/variables'; -.container { - display: flex; - margin-left: -$dfe-comments-width; - position: relative; -} - -.commentsList { - bottom: 0; - margin-right: $dfe-comments-gutter; - max-height: 100%; - overflow-y: auto; - padding: 2px; - position: absolute; - top: 0; - width: $dfe-comments-width - $dfe-comments-gutter; - - &.padTop { - margin-top: 200px; - } -} - .form { - margin-left: $dfe-comments-width; - width: 100%; - // stylelint-disable-next-line selector-no-qualifying-type img:not([alt]), img[alt=''] { diff --git a/src/explore-education-statistics-admin/src/components/editable/EditableContentForm.tsx b/src/explore-education-statistics-admin/src/components/editable/EditableContentForm.tsx index 0c35bc858f2..e7c94cebbd1 100644 --- a/src/explore-education-statistics-admin/src/components/editable/EditableContentForm.tsx +++ b/src/explore-education-statistics-admin/src/components/editable/EditableContentForm.tsx @@ -1,4 +1,5 @@ import { useCommentsContext } from '@admin/contexts/CommentsContext'; +import CommentsWrapper from '@admin/components/comments/CommentsWrapper'; import styles from '@admin/components/editable/EditableContentForm.module.scss'; import FormFieldEditor from '@admin/components/form/FormFieldEditor'; import { Element, Node } from '@admin/types/ckeditor'; @@ -6,8 +7,6 @@ import { ImageUploadCancelHandler, ImageUploadHandler, } from '@admin/utils/ckeditor/CustomUploadAdapter'; -import CommentAddForm from '@admin/components/comments/CommentAddForm'; -import CommentsList from '@admin/components/comments/CommentsList'; import Button from '@common/components/Button'; import ButtonGroup from '@common/components/ButtonGroup'; import { Form } from '@common/components/form'; @@ -17,7 +16,6 @@ import useToggle from '@common/hooks/useToggle'; import logger from '@common/services/logger'; import Yup from '@common/validation/yup'; import LoadingSpinner from '@common/components/LoadingSpinner'; -import classNames from 'classnames'; import { Formik, FormikHelpers } from 'formik'; import React, { useCallback, useRef } from 'react'; import { useIdleTimer } from 'react-idle-timer'; @@ -61,7 +59,7 @@ const EditableContentForm = ({ onImageUploadCancel, onSubmit, }: Props) => { - const { comments, clearPendingDeletions } = useCommentsContext(); + const { clearPendingDeletions } = useCommentsContext(); const containerRef = useRef(null); const [showCommentAddForm, toggleCommentAddForm] = useToggle(false); @@ -119,27 +117,15 @@ const EditableContentForm = ({ ); return ( -
    - {allowComments && ( -
    - {showCommentAddForm && ( - - )} - {comments.length > 0 && ( - - )} -
    - )} - +
    initialValues={{ @@ -195,7 +181,7 @@ const EditableContentForm = ({ }}
    -
    + ); }; export default EditableContentForm; diff --git a/src/explore-education-statistics-admin/src/components/editable/__tests__/EditableContentBlock.test.tsx b/src/explore-education-statistics-admin/src/components/editable/__tests__/EditableContentBlock.test.tsx index b0372708c1f..0b1bb0d5d61 100644 --- a/src/explore-education-statistics-admin/src/components/editable/__tests__/EditableContentBlock.test.tsx +++ b/src/explore-education-statistics-admin/src/components/editable/__tests__/EditableContentBlock.test.tsx @@ -303,9 +303,9 @@ Test paragraph render( Promise.resolve()} onCreate={jest.fn()} - onUpdate={noop} + onUpdate={() => Promise.resolve()} onPendingDelete={noop} onPendingDeleteUndo={noop} > @@ -333,9 +333,9 @@ Test paragraph render( Promise.resolve()} onCreate={jest.fn()} - onUpdate={noop} + onUpdate={() => Promise.resolve()} onPendingDelete={noop} onPendingDeleteUndo={noop} > diff --git a/src/explore-education-statistics-admin/src/components/editable/__tests__/EditableContentForm.test.tsx b/src/explore-education-statistics-admin/src/components/editable/__tests__/EditableContentForm.test.tsx index b56c0d1dd9c..f64faa06b4c 100644 --- a/src/explore-education-statistics-admin/src/components/editable/__tests__/EditableContentForm.test.tsx +++ b/src/explore-education-statistics-admin/src/components/editable/__tests__/EditableContentForm.test.tsx @@ -223,9 +223,9 @@ describe('EditableContentForm', () => { render( Promise.resolve()} onCreate={jest.fn()} - onUpdate={noop} + onUpdate={() => Promise.resolve()} onPendingDelete={noop} onPendingDeleteUndo={noop} > @@ -241,7 +241,7 @@ describe('EditableContentForm', () => { ); const unresolvedComments = within( - screen.getByTestId('unresolvedComments'), + screen.getByTestId('comments-unresolved'), ).getAllByTestId('comment'); expect(unresolvedComments).toHaveLength(3); @@ -250,7 +250,7 @@ describe('EditableContentForm', () => { expect(unresolvedComments[2]).toHaveTextContent('Comment 4 content'); const resolvedComments = within( - screen.getByTestId('resolvedComments'), + screen.getByTestId('comments-resolved'), ).getAllByTestId('comment'); expect(resolvedComments).toHaveLength(2); @@ -262,9 +262,9 @@ describe('EditableContentForm', () => { render( Promise.resolve()} onCreate={jest.fn()} - onUpdate={noop} + onUpdate={() => Promise.resolve()} onPendingDelete={noop} onPendingDeleteUndo={noop} > @@ -279,9 +279,9 @@ describe('EditableContentForm', () => { ); expect( - screen.queryByTestId('unresolvedComments'), + screen.queryByTestId('comments-unresolved'), ).not.toBeInTheDocument(); - expect(screen.queryByTestId('resolvedComments')).not.toBeInTheDocument(); + expect(screen.queryByTestId('comments-resolved')).not.toBeInTheDocument(); }); test('calls `onPendingDelete` handler when a comment delete button is clicked', async () => { @@ -295,9 +295,9 @@ describe('EditableContentForm', () => { > Promise.resolve()} onCreate={jest.fn()} - onUpdate={noop} + onUpdate={() => Promise.resolve()} onPendingDelete={handlePendingDelete} onPendingDeleteUndo={noop} > @@ -316,7 +316,7 @@ describe('EditableContentForm', () => { expect(handlePendingDelete).not.toHaveBeenCalled(); const unresolvedComments = within( - screen.getByTestId('unresolvedComments'), + screen.getByTestId('comments-unresolved'), ).getAllByTestId('comment'); userEvent.click( @@ -342,7 +342,7 @@ describe('EditableContentForm', () => { > Promise.resolve()} onCreate={jest.fn()} onUpdate={handleUpdate} onPendingDelete={noop} @@ -363,7 +363,7 @@ describe('EditableContentForm', () => { expect(handleUpdate).not.toHaveBeenCalled(); const unresolvedComments = within( - screen.getByTestId('unresolvedComments'), + screen.getByTestId('comments-unresolved'), ).getAllByTestId('comment'); const comment = within(unresolvedComments[0]); @@ -397,7 +397,7 @@ describe('EditableContentForm', () => { > Promise.resolve()} onCreate={jest.fn()} onUpdate={handleUpdate} onPendingDelete={noop} @@ -418,7 +418,7 @@ describe('EditableContentForm', () => { expect(handleUpdate).not.toHaveBeenCalled(); const unresolvedComments = within( - screen.getByTestId('unresolvedComments'), + screen.getByTestId('comments-unresolved'), ).getAllByTestId('comment'); const comment = within(unresolvedComments[0]); @@ -445,7 +445,7 @@ describe('EditableContentForm', () => { > Promise.resolve()} onCreate={jest.fn()} onUpdate={handleUpdate} onPendingDelete={noop} @@ -466,7 +466,7 @@ describe('EditableContentForm', () => { expect(handleUpdate).not.toHaveBeenCalled(); const resolvedComments = within( - screen.getByTestId('resolvedComments'), + screen.getByTestId('comments-resolved'), ).getAllByTestId('comment'); const comment = within(resolvedComments[0]); diff --git a/src/explore-education-statistics-admin/src/contexts/CommentsContext.tsx b/src/explore-education-statistics-admin/src/contexts/CommentsContext.tsx index 23734fbb7e8..305955dac9d 100644 --- a/src/explore-education-statistics-admin/src/contexts/CommentsContext.tsx +++ b/src/explore-education-statistics-admin/src/contexts/CommentsContext.tsx @@ -11,6 +11,7 @@ import React, { } from 'react'; import noop from 'lodash/noop'; +// TO DO maybe rename the interaction stuff to be more obviously ckeditor related export type CurrentCommentInteraction = | { type: 'adding' | 'removing' | 'resolving' | 'unresolving'; id: string } | undefined; @@ -24,12 +25,13 @@ export interface CommentsContextState { comments: Comment[]; currentInteraction: CurrentCommentInteraction; clearPendingDeletions: () => void; + deleteComment: (commentId: string) => Promise; markersOrder: string[]; pendingDeletions: Comment[]; reAddComment: MutableRefObject<(commentId: string) => void>; removeComment: MutableRefObject<(commentId: string) => void>; resolveComment: MutableRefObject< - (commentId: string, updateMarker?: boolean) => void + (commentId: string, updateMarker?: boolean) => Promise >; selectedComment: SelectedComment; setCurrentInteraction: ( @@ -38,7 +40,7 @@ export interface CommentsContextState { setMarkersOrder: (order: string[]) => void; setSelectedComment: (selectedComment: SelectedComment) => void; unresolveComment: MutableRefObject< - (commentId: string, updateMarker?: boolean) => void + (commentId: string, updateMarker?: boolean) => Promise >; updateComment: (comment: Comment) => void; } @@ -48,16 +50,17 @@ export const CommentsContext = createContext({ comments: [], currentInteraction: undefined, clearPendingDeletions: noop, + deleteComment: () => Promise.resolve(), markersOrder: [], pendingDeletions: [], reAddComment: { current: noop }, removeComment: { current: noop }, - resolveComment: { current: noop }, + resolveComment: { current: () => Promise.resolve() }, selectedComment: { id: '' }, setCurrentInteraction: noop, setMarkersOrder: noop, setSelectedComment: noop, - unresolveComment: { current: noop }, + unresolveComment: { current: () => Promise.resolve() }, updateComment: noop, }); @@ -67,10 +70,10 @@ export interface CommentsContextProviderProps { markersOrder?: string[]; pendingDeletions?: Comment[]; onCreate: (comment: CommentCreate) => Promise; - onDelete: (commentId: string) => void; + onDelete: (commentId: string) => Promise; onPendingDelete: (commentId: string) => void; onPendingDeleteUndo: (commentId: string) => void; - onUpdate: (comment: Comment) => void; + onUpdate: (comment: Comment) => Promise; } export const CommentsContextProvider = ({ @@ -215,11 +218,17 @@ export const CommentsContextProvider = ({ await onUpdate(comment); }; + const deleteComment: CommentsContextState['deleteComment'] = + async commentId => { + await onDelete(commentId); + }; + return { addComment, comments, currentInteraction, clearPendingDeletions, + deleteComment, markersOrder, pendingDeletions, reAddComment, diff --git a/src/explore-education-statistics-admin/src/pages/release/content/ReleaseContentPage.module.scss b/src/explore-education-statistics-admin/src/pages/release/content/ReleaseContentPage.module.scss new file mode 100644 index 00000000000..4b521e548bf --- /dev/null +++ b/src/explore-education-statistics-admin/src/pages/release/content/ReleaseContentPage.module.scss @@ -0,0 +1,9 @@ +@import '~govuk-frontend/govuk/base'; +@import '@admin/styles/variables'; + +.container { + @include govuk-media-query($from: desktop) { + margin-left: $dfe-comments-width; + margin-right: 0; + } +} diff --git a/src/explore-education-statistics-admin/src/pages/release/content/ReleaseContentPage.tsx b/src/explore-education-statistics-admin/src/pages/release/content/ReleaseContentPage.tsx index a747d3aea5e..3bb2c20eec0 100644 --- a/src/explore-education-statistics-admin/src/pages/release/content/ReleaseContentPage.tsx +++ b/src/explore-education-statistics-admin/src/pages/release/content/ReleaseContentPage.tsx @@ -8,6 +8,7 @@ import { ReleaseContentContextState, useReleaseContentState, } from '@admin/pages/release/content/contexts/ReleaseContentContext'; +import styles from '@admin/pages/release/content/ReleaseContentPage.module.scss'; import { ReleaseRouteParams } from '@admin/routes/releaseRoutes'; import permissionService from '@admin/services/permissionService'; import releaseContentService from '@admin/services/releaseContentService'; @@ -67,7 +68,7 @@ const ReleaseContentPageLoaded = () => {
    diff --git a/src/explore-education-statistics-admin/src/pages/release/content/components/ReleaseEditableBlock.tsx b/src/explore-education-statistics-admin/src/pages/release/content/components/ReleaseEditableBlock.tsx index a8affa37e39..72032b1438e 100644 --- a/src/explore-education-statistics-admin/src/pages/release/content/components/ReleaseEditableBlock.tsx +++ b/src/explore-education-statistics-admin/src/pages/release/content/components/ReleaseEditableBlock.tsx @@ -1,6 +1,7 @@ import EditableBlockWrapper from '@admin/components/editable/EditableBlockWrapper'; import EditableContentBlock from '@admin/components/editable/EditableContentBlock'; import EditableEmbedBlock from '@admin/components/editable/EditableEmbedBlock'; +import CommentsWrapper from '@admin/components/comments/CommentsWrapper'; import { CommentsContextProvider } from '@admin/contexts/CommentsContext'; import { useEditingContext } from '@admin/contexts/EditingContext'; import { useReleaseContentHubContext } from '@admin/contexts/ReleaseContentHubContext'; @@ -24,6 +25,7 @@ import { insertReleaseIdPlaceholders } from '@common/modules/release/utils/relea import isBrowser from '@common/utils/isBrowser'; import React, { useCallback, useEffect } from 'react'; import { generatePath } from 'react-router'; +import useToggle from '@common/hooks/useToggle'; interface Props { allowComments?: boolean; @@ -78,6 +80,8 @@ const ReleaseEditableBlock = ({ releaseId, }); + const [showCommentAddForm, toggleCommentAddForm] = useToggle(false); + const updateBlock = useCallback( (nextBlock: EditableBlock) => { dispatch({ @@ -310,54 +314,65 @@ const ReleaseEditableBlock = ({ const blockId = `block-${block.id}`; - switch (block.type) { - case 'DataBlock': - return ( -
    - ( - releaseDataBlockEditRoute.path, - { - publicationId, - releaseId, - dataBlockId: block.id, - }, - )} - onDelete={editable ? handleDelete : undefined} + function renderBlock() { + switch (block.type) { + case 'DataBlock': + return ( + - - - - -
    - ); - case 'EmbedBlockLink': { - return ( - - ); - } - case 'HtmlBlock': - case 'MarkDownBlock': { - return ( - + ( + releaseDataBlockEditRoute.path, + { + publicationId, + releaseId, + dataBlockId: block.id, + }, + )} + onDelete={editable ? handleDelete : undefined} + > + + + + + + ); + case 'EmbedBlockLink': { + return ( + + + + ); + } + case 'HtmlBlock': + case 'MarkDownBlock': { + return ( - - ); + ); + } + default: + return
    Unable to edit content
    ; } - default: - return
    Unable to edit content
    ; } + + return ( + + {renderBlock()} + + ); }; export default ReleaseEditableBlock; diff --git a/src/explore-education-statistics-admin/src/pages/release/content/components/__tests__/ReleaseEditableBlock.test.tsx b/src/explore-education-statistics-admin/src/pages/release/content/components/__tests__/ReleaseEditableBlock.test.tsx index c91abe037c0..396e3991a8a 100644 --- a/src/explore-education-statistics-admin/src/pages/release/content/components/__tests__/ReleaseEditableBlock.test.tsx +++ b/src/explore-education-statistics-admin/src/pages/release/content/components/__tests__/ReleaseEditableBlock.test.tsx @@ -1,5 +1,6 @@ import { AuthContextTestProvider } from '@admin/contexts/AuthContext'; import { ReleaseContentHubContextProvider } from '@admin/contexts/ReleaseContentHubContext'; +import { testComments } from '@admin/components/comments/__data__/testComments'; import { testEditableRelease } from '@admin/pages/release/__data__/testEditableRelease'; import ReleaseEditableBlock from '@admin/pages/release/content/components/ReleaseEditableBlock'; import { ReleaseContentProvider } from '@admin/pages/release/content/contexts/ReleaseContentContext'; @@ -9,6 +10,7 @@ import { GlobalPermissions } from '@admin/services/permissionService'; import _releaseContentService, { EditableRelease, } from '@admin/services/releaseContentService'; +import _releaseContentCommentService from '@admin/services/releaseContentCommentService'; import { EditableBlock } from '@admin/services/types/content'; import { UserDetails } from '@admin/services/types/user'; import mockDate from '@common-test/mockDate'; @@ -23,14 +25,20 @@ import { import userEvent from '@testing-library/user-event'; import noop from 'lodash/noop'; import React, { ReactNode } from 'react'; +import { MemoryRouter } from 'react-router'; jest.mock('@admin/services/hubs/utils/createConnection'); jest.mock('@admin/services/releaseContentService'); +jest.mock('@admin/services/releaseContentCommentService'); jest.mock('@admin/services/dataBlockService'); const releaseContentService = _releaseContentService as jest.Mocked< typeof _releaseContentService >; +const releaseContentCommentService = + _releaseContentCommentService as jest.Mocked< + typeof _releaseContentCommentService + >; describe('ReleaseEditableBlock', () => { const testHtmlBlock: EditableBlock = { @@ -43,7 +51,7 @@ describe('ReleaseEditableBlock', () => { }; const testEmbedBlock: EditableBlock = { - comments: [], + comments: testComments, id: 'embed-block-id', order: 0, title: 'Dashboard title', @@ -51,6 +59,33 @@ describe('ReleaseEditableBlock', () => { url: 'https://department-for-education.shinyapps.io/test-dashboard', }; + const testDataBlock: EditableBlock = { + name: 'Test data block', + heading: 'Data block heading', + source: '', + query: { + subjectId: 'subject-id', + filters: [], + indicators: [], + locationIds: [], + }, + charts: [], + table: { + indicators: [], + tableHeaders: { + columnGroups: [], + columns: [], + rowGroups: [], + rows: [], + }, + }, + type: 'DataBlock', + dataSetId: 'data-set-id', + id: 'data-block-id', + order: 0, + comments: testComments, + dataBlockParentId: 'daata-block-parent-id', + }; const testCurrentUser: UserDetails = { id: 'user-1', displayName: 'Jane Doe', @@ -851,6 +886,478 @@ describe('ReleaseEditableBlock', () => { ); }); + test('renders data block', () => { + render( + , + ); + + expect(screen.getByText('Edit data block')).toBeInTheDocument(); + }); + + describe('data block comments', () => { + test('renders comments correctly`', () => { + render( + , + ); + + expect( + screen.getByRole('button', { name: 'Add comment' }), + ).toBeInTheDocument(); + + const unresolvedComments = within( + screen.getByTestId('comments-unresolved'), + ).getAllByTestId('comment'); + + expect(unresolvedComments).toHaveLength(3); + expect(unresolvedComments[0]).toHaveTextContent('Comment 2 content'); + expect(unresolvedComments[1]).toHaveTextContent('Comment 3 content'); + expect(unresolvedComments[2]).toHaveTextContent('Comment 4 content'); + + const resolvedComments = within( + screen.getByTestId('comments-resolved'), + ).getAllByTestId('comment'); + + expect(resolvedComments).toHaveLength(2); + expect(resolvedComments[0]).toHaveTextContent('Comment 1 content'); + expect(resolvedComments[1]).toHaveTextContent('Comment 5 content'); + }); + + test('calls `deleteContentSectionComment` when a comment delete button is clicked', async () => { + render( + , + ); + + expect( + releaseContentCommentService.deleteContentSectionComment, + ).not.toHaveBeenCalled(); + + const unresolvedComments = within( + screen.getByTestId('comments-unresolved'), + ).getAllByTestId('comment'); + + userEvent.click( + within(unresolvedComments[0]).getByRole('button', { + name: 'Delete', + }), + ); + + await waitFor(() => { + expect( + releaseContentCommentService.deleteContentSectionComment, + ).toHaveBeenCalledTimes(1); + expect( + releaseContentCommentService.deleteContentSectionComment, + ).toHaveBeenCalledWith(testComments[1].id); + }); + }); + + test('calls `updateContentSectionComment` when a comment is edited', async () => { + render( + , + ); + + expect( + releaseContentCommentService.updateContentSectionComment, + ).not.toHaveBeenCalled(); + + const unresolvedComments = within( + screen.getByTestId('comments-unresolved'), + ).getAllByTestId('comment'); + + const comment = within(unresolvedComments[0]); + + userEvent.click(comment.getByRole('button', { name: 'Edit' })); + userEvent.clear(comment.getByRole('textbox')); + await userEvent.type( + comment.getByRole('textbox'), + 'Test updated content', + ); + + userEvent.click(comment.getByRole('button', { name: 'Update' })); + + await waitFor(() => { + expect( + releaseContentCommentService.updateContentSectionComment, + ).toHaveBeenCalledTimes(1); + expect( + releaseContentCommentService.updateContentSectionComment, + ).toHaveBeenCalledWith({ + ...testComments[1], + content: 'Test updated content', + }); + }); + }); + + test('calls `updateContentSectionComment` when a comment is resolved', async () => { + render( + , + ); + + expect( + releaseContentCommentService.updateContentSectionComment, + ).not.toHaveBeenCalled(); + + const unresolvedComments = within( + screen.getByTestId('comments-unresolved'), + ).getAllByTestId('comment'); + + const comment = within(unresolvedComments[0]); + + userEvent.click(comment.getByRole('button', { name: 'Resolve' })); + + await waitFor(() => { + expect( + releaseContentCommentService.updateContentSectionComment, + ).toHaveBeenCalledTimes(1); + expect( + releaseContentCommentService.updateContentSectionComment, + ).toHaveBeenCalledWith({ + ...testComments[1], + setResolved: true, + }); + }); + }); + + test('calls `updateContentSectionComment` when a comment is unresolved', async () => { + render( + , + ); + + expect( + releaseContentCommentService.updateContentSectionComment, + ).not.toHaveBeenCalled(); + + const resolvedComments = within( + screen.getByTestId('comments-resolved'), + ).getAllByTestId('comment'); + + const comment = within(resolvedComments[0]); + + userEvent.click( + comment.getByRole('button', { name: 'Unresolve', hidden: true }), + ); + + await waitFor(() => { + expect( + releaseContentCommentService.updateContentSectionComment, + ).toHaveBeenCalledTimes(1); + expect( + releaseContentCommentService.updateContentSectionComment, + ).toHaveBeenCalledWith({ + ...testComments[0], + setResolved: false, + }); + }); + }); + + test('calls `addContentSectionComment` when a comment is added', async () => { + render( + , + ); + + expect( + releaseContentCommentService.addContentSectionComment, + ).not.toHaveBeenCalled(); + + userEvent.click(screen.getByRole('button', { name: 'Add comment' })); + + await userEvent.type( + screen.getByRole('textbox', { + name: 'Comment', + }), + 'I am a comment', + ); + + userEvent.click( + screen.getByRole('button', { + name: 'Add comment', + }), + ); + + await waitFor(() => { + expect( + releaseContentCommentService.addContentSectionComment, + ).toHaveBeenCalledTimes(1); + expect( + releaseContentCommentService.addContentSectionComment, + ).toHaveBeenCalledWith('release-1', 'section-1', 'data-block-id', { + content: 'I am a comment', + }); + }); + }); + }); + + describe('embed block comments', () => { + test('renders comments correctly`', () => { + render( + , + ); + + expect( + screen.getByRole('button', { name: 'Add comment' }), + ).toBeInTheDocument(); + + const unresolvedComments = within( + screen.getByTestId('comments-unresolved'), + ).getAllByTestId('comment'); + + expect(unresolvedComments).toHaveLength(3); + expect(unresolvedComments[0]).toHaveTextContent('Comment 2 content'); + expect(unresolvedComments[1]).toHaveTextContent('Comment 3 content'); + expect(unresolvedComments[2]).toHaveTextContent('Comment 4 content'); + + const resolvedComments = within( + screen.getByTestId('comments-resolved'), + ).getAllByTestId('comment'); + + expect(resolvedComments).toHaveLength(2); + expect(resolvedComments[0]).toHaveTextContent('Comment 1 content'); + expect(resolvedComments[1]).toHaveTextContent('Comment 5 content'); + }); + + test('calls `deleteContentSectionComment` when a comment delete button is clicked', async () => { + render( + , + ); + + expect( + releaseContentCommentService.deleteContentSectionComment, + ).not.toHaveBeenCalled(); + + const unresolvedComments = within( + screen.getByTestId('comments-unresolved'), + ).getAllByTestId('comment'); + + userEvent.click( + within(unresolvedComments[0]).getByRole('button', { + name: 'Delete', + }), + ); + + await waitFor(() => { + expect( + releaseContentCommentService.deleteContentSectionComment, + ).toHaveBeenCalledTimes(1); + expect( + releaseContentCommentService.deleteContentSectionComment, + ).toHaveBeenCalledWith(testComments[1].id); + }); + }); + + test('calls `updateContentSectionComment` when a comment is edited', async () => { + render( + , + ); + + expect( + releaseContentCommentService.updateContentSectionComment, + ).not.toHaveBeenCalled(); + + const unresolvedComments = within( + screen.getByTestId('comments-unresolved'), + ).getAllByTestId('comment'); + + const comment = within(unresolvedComments[0]); + + userEvent.click(comment.getByRole('button', { name: 'Edit' })); + userEvent.clear(comment.getByRole('textbox')); + await userEvent.type( + comment.getByRole('textbox'), + 'Test updated content', + ); + + userEvent.click(comment.getByRole('button', { name: 'Update' })); + + await waitFor(() => { + expect( + releaseContentCommentService.updateContentSectionComment, + ).toHaveBeenCalledTimes(1); + expect( + releaseContentCommentService.updateContentSectionComment, + ).toHaveBeenCalledWith({ + ...testComments[1], + content: 'Test updated content', + }); + }); + }); + + test('calls `updateContentSectionComment` when a comment is resolved', async () => { + render( + , + ); + + expect( + releaseContentCommentService.updateContentSectionComment, + ).not.toHaveBeenCalled(); + + const unresolvedComments = within( + screen.getByTestId('comments-unresolved'), + ).getAllByTestId('comment'); + + const comment = within(unresolvedComments[0]); + + userEvent.click(comment.getByRole('button', { name: 'Resolve' })); + + await waitFor(() => { + expect( + releaseContentCommentService.updateContentSectionComment, + ).toHaveBeenCalledTimes(1); + expect( + releaseContentCommentService.updateContentSectionComment, + ).toHaveBeenCalledWith({ + ...testComments[1], + setResolved: true, + }); + }); + }); + + test('calls `updateContentSectionComment` when a comment is unresolved', async () => { + render( + , + ); + + expect( + releaseContentCommentService.updateContentSectionComment, + ).not.toHaveBeenCalled(); + + const resolvedComments = within( + screen.getByTestId('comments-resolved'), + ).getAllByTestId('comment'); + + const comment = within(resolvedComments[0]); + + userEvent.click( + comment.getByRole('button', { name: 'Unresolve', hidden: true }), + ); + + await waitFor(() => { + expect( + releaseContentCommentService.updateContentSectionComment, + ).toHaveBeenCalledTimes(1); + expect( + releaseContentCommentService.updateContentSectionComment, + ).toHaveBeenCalledWith({ + ...testComments[0], + setResolved: false, + }); + }); + }); + + test('calls `addContentSectionComment` when a comment is added', async () => { + render( + , + ); + + expect( + releaseContentCommentService.addContentSectionComment, + ).not.toHaveBeenCalled(); + + userEvent.click(screen.getByRole('button', { name: 'Add comment' })); + + await userEvent.type( + screen.getByRole('textbox', { + name: 'Comment', + }), + 'I am a comment', + ); + + userEvent.click( + screen.getByRole('button', { + name: 'Add comment', + }), + ); + + await waitFor(() => { + expect( + releaseContentCommentService.addContentSectionComment, + ).toHaveBeenCalledTimes(1); + expect( + releaseContentCommentService.addContentSectionComment, + ).toHaveBeenCalledWith('release-1', 'section-1', 'embed-block-id', { + content: 'I am a comment', + }); + }); + }); + }); + function render( child: ReactNode, release: EditableRelease = testEditableRelease, @@ -872,7 +1379,7 @@ describe('ReleaseEditableBlock', () => { unattachedDataBlocks: [], }} > - {child} + {child} , diff --git a/src/explore-education-statistics-admin/src/styles/_variables.scss b/src/explore-education-statistics-admin/src/styles/_variables.scss index ff301126691..bd481e36681 100644 --- a/src/explore-education-statistics-admin/src/styles/_variables.scss +++ b/src/explore-education-statistics-admin/src/styles/_variables.scss @@ -2,3 +2,4 @@ $dfe-page-width-wide: 1280px; $dfe-page-width-wide-gutter: 15px; $dfe-comments-width: 300px; $dfe-comments-gutter: 30px; +$dfe-comments-form-height: 194px; diff --git a/tests/robot-tests/tests/admin/bau/release_content_authoring.robot b/tests/robot-tests/tests/admin/bau/release_content_authoring.robot index cfbd71b16d1..8786c5208b5 100644 --- a/tests/robot-tests/tests/admin/bau/release_content_authoring.robot +++ b/tests/robot-tests/tests/admin/bau/release_content_authoring.robot @@ -12,8 +12,10 @@ Test Setup fail test fast if required ${RELEASE_NAME}= Academic year Q1 2020/21 ${PUBLICATION_NAME}= UI tests - release content authoring %{RUN_IDENTIFIER} ${SECTION_1_TITLE}= First content section +${SECTION_2_TITLE}= Second content section ${BLOCK_1_CONTENT}= Block 1 content ${BLOCK_2_CONTENT}= Block 2 content +${DATABLOCK_NAME}= Dates data block name *** Test Cases *** @@ -27,6 +29,18 @@ Create new release user navigates to publication page from dashboard ${PUBLICATION_NAME} user creates release from publication page ${PUBLICATION_NAME} Academic year Q1 2020 +Upload a subject + user navigates to draft release page from dashboard ${PUBLICATION_NAME} + ... ${RELEASE_NAME} + + user uploads subject Dates test subject dates.csv dates.meta.csv + +Create a data block + user creates data block for dates csv + ... Dates test subject + ... ${DATABLOCK_NAME} + ... Data Block 1 title + Give analyst1 publication owner permissions to work on release user gives analyst publication owner access ${PUBLICATION_NAME} @@ -88,8 +102,8 @@ Switch to bau1 to add review comments for first text block user presses keys CTRL+a user adds comment to selected text ${block} Test comment 1 - user checks list has x items testid:unresolvedComments 1 ${block} - user checks list item contains testid:unresolvedComments 1 Test comment 1 ${block} + user checks list has x items testid:comments-unresolved 1 ${block} + user checks list item contains testid:comments-unresolved 1 Test comment 1 ${block} ${editor}= get editor ${block} user sets focus to element ${editor} @@ -99,9 +113,9 @@ Switch to bau1 to add review comments for first text block sleep 0.1 # Prevent intermittent failure where toolbar button is disabled user adds comment to selected text ${block} Test comment 2 - user checks list has x items testid:unresolvedComments 2 ${block} - user checks list item contains testid:unresolvedComments 1 Test comment 1 ${block} - user checks list item contains testid:unresolvedComments 2 Test comment 2 ${block} + user checks list has x items testid:comments-unresolved 2 ${block} + user checks list item contains testid:comments-unresolved 1 Test comment 1 ${block} + user checks list item contains testid:comments-unresolved 2 Test comment 2 ${block} user saves autosaving text block ${block} @@ -115,8 +129,8 @@ Add review comment for second text block as bau1 user presses keys CTRL+a user adds comment to selected text ${block} Test comment 3 - user checks list has x items testid:unresolvedComments 1 ${block} - user checks list item contains testid:unresolvedComments 1 Test comment 3 ${block} + user checks list has x items testid:comments-unresolved 1 ${block} + user checks list item contains testid:comments-unresolved 1 Test comment 3 ${block} Switch to analyst1 to check second text block is locked user switches to analyst1 browser @@ -162,9 +176,9 @@ Switch to analyst1 to resolve comment for first text block user switches to analyst1 browser ${block}= get accordion section block First content section 1 id:releaseMainContent - user checks list has x items testid:unresolvedComments 2 ${block} - user checks list item contains testid:unresolvedComments 1 Test comment 1 ${block} - user checks list item contains testid:unresolvedComments 2 Test comment 2 ${block} + user checks list has x items testid:comments-unresolved 2 ${block} + user checks list item contains testid:comments-unresolved 1 Test comment 1 ${block} + user checks list item contains testid:comments-unresolved 2 Test comment 2 ${block} ${comment}= user gets unresolved comment Test comment 1 ${block} ${author}= get child element ${comment} testid:comment-author @@ -181,13 +195,13 @@ Switch to analyst1 to resolve comment for first text block ... ${block} ... testid:Expand Details Section Resolved comments (1) 10 - user checks list has x items testid:unresolvedComments 1 ${block} - user checks list item contains testid:unresolvedComments 1 Test comment 2 ${block} + user checks list has x items testid:comments-unresolved 1 ${block} + user checks list item contains testid:comments-unresolved 1 Test comment 2 ${block} user opens details dropdown Resolved comments (1) ${block} - user checks list has x items testid:resolvedComments 1 ${block} - user checks list item contains testid:resolvedComments 1 Test comment 1 ${block} + user checks list has x items testid:comments-resolved 1 ${block} + user checks list item contains testid:comments-resolved 1 Test comment 1 ${block} ${comment}= user gets resolved comment Test comment 1 ${block} user waits until element contains ${comment} Resolved by Analyst1 User1 @@ -210,8 +224,8 @@ Switch back to analyst1 to resolve second text block # avoid set page view box getting in the way user scrolls down 600 - user checks list has x items testid:unresolvedComments 1 ${block} - user checks list item contains testid:unresolvedComments 1 Test comment 3 ${block} + user checks list has x items testid:comments-unresolved 1 ${block} + user checks list item contains testid:comments-unresolved 1 Test comment 3 ${block} ${comment}= user gets unresolved comment Test comment 3 ${block} ${author}= get child element ${comment} testid:comment-author @@ -225,11 +239,11 @@ Switch back to analyst1 to resolve second text block ... ${block} ... testid:Expand Details Section Resolved comments (1) 10 - user waits until parent does not contain element ${block} testid:unresolvedComments + user waits until parent does not contain element ${block} testid:comments-unresolved user opens details dropdown Resolved comments (1) ${block} - user checks list has x items testid:resolvedComments 1 ${block} - user checks list item contains testid:resolvedComments 1 Test comment 3 ${block} + user checks list has x items testid:comments-resolved 1 ${block} + user checks list item contains testid:comments-resolved 1 Test comment 3 ${block} ${comment}= user gets resolved comment Test comment 3 ${block} user waits until element contains ${comment} Resolved by Analyst1 User1 @@ -253,10 +267,52 @@ Delete unresolved comment as bau1 ${block}= get accordion section block First content section 1 id:releaseMainContent ${comment}= user gets unresolved comment Test updated comment 2 ${block} user clicks button Delete ${comment} - user waits until parent does not contain element ${block} testid:unresolvedComments + user waits until parent does not contain element ${block} testid:comments-unresolved user saves autosaving text block ${block} +Add second content section as analyst1 + user switches to analyst1 browser + user scrolls to element xpath://button[text()="Add new section"] + user waits until button is enabled Add new section + user clicks button Add new section + user changes accordion section title 2 ${SECTION_2_TITLE} id:releaseMainContent + +Add data block + user adds data block to editable accordion section ${SECTION_2_TITLE} ${DATABLOCK_NAME} + ... id:releaseMainContent + ${block}= set variable xpath://*[@data-testid="Data block - ${DATABLOCK_NAME}"] + user waits until page contains element ${block} %{WAIT_SMALL} + +Add review comment for data block as bau1 + user switches to bau1 browser + user reloads page + user opens accordion section ${SECTION_2_TITLE} id:releaseMainContent + ${block}= set variable xpath://*[@data-testid="data-block-comments-${DATABLOCK_NAME}"] + user adds comment to data block ${block} Test data block comment + + user checks list has x items testid:comments-unresolved 1 ${block} + user checks list item contains testid:comments-unresolved 1 Test data block comment ${block} + +Resolve comment for data block as analyst1 + user switches to analyst1 browser + user reloads page + user closes Set Page View box + user opens accordion section ${SECTION_2_TITLE} id:releaseMainContent + user scrolls down 400 + ${block}= set variable xpath://*[@data-testid="data-block-comments-${DATABLOCK_NAME}"] + ${comment}= user gets unresolved comment Test data block comment ${block} + + user clicks button Resolve ${comment} + + user waits until parent contains element + ... ${block} + ... testid:Expand Details Section Resolved comments (1) 10 + + user opens details dropdown Resolved comments (1) ${block} + user checks list has x items testid:comments-resolved 1 ${block} + user checks list item contains testid:comments-resolved 1 Test data block comment ${block} + *** Keywords *** user adds comment to selected text @@ -271,3 +327,9 @@ user adds comment to selected text ${textarea}= get child element ${comments} label:Comment user enters text into element ${textarea} ${text} user clicks button Add comment ${comments} + +user adds comment to data block + [Arguments] ${block} ${text} + user clicks button Add comment ${block} + user enters text into element label:Comment ${text} + user clicks button Add comment ${block} diff --git a/tests/robot-tests/tests/libs/admin-common.robot b/tests/robot-tests/tests/libs/admin-common.robot index 5b566a6e88f..456ecbd37c8 100644 --- a/tests/robot-tests/tests/libs/admin-common.robot +++ b/tests/robot-tests/tests/libs/admin-common.robot @@ -780,14 +780,14 @@ user waits until modal is not visible user gets resolved comments [Arguments] ${parent}=css:body - user waits until parent contains element ${parent} testid:resolvedComments - ${comments}= get child element ${parent} testid:resolvedComments + user waits until parent contains element ${parent} testid:comments-resolved + ${comments}= get child element ${parent} testid:comments-resolved [Return] ${comments} user gets unresolved comments [Arguments] ${parent}=css:body - user waits until parent contains element ${parent} testid:unresolvedComments - ${comments}= get child element ${parent} testid:unresolvedComments + user waits until parent contains element ${parent} testid:comments-unresolved + ${comments}= get child element ${parent} testid:comments-unresolved [Return] ${comments} user gets unresolved comment