From e986f6e8973fab03c2c45ad13d20d7ec0b08991d Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Thu, 27 Aug 2026 16:20:37 +0200 Subject: [PATCH 1/7] Extract the comment display filter for re-use in the editor Move CommentDisplayFilterProvider from frontend/commenting to review/, so the editor's sidebar and preview iframe can share the mechanics of the preview's resolution toggle. The provider itself now only carries the context; useStoredCommentDisplayFilter keeps the resolution in local storage under a caller-chosen key, so the editor and the preview remember their setting separately. --- .../features/commentDisplayFilter-spec.js | 12 +++ .../CommentDisplayFilterProvider-spec.js | 75 +++++++++++++++++++ .../CommentDisplayFilterProvider.js | 22 ------ .../src/frontend/commenting/EditableText.js | 3 +- .../src/frontend/commenting/EntryDecorator.js | 11 ++- .../frontend/commenting/FloatingToolbar.js | 5 +- .../src/frontend/commenting/Popover.js | 3 +- .../frontend/commenting/SectionDecorator.js | 5 +- .../commenting/SelectedSubjectProvider.js | 3 +- .../review/CommentDisplayFilterProvider.js | 65 ++++++++++++++++ .../scrolled/package/src/review/index.js | 5 ++ 11 files changed, 174 insertions(+), 35 deletions(-) create mode 100644 entry_types/scrolled/package/spec/review/CommentDisplayFilterProvider-spec.js delete mode 100644 entry_types/scrolled/package/src/frontend/commenting/CommentDisplayFilterProvider.js create mode 100644 entry_types/scrolled/package/src/review/CommentDisplayFilterProvider.js 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/review/CommentDisplayFilterProvider-spec.js b/entry_types/scrolled/package/spec/review/CommentDisplayFilterProvider-spec.js new file mode 100644 index 0000000000..e6c7b6935c --- /dev/null +++ b/entry_types/scrolled/package/spec/review/CommentDisplayFilterProvider-spec.js @@ -0,0 +1,75 @@ +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, 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'); + }); +}); 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/review/CommentDisplayFilterProvider.js b/entry_types/scrolled/package/src/review/CommentDisplayFilterProvider.js new file mode 100644 index 0000000000..8e5eb7004e --- /dev/null +++ b/entry_types/scrolled/package/src/review/CommentDisplayFilterProvider.js @@ -0,0 +1,65 @@ +import React, {createContext, useCallback, useContext, useMemo, useState} from 'react'; + +const noop = () => {}; + +const CommentDisplayFilterContext = createContext({ + resolution: 'unresolved', + setResolution: noop +}); + +// Which resolutions of a thread the reviewer wants to see. 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 the resolution its sidebar menu holds. +export function CommentDisplayFilterProvider({ + resolution = 'unresolved', setResolution = noop, children +}) { + const value = useMemo(() => ({resolution, setResolution}), [resolution, 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/index.js b/entry_types/scrolled/package/src/review/index.js index 95c1e57155..bb9fbda064 100644 --- a/entry_types/scrolled/package/src/review/index.js +++ b/entry_types/scrolled/package/src/review/index.js @@ -2,6 +2,11 @@ export {review} from './api'; export {ReviewStateProvider, useCommentThreads, useCommentThread} 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'; From 4122be33f23af4741b31d7abf77917792cfcb0d7 Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Thu, 27 Aug 2026 16:23:35 +0200 Subject: [PATCH 2/7] Let a thread list follow a resolution filter Add a resolution prop to ThreadList: 'unresolved' drops the resolved threads together with their count pill, 'all' shows them expanded behind it. A thread the reviewer picked stays listed either way, so that following a resolved comment from the activity feed does not end up in an empty list. --- .../package/spec/review/ThreadList-spec.js | 57 +++++++++++++++++++ .../scrolled/package/src/review/ThreadList.js | 31 ++++++---- 2 files changed, 78 insertions(+), 10 deletions(-) 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/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 : From 3437fa000c373f5c3711f1538c370e3d13627c9a Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Thu, 27 Aug 2026 16:23:51 +0200 Subject: [PATCH 3/7] Filter resolved comment threads out of the editor's list Offer the resolution filter of the preview toolbar as two menu items beside the comments view's activity button. The setting lives on the entry, so both sidebar tabs and (later) the preview read the same resolution, and is remembered across editor sessions. Resolved threads and their count pill leave the entry-wide list while it shows unresolved threads only; headings and type separators left without threads go with them. --- entry_types/scrolled/config/locales/de.yml | 4 + entry_types/scrolled/config/locales/en.yml | 4 + .../models/CommentDisplayFilter-spec.js | 36 +++++++ .../spec/editor/views/CommentsView-spec.js | 48 ++++++++- .../editor/views/EntryCommentsView-spec.js | 98 ++++++++++++++++++- .../package/spec/support/useEditorGlobals.js | 4 + .../src/editor/models/CommentDisplayFilter.js | 33 +++++++ .../src/editor/models/ScrolledEntry/index.js | 3 + .../package/src/editor/views/CommentsView.js | 47 ++++++++- .../src/editor/views/CommentsView.module.css | 9 +- .../src/editor/views/EntryCommentsView.js | 51 +++++++--- .../scrolled/package/src/review/index.js | 4 +- package/src/editor/index.js | 1 + 13 files changed, 323 insertions(+), 19 deletions(-) create mode 100644 entry_types/scrolled/package/spec/editor/models/CommentDisplayFilter-spec.js create mode 100644 entry_types/scrolled/package/src/editor/models/CommentDisplayFilter.js diff --git a/entry_types/scrolled/config/locales/de.yml b/entry_types/scrolled/config/locales/de.yml index 05de73ce04..a3bdca26db 100644 --- a/entry_types/scrolled/config/locales/de.yml +++ b/entry_types/scrolled/config/locales/de.yml @@ -1643,6 +1643,10 @@ de: comments: Kommentare comments_view: activity: Letzte Aktivität + filter: + all: Alle Themen + label: Kommentare filtern + 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..40da18ac80 100644 --- a/entry_types/scrolled/config/locales/en.yml +++ b/entry_types/scrolled/config/locales/en.yml @@ -1625,6 +1625,10 @@ en: comments: Comments comments_view: activity: Latest activity + filter: + all: All topics + label: Filter comments + unresolved: Unresolved topics new_thread: New topic section: Section tabs: 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..aa48225a50 --- /dev/null +++ b/entry_types/scrolled/package/spec/editor/models/CommentDisplayFilter-spec.js @@ -0,0 +1,36 @@ +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('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..71d8ad7950 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,9 @@ 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.filter.label': 'Filter 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 +198,49 @@ describe('CommentsView', () => { }); }); + describe('resolution filter', () => { + 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'); + }); + }); + 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/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/models/CommentDisplayFilter.js b/entry_types/scrolled/package/src/editor/models/CommentDisplayFilter.js new file mode 100644 index 0000000000..bab5a2649e --- /dev/null +++ b/entry_types/scrolled/package/src/editor/models/CommentDisplayFilter.js @@ -0,0 +1,33 @@ +import Backbone from 'backbone'; + +import {getLocalStorage} from 'pageflow/editor'; + +const storageKey = 'pageflow.scrolled.editor.commentsResolution'; + +// Which resolutions of a comment thread the editor displays, in its +// sidebar lists as well as in the preview. Remembered under a key 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' + }, + + initialize() { + if (getLocalStorage()?.[storageKey] === 'all') { + this.set('resolution', 'all'); + } + + this.listenTo(this, 'change:resolution', function() { + const storage = getLocalStorage(); + + if (storage) { + storage[storageKey] = this.get('resolution'); + } + }); + }, + + showsResolved() { + return this.get('resolution') === 'all'; + } +}); 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..516906bd31 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,22 @@ 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.filter.label'), + alignMenu: 'right', + ellipsisIcon: true, + borderless: true, + openOnClick: true, + items: new ResolutionMenuItems( + [{name: 'unresolved'}, {name: 'all'}], + {commentDisplayFilter: entry.commentDisplayFilter} + ) + }), {to: this.$(cssModulesUtils.selector(styles, 'controls'))}); this._updateNewThreadButton(); this._updateActivityButton(); @@ -107,6 +122,32 @@ export const CommentsView = Marionette.ItemView.extend({ } }); +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 +}); + 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({ Date: Thu, 27 Aug 2026 17:46:27 +0200 Subject: [PATCH 4/7] Open the selection's resolved fold when the filter shows all The selection tab keeps its fold, so the reviewer can still peek at resolved threads of the current selection without turning them on everywhere. The filter only decides whether it starts out open. --- .../views/SelectionCommentsView-spec.js | 62 +++++++++++++++++++ .../src/editor/views/SelectionCommentsView.js | 16 ++++- 2 files changed, 76 insertions(+), 2 deletions(-) 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/src/editor/views/SelectionCommentsView.js b/entry_types/scrolled/package/src/editor/views/SelectionCommentsView.js index a706ca72b6..5c58611212 100644 --- a/entry_types/scrolled/package/src/editor/views/SelectionCommentsView.js +++ b/entry_types/scrolled/package/src/editor/views/SelectionCommentsView.js @@ -17,6 +17,9 @@ export const SelectionCommentsView = ReviewView.extend({ this.listenTo(entry, 'change:highlightedThreadId', () => 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 /> From 1cb9a8d4faa56eed911175f87a4d071393e849a9 Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Thu, 27 Aug 2026 17:51:07 +0200 Subject: [PATCH 5/7] Show badges of resolved threads in the editor preview Hand the sidebar's resolution over to the preview iframe, on every reload and whenever the reviewer picks another one, and let the section and content element badges follow it. A subject whose threads are all resolved reads as grey, the same as in the published entry's preview mode. --- .../PreviewMessageController-spec.js | 54 ++++++++++++- .../contentElementCommentBadges-spec.js | 70 +++++++++++++++++ .../features/sectionCommentBadges-spec.js | 75 +++++++++++++++++++ .../controllers/PreviewMessageController.js | 14 ++++ .../inlineEditing/ContentElementDecorator.js | 4 +- .../frontend/inlineEditing/EntryDecorator.js | 34 +++++++-- .../inlineEditing/SectionDecorator.js | 8 +- 7 files changed, 250 insertions(+), 9 deletions(-) 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..501d8e20f8 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,58 @@ 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 recordResolutions(iframeWindow) { + const resolutions = []; + + iframeWindow.addEventListener('message', event => { + if (event.data.type === 'CHANGE_COMMENT_DISPLAY_FILTER') { + resolutions.push(event.data.payload.resolution); + } + }); + + return resolutions; + } + + // The iframe starts out showing unresolved threads only, so a filter + // set to all has to be handed over again on every reload. + it('sends the resolution the editor displays after READY', async () => { + const entry = createEntry(); + entry.commentDisplayFilter.set('resolution', 'all'); + const iframeWindow = createIframeWindow(); + controller = new PreviewMessageController({entry, iframeWindow}); + + const resolutions = recordResolutions(iframeWindow); + await postReadyMessageAndWaitForAcknowledgement(iframeWindow); + await tick(); + + expect(resolutions).toEqual(['all']); + }); + + it('sends the resolution when the reviewer changes the filter', async () => { + const entry = createEntry(); + const iframeWindow = createIframeWindow(); + controller = new PreviewMessageController({entry, iframeWindow}); + + const resolutions = recordResolutions(iframeWindow); + await postReadyMessageAndWaitForAcknowledgement(iframeWindow); + entry.commentDisplayFilter.set('resolution', 'all'); + await tick(); + + expect(resolutions).toEqual(['unresolved', 'all']); + }); + }); + 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/frontend/inlineEditing/features/contentElementCommentBadges-spec.js b/entry_types/scrolled/package/spec/frontend/inlineEditing/features/contentElementCommentBadges-spec.js index 0fd3284e17..b8b29cd7ec 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,76 @@ describe('inline editing content element comment badges', () => { delete Element.prototype.scrollIntoView; }); + 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..64fc5d91e4 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,81 @@ describe('inline editing section comment badges', () => { }); }); + 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/src/editor/controllers/PreviewMessageController.js b/entry_types/scrolled/package/src/editor/controllers/PreviewMessageController.js index 766b365897..083611b79c 100644 --- a/entry_types/scrolled/package/src/editor/controllers/PreviewMessageController.js +++ b/entry_types/scrolled/package/src/editor/controllers/PreviewMessageController.js @@ -167,6 +167,13 @@ export const PreviewMessageController = Object.extend({ }) ); + this.listenTo(this.entry.commentDisplayFilter, 'change:resolution', filter => + postMessage({ + type: 'CHANGE_COMMENT_DISPLAY_FILTER', + payload: {resolution: filter.get('resolution')} + }) + ); + this.listenTo(this.entry, 'change:emulation_mode', entry => postMessage({ type: 'CHANGE_EMULATION_MODE', @@ -178,6 +185,13 @@ export const PreviewMessageController = Object.extend({ postMessage({type: 'ACK'}) if (this.entry.reviewSession) { + // A reloaded iframe starts out showing unresolved threads only, + // so the filter has to be handed over again. + postMessage({ + type: 'CHANGE_COMMENT_DISPLAY_FILTER', + payload: {resolution: this.entry.commentDisplayFilter.get('resolution')} + }); + this.entry.reviewSession.fetch(); } } diff --git a/entry_types/scrolled/package/src/frontend/inlineEditing/ContentElementDecorator.js b/entry_types/scrolled/package/src/frontend/inlineEditing/ContentElementDecorator.js index 4123facb12..75e6390d5d 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} = useCommentDisplayFilter(); const {t} = useI18n({locale: 'ui'}); const selectionRectRef = useRef(); @@ -86,6 +87,7 @@ function DefaultSelectionRect(props) { commentBadge={features.isEnabled('commenting') && threads.length === 0 ? selectNewThread() : diff --git a/entry_types/scrolled/package/src/frontend/inlineEditing/EntryDecorator.js b/entry_types/scrolled/package/src/frontend/inlineEditing/EntryDecorator.js index 2ba09668cd..661c79dd2f 100644 --- a/entry_types/scrolled/package/src/frontend/inlineEditing/EntryDecorator.js +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/EntryDecorator.js @@ -1,6 +1,10 @@ -import React, {useEffect, useCallback} from 'react'; +import React, {useEffect, useCallback, useState} from 'react'; -import {ReviewStateProvider, LocatedCommentThreadsProvider} from 'pageflow-scrolled/review'; +import { + ReviewStateProvider, + LocatedCommentThreadsProvider, + CommentDisplayFilterProvider +} from 'pageflow-scrolled/review'; import {useEntryStateDispatch} from 'pageflow-scrolled/entryState'; import {usePostMessageListener} from '../../shared/usePostMessageListener'; import {EditorStateProvider, useEditorSelection} from './EditorState'; @@ -17,15 +21,35 @@ export function EntryDecorator({commentingInitialState, children}) { - - {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 [resolution, setResolution] = useState('unresolved'); + + usePostMessageListener(useCallback(data => { + if (data.type === 'CHANGE_COMMENT_DISPLAY_FILTER') { + setResolution(data.payload.resolution); + } + }, [])); + + 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..a89420d6c9 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} = useCommentDisplayFilter(); const threads = useLocatedCommentThreadsForSubject({ subjectType: 'Section', subjectId: section.permaId, - resolution: 'unresolved' + resolution }); const hasThreads = threads.length > 0; @@ -134,6 +137,7 @@ export function SectionDecorator({backdrop, section, contentElements, transition
hasThreads ? selectComments() : selectNewThread()} /> From e4c2c6f41b0d0a623d33ef699aac6a8ded5e8b82 Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Thu, 27 Aug 2026 17:54:54 +0200 Subject: [PATCH 6/7] Mark commented text of resolved threads in the editor preview Let the highlight overlay and the badge column of an editable text follow the editor's filter, so that text a resolved thread refers to reads as grey instead of staying unmarked until the thread is picked. --- .../features/commentBadges-spec.js | 42 +++++++++++++++++++ .../features/commentHighlights-spec.js | 35 ++++++++++++++++ .../EditableText/useCommenting.js | 15 ++++--- 3 files changed, 87 insertions(+), 5 deletions(-) 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..90df15f37f 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,48 @@ describe('inline editing EditableText comment badges', () => { expect(badge.isActive()).toBe(true); }); + 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..cff7703f74 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,41 @@ describe('inline editing EditableText comment highlights', () => { expect(highlight).not.toHaveClass(highlightStyles.selected); }); + 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/src/frontend/inlineEditing/EditableText/useCommenting.js b/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/useCommenting.js index f22d30e624..a589fa2d53 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, @@ -41,6 +42,7 @@ export function useCommenting(editor) { const {trackedThreads, resetRangeRefs, getTrackedSubjectRanges} = useCommentRangeRefs(editor, threads); const {anchors, registerAnchor} = useRangeAnchors(); + const {resolution} = useCommentDisplayFilter(); const {highlightedThreadId, newThreadRange, selectThread} = useContentElementCommentSelection(); @@ -70,15 +72,17 @@ export function useCommenting(editor) { // 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. + // a badge; a resolved thread stays hidden unless the editor's filter + // shows resolved threads or it is the highlighted thread. const highlights = useCommentHighlights(trackedThreads, newThreadRange); const visibleHighlights = useMemo( () => highlights.filter( - h => !h.thread?.resolvedAt || h.thread.id === highlightedThreadId + h => !h.thread?.resolvedAt || + resolution === 'all' || + h.thread.id === highlightedThreadId ), - [highlights, highlightedThreadId] + [highlights, highlightedThreadId, resolution] ); const decorate = useMemo( @@ -110,7 +114,8 @@ 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, + resolution]); return { enabled, From c70b256e4a6a138450bf10f76a36b12a7f2df2e2 Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Thu, 27 Aug 2026 18:14:37 +0200 Subject: [PATCH 7/7] Offer an uncluttered view of the entry in the editor Add a checkbox to the comments view menu that stops comments from being displayed on anything the reviewer has not selected: no badge dots on sections and content elements, no highlights and badges on commented text. What reads as selected keeps its comments: a selected section or content element shows its badge, the selection rect of an editable text shows the comments of the blocks it spans, and the thread opened from the sidebar keeps marking the text it refers to, so a comment can still be followed into the preview. Put the other way around, nothing displays that would have been a mere dot. Framed as "always show comments" and checked by default, mirroring the preview toolbar's show/hide toggle. The menu holds more than the resolution filter now, so its title speaks of comment display. --- entry_types/scrolled/config/locales/de.yml | 3 +- entry_types/scrolled/config/locales/en.yml | 3 +- .../PreviewMessageController-spec.js | 39 ++-- .../models/CommentDisplayFilter-spec.js | 17 ++ .../spec/editor/views/CommentsView-spec.js | 35 +++- .../features/commentBadges-spec.js | 89 +++++++++ .../features/commentHighlights-spec.js | 175 ++++++++++++++++++ ...spec.js => rangeOverlapsSelection-spec.js} | 56 +++--- .../contentElementCommentBadges-spec.js | 64 +++++++ .../features/sectionCommentBadges-spec.js | 55 ++++++ .../CommentDisplayFilterProvider-spec.js | 24 ++- .../controllers/PreviewMessageController.js | 19 +- .../src/editor/models/CommentDisplayFilter.js | 37 +++- .../package/src/editor/views/CommentsView.js | 41 +++- .../inlineEditing/ContentElementDecorator.js | 6 +- .../inlineEditing/EditableText/BadgeColumn.js | 43 ++--- .../commentThreadIdsAtSelection.js | 4 +- .../inlineEditing/EditableText/index.js | 5 +- ...Selection.js => rangeOverlapsSelection.js} | 8 +- .../EditableText/useCommenting.js | 62 +++++-- .../EditableText/useOverlapSelection.js | 27 +++ .../frontend/inlineEditing/EntryDecorator.js | 9 +- .../inlineEditing/SectionDecorator.js | 5 +- .../scrolled/package/src/review/Badge.js | 2 + .../review/CommentDisplayFilterProvider.js | 17 +- 25 files changed, 712 insertions(+), 133 deletions(-) rename entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/{highlightOverlapsSelection-spec.js => rangeOverlapsSelection-spec.js} (51%) rename entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/{highlightOverlapsSelection.js => rangeOverlapsSelection.js} (52%) create mode 100644 entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/useOverlapSelection.js diff --git a/entry_types/scrolled/config/locales/de.yml b/entry_types/scrolled/config/locales/de.yml index a3bdca26db..1fd0d047ae 100644 --- a/entry_types/scrolled/config/locales/de.yml +++ b/entry_types/scrolled/config/locales/de.yml @@ -1643,9 +1643,10 @@ de: comments: Kommentare comments_view: activity: Letzte Aktivität + always_show_comments: Kommentare immer anzeigen + display_options: Kommentaranzeige filter: all: Alle Themen - label: Kommentare filtern unresolved: Ungelöste Themen new_thread: Neues Thema section: Abschnitt diff --git a/entry_types/scrolled/config/locales/en.yml b/entry_types/scrolled/config/locales/en.yml index 40da18ac80..3ffe87a56b 100644 --- a/entry_types/scrolled/config/locales/en.yml +++ b/entry_types/scrolled/config/locales/en.yml @@ -1625,9 +1625,10 @@ en: comments: Comments comments_view: activity: Latest activity + always_show_comments: Always show comments + display_options: Comment display filter: all: All topics - label: Filter comments unresolved: Unresolved topics new_thread: New topic section: Section 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 501d8e20f8..60597196bf 100644 --- a/entry_types/scrolled/package/spec/editor/controllers/PreviewMessageController-spec.js +++ b/entry_types/scrolled/package/spec/editor/controllers/PreviewMessageController-spec.js @@ -1187,31 +1187,31 @@ describe('PreviewMessageController', () => { return factories.entry(ScrolledEntry, {}, {entryTypeSeed: normalizeSeed()}); } - function recordResolutions(iframeWindow) { - const resolutions = []; + function recordPayloads(iframeWindow) { + const payloads = []; iframeWindow.addEventListener('message', event => { if (event.data.type === 'CHANGE_COMMENT_DISPLAY_FILTER') { - resolutions.push(event.data.payload.resolution); + payloads.push(event.data.payload); } }); - return resolutions; + return payloads; } - // The iframe starts out showing unresolved threads only, so a filter - // set to all has to be handed over again on every reload. - it('sends the resolution the editor displays after READY', async () => { + // 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'); + entry.commentDisplayFilter.set({resolution: 'all', alwaysShowComments: false}); const iframeWindow = createIframeWindow(); controller = new PreviewMessageController({entry, iframeWindow}); - const resolutions = recordResolutions(iframeWindow); + const payloads = recordPayloads(iframeWindow); await postReadyMessageAndWaitForAcknowledgement(iframeWindow); await tick(); - expect(resolutions).toEqual(['all']); + expect(payloads).toEqual([{resolution: 'all', alwaysShowComments: false}]); }); it('sends the resolution when the reviewer changes the filter', async () => { @@ -1219,12 +1219,27 @@ describe('PreviewMessageController', () => { const iframeWindow = createIframeWindow(); controller = new PreviewMessageController({entry, iframeWindow}); - const resolutions = recordResolutions(iframeWindow); + const payloads = recordPayloads(iframeWindow); await postReadyMessageAndWaitForAcknowledgement(iframeWindow); entry.commentDisplayFilter.set('resolution', 'all'); await tick(); - expect(resolutions).toEqual(['unresolved', 'all']); + 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]); }); }); diff --git a/entry_types/scrolled/package/spec/editor/models/CommentDisplayFilter-spec.js b/entry_types/scrolled/package/spec/editor/models/CommentDisplayFilter-spec.js index aa48225a50..e4bc7bec9a 100644 --- a/entry_types/scrolled/package/spec/editor/models/CommentDisplayFilter-spec.js +++ b/entry_types/scrolled/package/spec/editor/models/CommentDisplayFilter-spec.js @@ -28,6 +28,23 @@ describe('CommentDisplayFilter', () => { 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'; 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 71d8ad7950..8b86d51e0b 100644 --- a/entry_types/scrolled/package/spec/editor/views/CommentsView-spec.js +++ b/entry_types/scrolled/package/spec/editor/views/CommentsView-spec.js @@ -30,7 +30,8 @@ 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.filter.label': 'Filter comments', + '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' @@ -198,7 +199,7 @@ describe('CommentsView', () => { }); }); - describe('resolution filter', () => { + describe('display options menu', () => { let menuContainer; beforeEach(() => { @@ -239,6 +240,36 @@ describe('CommentsView', () => { 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', () => { 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 90df15f37f..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,95 @@ 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'}]}]; 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 cff7703f74..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,181 @@ 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: { 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 b8b29cd7ec..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,70 @@ 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({ 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 64fc5d91e4..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,61 @@ 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({ diff --git a/entry_types/scrolled/package/spec/review/CommentDisplayFilterProvider-spec.js b/entry_types/scrolled/package/spec/review/CommentDisplayFilterProvider-spec.js index e6c7b6935c..065a906efe 100644 --- a/entry_types/scrolled/package/spec/review/CommentDisplayFilterProvider-spec.js +++ b/entry_types/scrolled/package/spec/review/CommentDisplayFilterProvider-spec.js @@ -14,9 +14,13 @@ describe('comment display filter', () => { }); function Probe() { - const {resolution, setResolution} = useCommentDisplayFilter(); + const {resolution, alwaysShowComments, setResolution} = useCommentDisplayFilter(); - return ; + return ( + + ); } function Remembered({storageKey}) { @@ -72,4 +76,20 @@ describe('comment display filter', () => { 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/src/editor/controllers/PreviewMessageController.js b/entry_types/scrolled/package/src/editor/controllers/PreviewMessageController.js index 083611b79c..e467c8d1d4 100644 --- a/entry_types/scrolled/package/src/editor/controllers/PreviewMessageController.js +++ b/entry_types/scrolled/package/src/editor/controllers/PreviewMessageController.js @@ -167,10 +167,12 @@ export const PreviewMessageController = Object.extend({ }) ); - this.listenTo(this.entry.commentDisplayFilter, 'change:resolution', filter => + this.listenTo(this.entry.commentDisplayFilter, + 'change:resolution change:alwaysShowComments', + filter => postMessage({ type: 'CHANGE_COMMENT_DISPLAY_FILTER', - payload: {resolution: filter.get('resolution')} + payload: commentDisplayFilterPayload(filter) }) ); @@ -185,11 +187,11 @@ export const PreviewMessageController = Object.extend({ postMessage({type: 'ACK'}) if (this.entry.reviewSession) { - // A reloaded iframe starts out showing unresolved threads only, - // so the filter has to be handed over again. + // 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: {resolution: this.entry.commentDisplayFilter.get('resolution')} + payload: commentDisplayFilterPayload(this.entry.commentDisplayFilter) }); this.entry.reviewSession.fetch(); @@ -354,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 index bab5a2649e..c2615fbf75 100644 --- a/entry_types/scrolled/package/src/editor/models/CommentDisplayFilter.js +++ b/entry_types/scrolled/package/src/editor/models/CommentDisplayFilter.js @@ -2,28 +2,37 @@ import Backbone from 'backbone'; import {getLocalStorage} from 'pageflow/editor'; -const storageKey = 'pageflow.scrolled.editor.commentsResolution'; +const resolutionStorageKey = 'pageflow.scrolled.editor.commentsResolution'; +const alwaysShowStorageKey = 'pageflow.scrolled.editor.alwaysShowComments'; -// Which resolutions of a comment thread the editor displays, in its -// sidebar lists as well as in the preview. Remembered under a key of its -// own, so that the editor and the published entry's preview mode do not +// 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' + resolution: 'unresolved', + alwaysShowComments: true }, initialize() { - if (getLocalStorage()?.[storageKey] === 'all') { + 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() { - const storage = getLocalStorage(); + store(resolutionStorageKey, this.get('resolution')); + }); - if (storage) { - storage[storageKey] = this.get('resolution'); - } + this.listenTo(this, 'change:alwaysShowComments', function() { + store(alwaysShowStorageKey, this.get('alwaysShowComments')); }); }, @@ -31,3 +40,11 @@ export const CommentDisplayFilter = Backbone.Model.extend({ 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/views/CommentsView.js b/entry_types/scrolled/package/src/editor/views/CommentsView.js index 516906bd31..c54ecdeb8c 100644 --- a/entry_types/scrolled/package/src/editor/views/CommentsView.js +++ b/entry_types/scrolled/package/src/editor/views/CommentsView.js @@ -64,15 +64,12 @@ export const CommentsView = Marionette.ItemView.extend({ `); this.appendSubview(new DropDownButtonView({ - title: I18n.t('pageflow_scrolled.editor.comments_view.filter.label'), + title: I18n.t('pageflow_scrolled.editor.comments_view.display_options'), alignMenu: 'right', ellipsisIcon: true, borderless: true, openOnClick: true, - items: new ResolutionMenuItems( - [{name: 'unresolved'}, {name: 'all'}], - {commentDisplayFilter: entry.commentDisplayFilter} - ) + items: displayOptions(entry.commentDisplayFilter) }), {to: this.$(cssModulesUtils.selector(styles, 'controls'))}); this._updateNewThreadButton(); @@ -122,6 +119,17 @@ 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; @@ -148,6 +156,29 @@ 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/frontend/inlineEditing/ContentElementDecorator.js b/entry_types/scrolled/package/src/frontend/inlineEditing/ContentElementDecorator.js index 75e6390d5d..7826ae89ba 100644 --- a/entry_types/scrolled/package/src/frontend/inlineEditing/ContentElementDecorator.js +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/ContentElementDecorator.js @@ -54,7 +54,7 @@ function DefaultSelectionRect(props) { const {isSelected, type, select, selectComments, selectNewThread} = useContentElementEditorState(); const commentsSelected = type === 'contentElementComments' || type === 'newThread'; - const {resolution} = useCommentDisplayFilter(); + const {resolution, alwaysShowComments} = useCommentDisplayFilter(); const {t} = useI18n({locale: 'ui'}); const selectionRectRef = useRef(); @@ -88,7 +88,9 @@ function DefaultSelectionRect(props) { 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 a589fa2d53..1e6f78fc9c 100644 --- a/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/useCommenting.js +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/useCommenting.js @@ -18,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 = []; @@ -69,20 +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 unless the editor's filter - // shows resolved threads or 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 || - resolution === 'all' || - h.thread.id === highlightedThreadId - ), - [highlights, highlightedThreadId, resolution] + () => 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( @@ -93,7 +100,10 @@ export function useCommenting(editor) { const withCommentHighlightDecoration = useCallback(({attributes, children, leaf}) => { if (leaf.commentHighlight) { children = ( - + {children} ); @@ -115,12 +125,13 @@ export function useCommenting(editor) { // equality function does not compare `decorations`. // eslint-disable-next-line react-hooks/exhaustive-deps }, [registerAnchor, contentElementPermaId, threads, newThreadRange, highlightedThreadId, - resolution]); + highlightedRange, resolution]); return { enabled, highlights, visibleHighlights, + highlightedRange, anchors, decorate, withCommentHighlightDecoration, @@ -129,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. @@ -148,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 ( { if (data.type === 'CHANGE_COMMENT_DISPLAY_FILTER') { - setResolution(data.payload.resolution); + setFilter(data.payload); } }, [])); return ( - + {children} ); diff --git a/entry_types/scrolled/package/src/frontend/inlineEditing/SectionDecorator.js b/entry_types/scrolled/package/src/frontend/inlineEditing/SectionDecorator.js index a89420d6c9..5161c83932 100644 --- a/entry_types/scrolled/package/src/frontend/inlineEditing/SectionDecorator.js +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/SectionDecorator.js @@ -53,7 +53,7 @@ export function SectionDecorator({backdrop, section, contentElements, transition // section and the sidebar comment panel stay visually in sync. const isSelected = isSectionSelected || isPaddingSelected || commentsSelected; - const {resolution} = useCommentDisplayFilter(); + const {resolution, alwaysShowComments} = useCommentDisplayFilter(); const threads = useLocatedCommentThreadsForSubject({ subjectType: 'Section', subjectId: section.permaId, @@ -139,7 +139,8 @@ export function SectionDecorator({backdrop, section, contentElements, transition subjectId={section.permaId} resolution={resolution} mode={commentsSelected ? 'active' : - isSelected ? 'icon' : 'dot'} + isSelected ? 'icon' : + alwaysShowComments ? 'dot' : 'none'} onClick={() => 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 index 8e5eb7004e..581cce4315 100644 --- a/entry_types/scrolled/package/src/review/CommentDisplayFilterProvider.js +++ b/entry_types/scrolled/package/src/review/CommentDisplayFilterProvider.js @@ -4,17 +4,22 @@ const noop = () => {}; const CommentDisplayFilterContext = createContext({ resolution: 'unresolved', + alwaysShowComments: true, setResolution: noop }); -// Which resolutions of a thread the reviewer wants to see. 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 the resolution its sidebar menu holds. +// 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', setResolution = noop, children + resolution = 'unresolved', alwaysShowComments = true, setResolution = noop, children }) { - const value = useMemo(() => ({resolution, setResolution}), [resolution, setResolution]); + const value = useMemo( + () => ({resolution, alwaysShowComments, setResolution}), + [resolution, alwaysShowComments, setResolution] + ); return (