diff --git a/app/assets/stylesheets/pageflow/ui/properties.scss b/app/assets/stylesheets/pageflow/ui/properties.scss index 7aa24054fa..6b32f6fad3 100644 --- a/app/assets/stylesheets/pageflow/ui/properties.scss +++ b/app/assets/stylesheets/pageflow/ui/properties.scss @@ -63,6 +63,12 @@ --ui-accent-color-lightest: hsla(44, 100%, 65%, 0.1); --ui-on-accent-color: #795f0f; + --ui-accent-grey-color: hsl(44, 0%, 75%); + --ui-accent-grey-color-light: hsla(44, 0%, 75%, 0.6); + --ui-accent-grey-color-lighter: hsla(44, 0%, 75%, 0.3); + --ui-accent-grey-color-lightest: hsla(44, 0%, 75%, 0.1); + --ui-on-accent-grey-color: hsl(44, 0%, 27%); + --ui-on-button-color: var(--ui-primary-color); --ui-button-border-color: var(--ui-primary-color-light); --ui-button-hover-border-color: var(--ui-primary-color); diff --git a/entry_types/scrolled/config/locales/de.yml b/entry_types/scrolled/config/locales/de.yml index ba7f550af0..222c06b496 100644 --- a/entry_types/scrolled/config/locales/de.yml +++ b/entry_types/scrolled/config/locales/de.yml @@ -1586,6 +1586,7 @@ de: comments: Kommentare comments_view: new_thread: Neues Thema + section: Abschnitt tabs: comments: Alle Kommentare selection: Für Auswahl @@ -1925,6 +1926,7 @@ de: add_comment: Kommentar hinzufügen cancel_add_comment: Abbrechen select_content_element: Zum Kommentieren auswählen + select_section: Abschnitt zum Kommentieren auswählen select_text_to_comment: Text zum Kommentieren auswählen new_topic: Neues Thema add_comment_placeholder: Kommentar hinzufügen... diff --git a/entry_types/scrolled/config/locales/en.yml b/entry_types/scrolled/config/locales/en.yml index 00803396de..cabe3540d6 100644 --- a/entry_types/scrolled/config/locales/en.yml +++ b/entry_types/scrolled/config/locales/en.yml @@ -1569,6 +1569,7 @@ en: comments: Comments comments_view: new_thread: New topic + section: Section tabs: comments: All comments selection: For selection @@ -1754,6 +1755,7 @@ en: add_comment: Add comment cancel_add_comment: Cancel select_content_element: Select to comment + select_section: Select section to comment select_text_to_comment: Select text to comment new_topic: New topic add_comment_placeholder: Add a comment... diff --git a/entry_types/scrolled/package/.eslintrc.js b/entry_types/scrolled/package/.eslintrc.js index 01d6d9bc11..e58812dffc 100644 --- a/entry_types/scrolled/package/.eslintrc.js +++ b/entry_types/scrolled/package/.eslintrc.js @@ -6,7 +6,11 @@ module.exports = { "plugin:storybook/recommended" ], "rules": { - "no-trailing-spaces": "error" + "no-trailing-spaces": "error", + // react-app enables no-unused-expressions, but ESLint 6 predates + // ChainExpression and flags optional-chaining call statements like + // `node?.focus()` as unused. The other packages do not enable the rule. + "no-unused-expressions": "off" }, "settings": { "import/resolver": { 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 afee2505f4..ba8d3bca42 100644 --- a/entry_types/scrolled/package/spec/editor/controllers/PreviewMessageController-spec.js +++ b/entry_types/scrolled/package/spec/editor/controllers/PreviewMessageController-spec.js @@ -193,7 +193,7 @@ describe('PreviewMessageController', () => { } }); entry.trigger('selectNewThread', { - id: 10, + subjectId: 10, subjectType: 'ContentElement', range }); @@ -201,7 +201,7 @@ describe('PreviewMessageController', () => { type: 'SELECT', payload: { type: 'newThread', - id: 10, + subjectId: 10, subjectType: 'ContentElement', range } @@ -561,6 +561,22 @@ describe('PreviewMessageController', () => { })).resolves.toBe('/scrolled/comments?tab=selection'); }); + it('navigates to comments route with tab=selection on SELECTED sectionComments', () => { + const editor = factories.editorApi(); + const entry = factories.entry(ScrolledEntry, {}, { + entryTypeSeed: normalizeSeed({ + sections: [{id: 5, permaId: 50}] + }) + }); + const iframeWindow = createIframeWindow(); + controller = new PreviewMessageController({entry, iframeWindow, editor}); + + return expect(new Promise(resolve => { + editor.on('navigate', resolve); + window.postMessage({type: 'SELECTED', payload: {id: 5, type: 'sectionComments'}}, '*'); + })).resolves.toBe('/scrolled/comments?tab=selection'); + }); + it('does not navigate on SELECTED contentElementComments while on the comments route', async () => { Backbone.history.fragment = 'scrolled/comments'; @@ -646,22 +662,22 @@ describe('PreviewMessageController', () => { })).resolves.toBeUndefined(); }); - it('sets selectedContentElementCommentsId on SELECTED contentElementComments', () => { + it('sets selectedCommentsSubject 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)); + entry.once('change:selectedCommentsSubject', (model, value) => resolve(value)); window.postMessage({ type: 'SELECTED', payload: {id: 7, type: 'contentElementComments'} }, '*'); - })).resolves.toBe(7); + })).resolves.toEqual({subjectType: 'ContentElement', id: 7}); }); - it('sets selectedContentElementCommentsId on SELECTED contentElement', () => { + it('sets selectedCommentsSubject on SELECTED contentElement', () => { const editor = factories.editorApi(); const entry = factories.entry(ScrolledEntry, {}, { entryTypeSeed: normalizeSeed({contentElements: [{id: 4}]}) @@ -670,15 +686,48 @@ describe('PreviewMessageController', () => { controller = new PreviewMessageController({entry, iframeWindow, editor}); return expect(new Promise(resolve => { - entry.once('change:selectedContentElementCommentsId', (model, value) => resolve(value)); + entry.once('change:selectedCommentsSubject', (model, value) => resolve(value)); window.postMessage({ type: 'SELECTED', payload: {id: 4, type: 'contentElement'} }, '*'); - })).resolves.toBe(4); + })).resolves.toEqual({subjectType: 'ContentElement', id: 4}); + }); + + it('sets selectedCommentsSubject on SELECTED sectionComments', () => { + const editor = factories.editorApi(); + const entry = factories.entry(ScrolledEntry, {}, { + entryTypeSeed: normalizeSeed({sections: [{id: 5, permaId: 50}]}) + }); + const iframeWindow = createIframeWindow(); + controller = new PreviewMessageController({entry, iframeWindow, editor}); + + return expect(new Promise(resolve => { + entry.once('change:selectedCommentsSubject', (model, value) => resolve(value)); + window.postMessage({ + type: 'SELECTED', + payload: {id: 5, type: 'sectionComments'} + }, '*'); + })).resolves.toEqual({subjectType: 'Section', id: 5}); + }); + + ['sectionSettings', 'sectionPaddings', 'sectionTransition'].forEach(type => { + it(`sets selectedCommentsSubject to the section on SELECTED ${type}`, () => { + const editor = factories.editorApi(); + const entry = factories.entry(ScrolledEntry, {}, { + entryTypeSeed: normalizeSeed({sections: [{id: 5, permaId: 50}]}) + }); + const iframeWindow = createIframeWindow(); + controller = new PreviewMessageController({entry, iframeWindow, editor}); + + return expect(new Promise(resolve => { + entry.once('change:selectedCommentsSubject', (model, value) => resolve(value)); + window.postMessage({type: 'SELECTED', payload: {id: 5, type}}, '*'); + })).resolves.toEqual({subjectType: 'Section', id: 5}); + }); }); - it('sets selectedContentElementCommentsId from permaId on SELECTED newThread', () => { + it('sets selectedCommentsSubject from permaId on SELECTED newThread', () => { const editor = factories.editorApi(); const entry = factories.entry(ScrolledEntry, {}, { entryTypeSeed: normalizeSeed({contentElements: [{id: 4, permaId: 100}]}) @@ -687,32 +736,53 @@ describe('PreviewMessageController', () => { controller = new PreviewMessageController({entry, iframeWindow, editor}); return expect(new Promise(resolve => { - entry.once('change:selectedContentElementCommentsId', (model, value) => resolve(value)); + entry.once('change:selectedCommentsSubject', (model, value) => resolve(value)); window.postMessage({ type: 'SELECTED', payload: { - id: 100, type: 'newThread', subjectType: 'ContentElement', + subjectId: 100, range: {anchor: {path: [0, 0], offset: 0}, focus: {path: [0, 0], offset: 1}} } }, '*'); - })).resolves.toBe(4); + })).resolves.toEqual({subjectType: 'ContentElement', id: 4}); + }); + + it('sets selectedCommentsSubject from permaId on SELECTED newThread for a section', () => { + const editor = factories.editorApi(); + const entry = factories.entry(ScrolledEntry, {}, { + entryTypeSeed: normalizeSeed({sections: [{id: 5, permaId: 50}]}) + }); + const iframeWindow = createIframeWindow(); + controller = new PreviewMessageController({entry, iframeWindow, editor}); + + return expect(new Promise(resolve => { + entry.once('change:selectedCommentsSubject', (model, value) => resolve(value)); + window.postMessage({ + type: 'SELECTED', + payload: { + type: 'newThread', + subjectType: 'Section', + subjectId: 50 + } + }, '*'); + })).resolves.toEqual({subjectType: 'Section', id: 5}); }); - it('clears selectedContentElementCommentsId on SELECTED with non-content-element type', () => { + it('clears selectedCommentsSubject on SELECTED with non-comment type', () => { const editor = factories.editorApi(); const entry = factories.entry(ScrolledEntry, {}, {entryTypeSeed: normalizeSeed()}); - entry.set('selectedContentElementCommentsId', 9); + entry.set('selectedCommentsSubject', {subjectType: 'ContentElement', id: 9}); const iframeWindow = createIframeWindow(); controller = new PreviewMessageController({entry, iframeWindow, editor}); return expect(new Promise(resolve => { - entry.once('change:selectedContentElementCommentsId', (model, value) => resolve(value)); + entry.once('change:selectedCommentsSubject', (model, value) => resolve(value)); window.postMessage({ type: 'SELECTED', - payload: {id: 10, type: 'sectionSettings'} + payload: {id: 10, type: 'widget'} }, '*'); })).resolves.toBeUndefined(); }); @@ -734,8 +804,8 @@ describe('PreviewMessageController', () => { type: 'SELECTED', payload: { type: 'newThread', - id: 10, subjectType: 'ContentElement', + subjectId: 10, range } }, '*'); 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 e15ae778ce..f61bd3e9f0 100644 --- a/entry_types/scrolled/package/spec/editor/views/CommentsView-spec.js +++ b/entry_types/scrolled/package/spec/editor/views/CommentsView-spec.js @@ -35,7 +35,7 @@ describe('CommentsView', () => { const entry = createEntry({ contentElements: [{id: 1, permaId: 10, typeName: 'textBlock'}] }); - entry.set('selectedContentElementCommentsId', 1); + entry.set('selectedCommentsSubject', {subjectType: 'ContentElement', id: 1}); entry.reviewSession = factories.reviewSession({ commentThreads: [{ id: 1, @@ -128,7 +128,7 @@ describe('CommentsView', () => { it('enables the new-thread button when a content element is selected', () => { const entry = unselectedEntry(createEntry); - entry.set('selectedContentElementCommentsId', 1); + entry.set('selectedCommentsSubject', {subjectType: 'ContentElement', id: 1}); const view = new CommentsView({entry, editor}); const {getByRole} = renderBackboneView(view); @@ -136,7 +136,7 @@ describe('CommentsView', () => { expect(getByRole('button', {name: 'New topic'})).not.toBeDisabled(); }); - it("toggles the new-thread button when entry.selectedContentElementCommentsId changes", () => { + it("toggles the new-thread button when entry.selectedCommentsSubject changes", () => { const entry = unselectedEntry(createEntry); const view = new CommentsView({entry, editor}); @@ -144,14 +144,14 @@ describe('CommentsView', () => { expect(getByRole('button', {name: 'New topic'})).toBeDisabled(); - act(() => { entry.set('selectedContentElementCommentsId', 1); }); + act(() => { entry.set('selectedCommentsSubject', {subjectType: 'ContentElement', id: 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); + entry.set('selectedCommentsSubject', {subjectType: 'ContentElement', id: 1}); const range = {anchor: {path: [0, 0], offset: 0}, focus: {path: [0, 0], offset: 5}}; entry.contentElements.get(1).transientState .set('newCommentThreadSubjectRange', range); @@ -165,12 +165,34 @@ describe('CommentsView', () => { fireEvent.click(getByRole('button', {name: 'New topic'})); expect(listener).toHaveBeenCalledWith({ - id: 10, + subjectId: 10, subjectType: 'ContentElement', range }); }); + it('triggers selectNewThread for a selected section without a range', () => { + const entry = createEntry({ + sections: [{id: 5, permaId: 50}], + contentElements: [{id: 1, permaId: 10, typeName: 'textBlock', sectionId: 5}] + }); + entry.reviewSession = factories.reviewSession(); + entry.set('selectedCommentsSubject', {subjectType: 'Section', id: 5}); + + 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({ + subjectId: 50, + subjectType: 'Section' + }); + }); + it('does not trigger selectNewThread when clicking the button while disabled', () => { const entry = unselectedEntry(createEntry); 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 6a4dc5237c..7738cf7ec8 100644 --- a/entry_types/scrolled/package/spec/editor/views/EntryCommentsView-spec.js +++ b/entry_types/scrolled/package/spec/editor/views/EntryCommentsView-spec.js @@ -25,7 +25,8 @@ describe('EntryCommentsView', () => { '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' + 'pageflow_scrolled.editor.content_elements.image.name': 'Image', + 'pageflow_scrolled.editor.comments_view.section': 'Section' }); it('renders a thread group only for content elements that have threads', () => { @@ -186,7 +187,7 @@ describe('EntryCommentsView', () => { {id: 2, permaId: 11, typeName: 'image'} ] }); - entry.set('selectedContentElementCommentsId', 1); + entry.set('selectedCommentsSubject', {subjectType: 'ContentElement', id: 1}); entry.reviewSession = factories.reviewSession({ commentThreads: [ {id: 1, subjectType: 'ContentElement', subjectId: 10, @@ -210,7 +211,7 @@ describe('EntryCommentsView', () => { const entry = createEntry({ contentElements: [{id: 1, permaId: 10, typeName: 'textBlock'}] }); - entry.set('selectedContentElementCommentsId', 1); + entry.set('selectedCommentsSubject', {subjectType: 'ContentElement', id: 1}); entry.contentElements.get(1).transientState .set('commentThreadIdsAtSelection', [2]); entry.set('highlightedThreadId', 2); @@ -230,7 +231,7 @@ describe('EntryCommentsView', () => { expect(getByText('second').closest('[aria-current="true"]')).not.toBeNull(); }); - it('updates highlights when selectedContentElementCommentsId changes', () => { + it('updates highlights when selectedCommentsSubject changes', () => { const entry = createEntry({ contentElements: [ {id: 1, permaId: 10, typeName: 'image'}, @@ -252,7 +253,7 @@ describe('EntryCommentsView', () => { expect(getByText('on one').closest('[aria-current="true"]')).toBeNull(); expect(getByText('on two').closest('[aria-current="true"]')).toBeNull(); - act(() => { entry.set('selectedContentElementCommentsId', 2); }); + act(() => { entry.set('selectedCommentsSubject', {subjectType: 'ContentElement', id: 2}); }); expect(getByText('on one').closest('[aria-current="true"]')).toBeNull(); expect(getByText('on two').closest('[aria-current="true"]')).not.toBeNull(); @@ -364,4 +365,91 @@ describe('EntryCommentsView', () => { expect(pictogram).toBeInTheDocument(); expect(pictogram.style.maskImage).not.toBe(''); }); + + it('renders a section thread group with a label-only separator', () => { + const entry = createEntry({ + sections: [{id: 1, permaId: 10}], + contentElements: [{id: 1, permaId: 100, sectionId: 1, typeName: 'image'}] + }); + entry.reviewSession = factories.reviewSession({ + commentThreads: [{ + id: 1, subjectType: 'Section', subjectId: 10, + comments: [{id: 100, body: 'On the section', creatorName: 'Alice'}] + }] + }); + + const view = new EntryCommentsView({entry, editor}); + const {getByText} = renderBackboneView(view); + + expect(getByText('On the section')).toBeInTheDocument(); + expect(getByText('Section')).toBeInTheDocument(); + expect(view.el.querySelector(`.${styles.pictogram}`)).toBeNull(); + }); + + it("renders a section's comments above its content element comments", () => { + const entry = createEntry({ + sections: [{id: 1, permaId: 10}], + contentElements: [{id: 1, permaId: 100, sectionId: 1, typeName: 'image'}] + }); + entry.reviewSession = factories.reviewSession({ + commentThreads: [ + {id: 1, subjectType: 'ContentElement', subjectId: 100, + comments: [{id: 1, body: 'on element', creatorName: 'A'}]}, + {id: 2, subjectType: 'Section', subjectId: 10, + comments: [{id: 2, body: 'on section', creatorName: 'B'}]} + ] + }); + + const view = new EntryCommentsView({entry, editor}); + const {getByText} = renderBackboneView(view); + + const sectionComment = getByText('on section'); + const elementComment = getByText('on element'); + + expect(sectionComment.compareDocumentPosition(elementComment) & + Node.DOCUMENT_POSITION_FOLLOWING).toBeTruthy(); + }); + + it('does not render a section group when the section has no threads', () => { + const entry = createEntry({ + sections: [{id: 1, permaId: 10}], + contentElements: [{id: 1, permaId: 100, sectionId: 1, typeName: 'image'}] + }); + entry.reviewSession = factories.reviewSession({ + commentThreads: [{ + id: 1, subjectType: 'ContentElement', subjectId: 100, + comments: [{id: 100, body: 'on element', creatorName: 'Alice'}] + }] + }); + + const view = new EntryCommentsView({entry, editor}); + const {queryByText} = renderBackboneView(view); + + expect(queryByText('on element')).toBeInTheDocument(); + expect(queryByText('Section')).not.toBeInTheDocument(); + }); + + it('highlights all threads of the selected section', () => { + const entry = createEntry({ + sections: [{id: 1, permaId: 10}, {id: 2, permaId: 20}] + }); + entry.set('selectedCommentsSubject', {subjectType: 'Section', id: 1}); + entry.reviewSession = factories.reviewSession({ + commentThreads: [ + {id: 1, subjectType: 'Section', subjectId: 10, + comments: [{id: 10, body: 'on selected one', creatorName: 'A'}]}, + {id: 2, subjectType: 'Section', subjectId: 10, + comments: [{id: 20, body: 'on selected two', creatorName: 'B'}]}, + {id: 3, subjectType: 'Section', subjectId: 20, + 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(); + }); }); 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 2c5bf03d03..20242acd48 100644 --- a/entry_types/scrolled/package/spec/editor/views/NewThreadView-spec.js +++ b/entry_types/scrolled/package/spec/editor/views/NewThreadView-spec.js @@ -15,7 +15,6 @@ describe('NewThreadView', () => { useFakeTranslations({ 'pageflow_scrolled.review.add_comment_placeholder': 'Add a comment...', 'pageflow_scrolled.review.send': 'Send', - 'pageflow_scrolled.review.cancel': 'Cancel', 'pageflow_scrolled.editor.new_thread_view.tabs.newComment': 'New topic', 'pageflow_scrolled.editor.new_thread_view.back': 'Comments' }); diff --git a/entry_types/scrolled/package/spec/editor/views/ContentElementCommentsView-spec.js b/entry_types/scrolled/package/spec/editor/views/SelectionCommentsView-spec.js similarity index 59% rename from entry_types/scrolled/package/spec/editor/views/ContentElementCommentsView-spec.js rename to entry_types/scrolled/package/spec/editor/views/SelectionCommentsView-spec.js index 17edc0ed74..7c0a873dc1 100644 --- a/entry_types/scrolled/package/spec/editor/views/ContentElementCommentsView-spec.js +++ b/entry_types/scrolled/package/spec/editor/views/SelectionCommentsView-spec.js @@ -2,13 +2,13 @@ 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 {SelectionCommentsView} from 'editor/views/SelectionCommentsView'; import {factories, useFakeTranslations, renderBackboneView} from 'pageflow/testHelpers'; import {useEditorGlobals} from 'support'; import {act, waitFor} from '@testing-library/react'; -describe('ContentElementCommentsView', () => { +describe('SelectionCommentsView', () => { const {createEntry} = useEditorGlobals(); beforeAll(() => { @@ -21,14 +21,16 @@ describe('ContentElementCommentsView', () => { 'pageflow_scrolled.review.add_comment_placeholder': 'Add a comment...', 'pageflow_scrolled.review.new_topic': 'New topic', 'pageflow_scrolled.review.send': 'Send', - 'pageflow_scrolled.review.no_threads_yet': 'No comments yet' + 'pageflow_scrolled.review.no_threads_yet': 'No comments yet', + 'pageflow_scrolled.review.resolved_count.one': '1 resolved', + 'pageflow_scrolled.review.resolved_count.other': '%{count} resolved' }); it('displays threads of selected content element from session state', () => { const entry = createEntry({ contentElements: [{id: 1, permaId: 10, typeName: 'fixture'}] }); - entry.set('selectedContentElementCommentsId', 1); + entry.set('selectedCommentsSubject', {subjectType: 'ContentElement', id: 1}); entry.reviewSession = factories.reviewSession({ commentThreads: [{ id: 1, @@ -38,7 +40,7 @@ describe('ContentElementCommentsView', () => { }] }); - const view = new ContentElementCommentsView({entry, editor}); + const view = new SelectionCommentsView({entry, editor}); const {getByText} = renderBackboneView(view); @@ -49,7 +51,7 @@ describe('ContentElementCommentsView', () => { const entry = createEntry({ contentElements: [{id: 1, permaId: 10, typeName: 'fixture'}] }); - entry.set('selectedContentElementCommentsId', 1); + entry.set('selectedCommentsSubject', {subjectType: 'ContentElement', id: 1}); entry.contentElements.get(1).transientState .set('commentThreadIdsAtSelection', [1]); entry.reviewSession = factories.reviewSession({ @@ -61,7 +63,7 @@ describe('ContentElementCommentsView', () => { ] }); - const view = new ContentElementCommentsView({entry, editor}); + const view = new SelectionCommentsView({entry, editor}); const {getByText, queryByText} = renderBackboneView(view); @@ -73,7 +75,7 @@ describe('ContentElementCommentsView', () => { const entry = createEntry({ contentElements: [{id: 1, permaId: 10, typeName: 'fixture'}] }); - entry.set('selectedContentElementCommentsId', 1); + entry.set('selectedCommentsSubject', {subjectType: 'ContentElement', id: 1}); entry.reviewSession = factories.reviewSession({ commentThreads: [ {id: 1, subjectType: 'ContentElement', subjectId: 10, @@ -83,7 +85,7 @@ describe('ContentElementCommentsView', () => { ] }); - const view = new ContentElementCommentsView({entry, editor}); + const view = new SelectionCommentsView({entry, editor}); const {getByText} = renderBackboneView(view); @@ -95,7 +97,7 @@ describe('ContentElementCommentsView', () => { const entry = createEntry({ contentElements: [{id: 1, permaId: 10, typeName: 'fixture'}] }); - entry.set('selectedContentElementCommentsId', 1); + entry.set('selectedCommentsSubject', {subjectType: 'ContentElement', id: 1}); entry.reviewSession = factories.reviewSession({ commentThreads: [ {id: 1, subjectType: 'ContentElement', subjectId: 10, @@ -105,7 +107,7 @@ describe('ContentElementCommentsView', () => { ] }); - const view = new ContentElementCommentsView({entry, editor}); + const view = new SelectionCommentsView({entry, editor}); const {getByText, queryByText} = renderBackboneView(view); expect(getByText('Out of scope')).toBeInTheDocument(); @@ -121,14 +123,14 @@ describe('ContentElementCommentsView', () => { expect(getByText('In scope')).toBeInTheDocument(); }); - it('updates when selectedContentElementCommentsId changes', async () => { + it('updates when selectedCommentsSubject changes', async () => { const entry = createEntry({ contentElements: [ {id: 1, permaId: 10, typeName: 'fixture'}, {id: 2, permaId: 11, typeName: 'fixture'} ] }); - entry.set('selectedContentElementCommentsId', 1); + entry.set('selectedCommentsSubject', {subjectType: 'ContentElement', id: 1}); entry.reviewSession = factories.reviewSession({ commentThreads: [ {id: 1, subjectType: 'ContentElement', subjectId: 10, @@ -138,13 +140,13 @@ describe('ContentElementCommentsView', () => { ] }); - const view = new ContentElementCommentsView({entry, editor}); + const view = new SelectionCommentsView({entry, editor}); const {getByText, queryByText} = renderBackboneView(view); expect(getByText('On first')).toBeInTheDocument(); act(() => { - entry.set('selectedContentElementCommentsId', 2); + entry.set('selectedCommentsSubject', {subjectType: 'ContentElement', id: 2}); }); await waitFor(() => { @@ -157,7 +159,7 @@ describe('ContentElementCommentsView', () => { const entry = createEntry({ contentElements: [{id: 1, permaId: 10, typeName: 'fixture'}] }); - entry.set('selectedContentElementCommentsId', 1); + entry.set('selectedCommentsSubject', {subjectType: 'ContentElement', id: 1}); entry.contentElements.get(1).transientState .set('commentThreadIdsAtSelection', [1, 2]); entry.set('highlightedThreadId', 2); @@ -170,7 +172,7 @@ describe('ContentElementCommentsView', () => { ] }); - const view = new ContentElementCommentsView({entry, editor}); + const view = new SelectionCommentsView({entry, editor}); const {getByText} = renderBackboneView(view); expect(getByText('second').closest('[aria-current="true"]')).not.toBeNull(); @@ -181,7 +183,7 @@ describe('ContentElementCommentsView', () => { const entry = createEntry({ contentElements: [{id: 1, permaId: 10, typeName: 'fixture'}] }); - entry.set('selectedContentElementCommentsId', 1); + entry.set('selectedCommentsSubject', {subjectType: 'ContentElement', id: 1}); entry.contentElements.get(1).transientState .set('commentThreadIdsAtSelection', [1, 2]); entry.reviewSession = factories.reviewSession({ @@ -193,7 +195,7 @@ describe('ContentElementCommentsView', () => { ] }); - const view = new ContentElementCommentsView({entry, editor}); + const view = new SelectionCommentsView({entry, editor}); const {getByText} = renderBackboneView(view); expect(getByText('first').closest('[aria-current="true"]')).toBeNull(); @@ -209,7 +211,7 @@ describe('ContentElementCommentsView', () => { const entry = createEntry({ contentElements: [{id: 1, permaId: 10, typeName: 'fixture'}] }); - entry.set('selectedContentElementCommentsId', 1); + entry.set('selectedCommentsSubject', {subjectType: 'ContentElement', id: 1}); entry.contentElements.get(1).transientState .set('commentThreadIdsAtSelection', [7]); entry.reviewSession = factories.reviewSession({ @@ -222,7 +224,7 @@ describe('ContentElementCommentsView', () => { const listener = jest.fn(); entry.on('selectCommentThread', listener); - const view = new ContentElementCommentsView({entry, editor}); + const view = new SelectionCommentsView({entry, editor}); const {getByText} = renderBackboneView(view); await user.click(getByText('click me')); @@ -230,13 +232,42 @@ describe('ContentElementCommentsView', () => { expect(listener).toHaveBeenCalledWith(7); }); + it('shows resolved threads collapsed when their ids are at the selection', async () => { + const user = userEvent.setup(); + + const entry = createEntry({ + contentElements: [{id: 1, permaId: 10, typeName: 'fixture'}] + }); + entry.set('selectedCommentsSubject', {subjectType: 'ContentElement', id: 1}); + entry.contentElements.get(1).transientState + .set('commentThreadIdsAtSelection', [7]); + entry.reviewSession = factories.reviewSession({ + commentThreads: [{ + id: 7, + subjectType: 'ContentElement', + subjectId: 10, + resolvedAt: '2026-06-01T00:00:00Z', + comments: [{id: 100, body: 'Resolved comment', creatorName: 'Alice'}] + }] + }); + + const view = new SelectionCommentsView({entry, editor}); + const {getByText, getByRole, queryByText} = renderBackboneView(view); + + expect(queryByText('Resolved comment')).not.toBeInTheDocument(); + + await user.click(getByRole('button', {name: '1 resolved'})); + + expect(getByText('Resolved comment')).toBeInTheDocument(); + }); + it('does not highlight or trigger selectCommentThread when not scoped', async () => { const user = userEvent.setup(); const entry = createEntry({ contentElements: [{id: 1, permaId: 10, typeName: 'fixture'}] }); - entry.set('selectedContentElementCommentsId', 1); + entry.set('selectedCommentsSubject', {subjectType: 'ContentElement', id: 1}); entry.set('highlightedThreadId', 7); entry.reviewSession = factories.reviewSession({ commentThreads: [{ @@ -248,7 +279,7 @@ describe('ContentElementCommentsView', () => { const listener = jest.fn(); entry.on('selectCommentThread', listener); - const view = new ContentElementCommentsView({entry, editor}); + const view = new SelectionCommentsView({entry, editor}); const {getByText} = renderBackboneView(view); expect(getByText('click me').closest('[aria-current="true"]')).toBeNull(); @@ -262,7 +293,7 @@ describe('ContentElementCommentsView', () => { const entry = createEntry({ contentElements: [{id: 1, permaId: 10, typeName: 'fixture'}] }); - entry.set('selectedContentElementCommentsId', 1); + entry.set('selectedCommentsSubject', {subjectType: 'ContentElement', id: 1}); entry.reviewSession = factories.reviewSession({ commentThreads: [ {id: 1, subjectType: 'ContentElement', subjectId: 10, @@ -274,7 +305,7 @@ describe('ContentElementCommentsView', () => { ] }); - const view = new ContentElementCommentsView({entry, editor}); + const view = new SelectionCommentsView({entry, editor}); const {getByText} = renderBackboneView(view); const order = ['first', 'second'] @@ -287,7 +318,7 @@ describe('ContentElementCommentsView', () => { const entry = createEntry({ contentElements: [{id: 1, permaId: 10, typeName: 'fixture'}] }); - entry.set('selectedContentElementCommentsId', 1); + entry.set('selectedCommentsSubject', {subjectType: 'ContentElement', id: 1}); entry.contentElements.get(1).transientState .set('commentThreadIdsAtSelection', [1, 2]); entry.reviewSession = factories.reviewSession({ @@ -301,7 +332,7 @@ describe('ContentElementCommentsView', () => { ] }); - const view = new ContentElementCommentsView({entry, editor}); + const view = new SelectionCommentsView({entry, editor}); const {getByText} = renderBackboneView(view); const order = ['first', 'second'] @@ -314,10 +345,10 @@ describe('ContentElementCommentsView', () => { const entry = createEntry({ contentElements: [{id: 1, permaId: 10, typeName: 'textBlock'}] }); - entry.set('selectedContentElementCommentsId', 1); + entry.set('selectedCommentsSubject', {subjectType: 'ContentElement', id: 1}); entry.reviewSession = factories.reviewSession(); - const view = new ContentElementCommentsView({entry, editor}); + const view = new SelectionCommentsView({entry, editor}); const {queryByPlaceholderText} = renderBackboneView(view); expect(queryByPlaceholderText('Add a comment...')).not.toBeInTheDocument(); @@ -327,10 +358,10 @@ describe('ContentElementCommentsView', () => { const entry = createEntry({ contentElements: [{id: 1, permaId: 10, typeName: 'textBlock'}] }); - entry.set('selectedContentElementCommentsId', 1); + entry.set('selectedCommentsSubject', {subjectType: 'ContentElement', id: 1}); entry.reviewSession = factories.reviewSession(); - const view = new ContentElementCommentsView({entry, editor}); + const view = new SelectionCommentsView({entry, editor}); const {getByText} = renderBackboneView(view); expect(getByText('No comments yet')).toBeInTheDocument(); @@ -340,7 +371,7 @@ describe('ContentElementCommentsView', () => { const entry = createEntry({ contentElements: [{id: 1, permaId: 10, typeName: 'fixture'}] }); - entry.set('selectedContentElementCommentsId', 1); + entry.set('selectedCommentsSubject', {subjectType: 'ContentElement', id: 1}); entry.reviewSession = factories.reviewSession({ commentThreads: [{ id: 1, @@ -350,7 +381,7 @@ describe('ContentElementCommentsView', () => { }] }); - const view = new ContentElementCommentsView({entry, editor}); + const view = new SelectionCommentsView({entry, editor}); const {queryByRole} = renderBackboneView(view); expect(queryByRole('button', {name: 'New topic'})).not.toBeInTheDocument(); @@ -360,10 +391,10 @@ describe('ContentElementCommentsView', () => { const entry = createEntry({ contentElements: [{id: 1, permaId: 10, typeName: 'fixture'}] }); - entry.set('selectedContentElementCommentsId', 1); + entry.set('selectedCommentsSubject', {subjectType: 'ContentElement', id: 1}); entry.reviewSession = factories.reviewSession(); - const view = new ContentElementCommentsView({entry, editor}); + const view = new SelectionCommentsView({entry, editor}); const {getByText} = renderBackboneView(view); @@ -380,4 +411,117 @@ describe('ContentElementCommentsView', () => { expect(getByText('New comment')).toBeInTheDocument(); }); }); + + it('displays threads of selected section from session state', () => { + const entry = createEntry({ + sections: [{id: 5, permaId: 50}], + contentElements: [{id: 1, permaId: 10, typeName: 'fixture', sectionId: 5}] + }); + entry.set('selectedCommentsSubject', {subjectType: 'Section', id: 5}); + entry.reviewSession = factories.reviewSession({ + commentThreads: [{ + id: 1, + subjectType: 'Section', + subjectId: 50, + comments: [{id: 100, body: 'On the section', creatorName: 'Alice'}] + }] + }); + + const view = new SelectionCommentsView({entry, editor}); + + const {getByText} = renderBackboneView(view); + + expect(getByText('On the section')).toBeInTheDocument(); + }); + + it('shows all section threads since sections are never scoped', () => { + const entry = createEntry({ + sections: [{id: 5, permaId: 50}], + contentElements: [{id: 1, permaId: 10, typeName: 'fixture', sectionId: 5}] + }); + entry.set('selectedCommentsSubject', {subjectType: 'Section', id: 5}); + entry.reviewSession = factories.reviewSession({ + commentThreads: [ + {id: 1, subjectType: 'Section', subjectId: 50, + comments: [{id: 100, body: 'First', creatorName: 'Alice'}]}, + {id: 2, subjectType: 'Section', subjectId: 50, + comments: [{id: 200, body: 'Second', creatorName: 'Bob'}]} + ] + }); + + const view = new SelectionCommentsView({entry, editor}); + + const {getByText} = renderBackboneView(view); + + expect(getByText('First')).toBeInTheDocument(); + expect(getByText('Second')).toBeInTheDocument(); + }); + + it('updates when selection switches from content element to section', async () => { + const entry = createEntry({ + sections: [{id: 5, permaId: 50}], + contentElements: [{id: 1, permaId: 10, typeName: 'fixture', sectionId: 5}] + }); + entry.set('selectedCommentsSubject', {subjectType: 'ContentElement', id: 1}); + entry.reviewSession = factories.reviewSession({ + commentThreads: [ + {id: 1, subjectType: 'ContentElement', subjectId: 10, + comments: [{id: 100, body: 'On element', creatorName: 'Alice'}]}, + {id: 2, subjectType: 'Section', subjectId: 50, + comments: [{id: 200, body: 'On section', creatorName: 'Bob'}]} + ] + }); + + const view = new SelectionCommentsView({entry, editor}); + const {getByText, queryByText} = renderBackboneView(view); + + expect(getByText('On element')).toBeInTheDocument(); + + act(() => { + entry.set('selectedCommentsSubject', {subjectType: 'Section', id: 5}); + }); + + await waitFor(() => { + expect(getByText('On section')).toBeInTheDocument(); + }); + expect(queryByText('On element')).not.toBeInTheDocument(); + }); + + // The SELECTED message sets highlightedThreadId and + // selectedCommentsSubject in one go. The change:highlightedThreadId + // listener rerenders before change:selectedCommentsSubject refreshes + // the resolved model, so props must derive the model from the current + // subject rather than a stale cache. + it('does not crash switching from section to content element comments in one set', async () => { + const entry = createEntry({ + sections: [{id: 5, permaId: 50}], + contentElements: [{id: 1, permaId: 10, typeName: 'fixture', sectionId: 5}] + }); + entry.set('selectedCommentsSubject', {subjectType: 'Section', id: 5}); + entry.reviewSession = factories.reviewSession({ + commentThreads: [ + {id: 1, subjectType: 'Section', subjectId: 50, + comments: [{id: 100, body: 'On section', creatorName: 'Alice'}]}, + {id: 2, subjectType: 'ContentElement', subjectId: 10, + comments: [{id: 200, body: 'On element', creatorName: 'Bob'}]} + ] + }); + + const view = new SelectionCommentsView({entry, editor}); + const {getByText, queryByText} = renderBackboneView(view); + + expect(getByText('On section')).toBeInTheDocument(); + + act(() => { + entry.set({ + highlightedThreadId: 2, + selectedCommentsSubject: {subjectType: 'ContentElement', id: 1} + }); + }); + + await waitFor(() => { + expect(getByText('On element')).toBeInTheDocument(); + }); + expect(queryByText('On section')).not.toBeInTheDocument(); + }); }); diff --git a/entry_types/scrolled/package/spec/frontend/commenting/features/addCommentMode-spec.js b/entry_types/scrolled/package/spec/frontend/commenting/features/addCommentMode-spec.js index 9be7b6cfe5..1059ce2a9e 100644 --- a/entry_types/scrolled/package/spec/frontend/commenting/features/addCommentMode-spec.js +++ b/entry_types/scrolled/package/spec/frontend/commenting/features/addCommentMode-spec.js @@ -1,7 +1,6 @@ import React from 'react'; import '@testing-library/jest-dom/extend-expect'; import userEvent from '@testing-library/user-event'; -import {useFakeTranslations} from 'pageflow/testHelpers'; import {api} from 'frontend/api'; import {renderEntry, useCommentingPageObjects} from 'support/pageObjects/commenting'; @@ -9,10 +8,6 @@ import {renderEntry, useCommentingPageObjects} from 'support/pageObjects/comment describe('add comment mode', () => { useCommentingPageObjects(); - useFakeTranslations({ - 'pageflow_scrolled.review.cancel': 'Cancel' - }); - it('renders add comment button', () => { const entry = renderEntry({ seed: { @@ -86,24 +81,6 @@ describe('add comment mode', () => { expect(entry.getNewThreadInput()).toBeInTheDocument(); }); - it('closes popover when cancelling new thread form on element without threads', async () => { - const user = userEvent.setup(); - const entry = renderEntry({ - seed: { - contentElements: [{ - typeName: 'withTestId', - configuration: {testId: 5} - }] - } - }); - - await user.click(entry.getAddCommentButton()); - await user.click(entry.getContentElementByTestId(5).getSelectToCommentButton()); - await user.click(entry.getByRole('button', {name: 'Cancel'})); - - expect(entry.queryAllCommentBadges()).toHaveLength(0); - }); - it('exits add comment mode when overlay is clicked', async () => { const user = userEvent.setup(); const entry = renderEntry({ @@ -186,4 +163,19 @@ describe('add comment mode', () => { expect(contentElement.hasSelectToCommentButton()).toBe(false); }); + + it('opens new thread form when selecting a section', async () => { + const user = userEvent.setup(); + const entry = renderEntry({ + seed: { + sections: [{id: 1, permaId: 10}], + contentElements: [{typeName: 'withTestId', configuration: {testId: 5}}] + } + }); + + await user.click(entry.getAddCommentButton()); + await user.click(entry.getByRole('button', {name: 'Select section to comment'})); + + expect(entry.getNewThreadInput()).toBeInTheDocument(); + }); }); diff --git a/entry_types/scrolled/package/spec/frontend/commenting/features/commentingBadges-spec.js b/entry_types/scrolled/package/spec/frontend/commenting/features/commentingBadges-spec.js index 1162f3c6c2..e31aaf1ec9 100644 --- a/entry_types/scrolled/package/spec/frontend/commenting/features/commentingBadges-spec.js +++ b/entry_types/scrolled/package/spec/frontend/commenting/features/commentingBadges-spec.js @@ -55,4 +55,25 @@ describe('commenting badges', () => { expect(entry.getByText('Nice work')).toBeInTheDocument(); expect(entry.getByText('Bob')).toBeInTheDocument(); }); + + it('shows thread list when clicking a section badge', () => { + const entry = renderEntry({ + seed: { + sections: [{id: 1, permaId: 10}], + contentElements: [{typeName: 'withTestId', configuration: {testId: 5}}] + }, + commenting: { + currentUser: {id: 42, name: 'Alice'}, + commentThreads: [ + {id: 1, subjectType: 'Section', subjectId: 10, comments: [ + {id: 10, body: 'On the section', creatorName: 'Bob', creatorId: 2} + ]} + ] + } + }); + + fireEvent.click(entry.getAllCommentBadges()[0]); + + expect(entry.getByText('On the section')).toBeInTheDocument(); + }); }); diff --git a/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/features/commentBadges-spec.js b/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/features/commentBadges-spec.js index 17a63940f3..8c004e8392 100644 --- a/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/features/commentBadges-spec.js +++ b/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/features/commentBadges-spec.js @@ -4,6 +4,7 @@ import {features} from 'pageflow/frontend'; import {EditableText} from 'frontend'; import {renderEntry, useInlineEditingPageObjects} from 'support/pageObjects/inlineEditing'; +import {act, waitFor} from '@testing-library/react'; import '@testing-library/jest-dom/extend-expect'; describe('inline editing EditableText comment badges', () => { @@ -76,6 +77,45 @@ describe('inline editing EditableText comment badges', () => { expect(badges[1].isActive()).toBe(false); }); + it('renders a revealed resolved thread badge in resolved style', async () => { + const value = [{type: 'paragraph', children: [{text: 'Some text to comment on'}]}]; + + const entry = renderEntry({ + contentElement: { + ui: , + typeOptions: {inlineComments: true, customSelectionRect: true} + }, + commenting: { + currentUser: null, + commentThreads: [{ + id: 7, + subjectType: 'ContentElement', + subjectId: 10, + subjectRange: {anchor: {path: [0, 0], offset: 5}, focus: {path: [0, 0], offset: 9}}, + resolvedAt: '2026-06-01T00:00:00Z', + comments: [{id: 1, body: 'A comment', creatorName: 'Alice', creatorId: 1}] + }] + } + }); + + expect(entry.queryAllCommentBadges()).toHaveLength(0); + + act(() => { + window.dispatchEvent(new MessageEvent('message', { + data: {type: 'SELECT_COMMENT_THREAD', payload: {threadId: 7}}, + origin: window.location.origin + })); + }); + + await waitFor(() => { + expect(entry.queryAllCommentBadges()).toHaveLength(1); + }); + + const badge = entry.queryAllCommentBadges()[0]; + expect(badge.isResolved()).toBe(true); + expect(badge.isActive()).toBe(true); + }); + it('renders sibling badge in regular mode when in same block as highlighted thread', () => { const value = [ {type: 'paragraph', children: [{text: 'First paragraph with two threads'}]}, diff --git a/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/features/commentHighlights-spec.js b/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/features/commentHighlights-spec.js index bff7c586e6..f9fbe41b35 100644 --- a/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/features/commentHighlights-spec.js +++ b/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/features/commentHighlights-spec.js @@ -64,8 +64,8 @@ describe('inline editing EditableText comment highlights', () => { type: 'SELECT', payload: { type: 'newThread', - id: 10, subjectType: 'ContentElement', + subjectId: 10, range: subjectRange } }, '*'); @@ -81,6 +81,92 @@ describe('inline editing EditableText comment highlights', () => { expect(highlight).toHaveClass(highlightStyles.selected); }); + it('hides resolved thread highlights until the thread is selected', async () => { + const entry = renderEntry({ + contentElement: { + ui: , + typeOptions: {inlineComments: true, customSelectionRect: true} + }, + commenting: { + currentUser: null, + commentThreads: [{ + id: 7, + subjectType: 'ContentElement', + subjectId: 10, + subjectRange, + resolvedAt: '2026-06-01T00:00:00Z', + comments: [{id: 1, body: 'A comment', creatorName: 'Alice', creatorId: 1}] + }] + } + }); + + expect(entry.container.querySelector(`.${highlightStyles.highlight}`)) + .not.toBeInTheDocument(); + + act(() => { + window.postMessage({ + type: 'SELECT_COMMENT_THREAD', + payload: {threadId: 7} + }, '*'); + }); + + await waitFor(() => { + expect(entry.container.querySelector(`.${highlightStyles.highlight}`)) + .toBeInTheDocument(); + }); + + const highlight = entry.container.querySelector(`.${highlightStyles.highlight}`); + expect(highlight).toHaveClass(highlightStyles.resolved); + expect(highlight).not.toHaveClass(highlightStyles.selected); + }); + + it('keeps the resolved thread highlighted when a cursor sits in another block', () => { + const multiBlockValue = [ + {type: 'paragraph', children: [{text: 'First paragraph'}]}, + {type: 'paragraph', children: [{text: 'Second paragraph'}]} + ]; + + const entry = renderEntry({ + contentElement: { + ui: , + typeOptions: {inlineComments: true, customSelectionRect: true} + }, + commenting: { + currentUser: null, + commentThreads: [{ + id: 7, + subjectType: 'ContentElement', + subjectId: 10, + subjectRange: {anchor: {path: [1, 0], offset: 0}, focus: {path: [1, 0], offset: 6}}, + resolvedAt: '2026-06-01T00:00:00Z', + comments: [{id: 1, body: 'A comment', creatorName: 'Alice', creatorId: 1}] + }] + } + }); + + // The reviewer had clicked into the first paragraph, leaving a slate + // cursor there before opening the resolved comment from the sidebar. + act(() => { + window.dispatchEvent(new MessageEvent('message', { + data: {type: 'SELECT', payload: {type: 'contentElement', id: 1, range: [0, 1]}}, + origin: window.location.origin + })); + }); + + act(() => { + window.dispatchEvent(new MessageEvent('message', { + data: {type: 'SELECT_COMMENT_THREAD', payload: {threadId: 7}}, + origin: window.location.origin + })); + }); + + const highlight = entry.container.querySelector(`.${highlightStyles.highlight}`); + expect(highlight).toBeInTheDocument(); + expect(highlight).toHaveTextContent('Second'); + expect(highlight).toHaveClass(highlightStyles.resolved); + expect(highlight).not.toHaveClass(highlightStyles.selected); + }); + it('applies selected style to highlight when thread badge is clicked', () => { const entry = renderEntry({ contentElement: { diff --git a/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/features/commentSelection-spec.js b/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/features/commentSelection-spec.js index 7bc9bf1360..53862dcdd9 100644 --- a/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/features/commentSelection-spec.js +++ b/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/features/commentSelection-spec.js @@ -2,9 +2,10 @@ import React from 'react'; import {features} from 'pageflow/frontend'; import {EditableText} from 'frontend'; +import {useStartNewThread} from 'frontend/inlineEditing/EditableText/useStartNewThread'; import {renderEntry, useInlineEditingPageObjects} from 'support/pageObjects/inlineEditing'; -import {act} from '@testing-library/react'; +import {act, fireEvent, waitFor} from '@testing-library/react'; import '@testing-library/jest-dom/extend-expect'; describe('inline editing EditableText comment selection messages', () => { @@ -53,6 +54,34 @@ describe('inline editing EditableText comment selection messages', () => { }, expect.anything()); }); + it('posts SELECTED newThread with subject and range when starting a thread on a text range', () => { + const range = {anchor: {path: [0, 0], offset: 0}, focus: {path: [0, 0], offset: 5}}; + + function StartNewThreadButton() { + const startNewThread = useStartNewThread({selection: range}); + return ; + } + + const {getByText} = renderEntry({ + contentElement: { + ui: , + typeOptions: {inlineComments: true} + } + }); + + fireEvent.click(getByText('Start thread')); + + expect(window.parent.postMessage).toHaveBeenCalledWith({ + type: 'SELECTED', + payload: { + type: 'newThread', + subjectType: 'ContentElement', + subjectId: 10, + range + } + }, expect.anything()); + }); + it('runs badge click logic and scrolls into view on SELECT_COMMENT_THREAD message', async () => { const scrollIntoView = jest.fn(); Element.prototype.scrollIntoView = scrollIntoView; @@ -104,4 +133,85 @@ describe('inline editing EditableText comment selection messages', () => { expect(scrollIntoView).toHaveBeenCalled(); delete Element.prototype.scrollIntoView; }); + + it('includes resolved thread ids at the selection in transient state', async () => { + const value = [{type: 'paragraph', children: [{text: 'Some text to comment on'}]}]; + + renderEntry({ + contentElement: { + ui: , + typeOptions: {inlineComments: true, customSelectionRect: true} + }, + commenting: { + currentUser: null, + commentThreads: [ + {id: 1, subjectType: 'ContentElement', subjectId: 10, + subjectRange: {anchor: {path: [0, 0], offset: 0}, focus: {path: [0, 0], offset: 4}}, + comments: [{id: 10, body: 'Active', creatorName: 'Alice', creatorId: 1}]}, + {id: 7, subjectType: 'ContentElement', subjectId: 10, + subjectRange: {anchor: {path: [0, 0], offset: 5}, focus: {path: [0, 0], offset: 9}}, + resolvedAt: '2026-06-01T00:00:00Z', + comments: [{id: 20, body: 'Resolved', creatorName: 'Bob', creatorId: 2}]} + ] + } + }); + + act(() => { + window.dispatchEvent(new MessageEvent('message', { + data: {type: 'SELECT', payload: {type: 'contentElement', id: 1, range: [0, 1]}}, + origin: window.location.origin + })); + }); + + await waitFor(() => { + expect(window.parent.postMessage).toHaveBeenCalledWith( + expect.objectContaining({ + type: 'UPDATE_TRANSIENT_CONTENT_ELEMENT_STATE', + payload: expect.objectContaining({ + id: 1, + state: expect.objectContaining({ + commentThreadIdsAtSelection: expect.arrayContaining([1, 7]) + }) + }) + }), + expect.anything() + ); + }); + }); + + it('scrolls a resolved thread into view when selected via SELECT_COMMENT_THREAD', () => { + const scrollIntoView = jest.fn(); + Element.prototype.scrollIntoView = scrollIntoView; + + 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'}]}]; + + renderEntry({ + contentElement: { + ui: , + typeOptions: {inlineComments: true, customSelectionRect: true} + }, + commenting: { + currentUser: null, + commentThreads: [{ + id: 9, + subjectType: 'ContentElement', + subjectId: 10, + subjectRange, + resolvedAt: '2026-06-01T00:00:00Z', + comments: [{id: 1, body: 'A comment', creatorName: 'Alice', creatorId: 1}] + }] + } + }); + + act(() => { + window.dispatchEvent(new MessageEvent('message', { + data: {type: 'SELECT_COMMENT_THREAD', payload: {threadId: 9}}, + origin: window.location.origin + })); + }); + + expect(scrollIntoView).toHaveBeenCalled(); + delete Element.prototype.scrollIntoView; + }); }); diff --git a/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/useCommenting-spec.js b/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/useCommenting-spec.js new file mode 100644 index 0000000000..56f962bd1e --- /dev/null +++ b/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/useCommenting-spec.js @@ -0,0 +1,81 @@ +import React from 'react'; +import {renderHook, act} from '@testing-library/react-hooks'; +import {createEditor, Transforms} from 'slate'; +import {withReact} from 'slate-react'; + +import {features} from 'pageflow/frontend'; +import {ReviewStateProvider} from 'pageflow-scrolled/review'; +import {ContentElementAttributesProvider} from 'frontend/useContentElementAttributes'; +import {EditorStateProvider} from 'frontend/inlineEditing/EditorState'; + +import {useCommenting} from 'frontend/inlineEditing/EditableText/useCommenting'; + +describe('useCommenting', () => { + beforeEach(() => { + jest.spyOn(features, 'isEnabled').mockImplementation( + name => name === 'commenting' + ); + }); + + afterEach(() => { + features.isEnabled.mockRestore(); + }); + + function setup({commentThreads}) { + const editor = withReact(createEditor()); + editor.children = [ + {type: 'paragraph', children: [{text: 'Hello world'}]} + ]; + + const wrapper = ({children}) => ( + + + + {children} + + + + ); + + const {result} = renderHook(() => useCommenting(editor), {wrapper}); + + return {editor, result}; + } + + it('tracks both resolved and unresolved thread ranges across edits', () => { + // Thread 7 is resolved: it must be tracked just like the unresolved + // thread 1 so its range stays correct once the thread is reopened. + const {editor, result} = setup({ + commentThreads: [ + { + id: 1, + subjectType: 'ContentElement', + subjectId: 10, + subjectRange: {anchor: {path: [0, 0], offset: 2}, + focus: {path: [0, 0], offset: 5}}, + comments: [] + }, + { + id: 7, + subjectType: 'ContentElement', + subjectId: 10, + subjectRange: {anchor: {path: [0, 0], offset: 6}, + focus: {path: [0, 0], offset: 11}}, + resolvedAt: '2026-06-01T00:00:00Z', + comments: [] + } + ] + }); + + act(() => { + Transforms.insertText(editor, 'X', {at: {path: [0, 0], offset: 0}}); + }); + + expect(result.current.getTrackedSubjectRanges()).toEqual({ + 1: {anchor: {path: [0, 0], offset: 3}, + focus: {path: [0, 0], offset: 6}}, + 7: {anchor: {path: [0, 0], offset: 7}, + focus: {path: [0, 0], offset: 12}} + }); + }); +}); diff --git a/entry_types/scrolled/package/spec/frontend/inlineEditing/features/commentBadges-spec.js b/entry_types/scrolled/package/spec/frontend/inlineEditing/features/commentBadges-spec.js deleted file mode 100644 index 8d45206941..0000000000 --- a/entry_types/scrolled/package/spec/frontend/inlineEditing/features/commentBadges-spec.js +++ /dev/null @@ -1,85 +0,0 @@ -import '@testing-library/jest-dom/extend-expect'; -import {act, waitFor} from '@testing-library/react'; -import {useFakeFeatures} from 'pageflow/testHelpers'; - -import {useInlineEditingPageObjects, renderEntry} from 'support/pageObjects/inlineEditing'; - -import badgeStyles from 'review/Badge.module.css'; - -describe('inline editing comment badges', () => { - useInlineEditingPageObjects(); - useFakeFeatures('frontend', ['commenting']); - - it('does not display comment icon when element is not selected', () => { - const {queryByRole} = renderEntry({ - seed: { - contentElements: [{ - typeName: 'withTestId', - permaId: 10, - configuration: {testId: 5} - }] - } - }); - - expect(queryByRole('status')).not.toBeInTheDocument(); - }); - - it('displays dot badge when threads exist and element is not selected', async () => { - const {getByRole} = renderEntry({ - seed: { - contentElements: [{ - typeName: 'withTestId', - permaId: 10, - configuration: {testId: 5} - }] - } - }); - - act(() => { - window.dispatchEvent(new MessageEvent('message', { - data: { - type: 'REVIEW_STATE_RESET', - payload: { - currentUser: {id: 1}, - commentThreads: [{ - id: 1, - subjectType: 'ContentElement', - subjectId: 10, - comments: [{id: 100, body: 'Review this'}] - }] - } - }, - origin: window.location.origin - })); - }); - - await waitFor(() => { - expect(getByRole('status')).toBeInTheDocument(); - 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/inlineEditing/features/contentElementCommentBadges-spec.js b/entry_types/scrolled/package/spec/frontend/inlineEditing/features/contentElementCommentBadges-spec.js new file mode 100644 index 0000000000..6f38ba2493 --- /dev/null +++ b/entry_types/scrolled/package/spec/frontend/inlineEditing/features/contentElementCommentBadges-spec.js @@ -0,0 +1,236 @@ +import '@testing-library/jest-dom/extend-expect'; +import {act, waitFor} from '@testing-library/react'; +import {useFakeFeatures} from 'pageflow/testHelpers'; + +import {useInlineEditingPageObjects, renderEntry} from 'support/pageObjects/inlineEditing'; + +import badgeStyles from 'review/Badge.module.css'; + +describe('inline editing content element comment badges', () => { + useInlineEditingPageObjects(); + useFakeFeatures('frontend', ['commenting']); + + it('does not display comment icon when element is not selected', () => { + const {queryByRole} = renderEntry({ + seed: { + contentElements: [{ + typeName: 'withTestId', + permaId: 10, + configuration: {testId: 5} + }] + } + }); + + expect(queryByRole('status')).not.toBeInTheDocument(); + }); + + it('displays dot badge when threads exist and element is not selected', async () => { + const {getByRole} = renderEntry({ + seed: { + contentElements: [{ + typeName: 'withTestId', + permaId: 10, + configuration: {testId: 5} + }] + } + }); + + act(() => { + window.dispatchEvent(new MessageEvent('message', { + data: { + type: 'REVIEW_STATE_RESET', + payload: { + currentUser: {id: 1}, + commentThreads: [{ + id: 1, + subjectType: 'ContentElement', + subjectId: 10, + comments: [{id: 100, body: 'Review this'}] + }] + } + }, + origin: window.location.origin + })); + }); + + await waitFor(() => { + expect(getByRole('status')).toBeInTheDocument(); + 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', subjectType: 'ContentElement', subjectId: 10} + }, + origin: window.location.origin + })); + }); + + expect(getByRole('status')).toHaveClass(badgeStyles.active); + }); + + it('selects the thread and scrolls into view on SELECT_COMMENT_THREAD', async () => { + const scrollIntoView = jest.fn(); + Element.prototype.scrollIntoView = scrollIntoView; + + const {getByRole} = renderEntry({ + seed: { + contentElements: [{ + id: 1, + typeName: 'withTestId', + permaId: 10, + configuration: {testId: 5} + }] + } + }); + + act(() => { + window.dispatchEvent(new MessageEvent('message', { + data: { + type: 'REVIEW_STATE_RESET', + payload: { + currentUser: {id: 1}, + commentThreads: [{ + id: 7, + subjectType: 'ContentElement', + subjectId: 10, + comments: [{id: 100, body: 'Review this'}] + }] + } + }, + origin: window.location.origin + })); + }); + + await waitFor(() => { + expect(getByRole('status')).toBeInTheDocument(); + }); + + act(() => { + window.dispatchEvent(new MessageEvent('message', { + data: {type: 'SELECT_COMMENT_THREAD', payload: {threadId: 7}}, + origin: window.location.origin + })); + }); + + expect(window.parent.postMessage).toHaveBeenCalledWith({ + type: 'SELECTED', + payload: {type: 'contentElementComments', id: 1, highlightedThreadId: 7} + }, expect.anything()); + expect(scrollIntoView).toHaveBeenCalled(); + + delete Element.prototype.scrollIntoView; + }); + + it('reveals a resolved thread on SELECT_COMMENT_THREAD even without a badge', () => { + const scrollIntoView = jest.fn(); + Element.prototype.scrollIntoView = scrollIntoView; + + const {queryByRole} = renderEntry({ + seed: { + contentElements: [{ + id: 1, + typeName: 'withTestId', + permaId: 10, + configuration: {testId: 5} + }] + } + }); + + act(() => { + window.dispatchEvent(new MessageEvent('message', { + data: { + type: 'REVIEW_STATE_RESET', + payload: { + currentUser: {id: 1}, + commentThreads: [{ + id: 7, + subjectType: 'ContentElement', + subjectId: 10, + resolvedAt: '2026-06-01T00:00:00Z', + comments: [{id: 100, body: 'Resolved'}] + }] + } + }, + origin: window.location.origin + })); + }); + + // Resolved threads are not counted by the badge. + expect(queryByRole('status')).not.toBeInTheDocument(); + + act(() => { + window.dispatchEvent(new MessageEvent('message', { + data: {type: 'SELECT_COMMENT_THREAD', payload: {threadId: 7}}, + origin: window.location.origin + })); + }); + + expect(window.parent.postMessage).toHaveBeenCalledWith({ + type: 'SELECTED', + payload: {type: 'contentElementComments', id: 1, highlightedThreadId: 7} + }, expect.anything()); + expect(scrollIntoView).toHaveBeenCalled(); + + delete Element.prototype.scrollIntoView; + }); + + it('ignores SELECT_COMMENT_THREAD for a thread of another subject', () => { + renderEntry({ + seed: { + contentElements: [{ + id: 1, + typeName: 'withTestId', + permaId: 10, + configuration: {testId: 5} + }] + } + }); + + act(() => { + window.dispatchEvent(new MessageEvent('message', { + data: { + type: 'REVIEW_STATE_RESET', + payload: { + currentUser: {id: 1}, + commentThreads: [{ + id: 7, + subjectType: 'ContentElement', + subjectId: 10, + comments: [{id: 100, body: 'Review this'}] + }] + } + }, + origin: window.location.origin + })); + }); + + window.parent.postMessage.mockClear(); + + act(() => { + window.dispatchEvent(new MessageEvent('message', { + data: {type: 'SELECT_COMMENT_THREAD', payload: {threadId: 999}}, + origin: window.location.origin + })); + }); + + expect(window.parent.postMessage).not.toHaveBeenCalledWith( + expect.objectContaining({type: 'SELECTED'}), + expect.anything() + ); + }); +}); diff --git a/entry_types/scrolled/package/spec/frontend/inlineEditing/features/forcedSectionPadding-spec.js b/entry_types/scrolled/package/spec/frontend/inlineEditing/features/forcedSectionPadding-spec.js index b8fda7bb63..af31cc6674 100644 --- a/entry_types/scrolled/package/spec/frontend/inlineEditing/features/forcedSectionPadding-spec.js +++ b/entry_types/scrolled/package/spec/frontend/inlineEditing/features/forcedSectionPadding-spec.js @@ -1,12 +1,26 @@ import {renderEntry, useInlineEditingPageObjects} from 'support/pageObjects/inlineEditing'; import {useSectionMatchers} from 'support/matchers'; +import {features} from 'pageflow/frontend'; +import {act, fireEvent} from '@testing-library/react'; import '@testing-library/jest-dom/extend-expect'; describe('inline editing forced section padding', () => { useInlineEditingPageObjects(); useSectionMatchers(); + function resetReviewStateWith(commentThreads) { + act(() => { + window.dispatchEvent(new MessageEvent('message', { + data: { + type: 'REVIEW_STATE_RESET', + payload: {currentUser: {id: 1}, commentThreads} + }, + origin: window.location.origin + })); + }); + } + it('forces padding below full width element if section is selected', () => { const {getSectionByPermaId} = renderEntry({ seed: { @@ -38,6 +52,37 @@ describe('inline editing forced section padding', () => { expect(getSectionByPermaId(6)).toHaveForcedPadding(); }); + it('forces padding when section comments are selected', () => { + features.enable('frontend', ['commenting']); + + const {getByRole, getSectionByPermaId} = renderEntry({ + seed: {sections: [{id: 5, permaId: 6}]} + }); + + resetReviewStateWith([{ + id: 1, subjectType: 'Section', subjectId: 6, + comments: [{id: 100, body: 'Review this'}] + }]); + + fireEvent.click(getByRole('status')); + + expect(getSectionByPermaId(6)).toHaveForcedPadding(); + }); + + it('forces padding when a new thread on the section is selected', () => { + features.enable('frontend', ['commenting']); + + const {getByRole, getSectionByPermaId} = renderEntry({ + seed: {sections: [{id: 5, permaId: 6}]} + }); + + // Reveal the section's icon badge, then start a new thread from it. + getSectionByPermaId(6).select(); + fireEvent.click(getByRole('status')); + + expect(getSectionByPermaId(6)).toHaveForcedPadding(); + }); + it('does not force padding if padding is selected', () => { const {getSectionByPermaId} = renderEntry({ seed: { diff --git a/entry_types/scrolled/package/spec/frontend/inlineEditing/features/sectionCommentBadges-spec.js b/entry_types/scrolled/package/spec/frontend/inlineEditing/features/sectionCommentBadges-spec.js new file mode 100644 index 0000000000..117cd64b5f --- /dev/null +++ b/entry_types/scrolled/package/spec/frontend/inlineEditing/features/sectionCommentBadges-spec.js @@ -0,0 +1,253 @@ +import '@testing-library/jest-dom/extend-expect'; +import {act, waitFor} from '@testing-library/react'; +import {useFakeFeatures} from 'pageflow/testHelpers'; + +import {useInlineEditingPageObjects, renderEntry} from 'support/pageObjects/inlineEditing'; + +import badgeStyles from 'review/Badge.module.css'; +import sectionStyles from 'frontend/inlineEditing/SectionDecorator.module.css'; + +describe('inline editing section comment badges', () => { + useInlineEditingPageObjects(); + useFakeFeatures('frontend', ['commenting']); + + it('does not display comment icon when section is not selected', () => { + const {queryByRole} = renderEntry({ + seed: { + sections: [{id: 1, permaId: 10}], + contentElements: [{sectionId: 1, permaId: 100}] + } + }); + + expect(queryByRole('status')).not.toBeInTheDocument(); + }); + + it('displays dot badge when threads exist and section is not selected', async () => { + const {getByRole} = renderEntry({ + seed: { + sections: [{id: 1, permaId: 10}], + contentElements: [{sectionId: 1, permaId: 100}] + } + }); + + act(() => { + window.dispatchEvent(new MessageEvent('message', { + data: { + type: 'REVIEW_STATE_RESET', + payload: { + currentUser: {id: 1}, + commentThreads: [{ + id: 1, + subjectType: 'Section', + subjectId: 10, + comments: [{id: 100, body: 'Review this'}] + }] + } + }, + origin: window.location.origin + })); + }); + + await waitFor(() => { + expect(getByRole('status')).toBeInTheDocument(); + expect(getByRole('status')).not.toHaveTextContent(/\d/); + }); + }); + + it('renders badge in active mode when newThread is selected on the section', () => { + const {getByRole} = renderEntry({ + seed: { + sections: [{id: 1, permaId: 10}], + contentElements: [{sectionId: 1, permaId: 100}] + } + }); + + act(() => { + window.dispatchEvent(new MessageEvent('message', { + data: { + type: 'SELECT', + payload: {type: 'newThread', subjectType: 'Section', subjectId: 10} + }, + origin: window.location.origin + })); + }); + + expect(getByRole('status')).toHaveClass(badgeStyles.active); + }); + + it('keeps the badge sticky when section comments are selected without threads', () => { + const {getByRole} = renderEntry({ + seed: { + sections: [{id: 1, permaId: 10}], + contentElements: [{sectionId: 1, permaId: 100}] + } + }); + + act(() => { + window.dispatchEvent(new MessageEvent('message', { + data: { + type: 'SELECT', + payload: {type: 'sectionComments', id: 1} + }, + origin: window.location.origin + })); + }); + + expect(getByRole('status').closest(`.${sectionStyles.commentBadge}`)) + .toHaveClass(sectionStyles.sticky); + }); + + it('renders the section as selected when its comments are selected', () => { + const {getByLabelText} = renderEntry({ + seed: { + sections: [{id: 1, permaId: 10}], + contentElements: [{sectionId: 1, permaId: 100}] + } + }); + + act(() => { + window.dispatchEvent(new MessageEvent('message', { + data: { + type: 'SELECT', + payload: {type: 'sectionComments', id: 1} + }, + origin: window.location.origin + })); + }); + + expect(getByLabelText('Select section')).toHaveClass(sectionStyles.selected); + }); + + it('renders the section as selected when a new thread on it is selected', () => { + const {getByLabelText} = renderEntry({ + seed: { + sections: [{id: 1, permaId: 10}], + contentElements: [{sectionId: 1, permaId: 100}] + } + }); + + act(() => { + window.dispatchEvent(new MessageEvent('message', { + data: { + type: 'SELECT', + payload: {type: 'newThread', subjectType: 'Section', subjectId: 10} + }, + origin: window.location.origin + })); + }); + + expect(getByLabelText('Select section')).toHaveClass(sectionStyles.selected); + }); + + it('keeps the badge sticky when a new thread on the section is selected', () => { + const {getByRole} = renderEntry({ + seed: { + sections: [{id: 1, permaId: 10}], + contentElements: [{sectionId: 1, permaId: 100}] + } + }); + + act(() => { + window.dispatchEvent(new MessageEvent('message', { + data: { + type: 'SELECT', + payload: {type: 'newThread', subjectType: 'Section', subjectId: 10} + }, + origin: window.location.origin + })); + }); + + expect(getByRole('status').closest(`.${sectionStyles.commentBadge}`)) + .toHaveClass(sectionStyles.sticky); + }); + + it('scrolls the section into view on SELECT_COMMENT_THREAD for one of its threads', async () => { + const {getByRole, getByLabelText} = renderEntry({ + seed: { + sections: [{id: 1, permaId: 10}], + contentElements: [{sectionId: 1, permaId: 100}] + } + }); + + act(() => { + window.dispatchEvent(new MessageEvent('message', { + data: { + type: 'REVIEW_STATE_RESET', + payload: { + currentUser: {id: 1}, + commentThreads: [{ + id: 1, subjectType: 'Section', subjectId: 10, + comments: [{id: 100, body: 'Review this'}] + }] + } + }, + origin: window.location.origin + })); + }); + + await waitFor(() => { + expect(getByRole('status')).toBeInTheDocument(); + }); + + const section = getByLabelText('Select section'); + section.scrollIntoView = jest.fn(); + + act(() => { + window.dispatchEvent(new MessageEvent('message', { + data: {type: 'SELECT_COMMENT_THREAD', payload: {threadId: 1}}, + origin: window.location.origin + })); + }); + + expect(section.scrollIntoView).toHaveBeenCalled(); + expect(window.parent.postMessage).toHaveBeenCalledWith({ + type: 'SELECTED', + payload: {type: 'sectionComments', id: 1, highlightedThreadId: 1} + }, expect.anything()); + }); + + it('reveals a resolved section thread on SELECT_COMMENT_THREAD even without a badge', () => { + const {queryByRole, getByLabelText} = renderEntry({ + seed: { + sections: [{id: 1, permaId: 10}], + contentElements: [{sectionId: 1, permaId: 100}] + } + }); + + act(() => { + window.dispatchEvent(new MessageEvent('message', { + data: { + type: 'REVIEW_STATE_RESET', + payload: { + currentUser: {id: 1}, + commentThreads: [{ + id: 3, subjectType: 'Section', subjectId: 10, + resolvedAt: '2026-06-01T00:00:00Z', + comments: [{id: 100, body: 'Resolved'}] + }] + } + }, + origin: window.location.origin + })); + }); + + // Resolved threads are not counted by the badge. + expect(queryByRole('status')).not.toBeInTheDocument(); + + const section = getByLabelText('Select section'); + section.scrollIntoView = jest.fn(); + + act(() => { + window.dispatchEvent(new MessageEvent('message', { + data: {type: 'SELECT_COMMENT_THREAD', payload: {threadId: 3}}, + origin: window.location.origin + })); + }); + + expect(section.scrollIntoView).toHaveBeenCalled(); + expect(window.parent.postMessage).toHaveBeenCalledWith({ + type: 'SELECTED', + payload: {type: 'sectionComments', id: 1, highlightedThreadId: 3} + }, expect.anything()); + }); +}); diff --git a/entry_types/scrolled/package/spec/frontend/inlineEditing/features/selectedMessage-spec.js b/entry_types/scrolled/package/spec/frontend/inlineEditing/features/selectedMessage-spec.js index 41f220067f..377fd6fa5d 100644 --- a/entry_types/scrolled/package/spec/frontend/inlineEditing/features/selectedMessage-spec.js +++ b/entry_types/scrolled/package/spec/frontend/inlineEditing/features/selectedMessage-spec.js @@ -199,7 +199,7 @@ describe('inline editing SELECTED message', () => { expect(window.parent.postMessage).toHaveBeenCalledWith({ type: 'SELECTED', - payload: {type: 'newThread', id: 10} + payload: {type: 'newThread', subjectType: 'ContentElement', subjectId: 10} }, expect.anything()); }); @@ -241,4 +241,106 @@ describe('inline editing SELECTED message', () => { payload: {id: 1, type: 'contentElementComments'} }, expect.anything()); }); + + it('is posted with type newThread when section comment badge is clicked and no threads exist', () => { + features.enable('frontend', ['commenting']); + + const {getByRole, getSectionByPermaId} = renderEntry({ + seed: { + sections: [{id: 1, permaId: 10}], + contentElements: [{sectionId: 1, permaId: 100}] + } + }); + + // Select the section so the icon-mode badge becomes visible even + // without threads. + getSectionByPermaId(10).select(); + + fireEvent.click(getByRole('status')); + + expect(window.parent.postMessage).toHaveBeenCalledWith({ + type: 'SELECTED', + payload: {type: 'newThread', subjectType: 'Section', subjectId: 10} + }, expect.anything()); + }); + + it('is posted with type sectionComments when section comment badge is clicked', async () => { + features.enable('frontend', ['commenting']); + + const {getByRole} = renderEntry({ + seed: { + sections: [{id: 1, permaId: 10}], + contentElements: [{sectionId: 1, permaId: 100}] + } + }); + + act(() => { + window.dispatchEvent(new MessageEvent('message', { + data: { + type: 'REVIEW_STATE_RESET', + payload: { + currentUser: {id: 1}, + commentThreads: [{ + id: 1, + subjectType: 'Section', + subjectId: 10, + comments: [{id: 100, body: 'Review this'}] + }] + } + }, + origin: window.location.origin + })); + }); + + await waitFor(() => { + expect(getByRole('status')).toBeInTheDocument(); + }); + + fireEvent.click(getByRole('status')); + + expect(window.parent.postMessage).toHaveBeenCalledWith({ + type: 'SELECTED', + payload: {id: 1, type: 'sectionComments'} + }, expect.anything()); + }); + + it('does not select the section when its comment badge is pressed', async () => { + features.enable('frontend', ['commenting']); + + const {getByRole} = renderEntry({ + seed: { + sections: [{id: 1, permaId: 10}], + contentElements: [{sectionId: 1, permaId: 100}] + } + }); + + act(() => { + window.dispatchEvent(new MessageEvent('message', { + data: { + type: 'REVIEW_STATE_RESET', + payload: { + currentUser: {id: 1}, + commentThreads: [{ + id: 1, + subjectType: 'Section', + subjectId: 10, + comments: [{id: 100, body: 'Review this'}] + }] + } + }, + origin: window.location.origin + })); + }); + + await waitFor(() => { + expect(getByRole('status')).toBeInTheDocument(); + }); + + fireEvent.mouseDown(getByRole('status')); + + expect(window.parent.postMessage).not.toHaveBeenCalledWith({ + type: 'SELECTED', + payload: {id: 1, type: 'sectionSettings'} + }, expect.anything()); + }); }); diff --git a/entry_types/scrolled/package/spec/frontend/inlineEditing/features/useContentElementEditorState-spec.js b/entry_types/scrolled/package/spec/frontend/inlineEditing/features/useContentElementEditorState-spec.js index c00ed50241..a7a9ffb29b 100644 --- a/entry_types/scrolled/package/spec/frontend/inlineEditing/features/useContentElementEditorState-spec.js +++ b/entry_types/scrolled/package/spec/frontend/inlineEditing/features/useContentElementEditorState-spec.js @@ -67,7 +67,7 @@ describe('inline editing useContentElementEditorState', () => { expect(captured.type).toBe('contentElementComments'); expect(captured.isSelected).toBe(true); - act(() => setSelectionRef({type: 'newThread', id: 10})); + act(() => setSelectionRef({type: 'newThread', subjectType: 'ContentElement', subjectId: 10})); expect(captured.type).toBe('newThread'); expect(captured.isSelected).toBe(true); }); diff --git a/entry_types/scrolled/package/spec/review/ThreadList-spec.js b/entry_types/scrolled/package/spec/review/ThreadList-spec.js index 2576edf6bf..7268552a4f 100644 --- a/entry_types/scrolled/package/spec/review/ThreadList-spec.js +++ b/entry_types/scrolled/package/spec/review/ThreadList-spec.js @@ -12,7 +12,6 @@ describe('ThreadList', () => { 'pageflow_scrolled.review.reply_count.other': '%{count} replies', 'pageflow_scrolled.review.add_comment_placeholder': 'Add a comment...', 'pageflow_scrolled.review.new_topic': 'New topic', - 'pageflow_scrolled.review.cancel': 'Cancel', 'pageflow_scrolled.review.reply_placeholder': 'Reply...', 'pageflow_scrolled.review.send': 'Send', 'pageflow_scrolled.review.enter_for_new_line': 'Enter for new line', @@ -527,6 +526,46 @@ describe('ThreadList', () => { postMessage.mockRestore(); }); + it('submits new thread on Ctrl+Enter', async () => { + const user = userEvent.setup(); + const postMessage = jest.spyOn(window.top, 'postMessage').mockImplementation(() => {}); + + const {getByPlaceholderText} = renderWithReviewState( + + ); + + await user.type(getByPlaceholderText('Add a comment...'), + 'Quick thread{Control>}{Enter}{/Control}'); + + expect(postMessage).toHaveBeenCalledWith( + { + type: 'CREATE_COMMENT_THREAD', + payload: { + subjectType: 'ContentElement', + subjectId: 10, + body: 'Quick thread' + } + }, + window.location.origin + ); + + postMessage.mockRestore(); + }); + + it('shows keyboard hint while composing a new thread', async () => { + const user = userEvent.setup(); + + const {getByPlaceholderText, getByText, queryByText} = renderWithReviewState( + + ); + + expect(queryByText('Enter for new line')).not.toBeInTheDocument(); + + await user.type(getByPlaceholderText('Add a comment...'), 'Draft'); + + expect(getByText('Enter for new line')).toBeInTheDocument(); + }); + it('shows new topic form automatically when no threads exist', () => { const {getByPlaceholderText} = renderWithReviewState( @@ -654,10 +693,10 @@ describe('ThreadList', () => { expect(getByPlaceholderText('Add a comment...')).toBeInTheDocument(); }); - it('hides form when cancel is clicked', async () => { + it('does not show a cancel button after expanding the new thread form', async () => { const user = userEvent.setup(); - const {getByRole, queryByPlaceholderText} = renderWithReviewState( + const {getByRole, queryByRole, getByPlaceholderText} = renderWithReviewState( , { commentThreads: [ @@ -669,10 +708,9 @@ describe('ThreadList', () => { ); await user.click(getByRole('button', {name: 'New topic'})); - await user.click(getByRole('button', {name: 'Cancel'})); - expect(queryByPlaceholderText('Add a comment...')).not.toBeInTheDocument(); - expect(getByRole('button', {name: 'New topic'})).toBeInTheDocument(); + expect(getByPlaceholderText('Add a comment...')).toBeInTheDocument(); + expect(queryByRole('button', {name: 'Cancel'})).not.toBeInTheDocument(); }); it('posts create comment message when replying to thread', async () => { @@ -704,6 +742,35 @@ describe('ThreadList', () => { postMessage.mockRestore(); }); + it('submits reply on Ctrl+Enter', async () => { + const user = userEvent.setup(); + const postMessage = jest.spyOn(window.top, 'postMessage').mockImplementation(() => {}); + + const {getByPlaceholderText} = renderWithReviewState( + , + { + commentThreads: [ + {id: 1, subjectType: 'ContentElement', subjectId: 10, comments: [ + {id: 10, body: 'Start', creatorName: 'Bob', creatorId: 2} + ]} + ] + } + ); + + await user.type(getByPlaceholderText('Reply...'), + 'Quick reply{Control>}{Enter}{/Control}'); + + expect(postMessage).toHaveBeenCalledWith( + { + type: 'CREATE_COMMENT', + payload: {threadId: 1, body: 'Quick reply'} + }, + window.location.origin + ); + + postMessage.mockRestore(); + }); + it('hides reply submit button when textarea is empty', () => { const {queryByRole, getByPlaceholderText} = renderWithReviewState( , diff --git a/entry_types/scrolled/package/spec/review/ThreadsBadge-spec.js b/entry_types/scrolled/package/spec/review/ThreadsBadge-spec.js index 908d918099..f088df649b 100644 --- a/entry_types/scrolled/package/spec/review/ThreadsBadge-spec.js +++ b/entry_types/scrolled/package/spec/review/ThreadsBadge-spec.js @@ -3,7 +3,6 @@ import '@testing-library/jest-dom/extend-expect'; import {ThreadsBadge} from 'review/ThreadsBadge'; import {renderWithReviewState} from 'support/renderWithReviewState'; -import {fakeParentWindow} from 'support'; describe('ThreadsBadge', () => { it('does not display count for single thread', () => { @@ -147,70 +146,4 @@ 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/review/commentHighlights-spec.js b/entry_types/scrolled/package/spec/review/commentHighlights-spec.js new file mode 100644 index 0000000000..043e0acbec --- /dev/null +++ b/entry_types/scrolled/package/spec/review/commentHighlights-spec.js @@ -0,0 +1,50 @@ +import {createEditor} from 'slate'; + +import {decorateCommentHighlights} from 'pageflow-scrolled/review'; + +describe('decorateCommentHighlights', () => { + it('flags both ends of a highlight within a single text node', () => { + const editor = createEditor(); + editor.children = [ + {type: 'paragraph', children: [{text: 'Some text to comment on'}]} + ]; + + const decorate = decorateCommentHighlights(editor, [{ + key: '1', + range: { + anchor: {path: [0, 0], offset: 5}, + focus: {path: [0, 0], offset: 9} + } + }]); + + const [decoration] = decorate([editor.children[0].children[0], [0, 0]]); + + expect(decoration.firstInRange).toBe(true); + expect(decoration.lastInRange).toBe(true); + }); + + it('flags first and last node separately for a multi node highlight', () => { + const editor = createEditor(); + editor.children = [ + {type: 'paragraph', children: [{text: 'First paragraph'}]}, + {type: 'paragraph', children: [{text: 'Second paragraph'}]} + ]; + + const decorate = decorateCommentHighlights(editor, [{ + key: '1', + range: { + anchor: {path: [0, 0], offset: 2}, + focus: {path: [1, 0], offset: 3} + } + }]); + + const [firstDecoration] = decorate([editor.children[0].children[0], [0, 0]]); + const [lastDecoration] = decorate([editor.children[1].children[0], [1, 0]]); + + expect(firstDecoration.firstInRange).toBe(true); + expect(firstDecoration.lastInRange).toBeUndefined(); + + expect(lastDecoration.firstInRange).toBeUndefined(); + expect(lastDecoration.lastInRange).toBe(true); + }); +}); diff --git a/entry_types/scrolled/package/spec/support/pageObjects/commenting.js b/entry_types/scrolled/package/spec/support/pageObjects/commenting.js index 2cf7b58e70..95799971e6 100644 --- a/entry_types/scrolled/package/spec/support/pageObjects/commenting.js +++ b/entry_types/scrolled/package/spec/support/pageObjects/commenting.js @@ -45,6 +45,7 @@ export function useCommentingPageObjects() { 'pageflow_scrolled.review.add_comment': 'Add comment', 'pageflow_scrolled.review.cancel_add_comment': 'Cancel add comment', 'pageflow_scrolled.review.select_content_element': 'Select to comment', + 'pageflow_scrolled.review.select_section': 'Select section to comment', 'pageflow_scrolled.review.add_comment_placeholder': 'Add a comment...' }); diff --git a/entry_types/scrolled/package/spec/support/pageObjects/inlineEditing.js b/entry_types/scrolled/package/spec/support/pageObjects/inlineEditing.js index 4a0ef5f91f..5829320f4b 100644 --- a/entry_types/scrolled/package/spec/support/pageObjects/inlineEditing.js +++ b/entry_types/scrolled/package/spec/support/pageObjects/inlineEditing.js @@ -147,6 +147,7 @@ function createCommentBadgePageObject(el) { el, isInDotMode: () => el.classList.contains(badgeStyles.dot), isActive: () => el.classList.contains(badgeStyles.active), + isResolved: () => el.classList.contains(badgeStyles.resolved), select: () => fireEvent.click(el) }; } diff --git a/entry_types/scrolled/package/src/editor/controllers/PreviewMessageController.js b/entry_types/scrolled/package/src/editor/controllers/PreviewMessageController.js index 5a482e3106..5355635100 100644 --- a/entry_types/scrolled/package/src/editor/controllers/PreviewMessageController.js +++ b/entry_types/scrolled/package/src/editor/controllers/PreviewMessageController.js @@ -166,22 +166,18 @@ export const PreviewMessageController = Object.extend({ }); } else if (message.data.type === 'SELECTED') { - const {type, id, position} = message.data.payload; + const {type, id, subjectType, subjectId, 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 + selectedCommentsSubject: + selectedCommentsSubjectFor(this.entry, message.data.payload) }); - if (type === 'contentElementComments') { + if (type === 'contentElementComments' || type === 'sectionComments') { // Stay on the current tab when the user is already on the // comments route — only force the selection tab when arriving // there from elsewhere. @@ -190,11 +186,11 @@ export const PreviewMessageController = Object.extend({ } } else if (type === 'newThread') { - const {subjectType, range} = message.data.payload; + const {range} = message.data.payload; const payload = encodeURIComponent(JSON.stringify({subjectRange: range})); this.editor.navigate( `/scrolled/comment_threads/new?subjectType=${subjectType}` + - `&subjectId=${id}&payload=${payload}`, + `&subjectId=${subjectId}&payload=${payload}`, {trigger: true} ); } @@ -300,3 +296,25 @@ export const PreviewMessageController = Object.extend({ } } }); + +function selectedCommentsSubjectFor(entry, payload) { + const {type, id, subjectType, subjectId} = payload; + + if (type === 'contentElement' || type === 'contentElementComments') { + return {subjectType: 'ContentElement', id}; + } + + if (type === 'sectionComments' || + type === 'sectionSettings' || + type === 'sectionPaddings' || + type === 'sectionTransition') { + return {subjectType: 'Section', id}; + } + + if (type === 'newThread') { + const collection = subjectType === 'Section' ? entry.sections : entry.contentElements; + return {subjectType, id: collection.findWhere({permaId: subjectId})?.id}; + } + + return undefined; +} diff --git a/entry_types/scrolled/package/src/editor/views/CommentsView.js b/entry_types/scrolled/package/src/editor/views/CommentsView.js index 03f986b9c6..75ce799b48 100644 --- a/entry_types/scrolled/package/src/editor/views/CommentsView.js +++ b/entry_types/scrolled/package/src/editor/views/CommentsView.js @@ -5,7 +5,7 @@ import {editor} from 'pageflow/editor'; import {cssModulesUtils, TabsView} from 'pageflow/ui'; import {EntryCommentsView} from './EntryCommentsView'; -import {ContentElementCommentsView} from './ContentElementCommentsView'; +import {SelectionCommentsView} from './SelectionCommentsView'; import styles from './CommentsView.module.css'; @@ -31,7 +31,7 @@ export const CommentsView = Marionette.ItemView.extend({ initialize: function() { this.listenTo(this.options.entry, - 'change:selectedContentElementCommentsId', + 'change:selectedCommentsSubject', this._updateNewThreadButton); }, @@ -46,7 +46,7 @@ export const CommentsView = Marionette.ItemView.extend({ tabsView.tab('comments', () => new EntryCommentsView({entry, editor: editorApi})); tabsView.tab('selection', () => - new ContentElementCommentsView({entry, editor: editorApi})); + new SelectionCommentsView({entry, editor: editorApi})); this.appendSubview(tabsView, {to: this.ui.tabs}); this._updateNewThreadButton(); @@ -54,15 +54,24 @@ export const CommentsView = Marionette.ItemView.extend({ 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') - }); + const subject = entry.get('selectedCommentsSubject'); + if (!subject) return; + + if (subject.subjectType === 'Section') { + const section = entry.sections.get(subject.id); + entry.trigger('selectNewThread', { + subjectId: section.get('permaId'), + subjectType: 'Section' + }); + } + else { + const contentElement = entry.contentElements.get(subject.id); + entry.trigger('selectNewThread', { + subjectId: contentElement.get('permaId'), + subjectType: 'ContentElement', + range: contentElement.transientState.get('newCommentThreadSubjectRange') + }); + } }, goBack: function() { @@ -70,7 +79,7 @@ export const CommentsView = Marionette.ItemView.extend({ }, _updateNewThreadButton: function() { - const enabled = !!this.options.entry.get('selectedContentElementCommentsId'); + const enabled = !!this.options.entry.get('selectedCommentsSubject'); this.$(cssModulesUtils.selector(styles, 'newThreadButton')) .prop('disabled', !enabled); } diff --git a/entry_types/scrolled/package/src/editor/views/ContentElementCommentsView.js b/entry_types/scrolled/package/src/editor/views/ContentElementCommentsView.js deleted file mode 100644 index 497a461a8e..0000000000 --- a/entry_types/scrolled/package/src/editor/views/ContentElementCommentsView.js +++ /dev/null @@ -1,82 +0,0 @@ -import React from 'react'; - -import {ThreadList} from 'pageflow-scrolled/review'; - -import {ReviewView} from './ReviewView'; - -export const ContentElementCommentsView = ReviewView.extend({ - initialize() { - const {entry} = this.options; - - this.listenTo(entry, - 'change:selectedContentElementCommentsId', - this._onSelectedChange); - this.listenTo(entry, - 'change:highlightedThreadId', - () => this.rerender()); - this._observeContentElement(); - }, - - props() { - const {entry, editor} = this.options; - const contentElement = this._contentElement; - const typeName = contentElement?.get('typeName'); - return { - contentElement, - threadIds: contentElement?.transientState.get('commentThreadIdsAtSelection'), - compareRanges: typeName && editor.contentElementTypes.findCompareRanges(typeName), - highlightedThreadId: entry.get('highlightedThreadId'), - onThreadClick: thread => 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, compareRanges, highlightedThreadId, onThreadClick}) { - if (!contentElement) return null; - - if (threadIds === undefined) { - return ( - - ); - } - - return ( - threadIds.includes(thread.id)} - compareRanges={compareRanges} - highlightedThreadId={highlightedThreadId} - onThreadClick={onThreadClick} - showNewForm={false} - 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/EntryCommentsView.js b/entry_types/scrolled/package/src/editor/views/EntryCommentsView.js index 795a68d266..b1d7b3aca8 100644 --- a/entry_types/scrolled/package/src/editor/views/EntryCommentsView.js +++ b/entry_types/scrolled/package/src/editor/views/EntryCommentsView.js @@ -15,7 +15,7 @@ export const EntryCommentsView = ReviewView.extend({ 'change:highlightedThreadId', () => this.rerender()); this.listenTo(entry, - 'change:selectedContentElementCommentsId', + 'change:selectedCommentsSubject', this._onSelectedChange); this._observeSelectedElement(); }, @@ -23,8 +23,9 @@ export const EntryCommentsView = ReviewView.extend({ props() { const {entry, editor} = this.options; return { - items: collectContentElements(entry), + items: collectItems(entry), selectedElement: this._selectedElement, + selectedSection: this._selectedSection, // 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. @@ -36,18 +37,23 @@ export const EntryCommentsView = ReviewView.extend({ }; }, - renderContent({items, selectedElement, transientThreadIds, highlightedThreadId, onThreadClick, editor}) { + renderContent({items, selectedElement, selectedSection, transientThreadIds, highlightedThreadId, onThreadClick, editor}) { return (
- {items.map(({contentElement}) => ( - item.type === 'section' ? + : + - ))} + )}
); }, @@ -62,8 +68,15 @@ export const EntryCommentsView = ReviewView.extend({ this.stopListening(this._selectedElement.transientState); } - const id = this.options.entry.get('selectedContentElementCommentsId'); - this._selectedElement = id ? this.options.entry.contentElements.get(id) : null; + const subject = this.options.entry.get('selectedCommentsSubject'); + this._selectedElement = + subject?.subjectType === 'ContentElement' ? + this.options.entry.contentElements.get(subject.id) : + null; + this._selectedSection = + subject?.subjectType === 'Section' ? + this.options.entry.sections.get(subject.id) : + null; if (this._selectedElement) { this.listenTo(this._selectedElement.transientState, @@ -73,13 +86,17 @@ export const EntryCommentsView = ReviewView.extend({ } }); -function collectContentElements(entry) { +// Section comment groups precede the content element groups of the +// same section, so a reviewer sees feedback on the section as a whole +// above feedback on its individual elements. +function collectItems(entry) { const items = []; entry.chapters.each(chapter => { chapter.sections.each(section => { + items.push({type: 'section', section}); section.contentElements.each(contentElement => { - items.push({contentElement, section}); + items.push({type: 'contentElement', contentElement}); }); }); }); @@ -115,7 +132,7 @@ function ContentElementGroup({ return (
- + t.id) : highlightedThreadId; + + return ( +
+ + +
+ ); +} + +function Separator({label, pictogram}) { return (
diff --git a/entry_types/scrolled/package/src/editor/views/NewThreadView.js b/entry_types/scrolled/package/src/editor/views/NewThreadView.js index 3b79ef7897..4f960b2720 100644 --- a/entry_types/scrolled/package/src/editor/views/NewThreadView.js +++ b/entry_types/scrolled/package/src/editor/views/NewThreadView.js @@ -54,8 +54,7 @@ const NewThreadFormView = ReviewView.extend({ + onSubmit={leave} /> ); } }); diff --git a/entry_types/scrolled/package/src/editor/views/SelectionCommentsView.js b/entry_types/scrolled/package/src/editor/views/SelectionCommentsView.js new file mode 100644 index 0000000000..7b722c5ff1 --- /dev/null +++ b/entry_types/scrolled/package/src/editor/views/SelectionCommentsView.js @@ -0,0 +1,111 @@ +import React from 'react'; + +import {ThreadList} from 'pageflow-scrolled/review'; + +import {ReviewView} from './ReviewView'; + +export const SelectionCommentsView = ReviewView.extend({ + initialize() { + const {entry} = this.options; + + this.listenTo(entry, + 'change:selectedCommentsSubject', + this._onSelectedChange); + this.listenTo(entry, + 'change:highlightedThreadId', + () => this.rerender()); + this._observeSubject(); + }, + + props() { + const {entry, editor} = this.options; + const subject = entry.get('selectedCommentsSubject'); + + // Resolve the model from the current subject rather than the cached + // `this._model`: a `change:highlightedThreadId` rerender can run + // before `change:selectedCommentsSubject` refreshes `this._model`, + // leaving a stale Section model paired with a new ContentElement + // subject. + const model = this._resolveModel(); + + if (!subject || !model) { + return {}; + } + + if (subject.subjectType === 'ContentElement') { + const typeName = model.get('typeName'); + return { + subjectType: 'ContentElement', + subjectId: model.get('permaId'), + threadIds: model.transientState.get('commentThreadIdsAtSelection'), + compareRanges: typeName && editor.contentElementTypes.findCompareRanges(typeName), + highlightedThreadId: entry.get('highlightedThreadId'), + onThreadClick: thread => entry.trigger('selectCommentThread', thread.id) + }; + } + + return { + subjectType: subject.subjectType, + subjectId: model.get('permaId') + }; + }, + + // Highlight and per-thread click are only wired when the view is + // scoped to a list of thread ids. For unscoped subjects (whole + // content elements such as images, or whole sections) 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({subjectType, subjectId, threadIds, compareRanges, highlightedThreadId, onThreadClick}) { + if (!subjectType) return null; + + if (threadIds === undefined) { + return ( + + ); + } + + return ( + threadIds.includes(thread.id)} + compareRanges={compareRanges} + highlightedThreadId={highlightedThreadId} + onThreadClick={onThreadClick} + showNewForm={false} + hideNewTopicButton /> + ); + }, + + _onSelectedChange() { + this._observeSubject(); + this.rerender(); + }, + + _observeSubject() { + if (this._model?.transientState) { + this.stopListening(this._model.transientState); + } + + this._model = this._resolveModel(); + + if (this._model?.transientState) { + this.listenTo(this._model.transientState, + 'change:commentThreadIdsAtSelection', + () => this.rerender()); + } + }, + + _resolveModel() { + const {entry} = this.options; + const subject = entry.get('selectedCommentsSubject'); + if (!subject) return null; + + return subject.subjectType === 'Section' ? + entry.sections.get(subject.id) : + entry.contentElements.get(subject.id); + } +}); diff --git a/entry_types/scrolled/package/src/frontend/commenting/ContentElementDecorator.js b/entry_types/scrolled/package/src/frontend/commenting/ContentElementDecorator.js index fb75e0cbb8..0855727f68 100644 --- a/entry_types/scrolled/package/src/frontend/commenting/ContentElementDecorator.js +++ b/entry_types/scrolled/package/src/frontend/commenting/ContentElementDecorator.js @@ -2,15 +2,15 @@ import React from 'react'; import classNames from 'classnames'; import {api} from '../api'; +import {widths} from '../layouts'; import {useAddCommentMode} from './AddCommentModeProvider'; import {AddCommentOverlay} from './AddCommentOverlay'; -import {FloatingAnchor} from './FloatingAnchor'; import {Popover} from './Popover'; import {useSelectedSubject} from './SelectedSubjectProvider'; import styles from './ContentElementDecorator.module.css'; -export function ContentElementDecorator({type, permaId, children}) { +export function ContentElementDecorator({type, width, customMargin, permaId, children}) { const {inlineComments} = api.contentElementTypes.getOptions(type) || {}; if (inlineComments) { @@ -18,13 +18,14 @@ export function ContentElementDecorator({type, permaId, children}) { } return ( - + {children} ); } -function DefaultCommentDecorator({permaId, children}) { +function DefaultCommentDecorator({permaId, flush, children}) { const {active} = useAddCommentMode(); const {isSelected} = useSelectedSubject('ContentElement', permaId); @@ -32,13 +33,9 @@ function DefaultCommentDecorator({permaId, children}) {
{children}
- - {({placement}) => ( - - )} - +
+ +
); } diff --git a/entry_types/scrolled/package/src/frontend/commenting/ContentElementDecorator.module.css b/entry_types/scrolled/package/src/frontend/commenting/ContentElementDecorator.module.css index f31b7e61b3..f3e8fc53e5 100644 --- a/entry_types/scrolled/package/src/frontend/commenting/ContentElementDecorator.module.css +++ b/entry_types/scrolled/package/src/frontend/commenting/ContentElementDecorator.module.css @@ -6,3 +6,19 @@ box-shadow: 0 0 0 2px var(--ui-accent-color), 0 0 0 calc(2px + space(1)) var(--ui-accent-color-lighter); } + +.badge { + position: absolute; + bottom: -0.5em; + left: -0.5em; + transform: translate(-50%, 50%); + z-index: 5; +} + +/* Full-bleed elements reach the viewport edge, so keep the badge flush to the + left rather than overhanging it (mirrors the .tug treatment in inline + editing's SelectionRect). */ +.badgeFlush { + left: 0; + transform: translate(0, 50%); +} diff --git a/entry_types/scrolled/package/src/frontend/commenting/EditableText.js b/entry_types/scrolled/package/src/frontend/commenting/EditableText.js index c192fb7b08..31fb77e6cb 100644 --- a/entry_types/scrolled/package/src/frontend/commenting/EditableText.js +++ b/entry_types/scrolled/package/src/frontend/commenting/EditableText.js @@ -64,7 +64,7 @@ function CommentingEditableText({ ); } - if (leaf.firstInRange) { + if (leaf.lastInRange) { children = ( {children} diff --git a/entry_types/scrolled/package/src/frontend/commenting/FloatingAnchor.js b/entry_types/scrolled/package/src/frontend/commenting/FloatingAnchor.js deleted file mode 100644 index 22a7825d05..0000000000 --- a/entry_types/scrolled/package/src/frontend/commenting/FloatingAnchor.js +++ /dev/null @@ -1,40 +0,0 @@ -import React from 'react'; -import { - useFloating, FloatingPortal, - offset, flip, autoUpdate -} from '@floating-ui/react'; - -import {useFloatingPortalRoot} from '../FloatingPortalRootProvider'; -import useMediaQuery from '../useMediaQuery'; - -import styles from './FloatingAnchor.module.css'; - -const NARROW_BREAKPOINT = '(max-width: 960px)'; - -export function FloatingAnchor({children}) { - const portalRoot = useFloatingPortalRoot(); - const isNarrow = useMediaQuery(NARROW_BREAKPOINT); - - const {refs, floatingStyles, placement} = useFloating({ - placement: isNarrow ? 'bottom-start' : 'right-start', - middleware: [ - offset(isNarrow ? {mainAxis: -10} : {mainAxis: -16, crossAxis: -10}), - ...(!isNarrow ? [flip({crossAxis: false, padding: 8})] : []) - ], - whileElementsMounted: autoUpdate - }); - - return ( - <> -
- - -
- {children({placement})} -
-
- - ); -} diff --git a/entry_types/scrolled/package/src/frontend/commenting/FloatingAnchor.module.css b/entry_types/scrolled/package/src/frontend/commenting/FloatingAnchor.module.css deleted file mode 100644 index 9c82463d81..0000000000 --- a/entry_types/scrolled/package/src/frontend/commenting/FloatingAnchor.module.css +++ /dev/null @@ -1,9 +0,0 @@ -.anchor { - position: absolute; - inset: 0; - pointer-events: none; -} - -.floating { - z-index: 200; -} diff --git a/entry_types/scrolled/package/src/frontend/commenting/Popover.js b/entry_types/scrolled/package/src/frontend/commenting/Popover.js index ccc0c1c186..9b503420cb 100644 --- a/entry_types/scrolled/package/src/frontend/commenting/Popover.js +++ b/entry_types/scrolled/package/src/frontend/commenting/Popover.js @@ -1,14 +1,37 @@ -import React, {useEffect, useRef} from 'react'; -import classNames from 'classnames'; +import React, {useEffect} from 'react'; +import { + useFloating, FloatingPortal, + offset, flip, shift, autoUpdate +} from '@floating-ui/react'; import {ThreadsBadge, ThreadList} from 'pageflow-scrolled/review'; +import {useFloatingPortalRoot} from '../FloatingPortalRootProvider'; import {useSelectedSubject} from './SelectedSubjectProvider'; import styles from './Popover.module.css'; -export function Popover({subjectType, subjectId, subjectRange, placement, narrow, suppressNewForm, hideNewTopicButton}) { - const {isSelected, showNewForm, select, clearSelection} = useSelectedSubject(subjectType, subjectId, subjectRange); - const ref = useRef(null); +export function Popover({ + subjectType, subjectId, subjectRange, + placement = 'bottom-start', strategy = 'absolute', hideNewTopicButton +}) { + const {isSelected, showNewForm, select, clearSelection} = + useSelectedSubject(subjectType, subjectId, subjectRange); + const portalRoot = useFloatingPortalRoot(); + + const {refs, floatingStyles} = useFloating({ + open: isSelected, + placement, + strategy, + middleware: [ + offset(8), + flip({ + fallbackPlacements: [placement.startsWith('top') ? 'bottom-start' : 'top-start'], + rootBoundary: 'document' + }), + shift({padding: 8}) + ], + whileElementsMounted: autoUpdate + }); function handleBadgeClick() { if (isSelected) { @@ -23,9 +46,11 @@ export function Popover({subjectType, subjectId, subjectRange, placement, narrow if (!isSelected) return; function handleClick(event) { - if (ref.current && !ref.current.contains(event.target) && !event.target.closest('[data-comment-highlight]')) { - clearSelection(); - } + if (refs.reference.current?.contains(event.target)) return; + if (refs.floating.current?.contains(event.target)) return; + if (event.target.closest('[data-comment-highlight]')) return; + + clearSelection(); } function handleKeyDown(event) { @@ -41,33 +66,28 @@ export function Popover({subjectType, subjectId, subjectRange, placement, narrow document.removeEventListener('mousedown', handleClick); document.removeEventListener('keydown', handleKeyDown); }; - }, [isSelected, clearSelection]); - - const onLeft = placement && placement.startsWith('left'); - const onBottom = placement && placement.startsWith('bottom'); + }, [isSelected, clearSelection, refs.reference, refs.floating]); return ( -
+ -
- {isSelected && - } -
-
+ {isSelected && + +
+ +
+
} + ); } diff --git a/entry_types/scrolled/package/src/frontend/commenting/Popover.module.css b/entry_types/scrolled/package/src/frontend/commenting/Popover.module.css index d1617fc66e..e9d798e956 100644 --- a/entry_types/scrolled/package/src/frontend/commenting/Popover.module.css +++ b/entry_types/scrolled/package/src/frontend/commenting/Popover.module.css @@ -1,36 +1,12 @@ -.popover { - display: flex; - align-items: flex-start; - gap: space(2); - font-family: var(--ui-font-family); - font-size: var(--ui-font-size); - color: var(--ui-on-surface-color); -} - -.popover > * { +.badge { + display: inline-flex; pointer-events: auto; } -.reversed { - flex-direction: row-reverse; -} - -.bottom { - flex-direction: column; -} - -.narrow { - position: relative; -} - -.narrow .threadListContainer { - position: absolute; - top: 100%; - right: 0; - margin-top: space(2); -} - -.threadListContainer { +.threadList { min-width: space(72); max-width: space(96); + font-family: var(--ui-font-family); + font-size: var(--ui-font-size); + color: var(--ui-on-surface-color); } diff --git a/entry_types/scrolled/package/src/frontend/commenting/PopoversColumn.js b/entry_types/scrolled/package/src/frontend/commenting/PopoversColumn.js index d18aa1d285..93dfea8d08 100644 --- a/entry_types/scrolled/package/src/frontend/commenting/PopoversColumn.js +++ b/entry_types/scrolled/package/src/frontend/commenting/PopoversColumn.js @@ -1,16 +1,11 @@ -import React, {useLayoutEffect, useState} from 'react'; - -import {FloatingPortal} from '@floating-ui/react'; +import React from 'react'; import {useAnchoredFloating} from 'pageflow-scrolled/review'; import {useContentElementAttributes} from '../useContentElementAttributes'; -import {useFloatingPortalRoot} from '../FloatingPortalRootProvider'; import {Popover} from './Popover'; import styles from './PopoversColumn.module.css'; -const WIDE_POPOVER_WIDTH = 340; - export function PopoversColumn({highlights, anchors}) { return highlights.map(highlight => ( { - if (fits === true && isNarrow) setIsNarrow(false); - else if (fits === false && !isNarrow) setIsNarrow(true); - }, [fits, isNarrow]); + const {refs, floatingStyles, hasAnchor} = + useAnchoredFloating(rangeKey, anchors, {placement: 'left-end'}); if (!hasAnchor) return null; return ( - -
- -
-
+
+ +
); } diff --git a/entry_types/scrolled/package/src/frontend/commenting/SectionDecorator.js b/entry_types/scrolled/package/src/frontend/commenting/SectionDecorator.js new file mode 100644 index 0000000000..52bca34cad --- /dev/null +++ b/entry_types/scrolled/package/src/frontend/commenting/SectionDecorator.js @@ -0,0 +1,58 @@ +import React from 'react'; +import classNames from 'classnames'; + +import {useCommentThreads} from 'pageflow-scrolled/review'; +import {useI18n} from '../i18n'; +import {useAddCommentMode} from './AddCommentModeProvider'; +import {useSelectedSubject} from './SelectedSubjectProvider'; +import {Popover} from './Popover'; + +import AddCommentIcon from './images/addComment.svg'; +import styles from './SectionDecorator.module.css'; + +export function SectionDecorator({section, children}) { + const {active} = useAddCommentMode(); + const {isSelected} = useSelectedSubject('Section', section.permaId); + const threads = useCommentThreads( + {subjectType: 'Section', subjectId: section.permaId}, + {resolved: false} + ); + const hasThreads = threads.length > 0; + + return ( +
+ {children} +
+ {active ? + : + } +
+
+ ); +} + +function AddCommentButton({permaId}) { + const {t} = useI18n({locale: 'ui'}); + const {deactivate} = useAddCommentMode(); + const {select} = useSelectedSubject('Section', permaId); + + function handleClick() { + select({showNewForm: true}); + deactivate(); + } + + return ( + + ); +} diff --git a/entry_types/scrolled/package/src/frontend/commenting/SectionDecorator.module.css b/entry_types/scrolled/package/src/frontend/commenting/SectionDecorator.module.css new file mode 100644 index 0000000000..53c5fc797a --- /dev/null +++ b/entry_types/scrolled/package/src/frontend/commenting/SectionDecorator.module.css @@ -0,0 +1,46 @@ +.wrapper { + position: relative; +} + +.commentBadge { + box-sizing: border-box; + position: relative; + /* Sit above the section Foreground (z-index 3) so the badge stays + reachable; the section establishes no stacking context of its own. */ + z-index: 4; + display: flex; + align-items: flex-end; + height: 44px; + padding: 0 0 10px 10px; + pointer-events: none; +} + +.commentBadge > * { + pointer-events: auto; +} + +.commentBadge.sticky { + position: sticky; + bottom: 0; +} + +/* Absorb the badge box's height; on the section, not the sticky badge. */ +.wrapper > section { + margin-bottom: -44px; +} + +.addButton { + border: none; + background: none; + padding: 0; + cursor: pointer; +} + +.pill { + composes: pill from './AddCommentOverlay.module.css'; +} + +.addButton:hover .pill, +.addButton:focus .pill { + opacity: 1; +} diff --git a/entry_types/scrolled/package/src/frontend/commenting/extensions.js b/entry_types/scrolled/package/src/frontend/commenting/extensions.js index 829ce591e6..da73844007 100644 --- a/entry_types/scrolled/package/src/frontend/commenting/extensions.js +++ b/entry_types/scrolled/package/src/frontend/commenting/extensions.js @@ -1,10 +1,12 @@ import {EntryDecorator} from './EntryDecorator'; +import {SectionDecorator} from './SectionDecorator'; import {ContentElementDecorator} from './ContentElementDecorator'; import {EditableText} from './EditableText'; export const extensions = { decorators: { Entry: EntryDecorator, + Section: SectionDecorator, ContentElement: ContentElementDecorator }, alternatives: { diff --git a/entry_types/scrolled/package/src/frontend/inlineEditing/ContentElementDecorator.js b/entry_types/scrolled/package/src/frontend/inlineEditing/ContentElementDecorator.js index b7eebc7e05..d1fcff2d53 100644 --- a/entry_types/scrolled/package/src/frontend/inlineEditing/ContentElementDecorator.js +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/ContentElementDecorator.js @@ -1,9 +1,10 @@ -import React from 'react'; +import React, {useCallback, useRef} from 'react'; import {useDrag} from 'react-dnd'; import {features} from 'pageflow/frontend'; import {ThreadsBadge} from 'pageflow-scrolled/review'; import {useContentElementEditorState} from '../useContentElementEditorState'; +import {useSelectCommentThreadHandler} from './useSelectCommentThreadHandler'; import {useI18n} from '../i18n'; import {api} from '../api'; import {widths} from '../layouts'; @@ -55,12 +56,26 @@ function DefaultSelectionRect(props) { const commentsSelected = type === 'contentElementComments' || type === 'newThread'; const {t} = useI18n({locale: 'ui'}); + const selectionRectRef = useRef(); + + useSelectCommentThreadHandler({ + subjectType: 'ContentElement', + subjectId: props.permaId, + getScrollTarget: useCallback(() => selectionRectRef.current, []), + selectThread: useCallback(threadId => selectComments({ + type: 'contentElementComments', + id: props.id, + highlightedThreadId: threadId + }), [selectComments, props.id]) + }); + const [, drag, preview] = useDrag({ item: { type: 'contentElement', id: props.id } }); return ( - threads.length === 0 ? selectNewThread() : - selectComments()} - onSelectThread={threadId => selectComments({ - type: 'contentElementComments', - id: props.id, - highlightedThreadId: threadId - })} />} + selectComments()} />} 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/ContentElementEditorStateProvider.js b/entry_types/scrolled/package/src/frontend/inlineEditing/ContentElementEditorStateProvider.js index a235c57cec..970d71a3a4 100644 --- a/entry_types/scrolled/package/src/frontend/inlineEditing/ContentElementEditorStateProvider.js +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/ContentElementEditorStateProvider.js @@ -13,7 +13,10 @@ export function ContentElementEditorStateProvider({id, permaId, children}) { useMemo(() => ({id, type: 'contentElementComments'}), [id]) ); const {isSelected: newThreadSelected, select: selectNewThread} = useEditorSelection( - useMemo(() => ({id: permaId, type: 'newThread'}), [permaId]) + useMemo( + () => ({type: 'newThread', subjectType: 'ContentElement', subjectId: permaId}), + [permaId] + ) ); const storylineMode = useStorylineActivity(); 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 33338462c0..cf4d27edb8 100644 --- a/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/BadgeColumn.js +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/BadgeColumn.js @@ -1,20 +1,18 @@ import React, {useCallback} from 'react'; import {Range, Transforms} from 'slate'; -import {FloatingPortal} from '@floating-ui/react'; 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 {useContentElementCommentSelection} from '../useCommentSelection'; import {highlightOverlapsSelection} from './highlightOverlapsSelection'; import styles from './BadgeColumn.module.css'; export function BadgeColumn({highlights, anchors}) { const editor = useSlate(); + const {highlightedThreadId} = useContentElementCommentSelection(); + // Treat `editor.selection` as a live cursor only while the editor // is focused. After the user clicks away, slate-react's throttled // `selectionchange` listener can sync a clamped DOM cursor back @@ -22,25 +20,34 @@ export function BadgeColumn({highlights, anchors}) { // to overlap mode without any actual selection. const editorSelection = ReactEditor.isFocused(editor) ? editor.selection : null; + // When a thread is highlighted, fall back to its start point for the + // overlap check so siblings in the same block stay in regular mode + // even if focus has drifted away from the slate editor. Use just the + // start point (not the full range) to stay consistent with + // highlightOverlapsSelection, which anchors to highlight starts. The + // overlap selection is the same for every badge, so resolve it once + // here rather than per badge. + const highlightedRange = highlightedThreadId ? + highlights.find( + h => h.thread?.id === highlightedThreadId + )?.range : + null; + const fallbackPoint = highlightedRange && Range.start(highlightedRange); + const overlapSelection = editorSelection || + (fallbackPoint && {anchor: fallbackPoint, focus: fallbackPoint}); + return highlights.map(highlight => ( )); } -function PositionedBadge({editor, highlight, highlights, editorSelection, anchors}) { - const portalRoot = useFloatingPortalRoot(); - const {contentElementId, contentElementPermaId} = useContentElementAttributes(); - const {select: selectComments, selection: commentsSelection} = useEditorSelection({ - type: 'contentElementComments', id: contentElementId - }); - const {isSelected: newThreadActive} = useEditorSelection({ - type: 'newThread', id: contentElementPermaId - }); +function PositionedBadge({editor, highlight, overlapSelection, anchors}) { + const {selected, highlightedThreadId, selectComments, selectThread} = + useContentElementCommentSelection(); const {refs, floatingStyles, hasAnchor} = useAnchoredFloating(highlight.key, anchors, {placement: 'left-start'}); @@ -61,51 +68,25 @@ function PositionedBadge({editor, highlight, highlights, editorSelection, anchor // 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])); + selectThread(highlight.thread?.id); + }, [editor, highlight, selectComments, selectThread]); if (!hasAnchor) return null; const isHighlightedThread = !!highlight.thread && - commentsSelection?.highlightedThreadId === highlight.thread.id; + highlightedThreadId === highlight.thread.id; const isActive = isHighlightedThread || - (highlight.key === 'selection' && newThreadActive); - // When a thread is highlighted, fall back to its start point for the - // overlap check so siblings in the same block stay in regular mode - // even if focus has drifted away from the slate editor. Use just the - // start point (not the full range) to stay consistent with - // highlightOverlapsSelection, which anchors to highlight starts. - const highlightedRange = commentsSelection?.highlightedThreadId ? - highlights.find( - h => h.thread?.id === commentsSelection.highlightedThreadId - )?.range : - null; - const fallbackPoint = highlightedRange && Range.start(highlightedRange); - const overlapSelection = editorSelection || - (fallbackPoint && {anchor: fallbackPoint, focus: fallbackPoint}); + (highlight.key === 'selection' && selected === 'newThread'); const mode = isActive ? 'active' : highlightOverlapsSelection(highlight, overlapSelection) ? undefined : 'dot'; 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 ae333f8f7a..f9e82d2b54 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 @@ -9,6 +9,10 @@ width: 30px; height: 30px; scroll-margin: space(8); + /* The box is positioned via floating styles; keep it above the selection + rect (z-index 1) now that badges are no longer portaled into a higher + layer. */ + z-index: 2; } .onDark { diff --git a/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/PendingSelectionBadge.js b/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/PendingSelectionBadge.js index 39d0e0be27..4e48358e0c 100644 --- a/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/PendingSelectionBadge.js +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/PendingSelectionBadge.js @@ -1,10 +1,9 @@ import React, {useCallback, useState} from 'react'; import classNames from 'classnames'; -import {FloatingPortal, useFloating} from '@floating-ui/react'; +import {useFloating} from '@floating-ui/react'; import {ReactEditor, useSlate} from 'slate-react'; import {Badge, alignToContainerEdge} from 'pageflow-scrolled/review'; -import {useFloatingPortalRoot} from '../../FloatingPortalRootProvider'; import {useDarkBackground} from '../../backgroundColor'; import {useEffectiveSelection} from './useEffectiveSelection'; import {useStartNewThread} from './useStartNewThread'; @@ -13,7 +12,6 @@ import styles from './BadgeColumn.module.css'; export function PendingSelectionBadge({containerRef}) { const editor = useSlate(); - const portalRoot = useFloatingPortalRoot(); const [isVisible, setIsVisible] = useState(false); const darkBackground = useDarkBackground(); const startNewThread = useStartNewThread(editor); @@ -42,12 +40,10 @@ export function PendingSelectionBadge({containerRef}) { if (!isVisible) return null; return ( - -
- -
-
+
+ +
); } 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 8cd373ab65..064909295b 100644 --- a/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/Selection.js +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/Selection.js @@ -1,4 +1,4 @@ -import React, {useEffect, useMemo, useRef} from 'react'; +import React, {useEffect, useRef} from 'react'; import {Editor, Transforms, Node} from 'slate'; import {useSlate, ReactEditor} from 'slate-react'; import {useDrag} from 'react-dnd'; @@ -6,11 +6,10 @@ import {useDrag} from 'react-dnd'; import styles from './index.module.css'; import {SelectionRect} from '../SelectionRect'; -import {useContentElementAttributes} from '../../useContentElementAttributes'; import {useContentElementEditorState} from '../../useContentElementEditorState'; import {useI18n} from '../../i18n'; import {postInsertContentElementMessage} from '../postMessage'; -import {useEditorSelection} from '../EditorState'; +import {useContentElementCommentSelection} from '../useCommentSelection'; import {cursorLeftHighlightedThreadBlock} from './cursorLeftHighlightedThreadBlock'; import {cursorMovedFromPendingNewThreadRange} from './cursorMovedFromPendingNewThreadRange'; import {getUniformSelectedNode} from './getUniformSelectedNode'; @@ -44,20 +43,9 @@ export function Selection(props) { range } = useContentElementEditorState(); - const {contentElementPermaId} = useContentElementAttributes(); - - const {selection: commentsSelection} = useEditorSelection( - useMemo( - () => ({type: 'contentElementComments', id: props.contentElementId}), - [props.contentElementId] - ) - ); - const {isSelected: newThreadActive, range: newThreadRange} = useEditorSelection( - useMemo( - () => ({type: 'newThread', id: contentElementPermaId}), - [contentElementPermaId] - ) - ); + const {selected, highlightedThreadId, newThreadRange} = + useContentElementCommentSelection(); + const newThreadActive = selected === 'newThread'; useEffect(() => { const {selection} = editor; @@ -118,7 +106,7 @@ export function Selection(props) { if (type === 'contentElementComments' && cursorLeftHighlightedThreadBlock({ - editor, commentsSelection, highlights: props.highlights + editor, highlightedThreadId, highlights: props.highlights })) { select(); return; diff --git a/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/cursorLeftHighlightedThreadBlock.js b/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/cursorLeftHighlightedThreadBlock.js index 5c37111581..0675add69a 100644 --- a/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/cursorLeftHighlightedThreadBlock.js +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/cursorLeftHighlightedThreadBlock.js @@ -3,13 +3,13 @@ // thread should keep the highlight in place (so the comment can be // referenced while editing); only block changes mean the user moved // past the comment's anchor. -export function cursorLeftHighlightedThreadBlock({editor, commentsSelection, highlights}) { - if (!commentsSelection?.highlightedThreadId || !highlights) { +export function cursorLeftHighlightedThreadBlock({editor, highlightedThreadId, highlights}) { + if (!highlightedThreadId || !highlights) { return false; } const highlightedRange = highlights.find( - h => h.thread?.id === commentsSelection.highlightedThreadId + h => h.thread?.id === highlightedThreadId )?.range; if (!highlightedRange) return false; diff --git a/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/index.js b/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/index.js index f664069136..a6a87ea693 100644 --- a/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/index.js +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/index.js @@ -105,6 +105,7 @@ export const EditableText = React.memo(function EditableText({ enabled: commentingEnabled, anchors, highlights, + visibleHighlights, decorate: decorateComments, withCommentHighlightDecoration, resetRangeRefs, @@ -201,7 +202,7 @@ export const EditableText = React.memo(function EditableText({ {commentingEnabled && <> - + } diff --git a/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/useCommenting.js b/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/useCommenting.js index c5458e7911..e3a4c0fe21 100644 --- a/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/useCommenting.js +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/useCommenting.js @@ -1,5 +1,7 @@ import React, {useCallback, useMemo} from 'react'; import classNames from 'classnames'; +import {Range, Transforms} from 'slate'; +import {ReactEditor} from 'slate-react'; import {features} from 'pageflow/frontend'; import { @@ -12,7 +14,8 @@ import { } from 'pageflow-scrolled/review'; import {useContentElementAttributes} from '../../useContentElementAttributes'; -import {useEditorSelection} from '../EditorState'; +import {useContentElementCommentSelection} from '../useCommentSelection'; +import {useSelectCommentThreadHandler} from '../useSelectCommentThreadHandler'; import {useCommentRangeRefs} from './useCommentRangeRefs'; // Bundles all commenting-related state and render helpers for the @@ -23,33 +26,66 @@ export function useCommenting(editor) { const {contentElementPermaId, inlineComments} = useContentElementAttributes(); const enabled = features.isEnabled('commenting') && inlineComments; + // Track ranges for resolved threads too, so their subject ranges keep + // following live edits and stay correct once a thread is reopened. + // Resolved threads are merely hidden from the highlight overlay until + // they become the highlighted thread (see `visibleThreads`). const threads = useCommentThreads( - enabled ? {subjectType: 'ContentElement', subjectId: contentElementPermaId} : null, - {resolved: false} + enabled ? {subjectType: 'ContentElement', subjectId: contentElementPermaId} : null ); const {trackedThreads, resetRangeRefs, getTrackedSubjectRanges} = useCommentRangeRefs(editor, threads); const {anchors, registerAnchor} = useRangeAnchors(); - const {isSelected: newThreadActive, range: newThreadRange} = useEditorSelection({ - type: 'newThread', - id: contentElementPermaId + const {highlightedThreadId, newThreadRange, selectThread} = + useContentElementCommentSelection(); + + // Move the cursor into the thread's block before the handler selects + // it, so `Selection`'s `cursorLeftHighlightedThreadBlock` does not + // treat a pre-existing cursor as having left the comment and re-select + // the content element, which would hide the highlight again. Routing + // through the handler also reveals a resolved thread (via + // `visibleThreads`) when it is selected. + useSelectCommentThreadHandler({ + subjectType: 'ContentElement', + subjectId: contentElementPermaId, + getScrollTarget: useCallback(threadId => { + const range = getTrackedSubjectRanges()[threadId]; + return range ? domElementAtRangeStart(editor, range) : null; + }, [editor, getTrackedSubjectRanges]), + beforeSelect: useCallback(threadId => { + const range = getTrackedSubjectRanges()[threadId]; + + if (range) { + Transforms.select(editor, Range.start(range)); + } + }, [editor, getTrackedSubjectRanges]), + selectThread }); - const highlights = useCommentHighlights( - trackedThreads, - newThreadActive ? newThreadRange : undefined + // Build highlights for all tracked threads, resolved included, so the + // thread ids at the cursor (which scope the comments sidebar) cover + // resolved threads too. Only `visibleHighlights` get a text overlay and + // a badge; a resolved thread stays hidden until it is the highlighted + // thread. + const highlights = useCommentHighlights(trackedThreads, newThreadRange); + + const visibleHighlights = useMemo( + () => highlights.filter( + h => !h.thread?.resolvedAt || h.thread.id === highlightedThreadId + ), + [highlights, highlightedThreadId] ); const decorate = useMemo( - () => enabled ? decorateCommentHighlights(editor, highlights) : null, - [editor, highlights, enabled] + () => enabled ? decorateCommentHighlights(editor, visibleHighlights) : null, + [editor, visibleHighlights, enabled] ); const withCommentHighlightDecoration = useCallback(({attributes, children, leaf}) => { if (leaf.commentHighlight) { children = ( - + {children} ); @@ -70,11 +106,12 @@ export function useCommenting(editor) { // re-rendering leaves whose decorations changed because its memo // equality function does not compare `decorations`. // eslint-disable-next-line react-hooks/exhaustive-deps - }, [registerAnchor, contentElementPermaId, threads, newThreadRange]); + }, [registerAnchor, contentElementPermaId, threads, newThreadRange, highlightedThreadId]); return { enabled, highlights, + visibleHighlights, anchors, decorate, withCommentHighlightDecoration, @@ -83,21 +120,36 @@ export function useCommenting(editor) { }; } -function HighlightSpan({rangeKey, children}) { - const {contentElementId, contentElementPermaId} = useContentElementAttributes(); +// A resolved thread has no badge yet to scroll itself into view, so the +// commented text is scrolled directly. The leaf DOM already exists, so +// this resolves even before the highlight re-renders. +function domElementAtRangeStart(editor, range) { + try { + const start = Range.start(range); + const domRange = ReactEditor.toDOMRange(editor, {anchor: start, focus: start}); + const {startContainer} = domRange; + + return startContainer.nodeType === Node.ELEMENT_NODE ? + startContainer : + startContainer.parentElement; + } + catch (e) { + // toDOMRange throws when the range is not currently rendered. + return null; + } +} + +function HighlightSpan({rangeKey, resolved, children}) { const threadId = parseInt(rangeKey, 10); - const {isSelected: commentsSelected, selection: commentsSelection} = useEditorSelection({ - type: 'contentElementComments', id: contentElementId - }); - const {isSelected: newThreadActive} = useEditorSelection({ - type: 'newThread', id: contentElementPermaId - }); - const isSelected = (commentsSelected && commentsSelection?.highlightedThreadId === threadId) || - (rangeKey === 'selection' && newThreadActive); + const {selected, highlightedThreadId} = useContentElementCommentSelection(); + const isSelected = (selected === 'comments' && highlightedThreadId === threadId) || + (rangeKey === 'selection' && selected === 'newThread'); return ( - + {children} ); diff --git a/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/useStartNewThread.js b/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/useStartNewThread.js index 8abeec0e4a..47af34f6b3 100644 --- a/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/useStartNewThread.js +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/useStartNewThread.js @@ -1,22 +1,14 @@ import {features} from 'pageflow/frontend'; import {useContentElementAttributes} from '../../useContentElementAttributes'; -import {useEditorSelection} from '../EditorState'; +import {useContentElementCommentSelection} from '../useCommentSelection'; export function useStartNewThread(editor) { - const {contentElementPermaId, inlineComments} = useContentElementAttributes(); + const {inlineComments} = useContentElementAttributes(); const commentingEnabled = features.isEnabled('commenting') && inlineComments; - const {select: selectNewThread} = useEditorSelection({ - type: 'newThread', - id: contentElementPermaId - }); + const {selectNewThread} = useContentElementCommentSelection(); if (!commentingEnabled) return null; - return () => selectNewThread({ - type: 'newThread', - id: contentElementPermaId, - subjectType: 'ContentElement', - range: editor.selection - }); + return () => selectNewThread(editor.selection); } diff --git a/entry_types/scrolled/package/src/frontend/inlineEditing/EditorState.js b/entry_types/scrolled/package/src/frontend/inlineEditing/EditorState.js index 0a3889aac1..76282d5c99 100644 --- a/entry_types/scrolled/package/src/frontend/inlineEditing/EditorState.js +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/EditorState.js @@ -46,7 +46,9 @@ export function useEditorSelection(options) { const isSelected = !!(selection && options && selection.id === options.id && - selection.type === options.type); + selection.type === options.type && + selection.subjectType === options.subjectType && + selection.subjectId === options.subjectId); return { range: selection?.range, diff --git a/entry_types/scrolled/package/src/frontend/inlineEditing/SectionDecorator.js b/entry_types/scrolled/package/src/frontend/inlineEditing/SectionDecorator.js index 1d4e463119..16cd7437d8 100644 --- a/entry_types/scrolled/package/src/frontend/inlineEditing/SectionDecorator.js +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/SectionDecorator.js @@ -1,4 +1,4 @@ -import React from 'react'; +import React, {useCallback, useRef} from 'react'; import classNames from 'classnames'; import styles from './SectionDecorator.module.css'; import backdropStyles from './BackdropDecorator.module.css'; @@ -6,9 +6,13 @@ import contentElementStyles from './ContentElementDecorator.module.css'; import widgetSelectionRectStyles from './WidgetSelectionRect.module.css'; import paddingIndicatorStyles from './PaddingIndicator.module.css'; +import {features} from 'pageflow/frontend'; +import {ThreadsBadge, useCommentThreads} from 'pageflow-scrolled/review'; import {Toolbar} from './Toolbar'; import {ForcePaddingContext} from '../Foreground'; import {useEditorSelection} from './EditorState'; +import {useCommentSelection} from './useCommentSelection'; +import {useSelectCommentThreadHandler} from './useSelectCommentThreadHandler'; import {MotifAreaVisibilityProvider} from '../MotifAreaVisibilityProvider'; import {useI18n} from '../i18n'; @@ -16,6 +20,7 @@ import transitionIcon from './images/arrows.svg'; export function SectionDecorator({backdrop, section, contentElements, transitions, children}) { const {t} = useI18n({locale: 'ui'}); + const commentingEnabled = features.isEnabled('commenting'); const {isSelected: isSectionSelected, select, resetSelection} = useEditorSelection({ id: section.id, @@ -27,7 +32,41 @@ export function SectionDecorator({backdrop, section, contentElements, transition type: 'sectionPaddings' }); - const isSelected = isPaddingSelected || isSectionSelected; + const { + selected: commentSelectionState, + selectComments, + selectThread, + selectNewThread + } = useCommentSelection({ + type: 'sectionComments', + id: section.id, + subjectType: 'Section', + subjectId: section.permaId + }); + + // Viewing a section's comments or composing a new thread on it. + const commentsSelected = commentSelectionState !== null; + + // The section reads as selected while its comments are open, so the + // section and the sidebar comment panel stay visually in sync. + const isSelected = isSectionSelected || isPaddingSelected || commentsSelected; + + const threads = useCommentThreads( + {subjectType: 'Section', subjectId: section.permaId}, + {resolved: false} + ); + const hasThreads = threads.length > 0; + + const wrapperRef = useRef(); + + useSelectCommentThreadHandler({ + subjectType: 'Section', + subjectId: section.permaId, + getScrollTarget: useCallback(() => wrapperRef.current, []), + selectThread + }); + + const clipBadgeCorner = (isSectionSelected || isPaddingSelected) && !hasThreads; const {isSelected: isBackdropElementSelected} = useEditorSelection({ id: backdrop.contentElement?.id, @@ -61,6 +100,7 @@ export function SectionDecorator({backdrop, section, contentElements, transition !event.target.closest(`.${backdropStyles.wrapper}`) && !event.target.closest(`.${widgetSelectionRectStyles.wrapper}`) && !event.target.closest(`.${paddingIndicatorStyles.indicator}`) && + !event.target.closest(`.${styles.commentBadge}`) && !event.target.closest('#fullscreenRoot') && !event.target.closest('[data-floating-ui-portal]')) { isSectionSelected ? resetSelection() : select(); @@ -68,8 +108,10 @@ export function SectionDecorator({backdrop, section, contentElements, transition } return ( -
{renderEditTransitionButton({id: section.previousSection && section.id, @@ -82,21 +124,32 @@ export function SectionDecorator({backdrop, section, contentElements, transition + isHighlighted || + commentsSelected}> {children} + {commentingEnabled && +
+ hasThreads ? selectComments() : selectNewThread()} /> +
}
); } -function className(isSelected, transitionSelection, isHighlighted, isBackdropElementSelected, transitions) { +function className(isSelected, transitionSelection, isHighlighted, isBackdropElementSelected, transitions, clipBadgeCorner, commenting) { return classNames(styles.wrapper, { [styles.selected]: isSelected, [styles.highlighted]: isHighlighted, [styles.lineAbove]: isBackdropElementSelected && transitions[0].startsWith('fade'), [styles.lineBelow]: isBackdropElementSelected && transitions[1].startsWith('fade'), - [styles.transitionSelected]: transitionSelection.isSelected + [styles.transitionSelected]: transitionSelection.isSelected, + [styles.clipBadgeCorner]: clipBadgeCorner, + [styles.commenting]: commenting }); } diff --git a/entry_types/scrolled/package/src/frontend/inlineEditing/SectionDecorator.module.css b/entry_types/scrolled/package/src/frontend/inlineEditing/SectionDecorator.module.css index d54c359add..a7b2deb358 100644 --- a/entry_types/scrolled/package/src/frontend/inlineEditing/SectionDecorator.module.css +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/SectionDecorator.module.css @@ -84,6 +84,53 @@ section::before { overflow: hidden; } +.commentBadge { + box-sizing: border-box; + position: relative; + z-index: 11; + height: 44px; + pointer-events: none; +} + +.commentBadge.sticky { + position: sticky; + bottom: 0px; +} + +.commentBadge > button { + position: absolute; + transform: translate(0, 50%); + pointer-events: auto; + border-bottom-left-radius: 0; + border-top-left-radius: 0; +} + +.selected .commentBadge.sticky { + bottom: 10px; +} + +.selected .commentBadge > button { + bottom: 7px; + left: space(-1.5); +} + +.selected .commentBadge.sticky > button { + left: 0; +} + +/* clip-path clips the border and inset shadow behind the transparent icon. */ +.clipBadgeCorner > section::before { + --nodge: 15px; + clip-path: polygon(0 -10%, 100% -10%, 100% 110%, + var(--nodge) 110%, var(--nodge) calc(100% - var(--nodge)), 0 calc(100% - var(--nodge))); +} + +/* Absorb the badge box's height; on the section, not the sticky badge. Scoped + * to commenting since the box only renders then. */ +.commenting > section { + margin-bottom: -44px; +} + .transitionToolbar-before { composes: toolbar; top: 0; diff --git a/entry_types/scrolled/package/src/frontend/inlineEditing/SelectionRect.js b/entry_types/scrolled/package/src/frontend/inlineEditing/SelectionRect.js index 80d09eb0af..1d3d742c15 100644 --- a/entry_types/scrolled/package/src/frontend/inlineEditing/SelectionRect.js +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/SelectionRect.js @@ -1,4 +1,4 @@ -import React, {useState} from 'react'; +import React, {forwardRef, useState} from 'react'; import classNames from 'classnames'; import {Toolbar} from './Toolbar'; @@ -8,9 +8,10 @@ import styles from './SelectionRect.module.css'; import PlusIcon from './images/plus.svg'; import MoveIcon from './images/move.svg'; -export function SelectionRect(props) { +export const SelectionRect = forwardRef(function SelectionRect(props, ref) { return ( -
); -} +}); function InsertButton(props) { const [insertHovered, setInsertHovered] = useState(false); diff --git a/entry_types/scrolled/package/src/frontend/inlineEditing/useCommentSelection.js b/entry_types/scrolled/package/src/frontend/inlineEditing/useCommentSelection.js new file mode 100644 index 0000000000..b85385a72a --- /dev/null +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/useCommentSelection.js @@ -0,0 +1,60 @@ +import {useCallback, useMemo} from 'react'; + +import {useContentElementAttributes} from '../useContentElementAttributes'; +import {useEditorSelection} from './EditorState'; + +// Bundles an element's two comment-related editor selections - the +// highlighted comment thread and the pending new thread - which are +// mutually exclusive since the editor has a single selection at a time. +// Returns the active state plus actions to select the comments, a +// specific thread or a new thread. `type` is the comments selection type +// ('contentElementComments' or 'sectionComments'); `subjectType` and +// `subjectId` identify the new-thread subject. +export function useCommentSelection({type, id, subjectType, subjectId}) { + const { + isSelected: commentsSelected, + selection: commentsSelection, + select: selectComments + } = useEditorSelection(useMemo(() => ({type, id}), [type, id])); + + const { + isSelected: newThreadActive, + range: newThreadRange, + select: selectNewThreadSelection + } = useEditorSelection(useMemo( + () => ({type: 'newThread', subjectType, subjectId}), + [subjectType, subjectId] + )); + + const selectThread = useCallback( + threadId => selectComments({type, id, highlightedThreadId: threadId}), + [selectComments, type, id] + ); + + const selectNewThread = useCallback( + range => range ? + selectNewThreadSelection({type: 'newThread', subjectType, subjectId, range}) : + selectNewThreadSelection(), + [selectNewThreadSelection, subjectType, subjectId] + ); + + return { + selected: commentsSelected ? 'comments' : newThreadActive ? 'newThread' : null, + highlightedThreadId: commentsSelection?.highlightedThreadId, + newThreadRange: newThreadActive ? newThreadRange : undefined, + selectComments, + selectThread, + selectNewThread + }; +} + +export function useContentElementCommentSelection() { + const {contentElementId, contentElementPermaId} = useContentElementAttributes(); + + return useCommentSelection({ + type: 'contentElementComments', + id: contentElementId, + subjectType: 'ContentElement', + subjectId: contentElementPermaId + }); +} diff --git a/entry_types/scrolled/package/src/frontend/inlineEditing/useSelectCommentThreadHandler.js b/entry_types/scrolled/package/src/frontend/inlineEditing/useSelectCommentThreadHandler.js new file mode 100644 index 0000000000..78758254c9 --- /dev/null +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/useSelectCommentThreadHandler.js @@ -0,0 +1,38 @@ +import {useCallback} from 'react'; + +import {useCommentThreads} from 'pageflow-scrolled/review'; +import {usePostMessageListener} from '../../shared/usePostMessageListener'; + +// Handles the SELECT_COMMENT_THREAD message the editor posts when a +// thread is clicked in the comments sidebar. Fetches the subject's +// threads itself - including resolved ones, so a resolved thread can be +// revealed when clicked - ignores threads of other subjects, scrolls +// the relevant element into view and calls `selectThread` to make the +// matching editor selection. `beforeSelect` runs right before that, for +// callers that need to prepare for it (e.g. move the editor cursor into +// the thread's block). Shared by the content element and section +// decorators and the EditableText editor so the handling lives in one +// place instead of inside each badge. +export function useSelectCommentThreadHandler({ + subjectType, subjectId, getScrollTarget, beforeSelect, selectThread +}) { + const threads = useCommentThreads({subjectType, subjectId}); + + usePostMessageListener(useCallback(data => { + if (data.type !== 'SELECT_COMMENT_THREAD') return; + + const {threadId} = data.payload; + + if (!threads.some(thread => thread.id === threadId)) return; + + const scrollTarget = getScrollTarget && getScrollTarget(threadId); + + if (scrollTarget) { + scrollTarget.scrollIntoView({block: 'nearest', behavior: 'smooth'}); + } + + if (beforeSelect) beforeSelect(threadId); + + selectThread(threadId); + }, [threads, getScrollTarget, beforeSelect, selectThread])); +} diff --git a/entry_types/scrolled/package/src/review/Badge.js b/entry_types/scrolled/package/src/review/Badge.js index 57e0c6bdc2..0cc0f23295 100644 --- a/entry_types/scrolled/package/src/review/Badge.js +++ b/entry_types/scrolled/package/src/review/Badge.js @@ -4,7 +4,7 @@ import classNames from 'classnames'; import CommentIcon from './images/comment.svg'; import styles from './Badge.module.css'; -export const Badge = forwardRef(function Badge({counter, mode, onClick}, ref) { +export const Badge = forwardRef(function Badge({counter, mode, resolved, onClick}, ref) { const variant = resolveVariant(mode, counter > 0); if (!variant) { @@ -14,7 +14,8 @@ export const Badge = forwardRef(function Badge({counter, mode, onClick}, ref) { return (