diff --git a/entry_types/scrolled/config/locales/de.yml b/entry_types/scrolled/config/locales/de.yml index 5b425aead9..6b6c0c47d5 100644 --- a/entry_types/scrolled/config/locales/de.yml +++ b/entry_types/scrolled/config/locales/de.yml @@ -1584,6 +1584,12 @@ de: linkButtonVariant: label: Button-Variante blank: "(Standard)" + main_menu: + comments: Kommentare + comments_view: + tabs: + comments: Alle Kommentare + selection: Für Auswahl help_entries: content_elements: menu_item: Inhaltselemente diff --git a/entry_types/scrolled/config/locales/en.yml b/entry_types/scrolled/config/locales/en.yml index 5265eff0a1..92439f3d7c 100644 --- a/entry_types/scrolled/config/locales/en.yml +++ b/entry_types/scrolled/config/locales/en.yml @@ -1567,6 +1567,12 @@ en: linkButtonVariant: label: Button Variante blank: "(Default)" + main_menu: + comments: Comments + comments_view: + tabs: + comments: All comments + selection: For selection help_entries: content_elements: menu_item: Content Elements 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 15d55ab30a..eb9ec16c5d 100644 --- a/entry_types/scrolled/package/spec/editor/controllers/PreviewMessageController-spec.js +++ b/entry_types/scrolled/package/spec/editor/controllers/PreviewMessageController-spec.js @@ -1,4 +1,5 @@ import 'editor/config'; +import Backbone from 'backbone'; import {editor} from 'pageflow-scrolled/editor'; import {features} from 'pageflow/frontend'; import {ScrolledEntry} from 'editor/models/ScrolledEntry'; @@ -158,6 +159,24 @@ describe('PreviewMessageController', () => { })).resolves.toMatchObject({type: 'SELECT', payload: {id: 1, type: 'contentElement'}}); }); + it('posts SELECT_COMMENT_THREAD message on selectCommentThread event', async () => { + const entry = factories.entry(ScrolledEntry, {}, {entryTypeSeed: normalizeSeed()}); + 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_COMMENT_THREAD') resolve(event.data); + }); + entry.trigger('selectCommentThread', 7); + })).resolves.toMatchObject({ + type: 'SELECT_COMMENT_THREAD', + payload: {threadId: 7} + }); + }); + it('passes on range from selectContentElement event', async () => { const entry = factories.entry(ScrolledEntry, {}, { entryTypeSeed: normalizeSeed({ @@ -495,7 +514,7 @@ describe('PreviewMessageController', () => { })).resolves.toBe('/'); }); - it('navigates to comments route on SELECTED message for contentElementComments', () => { + it('navigates to comments route with tab=selection on SELECTED contentElementComments', () => { const editor = factories.editorApi(); const entry = factories.entry(ScrolledEntry, {}, { entryTypeSeed: normalizeSeed({ @@ -508,10 +527,12 @@ describe('PreviewMessageController', () => { return expect(new Promise(resolve => { editor.on('navigate', resolve); window.postMessage({type: 'SELECTED', payload: {id: 1, type: 'contentElementComments'}}, '*'); - })).resolves.toBe('/scrolled/content_elements/1/comments'); + })).resolves.toBe('/scrolled/comments?tab=selection'); }); - it('navigates to comments route with threadIds payload on SELECTED contentElementComments', () => { + it('does not navigate on SELECTED contentElementComments while on the comments route', async () => { + Backbone.history.fragment = 'scrolled/comments'; + const editor = factories.editorApi(); const entry = factories.entry(ScrolledEntry, {}, { entryTypeSeed: normalizeSeed({contentElements: [{id: 1}]}) @@ -519,17 +540,150 @@ describe('PreviewMessageController', () => { const iframeWindow = createIframeWindow(); controller = new PreviewMessageController({entry, iframeWindow, editor}); - const expectedPayload = encodeURIComponent(JSON.stringify({threadIds: [3, 7]})); + const navigate = jest.fn(); + editor.on('navigate', navigate); + + await new Promise(resolve => { + entry.once('change:highlightedThreadId', resolve); + window.postMessage({ + type: 'SELECTED', + payload: {id: 10, type: 'contentElementComments', highlightedThreadId: 7} + }, '*'); + }); + + expect(navigate).not.toHaveBeenCalled(); + expect(entry.get('highlightedThreadId')).toBe(7); + + Backbone.history.fragment = undefined; + }); + + it('does not navigate on SELECTED contentElementComments while on the comments selection tab', async () => { + Backbone.history.fragment = 'scrolled/comments?tab=selection'; + + const editor = factories.editorApi(); + const entry = factories.entry(ScrolledEntry, {}, { + entryTypeSeed: normalizeSeed({contentElements: [{id: 1}]}) + }); + const iframeWindow = createIframeWindow(); + controller = new PreviewMessageController({entry, iframeWindow, editor}); + + const navigate = jest.fn(); + editor.on('navigate', navigate); + + await new Promise(resolve => { + entry.once('change:highlightedThreadId', resolve); + window.postMessage({ + type: 'SELECTED', + payload: {id: 10, type: 'contentElementComments', highlightedThreadId: 7} + }, '*'); + }); + + expect(navigate).not.toHaveBeenCalled(); + + Backbone.history.fragment = undefined; + }); + + it('sets highlightedThreadId on entry on SELECTED contentElementComments', () => { + const editor = factories.editorApi(); + const entry = factories.entry(ScrolledEntry, {}, {entryTypeSeed: normalizeSeed()}); + const iframeWindow = createIframeWindow(); + controller = new PreviewMessageController({entry, iframeWindow, editor}); return expect(new Promise(resolve => { - editor.on('navigate', resolve); + entry.once('change:highlightedThreadId', (model, value) => resolve(value)); window.postMessage({ type: 'SELECTED', - payload: {id: 10, type: 'contentElementComments', threadIds: [3, 7]} + payload: {id: 10, type: 'contentElementComments', highlightedThreadId: 5} }, '*'); - })).resolves.toBe( - `/scrolled/content_elements/10/comments?payload=${expectedPayload}` - ); + })).resolves.toBe(5); + }); + + it('clears highlightedThreadId on entry on SELECTED with non-comments type', () => { + const editor = factories.editorApi(); + const entry = factories.entry(ScrolledEntry, {}, {entryTypeSeed: normalizeSeed()}); + entry.set('highlightedThreadId', 5); + + const iframeWindow = createIframeWindow(); + controller = new PreviewMessageController({entry, iframeWindow, editor}); + + return expect(new Promise(resolve => { + entry.once('change:highlightedThreadId', (model, value) => resolve(value)); + window.postMessage({ + type: 'SELECTED', + payload: {id: 10, type: 'sectionSettings'} + }, '*'); + })).resolves.toBeUndefined(); + }); + + it('sets selectedContentElementCommentsId on SELECTED contentElementComments', () => { + const editor = factories.editorApi(); + const entry = factories.entry(ScrolledEntry, {}, {entryTypeSeed: normalizeSeed()}); + const iframeWindow = createIframeWindow(); + controller = new PreviewMessageController({entry, iframeWindow, editor}); + + return expect(new Promise(resolve => { + entry.once('change:selectedContentElementCommentsId', (model, value) => resolve(value)); + window.postMessage({ + type: 'SELECTED', + payload: {id: 7, type: 'contentElementComments'} + }, '*'); + })).resolves.toBe(7); + }); + + it('sets selectedContentElementCommentsId on SELECTED contentElement', () => { + const editor = factories.editorApi(); + const entry = factories.entry(ScrolledEntry, {}, { + entryTypeSeed: normalizeSeed({contentElements: [{id: 4}]}) + }); + const iframeWindow = createIframeWindow(); + controller = new PreviewMessageController({entry, iframeWindow, editor}); + + return expect(new Promise(resolve => { + entry.once('change:selectedContentElementCommentsId', (model, value) => resolve(value)); + window.postMessage({ + type: 'SELECTED', + payload: {id: 4, type: 'contentElement'} + }, '*'); + })).resolves.toBe(4); + }); + + it('sets selectedContentElementCommentsId from permaId on SELECTED newThread', () => { + const editor = factories.editorApi(); + const entry = factories.entry(ScrolledEntry, {}, { + entryTypeSeed: normalizeSeed({contentElements: [{id: 4, permaId: 100}]}) + }); + const iframeWindow = createIframeWindow(); + controller = new PreviewMessageController({entry, iframeWindow, editor}); + + return expect(new Promise(resolve => { + entry.once('change:selectedContentElementCommentsId', (model, value) => resolve(value)); + window.postMessage({ + type: 'SELECTED', + payload: { + id: 100, + type: 'newThread', + subjectType: 'ContentElement', + range: {anchor: {path: [0, 0], offset: 0}, focus: {path: [0, 0], offset: 1}} + } + }, '*'); + })).resolves.toBe(4); + }); + + it('clears selectedContentElementCommentsId on SELECTED with non-content-element type', () => { + const editor = factories.editorApi(); + const entry = factories.entry(ScrolledEntry, {}, {entryTypeSeed: normalizeSeed()}); + entry.set('selectedContentElementCommentsId', 9); + + const iframeWindow = createIframeWindow(); + controller = new PreviewMessageController({entry, iframeWindow, editor}); + + return expect(new Promise(resolve => { + entry.once('change:selectedContentElementCommentsId', (model, value) => resolve(value)); + window.postMessage({ + type: 'SELECTED', + payload: {id: 10, type: 'sectionSettings'} + }, '*'); + })).resolves.toBeUndefined(); }); it('navigates to new thread route with encoded payload on SELECTED for newThread', () => { diff --git a/entry_types/scrolled/package/spec/editor/controllers/SideBarController-spec.js b/entry_types/scrolled/package/spec/editor/controllers/SideBarController-spec.js index 1a11f02604..b2a437ef85 100644 --- a/entry_types/scrolled/package/spec/editor/controllers/SideBarController-spec.js +++ b/entry_types/scrolled/package/spec/editor/controllers/SideBarController-spec.js @@ -1,7 +1,7 @@ import 'editor/config'; import {SideBarController} from 'editor/controllers/SideBarController'; -import {ContentElementCommentsView} from 'editor/views/ContentElementCommentsView'; +import {CommentsView} from 'editor/views/CommentsView'; import {factories} from 'pageflow/testHelpers'; import {useEditorGlobals} from 'support'; @@ -9,39 +9,31 @@ import {useEditorGlobals} from 'support'; describe('SideBarController', () => { const {createEntry} = useEditorGlobals(); - describe('#contentElementComments', () => { - it('shows a ContentElementCommentsView without threadIds when no payload given', () => { - const entry = createEntry({ - contentElements: [{id: 1, permaId: 10, typeName: 'textBlock'}] - }); + describe('#comments', () => { + it('shows a CommentsView in the region', () => { + const entry = createEntry({}); entry.reviewSession = factories.reviewSession(); const region = {show: jest.fn()}; const controller = new SideBarController({region, entry}); - controller.contentElementComments(1); + controller.comments(); const shown = region.show.mock.calls[0][0]; - expect(shown).toBeInstanceOf(ContentElementCommentsView); - expect(shown.options.threadIds).toBeUndefined(); + expect(shown).toBeInstanceOf(CommentsView); }); - it('decodes threadIds from the payload and passes it to the view', () => { - const entry = createEntry({ - contentElements: [{id: 1, permaId: 10, typeName: 'textBlock'}] - }); + it('passes the tab arg as defaultTab to the CommentsView', () => { + const entry = createEntry({}); entry.reviewSession = factories.reviewSession(); const region = {show: jest.fn()}; const controller = new SideBarController({region, entry}); - const payload = encodeURIComponent(JSON.stringify({threadIds: [3, 7]})); - - controller.contentElementComments(1, payload); + controller.comments('selection'); const shown = region.show.mock.calls[0][0]; - expect(shown).toBeInstanceOf(ContentElementCommentsView); - expect(shown.options.threadIds).toEqual([3, 7]); + expect(shown.options.defaultTab).toBe('selection'); }); }); }); diff --git a/entry_types/scrolled/package/spec/editor/views/CommentsView-spec.js b/entry_types/scrolled/package/spec/editor/views/CommentsView-spec.js new file mode 100644 index 0000000000..f86a2fc49a --- /dev/null +++ b/entry_types/scrolled/package/spec/editor/views/CommentsView-spec.js @@ -0,0 +1,111 @@ +import '@testing-library/jest-dom/extend-expect'; + +import {editor} from 'pageflow-scrolled/editor'; + +import {CommentsView} from 'editor/views/CommentsView'; + +import {factories, useFakeTranslations, renderBackboneView} from 'pageflow/testHelpers'; +import {useEditorGlobals} from 'support'; +import {fireEvent} from '@testing-library/dom'; +import {act} from '@testing-library/react'; + +describe('CommentsView', () => { + const {createEntry} = useEditorGlobals(); + + useFakeTranslations({ + 'pageflow_scrolled.review.new_topic': 'New topic', + 'pageflow_scrolled.review.add_comment_placeholder': 'Add a comment...', + 'pageflow_scrolled.review.send': 'Send', + '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.editor.templates.back_button_decorator.outline': 'Outline' + }); + + function setupEntry() { + const entry = createEntry({ + contentElements: [{id: 1, permaId: 10, typeName: 'textBlock'}] + }); + entry.set('selectedContentElementCommentsId', 1); + entry.reviewSession = factories.reviewSession({ + commentThreads: [{ + id: 1, + subjectType: 'ContentElement', + subjectId: 10, + comments: [{id: 100, body: 'A thread', creatorName: 'Alice'}] + }] + }); + return entry; + } + + it('renders both tab labels', () => { + const entry = setupEntry(); + + const view = new CommentsView({entry, editor}); + const {getAllByRole} = renderBackboneView(view); + + const labels = getAllByRole('tab').map(t => t.textContent); + expect(labels).toEqual(['All comments', 'For selection']); + }); + + it('shows the all-comments tab by default', () => { + const entry = setupEntry(); + + const view = new CommentsView({entry, editor}); + const {getByText} = renderBackboneView(view); + + expect(getByText('Text')).toBeInTheDocument(); + expect(getByText('A thread')).toBeInTheDocument(); + }); + + it('uses defaultTab option to pick the initial tab', () => { + const entry = setupEntry(); + + const view = new CommentsView({entry, editor, defaultTab: 'selection'}); + const {queryByText, getByText} = renderBackboneView(view); + + expect(getByText('A thread')).toBeInTheDocument(); + expect(queryByText('Text')).not.toBeInTheDocument(); + }); + + it('switches to the selection tab on click', () => { + const entry = setupEntry(); + + const view = new CommentsView({entry, editor}); + const {getByRole, queryByText, getByText} = renderBackboneView(view); + + expect(getByText('Text')).toBeInTheDocument(); + + act(() => { + fireEvent.click(getByRole('tab', {name: 'For selection'})); + }); + + expect(queryByText('Text')).not.toBeInTheDocument(); + expect(getByText('A thread')).toBeInTheDocument(); + }); + + it('renders a back link', () => { + const entry = setupEntry(); + + const view = new CommentsView({entry, editor}); + const {getByText} = renderBackboneView(view); + + expect(getByText('Outline')).toBeInTheDocument(); + }); + + it('navigates to root when the back link is clicked', () => { + const entry = setupEntry(); + + const view = new CommentsView({entry, editor}); + const {getByText} = renderBackboneView(view); + + const navigate = jest.spyOn(editor, 'navigate').mockImplementation(() => {}); + + fireEvent.click(getByText('Outline')); + + expect(navigate).toHaveBeenCalledWith('/', {trigger: true}); + + navigate.mockRestore(); + }); + +}); 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 d9f7008f98..ec9efccdcb 100644 --- a/entry_types/scrolled/package/spec/editor/views/ContentElementCommentsView-spec.js +++ b/entry_types/scrolled/package/spec/editor/views/ContentElementCommentsView-spec.js @@ -1,4 +1,5 @@ import '@testing-library/jest-dom/extend-expect'; +import userEvent from '@testing-library/user-event'; import {ContentElementCommentsView} from 'editor/views/ContentElementCommentsView'; @@ -15,10 +16,11 @@ describe('ContentElementCommentsView', () => { 'pageflow_scrolled.review.send': 'Send' }); - it('displays threads from session state', () => { + it('displays threads of selected content element from session state', () => { const entry = createEntry({ contentElements: [{id: 1, permaId: 10, typeName: 'textBlock'}] }); + entry.set('selectedContentElementCommentsId', 1); entry.reviewSession = factories.reviewSession({ commentThreads: [{ id: 1, @@ -28,21 +30,20 @@ describe('ContentElementCommentsView', () => { }] }); - const view = new ContentElementCommentsView({ - entry, - model: entry.contentElements.get(1), - editor: {} - }); + const view = new ContentElementCommentsView({entry, editor: {}}); const {getByText} = renderBackboneView(view); expect(getByText('Looks good')).toBeInTheDocument(); }); - it('filters threads by threadIds when given', () => { + it('filters threads by transient state commentThreadIdsAtSelection', () => { const entry = createEntry({ contentElements: [{id: 1, permaId: 10, typeName: 'textBlock'}] }); + entry.set('selectedContentElementCommentsId', 1); + entry.contentElements.get(1).transientState + .set('commentThreadIdsAtSelection', [1]); entry.reviewSession = factories.reviewSession({ commentThreads: [ {id: 1, subjectType: 'ContentElement', subjectId: 10, @@ -52,12 +53,7 @@ describe('ContentElementCommentsView', () => { ] }); - const view = new ContentElementCommentsView({ - entry, - model: entry.contentElements.get(1), - editor: {}, - threadIds: [1] - }); + const view = new ContentElementCommentsView({entry, editor: {}}); const {getByText, queryByText} = renderBackboneView(view); @@ -65,10 +61,11 @@ describe('ContentElementCommentsView', () => { expect(queryByText('Out of scope')).not.toBeInTheDocument(); }); - it('shows all threads when threadIds is not given', () => { + it('shows all threads when transient state has no commentThreadIdsAtSelection', () => { const entry = createEntry({ contentElements: [{id: 1, permaId: 10, typeName: 'textBlock'}] }); + entry.set('selectedContentElementCommentsId', 1); entry.reviewSession = factories.reviewSession({ commentThreads: [ {id: 1, subjectType: 'ContentElement', subjectId: 10, @@ -78,11 +75,7 @@ describe('ContentElementCommentsView', () => { ] }); - const view = new ContentElementCommentsView({ - entry, - model: entry.contentElements.get(1), - editor: {} - }); + const view = new ContentElementCommentsView({entry, editor: {}}); const {getByText} = renderBackboneView(view); @@ -90,17 +83,201 @@ describe('ContentElementCommentsView', () => { expect(getByText('Second')).toBeInTheDocument(); }); + it('updates filter when transient state changes', async () => { + const entry = createEntry({ + contentElements: [{id: 1, permaId: 10, typeName: 'textBlock'}] + }); + entry.set('selectedContentElementCommentsId', 1); + entry.reviewSession = factories.reviewSession({ + commentThreads: [ + {id: 1, subjectType: 'ContentElement', subjectId: 10, + comments: [{id: 100, body: 'In scope', creatorName: 'Alice'}]}, + {id: 2, subjectType: 'ContentElement', subjectId: 10, + comments: [{id: 200, body: 'Out of scope', creatorName: 'Bob'}]} + ] + }); + + const view = new ContentElementCommentsView({entry, editor: {}}); + const {getByText, queryByText} = renderBackboneView(view); + + expect(getByText('Out of scope')).toBeInTheDocument(); + + act(() => { + entry.contentElements.get(1).transientState + .set('commentThreadIdsAtSelection', [1]); + }); + + await waitFor(() => { + expect(queryByText('Out of scope')).not.toBeInTheDocument(); + }); + expect(getByText('In scope')).toBeInTheDocument(); + }); + + it('updates when selectedContentElementCommentsId changes', async () => { + const entry = createEntry({ + contentElements: [ + {id: 1, permaId: 10, typeName: 'textBlock'}, + {id: 2, permaId: 11, typeName: 'textBlock'} + ] + }); + entry.set('selectedContentElementCommentsId', 1); + entry.reviewSession = factories.reviewSession({ + commentThreads: [ + {id: 1, subjectType: 'ContentElement', subjectId: 10, + comments: [{id: 100, body: 'On first', creatorName: 'Alice'}]}, + {id: 2, subjectType: 'ContentElement', subjectId: 11, + comments: [{id: 200, body: 'On second', creatorName: 'Bob'}]} + ] + }); + + const view = new ContentElementCommentsView({entry, editor: {}}); + const {getByText, queryByText} = renderBackboneView(view); + + expect(getByText('On first')).toBeInTheDocument(); + + act(() => { + entry.set('selectedContentElementCommentsId', 2); + }); + + await waitFor(() => { + expect(getByText('On second')).toBeInTheDocument(); + }); + expect(queryByText('On first')).not.toBeInTheDocument(); + }); + + it('marks thread matching entry.highlightedThreadId with aria-current when scoped', () => { + const entry = createEntry({ + contentElements: [{id: 1, permaId: 10, typeName: 'textBlock'}] + }); + entry.set('selectedContentElementCommentsId', 1); + entry.contentElements.get(1).transientState + .set('commentThreadIdsAtSelection', [1, 2]); + entry.set('highlightedThreadId', 2); + 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'}]} + ] + }); + + const view = new ContentElementCommentsView({entry, editor: {}}); + const {getByText} = renderBackboneView(view); + + expect(getByText('second').closest('[aria-current="true"]')).not.toBeNull(); + expect(getByText('first').closest('[aria-current="true"]')).toBeNull(); + }); + + it('updates highlight when entry.highlightedThreadId changes', () => { + const entry = createEntry({ + contentElements: [{id: 1, permaId: 10, typeName: 'textBlock'}] + }); + entry.set('selectedContentElementCommentsId', 1); + entry.contentElements.get(1).transientState + .set('commentThreadIdsAtSelection', [1, 2]); + 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'}]} + ] + }); + + const view = new ContentElementCommentsView({entry, editor: {}}); + const {getByText} = renderBackboneView(view); + + expect(getByText('first').closest('[aria-current="true"]')).toBeNull(); + + act(() => { entry.set('highlightedThreadId', 1); }); + + expect(getByText('first').closest('[aria-current="true"]')).not.toBeNull(); + }); + + it('triggers selectCommentThread on entry when a thread is clicked while scoped', async () => { + const user = userEvent.setup(); + + const entry = createEntry({ + contentElements: [{id: 1, permaId: 10, typeName: 'textBlock'}] + }); + entry.set('selectedContentElementCommentsId', 1); + entry.contentElements.get(1).transientState + .set('commentThreadIdsAtSelection', [7]); + entry.reviewSession = factories.reviewSession({ + commentThreads: [{ + id: 7, subjectType: 'ContentElement', subjectId: 10, + comments: [{id: 100, body: 'click me', creatorName: 'Alice'}] + }] + }); + + const listener = jest.fn(); + entry.on('selectCommentThread', listener); + + const view = new ContentElementCommentsView({entry, editor: {}}); + const {getByText} = renderBackboneView(view); + + await user.click(getByText('click me')); + + expect(listener).toHaveBeenCalledWith(7); + }); + + it('does not highlight or trigger selectCommentThread when not scoped', async () => { + const user = userEvent.setup(); + + const entry = createEntry({ + contentElements: [{id: 1, permaId: 10, typeName: 'textBlock'}] + }); + entry.set('selectedContentElementCommentsId', 1); + entry.set('highlightedThreadId', 7); + entry.reviewSession = factories.reviewSession({ + commentThreads: [{ + id: 7, subjectType: 'ContentElement', subjectId: 10, + comments: [{id: 100, body: 'click me', creatorName: 'Alice'}] + }] + }); + + const listener = jest.fn(); + entry.on('selectCommentThread', listener); + + const view = new ContentElementCommentsView({entry, editor: {}}); + const {getByText} = renderBackboneView(view); + + expect(getByText('click me').closest('[aria-current="true"]')).toBeNull(); + + await user.click(getByText('click me')); + + expect(listener).not.toHaveBeenCalled(); + }); + + it('does not render a new-topic button', () => { + const entry = createEntry({ + contentElements: [{id: 1, permaId: 10, typeName: 'textBlock'}] + }); + entry.set('selectedContentElementCommentsId', 1); + entry.reviewSession = factories.reviewSession({ + commentThreads: [{ + id: 1, + subjectType: 'ContentElement', + subjectId: 10, + comments: [{id: 100, body: 'A thread', creatorName: 'Alice'}] + }] + }); + + const view = new ContentElementCommentsView({entry, editor: {}}); + const {queryByRole} = renderBackboneView(view); + + expect(queryByRole('button', {name: 'New topic'})).not.toBeInTheDocument(); + }); + it('updates when session emits change:thread', async () => { const entry = createEntry({ contentElements: [{id: 1, permaId: 10, typeName: 'textBlock'}] }); + entry.set('selectedContentElementCommentsId', 1); entry.reviewSession = factories.reviewSession(); - const view = new ContentElementCommentsView({ - entry, - model: entry.contentElements.get(1), - 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 new file mode 100644 index 0000000000..bfe480967d --- /dev/null +++ b/entry_types/scrolled/package/spec/editor/views/EntryCommentsView-spec.js @@ -0,0 +1,231 @@ +import '@testing-library/jest-dom/extend-expect'; +import userEvent from '@testing-library/user-event'; +import {act} from '@testing-library/react'; + +import {editor} from 'pageflow-scrolled/editor'; + +import {EntryCommentsView} from 'editor/views/EntryCommentsView'; +import styles from 'editor/views/EntryCommentsView.module.css'; + +import {factories, useFakeTranslations, renderBackboneView} from 'pageflow/testHelpers'; +import {useEditorGlobals} from 'support'; + +describe('EntryCommentsView', () => { + const {createEntry} = useEditorGlobals(); + + useFakeTranslations({ + 'pageflow_scrolled.review.new_topic': 'New topic', + 'pageflow_scrolled.review.add_comment_placeholder': 'Add a comment...', + 'pageflow_scrolled.review.send': 'Send', + 'pageflow_scrolled.editor.content_elements.textBlock.name': 'Text', + 'pageflow_scrolled.editor.content_elements.image.name': 'Image' + }); + + it('renders a thread group only for content elements that have threads', () => { + const entry = createEntry({ + contentElements: [ + {id: 1, permaId: 10, typeName: 'textBlock', position: 0}, + {id: 2, permaId: 20, typeName: 'image', position: 1} + ] + }); + entry.reviewSession = factories.reviewSession({ + commentThreads: [{ + id: 1, + subjectType: 'ContentElement', + subjectId: 20, + comments: [{id: 100, body: 'Looks good', creatorName: 'Alice'}] + }] + }); + + const view = new EntryCommentsView({entry, editor}); + + const {getByText, queryByText} = renderBackboneView(view); + + expect(getByText('Looks good')).toBeInTheDocument(); + expect(getByText('Image')).toBeInTheDocument(); + expect(queryByText('Text')).not.toBeInTheDocument(); + }); + + it('orders groups by chapter, section and content element position', () => { + const entry = createEntry({ + chapters: [ + {id: 100, permaId: 10, position: 1, storylineId: 1000}, + {id: 200, permaId: 20, position: 0, storylineId: 1000} + ], + sections: [ + {id: 11, permaId: 11, chapterId: 100, position: 0}, + {id: 12, permaId: 12, chapterId: 100, position: 1}, + {id: 21, permaId: 21, chapterId: 200, position: 0} + ], + contentElements: [ + {id: 1, permaId: 1, sectionId: 11, position: 1, typeName: 'textBlock'}, + {id: 2, permaId: 2, sectionId: 11, position: 0, typeName: 'image'}, + {id: 3, permaId: 3, sectionId: 12, position: 0, typeName: 'textBlock'}, + {id: 4, permaId: 4, sectionId: 21, position: 0, typeName: 'image'} + ] + }); + entry.reviewSession = factories.reviewSession({ + commentThreads: [ + {id: 1, subjectType: 'ContentElement', subjectId: 1, + comments: [{id: 1, body: 'fourth', creatorName: 'A'}]}, + {id: 2, subjectType: 'ContentElement', subjectId: 2, + comments: [{id: 2, body: 'third', creatorName: 'B'}]}, + {id: 3, subjectType: 'ContentElement', subjectId: 3, + comments: [{id: 3, body: 'second', creatorName: 'C'}]}, + {id: 4, subjectType: 'ContentElement', subjectId: 4, + comments: [{id: 4, body: 'first', creatorName: 'D'}]} + ] + }); + + const view = new EntryCommentsView({entry, editor}); + + const {getByText} = renderBackboneView(view); + + const order = ['first', 'second', 'third', 'fourth'] + .map(text => getByText(text).getBoundingClientRect().top); + + expect(order).toEqual([...order].sort((a, b) => a - b)); + }); + + it('does not render the new-topic button', () => { + const entry = createEntry({ + contentElements: [{id: 1, permaId: 10, typeName: 'textBlock'}] + }); + entry.reviewSession = factories.reviewSession({ + commentThreads: [{ + id: 1, subjectType: 'ContentElement', subjectId: 10, + comments: [{id: 100, body: 'A comment', creatorName: 'Alice'}] + }] + }); + + const view = new EntryCommentsView({entry, editor}); + const {queryByRole} = renderBackboneView(view); + + expect(queryByRole('button', {name: 'New topic'})).not.toBeInTheDocument(); + }); + + it('triggers selectCommentThread on entry when a thread is clicked', async () => { + const user = userEvent.setup(); + + const entry = createEntry({ + contentElements: [{id: 1, permaId: 10, typeName: 'textBlock'}] + }); + entry.reviewSession = factories.reviewSession({ + commentThreads: [{ + id: 7, subjectType: 'ContentElement', subjectId: 10, + comments: [{id: 100, body: 'A comment', creatorName: 'Alice'}] + }] + }); + + const listener = jest.fn(); + entry.on('selectCommentThread', listener); + + const view = new EntryCommentsView({entry, editor}); + const {getByText} = renderBackboneView(view); + + await user.click(getByText('A comment')); + + expect(listener).toHaveBeenCalledWith(7); + }); + + it('marks the thread matching entry.highlightedThreadId with aria-current', () => { + 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} = renderBackboneView(view); + + expect(getByText('second').closest('[aria-current="true"]')).not.toBeNull(); + expect(getByText('first').closest('[aria-current="true"]')).toBeNull(); + }); + + it('updates highlight when entry.highlightedThreadId changes', () => { + 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'}]} + ] + }); + + const view = new EntryCommentsView({entry, editor}); + const {getByText} = renderBackboneView(view); + + expect(getByText('first').closest('[aria-current="true"]')).toBeNull(); + + act(() => { entry.set('highlightedThreadId', 1); }); + + expect(getByText('first').closest('[aria-current="true"]')).not.toBeNull(); + }); + + it('shows the content element type name as group label', () => { + const entry = createEntry({ + contentElements: [{id: 1, permaId: 10, typeName: 'image'}] + }); + entry.reviewSession = factories.reviewSession({ + commentThreads: [{ + id: 1, subjectType: 'ContentElement', subjectId: 10, + comments: [{id: 100, body: 'A comment', creatorName: 'Alice'}] + }] + }); + + const view = new EntryCommentsView({entry, editor}); + const {getByText} = renderBackboneView(view); + + expect(getByText('Image')).toBeInTheDocument(); + }); + + it('renders the registered pictogram for the content element type', () => { + editor.contentElementTypes.register('image', {pictogram: '/some/image-pictogram.svg'}); + + const entry = createEntry({ + contentElements: [{id: 1, permaId: 10, typeName: 'image'}] + }); + entry.reviewSession = factories.reviewSession({ + commentThreads: [{ + id: 1, subjectType: 'ContentElement', subjectId: 10, + comments: [{id: 100, body: 'A comment', creatorName: 'Alice'}] + }] + }); + + const view = new EntryCommentsView({entry, editor}); + renderBackboneView(view); + + const pictogram = view.el.querySelector(`.${styles.pictogram}`); + expect(pictogram).toBeInTheDocument(); + expect(pictogram.style.maskImage).toContain('/some/image-pictogram.svg'); + }); + + it('falls back to the default pictogram for unknown content element types', () => { + const entry = createEntry({ + contentElements: [{id: 1, permaId: 10, typeName: 'unregistered'}] + }); + entry.reviewSession = factories.reviewSession({ + commentThreads: [{ + id: 1, subjectType: 'ContentElement', subjectId: 10, + comments: [{id: 100, body: 'A comment', creatorName: 'Alice'}] + }] + }); + + const view = new EntryCommentsView({entry, editor}); + renderBackboneView(view); + + const pictogram = view.el.querySelector(`.${styles.pictogram}`); + expect(pictogram).toBeInTheDocument(); + expect(pictogram.style.maskImage).not.toBe(''); + }); +}); diff --git a/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText-spec.js b/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText-spec.js index 20d8951492..31012f5a41 100644 --- a/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText-spec.js +++ b/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText-spec.js @@ -410,83 +410,87 @@ describe('EditableText', () => { expect(badges[2]).toHaveClass(badgeStyles.dot); }); - it('posts SELECTED contentElementComments with overlapping threadIds on badge click', () => { + it('posts SELECTED contentElementComments with highlightedThreadId on badge click', () => { fakeParentWindow(); window.parent.postMessage = jest.fn(); const value = [ - {type: 'paragraph', children: [{text: 'First paragraph'}]}, - {type: 'paragraph', children: [{text: 'Second paragraph'}]} + {type: 'paragraph', children: [{text: 'First paragraph'}]} ]; - const {getAllByRole} = renderWithCommenting( + const {getByRole} = renderWithCommenting( , { wrapper, commentThreads: [ {id: 5, subjectType: 'ContentElement', subjectId: 10, subjectRange: {anchor: {path: [0, 0], offset: 0}, focus: {path: [0, 0], offset: 5}}, - comments: [{id: 1, body: 'p0a', creatorName: 'Alice', creatorId: 1}]}, - {id: 6, subjectType: 'ContentElement', subjectId: 10, - subjectRange: {anchor: {path: [0, 0], offset: 6}, focus: {path: [0, 0], offset: 9}}, - comments: [{id: 2, body: 'p0b', creatorName: 'Bob', creatorId: 2}]}, - {id: 7, subjectType: 'ContentElement', subjectId: 10, - subjectRange: {anchor: {path: [1, 0], offset: 0}, focus: {path: [1, 0], offset: 6}}, - comments: [{id: 3, body: 'p1', creatorName: 'Eve', creatorId: 3}]} + comments: [{id: 1, body: 'a', creatorName: 'Alice', creatorId: 1}]} ] } ); - const badges = getAllByRole('status'); - fireEvent.click(badges[0]); + fireEvent.click(getByRole('status')); expect(window.parent.postMessage).toHaveBeenCalledWith({ type: 'SELECTED', payload: { type: 'contentElementComments', id: 1, - highlightedThreadId: 5, - threadIds: [5, 6] + highlightedThreadId: 5 } }, expect.anything()); }); - it('scopes threadIds to clicked highlight\'s start block, ignoring later blocks it spans', () => { + it('runs badge click logic and scrolls into view on SELECT_COMMENT_THREAD message', async () => { fakeParentWindow(); window.parent.postMessage = jest.fn(); + const scrollIntoView = jest.fn(); + Element.prototype.scrollIntoView = scrollIntoView; - const value = [ - {type: 'paragraph', children: [{text: 'First paragraph'}]}, - {type: 'paragraph', children: [{text: 'Second paragraph'}]} - ]; + const subjectRange = {anchor: {path: [0, 0], offset: 0}, focus: {path: [0, 0], offset: 5}}; + const value = [{type: 'paragraph', children: [{text: 'Some text to comment on'}]}]; - const {getAllByRole} = renderWithCommenting( + renderWithCommenting( , { wrapper, - commentThreads: [ - {id: 5, subjectType: 'ContentElement', subjectId: 10, - subjectRange: {anchor: {path: [0, 0], offset: 0}, focus: {path: [1, 0], offset: 6}}, - comments: [{id: 1, body: 'spans', creatorName: 'Alice', creatorId: 1}]}, - {id: 7, subjectType: 'ContentElement', subjectId: 10, - subjectRange: {anchor: {path: [1, 0], offset: 7}, focus: {path: [1, 0], offset: 16}}, - comments: [{id: 2, body: 'p1', creatorName: 'Eve', creatorId: 2}]} - ] + commentThreads: [{ + id: 9, + subjectType: 'ContentElement', + subjectId: 10, + subjectRange, + comments: [{id: 1, body: 'A comment', creatorName: 'Alice', creatorId: 1}] + }] } ); - const badges = getAllByRole('status'); - fireEvent.click(badges[0]); + window.parent.postMessage.mockClear(); - expect(window.parent.postMessage).toHaveBeenCalledWith({ + const echoed = new Promise(resolve => { + const original = window.parent.postMessage; + window.parent.postMessage = (data, ...rest) => { + original(data, ...rest); + if (data.type === 'SELECTED' && data.payload.type === 'contentElementComments') { + resolve(data); + } + }; + }); + + act(() => { + window.postMessage({ + type: 'SELECT_COMMENT_THREAD', + payload: {threadId: 9} + }, '*'); + }); + + await expect(echoed).resolves.toMatchObject({ type: 'SELECTED', - payload: { - type: 'contentElementComments', - id: 1, - highlightedThreadId: 5, - threadIds: [5] - } - }, expect.anything()); + payload: {type: 'contentElementComments', id: 1, highlightedThreadId: 9} + }); + + expect(scrollIntoView).toHaveBeenCalled(); + delete Element.prototype.scrollIntoView; }); }); }); diff --git a/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/commentThreadIdsAtSelection-spec.js b/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/commentThreadIdsAtSelection-spec.js new file mode 100644 index 0000000000..a9ccb0b185 --- /dev/null +++ b/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/commentThreadIdsAtSelection-spec.js @@ -0,0 +1,60 @@ +import {commentThreadIdsAtSelection} from 'frontend/inlineEditing/EditableText/commentThreadIdsAtSelection'; + +describe('commentThreadIdsAtSelection', () => { + const blockSelection = (block, offset = 0) => ({ + anchor: {path: [block, 0], offset}, + focus: {path: [block, 0], offset} + }); + + const highlight = (id, block) => ({ + thread: {id}, + range: { + anchor: {path: [block, 0], offset: 0}, + focus: {path: [block, 0], offset: 4} + } + }); + + it('returns empty array when selection is null', () => { + expect(commentThreadIdsAtSelection([highlight(5, 0)], null)).toEqual([]); + }); + + it('returns ids of highlights overlapping the selection block', () => { + const highlights = [ + highlight(5, 0), + highlight(6, 0), + highlight(7, 1) + ]; + + expect(commentThreadIdsAtSelection(highlights, blockSelection(0))) + .toEqual([5, 6]); + }); + + it('preserves the order from the highlights list', () => { + const highlights = [ + highlight(7, 1), + highlight(5, 0), + highlight(6, 0) + ]; + + expect(commentThreadIdsAtSelection(highlights, blockSelection(0))) + .toEqual([5, 6]); + }); + + it('skips highlights without a thread (e.g. pending new-thread highlight)', () => { + const pending = { + key: 'selection', + range: { + anchor: {path: [0, 0], offset: 0}, + focus: {path: [0, 0], offset: 4} + } + }; + + expect(commentThreadIdsAtSelection([pending, highlight(5, 0)], blockSelection(0))) + .toEqual([5]); + }); + + it('returns empty array when no highlights overlap', () => { + expect(commentThreadIdsAtSelection([highlight(5, 0)], blockSelection(2))) + .toEqual([]); + }); +}); diff --git a/entry_types/scrolled/package/spec/review/ThreadList-spec.js b/entry_types/scrolled/package/spec/review/ThreadList-spec.js index 1adfd087b2..8afd2375dc 100644 --- a/entry_types/scrolled/package/spec/review/ThreadList-spec.js +++ b/entry_types/scrolled/package/spec/review/ThreadList-spec.js @@ -109,6 +109,49 @@ describe('ThreadList', () => { expect(queryByText('Filtered out')).not.toBeInTheDocument(); }); + it('marks the thread matching highlightedThreadId with aria-current', () => { + 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} + ]} + ] + } + ); + + const highlighted = container.querySelector('[aria-current="true"]'); + expect(highlighted).toContainElement(getByText('second')); + expect(highlighted).not.toContainElement(getByText('first')); + }); + + it('fires onThreadClick with the clicked thread', async () => { + const user = userEvent.setup(); + const onThreadClick = jest.fn(); + const {getByText} = renderWithReviewState( + , + { + commentThreads: [ + {id: 7, subjectType: 'ContentElement', subjectId: 10, comments: [ + {id: 70, body: 'click me', creatorName: 'Bob', creatorId: 2} + ]} + ] + } + ); + + await user.click(getByText('click me')); + + expect(onThreadClick).toHaveBeenCalledWith(expect.objectContaining({id: 7})); + }); + it('applies filter prop to resolved threads', async () => { const user = userEvent.setup(); const {getByText, queryByText} = renderWithReviewState( diff --git a/entry_types/scrolled/package/spec/review/ThreadsBadge-spec.js b/entry_types/scrolled/package/spec/review/ThreadsBadge-spec.js index 17df44b9f5..e19b0c8f04 100644 --- a/entry_types/scrolled/package/spec/review/ThreadsBadge-spec.js +++ b/entry_types/scrolled/package/spec/review/ThreadsBadge-spec.js @@ -3,6 +3,7 @@ import '@testing-library/jest-dom/extend-expect'; import {ThreadsBadge} from 'review/ThreadsBadge'; import {renderWithReviewState} from 'testHelpers/renderWithReviewState'; +import {fakeParentWindow} from 'support'; describe('ThreadsBadge', () => { it('does not display count for single thread', () => { @@ -146,4 +147,70 @@ describe('ThreadsBadge', () => { expect(getByRole('status')).toBeInTheDocument(); }); }); + + describe('on SELECT_COMMENT_THREAD message', () => { + let scrollIntoView; + + beforeEach(() => { + fakeParentWindow(); + scrollIntoView = jest.fn(); + Element.prototype.scrollIntoView = scrollIntoView; + }); + + afterEach(() => { + delete Element.prototype.scrollIntoView; + }); + + it('scrolls into view and calls onSelectThread when threadId matches', async () => { + const onSelectThread = jest.fn(); + + renderWithReviewState( + , + { + commentThreads: [ + {id: 5, subjectType: 'ContentElement', subjectId: 10, comments: []} + ] + } + ); + + await new Promise(resolve => { + window.postMessage({ + type: 'SELECT_COMMENT_THREAD', + payload: {threadId: 5} + }, '*'); + setTimeout(resolve, 0); + }); + + expect(scrollIntoView).toHaveBeenCalled(); + expect(onSelectThread).toHaveBeenCalledWith(5); + }); + + it('ignores messages for non-matching threadIds', async () => { + const onSelectThread = jest.fn(); + + renderWithReviewState( + , + { + commentThreads: [ + {id: 5, subjectType: 'ContentElement', subjectId: 10, comments: []} + ] + } + ); + + await new Promise(resolve => { + window.postMessage({ + type: 'SELECT_COMMENT_THREAD', + payload: {threadId: 999} + }, '*'); + setTimeout(resolve, 0); + }); + + expect(scrollIntoView).not.toHaveBeenCalled(); + expect(onSelectThread).not.toHaveBeenCalled(); + }); + }); }); diff --git a/entry_types/scrolled/package/spec/frontend/usePostMessageListener-spec.js b/entry_types/scrolled/package/spec/shared/usePostMessageListener-spec.js similarity index 91% rename from entry_types/scrolled/package/spec/frontend/usePostMessageListener-spec.js rename to entry_types/scrolled/package/spec/shared/usePostMessageListener-spec.js index b917a304f6..bab9aaba82 100644 --- a/entry_types/scrolled/package/spec/frontend/usePostMessageListener-spec.js +++ b/entry_types/scrolled/package/spec/shared/usePostMessageListener-spec.js @@ -1,4 +1,4 @@ -import {usePostMessageListener} from 'frontend/usePostMessageListener'; +import {usePostMessageListener} from 'shared/usePostMessageListener'; import {renderHook} from '@testing-library/react-hooks'; import {fakeParentWindow, tick} from 'support'; diff --git a/entry_types/scrolled/package/src/editor/api/ContentElementTypeRegistry.js b/entry_types/scrolled/package/src/editor/api/ContentElementTypeRegistry.js index cbf6b01515..811d4b4f98 100644 --- a/entry_types/scrolled/package/src/editor/api/ContentElementTypeRegistry.js +++ b/entry_types/scrolled/package/src/editor/api/ContentElementTypeRegistry.js @@ -79,6 +79,10 @@ export class ContentElementTypeRegistry { }; } + findPictogram(typeName) { + return this.contentElementTypes[typeName]?.pictogram; + } + groupedByCategory() { const result = []; const categoriesByName = {}; diff --git a/entry_types/scrolled/package/src/editor/config.js b/entry_types/scrolled/package/src/editor/config.js index 241fb55d48..b3396a6e51 100644 --- a/entry_types/scrolled/package/src/editor/config.js +++ b/entry_types/scrolled/package/src/editor/config.js @@ -10,7 +10,7 @@ import {EntryPreviewView} from './views/EntryPreviewView'; import {SideBarRouter} from './routers/SideBarRouter'; import {SideBarController} from './controllers/SideBarController'; -import {browser} from 'pageflow/frontend'; +import {browser, features} from 'pageflow/frontend'; import { CheckBoxInputView, @@ -55,6 +55,16 @@ editor.registerSideBarRouting({ controller: SideBarController }); +editor.addInitializer(() => { + if (features.isEnabled('commenting')) { + editor.registerMainMenuItem({ + translationKey: 'pageflow_scrolled.editor.main_menu.comments', + path: '/scrolled/comments', + id: 'comments' + }); + } +}); + editor.registerFileSelectionHandler('contentElementConfiguration', ContentElementFileSelectionHandler); diff --git a/entry_types/scrolled/package/src/editor/controllers/PreviewMessageController.js b/entry_types/scrolled/package/src/editor/controllers/PreviewMessageController.js index ea5ee3caac..619c8c29f3 100644 --- a/entry_types/scrolled/package/src/editor/controllers/PreviewMessageController.js +++ b/entry_types/scrolled/package/src/editor/controllers/PreviewMessageController.js @@ -1,3 +1,4 @@ +import Backbone from 'backbone'; import {Object} from 'pageflow/ui'; import {ReviewMessageHandler} from 'pageflow-scrolled/review'; import {watchCollections} from 'pageflow-scrolled/entryState'; @@ -113,6 +114,13 @@ export const PreviewMessageController = Object.extend({ }) }); + this.listenTo(this.entry, 'selectCommentThread', threadId => { + postMessage({ + type: 'SELECT_COMMENT_THREAD', + payload: {threadId} + }) + }); + this.listenTo(this.entry, 'selectWidget', widget => { postMessage({ type: 'SELECT', @@ -154,13 +162,25 @@ export const PreviewMessageController = Object.extend({ const {type, id, position} = message.data.payload; this.preservedScrollTarget = null; + this.entry.set({ + highlightedThreadId: type === 'contentElementComments' ? + message.data.payload.highlightedThreadId : + undefined, + selectedContentElementCommentsId: + type === 'contentElement' || type === 'contentElementComments' ? + id : + type === 'newThread' ? + this.entry.contentElements.findWhere({permaId: id})?.id : + undefined + }); if (type === 'contentElementComments') { - const {threadIds} = message.data.payload; - const query = threadIds ? - `?payload=${encodeURIComponent(JSON.stringify({threadIds}))}` : - ''; - this.editor.navigate(`/scrolled/content_elements/${id}/comments${query}`, {trigger: true}) + // Stay on the current tab when the user is already on the + // comments route — only force the selection tab when arriving + // there from elsewhere. + if (!Backbone.history.fragment?.startsWith('scrolled/comments')) { + this.editor.navigate('/scrolled/comments?tab=selection', {trigger: true}) + } } else if (type === 'newThread') { const {subjectType, range} = message.data.payload; diff --git a/entry_types/scrolled/package/src/editor/controllers/SideBarController.js b/entry_types/scrolled/package/src/editor/controllers/SideBarController.js index 1c511ca48e..5d26f8d1d4 100644 --- a/entry_types/scrolled/package/src/editor/controllers/SideBarController.js +++ b/entry_types/scrolled/package/src/editor/controllers/SideBarController.js @@ -8,7 +8,7 @@ import {EditSectionView} from '../views/EditSectionView'; import {EditSectionTransitionView} from '../views/EditSectionTransitionView'; import {EditSectionPaddingsView} from '../views/EditSectionPaddingsView'; import {EditContentElementView} from '../views/EditContentElementView'; -import {ContentElementCommentsView} from '../views/ContentElementCommentsView'; +import {CommentsView} from '../views/CommentsView'; import {NewThreadView} from '../views/NewThreadView'; export const SideBarController = Marionette.Controller.extend({ @@ -50,16 +50,11 @@ export const SideBarController = Marionette.Controller.extend({ })); }, - contentElementComments: function(id, payload) { - const threadIds = payload ? - JSON.parse(decodeURIComponent(payload)).threadIds : - undefined; - - this.region.show(new ContentElementCommentsView({ + comments: function(tab) { + this.region.show(new CommentsView({ entry: this.entry, - model: this.entry.contentElements.get(id), editor, - threadIds + defaultTab: tab })); }, diff --git a/entry_types/scrolled/package/src/editor/routers/SideBarRouter.js b/entry_types/scrolled/package/src/editor/routers/SideBarRouter.js index 503cb91001..375e8db858 100644 --- a/entry_types/scrolled/package/src/editor/routers/SideBarRouter.js +++ b/entry_types/scrolled/package/src/editor/routers/SideBarRouter.js @@ -2,13 +2,13 @@ import Marionette from 'backbone.marionette'; export const SideBarRouter = Marionette.AppRouter.extend({ appRoutes: { + 'scrolled/comments?tab=:tab': 'comments', + 'scrolled/comments': 'comments', 'scrolled/chapters/:id': 'chapter', 'scrolled/sections/:id/transition': 'sectionTransition', 'scrolled/sections/:id/paddings?position=:position': 'sectionPaddings', 'scrolled/sections/:id/paddings': 'sectionPaddings', 'scrolled/sections/:id': 'section', - 'scrolled/content_elements/:id/comments?payload=:payload': 'contentElementComments', - 'scrolled/content_elements/:id/comments': 'contentElementComments', 'scrolled/content_elements/:id': 'contentElement', 'scrolled/comment_threads/new?subjectType=:subjectType&subjectId=:subjectId&payload=:payload': 'newThread' } diff --git a/entry_types/scrolled/package/src/editor/views/CommentsView.js b/entry_types/scrolled/package/src/editor/views/CommentsView.js new file mode 100644 index 0000000000..7f62b84a0b --- /dev/null +++ b/entry_types/scrolled/package/src/editor/views/CommentsView.js @@ -0,0 +1,45 @@ +import I18n from 'i18n-js'; +import Marionette from 'backbone.marionette'; + +import {editor} from 'pageflow/editor'; +import {TabsView} from 'pageflow/ui'; + +import {EntryCommentsView} from './EntryCommentsView'; +import {ContentElementCommentsView} from './ContentElementCommentsView'; + +export const CommentsView = Marionette.ItemView.extend({ + className: 'comments_view', + + template: () => ` + ${I18n.t('pageflow.editor.templates.back_button_decorator.outline')} +
+ `, + + ui: { + tabs: '.tabs' + }, + + events: { + 'click a.back': 'goBack' + }, + + onRender: function() { + const {entry, defaultTab} = this.options; + + const tabsView = new TabsView({ + i18n: 'pageflow_scrolled.editor.comments_view.tabs', + defaultTab: defaultTab || 'comments' + }); + + tabsView.tab('comments', () => + new EntryCommentsView({entry, editor: this.options.editor})); + tabsView.tab('selection', () => + new ContentElementCommentsView({entry, editor: this.options.editor})); + + this.appendSubview(tabsView, {to: this.ui.tabs}); + }, + + goBack: function() { + editor.navigate('/', {trigger: true}); + } +}); diff --git a/entry_types/scrolled/package/src/editor/views/ContentElementCommentsView.js b/entry_types/scrolled/package/src/editor/views/ContentElementCommentsView.js index dd146cbdec..17b82b4cbd 100644 --- a/entry_types/scrolled/package/src/editor/views/ContentElementCommentsView.js +++ b/entry_types/scrolled/package/src/editor/views/ContentElementCommentsView.js @@ -3,18 +3,74 @@ import React from 'react'; import {ThreadList} from 'pageflow-scrolled/review'; import {ReviewView} from './ReviewView'; -import styles from './ContentElementCommentsView.module.css'; export const ContentElementCommentsView = ReviewView.extend({ - renderContent() { - const {threadIds} = this.options; - const filter = threadIds ? thread => threadIds.includes(thread.id) : undefined; + initialize() { + const {entry} = this.options; + + this.listenTo(entry, + 'change:selectedContentElementCommentsId', + this._onSelectedChange); + this.listenTo(entry, + 'change:highlightedThreadId', + () => this.rerender()); + this._observeContentElement(); + }, + + props() { + const contentElement = this._contentElement; + return { + contentElement, + threadIds: contentElement?.transientState.get('commentThreadIdsAtSelection'), + highlightedThreadId: this.options.entry.get('highlightedThreadId'), + onThreadClick: thread => + this.options.entry.trigger('selectCommentThread', thread.id) + }; + }, + + // Highlight and per-thread click are only wired when the view is + // scoped to a list of thread ids. For unscoped content elements + // 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}) { + if (!contentElement) return null; + + if (threadIds === undefined) { + return ( + + ); + } return ( + subjectId={contentElement.get('permaId')} + filter={thread => threadIds.includes(thread.id)} + highlightedThreadId={highlightedThreadId} + onThreadClick={onThreadClick} + hideNewTopicButton /> ); + }, + + _onSelectedChange() { + this._observeContentElement(); + this.rerender(); + }, + + _observeContentElement() { + if (this._contentElement) { + this.stopListening(this._contentElement.transientState); + } + + const id = this.options.entry.get('selectedContentElementCommentsId'); + this._contentElement = id ? this.options.entry.contentElements.get(id) : null; + + if (this._contentElement) { + this.listenTo(this._contentElement.transientState, + 'change:commentThreadIdsAtSelection', + () => this.rerender()); + } } }); diff --git a/entry_types/scrolled/package/src/editor/views/ContentElementCommentsView.module.css b/entry_types/scrolled/package/src/editor/views/ContentElementCommentsView.module.css deleted file mode 100644 index a4f780243d..0000000000 --- a/entry_types/scrolled/package/src/editor/views/ContentElementCommentsView.module.css +++ /dev/null @@ -1,7 +0,0 @@ -.newTopicButton { - composes: secondaryIconButton from './buttons.module.css'; - display: flex !important; - position: absolute; - right: 0; - bottom: 100%; -} diff --git a/entry_types/scrolled/package/src/editor/views/EntryCommentsView.js b/entry_types/scrolled/package/src/editor/views/EntryCommentsView.js new file mode 100644 index 0000000000..85e3ba2bf4 --- /dev/null +++ b/entry_types/scrolled/package/src/editor/views/EntryCommentsView.js @@ -0,0 +1,98 @@ +import React from 'react'; +import I18n from 'i18n-js'; + +import {ThreadList, useCommentThreads} from 'pageflow-scrolled/review'; + +import {ReviewView} from './ReviewView'; +import defaultPictogram from './images/defaultPictogram.svg'; +import styles from './EntryCommentsView.module.css'; + +export const EntryCommentsView = ReviewView.extend({ + initialize() { + this.listenTo(this.options.entry, + 'change:highlightedThreadId', + () => this.rerender()); + }, + + props() { + const {entry, editor} = this.options; + return { + items: collectContentElements(entry), + highlightedThreadId: entry.get('highlightedThreadId'), + onThreadClick: thread => entry.trigger('selectCommentThread', thread.id), + editor + }; + }, + + renderContent({items, highlightedThreadId, onThreadClick, editor}) { + return ( +
+ {items.map(({contentElement}) => ( + + ))} +
+ ); + } +}); + +function collectContentElements(entry) { + const items = []; + + entry.chapters.each(chapter => { + chapter.sections.each(section => { + section.contentElements.each(contentElement => { + items.push({contentElement, section}); + }); + }); + }); + + return items; +} + +function ContentElementGroup({contentElement, highlightedThreadId, onThreadClick, editor}) { + const permaId = contentElement.get('permaId'); + const threads = useCommentThreads({ + subjectType: 'ContentElement', + subjectId: permaId + }); + + if (threads.length === 0) { + return null; + } + + const typeName = contentElement.get('typeName'); + const label = I18n.t(`pageflow_scrolled.editor.content_elements.${typeName}.name`); + const pictogram = editor.contentElementTypes.findPictogram(typeName) || defaultPictogram; + + return ( +
+ + +
+ ); +} + +function ContentElementTypeSeparator({label, pictogram}) { + return ( +
+ + {label} + {pictogram && + } + +
+ ); +} + +function escapeCssUrl(url) { + return url.replace(/'/g, "\\'").replace(/\n/g, ''); +} diff --git a/entry_types/scrolled/package/src/editor/views/EntryCommentsView.module.css b/entry_types/scrolled/package/src/editor/views/EntryCommentsView.module.css new file mode 100644 index 0000000000..6e5d536ddc --- /dev/null +++ b/entry_types/scrolled/package/src/editor/views/EntryCommentsView.module.css @@ -0,0 +1,33 @@ +.list { + display: flex; + flex-direction: column; +} + +.group { + cursor: pointer; +} + +.separator { + display: flex; + align-items: center; + gap: space(2); + padding: space(4) 0 space(2); +} + +.typeName { + font-weight: 500; +} + +.pictogram { + width: 16px; + height: 16px; + background: var(--ui-primary-color-light); + mask-size: contain; + mask-repeat: no-repeat; + mask-position: center; +} + +.rule { + flex: 1; + border-bottom: solid 1px var(--ui-on-surface-color-lightest); +} diff --git a/entry_types/scrolled/package/src/editor/views/ReviewView.js b/entry_types/scrolled/package/src/editor/views/ReviewView.js index fc3118ea04..b8f64c4a5f 100644 --- a/entry_types/scrolled/package/src/editor/views/ReviewView.js +++ b/entry_types/scrolled/package/src/editor/views/ReviewView.js @@ -9,11 +9,18 @@ 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, and a ReviewStateProvider seeded from -// the current session state. Subclasses implement `renderContent()` to -// return the React subtree. +// the current session state. 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: () => `
`, + props() { + return {}; + }, + onShow() { const session = this.options.entry.reviewSession; @@ -22,12 +29,7 @@ export const ReviewView = Marionette.ItemView.extend({ targetWindow: window }); - ReactDOM.render( - - {this.renderContent()} - , - this._containerEl() - ); + this.rerender(); }, onClose() { @@ -35,6 +37,16 @@ export const ReviewView = Marionette.ItemView.extend({ ReactDOM.unmountComponentAtNode(this._containerEl()); }, + rerender() { + const session = this.options.entry.reviewSession; + ReactDOM.render( + + {this.renderContent(this.props())} + , + this._containerEl() + ); + }, + _containerEl() { return this.$el.find(`.${styles.container}`)[0]; } diff --git a/entry_types/scrolled/package/src/frontend/Content.js b/entry_types/scrolled/package/src/frontend/Content.js index d51eb17048..79981a366b 100644 --- a/entry_types/scrolled/package/src/frontend/Content.js +++ b/entry_types/scrolled/package/src/frontend/Content.js @@ -6,7 +6,7 @@ import {useActiveExcursion} from './useActiveExcursion'; import {useCurrentSectionIndexState} from './useCurrentChapter'; import {useEntryStructure} from 'pageflow-scrolled/entryState'; import {extensible} from './extensions'; -import {usePostMessageListener} from './usePostMessageListener'; +import {usePostMessageListener} from '../shared/usePostMessageListener'; import {useSectionChangeEvents} from './useSectionChangeEvents'; import {sectionChangeMessagePoster} from './sectionChangeMessagePoster'; import {useScrollToTarget} from './useScrollTarget'; diff --git a/entry_types/scrolled/package/src/frontend/inlineEditing/ContentElementDecorator.js b/entry_types/scrolled/package/src/frontend/inlineEditing/ContentElementDecorator.js index 67b180dd78..f20e290c4b 100644 --- a/entry_types/scrolled/package/src/frontend/inlineEditing/ContentElementDecorator.js +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/ContentElementDecorator.js @@ -70,7 +70,12 @@ function DefaultSelectionRect(props) { selectComments()} />} + onClick={() => selectComments()} + onSelectThread={threadId => selectComments({ + type: 'contentElementComments', + id: props.id, + highlightedThreadId: threadId + })} />} commentBadgeInset={!isSelected} ariaLabel={t('pageflow_scrolled.inline_editing.select_content_element')} insertButtonTitles={t('pageflow_scrolled.inline_editing.insert_content_element')} diff --git a/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/BadgeColumn.js b/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/BadgeColumn.js index c240b4aeec..33338462c0 100644 --- a/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/BadgeColumn.js +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/BadgeColumn.js @@ -1,4 +1,4 @@ -import React from 'react'; +import React, {useCallback} from 'react'; import {Range, Transforms} from 'slate'; import {FloatingPortal} from '@floating-ui/react'; @@ -7,6 +7,7 @@ import {useSlate, ReactEditor} from 'slate-react'; import {Badge, useAnchoredFloating} from 'pageflow-scrolled/review'; import {useFloatingPortalRoot} from '../../FloatingPortalRootProvider'; import {useContentElementAttributes} from '../../useContentElementAttributes'; +import {usePostMessageListener} from '../../../shared/usePostMessageListener'; import {useEditorSelection} from '../EditorState'; import {highlightOverlapsSelection} from './highlightOverlapsSelection'; @@ -44,6 +45,39 @@ function PositionedBadge({editor, highlight, highlights, editorSelection, anchor const {refs, floatingStyles, hasAnchor} = useAnchoredFloating(highlight.key, anchors, {placement: 'left-start'}); + const handleClick = useCallback(() => { + if (highlight.key === 'selection') { + selectComments(); + return; + } + + // Don't try to also clear the DOM selection here: calling + // removeAllRanges fires a selectionchange that slate-react's + // listener picks up and uses to overwrite editor.selection back + // to null — undoing this Transforms.select and dropping the + // selection rect. The visible text selection therefore lingers + // on screen until the user's next interaction with the editor; + // slate's internal state (which downstream consumers depend on) + // stays correct. + Transforms.select(editor, Range.start(highlight.range)); + + selectComments({ + type: 'contentElementComments', + id: contentElementId, + highlightedThreadId: highlight.thread?.id + }); + }, [editor, highlight, selectComments, contentElementId]); + + usePostMessageListener(useCallback(data => { + if (data.type === 'SELECT_COMMENT_THREAD' && + data.payload.threadId === highlight.thread?.id) { + if (refs.floating.current) { + refs.floating.current.scrollIntoView({block: 'nearest', behavior: 'smooth'}); + } + handleClick(); + } + }, [highlight, handleClick, refs.floating])); + if (!hasAnchor) return null; const isHighlightedThread = !!highlight.thread && @@ -67,37 +101,6 @@ function PositionedBadge({editor, highlight, highlights, editorSelection, anchor highlightOverlapsSelection(highlight, overlapSelection) ? undefined : 'dot'; - function handleClick() { - if (highlight.key === 'selection') { - selectComments(); - return; - } - - // Don't try to also clear the DOM selection here: calling - // removeAllRanges fires a selectionchange that slate-react's - // listener picks up and uses to overwrite editor.selection back - // to null — undoing this Transforms.select and dropping the - // selection rect. The visible text selection therefore lingers - // on screen until the user's next interaction with the editor; - // slate's internal state (which downstream consumers depend on) - // stays correct. - const highlightStart = Range.start(highlight.range); - Transforms.select(editor, highlightStart); - - const threadIds = highlights - .filter(h => h.thread && highlightOverlapsSelection( - h, {anchor: highlightStart, focus: highlightStart} - )) - .map(h => h.thread.id); - - selectComments({ - type: 'contentElementComments', - id: contentElementId, - highlightedThreadId: highlight.thread?.id, - threadIds - }); - } - return (
diff --git a/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/BadgeColumn.module.css b/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/BadgeColumn.module.css index e40b060df4..ae333f8f7a 100644 --- a/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/BadgeColumn.module.css +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/BadgeColumn.module.css @@ -8,6 +8,7 @@ justify-content: center; width: 30px; height: 30px; + scroll-margin: space(8); } .onDark { 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 7dae64864c..744d4e48d8 100644 --- a/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/Selection.js +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/Selection.js @@ -15,6 +15,7 @@ import {cursorLeftHighlightedThreadBlock} from './cursorLeftHighlightedThreadBlo import {cursorMovedFromPendingNewThreadRange} from './cursorMovedFromPendingNewThreadRange'; import {getUniformSelectedNode} from './getUniformSelectedNode'; import {toggleBlock, isBlockActive} from './blocks'; +import {commentThreadIdsAtSelection} from './commentThreadIdsAtSelection'; import {computeBounds} from './computeBounds'; import TextIcon from '../images/text.svg'; @@ -135,6 +136,9 @@ export function Selection(props) { typographySize: getUniformSelectedNode(editor, 'size')?.size, color: getUniformSelectedNode(editor, 'color')?.color, textAlign: getUniformSelectedNode(editor, 'textAlign')?.textAlign, + commentThreadIdsAtSelection: commentThreadIdsAtSelection( + props.highlights || [], editor.selection + ), }); boundsRef.current = {start, end}; diff --git a/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/commentThreadIdsAtSelection.js b/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/commentThreadIdsAtSelection.js new file mode 100644 index 0000000000..2764a73801 --- /dev/null +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/commentThreadIdsAtSelection.js @@ -0,0 +1,9 @@ +import {highlightOverlapsSelection} from './highlightOverlapsSelection'; + +export function commentThreadIdsAtSelection(highlights, selection) { + if (!selection) return []; + + return highlights + .filter(h => h.thread && highlightOverlapsSelection(h, selection)) + .map(h => h.thread.id); +} diff --git a/entry_types/scrolled/package/src/frontend/inlineEditing/EntryDecorator.js b/entry_types/scrolled/package/src/frontend/inlineEditing/EntryDecorator.js index cf6d7d6d3d..f8d8199a45 100644 --- a/entry_types/scrolled/package/src/frontend/inlineEditing/EntryDecorator.js +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/EntryDecorator.js @@ -2,7 +2,7 @@ import React, {useEffect, useCallback} from 'react'; import {ReviewStateProvider} from 'pageflow-scrolled/review'; import {useEntryStateDispatch} from 'pageflow-scrolled/entryState'; -import {usePostMessageListener} from '../usePostMessageListener'; +import {usePostMessageListener} from '../../shared/usePostMessageListener'; import {EditorStateProvider, useEditorSelection} from './EditorState'; import { useContentElementEditorCommandEmitter, diff --git a/entry_types/scrolled/package/src/frontend/inlineEditing/scrollPoints.js b/entry_types/scrolled/package/src/frontend/inlineEditing/scrollPoints.js index e33005f8b5..f762312cc2 100644 --- a/entry_types/scrolled/package/src/frontend/inlineEditing/scrollPoints.js +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/scrollPoints.js @@ -1,5 +1,5 @@ import {useRef, useCallback} from 'react'; -import {usePostMessageListener} from '../usePostMessageListener'; +import {usePostMessageListener} from '../../shared/usePostMessageListener'; // Scroll points are used to preserve scroll position when toggling // the editor phone preview. Each ContentElementDecorator renders a diff --git a/entry_types/scrolled/package/src/review/Badge.js b/entry_types/scrolled/package/src/review/Badge.js index 39ca4c20a9..57e0c6bdc2 100644 --- a/entry_types/scrolled/package/src/review/Badge.js +++ b/entry_types/scrolled/package/src/review/Badge.js @@ -1,10 +1,10 @@ -import React from 'react'; +import React, {forwardRef} from 'react'; import classNames from 'classnames'; import CommentIcon from './images/comment.svg'; import styles from './Badge.module.css'; -export function Badge({counter, mode, onClick}) { +export const Badge = forwardRef(function Badge({counter, mode, onClick}, ref) { const variant = resolveVariant(mode, counter > 0); if (!variant) { @@ -12,14 +12,15 @@ export function Badge({counter, mode, onClick}) { } return ( - ); -} +}); function resolveVariant(mode, hasThreads) { switch (mode) { diff --git a/entry_types/scrolled/package/src/review/Thread.js b/entry_types/scrolled/package/src/review/Thread.js index 9b50823bdc..708c601c36 100644 --- a/entry_types/scrolled/package/src/review/Thread.js +++ b/entry_types/scrolled/package/src/review/Thread.js @@ -1,4 +1,5 @@ import React from 'react'; +import classNames from 'classnames'; import {useI18n} from '../frontend/i18n'; import {AvatarStack} from './Avatar'; @@ -10,14 +11,19 @@ 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}) { +export function Thread({thread, collapsed, onToggle, onResolve, onClick, highlighted}) { const {t} = useI18n({locale: 'ui'}); const firstComment = thread.comments[0]; const replies = thread.comments.slice(1); const repliesCollapsed = collapsed && replies.length > 0; return ( -
+
{replies.length > 0 &&
}
diff --git a/entry_types/scrolled/package/src/review/ThreadsBadge.js b/entry_types/scrolled/package/src/review/ThreadsBadge.js index face915829..8ece23ecad 100644 --- a/entry_types/scrolled/package/src/review/ThreadsBadge.js +++ b/entry_types/scrolled/package/src/review/ThreadsBadge.js @@ -1,10 +1,42 @@ -import React from 'react'; +import React, {useCallback, useRef} from 'react'; +import {usePostMessageListener} from '../shared/usePostMessageListener'; import {useCommentThreads} from './ReviewStateProvider'; import {Badge} from './Badge'; -export function ThreadsBadge({subjectType, subjectId, subjectRange, onClick, mode}) { +export function ThreadsBadge({subjectType, subjectId, subjectRange, onClick, onSelectThread, mode}) { const threads = useCommentThreads({subjectType, subjectId, subjectRange}, {resolved: false}); + const ref = useRef(); - return ; + return ( + <> + {threads.length > 0 && + } + + + ); +} + +// Side-effect-only host for the SELECT_COMMENT_THREAD listener. Only +// rendered while the badge has threads, so the listener never attaches +// for empty badges. Returns null because all the work happens in the +// effect. +function SelectThreadListener({threads, badgeRef, onSelectThread}) { + usePostMessageListener(useCallback(data => { + if (data.type !== 'SELECT_COMMENT_THREAD') return; + const threadId = data.payload.threadId; + if (!threads.some(t => t.id === threadId)) return; + + // Prefer scrolling an enclosing selectable (e.g. SelectionRect with + // aria-selected) into view so the user sees surrounding context, + // not just the badge anchored at the side. + const scrollTarget = badgeRef.current?.closest('[aria-selected]') || badgeRef.current; + if (scrollTarget) scrollTarget.scrollIntoView({block: 'nearest', behavior: 'smooth'}); + + if (onSelectThread) onSelectThread(threadId); + }, [threads, badgeRef, onSelectThread])); + + return null; } diff --git a/entry_types/scrolled/package/src/frontend/usePostMessageListener.js b/entry_types/scrolled/package/src/shared/usePostMessageListener.js similarity index 100% rename from entry_types/scrolled/package/src/frontend/usePostMessageListener.js rename to entry_types/scrolled/package/src/shared/usePostMessageListener.js