From e92b2cfa50e0a6acc3fe1f4e389ac8bcf0426339 Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Mon, 22 Jun 2026 13:36:52 +0200 Subject: [PATCH 01/10] Only set up floating UI for open comment popovers The commenting Popover called useFloating unconditionally, so every section and content element set up floating-positioning state to back a thread-list popover that is only ever shown while selected - and a badge that renders nothing at all when there are no unresolved threads. Move useFloating and the floating thread list into a child component that mounts only while the popover is selected. The badge stays always-mounted and serves as the floating reference; outside-click and Escape dismissal move into the open child alongside the floating state they depend on. --- .../features/commentingBadges-spec.js | 48 ++++++++++ .../src/frontend/commenting/Popover.js | 88 +++++++++++-------- 2 files changed, 101 insertions(+), 35 deletions(-) diff --git a/entry_types/scrolled/package/spec/frontend/commenting/features/commentingBadges-spec.js b/entry_types/scrolled/package/spec/frontend/commenting/features/commentingBadges-spec.js index e31aaf1ec9..c075762a66 100644 --- a/entry_types/scrolled/package/spec/frontend/commenting/features/commentingBadges-spec.js +++ b/entry_types/scrolled/package/spec/frontend/commenting/features/commentingBadges-spec.js @@ -76,4 +76,52 @@ describe('commenting badges', () => { expect(entry.getByText('On the section')).toBeInTheDocument(); }); + + it('closes the thread list when clicking outside', () => { + const entry = renderEntry({ + seed: { + sections: [{id: 1, permaId: 10}], + contentElements: [{typeName: 'withTestId', configuration: {testId: 5}}] + }, + commenting: { + currentUser: {id: 42, name: 'Alice'}, + commentThreads: [ + {id: 1, subjectType: 'Section', subjectId: 10, comments: [ + {id: 10, body: 'On the section', creatorName: 'Bob', creatorId: 2} + ]} + ] + } + }); + + fireEvent.click(entry.getAllCommentBadges()[0]); + expect(entry.getByText('On the section')).toBeInTheDocument(); + + fireEvent.mouseDown(document.body); + + expect(entry.queryByText('On the section')).not.toBeInTheDocument(); + }); + + it('closes the thread list when pressing Escape', () => { + const entry = renderEntry({ + seed: { + sections: [{id: 1, permaId: 10}], + contentElements: [{typeName: 'withTestId', configuration: {testId: 5}}] + }, + commenting: { + currentUser: {id: 42, name: 'Alice'}, + commentThreads: [ + {id: 1, subjectType: 'Section', subjectId: 10, comments: [ + {id: 10, body: 'On the section', creatorName: 'Bob', creatorId: 2} + ]} + ] + } + }); + + fireEvent.click(entry.getAllCommentBadges()[0]); + expect(entry.getByText('On the section')).toBeInTheDocument(); + + fireEvent.keyDown(document, {key: 'Escape'}); + + expect(entry.queryByText('On the section')).not.toBeInTheDocument(); + }); }); diff --git a/entry_types/scrolled/package/src/frontend/commenting/Popover.js b/entry_types/scrolled/package/src/frontend/commenting/Popover.js index 9b503420cb..89e254ecb6 100644 --- a/entry_types/scrolled/package/src/frontend/commenting/Popover.js +++ b/entry_types/scrolled/package/src/frontend/commenting/Popover.js @@ -1,4 +1,4 @@ -import React, {useEffect} from 'react'; +import React, {useEffect, useState} from 'react'; import { useFloating, FloatingPortal, offset, flip, shift, autoUpdate @@ -16,10 +16,47 @@ export function Popover({ }) { const {isSelected, showNewForm, select, clearSelection} = useSelectedSubject(subjectType, subjectId, subjectRange); + const [reference, setReference] = useState(null); + + function handleBadgeClick() { + if (isSelected) { + clearSelection(); + } + else { + select(); + } + } + + return ( + + + {isSelected && + } + + ); +} + +function OpenThreadList({ + reference, subjectType, subjectId, subjectRange, + placement, strategy, showNewForm, hideNewTopicButton, onDismiss +}) { const portalRoot = useFloatingPortalRoot(); const {refs, floatingStyles} = useFloating({ - open: isSelected, + open: true, + elements: {reference}, placement, strategy, middleware: [ @@ -33,29 +70,18 @@ export function Popover({ whileElementsMounted: autoUpdate }); - function handleBadgeClick() { - if (isSelected) { - clearSelection(); - } - else { - select(); - } - } - useEffect(() => { - if (!isSelected) return; - function handleClick(event) { - if (refs.reference.current?.contains(event.target)) return; + if (reference?.contains(event.target)) return; if (refs.floating.current?.contains(event.target)) return; if (event.target.closest('[data-comment-highlight]')) return; - clearSelection(); + onDismiss(); } function handleKeyDown(event) { if (event.key === 'Escape') { - clearSelection(); + onDismiss(); } } @@ -66,28 +92,20 @@ export function Popover({ document.removeEventListener('mousedown', handleClick); document.removeEventListener('keydown', handleKeyDown); }; - }, [isSelected, clearSelection, refs.reference, refs.floating]); + }, [reference, refs.floating, onDismiss]); return ( - - +
+ - {isSelected && - -
- -
- } - + showNewForm={showNewForm} + hideNewTopicButton={hideNewTopicButton} /> +
+ ); } From 89094f9f4051fdad8349f97498452875ebdec5cd Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Mon, 22 Jun 2026 14:25:13 +0200 Subject: [PATCH 02/10] Add resolution filter to comment display Add a CommentDisplayFilterProvider holding a resolution state (unresolved or all) and a segmented Unresolved/All control in the floating toolbar to switch between them. The commenting section and content element decorators, the preview popovers, and the editable text highlights read the filter instead of always showing only unresolved threads, so it reveals or hides resolved comments across the whole entry. ThreadsBadge gains an optional resolution prop defaulting to unresolved, leaving the inline editing badges unchanged. --- entry_types/scrolled/config/locales/de.yml | 4 + entry_types/scrolled/config/locales/en.yml | 4 + .../features/commentDisplayFilter-spec.js | 80 +++++++++++++++++++ .../spec/support/pageObjects/commenting.js | 9 ++- .../CommentDisplayFilterProvider.js | 22 +++++ .../src/frontend/commenting/EditableText.js | 4 +- .../src/frontend/commenting/EntryDecorator.js | 7 +- .../frontend/commenting/FloatingToolbar.js | 49 +++++++++--- .../commenting/FloatingToolbar.module.css | 24 ++++++ .../src/frontend/commenting/Popover.js | 3 + .../frontend/commenting/SectionDecorator.js | 4 +- .../package/src/review/ThreadsBadge.js | 4 +- 12 files changed, 197 insertions(+), 17 deletions(-) create mode 100644 entry_types/scrolled/package/spec/frontend/commenting/features/commentDisplayFilter-spec.js create mode 100644 entry_types/scrolled/package/src/frontend/commenting/CommentDisplayFilterProvider.js diff --git a/entry_types/scrolled/config/locales/de.yml b/entry_types/scrolled/config/locales/de.yml index 4210f71956..4e651e78b3 100644 --- a/entry_types/scrolled/config/locales/de.yml +++ b/entry_types/scrolled/config/locales/de.yml @@ -1926,6 +1926,10 @@ de: review: add_comment: Kommentar hinzufügen cancel_add_comment: Abbrechen + filter: + label: Kommentare filtern + unresolved: Ungelöst + all: Alle select_content_element: Zum Kommentieren auswählen select_section: Abschnitt zum Kommentieren auswählen select_text_to_comment: Text zum Kommentieren auswählen diff --git a/entry_types/scrolled/config/locales/en.yml b/entry_types/scrolled/config/locales/en.yml index 3e7b8f3a24..3c07cf16ae 100644 --- a/entry_types/scrolled/config/locales/en.yml +++ b/entry_types/scrolled/config/locales/en.yml @@ -1755,6 +1755,10 @@ en: review: add_comment: Add comment cancel_add_comment: Cancel + filter: + label: Filter comments + unresolved: Unresolved + all: All select_content_element: Select to comment select_section: Select section to comment select_text_to_comment: Select text to comment 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 new file mode 100644 index 0000000000..438ec71306 --- /dev/null +++ b/entry_types/scrolled/package/spec/frontend/commenting/features/commentDisplayFilter-spec.js @@ -0,0 +1,80 @@ +import '@testing-library/jest-dom/extend-expect'; +import {fireEvent} from '@testing-library/react'; + +import {renderEntry, useCommentingPageObjects} from 'support/pageObjects/commenting'; + +describe('comment display filter', () => { + useCommentingPageObjects(); + + it('shows unresolved comments with the unresolved segment active by default', () => { + const entry = renderEntryWithResolvedThread(); + + expect(entry.queryAllCommentBadges()).toHaveLength(0); + expect(entry.getCommentFilterButton('unresolved')) + .toHaveAttribute('aria-pressed', 'true'); + expect(entry.getCommentFilterButton('all')) + .toHaveAttribute('aria-pressed', 'false'); + }); + + it('shows resolved comments when selecting all', () => { + const entry = renderEntryWithResolvedThread(); + + fireEvent.click(entry.getCommentFilterButton('all')); + + expect(entry.queryAllCommentBadges()).toHaveLength(1); + expect(entry.getCommentFilterButton('all')) + .toHaveAttribute('aria-pressed', 'true'); + }); + + it('shows both resolved and unresolved comments when showing all', () => { + const entry = renderEntry({ + seed: { + contentElements: [ + {typeName: 'withTestId', configuration: {testId: 5}}, + {typeName: 'withTestId', configuration: {testId: 6}} + ] + }, + commenting: { + currentUser: {id: 42, name: 'Alice'}, + commentThreads: [ + {id: 1, subjectType: 'ContentElement', subjectId: 1, resolvedAt: null, + comments: [{id: 10, body: 'Open', creatorName: 'Bob', creatorId: 2}]}, + {id: 2, subjectType: 'ContentElement', subjectId: 2, resolvedAt: '2026-01-01', + comments: [{id: 11, body: 'Done', creatorName: 'Bob', creatorId: 2}]} + ] + } + }); + + expect(entry.queryAllCommentBadges()).toHaveLength(1); + + fireEvent.click(entry.getCommentFilterButton('all')); + + expect(entry.queryAllCommentBadges()).toHaveLength(2); + }); + + it('hides resolved comments again when selecting unresolved', () => { + const entry = renderEntryWithResolvedThread(); + + fireEvent.click(entry.getCommentFilterButton('all')); + expect(entry.queryAllCommentBadges()).toHaveLength(1); + + fireEvent.click(entry.getCommentFilterButton('unresolved')); + + expect(entry.queryAllCommentBadges()).toHaveLength(0); + }); + + function renderEntryWithResolvedThread() { + return renderEntry({ + seed: { + contentElements: [{typeName: 'withTestId', configuration: {testId: 5}}] + }, + commenting: { + currentUser: {id: 42, name: 'Alice'}, + commentThreads: [ + {id: 1, subjectType: 'ContentElement', subjectId: 1, resolvedAt: '2026-01-01', + comments: [{id: 10, body: 'Done', creatorName: 'Bob', creatorId: 2}]} + ] + } + }); + } +}); diff --git a/entry_types/scrolled/package/spec/support/pageObjects/commenting.js b/entry_types/scrolled/package/spec/support/pageObjects/commenting.js index 95799971e6..75a009cdc0 100644 --- a/entry_types/scrolled/package/spec/support/pageObjects/commenting.js +++ b/entry_types/scrolled/package/spec/support/pageObjects/commenting.js @@ -28,7 +28,9 @@ export function renderEntry({ getNewThreadInput: () => result.getByPlaceholderText('Add a comment...'), queryNewThreadInput: () => result.queryByPlaceholderText('Add a comment...'), getAllCommentBadges: () => result.getAllByRole('status'), - queryAllCommentBadges: () => result.queryAllByRole('status') + queryAllCommentBadges: () => result.queryAllByRole('status'), + getCommentFilterButton: resolution => + result.getByRole('button', {name: resolution === 'all' ? 'All' : 'Unresolved'}) }; } @@ -46,7 +48,10 @@ export function useCommentingPageObjects() { 'pageflow_scrolled.review.cancel_add_comment': 'Cancel add comment', 'pageflow_scrolled.review.select_content_element': 'Select to comment', 'pageflow_scrolled.review.select_section': 'Select section to comment', - 'pageflow_scrolled.review.add_comment_placeholder': 'Add a comment...' + 'pageflow_scrolled.review.add_comment_placeholder': 'Add a comment...', + 'pageflow_scrolled.review.filter.label': 'Filter comments', + 'pageflow_scrolled.review.filter.unresolved': 'Unresolved', + 'pageflow_scrolled.review.filter.all': 'All' }); usePageObjects(); diff --git a/entry_types/scrolled/package/src/frontend/commenting/CommentDisplayFilterProvider.js b/entry_types/scrolled/package/src/frontend/commenting/CommentDisplayFilterProvider.js new file mode 100644 index 0000000000..1faeb73176 --- /dev/null +++ b/entry_types/scrolled/package/src/frontend/commenting/CommentDisplayFilterProvider.js @@ -0,0 +1,22 @@ +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 c36930e432..d8d07e27c4 100644 --- a/entry_types/scrolled/package/src/frontend/commenting/EditableText.js +++ b/entry_types/scrolled/package/src/frontend/commenting/EditableText.js @@ -8,6 +8,7 @@ import {useCommentThreads, useCommentHighlights, decorateCommentHighlights, useR import {PlainEditableText, renderElement, renderLeaf} from '../EditableText'; import {useContentElementAttributes} from '../useContentElementAttributes'; import {useAddCommentMode} from './AddCommentModeProvider'; +import {useCommentDisplayFilter} from './CommentDisplayFilterProvider'; import {useSelectedSubject} from './SelectedSubjectProvider'; import {AddCommentHint} from './AddCommentHint'; import {PopoversColumn} from './PopoversColumn'; @@ -40,10 +41,11 @@ function CommentingEditableText({ const {contentElementPermaId} = useContentElementAttributes(); const {active, deactivate, preselect, clearPreselection} = useAddCommentMode(); const {subjectRange, select} = useSelectedSubject('ContentElement', contentElementPermaId); + const {resolution} = useCommentDisplayFilter(); const threads = useCommentThreads({ subjectType: 'ContentElement', subjectId: contentElementPermaId, - resolution: 'unresolved' + resolution }); const highlights = useCommentHighlights(threads, subjectRange); diff --git a/entry_types/scrolled/package/src/frontend/commenting/EntryDecorator.js b/entry_types/scrolled/package/src/frontend/commenting/EntryDecorator.js index a75ba008a4..eb3c86b435 100644 --- a/entry_types/scrolled/package/src/frontend/commenting/EntryDecorator.js +++ b/entry_types/scrolled/package/src/frontend/commenting/EntryDecorator.js @@ -4,6 +4,7 @@ import {useEntryMetadata} from 'pageflow-scrolled/entryState'; import {createReviewSession} from 'pageflow/review'; import {ReviewStateProvider, ReviewMessageHandler} from 'pageflow-scrolled/review'; import {AddCommentModeProvider} from './AddCommentModeProvider'; +import {CommentDisplayFilterProvider} from './CommentDisplayFilterProvider'; import {SelectedSubjectProvider} from './SelectedSubjectProvider'; import {FloatingToolbar} from './FloatingToolbar'; @@ -13,8 +14,10 @@ export function EntryDecorator({commentingInitialState, children}) { - {children} - + + {children} + + diff --git a/entry_types/scrolled/package/src/frontend/commenting/FloatingToolbar.js b/entry_types/scrolled/package/src/frontend/commenting/FloatingToolbar.js index 87b76743e6..c618ced9e4 100644 --- a/entry_types/scrolled/package/src/frontend/commenting/FloatingToolbar.js +++ b/entry_types/scrolled/package/src/frontend/commenting/FloatingToolbar.js @@ -2,12 +2,45 @@ import React from 'react'; import {useI18n} from '../i18n'; import {useAddCommentMode} from './AddCommentModeProvider'; +import {useCommentDisplayFilter} from './CommentDisplayFilterProvider'; import AddCommentIcon from './images/addComment.svg'; import CancelCommentIcon from './images/cancelComment.svg'; import styles from './FloatingToolbar.module.css'; export function FloatingToolbar() { + return ( +
+ + +
+ ); +} + +const resolutions = ['unresolved', 'all']; + +function ResolutionToggleButton() { + const {t} = useI18n({locale: 'ui'}); + const {resolution, setResolution} = useCommentDisplayFilter(); + + return ( +
+ {resolutions.map(value => ( + + ))} +
+ ); +} + +function AddCommentButton() { const {t} = useI18n({locale: 'ui'}); const {active, toggle} = useAddCommentMode(); @@ -17,14 +50,12 @@ export function FloatingToolbar() { : 'pageflow_scrolled.review.add_comment'); return ( -
- -
+ ); } diff --git a/entry_types/scrolled/package/src/frontend/commenting/FloatingToolbar.module.css b/entry_types/scrolled/package/src/frontend/commenting/FloatingToolbar.module.css index cf63497b7c..1b56b28cb8 100644 --- a/entry_types/scrolled/package/src/frontend/commenting/FloatingToolbar.module.css +++ b/entry_types/scrolled/package/src/frontend/commenting/FloatingToolbar.module.css @@ -4,6 +4,9 @@ right: 0; z-index: 1100; padding: space(2); + display: flex; + align-items: center; + gap: space(2); } .button { @@ -18,3 +21,24 @@ display: flex; align-items: center; } + +.segmented { + display: flex; + border-radius: rounded(md); + overflow: hidden; + box-shadow: var(--ui-box-shadow-md); +} + +.segment { + font-family: var(--ui-font-family); + color: var(--ui-on-surface-color); + background: var(--ui-surface-color); + border: none; + padding: space(2) space(3); + cursor: pointer; +} + +.segment[aria-pressed='true'] { + color: var(--ui-on-primary-color); + background: var(--ui-primary-color); +} diff --git a/entry_types/scrolled/package/src/frontend/commenting/Popover.js b/entry_types/scrolled/package/src/frontend/commenting/Popover.js index 89e254ecb6..354e6ec7f3 100644 --- a/entry_types/scrolled/package/src/frontend/commenting/Popover.js +++ b/entry_types/scrolled/package/src/frontend/commenting/Popover.js @@ -6,6 +6,7 @@ import { import {ThreadsBadge, ThreadList} from 'pageflow-scrolled/review'; import {useFloatingPortalRoot} from '../FloatingPortalRootProvider'; +import {useCommentDisplayFilter} from './CommentDisplayFilterProvider'; import {useSelectedSubject} from './SelectedSubjectProvider'; import styles from './Popover.module.css'; @@ -16,6 +17,7 @@ export function Popover({ }) { const {isSelected, showNewForm, select, clearSelection} = useSelectedSubject(subjectType, subjectId, subjectRange); + const {resolution} = useCommentDisplayFilter(); const [reference, setReference] = useState(null); function handleBadgeClick() { @@ -32,6 +34,7 @@ export function Popover({ {isSelected && diff --git a/entry_types/scrolled/package/src/frontend/commenting/SectionDecorator.js b/entry_types/scrolled/package/src/frontend/commenting/SectionDecorator.js index b0688d0115..64da981d2b 100644 --- a/entry_types/scrolled/package/src/frontend/commenting/SectionDecorator.js +++ b/entry_types/scrolled/package/src/frontend/commenting/SectionDecorator.js @@ -4,6 +4,7 @@ import classNames from 'classnames'; import {useCommentThreads} from 'pageflow-scrolled/review'; import {useI18n} from '../i18n'; import {useAddCommentMode} from './AddCommentModeProvider'; +import {useCommentDisplayFilter} from './CommentDisplayFilterProvider'; import {useSelectedSubject} from './SelectedSubjectProvider'; import {Popover} from './Popover'; @@ -13,10 +14,11 @@ import styles from './SectionDecorator.module.css'; export function SectionDecorator({section, children}) { const {active} = useAddCommentMode(); const {isSelected} = useSelectedSubject('Section', section.permaId); + const {resolution} = useCommentDisplayFilter(); const threads = useCommentThreads({ subjectType: 'Section', subjectId: section.permaId, - resolution: 'unresolved' + resolution }); const hasThreads = threads.length > 0; diff --git a/entry_types/scrolled/package/src/review/ThreadsBadge.js b/entry_types/scrolled/package/src/review/ThreadsBadge.js index 6179cfa013..0c73dbe5c0 100644 --- a/entry_types/scrolled/package/src/review/ThreadsBadge.js +++ b/entry_types/scrolled/package/src/review/ThreadsBadge.js @@ -3,8 +3,8 @@ import React, {useCallback} from 'react'; import {useCommentThreads} from './ReviewStateProvider'; import {Badge} from './Badge'; -export function ThreadsBadge({subjectType, subjectId, subjectRange, onClick, mode}) { - const threads = useCommentThreads({subjectType, subjectId, subjectRange, resolution: 'unresolved'}); +export function ThreadsBadge({subjectType, subjectId, subjectRange, onClick, mode, resolution = 'unresolved'}) { + const threads = useCommentThreads({subjectType, subjectId, subjectRange, resolution}); const handleClick = useCallback(() => { if (onClick) onClick(threads); From a21741790b2f73dea4ef684e3d0ec47cb83ba334 Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Mon, 22 Jun 2026 14:35:08 +0200 Subject: [PATCH 03/10] Add comment navigation to floating toolbar Add a CommentNavigationProvider that derives the located comment threads matching the current display filter in document order and exposes a count together with goToNext and goToPrevious actions. The floating toolbar shows the count and up/down arrows to cycle through the comments, wrapping around at the ends. Navigation is published through an emitter: each preview popover registers as the target for its subject and, when navigated to, scrolls into view and opens. Targets carry their section and excursion location so a later change can activate the right excursion before scrolling. --- entry_types/scrolled/config/locales/de.yml | 7 ++ entry_types/scrolled/config/locales/en.yml | 7 ++ .../features/commentNavigation-spec.js | 101 ++++++++++++++++ .../spec/support/pageObjects/commenting.js | 11 +- .../src/frontend/commenting/EntryDecorator.js | 12 +- .../frontend/commenting/FloatingToolbar.js | 40 ++++++- .../commenting/FloatingToolbar.module.css | 21 ++++ .../src/frontend/commenting/Popover.js | 14 ++- .../commenting/SelectedSubjectProvider.js | 113 +++++++++++++++++- .../frontend/commenting/images/chevron.svg | 1 + 10 files changed, 312 insertions(+), 15 deletions(-) create mode 100644 entry_types/scrolled/package/spec/frontend/commenting/features/commentNavigation-spec.js create mode 100644 entry_types/scrolled/package/src/frontend/commenting/images/chevron.svg diff --git a/entry_types/scrolled/config/locales/de.yml b/entry_types/scrolled/config/locales/de.yml index 4e651e78b3..93052ddad2 100644 --- a/entry_types/scrolled/config/locales/de.yml +++ b/entry_types/scrolled/config/locales/de.yml @@ -1926,10 +1926,17 @@ de: review: add_comment: Kommentar hinzufügen cancel_add_comment: Abbrechen + comment_toolbar: Kommentare filter: label: Kommentare filtern unresolved: Ungelöst all: Alle + previous_comment: Vorheriger Kommentar + next_comment: Nächster Kommentar + comment_count: + zero: Keine Kommentare + one: 1 Kommentar + other: '%{count} Kommentare' select_content_element: Zum Kommentieren auswählen select_section: Abschnitt zum Kommentieren auswählen select_text_to_comment: Text zum Kommentieren auswählen diff --git a/entry_types/scrolled/config/locales/en.yml b/entry_types/scrolled/config/locales/en.yml index 3c07cf16ae..acf8f3da58 100644 --- a/entry_types/scrolled/config/locales/en.yml +++ b/entry_types/scrolled/config/locales/en.yml @@ -1755,10 +1755,17 @@ en: review: add_comment: Add comment cancel_add_comment: Cancel + comment_toolbar: Comments filter: label: Filter comments unresolved: Unresolved all: All + previous_comment: Previous comment + next_comment: Next comment + comment_count: + zero: No comments + one: 1 comment + other: '%{count} comments' select_content_element: Select to comment select_section: Select section to comment select_text_to_comment: Select text to comment diff --git a/entry_types/scrolled/package/spec/frontend/commenting/features/commentNavigation-spec.js b/entry_types/scrolled/package/spec/frontend/commenting/features/commentNavigation-spec.js new file mode 100644 index 0000000000..2b16901282 --- /dev/null +++ b/entry_types/scrolled/package/spec/frontend/commenting/features/commentNavigation-spec.js @@ -0,0 +1,101 @@ +import '@testing-library/jest-dom/extend-expect'; +import {fireEvent, within} from '@testing-library/react'; + +import {renderEntry, useCommentingPageObjects} from 'support/pageObjects/commenting'; + +describe('comment navigation', () => { + useCommentingPageObjects(); + + beforeEach(() => { + window.HTMLElement.prototype.scrollIntoView = jest.fn(); + }); + + it('shows the number of navigable comments for the current filter', () => { + const entry = renderEntryWithTwoThreads(); + + expect(within(entry.getCommentToolbar()).getByTitle('2 comments')).toBeInTheDocument(); + }); + + it('opens each thread popover when cycling forward', () => { + const entry = renderEntryWithTwoThreads(); + + const next = entry.getNextCommentButton(); + + fireEvent.click(next); + expect(entry.getByText('First comment')).toBeInTheDocument(); + + fireEvent.click(next); + expect(entry.getByText('Second comment')).toBeInTheDocument(); + expect(entry.queryByText('First comment')).not.toBeInTheDocument(); + }); + + it('wraps to the last thread when cycling backward from the start', () => { + const entry = renderEntryWithTwoThreads(); + + fireEvent.click(entry.getPreviousCommentButton()); + + expect(entry.getByText('Second comment')).toBeInTheDocument(); + }); + + it('disables navigation when there are no comments for the filter', () => { + const entry = renderEntry({ + seed: {contentElements: [{typeName: 'withTestId', configuration: {testId: 5}}]}, + commenting: {currentUser: {id: 42, name: 'Alice'}, commentThreads: []} + }); + + expect(entry.getNextCommentButton()).toBeDisabled(); + expect(entry.getPreviousCommentButton()).toBeDisabled(); + }); + + it('skips resolved comments while navigating', () => { + const entry = renderEntry({ + seed: { + contentElements: [ + {typeName: 'withTestId', configuration: {testId: 5}}, + {typeName: 'withTestId', configuration: {testId: 6}}, + {typeName: 'withTestId', configuration: {testId: 7}} + ] + }, + commenting: { + currentUser: {id: 42, name: 'Alice'}, + commentThreads: [ + {id: 1, subjectType: 'ContentElement', subjectId: 1, resolvedAt: null, + comments: [{id: 10, body: 'First comment', creatorName: 'Bob', creatorId: 2}]}, + {id: 2, subjectType: 'ContentElement', subjectId: 2, resolvedAt: '2026-01-01', + comments: [{id: 11, body: 'Resolved comment', creatorName: 'Bob', creatorId: 2}]}, + {id: 3, subjectType: 'ContentElement', subjectId: 3, resolvedAt: null, + comments: [{id: 12, body: 'Third comment', creatorName: 'Bob', creatorId: 2}]} + ] + } + }); + + const next = entry.getNextCommentButton(); + + fireEvent.click(next); + expect(entry.getByText('First comment')).toBeInTheDocument(); + + fireEvent.click(next); + expect(entry.getByText('Third comment')).toBeInTheDocument(); + expect(entry.queryByText('Resolved comment')).not.toBeInTheDocument(); + }); + + function renderEntryWithTwoThreads() { + return renderEntry({ + seed: { + contentElements: [ + {typeName: 'withTestId', configuration: {testId: 5}}, + {typeName: 'withTestId', configuration: {testId: 6}} + ] + }, + commenting: { + currentUser: {id: 42, name: 'Alice'}, + commentThreads: [ + {id: 1, subjectType: 'ContentElement', subjectId: 1, resolvedAt: null, + comments: [{id: 10, body: 'First comment', creatorName: 'Bob', creatorId: 2}]}, + {id: 2, subjectType: 'ContentElement', subjectId: 2, resolvedAt: null, + comments: [{id: 11, body: 'Second comment', creatorName: 'Bob', creatorId: 2}]} + ] + } + }); + } +}); diff --git a/entry_types/scrolled/package/spec/support/pageObjects/commenting.js b/entry_types/scrolled/package/spec/support/pageObjects/commenting.js index 75a009cdc0..2768ad6000 100644 --- a/entry_types/scrolled/package/spec/support/pageObjects/commenting.js +++ b/entry_types/scrolled/package/spec/support/pageObjects/commenting.js @@ -23,6 +23,7 @@ export function renderEntry({ return { ...result, + getCommentToolbar: () => result.getByRole('group', {name: 'Comments'}), getAddCommentButton: () => result.getByRole('button', {name: 'Add comment'}), getCancelAddCommentButton: () => result.getByRole('button', {name: 'Cancel add comment'}), getNewThreadInput: () => result.getByPlaceholderText('Add a comment...'), @@ -30,7 +31,9 @@ export function renderEntry({ getAllCommentBadges: () => result.getAllByRole('status'), queryAllCommentBadges: () => result.queryAllByRole('status'), getCommentFilterButton: resolution => - result.getByRole('button', {name: resolution === 'all' ? 'All' : 'Unresolved'}) + result.getByRole('button', {name: resolution === 'all' ? 'All' : 'Unresolved'}), + getPreviousCommentButton: () => result.getByRole('button', {name: 'Previous comment'}), + getNextCommentButton: () => result.getByRole('button', {name: 'Next comment'}) }; } @@ -46,12 +49,16 @@ export function useCommentingPageObjects() { useFakeTranslations({ 'pageflow_scrolled.review.add_comment': 'Add comment', 'pageflow_scrolled.review.cancel_add_comment': 'Cancel add comment', + 'pageflow_scrolled.review.comment_toolbar': 'Comments', + 'pageflow_scrolled.review.comment_count': '%{count} comments', 'pageflow_scrolled.review.select_content_element': 'Select to comment', 'pageflow_scrolled.review.select_section': 'Select section to comment', 'pageflow_scrolled.review.add_comment_placeholder': 'Add a comment...', 'pageflow_scrolled.review.filter.label': 'Filter comments', 'pageflow_scrolled.review.filter.unresolved': 'Unresolved', - 'pageflow_scrolled.review.filter.all': 'All' + 'pageflow_scrolled.review.filter.all': 'All', + 'pageflow_scrolled.review.previous_comment': 'Previous comment', + 'pageflow_scrolled.review.next_comment': 'Next comment' }); usePageObjects(); diff --git a/entry_types/scrolled/package/src/frontend/commenting/EntryDecorator.js b/entry_types/scrolled/package/src/frontend/commenting/EntryDecorator.js index eb3c86b435..3341a4fdd8 100644 --- a/entry_types/scrolled/package/src/frontend/commenting/EntryDecorator.js +++ b/entry_types/scrolled/package/src/frontend/commenting/EntryDecorator.js @@ -12,14 +12,14 @@ export function EntryDecorator({commentingInitialState, children}) { 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 c618ced9e4..9e93efb578 100644 --- a/entry_types/scrolled/package/src/frontend/commenting/FloatingToolbar.js +++ b/entry_types/scrolled/package/src/frontend/commenting/FloatingToolbar.js @@ -1,17 +1,25 @@ import React from 'react'; +import classNames from 'classnames'; import {useI18n} from '../i18n'; import {useAddCommentMode} from './AddCommentModeProvider'; import {useCommentDisplayFilter} from './CommentDisplayFilterProvider'; +import {useCommentNavigation} from './SelectedSubjectProvider'; import AddCommentIcon from './images/addComment.svg'; import CancelCommentIcon from './images/cancelComment.svg'; +import ChevronIcon from './images/chevron.svg'; import styles from './FloatingToolbar.module.css'; export function FloatingToolbar() { + const {t} = useI18n({locale: 'ui'}); + return ( -
+
+
); @@ -40,6 +48,34 @@ function ResolutionToggleButton() { ); } +function ThreadNavigation() { + const {t} = useI18n({locale: 'ui'}); + const {count, goToPrevious, goToNext} = useCommentNavigation(); + + return ( + <> + + {count} + + + + + ); +} + function AddCommentButton() { const {t} = useI18n({locale: 'ui'}); const {active, toggle} = useAddCommentMode(); @@ -50,7 +86,7 @@ function AddCommentButton() { : 'pageflow_scrolled.review.add_comment'); return ( -
diff --git a/entry_types/scrolled/package/src/frontend/commenting/SelectedSubjectProvider.js b/entry_types/scrolled/package/src/frontend/commenting/SelectedSubjectProvider.js index 8efa081a7c..7a87719b7a 100644 --- a/entry_types/scrolled/package/src/frontend/commenting/SelectedSubjectProvider.js +++ b/entry_types/scrolled/package/src/frontend/commenting/SelectedSubjectProvider.js @@ -12,6 +12,7 @@ const SelectedSubjectContext = createContext({ const CommentNavigationContext = createContext({ count: 0, + position: 0, goToNext: () => {}, goToPrevious: () => {} }); @@ -64,6 +65,11 @@ export function SelectedSubjectProvider({children}) { }); }, [targets, selectedSubject, activateExcursionOfSection, returnFromExcursion]); + const position = useMemo( + () => currentTargetIndex(targets, selectedSubject) + 1, + [targets, selectedSubject] + ); + const selection = useMemo(() => ({ selectedSubject, setSelectedSubject, @@ -72,9 +78,10 @@ export function SelectedSubjectProvider({children}) { const navigation = useMemo(() => ({ count: targets.length, + position, goToNext: () => goTo(1), goToPrevious: () => goTo(-1) - }), [targets.length, goTo]); + }), [targets.length, position, goTo]); return ( diff --git a/entry_types/scrolled/package/src/review/Thread.module.css b/entry_types/scrolled/package/src/review/Thread.module.css index 94b4921b08..0413bce636 100644 --- a/entry_types/scrolled/package/src/review/Thread.module.css +++ b/entry_types/scrolled/package/src/review/Thread.module.css @@ -7,7 +7,7 @@ background: var(--ui-surface-color); border-radius: rounded(lg); box-shadow: var(--review-thread-box-shadow, var(--ui-box-shadow)); - border: var(--review-thread-border, none); + border: var(--review-thread-border, solid 1px var(--ui-surface-color)); scroll-margin-top: space(11); } From 3d07d6a3d951106fecbeeeb11e693972babbd29c Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Mon, 22 Jun 2026 16:19:57 +0200 Subject: [PATCH 07/10] Lay out and style the comment toolbar to match the design MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Arrange the floating comment toolbar as a single dark bar: the position indicator ("– /") first, then the resolution filter as a segmented Unresolved/All control showing each filter's comment count, then the up/down navigation arrows, then the add-comment button as a prominent accent action. The position indicator omits the total since the active segment's count already conveys it. Controls sit transparently on the shared dark (primary color) bar; the active filter segment reads as a lighter pill with a count chip. Segment counts come from the located comment threads; the count badges are aria-hidden so the segments keep their plain accessible names. --- .../features/commentDisplayFilter-spec.js | 30 ++++++- .../features/commentNavigation-spec.js | 10 +-- .../frontend/commenting/FloatingToolbar.js | 71 ++++++++++------ .../commenting/FloatingToolbar.module.css | 83 +++++++++++++++---- 4 files changed, 145 insertions(+), 49 deletions(-) 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 438ec71306..fa55f90eb5 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 @@ -1,11 +1,39 @@ import '@testing-library/jest-dom/extend-expect'; -import {fireEvent} from '@testing-library/react'; +import {fireEvent, within} from '@testing-library/react'; import {renderEntry, useCommentingPageObjects} from 'support/pageObjects/commenting'; describe('comment display filter', () => { useCommentingPageObjects(); + it('shows the unresolved and total comment counts in the segments', () => { + const entry = renderEntry({ + seed: { + contentElements: [ + {typeName: 'withTestId', configuration: {testId: 5}}, + {typeName: 'withTestId', configuration: {testId: 6}}, + {typeName: 'withTestId', configuration: {testId: 7}} + ] + }, + commenting: { + currentUser: {id: 42, name: 'Alice'}, + commentThreads: [ + {id: 1, subjectType: 'ContentElement', subjectId: 1, resolvedAt: null, + comments: [{id: 10, body: 'a', creatorName: 'Bob', creatorId: 2}]}, + {id: 2, subjectType: 'ContentElement', subjectId: 2, resolvedAt: null, + comments: [{id: 11, body: 'b', creatorName: 'Bob', creatorId: 2}]}, + {id: 3, subjectType: 'ContentElement', subjectId: 3, resolvedAt: '2026-01-01', + comments: [{id: 12, body: 'c', creatorName: 'Bob', creatorId: 2}]} + ] + } + }); + + expect(within(entry.getCommentFilterButton('unresolved')).getByText('2')) + .toBeInTheDocument(); + expect(within(entry.getCommentFilterButton('all')).getByText('3')) + .toBeInTheDocument(); + }); + it('shows unresolved comments with the unresolved segment active by default', () => { const entry = renderEntryWithResolvedThread(); diff --git a/entry_types/scrolled/package/spec/frontend/commenting/features/commentNavigation-spec.js b/entry_types/scrolled/package/spec/frontend/commenting/features/commentNavigation-spec.js index 6785b6398e..5fb69dbed2 100644 --- a/entry_types/scrolled/package/spec/frontend/commenting/features/commentNavigation-spec.js +++ b/entry_types/scrolled/package/spec/frontend/commenting/features/commentNavigation-spec.js @@ -10,10 +10,10 @@ describe('comment navigation', () => { window.HTMLElement.prototype.scrollIntoView = jest.fn(); }); - it('shows the number of navigable comments for the current filter', () => { + it('shows a dash in the position indicator before stepping', () => { const entry = renderEntryWithTwoThreads(); - expect(within(entry.getCommentToolbar()).getByTitle('2 comments')).toBeInTheDocument(); + expect(within(entry.getCommentToolbar()).getByText('– /')).toBeInTheDocument(); }); it('opens each thread popover when cycling forward', () => { @@ -79,17 +79,17 @@ describe('comment navigation', () => { expect(entry.queryByText('Resolved comment')).not.toBeInTheDocument(); }); - it('shows the current position alongside the total while stepping', () => { + it('shows the current position while stepping', () => { const entry = renderEntryWithTwoThreads(); const toolbar = entry.getCommentToolbar(); const next = entry.getNextCommentButton(); fireEvent.click(next); - expect(within(toolbar).getByText('1 / 2')).toBeInTheDocument(); + expect(within(toolbar).getByText('1 /')).toBeInTheDocument(); fireEvent.click(next); - expect(within(toolbar).getByText('2 / 2')).toBeInTheDocument(); + expect(within(toolbar).getByText('2 /')).toBeInTheDocument(); }); it('highlights the current thread when stepping within one element', () => { diff --git a/entry_types/scrolled/package/src/frontend/commenting/FloatingToolbar.js b/entry_types/scrolled/package/src/frontend/commenting/FloatingToolbar.js index d4e4ca036d..2886249e77 100644 --- a/entry_types/scrolled/package/src/frontend/commenting/FloatingToolbar.js +++ b/entry_types/scrolled/package/src/frontend/commenting/FloatingToolbar.js @@ -1,6 +1,7 @@ import React from 'react'; import classNames from 'classnames'; +import {useLocatedCommentThreads} from 'pageflow-scrolled/review'; import {useI18n} from '../i18n'; import {useAddCommentMode} from './AddCommentModeProvider'; import {useCommentDisplayFilter} from './CommentDisplayFilterProvider'; @@ -19,46 +20,32 @@ export function FloatingToolbar() { role="group" aria-label={t('pageflow_scrolled.review.comment_toolbar')} data-comment-toolbar> + - + ); } -const resolutions = ['unresolved', 'all']; - -function ResolutionToggleButton() { +function PositionIndicator() { const {t} = useI18n({locale: 'ui'}); - const {resolution, setResolution} = useCommentDisplayFilter(); + const {count, position} = useCommentNavigation(); return ( -
- {resolutions.map(value => ( - - ))} -
+ + {position || '–'} / + ); } -function ThreadNavigation() { +function NavigationArrows() { const {t} = useI18n({locale: 'ui'}); - const {count, position, goToPrevious, goToNext} = useCommentNavigation(); + const {count, goToPrevious, goToNext} = useCommentNavigation(); return ( - <> - - {position ? `${position} / ${count}` : count} - +
- +
+ ); +} + +const resolutions = ['unresolved', 'all']; + +function ResolutionToggleButton() { + const {t} = useI18n({locale: 'ui'}); + const {resolution, setResolution} = useCommentDisplayFilter(); + const {threads} = useLocatedCommentThreads(); + + const counts = { + unresolved: threads.filter(thread => !thread.resolvedAt).length, + all: threads.length + }; + + return ( +
+ {resolutions.map(value => ( + + ))} +
); } diff --git a/entry_types/scrolled/package/src/frontend/commenting/FloatingToolbar.module.css b/entry_types/scrolled/package/src/frontend/commenting/FloatingToolbar.module.css index 57acfa6668..adfcccc6c0 100644 --- a/entry_types/scrolled/package/src/frontend/commenting/FloatingToolbar.module.css +++ b/entry_types/scrolled/package/src/frontend/commenting/FloatingToolbar.module.css @@ -1,38 +1,53 @@ .toolbar { position: fixed; - bottom: 0; - right: 0; + bottom: space(3); + right: space(3); z-index: 1100; - padding: space(2); display: flex; align-items: center; gap: space(2); + padding: space(1); + background: var(--ui-primary-color); + border: 1px solid var(--ui-on-primary-color-lightest); + border-radius: rounded(lg); + box-shadow: var(--ui-box-shadow-md); +} + +.navigation { + display: flex; + align-items: center; + gap: space(1); } .button { font-family: var(--ui-font-family); color: var(--ui-on-primary-color); - background: var(--ui-primary-color); + background: transparent; border: none; - border-radius: rounded(md); - padding: space(2); + border-radius: rounded(sm); + padding: space(1); cursor: pointer; - box-shadow: var(--ui-box-shadow-md); display: flex; align-items: center; } .button:disabled { - opacity: 0.5; + opacity: 0.3; cursor: default; } +.count, +.segmented { + font-size: size(3.5); +} + .count { font-family: var(--ui-font-family); font-variant-numeric: tabular-nums; - color: var(--ui-on-surface-color); - min-width: 1.5em; - text-align: center; + color: var(--ui-on-primary-color-light); + padding: 0 space(1); + margin: 0 space(-1) 0 space(2); + white-space: nowrap; } .chevronUp { @@ -45,21 +60,55 @@ .segmented { display: flex; + align-items: center; + gap: space(1); + border: solid 1px var(--ui-on-primary-color-lightest); border-radius: rounded(md); - overflow: hidden; - box-shadow: var(--ui-box-shadow-md); } .segment { font-family: var(--ui-font-family); - color: var(--ui-on-surface-color); - background: var(--ui-surface-color); + display: flex; + align-items: center; + gap: space(1); + color: var(--ui-on-primary-color-light); + background: transparent; border: none; - padding: space(2) space(3); + padding: space(2) space(2); cursor: pointer; + white-space: nowrap; +} + +.segment:first-child { + border-top-left-radius: rounded(md); + border-bottom-left-radius: rounded(md); +} + +.segment:last-child { + border-top-right-radius: rounded(md); + border-bottom-right-radius: rounded(md); } .segment[aria-pressed='true'] { color: var(--ui-on-primary-color); - background: var(--ui-primary-color); + background: var(--ui-on-primary-color-lightest); +} + +.segmentCount { + font-variant-numeric: tabular-nums; + border-radius: rounded(sm); + padding: 0 space(1); + background: var(--ui-on-primary-color-lightest); +} + +.segment[aria-pressed='true'] .segmentCount { + background: var(--ui-on-primary-color-lighter); +} + +.addButton { + color: var(--ui-on-accent-color); + background: var(--ui-accent-color); + border-radius: rounded(md); + padding: space(2); + margin-left: space(2); } From 7883afb6835ecfd8a894f603abd139b1d0f961b6 Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Mon, 22 Jun 2026 17:50:13 +0200 Subject: [PATCH 08/10] Expand resolved threads when showing all comments Add an expandResolved prop to ThreadList. When set, the resolved section is expanded by default instead of collapsed behind the count pill, and when every thread of the subject is resolved the resolved comments are shown rather than auto-opening the new thread form. The manual toggle and explicit showNewForm still take precedence. The commenting popover sets expandResolved when the resolution filter is 'all', so switching the toolbar filter to all reveals resolved comments in place. Editor usages omit the prop and keep their previous behavior. --- .../features/commentDisplayFilter-spec.js | 23 +++++++++ .../package/spec/review/ThreadList-spec.js | 51 +++++++++++++++++++ .../src/frontend/commenting/Popover.js | 4 +- .../scrolled/package/src/review/ThreadList.js | 14 ++--- 4 files changed, 85 insertions(+), 7 deletions(-) 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 fa55f90eb5..b91f4fb491 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 @@ -6,6 +6,29 @@ import {renderEntry, useCommentingPageObjects} from 'support/pageObjects/comment describe('comment display filter', () => { useCommentingPageObjects(); + beforeEach(() => { + window.HTMLElement.prototype.scrollIntoView = jest.fn(); + }); + + it('expands resolved comments in the popover when showing all', () => { + const entry = renderEntry({ + seed: {contentElements: [{typeName: 'withTestId', configuration: {testId: 5}}]}, + commenting: { + currentUser: {id: 42, name: 'Alice'}, + commentThreads: [ + {id: 1, subjectType: 'ContentElement', subjectId: 1, resolvedAt: '2026-01-01', + comments: [{id: 10, body: 'Resolved comment', creatorName: 'Bob', creatorId: 2}]} + ] + } + }); + + fireEvent.click(entry.getCommentFilterButton('all')); + fireEvent.click(entry.getAllCommentBadges()[0]); + + expect(entry.getByText('Resolved comment')).toBeInTheDocument(); + expect(entry.queryByPlaceholderText('Add a comment...')).not.toBeInTheDocument(); + }); + it('shows the unresolved and total comment counts in the segments', () => { const entry = renderEntry({ seed: { diff --git a/entry_types/scrolled/package/spec/review/ThreadList-spec.js b/entry_types/scrolled/package/spec/review/ThreadList-spec.js index 3424e1b292..9ba5b123fe 100644 --- a/entry_types/scrolled/package/spec/review/ThreadList-spec.js +++ b/entry_types/scrolled/package/spec/review/ThreadList-spec.js @@ -967,4 +967,55 @@ describe('ThreadList', () => { expect(getAllByPlaceholderText('Reply...')).toHaveLength(2); }); + + it('expands resolved threads by default when expandResolved is set', () => { + const {getByText} = renderWithReviewState( + , + { + commentThreads: [ + {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}]} + ] + } + ); + + expect(getByText('Active thread')).toBeInTheDocument(); + expect(getByText('Resolved thread')).toBeInTheDocument(); + }); + + it('shows resolved threads instead of the new form when all are resolved and expandResolved is set', () => { + const {getByText, queryByPlaceholderText} = renderWithReviewState( + , + { + commentThreads: [ + {id: 1, subjectType: 'ContentElement', subjectId: 10, + resolvedAt: '2026-04-09T10:00:00Z', + comments: [{id: 10, body: 'Resolved thread', creatorName: 'Bob', creatorId: 2}]} + ] + } + ); + + expect(getByText('Resolved thread')).toBeInTheDocument(); + expect(queryByPlaceholderText('Add a comment...')).not.toBeInTheDocument(); + }); + + it('still auto-shows the new form for only-resolved threads without expandResolved', () => { + const {getByPlaceholderText, queryByText} = renderWithReviewState( + , + { + commentThreads: [ + {id: 1, subjectType: 'ContentElement', subjectId: 10, + resolvedAt: '2026-04-09T10:00:00Z', + comments: [{id: 10, body: 'Resolved thread', creatorName: 'Bob', creatorId: 2}]} + ] + } + ); + + expect(getByPlaceholderText('Add a comment...')).toBeInTheDocument(); + expect(queryByText('Resolved thread')).not.toBeInTheDocument(); + }); }); diff --git a/entry_types/scrolled/package/src/frontend/commenting/Popover.js b/entry_types/scrolled/package/src/frontend/commenting/Popover.js index bd742e566b..fec3bae330 100644 --- a/entry_types/scrolled/package/src/frontend/commenting/Popover.js +++ b/entry_types/scrolled/package/src/frontend/commenting/Popover.js @@ -59,6 +59,7 @@ export function Popover({ showNewForm={showNewForm} hideNewTopicButton={hideNewTopicButton} highlightedThreadId={highlightedThreadId} + expandResolved={resolution === 'all'} onDismiss={clearSelection} />} ); @@ -66,7 +67,7 @@ export function Popover({ function OpenThreadList({ reference, subjectType, subjectId, subjectRange, - placement, strategy, showNewForm, hideNewTopicButton, highlightedThreadId, onDismiss + placement, strategy, showNewForm, hideNewTopicButton, highlightedThreadId, expandResolved, onDismiss }) { const portalRoot = useFloatingPortalRoot(); @@ -121,6 +122,7 @@ function OpenThreadList({ subjectId={subjectId} subjectRange={subjectRange} highlightedThreadId={highlightedThreadId} + expandResolved={expandResolved} showNewForm={showNewForm} hideNewTopicButton={hideNewTopicButton} /> diff --git a/entry_types/scrolled/package/src/review/ThreadList.js b/entry_types/scrolled/package/src/review/ThreadList.js index 99c4f6e137..cc48e282cf 100644 --- a/entry_types/scrolled/package/src/review/ThreadList.js +++ b/entry_types/scrolled/package/src/review/ThreadList.js @@ -11,7 +11,7 @@ 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, compareRanges, highlightedThreadId, onThreadClick, restrictInteractionsToHighlighted, showNewForm: showNewFormProp, hideNewTopicButton, reversed}) { +export function ThreadList({subjectType, subjectId, subjectRange, filter, compareRanges, highlightedThreadId, onThreadClick, restrictInteractionsToHighlighted, showNewForm: showNewFormProp, hideNewTopicButton, reversed, expandResolved}) { const {t} = useI18n({locale: 'ui'}); const allActiveThreads = useCommentThreads({subjectType, subjectId, subjectRange, resolution: 'unresolved'}); const allResolvedThreads = useCommentThreads({subjectType, subjectId, subjectRange, resolution: 'resolved'}); @@ -31,17 +31,19 @@ export function ThreadList({subjectType, subjectId, subjectRange, filter, compar const [expandedThreadId, setExpandedThreadId] = useState(null); const [formToggled, setFormToggled] = useState(null); - const [showResolved, setShowResolved] = useState(false); + const [resolvedToggled, setResolvedToggled] = useState(null); + + const showResolved = resolvedToggled !== null ? resolvedToggled : !!expandResolved; + + const noThreads = activeThreads.length === 0 && resolvedThreads.length === 0; const showNewForm = formToggled !== null ? formToggled : showNewFormProp !== undefined ? showNewFormProp : - activeThreads.length === 0; + expandResolved ? noThreads : activeThreads.length === 0; function toggleThread(threadId) { setExpandedThreadId(expandedThreadId === threadId ? null : threadId); } - const noThreads = activeThreads.length === 0 && resolvedThreads.length === 0; - return (
{!showNewForm && !hideNewTopicButton && @@ -78,7 +80,7 @@ export function ThreadList({subjectType, subjectId, subjectRange, filter, compar {resolvedThreads.length > 0 &&
); } - -function sortByRange(threads, compareRanges) { - if (!compareRanges) return threads; - return [...threads].sort((a, b) => compareRanges(a.subjectRange, b.subjectRange)); -} diff --git a/entry_types/scrolled/package/src/review/api/ContentElementTypeRegistry.js b/entry_types/scrolled/package/src/review/api/ContentElementTypeRegistry.js new file mode 100644 index 0000000000..b589e9fb39 --- /dev/null +++ b/entry_types/scrolled/package/src/review/api/ContentElementTypeRegistry.js @@ -0,0 +1,18 @@ +// Holds per content element type behavior needed to display comment +// threads. Available both in the editor and in the frontend commenting +// mode. Kept separate from the editor and frontend registries so comment +// ordering logic (comparing Slate ranges) can be shared by both without +// pulling Slate into the always-on frontend bundle. +export class ContentElementTypeRegistry { + constructor() { + this.types = {}; + } + + register(typeName, options) { + this.types[typeName] = options; + } + + findCompareRanges(typeName) { + return this.types[typeName]?.compareRanges; + } +} diff --git a/entry_types/scrolled/package/src/review/api/index.js b/entry_types/scrolled/package/src/review/api/index.js new file mode 100644 index 0000000000..8ec21f2c2c --- /dev/null +++ b/entry_types/scrolled/package/src/review/api/index.js @@ -0,0 +1,5 @@ +import {ContentElementTypeRegistry} from './ContentElementTypeRegistry'; + +export const review = { + contentElementTypes: new ContentElementTypeRegistry() +}; diff --git a/entry_types/scrolled/package/src/review/index.js b/entry_types/scrolled/package/src/review/index.js index bebe5d7790..abe5b7ca22 100644 --- a/entry_types/scrolled/package/src/review/index.js +++ b/entry_types/scrolled/package/src/review/index.js @@ -1,3 +1,4 @@ +export {review} from './api'; export {ReviewStateProvider, useCommentThreads, useCommentThread} from './ReviewStateProvider'; export {useLocatedCommentThreads} from './useLocatedCommentThreads'; export {ReviewMessageHandler} from './ReviewMessageHandler'; diff --git a/entry_types/scrolled/package/src/review/sortByRange.js b/entry_types/scrolled/package/src/review/sortByRange.js new file mode 100644 index 0000000000..aee36decf0 --- /dev/null +++ b/entry_types/scrolled/package/src/review/sortByRange.js @@ -0,0 +1,4 @@ +export function sortByRange(threads, compareRanges) { + if (!compareRanges) return threads; + return [...threads].sort((a, b) => compareRanges(a.subjectRange, b.subjectRange)); +} diff --git a/entry_types/scrolled/package/src/review/useLocatedCommentThreads.js b/entry_types/scrolled/package/src/review/useLocatedCommentThreads.js index a72db984de..d632efdeee 100644 --- a/entry_types/scrolled/package/src/review/useLocatedCommentThreads.js +++ b/entry_types/scrolled/package/src/review/useLocatedCommentThreads.js @@ -2,7 +2,9 @@ import {useMemo} from 'react'; import {useEntryStructureWithContentElements} from 'pageflow-scrolled/entryState'; +import {review} from './api'; import {useCommentThreads} from './ReviewStateProvider'; +import {sortByRange} from './sortByRange'; /** * Joins the comment threads from the review state with the entry @@ -25,8 +27,11 @@ export function useLocatedCommentThreads() { const locatedThreads = new Set(); const threads = []; - const take = (subjectType, subjectId) => { - const subjectThreads = threadsBySubject[subjectKey(subjectType, subjectId)] || []; + const take = (subjectType, subjectId, compareRanges) => { + const subjectThreads = sortByRange( + threadsBySubject[subjectKey(subjectType, subjectId)] || [], + compareRanges + ); subjectThreads.forEach(thread => locatedThreads.add(thread)); threads.push(...subjectThreads); return subjectThreads; @@ -38,7 +43,11 @@ export function useLocatedCommentThreads() { threads: take('Section', section.permaId), contentElements: section.contentElements.map(contentElement => ({ ...contentElement, - threads: take('ContentElement', contentElement.permaId) + threads: take( + 'ContentElement', + contentElement.permaId, + review.contentElementTypes.findCompareRanges(contentElement.type) + ) })) })); From b55b8a14649265342c148b32a44e6db4ccc59c6b Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Wed, 24 Jun 2026 16:06:39 +0200 Subject: [PATCH 10/10] Show resolved appearance for badges whose threads are all resolved --- .../package/spec/review/ThreadsBadge-spec.js | 29 +++++++++++++++++++ .../package/src/review/ThreadsBadge.js | 5 +++- 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/entry_types/scrolled/package/spec/review/ThreadsBadge-spec.js b/entry_types/scrolled/package/spec/review/ThreadsBadge-spec.js index f088df649b..c87050b838 100644 --- a/entry_types/scrolled/package/spec/review/ThreadsBadge-spec.js +++ b/entry_types/scrolled/package/spec/review/ThreadsBadge-spec.js @@ -3,6 +3,7 @@ import '@testing-library/jest-dom/extend-expect'; import {ThreadsBadge} from 'review/ThreadsBadge'; import {renderWithReviewState} from 'support/renderWithReviewState'; +import badgeStyles from 'review/Badge.module.css'; describe('ThreadsBadge', () => { it('does not display count for single thread', () => { @@ -61,6 +62,34 @@ describe('ThreadsBadge', () => { expect(container).toBeEmptyDOMElement(); }); + it('renders the badge as resolved when all shown threads are resolved', () => { + const {getByRole} = renderWithReviewState( + , + { + commentThreads: [ + {id: 1, subjectType: 'ContentElement', subjectId: 10, resolvedAt: '2026-04-09T10:00:00Z', comments: []}, + {id: 2, subjectType: 'ContentElement', subjectId: 10, resolvedAt: '2026-04-09T10:00:00Z', comments: []} + ] + } + ); + + expect(getByRole('status')).toHaveClass(badgeStyles.resolved); + }); + + it('does not render the badge as resolved when an unresolved thread remains', () => { + const {getByRole} = renderWithReviewState( + , + { + commentThreads: [ + {id: 1, subjectType: 'ContentElement', subjectId: 10, resolvedAt: '2026-04-09T10:00:00Z', comments: []}, + {id: 2, subjectType: 'ContentElement', subjectId: 10, resolvedAt: null, comments: []} + ] + } + ); + + expect(getByRole('status')).not.toHaveClass(badgeStyles.resolved); + }); + it('renders nothing when no threads exist for subject', () => { const {container} = renderWithReviewState( diff --git a/entry_types/scrolled/package/src/review/ThreadsBadge.js b/entry_types/scrolled/package/src/review/ThreadsBadge.js index 0c73dbe5c0..754e958e6e 100644 --- a/entry_types/scrolled/package/src/review/ThreadsBadge.js +++ b/entry_types/scrolled/package/src/review/ThreadsBadge.js @@ -5,10 +5,13 @@ import {Badge} from './Badge'; export function ThreadsBadge({subjectType, subjectId, subjectRange, onClick, mode, resolution = 'unresolved'}) { const threads = useCommentThreads({subjectType, subjectId, subjectRange, resolution}); + const unresolvedThreads = useCommentThreads({subjectType, subjectId, subjectRange, resolution: 'unresolved'}); const handleClick = useCallback(() => { if (onClick) onClick(threads); }, [onClick, threads]); - return ; + const resolved = threads.length > 0 && unresolvedThreads.length === 0; + + return ; }