From 4dd71753d4f947e1236d78e6b7a8a898eba0863d Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Thu, 7 May 2026 18:12:06 +0200 Subject: [PATCH 01/13] Restrict reply form and resolve button to highlighted thread in ThreadList --- .../views/ContentElementCommentsView-spec.js | 23 +++++----- .../editor/views/EntryCommentsView-spec.js | 24 ++++++++++ .../package/spec/review/ThreadList-spec.js | 45 +++++++++++++++++++ .../src/editor/views/EntryCommentsView.js | 1 + .../scrolled/package/src/review/Thread.js | 6 +-- .../scrolled/package/src/review/ThreadList.js | 10 +++-- 6 files changed, 92 insertions(+), 17 deletions(-) diff --git a/entry_types/scrolled/package/spec/editor/views/ContentElementCommentsView-spec.js b/entry_types/scrolled/package/spec/editor/views/ContentElementCommentsView-spec.js index ec9efccdcb..ae000ed4d0 100644 --- a/entry_types/scrolled/package/spec/editor/views/ContentElementCommentsView-spec.js +++ b/entry_types/scrolled/package/spec/editor/views/ContentElementCommentsView-spec.js @@ -1,6 +1,7 @@ import '@testing-library/jest-dom/extend-expect'; import userEvent from '@testing-library/user-event'; +import {editor} from 'pageflow-scrolled/editor'; import {ContentElementCommentsView} from 'editor/views/ContentElementCommentsView'; import {factories, useFakeTranslations, renderBackboneView} from 'pageflow/testHelpers'; @@ -30,7 +31,7 @@ describe('ContentElementCommentsView', () => { }] }); - const view = new ContentElementCommentsView({entry, editor: {}}); + const view = new ContentElementCommentsView({entry, editor}); const {getByText} = renderBackboneView(view); @@ -53,7 +54,7 @@ describe('ContentElementCommentsView', () => { ] }); - const view = new ContentElementCommentsView({entry, editor: {}}); + const view = new ContentElementCommentsView({entry, editor}); const {getByText, queryByText} = renderBackboneView(view); @@ -75,7 +76,7 @@ describe('ContentElementCommentsView', () => { ] }); - const view = new ContentElementCommentsView({entry, editor: {}}); + const view = new ContentElementCommentsView({entry, editor}); const {getByText} = renderBackboneView(view); @@ -97,7 +98,7 @@ describe('ContentElementCommentsView', () => { ] }); - const view = new ContentElementCommentsView({entry, editor: {}}); + const view = new ContentElementCommentsView({entry, editor}); const {getByText, queryByText} = renderBackboneView(view); expect(getByText('Out of scope')).toBeInTheDocument(); @@ -130,7 +131,7 @@ describe('ContentElementCommentsView', () => { ] }); - const view = new ContentElementCommentsView({entry, editor: {}}); + const view = new ContentElementCommentsView({entry, editor}); const {getByText, queryByText} = renderBackboneView(view); expect(getByText('On first')).toBeInTheDocument(); @@ -162,7 +163,7 @@ describe('ContentElementCommentsView', () => { ] }); - const view = new ContentElementCommentsView({entry, editor: {}}); + const view = new ContentElementCommentsView({entry, editor}); const {getByText} = renderBackboneView(view); expect(getByText('second').closest('[aria-current="true"]')).not.toBeNull(); @@ -185,7 +186,7 @@ describe('ContentElementCommentsView', () => { ] }); - const view = new ContentElementCommentsView({entry, editor: {}}); + const view = new ContentElementCommentsView({entry, editor}); const {getByText} = renderBackboneView(view); expect(getByText('first').closest('[aria-current="true"]')).toBeNull(); @@ -214,7 +215,7 @@ describe('ContentElementCommentsView', () => { const listener = jest.fn(); entry.on('selectCommentThread', listener); - const view = new ContentElementCommentsView({entry, editor: {}}); + const view = new ContentElementCommentsView({entry, editor}); const {getByText} = renderBackboneView(view); await user.click(getByText('click me')); @@ -240,7 +241,7 @@ describe('ContentElementCommentsView', () => { const listener = jest.fn(); entry.on('selectCommentThread', listener); - const view = new ContentElementCommentsView({entry, editor: {}}); + const view = new ContentElementCommentsView({entry, editor}); const {getByText} = renderBackboneView(view); expect(getByText('click me').closest('[aria-current="true"]')).toBeNull(); @@ -264,7 +265,7 @@ describe('ContentElementCommentsView', () => { }] }); - const view = new ContentElementCommentsView({entry, editor: {}}); + const view = new ContentElementCommentsView({entry, editor}); const {queryByRole} = renderBackboneView(view); expect(queryByRole('button', {name: 'New topic'})).not.toBeInTheDocument(); @@ -277,7 +278,7 @@ describe('ContentElementCommentsView', () => { entry.set('selectedContentElementCommentsId', 1); entry.reviewSession = factories.reviewSession(); - const view = new ContentElementCommentsView({entry, editor: {}}); + const view = new ContentElementCommentsView({entry, editor}); const {getByText} = renderBackboneView(view); diff --git a/entry_types/scrolled/package/spec/editor/views/EntryCommentsView-spec.js b/entry_types/scrolled/package/spec/editor/views/EntryCommentsView-spec.js index bfe480967d..fee23e0de3 100644 --- a/entry_types/scrolled/package/spec/editor/views/EntryCommentsView-spec.js +++ b/entry_types/scrolled/package/spec/editor/views/EntryCommentsView-spec.js @@ -16,6 +16,7 @@ describe('EntryCommentsView', () => { useFakeTranslations({ 'pageflow_scrolled.review.new_topic': 'New topic', 'pageflow_scrolled.review.add_comment_placeholder': 'Add a comment...', + 'pageflow_scrolled.review.reply_placeholder': 'Reply...', 'pageflow_scrolled.review.send': 'Send', 'pageflow_scrolled.editor.content_elements.textBlock.name': 'Text', 'pageflow_scrolled.editor.content_elements.image.name': 'Image' @@ -172,6 +173,29 @@ describe('EntryCommentsView', () => { expect(getByText('first').closest('[aria-current="true"]')).not.toBeNull(); }); + it('only shows reply form on the highlighted thread', () => { + const entry = createEntry({ + contentElements: [{id: 1, permaId: 10, typeName: 'textBlock'}] + }); + entry.reviewSession = factories.reviewSession({ + commentThreads: [ + {id: 1, subjectType: 'ContentElement', subjectId: 10, + comments: [{id: 10, body: 'first', creatorName: 'Alice'}]}, + {id: 2, subjectType: 'ContentElement', subjectId: 10, + comments: [{id: 20, body: 'second', creatorName: 'Bob'}]} + ] + }); + entry.set('highlightedThreadId', 2); + + const view = new EntryCommentsView({entry, editor}); + const {getByText, queryAllByPlaceholderText} = renderBackboneView(view); + + const replyInputs = queryAllByPlaceholderText('Reply...'); + expect(replyInputs).toHaveLength(1); + expect(getByText('second').closest('[aria-current="true"]')) + .toContainElement(replyInputs[0]); + }); + it('shows the content element type name as group label', () => { const entry = createEntry({ contentElements: [{id: 1, permaId: 10, typeName: 'image'}] diff --git a/entry_types/scrolled/package/spec/review/ThreadList-spec.js b/entry_types/scrolled/package/spec/review/ThreadList-spec.js index 8afd2375dc..7d683e1bf8 100644 --- a/entry_types/scrolled/package/spec/review/ThreadList-spec.js +++ b/entry_types/scrolled/package/spec/review/ThreadList-spec.js @@ -152,6 +152,51 @@ describe('ThreadList', () => { expect(onThreadClick).toHaveBeenCalledWith(expect.objectContaining({id: 7})); }); + it('hides reply form on non-highlighted threads when restrictInteractionsToHighlighted', () => { + const {getByText, queryAllByPlaceholderText} = renderWithReviewState( + , + { + commentThreads: [ + {id: 1, subjectType: 'ContentElement', subjectId: 10, comments: [ + {id: 10, body: 'first', creatorName: 'Alice', creatorId: 1} + ]}, + {id: 2, subjectType: 'ContentElement', subjectId: 10, comments: [ + {id: 20, body: 'second', creatorName: 'Bob', creatorId: 2} + ]} + ] + } + ); + + const replyInputs = queryAllByPlaceholderText('Reply...'); + expect(replyInputs).toHaveLength(1); + expect(getByText('second').closest('[aria-current="true"]')) + .toContainElement(replyInputs[0]); + }); + + it('hides resolve button on non-highlighted threads when restrictInteractionsToHighlighted', () => { + const {queryAllByText} = renderWithReviewState( + , + { + commentThreads: [ + {id: 1, subjectType: 'ContentElement', subjectId: 10, comments: [ + {id: 10, body: 'first', creatorName: 'Alice', creatorId: 1} + ]}, + {id: 2, subjectType: 'ContentElement', subjectId: 10, comments: [ + {id: 20, body: 'second', creatorName: 'Bob', creatorId: 2} + ]} + ] + } + ); + + expect(queryAllByText('Mark as resolved')).toHaveLength(1); + }); + it('applies filter prop to resolved threads', async () => { const user = userEvent.setup(); const {getByText, queryByText} = renderWithReviewState( diff --git a/entry_types/scrolled/package/src/editor/views/EntryCommentsView.js b/entry_types/scrolled/package/src/editor/views/EntryCommentsView.js index 85e3ba2bf4..8b21986d0e 100644 --- a/entry_types/scrolled/package/src/editor/views/EntryCommentsView.js +++ b/entry_types/scrolled/package/src/editor/views/EntryCommentsView.js @@ -75,6 +75,7 @@ function ContentElementGroup({contentElement, highlightedThreadId, onThreadClick subjectId={permaId} highlightedThreadId={highlightedThreadId} onThreadClick={onThreadClick} + restrictInteractionsToHighlighted hideNewTopicButton /> ); diff --git a/entry_types/scrolled/package/src/review/Thread.js b/entry_types/scrolled/package/src/review/Thread.js index 708c601c36..4763143286 100644 --- a/entry_types/scrolled/package/src/review/Thread.js +++ b/entry_types/scrolled/package/src/review/Thread.js @@ -11,7 +11,7 @@ import ResolveIcon from './images/resolve.svg'; import UnresolveIcon from './images/unresolve.svg'; import styles from './Thread.module.css'; -export function Thread({thread, collapsed, onToggle, onResolve, onClick, highlighted}) { +export function Thread({thread, collapsed, onToggle, onResolve, onClick, highlighted, interactive = true}) { const {t} = useI18n({locale: 'ui'}); const firstComment = thread.comments[0]; const replies = thread.comments.slice(1); @@ -43,10 +43,10 @@ export function Thread({thread, collapsed, onToggle, onResolve, onClick, highlig ))} - {!thread.resolvedAt && !repliesCollapsed && + {interactive && !thread.resolvedAt && !repliesCollapsed && } - {onResolve && !repliesCollapsed && + {interactive && onResolve && !repliesCollapsed &&
} From 45cc5da52f60be9d8e350bcbcb44cb8e78f196aa Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Thu, 7 May 2026 18:16:06 +0200 Subject: [PATCH 02/13] Order comment threads in editor views by subject range --- .../contentElements/textBlock/editor-spec.js | 45 ++++++++++ .../views/ContentElementCommentsView-spec.js | 82 ++++++++++++++++--- .../editor/views/EntryCommentsView-spec.js | 33 ++++++++ .../package/spec/review/ThreadList-spec.js | 28 +++++++ .../src/contentElements/textBlock/editor.js | 11 +++ .../editor/api/ContentElementTypeRegistry.js | 4 + .../views/ContentElementCommentsView.js | 12 ++- .../src/editor/views/EntryCommentsView.js | 2 + .../scrolled/package/src/review/ThreadList.js | 15 ++-- 9 files changed, 211 insertions(+), 21 deletions(-) diff --git a/entry_types/scrolled/package/spec/contentElements/textBlock/editor-spec.js b/entry_types/scrolled/package/spec/contentElements/textBlock/editor-spec.js index ce12d2330a..e746be46aa 100644 --- a/entry_types/scrolled/package/spec/contentElements/textBlock/editor-spec.js +++ b/entry_types/scrolled/package/spec/contentElements/textBlock/editor-spec.js @@ -128,4 +128,49 @@ describe('contentElements/textBlock/editor', () => { }); }); }); + + describe('#compareRanges', () => { + function range(anchor, focus) { + return { + anchor: {path: [anchor[0], anchor[1]], offset: anchor[2]}, + focus: {path: [focus[0], focus[1]], offset: focus[2]} + }; + } + + it('orders ranges by start block', () => { + const earlier = range([0, 0, 0], [0, 0, 5]); + const later = range([1, 0, 0], [1, 0, 5]); + + expect(type.compareRanges(earlier, later)).toBeLessThan(0); + expect(type.compareRanges(later, earlier)).toBeGreaterThan(0); + }); + + it('orders ranges within the same block by offset', () => { + const earlier = range([0, 0, 2], [0, 0, 4]); + const later = range([0, 0, 6], [0, 0, 8]); + + expect(type.compareRanges(earlier, later)).toBeLessThan(0); + }); + + it('uses the smaller of anchor and focus as the start point', () => { + const reversed = range([0, 0, 8], [0, 0, 2]); + const forward = range([0, 0, 5], [0, 0, 6]); + + expect(type.compareRanges(reversed, forward)).toBeLessThan(0); + }); + + it('returns 0 for equal ranges', () => { + const r = range([0, 0, 0], [0, 0, 5]); + + expect(type.compareRanges(r, r)).toBe(0); + }); + + it('sorts range-less subjects last', () => { + const r = range([0, 0, 0], [0, 0, 5]); + + expect(type.compareRanges(undefined, r)).toBeGreaterThan(0); + expect(type.compareRanges(r, undefined)).toBeLessThan(0); + expect(type.compareRanges(undefined, undefined)).toBe(0); + }); + }); }); diff --git a/entry_types/scrolled/package/spec/editor/views/ContentElementCommentsView-spec.js b/entry_types/scrolled/package/spec/editor/views/ContentElementCommentsView-spec.js index ae000ed4d0..b7c9f8da7c 100644 --- a/entry_types/scrolled/package/spec/editor/views/ContentElementCommentsView-spec.js +++ b/entry_types/scrolled/package/spec/editor/views/ContentElementCommentsView-spec.js @@ -11,6 +11,12 @@ import {act, waitFor} from '@testing-library/react'; describe('ContentElementCommentsView', () => { const {createEntry} = useEditorGlobals(); + beforeAll(() => { + editor.contentElementTypes.register('fixture', { + compareRanges: (a, b) => (a?.start ?? 0) - (b?.start ?? 0) + }); + }); + useFakeTranslations({ 'pageflow_scrolled.review.add_comment_placeholder': 'Add a comment...', 'pageflow_scrolled.review.new_topic': 'New topic', @@ -19,7 +25,7 @@ describe('ContentElementCommentsView', () => { it('displays threads of selected content element from session state', () => { const entry = createEntry({ - contentElements: [{id: 1, permaId: 10, typeName: 'textBlock'}] + contentElements: [{id: 1, permaId: 10, typeName: 'fixture'}] }); entry.set('selectedContentElementCommentsId', 1); entry.reviewSession = factories.reviewSession({ @@ -40,7 +46,7 @@ describe('ContentElementCommentsView', () => { it('filters threads by transient state commentThreadIdsAtSelection', () => { const entry = createEntry({ - contentElements: [{id: 1, permaId: 10, typeName: 'textBlock'}] + contentElements: [{id: 1, permaId: 10, typeName: 'fixture'}] }); entry.set('selectedContentElementCommentsId', 1); entry.contentElements.get(1).transientState @@ -64,7 +70,7 @@ describe('ContentElementCommentsView', () => { it('shows all threads when transient state has no commentThreadIdsAtSelection', () => { const entry = createEntry({ - contentElements: [{id: 1, permaId: 10, typeName: 'textBlock'}] + contentElements: [{id: 1, permaId: 10, typeName: 'fixture'}] }); entry.set('selectedContentElementCommentsId', 1); entry.reviewSession = factories.reviewSession({ @@ -86,7 +92,7 @@ describe('ContentElementCommentsView', () => { it('updates filter when transient state changes', async () => { const entry = createEntry({ - contentElements: [{id: 1, permaId: 10, typeName: 'textBlock'}] + contentElements: [{id: 1, permaId: 10, typeName: 'fixture'}] }); entry.set('selectedContentElementCommentsId', 1); entry.reviewSession = factories.reviewSession({ @@ -117,8 +123,8 @@ describe('ContentElementCommentsView', () => { it('updates when selectedContentElementCommentsId changes', async () => { const entry = createEntry({ contentElements: [ - {id: 1, permaId: 10, typeName: 'textBlock'}, - {id: 2, permaId: 11, typeName: 'textBlock'} + {id: 1, permaId: 10, typeName: 'fixture'}, + {id: 2, permaId: 11, typeName: 'fixture'} ] }); entry.set('selectedContentElementCommentsId', 1); @@ -148,7 +154,7 @@ describe('ContentElementCommentsView', () => { it('marks thread matching entry.highlightedThreadId with aria-current when scoped', () => { const entry = createEntry({ - contentElements: [{id: 1, permaId: 10, typeName: 'textBlock'}] + contentElements: [{id: 1, permaId: 10, typeName: 'fixture'}] }); entry.set('selectedContentElementCommentsId', 1); entry.contentElements.get(1).transientState @@ -172,7 +178,7 @@ describe('ContentElementCommentsView', () => { it('updates highlight when entry.highlightedThreadId changes', () => { const entry = createEntry({ - contentElements: [{id: 1, permaId: 10, typeName: 'textBlock'}] + contentElements: [{id: 1, permaId: 10, typeName: 'fixture'}] }); entry.set('selectedContentElementCommentsId', 1); entry.contentElements.get(1).transientState @@ -200,7 +206,7 @@ describe('ContentElementCommentsView', () => { const user = userEvent.setup(); const entry = createEntry({ - contentElements: [{id: 1, permaId: 10, typeName: 'textBlock'}] + contentElements: [{id: 1, permaId: 10, typeName: 'fixture'}] }); entry.set('selectedContentElementCommentsId', 1); entry.contentElements.get(1).transientState @@ -227,7 +233,7 @@ describe('ContentElementCommentsView', () => { const user = userEvent.setup(); const entry = createEntry({ - contentElements: [{id: 1, permaId: 10, typeName: 'textBlock'}] + contentElements: [{id: 1, permaId: 10, typeName: 'fixture'}] }); entry.set('selectedContentElementCommentsId', 1); entry.set('highlightedThreadId', 7); @@ -251,9 +257,61 @@ describe('ContentElementCommentsView', () => { expect(listener).not.toHaveBeenCalled(); }); + it("orders threads by the type's compareRanges", () => { + const entry = createEntry({ + contentElements: [{id: 1, permaId: 10, typeName: 'fixture'}] + }); + entry.set('selectedContentElementCommentsId', 1); + entry.reviewSession = factories.reviewSession({ + commentThreads: [ + {id: 1, subjectType: 'ContentElement', subjectId: 10, + subjectRange: {start: 20}, + comments: [{id: 10, body: 'second', creatorName: 'A'}]}, + {id: 2, subjectType: 'ContentElement', subjectId: 10, + subjectRange: {start: 10}, + comments: [{id: 20, body: 'first', creatorName: 'B'}]} + ] + }); + + const view = new ContentElementCommentsView({entry, editor}); + const {getByText} = renderBackboneView(view); + + const order = ['first', 'second'] + .map(text => getByText(text).getBoundingClientRect().top); + + expect(order).toEqual([...order].sort((a, b) => a - b)); + }); + + it("orders scoped threads by the type's compareRanges", () => { + const entry = createEntry({ + contentElements: [{id: 1, permaId: 10, typeName: 'fixture'}] + }); + entry.set('selectedContentElementCommentsId', 1); + entry.contentElements.get(1).transientState + .set('commentThreadIdsAtSelection', [1, 2]); + entry.reviewSession = factories.reviewSession({ + commentThreads: [ + {id: 1, subjectType: 'ContentElement', subjectId: 10, + subjectRange: {start: 20}, + comments: [{id: 10, body: 'second', creatorName: 'A'}]}, + {id: 2, subjectType: 'ContentElement', subjectId: 10, + subjectRange: {start: 10}, + comments: [{id: 20, body: 'first', creatorName: 'B'}]} + ] + }); + + const view = new ContentElementCommentsView({entry, editor}); + const {getByText} = renderBackboneView(view); + + const order = ['first', 'second'] + .map(text => getByText(text).getBoundingClientRect().top); + + expect(order).toEqual([...order].sort((a, b) => a - b)); + }); + it('does not render a new-topic button', () => { const entry = createEntry({ - contentElements: [{id: 1, permaId: 10, typeName: 'textBlock'}] + contentElements: [{id: 1, permaId: 10, typeName: 'fixture'}] }); entry.set('selectedContentElementCommentsId', 1); entry.reviewSession = factories.reviewSession({ @@ -273,7 +331,7 @@ describe('ContentElementCommentsView', () => { it('updates when session emits change:thread', async () => { const entry = createEntry({ - contentElements: [{id: 1, permaId: 10, typeName: 'textBlock'}] + contentElements: [{id: 1, permaId: 10, typeName: 'fixture'}] }); entry.set('selectedContentElementCommentsId', 1); entry.reviewSession = factories.reviewSession(); 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 fee23e0de3..b7f39d39ec 100644 --- a/entry_types/scrolled/package/spec/editor/views/EntryCommentsView-spec.js +++ b/entry_types/scrolled/package/spec/editor/views/EntryCommentsView-spec.js @@ -13,6 +13,12 @@ import {useEditorGlobals} from 'support'; describe('EntryCommentsView', () => { const {createEntry} = useEditorGlobals(); + beforeAll(() => { + editor.contentElementTypes.register('fixture', { + compareRanges: (a, b) => (a?.start ?? 0) - (b?.start ?? 0) + }); + }); + useFakeTranslations({ 'pageflow_scrolled.review.new_topic': 'New topic', 'pageflow_scrolled.review.add_comment_placeholder': 'Add a comment...', @@ -196,6 +202,33 @@ describe('EntryCommentsView', () => { .toContainElement(replyInputs[0]); }); + it("orders threads within a group by the type's compareRanges", () => { + const entry = createEntry({ + contentElements: [{id: 1, permaId: 10, typeName: 'fixture'}] + }); + entry.reviewSession = factories.reviewSession({ + commentThreads: [ + {id: 1, subjectType: 'ContentElement', subjectId: 10, + subjectRange: {start: 30}, + comments: [{id: 10, body: 'third', creatorName: 'A'}]}, + {id: 2, subjectType: 'ContentElement', subjectId: 10, + subjectRange: {start: 10}, + comments: [{id: 20, body: 'first', creatorName: 'B'}]}, + {id: 3, subjectType: 'ContentElement', subjectId: 10, + subjectRange: {start: 20}, + comments: [{id: 30, body: 'second', creatorName: 'C'}]} + ] + }); + + const view = new EntryCommentsView({entry, editor}); + const {getByText} = renderBackboneView(view); + + const order = ['first', 'second', 'third'] + .map(text => getByText(text).getBoundingClientRect().top); + + expect(order).toEqual([...order].sort((a, b) => a - b)); + }); + it('shows the content element type name as group label', () => { const entry = createEntry({ contentElements: [{id: 1, permaId: 10, typeName: 'image'}] diff --git a/entry_types/scrolled/package/spec/review/ThreadList-spec.js b/entry_types/scrolled/package/spec/review/ThreadList-spec.js index 7d683e1bf8..8280598460 100644 --- a/entry_types/scrolled/package/spec/review/ThreadList-spec.js +++ b/entry_types/scrolled/package/spec/review/ThreadList-spec.js @@ -152,6 +152,34 @@ describe('ThreadList', () => { expect(onThreadClick).toHaveBeenCalledWith(expect.objectContaining({id: 7})); }); + it('orders threads via compareRanges when provided', () => { + const compareRanges = (a, b) => a.start - b.start; + + const {getAllByText} = renderWithReviewState( + , + { + commentThreads: [ + {id: 1, subjectType: 'ContentElement', subjectId: 10, + subjectRange: {start: 30}, + comments: [{id: 10, body: 'third', creatorName: 'Bob', creatorId: 2}]}, + {id: 2, subjectType: 'ContentElement', subjectId: 10, + subjectRange: {start: 10}, + comments: [{id: 20, body: 'first', creatorName: 'Alice', creatorId: 1}]}, + {id: 3, subjectType: 'ContentElement', subjectId: 10, + subjectRange: {start: 20}, + comments: [{id: 30, body: 'second', creatorName: 'Eve', creatorId: 3}]} + ] + } + ); + + const order = ['first', 'second', 'third'] + .map(text => getAllByText(text)[0].getBoundingClientRect().top); + + expect(order).toEqual([...order].sort((a, b) => a - b)); + }); + it('hides reply form on non-highlighted threads when restrictInteractionsToHighlighted', () => { const {getByText, queryAllByPlaceholderText} = renderWithReviewState( - this.options.entry.trigger('selectCommentThread', thread.id) + compareRanges: typeName && editor.contentElementTypes.findCompareRanges(typeName), + highlightedThreadId: entry.get('highlightedThreadId'), + onThreadClick: thread => entry.trigger('selectCommentThread', thread.id) }; }, @@ -33,13 +35,14 @@ export const ContentElementCommentsView = ReviewView.extend({ // there is no anchor for an individual thread in the iframe, so // highlighting one in the list would have no counterpart in the // preview. - renderContent({contentElement, threadIds, highlightedThreadId, onThreadClick}) { + renderContent({contentElement, threadIds, compareRanges, highlightedThreadId, onThreadClick}) { if (!contentElement) return null; if (threadIds === undefined) { return ( ); } @@ -48,6 +51,7 @@ export const ContentElementCommentsView = ReviewView.extend({ threadIds.includes(thread.id)} + compareRanges={compareRanges} highlightedThreadId={highlightedThreadId} onThreadClick={onThreadClick} hideNewTopicButton /> diff --git a/entry_types/scrolled/package/src/editor/views/EntryCommentsView.js b/entry_types/scrolled/package/src/editor/views/EntryCommentsView.js index 8b21986d0e..daa20c7909 100644 --- a/entry_types/scrolled/package/src/editor/views/EntryCommentsView.js +++ b/entry_types/scrolled/package/src/editor/views/EntryCommentsView.js @@ -67,12 +67,14 @@ function ContentElementGroup({contentElement, highlightedThreadId, onThreadClick const typeName = contentElement.get('typeName'); const label = I18n.t(`pageflow_scrolled.editor.content_elements.${typeName}.name`); const pictogram = editor.contentElementTypes.findPictogram(typeName) || defaultPictogram; + const compareRanges = editor.contentElementTypes.findCompareRanges(typeName); return (
filter ? allActiveThreads.filter(filter) : allActiveThreads, - [allActiveThreads, filter] + () => sortByRange(filter ? allActiveThreads.filter(filter) : allActiveThreads, compareRanges), + [allActiveThreads, filter, compareRanges] ); const resolvedThreads = useMemo( - () => filter ? allResolvedThreads.filter(filter) : allResolvedThreads, - [allResolvedThreads, filter] + () => sortByRange(filter ? allResolvedThreads.filter(filter) : allResolvedThreads, compareRanges), + [allResolvedThreads, filter, compareRanges] ); const [expandedThreadId, setExpandedThreadId] = useState(null); @@ -93,3 +93,8 @@ export function ThreadList({subjectType, subjectId, subjectRange, filter, highli
); } + +function sortByRange(threads, compareRanges) { + if (!compareRanges) return threads; + return [...threads].sort((a, b) => compareRanges(a.subjectRange, b.subjectRange)); +} From 4be2b02169946296d041be543b3d4daed97cb046 Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Thu, 7 May 2026 21:09:38 +0200 Subject: [PATCH 03/13] Highlight all threads of selected element when it has no commentThreadIdsAtSelection --- .../editor/views/EntryCommentsView-spec.js | 79 +++++++++++++++++++ .../package/spec/review/ThreadList-spec.js | 27 +++++++ .../src/editor/views/EntryCommentsView.js | 52 +++++++++++- .../scrolled/package/src/review/ThreadList.js | 14 ++-- 4 files changed, 162 insertions(+), 10 deletions(-) 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 b7f39d39ec..6a4dc5237c 100644 --- a/entry_types/scrolled/package/spec/editor/views/EntryCommentsView-spec.js +++ b/entry_types/scrolled/package/spec/editor/views/EntryCommentsView-spec.js @@ -179,6 +179,85 @@ describe('EntryCommentsView', () => { expect(getByText('first').closest('[aria-current="true"]')).not.toBeNull(); }); + it("highlights all threads of the selected element when it has no commentThreadIdsAtSelection", () => { + const entry = createEntry({ + contentElements: [ + {id: 1, permaId: 10, typeName: 'image'}, + {id: 2, permaId: 11, typeName: 'image'} + ] + }); + entry.set('selectedContentElementCommentsId', 1); + entry.reviewSession = factories.reviewSession({ + commentThreads: [ + {id: 1, subjectType: 'ContentElement', subjectId: 10, + comments: [{id: 10, body: 'on selected one', creatorName: 'A'}]}, + {id: 2, subjectType: 'ContentElement', subjectId: 10, + comments: [{id: 20, body: 'on selected two', creatorName: 'B'}]}, + {id: 3, subjectType: 'ContentElement', subjectId: 11, + comments: [{id: 30, body: 'on other', creatorName: 'C'}]} + ] + }); + + const view = new EntryCommentsView({entry, editor}); + const {getByText} = renderBackboneView(view); + + expect(getByText('on selected one').closest('[aria-current="true"]')).not.toBeNull(); + expect(getByText('on selected two').closest('[aria-current="true"]')).not.toBeNull(); + expect(getByText('on other').closest('[aria-current="true"]')).toBeNull(); + }); + + it("falls back to highlightedThreadId when the selected element has commentThreadIdsAtSelection", () => { + const entry = createEntry({ + contentElements: [{id: 1, permaId: 10, typeName: 'textBlock'}] + }); + entry.set('selectedContentElementCommentsId', 1); + entry.contentElements.get(1).transientState + .set('commentThreadIdsAtSelection', [2]); + entry.set('highlightedThreadId', 2); + entry.reviewSession = factories.reviewSession({ + commentThreads: [ + {id: 1, subjectType: 'ContentElement', subjectId: 10, + comments: [{id: 10, body: 'first', creatorName: 'A'}]}, + {id: 2, subjectType: 'ContentElement', subjectId: 10, + comments: [{id: 20, body: 'second', creatorName: 'B'}]} + ] + }); + + const view = new EntryCommentsView({entry, editor}); + const {getByText} = renderBackboneView(view); + + expect(getByText('first').closest('[aria-current="true"]')).toBeNull(); + expect(getByText('second').closest('[aria-current="true"]')).not.toBeNull(); + }); + + it('updates highlights when selectedContentElementCommentsId changes', () => { + const entry = createEntry({ + contentElements: [ + {id: 1, permaId: 10, typeName: 'image'}, + {id: 2, permaId: 11, typeName: 'image'} + ] + }); + entry.reviewSession = factories.reviewSession({ + commentThreads: [ + {id: 1, subjectType: 'ContentElement', subjectId: 10, + comments: [{id: 10, body: 'on one', creatorName: 'A'}]}, + {id: 2, subjectType: 'ContentElement', subjectId: 11, + comments: [{id: 20, body: 'on two', creatorName: 'B'}]} + ] + }); + + const view = new EntryCommentsView({entry, editor}); + const {getByText} = renderBackboneView(view); + + expect(getByText('on one').closest('[aria-current="true"]')).toBeNull(); + expect(getByText('on two').closest('[aria-current="true"]')).toBeNull(); + + act(() => { entry.set('selectedContentElementCommentsId', 2); }); + + expect(getByText('on one').closest('[aria-current="true"]')).toBeNull(); + expect(getByText('on two').closest('[aria-current="true"]')).not.toBeNull(); + }); + it('only shows reply form on the highlighted thread', () => { const entry = createEntry({ contentElements: [{id: 1, permaId: 10, typeName: 'textBlock'}] diff --git a/entry_types/scrolled/package/spec/review/ThreadList-spec.js b/entry_types/scrolled/package/spec/review/ThreadList-spec.js index 8280598460..98511ee4bf 100644 --- a/entry_types/scrolled/package/spec/review/ThreadList-spec.js +++ b/entry_types/scrolled/package/spec/review/ThreadList-spec.js @@ -131,6 +131,33 @@ describe('ThreadList', () => { expect(highlighted).not.toContainElement(getByText('first')); }); + it('highlights every thread when highlightedThreadId is an array of ids', () => { + const {container, getByText} = renderWithReviewState( + , + { + commentThreads: [ + {id: 1, subjectType: 'ContentElement', subjectId: 10, comments: [ + {id: 10, body: 'first', creatorName: 'Alice', creatorId: 1} + ]}, + {id: 2, subjectType: 'ContentElement', subjectId: 10, comments: [ + {id: 20, body: 'second', creatorName: 'Bob', creatorId: 2} + ]}, + {id: 3, subjectType: 'ContentElement', subjectId: 10, comments: [ + {id: 30, body: 'third', creatorName: 'Eve', creatorId: 3} + ]} + ] + } + ); + + const highlighted = container.querySelectorAll('[aria-current="true"]'); + expect(highlighted).toHaveLength(2); + expect(getByText('first').closest('[aria-current="true"]')).not.toBeNull(); + expect(getByText('second').closest('[aria-current="true"]')).not.toBeNull(); + expect(getByText('third').closest('[aria-current="true"]')).toBeNull(); + }); + it('fires onThreadClick with the clicked thread', async () => { const user = userEvent.setup(); const onThreadClick = jest.fn(); diff --git a/entry_types/scrolled/package/src/editor/views/EntryCommentsView.js b/entry_types/scrolled/package/src/editor/views/EntryCommentsView.js index daa20c7909..eb54f7cf10 100644 --- a/entry_types/scrolled/package/src/editor/views/EntryCommentsView.js +++ b/entry_types/scrolled/package/src/editor/views/EntryCommentsView.js @@ -9,33 +9,67 @@ import styles from './EntryCommentsView.module.css'; export const EntryCommentsView = ReviewView.extend({ initialize() { - this.listenTo(this.options.entry, + const {entry} = this.options; + + this.listenTo(entry, 'change:highlightedThreadId', () => this.rerender()); + this.listenTo(entry, + 'change:selectedContentElementCommentsId', + this._onSelectedChange); + this._observeSelectedElement(); }, props() { const {entry, editor} = this.options; return { items: collectContentElements(entry), + selectedElement: this._selectedElement, + // Undefined for elements without a slate cursor (e.g. images); + // an array (possibly empty) for textBlocks where Selection.js + // has reported the cursor's overlapping threads. + transientThreadIds: + this._selectedElement?.transientState.get('commentThreadIdsAtSelection'), highlightedThreadId: entry.get('highlightedThreadId'), onThreadClick: thread => entry.trigger('selectCommentThread', thread.id), editor }; }, - renderContent({items, highlightedThreadId, onThreadClick, editor}) { + renderContent({items, selectedElement, transientThreadIds, highlightedThreadId, onThreadClick, editor}) { return (
{items.map(({contentElement}) => ( ))}
); + }, + + _onSelectedChange() { + this._observeSelectedElement(); + this.rerender(); + }, + + _observeSelectedElement() { + if (this._selectedElement) { + this.stopListening(this._selectedElement.transientState); + } + + const id = this.options.entry.get('selectedContentElementCommentsId'); + this._selectedElement = id ? this.options.entry.contentElements.get(id) : null; + + if (this._selectedElement) { + this.listenTo(this._selectedElement.transientState, + 'change:commentThreadIdsAtSelection', + () => this.rerender()); + } } }); @@ -53,7 +87,10 @@ function collectContentElements(entry) { return items; } -function ContentElementGroup({contentElement, highlightedThreadId, onThreadClick, editor}) { +function ContentElementGroup({ + contentElement, isSelected, selectedHasTransientThreadIds, + highlightedThreadId, onThreadClick, editor +}) { const permaId = contentElement.get('permaId'); const threads = useCommentThreads({ subjectType: 'ContentElement', @@ -69,13 +106,20 @@ function ContentElementGroup({contentElement, highlightedThreadId, onThreadClick const pictogram = editor.contentElementTypes.findPictogram(typeName) || defaultPictogram; const compareRanges = editor.contentElementTypes.findCompareRanges(typeName); + // Element-level badges (e.g. on images) have no per-thread anchor in + // the iframe, so clicking such a badge highlights every thread of + // the element rather than just one. + const groupHighlight = isSelected && !selectedHasTransientThreadIds ? + threads.map(t => t.id) : + highlightedThreadId; + return (
diff --git a/entry_types/scrolled/package/src/review/ThreadList.js b/entry_types/scrolled/package/src/review/ThreadList.js index ef4732a83b..89856cb696 100644 --- a/entry_types/scrolled/package/src/review/ThreadList.js +++ b/entry_types/scrolled/package/src/review/ThreadList.js @@ -25,6 +25,10 @@ export function ThreadList({subjectType, subjectId, subjectRange, filter, compar [allResolvedThreads, filter, compareRanges] ); + const isHighlighted = thread => Array.isArray(highlightedThreadId) ? + highlightedThreadId.includes(thread.id) : + thread.id === highlightedThreadId; + const [expandedThreadId, setExpandedThreadId] = useState(null); const [formToggled, setFormToggled] = useState(null); const [showResolved, setShowResolved] = useState(false); @@ -64,9 +68,8 @@ export function ThreadList({subjectType, subjectId, subjectRange, filter, compar onToggle={() => toggleThread(thread.id)} onResolve={() => postUpdateThreadMessage({threadId: thread.id, resolved: true})} onClick={onThreadClick && (() => onThreadClick(thread))} - highlighted={thread.id === highlightedThreadId} - interactive={!restrictInteractionsToHighlighted || - thread.id === highlightedThreadId} /> + highlighted={isHighlighted(thread)} + interactive={!restrictInteractionsToHighlighted || isHighlighted(thread)} /> ))} {resolvedThreads.length > 0 && @@ -85,9 +88,8 @@ export function ThreadList({subjectType, subjectId, subjectRange, filter, compar onToggle={() => toggleThread(thread.id)} onResolve={() => postUpdateThreadMessage({threadId: thread.id, resolved: false})} onClick={onThreadClick && (() => onThreadClick(thread))} - highlighted={thread.id === highlightedThreadId} - interactive={!restrictInteractionsToHighlighted || - thread.id === highlightedThreadId} /> + highlighted={isHighlighted(thread)} + interactive={!restrictInteractionsToHighlighted || isHighlighted(thread)} /> ))}
} From 29f2138ddddf3cf4a9d9097a7cd09e6827e1b67d Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Thu, 7 May 2026 21:23:14 +0200 Subject: [PATCH 04/13] Pin CommentsView tabs to top of sidebar scroller --- .../package/src/editor/views/CommentsView.js | 4 +++- .../src/editor/views/CommentsView.module.css | 16 ++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) create mode 100644 entry_types/scrolled/package/src/editor/views/CommentsView.module.css diff --git a/entry_types/scrolled/package/src/editor/views/CommentsView.js b/entry_types/scrolled/package/src/editor/views/CommentsView.js index 7f62b84a0b..6746999931 100644 --- a/entry_types/scrolled/package/src/editor/views/CommentsView.js +++ b/entry_types/scrolled/package/src/editor/views/CommentsView.js @@ -7,8 +7,10 @@ import {TabsView} from 'pageflow/ui'; import {EntryCommentsView} from './EntryCommentsView'; import {ContentElementCommentsView} from './ContentElementCommentsView'; +import styles from './CommentsView.module.css'; + export const CommentsView = Marionette.ItemView.extend({ - className: 'comments_view', + className: styles.root, template: () => ` ${I18n.t('pageflow.editor.templates.back_button_decorator.outline')} diff --git a/entry_types/scrolled/package/src/editor/views/CommentsView.module.css b/entry_types/scrolled/package/src/editor/views/CommentsView.module.css new file mode 100644 index 0000000000..b96262d27a --- /dev/null +++ b/entry_types/scrolled/package/src/editor/views/CommentsView.module.css @@ -0,0 +1,16 @@ +/* + * Pin the tab bar to the top of the sidebar's scrolling area so it + * stays visible while the active tab's thread list scrolls past + * underneath. The outer sidebar scroller does the scrolling. The + * negative top offset cancels the scroller's 10px padding so the bar + * meets the sidebar edge cleanly. The box-shadow extends the surface + * color sideways to hide highlighted threads' accent halo as they + * scroll past underneath. + */ +.root :global(.tabs_view-scroller) { + position: sticky; + top: -10px; + z-index: 1; + background-color: var(--ui-surface-color); + box-shadow: 0 0 0 space(1) var(--ui-surface-color); +} From 60a3adb971675299ee19329ed2c2dd3f2db8a77e Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Thu, 7 May 2026 21:31:18 +0200 Subject: [PATCH 05/13] Scroll highlighted thread into view --- entry_types/scrolled/package/jest.config.js | 1 + .../package/spec/review/ThreadList-spec.js | 23 +++++++++++++++++++ .../spec/support/scrollIntoViewStub.js | 6 +++++ .../scrolled/package/src/review/Thread.js | 13 +++++++++-- .../package/src/review/Thread.module.css | 1 + 5 files changed, 42 insertions(+), 2 deletions(-) create mode 100644 entry_types/scrolled/package/spec/support/scrollIntoViewStub.js diff --git a/entry_types/scrolled/package/jest.config.js b/entry_types/scrolled/package/jest.config.js index 269d8a3383..3030b327b0 100644 --- a/entry_types/scrolled/package/jest.config.js +++ b/entry_types/scrolled/package/jest.config.js @@ -14,6 +14,7 @@ module.exports = { setupFilesAfterEnv: [ '/spec/support/matchMediaStub.js', '/spec/support/requestAnimationFrameStub.js', + '/spec/support/scrollIntoViewStub.js', '/spec/support/fakeBrowserFeatures.js' ], modulePaths: ['/src', '/spec'], diff --git a/entry_types/scrolled/package/spec/review/ThreadList-spec.js b/entry_types/scrolled/package/spec/review/ThreadList-spec.js index 98511ee4bf..793a251465 100644 --- a/entry_types/scrolled/package/spec/review/ThreadList-spec.js +++ b/entry_types/scrolled/package/spec/review/ThreadList-spec.js @@ -179,6 +179,29 @@ describe('ThreadList', () => { expect(onThreadClick).toHaveBeenCalledWith(expect.objectContaining({id: 7})); }); + it('scrolls the highlighted thread into view', () => { + const {getByText} = renderWithReviewState( + , + { + commentThreads: [ + {id: 1, subjectType: 'ContentElement', subjectId: 10, comments: [ + {id: 10, body: 'first', creatorName: 'Alice', creatorId: 1} + ]}, + {id: 2, subjectType: 'ContentElement', subjectId: 10, comments: [ + {id: 20, body: 'second', creatorName: 'Bob', creatorId: 2} + ]} + ] + } + ); + + const scrollIntoView = Element.prototype.scrollIntoView; + expect(scrollIntoView).toHaveBeenCalled(); + expect(scrollIntoView.mock.instances[0]) + .toBe(getByText('second').closest('[aria-current="true"]')); + }); + it('orders threads via compareRanges when provided', () => { const compareRanges = (a, b) => a.start - b.start; diff --git a/entry_types/scrolled/package/spec/support/scrollIntoViewStub.js b/entry_types/scrolled/package/spec/support/scrollIntoViewStub.js new file mode 100644 index 0000000000..56b6f4456e --- /dev/null +++ b/entry_types/scrolled/package/spec/support/scrollIntoViewStub.js @@ -0,0 +1,6 @@ +// jsdom does not implement scrollIntoView. Stub it so components that +// call it (e.g. Thread auto-scrolling its highlighted state into view) +// don't blow up during render. +beforeEach(() => { + Element.prototype.scrollIntoView = jest.fn(); +}); diff --git a/entry_types/scrolled/package/src/review/Thread.js b/entry_types/scrolled/package/src/review/Thread.js index 4763143286..4cf6c8915a 100644 --- a/entry_types/scrolled/package/src/review/Thread.js +++ b/entry_types/scrolled/package/src/review/Thread.js @@ -1,4 +1,4 @@ -import React from 'react'; +import React, {useEffect, useRef} from 'react'; import classNames from 'classnames'; import {useI18n} from '../frontend/i18n'; @@ -17,8 +17,17 @@ export function Thread({thread, collapsed, onToggle, onResolve, onClick, highlig const replies = thread.comments.slice(1); const repliesCollapsed = collapsed && replies.length > 0; + const ref = useRef(); + + useEffect(() => { + if (highlighted && ref.current) { + ref.current.scrollIntoView({block: 'nearest', behavior: 'smooth'}); + } + }, [highlighted]); + return ( -
Date: Fri, 8 May 2026 08:32:02 +0200 Subject: [PATCH 06/13] Source new thread subject range from transient state --- .../newCommentThreadSubjectRange-spec.js | 56 +++++++++++++++++++ .../inlineEditing/EditableText/Selection.js | 2 + .../newCommentThreadSubjectRange.js | 19 +++++++ 3 files changed, 77 insertions(+) create mode 100644 entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/newCommentThreadSubjectRange-spec.js create mode 100644 entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/newCommentThreadSubjectRange.js diff --git a/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/newCommentThreadSubjectRange-spec.js b/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/newCommentThreadSubjectRange-spec.js new file mode 100644 index 0000000000..635ee8e76e --- /dev/null +++ b/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/newCommentThreadSubjectRange-spec.js @@ -0,0 +1,56 @@ +/** @jsx jsx */ +import {newCommentThreadSubjectRange} + from 'frontend/inlineEditing/EditableText/newCommentThreadSubjectRange'; + +import {createHyperscript} from 'slate-hyperscript'; + +const h = createHyperscript({ + elements: {paragraph: {type: 'paragraph'}} +}); + +const jsx = (tagName, attributes, ...children) => { + delete attributes.__self; + delete attributes.__source; + return h(tagName, attributes, ...children); +}; + +describe('newCommentThreadSubjectRange', () => { + it('returns undefined when there is no selection', () => { + const editor = ( + + Some text + + ); + + expect(newCommentThreadSubjectRange(editor)).toBeUndefined(); + }); + + it("returns the surrounding block's range when selection is collapsed", () => { + const editor = ( + + First + Second + + ); + + expect(newCommentThreadSubjectRange(editor)).toEqual({ + anchor: {path: [1, 0], offset: 0}, + focus: {path: [1, 0], offset: 6} + }); + }); + + it('returns the editor selection when expanded', () => { + const editor = ( + + + First + + + ); + + expect(newCommentThreadSubjectRange(editor)).toEqual({ + anchor: {path: [0, 0], offset: 0}, + focus: {path: [0, 0], offset: 5} + }); + }); +}); diff --git a/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/Selection.js b/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/Selection.js index 744d4e48d8..8cd373ab65 100644 --- a/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/Selection.js +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/Selection.js @@ -17,6 +17,7 @@ import {getUniformSelectedNode} from './getUniformSelectedNode'; import {toggleBlock, isBlockActive} from './blocks'; import {commentThreadIdsAtSelection} from './commentThreadIdsAtSelection'; import {computeBounds} from './computeBounds'; +import {newCommentThreadSubjectRange} from './newCommentThreadSubjectRange'; import TextIcon from '../images/text.svg'; import HeadingIcon from '../images/heading.svg'; @@ -139,6 +140,7 @@ export function Selection(props) { commentThreadIdsAtSelection: commentThreadIdsAtSelection( props.highlights || [], editor.selection ), + newCommentThreadSubjectRange: newCommentThreadSubjectRange(editor), }); boundsRef.current = {start, end}; diff --git a/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/newCommentThreadSubjectRange.js b/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/newCommentThreadSubjectRange.js new file mode 100644 index 0000000000..61323b6945 --- /dev/null +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/newCommentThreadSubjectRange.js @@ -0,0 +1,19 @@ +import {Editor, Range} from 'slate'; + +// Range to seed a new comment thread on the current slate selection. +// When the selection is collapsed (just a cursor), expand to the full +// surrounding top-level block so the new thread anchors to the whole +// paragraph rather than a zero-width point. +export function newCommentThreadSubjectRange(editor) { + if (!editor.selection) return undefined; + + if (Range.isCollapsed(editor.selection)) { + const blockIdx = editor.selection.anchor.path[0]; + return { + anchor: Editor.point(editor, [blockIdx], {edge: 'start'}), + focus: Editor.point(editor, [blockIdx], {edge: 'end'}) + }; + } + + return editor.selection; +} From b2e6d17ac6ed8518231e1d7d615ea76b74f32510 Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Fri, 8 May 2026 08:36:34 +0200 Subject: [PATCH 07/13] Add new-thread button to CommentsView --- entry_types/scrolled/config/locales/de.yml | 4 + entry_types/scrolled/config/locales/en.yml | 4 + .../PreviewMessageController-spec.js | 31 ++++++++ .../spec/editor/views/CommentsView-spec.js | 77 +++++++++++++++++++ .../spec/editor/views/NewThreadView-spec.js | 27 ++++++- .../controllers/PreviewMessageController.js | 7 ++ .../package/src/editor/views/CommentsView.js | 40 ++++++++-- .../src/editor/views/CommentsView.module.css | 9 +++ .../package/src/editor/views/NewThreadView.js | 32 +++++++- .../src/editor/views/NewThreadView.module.css | 3 + .../src/editor/views/buttons.module.css | 5 ++ .../package/src/editor/views/icons.module.css | 5 ++ 12 files changed, 232 insertions(+), 12 deletions(-) create mode 100644 entry_types/scrolled/package/src/editor/views/NewThreadView.module.css diff --git a/entry_types/scrolled/config/locales/de.yml b/entry_types/scrolled/config/locales/de.yml index 6b6c0c47d5..abeb3738ed 100644 --- a/entry_types/scrolled/config/locales/de.yml +++ b/entry_types/scrolled/config/locales/de.yml @@ -1587,9 +1587,13 @@ de: main_menu: comments: Kommentare comments_view: + new_thread: Neues Thema tabs: comments: Alle Kommentare selection: Für Auswahl + new_thread_view: + tabs: + newComment: Neues Thema help_entries: content_elements: menu_item: Inhaltselemente diff --git a/entry_types/scrolled/config/locales/en.yml b/entry_types/scrolled/config/locales/en.yml index 92439f3d7c..528892bdcd 100644 --- a/entry_types/scrolled/config/locales/en.yml +++ b/entry_types/scrolled/config/locales/en.yml @@ -1570,9 +1570,13 @@ en: main_menu: comments: Comments comments_view: + new_thread: New topic tabs: comments: All comments selection: For selection + new_thread_view: + tabs: + newComment: New topic help_entries: content_elements: menu_item: Content Elements diff --git a/entry_types/scrolled/package/spec/editor/controllers/PreviewMessageController-spec.js b/entry_types/scrolled/package/spec/editor/controllers/PreviewMessageController-spec.js index eb9ec16c5d..afee2505f4 100644 --- a/entry_types/scrolled/package/spec/editor/controllers/PreviewMessageController-spec.js +++ b/entry_types/scrolled/package/spec/editor/controllers/PreviewMessageController-spec.js @@ -177,6 +177,37 @@ describe('PreviewMessageController', () => { }); }); + it('forwards selectNewThread events to iframe as SELECT newThread', async () => { + const entry = factories.entry(ScrolledEntry, {}, {entryTypeSeed: normalizeSeed()}); + const iframeWindow = createIframeWindow(); + controller = new PreviewMessageController({entry, iframeWindow}); + + await postReadyMessageAndWaitForAcknowledgement(iframeWindow); + + const range = {anchor: {path: [0, 0], offset: 0}, focus: {path: [0, 0], offset: 5}}; + + return expect(new Promise(resolve => { + iframeWindow.addEventListener('message', event => { + if (event.data.type === 'SELECT' && event.data.payload.type === 'newThread') { + resolve(event.data); + } + }); + entry.trigger('selectNewThread', { + id: 10, + subjectType: 'ContentElement', + range + }); + })).resolves.toMatchObject({ + type: 'SELECT', + payload: { + type: 'newThread', + id: 10, + subjectType: 'ContentElement', + range + } + }); + }); + it('passes on range from selectContentElement event', async () => { const entry = factories.entry(ScrolledEntry, {}, { entryTypeSeed: normalizeSeed({ 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 f86a2fc49a..e15ae778ce 100644 --- a/entry_types/scrolled/package/spec/editor/views/CommentsView-spec.js +++ b/entry_types/scrolled/package/spec/editor/views/CommentsView-spec.js @@ -9,6 +9,14 @@ import {useEditorGlobals} from 'support'; import {fireEvent} from '@testing-library/dom'; import {act} from '@testing-library/react'; +function unselectedEntry(createEntry) { + const entry = createEntry({ + contentElements: [{id: 1, permaId: 10, typeName: 'textBlock'}] + }); + entry.reviewSession = factories.reviewSession(); + return entry; +} + describe('CommentsView', () => { const {createEntry} = useEditorGlobals(); @@ -19,6 +27,7 @@ describe('CommentsView', () => { 'pageflow_scrolled.editor.content_elements.textBlock.name': 'Text', 'pageflow_scrolled.editor.comments_view.tabs.comments': 'All comments', 'pageflow_scrolled.editor.comments_view.tabs.selection': 'For selection', + 'pageflow_scrolled.editor.comments_view.new_thread': 'New topic', 'pageflow.editor.templates.back_button_decorator.outline': 'Outline' }); @@ -108,4 +117,72 @@ describe('CommentsView', () => { navigate.mockRestore(); }); + it('disables the new-thread button when no content element is selected', () => { + const entry = unselectedEntry(createEntry); + + const view = new CommentsView({entry, editor}); + const {getByRole} = renderBackboneView(view); + + expect(getByRole('button', {name: 'New topic'})).toBeDisabled(); + }); + + it('enables the new-thread button when a content element is selected', () => { + const entry = unselectedEntry(createEntry); + entry.set('selectedContentElementCommentsId', 1); + + const view = new CommentsView({entry, editor}); + const {getByRole} = renderBackboneView(view); + + expect(getByRole('button', {name: 'New topic'})).not.toBeDisabled(); + }); + + it("toggles the new-thread button when entry.selectedContentElementCommentsId changes", () => { + const entry = unselectedEntry(createEntry); + + const view = new CommentsView({entry, editor}); + const {getByRole} = renderBackboneView(view); + + expect(getByRole('button', {name: 'New topic'})).toBeDisabled(); + + act(() => { entry.set('selectedContentElementCommentsId', 1); }); + + expect(getByRole('button', {name: 'New topic'})).not.toBeDisabled(); + }); + + it('triggers selectNewThread on entry when the new-thread button is clicked', () => { + const entry = unselectedEntry(createEntry); + entry.set('selectedContentElementCommentsId', 1); + const range = {anchor: {path: [0, 0], offset: 0}, focus: {path: [0, 0], offset: 5}}; + entry.contentElements.get(1).transientState + .set('newCommentThreadSubjectRange', range); + + const view = new CommentsView({entry, editor}); + const {getByRole} = renderBackboneView(view); + + const listener = jest.fn(); + entry.on('selectNewThread', listener); + + fireEvent.click(getByRole('button', {name: 'New topic'})); + + expect(listener).toHaveBeenCalledWith({ + id: 10, + subjectType: 'ContentElement', + range + }); + }); + + it('does not trigger selectNewThread when clicking the button while disabled', () => { + const entry = unselectedEntry(createEntry); + + const view = new CommentsView({entry, editor}); + const {getByRole} = renderBackboneView(view); + + const listener = jest.fn(); + entry.on('selectNewThread', listener); + + fireEvent.click(getByRole('button', {name: 'New topic'})); + + expect(listener).not.toHaveBeenCalled(); + }); + }); diff --git a/entry_types/scrolled/package/spec/editor/views/NewThreadView-spec.js b/entry_types/scrolled/package/spec/editor/views/NewThreadView-spec.js index 10ee72616f..f23a128946 100644 --- a/entry_types/scrolled/package/spec/editor/views/NewThreadView-spec.js +++ b/entry_types/scrolled/package/spec/editor/views/NewThreadView-spec.js @@ -1,5 +1,6 @@ import '@testing-library/jest-dom/extend-expect'; +import {editor} from 'pageflow-scrolled/editor'; import {NewThreadView} from 'editor/views/NewThreadView'; import {factories, useFakeTranslations, renderBackboneView} from 'pageflow/testHelpers'; @@ -11,7 +12,8 @@ describe('NewThreadView', () => { useFakeTranslations({ 'pageflow_scrolled.review.add_comment_placeholder': 'Add a comment...', 'pageflow_scrolled.review.send': 'Send', - 'pageflow_scrolled.review.cancel': 'Cancel' + 'pageflow_scrolled.review.cancel': 'Cancel', + 'pageflow_scrolled.editor.new_thread_view.tabs.newComment': 'New topic' }); it('renders a new thread form for the pending subject', () => { @@ -26,12 +28,31 @@ describe('NewThreadView', () => { anchor: {path: [0, 0], offset: 0}, focus: {path: [0, 0], offset: 5} }, - editor: {} + editor }); const {getByPlaceholderText} = renderBackboneView(view); - view.onShow(); expect(getByPlaceholderText('Add a comment...')).toBeInTheDocument(); }); + + it('renders a New topic tab label above the form', () => { + const entry = createEntry({}); + entry.reviewSession = factories.reviewSession(); + + const view = new NewThreadView({ + entry, + subjectType: 'ContentElement', + subjectId: 10, + subjectRange: { + anchor: {path: [0, 0], offset: 0}, + focus: {path: [0, 0], offset: 5} + }, + editor + }); + + const {getByRole} = renderBackboneView(view); + + expect(getByRole('tab', {name: 'New topic'})).toBeInTheDocument(); + }); }); diff --git a/entry_types/scrolled/package/src/editor/controllers/PreviewMessageController.js b/entry_types/scrolled/package/src/editor/controllers/PreviewMessageController.js index 619c8c29f3..5a482e3106 100644 --- a/entry_types/scrolled/package/src/editor/controllers/PreviewMessageController.js +++ b/entry_types/scrolled/package/src/editor/controllers/PreviewMessageController.js @@ -121,6 +121,13 @@ export const PreviewMessageController = Object.extend({ }) }); + this.listenTo(this.entry, 'selectNewThread', payload => { + postMessage({ + type: 'SELECT', + payload: {type: 'newThread', ...payload} + }) + }); + this.listenTo(this.entry, 'selectWidget', widget => { postMessage({ type: 'SELECT', diff --git a/entry_types/scrolled/package/src/editor/views/CommentsView.js b/entry_types/scrolled/package/src/editor/views/CommentsView.js index 6746999931..03f986b9c6 100644 --- a/entry_types/scrolled/package/src/editor/views/CommentsView.js +++ b/entry_types/scrolled/package/src/editor/views/CommentsView.js @@ -2,7 +2,7 @@ import I18n from 'i18n-js'; import Marionette from 'backbone.marionette'; import {editor} from 'pageflow/editor'; -import {TabsView} from 'pageflow/ui'; +import {cssModulesUtils, TabsView} from 'pageflow/ui'; import {EntryCommentsView} from './EntryCommentsView'; import {ContentElementCommentsView} from './ContentElementCommentsView'; @@ -14,6 +14,7 @@ export const CommentsView = Marionette.ItemView.extend({ template: () => ` ${I18n.t('pageflow.editor.templates.back_button_decorator.outline')} +
`, @@ -22,11 +23,20 @@ export const CommentsView = Marionette.ItemView.extend({ }, events: { - 'click a.back': 'goBack' + 'click a.back': 'goBack', + ...cssModulesUtils.events(styles, { + 'click newThreadButton': 'startNewThread' + }) + }, + + initialize: function() { + this.listenTo(this.options.entry, + 'change:selectedContentElementCommentsId', + this._updateNewThreadButton); }, onRender: function() { - const {entry, defaultTab} = this.options; + const {entry, defaultTab, editor: editorApi} = this.options; const tabsView = new TabsView({ i18n: 'pageflow_scrolled.editor.comments_view.tabs', @@ -34,14 +44,34 @@ export const CommentsView = Marionette.ItemView.extend({ }); tabsView.tab('comments', () => - new EntryCommentsView({entry, editor: this.options.editor})); + new EntryCommentsView({entry, editor: editorApi})); tabsView.tab('selection', () => - new ContentElementCommentsView({entry, editor: this.options.editor})); + new ContentElementCommentsView({entry, editor: editorApi})); this.appendSubview(tabsView, {to: this.ui.tabs}); + this._updateNewThreadButton(); + }, + + startNewThread: function() { + const {entry} = this.options; + const id = entry.get('selectedContentElementCommentsId'); + if (!id) return; + + const contentElement = entry.contentElements.get(id); + entry.trigger('selectNewThread', { + id: contentElement.get('permaId'), + subjectType: 'ContentElement', + range: contentElement.transientState.get('newCommentThreadSubjectRange') + }); }, goBack: function() { editor.navigate('/', {trigger: true}); + }, + + _updateNewThreadButton: function() { + const enabled = !!this.options.entry.get('selectedContentElementCommentsId'); + this.$(cssModulesUtils.selector(styles, 'newThreadButton')) + .prop('disabled', !enabled); } }); 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 b96262d27a..68f370eb21 100644 --- a/entry_types/scrolled/package/src/editor/views/CommentsView.module.css +++ b/entry_types/scrolled/package/src/editor/views/CommentsView.module.css @@ -7,6 +7,10 @@ * color sideways to hide highlighted threads' accent halo as they * scroll past underneath. */ +.root > div { + margin: space(2.5) 0; +} + .root :global(.tabs_view-scroller) { position: sticky; top: -10px; @@ -14,3 +18,8 @@ background-color: var(--ui-surface-color); box-shadow: 0 0 0 space(1) var(--ui-surface-color); } + +.newThreadButton { + composes: secondaryAddButton from './buttons.module.css'; + float: right; +} diff --git a/entry_types/scrolled/package/src/editor/views/NewThreadView.js b/entry_types/scrolled/package/src/editor/views/NewThreadView.js index 2282723814..e694b2f6e4 100644 --- a/entry_types/scrolled/package/src/editor/views/NewThreadView.js +++ b/entry_types/scrolled/package/src/editor/views/NewThreadView.js @@ -1,14 +1,38 @@ import React from 'react'; +import Marionette from 'backbone.marionette'; +import {TabsView} from 'pageflow/ui'; import {NewThreadForm} from 'pageflow-scrolled/review'; import {ReviewView} from './ReviewView'; -export const NewThreadView = ReviewView.extend({ - renderContent() { - const entry = this.options.entry; - const {subjectType, subjectId, subjectRange} = this.options; +import styles from './NewThreadView.module.css'; + +export const NewThreadView = Marionette.View.extend({ + className: `new_thread_view ${styles.root}`, + + initialize: function() { + const {entry, subjectType, subjectId, subjectRange} = this.options; + + this.tabsView = new TabsView({ + i18n: 'pageflow_scrolled.editor.new_thread_view.tabs' + }); + + this.tabsView.tab('newComment', () => new NewThreadFormView({ + entry, subjectType, subjectId, subjectRange + })); + }, + render: function() { + this.$el.empty(); + this.appendSubview(this.tabsView); + return this; + } +}); + +const NewThreadFormView = ReviewView.extend({ + renderContent() { + const {entry, subjectType, subjectId, subjectRange} = this.options; const leave = () => entry.trigger('resetSelection'); return ( diff --git a/entry_types/scrolled/package/src/editor/views/NewThreadView.module.css b/entry_types/scrolled/package/src/editor/views/NewThreadView.module.css new file mode 100644 index 0000000000..00a1b52ddc --- /dev/null +++ b/entry_types/scrolled/package/src/editor/views/NewThreadView.module.css @@ -0,0 +1,3 @@ +.root > div { + margin: space(2.5) 0; +} diff --git a/entry_types/scrolled/package/src/editor/views/buttons.module.css b/entry_types/scrolled/package/src/editor/views/buttons.module.css index f94e067ff7..2a995ace47 100644 --- a/entry_types/scrolled/package/src/editor/views/buttons.module.css +++ b/entry_types/scrolled/package/src/editor/views/buttons.module.css @@ -18,6 +18,11 @@ composes: plusCircled from './icons.module.css'; } +.secondaryAddButton { + composes: secondaryIconButton; + composes: plus from './icons.module.css'; +} + .cancelButton { composes: secondaryIconButton; composes: cancel from './icons.module.css'; diff --git a/entry_types/scrolled/package/src/editor/views/icons.module.css b/entry_types/scrolled/package/src/editor/views/icons.module.css index a09491e27c..8f068895f3 100644 --- a/entry_types/scrolled/package/src/editor/views/icons.module.css +++ b/entry_types/scrolled/package/src/editor/views/icons.module.css @@ -10,6 +10,7 @@ .check, .drag, .helpCircled, +.plus, .plusCircled, .rightBold, .rightOpen, @@ -60,6 +61,10 @@ content: "\e704"; } +.plus::before { + content: "\2b"; +} + .plusCircled::before { content: "\2795"; } From 3566be70c50abdc8e698ae7a72c2d513be298ed5 Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Fri, 8 May 2026 10:19:21 +0200 Subject: [PATCH 08/13] Make ContentElementEditorState aware of newThread selection --- .../inEditorPreview-spec.js | 6 +++++- .../frontend/inlineEditing/ContentElementDecorator.js | 2 +- .../inlineEditing/ContentElementEditorStateProvider.js | 10 ++++++++-- .../src/frontend/useContentElementEditorState.js | 3 ++- 4 files changed, 16 insertions(+), 5 deletions(-) diff --git a/entry_types/scrolled/package/spec/frontend/useContentElementEditorState/inEditorPreview-spec.js b/entry_types/scrolled/package/spec/frontend/useContentElementEditorState/inEditorPreview-spec.js index ea8706ce9c..d959146e21 100644 --- a/entry_types/scrolled/package/spec/frontend/useContentElementEditorState/inEditorPreview-spec.js +++ b/entry_types/scrolled/package/spec/frontend/useContentElementEditorState/inEditorPreview-spec.js @@ -57,7 +57,7 @@ describe('useContentElementEditorState in editor preview', () => { }); renderInEntry(, { - seed: {contentElements: [{id: 7, typeName: 'test'}]} + seed: {contentElements: [{id: 7, permaId: 70, typeName: 'test'}]} }); expect(captured.type).toBeNull(); @@ -69,6 +69,10 @@ describe('useContentElementEditorState in editor preview', () => { act(() => setSelectionRef({type: 'contentElementComments', id: 7})); expect(captured.type).toBe('contentElementComments'); expect(captured.isSelected).toBe(true); + + act(() => setSelectionRef({type: 'newThread', id: 70})); + expect(captured.type).toBe('newThread'); + expect(captured.isSelected).toBe(true); }); it('returns true for isEditable', () => { diff --git a/entry_types/scrolled/package/src/frontend/inlineEditing/ContentElementDecorator.js b/entry_types/scrolled/package/src/frontend/inlineEditing/ContentElementDecorator.js index f20e290c4b..fec278b6a0 100644 --- a/entry_types/scrolled/package/src/frontend/inlineEditing/ContentElementDecorator.js +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/ContentElementDecorator.js @@ -22,7 +22,7 @@ import {ContentElementEditorStateProvider} from './ContentElementEditorStateProv export function ContentElementDecorator(props) { return (
- + {renderMarginIndicators(props)} diff --git a/entry_types/scrolled/package/src/frontend/inlineEditing/ContentElementEditorStateProvider.js b/entry_types/scrolled/package/src/frontend/inlineEditing/ContentElementEditorStateProvider.js index 5d07cbf385..a235c57cec 100644 --- a/entry_types/scrolled/package/src/frontend/inlineEditing/ContentElementEditorStateProvider.js +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/ContentElementEditorStateProvider.js @@ -5,19 +5,23 @@ import {useEditorSelection} from './EditorState'; import {postUpdateTransientContentElementStateMessage} from './postMessage'; import {useStorylineActivity} from '../storylineActivity'; -export function ContentElementEditorStateProvider({id, children}) { +export function ContentElementEditorStateProvider({id, permaId, children}) { const {isSelected, select, range} = useEditorSelection( useMemo(() => ({id, type: 'contentElement'}), [id]) ); const {isSelected: commentsSelected, select: selectComments} = useEditorSelection( useMemo(() => ({id, type: 'contentElementComments'}), [id]) ); + const {isSelected: newThreadSelected, select: selectNewThread} = useEditorSelection( + useMemo(() => ({id: permaId, type: 'newThread'}), [permaId]) + ); const storylineMode = useStorylineActivity(); const inForeground = storylineMode === 'active'; const type = inForeground ? (isSelected ? 'contentElement' : commentsSelected ? 'contentElementComments' : + newThreadSelected ? 'newThread' : null) : null; @@ -33,11 +37,12 @@ export function ContentElementEditorStateProvider({id, children}) { isEditable: true, select, selectComments, + selectNewThread, isSelected: !!type, type, range, setTransientState - }), [select, selectComments, type, range, setTransientState]); + }), [select, selectComments, selectNewThread, type, range, setTransientState]); return ( @@ -45,6 +50,7 @@ export function ContentElementEditorStateProvider({id, children}) { ) } + function shallowEqual(obj1, obj2) { return Object.keys(obj1).length === Object.keys(obj2).length && Object.keys(obj1).every(key => diff --git a/entry_types/scrolled/package/src/frontend/useContentElementEditorState.js b/entry_types/scrolled/package/src/frontend/useContentElementEditorState.js index b7d80f81bd..704a7a2903 100644 --- a/entry_types/scrolled/package/src/frontend/useContentElementEditorState.js +++ b/entry_types/scrolled/package/src/frontend/useContentElementEditorState.js @@ -4,7 +4,8 @@ export const ContentElementEditorStateContext = createContext({ isSelected: false, isEditable: false, setTransientState() {}, - select() {} + select() {}, + selectNewThread() {} }); /** From 646f9ddd05df3ec4207fb13b815dce5f38e51ac7 Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Fri, 8 May 2026 10:21:25 +0200 Subject: [PATCH 09/13] Select newThread on content element when threads badge clicked without threads --- .../frontend/features/selectedMessage-spec.js | 21 +++++++++++++++++++ .../inlineEditing/ContentElementDecorator.js | 7 +++++-- .../package/src/review/ThreadsBadge.js | 6 +++++- 3 files changed, 31 insertions(+), 3 deletions(-) diff --git a/entry_types/scrolled/package/spec/frontend/features/selectedMessage-spec.js b/entry_types/scrolled/package/spec/frontend/features/selectedMessage-spec.js index 67127c41f3..4892a5d42b 100644 --- a/entry_types/scrolled/package/spec/frontend/features/selectedMessage-spec.js +++ b/entry_types/scrolled/package/spec/frontend/features/selectedMessage-spec.js @@ -187,6 +187,27 @@ describe('SELECTED message', () => { }, expect.anything()); }); + it('is posted with type newThread when comment badge is clicked and no threads exist', () => { + features.enable('frontend', ['commenting']); + + const {getByRole, getByTestId} = renderEntry({ + seed: { + contentElements: [{id: 1, permaId: 10, typeName: 'withTestId', configuration: {testId: 5}}] + } + }); + + // Select the content element so the icon-mode badge becomes visible + // even without threads. + fireEvent.click(getByTestId('contentElement-5')); + + fireEvent.click(getByRole('status')); + + expect(window.parent.postMessage).toHaveBeenCalledWith({ + type: 'SELECTED', + payload: {type: 'newThread', id: 10} + }, expect.anything()); + }); + it('is posted with type contentElementComments when comment badge is clicked', async () => { features.enable('frontend', ['commenting']); diff --git a/entry_types/scrolled/package/src/frontend/inlineEditing/ContentElementDecorator.js b/entry_types/scrolled/package/src/frontend/inlineEditing/ContentElementDecorator.js index fec278b6a0..b9d38f23de 100644 --- a/entry_types/scrolled/package/src/frontend/inlineEditing/ContentElementDecorator.js +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/ContentElementDecorator.js @@ -50,7 +50,8 @@ function OptionalSelectionRect(props) { } function DefaultSelectionRect(props) { - const {isSelected, type, select, selectComments} = useContentElementEditorState(); + const {isSelected, type, select, selectComments, selectNewThread} = + useContentElementEditorState(); const commentsSelected = type === 'contentElementComments'; const {t} = useI18n({locale: 'ui'}); @@ -70,7 +71,9 @@ function DefaultSelectionRect(props) { selectComments()} + onClick={threads => threads.length === 0 ? + selectNewThread() : + selectComments()} onSelectThread={threadId => selectComments({ type: 'contentElementComments', id: props.id, diff --git a/entry_types/scrolled/package/src/review/ThreadsBadge.js b/entry_types/scrolled/package/src/review/ThreadsBadge.js index 8ece23ecad..2b7f255adb 100644 --- a/entry_types/scrolled/package/src/review/ThreadsBadge.js +++ b/entry_types/scrolled/package/src/review/ThreadsBadge.js @@ -8,13 +8,17 @@ export function ThreadsBadge({subjectType, subjectId, subjectRange, onClick, onS const threads = useCommentThreads({subjectType, subjectId, subjectRange}, {resolved: false}); const ref = useRef(); + const handleClick = useCallback(() => { + if (onClick) onClick(threads); + }, [onClick, threads]); + return ( <> {threads.length > 0 && } - + ); } From 9453efe613196ecfaaa388075a9c46e06ad7413a Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Fri, 8 May 2026 10:31:37 +0200 Subject: [PATCH 10/13] Inline back button into NewThreadView; back navigates to comments selection tab --- entry_types/scrolled/config/locales/de.yml | 1 + entry_types/scrolled/config/locales/en.yml | 1 + .../spec/editor/views/NewThreadView-spec.js | 59 +++++++++++++------ .../editor/controllers/SideBarController.js | 15 ++--- .../package/src/editor/views/NewThreadView.js | 31 +++++++--- 5 files changed, 71 insertions(+), 36 deletions(-) diff --git a/entry_types/scrolled/config/locales/de.yml b/entry_types/scrolled/config/locales/de.yml index abeb3738ed..07ea1e9f0a 100644 --- a/entry_types/scrolled/config/locales/de.yml +++ b/entry_types/scrolled/config/locales/de.yml @@ -1592,6 +1592,7 @@ de: comments: Alle Kommentare selection: Für Auswahl new_thread_view: + back: Kommentare tabs: newComment: Neues Thema help_entries: diff --git a/entry_types/scrolled/config/locales/en.yml b/entry_types/scrolled/config/locales/en.yml index 528892bdcd..94f886b3d3 100644 --- a/entry_types/scrolled/config/locales/en.yml +++ b/entry_types/scrolled/config/locales/en.yml @@ -1575,6 +1575,7 @@ en: comments: All comments selection: For selection new_thread_view: + back: Comments tabs: newComment: New topic help_entries: diff --git a/entry_types/scrolled/package/spec/editor/views/NewThreadView-spec.js b/entry_types/scrolled/package/spec/editor/views/NewThreadView-spec.js index f23a128946..2c5bf03d03 100644 --- a/entry_types/scrolled/package/spec/editor/views/NewThreadView-spec.js +++ b/entry_types/scrolled/package/spec/editor/views/NewThreadView-spec.js @@ -1,10 +1,13 @@ import '@testing-library/jest-dom/extend-expect'; import {editor} from 'pageflow-scrolled/editor'; + + import {NewThreadView} from 'editor/views/NewThreadView'; import {factories, useFakeTranslations, renderBackboneView} from 'pageflow/testHelpers'; import {useEditorGlobals} from 'support'; +import {fireEvent} from '@testing-library/dom'; describe('NewThreadView', () => { const {createEntry} = useEditorGlobals(); @@ -13,14 +16,12 @@ describe('NewThreadView', () => { 'pageflow_scrolled.review.add_comment_placeholder': 'Add a comment...', 'pageflow_scrolled.review.send': 'Send', 'pageflow_scrolled.review.cancel': 'Cancel', - 'pageflow_scrolled.editor.new_thread_view.tabs.newComment': 'New topic' + 'pageflow_scrolled.editor.new_thread_view.tabs.newComment': 'New topic', + 'pageflow_scrolled.editor.new_thread_view.back': 'Comments' }); - it('renders a new thread form for the pending subject', () => { - const entry = createEntry({}); - entry.reviewSession = factories.reviewSession(); - - const view = new NewThreadView({ + function buildView(entry) { + return new NewThreadView({ entry, subjectType: 'ContentElement', subjectId: 10, @@ -30,8 +31,13 @@ describe('NewThreadView', () => { }, editor }); + } + + it('renders a new thread form for the pending subject', () => { + const entry = createEntry({}); + entry.reviewSession = factories.reviewSession(); - const {getByPlaceholderText} = renderBackboneView(view); + const {getByPlaceholderText} = renderBackboneView(buildView(entry)); expect(getByPlaceholderText('Add a comment...')).toBeInTheDocument(); }); @@ -40,19 +46,34 @@ describe('NewThreadView', () => { const entry = createEntry({}); entry.reviewSession = factories.reviewSession(); - const view = new NewThreadView({ - entry, - subjectType: 'ContentElement', - subjectId: 10, - subjectRange: { - anchor: {path: [0, 0], offset: 0}, - focus: {path: [0, 0], offset: 5} - }, - editor - }); - - const {getByRole} = renderBackboneView(view); + const {getByRole} = renderBackboneView(buildView(entry)); expect(getByRole('tab', {name: 'New topic'})).toBeInTheDocument(); }); + + it('renders a back link', () => { + const entry = createEntry({}); + entry.reviewSession = factories.reviewSession(); + + const {getByText} = renderBackboneView(buildView(entry)); + + expect(getByText('Comments')).toBeInTheDocument(); + }); + + it('navigates to the comments selection tab when the back link is clicked', () => { + const entry = createEntry({}); + entry.reviewSession = factories.reviewSession(); + + const {getByText} = renderBackboneView(buildView(entry)); + + const navigate = jest.spyOn(editor, 'navigate').mockImplementation(() => {}); + + fireEvent.click(getByText('Comments')); + + expect(navigate).toHaveBeenCalledWith( + '/scrolled/comments?tab=selection', {trigger: true} + ); + + navigate.mockRestore(); + }); }); diff --git a/entry_types/scrolled/package/src/editor/controllers/SideBarController.js b/entry_types/scrolled/package/src/editor/controllers/SideBarController.js index 5d26f8d1d4..66fedbed1f 100644 --- a/entry_types/scrolled/package/src/editor/controllers/SideBarController.js +++ b/entry_types/scrolled/package/src/editor/controllers/SideBarController.js @@ -1,7 +1,6 @@ import Marionette from 'backbone.marionette'; import {editor} from 'pageflow-scrolled/editor'; -import {BackButtonDecoratorView} from 'pageflow/editor'; import {EditChapterView} from '../views/EditChapterView'; import {EditSectionView} from '../views/EditSectionView'; @@ -69,14 +68,12 @@ export const SideBarController = Marionette.Controller.extend({ newThread: function(subjectType, subjectId, payload) { const {subjectRange} = JSON.parse(decodeURIComponent(payload)); - this.region.show(new BackButtonDecoratorView({ - view: new NewThreadView({ - entry: this.entry, - subjectType, - subjectId: parseInt(subjectId, 10), - subjectRange, - editor - }) + this.region.show(new NewThreadView({ + entry: this.entry, + subjectType, + subjectId: parseInt(subjectId, 10), + subjectRange, + editor })); } }); diff --git a/entry_types/scrolled/package/src/editor/views/NewThreadView.js b/entry_types/scrolled/package/src/editor/views/NewThreadView.js index e694b2f6e4..3b79ef7897 100644 --- a/entry_types/scrolled/package/src/editor/views/NewThreadView.js +++ b/entry_types/scrolled/package/src/editor/views/NewThreadView.js @@ -1,6 +1,8 @@ import React from 'react'; +import I18n from 'i18n-js'; import Marionette from 'backbone.marionette'; +import {editor} from 'pageflow/editor'; import {TabsView} from 'pageflow/ui'; import {NewThreadForm} from 'pageflow-scrolled/review'; @@ -8,25 +10,38 @@ import {ReviewView} from './ReviewView'; import styles from './NewThreadView.module.css'; -export const NewThreadView = Marionette.View.extend({ +export const NewThreadView = Marionette.ItemView.extend({ className: `new_thread_view ${styles.root}`, - initialize: function() { + template: () => ` + ${I18n.t('pageflow_scrolled.editor.new_thread_view.back')} +
+ `, + + ui: { + tabs: '.tabs' + }, + + events: { + 'click a.back': 'goBack' + }, + + onRender: function() { const {entry, subjectType, subjectId, subjectRange} = this.options; - this.tabsView = new TabsView({ + const tabsView = new TabsView({ i18n: 'pageflow_scrolled.editor.new_thread_view.tabs' }); - this.tabsView.tab('newComment', () => new NewThreadFormView({ + tabsView.tab('newComment', () => new NewThreadFormView({ entry, subjectType, subjectId, subjectRange })); + + this.appendSubview(tabsView, {to: this.ui.tabs}); }, - render: function() { - this.$el.empty(); - this.appendSubview(this.tabsView); - return this; + goBack: function() { + editor.navigate('/scrolled/comments?tab=selection', {trigger: true}); } }); From c611ccfd293c606d4426a39782d9bb385269e669 Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Fri, 8 May 2026 10:56:33 +0200 Subject: [PATCH 11/13] Show blank slate in empty ThreadList when new form is suppressed --- entry_types/scrolled/config/locales/de.yml | 1 + entry_types/scrolled/config/locales/en.yml | 1 + .../views/ContentElementCommentsView-spec.js | 29 ++++++++++- .../package/spec/review/ThreadList-spec.js | 49 ++++++++++++++++++- .../views/ContentElementCommentsView.js | 2 + .../src/editor/views/EntryCommentsView.js | 1 + .../scrolled/package/src/review/ThreadList.js | 7 +++ .../package/src/review/ThreadList.module.css | 7 +++ 8 files changed, 95 insertions(+), 2 deletions(-) diff --git a/entry_types/scrolled/config/locales/de.yml b/entry_types/scrolled/config/locales/de.yml index 07ea1e9f0a..e52ba7b22d 100644 --- a/entry_types/scrolled/config/locales/de.yml +++ b/entry_types/scrolled/config/locales/de.yml @@ -1943,3 +1943,4 @@ de: reply_count: one: 1 Antwort other: '%{count} Antworten' + no_threads_yet: Noch keine Kommentare diff --git a/entry_types/scrolled/config/locales/en.yml b/entry_types/scrolled/config/locales/en.yml index 94f886b3d3..a849f87591 100644 --- a/entry_types/scrolled/config/locales/en.yml +++ b/entry_types/scrolled/config/locales/en.yml @@ -1772,3 +1772,4 @@ en: reply_count: one: 1 reply other: '%{count} replies' + no_threads_yet: No comments yet diff --git a/entry_types/scrolled/package/spec/editor/views/ContentElementCommentsView-spec.js b/entry_types/scrolled/package/spec/editor/views/ContentElementCommentsView-spec.js index b7c9f8da7c..17edc0ed74 100644 --- a/entry_types/scrolled/package/spec/editor/views/ContentElementCommentsView-spec.js +++ b/entry_types/scrolled/package/spec/editor/views/ContentElementCommentsView-spec.js @@ -20,7 +20,8 @@ describe('ContentElementCommentsView', () => { useFakeTranslations({ 'pageflow_scrolled.review.add_comment_placeholder': 'Add a comment...', 'pageflow_scrolled.review.new_topic': 'New topic', - 'pageflow_scrolled.review.send': 'Send' + 'pageflow_scrolled.review.send': 'Send', + 'pageflow_scrolled.review.no_threads_yet': 'No comments yet' }); it('displays threads of selected content element from session state', () => { @@ -309,6 +310,32 @@ describe('ContentElementCommentsView', () => { expect(order).toEqual([...order].sort((a, b) => a - b)); }); + it('does not render the new thread form when selected element has no threads', () => { + const entry = createEntry({ + contentElements: [{id: 1, permaId: 10, typeName: 'textBlock'}] + }); + entry.set('selectedContentElementCommentsId', 1); + entry.reviewSession = factories.reviewSession(); + + const view = new ContentElementCommentsView({entry, editor}); + const {queryByPlaceholderText} = renderBackboneView(view); + + expect(queryByPlaceholderText('Add a comment...')).not.toBeInTheDocument(); + }); + + it('shows blank slate when selected element has no threads', () => { + const entry = createEntry({ + contentElements: [{id: 1, permaId: 10, typeName: 'textBlock'}] + }); + entry.set('selectedContentElementCommentsId', 1); + entry.reviewSession = factories.reviewSession(); + + const view = new ContentElementCommentsView({entry, editor}); + const {getByText} = renderBackboneView(view); + + expect(getByText('No comments yet')).toBeInTheDocument(); + }); + it('does not render a new-topic button', () => { const entry = createEntry({ contentElements: [{id: 1, permaId: 10, typeName: 'fixture'}] diff --git a/entry_types/scrolled/package/spec/review/ThreadList-spec.js b/entry_types/scrolled/package/spec/review/ThreadList-spec.js index 793a251465..7356dfaf08 100644 --- a/entry_types/scrolled/package/spec/review/ThreadList-spec.js +++ b/entry_types/scrolled/package/spec/review/ThreadList-spec.js @@ -20,7 +20,8 @@ describe('ThreadList', () => { 'pageflow_scrolled.review.resolve': 'Mark as resolved', 'pageflow_scrolled.review.unresolve': 'Mark as unresolved', 'pageflow_scrolled.review.resolved_count.one': '1 resolved', - 'pageflow_scrolled.review.resolved_count.other': '%{count} resolved' + 'pageflow_scrolled.review.resolved_count.other': '%{count} resolved', + 'pageflow_scrolled.review.no_threads_yet': 'No comments yet' }); it('displays comments of threads for subject', () => { const {getByText} = renderWithReviewState( @@ -557,6 +558,52 @@ describe('ThreadList', () => { expect(queryByPlaceholderText('Add a comment...')).not.toBeInTheDocument(); }); + it('shows blank slate when no threads exist and showNewForm is false', () => { + const {getByText} = renderWithReviewState( + + ); + + expect(getByText('No comments yet')).toBeInTheDocument(); + }); + + it('does not show blank slate when active threads exist', () => { + const {queryByText} = renderWithReviewState( + , + { + commentThreads: [ + {id: 1, subjectType: 'ContentElement', subjectId: 10, comments: [ + {id: 10, body: 'Looks good', creatorName: 'Bob', creatorId: 2} + ]} + ] + } + ); + + expect(queryByText('No comments yet')).not.toBeInTheDocument(); + }); + + it('does not show blank slate when only resolved threads exist', () => { + const {queryByText} = renderWithReviewState( + , + { + commentThreads: [ + {id: 1, subjectType: 'ContentElement', subjectId: 10, + resolvedAt: '2026-04-09T10:00:00Z', + comments: [{id: 10, body: 'Resolved', creatorName: 'Bob', creatorId: 2}]} + ] + } + ); + + expect(queryByText('No comments yet')).not.toBeInTheDocument(); + }); + + it('does not show blank slate when showNewForm is true', () => { + const {queryByText} = renderWithReviewState( + + ); + + expect(queryByText('No comments yet')).not.toBeInTheDocument(); + }); + it('hides new topic button when hideNewTopicButton is true', () => { const {queryByRole} = renderWithReviewState( , diff --git a/entry_types/scrolled/package/src/editor/views/ContentElementCommentsView.js b/entry_types/scrolled/package/src/editor/views/ContentElementCommentsView.js index 0e2e93d1c5..497a461a8e 100644 --- a/entry_types/scrolled/package/src/editor/views/ContentElementCommentsView.js +++ b/entry_types/scrolled/package/src/editor/views/ContentElementCommentsView.js @@ -43,6 +43,7 @@ export const ContentElementCommentsView = ReviewView.extend({ ); } @@ -54,6 +55,7 @@ export const ContentElementCommentsView = ReviewView.extend({ compareRanges={compareRanges} highlightedThreadId={highlightedThreadId} onThreadClick={onThreadClick} + showNewForm={false} hideNewTopicButton /> ); }, diff --git a/entry_types/scrolled/package/src/editor/views/EntryCommentsView.js b/entry_types/scrolled/package/src/editor/views/EntryCommentsView.js index eb54f7cf10..795a68d266 100644 --- a/entry_types/scrolled/package/src/editor/views/EntryCommentsView.js +++ b/entry_types/scrolled/package/src/editor/views/EntryCommentsView.js @@ -122,6 +122,7 @@ function ContentElementGroup({ highlightedThreadId={groupHighlight} onThreadClick={onThreadClick} restrictInteractionsToHighlighted + showNewForm={false} hideNewTopicButton />
); diff --git a/entry_types/scrolled/package/src/review/ThreadList.js b/entry_types/scrolled/package/src/review/ThreadList.js index 89856cb696..83a1a4f695 100644 --- a/entry_types/scrolled/package/src/review/ThreadList.js +++ b/entry_types/scrolled/package/src/review/ThreadList.js @@ -40,6 +40,8 @@ export function ThreadList({subjectType, subjectId, subjectRange, filter, compar setExpandedThreadId(expandedThreadId === threadId ? null : threadId); } + const noThreads = activeThreads.length === 0 && resolvedThreads.length === 0; + return (
{!showNewForm && !hideNewTopicButton && @@ -61,6 +63,11 @@ export function ThreadList({subjectType, subjectId, subjectRange, filter, compar if (activeThreads.length === 0 && onDismiss) onDismiss(); }} />} + {noThreads && !showNewForm && +

+ {t('pageflow_scrolled.review.no_threads_yet')} +

} + {activeThreads.map(thread => ( Date: Fri, 8 May 2026 11:00:22 +0200 Subject: [PATCH 12/13] Render threads badge active when newThread is selected for content element --- .../features/editorCommentBadges-spec.js | 26 +++++++++++++++++++ .../inlineEditing/ContentElementDecorator.js | 2 +- 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/entry_types/scrolled/package/spec/frontend/features/editorCommentBadges-spec.js b/entry_types/scrolled/package/spec/frontend/features/editorCommentBadges-spec.js index 914fbd536b..44abfd4fab 100644 --- a/entry_types/scrolled/package/spec/frontend/features/editorCommentBadges-spec.js +++ b/entry_types/scrolled/package/spec/frontend/features/editorCommentBadges-spec.js @@ -5,6 +5,8 @@ import {features} from 'pageflow/frontend'; import {useInlineEditingPageObjects, renderEntry} from 'support/pageObjects'; import {fakeParentWindow} from 'support'; +import badgeStyles from 'review/Badge.module.css'; + describe('editor comment badges', () => { useInlineEditingPageObjects(); @@ -62,4 +64,28 @@ describe('editor comment badges', () => { expect(getByRole('status')).not.toHaveTextContent(/\d/); }); }); + + it('renders badge in active mode when newThread is selected on the element', () => { + const {getByRole} = renderEntry({ + seed: { + contentElements: [{ + typeName: 'withTestId', + permaId: 10, + configuration: {testId: 5} + }] + } + }); + + act(() => { + window.dispatchEvent(new MessageEvent('message', { + data: { + type: 'SELECT', + payload: {type: 'newThread', id: 10} + }, + origin: window.location.origin + })); + }); + + expect(getByRole('status')).toHaveClass(badgeStyles.active); + }); }); diff --git a/entry_types/scrolled/package/src/frontend/inlineEditing/ContentElementDecorator.js b/entry_types/scrolled/package/src/frontend/inlineEditing/ContentElementDecorator.js index b9d38f23de..b7eebc7e05 100644 --- a/entry_types/scrolled/package/src/frontend/inlineEditing/ContentElementDecorator.js +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/ContentElementDecorator.js @@ -52,7 +52,7 @@ function OptionalSelectionRect(props) { function DefaultSelectionRect(props) { const {isSelected, type, select, selectComments, selectNewThread} = useContentElementEditorState(); - const commentsSelected = type === 'contentElementComments'; + const commentsSelected = type === 'contentElementComments' || type === 'newThread'; const {t} = useI18n({locale: 'ui'}); const [, drag, preview] = useDrag({ From 33afcdbfee937b95581a83b50256c6dff443686f Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Fri, 8 May 2026 11:16:33 +0200 Subject: [PATCH 13/13] De-emphasize resolved threads and count pill --- .../package/src/editor/views/ReviewView.module.css | 5 +++++ entry_types/scrolled/package/src/review/Thread.js | 3 ++- entry_types/scrolled/package/src/review/Thread.module.css | 7 ++++++- .../scrolled/package/src/review/ThreadList.module.css | 6 +++--- 4 files changed, 16 insertions(+), 5 deletions(-) diff --git a/entry_types/scrolled/package/src/editor/views/ReviewView.module.css b/entry_types/scrolled/package/src/editor/views/ReviewView.module.css index 10eb4f2d9f..8c51077e3e 100644 --- a/entry_types/scrolled/package/src/editor/views/ReviewView.module.css +++ b/entry_types/scrolled/package/src/editor/views/ReviewView.module.css @@ -3,4 +3,9 @@ padding-top: space(4); --review-thread-box-shadow: none; --review-thread-border: 1px solid var(--ui-on-surface-color-lightest); + --review-resolved-threads-pill-align: flex-end; + --review-resolved-threads-pill-color: var(--ui-on-surface-color); + --review-resolved-threads-pill-background-color: transparent; + --review-resolved-thread-opacity: 0.6; + --review-resolved-thread-background: var(--ui-on-surface-color-lightest); } diff --git a/entry_types/scrolled/package/src/review/Thread.js b/entry_types/scrolled/package/src/review/Thread.js index 4cf6c8915a..c06bd5a68c 100644 --- a/entry_types/scrolled/package/src/review/Thread.js +++ b/entry_types/scrolled/package/src/review/Thread.js @@ -29,7 +29,8 @@ export function Thread({thread, collapsed, onToggle, onResolve, onClick, highlig
diff --git a/entry_types/scrolled/package/src/review/Thread.module.css b/entry_types/scrolled/package/src/review/Thread.module.css index 3f65379c3e..94b4921b08 100644 --- a/entry_types/scrolled/package/src/review/Thread.module.css +++ b/entry_types/scrolled/package/src/review/Thread.module.css @@ -19,11 +19,16 @@ border-color: var(--review-thread-hover-border-color, var(--ui-accent-color-light)); } -.highlighted { +.highlighted:not(.resolved) { --review-thread-border: solid 1px var(--ui-accent-color); --review-thread-box-shadow: 0 0 0 space(1) var(--ui-accent-color-lighter); } +.resolved { + opacity: var(--review-resolved-thread-opacity, 1); + background: var(--review-resolved-thread-background, var(--ui-surface-color)); +} + .chevronButton { position: absolute; top: space(3); diff --git a/entry_types/scrolled/package/src/review/ThreadList.module.css b/entry_types/scrolled/package/src/review/ThreadList.module.css index 013cf81208..68285827b7 100644 --- a/entry_types/scrolled/package/src/review/ThreadList.module.css +++ b/entry_types/scrolled/package/src/review/ThreadList.module.css @@ -33,7 +33,7 @@ } .resolvedPill { - align-self: center; + align-self: var(--review-resolved-threads-pill-align, center); display: flex; align-items: center; gap: space(1); @@ -41,8 +41,8 @@ font-size: space(3); font-weight: 500; white-space: nowrap; - color: var(--ui-on-primary-color); - background: var(--ui-primary-color); + color: var(--review-resolved-threads-pill-color, var(--ui-on-primary-color)); + background: var(--review-resolved-threads-pill-background-color, var(--ui-primary-color)); border: 0; border-radius: rounded(full); padding: space(1) space(1.5) space(1) space(2.5);