diff --git a/entry_types/scrolled/config/locales/de.yml b/entry_types/scrolled/config/locales/de.yml index 6b6c0c47d5..e52ba7b22d 100644 --- a/entry_types/scrolled/config/locales/de.yml +++ b/entry_types/scrolled/config/locales/de.yml @@ -1587,9 +1587,14 @@ de: main_menu: comments: Kommentare comments_view: + new_thread: Neues Thema tabs: comments: Alle Kommentare selection: Für Auswahl + new_thread_view: + back: Kommentare + tabs: + newComment: Neues Thema help_entries: content_elements: menu_item: Inhaltselemente @@ -1938,3 +1943,4 @@ de: reply_count: one: 1 Antwort other: '%{count} Antworten' + no_threads_yet: Noch keine Kommentare diff --git a/entry_types/scrolled/config/locales/en.yml b/entry_types/scrolled/config/locales/en.yml index 92439f3d7c..a849f87591 100644 --- a/entry_types/scrolled/config/locales/en.yml +++ b/entry_types/scrolled/config/locales/en.yml @@ -1570,9 +1570,14 @@ en: main_menu: comments: Comments comments_view: + new_thread: New topic tabs: comments: All comments selection: For selection + new_thread_view: + back: Comments + tabs: + newComment: New topic help_entries: content_elements: menu_item: Content Elements @@ -1767,3 +1772,4 @@ en: reply_count: one: 1 reply other: '%{count} replies' + no_threads_yet: No comments yet diff --git a/entry_types/scrolled/package/jest.config.js b/entry_types/scrolled/package/jest.config.js index 269d8a3383..3030b327b0 100644 --- a/entry_types/scrolled/package/jest.config.js +++ b/entry_types/scrolled/package/jest.config.js @@ -14,6 +14,7 @@ module.exports = { setupFilesAfterEnv: [ '/spec/support/matchMediaStub.js', '/spec/support/requestAnimationFrameStub.js', + '/spec/support/scrollIntoViewStub.js', '/spec/support/fakeBrowserFeatures.js' ], modulePaths: ['/src', '/spec'], diff --git a/entry_types/scrolled/package/spec/contentElements/textBlock/editor-spec.js b/entry_types/scrolled/package/spec/contentElements/textBlock/editor-spec.js index ce12d2330a..e746be46aa 100644 --- a/entry_types/scrolled/package/spec/contentElements/textBlock/editor-spec.js +++ b/entry_types/scrolled/package/spec/contentElements/textBlock/editor-spec.js @@ -128,4 +128,49 @@ describe('contentElements/textBlock/editor', () => { }); }); }); + + describe('#compareRanges', () => { + function range(anchor, focus) { + return { + anchor: {path: [anchor[0], anchor[1]], offset: anchor[2]}, + focus: {path: [focus[0], focus[1]], offset: focus[2]} + }; + } + + it('orders ranges by start block', () => { + const earlier = range([0, 0, 0], [0, 0, 5]); + const later = range([1, 0, 0], [1, 0, 5]); + + expect(type.compareRanges(earlier, later)).toBeLessThan(0); + expect(type.compareRanges(later, earlier)).toBeGreaterThan(0); + }); + + it('orders ranges within the same block by offset', () => { + const earlier = range([0, 0, 2], [0, 0, 4]); + const later = range([0, 0, 6], [0, 0, 8]); + + expect(type.compareRanges(earlier, later)).toBeLessThan(0); + }); + + it('uses the smaller of anchor and focus as the start point', () => { + const reversed = range([0, 0, 8], [0, 0, 2]); + const forward = range([0, 0, 5], [0, 0, 6]); + + expect(type.compareRanges(reversed, forward)).toBeLessThan(0); + }); + + it('returns 0 for equal ranges', () => { + const r = range([0, 0, 0], [0, 0, 5]); + + expect(type.compareRanges(r, r)).toBe(0); + }); + + it('sorts range-less subjects last', () => { + const r = range([0, 0, 0], [0, 0, 5]); + + expect(type.compareRanges(undefined, r)).toBeGreaterThan(0); + expect(type.compareRanges(r, undefined)).toBeLessThan(0); + expect(type.compareRanges(undefined, undefined)).toBe(0); + }); + }); }); 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 eb9ec16c5d..afee2505f4 100644 --- a/entry_types/scrolled/package/spec/editor/controllers/PreviewMessageController-spec.js +++ b/entry_types/scrolled/package/spec/editor/controllers/PreviewMessageController-spec.js @@ -177,6 +177,37 @@ describe('PreviewMessageController', () => { }); }); + it('forwards selectNewThread events to iframe as SELECT newThread', async () => { + const entry = factories.entry(ScrolledEntry, {}, {entryTypeSeed: normalizeSeed()}); + const iframeWindow = createIframeWindow(); + controller = new PreviewMessageController({entry, iframeWindow}); + + await postReadyMessageAndWaitForAcknowledgement(iframeWindow); + + const range = {anchor: {path: [0, 0], offset: 0}, focus: {path: [0, 0], offset: 5}}; + + return expect(new Promise(resolve => { + iframeWindow.addEventListener('message', event => { + if (event.data.type === 'SELECT' && event.data.payload.type === 'newThread') { + resolve(event.data); + } + }); + entry.trigger('selectNewThread', { + id: 10, + subjectType: 'ContentElement', + range + }); + })).resolves.toMatchObject({ + type: 'SELECT', + payload: { + type: 'newThread', + id: 10, + subjectType: 'ContentElement', + range + } + }); + }); + it('passes on range from selectContentElement event', async () => { const entry = factories.entry(ScrolledEntry, {}, { entryTypeSeed: normalizeSeed({ diff --git a/entry_types/scrolled/package/spec/editor/views/CommentsView-spec.js b/entry_types/scrolled/package/spec/editor/views/CommentsView-spec.js index f86a2fc49a..e15ae778ce 100644 --- a/entry_types/scrolled/package/spec/editor/views/CommentsView-spec.js +++ b/entry_types/scrolled/package/spec/editor/views/CommentsView-spec.js @@ -9,6 +9,14 @@ import {useEditorGlobals} from 'support'; import {fireEvent} from '@testing-library/dom'; import {act} from '@testing-library/react'; +function unselectedEntry(createEntry) { + const entry = createEntry({ + contentElements: [{id: 1, permaId: 10, typeName: 'textBlock'}] + }); + entry.reviewSession = factories.reviewSession(); + return entry; +} + describe('CommentsView', () => { const {createEntry} = useEditorGlobals(); @@ -19,6 +27,7 @@ describe('CommentsView', () => { 'pageflow_scrolled.editor.content_elements.textBlock.name': 'Text', 'pageflow_scrolled.editor.comments_view.tabs.comments': 'All comments', 'pageflow_scrolled.editor.comments_view.tabs.selection': 'For selection', + 'pageflow_scrolled.editor.comments_view.new_thread': 'New topic', 'pageflow.editor.templates.back_button_decorator.outline': 'Outline' }); @@ -108,4 +117,72 @@ describe('CommentsView', () => { navigate.mockRestore(); }); + it('disables the new-thread button when no content element is selected', () => { + const entry = unselectedEntry(createEntry); + + const view = new CommentsView({entry, editor}); + const {getByRole} = renderBackboneView(view); + + expect(getByRole('button', {name: 'New topic'})).toBeDisabled(); + }); + + it('enables the new-thread button when a content element is selected', () => { + const entry = unselectedEntry(createEntry); + entry.set('selectedContentElementCommentsId', 1); + + const view = new CommentsView({entry, editor}); + const {getByRole} = renderBackboneView(view); + + expect(getByRole('button', {name: 'New topic'})).not.toBeDisabled(); + }); + + it("toggles the new-thread button when entry.selectedContentElementCommentsId changes", () => { + const entry = unselectedEntry(createEntry); + + const view = new CommentsView({entry, editor}); + const {getByRole} = renderBackboneView(view); + + expect(getByRole('button', {name: 'New topic'})).toBeDisabled(); + + act(() => { entry.set('selectedContentElementCommentsId', 1); }); + + expect(getByRole('button', {name: 'New topic'})).not.toBeDisabled(); + }); + + it('triggers selectNewThread on entry when the new-thread button is clicked', () => { + const entry = unselectedEntry(createEntry); + entry.set('selectedContentElementCommentsId', 1); + const range = {anchor: {path: [0, 0], offset: 0}, focus: {path: [0, 0], offset: 5}}; + entry.contentElements.get(1).transientState + .set('newCommentThreadSubjectRange', range); + + const view = new CommentsView({entry, editor}); + const {getByRole} = renderBackboneView(view); + + const listener = jest.fn(); + entry.on('selectNewThread', listener); + + fireEvent.click(getByRole('button', {name: 'New topic'})); + + expect(listener).toHaveBeenCalledWith({ + id: 10, + subjectType: 'ContentElement', + range + }); + }); + + it('does not trigger selectNewThread when clicking the button while disabled', () => { + const entry = unselectedEntry(createEntry); + + const view = new CommentsView({entry, editor}); + const {getByRole} = renderBackboneView(view); + + const listener = jest.fn(); + entry.on('selectNewThread', listener); + + fireEvent.click(getByRole('button', {name: 'New topic'})); + + expect(listener).not.toHaveBeenCalled(); + }); + }); diff --git a/entry_types/scrolled/package/spec/editor/views/ContentElementCommentsView-spec.js b/entry_types/scrolled/package/spec/editor/views/ContentElementCommentsView-spec.js index ec9efccdcb..17edc0ed74 100644 --- a/entry_types/scrolled/package/spec/editor/views/ContentElementCommentsView-spec.js +++ b/entry_types/scrolled/package/spec/editor/views/ContentElementCommentsView-spec.js @@ -1,6 +1,7 @@ import '@testing-library/jest-dom/extend-expect'; import userEvent from '@testing-library/user-event'; +import {editor} from 'pageflow-scrolled/editor'; import {ContentElementCommentsView} from 'editor/views/ContentElementCommentsView'; import {factories, useFakeTranslations, renderBackboneView} from 'pageflow/testHelpers'; @@ -10,15 +11,22 @@ import {act, waitFor} from '@testing-library/react'; describe('ContentElementCommentsView', () => { const {createEntry} = useEditorGlobals(); + beforeAll(() => { + editor.contentElementTypes.register('fixture', { + compareRanges: (a, b) => (a?.start ?? 0) - (b?.start ?? 0) + }); + }); + useFakeTranslations({ 'pageflow_scrolled.review.add_comment_placeholder': 'Add a comment...', 'pageflow_scrolled.review.new_topic': 'New topic', - 'pageflow_scrolled.review.send': 'Send' + 'pageflow_scrolled.review.send': 'Send', + 'pageflow_scrolled.review.no_threads_yet': 'No comments yet' }); it('displays threads of selected content element from session state', () => { const entry = createEntry({ - contentElements: [{id: 1, permaId: 10, typeName: 'textBlock'}] + contentElements: [{id: 1, permaId: 10, typeName: 'fixture'}] }); entry.set('selectedContentElementCommentsId', 1); entry.reviewSession = factories.reviewSession({ @@ -30,7 +38,7 @@ describe('ContentElementCommentsView', () => { }] }); - const view = new ContentElementCommentsView({entry, editor: {}}); + const view = new ContentElementCommentsView({entry, editor}); const {getByText} = renderBackboneView(view); @@ -39,7 +47,7 @@ describe('ContentElementCommentsView', () => { it('filters threads by transient state commentThreadIdsAtSelection', () => { const entry = createEntry({ - contentElements: [{id: 1, permaId: 10, typeName: 'textBlock'}] + contentElements: [{id: 1, permaId: 10, typeName: 'fixture'}] }); entry.set('selectedContentElementCommentsId', 1); entry.contentElements.get(1).transientState @@ -53,7 +61,7 @@ describe('ContentElementCommentsView', () => { ] }); - const view = new ContentElementCommentsView({entry, editor: {}}); + const view = new ContentElementCommentsView({entry, editor}); const {getByText, queryByText} = renderBackboneView(view); @@ -63,7 +71,7 @@ describe('ContentElementCommentsView', () => { it('shows all threads when transient state has no commentThreadIdsAtSelection', () => { const entry = createEntry({ - contentElements: [{id: 1, permaId: 10, typeName: 'textBlock'}] + contentElements: [{id: 1, permaId: 10, typeName: 'fixture'}] }); entry.set('selectedContentElementCommentsId', 1); entry.reviewSession = factories.reviewSession({ @@ -75,7 +83,7 @@ describe('ContentElementCommentsView', () => { ] }); - const view = new ContentElementCommentsView({entry, editor: {}}); + const view = new ContentElementCommentsView({entry, editor}); const {getByText} = renderBackboneView(view); @@ -85,7 +93,7 @@ describe('ContentElementCommentsView', () => { it('updates filter when transient state changes', async () => { const entry = createEntry({ - contentElements: [{id: 1, permaId: 10, typeName: 'textBlock'}] + contentElements: [{id: 1, permaId: 10, typeName: 'fixture'}] }); entry.set('selectedContentElementCommentsId', 1); entry.reviewSession = factories.reviewSession({ @@ -97,7 +105,7 @@ describe('ContentElementCommentsView', () => { ] }); - const view = new ContentElementCommentsView({entry, editor: {}}); + const view = new ContentElementCommentsView({entry, editor}); const {getByText, queryByText} = renderBackboneView(view); expect(getByText('Out of scope')).toBeInTheDocument(); @@ -116,8 +124,8 @@ describe('ContentElementCommentsView', () => { it('updates when selectedContentElementCommentsId changes', async () => { const entry = createEntry({ contentElements: [ - {id: 1, permaId: 10, typeName: 'textBlock'}, - {id: 2, permaId: 11, typeName: 'textBlock'} + {id: 1, permaId: 10, typeName: 'fixture'}, + {id: 2, permaId: 11, typeName: 'fixture'} ] }); entry.set('selectedContentElementCommentsId', 1); @@ -130,7 +138,7 @@ describe('ContentElementCommentsView', () => { ] }); - const view = new ContentElementCommentsView({entry, editor: {}}); + const view = new ContentElementCommentsView({entry, editor}); const {getByText, queryByText} = renderBackboneView(view); expect(getByText('On first')).toBeInTheDocument(); @@ -147,7 +155,7 @@ describe('ContentElementCommentsView', () => { it('marks thread matching entry.highlightedThreadId with aria-current when scoped', () => { const entry = createEntry({ - contentElements: [{id: 1, permaId: 10, typeName: 'textBlock'}] + contentElements: [{id: 1, permaId: 10, typeName: 'fixture'}] }); entry.set('selectedContentElementCommentsId', 1); entry.contentElements.get(1).transientState @@ -162,7 +170,7 @@ describe('ContentElementCommentsView', () => { ] }); - const view = new ContentElementCommentsView({entry, editor: {}}); + const view = new ContentElementCommentsView({entry, editor}); const {getByText} = renderBackboneView(view); expect(getByText('second').closest('[aria-current="true"]')).not.toBeNull(); @@ -171,7 +179,7 @@ describe('ContentElementCommentsView', () => { it('updates highlight when entry.highlightedThreadId changes', () => { const entry = createEntry({ - contentElements: [{id: 1, permaId: 10, typeName: 'textBlock'}] + contentElements: [{id: 1, permaId: 10, typeName: 'fixture'}] }); entry.set('selectedContentElementCommentsId', 1); entry.contentElements.get(1).transientState @@ -185,7 +193,7 @@ describe('ContentElementCommentsView', () => { ] }); - const view = new ContentElementCommentsView({entry, editor: {}}); + const view = new ContentElementCommentsView({entry, editor}); const {getByText} = renderBackboneView(view); expect(getByText('first').closest('[aria-current="true"]')).toBeNull(); @@ -199,7 +207,7 @@ describe('ContentElementCommentsView', () => { const user = userEvent.setup(); const entry = createEntry({ - contentElements: [{id: 1, permaId: 10, typeName: 'textBlock'}] + contentElements: [{id: 1, permaId: 10, typeName: 'fixture'}] }); entry.set('selectedContentElementCommentsId', 1); entry.contentElements.get(1).transientState @@ -214,7 +222,7 @@ describe('ContentElementCommentsView', () => { const listener = jest.fn(); entry.on('selectCommentThread', listener); - const view = new ContentElementCommentsView({entry, editor: {}}); + const view = new ContentElementCommentsView({entry, editor}); const {getByText} = renderBackboneView(view); await user.click(getByText('click me')); @@ -226,7 +234,7 @@ describe('ContentElementCommentsView', () => { const user = userEvent.setup(); const entry = createEntry({ - contentElements: [{id: 1, permaId: 10, typeName: 'textBlock'}] + contentElements: [{id: 1, permaId: 10, typeName: 'fixture'}] }); entry.set('selectedContentElementCommentsId', 1); entry.set('highlightedThreadId', 7); @@ -240,7 +248,7 @@ describe('ContentElementCommentsView', () => { const listener = jest.fn(); entry.on('selectCommentThread', listener); - const view = new ContentElementCommentsView({entry, editor: {}}); + const view = new ContentElementCommentsView({entry, editor}); const {getByText} = renderBackboneView(view); expect(getByText('click me').closest('[aria-current="true"]')).toBeNull(); @@ -250,11 +258,89 @@ describe('ContentElementCommentsView', () => { expect(listener).not.toHaveBeenCalled(); }); - it('does not render a new-topic button', () => { + it("orders threads by the type's compareRanges", () => { + const entry = createEntry({ + contentElements: [{id: 1, permaId: 10, typeName: 'fixture'}] + }); + entry.set('selectedContentElementCommentsId', 1); + entry.reviewSession = factories.reviewSession({ + commentThreads: [ + {id: 1, subjectType: 'ContentElement', subjectId: 10, + subjectRange: {start: 20}, + comments: [{id: 10, body: 'second', creatorName: 'A'}]}, + {id: 2, subjectType: 'ContentElement', subjectId: 10, + subjectRange: {start: 10}, + comments: [{id: 20, body: 'first', creatorName: 'B'}]} + ] + }); + + const view = new ContentElementCommentsView({entry, editor}); + const {getByText} = renderBackboneView(view); + + const order = ['first', 'second'] + .map(text => getByText(text).getBoundingClientRect().top); + + expect(order).toEqual([...order].sort((a, b) => a - b)); + }); + + it("orders scoped threads by the type's compareRanges", () => { + const entry = createEntry({ + contentElements: [{id: 1, permaId: 10, typeName: 'fixture'}] + }); + entry.set('selectedContentElementCommentsId', 1); + entry.contentElements.get(1).transientState + .set('commentThreadIdsAtSelection', [1, 2]); + entry.reviewSession = factories.reviewSession({ + commentThreads: [ + {id: 1, subjectType: 'ContentElement', subjectId: 10, + subjectRange: {start: 20}, + comments: [{id: 10, body: 'second', creatorName: 'A'}]}, + {id: 2, subjectType: 'ContentElement', subjectId: 10, + subjectRange: {start: 10}, + comments: [{id: 20, body: 'first', creatorName: 'B'}]} + ] + }); + + const view = new ContentElementCommentsView({entry, editor}); + const {getByText} = renderBackboneView(view); + + const order = ['first', 'second'] + .map(text => getByText(text).getBoundingClientRect().top); + + expect(order).toEqual([...order].sort((a, b) => a - b)); + }); + + it('does not render the new thread form when selected element has no threads', () => { + const entry = createEntry({ + contentElements: [{id: 1, permaId: 10, typeName: 'textBlock'}] + }); + entry.set('selectedContentElementCommentsId', 1); + entry.reviewSession = factories.reviewSession(); + + const view = new ContentElementCommentsView({entry, editor}); + const {queryByPlaceholderText} = renderBackboneView(view); + + expect(queryByPlaceholderText('Add a comment...')).not.toBeInTheDocument(); + }); + + it('shows blank slate when selected element has no threads', () => { const entry = createEntry({ contentElements: [{id: 1, permaId: 10, typeName: 'textBlock'}] }); entry.set('selectedContentElementCommentsId', 1); + entry.reviewSession = factories.reviewSession(); + + const view = new ContentElementCommentsView({entry, editor}); + const {getByText} = renderBackboneView(view); + + expect(getByText('No comments yet')).toBeInTheDocument(); + }); + + it('does not render a new-topic button', () => { + const entry = createEntry({ + contentElements: [{id: 1, permaId: 10, typeName: 'fixture'}] + }); + entry.set('selectedContentElementCommentsId', 1); entry.reviewSession = factories.reviewSession({ commentThreads: [{ id: 1, @@ -264,7 +350,7 @@ describe('ContentElementCommentsView', () => { }] }); - const view = new ContentElementCommentsView({entry, editor: {}}); + const view = new ContentElementCommentsView({entry, editor}); const {queryByRole} = renderBackboneView(view); expect(queryByRole('button', {name: 'New topic'})).not.toBeInTheDocument(); @@ -272,12 +358,12 @@ describe('ContentElementCommentsView', () => { it('updates when session emits change:thread', async () => { const entry = createEntry({ - contentElements: [{id: 1, permaId: 10, typeName: 'textBlock'}] + contentElements: [{id: 1, permaId: 10, typeName: 'fixture'}] }); entry.set('selectedContentElementCommentsId', 1); entry.reviewSession = factories.reviewSession(); - const view = new ContentElementCommentsView({entry, editor: {}}); + const view = new ContentElementCommentsView({entry, editor}); const {getByText} = renderBackboneView(view); diff --git a/entry_types/scrolled/package/spec/editor/views/EntryCommentsView-spec.js b/entry_types/scrolled/package/spec/editor/views/EntryCommentsView-spec.js index bfe480967d..6a4dc5237c 100644 --- a/entry_types/scrolled/package/spec/editor/views/EntryCommentsView-spec.js +++ b/entry_types/scrolled/package/spec/editor/views/EntryCommentsView-spec.js @@ -13,9 +13,16 @@ import {useEditorGlobals} from 'support'; describe('EntryCommentsView', () => { const {createEntry} = useEditorGlobals(); + beforeAll(() => { + editor.contentElementTypes.register('fixture', { + compareRanges: (a, b) => (a?.start ?? 0) - (b?.start ?? 0) + }); + }); + useFakeTranslations({ 'pageflow_scrolled.review.new_topic': 'New topic', 'pageflow_scrolled.review.add_comment_placeholder': 'Add a comment...', + 'pageflow_scrolled.review.reply_placeholder': 'Reply...', 'pageflow_scrolled.review.send': 'Send', 'pageflow_scrolled.editor.content_elements.textBlock.name': 'Text', 'pageflow_scrolled.editor.content_elements.image.name': 'Image' @@ -172,6 +179,135 @@ describe('EntryCommentsView', () => { expect(getByText('first').closest('[aria-current="true"]')).not.toBeNull(); }); + it("highlights all threads of the selected element when it has no commentThreadIdsAtSelection", () => { + const entry = createEntry({ + contentElements: [ + {id: 1, permaId: 10, typeName: 'image'}, + {id: 2, permaId: 11, typeName: 'image'} + ] + }); + entry.set('selectedContentElementCommentsId', 1); + entry.reviewSession = factories.reviewSession({ + commentThreads: [ + {id: 1, subjectType: 'ContentElement', subjectId: 10, + comments: [{id: 10, body: 'on selected one', creatorName: 'A'}]}, + {id: 2, subjectType: 'ContentElement', subjectId: 10, + comments: [{id: 20, body: 'on selected two', creatorName: 'B'}]}, + {id: 3, subjectType: 'ContentElement', subjectId: 11, + comments: [{id: 30, body: 'on other', creatorName: 'C'}]} + ] + }); + + const view = new EntryCommentsView({entry, editor}); + const {getByText} = renderBackboneView(view); + + expect(getByText('on selected one').closest('[aria-current="true"]')).not.toBeNull(); + expect(getByText('on selected two').closest('[aria-current="true"]')).not.toBeNull(); + expect(getByText('on other').closest('[aria-current="true"]')).toBeNull(); + }); + + it("falls back to highlightedThreadId when the selected element has commentThreadIdsAtSelection", () => { + const entry = createEntry({ + contentElements: [{id: 1, permaId: 10, typeName: 'textBlock'}] + }); + entry.set('selectedContentElementCommentsId', 1); + entry.contentElements.get(1).transientState + .set('commentThreadIdsAtSelection', [2]); + entry.set('highlightedThreadId', 2); + entry.reviewSession = factories.reviewSession({ + commentThreads: [ + {id: 1, subjectType: 'ContentElement', subjectId: 10, + comments: [{id: 10, body: 'first', creatorName: 'A'}]}, + {id: 2, subjectType: 'ContentElement', subjectId: 10, + comments: [{id: 20, body: 'second', creatorName: 'B'}]} + ] + }); + + const view = new EntryCommentsView({entry, editor}); + const {getByText} = renderBackboneView(view); + + expect(getByText('first').closest('[aria-current="true"]')).toBeNull(); + expect(getByText('second').closest('[aria-current="true"]')).not.toBeNull(); + }); + + it('updates highlights when selectedContentElementCommentsId changes', () => { + const entry = createEntry({ + contentElements: [ + {id: 1, permaId: 10, typeName: 'image'}, + {id: 2, permaId: 11, typeName: 'image'} + ] + }); + entry.reviewSession = factories.reviewSession({ + commentThreads: [ + {id: 1, subjectType: 'ContentElement', subjectId: 10, + comments: [{id: 10, body: 'on one', creatorName: 'A'}]}, + {id: 2, subjectType: 'ContentElement', subjectId: 11, + comments: [{id: 20, body: 'on two', creatorName: 'B'}]} + ] + }); + + const view = new EntryCommentsView({entry, editor}); + const {getByText} = renderBackboneView(view); + + expect(getByText('on one').closest('[aria-current="true"]')).toBeNull(); + expect(getByText('on two').closest('[aria-current="true"]')).toBeNull(); + + act(() => { entry.set('selectedContentElementCommentsId', 2); }); + + expect(getByText('on one').closest('[aria-current="true"]')).toBeNull(); + expect(getByText('on two').closest('[aria-current="true"]')).not.toBeNull(); + }); + + it('only shows reply form on the highlighted thread', () => { + const entry = createEntry({ + contentElements: [{id: 1, permaId: 10, typeName: 'textBlock'}] + }); + entry.reviewSession = factories.reviewSession({ + commentThreads: [ + {id: 1, subjectType: 'ContentElement', subjectId: 10, + comments: [{id: 10, body: 'first', creatorName: 'Alice'}]}, + {id: 2, subjectType: 'ContentElement', subjectId: 10, + comments: [{id: 20, body: 'second', creatorName: 'Bob'}]} + ] + }); + entry.set('highlightedThreadId', 2); + + const view = new EntryCommentsView({entry, editor}); + const {getByText, queryAllByPlaceholderText} = renderBackboneView(view); + + const replyInputs = queryAllByPlaceholderText('Reply...'); + expect(replyInputs).toHaveLength(1); + expect(getByText('second').closest('[aria-current="true"]')) + .toContainElement(replyInputs[0]); + }); + + it("orders threads within a group by the type's compareRanges", () => { + const entry = createEntry({ + contentElements: [{id: 1, permaId: 10, typeName: 'fixture'}] + }); + entry.reviewSession = factories.reviewSession({ + commentThreads: [ + {id: 1, subjectType: 'ContentElement', subjectId: 10, + subjectRange: {start: 30}, + comments: [{id: 10, body: 'third', creatorName: 'A'}]}, + {id: 2, subjectType: 'ContentElement', subjectId: 10, + subjectRange: {start: 10}, + comments: [{id: 20, body: 'first', creatorName: 'B'}]}, + {id: 3, subjectType: 'ContentElement', subjectId: 10, + subjectRange: {start: 20}, + comments: [{id: 30, body: 'second', creatorName: 'C'}]} + ] + }); + + const view = new EntryCommentsView({entry, editor}); + const {getByText} = renderBackboneView(view); + + const order = ['first', 'second', 'third'] + .map(text => getByText(text).getBoundingClientRect().top); + + expect(order).toEqual([...order].sort((a, b) => a - b)); + }); + it('shows the content element type name as group label', () => { const entry = createEntry({ contentElements: [{id: 1, permaId: 10, typeName: 'image'}] 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 10ee72616f..2c5bf03d03 100644 --- a/entry_types/scrolled/package/spec/editor/views/NewThreadView-spec.js +++ b/entry_types/scrolled/package/spec/editor/views/NewThreadView-spec.js @@ -1,9 +1,13 @@ import '@testing-library/jest-dom/extend-expect'; +import {editor} from 'pageflow-scrolled/editor'; + + import {NewThreadView} from 'editor/views/NewThreadView'; import {factories, useFakeTranslations, renderBackboneView} from 'pageflow/testHelpers'; import {useEditorGlobals} from 'support'; +import {fireEvent} from '@testing-library/dom'; describe('NewThreadView', () => { const {createEntry} = useEditorGlobals(); @@ -11,14 +15,13 @@ describe('NewThreadView', () => { useFakeTranslations({ 'pageflow_scrolled.review.add_comment_placeholder': 'Add a comment...', 'pageflow_scrolled.review.send': 'Send', - 'pageflow_scrolled.review.cancel': 'Cancel' + 'pageflow_scrolled.review.cancel': 'Cancel', + 'pageflow_scrolled.editor.new_thread_view.tabs.newComment': 'New topic', + 'pageflow_scrolled.editor.new_thread_view.back': 'Comments' }); - it('renders a new thread form for the pending subject', () => { - const entry = createEntry({}); - entry.reviewSession = factories.reviewSession(); - - const view = new NewThreadView({ + function buildView(entry) { + return new NewThreadView({ entry, subjectType: 'ContentElement', subjectId: 10, @@ -26,12 +29,51 @@ describe('NewThreadView', () => { anchor: {path: [0, 0], offset: 0}, focus: {path: [0, 0], offset: 5} }, - editor: {} + editor }); + } + + it('renders a new thread form for the pending subject', () => { + const entry = createEntry({}); + entry.reviewSession = factories.reviewSession(); - const {getByPlaceholderText} = renderBackboneView(view); - view.onShow(); + const {getByPlaceholderText} = renderBackboneView(buildView(entry)); expect(getByPlaceholderText('Add a comment...')).toBeInTheDocument(); }); + + it('renders a New topic tab label above the form', () => { + const entry = createEntry({}); + entry.reviewSession = factories.reviewSession(); + + const {getByRole} = renderBackboneView(buildView(entry)); + + expect(getByRole('tab', {name: 'New topic'})).toBeInTheDocument(); + }); + + it('renders a back link', () => { + const entry = createEntry({}); + entry.reviewSession = factories.reviewSession(); + + const {getByText} = renderBackboneView(buildView(entry)); + + expect(getByText('Comments')).toBeInTheDocument(); + }); + + it('navigates to the comments selection tab when the back link is clicked', () => { + const entry = createEntry({}); + entry.reviewSession = factories.reviewSession(); + + const {getByText} = renderBackboneView(buildView(entry)); + + const navigate = jest.spyOn(editor, 'navigate').mockImplementation(() => {}); + + fireEvent.click(getByText('Comments')); + + expect(navigate).toHaveBeenCalledWith( + '/scrolled/comments?tab=selection', {trigger: true} + ); + + navigate.mockRestore(); + }); }); diff --git a/entry_types/scrolled/package/spec/frontend/features/editorCommentBadges-spec.js b/entry_types/scrolled/package/spec/frontend/features/editorCommentBadges-spec.js index 914fbd536b..44abfd4fab 100644 --- a/entry_types/scrolled/package/spec/frontend/features/editorCommentBadges-spec.js +++ b/entry_types/scrolled/package/spec/frontend/features/editorCommentBadges-spec.js @@ -5,6 +5,8 @@ import {features} from 'pageflow/frontend'; import {useInlineEditingPageObjects, renderEntry} from 'support/pageObjects'; import {fakeParentWindow} from 'support'; +import badgeStyles from 'review/Badge.module.css'; + describe('editor comment badges', () => { useInlineEditingPageObjects(); @@ -62,4 +64,28 @@ describe('editor comment badges', () => { expect(getByRole('status')).not.toHaveTextContent(/\d/); }); }); + + it('renders badge in active mode when newThread is selected on the element', () => { + const {getByRole} = renderEntry({ + seed: { + contentElements: [{ + typeName: 'withTestId', + permaId: 10, + configuration: {testId: 5} + }] + } + }); + + act(() => { + window.dispatchEvent(new MessageEvent('message', { + data: { + type: 'SELECT', + payload: {type: 'newThread', id: 10} + }, + origin: window.location.origin + })); + }); + + expect(getByRole('status')).toHaveClass(badgeStyles.active); + }); }); diff --git a/entry_types/scrolled/package/spec/frontend/features/selectedMessage-spec.js b/entry_types/scrolled/package/spec/frontend/features/selectedMessage-spec.js index 67127c41f3..4892a5d42b 100644 --- a/entry_types/scrolled/package/spec/frontend/features/selectedMessage-spec.js +++ b/entry_types/scrolled/package/spec/frontend/features/selectedMessage-spec.js @@ -187,6 +187,27 @@ describe('SELECTED message', () => { }, expect.anything()); }); + it('is posted with type newThread when comment badge is clicked and no threads exist', () => { + features.enable('frontend', ['commenting']); + + const {getByRole, getByTestId} = renderEntry({ + seed: { + contentElements: [{id: 1, permaId: 10, typeName: 'withTestId', configuration: {testId: 5}}] + } + }); + + // Select the content element so the icon-mode badge becomes visible + // even without threads. + fireEvent.click(getByTestId('contentElement-5')); + + fireEvent.click(getByRole('status')); + + expect(window.parent.postMessage).toHaveBeenCalledWith({ + type: 'SELECTED', + payload: {type: 'newThread', id: 10} + }, expect.anything()); + }); + it('is posted with type contentElementComments when comment badge is clicked', async () => { features.enable('frontend', ['commenting']); diff --git a/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/newCommentThreadSubjectRange-spec.js b/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/newCommentThreadSubjectRange-spec.js new file mode 100644 index 0000000000..635ee8e76e --- /dev/null +++ b/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/newCommentThreadSubjectRange-spec.js @@ -0,0 +1,56 @@ +/** @jsx jsx */ +import {newCommentThreadSubjectRange} + from 'frontend/inlineEditing/EditableText/newCommentThreadSubjectRange'; + +import {createHyperscript} from 'slate-hyperscript'; + +const h = createHyperscript({ + elements: {paragraph: {type: 'paragraph'}} +}); + +const jsx = (tagName, attributes, ...children) => { + delete attributes.__self; + delete attributes.__source; + return h(tagName, attributes, ...children); +}; + +describe('newCommentThreadSubjectRange', () => { + it('returns undefined when there is no selection', () => { + const editor = ( + + Some text + + ); + + expect(newCommentThreadSubjectRange(editor)).toBeUndefined(); + }); + + it("returns the surrounding block's range when selection is collapsed", () => { + const editor = ( + + First + Second + + ); + + expect(newCommentThreadSubjectRange(editor)).toEqual({ + anchor: {path: [1, 0], offset: 0}, + focus: {path: [1, 0], offset: 6} + }); + }); + + it('returns the editor selection when expanded', () => { + const editor = ( + + + First + + + ); + + expect(newCommentThreadSubjectRange(editor)).toEqual({ + anchor: {path: [0, 0], offset: 0}, + focus: {path: [0, 0], offset: 5} + }); + }); +}); diff --git a/entry_types/scrolled/package/spec/frontend/useContentElementEditorState/inEditorPreview-spec.js b/entry_types/scrolled/package/spec/frontend/useContentElementEditorState/inEditorPreview-spec.js index ea8706ce9c..d959146e21 100644 --- a/entry_types/scrolled/package/spec/frontend/useContentElementEditorState/inEditorPreview-spec.js +++ b/entry_types/scrolled/package/spec/frontend/useContentElementEditorState/inEditorPreview-spec.js @@ -57,7 +57,7 @@ describe('useContentElementEditorState in editor preview', () => { }); renderInEntry(, { - seed: {contentElements: [{id: 7, typeName: 'test'}]} + seed: {contentElements: [{id: 7, permaId: 70, typeName: 'test'}]} }); expect(captured.type).toBeNull(); @@ -69,6 +69,10 @@ describe('useContentElementEditorState in editor preview', () => { act(() => setSelectionRef({type: 'contentElementComments', id: 7})); expect(captured.type).toBe('contentElementComments'); expect(captured.isSelected).toBe(true); + + act(() => setSelectionRef({type: 'newThread', id: 70})); + expect(captured.type).toBe('newThread'); + expect(captured.isSelected).toBe(true); }); it('returns true for isEditable', () => { diff --git a/entry_types/scrolled/package/spec/review/ThreadList-spec.js b/entry_types/scrolled/package/spec/review/ThreadList-spec.js index 8afd2375dc..7356dfaf08 100644 --- a/entry_types/scrolled/package/spec/review/ThreadList-spec.js +++ b/entry_types/scrolled/package/spec/review/ThreadList-spec.js @@ -20,7 +20,8 @@ describe('ThreadList', () => { 'pageflow_scrolled.review.resolve': 'Mark as resolved', 'pageflow_scrolled.review.unresolve': 'Mark as unresolved', 'pageflow_scrolled.review.resolved_count.one': '1 resolved', - 'pageflow_scrolled.review.resolved_count.other': '%{count} resolved' + 'pageflow_scrolled.review.resolved_count.other': '%{count} resolved', + 'pageflow_scrolled.review.no_threads_yet': 'No comments yet' }); it('displays comments of threads for subject', () => { const {getByText} = renderWithReviewState( @@ -131,6 +132,33 @@ describe('ThreadList', () => { expect(highlighted).not.toContainElement(getByText('first')); }); + it('highlights every thread when highlightedThreadId is an array of ids', () => { + const {container, getByText} = renderWithReviewState( + , + { + commentThreads: [ + {id: 1, subjectType: 'ContentElement', subjectId: 10, comments: [ + {id: 10, body: 'first', creatorName: 'Alice', creatorId: 1} + ]}, + {id: 2, subjectType: 'ContentElement', subjectId: 10, comments: [ + {id: 20, body: 'second', creatorName: 'Bob', creatorId: 2} + ]}, + {id: 3, subjectType: 'ContentElement', subjectId: 10, comments: [ + {id: 30, body: 'third', creatorName: 'Eve', creatorId: 3} + ]} + ] + } + ); + + const highlighted = container.querySelectorAll('[aria-current="true"]'); + expect(highlighted).toHaveLength(2); + expect(getByText('first').closest('[aria-current="true"]')).not.toBeNull(); + expect(getByText('second').closest('[aria-current="true"]')).not.toBeNull(); + expect(getByText('third').closest('[aria-current="true"]')).toBeNull(); + }); + it('fires onThreadClick with the clicked thread', async () => { const user = userEvent.setup(); const onThreadClick = jest.fn(); @@ -152,6 +180,102 @@ describe('ThreadList', () => { expect(onThreadClick).toHaveBeenCalledWith(expect.objectContaining({id: 7})); }); + it('scrolls the highlighted thread into view', () => { + const {getByText} = renderWithReviewState( + , + { + commentThreads: [ + {id: 1, subjectType: 'ContentElement', subjectId: 10, comments: [ + {id: 10, body: 'first', creatorName: 'Alice', creatorId: 1} + ]}, + {id: 2, subjectType: 'ContentElement', subjectId: 10, comments: [ + {id: 20, body: 'second', creatorName: 'Bob', creatorId: 2} + ]} + ] + } + ); + + const scrollIntoView = Element.prototype.scrollIntoView; + expect(scrollIntoView).toHaveBeenCalled(); + expect(scrollIntoView.mock.instances[0]) + .toBe(getByText('second').closest('[aria-current="true"]')); + }); + + it('orders threads via compareRanges when provided', () => { + const compareRanges = (a, b) => a.start - b.start; + + const {getAllByText} = renderWithReviewState( + , + { + commentThreads: [ + {id: 1, subjectType: 'ContentElement', subjectId: 10, + subjectRange: {start: 30}, + comments: [{id: 10, body: 'third', creatorName: 'Bob', creatorId: 2}]}, + {id: 2, subjectType: 'ContentElement', subjectId: 10, + subjectRange: {start: 10}, + comments: [{id: 20, body: 'first', creatorName: 'Alice', creatorId: 1}]}, + {id: 3, subjectType: 'ContentElement', subjectId: 10, + subjectRange: {start: 20}, + comments: [{id: 30, body: 'second', creatorName: 'Eve', creatorId: 3}]} + ] + } + ); + + const order = ['first', 'second', 'third'] + .map(text => getAllByText(text)[0].getBoundingClientRect().top); + + expect(order).toEqual([...order].sort((a, b) => a - b)); + }); + + it('hides reply form on non-highlighted threads when restrictInteractionsToHighlighted', () => { + const {getByText, queryAllByPlaceholderText} = renderWithReviewState( + , + { + commentThreads: [ + {id: 1, subjectType: 'ContentElement', subjectId: 10, comments: [ + {id: 10, body: 'first', creatorName: 'Alice', creatorId: 1} + ]}, + {id: 2, subjectType: 'ContentElement', subjectId: 10, comments: [ + {id: 20, body: 'second', creatorName: 'Bob', creatorId: 2} + ]} + ] + } + ); + + const replyInputs = queryAllByPlaceholderText('Reply...'); + expect(replyInputs).toHaveLength(1); + expect(getByText('second').closest('[aria-current="true"]')) + .toContainElement(replyInputs[0]); + }); + + it('hides resolve button on non-highlighted threads when restrictInteractionsToHighlighted', () => { + const {queryAllByText} = renderWithReviewState( + , + { + commentThreads: [ + {id: 1, subjectType: 'ContentElement', subjectId: 10, comments: [ + {id: 10, body: 'first', creatorName: 'Alice', creatorId: 1} + ]}, + {id: 2, subjectType: 'ContentElement', subjectId: 10, comments: [ + {id: 20, body: 'second', creatorName: 'Bob', creatorId: 2} + ]} + ] + } + ); + + expect(queryAllByText('Mark as resolved')).toHaveLength(1); + }); + it('applies filter prop to resolved threads', async () => { const user = userEvent.setup(); const {getByText, queryByText} = renderWithReviewState( @@ -434,6 +558,52 @@ describe('ThreadList', () => { expect(queryByPlaceholderText('Add a comment...')).not.toBeInTheDocument(); }); + it('shows blank slate when no threads exist and showNewForm is false', () => { + const {getByText} = renderWithReviewState( + + ); + + expect(getByText('No comments yet')).toBeInTheDocument(); + }); + + it('does not show blank slate when active threads exist', () => { + const {queryByText} = renderWithReviewState( + , + { + commentThreads: [ + {id: 1, subjectType: 'ContentElement', subjectId: 10, comments: [ + {id: 10, body: 'Looks good', creatorName: 'Bob', creatorId: 2} + ]} + ] + } + ); + + expect(queryByText('No comments yet')).not.toBeInTheDocument(); + }); + + it('does not show blank slate when only resolved threads exist', () => { + const {queryByText} = renderWithReviewState( + , + { + commentThreads: [ + {id: 1, subjectType: 'ContentElement', subjectId: 10, + resolvedAt: '2026-04-09T10:00:00Z', + comments: [{id: 10, body: 'Resolved', creatorName: 'Bob', creatorId: 2}]} + ] + } + ); + + expect(queryByText('No comments yet')).not.toBeInTheDocument(); + }); + + it('does not show blank slate when showNewForm is true', () => { + const {queryByText} = renderWithReviewState( + + ); + + expect(queryByText('No comments yet')).not.toBeInTheDocument(); + }); + it('hides new topic button when hideNewTopicButton is true', () => { const {queryByRole} = renderWithReviewState( , diff --git a/entry_types/scrolled/package/spec/support/scrollIntoViewStub.js b/entry_types/scrolled/package/spec/support/scrollIntoViewStub.js new file mode 100644 index 0000000000..56b6f4456e --- /dev/null +++ b/entry_types/scrolled/package/spec/support/scrollIntoViewStub.js @@ -0,0 +1,6 @@ +// jsdom does not implement scrollIntoView. Stub it so components that +// call it (e.g. Thread auto-scrolling its highlighted state into view) +// don't blow up during render. +beforeEach(() => { + Element.prototype.scrollIntoView = jest.fn(); +}); diff --git a/entry_types/scrolled/package/src/contentElements/textBlock/editor.js b/entry_types/scrolled/package/src/contentElements/textBlock/editor.js index 3e3237d240..cb6381d8b1 100644 --- a/entry_types/scrolled/package/src/contentElements/textBlock/editor.js +++ b/entry_types/scrolled/package/src/contentElements/textBlock/editor.js @@ -158,6 +158,13 @@ editor.contentElementTypes.register('textBlock', { return getValue(configuration).length; }, + compareRanges(a, b) { + if (!a && !b) return 0; + if (!a) return 1; + if (!b) return -1; + return Point.compare(rangeStart(a), rangeStart(b)); + }, + handleDestroy(contentElement) { const transientState = contentElement.get('transientState') || {}; @@ -195,6 +202,10 @@ function shiftPoint(point, delta) { return {...point, path: [point.path[0] + delta, ...point.path.slice(1)]}; } +function rangeStart(range) { + return Point.isBefore(range.anchor, range.focus) ? range.anchor : range.focus; +} + function endOfBlock(value, blockIndex) { const [lastNode, lastPath] = Node.last({children: value}, [blockIndex]); return {path: lastPath, offset: lastNode.text.length}; diff --git a/entry_types/scrolled/package/src/editor/api/ContentElementTypeRegistry.js b/entry_types/scrolled/package/src/editor/api/ContentElementTypeRegistry.js index 811d4b4f98..2c4c5c06b0 100644 --- a/entry_types/scrolled/package/src/editor/api/ContentElementTypeRegistry.js +++ b/entry_types/scrolled/package/src/editor/api/ContentElementTypeRegistry.js @@ -83,6 +83,10 @@ export class ContentElementTypeRegistry { return this.contentElementTypes[typeName]?.pictogram; } + findCompareRanges(typeName) { + return this.contentElementTypes[typeName]?.compareRanges; + } + groupedByCategory() { const result = []; const categoriesByName = {}; diff --git a/entry_types/scrolled/package/src/editor/controllers/PreviewMessageController.js b/entry_types/scrolled/package/src/editor/controllers/PreviewMessageController.js index 619c8c29f3..5a482e3106 100644 --- a/entry_types/scrolled/package/src/editor/controllers/PreviewMessageController.js +++ b/entry_types/scrolled/package/src/editor/controllers/PreviewMessageController.js @@ -121,6 +121,13 @@ export const PreviewMessageController = Object.extend({ }) }); + this.listenTo(this.entry, 'selectNewThread', payload => { + postMessage({ + type: 'SELECT', + payload: {type: 'newThread', ...payload} + }) + }); + this.listenTo(this.entry, 'selectWidget', widget => { postMessage({ type: 'SELECT', diff --git a/entry_types/scrolled/package/src/editor/controllers/SideBarController.js b/entry_types/scrolled/package/src/editor/controllers/SideBarController.js index 5d26f8d1d4..66fedbed1f 100644 --- a/entry_types/scrolled/package/src/editor/controllers/SideBarController.js +++ b/entry_types/scrolled/package/src/editor/controllers/SideBarController.js @@ -1,7 +1,6 @@ import Marionette from 'backbone.marionette'; import {editor} from 'pageflow-scrolled/editor'; -import {BackButtonDecoratorView} from 'pageflow/editor'; import {EditChapterView} from '../views/EditChapterView'; import {EditSectionView} from '../views/EditSectionView'; @@ -69,14 +68,12 @@ export const SideBarController = Marionette.Controller.extend({ newThread: function(subjectType, subjectId, payload) { const {subjectRange} = JSON.parse(decodeURIComponent(payload)); - this.region.show(new BackButtonDecoratorView({ - view: new NewThreadView({ - entry: this.entry, - subjectType, - subjectId: parseInt(subjectId, 10), - subjectRange, - editor - }) + this.region.show(new NewThreadView({ + entry: this.entry, + subjectType, + subjectId: parseInt(subjectId, 10), + subjectRange, + editor })); } }); diff --git a/entry_types/scrolled/package/src/editor/views/CommentsView.js b/entry_types/scrolled/package/src/editor/views/CommentsView.js index 7f62b84a0b..03f986b9c6 100644 --- a/entry_types/scrolled/package/src/editor/views/CommentsView.js +++ b/entry_types/scrolled/package/src/editor/views/CommentsView.js @@ -2,16 +2,19 @@ import I18n from 'i18n-js'; import Marionette from 'backbone.marionette'; import {editor} from 'pageflow/editor'; -import {TabsView} from 'pageflow/ui'; +import {cssModulesUtils, TabsView} from 'pageflow/ui'; import {EntryCommentsView} from './EntryCommentsView'; import {ContentElementCommentsView} from './ContentElementCommentsView'; +import styles from './CommentsView.module.css'; + export const CommentsView = Marionette.ItemView.extend({ - className: 'comments_view', + className: styles.root, template: () => ` ${I18n.t('pageflow.editor.templates.back_button_decorator.outline')} +
`, @@ -20,11 +23,20 @@ export const CommentsView = Marionette.ItemView.extend({ }, events: { - 'click a.back': 'goBack' + 'click a.back': 'goBack', + ...cssModulesUtils.events(styles, { + 'click newThreadButton': 'startNewThread' + }) + }, + + initialize: function() { + this.listenTo(this.options.entry, + 'change:selectedContentElementCommentsId', + this._updateNewThreadButton); }, onRender: function() { - const {entry, defaultTab} = this.options; + const {entry, defaultTab, editor: editorApi} = this.options; const tabsView = new TabsView({ i18n: 'pageflow_scrolled.editor.comments_view.tabs', @@ -32,14 +44,34 @@ export const CommentsView = Marionette.ItemView.extend({ }); tabsView.tab('comments', () => - new EntryCommentsView({entry, editor: this.options.editor})); + new EntryCommentsView({entry, editor: editorApi})); tabsView.tab('selection', () => - new ContentElementCommentsView({entry, editor: this.options.editor})); + new ContentElementCommentsView({entry, editor: editorApi})); this.appendSubview(tabsView, {to: this.ui.tabs}); + this._updateNewThreadButton(); + }, + + startNewThread: function() { + const {entry} = this.options; + const id = entry.get('selectedContentElementCommentsId'); + if (!id) return; + + const contentElement = entry.contentElements.get(id); + entry.trigger('selectNewThread', { + id: contentElement.get('permaId'), + subjectType: 'ContentElement', + range: contentElement.transientState.get('newCommentThreadSubjectRange') + }); }, goBack: function() { editor.navigate('/', {trigger: true}); + }, + + _updateNewThreadButton: function() { + const enabled = !!this.options.entry.get('selectedContentElementCommentsId'); + this.$(cssModulesUtils.selector(styles, 'newThreadButton')) + .prop('disabled', !enabled); } }); diff --git a/entry_types/scrolled/package/src/editor/views/CommentsView.module.css b/entry_types/scrolled/package/src/editor/views/CommentsView.module.css new file mode 100644 index 0000000000..68f370eb21 --- /dev/null +++ b/entry_types/scrolled/package/src/editor/views/CommentsView.module.css @@ -0,0 +1,25 @@ +/* + * Pin the tab bar to the top of the sidebar's scrolling area so it + * stays visible while the active tab's thread list scrolls past + * underneath. The outer sidebar scroller does the scrolling. The + * negative top offset cancels the scroller's 10px padding so the bar + * meets the sidebar edge cleanly. The box-shadow extends the surface + * color sideways to hide highlighted threads' accent halo as they + * scroll past underneath. + */ +.root > div { + margin: space(2.5) 0; +} + +.root :global(.tabs_view-scroller) { + position: sticky; + top: -10px; + z-index: 1; + background-color: var(--ui-surface-color); + box-shadow: 0 0 0 space(1) var(--ui-surface-color); +} + +.newThreadButton { + composes: secondaryAddButton from './buttons.module.css'; + float: right; +} diff --git a/entry_types/scrolled/package/src/editor/views/ContentElementCommentsView.js b/entry_types/scrolled/package/src/editor/views/ContentElementCommentsView.js index 17b82b4cbd..497a461a8e 100644 --- a/entry_types/scrolled/package/src/editor/views/ContentElementCommentsView.js +++ b/entry_types/scrolled/package/src/editor/views/ContentElementCommentsView.js @@ -18,13 +18,15 @@ export const ContentElementCommentsView = ReviewView.extend({ }, props() { + const {entry, editor} = this.options; const contentElement = this._contentElement; + const typeName = contentElement?.get('typeName'); return { contentElement, threadIds: contentElement?.transientState.get('commentThreadIdsAtSelection'), - highlightedThreadId: this.options.entry.get('highlightedThreadId'), - onThreadClick: thread => - this.options.entry.trigger('selectCommentThread', thread.id) + compareRanges: typeName && editor.contentElementTypes.findCompareRanges(typeName), + highlightedThreadId: entry.get('highlightedThreadId'), + onThreadClick: thread => entry.trigger('selectCommentThread', thread.id) }; }, @@ -33,13 +35,15 @@ export const ContentElementCommentsView = ReviewView.extend({ // there is no anchor for an individual thread in the iframe, so // highlighting one in the list would have no counterpart in the // preview. - renderContent({contentElement, threadIds, highlightedThreadId, onThreadClick}) { + renderContent({contentElement, threadIds, compareRanges, highlightedThreadId, onThreadClick}) { if (!contentElement) return null; if (threadIds === undefined) { return ( ); } @@ -48,8 +52,10 @@ export const ContentElementCommentsView = ReviewView.extend({ threadIds.includes(thread.id)} + compareRanges={compareRanges} highlightedThreadId={highlightedThreadId} onThreadClick={onThreadClick} + showNewForm={false} hideNewTopicButton /> ); }, diff --git a/entry_types/scrolled/package/src/editor/views/EntryCommentsView.js b/entry_types/scrolled/package/src/editor/views/EntryCommentsView.js index 85e3ba2bf4..795a68d266 100644 --- a/entry_types/scrolled/package/src/editor/views/EntryCommentsView.js +++ b/entry_types/scrolled/package/src/editor/views/EntryCommentsView.js @@ -9,33 +9,67 @@ import styles from './EntryCommentsView.module.css'; export const EntryCommentsView = ReviewView.extend({ initialize() { - this.listenTo(this.options.entry, + const {entry} = this.options; + + this.listenTo(entry, 'change:highlightedThreadId', () => this.rerender()); + this.listenTo(entry, + 'change:selectedContentElementCommentsId', + this._onSelectedChange); + this._observeSelectedElement(); }, props() { const {entry, editor} = this.options; return { items: collectContentElements(entry), + selectedElement: this._selectedElement, + // Undefined for elements without a slate cursor (e.g. images); + // an array (possibly empty) for textBlocks where Selection.js + // has reported the cursor's overlapping threads. + transientThreadIds: + this._selectedElement?.transientState.get('commentThreadIdsAtSelection'), highlightedThreadId: entry.get('highlightedThreadId'), onThreadClick: thread => entry.trigger('selectCommentThread', thread.id), editor }; }, - renderContent({items, highlightedThreadId, onThreadClick, editor}) { + renderContent({items, selectedElement, transientThreadIds, highlightedThreadId, onThreadClick, editor}) { return (
{items.map(({contentElement}) => ( ))}
); + }, + + _onSelectedChange() { + this._observeSelectedElement(); + this.rerender(); + }, + + _observeSelectedElement() { + if (this._selectedElement) { + this.stopListening(this._selectedElement.transientState); + } + + const id = this.options.entry.get('selectedContentElementCommentsId'); + this._selectedElement = id ? this.options.entry.contentElements.get(id) : null; + + if (this._selectedElement) { + this.listenTo(this._selectedElement.transientState, + 'change:commentThreadIdsAtSelection', + () => this.rerender()); + } } }); @@ -53,7 +87,10 @@ function collectContentElements(entry) { return items; } -function ContentElementGroup({contentElement, highlightedThreadId, onThreadClick, editor}) { +function ContentElementGroup({ + contentElement, isSelected, selectedHasTransientThreadIds, + highlightedThreadId, onThreadClick, editor +}) { const permaId = contentElement.get('permaId'); const threads = useCommentThreads({ subjectType: 'ContentElement', @@ -67,14 +104,25 @@ function ContentElementGroup({contentElement, highlightedThreadId, onThreadClick const typeName = contentElement.get('typeName'); const label = I18n.t(`pageflow_scrolled.editor.content_elements.${typeName}.name`); const pictogram = editor.contentElementTypes.findPictogram(typeName) || defaultPictogram; + const compareRanges = editor.contentElementTypes.findCompareRanges(typeName); + + // Element-level badges (e.g. on images) have no per-thread anchor in + // the iframe, so clicking such a badge highlights every thread of + // the element rather than just one. + const groupHighlight = isSelected && !selectedHasTransientThreadIds ? + threads.map(t => t.id) : + highlightedThreadId; return (
); diff --git a/entry_types/scrolled/package/src/editor/views/NewThreadView.js b/entry_types/scrolled/package/src/editor/views/NewThreadView.js index 2282723814..3b79ef7897 100644 --- a/entry_types/scrolled/package/src/editor/views/NewThreadView.js +++ b/entry_types/scrolled/package/src/editor/views/NewThreadView.js @@ -1,14 +1,53 @@ import React from 'react'; +import I18n from 'i18n-js'; +import Marionette from 'backbone.marionette'; +import {editor} from 'pageflow/editor'; +import {TabsView} from 'pageflow/ui'; import {NewThreadForm} from 'pageflow-scrolled/review'; import {ReviewView} from './ReviewView'; -export const NewThreadView = ReviewView.extend({ - renderContent() { - const entry = this.options.entry; - const {subjectType, subjectId, subjectRange} = this.options; +import styles from './NewThreadView.module.css'; + +export const NewThreadView = Marionette.ItemView.extend({ + className: `new_thread_view ${styles.root}`, + + template: () => ` + ${I18n.t('pageflow_scrolled.editor.new_thread_view.back')} +
+ `, + + ui: { + tabs: '.tabs' + }, + + events: { + 'click a.back': 'goBack' + }, + + onRender: function() { + const {entry, subjectType, subjectId, subjectRange} = this.options; + + const tabsView = new TabsView({ + i18n: 'pageflow_scrolled.editor.new_thread_view.tabs' + }); + tabsView.tab('newComment', () => new NewThreadFormView({ + entry, subjectType, subjectId, subjectRange + })); + + this.appendSubview(tabsView, {to: this.ui.tabs}); + }, + + goBack: function() { + editor.navigate('/scrolled/comments?tab=selection', {trigger: true}); + } +}); + +const NewThreadFormView = ReviewView.extend({ + renderContent() { + const {entry, subjectType, subjectId, subjectRange} = this.options; const leave = () => entry.trigger('resetSelection'); return ( diff --git a/entry_types/scrolled/package/src/editor/views/NewThreadView.module.css b/entry_types/scrolled/package/src/editor/views/NewThreadView.module.css new file mode 100644 index 0000000000..00a1b52ddc --- /dev/null +++ b/entry_types/scrolled/package/src/editor/views/NewThreadView.module.css @@ -0,0 +1,3 @@ +.root > div { + margin: space(2.5) 0; +} diff --git a/entry_types/scrolled/package/src/editor/views/ReviewView.module.css b/entry_types/scrolled/package/src/editor/views/ReviewView.module.css index 10eb4f2d9f..8c51077e3e 100644 --- a/entry_types/scrolled/package/src/editor/views/ReviewView.module.css +++ b/entry_types/scrolled/package/src/editor/views/ReviewView.module.css @@ -3,4 +3,9 @@ padding-top: space(4); --review-thread-box-shadow: none; --review-thread-border: 1px solid var(--ui-on-surface-color-lightest); + --review-resolved-threads-pill-align: flex-end; + --review-resolved-threads-pill-color: var(--ui-on-surface-color); + --review-resolved-threads-pill-background-color: transparent; + --review-resolved-thread-opacity: 0.6; + --review-resolved-thread-background: var(--ui-on-surface-color-lightest); } diff --git a/entry_types/scrolled/package/src/editor/views/buttons.module.css b/entry_types/scrolled/package/src/editor/views/buttons.module.css index f94e067ff7..2a995ace47 100644 --- a/entry_types/scrolled/package/src/editor/views/buttons.module.css +++ b/entry_types/scrolled/package/src/editor/views/buttons.module.css @@ -18,6 +18,11 @@ composes: plusCircled from './icons.module.css'; } +.secondaryAddButton { + composes: secondaryIconButton; + composes: plus from './icons.module.css'; +} + .cancelButton { composes: secondaryIconButton; composes: cancel from './icons.module.css'; diff --git a/entry_types/scrolled/package/src/editor/views/icons.module.css b/entry_types/scrolled/package/src/editor/views/icons.module.css index a09491e27c..8f068895f3 100644 --- a/entry_types/scrolled/package/src/editor/views/icons.module.css +++ b/entry_types/scrolled/package/src/editor/views/icons.module.css @@ -10,6 +10,7 @@ .check, .drag, .helpCircled, +.plus, .plusCircled, .rightBold, .rightOpen, @@ -60,6 +61,10 @@ content: "\e704"; } +.plus::before { + content: "\2b"; +} + .plusCircled::before { content: "\2795"; } diff --git a/entry_types/scrolled/package/src/frontend/inlineEditing/ContentElementDecorator.js b/entry_types/scrolled/package/src/frontend/inlineEditing/ContentElementDecorator.js index f20e290c4b..b7eebc7e05 100644 --- a/entry_types/scrolled/package/src/frontend/inlineEditing/ContentElementDecorator.js +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/ContentElementDecorator.js @@ -22,7 +22,7 @@ import {ContentElementEditorStateProvider} from './ContentElementEditorStateProv export function ContentElementDecorator(props) { return (
- + {renderMarginIndicators(props)} @@ -50,8 +50,9 @@ function OptionalSelectionRect(props) { } function DefaultSelectionRect(props) { - const {isSelected, type, select, selectComments} = useContentElementEditorState(); - const commentsSelected = type === 'contentElementComments'; + const {isSelected, type, select, selectComments, selectNewThread} = + useContentElementEditorState(); + const commentsSelected = type === 'contentElementComments' || type === 'newThread'; const {t} = useI18n({locale: 'ui'}); const [, drag, preview] = useDrag({ @@ -70,7 +71,9 @@ function DefaultSelectionRect(props) { selectComments()} + onClick={threads => threads.length === 0 ? + selectNewThread() : + selectComments()} onSelectThread={threadId => selectComments({ type: 'contentElementComments', id: props.id, diff --git a/entry_types/scrolled/package/src/frontend/inlineEditing/ContentElementEditorStateProvider.js b/entry_types/scrolled/package/src/frontend/inlineEditing/ContentElementEditorStateProvider.js index 5d07cbf385..a235c57cec 100644 --- a/entry_types/scrolled/package/src/frontend/inlineEditing/ContentElementEditorStateProvider.js +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/ContentElementEditorStateProvider.js @@ -5,19 +5,23 @@ import {useEditorSelection} from './EditorState'; import {postUpdateTransientContentElementStateMessage} from './postMessage'; import {useStorylineActivity} from '../storylineActivity'; -export function ContentElementEditorStateProvider({id, children}) { +export function ContentElementEditorStateProvider({id, permaId, children}) { const {isSelected, select, range} = useEditorSelection( useMemo(() => ({id, type: 'contentElement'}), [id]) ); const {isSelected: commentsSelected, select: selectComments} = useEditorSelection( useMemo(() => ({id, type: 'contentElementComments'}), [id]) ); + const {isSelected: newThreadSelected, select: selectNewThread} = useEditorSelection( + useMemo(() => ({id: permaId, type: 'newThread'}), [permaId]) + ); const storylineMode = useStorylineActivity(); const inForeground = storylineMode === 'active'; const type = inForeground ? (isSelected ? 'contentElement' : commentsSelected ? 'contentElementComments' : + newThreadSelected ? 'newThread' : null) : null; @@ -33,11 +37,12 @@ export function ContentElementEditorStateProvider({id, children}) { isEditable: true, select, selectComments, + selectNewThread, isSelected: !!type, type, range, setTransientState - }), [select, selectComments, type, range, setTransientState]); + }), [select, selectComments, selectNewThread, type, range, setTransientState]); return ( @@ -45,6 +50,7 @@ export function ContentElementEditorStateProvider({id, children}) { ) } + function shallowEqual(obj1, obj2) { return Object.keys(obj1).length === Object.keys(obj2).length && Object.keys(obj1).every(key => diff --git a/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/Selection.js b/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/Selection.js index 744d4e48d8..8cd373ab65 100644 --- a/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/Selection.js +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/Selection.js @@ -17,6 +17,7 @@ import {getUniformSelectedNode} from './getUniformSelectedNode'; import {toggleBlock, isBlockActive} from './blocks'; import {commentThreadIdsAtSelection} from './commentThreadIdsAtSelection'; import {computeBounds} from './computeBounds'; +import {newCommentThreadSubjectRange} from './newCommentThreadSubjectRange'; import TextIcon from '../images/text.svg'; import HeadingIcon from '../images/heading.svg'; @@ -139,6 +140,7 @@ export function Selection(props) { commentThreadIdsAtSelection: commentThreadIdsAtSelection( props.highlights || [], editor.selection ), + newCommentThreadSubjectRange: newCommentThreadSubjectRange(editor), }); boundsRef.current = {start, end}; diff --git a/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/newCommentThreadSubjectRange.js b/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/newCommentThreadSubjectRange.js new file mode 100644 index 0000000000..61323b6945 --- /dev/null +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/newCommentThreadSubjectRange.js @@ -0,0 +1,19 @@ +import {Editor, Range} from 'slate'; + +// Range to seed a new comment thread on the current slate selection. +// When the selection is collapsed (just a cursor), expand to the full +// surrounding top-level block so the new thread anchors to the whole +// paragraph rather than a zero-width point. +export function newCommentThreadSubjectRange(editor) { + if (!editor.selection) return undefined; + + if (Range.isCollapsed(editor.selection)) { + const blockIdx = editor.selection.anchor.path[0]; + return { + anchor: Editor.point(editor, [blockIdx], {edge: 'start'}), + focus: Editor.point(editor, [blockIdx], {edge: 'end'}) + }; + } + + return editor.selection; +} diff --git a/entry_types/scrolled/package/src/frontend/useContentElementEditorState.js b/entry_types/scrolled/package/src/frontend/useContentElementEditorState.js index b7d80f81bd..704a7a2903 100644 --- a/entry_types/scrolled/package/src/frontend/useContentElementEditorState.js +++ b/entry_types/scrolled/package/src/frontend/useContentElementEditorState.js @@ -4,7 +4,8 @@ export const ContentElementEditorStateContext = createContext({ isSelected: false, isEditable: false, setTransientState() {}, - select() {} + select() {}, + selectNewThread() {} }); /** diff --git a/entry_types/scrolled/package/src/review/Thread.js b/entry_types/scrolled/package/src/review/Thread.js index 708c601c36..c06bd5a68c 100644 --- a/entry_types/scrolled/package/src/review/Thread.js +++ b/entry_types/scrolled/package/src/review/Thread.js @@ -1,4 +1,4 @@ -import React from 'react'; +import React, {useEffect, useRef} from 'react'; import classNames from 'classnames'; import {useI18n} from '../frontend/i18n'; @@ -11,16 +11,26 @@ import ResolveIcon from './images/resolve.svg'; import UnresolveIcon from './images/unresolve.svg'; import styles from './Thread.module.css'; -export function Thread({thread, collapsed, onToggle, onResolve, onClick, highlighted}) { +export function Thread({thread, collapsed, onToggle, onResolve, onClick, highlighted, interactive = true}) { const {t} = useI18n({locale: 'ui'}); const firstComment = thread.comments[0]; const replies = thread.comments.slice(1); const repliesCollapsed = collapsed && replies.length > 0; + const ref = useRef(); + + useEffect(() => { + if (highlighted && ref.current) { + ref.current.scrollIntoView({block: 'nearest', behavior: 'smooth'}); + } + }, [highlighted]); + return ( -
@@ -43,10 +53,10 @@ export function Thread({thread, collapsed, onToggle, onResolve, onClick, highlig ))} - {!thread.resolvedAt && !repliesCollapsed && + {interactive && !thread.resolvedAt && !repliesCollapsed && } - {onResolve && !repliesCollapsed && + {interactive && onResolve && !repliesCollapsed &&
); } + +function sortByRange(threads, compareRanges) { + if (!compareRanges) return threads; + return [...threads].sort((a, b) => compareRanges(a.subjectRange, b.subjectRange)); +} diff --git a/entry_types/scrolled/package/src/review/ThreadList.module.css b/entry_types/scrolled/package/src/review/ThreadList.module.css index 3ab6d15f3c..68285827b7 100644 --- a/entry_types/scrolled/package/src/review/ThreadList.module.css +++ b/entry_types/scrolled/package/src/review/ThreadList.module.css @@ -33,7 +33,7 @@ } .resolvedPill { - align-self: center; + align-self: var(--review-resolved-threads-pill-align, center); display: flex; align-items: center; gap: space(1); @@ -41,8 +41,8 @@ font-size: space(3); font-weight: 500; white-space: nowrap; - color: var(--ui-on-primary-color); - background: var(--ui-primary-color); + color: var(--review-resolved-threads-pill-color, var(--ui-on-primary-color)); + background: var(--review-resolved-threads-pill-background-color, var(--ui-primary-color)); border: 0; border-radius: rounded(full); padding: space(1) space(1.5) space(1) space(2.5); @@ -56,3 +56,10 @@ .chevronExpanded { transform: rotate(180deg); } + +.blankSlate { + margin: 0; + padding: space(4) space(2); + text-align: center; + color: var(--ui-on-surface-color-light); +} diff --git a/entry_types/scrolled/package/src/review/ThreadsBadge.js b/entry_types/scrolled/package/src/review/ThreadsBadge.js index 8ece23ecad..2b7f255adb 100644 --- a/entry_types/scrolled/package/src/review/ThreadsBadge.js +++ b/entry_types/scrolled/package/src/review/ThreadsBadge.js @@ -8,13 +8,17 @@ export function ThreadsBadge({subjectType, subjectId, subjectRange, onClick, onS const threads = useCommentThreads({subjectType, subjectId, subjectRange}, {resolved: false}); const ref = useRef(); + const handleClick = useCallback(() => { + if (onClick) onClick(threads); + }, [onClick, threads]); + return ( <> {threads.length > 0 && } - + ); }