From 50a63ff1d311e6e1d9e618ce78d219ad9cc4e0f7 Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Mon, 22 Jun 2026 10:54:04 +0200 Subject: [PATCH 1/4] Add useEntryStructureWithContentElements hook Nests an ordered contentElements array (including backdrop elements) into each section of the entry structure. Kept as a separate hook from useEntryStructure so the more frequently used variant does not re-derive when content elements change. Both share a buildEntryStructure helper so the nested shape stays consistent. --- .../package/spec/entryState/structure-spec.js | 88 ++++++++++++ .../scrolled/package/src/entryState/index.js | 1 + .../package/src/entryState/structure.js | 126 +++++++++++++----- 3 files changed, 178 insertions(+), 37 deletions(-) diff --git a/entry_types/scrolled/package/spec/entryState/structure-spec.js b/entry_types/scrolled/package/spec/entryState/structure-spec.js index f5b9354237..5518088871 100644 --- a/entry_types/scrolled/package/spec/entryState/structure-spec.js +++ b/entry_types/scrolled/package/spec/entryState/structure-spec.js @@ -1,5 +1,6 @@ import { useEntryStructure, + useEntryStructureWithContentElements, useSectionsWithChapter, useChapter, useChapters, @@ -274,6 +275,93 @@ describe('useEntryStructure', () => { }); }); +describe('useEntryStructureWithContentElements', () => { + it('nests ordered content elements under each section', () => { + const {result} = renderHookInEntry( + () => useEntryStructureWithContentElements(), + { + seed: { + storylines: storylinesSeed, + chapters: chaptersSeed, + sections: sectionsSeed, + contentElements: contentElementsSeed + } + } + ); + + expect(result.current.main[0].sections[0].contentElements).toMatchObject([ + {id: 1, permaId: 1001, sectionId: 1, type: 'heading'}, + {id: 2, permaId: 1002, sectionId: 1, type: 'textBlock'} + ]); + }); + + it('nests content elements under excursion sections', () => { + const {result} = renderHookInEntry( + () => useEntryStructureWithContentElements(), + { + seed: { + storylines: storylinesSeed, + chapters: chaptersSeed, + sections: sectionsSeed, + contentElements: [ + ...contentElementsSeed, + {id: 6, permaId: 1006, sectionId: 3, typeName: 'textBlock', configuration: {}} + ] + } + } + ); + + expect(result.current.excursions[0].sections[0].contentElements).toMatchObject([ + {permaId: 1006, type: 'textBlock'} + ]); + }); + + it('includes content elements with backdrop position', () => { + const {result} = renderHookInEntry( + () => useEntryStructureWithContentElements(), + { + seed: { + sections: [{id: 1, permaId: 101}], + contentElements: [ + {id: 1, permaId: 1001, sectionId: 1, typeName: 'image', + configuration: {position: 'backdrop'}} + ] + } + } + ); + + expect(result.current.main[0].sections[0].contentElements).toMatchObject([ + {permaId: 1001, type: 'image'} + ]); + }); + + it('keeps the fields useEntryStructure returns on sections and chapters', () => { + const {result} = renderHookInEntry( + () => useEntryStructureWithContentElements(), + { + seed: { + storylines: storylinesSeed, + chapters: chaptersSeed, + sections: sectionsSeed, + contentElements: contentElementsSeed + } + } + ); + + expect(result.current.main[0]).toMatchObject({ + permaId: 10, + title: 'Chapter 1', + isExcursion: false + }); + expect(result.current.main[0].sections[0]).toMatchObject({ + permaId: 101, + sectionIndex: 0, + transition: 'scroll' + }); + expect(result.current.mainSectionsCount).toBe(2); + }); +}); + describe('useSectionsWithChapter', () => { it('returns sections with nested chapter object', () => { const {result} = renderHookInEntry( diff --git a/entry_types/scrolled/package/src/entryState/index.js b/entry_types/scrolled/package/src/entryState/index.js index bcdfc57d24..0f68e2d249 100644 --- a/entry_types/scrolled/package/src/entryState/index.js +++ b/entry_types/scrolled/package/src/entryState/index.js @@ -11,6 +11,7 @@ export { export { normalizeSectionConfigurationData, useEntryStructure, + useEntryStructureWithContentElements, useSectionsWithChapter, useSection, useMainChapters, diff --git a/entry_types/scrolled/package/src/entryState/structure.js b/entry_types/scrolled/package/src/entryState/structure.js index f5c78bcc98..2748b3c9ab 100644 --- a/entry_types/scrolled/package/src/entryState/structure.js +++ b/entry_types/scrolled/package/src/entryState/structure.js @@ -45,48 +45,100 @@ export function useEntryStructure() { const chapters = useChapters(); const sections = useEntryStateCollectionItems('sections'); + return useMemo( + () => buildEntryStructure({mainStoryline, chapters, sections}), + [mainStoryline, chapters, sections] + ); +}; + +/** + * Like {@link useEntryStructure}, but additionally nests an ordered + * `contentElements` array (including backdrop elements) into each + * section. Kept separate so the more frequently used + * `useEntryStructure` does not re-derive when content elements change. + * + * @private + */ +export function useEntryStructureWithContentElements() { + const mainStoryline = useMainStoryline(); + const chapters = useChapters(); + const sections = useEntryStateCollectionItems('sections'); + const contentElements = useEntryStateCollectionItems('contentElements'); + return useMemo(() => { - const enrichedSections = sections.map(section => sectionData(section)); - - const main = []; - const excursions = []; - - chapters.forEach(chapter => { - const chapterSections = enrichedSections.filter( - item => item.chapterId === chapter.id - ); - - const isExcursion = chapter.storylineId !== mainStoryline.id; - - chapter = { - ...chapter, - isExcursion, - sections: chapterSections - }; - - chapterSections.forEach(section => - section.chapter = chapter - ); - - if (isExcursion) { - excursions.push(chapter); - } - else { - main.push(chapter); - } + const contentElementsBySectionId = {}; + + contentElements.forEach(contentElement => { + const sectionContentElements = + contentElementsBySectionId[contentElement.sectionId] || + (contentElementsBySectionId[contentElement.sectionId] = []); + + sectionContentElements.push(contentElementSubjectData(contentElement)); }); - const mainSections = main.flatMap(chapter => chapter.sections); - linkAndIndexSections(mainSections); - excursions.forEach(excursion => linkAndIndexSections(excursion.sections)); + return buildEntryStructure({ + mainStoryline, + chapters, + sections, + contentElementsBySectionId + }); + }, [mainStoryline, chapters, sections, contentElements]); +}; + +function buildEntryStructure({mainStoryline, chapters, sections, contentElementsBySectionId}) { + const enrichedSections = sections.map(section => ({ + ...sectionData(section), + ...(contentElementsBySectionId && + {contentElements: contentElementsBySectionId[section.id] || []}) + })); + + const main = []; + const excursions = []; + + chapters.forEach(chapter => { + const chapterSections = enrichedSections.filter( + item => item.chapterId === chapter.id + ); - return { - main, - excursions, - mainSectionsCount: mainSections.length + const isExcursion = chapter.storylineId !== mainStoryline.id; + + chapter = { + ...chapter, + isExcursion, + sections: chapterSections + }; + + chapterSections.forEach(section => + section.chapter = chapter + ); + + if (isExcursion) { + excursions.push(chapter); } - }, [mainStoryline, chapters, sections]); -}; + else { + main.push(chapter); + } + }); + + const mainSections = main.flatMap(chapter => chapter.sections); + linkAndIndexSections(mainSections); + excursions.forEach(excursion => linkAndIndexSections(excursion.sections)); + + return { + main, + excursions, + mainSectionsCount: mainSections.length + }; +} + +function contentElementSubjectData(contentElement) { + return { + id: contentElement.id, + permaId: contentElement.permaId, + sectionId: contentElement.sectionId, + type: contentElement.typeName + }; +} function linkAndIndexSections(sections) { sections.forEach((section, index) => { From 8d41a4f4dae0cfcc799aa6123f7ff5c166e2db90 Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Mon, 22 Jun 2026 12:09:01 +0200 Subject: [PATCH 2/4] Let useCommentThreads take a single options object MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the positional (subject, {resolved}) signature with one {subjectType, subjectId, subjectRange, resolution} options object. Omitting the subject fields returns all threads, and resolution ('all' | 'resolved' | 'unresolved', default 'all') filters by resolved state — so threads can be filtered by resolution alone. Subject-only call sites keep working unchanged. --- .../spec/review/ReviewStateProvider-spec.js | 69 ++++++++++++++++++- .../src/frontend/commenting/EditableText.js | 9 +-- .../frontend/commenting/SectionDecorator.js | 9 +-- .../EditableText/useCommenting.js | 10 ++- .../inlineEditing/SectionDecorator.js | 9 +-- .../package/src/review/ReviewStateProvider.js | 25 ++++--- .../scrolled/package/src/review/ThreadList.js | 4 +- .../package/src/review/ThreadsBadge.js | 2 +- 8 files changed, 107 insertions(+), 30 deletions(-) diff --git a/entry_types/scrolled/package/spec/review/ReviewStateProvider-spec.js b/entry_types/scrolled/package/spec/review/ReviewStateProvider-spec.js index 017086ebb1..b0ac76a525 100644 --- a/entry_types/scrolled/package/spec/review/ReviewStateProvider-spec.js +++ b/entry_types/scrolled/package/spec/review/ReviewStateProvider-spec.js @@ -59,9 +59,52 @@ describe('ReviewStateProvider', () => { expect(result.current[0].id).toBe(1); }); - it('filters by resolved option', () => { + it('returns all threads when no subject is given', () => { const {result} = renderHook( - () => useCommentThreads({subjectType: 'CE', subjectId: 10}, {resolved: false}), + () => useCommentThreads(), + { + wrapper: ({children}) => ( + + {children} + + ) + } + ); + + expect(result.current.map(t => t.id)).toEqual([1, 2]); + }); + + it('filters all threads by resolution when no subject is given', () => { + const {result} = renderHook( + () => useCommentThreads({resolution: 'unresolved'}), + { + wrapper: ({children}) => ( + + {children} + + ) + } + ); + + expect(result.current.map(t => t.id)).toEqual([1, 3]); + }); + + it('filters by resolution unresolved', () => { + const {result} = renderHook( + () => useCommentThreads({subjectType: 'CE', subjectId: 10, resolution: 'unresolved'}), { wrapper: ({children}) => ( { expect(result.current.map(t => t.id)).toEqual([1, 3]); }); + it('filters by resolution resolved', () => { + const {result} = renderHook( + () => useCommentThreads({subjectType: 'CE', subjectId: 10, resolution: 'resolved'}), + { + wrapper: ({children}) => ( + + {children} + + ) + } + ); + + expect(result.current.map(t => t.id)).toEqual([2, 3]); + }); + it('updates single thread on thread change message', async () => { const {result, waitForNextUpdate} = renderHook( () => useCommentThreads({subjectType: 'CE', subjectId: 10}), diff --git a/entry_types/scrolled/package/src/frontend/commenting/EditableText.js b/entry_types/scrolled/package/src/frontend/commenting/EditableText.js index 31fb77e6cb..c36930e432 100644 --- a/entry_types/scrolled/package/src/frontend/commenting/EditableText.js +++ b/entry_types/scrolled/package/src/frontend/commenting/EditableText.js @@ -40,10 +40,11 @@ function CommentingEditableText({ const {contentElementPermaId} = useContentElementAttributes(); const {active, deactivate, preselect, clearPreselection} = useAddCommentMode(); const {subjectRange, select} = useSelectedSubject('ContentElement', contentElementPermaId); - const threads = useCommentThreads( - {subjectType: 'ContentElement', subjectId: contentElementPermaId}, - {resolved: false} - ); + const threads = useCommentThreads({ + subjectType: 'ContentElement', + subjectId: contentElementPermaId, + resolution: 'unresolved' + }); const highlights = useCommentHighlights(threads, subjectRange); diff --git a/entry_types/scrolled/package/src/frontend/commenting/SectionDecorator.js b/entry_types/scrolled/package/src/frontend/commenting/SectionDecorator.js index 52bca34cad..b0688d0115 100644 --- a/entry_types/scrolled/package/src/frontend/commenting/SectionDecorator.js +++ b/entry_types/scrolled/package/src/frontend/commenting/SectionDecorator.js @@ -13,10 +13,11 @@ 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 threads = useCommentThreads({ + subjectType: 'Section', + subjectId: section.permaId, + resolution: 'unresolved' + }); const hasThreads = threads.length > 0; 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 e3a4c0fe21..f22d30e624 100644 --- a/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/useCommenting.js +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/useCommenting.js @@ -18,6 +18,8 @@ import {useContentElementCommentSelection} from '../useCommentSelection'; import {useSelectCommentThreadHandler} from '../useSelectCommentThreadHandler'; import {useCommentRangeRefs} from './useCommentRangeRefs'; +const noThreads = []; + // Bundles all commenting-related state and render helpers for the // EditableText editor. Returns `enabled: false` when commenting is // disabled for the current content element; consumers can then skip @@ -30,9 +32,11 @@ export function useCommenting(editor) { // 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 - ); + const elementThreads = useCommentThreads({ + subjectType: 'ContentElement', + subjectId: contentElementPermaId + }); + const threads = enabled ? elementThreads : noThreads; const {trackedThreads, resetRangeRefs, getTrackedSubjectRanges} = useCommentRangeRefs(editor, threads); diff --git a/entry_types/scrolled/package/src/frontend/inlineEditing/SectionDecorator.js b/entry_types/scrolled/package/src/frontend/inlineEditing/SectionDecorator.js index fb67b92966..5bf4b33c8c 100644 --- a/entry_types/scrolled/package/src/frontend/inlineEditing/SectionDecorator.js +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/SectionDecorator.js @@ -51,10 +51,11 @@ export function SectionDecorator({backdrop, section, contentElements, transition // 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 threads = useCommentThreads({ + subjectType: 'Section', + subjectId: section.permaId, + resolution: 'unresolved' + }); const hasThreads = threads.length > 0; const wrapperRef = useRef(); diff --git a/entry_types/scrolled/package/src/review/ReviewStateProvider.js b/entry_types/scrolled/package/src/review/ReviewStateProvider.js index a43de4811d..f61a0b3804 100644 --- a/entry_types/scrolled/package/src/review/ReviewStateProvider.js +++ b/entry_types/scrolled/package/src/review/ReviewStateProvider.js @@ -44,24 +44,29 @@ export function useCommentThread(threadId) { return context?.commentThreads.find(t => t.id === threadId); } -export function useCommentThreads(subject, {resolved} = {}) { +export function useCommentThreads({subjectType, subjectId, subjectRange, resolution = 'all'} = {}) { const context = useContext(ReviewStateContext); const commentThreads = context ? context.commentThreads : []; - const {subjectType, subjectId, subjectRange} = subject || {}; + const hasSubject = subjectType !== undefined; return useMemo(() => { const rangeKey = subjectRange ? JSON.stringify(subjectRange) : undefined; return commentThreads.filter( - thread => thread.subjectType === subjectType && - thread.subjectId === subjectId && - (!rangeKey || - JSON.stringify(thread.subjectRange) === rangeKey) && - (resolved === undefined || - (resolved === false && !thread.resolvedAt) || - (resolved === true && !!thread.resolvedAt)) + thread => (!hasSubject || + (thread.subjectType === subjectType && + thread.subjectId === subjectId && + (!rangeKey || + JSON.stringify(thread.subjectRange) === rangeKey))) && + matchesResolution(thread, resolution) ); - }, [commentThreads, subjectType, subjectId, subjectRange, resolved]); + }, [commentThreads, hasSubject, subjectType, subjectId, subjectRange, resolution]); +} + +function matchesResolution(thread, resolution) { + return resolution === 'all' || + (resolution === 'unresolved' && !thread.resolvedAt) || + (resolution === 'resolved' && !!thread.resolvedAt); } function initState(initialState) { diff --git a/entry_types/scrolled/package/src/review/ThreadList.js b/entry_types/scrolled/package/src/review/ThreadList.js index 733bdfe52f..99c4f6e137 100644 --- a/entry_types/scrolled/package/src/review/ThreadList.js +++ b/entry_types/scrolled/package/src/review/ThreadList.js @@ -13,8 +13,8 @@ import styles from './ThreadList.module.css'; export function ThreadList({subjectType, subjectId, subjectRange, filter, compareRanges, highlightedThreadId, onThreadClick, restrictInteractionsToHighlighted, showNewForm: showNewFormProp, hideNewTopicButton, reversed}) { const {t} = useI18n({locale: 'ui'}); - const allActiveThreads = useCommentThreads({subjectType, subjectId, subjectRange}, {resolved: false}); - const allResolvedThreads = useCommentThreads({subjectType, subjectId, subjectRange}, {resolved: true}); + const allActiveThreads = useCommentThreads({subjectType, subjectId, subjectRange, resolution: 'unresolved'}); + const allResolvedThreads = useCommentThreads({subjectType, subjectId, subjectRange, resolution: 'resolved'}); const activeThreads = useMemo( () => sortByRange(filter ? allActiveThreads.filter(filter) : allActiveThreads, compareRanges), diff --git a/entry_types/scrolled/package/src/review/ThreadsBadge.js b/entry_types/scrolled/package/src/review/ThreadsBadge.js index dea6be35b1..6179cfa013 100644 --- a/entry_types/scrolled/package/src/review/ThreadsBadge.js +++ b/entry_types/scrolled/package/src/review/ThreadsBadge.js @@ -4,7 +4,7 @@ import {useCommentThreads} from './ReviewStateProvider'; import {Badge} from './Badge'; export function ThreadsBadge({subjectType, subjectId, subjectRange, onClick, mode}) { - const threads = useCommentThreads({subjectType, subjectId, subjectRange}, {resolved: false}); + const threads = useCommentThreads({subjectType, subjectId, subjectRange, resolution: 'unresolved'}); const handleClick = useCallback(() => { if (onClick) onClick(threads); From 7f68dd59e8e036f77659a9b0929eb26bad6e8175 Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Mon, 22 Jun 2026 10:54:04 +0200 Subject: [PATCH 3/4] Add useLocatedCommentThreads hook Joins comment threads from the review state with the entry structure, grouping them by chapter, section and content element (main storyline first, excursions last). Returns the located chapters, a flat list of threads in document order for the upcoming preview navigator, and the threads whose subject is no longer part of the entry. --- .../review/useLocatedCommentThreads-spec.js | 110 ++++++++++++++++++ .../scrolled/package/src/review/index.js | 1 + .../src/review/useLocatedCommentThreads.js | 78 +++++++++++++ 3 files changed, 189 insertions(+) create mode 100644 entry_types/scrolled/package/spec/review/useLocatedCommentThreads-spec.js create mode 100644 entry_types/scrolled/package/src/review/useLocatedCommentThreads.js diff --git a/entry_types/scrolled/package/spec/review/useLocatedCommentThreads-spec.js b/entry_types/scrolled/package/spec/review/useLocatedCommentThreads-spec.js new file mode 100644 index 0000000000..20130e3dba --- /dev/null +++ b/entry_types/scrolled/package/spec/review/useLocatedCommentThreads-spec.js @@ -0,0 +1,110 @@ +import React from 'react'; + +import {useLocatedCommentThreads} from 'review/useLocatedCommentThreads'; +import {ReviewStateProvider} from 'review/ReviewStateProvider'; + +import {renderHookInEntry} from 'support'; + +const storylinesSeed = [ + {id: 1, permaId: 10, position: 1, configuration: {main: true}}, + {id: 2, permaId: 11, position: 2, configuration: {}} +]; +const chaptersSeed = [ + {id: 1, permaId: 100, storylineId: 1, position: 1, configuration: {title: 'Main chapter'}}, + {id: 2, permaId: 200, storylineId: 2, position: 1, configuration: {title: 'Excursion chapter'}} +]; +const sectionsSeed = [ + {id: 1, permaId: 1000, chapterId: 1, position: 1}, + {id: 2, permaId: 2000, chapterId: 2, position: 1} +]; +const contentElementsSeed = [ + {id: 1, permaId: 10001, sectionId: 1, typeName: 'textBlock'}, + {id: 2, permaId: 10002, sectionId: 1, typeName: 'image'}, + {id: 3, permaId: 20001, sectionId: 2, typeName: 'image'} +]; + +function renderLocatedCommentThreads(commentThreads) { + return renderHookInEntry(() => useLocatedCommentThreads(), { + seed: { + storylines: storylinesSeed, + chapters: chaptersSeed, + sections: sectionsSeed, + contentElements: contentElementsSeed + }, + wrapper: ({children}) => ( + + {children} + + ) + }); +} + +describe('useLocatedCommentThreads', () => { + it('attaches threads to their content element and section', () => { + const {result} = renderLocatedCommentThreads([ + {id: 1, subjectType: 'ContentElement', subjectId: 10001, comments: []}, + {id: 2, subjectType: 'Section', subjectId: 1000, comments: []} + ]); + + const section = result.current.chapters[0].sections[0]; + + expect(section.threads.map(t => t.id)).toEqual([2]); + expect(section.contentElements[0].threads.map(t => t.id)).toEqual([1]); + expect(section.contentElements[1].threads).toEqual([]); + }); + + it('orders chapters main first then excursions and includes excursion threads', () => { + const {result} = renderLocatedCommentThreads([ + {id: 3, subjectType: 'ContentElement', subjectId: 20001, comments: []} + ]); + + const chapters = result.current.chapters; + + expect(chapters.map(c => c.title)).toEqual(['Main chapter', 'Excursion chapter']); + expect(chapters[0].isExcursion).toBe(false); + expect(chapters[1].isExcursion).toBe(true); + expect(chapters[1].sections[0].contentElements[0].threads.map(t => t.id)).toEqual([3]); + }); + + it('counts all threads of a chapter', () => { + const {result} = renderLocatedCommentThreads([ + {id: 1, subjectType: 'ContentElement', subjectId: 10001, comments: []}, + {id: 2, subjectType: 'Section', subjectId: 1000, comments: []}, + {id: 3, subjectType: 'ContentElement', subjectId: 20001, comments: []} + ]); + + expect(result.current.chapters[0].threadCount).toBe(2); + expect(result.current.chapters[1].threadCount).toBe(1); + }); + + it('provides a flat list of threads in document order, section before its elements', () => { + const {result} = renderLocatedCommentThreads([ + {id: 1, subjectType: 'ContentElement', subjectId: 10001, comments: []}, + {id: 2, subjectType: 'Section', subjectId: 1000, comments: []}, + {id: 3, subjectType: 'ContentElement', subjectId: 20001, comments: []} + ]); + + expect(result.current.threads.map(t => t.id)).toEqual([2, 1, 3]); + }); + + it('buckets threads with unknown subject into orphanedThreads', () => { + const {result} = renderLocatedCommentThreads([ + {id: 1, subjectType: 'ContentElement', subjectId: 10001, comments: []}, + {id: 4, subjectType: 'ContentElement', subjectId: 99999, comments: []} + ]); + + expect(result.current.orphanedThreads.map(t => t.id)).toEqual([4]); + expect(result.current.threads.map(t => t.id)).toEqual([1]); + }); + + it('attaches resolved threads as well', () => { + const {result} = renderLocatedCommentThreads([ + {id: 5, subjectType: 'ContentElement', subjectId: 10002, + resolvedAt: '2026-04-09', comments: []} + ]); + + const section = result.current.chapters[0].sections[0]; + + expect(section.contentElements[1].threads.map(t => t.id)).toEqual([5]); + }); +}); diff --git a/entry_types/scrolled/package/src/review/index.js b/entry_types/scrolled/package/src/review/index.js index 572254ff99..21fe72da8f 100644 --- a/entry_types/scrolled/package/src/review/index.js +++ b/entry_types/scrolled/package/src/review/index.js @@ -1,4 +1,5 @@ export {ReviewStateProvider, useCommentThreads, useCommentThread} from './ReviewStateProvider'; +export {useLocatedCommentThreads} from './useLocatedCommentThreads'; export {ReviewMessageHandler} from './ReviewMessageHandler'; export {ThreadsBadge} from './ThreadsBadge'; export {Badge} from './Badge'; diff --git a/entry_types/scrolled/package/src/review/useLocatedCommentThreads.js b/entry_types/scrolled/package/src/review/useLocatedCommentThreads.js new file mode 100644 index 0000000000..a72db984de --- /dev/null +++ b/entry_types/scrolled/package/src/review/useLocatedCommentThreads.js @@ -0,0 +1,78 @@ +import {useMemo} from 'react'; + +import {useEntryStructureWithContentElements} from 'pageflow-scrolled/entryState'; + +import {useCommentThreads} from './ReviewStateProvider'; + +/** + * Joins the comment threads from the review state with the entry + * structure so both the editor sidebar and the preview navigator can + * present threads grouped by their location in the entry. + * + * Returns the chapters (main storyline first, excursions last) with + * threads attached to the section or content element they belong to, + * a flat list of all located threads in document order, and the + * threads whose subject is no longer part of the entry. + * + * @private + */ +export function useLocatedCommentThreads() { + const structure = useEntryStructureWithContentElements(); + const allThreads = useCommentThreads(); + + return useMemo(() => { + const threadsBySubject = groupBySubject(allThreads); + const locatedThreads = new Set(); + const threads = []; + + const take = (subjectType, subjectId) => { + const subjectThreads = threadsBySubject[subjectKey(subjectType, subjectId)] || []; + subjectThreads.forEach(thread => locatedThreads.add(thread)); + threads.push(...subjectThreads); + return subjectThreads; + }; + + const locateChapter = chapter => { + const sections = chapter.sections.map(section => ({ + ...section, + threads: take('Section', section.permaId), + contentElements: section.contentElements.map(contentElement => ({ + ...contentElement, + threads: take('ContentElement', contentElement.permaId) + })) + })); + + return {...chapter, sections, threadCount: countThreads(sections)}; + }; + + const chapters = [...structure.main, ...structure.excursions].map(locateChapter); + const orphanedThreads = allThreads.filter(thread => !locatedThreads.has(thread)); + + return {chapters, threads, orphanedThreads}; + }, [structure, allThreads]); +} + +function countThreads(sections) { + return sections.reduce( + (count, section) => + count + + section.threads.length + + section.contentElements.reduce((sum, element) => sum + element.threads.length, 0), + 0 + ); +} + +function groupBySubject(threads) { + const result = {}; + + threads.forEach(thread => { + const key = subjectKey(thread.subjectType, thread.subjectId); + (result[key] || (result[key] = [])).push(thread); + }); + + return result; +} + +function subjectKey(subjectType, subjectId) { + return `${subjectType}:${subjectId}`; +} From 73cd2a615f00eb64ccf337af36a4838d2dbb0b38 Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Mon, 22 Jun 2026 11:08:20 +0200 Subject: [PATCH 4/4] Group entry comments by chapter with headings Render the comments sidebar from the entry structure instead of a Backbone collection of items: EntryCommentsView now provides entry state (seeded from the entry and kept live via watchCollections) and builds its groups from useLocatedCommentThreads. Each chapter that has threads gets a heading using the same number and title as the chapter outline, and excursion chapters are grouped below the main chapters. Selection, highlighting and transient-thread behavior is preserved. --- entry_types/scrolled/config/locales/de.yml | 1 + entry_types/scrolled/config/locales/en.yml | 1 + .../editor/views/EntryCommentsView-spec.js | 101 ++++++++++- .../src/editor/views/EntryCommentsView.js | 157 ++++++++++-------- .../editor/views/EntryCommentsView.module.css | 21 +++ .../src/editor/views/ReviewView.module.css | 1 - .../src/editor/views/SelectionCommentsView.js | 3 + .../views/SelectionCommentsView.module.css | 3 + 8 files changed, 220 insertions(+), 68 deletions(-) create mode 100644 entry_types/scrolled/package/src/editor/views/SelectionCommentsView.module.css diff --git a/entry_types/scrolled/config/locales/de.yml b/entry_types/scrolled/config/locales/de.yml index 222c06b496..4210f71956 100644 --- a/entry_types/scrolled/config/locales/de.yml +++ b/entry_types/scrolled/config/locales/de.yml @@ -167,6 +167,7 @@ de: drag_hint: Ziehen, um das Kapitel zu verschieben save_error: Beim Speichern des Kapitels ist ein Fehler aufgetreten. chapter: Kapitel + excursion: Exkurs unnamed: Unbenannt hidden_in_navigation: In der Navigationsleiste ausgeblendet common_content_element_attributes: diff --git a/entry_types/scrolled/config/locales/en.yml b/entry_types/scrolled/config/locales/en.yml index cabe3540d6..3e7b8f3a24 100644 --- a/entry_types/scrolled/config/locales/en.yml +++ b/entry_types/scrolled/config/locales/en.yml @@ -167,6 +167,7 @@ en: drag_hint: Drag to move chapter save_error: There was an error while saving this chapter. chapter: Chapter + excursion: Excursion unnamed: Untitled hidden_in_navigation: Hidden in navigation bar common_content_element_attributes: 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 7738cf7ec8..e80551e145 100644 --- a/entry_types/scrolled/package/spec/editor/views/EntryCommentsView-spec.js +++ b/entry_types/scrolled/package/spec/editor/views/EntryCommentsView-spec.js @@ -26,7 +26,106 @@ describe('EntryCommentsView', () => { 'pageflow_scrolled.review.send': 'Send', 'pageflow_scrolled.editor.content_elements.textBlock.name': 'Text', 'pageflow_scrolled.editor.content_elements.image.name': 'Image', - 'pageflow_scrolled.editor.comments_view.section': 'Section' + 'pageflow_scrolled.editor.comments_view.section': 'Section', + 'pageflow_scrolled.editor.chapter_item.chapter': 'Chapter', + 'pageflow_scrolled.editor.chapter_item.excursion': 'Excursion' + }); + + it('renders a chapter heading with number and title above its groups', () => { + const entry = createEntry({ + chapters: [ + {id: 1, permaId: 10, storylineId: 1000, position: 0, configuration: {title: 'Intro'}} + ], + sections: [{id: 1, permaId: 100, chapterId: 1, position: 0}], + contentElements: [{id: 1, permaId: 1000, sectionId: 1, typeName: 'image'}] + }); + entry.reviewSession = factories.reviewSession({ + commentThreads: [{ + id: 1, subjectType: 'ContentElement', subjectId: 1000, + comments: [{id: 100, body: 'A comment', creatorName: 'Alice'}] + }] + }); + + const view = new EntryCommentsView({entry, editor}); + const {getByText} = renderBackboneView(view); + + const heading = getByText('Intro'); + const comment = getByText('A comment'); + + expect(getByText('Chapter 1')).toBeInTheDocument(); + expect(heading.compareDocumentPosition(comment) & + Node.DOCUMENT_POSITION_FOLLOWING).toBeTruthy(); + }); + + it('groups excursion chapters below main chapters', () => { + const entry = createEntry({ + storylines: [ + {id: 1000, permaId: 100, position: 0, configuration: {main: true}}, + {id: 2000, permaId: 200, position: 1, configuration: {}} + ], + chapters: [ + {id: 1, permaId: 10, storylineId: 1000, position: 0, configuration: {title: 'Main chapter'}}, + {id: 2, permaId: 20, storylineId: 2000, position: 0, configuration: {title: 'My excursion'}} + ], + sections: [ + {id: 1, permaId: 100, chapterId: 1, position: 0}, + {id: 2, permaId: 200, chapterId: 2, position: 0} + ], + contentElements: [ + {id: 1, permaId: 1000, sectionId: 1, typeName: 'image'}, + {id: 2, permaId: 2000, sectionId: 2, typeName: 'image'} + ] + }); + entry.reviewSession = factories.reviewSession({ + commentThreads: [ + {id: 1, subjectType: 'ContentElement', subjectId: 1000, + comments: [{id: 1, body: 'in main', creatorName: 'A'}]}, + {id: 2, subjectType: 'ContentElement', subjectId: 2000, + comments: [{id: 2, body: 'in excursion', creatorName: 'B'}]} + ] + }); + + const view = new EntryCommentsView({entry, editor}); + const {getByText} = renderBackboneView(view); + + const mainComment = getByText('in main'); + const excursionHeading = getByText('My excursion'); + const excursionComment = getByText('in excursion'); + + expect(getByText('Excursion')).toBeInTheDocument(); + expect(mainComment.compareDocumentPosition(excursionHeading) & + Node.DOCUMENT_POSITION_FOLLOWING).toBeTruthy(); + expect(mainComment.compareDocumentPosition(excursionComment) & + Node.DOCUMENT_POSITION_FOLLOWING).toBeTruthy(); + }); + + it('does not render a heading for a chapter without threads', () => { + const entry = createEntry({ + chapters: [ + {id: 1, permaId: 10, storylineId: 1000, position: 0, configuration: {title: 'Has comments'}}, + {id: 2, permaId: 20, storylineId: 1000, position: 1, configuration: {title: 'Empty chapter'}} + ], + sections: [ + {id: 1, permaId: 100, chapterId: 1, position: 0}, + {id: 2, permaId: 200, chapterId: 2, position: 0} + ], + contentElements: [ + {id: 1, permaId: 1000, sectionId: 1, typeName: 'image'}, + {id: 2, permaId: 2000, sectionId: 2, typeName: 'image'} + ] + }); + entry.reviewSession = factories.reviewSession({ + commentThreads: [{ + id: 1, subjectType: 'ContentElement', subjectId: 1000, + comments: [{id: 1, body: 'a comment', creatorName: 'A'}] + }] + }); + + const view = new EntryCommentsView({entry, editor}); + const {getByText, queryByText} = renderBackboneView(view); + + expect(getByText('Has comments')).toBeInTheDocument(); + expect(queryByText('Empty chapter')).not.toBeInTheDocument(); }); it('renders a thread group only for content elements that have threads', () => { diff --git a/entry_types/scrolled/package/src/editor/views/EntryCommentsView.js b/entry_types/scrolled/package/src/editor/views/EntryCommentsView.js index b1d7b3aca8..0b5b16f1cd 100644 --- a/entry_types/scrolled/package/src/editor/views/EntryCommentsView.js +++ b/entry_types/scrolled/package/src/editor/views/EntryCommentsView.js @@ -1,7 +1,8 @@ -import React from 'react'; +import React, {useEffect} from 'react'; import I18n from 'i18n-js'; -import {ThreadList, useCommentThreads} from 'pageflow-scrolled/review'; +import {EntryStateProvider, useEntryStateDispatch, watchCollections} from 'pageflow-scrolled/entryState'; +import {ThreadList, useLocatedCommentThreads} from 'pageflow-scrolled/review'; import {ReviewView} from './ReviewView'; import defaultPictogram from './images/defaultPictogram.svg'; @@ -23,9 +24,8 @@ export const EntryCommentsView = ReviewView.extend({ props() { const {entry, editor} = this.options; return { - items: collectItems(entry), - selectedElement: this._selectedElement, - selectedSection: this._selectedSection, + entry, + selectedSubject: entry.get('selectedCommentsSubject') || null, // 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. @@ -37,24 +37,12 @@ export const EntryCommentsView = ReviewView.extend({ }; }, - renderContent({items, selectedElement, selectedSection, transientThreadIds, highlightedThreadId, onThreadClick, editor}) { + renderContent({entry, ...props}) { return ( -
- {items.map(item => item.type === 'section' ? - : - - )} -
+ + + + ); }, @@ -73,10 +61,6 @@ 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, @@ -86,47 +70,93 @@ export const EntryCommentsView = ReviewView.extend({ } }); -// 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({type: 'contentElement', contentElement}); - }); - }); - }); - - return items; +function WatchEntryCollections({entry}) { + const dispatch = useEntryStateDispatch(); + + useEffect(() => watchCollections(entry, {dispatch}), [entry, dispatch]); + + return null; } -function ContentElementGroup({ - contentElement, isSelected, selectedHasTransientThreadIds, - highlightedThreadId, onThreadClick, editor -}) { - const permaId = contentElement.get('permaId'); - const threads = useCommentThreads({ - subjectType: 'ContentElement', - subjectId: permaId - }); +function CommentsList({selectedSubject, transientThreadIds, highlightedThreadId, onThreadClick, editor}) { + const {chapters} = useLocatedCommentThreads(); - if (threads.length === 0) { + return ( +
+ {chapters.map((chapter, index) => + + )} +
+ ); +} + +function ChapterGroup({chapter, number, ...groupProps}) { + if (chapter.threadCount === 0) { return null; } - const typeName = contentElement.get('typeName'); - const label = I18n.t(`pageflow_scrolled.editor.content_elements.${typeName}.name`); - const pictogram = editor.contentElementTypes.findPictogram(typeName) || defaultPictogram; - const compareRanges = editor.contentElementTypes.findCompareRanges(typeName); + return ( +
+ + {/* 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. */} + {chapter.sections.map(section => ( + + {section.threads.length > 0 && + } + {section.contentElements.map(contentElement => + contentElement.threads.length > 0 && + + )} + + ))} +
+ ); +} + +function ChapterHeading({number, title}) { + return ( +
+ + + {number != null ? + `${I18n.t('pageflow_scrolled.editor.chapter_item.chapter')} ${number}` : + I18n.t('pageflow_scrolled.editor.chapter_item.excursion')} + + + {title} + + +
+ ); +} + +function ContentElementGroup({ + contentElement, selectedSubject, transientThreadIds, + highlightedThreadId, onThreadClick, editor +}) { + const {permaId, type, threads} = contentElement; + const label = I18n.t(`pageflow_scrolled.editor.content_elements.${type}.name`); + const pictogram = editor.contentElementTypes.findPictogram(type) || defaultPictogram; + const compareRanges = editor.contentElementTypes.findCompareRanges(type); + + const isSelected = selectedSubject?.subjectType === 'ContentElement' && + selectedSubject.id === contentElement.id; // Element-level badges (e.g. on images) have no per-thread anchor in // the iframe, so clicking such a badge highlights every thread of // the element rather than just one. - const groupHighlight = isSelected && !selectedHasTransientThreadIds ? + const groupHighlight = isSelected && transientThreadIds === undefined ? threads.map(t => t.id) : highlightedThreadId; @@ -145,16 +175,11 @@ function ContentElementGroup({ ); } -function SectionGroup({section, isSelected, highlightedThreadId, onThreadClick}) { - const permaId = section.get('permaId'); - const threads = useCommentThreads({ - subjectType: 'Section', - subjectId: permaId - }); +function SectionGroup({section, selectedSubject, highlightedThreadId, onThreadClick}) { + const {permaId, threads} = section; - if (threads.length === 0) { - return null; - } + const isSelected = selectedSubject?.subjectType === 'Section' && + selectedSubject.id === section.id; // A section has no per-thread anchor in the preview, so selecting it // highlights all its threads at once, like a whole-element image badge. diff --git a/entry_types/scrolled/package/src/editor/views/EntryCommentsView.module.css b/entry_types/scrolled/package/src/editor/views/EntryCommentsView.module.css index 6e5d536ddc..9e95b600cc 100644 --- a/entry_types/scrolled/package/src/editor/views/EntryCommentsView.module.css +++ b/entry_types/scrolled/package/src/editor/views/EntryCommentsView.module.css @@ -3,6 +3,27 @@ flex-direction: column; } +.chapter { + display: flex; + flex-direction: column; +} + +.chapterHeading { + display: flex; + align-items: center; + gap: space(2); + padding: space(5) 0 space(2); +} + +.chapterNumber { + font-weight: bold; + color: var(--ui-on-surface-color-light); +} + +.chapterTitle { + font-weight: bold; +} + .group { cursor: pointer; } diff --git a/entry_types/scrolled/package/src/editor/views/ReviewView.module.css b/entry_types/scrolled/package/src/editor/views/ReviewView.module.css index 8c51077e3e..89824fa606 100644 --- a/entry_types/scrolled/package/src/editor/views/ReviewView.module.css +++ b/entry_types/scrolled/package/src/editor/views/ReviewView.module.css @@ -1,6 +1,5 @@ .container { position: relative; - padding-top: space(4); --review-thread-box-shadow: none; --review-thread-border: 1px solid var(--ui-on-surface-color-lightest); --review-resolved-threads-pill-align: flex-end; diff --git a/entry_types/scrolled/package/src/editor/views/SelectionCommentsView.js b/entry_types/scrolled/package/src/editor/views/SelectionCommentsView.js index 7b722c5ff1..c2a367055b 100644 --- a/entry_types/scrolled/package/src/editor/views/SelectionCommentsView.js +++ b/entry_types/scrolled/package/src/editor/views/SelectionCommentsView.js @@ -3,8 +3,11 @@ import React from 'react'; import {ThreadList} from 'pageflow-scrolled/review'; import {ReviewView} from './ReviewView'; +import styles from './SelectionCommentsView.module.css'; export const SelectionCommentsView = ReviewView.extend({ + className: styles.root, + initialize() { const {entry} = this.options; diff --git a/entry_types/scrolled/package/src/editor/views/SelectionCommentsView.module.css b/entry_types/scrolled/package/src/editor/views/SelectionCommentsView.module.css new file mode 100644 index 0000000000..6deec5cf28 --- /dev/null +++ b/entry_types/scrolled/package/src/editor/views/SelectionCommentsView.module.css @@ -0,0 +1,3 @@ +.root { + padding-top: space(4); +}