diff --git a/entry_types/scrolled/config/locales/de.yml b/entry_types/scrolled/config/locales/de.yml index 05de73ce04..1fd0d047ae 100644 --- a/entry_types/scrolled/config/locales/de.yml +++ b/entry_types/scrolled/config/locales/de.yml @@ -1643,6 +1643,11 @@ de: comments: Kommentare comments_view: activity: Letzte Aktivität + always_show_comments: Kommentare immer anzeigen + display_options: Kommentaranzeige + filter: + all: Alle Themen + unresolved: Ungelöste Themen new_thread: Neues Thema section: Abschnitt tabs: diff --git a/entry_types/scrolled/config/locales/en.yml b/entry_types/scrolled/config/locales/en.yml index 7c9c5d748c..3ffe87a56b 100644 --- a/entry_types/scrolled/config/locales/en.yml +++ b/entry_types/scrolled/config/locales/en.yml @@ -1625,6 +1625,11 @@ en: comments: Comments comments_view: activity: Latest activity + always_show_comments: Always show comments + display_options: Comment display + filter: + all: All topics + unresolved: Unresolved topics new_thread: New topic section: Section tabs: 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 60477dc803..60597196bf 100644 --- a/entry_types/scrolled/package/spec/editor/controllers/PreviewMessageController-spec.js +++ b/entry_types/scrolled/package/spec/editor/controllers/PreviewMessageController-spec.js @@ -14,7 +14,7 @@ import { postSelectLinkDestinationMessage } from 'frontend/inlineEditing/postMessage'; import {setupGlobals} from 'pageflow/testHelpers'; -import {normalizeSeed, factories, createIframeWindow, useFakeXhr} from 'support'; +import {normalizeSeed, factories, createIframeWindow, tick, useFakeXhr} from 'support'; import {enableFetchMocks} from 'jest-fetch-mock'; enableFetchMocks(); @@ -1176,6 +1176,73 @@ describe('PreviewMessageController', () => { })).resolves.toEqual(1); }); + describe('comment display filter', () => { + beforeEach(() => { + features.enable('frontend', ['commenting']); + fetch.mockResponse(JSON.stringify({currentUser: {id: 1}, commentThreads: []})); + window.localStorage.clear(); + }); + + function createEntry() { + return factories.entry(ScrolledEntry, {}, {entryTypeSeed: normalizeSeed()}); + } + + function recordPayloads(iframeWindow) { + const payloads = []; + + iframeWindow.addEventListener('message', event => { + if (event.data.type === 'CHANGE_COMMENT_DISPLAY_FILTER') { + payloads.push(event.data.payload); + } + }); + + return payloads; + } + + // The iframe starts out displaying unresolved threads everywhere, so + // anything else has to be handed over again on every reload. + it('sends what the editor displays after READY', async () => { + const entry = createEntry(); + entry.commentDisplayFilter.set({resolution: 'all', alwaysShowComments: false}); + const iframeWindow = createIframeWindow(); + controller = new PreviewMessageController({entry, iframeWindow}); + + const payloads = recordPayloads(iframeWindow); + await postReadyMessageAndWaitForAcknowledgement(iframeWindow); + await tick(); + + expect(payloads).toEqual([{resolution: 'all', alwaysShowComments: false}]); + }); + + it('sends the resolution when the reviewer changes the filter', async () => { + const entry = createEntry(); + const iframeWindow = createIframeWindow(); + controller = new PreviewMessageController({entry, iframeWindow}); + + const payloads = recordPayloads(iframeWindow); + await postReadyMessageAndWaitForAcknowledgement(iframeWindow); + entry.commentDisplayFilter.set('resolution', 'all'); + await tick(); + + expect(payloads.map(payload => payload.resolution)) + .toEqual(['unresolved', 'all']); + }); + + it('sends along that comments only show for the selection', async () => { + const entry = createEntry(); + const iframeWindow = createIframeWindow(); + controller = new PreviewMessageController({entry, iframeWindow}); + + const payloads = recordPayloads(iframeWindow); + await postReadyMessageAndWaitForAcknowledgement(iframeWindow); + entry.commentDisplayFilter.set('alwaysShowComments', false); + await tick(); + + expect(payloads.map(payload => payload.alwaysShowComments)) + .toEqual([true, false]); + }); + }); + it('sends CHANGE_EMULATION_MODE message to iframe on change:emulation_mode event on model', async () => { const entry = factories.entry(ScrolledEntry, {}, { entryTypeSeed: normalizeSeed({ diff --git a/entry_types/scrolled/package/spec/editor/models/CommentDisplayFilter-spec.js b/entry_types/scrolled/package/spec/editor/models/CommentDisplayFilter-spec.js new file mode 100644 index 0000000000..e4bc7bec9a --- /dev/null +++ b/entry_types/scrolled/package/spec/editor/models/CommentDisplayFilter-spec.js @@ -0,0 +1,53 @@ +import {CommentDisplayFilter} from 'editor/models/CommentDisplayFilter'; + +describe('CommentDisplayFilter', () => { + beforeEach(() => { + window.localStorage.clear(); + }); + + it('shows unresolved threads by default', () => { + const filter = new CommentDisplayFilter(); + + expect(filter.get('resolution')).toEqual('unresolved'); + expect(filter.showsResolved()).toBe(false); + }); + + it('remembers the resolution across editor sessions', () => { + new CommentDisplayFilter().set('resolution', 'all'); + + const filter = new CommentDisplayFilter(); + + expect(filter.get('resolution')).toEqual('all'); + expect(filter.showsResolved()).toBe(true); + }); + + it('remembers going back to unresolved threads', () => { + new CommentDisplayFilter().set('resolution', 'all'); + new CommentDisplayFilter().set('resolution', 'unresolved'); + + expect(new CommentDisplayFilter().get('resolution')).toEqual('unresolved'); + }); + + it('displays comments everywhere by default', () => { + expect(new CommentDisplayFilter().get('alwaysShowComments')).toBe(true); + }); + + it('remembers displaying comments only for the selection', () => { + new CommentDisplayFilter().set('alwaysShowComments', false); + + expect(new CommentDisplayFilter().get('alwaysShowComments')).toBe(false); + }); + + it('remembers going back to displaying comments everywhere', () => { + new CommentDisplayFilter().set('alwaysShowComments', false); + new CommentDisplayFilter().set('alwaysShowComments', true); + + expect(new CommentDisplayFilter().get('alwaysShowComments')).toBe(true); + }); + + it('does not inherit the resolution of the published entry preview', () => { + window.localStorage['pageflow.scrolled.commentsResolution'] = 'all'; + + expect(new CommentDisplayFilter().get('resolution')).toEqual('unresolved'); + }); +}); 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 f4b89e97c7..8b86d51e0b 100644 --- a/entry_types/scrolled/package/spec/editor/views/CommentsView-spec.js +++ b/entry_types/scrolled/package/spec/editor/views/CommentsView-spec.js @@ -7,7 +7,7 @@ import styles from 'editor/views/CommentsView.module.css'; import {factories, useFakeTranslations, renderBackboneView} from 'pageflow/testHelpers'; import {useEditorGlobals} from 'support'; -import {fireEvent} from '@testing-library/dom'; +import {fireEvent, within} from '@testing-library/dom'; import {act} from '@testing-library/react'; function unselectedEntry(createEntry) { @@ -30,6 +30,10 @@ describe('CommentsView', () => { 'pageflow_scrolled.editor.comments_view.tabs.selection': 'For selection', 'pageflow_scrolled.editor.comments_view.new_thread': 'New topic', 'pageflow_scrolled.editor.comments_view.activity': 'Latest activity', + 'pageflow_scrolled.editor.comments_view.display_options': 'Comment display', + 'pageflow_scrolled.editor.comments_view.always_show_comments': 'Always show comments', + 'pageflow_scrolled.editor.comments_view.filter.unresolved': 'Unresolved topics', + 'pageflow_scrolled.editor.comments_view.filter.all': 'All topics', 'pageflow.editor.templates.back_button_decorator.outline': 'Outline' }); @@ -195,6 +199,79 @@ describe('CommentsView', () => { }); }); + describe('display options menu', () => { + let menuContainer; + + beforeEach(() => { + menuContainer = document.createElement('div'); + menuContainer.id = 'editor_menu_container'; + document.body.appendChild(menuContainer); + }); + + afterEach(() => { + menuContainer.remove(); + }); + + function renderMenu(entry) { + renderBackboneView(new CommentsView({entry, editor})); + + return within(menuContainer); + } + + it('checks the menu item of the resolution the filter is set to', () => { + const entry = setupEntry(); + entry.commentDisplayFilter.set('resolution', 'all'); + + const {getByRole} = renderMenu(entry); + + expect(getByRole('link', {name: 'All topics'}).closest('li')) + .toHaveClass('is_checked'); + expect(getByRole('link', {name: 'Unresolved topics'}).closest('li')) + .not.toHaveClass('is_checked'); + }); + + it('sets the resolution when a menu item is clicked', () => { + const entry = setupEntry(); + + const {getByRole} = renderMenu(entry); + fireEvent.click(getByRole('link', {name: 'All topics'})); + + expect(entry.commentDisplayFilter.get('resolution')).toEqual('all'); + expect(getByRole('link', {name: 'All topics'}).closest('li')) + .toHaveClass('is_checked'); + }); + + it('checks displaying comments always while the entry does', () => { + const entry = setupEntry(); + + const {getByRole} = renderMenu(entry); + + expect(getByRole('link', {name: 'Always show comments'}).closest('li')) + .toHaveClass('is_checked'); + }); + + it('displays comments only for the selection when unchecked', () => { + const entry = setupEntry(); + + const {getByRole} = renderMenu(entry); + fireEvent.click(getByRole('link', {name: 'Always show comments'})); + + expect(entry.commentDisplayFilter.get('alwaysShowComments')).toBe(false); + expect(getByRole('link', {name: 'Always show comments'}).closest('li')) + .not.toHaveClass('is_checked'); + }); + + it('displays comments everywhere again when checked back', () => { + const entry = setupEntry(); + entry.commentDisplayFilter.set('alwaysShowComments', false); + + const {getByRole} = renderMenu(entry); + fireEvent.click(getByRole('link', {name: 'Always show comments'})); + + expect(entry.commentDisplayFilter.get('alwaysShowComments')).toBe(true); + }); + }); + describe('activity link', () => { it('navigates to the activity route when clicked', () => { const entry = setupEntry(); 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 8819388239..3bf88e370f 100644 --- a/entry_types/scrolled/package/spec/editor/views/EntryCommentsView-spec.js +++ b/entry_types/scrolled/package/spec/editor/views/EntryCommentsView-spec.js @@ -32,6 +32,8 @@ describe('EntryCommentsView', () => { 'pageflow_scrolled.editor.chapter_item.chapter': 'Chapter', 'pageflow_scrolled.editor.chapter_item.excursion': 'Excursion', 'pageflow_scrolled.review.refers_to_deleted_element': 'Refers to a deleted element', + 'pageflow_scrolled.review.resolved_count.one': '1 resolved', + 'pageflow_scrolled.review.resolved_count.other': '%{count} resolved', 'pageflow_scrolled.review.reply_count.one': '1 reply', 'pageflow_scrolled.review.reply_count.other': '%{count} replies' }); @@ -394,7 +396,7 @@ describe('EntryCommentsView', () => { expect(getByText('on other').closest('[aria-current="true"]')).toBeNull(); }); - it('keeps resolved threads folded away when an element is selected', () => { + it('keeps resolved threads out of the list when an element is selected', () => { const entry = createEntry({ contentElements: [{id: 1, permaId: 10, typeName: 'image'}] }); @@ -681,6 +683,100 @@ describe('EntryCommentsView', () => { Node.DOCUMENT_POSITION_FOLLOWING).toBeTruthy(); }); + describe('resolution filter', () => { + function createEntryWithResolvedThread() { + const entry = createEntry({ + chapters: [ + {id: 1, permaId: 10, storylineId: 1000, position: 0, + configuration: {title: 'Intro'}} + ], + sections: [{id: 1, permaId: 100, chapterId: 1, position: 0}], + contentElements: [ + {id: 1, permaId: 1000, sectionId: 1, typeName: 'textBlock', position: 0}, + {id: 2, permaId: 2000, sectionId: 1, typeName: 'image', position: 1} + ] + }); + entry.reviewSession = factories.reviewSession({ + commentThreads: [ + {id: 1, subjectType: 'ContentElement', subjectId: 1000, + comments: [{id: 10, body: 'still open', creatorName: 'Alice'}]}, + {id: 2, subjectType: 'ContentElement', subjectId: 2000, + resolvedAt: '2026-08-17T10:00:00.000Z', + comments: [{id: 20, body: 'already resolved', creatorName: 'Bob'}]} + ] + }); + + return entry; + } + + it('omits resolved threads and their pill while showing unresolved ones', () => { + const entry = createEntryWithResolvedThread(); + + const {getByText, queryByText} = renderBackboneView( + new EntryCommentsView({entry, editor}) + ); + + expect(getByText('still open')).toBeInTheDocument(); + expect(queryByText('already resolved')).not.toBeInTheDocument(); + expect(queryByText('1 resolved')).not.toBeInTheDocument(); + }); + + it('leaves out the group of an element whose threads are all resolved', () => { + const entry = createEntryWithResolvedThread(); + + const {getByText, queryByText} = renderBackboneView( + new EntryCommentsView({entry, editor}) + ); + + expect(getByText('Text')).toBeInTheDocument(); + expect(queryByText('Image')).not.toBeInTheDocument(); + }); + + it('leaves out the heading of a chapter whose threads are all resolved', () => { + const entry = createEntryWithResolvedThread(); + entry.reviewSession = factories.reviewSession({ + commentThreads: [ + {id: 2, subjectType: 'ContentElement', subjectId: 2000, + resolvedAt: '2026-08-17T10:00:00.000Z', + comments: [{id: 20, body: 'already resolved', creatorName: 'Bob'}]} + ] + }); + + const {queryByText} = renderBackboneView( + new EntryCommentsView({entry, editor}) + ); + + expect(queryByText('Intro')).not.toBeInTheDocument(); + }); + + it('lists resolved threads while showing all', () => { + const entry = createEntryWithResolvedThread(); + entry.commentDisplayFilter.set('resolution', 'all'); + + const {getByText} = renderBackboneView( + new EntryCommentsView({entry, editor}) + ); + + expect(getByText('still open')).toBeInTheDocument(); + expect(getByText('already resolved')).toBeInTheDocument(); + expect(getByText('Image')).toBeInTheDocument(); + }); + + it('follows the filter while the reviewer changes it', () => { + const entry = createEntryWithResolvedThread(); + + const {getByText, queryByText} = renderBackboneView( + new EntryCommentsView({entry, editor}) + ); + + expect(queryByText('already resolved')).not.toBeInTheDocument(); + + act(() => { entry.commentDisplayFilter.set('resolution', 'all'); }); + + expect(getByText('already resolved')).toBeInTheDocument(); + }); + }); + it('highlights all threads of the selected section', () => { const entry = createEntry({ sections: [{id: 1, permaId: 10}, {id: 2, permaId: 20}] diff --git a/entry_types/scrolled/package/spec/editor/views/SelectionCommentsView-spec.js b/entry_types/scrolled/package/spec/editor/views/SelectionCommentsView-spec.js index 9faacf4422..7aecd65ebb 100644 --- a/entry_types/scrolled/package/spec/editor/views/SelectionCommentsView-spec.js +++ b/entry_types/scrolled/package/spec/editor/views/SelectionCommentsView-spec.js @@ -263,6 +263,68 @@ describe('SelectionCommentsView', () => { expect(getByText('Resolved comment')).toBeInTheDocument(); }); + describe('resolution filter', () => { + function createEntryWithResolvedThread({scoped} = {}) { + const entry = createEntry({ + contentElements: [{id: 1, permaId: 10, typeName: 'fixture'}] + }); + entry.set('selectedCommentsSubject', {subjectType: 'ContentElement', id: 1}); + + if (scoped) { + 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'}] + }] + }); + + return entry; + } + + it('expands resolved threads of the selection while showing all', () => { + const entry = createEntryWithResolvedThread({scoped: true}); + entry.commentDisplayFilter.set('resolution', 'all'); + + const {getByText} = renderBackboneView( + new SelectionCommentsView({entry, editor}) + ); + + expect(getByText('Resolved comment')).toBeInTheDocument(); + }); + + it('expands resolved threads of an unscoped subject while showing all', () => { + const entry = createEntryWithResolvedThread(); + entry.commentDisplayFilter.set('resolution', 'all'); + + const {getByText} = renderBackboneView( + new SelectionCommentsView({entry, editor}) + ); + + expect(getByText('Resolved comment')).toBeInTheDocument(); + }); + + it('expands resolved threads once the reviewer changes the filter', () => { + const entry = createEntryWithResolvedThread({scoped: true}); + + const {getByText, queryByText} = renderBackboneView( + new SelectionCommentsView({entry, editor}) + ); + + expect(queryByText('Resolved comment')).not.toBeInTheDocument(); + + act(() => { entry.commentDisplayFilter.set('resolution', 'all'); }); + + expect(getByText('Resolved comment')).toBeInTheDocument(); + }); + }); + it('does not highlight or trigger selectCommentThread when not scoped', async () => { const user = userEvent.setup(); diff --git a/entry_types/scrolled/package/spec/frontend/commenting/features/commentDisplayFilter-spec.js b/entry_types/scrolled/package/spec/frontend/commenting/features/commentDisplayFilter-spec.js index b91f4fb491..74ade04e08 100644 --- a/entry_types/scrolled/package/spec/frontend/commenting/features/commentDisplayFilter-spec.js +++ b/entry_types/scrolled/package/spec/frontend/commenting/features/commentDisplayFilter-spec.js @@ -103,6 +103,18 @@ describe('comment display filter', () => { expect(entry.queryAllCommentBadges()).toHaveLength(2); }); + it('remembers showing all comments across reloads', () => { + const entry = renderEntryWithResolvedThread(); + fireEvent.click(entry.getCommentFilterButton('all')); + entry.unmount(); + + const reloadedEntry = renderEntryWithResolvedThread(); + + expect(reloadedEntry.getCommentFilterButton('all')) + .toHaveAttribute('aria-pressed', 'true'); + expect(reloadedEntry.queryAllCommentBadges()).toHaveLength(1); + }); + it('hides resolved comments again when selecting unresolved', () => { const entry = renderEntryWithResolvedThread(); 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 695db8c9db..e5b6334503 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 @@ -154,6 +154,137 @@ describe('inline editing EditableText comment badges', () => { expect(badge.isActive()).toBe(true); }); + it('hides the badge of a thread while comments show only for the selection', 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: 5, + subjectType: 'ContentElement', + subjectId: 10, + subjectRange: {anchor: {path: [0, 0], offset: 5}, focus: {path: [0, 0], offset: 9}}, + comments: [{id: 1, body: 'A comment', creatorName: 'Alice', creatorId: 1}] + }] + } + }); + + expect(entry.queryAllCommentBadges()).toHaveLength(1); + + act(() => { + window.dispatchEvent(new MessageEvent('message', { + data: { + type: 'CHANGE_COMMENT_DISPLAY_FILTER', + payload: {resolution: 'unresolved', alwaysShowComments: false} + }, + origin: window.location.origin + })); + }); + + await waitFor(() => { + expect(entry.queryAllCommentBadges()).toHaveLength(0); + }); + }); + + it('shows the badge within the selection rect once the text is selected', async () => { + const entry = 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: 5}}, + comments: [{id: 10, body: 'On the first', creatorName: 'Alice', creatorId: 1}]}, + {id: 2, subjectType: 'ContentElement', subjectId: 10, + subjectRange: {anchor: {path: [1, 0], offset: 0}, + focus: {path: [1, 0], offset: 6}}, + comments: [{id: 20, body: 'On the second', creatorName: 'Bob', creatorId: 2}]} + ] + } + }); + + act(() => { + window.dispatchEvent(new MessageEvent('message', { + data: { + type: 'CHANGE_COMMENT_DISPLAY_FILTER', + payload: {resolution: 'unresolved', alwaysShowComments: false} + }, + origin: window.location.origin + })); + }); + + await waitFor(() => { + expect(entry.queryAllCommentBadges()).toHaveLength(0); + }); + + act(() => { + window.dispatchEvent(new MessageEvent('message', { + data: {type: 'SELECT', payload: {type: 'contentElement', id: 1, range: [0, 1]}}, + origin: window.location.origin + })); + }); + + await waitFor(() => { + expect(entry.queryAllCommentBadges()).toHaveLength(1); + }); + + expect(entry.queryAllCommentBadges()[0].isInDotMode()).toBe(false); + }); + + it('renders the badge of a resolved thread while the editor shows all resolutions', 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: 'CHANGE_COMMENT_DISPLAY_FILTER', + payload: {resolution: 'all'} + }, + 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(false); + }); + it('keeps the badge of an overlapped thread after a resolved thread is revealed', async () => { const value = [{type: 'paragraph', children: [{text: 'Alpha beta gamma delta'}]}]; 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 f9fbe41b35..41f493ff67 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 @@ -120,6 +120,216 @@ describe('inline editing EditableText comment highlights', () => { expect(highlight).not.toHaveClass(highlightStyles.selected); }); + describe('while comments show only for the selection', () => { + function renderEntryWithThread() { + const entry = renderEntry({ + contentElement: { + ui: , + typeOptions: {inlineComments: true, customSelectionRect: true} + }, + commenting: { + currentUser: null, + commentThreads: [{ + id: 7, + subjectType: 'ContentElement', + subjectId: 10, + subjectRange, + comments: [{id: 1, body: 'A comment', creatorName: 'Alice', creatorId: 1}] + }] + } + }); + + act(() => { + window.postMessage({ + type: 'CHANGE_COMMENT_DISPLAY_FILTER', + payload: {resolution: 'unresolved', alwaysShowComments: false} + }, '*'); + }); + + return entry; + } + + it('leaves commented text unmarked', async () => { + const entry = renderEntryWithThread(); + + await waitFor(() => { + expect(entry.container.querySelector(`.${highlightStyles.highlight}`)) + .not.toBeInTheDocument(); + }); + }); + + // The selection rect the editable text draws spans the blocks the + // cursor covers, which is what reads as the selection. + it('marks commented text of the blocks within the selection rect', async () => { + const entry = renderEntryWithThreadPerParagraph(); + + act(() => { + window.postMessage({ + type: 'SELECT', + payload: {type: 'contentElement', id: 1, range: [0, 1]} + }, '*'); + }); + + await waitFor(() => { + expect(highlightTexts(entry)).toEqual(['First']); + }); + }); + + // Moving the cursor re-renders neither the editable text nor its + // `renderLeaf`, so the marks can only follow along because the spans + // read the cursor from the slate context themselves. + it('follows the selection into another block', async () => { + const entry = renderEntryWithThreadPerParagraph(); + + act(() => { + window.postMessage({ + type: 'SELECT', + payload: {type: 'contentElement', id: 1, range: [0, 1]} + }, '*'); + }); + + await waitFor(() => { + expect(highlightTexts(entry)).toEqual(['First']); + }); + + act(() => { + window.postMessage({ + type: 'SELECT', + payload: {type: 'contentElement', id: 1, range: [1, 2]} + }, '*'); + }); + + await waitFor(() => { + expect(highlightTexts(entry)).toEqual(['Second']); + }); + }); + + // Following a comment from the sidebar leaves focus outside the + // editor, so the picked thread's block stands in for the cursor — + // the badge column measures against the same point. + it('marks the comments sharing a block with a thread picked from the sidebar', async () => { + const entry = renderEntryWithThreadPerParagraph({ + commentThreads: [ + {id: 7, subjectType: 'ContentElement', subjectId: 10, + subjectRange: {anchor: {path: [0, 0], offset: 0}, + focus: {path: [0, 0], offset: 5}}, + comments: [{id: 10, body: 'Picked', creatorName: 'Alice', creatorId: 1}]}, + {id: 8, subjectType: 'ContentElement', subjectId: 10, + subjectRange: {anchor: {path: [0, 0], offset: 6}, + focus: {path: [0, 0], offset: 15}}, + comments: [{id: 20, body: 'Sibling', creatorName: 'Bob', creatorId: 2}]}, + {id: 9, subjectType: 'ContentElement', subjectId: 10, + subjectRange: {anchor: {path: [1, 0], offset: 0}, + focus: {path: [1, 0], offset: 6}}, + comments: [{id: 30, body: 'Elsewhere', creatorName: 'Bob', creatorId: 2}]} + ] + }); + + act(() => { + window.postMessage({ + type: 'SELECT_COMMENT_THREAD', + payload: {threadId: 7} + }, '*'); + }); + + await waitFor(() => { + expect(highlightTexts(entry)).toEqual(['First', 'paragraph']); + }); + }); + + function renderEntryWithThreadPerParagraph({commentThreads} = {}) { + const entry = renderEntry({ + contentElement: { + ui: , + typeOptions: {inlineComments: true, customSelectionRect: true} + }, + commenting: { + currentUser: null, + commentThreads: commentThreads || [ + {id: 1, subjectType: 'ContentElement', subjectId: 10, + subjectRange: {anchor: {path: [0, 0], offset: 0}, + focus: {path: [0, 0], offset: 5}}, + comments: [{id: 10, body: 'On the first', creatorName: 'Alice', creatorId: 1}]}, + {id: 2, subjectType: 'ContentElement', subjectId: 10, + subjectRange: {anchor: {path: [1, 0], offset: 0}, + focus: {path: [1, 0], offset: 6}}, + comments: [{id: 20, body: 'On the second', creatorName: 'Bob', creatorId: 2}]} + ] + } + }); + + act(() => { + window.postMessage({ + type: 'CHANGE_COMMENT_DISPLAY_FILTER', + payload: {resolution: 'unresolved', alwaysShowComments: false} + }, '*'); + }); + + return entry; + } + + function highlightTexts(entry) { + return [...entry.container.querySelectorAll(`.${highlightStyles.highlight}`)] + .map(element => element.textContent); + } + + // Opening a comment in the sidebar has to keep pointing at the text it + // refers to. + it('marks the text of a thread selected from the sidebar', async () => { + const entry = renderEntryWithThread(); + + act(() => { + window.postMessage({ + type: 'SELECT_COMMENT_THREAD', + payload: {threadId: 7} + }, '*'); + }); + + await waitFor(() => { + expect(entry.container.querySelector(`.${highlightStyles.highlight}`)) + .toBeInTheDocument(); + }); + }); + }); + + it('highlights a resolved thread while the editor shows all resolutions', 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}] + }] + } + }); + + act(() => { + window.postMessage({ + type: 'CHANGE_COMMENT_DISPLAY_FILTER', + payload: {resolution: 'all'} + }, '*'); + }); + + await waitFor(() => { + expect(entry.container.querySelector(`.${highlightStyles.highlight}`)) + .toBeInTheDocument(); + }); + + expect(entry.container.querySelector(`.${highlightStyles.highlight}`)) + .toHaveClass(highlightStyles.resolved); + }); + it('keeps the resolved thread highlighted when a cursor sits in another block', () => { const multiBlockValue = [ {type: 'paragraph', children: [{text: 'First paragraph'}]}, diff --git a/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/highlightOverlapsSelection-spec.js b/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/rangeOverlapsSelection-spec.js similarity index 51% rename from entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/highlightOverlapsSelection-spec.js rename to entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/rangeOverlapsSelection-spec.js index 6473e11dbe..8e58af333b 100644 --- a/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/highlightOverlapsSelection-spec.js +++ b/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/rangeOverlapsSelection-spec.js @@ -1,24 +1,22 @@ -import {highlightOverlapsSelection} from 'frontend/inlineEditing/EditableText/highlightOverlapsSelection'; - -describe('highlightOverlapsSelection', () => { - const highlight = { - range: { - anchor: {path: [1, 0], offset: 5}, - focus: {path: [1, 0], offset: 9} - } +import {rangeOverlapsSelection} from 'frontend/inlineEditing/EditableText/rangeOverlapsSelection'; + +describe('rangeOverlapsSelection', () => { + const range = { + anchor: {path: [1, 0], offset: 5}, + focus: {path: [1, 0], offset: 9} }; it('returns false when selection is null', () => { - expect(highlightOverlapsSelection(highlight, null)).toBe(false); + expect(rangeOverlapsSelection(range, null)).toBe(false); }); - it('returns false when highlight has no range', () => { + it('returns false without a range', () => { const selection = { anchor: {path: [1, 0], offset: 0}, focus: {path: [1, 0], offset: 0} }; - expect(highlightOverlapsSelection({}, selection)).toBe(false); + expect(rangeOverlapsSelection(null, selection)).toBe(false); }); it('returns true when selection is in the same top-level block', () => { @@ -27,7 +25,7 @@ describe('highlightOverlapsSelection', () => { focus: {path: [1, 0], offset: 0} }; - expect(highlightOverlapsSelection(highlight, selection)).toBe(true); + expect(rangeOverlapsSelection(range, selection)).toBe(true); }); it('returns false when selection is in a different top-level block', () => { @@ -36,46 +34,42 @@ describe('highlightOverlapsSelection', () => { focus: {path: [0, 0], offset: 0} }; - expect(highlightOverlapsSelection(highlight, selection)).toBe(false); + expect(rangeOverlapsSelection(range, selection)).toBe(false); }); - it('returns false when selection is in a middle block of a multi-block highlight', () => { - const multiBlockHighlight = { - range: { - anchor: {path: [1, 0], offset: 0}, - focus: {path: [3, 0], offset: 5} - } + it('returns false when selection is in a middle block of a multi-block range', () => { + const multiBlockRange = { + anchor: {path: [1, 0], offset: 0}, + focus: {path: [3, 0], offset: 5} }; const selection = { anchor: {path: [2, 0], offset: 0}, focus: {path: [2, 0], offset: 0} }; - expect(highlightOverlapsSelection(multiBlockHighlight, selection)).toBe(false); + expect(rangeOverlapsSelection(multiBlockRange, selection)).toBe(false); }); - it('returns true when selection is in the same block as a multi-block highlight start', () => { - const multiBlockHighlight = { - range: { - anchor: {path: [1, 0], offset: 0}, - focus: {path: [3, 0], offset: 5} - } + it('returns true when selection is in the same block as a multi-block range start', () => { + const multiBlockRange = { + anchor: {path: [1, 0], offset: 0}, + focus: {path: [3, 0], offset: 5} }; const selection = { anchor: {path: [1, 0], offset: 2}, focus: {path: [1, 0], offset: 2} }; - expect(highlightOverlapsSelection(multiBlockHighlight, selection)).toBe(true); + expect(rangeOverlapsSelection(multiBlockRange, selection)).toBe(true); }); - it('returns true for highlight in block within multi-block selection span', () => { + it('returns true for a range in a block within multi-block selection span', () => { const selection = { anchor: {path: [0, 0], offset: 2}, focus: {path: [2, 0], offset: 3} }; - expect(highlightOverlapsSelection(highlight, selection)).toBe(true); + expect(rangeOverlapsSelection(range, selection)).toBe(true); }); it('works when selection anchor/focus are in reverse order', () => { @@ -84,7 +78,7 @@ describe('highlightOverlapsSelection', () => { focus: {path: [0, 0], offset: 2} }; - expect(highlightOverlapsSelection(highlight, selection)).toBe(true); + expect(rangeOverlapsSelection(range, selection)).toBe(true); }); it('excludes the trailing block when selection ends at its start offset', () => { @@ -93,6 +87,6 @@ describe('highlightOverlapsSelection', () => { focus: {path: [1, 0], offset: 0} }; - expect(highlightOverlapsSelection(highlight, selection)).toBe(false); + expect(rangeOverlapsSelection(range, selection)).toBe(false); }); }); 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 index 0fd3284e17..c3765e395d 100644 --- a/entry_types/scrolled/package/spec/frontend/inlineEditing/features/contentElementCommentBadges-spec.js +++ b/entry_types/scrolled/package/spec/frontend/inlineEditing/features/contentElementCommentBadges-spec.js @@ -189,6 +189,140 @@ describe('inline editing content element comment badges', () => { delete Element.prototype.scrollIntoView; }); + describe('with the editor displaying comments only for the selection', () => { + function renderEntryWithThread() { + const result = 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: 1, + subjectType: 'ContentElement', + subjectId: 10, + comments: [{id: 100, body: 'Review this'}] + }] + } + }, + origin: window.location.origin + })); + }); + + act(() => { + window.dispatchEvent(new MessageEvent('message', { + data: { + type: 'CHANGE_COMMENT_DISPLAY_FILTER', + payload: {resolution: 'unresolved', alwaysShowComments: false} + }, + origin: window.location.origin + })); + }); + + return result; + } + + it('hides the dot badge of an unselected element', async () => { + const {queryByRole} = renderEntryWithThread(); + + await waitFor(() => expect(queryByRole('status')).not.toBeInTheDocument()); + }); + + it('keeps the badge of the selected element', async () => { + const {getByRole} = renderEntryWithThread(); + + act(() => { + window.dispatchEvent(new MessageEvent('message', { + data: {type: 'SELECT', payload: {type: 'contentElement', id: 1}}, + origin: window.location.origin + })); + }); + + await waitFor(() => expect(getByRole('status')).toBeInTheDocument()); + }); + }); + + describe('with the editor showing all resolutions', () => { + function renderEntryWithResolvedThread() { + const result = 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 + })); + }); + + return result; + } + + function changeCommentDisplayFilter(resolution) { + act(() => { + window.dispatchEvent(new MessageEvent('message', { + data: { + type: 'CHANGE_COMMENT_DISPLAY_FILTER', + payload: {resolution} + }, + origin: window.location.origin + })); + }); + } + + it('displays the badge of a resolved thread', async () => { + const {getByRole} = renderEntryWithResolvedThread(); + + changeCommentDisplayFilter('all'); + + await waitFor(() => { + expect(getByRole('status')).toBeInTheDocument(); + expect(getByRole('status')).toHaveClass(badgeStyles.resolved); + }); + }); + + it('hides the badge again once only unresolved threads are shown', async () => { + const {getByRole, queryByRole} = renderEntryWithResolvedThread(); + + changeCommentDisplayFilter('all'); + await waitFor(() => expect(getByRole('status')).toBeInTheDocument()); + + changeCommentDisplayFilter('unresolved'); + + await waitFor(() => expect(queryByRole('status')).not.toBeInTheDocument()); + }); + }); + it('ignores SELECT_COMMENT_THREAD for a thread of another subject', () => { 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 index 4025a6d2d9..4b5493f3cd 100644 --- a/entry_types/scrolled/package/spec/frontend/inlineEditing/features/sectionCommentBadges-spec.js +++ b/entry_types/scrolled/package/spec/frontend/inlineEditing/features/sectionCommentBadges-spec.js @@ -83,6 +83,136 @@ describe('inline editing section comment badges', () => { }); }); + describe('with the editor displaying comments only for the selection', () => { + function renderEntryWithThread() { + const result = 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 + })); + }); + + act(() => { + window.dispatchEvent(new MessageEvent('message', { + data: { + type: 'CHANGE_COMMENT_DISPLAY_FILTER', + payload: {resolution: 'unresolved', alwaysShowComments: false} + }, + origin: window.location.origin + })); + }); + + return result; + } + + it('hides the dot badge of an unselected section', async () => { + const {queryByRole} = renderEntryWithThread(); + + await waitFor(() => expect(queryByRole('status')).not.toBeInTheDocument()); + }); + + it('keeps the badge of the selected section', async () => { + const {getByRole, getSectionByPermaId} = renderEntryWithThread(); + + getSectionByPermaId(10).select(); + + await waitFor(() => expect(getByRole('status')).toBeInTheDocument()); + }); + }); + + describe('with the editor showing all resolutions', () => { + function renderEntryWithResolvedThread() { + const result = 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: 7, + subjectType: 'Section', + subjectId: 10, + resolvedAt: '2026-06-01T00:00:00Z', + comments: [{id: 100, body: 'Resolved'}] + }] + } + }, + origin: window.location.origin + })); + }); + + return result; + } + + function changeCommentDisplayFilter(resolution) { + act(() => { + window.dispatchEvent(new MessageEvent('message', { + data: { + type: 'CHANGE_COMMENT_DISPLAY_FILTER', + payload: {resolution} + }, + origin: window.location.origin + })); + }); + } + + it('displays the badge of a resolved thread', async () => { + const {getByRole} = renderEntryWithResolvedThread(); + + changeCommentDisplayFilter('all'); + + await waitFor(() => { + expect(getByRole('status')).toBeInTheDocument(); + expect(getByRole('status')).toHaveClass(badgeStyles.resolved); + }); + }); + + it('keeps the badge sticky like the badge of an unresolved thread', async () => { + const {getByRole} = renderEntryWithResolvedThread(); + + changeCommentDisplayFilter('all'); + + await waitFor(() => expect(getByRole('status')).toBeInTheDocument()); + expect(getByRole('status').parentNode).toHaveClass(sectionStyles.sticky); + }); + + it('hides the badge again once only unresolved threads are shown', async () => { + const {getByRole, queryByRole} = renderEntryWithResolvedThread(); + + changeCommentDisplayFilter('all'); + await waitFor(() => expect(getByRole('status')).toBeInTheDocument()); + + changeCommentDisplayFilter('unresolved'); + + await waitFor(() => expect(queryByRole('status')).not.toBeInTheDocument()); + }); + }); + it('clips the badge corner when the section is selected without threads', () => { const {getByLabelText, getSectionByPermaId} = renderEntry({ seed: { diff --git a/entry_types/scrolled/package/spec/review/CommentDisplayFilterProvider-spec.js b/entry_types/scrolled/package/spec/review/CommentDisplayFilterProvider-spec.js new file mode 100644 index 0000000000..065a906efe --- /dev/null +++ b/entry_types/scrolled/package/spec/review/CommentDisplayFilterProvider-spec.js @@ -0,0 +1,95 @@ +import React from 'react'; +import {fireEvent, render} from '@testing-library/react'; +import '@testing-library/jest-dom/extend-expect'; + +import { + CommentDisplayFilterProvider, + useCommentDisplayFilter, + useStoredCommentDisplayFilter +} from 'review/CommentDisplayFilterProvider'; + +describe('comment display filter', () => { + beforeEach(() => { + window.localStorage.clear(); + }); + + function Probe() { + const {resolution, alwaysShowComments, setResolution} = useCommentDisplayFilter(); + + return ( + + ); + } + + function Remembered({storageKey}) { + const filter = useStoredCommentDisplayFilter(storageKey); + + return ( + + + + ); + } + + it('shows unresolved threads by default', () => { + const {getByRole} = render(); + + expect(getByRole('button')).toHaveTextContent('unresolved'); + }); + + it('passes the selected resolution to consumers', () => { + const {getByRole} = render(); + + fireEvent.click(getByRole('button')); + + expect(getByRole('button')).toHaveTextContent('all'); + }); + + it('remembers the resolution under the given storage key', () => { + const {getByRole, unmount} = render(); + fireEvent.click(getByRole('button')); + unmount(); + + const {getByRole: getByRoleAgain} = render(); + + expect(getByRoleAgain('button')).toHaveTextContent('all'); + }); + + it('keeps the resolutions of separate storage keys apart', () => { + const {getByRole, unmount} = render(); + fireEvent.click(getByRole('button')); + unmount(); + + const {getByRole: getPreviewButton} = render(); + + expect(getPreviewButton('button')).toHaveTextContent('unresolved'); + }); + + it('takes the resolution from a provider controlled from outside', () => { + const {getByRole} = render( + + + + ); + + expect(getByRole('button')).toHaveTextContent('all'); + }); + + it('displays comments everywhere by default', () => { + const {getByRole} = render(); + + expect(getByRole('button')).toHaveTextContent('everywhere'); + }); + + it('takes displaying comments for the selection only from outside', () => { + const {getByRole} = render( + + + + ); + + expect(getByRole('button')).toHaveTextContent('for the selection'); + }); +}); diff --git a/entry_types/scrolled/package/spec/review/ThreadList-spec.js b/entry_types/scrolled/package/spec/review/ThreadList-spec.js index 3ec63a28a2..7a482a7c0e 100644 --- a/entry_types/scrolled/package/spec/review/ThreadList-spec.js +++ b/entry_types/scrolled/package/spec/review/ThreadList-spec.js @@ -1130,6 +1130,63 @@ describe('ThreadList', () => { expect(queryByPlaceholderText('Add a comment...')).not.toBeInTheDocument(); }); + describe('with a resolution', () => { + const threads = [ + {id: 1, subjectType: 'ContentElement', subjectId: 10, + resolvedAt: null, + comments: [{id: 10, body: 'Active thread', creatorName: 'Alice', creatorId: 1}]}, + {id: 2, subjectType: 'ContentElement', subjectId: 10, + resolvedAt: '2026-04-09T10:00:00Z', + comments: [{id: 20, body: 'Resolved thread', creatorName: 'Bob', creatorId: 2}]} + ]; + + it('omits resolved threads and their pill when listing unresolved ones', () => { + const {getByText, queryByText} = renderThreadList( + , + {commentThreads: threads} + ); + + expect(getByText('Active thread')).toBeInTheDocument(); + expect(queryByText('Resolved thread')).not.toBeInTheDocument(); + expect(queryByText('1 resolved')).not.toBeInTheDocument(); + }); + + it('expands resolved threads when listing all', () => { + const {getByText} = renderThreadList( + , + {commentThreads: threads} + ); + + expect(getByText('Active thread')).toBeInTheDocument(); + expect(getByText('Resolved thread')).toBeInTheDocument(); + }); + + it('folds resolved threads away again when the pill is clicked', async () => { + const user = userEvent.setup(); + + const {getByText, queryByText} = renderThreadList( + , + {commentThreads: threads} + ); + + await user.click(getByText('1 resolved')); + + expect(queryByText('Resolved thread')).not.toBeInTheDocument(); + }); + + it('keeps a highlighted resolved thread while listing unresolved ones', () => { + const {getByText} = renderThreadList( + , + {commentThreads: threads} + ); + + expect(getByText('Resolved thread')).toBeInTheDocument(); + }); + }); + it('still auto-shows the new form for only-resolved threads without expandResolved', () => { const {getByPlaceholderText, queryByText} = renderThreadList( , diff --git a/entry_types/scrolled/package/spec/support/useEditorGlobals.js b/entry_types/scrolled/package/spec/support/useEditorGlobals.js index df915cada5..cce40a888c 100644 --- a/entry_types/scrolled/package/spec/support/useEditorGlobals.js +++ b/entry_types/scrolled/package/spec/support/useEditorGlobals.js @@ -31,6 +31,10 @@ export function useEditorGlobals({fileTypes = []} = {}) { beforeEach(() => { window.I18n = I18n; + + // Entries read remembered editor settings (e.g. which comment + // resolutions to display) from local storage on creation. + window.localStorage.clear(); }); return { diff --git a/entry_types/scrolled/package/src/editor/controllers/PreviewMessageController.js b/entry_types/scrolled/package/src/editor/controllers/PreviewMessageController.js index 766b365897..e467c8d1d4 100644 --- a/entry_types/scrolled/package/src/editor/controllers/PreviewMessageController.js +++ b/entry_types/scrolled/package/src/editor/controllers/PreviewMessageController.js @@ -167,6 +167,15 @@ export const PreviewMessageController = Object.extend({ }) ); + this.listenTo(this.entry.commentDisplayFilter, + 'change:resolution change:alwaysShowComments', + filter => + postMessage({ + type: 'CHANGE_COMMENT_DISPLAY_FILTER', + payload: commentDisplayFilterPayload(filter) + }) + ); + this.listenTo(this.entry, 'change:emulation_mode', entry => postMessage({ type: 'CHANGE_EMULATION_MODE', @@ -178,6 +187,13 @@ export const PreviewMessageController = Object.extend({ postMessage({type: 'ACK'}) if (this.entry.reviewSession) { + // A reloaded iframe starts out displaying unresolved threads + // everywhere, so the filter has to be handed over again. + postMessage({ + type: 'CHANGE_COMMENT_DISPLAY_FILTER', + payload: commentDisplayFilterPayload(this.entry.commentDisplayFilter) + }); + this.entry.reviewSession.fetch(); } } @@ -340,6 +356,13 @@ function selectedCommentsSubjectFor(entry, payload) { return undefined; } +function commentDisplayFilterPayload(filter) { + return { + resolution: filter.get('resolution'), + alwaysShowComments: filter.get('alwaysShowComments') + }; +} + function modelForSubject(entry, {subjectType, subjectId}) { const collection = subjectType === 'Section' ? entry.sections : entry.contentElements; return collection.findWhere({permaId: subjectId}); diff --git a/entry_types/scrolled/package/src/editor/models/CommentDisplayFilter.js b/entry_types/scrolled/package/src/editor/models/CommentDisplayFilter.js new file mode 100644 index 0000000000..c2615fbf75 --- /dev/null +++ b/entry_types/scrolled/package/src/editor/models/CommentDisplayFilter.js @@ -0,0 +1,50 @@ +import Backbone from 'backbone'; + +import {getLocalStorage} from 'pageflow/editor'; + +const resolutionStorageKey = 'pageflow.scrolled.editor.commentsResolution'; +const alwaysShowStorageKey = 'pageflow.scrolled.editor.alwaysShowComments'; + +// Which comments the editor displays, in its sidebar lists as well as in +// the preview: the resolutions of a thread, and whether comments show +// anywhere or only on what is selected. Remembered under keys of its own, +// so that the editor and the published entry's preview mode do not +// inherit each other's setting. +export const CommentDisplayFilter = Backbone.Model.extend({ + defaults: { + resolution: 'unresolved', + alwaysShowComments: true + }, + + initialize() { + const storage = getLocalStorage(); + + if (storage?.[resolutionStorageKey] === 'all') { + this.set('resolution', 'all'); + } + + if (storage?.[alwaysShowStorageKey] === 'false') { + this.set('alwaysShowComments', false); + } + + this.listenTo(this, 'change:resolution', function() { + store(resolutionStorageKey, this.get('resolution')); + }); + + this.listenTo(this, 'change:alwaysShowComments', function() { + store(alwaysShowStorageKey, this.get('alwaysShowComments')); + }); + }, + + showsResolved() { + return this.get('resolution') === 'all'; + } +}); + +function store(storageKey, value) { + const storage = getLocalStorage(); + + if (storage) { + storage[storageKey] = value; + } +} diff --git a/entry_types/scrolled/package/src/editor/models/ScrolledEntry/index.js b/entry_types/scrolled/package/src/editor/models/ScrolledEntry/index.js index 41f8b9d00e..64b78cd2a9 100644 --- a/entry_types/scrolled/package/src/editor/models/ScrolledEntry/index.js +++ b/entry_types/scrolled/package/src/editor/models/ScrolledEntry/index.js @@ -13,6 +13,7 @@ import { ContentElementsCollection } from '../../collections'; +import {CommentDisplayFilter} from '../CommentDisplayFilter'; import {ContentElement} from '../ContentElement'; import {Cutoff} from '../Cutoff'; @@ -80,6 +81,8 @@ export const ScrolledEntry = Entry.extend({ this.scrolledSeed = seed; + this.commentDisplayFilter = new CommentDisplayFilter(); + if (features.isEnabled('commenting')) { this.reviewSession = createReviewSession({entryId: this.id}); watchUnreadComments({entry: this, session: this.reviewSession}); diff --git a/entry_types/scrolled/package/src/editor/views/CommentsView.js b/entry_types/scrolled/package/src/editor/views/CommentsView.js index 9d175bd9ab..c54ecdeb8c 100644 --- a/entry_types/scrolled/package/src/editor/views/CommentsView.js +++ b/entry_types/scrolled/package/src/editor/views/CommentsView.js @@ -1,7 +1,8 @@ +import Backbone from 'backbone'; import I18n from 'i18n-js'; import Marionette from 'backbone.marionette'; -import {editor} from 'pageflow/editor'; +import {DropDownButtonView, editor} from 'pageflow/editor'; import {cssModulesUtils, TabsView} from 'pageflow/ui'; import {EntryCommentsView} from './EntryCommentsView'; @@ -57,8 +58,19 @@ export const CommentsView = Marionette.ItemView.extend({ this.appendSubview(tabsView, {to: this.ui.tabs}); // Beside the tab list rather than inside it, which is a tablist the - // link is not part of. - this.$('.tabs_view-scroller').append(activityButton()); + // controls are not part of. + this.$('.tabs_view-scroller').append(` +
${activityButton()}
+ `); + + this.appendSubview(new DropDownButtonView({ + title: I18n.t('pageflow_scrolled.editor.comments_view.display_options'), + alignMenu: 'right', + ellipsisIcon: true, + borderless: true, + openOnClick: true, + items: displayOptions(entry.commentDisplayFilter) + }), {to: this.$(cssModulesUtils.selector(styles, 'controls'))}); this._updateNewThreadButton(); this._updateActivityButton(); @@ -107,6 +119,66 @@ export const CommentsView = Marionette.ItemView.extend({ } }); +function displayOptions(commentDisplayFilter) { + const items = new ResolutionMenuItems( + [{name: 'unresolved'}, {name: 'all'}], + {commentDisplayFilter} + ); + + items.add(new AlwaysShowCommentsMenuItem({}, {commentDisplayFilter})); + + return items; +} + +const ResolutionMenuItem = Backbone.Model.extend({ + initialize(attributes, options) { + this.commentDisplayFilter = options.commentDisplayFilter; + + this.set('label', I18n.t('pageflow_scrolled.editor.comments_view.filter.' + + this.get('name'))); + this.set('kind', 'radio'); + + const updateChecked = () => { + this.set('checked', + this.commentDisplayFilter.get('resolution') === this.get('name')); + }; + + this.listenTo(this.commentDisplayFilter, 'change:resolution', updateChecked); + updateChecked(); + }, + + selected() { + this.commentDisplayFilter.set('resolution', this.get('name')); + } +}); + +const ResolutionMenuItems = Backbone.Collection.extend({ + model: ResolutionMenuItem +}); + +const AlwaysShowCommentsMenuItem = Backbone.Model.extend({ + initialize(attributes, options) { + this.commentDisplayFilter = options.commentDisplayFilter; + + this.set('label', + I18n.t('pageflow_scrolled.editor.comments_view.always_show_comments')); + this.set('kind', 'checkBox'); + this.set('separated', true); + + const updateChecked = () => { + this.set('checked', this.commentDisplayFilter.get('alwaysShowComments')); + }; + + this.listenTo(this.commentDisplayFilter, 'change:alwaysShowComments', updateChecked); + updateChecked(); + }, + + selected() { + this.commentDisplayFilter.set('alwaysShowComments', + !this.commentDisplayFilter.get('alwaysShowComments')); + } +}); + function activityButton() { const label = I18n.t('pageflow_scrolled.editor.comments_view.activity'); diff --git a/entry_types/scrolled/package/src/editor/views/CommentsView.module.css b/entry_types/scrolled/package/src/editor/views/CommentsView.module.css index 45a77f4563..5188276d4d 100644 --- a/entry_types/scrolled/package/src/editor/views/CommentsView.module.css +++ b/entry_types/scrolled/package/src/editor/views/CommentsView.module.css @@ -26,12 +26,19 @@ /* Rides the tab bar at its far end. The bar is the scroller's containing block, and its tab list is absolutely positioned for the same reason. */ -.activityButton { +.controls { position: absolute; top: 0; right: 0; display: flex; align-items: center; + height: 100%; +} + +.activityButton { + position: relative; + display: flex; + align-items: center; justify-content: center; height: 100%; background: none; diff --git a/entry_types/scrolled/package/src/editor/views/EntryCommentsView.js b/entry_types/scrolled/package/src/editor/views/EntryCommentsView.js index f059da1b6c..1b046b792f 100644 --- a/entry_types/scrolled/package/src/editor/views/EntryCommentsView.js +++ b/entry_types/scrolled/package/src/editor/views/EntryCommentsView.js @@ -1,7 +1,9 @@ import React from 'react'; import I18n from 'i18n-js'; -import {ThreadList, useLocatedCommentThreads} from 'pageflow-scrolled/review'; +import { + ThreadList, matchesResolution, useLocatedCommentThreads +} from 'pageflow-scrolled/review'; import {ReviewView} from './ReviewView'; import defaultPictogram from './images/defaultPictogram.svg'; @@ -17,6 +19,9 @@ export const EntryCommentsView = ReviewView.extend({ this.listenTo(entry, 'change:selectedCommentsSubject', this._onSelectedChange); + this.listenTo(entry.commentDisplayFilter, + 'change:resolution', + () => this.rerender()); this._observeSelectedElement(); }, @@ -30,6 +35,7 @@ export const EntryCommentsView = ReviewView.extend({ transientThreadIds: this._selectedElement?.transientState.get('commentThreadIdsAtSelection'), highlightedThreadId: entry.get('highlightedThreadId'), + resolution: entry.commentDisplayFilter.get('resolution'), onThreadClick: thread => entry.trigger('selectCommentThread', thread.id), editor }; @@ -63,18 +69,27 @@ export const EntryCommentsView = ReviewView.extend({ } }); -function CommentsList({selectedSubject, transientThreadIds, highlightedThreadId, onThreadClick, editor}) { +function CommentsList({selectedSubject, transientThreadIds, highlightedThreadId, resolution, onThreadClick, editor}) { const {chapters} = useLocatedCommentThreads(); + // Threads the filter hides must not leave their chapter heading and + // type separator behind. A thread the reviewer picked in the preview + // stays listed even where the filter hides resolved ones, just as it + // does within a group. + const isListed = thread => matchesResolution(thread, resolution) || + thread.id === highlightedThreadId; + return (
{chapters.map((chapter, index) => )} @@ -82,8 +97,8 @@ function CommentsList({selectedSubject, transientThreadIds, highlightedThreadId, ); } -function ChapterGroup({chapter, number, ...groupProps}) { - if (chapter.threadCount === 0) { +function ChapterGroup({chapter, number, isListed, ...groupProps}) { + if (!chapter.sections.some(section => hasListedThreads(section, isListed))) { return null; } @@ -95,12 +110,15 @@ function ChapterGroup({chapter, number, ...groupProps}) { as a whole above feedback on its individual elements. */} {chapter.sections.map(section => ( - {section.threads.length > 0 && - } + {section.threads.some(isListed) && + } {section.contentElements.map(contentElement => - contentElement.threads.length > 0 && + contentElement.threads.some(isListed) && )} @@ -109,6 +127,13 @@ function ChapterGroup({chapter, number, ...groupProps}) { ); } +function hasListedThreads(section, isListed) { + return section.threads.some(isListed) || + section.contentElements.some( + contentElement => contentElement.threads.some(isListed) + ); +} + function ChapterHeading({number, title}) { return (
@@ -127,10 +152,10 @@ function ChapterHeading({number, title}) { } function ContentElementGroup({ - contentElement, selectedSubject, transientThreadIds, - highlightedThreadId, onThreadClick, editor + contentElement, threads, selectedSubject, transientThreadIds, + highlightedThreadId, resolution, onThreadClick, editor }) { - const {permaId, type, threads} = contentElement; + const {permaId, type} = contentElement; const label = I18n.t(`pageflow_scrolled.editor.content_elements.${type}.name`); const pictogram = editor.contentElementTypes.findPictogram(type) || defaultPictogram; @@ -149,6 +174,7 @@ function ContentElementGroup({ this.rerender()); + this.listenTo(entry.commentDisplayFilter, + 'change:resolution', + () => this.rerender()); this._observeSubject(); }, @@ -35,19 +38,26 @@ export const SelectionCommentsView = ReviewView.extend({ return {}; } + // The fold stays, so that the reviewer can peek at resolved threads + // of the selection without turning them on everywhere; the filter + // only decides whether it starts out open. + const expandResolved = entry.commentDisplayFilter.showsResolved(); + if (subject.subjectType === 'ContentElement') { return { subjectType: 'ContentElement', subjectId: model.get('permaId'), threadIds: model.transientState.get('commentThreadIdsAtSelection'), highlightedThreadId: entry.get('highlightedThreadId'), + expandResolved, onThreadClick: thread => entry.trigger('selectCommentThread', thread.id) }; } return { subjectType: subject.subjectType, - subjectId: model.get('permaId') + subjectId: model.get('permaId'), + expandResolved }; }, @@ -58,13 +68,14 @@ export const SelectionCommentsView = ReviewView.extend({ // in the list would have no counterpart in the preview. A section's // list also surfaces the threads of its deleted content elements, // since ThreadList resolves them from the located threads. - renderContent({subjectType, subjectId, threadIds, highlightedThreadId, onThreadClick}) { + renderContent({subjectType, subjectId, threadIds, highlightedThreadId, expandResolved, onThreadClick}) { if (!subjectType) return null; if (threadIds === undefined) { return ( ); @@ -75,6 +86,7 @@ export const SelectionCommentsView = ReviewView.extend({ subjectId={subjectId} filter={thread => threadIds.includes(thread.id)} highlightedThreadId={highlightedThreadId} + expandResolved={expandResolved} onThreadClick={onThreadClick} showNewForm={false} hideNewTopicButton /> diff --git a/entry_types/scrolled/package/src/frontend/commenting/CommentDisplayFilterProvider.js b/entry_types/scrolled/package/src/frontend/commenting/CommentDisplayFilterProvider.js deleted file mode 100644 index 1faeb73176..0000000000 --- a/entry_types/scrolled/package/src/frontend/commenting/CommentDisplayFilterProvider.js +++ /dev/null @@ -1,22 +0,0 @@ -import React, {createContext, useContext, useMemo, useState} from 'react'; - -const CommentDisplayFilterContext = createContext({ - resolution: 'unresolved', - setResolution: () => {} -}); - -export function CommentDisplayFilterProvider({children}) { - const [resolution, setResolution] = useState('unresolved'); - - const value = useMemo(() => ({resolution, setResolution}), [resolution]); - - return ( - - {children} - - ); -} - -export function useCommentDisplayFilter() { - return useContext(CommentDisplayFilterContext); -} diff --git a/entry_types/scrolled/package/src/frontend/commenting/EditableText.js b/entry_types/scrolled/package/src/frontend/commenting/EditableText.js index ba93bea6a4..28302cdf9d 100644 --- a/entry_types/scrolled/package/src/frontend/commenting/EditableText.js +++ b/entry_types/scrolled/package/src/frontend/commenting/EditableText.js @@ -4,11 +4,10 @@ import {createEditor, Editor} from 'slate'; import {Slate, Editable, ReactEditor, withReact} from 'slate-react'; import {Text} from '../Text'; -import {useCommentThreads, useCommentHighlights, decorateCommentHighlights, useRangeAnchors, RangeAnchor, commentHighlightStyles as highlightStyles} from 'pageflow-scrolled/review'; +import {useCommentDisplayFilter, useCommentThreads, useCommentHighlights, decorateCommentHighlights, useRangeAnchors, RangeAnchor, commentHighlightStyles as highlightStyles} from 'pageflow-scrolled/review'; import {PlainEditableText, renderElement, renderLeaf} from '../EditableText'; import {useContentElementAttributes} from '../useContentElementAttributes'; import {useAddCommentMode} from './AddCommentModeProvider'; -import {useCommentDisplayFilter} from './CommentDisplayFilterProvider'; import {useCommentingVisibility} from './CommentingVisibilityProvider'; import {useSelectedSubject} from './SelectedSubjectProvider'; import {AddCommentHint} from './AddCommentHint'; diff --git a/entry_types/scrolled/package/src/frontend/commenting/EntryDecorator.js b/entry_types/scrolled/package/src/frontend/commenting/EntryDecorator.js index d4a19d7aef..615b1bb0d5 100644 --- a/entry_types/scrolled/package/src/frontend/commenting/EntryDecorator.js +++ b/entry_types/scrolled/package/src/frontend/commenting/EntryDecorator.js @@ -5,21 +5,26 @@ import {createReviewSession} from 'pageflow/review'; import { ReviewStateProvider, ReviewMessageHandler, - LocatedCommentThreadsProvider + LocatedCommentThreadsProvider, + CommentDisplayFilterProvider, + useStoredCommentDisplayFilter } from 'pageflow-scrolled/review'; import {AddCommentModeProvider} from './AddCommentModeProvider'; -import {CommentDisplayFilterProvider} from './CommentDisplayFilterProvider'; import {CommentingVisibilityProvider} from './CommentingVisibilityProvider'; import {SelectedSubjectProvider} from './SelectedSubjectProvider'; import {FloatingToolbar} from './FloatingToolbar'; +const resolutionStorageKey = 'pageflow.scrolled.commentsResolution'; + export function EntryDecorator({commentingInitialState, children}) { + const commentDisplayFilter = useStoredCommentDisplayFilter(resolutionStorageKey); + return ( - + {children} diff --git a/entry_types/scrolled/package/src/frontend/commenting/FloatingToolbar.js b/entry_types/scrolled/package/src/frontend/commenting/FloatingToolbar.js index 875332b447..372367d5d3 100644 --- a/entry_types/scrolled/package/src/frontend/commenting/FloatingToolbar.js +++ b/entry_types/scrolled/package/src/frontend/commenting/FloatingToolbar.js @@ -1,10 +1,11 @@ import React, {useEffect} from 'react'; import classNames from 'classnames'; -import {useLocatedCommentThreads, useUnreadThreadCount} from 'pageflow-scrolled/review'; +import { + useCommentDisplayFilter, useLocatedCommentThreads, useUnreadThreadCount +} from 'pageflow-scrolled/review'; import {useI18n} from '../i18n'; import {useAddCommentMode} from './AddCommentModeProvider'; -import {useCommentDisplayFilter} from './CommentDisplayFilterProvider'; import {useCommentingVisibility} from './CommentingVisibilityProvider'; import {useCommentNavigation} from './SelectedSubjectProvider'; import {ActivityButton} from './ActivityButton'; diff --git a/entry_types/scrolled/package/src/frontend/commenting/Popover.js b/entry_types/scrolled/package/src/frontend/commenting/Popover.js index f567e91ada..702349be7b 100644 --- a/entry_types/scrolled/package/src/frontend/commenting/Popover.js +++ b/entry_types/scrolled/package/src/frontend/commenting/Popover.js @@ -4,9 +4,8 @@ import { offset, flip, shift, autoUpdate } from '@floating-ui/react'; -import {ThreadsBadge, ThreadList} from 'pageflow-scrolled/review'; +import {ThreadsBadge, ThreadList, useCommentDisplayFilter} from 'pageflow-scrolled/review'; import {useFloatingPortalRoot} from '../FloatingPortalRootProvider'; -import {useCommentDisplayFilter} from './CommentDisplayFilterProvider'; import {useSelectedSubject} from './SelectedSubjectProvider'; import styles from './Popover.module.css'; diff --git a/entry_types/scrolled/package/src/frontend/commenting/SectionDecorator.js b/entry_types/scrolled/package/src/frontend/commenting/SectionDecorator.js index ea62dbf402..79aaeb702b 100644 --- a/entry_types/scrolled/package/src/frontend/commenting/SectionDecorator.js +++ b/entry_types/scrolled/package/src/frontend/commenting/SectionDecorator.js @@ -1,10 +1,11 @@ import React from 'react'; import classNames from 'classnames'; -import {useLocatedCommentThreadsForSubject} from 'pageflow-scrolled/review'; +import { + useCommentDisplayFilter, useLocatedCommentThreadsForSubject +} from 'pageflow-scrolled/review'; import {useI18n} from '../i18n'; import {useAddCommentMode} from './AddCommentModeProvider'; -import {useCommentDisplayFilter} from './CommentDisplayFilterProvider'; import {useCommentingVisibility} from './CommentingVisibilityProvider'; import {useSelectedSubject} from './SelectedSubjectProvider'; import {Popover} from './Popover'; diff --git a/entry_types/scrolled/package/src/frontend/commenting/SelectedSubjectProvider.js b/entry_types/scrolled/package/src/frontend/commenting/SelectedSubjectProvider.js index 6b47793df5..9035198f8a 100644 --- a/entry_types/scrolled/package/src/frontend/commenting/SelectedSubjectProvider.js +++ b/entry_types/scrolled/package/src/frontend/commenting/SelectedSubjectProvider.js @@ -1,8 +1,7 @@ import React, {createContext, useCallback, useContext, useMemo, useState} from 'react'; -import {useLocatedCommentThreads} from 'pageflow-scrolled/review'; +import {useCommentDisplayFilter, useLocatedCommentThreads} from 'pageflow-scrolled/review'; import {useActiveExcursion} from '../useActiveExcursion'; -import {useCommentDisplayFilter} from './CommentDisplayFilterProvider'; const SelectedSubjectContext = createContext({ selectedSubject: null, diff --git a/entry_types/scrolled/package/src/frontend/inlineEditing/ContentElementDecorator.js b/entry_types/scrolled/package/src/frontend/inlineEditing/ContentElementDecorator.js index 4123facb12..7826ae89ba 100644 --- a/entry_types/scrolled/package/src/frontend/inlineEditing/ContentElementDecorator.js +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/ContentElementDecorator.js @@ -2,7 +2,7 @@ import React, {useCallback, useRef} from 'react'; import {useDrag} from 'react-dnd'; import {features} from 'pageflow/frontend'; -import {ThreadsBadge} from 'pageflow-scrolled/review'; +import {ThreadsBadge, useCommentDisplayFilter} from 'pageflow-scrolled/review'; import {useContentElementEditorState} from '../useContentElementEditorState'; import {useSelectCommentThreadHandler} from './useSelectCommentThreadHandler'; import {useI18n} from '../i18n'; @@ -54,6 +54,7 @@ function DefaultSelectionRect(props) { const {isSelected, type, select, selectComments, selectNewThread} = useContentElementEditorState(); const commentsSelected = type === 'contentElementComments' || type === 'newThread'; + const {resolution, alwaysShowComments} = useCommentDisplayFilter(); const {t} = useI18n({locale: 'ui'}); const selectionRectRef = useRef(); @@ -86,7 +87,10 @@ function DefaultSelectionRect(props) { commentBadge={features.isEnabled('commenting') && threads.length === 0 ? selectNewThread() : selectComments()} />} 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 b50d768311..0a2b70b1e1 100644 --- a/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/BadgeColumn.js +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/BadgeColumn.js @@ -1,42 +1,24 @@ import React, {useCallback, useMemo} from 'react'; import {Range, Transforms} from 'slate'; -import {useSlate, ReactEditor} from 'slate-react'; +import {useSlate} from 'slate-react'; -import {Badge, useAnchoredFloating, useUnreadActivityCount} from 'pageflow-scrolled/review'; +import { + Badge, useAnchoredFloating, useCommentDisplayFilter, useUnreadActivityCount +} from 'pageflow-scrolled/review'; import {useContentElementCommentSelection} from '../useCommentSelection'; -import {highlightOverlapsSelection} from './highlightOverlapsSelection'; +import {rangeOverlapsSelection} from './rangeOverlapsSelection'; +import {useOverlapSelection} from './useOverlapSelection'; import styles from './BadgeColumn.module.css'; const noThreads = []; -export function BadgeColumn({highlights, anchors}) { +export function BadgeColumn({highlights, highlightedRange, 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 - // into `editor.selection`, which would otherwise flip badges back - // 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}); + + // The same for every badge, so resolved once here rather than per badge. + const overlapSelection = useOverlapSelection(highlightedRange); return highlights.map(highlight => ( diff --git a/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/commentThreadIdsAtSelection.js b/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/commentThreadIdsAtSelection.js index 2764a73801..230b9a0a26 100644 --- a/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/commentThreadIdsAtSelection.js +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/commentThreadIdsAtSelection.js @@ -1,9 +1,9 @@ -import {highlightOverlapsSelection} from './highlightOverlapsSelection'; +import {rangeOverlapsSelection} from './rangeOverlapsSelection'; export function commentThreadIdsAtSelection(highlights, selection) { if (!selection) return []; return highlights - .filter(h => h.thread && highlightOverlapsSelection(h, selection)) + .filter(h => h.thread && rangeOverlapsSelection(h.range, selection)) .map(h => h.thread.id); } 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 a6a87ea693..03d002aa2d 100644 --- a/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/index.js +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/index.js @@ -106,6 +106,7 @@ export const EditableText = React.memo(function EditableText({ anchors, highlights, visibleHighlights, + highlightedRange, decorate: decorateComments, withCommentHighlightDecoration, resetRangeRefs, @@ -202,7 +203,9 @@ export const EditableText = React.memo(function EditableText({ {commentingEnabled && <> - + } diff --git a/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/highlightOverlapsSelection.js b/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/rangeOverlapsSelection.js similarity index 52% rename from entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/highlightOverlapsSelection.js rename to entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/rangeOverlapsSelection.js index 86f6f1ff6c..9e195772eb 100644 --- a/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/highlightOverlapsSelection.js +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/rangeOverlapsSelection.js @@ -1,7 +1,7 @@ import {Range} from 'slate'; -export function highlightOverlapsSelection(highlight, selection) { - if (!highlight?.range || !selection) return false; +export function rangeOverlapsSelection(range, selection) { + if (!range || !selection) return false; const selStart = Range.start(selection); const selEnd = Range.end(selection); @@ -12,7 +12,7 @@ export function highlightOverlapsSelection(highlight, selection) { selEndBlock -= 1; } - const hlStartBlock = Range.start(highlight.range).path[0]; + const startBlock = Range.start(range).path[0]; - return selStartBlock <= hlStartBlock && hlStartBlock <= selEndBlock; + return selStartBlock <= startBlock && startBlock <= selEndBlock; } 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 f22d30e624..1e6f78fc9c 100644 --- a/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/useCommenting.js +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/useCommenting.js @@ -5,6 +5,7 @@ import {ReactEditor} from 'slate-react'; import {features} from 'pageflow/frontend'; import { + useCommentDisplayFilter, useCommentThreads, useCommentHighlights, decorateCommentHighlights, @@ -17,6 +18,8 @@ import {useContentElementAttributes} from '../../useContentElementAttributes'; import {useContentElementCommentSelection} from '../useCommentSelection'; import {useSelectCommentThreadHandler} from '../useSelectCommentThreadHandler'; import {useCommentRangeRefs} from './useCommentRangeRefs'; +import {rangeOverlapsSelection} from './rangeOverlapsSelection'; +import {useOverlapSelection} from './useOverlapSelection'; const noThreads = []; @@ -41,6 +44,7 @@ export function useCommenting(editor) { const {trackedThreads, resetRangeRefs, getTrackedSubjectRanges} = useCommentRangeRefs(editor, threads); const {anchors, registerAnchor} = useRangeAnchors(); + const {resolution} = useCommentDisplayFilter(); const {highlightedThreadId, newThreadRange, selectThread} = useContentElementCommentSelection(); @@ -67,18 +71,25 @@ export function useCommenting(editor) { selectThread }); - // 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. + // Build highlights for all tracked threads, resolved ones included, so + // the thread ids at the cursor (which scope the comments sidebar) cover + // them too. Only `visibleHighlights` get a text overlay and a badge. const highlights = useCommentHighlights(trackedThreads, newThreadRange); const visibleHighlights = useMemo( - () => highlights.filter( - h => !h.thread?.resolvedAt || h.thread.id === highlightedThreadId - ), - [highlights, highlightedThreadId] + () => highlights.filter(highlight => isVisible(highlight, { + resolution, highlightedThreadId + })), + [highlights, resolution, highlightedThreadId] + ); + + // Stands in for the cursor once focus has left the editor, for the + // badges as much as for the overlay. + const highlightedRange = useMemo( + () => visibleHighlights.find( + highlight => highlight.thread?.id === highlightedThreadId + )?.range, + [visibleHighlights, highlightedThreadId] ); const decorate = useMemo( @@ -89,7 +100,10 @@ export function useCommenting(editor) { const withCommentHighlightDecoration = useCallback(({attributes, children, leaf}) => { if (leaf.commentHighlight) { children = ( - + {children} ); @@ -110,12 +124,14 @@ 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, highlightedThreadId]); + }, [registerAnchor, contentElementPermaId, threads, newThreadRange, highlightedThreadId, + highlightedRange, resolution]); return { enabled, highlights, visibleHighlights, + highlightedRange, anchors, decorate, withCommentHighlightDecoration, @@ -124,6 +140,15 @@ export function useCommenting(editor) { }; } +// The range of a thread being composed and the thread the reviewer picked +// show even where the resolution filter would hide them. +function isVisible({thread}, {resolution, highlightedThreadId}) { + return !thread || + thread.id === highlightedThreadId || + resolution === 'all' || + !thread.resolvedAt; +} + // 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. @@ -143,12 +168,26 @@ function domElementAtRangeStart(editor, range) { } } -function HighlightSpan({rangeKey, resolved, children}) { +function HighlightSpan({rangeKey, subjectRange, highlightedRange, resolved, children}) { const threadId = parseInt(rangeKey, 10); const {selected, highlightedThreadId} = useContentElementCommentSelection(); + const {alwaysShowComments} = useCommentDisplayFilter(); + + // Comes through the slate context, which reaches this span on every + // selection change even though the memoized leaf around it does not + // re-render for one. + const overlapSelection = useOverlapSelection(highlightedRange); + const isSelected = (selected === 'comments' && highlightedThreadId === threadId) || (rangeKey === 'selection' && selected === 'newThread'); + // The question the badge column asks as well: what the reviewer has not + // selected is what an uncluttered view leaves out. + if (!alwaysShowComments && !isSelected && + !rangeOverlapsSelection(subjectRange, overlapSelection)) { + return children; + } + return ( - - {children} - + + + {children} + + ); } +// The reviewer picks which resolutions to see in the editor's sidebar +// menu, so the preview only follows what the editor tells it. +function CommentDisplayFilterFromEditor({children}) { + const [filter, setFilter] = useState( + {resolution: 'unresolved', alwaysShowComments: true} + ); + + usePostMessageListener(useCallback(data => { + if (data.type === 'CHANGE_COMMENT_DISPLAY_FILTER') { + setFilter(data.payload); + } + }, [])); + + return ( + + {children} + + ); +} + function MessageHandler({contentElementEditorCommandEmitter}) { const {select} = useEditorSelection() const dispatch = useEntryStateDispatch(); diff --git a/entry_types/scrolled/package/src/frontend/inlineEditing/SectionDecorator.js b/entry_types/scrolled/package/src/frontend/inlineEditing/SectionDecorator.js index 41e1cd478b..5161c83932 100644 --- a/entry_types/scrolled/package/src/frontend/inlineEditing/SectionDecorator.js +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/SectionDecorator.js @@ -7,7 +7,9 @@ import widgetSelectionRectStyles from './WidgetSelectionRect.module.css'; import paddingIndicatorStyles from './PaddingIndicator.module.css'; import {features} from 'pageflow/frontend'; -import {ThreadsBadge, useLocatedCommentThreadsForSubject} from 'pageflow-scrolled/review'; +import { + ThreadsBadge, useCommentDisplayFilter, useLocatedCommentThreadsForSubject +} from 'pageflow-scrolled/review'; import {Toolbar} from './Toolbar'; import {ForcePaddingContext} from '../Foreground'; import {useEditorSelection} from './EditorState'; @@ -51,10 +53,11 @@ export function SectionDecorator({backdrop, section, contentElements, transition // section and the sidebar comment panel stay visually in sync. const isSelected = isSectionSelected || isPaddingSelected || commentsSelected; + const {resolution, alwaysShowComments} = useCommentDisplayFilter(); const threads = useLocatedCommentThreadsForSubject({ subjectType: 'Section', subjectId: section.permaId, - resolution: 'unresolved' + resolution }); const hasThreads = threads.length > 0; @@ -134,8 +137,10 @@ export function SectionDecorator({backdrop, section, contentElements, transition
hasThreads ? selectComments() : selectNewThread()} />
}
diff --git a/entry_types/scrolled/package/src/review/Badge.js b/entry_types/scrolled/package/src/review/Badge.js index 5e478bdfec..258c8642c9 100644 --- a/entry_types/scrolled/package/src/review/Badge.js +++ b/entry_types/scrolled/package/src/review/Badge.js @@ -40,6 +40,8 @@ function resolveVariant(mode, hasThreads, unread) { return 'active'; case 'icon': return hasThreads ? 'expanded' : 'iconOnly'; + case 'none': + return null; case 'dot': // Collapsing to a dot would leave the unread dot sitting on a dot. // Unseen comments are worth the space of the full badge anyway. diff --git a/entry_types/scrolled/package/src/review/CommentDisplayFilterProvider.js b/entry_types/scrolled/package/src/review/CommentDisplayFilterProvider.js new file mode 100644 index 0000000000..581cce4315 --- /dev/null +++ b/entry_types/scrolled/package/src/review/CommentDisplayFilterProvider.js @@ -0,0 +1,70 @@ +import React, {createContext, useCallback, useContext, useMemo, useState} from 'react'; + +const noop = () => {}; + +const CommentDisplayFilterContext = createContext({ + resolution: 'unresolved', + alwaysShowComments: true, + setResolution: noop +}); + +// Which comments the reviewer wants to see: the resolutions of a thread, +// and whether comments show anywhere or only on what is selected. The +// editor and the preview each run their own filter: the preview drives it +// from the toolbar via `useStoredCommentDisplayFilter`, while the editor's +// preview iframe is handed what its sidebar menu holds. +export function CommentDisplayFilterProvider({ + resolution = 'unresolved', alwaysShowComments = true, setResolution = noop, children +}) { + const value = useMemo( + () => ({resolution, alwaysShowComments, setResolution}), + [resolution, alwaysShowComments, setResolution] + ); + + return ( + + {children} + + ); +} + +export function useCommentDisplayFilter() { + return useContext(CommentDisplayFilterContext); +} + +// Keeps the resolution in local storage under the given key, so that the +// editor and the preview remember what they were last set to without +// inheriting each other's setting. +export function useStoredCommentDisplayFilter(storageKey) { + const [resolution, setResolution] = useState(() => readResolution(storageKey)); + + const store = useCallback(resolution => { + setResolution(resolution); + storeResolution(storageKey, resolution); + }, [storageKey]); + + return useMemo(() => ({resolution, setResolution: store}), [resolution, store]); +} + +function readResolution(storageKey) { + return getLocalStorage()?.[storageKey] === 'all' ? 'all' : 'unresolved'; +} + +function storeResolution(storageKey, resolution) { + const storage = getLocalStorage(); + + if (storage) { + storage[storageKey] = resolution; + } +} + +function getLocalStorage() { + try { + return typeof window === 'undefined' ? null : window.localStorage; + } + catch(e) { + // Safari throws SecurityError when accessing window.localStorage + // if cookies/website data are disabled. + return null; + } +} diff --git a/entry_types/scrolled/package/src/review/ThreadList.js b/entry_types/scrolled/package/src/review/ThreadList.js index 70032088f9..37bad7a25d 100644 --- a/entry_types/scrolled/package/src/review/ThreadList.js +++ b/entry_types/scrolled/package/src/review/ThreadList.js @@ -13,12 +13,14 @@ import ChevronIcon from './images/chevron.svg'; import NewTopicIcon from './images/newTopic.svg'; import styles from './ThreadList.module.css'; -export function ThreadList({subjectType, subjectId, subjectRange, filter, highlightedThreadId, onThreadClick, restrictInteractionsToHighlighted, showNewForm: showNewFormProp, hideNewTopicButton, reversed, expandResolved, startCollapsed, markReadWhenHighlighted}) { +export function ThreadList({subjectType, subjectId, subjectRange, filter, resolution, highlightedThreadId, onThreadClick, restrictInteractionsToHighlighted, showNewForm: showNewFormProp, hideNewTopicButton, reversed, expandResolved, startCollapsed, markReadWhenHighlighted}) { const {t} = useI18n({locale: 'ui'}); // Threads arrive already located: in display order, with orphans of // deleted content elements folded into their section on top and flagged. // The list only filters by the selection and splits resolved from active. + // `resolution` states which threads the surrounding filter lets through; + // without it, resolved threads stay listed behind a collapsed pill. const allActiveThreads = useLocatedCommentThreadsForSubject({subjectType, subjectId, subjectRange, resolution: 'unresolved'}); const allResolvedThreads = @@ -28,10 +30,22 @@ export function ThreadList({subjectType, subjectId, subjectRange, filter, highli () => (filter ? allActiveThreads.filter(filter) : allActiveThreads), [allActiveThreads, filter] ); - const resolvedThreads = useMemo( - () => (filter ? allResolvedThreads.filter(filter) : allResolvedThreads), - [allResolvedThreads, filter] - ); + + // A group highlight covers every thread of the subject; only one naming + // a single thread says the reviewer picked it out, which is what brings + // a resolved thread out of the fold. + const pickedThreadId = Array.isArray(highlightedThreadId) ? null : highlightedThreadId; + + // A picked thread stays listed even where the filter hides resolved + // ones, so that following a comment from the activity feed does not + // lead to an empty list. + const resolvedThreads = useMemo(() => { + const threads = filter ? allResolvedThreads.filter(filter) : allResolvedThreads; + + return resolution === 'unresolved' ? + threads.filter(thread => thread.id === pickedThreadId) : + threads; + }, [allResolvedThreads, filter, resolution, pickedThreadId]); const isHighlighted = thread => Array.isArray(highlightedThreadId) ? highlightedThreadId.includes(thread.id) : @@ -45,13 +59,10 @@ export function ThreadList({subjectType, subjectId, subjectRange, filter, highli (soleThread(activeThreads) || soleThread(resolvedThreads))?.id ); const [resolvedToggled, setResolvedToggled] = useState(null); - // A group highlight covers every thread of the subject; only one naming - // a single thread says the reviewer picked it out, which is what brings - // a resolved thread out of the fold. - const pickedThreadId = Array.isArray(highlightedThreadId) ? null : highlightedThreadId; const revealsResolved = - !!expandResolved || resolvedThreads.some(thread => thread.id === pickedThreadId); + !!expandResolved || resolution === 'all' || + resolvedThreads.some(thread => thread.id === pickedThreadId); const [formToggled, setFormToggled] = useState( showNewFormProp !== undefined ? showNewFormProp : diff --git a/entry_types/scrolled/package/src/review/index.js b/entry_types/scrolled/package/src/review/index.js index 95c1e57155..2a3624d2a6 100644 --- a/entry_types/scrolled/package/src/review/index.js +++ b/entry_types/scrolled/package/src/review/index.js @@ -1,7 +1,14 @@ export {review} from './api'; -export {ReviewStateProvider, useCommentThreads, useCommentThread} from './ReviewStateProvider'; +export { + ReviewStateProvider, useCommentThreads, useCommentThread, matchesResolution +} from './ReviewStateProvider'; export {LocatedCommentThreadsProvider, useLocatedCommentThreads} from './useLocatedCommentThreads'; export {useLocatedCommentThreadsForSubject} from './useLocatedCommentThreadsForSubject'; +export { + CommentDisplayFilterProvider, + useCommentDisplayFilter, + useStoredCommentDisplayFilter +} from './CommentDisplayFilterProvider'; export {ReviewMessageHandler} from './ReviewMessageHandler'; export {watchUnreadComments} from './watchUnreadComments'; export {useUnreadActivityCount} from './unreadActivity'; diff --git a/package/src/editor/index.js b/package/src/editor/index.js index 552b00ebd6..9c8ced8d8d 100644 --- a/package/src/editor/index.js +++ b/package/src/editor/index.js @@ -10,6 +10,7 @@ export * from './base'; export * from './utils/entryTypeEditorControllerUrls'; export * from './utils/filesPath'; +export * from './utils/localStorage'; export * from './utils/formDataUtils'; export * from './utils/stylesheet';