diff --git a/entry_types/scrolled/package/spec/editor/controllers/PreviewMessageController-spec.js b/entry_types/scrolled/package/spec/editor/controllers/PreviewMessageController-spec.js index 9977e77087..1e5300948f 100644 --- a/entry_types/scrolled/package/spec/editor/controllers/PreviewMessageController-spec.js +++ b/entry_types/scrolled/package/spec/editor/controllers/PreviewMessageController-spec.js @@ -364,6 +364,92 @@ describe('PreviewMessageController', () => { })).resolves.toMatchObject({contentElementId: 1, command: {some: 'COMMAND'}}); }); + it('selects and highlights a created content element thread in iframe', async () => { + const entry = factories.entry(ScrolledEntry, {}, { + entryTypeSeed: normalizeSeed({ + contentElements: [{id: 1, permaId: 10}] + }) + }); + entry.reviewSession = factories.reviewSession(); + const iframeWindow = createIframeWindow(); + controller = new PreviewMessageController({entry, iframeWindow}); + + await postReadyMessageAndWaitForAcknowledgement(iframeWindow); + + return expect(new Promise(resolve => { + iframeWindow.addEventListener('message', event => { + if (event.data.type === 'SELECT') { + resolve(event.data); + } + }); + entry.reviewSession.trigger('create:thread', { + id: 7, subjectType: 'ContentElement', subjectId: 10 + }); + })).resolves.toMatchObject({ + type: 'SELECT', + payload: {id: 1, type: 'contentElementComments', highlightedThreadId: 7} + }); + }); + + it('selects and highlights a created section thread in iframe', async () => { + const entry = factories.entry(ScrolledEntry, {}, { + entryTypeSeed: normalizeSeed({ + sections: [{id: 5, permaId: 50}] + }) + }); + entry.reviewSession = factories.reviewSession(); + const iframeWindow = createIframeWindow(); + controller = new PreviewMessageController({entry, iframeWindow}); + + await postReadyMessageAndWaitForAcknowledgement(iframeWindow); + + return expect(new Promise(resolve => { + iframeWindow.addEventListener('message', event => { + if (event.data.type === 'SELECT') { + resolve(event.data); + } + }); + entry.reviewSession.trigger('create:thread', { + id: 3, subjectType: 'Section', subjectId: 50 + }); + })).resolves.toMatchObject({ + type: 'SELECT', + payload: {id: 5, type: 'sectionComments', highlightedThreadId: 3} + }); + }); + + it('ignores created threads of deleted subjects', async () => { + const entry = factories.entry(ScrolledEntry, {}, { + entryTypeSeed: normalizeSeed({ + contentElements: [{id: 1, permaId: 10}] + }) + }); + entry.reviewSession = factories.reviewSession(); + const iframeWindow = createIframeWindow(); + controller = new PreviewMessageController({entry, iframeWindow}); + + await postReadyMessageAndWaitForAcknowledgement(iframeWindow); + + // Expecting the SELECT of the known subject to arrive first pins the + // absence of one for the deleted subject. + return expect(new Promise(resolve => { + iframeWindow.addEventListener('message', event => { + if (event.data.type === 'SELECT') { + resolve(event.data); + } + }); + entry.reviewSession.trigger('create:thread', { + id: 7, subjectType: 'ContentElement', subjectId: 999 + }); + entry.reviewSession.trigger('create:thread', { + id: 8, subjectType: 'ContentElement', subjectId: 10 + }); + })).resolves.toMatchObject({ + type: 'SELECT', + payload: {id: 1, type: 'contentElementComments', highlightedThreadId: 8} + }); + }); + it('sends SELECT message to iframe on resetSelection event on model', async () => { const entry = factories.entry(ScrolledEntry, {}, { entryTypeSeed: normalizeSeed({ diff --git a/entry_types/scrolled/package/spec/editor/views/NewThreadView-spec.js b/entry_types/scrolled/package/spec/editor/views/NewThreadView-spec.js index 20242acd48..bd7e00cd21 100644 --- a/entry_types/scrolled/package/spec/editor/views/NewThreadView-spec.js +++ b/entry_types/scrolled/package/spec/editor/views/NewThreadView-spec.js @@ -8,6 +8,7 @@ import {NewThreadView} from 'editor/views/NewThreadView'; import {factories, useFakeTranslations, renderBackboneView} from 'pageflow/testHelpers'; import {useEditorGlobals} from 'support'; import {fireEvent} from '@testing-library/dom'; +import userEvent from '@testing-library/user-event'; describe('NewThreadView', () => { const {createEntry} = useEditorGlobals(); @@ -59,6 +60,42 @@ describe('NewThreadView', () => { expect(getByText('Comments')).toBeInTheDocument(); }); + // Submitting cannot reach the session here: jsdom leaves MessageEvent + // source unset for same-window posts, so the handler drops the create + // message. NewThreadForm-spec covers the form disabling itself. + it('stores the entered text as a draft when the view is closed', async () => { + const user = userEvent.setup(); + const entry = createEntry({}); + entry.reviewSession = factories.reviewSession(); + const view = buildView(entry); + + const {getByPlaceholderText} = renderBackboneView(view); + + await user.type(getByPlaceholderText('Add a comment...'), 'Half a thought'); + view.close(); + + expect(entry.reviewSession.drafts).toEqual({ + 'ContentElement:10': { + subjectType: 'ContentElement', + subjectId: 10, + body: 'Half a thought', + pending: false + } + }); + }); + + it('restores the draft stored for the subject', () => { + const entry = createEntry({}); + entry.reviewSession = factories.reviewSession(); + entry.reviewSession.setDraft({ + subjectType: 'ContentElement', subjectId: 10, body: 'Half a thought' + }); + + const {getByPlaceholderText} = renderBackboneView(buildView(entry)); + + expect(getByPlaceholderText('Add a comment...')).toHaveValue('Half a thought'); + }); + it('navigates to the comments selection tab when the back link is clicked', () => { const entry = createEntry({}); entry.reviewSession = factories.reviewSession(); diff --git a/entry_types/scrolled/package/spec/review/NewThreadForm-spec.js b/entry_types/scrolled/package/spec/review/NewThreadForm-spec.js new file mode 100644 index 0000000000..7466a86042 --- /dev/null +++ b/entry_types/scrolled/package/spec/review/NewThreadForm-spec.js @@ -0,0 +1,188 @@ +import React from 'react'; +import '@testing-library/jest-dom/extend-expect'; +import userEvent from '@testing-library/user-event'; +import {act, waitFor} from '@testing-library/react'; +import {useFakeTranslations} from 'pageflow/testHelpers'; + +import {NewThreadForm} from 'review/NewThreadForm'; +import {postReviewStateDraftsChangeMessage} from 'review/postMessage'; +import {renderWithReviewState} from 'support/renderWithReviewState'; + +const seed = { + sections: [{id: 1, permaId: 1}], + contentElements: [{id: 1, permaId: 10, sectionId: 1, typeName: 'textBlock'}] +}; + +function draft({body, pending = false}) { + return { + 'ContentElement:10': { + subjectType: 'ContentElement', subjectId: 10, body, pending + } + }; +} + +function renderForm(options = {}) { + return renderWithReviewState( + , + {seed, ...options} + ); +} + +function postDraftsChange(drafts) { + act(() => postReviewStateDraftsChangeMessage(window, drafts)); +} + +describe('NewThreadForm', () => { + // A spy left behind by a failing spec would swallow the simulated + // review state messages of the ones after it. + afterEach(() => jest.restoreAllMocks()); + + useFakeTranslations({ + 'pageflow_scrolled.review.add_comment_placeholder': 'Add a comment...', + 'pageflow_scrolled.review.send': 'Send', + 'pageflow_scrolled.review.enter_for_new_line': 'Enter for new line' + }); + + describe('draft restoring', () => { + it('starts out with the stored draft of the subject', () => { + const {getByPlaceholderText} = renderForm({ + drafts: draft({body: 'Half a thought'}) + }); + + expect(getByPlaceholderText('Add a comment...')).toHaveValue('Half a thought'); + }); + + it('starts out empty without a draft', () => { + const {getByPlaceholderText} = renderForm(); + + expect(getByPlaceholderText('Add a comment...')).toHaveValue(''); + }); + }); + + describe('draft storing', () => { + it('stores the text when the form goes away', async () => { + const user = userEvent.setup(); + const setDraft = jest.fn(); + + const {getByPlaceholderText, unmount} = renderForm({setDraft}); + + await user.type(getByPlaceholderText('Add a comment...'), 'Half a thought'); + unmount(); + + expect(setDraft).toHaveBeenCalledWith({ + subjectType: 'ContentElement', + subjectId: 10, + body: 'Half a thought' + }); + }); + + // Blank bodies make the session drop the draft. + it('stores a blank text once the input has been cleared', async () => { + const user = userEvent.setup(); + const setDraft = jest.fn(); + + const {getByPlaceholderText, unmount} = renderForm({ + drafts: draft({body: 'Never mind'}), setDraft + }); + + await user.clear(getByPlaceholderText('Add a comment...')); + unmount(); + + expect(setDraft).toHaveBeenLastCalledWith( + expect.objectContaining({body: ''}) + ); + }); + + // The form does not render again between the draft being dropped and + // the list taking it off screen, so the pending draft is the last one + // it saw. + it('does not store the text while the session is creating the thread', () => { + const setDraft = jest.fn(); + + const {unmount} = renderForm({ + drafts: draft({body: 'Looks good', pending: true}), + setDraft + }); + + unmount(); + + expect(setDraft).not.toHaveBeenCalled(); + }); + + it('does not store the text after submitting', async () => { + const user = userEvent.setup(); + const setDraft = jest.fn(); + + const {getByPlaceholderText, getByRole, unmount} = renderForm({setDraft}); + + await user.type(getByPlaceholderText('Add a comment...'), 'Looks good'); + await user.click(getByRole('button', {name: 'Send'})); + + unmount(); + + expect(setDraft).not.toHaveBeenCalled(); + }); + + it('stores the text again when creating the thread failed', async () => { + const user = userEvent.setup(); + const setDraft = jest.fn(); + + const {getByPlaceholderText, getByRole, unmount} = renderForm({setDraft}); + + await user.type(getByPlaceholderText('Add a comment...'), 'Looks good'); + await user.click(getByRole('button', {name: 'Send'})); + + postDraftsChange(draft({body: 'Looks good', pending: false})); + await waitFor(() => + expect(getByPlaceholderText('Add a comment...')).not.toBeDisabled() + ); + + await user.type(getByPlaceholderText('Add a comment...'), ' after all'); + unmount(); + + expect(setDraft).toHaveBeenCalledWith( + expect.objectContaining({body: 'Looks good after all'}) + ); + }); + }); + + describe('while the thread is being created', () => { + it('disables the input and shows a spinner', () => { + const {getByPlaceholderText, getByRole, container} = renderForm({ + drafts: draft({body: 'Looks good', pending: true}) + }); + + expect(getByPlaceholderText('Add a comment...')).toBeDisabled(); + expect(getByRole('button', {name: 'Send'})).toBeDisabled(); + expect(container.querySelector('[data-file-name="SvgSpinner"]')).not.toBeNull(); + }); + + it('ignores further submits', async () => { + const user = userEvent.setup(); + const postMessage = jest.spyOn(window.top, 'postMessage').mockImplementation(() => {}); + + const {getByRole} = renderForm({ + drafts: draft({body: 'Looks good', pending: true}) + }); + + await user.click(getByRole('button', {name: 'Send'})); + + expect(postMessage).not.toHaveBeenCalledWith( + expect.objectContaining({type: 'CREATE_COMMENT_THREAD'}), + expect.anything() + ); + + postMessage.mockRestore(); + }); + + it('is editable again once the draft is no longer pending', () => { + const {getByPlaceholderText, getByRole} = renderForm({ + drafts: draft({body: 'Looks good', pending: false}) + }); + + expect(getByPlaceholderText('Add a comment...')).not.toBeDisabled(); + expect(getByRole('button', {name: 'Send'})).toBeEnabled(); + expect(getByPlaceholderText('Add a comment...')).toHaveValue('Looks good'); + }); + }); +}); diff --git a/entry_types/scrolled/package/spec/review/ReplyForm-spec.js b/entry_types/scrolled/package/spec/review/ReplyForm-spec.js new file mode 100644 index 0000000000..9542d2faac --- /dev/null +++ b/entry_types/scrolled/package/spec/review/ReplyForm-spec.js @@ -0,0 +1,115 @@ +import React from 'react'; +import '@testing-library/jest-dom/extend-expect'; +import userEvent from '@testing-library/user-event'; +import {act, waitFor} from '@testing-library/react'; +import {useFakeTranslations} from 'pageflow/testHelpers'; + +import {ReplyForm} from 'review/ReplyForm'; +import {postReviewStateDraftsChangeMessage} from 'review/postMessage'; +import {renderWithReviewState} from 'support/renderWithReviewState'; + +const seed = { + sections: [{id: 1, permaId: 1}], + contentElements: [{id: 1, permaId: 10, sectionId: 1, typeName: 'textBlock'}] +}; + +function draft({body, pending = false}) { + return {'Thread:7': {threadId: 7, body, pending}}; +} + +function renderForm(options = {}) { + return renderWithReviewState( + , + {seed, ...options} + ); +} + +function postDraftsChange(drafts) { + act(() => postReviewStateDraftsChangeMessage(window, drafts)); +} + +describe('ReplyForm', () => { + // A spy left behind by a failing spec would swallow the simulated + // review state messages of the ones after it. + afterEach(() => jest.restoreAllMocks()); + + useFakeTranslations({ + 'pageflow_scrolled.review.reply_placeholder': 'Reply...', + 'pageflow_scrolled.review.send': 'Send', + 'pageflow_scrolled.review.enter_for_new_line': 'Enter for new line' + }); + + it('starts out with the stored draft of the thread', () => { + const {getByPlaceholderText, getByRole} = renderForm({ + drafts: draft({body: 'Half a reply'}) + }); + + expect(getByPlaceholderText('Reply...')).toHaveValue('Half a reply'); + expect(getByRole('button', {name: 'Send'})).toBeInTheDocument(); + }); + + it('starts out empty without a draft', () => { + const {getByPlaceholderText, queryByRole} = renderForm(); + + expect(getByPlaceholderText('Reply...')).toHaveValue(''); + expect(queryByRole('button', {name: 'Send'})).toBeNull(); + }); + + it('stores the text as a draft when the form goes away', async () => { + const user = userEvent.setup(); + const setDraft = jest.fn(); + + const {getByPlaceholderText, unmount} = renderForm({setDraft}); + + await user.type(getByPlaceholderText('Reply...'), 'Half a reply'); + unmount(); + + expect(setDraft).toHaveBeenCalledWith({threadId: 7, body: 'Half a reply'}); + }); + + it('disables the input and shows a spinner while the reply is created', async () => { + const user = userEvent.setup(); + + const {getByPlaceholderText, getByRole, container} = renderForm(); + + await user.type(getByPlaceholderText('Reply...'), 'A reply'); + await user.click(getByRole('button', {name: 'Send'})); + + expect(getByPlaceholderText('Reply...')).toBeDisabled(); + expect(getByPlaceholderText('Reply...')).toHaveValue('A reply'); + expect(container.querySelector('[data-file-name="SvgSpinner"]')).not.toBeNull(); + }); + + // The form stays around for the next reply, unlike the new thread form. + it('clears the input once the reply has been created', async () => { + const user = userEvent.setup(); + + const {getByPlaceholderText, getByRole} = renderForm(); + + await user.type(getByPlaceholderText('Reply...'), 'A reply'); + await user.click(getByRole('button', {name: 'Send'})); + + postDraftsChange({}); + + await waitFor(() => + expect(getByPlaceholderText('Reply...')).toHaveValue('') + ); + expect(getByPlaceholderText('Reply...')).not.toBeDisabled(); + }); + + it('keeps the text when creating the reply failed', async () => { + const user = userEvent.setup(); + + const {getByPlaceholderText, getByRole} = renderForm(); + + await user.type(getByPlaceholderText('Reply...'), 'A reply'); + await user.click(getByRole('button', {name: 'Send'})); + + postDraftsChange(draft({body: 'A reply', pending: false})); + + await waitFor(() => + expect(getByPlaceholderText('Reply...')).not.toBeDisabled() + ); + expect(getByPlaceholderText('Reply...')).toHaveValue('A reply'); + }); +}); diff --git a/entry_types/scrolled/package/spec/review/ReviewMessageHandler-spec.js b/entry_types/scrolled/package/spec/review/ReviewMessageHandler-spec.js index f9419719d1..4134c66b49 100644 --- a/entry_types/scrolled/package/spec/review/ReviewMessageHandler-spec.js +++ b/entry_types/scrolled/package/spec/review/ReviewMessageHandler-spec.js @@ -6,7 +6,8 @@ function fakeReviewSession() { const session = { createThread: jest.fn().mockResolvedValue(), createComment: jest.fn().mockResolvedValue(), - updateThread: jest.fn().mockResolvedValue() + updateThread: jest.fn().mockResolvedValue(), + setDraft: jest.fn() }; Object.assign(session, BackboneEvents); @@ -135,6 +136,47 @@ describe('ReviewMessageHandler', () => { }); }); + it('calls session.setDraft on SET_COMMENT_DRAFT message from targetWindow', async () => { + const session = fakeReviewSession(); + + ReviewMessageHandler.create({session, targetWindow: window}); + + window.dispatchEvent(new MessageEvent('message', { + data: { + type: 'SET_COMMENT_DRAFT', + payload: {subjectType: 'CE', subjectId: 10, body: 'Half a thought'} + }, + origin: window.location.origin, + source: window + })); + + await new Promise(resolve => setTimeout(resolve, 0)); + + expect(session.setDraft).toHaveBeenCalledWith({ + subjectType: 'CE', subjectId: 10, body: 'Half a thought' + }); + }); + + it('posts REVIEW_STATE_DRAFTS_CHANGE on change:drafts event', () => { + const session = fakeReviewSession(); + const postMessage = jest.fn(); + jest.spyOn(window, 'postMessage').mockImplementation(postMessage); + + ReviewMessageHandler.create({session, targetWindow: window}); + + const drafts = { + 'CE:10': {subjectType: 'CE', subjectId: 10, body: 'Half a thought', pending: false} + }; + session.trigger('change:drafts', drafts); + + expect(postMessage).toHaveBeenCalledWith( + {type: 'REVIEW_STATE_DRAFTS_CHANGE', payload: drafts}, + window.location.origin + ); + + window.postMessage.mockRestore(); + }); + it('ignores messages not from targetWindow', async () => { const session = fakeReviewSession(); const iframeWindow = {}; diff --git a/entry_types/scrolled/package/spec/review/ReviewStateProvider-spec.js b/entry_types/scrolled/package/spec/review/ReviewStateProvider-spec.js index b0ac76a525..42373d1a3c 100644 --- a/entry_types/scrolled/package/spec/review/ReviewStateProvider-spec.js +++ b/entry_types/scrolled/package/spec/review/ReviewStateProvider-spec.js @@ -5,13 +5,18 @@ import {renderHook} from '@testing-library/react-hooks'; import { ReviewStateProvider, + useCommentDraft, useCommentThread, - useCommentThreads + useCommentThreads, + useCreateComment, + useCreateCommentThread } from 'review/ReviewStateProvider'; import { postReviewStateResetMessage, - postReviewStateThreadChangeMessage + postReviewStateThreadChangeMessage, + postReviewStateDraftsChangeMessage } from 'review/postMessage'; +import {renderHookWithReviewState} from 'support/renderWithReviewState'; function wrapper({children}) { return {children}; @@ -239,4 +244,249 @@ describe('ReviewStateProvider', () => { expect(result.current).toBeUndefined(); }); }); + + describe('useCommentDraft', () => { + const draft = { + subjectType: 'CE', subjectId: 10, body: 'Half a thought', pending: false + }; + + function renderDraftHook({initialDrafts, setDraft} = {}) { + return renderHook( + () => useCommentDraft({subjectType: 'CE', subjectId: 10}), + { + wrapper: ({children}) => ( + + {children} + + ) + } + ); + } + + it('returns undefined without a draft for the subject', () => { + const {result} = renderDraftHook(); + + expect(result.current[0]).toBeUndefined(); + }); + + it('returns drafts passed as initial drafts', () => { + const {result} = renderDraftHook({initialDrafts: {'CE:10': draft}}); + + expect(result.current[0]).toEqual(draft); + }); + + it('updates on drafts change message', async () => { + const {result, waitForNextUpdate} = renderDraftHook(); + + act(() => { + postReviewStateDraftsChangeMessage(window, {'CE:10': draft}); + }); + await waitForNextUpdate(); + + expect(result.current[0]).toEqual(draft); + }); + + it('stores a draft for the subject by posting a message', () => { + const postMessage = jest.spyOn(window.top, 'postMessage').mockImplementation(() => {}); + + const {result} = renderDraftHook(); + + result.current[1]('Half a thought'); + + expect(postMessage).toHaveBeenCalledWith( + { + type: 'SET_COMMENT_DRAFT', + payload: {subjectType: 'CE', subjectId: 10, body: 'Half a thought'} + }, + window.location.origin + ); + + postMessage.mockRestore(); + }); + + it('stores a draft through the function passed to the provider instead', () => { + const setDraft = jest.fn(); + + const {result} = renderDraftHook({setDraft}); + + result.current[1]('Half a thought'); + + expect(setDraft).toHaveBeenCalledWith({ + subjectType: 'CE', subjectId: 10, body: 'Half a thought' + }); + }); + + it('keeps drafts on reset message', async () => { + const {result, waitForNextUpdate} = renderHook( + () => ({ + draft: useCommentDraft({subjectType: 'CE', subjectId: 10})[0], + threads: useCommentThreads() + }), + { + wrapper: ({children}) => ( + + {children} + + ) + } + ); + + postReset({ + currentUser: {id: 42, name: 'Alice'}, + commentThreads: [{id: 1, subjectType: 'CE', subjectId: 10, comments: []}] + }); + await waitForNextUpdate(); + + expect(result.current.threads).toHaveLength(1); + expect(result.current.draft).toEqual(draft); + }); + }); + + describe('useCommentDraft of a thread', () => { + const draft = {threadId: 7, body: 'Half a reply', pending: false}; + + it('returns the draft stored for the thread', () => { + const {result} = renderHook(() => useCommentDraft({threadId: 7}), { + wrapper: ({children}) => ( + + {children} + + ) + }); + + expect(result.current[0]).toEqual(draft); + }); + + it('stores a draft for the thread', () => { + const setDraft = jest.fn(); + + const {result} = renderHook(() => useCommentDraft({threadId: 7}), { + wrapper: ({children}) => ( + {children} + ) + }); + + result.current[1]('Half a reply'); + + expect(setDraft).toHaveBeenCalledWith({threadId: 7, body: 'Half a reply'}); + }); + }); + + describe('useCreateComment', () => { + const seed = { + sections: [{id: 1, permaId: 7}], + contentElements: [{id: 1, permaId: 10, sectionId: 1, typeName: 'textBlock'}] + }; + + function renderCreateHook() { + return renderHookWithReviewState( + () => ({ + createComment: useCreateComment({ + threadId: 7, subjectType: 'ContentElement', subjectId: 10 + }), + draft: useCommentDraft({threadId: 7})[0] + }), + {seed} + ); + } + + it('marks the draft of the thread pending right away', () => { + const {result} = renderCreateHook(); + + act(() => result.current.createComment('A reply')); + + expect(result.current.draft).toEqual({ + threadId: 7, body: 'A reply', pending: true + }); + }); + + it('posts a create comment message', () => { + const postMessage = jest.spyOn(window.top, 'postMessage').mockImplementation(() => {}); + + const {result} = renderCreateHook(); + + act(() => result.current.createComment('A reply')); + + expect(postMessage).toHaveBeenCalledWith( + { + type: 'CREATE_COMMENT', + payload: expect.objectContaining({threadId: 7, body: 'A reply'}) + }, + window.location.origin + ); + + postMessage.mockRestore(); + }); + }); + + // Assembling the payload needs the entry structure the subject lives in. + describe('useCreateCommentThread', () => { + const seed = { + sections: [{id: 1, permaId: 7}], + contentElements: [{id: 1, permaId: 10, sectionId: 1, typeName: 'textBlock'}] + }; + + function renderCreateHook({drafts} = {}) { + return renderHookWithReviewState( + () => ({ + createThread: useCreateCommentThread({ + subjectType: 'ContentElement', subjectId: 10 + }), + draft: useCommentDraft({subjectType: 'ContentElement', subjectId: 10})[0], + other: useCommentDraft({subjectType: 'ContentElement', subjectId: 20})[0] + }), + {seed, drafts} + ); + } + + it('marks the draft of the subject pending right away', () => { + const {result} = renderCreateHook(); + + act(() => result.current.createThread('Looks good')); + + expect(result.current.draft).toEqual({ + subjectType: 'ContentElement', + subjectId: 10, + body: 'Looks good', + pending: true + }); + }); + + it('posts a create thread message with the section of the subject', () => { + const postMessage = jest.spyOn(window.top, 'postMessage').mockImplementation(() => {}); + + const {result} = renderCreateHook(); + + act(() => result.current.createThread('Looks good')); + + expect(postMessage).toHaveBeenCalledWith( + { + type: 'CREATE_COMMENT_THREAD', + payload: expect.objectContaining({ + subjectType: 'ContentElement', + subjectId: 10, + sectionPermaId: 7, + body: 'Looks good' + }) + }, + window.location.origin + ); + + postMessage.mockRestore(); + }); + + it('keeps drafts of other subjects', () => { + const {result} = renderCreateHook({ + drafts: { + 'ContentElement:20': { + subjectType: 'ContentElement', subjectId: 20, body: 'Elsewhere', pending: false + } + } + }); + + act(() => result.current.createThread('Looks good')); + + expect(result.current.other).toMatchObject({body: 'Elsewhere'}); + }); + }); }); diff --git a/entry_types/scrolled/package/spec/review/Thread-spec.js b/entry_types/scrolled/package/spec/review/Thread-spec.js index 56843ee2d9..942580bc70 100644 --- a/entry_types/scrolled/package/spec/review/Thread-spec.js +++ b/entry_types/scrolled/package/spec/review/Thread-spec.js @@ -8,7 +8,11 @@ import {renderWithReviewState} from 'support/renderWithReviewState'; describe('Thread', () => { useFakeTranslations({ - 'pageflow_scrolled.review.refers_to_deleted_element': 'Refers to a deleted element' + 'pageflow_scrolled.review.refers_to_deleted_element': 'Refers to a deleted element', + 'pageflow_scrolled.review.reply_placeholder': 'Reply...', + 'pageflow_scrolled.review.reply_count.one': '1 reply', + 'pageflow_scrolled.review.reply_count.other': '%{count} replies', + 'pageflow_scrolled.review.toggle_replies': 'Toggle replies' }); const thread = { @@ -16,6 +20,29 @@ describe('Thread', () => { comments: [{id: 10, body: 'On the pull quote', creatorName: 'Bob', creatorId: 2}] }; + // The reply form is hidden while a thread with replies is collapsed, + // which would leave a drafted reply out of reach. + it('expands a collapsed thread that has a drafted reply', () => { + const threadWithReply = { + ...thread, + comments: [ + ...thread.comments, + {id: 11, body: 'A first reply', creatorName: 'Alice', creatorId: 1} + ] + }; + + const {getByPlaceholderText} = renderWithReviewState( + , + { + drafts: { + 'Thread:1': {threadId: 1, body: 'Half a reply', pending: false} + } + } + ); + + expect(getByPlaceholderText('Reply...')).toHaveValue('Half a reply'); + }); + it('renders a deleted-element hint when the thread is orphaned', () => { const {getByText} = renderWithReviewState( diff --git a/entry_types/scrolled/package/spec/review/ThreadList-spec.js b/entry_types/scrolled/package/spec/review/ThreadList-spec.js index e5cf50009f..377e310f2d 100644 --- a/entry_types/scrolled/package/spec/review/ThreadList-spec.js +++ b/entry_types/scrolled/package/spec/review/ThreadList-spec.js @@ -1,9 +1,14 @@ import React from 'react'; import '@testing-library/jest-dom/extend-expect'; import userEvent from '@testing-library/user-event'; +import {act, waitFor} from '@testing-library/react'; import {useFakeTranslations} from 'pageflow/testHelpers'; import {ThreadList} from 'review/ThreadList'; +import { + postReviewStateDraftsChangeMessage, + postReviewStateThreadChangeMessage +} from 'review/postMessage'; import {review} from 'review/api'; import {ScrollHighlightedThreadIntoViewProvider} from 'review/scrollHighlightedThreadIntoView'; import {renderWithReviewState} from 'support/renderWithReviewState'; @@ -15,8 +20,16 @@ const seed = { contentElements: [{id: 1, permaId: 10, sectionId: 1, typeName: 'textBlock'}] }; -function renderThreadList(ui, {commentThreads = []} = {}) { - return renderWithReviewState(ui, {seed, commentThreads}); +function renderThreadList(ui, options = {}) { + return renderWithReviewState(ui, {seed, ...options}); +} + +function postDraftsChange(drafts) { + act(() => postReviewStateDraftsChangeMessage(window, drafts)); +} + +function postThreadChange(thread) { + act(() => postReviewStateThreadChangeMessage(window, thread)); } describe('ThreadList', () => { @@ -508,6 +521,27 @@ describe('ThreadList', () => { expect(getByPlaceholderText('Add a comment...')).toBeInTheDocument(); }); + // Whether the list starts out with a form is decided when it mounts, + // so resolving the last thread does not invite a new one. + it('does not open the form when the last thread becomes resolved', async () => { + const thread = { + id: 1, + subjectType: 'ContentElement', + subjectId: 10, + comments: [{id: 10, body: 'Existing', creatorName: 'Bob', creatorId: 2}] + }; + + const {getByText, queryByPlaceholderText} = renderThreadList( + , + {commentThreads: [thread]} + ); + + postThreadChange({...thread, resolvedAt: '2026-07-31'}); + + await waitFor(() => expect(getByText('1 resolved')).toBeInTheDocument()); + expect(queryByPlaceholderText('Add a comment...')).toBeNull(); + }); + it('shows new topic form when showNewForm is true', () => { const {getByPlaceholderText} = renderThreadList( , @@ -587,6 +621,116 @@ describe('ThreadList', () => { expect(getByPlaceholderText('Add a comment...')).toBeInTheDocument(); }); + + it('shows the form again when a draft exists for the subject', () => { + const {getByPlaceholderText} = renderThreadList( + , + { + commentThreads: [ + {id: 1, subjectType: 'ContentElement', subjectId: 10, comments: [ + {id: 10, body: 'Existing', creatorName: 'Bob', creatorId: 2} + ]} + ], + drafts: { + 'ContentElement:10': { + subjectType: 'ContentElement', subjectId: 10, body: 'Half a thought' + } + } + } + ); + + expect(getByPlaceholderText('Add a comment...')).toHaveValue('Half a thought'); + }); + + it('keeps the form open while the thread is being created', () => { + const {getByPlaceholderText} = renderThreadList( + , + { + drafts: { + 'ContentElement:10': { + subjectType: 'ContentElement', + subjectId: 10, + body: 'Looks good', + pending: true + } + } + } + ); + + expect(getByPlaceholderText('Add a comment...')).toBeDisabled(); + }); + + // The editor sidebar lists opt out of the form entirely, since new + // threads are composed in a view of their own there. + it('does not show the form for a draft when showNewForm is false', () => { + const {queryByPlaceholderText} = renderThreadList( + , + { + drafts: { + 'ContentElement:10': { + subjectType: 'ContentElement', subjectId: 10, body: 'Half a thought' + } + } + } + ); + + expect(queryByPlaceholderText('Add a comment...')).toBeNull(); + }); + + it('keeps the form open with the text on submit', async () => { + const user = userEvent.setup(); + + const {getByPlaceholderText, getByRole} = renderThreadList( + + ); + + await user.type(getByPlaceholderText('Add a comment...'), 'New thread'); + await user.click(getByRole('button', {name: 'Send'})); + + expect(getByPlaceholderText('Add a comment...')).toBeDisabled(); + expect(getByPlaceholderText('Add a comment...')).toHaveValue('New thread'); + }); + + // Storing the text on the way out would bring back the draft the + // session has just dropped and reopen the form. + it('closes the form for good after the thread has been created', async () => { + const user = userEvent.setup(); + const setDraft = jest.fn(); + + const {getByPlaceholderText, queryByPlaceholderText, getByRole} = renderThreadList( + , + {setDraft} + ); + + await user.type(getByPlaceholderText('Add a comment...'), 'New thread'); + await user.click(getByRole('button', {name: 'Send'})); + + postDraftsChange({ + 'ContentElement:10': { + subjectType: 'ContentElement', + subjectId: 10, + body: 'New thread', + pending: true + } + }); + await waitFor(() => + expect(getByPlaceholderText('Add a comment...')).toBeDisabled() + ); + setDraft.mockClear(); + + postThreadChange({ + id: 1, + subjectType: 'ContentElement', + subjectId: 10, + comments: [{id: 10, body: 'New thread', creatorName: 'Alice', creatorId: 1}] + }); + postDraftsChange({}); + + await waitFor(() => + expect(queryByPlaceholderText('Add a comment...')).toBeNull() + ); + expect(setDraft).not.toHaveBeenCalled(); + }); }); describe('replies', () => { diff --git a/entry_types/scrolled/package/spec/support/renderWithReviewState.js b/entry_types/scrolled/package/spec/support/renderWithReviewState.js index 34ca29dfcb..ad31f6435c 100644 --- a/entry_types/scrolled/package/spec/support/renderWithReviewState.js +++ b/entry_types/scrolled/package/spec/support/renderWithReviewState.js @@ -9,25 +9,27 @@ import {LocatedCommentThreadsProvider} from 'review/useLocatedCommentThreads'; // lives in), so it needs the entry scope just like any other frontend // component — review state is layered on top via the entry helper's // `wrapper` option. -export function renderWithReviewState(ui, {commentThreads = [], currentUser = null, seed = {}} = {}) { +export function renderWithReviewState(ui, {commentThreads = [], currentUser = null, drafts, setDraft, seed = {}} = {}) { return renderInEntry(ui, { seed, - wrapper: reviewStateWrapper({commentThreads, currentUser}) + wrapper: reviewStateWrapper({commentThreads, currentUser, drafts, setDraft}) }); } // Counterpart for selector hooks that read entry state and review state. -export function renderHookWithReviewState(hook, {commentThreads = [], currentUser = null, seed = {}} = {}) { +export function renderHookWithReviewState(hook, {commentThreads = [], currentUser = null, drafts, setDraft, seed = {}} = {}) { return renderHookInEntry(hook, { seed, - wrapper: reviewStateWrapper({commentThreads, currentUser}) + wrapper: reviewStateWrapper({commentThreads, currentUser, drafts, setDraft}) }); } -function reviewStateWrapper({commentThreads = [], currentUser = null} = {}) { +function reviewStateWrapper({commentThreads = [], currentUser = null, drafts, setDraft} = {}) { return function ReviewStateWrapper({children}) { return ( - + {children} diff --git a/entry_types/scrolled/package/src/editor/controllers/PreviewMessageController.js b/entry_types/scrolled/package/src/editor/controllers/PreviewMessageController.js index 5355635100..85b1fb990e 100644 --- a/entry_types/scrolled/package/src/editor/controllers/PreviewMessageController.js +++ b/entry_types/scrolled/package/src/editor/controllers/PreviewMessageController.js @@ -128,6 +128,28 @@ export const PreviewMessageController = Object.extend({ }) }); + // Passing the thread id here rather than posting + // SELECT_COMMENT_THREAD avoids depending on the preview having + // processed the new thread already: that message is dropped for + // threads the iframe does not know yet. + if (this.entry.reviewSession) { + this.listenTo(this.entry.reviewSession, 'create:thread', thread => { + const model = modelForSubject(this.entry, thread); + + if (!model) return; + + postMessage({ + type: 'SELECT', + payload: { + id: model.id, + type: thread.subjectType === 'Section' ? 'sectionComments' : + 'contentElementComments', + highlightedThreadId: thread.id + } + }) + }); + } + this.listenTo(this.entry, 'selectWidget', widget => { postMessage({ type: 'SELECT', @@ -312,9 +334,13 @@ function selectedCommentsSubjectFor(entry, payload) { } if (type === 'newThread') { - const collection = subjectType === 'Section' ? entry.sections : entry.contentElements; - return {subjectType, id: collection.findWhere({permaId: subjectId})?.id}; + return {subjectType, id: modelForSubject(entry, {subjectType, subjectId})?.id}; } return undefined; } + +function modelForSubject(entry, {subjectType, subjectId}) { + const collection = subjectType === 'Section' ? entry.sections : entry.contentElements; + return collection.findWhere({permaId: subjectId}); +} diff --git a/entry_types/scrolled/package/src/editor/views/NewThreadView.js b/entry_types/scrolled/package/src/editor/views/NewThreadView.js index 2cc80348f9..da734ae235 100644 --- a/entry_types/scrolled/package/src/editor/views/NewThreadView.js +++ b/entry_types/scrolled/package/src/editor/views/NewThreadView.js @@ -49,14 +49,12 @@ const NewThreadFormView = ReviewView.extend({ className: styles.form, renderContent() { - const {entry, subjectType, subjectId, subjectRange} = this.options; - const leave = () => entry.trigger('resetSelection'); + const {subjectType, subjectId, subjectRange} = this.options; return ( + subjectRange={subjectRange} /> ); } }); diff --git a/entry_types/scrolled/package/src/editor/views/ReviewView.js b/entry_types/scrolled/package/src/editor/views/ReviewView.js index b16145381c..2aa12191be 100644 --- a/entry_types/scrolled/package/src/editor/views/ReviewView.js +++ b/entry_types/scrolled/package/src/editor/views/ReviewView.js @@ -19,13 +19,13 @@ import styles from './ReviewView.module.css'; // Base Marionette view for comment-related sidebar panels. Provides the // shared wiring: a container div, a ReviewMessageHandler bridging the // session to the preview iframe, a ReviewStateProvider seeded from the -// current session state, and an EntryStateProvider so the rendered tree -// can resolve entry structure (e.g. the section a comment subject lives -// in). Subclasses implement `renderContent(props)` to return the React -// subtree. Props are produced by `props()` (default: empty) and -// re-evaluated whenever the subclass calls `rerender()` — useful for -// re-rendering on backbone change events without requiring React -// subscription hooks inside the rendered tree. +// current session state and its drafts, and an EntryStateProvider so the +// rendered tree can resolve entry structure (e.g. the section a comment +// subject lives in). Subclasses implement `renderContent(props)` to +// return the React subtree. Props are produced by `props()` (default: +// empty) and re-evaluated whenever the subclass calls `rerender()` — +// useful for re-rendering on backbone change events without requiring +// React subscription hooks inside the rendered tree. export const ReviewView = Marionette.ItemView.extend({ template: () => `
`, @@ -41,6 +41,8 @@ export const ReviewView = Marionette.ItemView.extend({ targetWindow: window }); + this.setDraft = draft => session.setDraft(draft); + this.rerender(); }, @@ -52,7 +54,9 @@ export const ReviewView = Marionette.ItemView.extend({ rerender() { const {entry} = this.options; ReactDOM.render( - + diff --git a/entry_types/scrolled/package/src/review/NewThreadForm.js b/entry_types/scrolled/package/src/review/NewThreadForm.js index bec4dba95e..fbebb9cd5d 100644 --- a/entry_types/scrolled/package/src/review/NewThreadForm.js +++ b/entry_types/scrolled/package/src/review/NewThreadForm.js @@ -1,26 +1,24 @@ -import React, {useCallback, useState} from 'react'; - -import {useSectionPermaIdOfSubject} from 'pageflow-scrolled/entryState'; +import React, {useCallback} from 'react'; import {useI18n} from '../frontend/i18n'; -import {postCreateCommentThreadMessage} from './postMessage'; -import {useSubjectQuote} from './subjectQuote'; +import {useCreateCommentThread} from './ReviewStateProvider'; +import {useDraftedBody} from './useDraftedBody'; import {autoGrow, autoResize} from './autoGrow'; import {isSubmitShortcut} from './submitShortcut'; import SendIcon from './images/send.svg'; +import SpinnerIcon from '../frontend/icons/spinner.svg'; import styles from './NewThreadForm.module.css'; export function NewThreadForm({subjectType, subjectId, subjectRange, onSubmit}) { const {t} = useI18n({locale: 'ui'}); - const [body, setBody] = useState(''); - const hasText = body.trim().length > 0; - const sectionPermaId = useSectionPermaIdOfSubject({subjectType, subjectId}); + const {body, setBody, submitting} = useDraftedBody({subjectType, subjectId}); + const hasText = body.trim().length > 0; - // Recorded now since the commented text can change afterwards, leaving the - // comment without the wording it referred to. - const quote = useSubjectQuote({subjectType, subjectId, subjectRange}); + const createCommentThread = useCreateCommentThread({ + subjectType, subjectId, subjectRange + }); // preventScroll keeps focus from yanking the page to the top before the // portaled popover has been positioned by floating-ui. @@ -47,33 +45,33 @@ export function NewThreadForm({subjectType, subjectId, subjectRange, onSubmit}) } function createThread() { - if (!hasText) return; + if (!hasText || submitting) return; - postCreateCommentThreadMessage({ - subjectType, subjectId, subjectRange, sectionPermaId, body, quote - }); - setBody(''); + createCommentThread(body); if (onSubmit) onSubmit(); } return ( -
+