From 894d16d5194000db496930062e254c9a7c38e5c3 Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Mon, 15 Jun 2026 13:22:54 +0200 Subject: [PATCH 01/18] Preserve line breaks in comment bodies REDMINE-21261 --- entry_types/scrolled/package/src/review/Comment.module.css | 1 + 1 file changed, 1 insertion(+) diff --git a/entry_types/scrolled/package/src/review/Comment.module.css b/entry_types/scrolled/package/src/review/Comment.module.css index 1d9cb10937..e21f94b58b 100644 --- a/entry_types/scrolled/package/src/review/Comment.module.css +++ b/entry_types/scrolled/package/src/review/Comment.module.css @@ -20,4 +20,5 @@ padding-left: space(8); line-height: 1.4; color: var(--ui-on-surface-color); + white-space: pre-wrap; } From 3395c5c00d7be3257d10aee7d3a3e324fc584693 Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Tue, 16 Jun 2026 11:25:48 +0200 Subject: [PATCH 02/18] Display Commenting section comment badge --- entry_types/scrolled/config/locales/de.yml | 1 + entry_types/scrolled/config/locales/en.yml | 1 + .../features/addCommentMode-spec.js | 15 +++++ .../features/commentingBadges-spec.js | 21 +++++++ .../spec/support/pageObjects/commenting.js | 1 + .../frontend/commenting/SectionDecorator.js | 55 +++++++++++++++++++ .../commenting/SectionDecorator.module.css | 46 ++++++++++++++++ .../src/frontend/commenting/extensions.js | 2 + 8 files changed, 142 insertions(+) create mode 100644 entry_types/scrolled/package/src/frontend/commenting/SectionDecorator.js create mode 100644 entry_types/scrolled/package/src/frontend/commenting/SectionDecorator.module.css diff --git a/entry_types/scrolled/config/locales/de.yml b/entry_types/scrolled/config/locales/de.yml index ba7f550af0..cbb4fdcb55 100644 --- a/entry_types/scrolled/config/locales/de.yml +++ b/entry_types/scrolled/config/locales/de.yml @@ -1925,6 +1925,7 @@ de: add_comment: Kommentar hinzufügen cancel_add_comment: Abbrechen select_content_element: Zum Kommentieren auswählen + select_section: Abschnitt zum Kommentieren auswählen select_text_to_comment: Text zum Kommentieren auswählen new_topic: Neues Thema add_comment_placeholder: Kommentar hinzufügen... diff --git a/entry_types/scrolled/config/locales/en.yml b/entry_types/scrolled/config/locales/en.yml index 00803396de..c3c0781d84 100644 --- a/entry_types/scrolled/config/locales/en.yml +++ b/entry_types/scrolled/config/locales/en.yml @@ -1754,6 +1754,7 @@ en: add_comment: Add comment cancel_add_comment: Cancel select_content_element: Select to comment + select_section: Select section to comment select_text_to_comment: Select text to comment new_topic: New topic add_comment_placeholder: Add a comment... diff --git a/entry_types/scrolled/package/spec/frontend/commenting/features/addCommentMode-spec.js b/entry_types/scrolled/package/spec/frontend/commenting/features/addCommentMode-spec.js index 9be7b6cfe5..339e6df334 100644 --- a/entry_types/scrolled/package/spec/frontend/commenting/features/addCommentMode-spec.js +++ b/entry_types/scrolled/package/spec/frontend/commenting/features/addCommentMode-spec.js @@ -186,4 +186,19 @@ describe('add comment mode', () => { expect(contentElement.hasSelectToCommentButton()).toBe(false); }); + + it('opens new thread form when selecting a section', async () => { + const user = userEvent.setup(); + const entry = renderEntry({ + seed: { + sections: [{id: 1, permaId: 10}], + contentElements: [{typeName: 'withTestId', configuration: {testId: 5}}] + } + }); + + await user.click(entry.getAddCommentButton()); + await user.click(entry.getByRole('button', {name: 'Select section to comment'})); + + expect(entry.getNewThreadInput()).toBeInTheDocument(); + }); }); diff --git a/entry_types/scrolled/package/spec/frontend/commenting/features/commentingBadges-spec.js b/entry_types/scrolled/package/spec/frontend/commenting/features/commentingBadges-spec.js index 1162f3c6c2..e31aaf1ec9 100644 --- a/entry_types/scrolled/package/spec/frontend/commenting/features/commentingBadges-spec.js +++ b/entry_types/scrolled/package/spec/frontend/commenting/features/commentingBadges-spec.js @@ -55,4 +55,25 @@ describe('commenting badges', () => { expect(entry.getByText('Nice work')).toBeInTheDocument(); expect(entry.getByText('Bob')).toBeInTheDocument(); }); + + it('shows thread list when clicking a section badge', () => { + const entry = renderEntry({ + seed: { + sections: [{id: 1, permaId: 10}], + contentElements: [{typeName: 'withTestId', configuration: {testId: 5}}] + }, + commenting: { + currentUser: {id: 42, name: 'Alice'}, + commentThreads: [ + {id: 1, subjectType: 'Section', subjectId: 10, comments: [ + {id: 10, body: 'On the section', creatorName: 'Bob', creatorId: 2} + ]} + ] + } + }); + + fireEvent.click(entry.getAllCommentBadges()[0]); + + expect(entry.getByText('On the section')).toBeInTheDocument(); + }); }); diff --git a/entry_types/scrolled/package/spec/support/pageObjects/commenting.js b/entry_types/scrolled/package/spec/support/pageObjects/commenting.js index 2cf7b58e70..95799971e6 100644 --- a/entry_types/scrolled/package/spec/support/pageObjects/commenting.js +++ b/entry_types/scrolled/package/spec/support/pageObjects/commenting.js @@ -45,6 +45,7 @@ export function useCommentingPageObjects() { 'pageflow_scrolled.review.add_comment': 'Add comment', 'pageflow_scrolled.review.cancel_add_comment': 'Cancel add comment', 'pageflow_scrolled.review.select_content_element': 'Select to comment', + 'pageflow_scrolled.review.select_section': 'Select section to comment', 'pageflow_scrolled.review.add_comment_placeholder': 'Add a comment...' }); diff --git a/entry_types/scrolled/package/src/frontend/commenting/SectionDecorator.js b/entry_types/scrolled/package/src/frontend/commenting/SectionDecorator.js new file mode 100644 index 0000000000..e8a87e55d3 --- /dev/null +++ b/entry_types/scrolled/package/src/frontend/commenting/SectionDecorator.js @@ -0,0 +1,55 @@ +import React from 'react'; +import classNames from 'classnames'; + +import {useCommentThreads} from 'pageflow-scrolled/review'; +import {useI18n} from '../i18n'; +import {useAddCommentMode} from './AddCommentModeProvider'; +import {useSelectedSubject} from './SelectedSubjectProvider'; +import {Popover} from './Popover'; + +import AddCommentIcon from './images/addComment.svg'; +import styles from './SectionDecorator.module.css'; + +export function SectionDecorator({section, children}) { + const {active} = useAddCommentMode(); + const {isSelected} = useSelectedSubject('Section', section.permaId); + const threads = useCommentThreads( + {subjectType: 'Section', subjectId: section.permaId}, + {resolved: false} + ); + const hasThreads = threads.length > 0; + + return ( +
+ {children} +
+ {active ? + : + } +
+
+ ); +} + +function AddCommentButton({permaId}) { + const {t} = useI18n({locale: 'ui'}); + const {deactivate} = useAddCommentMode(); + const {select} = useSelectedSubject('Section', permaId); + + function handleClick() { + select({showNewForm: true}); + deactivate(); + } + + return ( + + ); +} diff --git a/entry_types/scrolled/package/src/frontend/commenting/SectionDecorator.module.css b/entry_types/scrolled/package/src/frontend/commenting/SectionDecorator.module.css new file mode 100644 index 0000000000..53c5fc797a --- /dev/null +++ b/entry_types/scrolled/package/src/frontend/commenting/SectionDecorator.module.css @@ -0,0 +1,46 @@ +.wrapper { + position: relative; +} + +.commentBadge { + box-sizing: border-box; + position: relative; + /* Sit above the section Foreground (z-index 3) so the badge stays + reachable; the section establishes no stacking context of its own. */ + z-index: 4; + display: flex; + align-items: flex-end; + height: 44px; + padding: 0 0 10px 10px; + pointer-events: none; +} + +.commentBadge > * { + pointer-events: auto; +} + +.commentBadge.sticky { + position: sticky; + bottom: 0; +} + +/* Absorb the badge box's height; on the section, not the sticky badge. */ +.wrapper > section { + margin-bottom: -44px; +} + +.addButton { + border: none; + background: none; + padding: 0; + cursor: pointer; +} + +.pill { + composes: pill from './AddCommentOverlay.module.css'; +} + +.addButton:hover .pill, +.addButton:focus .pill { + opacity: 1; +} diff --git a/entry_types/scrolled/package/src/frontend/commenting/extensions.js b/entry_types/scrolled/package/src/frontend/commenting/extensions.js index 829ce591e6..da73844007 100644 --- a/entry_types/scrolled/package/src/frontend/commenting/extensions.js +++ b/entry_types/scrolled/package/src/frontend/commenting/extensions.js @@ -1,10 +1,12 @@ import {EntryDecorator} from './EntryDecorator'; +import {SectionDecorator} from './SectionDecorator'; import {ContentElementDecorator} from './ContentElementDecorator'; import {EditableText} from './EditableText'; export const extensions = { decorators: { Entry: EntryDecorator, + Section: SectionDecorator, ContentElement: ContentElementDecorator }, alternatives: { From 9e16935f1b97b9e101d04474b93a7e29365e3a0d Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Wed, 17 Jun 2026 14:09:51 +0200 Subject: [PATCH 03/18] Fix broken new comment thread on whole content elements Selecting "new thread" from a content element's comment badge navigated to a broken form URL because the selection carried only an id, leaving the subject type unset. Use subject type and perma id consistently as the new-thread selection contract across the editor and inline editing so starting a thread works in every case. REDMINE-21261 --- .../PreviewMessageController-spec.js | 8 ++--- .../spec/editor/views/CommentsView-spec.js | 2 +- .../features/commentHighlights-spec.js | 2 +- .../features/commentSelection-spec.js | 31 ++++++++++++++++++- .../features/commentBadges-spec.js | 2 +- .../features/selectedMessage-spec.js | 2 +- .../useContentElementEditorState-spec.js | 2 +- .../controllers/PreviewMessageController.js | 8 ++--- .../package/src/editor/views/CommentsView.js | 2 +- .../ContentElementEditorStateProvider.js | 5 ++- .../inlineEditing/EditableText/BadgeColumn.js | 2 +- .../inlineEditing/EditableText/Selection.js | 2 +- .../EditableText/useCommenting.js | 5 +-- .../EditableText/useStartNewThread.js | 5 +-- .../src/frontend/inlineEditing/EditorState.js | 4 ++- 15 files changed, 59 insertions(+), 23 deletions(-) diff --git a/entry_types/scrolled/package/spec/editor/controllers/PreviewMessageController-spec.js b/entry_types/scrolled/package/spec/editor/controllers/PreviewMessageController-spec.js index afee2505f4..b12b289198 100644 --- a/entry_types/scrolled/package/spec/editor/controllers/PreviewMessageController-spec.js +++ b/entry_types/scrolled/package/spec/editor/controllers/PreviewMessageController-spec.js @@ -193,7 +193,7 @@ describe('PreviewMessageController', () => { } }); entry.trigger('selectNewThread', { - id: 10, + subjectId: 10, subjectType: 'ContentElement', range }); @@ -201,7 +201,7 @@ describe('PreviewMessageController', () => { type: 'SELECT', payload: { type: 'newThread', - id: 10, + subjectId: 10, subjectType: 'ContentElement', range } @@ -691,9 +691,9 @@ describe('PreviewMessageController', () => { window.postMessage({ type: 'SELECTED', payload: { - id: 100, type: 'newThread', subjectType: 'ContentElement', + subjectId: 100, range: {anchor: {path: [0, 0], offset: 0}, focus: {path: [0, 0], offset: 1}} } }, '*'); @@ -734,8 +734,8 @@ describe('PreviewMessageController', () => { type: 'SELECTED', payload: { type: 'newThread', - id: 10, subjectType: 'ContentElement', + subjectId: 10, range } }, '*'); diff --git a/entry_types/scrolled/package/spec/editor/views/CommentsView-spec.js b/entry_types/scrolled/package/spec/editor/views/CommentsView-spec.js index e15ae778ce..617262398b 100644 --- a/entry_types/scrolled/package/spec/editor/views/CommentsView-spec.js +++ b/entry_types/scrolled/package/spec/editor/views/CommentsView-spec.js @@ -165,7 +165,7 @@ describe('CommentsView', () => { fireEvent.click(getByRole('button', {name: 'New topic'})); expect(listener).toHaveBeenCalledWith({ - id: 10, + subjectId: 10, subjectType: 'ContentElement', range }); diff --git a/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/features/commentHighlights-spec.js b/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/features/commentHighlights-spec.js index bff7c586e6..50bb06fbb0 100644 --- a/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/features/commentHighlights-spec.js +++ b/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/features/commentHighlights-spec.js @@ -64,8 +64,8 @@ describe('inline editing EditableText comment highlights', () => { type: 'SELECT', payload: { type: 'newThread', - id: 10, subjectType: 'ContentElement', + subjectId: 10, range: subjectRange } }, '*'); diff --git a/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/features/commentSelection-spec.js b/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/features/commentSelection-spec.js index 7bc9bf1360..22dae759ab 100644 --- a/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/features/commentSelection-spec.js +++ b/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/features/commentSelection-spec.js @@ -2,9 +2,10 @@ import React from 'react'; import {features} from 'pageflow/frontend'; import {EditableText} from 'frontend'; +import {useStartNewThread} from 'frontend/inlineEditing/EditableText/useStartNewThread'; import {renderEntry, useInlineEditingPageObjects} from 'support/pageObjects/inlineEditing'; -import {act} from '@testing-library/react'; +import {act, fireEvent} from '@testing-library/react'; import '@testing-library/jest-dom/extend-expect'; describe('inline editing EditableText comment selection messages', () => { @@ -53,6 +54,34 @@ describe('inline editing EditableText comment selection messages', () => { }, expect.anything()); }); + it('posts SELECTED newThread with subject and range when starting a thread on a text range', () => { + const range = {anchor: {path: [0, 0], offset: 0}, focus: {path: [0, 0], offset: 5}}; + + function StartNewThreadButton() { + const startNewThread = useStartNewThread({selection: range}); + return ; + } + + const {getByText} = renderEntry({ + contentElement: { + ui: , + typeOptions: {inlineComments: true} + } + }); + + fireEvent.click(getByText('Start thread')); + + expect(window.parent.postMessage).toHaveBeenCalledWith({ + type: 'SELECTED', + payload: { + type: 'newThread', + subjectType: 'ContentElement', + subjectId: 10, + range + } + }, expect.anything()); + }); + it('runs badge click logic and scrolls into view on SELECT_COMMENT_THREAD message', async () => { const scrollIntoView = jest.fn(); Element.prototype.scrollIntoView = scrollIntoView; diff --git a/entry_types/scrolled/package/spec/frontend/inlineEditing/features/commentBadges-spec.js b/entry_types/scrolled/package/spec/frontend/inlineEditing/features/commentBadges-spec.js index 8d45206941..837b0dc57b 100644 --- a/entry_types/scrolled/package/spec/frontend/inlineEditing/features/commentBadges-spec.js +++ b/entry_types/scrolled/package/spec/frontend/inlineEditing/features/commentBadges-spec.js @@ -74,7 +74,7 @@ describe('inline editing comment badges', () => { window.dispatchEvent(new MessageEvent('message', { data: { type: 'SELECT', - payload: {type: 'newThread', id: 10} + payload: {type: 'newThread', subjectType: 'ContentElement', subjectId: 10} }, origin: window.location.origin })); diff --git a/entry_types/scrolled/package/spec/frontend/inlineEditing/features/selectedMessage-spec.js b/entry_types/scrolled/package/spec/frontend/inlineEditing/features/selectedMessage-spec.js index 41f220067f..d6618646c5 100644 --- a/entry_types/scrolled/package/spec/frontend/inlineEditing/features/selectedMessage-spec.js +++ b/entry_types/scrolled/package/spec/frontend/inlineEditing/features/selectedMessage-spec.js @@ -199,7 +199,7 @@ describe('inline editing SELECTED message', () => { expect(window.parent.postMessage).toHaveBeenCalledWith({ type: 'SELECTED', - payload: {type: 'newThread', id: 10} + payload: {type: 'newThread', subjectType: 'ContentElement', subjectId: 10} }, expect.anything()); }); diff --git a/entry_types/scrolled/package/spec/frontend/inlineEditing/features/useContentElementEditorState-spec.js b/entry_types/scrolled/package/spec/frontend/inlineEditing/features/useContentElementEditorState-spec.js index c00ed50241..a7a9ffb29b 100644 --- a/entry_types/scrolled/package/spec/frontend/inlineEditing/features/useContentElementEditorState-spec.js +++ b/entry_types/scrolled/package/spec/frontend/inlineEditing/features/useContentElementEditorState-spec.js @@ -67,7 +67,7 @@ describe('inline editing useContentElementEditorState', () => { expect(captured.type).toBe('contentElementComments'); expect(captured.isSelected).toBe(true); - act(() => setSelectionRef({type: 'newThread', id: 10})); + act(() => setSelectionRef({type: 'newThread', subjectType: 'ContentElement', subjectId: 10})); expect(captured.type).toBe('newThread'); expect(captured.isSelected).toBe(true); }); diff --git a/entry_types/scrolled/package/src/editor/controllers/PreviewMessageController.js b/entry_types/scrolled/package/src/editor/controllers/PreviewMessageController.js index 5a482e3106..8a2865863e 100644 --- a/entry_types/scrolled/package/src/editor/controllers/PreviewMessageController.js +++ b/entry_types/scrolled/package/src/editor/controllers/PreviewMessageController.js @@ -166,7 +166,7 @@ export const PreviewMessageController = Object.extend({ }); } else if (message.data.type === 'SELECTED') { - const {type, id, position} = message.data.payload; + const {type, id, subjectType, subjectId, position} = message.data.payload; this.preservedScrollTarget = null; this.entry.set({ @@ -177,7 +177,7 @@ export const PreviewMessageController = Object.extend({ type === 'contentElement' || type === 'contentElementComments' ? id : type === 'newThread' ? - this.entry.contentElements.findWhere({permaId: id})?.id : + this.entry.contentElements.findWhere({permaId: subjectId})?.id : undefined }); @@ -190,11 +190,11 @@ export const PreviewMessageController = Object.extend({ } } else if (type === 'newThread') { - const {subjectType, range} = message.data.payload; + const {range} = message.data.payload; const payload = encodeURIComponent(JSON.stringify({subjectRange: range})); this.editor.navigate( `/scrolled/comment_threads/new?subjectType=${subjectType}` + - `&subjectId=${id}&payload=${payload}`, + `&subjectId=${subjectId}&payload=${payload}`, {trigger: true} ); } diff --git a/entry_types/scrolled/package/src/editor/views/CommentsView.js b/entry_types/scrolled/package/src/editor/views/CommentsView.js index 03f986b9c6..c914874ffe 100644 --- a/entry_types/scrolled/package/src/editor/views/CommentsView.js +++ b/entry_types/scrolled/package/src/editor/views/CommentsView.js @@ -59,7 +59,7 @@ export const CommentsView = Marionette.ItemView.extend({ const contentElement = entry.contentElements.get(id); entry.trigger('selectNewThread', { - id: contentElement.get('permaId'), + subjectId: contentElement.get('permaId'), subjectType: 'ContentElement', range: contentElement.transientState.get('newCommentThreadSubjectRange') }); diff --git a/entry_types/scrolled/package/src/frontend/inlineEditing/ContentElementEditorStateProvider.js b/entry_types/scrolled/package/src/frontend/inlineEditing/ContentElementEditorStateProvider.js index a235c57cec..970d71a3a4 100644 --- a/entry_types/scrolled/package/src/frontend/inlineEditing/ContentElementEditorStateProvider.js +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/ContentElementEditorStateProvider.js @@ -13,7 +13,10 @@ export function ContentElementEditorStateProvider({id, permaId, children}) { useMemo(() => ({id, type: 'contentElementComments'}), [id]) ); const {isSelected: newThreadSelected, select: selectNewThread} = useEditorSelection( - useMemo(() => ({id: permaId, type: 'newThread'}), [permaId]) + useMemo( + () => ({type: 'newThread', subjectType: 'ContentElement', subjectId: permaId}), + [permaId] + ) ); const storylineMode = useStorylineActivity(); diff --git a/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/BadgeColumn.js b/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/BadgeColumn.js index 33338462c0..217b29e2c8 100644 --- a/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/BadgeColumn.js +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/BadgeColumn.js @@ -39,7 +39,7 @@ function PositionedBadge({editor, highlight, highlights, editorSelection, anchor type: 'contentElementComments', id: contentElementId }); const {isSelected: newThreadActive} = useEditorSelection({ - type: 'newThread', id: contentElementPermaId + type: 'newThread', subjectType: 'ContentElement', subjectId: contentElementPermaId }); const {refs, floatingStyles, hasAnchor} = diff --git a/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/Selection.js b/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/Selection.js index 8cd373ab65..08a398f4d7 100644 --- a/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/Selection.js +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/Selection.js @@ -54,7 +54,7 @@ export function Selection(props) { ); const {isSelected: newThreadActive, range: newThreadRange} = useEditorSelection( useMemo( - () => ({type: 'newThread', id: contentElementPermaId}), + () => ({type: 'newThread', subjectType: 'ContentElement', subjectId: contentElementPermaId}), [contentElementPermaId] ) ); diff --git a/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/useCommenting.js b/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/useCommenting.js index c5458e7911..c9e9064c58 100644 --- a/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/useCommenting.js +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/useCommenting.js @@ -33,7 +33,8 @@ export function useCommenting(editor) { const {anchors, registerAnchor} = useRangeAnchors(); const {isSelected: newThreadActive, range: newThreadRange} = useEditorSelection({ type: 'newThread', - id: contentElementPermaId + subjectType: 'ContentElement', + subjectId: contentElementPermaId }); const highlights = useCommentHighlights( @@ -90,7 +91,7 @@ function HighlightSpan({rangeKey, children}) { type: 'contentElementComments', id: contentElementId }); const {isSelected: newThreadActive} = useEditorSelection({ - type: 'newThread', id: contentElementPermaId + type: 'newThread', subjectType: 'ContentElement', subjectId: contentElementPermaId }); const isSelected = (commentsSelected && commentsSelection?.highlightedThreadId === threadId) || (rangeKey === 'selection' && newThreadActive); diff --git a/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/useStartNewThread.js b/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/useStartNewThread.js index 8abeec0e4a..0f90814ed4 100644 --- a/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/useStartNewThread.js +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/useStartNewThread.js @@ -8,15 +8,16 @@ export function useStartNewThread(editor) { const commentingEnabled = features.isEnabled('commenting') && inlineComments; const {select: selectNewThread} = useEditorSelection({ type: 'newThread', - id: contentElementPermaId + subjectType: 'ContentElement', + subjectId: contentElementPermaId }); if (!commentingEnabled) return null; return () => selectNewThread({ type: 'newThread', - id: contentElementPermaId, subjectType: 'ContentElement', + subjectId: contentElementPermaId, range: editor.selection }); } diff --git a/entry_types/scrolled/package/src/frontend/inlineEditing/EditorState.js b/entry_types/scrolled/package/src/frontend/inlineEditing/EditorState.js index 0a3889aac1..76282d5c99 100644 --- a/entry_types/scrolled/package/src/frontend/inlineEditing/EditorState.js +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/EditorState.js @@ -46,7 +46,9 @@ export function useEditorSelection(options) { const isSelected = !!(selection && options && selection.id === options.id && - selection.type === options.type); + selection.type === options.type && + selection.subjectType === options.subjectType && + selection.subjectId === options.subjectId); return { range: selection?.range, From 00c8f6abf9221f70fe74112f44d5fc9804684ff0 Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Wed, 17 Jun 2026 14:44:00 +0200 Subject: [PATCH 04/18] Support comments on sections in editor sidebar Generalize the comments selection tab so it can display and start threads for any commentable subject, not just content elements. This unlocks section-level commenting alongside the existing content-element comments. REDMINE-21261 --- .../PreviewMessageController-spec.js | 96 ++++++++++-- .../spec/editor/views/CommentsView-spec.js | 32 +++- .../editor/views/EntryCommentsView-spec.js | 8 +- ...-spec.js => SelectionCommentsView-spec.js} | 143 +++++++++++++----- .../controllers/PreviewMessageController.js | 32 +++- .../package/src/editor/views/CommentsView.js | 35 +++-- .../views/ContentElementCommentsView.js | 82 ---------- .../src/editor/views/EntryCommentsView.js | 9 +- .../src/editor/views/SelectionCommentsView.js | 105 +++++++++++++ 9 files changed, 381 insertions(+), 161 deletions(-) rename entry_types/scrolled/package/spec/editor/views/{ContentElementCommentsView-spec.js => SelectionCommentsView-spec.js} (69%) delete mode 100644 entry_types/scrolled/package/src/editor/views/ContentElementCommentsView.js create mode 100644 entry_types/scrolled/package/src/editor/views/SelectionCommentsView.js 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 b12b289198..ba8d3bca42 100644 --- a/entry_types/scrolled/package/spec/editor/controllers/PreviewMessageController-spec.js +++ b/entry_types/scrolled/package/spec/editor/controllers/PreviewMessageController-spec.js @@ -561,6 +561,22 @@ describe('PreviewMessageController', () => { })).resolves.toBe('/scrolled/comments?tab=selection'); }); + it('navigates to comments route with tab=selection on SELECTED sectionComments', () => { + const editor = factories.editorApi(); + const entry = factories.entry(ScrolledEntry, {}, { + entryTypeSeed: normalizeSeed({ + sections: [{id: 5, permaId: 50}] + }) + }); + const iframeWindow = createIframeWindow(); + controller = new PreviewMessageController({entry, iframeWindow, editor}); + + return expect(new Promise(resolve => { + editor.on('navigate', resolve); + window.postMessage({type: 'SELECTED', payload: {id: 5, type: 'sectionComments'}}, '*'); + })).resolves.toBe('/scrolled/comments?tab=selection'); + }); + it('does not navigate on SELECTED contentElementComments while on the comments route', async () => { Backbone.history.fragment = 'scrolled/comments'; @@ -646,22 +662,22 @@ describe('PreviewMessageController', () => { })).resolves.toBeUndefined(); }); - it('sets selectedContentElementCommentsId on SELECTED contentElementComments', () => { + it('sets selectedCommentsSubject on SELECTED contentElementComments', () => { const editor = factories.editorApi(); const entry = factories.entry(ScrolledEntry, {}, {entryTypeSeed: normalizeSeed()}); const iframeWindow = createIframeWindow(); controller = new PreviewMessageController({entry, iframeWindow, editor}); return expect(new Promise(resolve => { - entry.once('change:selectedContentElementCommentsId', (model, value) => resolve(value)); + entry.once('change:selectedCommentsSubject', (model, value) => resolve(value)); window.postMessage({ type: 'SELECTED', payload: {id: 7, type: 'contentElementComments'} }, '*'); - })).resolves.toBe(7); + })).resolves.toEqual({subjectType: 'ContentElement', id: 7}); }); - it('sets selectedContentElementCommentsId on SELECTED contentElement', () => { + it('sets selectedCommentsSubject on SELECTED contentElement', () => { const editor = factories.editorApi(); const entry = factories.entry(ScrolledEntry, {}, { entryTypeSeed: normalizeSeed({contentElements: [{id: 4}]}) @@ -670,15 +686,48 @@ describe('PreviewMessageController', () => { controller = new PreviewMessageController({entry, iframeWindow, editor}); return expect(new Promise(resolve => { - entry.once('change:selectedContentElementCommentsId', (model, value) => resolve(value)); + entry.once('change:selectedCommentsSubject', (model, value) => resolve(value)); window.postMessage({ type: 'SELECTED', payload: {id: 4, type: 'contentElement'} }, '*'); - })).resolves.toBe(4); + })).resolves.toEqual({subjectType: 'ContentElement', id: 4}); + }); + + it('sets selectedCommentsSubject on SELECTED sectionComments', () => { + const editor = factories.editorApi(); + const entry = factories.entry(ScrolledEntry, {}, { + entryTypeSeed: normalizeSeed({sections: [{id: 5, permaId: 50}]}) + }); + const iframeWindow = createIframeWindow(); + controller = new PreviewMessageController({entry, iframeWindow, editor}); + + return expect(new Promise(resolve => { + entry.once('change:selectedCommentsSubject', (model, value) => resolve(value)); + window.postMessage({ + type: 'SELECTED', + payload: {id: 5, type: 'sectionComments'} + }, '*'); + })).resolves.toEqual({subjectType: 'Section', id: 5}); + }); + + ['sectionSettings', 'sectionPaddings', 'sectionTransition'].forEach(type => { + it(`sets selectedCommentsSubject to the section on SELECTED ${type}`, () => { + const editor = factories.editorApi(); + const entry = factories.entry(ScrolledEntry, {}, { + entryTypeSeed: normalizeSeed({sections: [{id: 5, permaId: 50}]}) + }); + const iframeWindow = createIframeWindow(); + controller = new PreviewMessageController({entry, iframeWindow, editor}); + + return expect(new Promise(resolve => { + entry.once('change:selectedCommentsSubject', (model, value) => resolve(value)); + window.postMessage({type: 'SELECTED', payload: {id: 5, type}}, '*'); + })).resolves.toEqual({subjectType: 'Section', id: 5}); + }); }); - it('sets selectedContentElementCommentsId from permaId on SELECTED newThread', () => { + it('sets selectedCommentsSubject from permaId on SELECTED newThread', () => { const editor = factories.editorApi(); const entry = factories.entry(ScrolledEntry, {}, { entryTypeSeed: normalizeSeed({contentElements: [{id: 4, permaId: 100}]}) @@ -687,7 +736,7 @@ describe('PreviewMessageController', () => { controller = new PreviewMessageController({entry, iframeWindow, editor}); return expect(new Promise(resolve => { - entry.once('change:selectedContentElementCommentsId', (model, value) => resolve(value)); + entry.once('change:selectedCommentsSubject', (model, value) => resolve(value)); window.postMessage({ type: 'SELECTED', payload: { @@ -697,22 +746,43 @@ describe('PreviewMessageController', () => { range: {anchor: {path: [0, 0], offset: 0}, focus: {path: [0, 0], offset: 1}} } }, '*'); - })).resolves.toBe(4); + })).resolves.toEqual({subjectType: 'ContentElement', id: 4}); + }); + + it('sets selectedCommentsSubject from permaId on SELECTED newThread for a section', () => { + const editor = factories.editorApi(); + const entry = factories.entry(ScrolledEntry, {}, { + entryTypeSeed: normalizeSeed({sections: [{id: 5, permaId: 50}]}) + }); + const iframeWindow = createIframeWindow(); + controller = new PreviewMessageController({entry, iframeWindow, editor}); + + return expect(new Promise(resolve => { + entry.once('change:selectedCommentsSubject', (model, value) => resolve(value)); + window.postMessage({ + type: 'SELECTED', + payload: { + type: 'newThread', + subjectType: 'Section', + subjectId: 50 + } + }, '*'); + })).resolves.toEqual({subjectType: 'Section', id: 5}); }); - it('clears selectedContentElementCommentsId on SELECTED with non-content-element type', () => { + it('clears selectedCommentsSubject on SELECTED with non-comment type', () => { const editor = factories.editorApi(); const entry = factories.entry(ScrolledEntry, {}, {entryTypeSeed: normalizeSeed()}); - entry.set('selectedContentElementCommentsId', 9); + entry.set('selectedCommentsSubject', {subjectType: 'ContentElement', id: 9}); const iframeWindow = createIframeWindow(); controller = new PreviewMessageController({entry, iframeWindow, editor}); return expect(new Promise(resolve => { - entry.once('change:selectedContentElementCommentsId', (model, value) => resolve(value)); + entry.once('change:selectedCommentsSubject', (model, value) => resolve(value)); window.postMessage({ type: 'SELECTED', - payload: {id: 10, type: 'sectionSettings'} + payload: {id: 10, type: 'widget'} }, '*'); })).resolves.toBeUndefined(); }); 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 617262398b..f61bd3e9f0 100644 --- a/entry_types/scrolled/package/spec/editor/views/CommentsView-spec.js +++ b/entry_types/scrolled/package/spec/editor/views/CommentsView-spec.js @@ -35,7 +35,7 @@ describe('CommentsView', () => { const entry = createEntry({ contentElements: [{id: 1, permaId: 10, typeName: 'textBlock'}] }); - entry.set('selectedContentElementCommentsId', 1); + entry.set('selectedCommentsSubject', {subjectType: 'ContentElement', id: 1}); entry.reviewSession = factories.reviewSession({ commentThreads: [{ id: 1, @@ -128,7 +128,7 @@ describe('CommentsView', () => { it('enables the new-thread button when a content element is selected', () => { const entry = unselectedEntry(createEntry); - entry.set('selectedContentElementCommentsId', 1); + entry.set('selectedCommentsSubject', {subjectType: 'ContentElement', id: 1}); const view = new CommentsView({entry, editor}); const {getByRole} = renderBackboneView(view); @@ -136,7 +136,7 @@ describe('CommentsView', () => { expect(getByRole('button', {name: 'New topic'})).not.toBeDisabled(); }); - it("toggles the new-thread button when entry.selectedContentElementCommentsId changes", () => { + it("toggles the new-thread button when entry.selectedCommentsSubject changes", () => { const entry = unselectedEntry(createEntry); const view = new CommentsView({entry, editor}); @@ -144,14 +144,14 @@ describe('CommentsView', () => { expect(getByRole('button', {name: 'New topic'})).toBeDisabled(); - act(() => { entry.set('selectedContentElementCommentsId', 1); }); + act(() => { entry.set('selectedCommentsSubject', {subjectType: 'ContentElement', id: 1}); }); expect(getByRole('button', {name: 'New topic'})).not.toBeDisabled(); }); it('triggers selectNewThread on entry when the new-thread button is clicked', () => { const entry = unselectedEntry(createEntry); - entry.set('selectedContentElementCommentsId', 1); + entry.set('selectedCommentsSubject', {subjectType: 'ContentElement', id: 1}); const range = {anchor: {path: [0, 0], offset: 0}, focus: {path: [0, 0], offset: 5}}; entry.contentElements.get(1).transientState .set('newCommentThreadSubjectRange', range); @@ -171,6 +171,28 @@ describe('CommentsView', () => { }); }); + it('triggers selectNewThread for a selected section without a range', () => { + const entry = createEntry({ + sections: [{id: 5, permaId: 50}], + contentElements: [{id: 1, permaId: 10, typeName: 'textBlock', sectionId: 5}] + }); + entry.reviewSession = factories.reviewSession(); + entry.set('selectedCommentsSubject', {subjectType: 'Section', id: 5}); + + const view = new CommentsView({entry, editor}); + const {getByRole} = renderBackboneView(view); + + const listener = jest.fn(); + entry.on('selectNewThread', listener); + + fireEvent.click(getByRole('button', {name: 'New topic'})); + + expect(listener).toHaveBeenCalledWith({ + subjectId: 50, + subjectType: 'Section' + }); + }); + it('does not trigger selectNewThread when clicking the button while disabled', () => { const entry = unselectedEntry(createEntry); diff --git a/entry_types/scrolled/package/spec/editor/views/EntryCommentsView-spec.js b/entry_types/scrolled/package/spec/editor/views/EntryCommentsView-spec.js index 6a4dc5237c..a7239f4c93 100644 --- a/entry_types/scrolled/package/spec/editor/views/EntryCommentsView-spec.js +++ b/entry_types/scrolled/package/spec/editor/views/EntryCommentsView-spec.js @@ -186,7 +186,7 @@ describe('EntryCommentsView', () => { {id: 2, permaId: 11, typeName: 'image'} ] }); - entry.set('selectedContentElementCommentsId', 1); + entry.set('selectedCommentsSubject', {subjectType: 'ContentElement', id: 1}); entry.reviewSession = factories.reviewSession({ commentThreads: [ {id: 1, subjectType: 'ContentElement', subjectId: 10, @@ -210,7 +210,7 @@ describe('EntryCommentsView', () => { const entry = createEntry({ contentElements: [{id: 1, permaId: 10, typeName: 'textBlock'}] }); - entry.set('selectedContentElementCommentsId', 1); + entry.set('selectedCommentsSubject', {subjectType: 'ContentElement', id: 1}); entry.contentElements.get(1).transientState .set('commentThreadIdsAtSelection', [2]); entry.set('highlightedThreadId', 2); @@ -230,7 +230,7 @@ describe('EntryCommentsView', () => { expect(getByText('second').closest('[aria-current="true"]')).not.toBeNull(); }); - it('updates highlights when selectedContentElementCommentsId changes', () => { + it('updates highlights when selectedCommentsSubject changes', () => { const entry = createEntry({ contentElements: [ {id: 1, permaId: 10, typeName: 'image'}, @@ -252,7 +252,7 @@ describe('EntryCommentsView', () => { expect(getByText('on one').closest('[aria-current="true"]')).toBeNull(); expect(getByText('on two').closest('[aria-current="true"]')).toBeNull(); - act(() => { entry.set('selectedContentElementCommentsId', 2); }); + act(() => { entry.set('selectedCommentsSubject', {subjectType: 'ContentElement', id: 2}); }); expect(getByText('on one').closest('[aria-current="true"]')).toBeNull(); expect(getByText('on two').closest('[aria-current="true"]')).not.toBeNull(); diff --git a/entry_types/scrolled/package/spec/editor/views/ContentElementCommentsView-spec.js b/entry_types/scrolled/package/spec/editor/views/SelectionCommentsView-spec.js similarity index 69% rename from entry_types/scrolled/package/spec/editor/views/ContentElementCommentsView-spec.js rename to entry_types/scrolled/package/spec/editor/views/SelectionCommentsView-spec.js index 17edc0ed74..45abe31f6a 100644 --- a/entry_types/scrolled/package/spec/editor/views/ContentElementCommentsView-spec.js +++ b/entry_types/scrolled/package/spec/editor/views/SelectionCommentsView-spec.js @@ -2,13 +2,13 @@ import '@testing-library/jest-dom/extend-expect'; import userEvent from '@testing-library/user-event'; import {editor} from 'pageflow-scrolled/editor'; -import {ContentElementCommentsView} from 'editor/views/ContentElementCommentsView'; +import {SelectionCommentsView} from 'editor/views/SelectionCommentsView'; import {factories, useFakeTranslations, renderBackboneView} from 'pageflow/testHelpers'; import {useEditorGlobals} from 'support'; import {act, waitFor} from '@testing-library/react'; -describe('ContentElementCommentsView', () => { +describe('SelectionCommentsView', () => { const {createEntry} = useEditorGlobals(); beforeAll(() => { @@ -28,7 +28,7 @@ describe('ContentElementCommentsView', () => { const entry = createEntry({ contentElements: [{id: 1, permaId: 10, typeName: 'fixture'}] }); - entry.set('selectedContentElementCommentsId', 1); + entry.set('selectedCommentsSubject', {subjectType: 'ContentElement', id: 1}); entry.reviewSession = factories.reviewSession({ commentThreads: [{ id: 1, @@ -38,7 +38,7 @@ describe('ContentElementCommentsView', () => { }] }); - const view = new ContentElementCommentsView({entry, editor}); + const view = new SelectionCommentsView({entry, editor}); const {getByText} = renderBackboneView(view); @@ -49,7 +49,7 @@ describe('ContentElementCommentsView', () => { const entry = createEntry({ contentElements: [{id: 1, permaId: 10, typeName: 'fixture'}] }); - entry.set('selectedContentElementCommentsId', 1); + entry.set('selectedCommentsSubject', {subjectType: 'ContentElement', id: 1}); entry.contentElements.get(1).transientState .set('commentThreadIdsAtSelection', [1]); entry.reviewSession = factories.reviewSession({ @@ -61,7 +61,7 @@ describe('ContentElementCommentsView', () => { ] }); - const view = new ContentElementCommentsView({entry, editor}); + const view = new SelectionCommentsView({entry, editor}); const {getByText, queryByText} = renderBackboneView(view); @@ -73,7 +73,7 @@ describe('ContentElementCommentsView', () => { const entry = createEntry({ contentElements: [{id: 1, permaId: 10, typeName: 'fixture'}] }); - entry.set('selectedContentElementCommentsId', 1); + entry.set('selectedCommentsSubject', {subjectType: 'ContentElement', id: 1}); entry.reviewSession = factories.reviewSession({ commentThreads: [ {id: 1, subjectType: 'ContentElement', subjectId: 10, @@ -83,7 +83,7 @@ describe('ContentElementCommentsView', () => { ] }); - const view = new ContentElementCommentsView({entry, editor}); + const view = new SelectionCommentsView({entry, editor}); const {getByText} = renderBackboneView(view); @@ -95,7 +95,7 @@ describe('ContentElementCommentsView', () => { const entry = createEntry({ contentElements: [{id: 1, permaId: 10, typeName: 'fixture'}] }); - entry.set('selectedContentElementCommentsId', 1); + entry.set('selectedCommentsSubject', {subjectType: 'ContentElement', id: 1}); entry.reviewSession = factories.reviewSession({ commentThreads: [ {id: 1, subjectType: 'ContentElement', subjectId: 10, @@ -105,7 +105,7 @@ describe('ContentElementCommentsView', () => { ] }); - const view = new ContentElementCommentsView({entry, editor}); + const view = new SelectionCommentsView({entry, editor}); const {getByText, queryByText} = renderBackboneView(view); expect(getByText('Out of scope')).toBeInTheDocument(); @@ -121,14 +121,14 @@ describe('ContentElementCommentsView', () => { expect(getByText('In scope')).toBeInTheDocument(); }); - it('updates when selectedContentElementCommentsId changes', async () => { + it('updates when selectedCommentsSubject changes', async () => { const entry = createEntry({ contentElements: [ {id: 1, permaId: 10, typeName: 'fixture'}, {id: 2, permaId: 11, typeName: 'fixture'} ] }); - entry.set('selectedContentElementCommentsId', 1); + entry.set('selectedCommentsSubject', {subjectType: 'ContentElement', id: 1}); entry.reviewSession = factories.reviewSession({ commentThreads: [ {id: 1, subjectType: 'ContentElement', subjectId: 10, @@ -138,13 +138,13 @@ describe('ContentElementCommentsView', () => { ] }); - const view = new ContentElementCommentsView({entry, editor}); + const view = new SelectionCommentsView({entry, editor}); const {getByText, queryByText} = renderBackboneView(view); expect(getByText('On first')).toBeInTheDocument(); act(() => { - entry.set('selectedContentElementCommentsId', 2); + entry.set('selectedCommentsSubject', {subjectType: 'ContentElement', id: 2}); }); await waitFor(() => { @@ -157,7 +157,7 @@ describe('ContentElementCommentsView', () => { const entry = createEntry({ contentElements: [{id: 1, permaId: 10, typeName: 'fixture'}] }); - entry.set('selectedContentElementCommentsId', 1); + entry.set('selectedCommentsSubject', {subjectType: 'ContentElement', id: 1}); entry.contentElements.get(1).transientState .set('commentThreadIdsAtSelection', [1, 2]); entry.set('highlightedThreadId', 2); @@ -170,7 +170,7 @@ describe('ContentElementCommentsView', () => { ] }); - const view = new ContentElementCommentsView({entry, editor}); + const view = new SelectionCommentsView({entry, editor}); const {getByText} = renderBackboneView(view); expect(getByText('second').closest('[aria-current="true"]')).not.toBeNull(); @@ -181,7 +181,7 @@ describe('ContentElementCommentsView', () => { const entry = createEntry({ contentElements: [{id: 1, permaId: 10, typeName: 'fixture'}] }); - entry.set('selectedContentElementCommentsId', 1); + entry.set('selectedCommentsSubject', {subjectType: 'ContentElement', id: 1}); entry.contentElements.get(1).transientState .set('commentThreadIdsAtSelection', [1, 2]); entry.reviewSession = factories.reviewSession({ @@ -193,7 +193,7 @@ describe('ContentElementCommentsView', () => { ] }); - const view = new ContentElementCommentsView({entry, editor}); + const view = new SelectionCommentsView({entry, editor}); const {getByText} = renderBackboneView(view); expect(getByText('first').closest('[aria-current="true"]')).toBeNull(); @@ -209,7 +209,7 @@ describe('ContentElementCommentsView', () => { const entry = createEntry({ contentElements: [{id: 1, permaId: 10, typeName: 'fixture'}] }); - entry.set('selectedContentElementCommentsId', 1); + entry.set('selectedCommentsSubject', {subjectType: 'ContentElement', id: 1}); entry.contentElements.get(1).transientState .set('commentThreadIdsAtSelection', [7]); entry.reviewSession = factories.reviewSession({ @@ -222,7 +222,7 @@ describe('ContentElementCommentsView', () => { const listener = jest.fn(); entry.on('selectCommentThread', listener); - const view = new ContentElementCommentsView({entry, editor}); + const view = new SelectionCommentsView({entry, editor}); const {getByText} = renderBackboneView(view); await user.click(getByText('click me')); @@ -236,7 +236,7 @@ describe('ContentElementCommentsView', () => { const entry = createEntry({ contentElements: [{id: 1, permaId: 10, typeName: 'fixture'}] }); - entry.set('selectedContentElementCommentsId', 1); + entry.set('selectedCommentsSubject', {subjectType: 'ContentElement', id: 1}); entry.set('highlightedThreadId', 7); entry.reviewSession = factories.reviewSession({ commentThreads: [{ @@ -248,7 +248,7 @@ describe('ContentElementCommentsView', () => { const listener = jest.fn(); entry.on('selectCommentThread', listener); - const view = new ContentElementCommentsView({entry, editor}); + const view = new SelectionCommentsView({entry, editor}); const {getByText} = renderBackboneView(view); expect(getByText('click me').closest('[aria-current="true"]')).toBeNull(); @@ -262,7 +262,7 @@ describe('ContentElementCommentsView', () => { const entry = createEntry({ contentElements: [{id: 1, permaId: 10, typeName: 'fixture'}] }); - entry.set('selectedContentElementCommentsId', 1); + entry.set('selectedCommentsSubject', {subjectType: 'ContentElement', id: 1}); entry.reviewSession = factories.reviewSession({ commentThreads: [ {id: 1, subjectType: 'ContentElement', subjectId: 10, @@ -274,7 +274,7 @@ describe('ContentElementCommentsView', () => { ] }); - const view = new ContentElementCommentsView({entry, editor}); + const view = new SelectionCommentsView({entry, editor}); const {getByText} = renderBackboneView(view); const order = ['first', 'second'] @@ -287,7 +287,7 @@ describe('ContentElementCommentsView', () => { const entry = createEntry({ contentElements: [{id: 1, permaId: 10, typeName: 'fixture'}] }); - entry.set('selectedContentElementCommentsId', 1); + entry.set('selectedCommentsSubject', {subjectType: 'ContentElement', id: 1}); entry.contentElements.get(1).transientState .set('commentThreadIdsAtSelection', [1, 2]); entry.reviewSession = factories.reviewSession({ @@ -301,7 +301,7 @@ describe('ContentElementCommentsView', () => { ] }); - const view = new ContentElementCommentsView({entry, editor}); + const view = new SelectionCommentsView({entry, editor}); const {getByText} = renderBackboneView(view); const order = ['first', 'second'] @@ -314,10 +314,10 @@ describe('ContentElementCommentsView', () => { const entry = createEntry({ contentElements: [{id: 1, permaId: 10, typeName: 'textBlock'}] }); - entry.set('selectedContentElementCommentsId', 1); + entry.set('selectedCommentsSubject', {subjectType: 'ContentElement', id: 1}); entry.reviewSession = factories.reviewSession(); - const view = new ContentElementCommentsView({entry, editor}); + const view = new SelectionCommentsView({entry, editor}); const {queryByPlaceholderText} = renderBackboneView(view); expect(queryByPlaceholderText('Add a comment...')).not.toBeInTheDocument(); @@ -327,10 +327,10 @@ describe('ContentElementCommentsView', () => { const entry = createEntry({ contentElements: [{id: 1, permaId: 10, typeName: 'textBlock'}] }); - entry.set('selectedContentElementCommentsId', 1); + entry.set('selectedCommentsSubject', {subjectType: 'ContentElement', id: 1}); entry.reviewSession = factories.reviewSession(); - const view = new ContentElementCommentsView({entry, editor}); + const view = new SelectionCommentsView({entry, editor}); const {getByText} = renderBackboneView(view); expect(getByText('No comments yet')).toBeInTheDocument(); @@ -340,7 +340,7 @@ describe('ContentElementCommentsView', () => { const entry = createEntry({ contentElements: [{id: 1, permaId: 10, typeName: 'fixture'}] }); - entry.set('selectedContentElementCommentsId', 1); + entry.set('selectedCommentsSubject', {subjectType: 'ContentElement', id: 1}); entry.reviewSession = factories.reviewSession({ commentThreads: [{ id: 1, @@ -350,7 +350,7 @@ describe('ContentElementCommentsView', () => { }] }); - const view = new ContentElementCommentsView({entry, editor}); + const view = new SelectionCommentsView({entry, editor}); const {queryByRole} = renderBackboneView(view); expect(queryByRole('button', {name: 'New topic'})).not.toBeInTheDocument(); @@ -360,10 +360,10 @@ describe('ContentElementCommentsView', () => { const entry = createEntry({ contentElements: [{id: 1, permaId: 10, typeName: 'fixture'}] }); - entry.set('selectedContentElementCommentsId', 1); + entry.set('selectedCommentsSubject', {subjectType: 'ContentElement', id: 1}); entry.reviewSession = factories.reviewSession(); - const view = new ContentElementCommentsView({entry, editor}); + const view = new SelectionCommentsView({entry, editor}); const {getByText} = renderBackboneView(view); @@ -380,4 +380,79 @@ describe('ContentElementCommentsView', () => { expect(getByText('New comment')).toBeInTheDocument(); }); }); + + it('displays threads of selected section from session state', () => { + const entry = createEntry({ + sections: [{id: 5, permaId: 50}], + contentElements: [{id: 1, permaId: 10, typeName: 'fixture', sectionId: 5}] + }); + entry.set('selectedCommentsSubject', {subjectType: 'Section', id: 5}); + entry.reviewSession = factories.reviewSession({ + commentThreads: [{ + id: 1, + subjectType: 'Section', + subjectId: 50, + comments: [{id: 100, body: 'On the section', creatorName: 'Alice'}] + }] + }); + + const view = new SelectionCommentsView({entry, editor}); + + const {getByText} = renderBackboneView(view); + + expect(getByText('On the section')).toBeInTheDocument(); + }); + + it('shows all section threads since sections are never scoped', () => { + const entry = createEntry({ + sections: [{id: 5, permaId: 50}], + contentElements: [{id: 1, permaId: 10, typeName: 'fixture', sectionId: 5}] + }); + entry.set('selectedCommentsSubject', {subjectType: 'Section', id: 5}); + entry.reviewSession = factories.reviewSession({ + commentThreads: [ + {id: 1, subjectType: 'Section', subjectId: 50, + comments: [{id: 100, body: 'First', creatorName: 'Alice'}]}, + {id: 2, subjectType: 'Section', subjectId: 50, + comments: [{id: 200, body: 'Second', creatorName: 'Bob'}]} + ] + }); + + const view = new SelectionCommentsView({entry, editor}); + + const {getByText} = renderBackboneView(view); + + expect(getByText('First')).toBeInTheDocument(); + expect(getByText('Second')).toBeInTheDocument(); + }); + + it('updates when selection switches from content element to section', async () => { + const entry = createEntry({ + sections: [{id: 5, permaId: 50}], + contentElements: [{id: 1, permaId: 10, typeName: 'fixture', sectionId: 5}] + }); + entry.set('selectedCommentsSubject', {subjectType: 'ContentElement', id: 1}); + entry.reviewSession = factories.reviewSession({ + commentThreads: [ + {id: 1, subjectType: 'ContentElement', subjectId: 10, + comments: [{id: 100, body: 'On element', creatorName: 'Alice'}]}, + {id: 2, subjectType: 'Section', subjectId: 50, + comments: [{id: 200, body: 'On section', creatorName: 'Bob'}]} + ] + }); + + const view = new SelectionCommentsView({entry, editor}); + const {getByText, queryByText} = renderBackboneView(view); + + expect(getByText('On element')).toBeInTheDocument(); + + act(() => { + entry.set('selectedCommentsSubject', {subjectType: 'Section', id: 5}); + }); + + await waitFor(() => { + expect(getByText('On section')).toBeInTheDocument(); + }); + expect(queryByText('On element')).not.toBeInTheDocument(); + }); }); diff --git a/entry_types/scrolled/package/src/editor/controllers/PreviewMessageController.js b/entry_types/scrolled/package/src/editor/controllers/PreviewMessageController.js index 8a2865863e..5355635100 100644 --- a/entry_types/scrolled/package/src/editor/controllers/PreviewMessageController.js +++ b/entry_types/scrolled/package/src/editor/controllers/PreviewMessageController.js @@ -173,15 +173,11 @@ export const PreviewMessageController = Object.extend({ highlightedThreadId: type === 'contentElementComments' ? message.data.payload.highlightedThreadId : undefined, - selectedContentElementCommentsId: - type === 'contentElement' || type === 'contentElementComments' ? - id : - type === 'newThread' ? - this.entry.contentElements.findWhere({permaId: subjectId})?.id : - undefined + selectedCommentsSubject: + selectedCommentsSubjectFor(this.entry, message.data.payload) }); - if (type === 'contentElementComments') { + if (type === 'contentElementComments' || type === 'sectionComments') { // Stay on the current tab when the user is already on the // comments route — only force the selection tab when arriving // there from elsewhere. @@ -300,3 +296,25 @@ export const PreviewMessageController = Object.extend({ } } }); + +function selectedCommentsSubjectFor(entry, payload) { + const {type, id, subjectType, subjectId} = payload; + + if (type === 'contentElement' || type === 'contentElementComments') { + return {subjectType: 'ContentElement', id}; + } + + if (type === 'sectionComments' || + type === 'sectionSettings' || + type === 'sectionPaddings' || + type === 'sectionTransition') { + return {subjectType: 'Section', id}; + } + + if (type === 'newThread') { + const collection = subjectType === 'Section' ? entry.sections : entry.contentElements; + return {subjectType, id: collection.findWhere({permaId: subjectId})?.id}; + } + + return undefined; +} diff --git a/entry_types/scrolled/package/src/editor/views/CommentsView.js b/entry_types/scrolled/package/src/editor/views/CommentsView.js index c914874ffe..75ce799b48 100644 --- a/entry_types/scrolled/package/src/editor/views/CommentsView.js +++ b/entry_types/scrolled/package/src/editor/views/CommentsView.js @@ -5,7 +5,7 @@ import {editor} from 'pageflow/editor'; import {cssModulesUtils, TabsView} from 'pageflow/ui'; import {EntryCommentsView} from './EntryCommentsView'; -import {ContentElementCommentsView} from './ContentElementCommentsView'; +import {SelectionCommentsView} from './SelectionCommentsView'; import styles from './CommentsView.module.css'; @@ -31,7 +31,7 @@ export const CommentsView = Marionette.ItemView.extend({ initialize: function() { this.listenTo(this.options.entry, - 'change:selectedContentElementCommentsId', + 'change:selectedCommentsSubject', this._updateNewThreadButton); }, @@ -46,7 +46,7 @@ export const CommentsView = Marionette.ItemView.extend({ tabsView.tab('comments', () => new EntryCommentsView({entry, editor: editorApi})); tabsView.tab('selection', () => - new ContentElementCommentsView({entry, editor: editorApi})); + new SelectionCommentsView({entry, editor: editorApi})); this.appendSubview(tabsView, {to: this.ui.tabs}); this._updateNewThreadButton(); @@ -54,15 +54,24 @@ export const CommentsView = Marionette.ItemView.extend({ startNewThread: function() { const {entry} = this.options; - const id = entry.get('selectedContentElementCommentsId'); - if (!id) return; - - const contentElement = entry.contentElements.get(id); - entry.trigger('selectNewThread', { - subjectId: contentElement.get('permaId'), - subjectType: 'ContentElement', - range: contentElement.transientState.get('newCommentThreadSubjectRange') - }); + const subject = entry.get('selectedCommentsSubject'); + if (!subject) return; + + if (subject.subjectType === 'Section') { + const section = entry.sections.get(subject.id); + entry.trigger('selectNewThread', { + subjectId: section.get('permaId'), + subjectType: 'Section' + }); + } + else { + const contentElement = entry.contentElements.get(subject.id); + entry.trigger('selectNewThread', { + subjectId: contentElement.get('permaId'), + subjectType: 'ContentElement', + range: contentElement.transientState.get('newCommentThreadSubjectRange') + }); + } }, goBack: function() { @@ -70,7 +79,7 @@ export const CommentsView = Marionette.ItemView.extend({ }, _updateNewThreadButton: function() { - const enabled = !!this.options.entry.get('selectedContentElementCommentsId'); + const enabled = !!this.options.entry.get('selectedCommentsSubject'); this.$(cssModulesUtils.selector(styles, 'newThreadButton')) .prop('disabled', !enabled); } diff --git a/entry_types/scrolled/package/src/editor/views/ContentElementCommentsView.js b/entry_types/scrolled/package/src/editor/views/ContentElementCommentsView.js deleted file mode 100644 index 497a461a8e..0000000000 --- a/entry_types/scrolled/package/src/editor/views/ContentElementCommentsView.js +++ /dev/null @@ -1,82 +0,0 @@ -import React from 'react'; - -import {ThreadList} from 'pageflow-scrolled/review'; - -import {ReviewView} from './ReviewView'; - -export const ContentElementCommentsView = ReviewView.extend({ - initialize() { - const {entry} = this.options; - - this.listenTo(entry, - 'change:selectedContentElementCommentsId', - this._onSelectedChange); - this.listenTo(entry, - 'change:highlightedThreadId', - () => this.rerender()); - this._observeContentElement(); - }, - - props() { - const {entry, editor} = this.options; - const contentElement = this._contentElement; - const typeName = contentElement?.get('typeName'); - return { - contentElement, - threadIds: contentElement?.transientState.get('commentThreadIdsAtSelection'), - compareRanges: typeName && editor.contentElementTypes.findCompareRanges(typeName), - highlightedThreadId: entry.get('highlightedThreadId'), - onThreadClick: thread => entry.trigger('selectCommentThread', thread.id) - }; - }, - - // Highlight and per-thread click are only wired when the view is - // scoped to a list of thread ids. For unscoped content elements - // there is no anchor for an individual thread in the iframe, so - // highlighting one in the list would have no counterpart in the - // preview. - renderContent({contentElement, threadIds, compareRanges, highlightedThreadId, onThreadClick}) { - if (!contentElement) return null; - - if (threadIds === undefined) { - return ( - - ); - } - - return ( - threadIds.includes(thread.id)} - compareRanges={compareRanges} - highlightedThreadId={highlightedThreadId} - onThreadClick={onThreadClick} - showNewForm={false} - hideNewTopicButton /> - ); - }, - - _onSelectedChange() { - this._observeContentElement(); - this.rerender(); - }, - - _observeContentElement() { - if (this._contentElement) { - this.stopListening(this._contentElement.transientState); - } - - const id = this.options.entry.get('selectedContentElementCommentsId'); - this._contentElement = id ? this.options.entry.contentElements.get(id) : null; - - if (this._contentElement) { - this.listenTo(this._contentElement.transientState, - 'change:commentThreadIdsAtSelection', - () => this.rerender()); - } - } -}); diff --git a/entry_types/scrolled/package/src/editor/views/EntryCommentsView.js b/entry_types/scrolled/package/src/editor/views/EntryCommentsView.js index 795a68d266..e70c502fb1 100644 --- a/entry_types/scrolled/package/src/editor/views/EntryCommentsView.js +++ b/entry_types/scrolled/package/src/editor/views/EntryCommentsView.js @@ -15,7 +15,7 @@ export const EntryCommentsView = ReviewView.extend({ 'change:highlightedThreadId', () => this.rerender()); this.listenTo(entry, - 'change:selectedContentElementCommentsId', + 'change:selectedCommentsSubject', this._onSelectedChange); this._observeSelectedElement(); }, @@ -62,8 +62,11 @@ export const EntryCommentsView = ReviewView.extend({ this.stopListening(this._selectedElement.transientState); } - const id = this.options.entry.get('selectedContentElementCommentsId'); - this._selectedElement = id ? this.options.entry.contentElements.get(id) : null; + const subject = this.options.entry.get('selectedCommentsSubject'); + this._selectedElement = + subject?.subjectType === 'ContentElement' ? + this.options.entry.contentElements.get(subject.id) : + null; if (this._selectedElement) { this.listenTo(this._selectedElement.transientState, diff --git a/entry_types/scrolled/package/src/editor/views/SelectionCommentsView.js b/entry_types/scrolled/package/src/editor/views/SelectionCommentsView.js new file mode 100644 index 0000000000..509d1fb8db --- /dev/null +++ b/entry_types/scrolled/package/src/editor/views/SelectionCommentsView.js @@ -0,0 +1,105 @@ +import React from 'react'; + +import {ThreadList} from 'pageflow-scrolled/review'; + +import {ReviewView} from './ReviewView'; + +export const SelectionCommentsView = ReviewView.extend({ + initialize() { + const {entry} = this.options; + + this.listenTo(entry, + 'change:selectedCommentsSubject', + this._onSelectedChange); + this.listenTo(entry, + 'change:highlightedThreadId', + () => this.rerender()); + this._observeSubject(); + }, + + props() { + const {entry, editor} = this.options; + const subject = entry.get('selectedCommentsSubject'); + const model = this._model; + + if (!subject || !model) { + return {}; + } + + if (subject.subjectType === 'ContentElement') { + const typeName = model.get('typeName'); + return { + subjectType: 'ContentElement', + subjectId: model.get('permaId'), + threadIds: model.transientState.get('commentThreadIdsAtSelection'), + compareRanges: typeName && editor.contentElementTypes.findCompareRanges(typeName), + highlightedThreadId: entry.get('highlightedThreadId'), + onThreadClick: thread => entry.trigger('selectCommentThread', thread.id) + }; + } + + return { + subjectType: subject.subjectType, + subjectId: model.get('permaId') + }; + }, + + // Highlight and per-thread click are only wired when the view is + // scoped to a list of thread ids. For unscoped subjects (whole + // content elements such as images, or whole sections) there is no + // anchor for an individual thread in the iframe, so highlighting one + // in the list would have no counterpart in the preview. + renderContent({subjectType, subjectId, threadIds, compareRanges, highlightedThreadId, onThreadClick}) { + if (!subjectType) return null; + + if (threadIds === undefined) { + return ( + + ); + } + + return ( + threadIds.includes(thread.id)} + compareRanges={compareRanges} + highlightedThreadId={highlightedThreadId} + onThreadClick={onThreadClick} + showNewForm={false} + hideNewTopicButton /> + ); + }, + + _onSelectedChange() { + this._observeSubject(); + this.rerender(); + }, + + _observeSubject() { + if (this._model?.transientState) { + this.stopListening(this._model.transientState); + } + + this._model = this._resolveModel(); + + if (this._model?.transientState) { + this.listenTo(this._model.transientState, + 'change:commentThreadIdsAtSelection', + () => this.rerender()); + } + }, + + _resolveModel() { + const {entry} = this.options; + const subject = entry.get('selectedCommentsSubject'); + if (!subject) return null; + + return subject.subjectType === 'Section' ? + entry.sections.get(subject.id) : + entry.contentElements.get(subject.id); + } +}); From 1e13fbaca8b1e1010c53a2ce910057ad5cdebba3 Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Tue, 16 Jun 2026 11:25:48 +0200 Subject: [PATCH 05/18] Emit section comments from inline editing decorator Wire the inline editing SectionDecorator to render a section comment badge and emit sectionComments and newThread selections, so a reviewer can open or start comment threads on a whole section just as they can on a content element. REDMINE-21261 --- ...js => contentElementCommentBadges-spec.js} | 2 +- .../features/forcedSectionPadding-spec.js | 45 ++++ .../features/sectionCommentBadges-spec.js | 204 ++++++++++++++++++ .../features/selectedMessage-spec.js | 102 +++++++++ .../inlineEditing/SectionDecorator.js | 55 ++++- .../inlineEditing/SectionDecorator.module.css | 47 ++++ 6 files changed, 449 insertions(+), 6 deletions(-) rename entry_types/scrolled/package/spec/frontend/inlineEditing/features/{commentBadges-spec.js => contentElementCommentBadges-spec.js} (97%) create mode 100644 entry_types/scrolled/package/spec/frontend/inlineEditing/features/sectionCommentBadges-spec.js diff --git a/entry_types/scrolled/package/spec/frontend/inlineEditing/features/commentBadges-spec.js b/entry_types/scrolled/package/spec/frontend/inlineEditing/features/contentElementCommentBadges-spec.js similarity index 97% rename from entry_types/scrolled/package/spec/frontend/inlineEditing/features/commentBadges-spec.js rename to entry_types/scrolled/package/spec/frontend/inlineEditing/features/contentElementCommentBadges-spec.js index 837b0dc57b..06580f9fb1 100644 --- a/entry_types/scrolled/package/spec/frontend/inlineEditing/features/commentBadges-spec.js +++ b/entry_types/scrolled/package/spec/frontend/inlineEditing/features/contentElementCommentBadges-spec.js @@ -6,7 +6,7 @@ import {useInlineEditingPageObjects, renderEntry} from 'support/pageObjects/inli import badgeStyles from 'review/Badge.module.css'; -describe('inline editing comment badges', () => { +describe('inline editing content element comment badges', () => { useInlineEditingPageObjects(); useFakeFeatures('frontend', ['commenting']); diff --git a/entry_types/scrolled/package/spec/frontend/inlineEditing/features/forcedSectionPadding-spec.js b/entry_types/scrolled/package/spec/frontend/inlineEditing/features/forcedSectionPadding-spec.js index b8fda7bb63..af31cc6674 100644 --- a/entry_types/scrolled/package/spec/frontend/inlineEditing/features/forcedSectionPadding-spec.js +++ b/entry_types/scrolled/package/spec/frontend/inlineEditing/features/forcedSectionPadding-spec.js @@ -1,12 +1,26 @@ import {renderEntry, useInlineEditingPageObjects} from 'support/pageObjects/inlineEditing'; import {useSectionMatchers} from 'support/matchers'; +import {features} from 'pageflow/frontend'; +import {act, fireEvent} from '@testing-library/react'; import '@testing-library/jest-dom/extend-expect'; describe('inline editing forced section padding', () => { useInlineEditingPageObjects(); useSectionMatchers(); + function resetReviewStateWith(commentThreads) { + act(() => { + window.dispatchEvent(new MessageEvent('message', { + data: { + type: 'REVIEW_STATE_RESET', + payload: {currentUser: {id: 1}, commentThreads} + }, + origin: window.location.origin + })); + }); + } + it('forces padding below full width element if section is selected', () => { const {getSectionByPermaId} = renderEntry({ seed: { @@ -38,6 +52,37 @@ describe('inline editing forced section padding', () => { expect(getSectionByPermaId(6)).toHaveForcedPadding(); }); + it('forces padding when section comments are selected', () => { + features.enable('frontend', ['commenting']); + + const {getByRole, getSectionByPermaId} = renderEntry({ + seed: {sections: [{id: 5, permaId: 6}]} + }); + + resetReviewStateWith([{ + id: 1, subjectType: 'Section', subjectId: 6, + comments: [{id: 100, body: 'Review this'}] + }]); + + fireEvent.click(getByRole('status')); + + expect(getSectionByPermaId(6)).toHaveForcedPadding(); + }); + + it('forces padding when a new thread on the section is selected', () => { + features.enable('frontend', ['commenting']); + + const {getByRole, getSectionByPermaId} = renderEntry({ + seed: {sections: [{id: 5, permaId: 6}]} + }); + + // Reveal the section's icon badge, then start a new thread from it. + getSectionByPermaId(6).select(); + fireEvent.click(getByRole('status')); + + expect(getSectionByPermaId(6)).toHaveForcedPadding(); + }); + it('does not force padding if padding is selected', () => { const {getSectionByPermaId} = renderEntry({ seed: { diff --git a/entry_types/scrolled/package/spec/frontend/inlineEditing/features/sectionCommentBadges-spec.js b/entry_types/scrolled/package/spec/frontend/inlineEditing/features/sectionCommentBadges-spec.js new file mode 100644 index 0000000000..99d47a6acf --- /dev/null +++ b/entry_types/scrolled/package/spec/frontend/inlineEditing/features/sectionCommentBadges-spec.js @@ -0,0 +1,204 @@ +import '@testing-library/jest-dom/extend-expect'; +import {act, waitFor} from '@testing-library/react'; +import {useFakeFeatures} from 'pageflow/testHelpers'; + +import {useInlineEditingPageObjects, renderEntry} from 'support/pageObjects/inlineEditing'; + +import badgeStyles from 'review/Badge.module.css'; +import sectionStyles from 'frontend/inlineEditing/SectionDecorator.module.css'; + +describe('inline editing section comment badges', () => { + useInlineEditingPageObjects(); + useFakeFeatures('frontend', ['commenting']); + + it('does not display comment icon when section is not selected', () => { + const {queryByRole} = renderEntry({ + seed: { + sections: [{id: 1, permaId: 10}], + contentElements: [{sectionId: 1, permaId: 100}] + } + }); + + expect(queryByRole('status')).not.toBeInTheDocument(); + }); + + it('displays dot badge when threads exist and section is not selected', async () => { + const {getByRole} = renderEntry({ + seed: { + sections: [{id: 1, permaId: 10}], + contentElements: [{sectionId: 1, permaId: 100}] + } + }); + + act(() => { + window.dispatchEvent(new MessageEvent('message', { + data: { + type: 'REVIEW_STATE_RESET', + payload: { + currentUser: {id: 1}, + commentThreads: [{ + id: 1, + subjectType: 'Section', + subjectId: 10, + comments: [{id: 100, body: 'Review this'}] + }] + } + }, + origin: window.location.origin + })); + }); + + await waitFor(() => { + expect(getByRole('status')).toBeInTheDocument(); + expect(getByRole('status')).not.toHaveTextContent(/\d/); + }); + }); + + it('renders badge in active mode when newThread is selected on the section', () => { + const {getByRole} = renderEntry({ + seed: { + sections: [{id: 1, permaId: 10}], + contentElements: [{sectionId: 1, permaId: 100}] + } + }); + + act(() => { + window.dispatchEvent(new MessageEvent('message', { + data: { + type: 'SELECT', + payload: {type: 'newThread', subjectType: 'Section', subjectId: 10} + }, + origin: window.location.origin + })); + }); + + expect(getByRole('status')).toHaveClass(badgeStyles.active); + }); + + it('keeps the badge sticky when section comments are selected without threads', () => { + const {getByRole} = renderEntry({ + seed: { + sections: [{id: 1, permaId: 10}], + contentElements: [{sectionId: 1, permaId: 100}] + } + }); + + act(() => { + window.dispatchEvent(new MessageEvent('message', { + data: { + type: 'SELECT', + payload: {type: 'sectionComments', id: 1} + }, + origin: window.location.origin + })); + }); + + expect(getByRole('status').closest(`.${sectionStyles.commentBadge}`)) + .toHaveClass(sectionStyles.sticky); + }); + + it('renders the section as selected when its comments are selected', () => { + const {getByLabelText} = renderEntry({ + seed: { + sections: [{id: 1, permaId: 10}], + contentElements: [{sectionId: 1, permaId: 100}] + } + }); + + act(() => { + window.dispatchEvent(new MessageEvent('message', { + data: { + type: 'SELECT', + payload: {type: 'sectionComments', id: 1} + }, + origin: window.location.origin + })); + }); + + expect(getByLabelText('Select section')).toHaveClass(sectionStyles.selected); + }); + + it('renders the section as selected when a new thread on it is selected', () => { + const {getByLabelText} = renderEntry({ + seed: { + sections: [{id: 1, permaId: 10}], + contentElements: [{sectionId: 1, permaId: 100}] + } + }); + + act(() => { + window.dispatchEvent(new MessageEvent('message', { + data: { + type: 'SELECT', + payload: {type: 'newThread', subjectType: 'Section', subjectId: 10} + }, + origin: window.location.origin + })); + }); + + expect(getByLabelText('Select section')).toHaveClass(sectionStyles.selected); + }); + + it('keeps the badge sticky when a new thread on the section is selected', () => { + const {getByRole} = renderEntry({ + seed: { + sections: [{id: 1, permaId: 10}], + contentElements: [{sectionId: 1, permaId: 100}] + } + }); + + act(() => { + window.dispatchEvent(new MessageEvent('message', { + data: { + type: 'SELECT', + payload: {type: 'newThread', subjectType: 'Section', subjectId: 10} + }, + origin: window.location.origin + })); + }); + + expect(getByRole('status').closest(`.${sectionStyles.commentBadge}`)) + .toHaveClass(sectionStyles.sticky); + }); + + it('scrolls the section into view on SELECT_COMMENT_THREAD for one of its threads', async () => { + const {getByRole, getByLabelText} = renderEntry({ + seed: { + sections: [{id: 1, permaId: 10}], + contentElements: [{sectionId: 1, permaId: 100}] + } + }); + + act(() => { + window.dispatchEvent(new MessageEvent('message', { + data: { + type: 'REVIEW_STATE_RESET', + payload: { + currentUser: {id: 1}, + commentThreads: [{ + id: 1, subjectType: 'Section', subjectId: 10, + comments: [{id: 100, body: 'Review this'}] + }] + } + }, + origin: window.location.origin + })); + }); + + await waitFor(() => { + expect(getByRole('status')).toBeInTheDocument(); + }); + + const section = getByLabelText('Select section'); + section.scrollIntoView = jest.fn(); + + act(() => { + window.dispatchEvent(new MessageEvent('message', { + data: {type: 'SELECT_COMMENT_THREAD', payload: {threadId: 1}}, + origin: window.location.origin + })); + }); + + expect(section.scrollIntoView).toHaveBeenCalled(); + }); +}); diff --git a/entry_types/scrolled/package/spec/frontend/inlineEditing/features/selectedMessage-spec.js b/entry_types/scrolled/package/spec/frontend/inlineEditing/features/selectedMessage-spec.js index d6618646c5..377fd6fa5d 100644 --- a/entry_types/scrolled/package/spec/frontend/inlineEditing/features/selectedMessage-spec.js +++ b/entry_types/scrolled/package/spec/frontend/inlineEditing/features/selectedMessage-spec.js @@ -241,4 +241,106 @@ describe('inline editing SELECTED message', () => { payload: {id: 1, type: 'contentElementComments'} }, expect.anything()); }); + + it('is posted with type newThread when section comment badge is clicked and no threads exist', () => { + features.enable('frontend', ['commenting']); + + const {getByRole, getSectionByPermaId} = renderEntry({ + seed: { + sections: [{id: 1, permaId: 10}], + contentElements: [{sectionId: 1, permaId: 100}] + } + }); + + // Select the section so the icon-mode badge becomes visible even + // without threads. + getSectionByPermaId(10).select(); + + fireEvent.click(getByRole('status')); + + expect(window.parent.postMessage).toHaveBeenCalledWith({ + type: 'SELECTED', + payload: {type: 'newThread', subjectType: 'Section', subjectId: 10} + }, expect.anything()); + }); + + it('is posted with type sectionComments when section comment badge is clicked', async () => { + features.enable('frontend', ['commenting']); + + const {getByRole} = renderEntry({ + seed: { + sections: [{id: 1, permaId: 10}], + contentElements: [{sectionId: 1, permaId: 100}] + } + }); + + act(() => { + window.dispatchEvent(new MessageEvent('message', { + data: { + type: 'REVIEW_STATE_RESET', + payload: { + currentUser: {id: 1}, + commentThreads: [{ + id: 1, + subjectType: 'Section', + subjectId: 10, + comments: [{id: 100, body: 'Review this'}] + }] + } + }, + origin: window.location.origin + })); + }); + + await waitFor(() => { + expect(getByRole('status')).toBeInTheDocument(); + }); + + fireEvent.click(getByRole('status')); + + expect(window.parent.postMessage).toHaveBeenCalledWith({ + type: 'SELECTED', + payload: {id: 1, type: 'sectionComments'} + }, expect.anything()); + }); + + it('does not select the section when its comment badge is pressed', async () => { + features.enable('frontend', ['commenting']); + + const {getByRole} = renderEntry({ + seed: { + sections: [{id: 1, permaId: 10}], + contentElements: [{sectionId: 1, permaId: 100}] + } + }); + + act(() => { + window.dispatchEvent(new MessageEvent('message', { + data: { + type: 'REVIEW_STATE_RESET', + payload: { + currentUser: {id: 1}, + commentThreads: [{ + id: 1, + subjectType: 'Section', + subjectId: 10, + comments: [{id: 100, body: 'Review this'}] + }] + } + }, + origin: window.location.origin + })); + }); + + await waitFor(() => { + expect(getByRole('status')).toBeInTheDocument(); + }); + + fireEvent.mouseDown(getByRole('status')); + + expect(window.parent.postMessage).not.toHaveBeenCalledWith({ + type: 'SELECTED', + payload: {id: 1, type: 'sectionSettings'} + }, expect.anything()); + }); }); diff --git a/entry_types/scrolled/package/src/frontend/inlineEditing/SectionDecorator.js b/entry_types/scrolled/package/src/frontend/inlineEditing/SectionDecorator.js index 1d4e463119..2528a26f8e 100644 --- a/entry_types/scrolled/package/src/frontend/inlineEditing/SectionDecorator.js +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/SectionDecorator.js @@ -6,6 +6,8 @@ import contentElementStyles from './ContentElementDecorator.module.css'; import widgetSelectionRectStyles from './WidgetSelectionRect.module.css'; import paddingIndicatorStyles from './PaddingIndicator.module.css'; +import {features} from 'pageflow/frontend'; +import {ThreadsBadge, useCommentThreads} from 'pageflow-scrolled/review'; import {Toolbar} from './Toolbar'; import {ForcePaddingContext} from '../Foreground'; import {useEditorSelection} from './EditorState'; @@ -16,6 +18,7 @@ import transitionIcon from './images/arrows.svg'; export function SectionDecorator({backdrop, section, contentElements, transitions, children}) { const {t} = useI18n({locale: 'ui'}); + const commentingEnabled = features.isEnabled('commenting'); const {isSelected: isSectionSelected, select, resetSelection} = useEditorSelection({ id: section.id, @@ -27,7 +30,31 @@ export function SectionDecorator({backdrop, section, contentElements, transition type: 'sectionPaddings' }); - const isSelected = isPaddingSelected || isSectionSelected; + const {isSelected: isCommentsSelected, select: selectComments} = useEditorSelection({ + id: section.id, + type: 'sectionComments' + }); + + const {isSelected: isNewThreadSelected, select: selectNewThread} = useEditorSelection({ + type: 'newThread', + subjectType: 'Section', + subjectId: section.permaId + }); + + // Viewing a section's comments or composing a new thread on it. + const commentsSelected = isCommentsSelected || isNewThreadSelected; + + // The section reads as selected while its comments are open, so the + // section and the sidebar comment panel stay visually in sync. + const isSelected = isSectionSelected || isPaddingSelected || commentsSelected; + + const threads = useCommentThreads( + {subjectType: 'Section', subjectId: section.permaId}, + {resolved: false} + ); + const hasThreads = threads.length > 0; + + const clipBadgeCorner = (isSectionSelected || isPaddingSelected) && !hasThreads; const {isSelected: isBackdropElementSelected} = useEditorSelection({ id: backdrop.contentElement?.id, @@ -61,6 +88,7 @@ export function SectionDecorator({backdrop, section, contentElements, transition !event.target.closest(`.${backdropStyles.wrapper}`) && !event.target.closest(`.${widgetSelectionRectStyles.wrapper}`) && !event.target.closest(`.${paddingIndicatorStyles.indicator}`) && + !event.target.closest(`.${styles.commentBadge}`) && !event.target.closest('#fullscreenRoot') && !event.target.closest('[data-floating-ui-portal]')) { isSectionSelected ? resetSelection() : select(); @@ -69,7 +97,8 @@ export function SectionDecorator({backdrop, section, contentElements, transition return (
{renderEditTransitionButton({id: section.previousSection && section.id, @@ -82,21 +111,37 @@ export function SectionDecorator({backdrop, section, contentElements, transition + isHighlighted || + commentsSelected}> {children} + {commentingEnabled && +
+ hasThreads ? selectComments() : selectNewThread()} + onSelectThread={threadId => selectComments({ + type: 'sectionComments', + id: section.id, + highlightedThreadId: threadId + })} /> +
}
); } -function className(isSelected, transitionSelection, isHighlighted, isBackdropElementSelected, transitions) { +function className(isSelected, transitionSelection, isHighlighted, isBackdropElementSelected, transitions, clipBadgeCorner, commenting) { return classNames(styles.wrapper, { [styles.selected]: isSelected, [styles.highlighted]: isHighlighted, [styles.lineAbove]: isBackdropElementSelected && transitions[0].startsWith('fade'), [styles.lineBelow]: isBackdropElementSelected && transitions[1].startsWith('fade'), - [styles.transitionSelected]: transitionSelection.isSelected + [styles.transitionSelected]: transitionSelection.isSelected, + [styles.clipBadgeCorner]: clipBadgeCorner, + [styles.commenting]: commenting }); } diff --git a/entry_types/scrolled/package/src/frontend/inlineEditing/SectionDecorator.module.css b/entry_types/scrolled/package/src/frontend/inlineEditing/SectionDecorator.module.css index d54c359add..a7b2deb358 100644 --- a/entry_types/scrolled/package/src/frontend/inlineEditing/SectionDecorator.module.css +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/SectionDecorator.module.css @@ -84,6 +84,53 @@ section::before { overflow: hidden; } +.commentBadge { + box-sizing: border-box; + position: relative; + z-index: 11; + height: 44px; + pointer-events: none; +} + +.commentBadge.sticky { + position: sticky; + bottom: 0px; +} + +.commentBadge > button { + position: absolute; + transform: translate(0, 50%); + pointer-events: auto; + border-bottom-left-radius: 0; + border-top-left-radius: 0; +} + +.selected .commentBadge.sticky { + bottom: 10px; +} + +.selected .commentBadge > button { + bottom: 7px; + left: space(-1.5); +} + +.selected .commentBadge.sticky > button { + left: 0; +} + +/* clip-path clips the border and inset shadow behind the transparent icon. */ +.clipBadgeCorner > section::before { + --nodge: 15px; + clip-path: polygon(0 -10%, 100% -10%, 100% 110%, + var(--nodge) 110%, var(--nodge) calc(100% - var(--nodge)), 0 calc(100% - var(--nodge))); +} + +/* Absorb the badge box's height; on the section, not the sticky badge. Scoped + * to commenting since the box only renders then. */ +.commenting > section { + margin-bottom: -44px; +} + .transitionToolbar-before { composes: toolbar; top: 0; From 87890f464151dcee7512b15df85266f97142c08a Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Wed, 17 Jun 2026 15:22:18 +0200 Subject: [PATCH 06/18] Show section comments in the entry comments view List a section's comment threads under their own label-only separator, positioned above the comment groups of the section's content elements, so the All comments tab surfaces feedback left on whole sections, not just on individual content elements. REDMINE-21261 --- entry_types/scrolled/config/locales/de.yml | 1 + entry_types/scrolled/config/locales/en.yml | 1 + .../editor/views/EntryCommentsView-spec.js | 90 ++++++++++++++++++- .../views/SelectionCommentsView-spec.js | 38 ++++++++ .../src/editor/views/EntryCommentsView.js | 65 +++++++++++--- .../src/editor/views/SelectionCommentsView.js | 8 +- 6 files changed, 190 insertions(+), 13 deletions(-) diff --git a/entry_types/scrolled/config/locales/de.yml b/entry_types/scrolled/config/locales/de.yml index cbb4fdcb55..222c06b496 100644 --- a/entry_types/scrolled/config/locales/de.yml +++ b/entry_types/scrolled/config/locales/de.yml @@ -1586,6 +1586,7 @@ de: comments: Kommentare comments_view: new_thread: Neues Thema + section: Abschnitt tabs: comments: Alle Kommentare selection: Für Auswahl diff --git a/entry_types/scrolled/config/locales/en.yml b/entry_types/scrolled/config/locales/en.yml index c3c0781d84..cabe3540d6 100644 --- a/entry_types/scrolled/config/locales/en.yml +++ b/entry_types/scrolled/config/locales/en.yml @@ -1569,6 +1569,7 @@ en: comments: Comments comments_view: new_thread: New topic + section: Section tabs: comments: All comments selection: For selection 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 a7239f4c93..7738cf7ec8 100644 --- a/entry_types/scrolled/package/spec/editor/views/EntryCommentsView-spec.js +++ b/entry_types/scrolled/package/spec/editor/views/EntryCommentsView-spec.js @@ -25,7 +25,8 @@ describe('EntryCommentsView', () => { 'pageflow_scrolled.review.reply_placeholder': 'Reply...', 'pageflow_scrolled.review.send': 'Send', 'pageflow_scrolled.editor.content_elements.textBlock.name': 'Text', - 'pageflow_scrolled.editor.content_elements.image.name': 'Image' + 'pageflow_scrolled.editor.content_elements.image.name': 'Image', + 'pageflow_scrolled.editor.comments_view.section': 'Section' }); it('renders a thread group only for content elements that have threads', () => { @@ -364,4 +365,91 @@ describe('EntryCommentsView', () => { expect(pictogram).toBeInTheDocument(); expect(pictogram.style.maskImage).not.toBe(''); }); + + it('renders a section thread group with a label-only separator', () => { + const entry = createEntry({ + sections: [{id: 1, permaId: 10}], + contentElements: [{id: 1, permaId: 100, sectionId: 1, typeName: 'image'}] + }); + entry.reviewSession = factories.reviewSession({ + commentThreads: [{ + id: 1, subjectType: 'Section', subjectId: 10, + comments: [{id: 100, body: 'On the section', creatorName: 'Alice'}] + }] + }); + + const view = new EntryCommentsView({entry, editor}); + const {getByText} = renderBackboneView(view); + + expect(getByText('On the section')).toBeInTheDocument(); + expect(getByText('Section')).toBeInTheDocument(); + expect(view.el.querySelector(`.${styles.pictogram}`)).toBeNull(); + }); + + it("renders a section's comments above its content element comments", () => { + const entry = createEntry({ + sections: [{id: 1, permaId: 10}], + contentElements: [{id: 1, permaId: 100, sectionId: 1, typeName: 'image'}] + }); + entry.reviewSession = factories.reviewSession({ + commentThreads: [ + {id: 1, subjectType: 'ContentElement', subjectId: 100, + comments: [{id: 1, body: 'on element', creatorName: 'A'}]}, + {id: 2, subjectType: 'Section', subjectId: 10, + comments: [{id: 2, body: 'on section', creatorName: 'B'}]} + ] + }); + + const view = new EntryCommentsView({entry, editor}); + const {getByText} = renderBackboneView(view); + + const sectionComment = getByText('on section'); + const elementComment = getByText('on element'); + + expect(sectionComment.compareDocumentPosition(elementComment) & + Node.DOCUMENT_POSITION_FOLLOWING).toBeTruthy(); + }); + + it('does not render a section group when the section has no threads', () => { + const entry = createEntry({ + sections: [{id: 1, permaId: 10}], + contentElements: [{id: 1, permaId: 100, sectionId: 1, typeName: 'image'}] + }); + entry.reviewSession = factories.reviewSession({ + commentThreads: [{ + id: 1, subjectType: 'ContentElement', subjectId: 100, + comments: [{id: 100, body: 'on element', creatorName: 'Alice'}] + }] + }); + + const view = new EntryCommentsView({entry, editor}); + const {queryByText} = renderBackboneView(view); + + expect(queryByText('on element')).toBeInTheDocument(); + expect(queryByText('Section')).not.toBeInTheDocument(); + }); + + it('highlights all threads of the selected section', () => { + const entry = createEntry({ + sections: [{id: 1, permaId: 10}, {id: 2, permaId: 20}] + }); + entry.set('selectedCommentsSubject', {subjectType: 'Section', id: 1}); + entry.reviewSession = factories.reviewSession({ + commentThreads: [ + {id: 1, subjectType: 'Section', subjectId: 10, + comments: [{id: 10, body: 'on selected one', creatorName: 'A'}]}, + {id: 2, subjectType: 'Section', subjectId: 10, + comments: [{id: 20, body: 'on selected two', creatorName: 'B'}]}, + {id: 3, subjectType: 'Section', subjectId: 20, + comments: [{id: 30, body: 'on other', creatorName: 'C'}]} + ] + }); + + const view = new EntryCommentsView({entry, editor}); + const {getByText} = renderBackboneView(view); + + expect(getByText('on selected one').closest('[aria-current="true"]')).not.toBeNull(); + expect(getByText('on selected two').closest('[aria-current="true"]')).not.toBeNull(); + expect(getByText('on other').closest('[aria-current="true"]')).toBeNull(); + }); }); diff --git a/entry_types/scrolled/package/spec/editor/views/SelectionCommentsView-spec.js b/entry_types/scrolled/package/spec/editor/views/SelectionCommentsView-spec.js index 45abe31f6a..087aa82b3a 100644 --- a/entry_types/scrolled/package/spec/editor/views/SelectionCommentsView-spec.js +++ b/entry_types/scrolled/package/spec/editor/views/SelectionCommentsView-spec.js @@ -455,4 +455,42 @@ describe('SelectionCommentsView', () => { }); expect(queryByText('On element')).not.toBeInTheDocument(); }); + + // The SELECTED message sets highlightedThreadId and + // selectedCommentsSubject in one go. The change:highlightedThreadId + // listener rerenders before change:selectedCommentsSubject refreshes + // the resolved model, so props must derive the model from the current + // subject rather than a stale cache. + it('does not crash switching from section to content element comments in one set', async () => { + const entry = createEntry({ + sections: [{id: 5, permaId: 50}], + contentElements: [{id: 1, permaId: 10, typeName: 'fixture', sectionId: 5}] + }); + entry.set('selectedCommentsSubject', {subjectType: 'Section', id: 5}); + entry.reviewSession = factories.reviewSession({ + commentThreads: [ + {id: 1, subjectType: 'Section', subjectId: 50, + comments: [{id: 100, body: 'On section', creatorName: 'Alice'}]}, + {id: 2, subjectType: 'ContentElement', subjectId: 10, + comments: [{id: 200, body: 'On element', creatorName: 'Bob'}]} + ] + }); + + const view = new SelectionCommentsView({entry, editor}); + const {getByText, queryByText} = renderBackboneView(view); + + expect(getByText('On section')).toBeInTheDocument(); + + act(() => { + entry.set({ + highlightedThreadId: 2, + selectedCommentsSubject: {subjectType: 'ContentElement', id: 1} + }); + }); + + await waitFor(() => { + expect(getByText('On element')).toBeInTheDocument(); + }); + expect(queryByText('On section')).not.toBeInTheDocument(); + }); }); diff --git a/entry_types/scrolled/package/src/editor/views/EntryCommentsView.js b/entry_types/scrolled/package/src/editor/views/EntryCommentsView.js index e70c502fb1..b1d7b3aca8 100644 --- a/entry_types/scrolled/package/src/editor/views/EntryCommentsView.js +++ b/entry_types/scrolled/package/src/editor/views/EntryCommentsView.js @@ -23,8 +23,9 @@ export const EntryCommentsView = ReviewView.extend({ props() { const {entry, editor} = this.options; return { - items: collectContentElements(entry), + items: collectItems(entry), selectedElement: this._selectedElement, + selectedSection: this._selectedSection, // Undefined for elements without a slate cursor (e.g. images); // an array (possibly empty) for textBlocks where Selection.js // has reported the cursor's overlapping threads. @@ -36,18 +37,23 @@ export const EntryCommentsView = ReviewView.extend({ }; }, - renderContent({items, selectedElement, transientThreadIds, highlightedThreadId, onThreadClick, editor}) { + renderContent({items, selectedElement, selectedSection, transientThreadIds, highlightedThreadId, onThreadClick, editor}) { return (
- {items.map(({contentElement}) => ( - item.type === 'section' ? + : + - ))} + )}
); }, @@ -67,6 +73,10 @@ export const EntryCommentsView = ReviewView.extend({ subject?.subjectType === 'ContentElement' ? this.options.entry.contentElements.get(subject.id) : null; + this._selectedSection = + subject?.subjectType === 'Section' ? + this.options.entry.sections.get(subject.id) : + null; if (this._selectedElement) { this.listenTo(this._selectedElement.transientState, @@ -76,13 +86,17 @@ export const EntryCommentsView = ReviewView.extend({ } }); -function collectContentElements(entry) { +// Section comment groups precede the content element groups of the +// same section, so a reviewer sees feedback on the section as a whole +// above feedback on its individual elements. +function collectItems(entry) { const items = []; entry.chapters.each(chapter => { chapter.sections.each(section => { + items.push({type: 'section', section}); section.contentElements.each(contentElement => { - items.push({contentElement, section}); + items.push({type: 'contentElement', contentElement}); }); }); }); @@ -118,7 +132,7 @@ function ContentElementGroup({ return (
- + t.id) : highlightedThreadId; + + return ( +
+ + +
+ ); +} + +function Separator({label, pictogram}) { return (
diff --git a/entry_types/scrolled/package/src/editor/views/SelectionCommentsView.js b/entry_types/scrolled/package/src/editor/views/SelectionCommentsView.js index 509d1fb8db..7b722c5ff1 100644 --- a/entry_types/scrolled/package/src/editor/views/SelectionCommentsView.js +++ b/entry_types/scrolled/package/src/editor/views/SelectionCommentsView.js @@ -20,7 +20,13 @@ export const SelectionCommentsView = ReviewView.extend({ props() { const {entry, editor} = this.options; const subject = entry.get('selectedCommentsSubject'); - const model = this._model; + + // Resolve the model from the current subject rather than the cached + // `this._model`: a `change:highlightedThreadId` rerender can run + // before `change:selectedCommentsSubject` refreshes `this._model`, + // leaving a stale Section model paired with a new ContentElement + // subject. + const model = this._resolveModel(); if (!subject || !model) { return {}; From 3a0a316dd04c19378c94371fdb9b7328e8673515 Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Wed, 17 Jun 2026 19:02:41 +0200 Subject: [PATCH 07/18] Reveal resolved comment threads from the comments sidebar Fetch resolved threads alongside unresolved ones in EditableText so their subject ranges keep following live edits and stay correct once a thread is reopened. Keep resolved threads hidden from the highlight overlay until they become the highlighted thread. Reveal a resolved thread when it is selected from the entry comments sidebar - for content elements and sections as well as inline text ranges. A resolved thread has no badge of its own to react to the SELECT_COMMENT_THREAD message, so promote it to the highlighted thread, which reveals it, and scroll it into view. In EditableText, move the cursor into the thread's block first so Selection's cursorLeftHighlightedThreadBlock does not treat the pre-existing cursor as having left the comment and re-select the content element, which would hide the highlight again. Extract the SELECT_COMMENT_THREAD handling into a shared useSelectCommentThreadHandler hook used by the EditableText editor and the content element and section decorators, instead of duplicating it across ThreadsBadge, BadgeColumn and the editor. The hook retrieves the subject's threads and, on a match, scrolls into view and calls the caller-provided selectThread; an optional beforeSelect runs first. ThreadsBadge no longer needs its confusing onSelectThread prop. --- .../features/commentHighlights-spec.js | 84 ++++++++++ .../features/commentSelection-spec.js | 36 +++++ .../EditableText/useCommenting-spec.js | 81 ++++++++++ .../contentElementCommentBadges-spec.js | 151 ++++++++++++++++++ .../features/sectionCommentBadges-spec.js | 49 ++++++ .../package/spec/review/ThreadsBadge-spec.js | 67 -------- .../inlineEditing/ContentElementDecorator.js | 26 ++- .../inlineEditing/EditableText/BadgeColumn.js | 11 -- .../EditableText/useCommenting.js | 74 ++++++++- .../inlineEditing/SectionDecorator.js | 26 ++- .../frontend/inlineEditing/SelectionRect.js | 9 +- .../useSelectCommentThreadHandler.js | 38 +++++ .../package/src/review/ThreadsBadge.js | 38 +---- 13 files changed, 552 insertions(+), 138 deletions(-) create mode 100644 entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/useCommenting-spec.js create mode 100644 entry_types/scrolled/package/src/frontend/inlineEditing/useSelectCommentThreadHandler.js diff --git a/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/features/commentHighlights-spec.js b/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/features/commentHighlights-spec.js index 50bb06fbb0..cfa29a1698 100644 --- a/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/features/commentHighlights-spec.js +++ b/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/features/commentHighlights-spec.js @@ -81,6 +81,90 @@ describe('inline editing EditableText comment highlights', () => { expect(highlight).toHaveClass(highlightStyles.selected); }); + it('hides resolved thread highlights until the thread is selected', async () => { + const entry = renderEntry({ + contentElement: { + ui: , + typeOptions: {inlineComments: true, customSelectionRect: true} + }, + commenting: { + currentUser: null, + commentThreads: [{ + id: 7, + subjectType: 'ContentElement', + subjectId: 10, + subjectRange, + resolvedAt: '2026-06-01T00:00:00Z', + comments: [{id: 1, body: 'A comment', creatorName: 'Alice', creatorId: 1}] + }] + } + }); + + expect(entry.container.querySelector(`.${highlightStyles.highlight}`)) + .not.toBeInTheDocument(); + + act(() => { + window.postMessage({ + type: 'SELECT_COMMENT_THREAD', + payload: {threadId: 7} + }, '*'); + }); + + await waitFor(() => { + expect(entry.container.querySelector(`.${highlightStyles.highlight}`)) + .toBeInTheDocument(); + }); + + expect(entry.container.querySelector(`.${highlightStyles.highlight}`)) + .toHaveClass(highlightStyles.selected); + }); + + it('keeps the resolved thread highlighted when a cursor sits in another block', () => { + const multiBlockValue = [ + {type: 'paragraph', children: [{text: 'First paragraph'}]}, + {type: 'paragraph', children: [{text: 'Second paragraph'}]} + ]; + + const entry = renderEntry({ + contentElement: { + ui: , + typeOptions: {inlineComments: true, customSelectionRect: true} + }, + commenting: { + currentUser: null, + commentThreads: [{ + id: 7, + subjectType: 'ContentElement', + subjectId: 10, + subjectRange: {anchor: {path: [1, 0], offset: 0}, focus: {path: [1, 0], offset: 6}}, + resolvedAt: '2026-06-01T00:00:00Z', + comments: [{id: 1, body: 'A comment', creatorName: 'Alice', creatorId: 1}] + }] + } + }); + + // The reviewer had clicked into the first paragraph, leaving a slate + // cursor there before opening the resolved comment from the sidebar. + act(() => { + window.dispatchEvent(new MessageEvent('message', { + data: {type: 'SELECT', payload: {type: 'contentElement', id: 1, range: [0, 1]}}, + origin: window.location.origin + })); + }); + + act(() => { + window.dispatchEvent(new MessageEvent('message', { + data: {type: 'SELECT_COMMENT_THREAD', payload: {threadId: 7}}, + origin: window.location.origin + })); + }); + + const highlight = entry.container.querySelector(`.${highlightStyles.highlight}`); + expect(highlight).toBeInTheDocument(); + expect(highlight).toHaveTextContent('Second'); + expect(highlight).toHaveClass(highlightStyles.selected); + }); + it('applies selected style to highlight when thread badge is clicked', () => { const entry = renderEntry({ contentElement: { diff --git a/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/features/commentSelection-spec.js b/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/features/commentSelection-spec.js index 22dae759ab..8a5aaf4f97 100644 --- a/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/features/commentSelection-spec.js +++ b/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/features/commentSelection-spec.js @@ -133,4 +133,40 @@ describe('inline editing EditableText comment selection messages', () => { expect(scrollIntoView).toHaveBeenCalled(); delete Element.prototype.scrollIntoView; }); + + it('scrolls a resolved thread into view when selected via SELECT_COMMENT_THREAD', () => { + const scrollIntoView = jest.fn(); + Element.prototype.scrollIntoView = scrollIntoView; + + const subjectRange = {anchor: {path: [0, 0], offset: 0}, focus: {path: [0, 0], offset: 5}}; + const value = [{type: 'paragraph', children: [{text: 'Some text to comment on'}]}]; + + renderEntry({ + contentElement: { + ui: , + typeOptions: {inlineComments: true, customSelectionRect: true} + }, + commenting: { + currentUser: null, + commentThreads: [{ + id: 9, + subjectType: 'ContentElement', + subjectId: 10, + subjectRange, + resolvedAt: '2026-06-01T00:00:00Z', + comments: [{id: 1, body: 'A comment', creatorName: 'Alice', creatorId: 1}] + }] + } + }); + + act(() => { + window.dispatchEvent(new MessageEvent('message', { + data: {type: 'SELECT_COMMENT_THREAD', payload: {threadId: 9}}, + origin: window.location.origin + })); + }); + + expect(scrollIntoView).toHaveBeenCalled(); + delete Element.prototype.scrollIntoView; + }); }); diff --git a/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/useCommenting-spec.js b/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/useCommenting-spec.js new file mode 100644 index 0000000000..56f962bd1e --- /dev/null +++ b/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/useCommenting-spec.js @@ -0,0 +1,81 @@ +import React from 'react'; +import {renderHook, act} from '@testing-library/react-hooks'; +import {createEditor, Transforms} from 'slate'; +import {withReact} from 'slate-react'; + +import {features} from 'pageflow/frontend'; +import {ReviewStateProvider} from 'pageflow-scrolled/review'; +import {ContentElementAttributesProvider} from 'frontend/useContentElementAttributes'; +import {EditorStateProvider} from 'frontend/inlineEditing/EditorState'; + +import {useCommenting} from 'frontend/inlineEditing/EditableText/useCommenting'; + +describe('useCommenting', () => { + beforeEach(() => { + jest.spyOn(features, 'isEnabled').mockImplementation( + name => name === 'commenting' + ); + }); + + afterEach(() => { + features.isEnabled.mockRestore(); + }); + + function setup({commentThreads}) { + const editor = withReact(createEditor()); + editor.children = [ + {type: 'paragraph', children: [{text: 'Hello world'}]} + ]; + + const wrapper = ({children}) => ( + + + + {children} + + + + ); + + const {result} = renderHook(() => useCommenting(editor), {wrapper}); + + return {editor, result}; + } + + it('tracks both resolved and unresolved thread ranges across edits', () => { + // Thread 7 is resolved: it must be tracked just like the unresolved + // thread 1 so its range stays correct once the thread is reopened. + const {editor, result} = setup({ + commentThreads: [ + { + id: 1, + subjectType: 'ContentElement', + subjectId: 10, + subjectRange: {anchor: {path: [0, 0], offset: 2}, + focus: {path: [0, 0], offset: 5}}, + comments: [] + }, + { + id: 7, + subjectType: 'ContentElement', + subjectId: 10, + subjectRange: {anchor: {path: [0, 0], offset: 6}, + focus: {path: [0, 0], offset: 11}}, + resolvedAt: '2026-06-01T00:00:00Z', + comments: [] + } + ] + }); + + act(() => { + Transforms.insertText(editor, 'X', {at: {path: [0, 0], offset: 0}}); + }); + + expect(result.current.getTrackedSubjectRanges()).toEqual({ + 1: {anchor: {path: [0, 0], offset: 3}, + focus: {path: [0, 0], offset: 6}}, + 7: {anchor: {path: [0, 0], offset: 7}, + focus: {path: [0, 0], offset: 12}} + }); + }); +}); diff --git a/entry_types/scrolled/package/spec/frontend/inlineEditing/features/contentElementCommentBadges-spec.js b/entry_types/scrolled/package/spec/frontend/inlineEditing/features/contentElementCommentBadges-spec.js index 06580f9fb1..6f38ba2493 100644 --- a/entry_types/scrolled/package/spec/frontend/inlineEditing/features/contentElementCommentBadges-spec.js +++ b/entry_types/scrolled/package/spec/frontend/inlineEditing/features/contentElementCommentBadges-spec.js @@ -82,4 +82,155 @@ describe('inline editing content element comment badges', () => { expect(getByRole('status')).toHaveClass(badgeStyles.active); }); + + it('selects the thread and scrolls into view on SELECT_COMMENT_THREAD', async () => { + const scrollIntoView = jest.fn(); + Element.prototype.scrollIntoView = scrollIntoView; + + const {getByRole} = renderEntry({ + seed: { + contentElements: [{ + id: 1, + typeName: 'withTestId', + permaId: 10, + configuration: {testId: 5} + }] + } + }); + + act(() => { + window.dispatchEvent(new MessageEvent('message', { + data: { + type: 'REVIEW_STATE_RESET', + payload: { + currentUser: {id: 1}, + commentThreads: [{ + id: 7, + subjectType: 'ContentElement', + subjectId: 10, + comments: [{id: 100, body: 'Review this'}] + }] + } + }, + origin: window.location.origin + })); + }); + + await waitFor(() => { + expect(getByRole('status')).toBeInTheDocument(); + }); + + act(() => { + window.dispatchEvent(new MessageEvent('message', { + data: {type: 'SELECT_COMMENT_THREAD', payload: {threadId: 7}}, + origin: window.location.origin + })); + }); + + expect(window.parent.postMessage).toHaveBeenCalledWith({ + type: 'SELECTED', + payload: {type: 'contentElementComments', id: 1, highlightedThreadId: 7} + }, expect.anything()); + expect(scrollIntoView).toHaveBeenCalled(); + + delete Element.prototype.scrollIntoView; + }); + + it('reveals a resolved thread on SELECT_COMMENT_THREAD even without a badge', () => { + const scrollIntoView = jest.fn(); + Element.prototype.scrollIntoView = scrollIntoView; + + const {queryByRole} = renderEntry({ + seed: { + contentElements: [{ + id: 1, + typeName: 'withTestId', + permaId: 10, + configuration: {testId: 5} + }] + } + }); + + act(() => { + window.dispatchEvent(new MessageEvent('message', { + data: { + type: 'REVIEW_STATE_RESET', + payload: { + currentUser: {id: 1}, + commentThreads: [{ + id: 7, + subjectType: 'ContentElement', + subjectId: 10, + resolvedAt: '2026-06-01T00:00:00Z', + comments: [{id: 100, body: 'Resolved'}] + }] + } + }, + origin: window.location.origin + })); + }); + + // Resolved threads are not counted by the badge. + expect(queryByRole('status')).not.toBeInTheDocument(); + + act(() => { + window.dispatchEvent(new MessageEvent('message', { + data: {type: 'SELECT_COMMENT_THREAD', payload: {threadId: 7}}, + origin: window.location.origin + })); + }); + + expect(window.parent.postMessage).toHaveBeenCalledWith({ + type: 'SELECTED', + payload: {type: 'contentElementComments', id: 1, highlightedThreadId: 7} + }, expect.anything()); + expect(scrollIntoView).toHaveBeenCalled(); + + delete Element.prototype.scrollIntoView; + }); + + it('ignores SELECT_COMMENT_THREAD for a thread of another subject', () => { + renderEntry({ + seed: { + contentElements: [{ + id: 1, + typeName: 'withTestId', + permaId: 10, + configuration: {testId: 5} + }] + } + }); + + act(() => { + window.dispatchEvent(new MessageEvent('message', { + data: { + type: 'REVIEW_STATE_RESET', + payload: { + currentUser: {id: 1}, + commentThreads: [{ + id: 7, + subjectType: 'ContentElement', + subjectId: 10, + comments: [{id: 100, body: 'Review this'}] + }] + } + }, + origin: window.location.origin + })); + }); + + window.parent.postMessage.mockClear(); + + act(() => { + window.dispatchEvent(new MessageEvent('message', { + data: {type: 'SELECT_COMMENT_THREAD', payload: {threadId: 999}}, + origin: window.location.origin + })); + }); + + expect(window.parent.postMessage).not.toHaveBeenCalledWith( + expect.objectContaining({type: 'SELECTED'}), + expect.anything() + ); + }); }); diff --git a/entry_types/scrolled/package/spec/frontend/inlineEditing/features/sectionCommentBadges-spec.js b/entry_types/scrolled/package/spec/frontend/inlineEditing/features/sectionCommentBadges-spec.js index 99d47a6acf..117cd64b5f 100644 --- a/entry_types/scrolled/package/spec/frontend/inlineEditing/features/sectionCommentBadges-spec.js +++ b/entry_types/scrolled/package/spec/frontend/inlineEditing/features/sectionCommentBadges-spec.js @@ -200,5 +200,54 @@ describe('inline editing section comment badges', () => { }); expect(section.scrollIntoView).toHaveBeenCalled(); + expect(window.parent.postMessage).toHaveBeenCalledWith({ + type: 'SELECTED', + payload: {type: 'sectionComments', id: 1, highlightedThreadId: 1} + }, expect.anything()); + }); + + it('reveals a resolved section thread on SELECT_COMMENT_THREAD even without a badge', () => { + const {queryByRole, getByLabelText} = renderEntry({ + seed: { + sections: [{id: 1, permaId: 10}], + contentElements: [{sectionId: 1, permaId: 100}] + } + }); + + act(() => { + window.dispatchEvent(new MessageEvent('message', { + data: { + type: 'REVIEW_STATE_RESET', + payload: { + currentUser: {id: 1}, + commentThreads: [{ + id: 3, subjectType: 'Section', subjectId: 10, + resolvedAt: '2026-06-01T00:00:00Z', + comments: [{id: 100, body: 'Resolved'}] + }] + } + }, + origin: window.location.origin + })); + }); + + // Resolved threads are not counted by the badge. + expect(queryByRole('status')).not.toBeInTheDocument(); + + const section = getByLabelText('Select section'); + section.scrollIntoView = jest.fn(); + + act(() => { + window.dispatchEvent(new MessageEvent('message', { + data: {type: 'SELECT_COMMENT_THREAD', payload: {threadId: 3}}, + origin: window.location.origin + })); + }); + + expect(section.scrollIntoView).toHaveBeenCalled(); + expect(window.parent.postMessage).toHaveBeenCalledWith({ + type: 'SELECTED', + payload: {type: 'sectionComments', id: 1, highlightedThreadId: 3} + }, expect.anything()); }); }); diff --git a/entry_types/scrolled/package/spec/review/ThreadsBadge-spec.js b/entry_types/scrolled/package/spec/review/ThreadsBadge-spec.js index 908d918099..f088df649b 100644 --- a/entry_types/scrolled/package/spec/review/ThreadsBadge-spec.js +++ b/entry_types/scrolled/package/spec/review/ThreadsBadge-spec.js @@ -3,7 +3,6 @@ import '@testing-library/jest-dom/extend-expect'; import {ThreadsBadge} from 'review/ThreadsBadge'; import {renderWithReviewState} from 'support/renderWithReviewState'; -import {fakeParentWindow} from 'support'; describe('ThreadsBadge', () => { it('does not display count for single thread', () => { @@ -147,70 +146,4 @@ describe('ThreadsBadge', () => { expect(getByRole('status')).toBeInTheDocument(); }); }); - - describe('on SELECT_COMMENT_THREAD message', () => { - let scrollIntoView; - - beforeEach(() => { - fakeParentWindow(); - scrollIntoView = jest.fn(); - Element.prototype.scrollIntoView = scrollIntoView; - }); - - afterEach(() => { - delete Element.prototype.scrollIntoView; - }); - - it('scrolls into view and calls onSelectThread when threadId matches', async () => { - const onSelectThread = jest.fn(); - - renderWithReviewState( - , - { - commentThreads: [ - {id: 5, subjectType: 'ContentElement', subjectId: 10, comments: []} - ] - } - ); - - await new Promise(resolve => { - window.postMessage({ - type: 'SELECT_COMMENT_THREAD', - payload: {threadId: 5} - }, '*'); - setTimeout(resolve, 0); - }); - - expect(scrollIntoView).toHaveBeenCalled(); - expect(onSelectThread).toHaveBeenCalledWith(5); - }); - - it('ignores messages for non-matching threadIds', async () => { - const onSelectThread = jest.fn(); - - renderWithReviewState( - , - { - commentThreads: [ - {id: 5, subjectType: 'ContentElement', subjectId: 10, comments: []} - ] - } - ); - - await new Promise(resolve => { - window.postMessage({ - type: 'SELECT_COMMENT_THREAD', - payload: {threadId: 999} - }, '*'); - setTimeout(resolve, 0); - }); - - expect(scrollIntoView).not.toHaveBeenCalled(); - expect(onSelectThread).not.toHaveBeenCalled(); - }); - }); }); diff --git a/entry_types/scrolled/package/src/frontend/inlineEditing/ContentElementDecorator.js b/entry_types/scrolled/package/src/frontend/inlineEditing/ContentElementDecorator.js index b7eebc7e05..d1fcff2d53 100644 --- a/entry_types/scrolled/package/src/frontend/inlineEditing/ContentElementDecorator.js +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/ContentElementDecorator.js @@ -1,9 +1,10 @@ -import React from 'react'; +import React, {useCallback, useRef} from 'react'; import {useDrag} from 'react-dnd'; import {features} from 'pageflow/frontend'; import {ThreadsBadge} from 'pageflow-scrolled/review'; import {useContentElementEditorState} from '../useContentElementEditorState'; +import {useSelectCommentThreadHandler} from './useSelectCommentThreadHandler'; import {useI18n} from '../i18n'; import {api} from '../api'; import {widths} from '../layouts'; @@ -55,12 +56,26 @@ function DefaultSelectionRect(props) { const commentsSelected = type === 'contentElementComments' || type === 'newThread'; const {t} = useI18n({locale: 'ui'}); + const selectionRectRef = useRef(); + + useSelectCommentThreadHandler({ + subjectType: 'ContentElement', + subjectId: props.permaId, + getScrollTarget: useCallback(() => selectionRectRef.current, []), + selectThread: useCallback(threadId => selectComments({ + type: 'contentElementComments', + id: props.id, + highlightedThreadId: threadId + }), [selectComments, props.id]) + }); + const [, drag, preview] = useDrag({ item: { type: 'contentElement', id: props.id } }); return ( - threads.length === 0 ? selectNewThread() : - selectComments()} - onSelectThread={threadId => selectComments({ - type: 'contentElementComments', - id: props.id, - highlightedThreadId: threadId - })} />} + selectComments()} />} commentBadgeInset={!isSelected} ariaLabel={t('pageflow_scrolled.inline_editing.select_content_element')} insertButtonTitles={t('pageflow_scrolled.inline_editing.insert_content_element')} diff --git a/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/BadgeColumn.js b/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/BadgeColumn.js index 217b29e2c8..85087db01b 100644 --- a/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/BadgeColumn.js +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/BadgeColumn.js @@ -7,7 +7,6 @@ import {useSlate, ReactEditor} from 'slate-react'; import {Badge, useAnchoredFloating} from 'pageflow-scrolled/review'; import {useFloatingPortalRoot} from '../../FloatingPortalRootProvider'; import {useContentElementAttributes} from '../../useContentElementAttributes'; -import {usePostMessageListener} from '../../../shared/usePostMessageListener'; import {useEditorSelection} from '../EditorState'; import {highlightOverlapsSelection} from './highlightOverlapsSelection'; @@ -68,16 +67,6 @@ function PositionedBadge({editor, highlight, highlights, editorSelection, anchor }); }, [editor, highlight, selectComments, contentElementId]); - usePostMessageListener(useCallback(data => { - if (data.type === 'SELECT_COMMENT_THREAD' && - data.payload.threadId === highlight.thread?.id) { - if (refs.floating.current) { - refs.floating.current.scrollIntoView({block: 'nearest', behavior: 'smooth'}); - } - handleClick(); - } - }, [highlight, handleClick, refs.floating])); - if (!hasAnchor) return null; const isHighlightedThread = !!highlight.thread && diff --git a/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/useCommenting.js b/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/useCommenting.js index c9e9064c58..5ea955d7c9 100644 --- a/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/useCommenting.js +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/useCommenting.js @@ -1,5 +1,7 @@ import React, {useCallback, useMemo} from 'react'; import classNames from 'classnames'; +import {Range, Transforms} from 'slate'; +import {ReactEditor} from 'slate-react'; import {features} from 'pageflow/frontend'; import { @@ -13,6 +15,7 @@ import { import {useContentElementAttributes} from '../../useContentElementAttributes'; import {useEditorSelection} from '../EditorState'; +import {useSelectCommentThreadHandler} from '../useSelectCommentThreadHandler'; import {useCommentRangeRefs} from './useCommentRangeRefs'; // Bundles all commenting-related state and render helpers for the @@ -20,12 +23,16 @@ import {useCommentRangeRefs} from './useCommentRangeRefs'; // disabled for the current content element; consumers can then skip // the commenting-specific decorate/BadgeColumn paths. export function useCommenting(editor) { - const {contentElementPermaId, inlineComments} = useContentElementAttributes(); + const {contentElementId, contentElementPermaId, inlineComments} = + useContentElementAttributes(); const enabled = features.isEnabled('commenting') && inlineComments; + // Track ranges for resolved threads too, so their subject ranges keep + // following live edits and stay correct once a thread is reopened. + // Resolved threads are merely hidden from the highlight overlay until + // they become the highlighted thread (see `visibleThreads`). const threads = useCommentThreads( - enabled ? {subjectType: 'ContentElement', subjectId: contentElementPermaId} : null, - {resolved: false} + enabled ? {subjectType: 'ContentElement', subjectId: contentElementPermaId} : null ); const {trackedThreads, resetRangeRefs, getTrackedSubjectRanges} = @@ -37,8 +44,46 @@ export function useCommenting(editor) { subjectId: contentElementPermaId }); + const commentsSelectionOptions = useMemo( + () => ({type: 'contentElementComments', id: contentElementId}), + [contentElementId] + ); + const {select: selectComments, selection: commentsSelection} = + useEditorSelection(commentsSelectionOptions); + const highlightedThreadId = commentsSelection?.highlightedThreadId; + + // Move the cursor into the thread's block before the handler selects + // it, so `Selection`'s `cursorLeftHighlightedThreadBlock` does not + // treat a pre-existing cursor as having left the comment and re-select + // the content element, which would hide the highlight again. Routing + // through the handler also reveals a resolved thread (via + // `visibleThreads`) when it is selected. + useSelectCommentThreadHandler({ + subjectType: 'ContentElement', + subjectId: contentElementPermaId, + getScrollTarget: useCallback(threadId => { + const range = getTrackedSubjectRanges()[threadId]; + return range ? domElementAtRangeStart(editor, range) : null; + }, [editor, getTrackedSubjectRanges]), + beforeSelect: useCallback(threadId => { + const range = getTrackedSubjectRanges()[threadId]; + + if (range) { + Transforms.select(editor, Range.start(range)); + } + }, [editor, getTrackedSubjectRanges]), + selectThread: useCallback(threadId => selectComments({ + type: 'contentElementComments', + id: contentElementId, + highlightedThreadId: threadId + }), [selectComments, contentElementId]) + }); + + const visibleThreads = + trackedThreads.filter(t => !t.resolvedAt || t.id === highlightedThreadId); + const highlights = useCommentHighlights( - trackedThreads, + visibleThreads, newThreadActive ? newThreadRange : undefined ); @@ -71,7 +116,7 @@ export function useCommenting(editor) { // re-rendering leaves whose decorations changed because its memo // equality function does not compare `decorations`. // eslint-disable-next-line react-hooks/exhaustive-deps - }, [registerAnchor, contentElementPermaId, threads, newThreadRange]); + }, [registerAnchor, contentElementPermaId, threads, newThreadRange, highlightedThreadId]); return { enabled, @@ -84,6 +129,25 @@ export function useCommenting(editor) { }; } +// A resolved thread has no badge yet to scroll itself into view, so the +// commented text is scrolled directly. The leaf DOM already exists, so +// this resolves even before the highlight re-renders. +function domElementAtRangeStart(editor, range) { + try { + const start = Range.start(range); + const domRange = ReactEditor.toDOMRange(editor, {anchor: start, focus: start}); + const {startContainer} = domRange; + + return startContainer.nodeType === Node.ELEMENT_NODE ? + startContainer : + startContainer.parentElement; + } + catch (e) { + // toDOMRange throws when the range is not currently rendered. + return null; + } +} + function HighlightSpan({rangeKey, children}) { const {contentElementId, contentElementPermaId} = useContentElementAttributes(); const threadId = parseInt(rangeKey, 10); diff --git a/entry_types/scrolled/package/src/frontend/inlineEditing/SectionDecorator.js b/entry_types/scrolled/package/src/frontend/inlineEditing/SectionDecorator.js index 2528a26f8e..0785df95ea 100644 --- a/entry_types/scrolled/package/src/frontend/inlineEditing/SectionDecorator.js +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/SectionDecorator.js @@ -1,4 +1,4 @@ -import React from 'react'; +import React, {useCallback, useRef} from 'react'; import classNames from 'classnames'; import styles from './SectionDecorator.module.css'; import backdropStyles from './BackdropDecorator.module.css'; @@ -11,6 +11,7 @@ import {ThreadsBadge, useCommentThreads} from 'pageflow-scrolled/review'; import {Toolbar} from './Toolbar'; import {ForcePaddingContext} from '../Foreground'; import {useEditorSelection} from './EditorState'; +import {useSelectCommentThreadHandler} from './useSelectCommentThreadHandler'; import {MotifAreaVisibilityProvider} from '../MotifAreaVisibilityProvider'; import {useI18n} from '../i18n'; @@ -54,6 +55,19 @@ export function SectionDecorator({backdrop, section, contentElements, transition ); const hasThreads = threads.length > 0; + const wrapperRef = useRef(); + + useSelectCommentThreadHandler({ + subjectType: 'Section', + subjectId: section.permaId, + getScrollTarget: useCallback(() => wrapperRef.current, []), + selectThread: useCallback(threadId => selectComments({ + type: 'sectionComments', + id: section.id, + highlightedThreadId: threadId + }), [selectComments, section.id]) + }); + const clipBadgeCorner = (isSectionSelected || isPaddingSelected) && !hasThreads; const {isSelected: isBackdropElementSelected} = useEditorSelection({ @@ -96,7 +110,8 @@ export function SectionDecorator({backdrop, section, contentElements, transition } return ( -
@@ -122,12 +137,7 @@ export function SectionDecorator({backdrop, section, contentElements, transition subjectId={section.permaId} mode={commentsSelected ? 'active' : isSelected ? 'icon' : 'dot'} - onClick={() => hasThreads ? selectComments() : selectNewThread()} - onSelectThread={threadId => selectComments({ - type: 'sectionComments', - id: section.id, - highlightedThreadId: threadId - })} /> + onClick={() => hasThreads ? selectComments() : selectNewThread()} />
}
); diff --git a/entry_types/scrolled/package/src/frontend/inlineEditing/SelectionRect.js b/entry_types/scrolled/package/src/frontend/inlineEditing/SelectionRect.js index 80d09eb0af..1d3d742c15 100644 --- a/entry_types/scrolled/package/src/frontend/inlineEditing/SelectionRect.js +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/SelectionRect.js @@ -1,4 +1,4 @@ -import React, {useState} from 'react'; +import React, {forwardRef, useState} from 'react'; import classNames from 'classnames'; import {Toolbar} from './Toolbar'; @@ -8,9 +8,10 @@ import styles from './SelectionRect.module.css'; import PlusIcon from './images/plus.svg'; import MoveIcon from './images/move.svg'; -export function SelectionRect(props) { +export const SelectionRect = forwardRef(function SelectionRect(props, ref) { return ( -
); -} +}); function InsertButton(props) { const [insertHovered, setInsertHovered] = useState(false); diff --git a/entry_types/scrolled/package/src/frontend/inlineEditing/useSelectCommentThreadHandler.js b/entry_types/scrolled/package/src/frontend/inlineEditing/useSelectCommentThreadHandler.js new file mode 100644 index 0000000000..78758254c9 --- /dev/null +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/useSelectCommentThreadHandler.js @@ -0,0 +1,38 @@ +import {useCallback} from 'react'; + +import {useCommentThreads} from 'pageflow-scrolled/review'; +import {usePostMessageListener} from '../../shared/usePostMessageListener'; + +// Handles the SELECT_COMMENT_THREAD message the editor posts when a +// thread is clicked in the comments sidebar. Fetches the subject's +// threads itself - including resolved ones, so a resolved thread can be +// revealed when clicked - ignores threads of other subjects, scrolls +// the relevant element into view and calls `selectThread` to make the +// matching editor selection. `beforeSelect` runs right before that, for +// callers that need to prepare for it (e.g. move the editor cursor into +// the thread's block). Shared by the content element and section +// decorators and the EditableText editor so the handling lives in one +// place instead of inside each badge. +export function useSelectCommentThreadHandler({ + subjectType, subjectId, getScrollTarget, beforeSelect, selectThread +}) { + const threads = useCommentThreads({subjectType, subjectId}); + + usePostMessageListener(useCallback(data => { + if (data.type !== 'SELECT_COMMENT_THREAD') return; + + const {threadId} = data.payload; + + if (!threads.some(thread => thread.id === threadId)) return; + + const scrollTarget = getScrollTarget && getScrollTarget(threadId); + + if (scrollTarget) { + scrollTarget.scrollIntoView({block: 'nearest', behavior: 'smooth'}); + } + + if (beforeSelect) beforeSelect(threadId); + + selectThread(threadId); + }, [threads, getScrollTarget, beforeSelect, selectThread])); +} diff --git a/entry_types/scrolled/package/src/review/ThreadsBadge.js b/entry_types/scrolled/package/src/review/ThreadsBadge.js index 2b7f255adb..dea6be35b1 100644 --- a/entry_types/scrolled/package/src/review/ThreadsBadge.js +++ b/entry_types/scrolled/package/src/review/ThreadsBadge.js @@ -1,46 +1,14 @@ -import React, {useCallback, useRef} from 'react'; +import React, {useCallback} from 'react'; -import {usePostMessageListener} from '../shared/usePostMessageListener'; import {useCommentThreads} from './ReviewStateProvider'; import {Badge} from './Badge'; -export function ThreadsBadge({subjectType, subjectId, subjectRange, onClick, onSelectThread, mode}) { +export function ThreadsBadge({subjectType, subjectId, subjectRange, onClick, mode}) { const threads = useCommentThreads({subjectType, subjectId, subjectRange}, {resolved: false}); - const ref = useRef(); const handleClick = useCallback(() => { if (onClick) onClick(threads); }, [onClick, threads]); - return ( - <> - {threads.length > 0 && - } - - - ); -} - -// Side-effect-only host for the SELECT_COMMENT_THREAD listener. Only -// rendered while the badge has threads, so the listener never attaches -// for empty badges. Returns null because all the work happens in the -// effect. -function SelectThreadListener({threads, badgeRef, onSelectThread}) { - usePostMessageListener(useCallback(data => { - if (data.type !== 'SELECT_COMMENT_THREAD') return; - const threadId = data.payload.threadId; - if (!threads.some(t => t.id === threadId)) return; - - // Prefer scrolling an enclosing selectable (e.g. SelectionRect with - // aria-selected) into view so the user sees surrounding context, - // not just the badge anchored at the side. - const scrollTarget = badgeRef.current?.closest('[aria-selected]') || badgeRef.current; - if (scrollTarget) scrollTarget.scrollIntoView({block: 'nearest', behavior: 'smooth'}); - - if (onSelectThread) onSelectThread(threadId); - }, [threads, badgeRef, onSelectThread])); - - return null; + return ; } From 20de455a95dadfc32673e5a3d599628b9bff4eb7 Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Thu, 18 Jun 2026 11:20:41 +0200 Subject: [PATCH 08/18] Extract content element comment selection into a hook The contentElementComments and newThread editor selections were set up with near-identical useEditorSelection calls in useCommenting, HighlightSpan, BadgeColumn, Selection and useStartNewThread. Extract a generic useCommentSelection hook (with a useContentElementCommentSelection convenience wrapper) that bundles both - mutually exclusive since the editor has one selection at a time - and exposes a normalized state (selected, highlightedThreadId, newThreadRange) plus selectThread, selectComments and startNewThread actions. Route the EditableText sites and the section decorator through it, dropping the duplicated selection setup and the literal option objects. The handler receives selectThread directly. The content element decorator keeps using ContentElementEditorStateProvider, which already aggregates these selections. --- .../inlineEditing/EditableText/BadgeColumn.js | 28 +++------ .../inlineEditing/EditableText/Selection.js | 24 ++------ .../cursorLeftHighlightedThreadBlock.js | 6 +- .../EditableText/useCommenting.js | 43 +++---------- .../EditableText/useStartNewThread.js | 17 ++---- .../inlineEditing/SectionDecorator.js | 22 ++++--- .../inlineEditing/useCommentSelection.js | 60 +++++++++++++++++++ 7 files changed, 101 insertions(+), 99 deletions(-) create mode 100644 entry_types/scrolled/package/src/frontend/inlineEditing/useCommentSelection.js diff --git a/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/BadgeColumn.js b/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/BadgeColumn.js index 85087db01b..f2b4d88b3d 100644 --- a/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/BadgeColumn.js +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/BadgeColumn.js @@ -6,8 +6,7 @@ import {useSlate, ReactEditor} from 'slate-react'; import {Badge, useAnchoredFloating} from 'pageflow-scrolled/review'; import {useFloatingPortalRoot} from '../../FloatingPortalRootProvider'; -import {useContentElementAttributes} from '../../useContentElementAttributes'; -import {useEditorSelection} from '../EditorState'; +import {useContentElementCommentSelection} from '../useCommentSelection'; import {highlightOverlapsSelection} from './highlightOverlapsSelection'; import styles from './BadgeColumn.module.css'; @@ -33,13 +32,8 @@ export function BadgeColumn({highlights, anchors}) { function PositionedBadge({editor, highlight, highlights, editorSelection, anchors}) { const portalRoot = useFloatingPortalRoot(); - const {contentElementId, contentElementPermaId} = useContentElementAttributes(); - const {select: selectComments, selection: commentsSelection} = useEditorSelection({ - type: 'contentElementComments', id: contentElementId - }); - const {isSelected: newThreadActive} = useEditorSelection({ - type: 'newThread', subjectType: 'ContentElement', subjectId: contentElementPermaId - }); + const {selected, highlightedThreadId, selectComments, selectThread} = + useContentElementCommentSelection(); const {refs, floatingStyles, hasAnchor} = useAnchoredFloating(highlight.key, anchors, {placement: 'left-start'}); @@ -60,27 +54,23 @@ function PositionedBadge({editor, highlight, highlights, editorSelection, anchor // stays correct. Transforms.select(editor, Range.start(highlight.range)); - selectComments({ - type: 'contentElementComments', - id: contentElementId, - highlightedThreadId: highlight.thread?.id - }); - }, [editor, highlight, selectComments, contentElementId]); + selectThread(highlight.thread?.id); + }, [editor, highlight, selectComments, selectThread]); if (!hasAnchor) return null; const isHighlightedThread = !!highlight.thread && - commentsSelection?.highlightedThreadId === highlight.thread.id; + highlightedThreadId === highlight.thread.id; const isActive = isHighlightedThread || - (highlight.key === 'selection' && newThreadActive); + (highlight.key === 'selection' && selected === 'newThread'); // When a thread is highlighted, fall back to its start point for the // overlap check so siblings in the same block stay in regular mode // even if focus has drifted away from the slate editor. Use just the // start point (not the full range) to stay consistent with // highlightOverlapsSelection, which anchors to highlight starts. - const highlightedRange = commentsSelection?.highlightedThreadId ? + const highlightedRange = highlightedThreadId ? highlights.find( - h => h.thread?.id === commentsSelection.highlightedThreadId + h => h.thread?.id === highlightedThreadId )?.range : null; const fallbackPoint = highlightedRange && Range.start(highlightedRange); 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 08a398f4d7..064909295b 100644 --- a/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/Selection.js +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/Selection.js @@ -1,4 +1,4 @@ -import React, {useEffect, useMemo, useRef} from 'react'; +import React, {useEffect, useRef} from 'react'; import {Editor, Transforms, Node} from 'slate'; import {useSlate, ReactEditor} from 'slate-react'; import {useDrag} from 'react-dnd'; @@ -6,11 +6,10 @@ import {useDrag} from 'react-dnd'; import styles from './index.module.css'; import {SelectionRect} from '../SelectionRect'; -import {useContentElementAttributes} from '../../useContentElementAttributes'; import {useContentElementEditorState} from '../../useContentElementEditorState'; import {useI18n} from '../../i18n'; import {postInsertContentElementMessage} from '../postMessage'; -import {useEditorSelection} from '../EditorState'; +import {useContentElementCommentSelection} from '../useCommentSelection'; import {cursorLeftHighlightedThreadBlock} from './cursorLeftHighlightedThreadBlock'; import {cursorMovedFromPendingNewThreadRange} from './cursorMovedFromPendingNewThreadRange'; import {getUniformSelectedNode} from './getUniformSelectedNode'; @@ -44,20 +43,9 @@ export function Selection(props) { range } = useContentElementEditorState(); - const {contentElementPermaId} = useContentElementAttributes(); - - const {selection: commentsSelection} = useEditorSelection( - useMemo( - () => ({type: 'contentElementComments', id: props.contentElementId}), - [props.contentElementId] - ) - ); - const {isSelected: newThreadActive, range: newThreadRange} = useEditorSelection( - useMemo( - () => ({type: 'newThread', subjectType: 'ContentElement', subjectId: contentElementPermaId}), - [contentElementPermaId] - ) - ); + const {selected, highlightedThreadId, newThreadRange} = + useContentElementCommentSelection(); + const newThreadActive = selected === 'newThread'; useEffect(() => { const {selection} = editor; @@ -118,7 +106,7 @@ export function Selection(props) { if (type === 'contentElementComments' && cursorLeftHighlightedThreadBlock({ - editor, commentsSelection, highlights: props.highlights + editor, highlightedThreadId, highlights: props.highlights })) { select(); return; diff --git a/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/cursorLeftHighlightedThreadBlock.js b/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/cursorLeftHighlightedThreadBlock.js index 5c37111581..0675add69a 100644 --- a/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/cursorLeftHighlightedThreadBlock.js +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/cursorLeftHighlightedThreadBlock.js @@ -3,13 +3,13 @@ // thread should keep the highlight in place (so the comment can be // referenced while editing); only block changes mean the user moved // past the comment's anchor. -export function cursorLeftHighlightedThreadBlock({editor, commentsSelection, highlights}) { - if (!commentsSelection?.highlightedThreadId || !highlights) { +export function cursorLeftHighlightedThreadBlock({editor, highlightedThreadId, highlights}) { + if (!highlightedThreadId || !highlights) { return false; } const highlightedRange = highlights.find( - h => h.thread?.id === commentsSelection.highlightedThreadId + h => h.thread?.id === highlightedThreadId )?.range; if (!highlightedRange) return false; diff --git a/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/useCommenting.js b/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/useCommenting.js index 5ea955d7c9..d1184e4122 100644 --- a/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/useCommenting.js +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/useCommenting.js @@ -14,7 +14,7 @@ import { } from 'pageflow-scrolled/review'; import {useContentElementAttributes} from '../../useContentElementAttributes'; -import {useEditorSelection} from '../EditorState'; +import {useContentElementCommentSelection} from '../useCommentSelection'; import {useSelectCommentThreadHandler} from '../useSelectCommentThreadHandler'; import {useCommentRangeRefs} from './useCommentRangeRefs'; @@ -23,8 +23,7 @@ import {useCommentRangeRefs} from './useCommentRangeRefs'; // disabled for the current content element; consumers can then skip // the commenting-specific decorate/BadgeColumn paths. export function useCommenting(editor) { - const {contentElementId, contentElementPermaId, inlineComments} = - useContentElementAttributes(); + const {contentElementPermaId, inlineComments} = useContentElementAttributes(); const enabled = features.isEnabled('commenting') && inlineComments; // Track ranges for resolved threads too, so their subject ranges keep @@ -38,19 +37,8 @@ export function useCommenting(editor) { const {trackedThreads, resetRangeRefs, getTrackedSubjectRanges} = useCommentRangeRefs(editor, threads); const {anchors, registerAnchor} = useRangeAnchors(); - const {isSelected: newThreadActive, range: newThreadRange} = useEditorSelection({ - type: 'newThread', - subjectType: 'ContentElement', - subjectId: contentElementPermaId - }); - - const commentsSelectionOptions = useMemo( - () => ({type: 'contentElementComments', id: contentElementId}), - [contentElementId] - ); - const {select: selectComments, selection: commentsSelection} = - useEditorSelection(commentsSelectionOptions); - const highlightedThreadId = commentsSelection?.highlightedThreadId; + const {highlightedThreadId, newThreadRange, selectThread} = + useContentElementCommentSelection(); // Move the cursor into the thread's block before the handler selects // it, so `Selection`'s `cursorLeftHighlightedThreadBlock` does not @@ -72,20 +60,13 @@ export function useCommenting(editor) { Transforms.select(editor, Range.start(range)); } }, [editor, getTrackedSubjectRanges]), - selectThread: useCallback(threadId => selectComments({ - type: 'contentElementComments', - id: contentElementId, - highlightedThreadId: threadId - }), [selectComments, contentElementId]) + selectThread }); const visibleThreads = trackedThreads.filter(t => !t.resolvedAt || t.id === highlightedThreadId); - const highlights = useCommentHighlights( - visibleThreads, - newThreadActive ? newThreadRange : undefined - ); + const highlights = useCommentHighlights(visibleThreads, newThreadRange); const decorate = useMemo( () => enabled ? decorateCommentHighlights(editor, highlights) : null, @@ -149,16 +130,10 @@ function domElementAtRangeStart(editor, range) { } function HighlightSpan({rangeKey, children}) { - const {contentElementId, contentElementPermaId} = useContentElementAttributes(); const threadId = parseInt(rangeKey, 10); - const {isSelected: commentsSelected, selection: commentsSelection} = useEditorSelection({ - type: 'contentElementComments', id: contentElementId - }); - const {isSelected: newThreadActive} = useEditorSelection({ - type: 'newThread', subjectType: 'ContentElement', subjectId: contentElementPermaId - }); - const isSelected = (commentsSelected && commentsSelection?.highlightedThreadId === threadId) || - (rangeKey === 'selection' && newThreadActive); + const {selected, highlightedThreadId} = useContentElementCommentSelection(); + const isSelected = (selected === 'comments' && highlightedThreadId === threadId) || + (rangeKey === 'selection' && selected === 'newThread'); return ( selectNewThread({ - type: 'newThread', - subjectType: 'ContentElement', - subjectId: contentElementPermaId, - range: editor.selection - }); + return () => selectNewThread(editor.selection); } diff --git a/entry_types/scrolled/package/src/frontend/inlineEditing/SectionDecorator.js b/entry_types/scrolled/package/src/frontend/inlineEditing/SectionDecorator.js index 0785df95ea..16cd7437d8 100644 --- a/entry_types/scrolled/package/src/frontend/inlineEditing/SectionDecorator.js +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/SectionDecorator.js @@ -11,6 +11,7 @@ import {ThreadsBadge, useCommentThreads} from 'pageflow-scrolled/review'; import {Toolbar} from './Toolbar'; import {ForcePaddingContext} from '../Foreground'; import {useEditorSelection} from './EditorState'; +import {useCommentSelection} from './useCommentSelection'; import {useSelectCommentThreadHandler} from './useSelectCommentThreadHandler'; import {MotifAreaVisibilityProvider} from '../MotifAreaVisibilityProvider'; import {useI18n} from '../i18n'; @@ -31,19 +32,20 @@ export function SectionDecorator({backdrop, section, contentElements, transition type: 'sectionPaddings' }); - const {isSelected: isCommentsSelected, select: selectComments} = useEditorSelection({ + const { + selected: commentSelectionState, + selectComments, + selectThread, + selectNewThread + } = useCommentSelection({ + type: 'sectionComments', id: section.id, - type: 'sectionComments' - }); - - const {isSelected: isNewThreadSelected, select: selectNewThread} = useEditorSelection({ - type: 'newThread', subjectType: 'Section', subjectId: section.permaId }); // Viewing a section's comments or composing a new thread on it. - const commentsSelected = isCommentsSelected || isNewThreadSelected; + const commentsSelected = commentSelectionState !== null; // The section reads as selected while its comments are open, so the // section and the sidebar comment panel stay visually in sync. @@ -61,11 +63,7 @@ export function SectionDecorator({backdrop, section, contentElements, transition subjectType: 'Section', subjectId: section.permaId, getScrollTarget: useCallback(() => wrapperRef.current, []), - selectThread: useCallback(threadId => selectComments({ - type: 'sectionComments', - id: section.id, - highlightedThreadId: threadId - }), [selectComments, section.id]) + selectThread }); const clipBadgeCorner = (isSectionSelected || isPaddingSelected) && !hasThreads; diff --git a/entry_types/scrolled/package/src/frontend/inlineEditing/useCommentSelection.js b/entry_types/scrolled/package/src/frontend/inlineEditing/useCommentSelection.js new file mode 100644 index 0000000000..b85385a72a --- /dev/null +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/useCommentSelection.js @@ -0,0 +1,60 @@ +import {useCallback, useMemo} from 'react'; + +import {useContentElementAttributes} from '../useContentElementAttributes'; +import {useEditorSelection} from './EditorState'; + +// Bundles an element's two comment-related editor selections - the +// highlighted comment thread and the pending new thread - which are +// mutually exclusive since the editor has a single selection at a time. +// Returns the active state plus actions to select the comments, a +// specific thread or a new thread. `type` is the comments selection type +// ('contentElementComments' or 'sectionComments'); `subjectType` and +// `subjectId` identify the new-thread subject. +export function useCommentSelection({type, id, subjectType, subjectId}) { + const { + isSelected: commentsSelected, + selection: commentsSelection, + select: selectComments + } = useEditorSelection(useMemo(() => ({type, id}), [type, id])); + + const { + isSelected: newThreadActive, + range: newThreadRange, + select: selectNewThreadSelection + } = useEditorSelection(useMemo( + () => ({type: 'newThread', subjectType, subjectId}), + [subjectType, subjectId] + )); + + const selectThread = useCallback( + threadId => selectComments({type, id, highlightedThreadId: threadId}), + [selectComments, type, id] + ); + + const selectNewThread = useCallback( + range => range ? + selectNewThreadSelection({type: 'newThread', subjectType, subjectId, range}) : + selectNewThreadSelection(), + [selectNewThreadSelection, subjectType, subjectId] + ); + + return { + selected: commentsSelected ? 'comments' : newThreadActive ? 'newThread' : null, + highlightedThreadId: commentsSelection?.highlightedThreadId, + newThreadRange: newThreadActive ? newThreadRange : undefined, + selectComments, + selectThread, + selectNewThread + }; +} + +export function useContentElementCommentSelection() { + const {contentElementId, contentElementPermaId} = useContentElementAttributes(); + + return useCommentSelection({ + type: 'contentElementComments', + id: contentElementId, + subjectType: 'ContentElement', + subjectId: contentElementPermaId + }); +} From 2b7f563d89a8e61faff82bc4672b362cc5dc8f87 Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Thu, 18 Jun 2026 14:10:57 +0200 Subject: [PATCH 09/18] Show resolved comment threads in the selection comments sidebar Feed all tracked threads (resolved included) into the highlights that Selection turns into commentThreadIdsAtSelection, so the selection comments sidebar scopes to resolved threads of the cursor's block and shows them collapsed. Keep decoration and badges on the visible subset, so resolved threads stay hidden in the editor until highlighted. --- .../views/SelectionCommentsView-spec.js | 33 ++++++++++++- .../features/commentSelection-spec.js | 47 ++++++++++++++++++- .../inlineEditing/EditableText/index.js | 3 +- .../EditableText/useCommenting.js | 22 ++++++--- 4 files changed, 96 insertions(+), 9 deletions(-) diff --git a/entry_types/scrolled/package/spec/editor/views/SelectionCommentsView-spec.js b/entry_types/scrolled/package/spec/editor/views/SelectionCommentsView-spec.js index 087aa82b3a..7c0a873dc1 100644 --- a/entry_types/scrolled/package/spec/editor/views/SelectionCommentsView-spec.js +++ b/entry_types/scrolled/package/spec/editor/views/SelectionCommentsView-spec.js @@ -21,7 +21,9 @@ describe('SelectionCommentsView', () => { 'pageflow_scrolled.review.add_comment_placeholder': 'Add a comment...', 'pageflow_scrolled.review.new_topic': 'New topic', 'pageflow_scrolled.review.send': 'Send', - 'pageflow_scrolled.review.no_threads_yet': 'No comments yet' + 'pageflow_scrolled.review.no_threads_yet': 'No comments yet', + 'pageflow_scrolled.review.resolved_count.one': '1 resolved', + 'pageflow_scrolled.review.resolved_count.other': '%{count} resolved' }); it('displays threads of selected content element from session state', () => { @@ -230,6 +232,35 @@ describe('SelectionCommentsView', () => { expect(listener).toHaveBeenCalledWith(7); }); + it('shows resolved threads collapsed when their ids are at the selection', async () => { + const user = userEvent.setup(); + + const entry = createEntry({ + contentElements: [{id: 1, permaId: 10, typeName: 'fixture'}] + }); + entry.set('selectedCommentsSubject', {subjectType: 'ContentElement', id: 1}); + entry.contentElements.get(1).transientState + .set('commentThreadIdsAtSelection', [7]); + entry.reviewSession = factories.reviewSession({ + commentThreads: [{ + id: 7, + subjectType: 'ContentElement', + subjectId: 10, + resolvedAt: '2026-06-01T00:00:00Z', + comments: [{id: 100, body: 'Resolved comment', creatorName: 'Alice'}] + }] + }); + + const view = new SelectionCommentsView({entry, editor}); + const {getByText, getByRole, queryByText} = renderBackboneView(view); + + expect(queryByText('Resolved comment')).not.toBeInTheDocument(); + + await user.click(getByRole('button', {name: '1 resolved'})); + + expect(getByText('Resolved comment')).toBeInTheDocument(); + }); + it('does not highlight or trigger selectCommentThread when not scoped', async () => { const user = userEvent.setup(); diff --git a/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/features/commentSelection-spec.js b/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/features/commentSelection-spec.js index 8a5aaf4f97..53862dcdd9 100644 --- a/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/features/commentSelection-spec.js +++ b/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/features/commentSelection-spec.js @@ -5,7 +5,7 @@ import {EditableText} from 'frontend'; import {useStartNewThread} from 'frontend/inlineEditing/EditableText/useStartNewThread'; import {renderEntry, useInlineEditingPageObjects} from 'support/pageObjects/inlineEditing'; -import {act, fireEvent} from '@testing-library/react'; +import {act, fireEvent, waitFor} from '@testing-library/react'; import '@testing-library/jest-dom/extend-expect'; describe('inline editing EditableText comment selection messages', () => { @@ -134,6 +134,51 @@ describe('inline editing EditableText comment selection messages', () => { delete Element.prototype.scrollIntoView; }); + it('includes resolved thread ids at the selection in transient state', async () => { + const value = [{type: 'paragraph', children: [{text: 'Some text to comment on'}]}]; + + renderEntry({ + contentElement: { + ui: , + typeOptions: {inlineComments: true, customSelectionRect: true} + }, + commenting: { + currentUser: null, + commentThreads: [ + {id: 1, subjectType: 'ContentElement', subjectId: 10, + subjectRange: {anchor: {path: [0, 0], offset: 0}, focus: {path: [0, 0], offset: 4}}, + comments: [{id: 10, body: 'Active', creatorName: 'Alice', creatorId: 1}]}, + {id: 7, subjectType: 'ContentElement', subjectId: 10, + subjectRange: {anchor: {path: [0, 0], offset: 5}, focus: {path: [0, 0], offset: 9}}, + resolvedAt: '2026-06-01T00:00:00Z', + comments: [{id: 20, body: 'Resolved', creatorName: 'Bob', creatorId: 2}]} + ] + } + }); + + act(() => { + window.dispatchEvent(new MessageEvent('message', { + data: {type: 'SELECT', payload: {type: 'contentElement', id: 1, range: [0, 1]}}, + origin: window.location.origin + })); + }); + + await waitFor(() => { + expect(window.parent.postMessage).toHaveBeenCalledWith( + expect.objectContaining({ + type: 'UPDATE_TRANSIENT_CONTENT_ELEMENT_STATE', + payload: expect.objectContaining({ + id: 1, + state: expect.objectContaining({ + commentThreadIdsAtSelection: expect.arrayContaining([1, 7]) + }) + }) + }), + expect.anything() + ); + }); + }); + it('scrolls a resolved thread into view when selected via SELECT_COMMENT_THREAD', () => { const scrollIntoView = jest.fn(); Element.prototype.scrollIntoView = scrollIntoView; diff --git a/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/index.js b/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/index.js index f664069136..a6a87ea693 100644 --- a/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/index.js +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/index.js @@ -105,6 +105,7 @@ export const EditableText = React.memo(function EditableText({ enabled: commentingEnabled, anchors, highlights, + visibleHighlights, decorate: decorateComments, withCommentHighlightDecoration, resetRangeRefs, @@ -201,7 +202,7 @@ export const EditableText = React.memo(function EditableText({ {commentingEnabled && <> - + } diff --git a/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/useCommenting.js b/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/useCommenting.js index d1184e4122..dc99e189b8 100644 --- a/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/useCommenting.js +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/useCommenting.js @@ -63,14 +63,23 @@ export function useCommenting(editor) { selectThread }); - const visibleThreads = - trackedThreads.filter(t => !t.resolvedAt || t.id === highlightedThreadId); - - const highlights = useCommentHighlights(visibleThreads, newThreadRange); + // Build highlights for all tracked threads, resolved included, so the + // thread ids at the cursor (which scope the comments sidebar) cover + // resolved threads too. Only `visibleHighlights` get a text overlay and + // a badge; a resolved thread stays hidden until it is the highlighted + // thread. + const highlights = useCommentHighlights(trackedThreads, newThreadRange); + + const visibleHighlights = useMemo( + () => highlights.filter( + h => !h.thread?.resolvedAt || h.thread.id === highlightedThreadId + ), + [highlights, highlightedThreadId] + ); const decorate = useMemo( - () => enabled ? decorateCommentHighlights(editor, highlights) : null, - [editor, highlights, enabled] + () => enabled ? decorateCommentHighlights(editor, visibleHighlights) : null, + [editor, visibleHighlights, enabled] ); const withCommentHighlightDecoration = useCallback(({attributes, children, leaf}) => { @@ -102,6 +111,7 @@ export function useCommenting(editor) { return { enabled, highlights, + visibleHighlights, anchors, decorate, withCommentHighlightDecoration, From a927797a9c2fe1b63f6a58cc4c02809d7351f223 Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Thu, 18 Jun 2026 14:12:53 +0200 Subject: [PATCH 10/18] Resolve badge overlap selection once per render The overlap selection (live cursor, or the highlighted thread's start when the editor is unfocused) is identical for every badge, so compute it once in BadgeColumn instead of searching the highlights for the highlighted thread inside each PositionedBadge. --- .../inlineEditing/EditableText/BadgeColumn.js | 36 ++++++++++--------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/BadgeColumn.js b/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/BadgeColumn.js index f2b4d88b3d..0f80407831 100644 --- a/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/BadgeColumn.js +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/BadgeColumn.js @@ -13,6 +13,8 @@ import styles from './BadgeColumn.module.css'; export function BadgeColumn({highlights, anchors}) { const editor = useSlate(); + const {highlightedThreadId} = useContentElementCommentSelection(); + // Treat `editor.selection` as a live cursor only while the editor // is focused. After the user clicks away, slate-react's throttled // `selectionchange` listener can sync a clamped DOM cursor back @@ -20,17 +22,32 @@ export function BadgeColumn({highlights, anchors}) { // to overlap mode without any actual selection. const editorSelection = ReactEditor.isFocused(editor) ? editor.selection : null; + // When a thread is highlighted, fall back to its start point for the + // overlap check so siblings in the same block stay in regular mode + // even if focus has drifted away from the slate editor. Use just the + // start point (not the full range) to stay consistent with + // highlightOverlapsSelection, which anchors to highlight starts. The + // overlap selection is the same for every badge, so resolve it once + // here rather than per badge. + const highlightedRange = highlightedThreadId ? + highlights.find( + h => h.thread?.id === highlightedThreadId + )?.range : + null; + const fallbackPoint = highlightedRange && Range.start(highlightedRange); + const overlapSelection = editorSelection || + (fallbackPoint && {anchor: fallbackPoint, focus: fallbackPoint}); + return highlights.map(highlight => ( )); } -function PositionedBadge({editor, highlight, highlights, editorSelection, anchors}) { +function PositionedBadge({editor, highlight, overlapSelection, anchors}) { const portalRoot = useFloatingPortalRoot(); const {selected, highlightedThreadId, selectComments, selectThread} = useContentElementCommentSelection(); @@ -63,19 +80,6 @@ function PositionedBadge({editor, highlight, highlights, editorSelection, anchor highlightedThreadId === highlight.thread.id; const isActive = isHighlightedThread || (highlight.key === 'selection' && selected === 'newThread'); - // When a thread is highlighted, fall back to its start point for the - // overlap check so siblings in the same block stay in regular mode - // even if focus has drifted away from the slate editor. Use just the - // start point (not the full range) to stay consistent with - // highlightOverlapsSelection, which anchors to highlight starts. - const highlightedRange = highlightedThreadId ? - highlights.find( - h => h.thread?.id === highlightedThreadId - )?.range : - null; - const fallbackPoint = highlightedRange && Range.start(highlightedRange); - const overlapSelection = editorSelection || - (fallbackPoint && {anchor: fallbackPoint, focus: fallbackPoint}); const mode = isActive ? 'active' : highlightOverlapsSelection(highlight, overlapSelection) ? undefined : 'dot'; From ddd052aa84e98553e74c10b9eb81a73928889cc6 Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Thu, 18 Jun 2026 14:43:59 +0200 Subject: [PATCH 11/18] Render temporarily shown resolved comment highlight and badge in grey A resolved thread revealed from the comments sidebar shows its inline highlight and badge only while it is the highlighted thread. Mute both to grey, instead of the accent color, so they read as resolved - matching how the comments sidebar de-emphasizes resolved threads. --- .../stylesheets/pageflow/ui/properties.scss | 6 +++ .../features/commentBadges-spec.js | 40 +++++++++++++++++++ .../features/commentHighlights-spec.js | 8 ++-- .../spec/support/pageObjects/inlineEditing.js | 1 + .../inlineEditing/EditableText/BadgeColumn.js | 5 ++- .../EditableText/useCommenting.js | 10 +++-- .../scrolled/package/src/review/Badge.js | 5 ++- .../package/src/review/Badge.module.css | 12 ++++++ .../package/src/review/commentHighlights.js | 1 + .../src/review/commentHighlights.module.css | 8 ++++ 10 files changed, 86 insertions(+), 10 deletions(-) diff --git a/app/assets/stylesheets/pageflow/ui/properties.scss b/app/assets/stylesheets/pageflow/ui/properties.scss index 7aa24054fa..6b32f6fad3 100644 --- a/app/assets/stylesheets/pageflow/ui/properties.scss +++ b/app/assets/stylesheets/pageflow/ui/properties.scss @@ -63,6 +63,12 @@ --ui-accent-color-lightest: hsla(44, 100%, 65%, 0.1); --ui-on-accent-color: #795f0f; + --ui-accent-grey-color: hsl(44, 0%, 75%); + --ui-accent-grey-color-light: hsla(44, 0%, 75%, 0.6); + --ui-accent-grey-color-lighter: hsla(44, 0%, 75%, 0.3); + --ui-accent-grey-color-lightest: hsla(44, 0%, 75%, 0.1); + --ui-on-accent-grey-color: hsl(44, 0%, 27%); + --ui-on-button-color: var(--ui-primary-color); --ui-button-border-color: var(--ui-primary-color-light); --ui-button-hover-border-color: var(--ui-primary-color); diff --git a/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/features/commentBadges-spec.js b/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/features/commentBadges-spec.js index 17a63940f3..8c004e8392 100644 --- a/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/features/commentBadges-spec.js +++ b/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/features/commentBadges-spec.js @@ -4,6 +4,7 @@ import {features} from 'pageflow/frontend'; import {EditableText} from 'frontend'; import {renderEntry, useInlineEditingPageObjects} from 'support/pageObjects/inlineEditing'; +import {act, waitFor} from '@testing-library/react'; import '@testing-library/jest-dom/extend-expect'; describe('inline editing EditableText comment badges', () => { @@ -76,6 +77,45 @@ describe('inline editing EditableText comment badges', () => { expect(badges[1].isActive()).toBe(false); }); + it('renders a revealed resolved thread badge in resolved style', async () => { + const value = [{type: 'paragraph', children: [{text: 'Some text to comment on'}]}]; + + const entry = renderEntry({ + contentElement: { + ui: , + typeOptions: {inlineComments: true, customSelectionRect: true} + }, + commenting: { + currentUser: null, + commentThreads: [{ + id: 7, + subjectType: 'ContentElement', + subjectId: 10, + subjectRange: {anchor: {path: [0, 0], offset: 5}, focus: {path: [0, 0], offset: 9}}, + resolvedAt: '2026-06-01T00:00:00Z', + comments: [{id: 1, body: 'A comment', creatorName: 'Alice', creatorId: 1}] + }] + } + }); + + expect(entry.queryAllCommentBadges()).toHaveLength(0); + + act(() => { + window.dispatchEvent(new MessageEvent('message', { + data: {type: 'SELECT_COMMENT_THREAD', payload: {threadId: 7}}, + origin: window.location.origin + })); + }); + + await waitFor(() => { + expect(entry.queryAllCommentBadges()).toHaveLength(1); + }); + + const badge = entry.queryAllCommentBadges()[0]; + expect(badge.isResolved()).toBe(true); + expect(badge.isActive()).toBe(true); + }); + it('renders sibling badge in regular mode when in same block as highlighted thread', () => { const value = [ {type: 'paragraph', children: [{text: 'First paragraph with two threads'}]}, diff --git a/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/features/commentHighlights-spec.js b/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/features/commentHighlights-spec.js index cfa29a1698..f9fbe41b35 100644 --- a/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/features/commentHighlights-spec.js +++ b/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/features/commentHighlights-spec.js @@ -115,8 +115,9 @@ describe('inline editing EditableText comment highlights', () => { .toBeInTheDocument(); }); - expect(entry.container.querySelector(`.${highlightStyles.highlight}`)) - .toHaveClass(highlightStyles.selected); + const highlight = entry.container.querySelector(`.${highlightStyles.highlight}`); + expect(highlight).toHaveClass(highlightStyles.resolved); + expect(highlight).not.toHaveClass(highlightStyles.selected); }); it('keeps the resolved thread highlighted when a cursor sits in another block', () => { @@ -162,7 +163,8 @@ describe('inline editing EditableText comment highlights', () => { const highlight = entry.container.querySelector(`.${highlightStyles.highlight}`); expect(highlight).toBeInTheDocument(); expect(highlight).toHaveTextContent('Second'); - expect(highlight).toHaveClass(highlightStyles.selected); + expect(highlight).toHaveClass(highlightStyles.resolved); + expect(highlight).not.toHaveClass(highlightStyles.selected); }); it('applies selected style to highlight when thread badge is clicked', () => { diff --git a/entry_types/scrolled/package/spec/support/pageObjects/inlineEditing.js b/entry_types/scrolled/package/spec/support/pageObjects/inlineEditing.js index 4a0ef5f91f..5829320f4b 100644 --- a/entry_types/scrolled/package/spec/support/pageObjects/inlineEditing.js +++ b/entry_types/scrolled/package/spec/support/pageObjects/inlineEditing.js @@ -147,6 +147,7 @@ function createCommentBadgePageObject(el) { el, isInDotMode: () => el.classList.contains(badgeStyles.dot), isActive: () => el.classList.contains(badgeStyles.active), + isResolved: () => el.classList.contains(badgeStyles.resolved), select: () => fireEvent.click(el) }; } diff --git a/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/BadgeColumn.js b/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/BadgeColumn.js index 0f80407831..cc65948d6d 100644 --- a/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/BadgeColumn.js +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/BadgeColumn.js @@ -87,7 +87,10 @@ function PositionedBadge({editor, highlight, overlapSelection, anchors}) { return (
- +
); diff --git a/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/useCommenting.js b/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/useCommenting.js index dc99e189b8..e3a4c0fe21 100644 --- a/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/useCommenting.js +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/useCommenting.js @@ -85,7 +85,7 @@ export function useCommenting(editor) { const withCommentHighlightDecoration = useCallback(({attributes, children, leaf}) => { if (leaf.commentHighlight) { children = ( - + {children} ); @@ -139,15 +139,17 @@ function domElementAtRangeStart(editor, range) { } } -function HighlightSpan({rangeKey, children}) { +function HighlightSpan({rangeKey, resolved, children}) { const threadId = parseInt(rangeKey, 10); const {selected, highlightedThreadId} = useContentElementCommentSelection(); const isSelected = (selected === 'comments' && highlightedThreadId === threadId) || (rangeKey === 'selection' && selected === 'newThread'); return ( - + {children} ); diff --git a/entry_types/scrolled/package/src/review/Badge.js b/entry_types/scrolled/package/src/review/Badge.js index 57e0c6bdc2..0cc0f23295 100644 --- a/entry_types/scrolled/package/src/review/Badge.js +++ b/entry_types/scrolled/package/src/review/Badge.js @@ -4,7 +4,7 @@ import classNames from 'classnames'; import CommentIcon from './images/comment.svg'; import styles from './Badge.module.css'; -export const Badge = forwardRef(function Badge({counter, mode, onClick}, ref) { +export const Badge = forwardRef(function Badge({counter, mode, resolved, onClick}, ref) { const variant = resolveVariant(mode, counter > 0); if (!variant) { @@ -14,7 +14,8 @@ export const Badge = forwardRef(function Badge({counter, mode, onClick}, ref) { return (
); diff --git a/entry_types/scrolled/package/src/review/commentHighlights.js b/entry_types/scrolled/package/src/review/commentHighlights.js index 0deb9f0f01..2e9e6f3b6c 100644 --- a/entry_types/scrolled/package/src/review/commentHighlights.js +++ b/entry_types/scrolled/package/src/review/commentHighlights.js @@ -31,6 +31,10 @@ export function decorateCommentHighlights(editor, highlights) { Range.start(intersection), Range.start(highlight.range) ); + const isLast = Point.equals( + Range.end(intersection), + Range.end(highlight.range) + ); decorations.push({ ...intersection, @@ -38,7 +42,8 @@ export function decorateCommentHighlights(editor, highlights) { subjectRange: highlight.range, rangeKey: highlight.key, resolved: !!highlight.thread?.resolvedAt, - ...(isFirst && {firstInRange: true}) + ...(isFirst && {firstInRange: true}), + ...(isLast && {lastInRange: true}) }); } } From c5f1c15efac7e0c8956086c17f72c66999fa9558 Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Fri, 19 Jun 2026 08:11:33 +0200 Subject: [PATCH 14/18] Allow optional-chaining call statements in scrolled lint --- entry_types/scrolled/package/.eslintrc.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/entry_types/scrolled/package/.eslintrc.js b/entry_types/scrolled/package/.eslintrc.js index 01d6d9bc11..e58812dffc 100644 --- a/entry_types/scrolled/package/.eslintrc.js +++ b/entry_types/scrolled/package/.eslintrc.js @@ -6,7 +6,11 @@ module.exports = { "plugin:storybook/recommended" ], "rules": { - "no-trailing-spaces": "error" + "no-trailing-spaces": "error", + // react-app enables no-unused-expressions, but ESLint 6 predates + // ChainExpression and flags optional-chaining call statements like + // `node?.focus()` as unused. The other packages do not enable the rule. + "no-unused-expressions": "off" }, "settings": { "import/resolver": { From 6ffa6dc69f22336309c289e21f661be72cbd8803 Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Thu, 18 Jun 2026 17:52:47 +0200 Subject: [PATCH 15/18] Focus new comment field without scrolling the page Replace the textarea's autoFocus with an explicit focus({preventScroll: true}) so focusing the field does not yank the page to the top before the portaled popover has been positioned by floating-ui. --- .../scrolled/package/src/review/NewThreadForm.js | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/entry_types/scrolled/package/src/review/NewThreadForm.js b/entry_types/scrolled/package/src/review/NewThreadForm.js index 32f823820c..01472bc34d 100644 --- a/entry_types/scrolled/package/src/review/NewThreadForm.js +++ b/entry_types/scrolled/package/src/review/NewThreadForm.js @@ -1,4 +1,4 @@ -import React, {useState} from 'react'; +import React, {useCallback, useState} from 'react'; import {useI18n} from '../frontend/i18n'; import {postCreateCommentThreadMessage} from './postMessage'; @@ -11,6 +11,13 @@ export function NewThreadForm({subjectType, subjectId, subjectRange, onSubmit, o const {t} = useI18n({locale: 'ui'}); const [body, setBody] = useState(''); + // preventScroll keeps focus from yanking the page to the top before the + // portaled popover has been positioned by floating-ui. + const setInputRef = useCallback(node => { + autoResize(node); + node?.focus({preventScroll: true}); + }, []); + function handleChange(e) { setBody(e.target.value); autoGrow(e.target); @@ -29,8 +36,7 @@ export function NewThreadForm({subjectType, subjectId, subjectRange, onSubmit, o return (