From d5d6c4976f7b16da42cc586d30bf4491eeaa47d6 Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Thu, 27 Aug 2026 11:22:13 +0200 Subject: [PATCH 01/29] Keep a badge anchored while any of its leaves survives Slate splits a decorated range wherever another decoration overlaps it, and every piece carries the range's key - so a range that another thread overlaps registers several anchor elements under one key. The registry held one element per key and deleted the key outright when an element went away, which let a piece unmounting take down the anchor its siblings still provided. The badge then had no reference to position against and rendered nothing. Revealing a resolved thread and hiding it again is enough to trigger it: the highlight comes and goes, the leaves around it are split and merged, and one of the overlapped threads loses its badge until something else makes its leaf re-register. The key now holds every element registered under it, and the badge anchors to whichever comes first in the document - where the range starts, rather than wherever the last piece to mount happened to be. --- .../features/commentBadges-spec.js | 48 +++++++++++++++ .../package/spec/review/rangeAnchors-spec.js | 59 +++++++++++++++++++ .../package/src/review/rangeAnchors.js | 38 +++++++++--- 3 files changed, 137 insertions(+), 8 deletions(-) create mode 100644 entry_types/scrolled/package/spec/review/rangeAnchors-spec.js 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 8c004e8392..1de284f329 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 @@ -116,6 +116,54 @@ describe('inline editing EditableText comment badges', () => { expect(badge.isActive()).toBe(true); }); + it('keeps the badge of an overlapped thread after a resolved thread is revealed', async () => { + const value = [{type: 'paragraph', children: [{text: 'Alpha beta gamma delta'}]}]; + + function range(start, end) { + return {anchor: {path: [0, 0], offset: start}, focus: {path: [0, 0], offset: end}}; + } + + function selectThread(threadId) { + act(() => { + window.dispatchEvent(new MessageEvent('message', { + data: {type: 'SELECT_COMMENT_THREAD', payload: {threadId}}, + origin: window.location.origin + })); + }); + } + + const entry = renderEntry({ + contentElement: { + ui: , + typeOptions: {inlineComments: true, customSelectionRect: true} + }, + commenting: { + currentUser: null, + commentThreads: [ + {id: 5, subjectType: 'ContentElement', subjectId: 10, subjectRange: range(0, 5), + comments: [{id: 1, body: 'a', creatorName: 'Alice', creatorId: 1}]}, + {id: 6, subjectType: 'ContentElement', subjectId: 10, subjectRange: range(6, 16), + comments: [{id: 2, body: 'b', creatorName: 'Bob', creatorId: 2}]}, + {id: 7, subjectType: 'ContentElement', subjectId: 10, subjectRange: range(17, 22), + comments: [{id: 3, body: 'c', creatorName: 'Carol', creatorId: 3}]}, + {id: 8, subjectType: 'ContentElement', subjectId: 10, subjectRange: range(8, 12), + resolvedAt: '2026-06-01T00:00:00Z', + comments: [{id: 4, body: 'r', creatorName: 'Dave', creatorId: 4}]} + ] + } + }); + + expect(entry.queryAllCommentBadges()).toHaveLength(3); + + selectThread(8); + await waitFor(() => expect(entry.queryAllCommentBadges()).toHaveLength(4)); + + selectThread(5); + await waitFor(() => expect(entry.queryAllCommentBadges()[0].isActive()).toBe(true)); + + expect(entry.queryAllCommentBadges()).toHaveLength(3); + }); + 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/review/rangeAnchors-spec.js b/entry_types/scrolled/package/spec/review/rangeAnchors-spec.js new file mode 100644 index 0000000000..d45a839c44 --- /dev/null +++ b/entry_types/scrolled/package/spec/review/rangeAnchors-spec.js @@ -0,0 +1,59 @@ +import React, {useEffect, useState} from 'react'; +import {render} from '@testing-library/react'; +import '@testing-library/jest-dom/extend-expect'; + +import {useRangeAnchors, RangeAnchor, useAnchoredFloating} from 'review/rangeAnchors'; + +describe('range anchors', () => { + function Anchored({rangeKey, anchors}) { + const {refs, hasAnchor} = useAnchoredFloating(rangeKey, anchors); + const [anchorText, setAnchorText] = useState(); + + useEffect(() => { + setAnchorText(refs.reference.current?.textContent); + }, [refs.reference, hasAnchor]); + + return ( + + {hasAnchor ? anchorText : 'none'} + + ); + } + + function Anchors({texts}) { + const {anchors, registerAnchor} = useRangeAnchors(); + + return ( +
+ {texts.map((text, index) => + + {text} + + )} + +
+ ); + } + + it('anchors a range to the first of its elements in the document', () => { + const {getByTestId} = render(); + + expect(getByTestId('anchored-a')).toHaveTextContent('first'); + }); + + it('keeps a range anchored while one of its elements goes away', () => { + const {getByTestId, rerender} = render(); + + rerender(); + + expect(getByTestId('anchored-a')).toHaveTextContent('first'); + }); + + it('drops the anchor once the last of its elements goes away', () => { + const {getByTestId, rerender} = render(); + + rerender(); + + expect(getByTestId('anchored-a')).toHaveTextContent('none'); + }); +}); diff --git a/entry_types/scrolled/package/src/review/rangeAnchors.js b/entry_types/scrolled/package/src/review/rangeAnchors.js index bb597e6327..09a799644d 100644 --- a/entry_types/scrolled/package/src/review/rangeAnchors.js +++ b/entry_types/scrolled/package/src/review/rangeAnchors.js @@ -10,9 +10,19 @@ export function useRangeAnchors() { const elements = useRef(new Map()); const [version, setVersion] = useState(0); - const registerAnchor = useCallback((rangeKey, el) => { - if (el) { - elements.current.set(rangeKey, el); + // Several elements can carry the same range key: Slate splits a + // decorated range wherever another decoration overlaps it, and every + // piece keeps the key. Unregistering by key alone would let a piece + // going away drop the anchor the remaining pieces still provide. + const registerAnchor = useCallback((rangeKey, el, mounted = true) => { + if (!el) return; + + const registered = elements.current.get(rangeKey) || []; + const remaining = mounted ? [...registered, el] : + registered.filter(other => other !== el); + + if (remaining.length) { + elements.current.set(rangeKey, remaining); } else { elements.current.delete(rangeKey); @@ -33,9 +43,13 @@ export function RangeAnchor({rangeKey, onRegister, children}) { const ref = useRef(null); useEffect(() => { - onRegister(rangeKey, ref.current); + // Read here rather than in the cleanup, which runs once React has + // detached the ref. + const el = ref.current; - return () => onRegister(rangeKey, null); + onRegister(rangeKey, el); + + return () => onRegister(rangeKey, el, false); }, [rangeKey, onRegister]); return {children}; @@ -63,10 +77,10 @@ export function useAnchoredFloating(rangeKey, anchors, { }); useEffect(() => { - const el = anchors._elements.current.get(rangeKey); + const registered = anchors._elements.current.get(rangeKey); - if (el) { - refs.setReference(el); + if (registered) { + refs.setReference(firstInDocument(registered)); } }, [refs, anchors, rangeKey, anchors._version]); @@ -75,6 +89,14 @@ export function useAnchoredFloating(rangeKey, anchors, { return {refs, floatingStyles, placement: resolvedPlacement, isPositioned, hasAnchor, fits}; } +// The pieces of a split range are all anchors for it, but the badge +// belongs beside where the range starts. +function firstInDocument(elements) { + return elements.reduce((first, el) => + first.compareDocumentPosition(el) & Node.DOCUMENT_POSITION_PRECEDING ? el : first + ); +} + export function alignToContainerEdge(containerRef, { mainAxisOffset = 0, viewportPadding = 0, From 6a494b008a14e099a9e747abcf1388665cd95e0c Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Wed, 26 Aug 2026 17:31:56 +0200 Subject: [PATCH 02/29] Speak of unread comments rather than new ones Two words for the same thing had grown up side by side: the badges and the entry list's dot speak of unread comments, the markers inside a thread of new ones. The activity feed puts them in one view, where "new" reads as recently written - which is what the feed's rows are about, while the markers are about what the reviewer has read. The two counts part ways as soon as a thread was read halfway, so the words have to as well. Everything that means "not read yet" is unread now, down to the summary behind the entry list's dot: #new? becomes #unread?, and its counts follow. --- app/helpers/pageflow/admin/entries_helper.rb | 10 ++--- app/models/pageflow/entry_comment_summary.rb | 24 ++++++------ config/locales/de.yml | 12 +++--- config/locales/en.yml | 12 +++--- entry_types/scrolled/config/locales/de.yml | 8 ++-- entry_types/scrolled/config/locales/en.yml | 8 ++-- ...wMarkers-spec.js => unreadMarkers-spec.js} | 36 +++++++++--------- ...r-spec.js => unreadRepliesDivider-spec.js} | 14 +++---- .../scrolled/package/src/review/Thread.js | 38 +++++++++---------- .../package/src/review/Thread.module.css | 12 +++--- .../scrolled/package/src/review/ThreadList.js | 4 +- ...seeing_comment_activity_of_entries_spec.rb | 2 +- .../pageflow/admin/entries_helper_spec.rb | 4 +- .../pageflow/entry_comment_summary_spec.rb | 34 ++++++++--------- 14 files changed, 110 insertions(+), 108 deletions(-) rename entry_types/scrolled/package/spec/review/Thread/features/{newMarkers-spec.js => unreadMarkers-spec.js} (78%) rename entry_types/scrolled/package/spec/review/Thread/features/{newRepliesDivider-spec.js => unreadRepliesDivider-spec.js} (88%) diff --git a/app/helpers/pageflow/admin/entries_helper.rb b/app/helpers/pageflow/admin/entries_helper.rb index 9b85aa1851..64e6e22f31 100644 --- a/app/helpers/pageflow/admin/entries_helper.rb +++ b/app/helpers/pageflow/admin/entries_helper.rb @@ -27,7 +27,7 @@ def entry_comments_indicator(entry, summaries: entry_comment_summaries) class: 'entry_comments_indicator', data: {tooltip: entry_comments_tooltip(summary)}) do safe_join([summary.topic_count.to_s, - (content_tag(:span, '', class: 'unread_dot') if summary.new?)].compact) + (content_tag(:span, '', class: 'unread_dot') if summary.unread?)].compact) end end @@ -44,12 +44,12 @@ def entry_comments_tooltip(summary) parts = [t("#{scope}.topic_count", count: summary.topic_count)] - if summary.new_topic_count.positive? - parts << t("#{scope}.new_topic_count", count: summary.new_topic_count) + if summary.unread_topic_count.positive? + parts << t("#{scope}.unread_topic_count", count: summary.unread_topic_count) end - if summary.new_reply_count.positive? - parts << t("#{scope}.new_reply_count", count: summary.new_reply_count) + if summary.unread_reply_count.positive? + parts << t("#{scope}.unread_reply_count", count: summary.unread_reply_count) end t("#{scope}.tooltip", summary: parts.join(', ')) diff --git a/app/models/pageflow/entry_comment_summary.rb b/app/models/pageflow/entry_comment_summary.rb index 10e1674f85..bfbe3fea59 100644 --- a/app/models/pageflow/entry_comment_summary.rb +++ b/app/models/pageflow/entry_comment_summary.rb @@ -7,7 +7,7 @@ module Pageflow # # @api private class EntryCommentSummary - attr_reader :topic_count, :new_topic_count, :new_reply_count + attr_reader :topic_count, :unread_topic_count, :unread_reply_count def self.for_entries(entries, user:) entries = entries.to_a @@ -23,18 +23,18 @@ def self.for_entries(entries, user:) end end - def initialize(topic_count:, new_topic_count:, new_reply_count:) + def initialize(topic_count:, unread_topic_count:, unread_reply_count:) @topic_count = topic_count - @new_topic_count = new_topic_count - @new_reply_count = new_reply_count + @unread_topic_count = unread_topic_count + @unread_reply_count = unread_reply_count end def any? topic_count.positive? end - def new? - new_topic_count.positive? || new_reply_count.positive? + def unread? + unread_topic_count.positive? || unread_reply_count.positive? end # Comment threads live on the draft revision, so entries are reached @@ -62,18 +62,20 @@ def self.read_at_by_entry_id(entries, user) private_class_method :read_at_by_entry_id def self.build(threads, read_at:, user:) - new_topics = 0 - new_replies = 0 + unread_topics = 0 + unread_replies = 0 threads.each do |thread| first, *replies = thread.comments.sort_by(&:id) seen_up_to = [read_at[thread.perma_id], user.unread_comments_since_at].compact.max - new_topics += 1 if first && unread?(first, seen_up_to, user) - new_replies += replies.count { |reply| unread?(reply, seen_up_to, user) } + unread_topics += 1 if first && unread?(first, seen_up_to, user) + unread_replies += replies.count { |reply| unread?(reply, seen_up_to, user) } end - new(topic_count: threads.size, new_topic_count: new_topics, new_reply_count: new_replies) + new(topic_count: threads.size, + unread_topic_count: unread_topics, + unread_reply_count: unread_replies) end private_class_method :build diff --git a/config/locales/de.yml b/config/locales/de.yml index 8f75ebff71..b132c0e5e5 100644 --- a/config/locales/de.yml +++ b/config/locales/de.yml @@ -829,16 +829,16 @@ de: entries: add_folder: Ordner hinzufügen comments: - new_reply_count: - one: 1 neue Antwort - other: "%{count} neue Antworten" - new_topic_count: - one: 1 neues Thema - other: "%{count} neue Themen" tooltip: "Kommentare: %{summary}" topic_count: one: 1 ungelöstes Thema other: "%{count} ungelöste Themen" + unread_reply_count: + one: 1 ungelesene Antwort + other: "%{count} ungelesene Antworten" + unread_topic_count: + one: 1 ungelesenes Thema + other: "%{count} ungelesene Themen" confirm_depublish: Soll der Beitrag wirklich depubliziert werden? confirm_duplicate: Beitrag wirklich duplizieren? confirm_restore: Soll der Beitrag wirklich auf den Stand dieser Revision zurückgesetzt werden? Vor dem Zurücksetzen wird eine automatische Sicherung des aktuellen Standes erstellt. diff --git a/config/locales/en.yml b/config/locales/en.yml index 510f61afe9..1bee43c179 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -828,16 +828,16 @@ en: entries: add_folder: Add folder comments: - new_reply_count: - one: 1 new reply - other: "%{count} new replies" - new_topic_count: - one: 1 new topic - other: "%{count} new topics" tooltip: "Comments: %{summary}" topic_count: one: 1 unresolved topic other: "%{count} unresolved topics" + unread_reply_count: + one: 1 unread reply + other: "%{count} unread replies" + unread_topic_count: + one: 1 unread topic + other: "%{count} unread topics" confirm_depublish: Depublish this story? confirm_duplicate: Duplicate this story? confirm_restore: Restore story to the selected version? A snapshot will be created, so that you can roll back later. diff --git a/entry_types/scrolled/config/locales/de.yml b/entry_types/scrolled/config/locales/de.yml index 314888e115..31f3dc4654 100644 --- a/entry_types/scrolled/config/locales/de.yml +++ b/entry_types/scrolled/config/locales/de.yml @@ -2001,10 +2001,10 @@ de: unread_comment_count: one: 1 ungelesener Kommentar other: '%{count} ungelesene Kommentare' - new_reply_count: - one: 1 neu - other: '%{count} neu' - new_replies: Neue Antworten + unread_reply_count: + one: 1 ungelesen + other: '%{count} ungelesen' + unread_replies: Ungelesene Antworten select_content_element: Zum Kommentieren auswählen select_section: Abschnitt zum Kommentieren auswählen select_text_to_comment: Text zum Kommentieren auswählen diff --git a/entry_types/scrolled/config/locales/en.yml b/entry_types/scrolled/config/locales/en.yml index 99c00d6ae1..13ed4a492b 100644 --- a/entry_types/scrolled/config/locales/en.yml +++ b/entry_types/scrolled/config/locales/en.yml @@ -1829,10 +1829,10 @@ en: unread_comment_count: one: 1 unread comment other: '%{count} unread comments' - new_reply_count: - one: 1 new - other: '%{count} new' - new_replies: New replies + unread_reply_count: + one: 1 unread + other: '%{count} unread' + unread_replies: Unread replies select_content_element: Select to comment select_section: Select section to comment select_text_to_comment: Select text to comment diff --git a/entry_types/scrolled/package/spec/review/Thread/features/newMarkers-spec.js b/entry_types/scrolled/package/spec/review/Thread/features/unreadMarkers-spec.js similarity index 78% rename from entry_types/scrolled/package/spec/review/Thread/features/newMarkers-spec.js rename to entry_types/scrolled/package/spec/review/Thread/features/unreadMarkers-spec.js index ac944b9c97..9f38779c94 100644 --- a/entry_types/scrolled/package/spec/review/Thread/features/newMarkers-spec.js +++ b/entry_types/scrolled/package/spec/review/Thread/features/unreadMarkers-spec.js @@ -6,13 +6,13 @@ import {Thread} from 'review/Thread'; import {renderWithReviewState} from 'support/renderWithReviewState'; import styles from 'review/Thread.module.css'; -describe('Thread new markers', () => { +describe('Thread unread markers', () => { useFakeTranslations({ 'pageflow_scrolled.review.reply_placeholder': 'Reply...', 'pageflow_scrolled.review.reply_count.one': '1 reply', 'pageflow_scrolled.review.reply_count.other': '%{count} replies', - 'pageflow_scrolled.review.new_reply_count.one': '1 new', - 'pageflow_scrolled.review.new_reply_count.other': '%{count} new', + 'pageflow_scrolled.review.unread_reply_count.one': '1 unread', + 'pageflow_scrolled.review.unread_reply_count.other': '%{count} unread', 'pageflow_scrolled.review.toggle_replies': 'Toggle replies', 'pageflow_scrolled.review.unread_comment_count.one': '1 unread comment', 'pageflow_scrolled.review.unread_comment_count.other': '%{count} unread comments' @@ -51,53 +51,53 @@ describe('Thread new markers', () => { return renderWithReviewState(ui, {currentUser, commentThreads: [thread], ...options}); } - function newDot(container) { - return container.querySelector(`.${styles.newDot}`); + function unreadDot(container) { + return container.querySelector(`.${styles.unreadDot}`); } describe('dot on the thread', () => { it('marks a thread with unseen comments', () => { const {container, getByLabelText} = render( - + ); - expect(newDot(container)).not.toBeNull(); + expect(unreadDot(container)).not.toBeNull(); expect(getByLabelText('1 unread comment')).toBeInTheDocument(); }); it('marks a thread whose replies are unseen', () => { const {container} = render( - , + , { commentThreads: [threadWithReplies], commentThreadReads: {5: '2026-08-17T10:00:00.000Z'} } ); - expect(newDot(container)).not.toBeNull(); + expect(unreadDot(container)).not.toBeNull(); }); it('does not mark a thread without unseen comments', () => { const {container} = render( - , + , {commentThreadReads: {5: '2026-08-17T12:00:00.000Z'}} ); - expect(newDot(container)).toBeNull(); + expect(unreadDot(container)).toBeNull(); }); // The badge that opened the list already carries the same information. it('does not mark a thread shown on its own', () => { const {container} = render(); - expect(newDot(container)).toBeNull(); + expect(unreadDot(container)).toBeNull(); }); }); describe('new reply count', () => { it('counts unseen replies hidden by collapsing', () => { const {getByText} = render( - , + , { commentThreads: [threadWithReplies], commentThreadReads: {5: '2026-08-17T10:00:00.000Z'} @@ -105,7 +105,7 @@ describe('Thread new markers', () => { ); expect(getByText('1 reply')).toBeInTheDocument(); - expect(getByText('1 new')).toBeInTheDocument(); + expect(getByText('1 unread')).toBeInTheDocument(); }); // An unread first comment must not be counted among the replies it @@ -120,17 +120,17 @@ describe('Thread new markers', () => { }; const {getByText, queryByText} = render( - , + , {commentThreads: [threadWithOwnReply]} ); expect(getByText('1 reply')).toBeInTheDocument(); - expect(queryByText('1 new')).toBeNull(); + expect(queryByText('1 unread')).toBeNull(); }); it('does not show a count when all replies have been seen', () => { const {queryByText} = render( - , + , { commentThreads: [threadWithReplies], commentThreadReads: {5: '2026-08-17T13:00:00.000Z'} @@ -138,7 +138,7 @@ describe('Thread new markers', () => { ); expect(queryByText('1 reply')).toBeInTheDocument(); - expect(queryByText('1 new')).toBeNull(); + expect(queryByText('1 unread')).toBeNull(); }); }); }); diff --git a/entry_types/scrolled/package/spec/review/Thread/features/newRepliesDivider-spec.js b/entry_types/scrolled/package/spec/review/Thread/features/unreadRepliesDivider-spec.js similarity index 88% rename from entry_types/scrolled/package/spec/review/Thread/features/newRepliesDivider-spec.js rename to entry_types/scrolled/package/spec/review/Thread/features/unreadRepliesDivider-spec.js index c91f953f0c..f4179642a2 100644 --- a/entry_types/scrolled/package/spec/review/Thread/features/newRepliesDivider-spec.js +++ b/entry_types/scrolled/package/spec/review/Thread/features/unreadRepliesDivider-spec.js @@ -5,13 +5,13 @@ import {useFakeTranslations} from 'pageflow/testHelpers'; import {Thread} from 'review/Thread'; import {renderWithReviewState} from 'support/renderWithReviewState'; -describe('Thread new replies divider', () => { +describe('Thread unread replies divider', () => { useFakeTranslations({ 'pageflow_scrolled.review.reply_placeholder': 'Reply...', 'pageflow_scrolled.review.reply_count.one': '1 reply', 'pageflow_scrolled.review.reply_count.other': '%{count} replies', 'pageflow_scrolled.review.toggle_replies': 'Toggle replies', - 'pageflow_scrolled.review.new_replies': 'New replies' + 'pageflow_scrolled.review.unread_replies': 'Unread replies' }); const currentUser = {id: 42, name: 'Alice'}; @@ -45,7 +45,7 @@ describe('Thread new replies divider', () => { it('separates unseen replies from the ones already seen', () => { const {getByText} = render({commentThreadReads: {5: '2026-08-17T10:00:00.000Z'}}); - const divider = getByText('New replies'); + const divider = getByText('Unread replies'); const seen = getByText('Older reply'); const unseen = getByText('Newer reply'); @@ -58,14 +58,14 @@ describe('Thread new replies divider', () => { it('renders no divider when every reply has been seen', () => { const {queryByText} = render({commentThreadReads: {5: '2026-08-17T12:00:00.000Z'}}); - expect(queryByText('New replies')).toBeNull(); + expect(queryByText('Unread replies')).toBeNull(); }); // The thread's own marker already says the whole thread is new. it('renders no divider when the thread is new all through', () => { const {queryByText} = render(); - expect(queryByText('New replies')).toBeNull(); + expect(queryByText('Unread replies')).toBeNull(); }); it('renders no divider while replies are collapsed', () => { @@ -78,7 +78,7 @@ describe('Thread new replies divider', () => { } ); - expect(queryByText('New replies')).toBeNull(); + expect(queryByText('Unread replies')).toBeNull(); }); it('renders no divider before the current user own replies', () => { @@ -103,6 +103,6 @@ describe('Thread new replies divider', () => { } ); - expect(queryByText('New replies')).toBeNull(); + expect(queryByText('Unread replies')).toBeNull(); }); }); diff --git a/entry_types/scrolled/package/src/review/Thread.js b/entry_types/scrolled/package/src/review/Thread.js index 1d21c33664..b580db8795 100644 --- a/entry_types/scrolled/package/src/review/Thread.js +++ b/entry_types/scrolled/package/src/review/Thread.js @@ -17,7 +17,7 @@ import ResolveIcon from './images/resolve.svg'; import UnresolveIcon from './images/unresolve.svg'; import styles from './Thread.module.css'; -export function Thread({thread, collapsed: collapsedProp, onToggle, onResolve, onClick, highlighted, showNewMarker, interactive = true}) { +export function Thread({thread, collapsed: collapsedProp, onToggle, onResolve, onClick, highlighted, showUnreadMarker, interactive = true}) { const {t} = useI18n({locale: 'ui'}); const firstComment = thread.comments[0]; const replies = thread.comments.slice(1); @@ -29,25 +29,25 @@ export function Thread({thread, collapsed: collapsedProp, onToggle, onResolve, o const repliesCollapsed = collapsed && replies.length > 0; - const newComments = useUnreadComments(thread); - const newReplyCount = useMemo(() => { - const ids = new Set(newComments.map(comment => comment.id)); + const unreadComments = useUnreadComments(thread); + const unreadReplyCount = useMemo(() => { + const ids = new Set(unreadComments.map(comment => comment.id)); return replies.filter(reply => ids.has(reply.id)).length; - }, [newComments, replies]); + }, [unreadComments, replies]); - const hidesNewReplies = repliesCollapsed && newReplyCount > 0; + const hidesUnreadReplies = repliesCollapsed && unreadReplyCount > 0; // Where the unseen part of the thread starts. Only meaningful with // seen comments above it: a thread that is new all through says so // through its dot instead of repeating it at the very top. - const firstNewReplyId = useMemo(() => { - if (!newComments.length || newComments[0].id === firstComment?.id) { + const firstUnreadReplyId = useMemo(() => { + if (!unreadComments.length || unreadComments[0].id === firstComment?.id) { return null; } - const ids = new Set(newComments.map(comment => comment.id)); + const ids = new Set(unreadComments.map(comment => comment.id)); return replies.find(reply => ids.has(reply.id))?.id; - }, [newComments, replies, firstComment]); + }, [unreadComments, replies, firstComment]); // Kept here rather than per comment so that a thread never shows two // textareas at once: neither two comments being edited, nor an edit next @@ -92,11 +92,11 @@ export function Thread({thread, collapsed: collapsedProp, onToggle, onResolve, o aria-current={highlighted ? 'true' : undefined}> {/* A lone thread needs no marker of its own: the badge that opened the list already says the same thing right next to it. */} - {showNewMarker && newComments.length > 0 && + {showUnreadMarker && unreadComments.length > 0 && } + {count: unreadComments.length})} />} {replies.length > 0 && } - {thread.orphaned &&

{t('pageflow_scrolled.review.refers_to_deleted_element')} @@ -115,16 +108,24 @@ export function Thread({thread, collapsed: collapsedProp, onToggle, onResolve, o showQuote={outdatedQuotes.has(firstComment.id)} {...editProps(firstComment)} />} - {repliesCollapsed && - } {!collapsed && replies.map(comment => ( diff --git a/entry_types/scrolled/package/src/review/Thread.module.css b/entry_types/scrolled/package/src/review/Thread.module.css index 225177566b..2a6ddc3c5e 100644 --- a/entry_types/scrolled/package/src/review/Thread.module.css +++ b/entry_types/scrolled/package/src/review/Thread.module.css @@ -31,7 +31,7 @@ /* Locates the new thread among several. Matches the badge dot, which carries the same meaning one level up. Straddles the corner so it - stays clear of the chevron button inside it. */ + stays clear of the menu button inside it. */ .unreadDot { position: absolute; top: space(-1); @@ -49,9 +49,20 @@ gap: space(2); } -.unreadReplyCount::before { +/* Wraps between the counts but never inside one. Holding the chevron + outside keeps it off a line of its own. */ +.counts { + display: flex; + flex-wrap: wrap; + gap: space(1) space(2); + white-space: nowrap; +} + +/* Part of the reply count rather than of what it separates, so that it + stays behind on the first line when the two counts stack. */ +.count:has(+ .unreadReplyCount)::after { content: '·'; - margin-right: space(2); + margin-left: space(2); color: var(--ui-on-surface-color-light); } @@ -84,23 +95,9 @@ color: var(--ui-on-surface-color-light); } -.chevronButton { - position: absolute; - top: space(3); - right: space(4); - display: flex; - align-items: center; - justify-content: center; - height: space(6); - color: var(--ui-on-surface-color-light); - background: none; - border: none; - cursor: pointer; - padding: 0; -} - -.chevronButton:hover { - color: var(--ui-on-surface-color); +.replyChevron { + width: space(3); + height: space(3); } .chevronExpanded { @@ -108,11 +105,14 @@ transition: transform 0.2s; } -.expandButton { +/* The height of the avatars it holds while collapsed, so that toggling + the replies does not shift everything below. */ +.repliesToggle { display: flex; align-items: center; justify-content: space-between; width: 100%; + min-height: space(6); gap: space(2); padding: 0 0 0 space(8); font: inherit; @@ -123,7 +123,7 @@ cursor: pointer; } -.expandButton:hover { +.repliesToggle:hover { color: var(--ui-primary-color); } diff --git a/entry_types/scrolled/package/src/review/ThreadList.js b/entry_types/scrolled/package/src/review/ThreadList.js index c900572544..ff795b3c4c 100644 --- a/entry_types/scrolled/package/src/review/ThreadList.js +++ b/entry_types/scrolled/package/src/review/ThreadList.js @@ -13,7 +13,7 @@ import ChevronIcon from './images/chevron.svg'; import NewTopicIcon from './images/newTopic.svg'; import styles from './ThreadList.module.css'; -export function ThreadList({subjectType, subjectId, subjectRange, filter, highlightedThreadId, onThreadClick, restrictInteractionsToHighlighted, showNewForm: showNewFormProp, hideNewTopicButton, reversed, expandResolved}) { +export function ThreadList({subjectType, subjectId, subjectRange, filter, highlightedThreadId, onThreadClick, restrictInteractionsToHighlighted, showNewForm: showNewFormProp, hideNewTopicButton, reversed, expandResolved, startCollapsed}) { const {t} = useI18n({locale: 'ui'}); // Threads arrive already located: in display order, with orphans of @@ -40,7 +40,10 @@ export function ThreadList({subjectType, subjectId, subjectRange, filter, highli const noThreads = activeThreads.length === 0 && resolvedThreads.length === 0; const [draft] = useCommentDraft({subjectType, subjectId}); - const [expandedThreadId, setExpandedThreadId] = useState(null); + const [expandedThreadId, setExpandedThreadId] = useState( + () => startCollapsed ? undefined : + (soleThread(activeThreads) || soleThread(resolvedThreads))?.id + ); const [resolvedToggled, setResolvedToggled] = useState(null); const [formToggled, setFormToggled] = useState( showNewFormProp !== undefined ? showNewFormProp : @@ -85,7 +88,7 @@ export function ThreadList({subjectType, subjectId, subjectRange, filter, highli {activeThreads.map(thread => ( 1 && expandedThreadId !== thread.id} + collapsed={expandedThreadId !== thread.id} showUnreadMarker={activeThreads.length > 1} onToggle={() => toggleThread(thread.id)} onResolve={() => postUpdateThreadMessage({threadId: thread.id, resolved: true})} @@ -106,7 +109,7 @@ export function ThreadList({subjectType, subjectId, subjectRange, filter, highli {showResolved && resolvedThreads.map(thread => ( 1 && expandedThreadId !== thread.id} + collapsed={expandedThreadId !== thread.id} showUnreadMarker={resolvedThreads.length > 1} onToggle={() => toggleThread(thread.id)} onResolve={() => postUpdateThreadMessage({threadId: thread.id, resolved: false})} @@ -119,3 +122,7 @@ export function ThreadList({subjectType, subjectId, subjectRange, filter, highli ); } + +function soleThread(threads) { + return threads.length === 1 ? threads[0] : undefined; +} From 94ed7a7750004d7579f8325c7a99311aefcf9324 Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Mon, 24 Aug 2026 13:16:36 +0200 Subject: [PATCH 04/29] Order comments of a thread by id Clients read position as meaning: the first comment of a thread is the topic and the rest are replies. Nothing guaranteed that order so far - it held because InnoDB returns rows of a secondary index in primary key order, not because the query asked for it. No behavior change on the database Pageflow runs on, so the spec is a regression guard rather than a failing test made to pass. --- app/models/pageflow/comment_thread.rb | 4 +++- spec/models/pageflow/comment_thread_spec.rb | 13 +++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/app/models/pageflow/comment_thread.rb b/app/models/pageflow/comment_thread.rb index 254cd35e26..f6e5fc0e05 100644 --- a/app/models/pageflow/comment_thread.rb +++ b/app/models/pageflow/comment_thread.rb @@ -7,7 +7,9 @@ class CommentThread < ApplicationRecord belongs_to :creator, class_name: 'User' belongs_to :resolver, class_name: 'User', foreign_key: :resolved_by_id, optional: true - has_many :comments, dependent: :destroy + # Ordered because clients read position as meaning: the first comment + # is the topic, the rest are replies. + has_many :comments, -> { order(:id) }, dependent: :destroy nested_revision_components :comments diff --git a/spec/models/pageflow/comment_thread_spec.rb b/spec/models/pageflow/comment_thread_spec.rb index d2f4d61a26..6da88bc501 100644 --- a/spec/models/pageflow/comment_thread_spec.rb +++ b/spec/models/pageflow/comment_thread_spec.rb @@ -2,6 +2,19 @@ module Pageflow describe CommentThread do + describe '#comments' do + # Clients read position as meaning: the first comment is the topic + # and the rest are replies. Without an order that rests on how the + # database happens to return rows. + it 'are ordered by id' do + thread = create(:comment_thread) + second = create(:comment, comment_thread: thread, id: 5) + first = create(:comment, comment_thread: thread, id: 3) + + expect(thread.reload.comments.map(&:id)).to eq([first.id, second.id]) + end + end + describe '.migrate_to_subject' do it 'updates subject_id of matching threads' do revision = create(:revision) From 115f2dd6e261587e8faaea21da065dd0bebdb9d0 Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Wed, 19 Aug 2026 16:06:25 +0200 Subject: [PATCH 05/29] Name the user that resolved a comment thread The thread JSON starts carrying resolvedById and resolverName, so that clients can say who resolved a thread rather than only when it happened. Both attributes already existed on the model; only the JSON left them out. --- .../review/comment_threads_controller.rb | 2 +- .../_comment_thread.json.jbuilder | 3 ++ .../review/comment_threads_controller_spec.rb | 51 ++++++++++++++++++- 3 files changed, 53 insertions(+), 3 deletions(-) diff --git a/app/controllers/pageflow/review/comment_threads_controller.rb b/app/controllers/pageflow/review/comment_threads_controller.rb index 59e0d62f2f..ab286c7f4b 100644 --- a/app/controllers/pageflow/review/comment_threads_controller.rb +++ b/app/controllers/pageflow/review/comment_threads_controller.rb @@ -9,7 +9,7 @@ def index entry = DraftEntry.find(params[:entry_id]) authorize!(:read, entry.to_model) - @comment_threads = entry.comment_threads.includes(comments: :creator) + @comment_threads = entry.comment_threads.includes(:resolver, comments: :creator) @read_at_by_perma_id = CommentThreadRead.read_at_by_perma_id(entry: entry.to_model, user: current_user) end diff --git a/app/views/pageflow/review/comment_threads/_comment_thread.json.jbuilder b/app/views/pageflow/review/comment_threads/_comment_thread.json.jbuilder index e9ef112fb2..6dcf044a30 100644 --- a/app/views/pageflow/review/comment_threads/_comment_thread.json.jbuilder +++ b/app/views/pageflow/review/comment_threads/_comment_thread.json.jbuilder @@ -9,9 +9,12 @@ json.call(comment_thread, :section_perma_id, :creator_id, :resolved_at, + :resolved_by_id, :created_at, :updated_at) +json.resolver_name comment_thread.resolver&.full_name + json.comments(comment_thread.comments) do |comment| json.partial!('pageflow/review/comments/comment', comment:) end diff --git a/spec/controllers/pageflow/review/comment_threads_controller_spec.rb b/spec/controllers/pageflow/review/comment_threads_controller_spec.rb index a6879ea04b..86c70a240f 100644 --- a/spec/controllers/pageflow/review/comment_threads_controller_spec.rb +++ b/spec/controllers/pageflow/review/comment_threads_controller_spec.rb @@ -96,6 +96,49 @@ module Pageflow ) end + it 'returns resolver of resolved threads' do + user = create(:user) + entry = create(:entry, with_previewer: user) + resolver = create(:user, first_name: 'Ada', last_name: 'Lovelace') + + create(:comment_thread, + revision: entry.draft, + creator: user, + resolved_at: Time.current, + resolver:) + + sign_in(user, scope: :user) + get(:index, params: {entry_id: entry.id}, format: 'json') + + expect(response.body).to include_json( + commentThreads: [ + { + resolvedById: resolver.id, + resolverName: 'Ada Lovelace' + } + ] + ) + end + + it 'returns no resolver for unresolved threads' do + user = create(:user) + entry = create(:entry, with_previewer: user) + + create(:comment_thread, revision: entry.draft, creator: user) + + sign_in(user, scope: :user) + get(:index, params: {entry_id: entry.id}, format: 'json') + + expect(response.body).to include_json( + commentThreads: [ + { + resolvedById: nil, + resolverName: nil + } + ] + ) + end + it 'does not have N+1 queries' do user = create(:user) entry = create(:entry, with_previewer: user) @@ -325,7 +368,9 @@ module Pageflow expect(thread.resolver).to eq(user) expect(response.body).to include_json( id: thread.id, - resolvedAt: be_present + resolvedAt: be_present, + resolvedById: user.id, + resolverName: user.full_name ) end @@ -351,7 +396,9 @@ module Pageflow expect(thread.resolver).to be_nil expect(response.body).to include_json( id: thread.id, - resolvedAt: nil + resolvedAt: nil, + resolvedById: nil, + resolverName: nil ) end From e5bf7e021a3754dbe9870048a29f71fa57fe7aaa Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Wed, 19 Aug 2026 16:06:36 +0200 Subject: [PATCH 06/29] Extract comment date formatting Moves formatDate and formatDateTime out of Comment.js so that other review components can render timestamps the same way. --- .../scrolled/package/src/review/Comment.js | 17 +---------------- .../scrolled/package/src/review/formatDate.js | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 16 deletions(-) create mode 100644 entry_types/scrolled/package/src/review/formatDate.js diff --git a/entry_types/scrolled/package/src/review/Comment.js b/entry_types/scrolled/package/src/review/Comment.js index 5936282f49..0446819307 100644 --- a/entry_types/scrolled/package/src/review/Comment.js +++ b/entry_types/scrolled/package/src/review/Comment.js @@ -5,6 +5,7 @@ import {Avatar} from './Avatar'; import {CommentMenu} from './CommentMenu'; import {useCurrentUser, useUpdateComment} from './ReviewStateProvider'; import {autoGrow, autoResize} from './autoGrow'; +import {formatDate, formatDateTime} from './formatDate'; import {isSubmitShortcut} from './submitShortcut'; import styles from './Comment.module.css'; @@ -108,19 +109,3 @@ function EditForm({comment, threadId, onDone}) { ); } - -function formatDate(isoString, locale, options) { - const date = new Date(isoString); - const fromCurrentYear = date.getFullYear() === new Date().getFullYear(); - - return date.toLocaleString(locale, { - month: 'short', - day: 'numeric', - ...(!fromCurrentYear && {year: 'numeric'}), - ...options - }); -} - -function formatDateTime(isoString, locale) { - return formatDate(isoString, locale, {hour: 'numeric', minute: '2-digit'}); -} diff --git a/entry_types/scrolled/package/src/review/formatDate.js b/entry_types/scrolled/package/src/review/formatDate.js new file mode 100644 index 0000000000..68b2b01cc5 --- /dev/null +++ b/entry_types/scrolled/package/src/review/formatDate.js @@ -0,0 +1,15 @@ +export function formatDate(isoString, locale, options) { + const date = new Date(isoString); + const fromCurrentYear = date.getFullYear() === new Date().getFullYear(); + + return date.toLocaleString(locale, { + month: 'short', + day: 'numeric', + ...(!fromCurrentYear && {year: 'numeric'}), + ...options + }); +} + +export function formatDateTime(isoString, locale) { + return formatDate(isoString, locale, {hour: 'numeric', minute: '2-digit'}); +} From 86cc2bebeec01a8c29a02be85e9fd1e8b2ceb75c Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Mon, 24 Aug 2026 15:34:47 +0200 Subject: [PATCH 07/29] Show who resolved a comment thread The resolve button says a thread is resolved, but not by whom. Threads carry the resolver since the review index started naming them, so every list that shows a thread can now close it off with a line of its own - the popover and the editor's lists included, which had no way of telling who ended a discussion. --- entry_types/scrolled/config/locales/de.yml | 3 + entry_types/scrolled/config/locales/en.yml | 3 + .../package/spec/review/CommentMenu-spec.js | 38 +++--- .../review/Thread/features/resolution-spec.js | 108 ++++++++++++++++++ .../src/editor/views/ReviewView.module.css | 2 - .../scrolled/package/src/review/Comment.js | 6 +- .../package/src/review/CommentMenu.js | 12 +- .../scrolled/package/src/review/Thread.js | 54 +++++++-- .../package/src/review/Thread.module.css | 60 ++++++++-- .../commenting_on_content_elements_spec.rb | 2 + 10 files changed, 240 insertions(+), 48 deletions(-) create mode 100644 entry_types/scrolled/package/spec/review/Thread/features/resolution-spec.js diff --git a/entry_types/scrolled/config/locales/de.yml b/entry_types/scrolled/config/locales/de.yml index 36ce065f4d..ed19a35be9 100644 --- a/entry_types/scrolled/config/locales/de.yml +++ b/entry_types/scrolled/config/locales/de.yml @@ -2020,6 +2020,9 @@ de: edited: Bearbeitet %{date} resolve: Als gelöst markieren unresolve: Als ungelöst markieren + resolution: Als gelöst markiert + resolution_by: Als gelöst markiert von + thread_actions: Aktionen für Thema resolved_count: one: 1 erledigt other: '%{count} erledigt' diff --git a/entry_types/scrolled/config/locales/en.yml b/entry_types/scrolled/config/locales/en.yml index eb77c39c5c..28c01986d8 100644 --- a/entry_types/scrolled/config/locales/en.yml +++ b/entry_types/scrolled/config/locales/en.yml @@ -1848,6 +1848,9 @@ en: edited: Edited %{date} resolve: Mark as resolved unresolve: Mark as unresolved + resolution: Marked as resolved + resolution_by: Marked as resolved by + thread_actions: Thread actions resolved_count: one: 1 resolved other: '%{count} resolved' diff --git a/entry_types/scrolled/package/spec/review/CommentMenu-spec.js b/entry_types/scrolled/package/spec/review/CommentMenu-spec.js index 701bec44f0..564252c0ea 100644 --- a/entry_types/scrolled/package/spec/review/CommentMenu-spec.js +++ b/entry_types/scrolled/package/spec/review/CommentMenu-spec.js @@ -1,21 +1,23 @@ import React from 'react'; import '@testing-library/jest-dom/extend-expect'; import userEvent from '@testing-library/user-event'; -import {useFakeTranslations} from 'pageflow/testHelpers'; - import {CommentMenu} from 'review/CommentMenu'; +import EditIcon from 'review/images/edit.svg'; import {renderWithReviewState} from 'support/renderWithReviewState'; describe('CommentMenu', () => { - useFakeTranslations({ - 'pageflow_scrolled.review.comment_actions': 'Comment actions', - 'pageflow_scrolled.review.edit_comment': 'Edit' - }); + function menu(props) { + return ( + {}}]} + {...props} /> + ); + } it('only renders the menu once the button has been clicked', async () => { const user = userEvent.setup(); - const {getByRole, queryByRole} = renderWithReviewState(); + const {getByRole, queryByRole} = renderWithReviewState(menu()); expect(queryByRole('menu')).toBeNull(); @@ -27,7 +29,7 @@ describe('CommentMenu', () => { // The form styles in pageflow/ui/forms.scss turn buttons announcing a // popup into full width select lookalikes, but spare this value. it('announces a menu rather than a generic popup', () => { - const {getByRole} = renderWithReviewState(); + const {getByRole} = renderWithReviewState(menu()); expect(getByRole('button', {name: 'Comment actions'})) .toHaveAttribute('aria-haspopup', 'menu'); @@ -36,7 +38,7 @@ describe('CommentMenu', () => { it('exposes the expanded state of the menu', async () => { const user = userEvent.setup(); - const {getByRole} = renderWithReviewState(); + const {getByRole} = renderWithReviewState(menu()); const button = getByRole('button', {name: 'Comment actions'}); expect(button).toHaveAttribute('aria-expanded', 'false'); @@ -46,23 +48,25 @@ describe('CommentMenu', () => { expect(button).toHaveAttribute('aria-expanded', 'true'); }); - it('invokes onEdit and closes the menu when the item is selected', async () => { + it('invokes onSelect and closes the menu when the item is selected', async () => { const user = userEvent.setup(); - const onEdit = jest.fn(); + const onSelect = jest.fn(); - const {getByRole, queryByRole} = renderWithReviewState(); + const {getByRole, queryByRole} = renderWithReviewState( + menu({items: [{icon: EditIcon, label: 'Edit', onSelect}]}) + ); await user.click(getByRole('button', {name: 'Comment actions'})); await user.click(getByRole('menuitem', {name: 'Edit'})); - expect(onEdit).toHaveBeenCalled(); + expect(onSelect).toHaveBeenCalled(); expect(queryByRole('menu')).toBeNull(); }); it('closes the menu on escape', async () => { const user = userEvent.setup(); - const {getByRole, queryByRole} = renderWithReviewState(); + const {getByRole, queryByRole} = renderWithReviewState(menu()); await user.click(getByRole('button', {name: 'Comment actions'})); await user.keyboard('{Escape}'); @@ -77,7 +81,7 @@ describe('CommentMenu', () => { const user = userEvent.setup(); const listener = jest.fn(); - const {getByRole, queryByRole} = renderWithReviewState(); + const {getByRole, queryByRole} = renderWithReviewState(menu()); await user.click(getByRole('button', {name: 'Comment actions'})); @@ -97,7 +101,7 @@ describe('CommentMenu', () => { it('moves focus to items via arrow keys', async () => { const user = userEvent.setup(); - const {getByRole} = renderWithReviewState(); + const {getByRole} = renderWithReviewState(menu()); await user.click(getByRole('button', {name: 'Comment actions'})); await user.keyboard('{ArrowDown}'); @@ -112,7 +116,7 @@ describe('CommentMenu', () => { const {getByRole} = renderWithReviewState(

- + {menu()}
); diff --git a/entry_types/scrolled/package/spec/review/Thread/features/resolution-spec.js b/entry_types/scrolled/package/spec/review/Thread/features/resolution-spec.js new file mode 100644 index 0000000000..f05c52b809 --- /dev/null +++ b/entry_types/scrolled/package/spec/review/Thread/features/resolution-spec.js @@ -0,0 +1,108 @@ +import React from 'react'; +import '@testing-library/jest-dom/extend-expect'; +import userEvent from '@testing-library/user-event'; +import {useFakeTranslations} from 'pageflow/testHelpers'; + +import {Thread} from 'review/Thread'; +import {renderWithReviewState} from 'support/renderWithReviewState'; + +describe('Thread resolution', () => { + useFakeTranslations({ + 'pageflow_scrolled.review.resolution': 'Marked as resolved', + 'pageflow_scrolled.review.resolution_by': 'Marked as resolved by', + 'pageflow_scrolled.review.resolve': 'Mark as resolved', + 'pageflow_scrolled.review.unresolve': 'Mark as unresolved', + 'pageflow_scrolled.review.thread_actions': 'Thread actions', + 'pageflow_scrolled.review.reply_placeholder': 'Reply...', + 'pageflow_scrolled.review.send': 'Send', + 'pageflow_scrolled.review.enter_for_new_line': 'Enter for new line' + }); + + const thread = { + id: 1, + comments: [{id: 10, body: 'On the pull quote', creatorName: 'Bob', creatorId: 2}] + }; + + const resolved = { + ...thread, + resolvedAt: '2026-08-19T10:00:00.000Z', + resolvedById: 3, + resolverName: 'Ada' + }; + + it('names the user who resolved the thread', () => { + const {getByText} = renderWithReviewState( + + ); + + expect(getByText('Marked as resolved by')).toBeInTheDocument(); + expect(getByText('Ada')).toBeInTheDocument(); + }); + + it('renders the time the thread was resolved', () => { + const {container} = renderWithReviewState( + + ); + + expect(container.querySelector('time[datetime="2026-08-19T10:00:00.000Z"]')) + .toBeInTheDocument(); + }); + + it('says a thread was resolved even when the resolver is not known', () => { + const {getByText, queryByText} = renderWithReviewState( + + ); + + expect(getByText('Marked as resolved')).toBeInTheDocument(); + expect(queryByText('Marked as resolved by')).toBeNull(); + }); + + it('does not render a resolution for an unresolved thread', () => { + const {queryByText} = renderWithReviewState( + {}} /> + ); + + expect(queryByText('Marked as resolved by')).toBeNull(); + expect(queryByText('Marked as resolved')).toBeNull(); + }); + + it('replaces the resolve button once the thread is resolved', () => { + const {queryByRole} = renderWithReviewState( + {}} /> + ); + + expect(queryByRole('button', {name: 'Mark as resolved'})).toBeNull(); + }); + + it('offers the resolve button while the thread is unresolved', () => { + const {getByRole} = renderWithReviewState( + {}} /> + ); + + expect(getByRole('button', {name: 'Mark as resolved'})).toBeInTheDocument(); + }); + + it('undoes the resolution through a menu', async () => { + const user = userEvent.setup(); + const onResolve = jest.fn(); + + const {getByRole} = renderWithReviewState( + + ); + + await user.click(getByRole('button', {name: 'Thread actions'})); + await user.click(getByRole('menuitem', {name: 'Mark as unresolved'})); + + expect(onResolve).toHaveBeenCalled(); + }); + + it('renders no menu without a way to resolve', () => { + const {getByText, queryByRole} = renderWithReviewState( + + ); + + expect(getByText('Ada')).toBeInTheDocument(); + expect(queryByRole('button', {name: 'Thread actions'})).toBeNull(); + }); +}); 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 89824fa606..f1cf713588 100644 --- a/entry_types/scrolled/package/src/editor/views/ReviewView.module.css +++ b/entry_types/scrolled/package/src/editor/views/ReviewView.module.css @@ -5,6 +5,4 @@ --review-resolved-threads-pill-align: flex-end; --review-resolved-threads-pill-color: var(--ui-on-surface-color); --review-resolved-threads-pill-background-color: transparent; - --review-resolved-thread-opacity: 0.6; - --review-resolved-thread-background: var(--ui-on-surface-color-lightest); } diff --git a/entry_types/scrolled/package/src/review/Comment.js b/entry_types/scrolled/package/src/review/Comment.js index 0446819307..51b55983a6 100644 --- a/entry_types/scrolled/package/src/review/Comment.js +++ b/entry_types/scrolled/package/src/review/Comment.js @@ -8,6 +8,7 @@ import {autoGrow, autoResize} from './autoGrow'; import {formatDate, formatDateTime} from './formatDate'; import {isSubmitShortcut} from './submitShortcut'; +import EditIcon from './images/edit.svg'; import styles from './Comment.module.css'; export function Comment({comment, threadId, showQuote, editing, onEdit, onEditEnd}) { @@ -30,7 +31,10 @@ export function Comment({comment, threadId, showQuote, editing, onEdit, onEditEn {editable && - + } {showQuote && diff --git a/entry_types/scrolled/package/src/review/CommentMenu.js b/entry_types/scrolled/package/src/review/CommentMenu.js index d122a6d526..c4b99f1d01 100644 --- a/entry_types/scrolled/package/src/review/CommentMenu.js +++ b/entry_types/scrolled/package/src/review/CommentMenu.js @@ -5,23 +5,17 @@ import { offset, flip, shift, autoUpdate } from '@floating-ui/react'; -import {useI18n, useFloatingPortalRoot} from 'pageflow-scrolled/frontend'; +import {useFloatingPortalRoot} from 'pageflow-scrolled/frontend'; import EllipsisIcon from './images/ellipsis.svg'; -import EditIcon from './images/edit.svg'; import styles from './CommentMenu.module.css'; -export function CommentMenu({onEdit}) { - const {t} = useI18n({locale: 'ui'}); +export function CommentMenu({label, items}) { const portalRoot = useFloatingPortalRoot(); const [open, setOpen] = useState(false); const [activeIndex, setActiveIndex] = useState(null); - const items = [ - {icon: EditIcon, label: t('pageflow_scrolled.review.edit_comment'), onSelect: onEdit} - ]; - const elementsRef = useRef([]); const labelsRef = useRef([]); labelsRef.current = items.map(item => item.label); @@ -84,7 +78,7 @@ export function CommentMenu({onEdit}) { diff --git a/entry_types/scrolled/package/src/review/Thread.js b/entry_types/scrolled/package/src/review/Thread.js index c6574bf11b..eac4509c78 100644 --- a/entry_types/scrolled/package/src/review/Thread.js +++ b/entry_types/scrolled/package/src/review/Thread.js @@ -1,9 +1,11 @@ import React, {useEffect, useMemo, useRef, useState} from 'react'; import classNames from 'classnames'; -import {useI18n} from 'pageflow-scrolled/frontend'; +import {useI18n, useLocale} from 'pageflow-scrolled/frontend'; import {AvatarStack} from './Avatar'; import {Comment} from './Comment'; +import {CommentMenu} from './CommentMenu'; +import {formatDate} from './formatDate'; import {ReplyForm} from './ReplyForm'; import {useCommentDraft} from './ReviewStateProvider'; import {useSubjectQuote} from './subjectQuote'; @@ -85,8 +87,7 @@ export function Thread({thread, collapsed: collapsedProp, onToggle, onResolve, o
@@ -146,15 +147,48 @@ export function Thread({thread, collapsed: collapsedProp, onToggle, onResolve, o subjectId={thread.subjectId} subjectRange={thread.subjectRange} />} - {interactive && onResolve && !repliesCollapsed && + {(thread.resolvedAt || (interactive && onResolve)) && !repliesCollapsed &&
- + {thread.resolvedAt ? + : + }
}
); } + + +function Resolution({thread, onUnresolve}) { + const {t} = useI18n({locale: 'ui'}); + const locale = useLocale({locale: 'ui'}); + + return ( + <> + +
+ + {t(thread.resolverName + ? 'pageflow_scrolled.review.resolution_by' + : 'pageflow_scrolled.review.resolution')} + + + {thread.resolverName && + {thread.resolverName}} + + +
+ + {onUnresolve && + } + + ); +} diff --git a/entry_types/scrolled/package/src/review/Thread.module.css b/entry_types/scrolled/package/src/review/Thread.module.css index 2a6ddc3c5e..28972dd9db 100644 --- a/entry_types/scrolled/package/src/review/Thread.module.css +++ b/entry_types/scrolled/package/src/review/Thread.module.css @@ -19,16 +19,11 @@ border-color: var(--review-thread-hover-border-color, var(--ui-accent-color-light)); } -.highlighted:not(.resolved) { +.highlighted { --review-thread-border: solid 1px var(--ui-accent-color); --review-thread-box-shadow: 0 0 0 space(1) var(--ui-accent-color-lighter); } -.resolved { - opacity: var(--review-resolved-thread-opacity, 1); - background: var(--review-resolved-thread-background, var(--ui-surface-color)); -} - /* Locates the new thread among several. Matches the badge dot, which carries the same meaning one level up. Straddles the corner so it stays clear of the menu button inside it. */ @@ -129,7 +124,8 @@ .resolveRow { display: flex; - justify-content: center; + align-items: center; + gap: space(2); margin-top: space(2); padding-top: space(2); border-top: 1px solid var(--ui-on-surface-color-lightest); @@ -140,7 +136,7 @@ font-weight: 500; display: flex; align-items: center; - gap: space(1); + gap: space(2); color: var(--ui-on-surface-color-light); background: none; border: none; @@ -148,6 +144,52 @@ padding: 0; } -.resolveButton:hover { +.resolveButton:hover, +.resolveButton:focus-visible { + color: var(--ui-primary-color); +} + +/* Takes the width of an avatar, so that icon and text line up with the + comments above. */ +.resolveRowIcon { + flex-shrink: 0; + margin: 0 space(1); +} + +.resolveIcon { + composes: resolveRowIcon; + color: var(--ui-on-surface-color-lighter); +} + +.resolveButton:hover .resolveIcon, +.resolveButton:focus-visible .resolveIcon { + color: inherit; +} + +.resolution { + display: flex; + flex-direction: column; + gap: space(1); + margin-right: auto; + color: var(--ui-on-surface-color-light); +} + +.resolutionIcon { + composes: resolveRowIcon; color: var(--ui-primary-color); } + +.resolutionMeta { + display: flex; + flex-wrap: wrap; + align-items: baseline; + gap: 0 space(2); +} + +.resolver { + font-weight: 600; + overflow-wrap: anywhere; + color: var(--ui-on-surface-color); +} + + diff --git a/entry_types/scrolled/spec/features/entry_previewer/commenting_on_content_elements_spec.rb b/entry_types/scrolled/spec/features/entry_previewer/commenting_on_content_elements_spec.rb index b794ef7f8a..e5afb638dd 100644 --- a/entry_types/scrolled/spec/features/entry_previewer/commenting_on_content_elements_spec.rb +++ b/entry_types/scrolled/spec/features/entry_previewer/commenting_on_content_elements_spec.rb @@ -13,6 +13,7 @@ translation('en', 'pageflow_scrolled.review.select_text_to_comment', 'Select text to comment') translation('en', 'pageflow_scrolled.review.resolve', 'Mark as resolved') translation('en', 'pageflow_scrolled.review.unresolve', 'Mark as unresolved') + translation('en', 'pageflow_scrolled.review.thread_actions', 'Thread actions') translation('en', 'pageflow_scrolled.review.resolved_count.one', '1 resolved') translation('en', 'pageflow_scrolled.review.resolved_count.other', '%{count} resolved') end @@ -158,6 +159,7 @@ click_button('1 resolved') expect(page).to have_text('Needs work', wait: 10) + find('[aria-label="Thread actions"]').click click_button('Mark as unresolved') expect(page).not_to have_text('1 resolved', wait: 10) From d139385cdcf598ea9df7e7b1997e0de5107c4f71 Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Wed, 19 Aug 2026 16:07:37 +0200 Subject: [PATCH 08/29] Derive a chronological feed of comment activity Answers what happened since the reviewer last looked, which neither the structural list nor the toolbar arrows can: both are ordered by document position, not by time. One entry per thread, ordered by the thread's most recent event. A thread rather than an event is the unit because both kinds of context a reviewer needs - the content a comment is about and the discussion around it - belong to the thread; per event, a thread that gained four replies would repeat itself four times over. Where a thread sits in the entry is left to the thread as well, so the entry carries it rather than a description of it. Lifts the unread predicate out of unreadComments so that the feed marks entries by the same rule - baseline included - rather than restating it, and so that the server side summary has a named counterpart to mirror. Resolutions have no read mark of their own and go by their thread's. --- app/models/pageflow/entry_comment_summary.rb | 2 + .../spec/review/activityEntries-spec.js | 322 ++++++++++++++++++ .../spec/review/unreadComments-spec.js | 71 +++- .../package/src/review/activityEntries.js | 88 +++++ .../scrolled/package/src/review/index.js | 1 + .../package/src/review/unreadComments.js | 27 +- 6 files changed, 499 insertions(+), 12 deletions(-) create mode 100644 entry_types/scrolled/package/spec/review/activityEntries-spec.js create mode 100644 entry_types/scrolled/package/src/review/activityEntries.js diff --git a/app/models/pageflow/entry_comment_summary.rb b/app/models/pageflow/entry_comment_summary.rb index bfbe3fea59..14bbfdec36 100644 --- a/app/models/pageflow/entry_comment_summary.rb +++ b/app/models/pageflow/entry_comment_summary.rb @@ -81,6 +81,8 @@ def self.build(threads, read_at:, user:) # Mirrors the unread rule of the review interface: own comments never # count, and neither do comments from before the user's baseline. + # Kept in sync with isUnseen in + # entry_types/scrolled/package/src/review/unreadComments.js. def self.unread?(comment, seen_up_to, user) comment.creator_id != user.id && (seen_up_to.nil? || comment.created_at > seen_up_to) diff --git a/entry_types/scrolled/package/spec/review/activityEntries-spec.js b/entry_types/scrolled/package/spec/review/activityEntries-spec.js new file mode 100644 index 0000000000..6ba1e93658 --- /dev/null +++ b/entry_types/scrolled/package/spec/review/activityEntries-spec.js @@ -0,0 +1,322 @@ +import { + activityEntries, + useActivityEntries, + useUnseenActivityCount +} from 'review/activityEntries'; +import {renderHookWithReviewState} from 'support/renderWithReviewState'; + +const currentUser = {id: 42, name: 'Alice'}; + +function thread({id = 1, permaId = id + 4, comments = [], ...rest}) { + return {id, permaId, comments, ...rest}; +} + +function comment({id = 100, creatorId = 43, creatorName = 'Bob', body = 'A comment', createdAt}) { + return {id, creatorId, creatorName, body, createdAt}; +} + +describe('activityEntries', () => { + it('emits one entry per thread', () => { + const entries = activityEntries({ + threads: [thread({ + comments: [ + comment({id: 100, body: 'A topic', createdAt: '2026-08-17T09:00:00.000Z'}), + comment({id: 101, body: 'A reply', createdAt: '2026-08-17T10:00:00.000Z'}) + ] + })], + currentUser, + commentThreadReads: {} + }); + + expect(entries).toHaveLength(1); + expect(entries[0]).toMatchObject({key: 'thread-1', threadId: 1, threadPermaId: 5}); + }); + + it('carries the thread along for rendering', () => { + const commentThread = thread({ + comments: [comment({createdAt: '2026-08-17T09:00:00.000Z'})] + }); + + const entries = activityEntries({ + threads: [commentThread], + currentUser, + commentThreadReads: {} + }); + + expect(entries[0].thread).toBe(commentThread); + }); + + it('skips threads without any event', () => { + const entries = activityEntries({ + threads: [thread({comments: []})], + currentUser, + commentThreadReads: {} + }); + + expect(entries).toEqual([]); + }); + + describe('latest event', () => { + it('is the opening comment of a thread without replies', () => { + const entries = activityEntries({ + threads: [thread({ + comments: [comment({createdAt: '2026-08-17T09:00:00.000Z'})] + })], + currentUser, + commentThreadReads: {} + }); + + expect(entries[0]).toMatchObject({at: '2026-08-17T09:00:00.000Z'}); + }); + + it('is the most recent reply of a thread with replies', () => { + const entries = activityEntries({ + threads: [thread({ + comments: [ + comment({id: 100, createdAt: '2026-08-17T09:00:00.000Z'}), + comment({id: 101, createdAt: '2026-08-17T10:00:00.000Z'}) + ] + })], + currentUser, + commentThreadReads: {} + }); + + expect(entries[0]).toMatchObject({at: '2026-08-17T10:00:00.000Z'}); + }); + + it('is the resolution of a resolved thread', () => { + const entries = activityEntries({ + threads: [thread({ + comments: [comment({createdAt: '2026-08-17T09:00:00.000Z'})], + resolvedAt: '2026-08-17T10:00:00.000Z', + resolvedById: 44 + })], + currentUser, + commentThreadReads: {} + }); + + expect(entries[0]).toMatchObject({ + at: '2026-08-17T10:00:00.000Z', + resolved: true + }); + }); + }); + + describe('order', () => { + it('puts threads with the most recent activity first', () => { + const entries = activityEntries({ + threads: [ + thread({id: 1, comments: [comment({id: 100, createdAt: '2026-08-17T09:00:00.000Z'})]}), + thread({id: 2, comments: [comment({id: 200, createdAt: '2026-08-17T11:00:00.000Z'})]}) + ], + currentUser, + commentThreadReads: {} + }); + + expect(entries.map(entry => entry.threadId)).toEqual([2, 1]); + }); + + it('goes by the latest event rather than when the thread started', () => { + const entries = activityEntries({ + threads: [ + thread({ + id: 1, + comments: [ + comment({id: 100, createdAt: '2026-08-10T09:00:00.000Z'}), + comment({id: 101, createdAt: '2026-08-17T12:00:00.000Z'}) + ] + }), + thread({id: 2, comments: [comment({id: 200, createdAt: '2026-08-17T11:00:00.000Z'})]}) + ], + currentUser, + commentThreadReads: {} + }); + + expect(entries.map(entry => entry.threadId)).toEqual([1, 2]); + }); + + it('keeps threads sharing a timestamp in a stable order', () => { + const entries = activityEntries({ + threads: [ + thread({id: 2, comments: [comment({id: 200, createdAt: '2026-08-17T09:00:00.000Z'})]}), + thread({id: 1, comments: [comment({id: 100, createdAt: '2026-08-17T09:00:00.000Z'})]}) + ], + currentUser, + commentThreadReads: {} + }); + + expect(entries.map(entry => entry.threadId)).toEqual([1, 2]); + }); + }); + + describe('unseen', () => { + it('counts the events the reviewer has not seen', () => { + const entries = activityEntries({ + threads: [thread({ + permaId: 5, + comments: [ + comment({id: 100, createdAt: '2026-08-17T09:00:00.000Z'}), + comment({id: 101, createdAt: '2026-08-17T11:00:00.000Z'}), + comment({id: 102, createdAt: '2026-08-17T12:00:00.000Z'}) + ] + })], + currentUser, + commentThreadReads: {5: '2026-08-17T10:00:00.000Z'} + }); + + expect(entries[0]).toMatchObject({ + unseenCount: 2, + unseenCommentIds: [101, 102] + }); + }); + + it('does not count own comments', () => { + const entries = activityEntries({ + threads: [thread({ + comments: [comment({creatorId: 42, createdAt: '2026-08-17T11:00:00.000Z'})] + })], + currentUser, + commentThreadReads: {} + }); + + expect(entries[0]).toMatchObject({unseenCount: 0, unseenCommentIds: []}); + }); + + // A resolution has no read record of its own, so it goes by the + // thread's: opening the thread clears it. + it('counts a resolution by the read state of its thread', () => { + const threads = permaId => [thread({ + permaId, + comments: [comment({createdAt: '2026-08-17T09:00:00.000Z'})], + resolvedAt: '2026-08-17T11:00:00.000Z', + resolvedById: 44 + })]; + + const unread = activityEntries({ + threads: threads(5), + currentUser, + commentThreadReads: {5: '2026-08-17T10:00:00.000Z'} + }); + const read = activityEntries({ + threads: threads(6), + currentUser, + commentThreadReads: {6: '2026-08-17T12:00:00.000Z'} + }); + + expect(unread[0].unseenCount).toEqual(1); + expect(read[0].unseenCount).toEqual(0); + }); + + it('leaves the resolution out of the unseen comment ids', () => { + const entries = activityEntries({ + threads: [thread({ + permaId: 5, + comments: [ + comment({id: 100, createdAt: '2026-08-17T09:00:00.000Z'}), + comment({id: 101, createdAt: '2026-08-17T11:00:00.000Z'}) + ], + resolvedAt: '2026-08-17T12:00:00.000Z', + resolvedById: 44 + })], + currentUser, + commentThreadReads: {5: '2026-08-17T10:00:00.000Z'} + }); + + expect(entries[0]).toMatchObject({unseenCount: 2, unseenCommentIds: [101]}); + }); + + it('counts nothing while the current user is unknown', () => { + const entries = activityEntries({ + threads: [thread({ + comments: [comment({createdAt: '2026-08-17T11:00:00.000Z'})] + })], + currentUser: null, + commentThreadReads: {} + }); + + expect(entries[0].unseenCount).toEqual(0); + }); + }); + + describe('useActivityEntries', () => { + const seed = { + sections: [{id: 1, permaId: 100}], + contentElements: [{id: 1, permaId: 10, sectionId: 1, typeName: 'textBlock'}] + }; + + const commentThread = { + id: 1, permaId: 5, + subjectType: 'ContentElement', subjectId: 10, + comments: [ + {id: 100, creatorId: 43, body: 'A topic', createdAt: '2026-08-17T09:00:00.000Z'}, + {id: 101, creatorId: 43, body: 'A reply', createdAt: '2026-08-17T11:00:00.000Z'} + ] + }; + + it('derives entries from located threads and review state', () => { + const {result} = renderHookWithReviewState( + () => useActivityEntries(), + { + seed, + currentUser, + commentThreads: [commentThread], + commentThreadReads: {5: '2026-08-17T10:00:00.000Z'} + } + ); + + expect(result.current).toHaveLength(1); + expect(result.current[0]).toMatchObject({ + threadId: 1, + unseenCount: 1, + unseenCommentIds: [101] + }); + }); + + it('includes threads whose content element is gone', () => { + const orphan = { + id: 2, permaId: 6, + subjectType: 'ContentElement', subjectId: 999, + sectionPermaId: 100, + comments: [{id: 200, creatorId: 43, createdAt: '2026-08-17T09:00:00.000Z'}] + }; + + const {result} = renderHookWithReviewState( + () => useActivityEntries(), + {seed, currentUser, commentThreads: [orphan]} + ); + + expect(result.current).toHaveLength(1); + expect(result.current[0].thread.orphaned).toBe(true); + }); + + describe('useUnseenActivityCount', () => { + it('counts the threads carrying something new', () => { + const {result} = renderHookWithReviewState( + () => useUnseenActivityCount(), + { + seed, + currentUser, + commentThreads: [commentThread], + commentThreadReads: {5: '2026-08-17T10:00:00.000Z'} + } + ); + + expect(result.current).toEqual(1); + }); + + it('counts nothing once the thread has been read', () => { + const {result} = renderHookWithReviewState( + () => useUnseenActivityCount(), + { + seed, + currentUser, + commentThreads: [commentThread], + commentThreadReads: {5: '2026-08-17T12:00:00.000Z'} + } + ); + + expect(result.current).toEqual(0); + }); + }); + }); +}); diff --git a/entry_types/scrolled/package/spec/review/unreadComments-spec.js b/entry_types/scrolled/package/spec/review/unreadComments-spec.js index b02437e3b6..21fa6eb214 100644 --- a/entry_types/scrolled/package/spec/review/unreadComments-spec.js +++ b/entry_types/scrolled/package/spec/review/unreadComments-spec.js @@ -1,4 +1,4 @@ -import {unreadComments, useUnreadComments} from 'review/unreadComments'; +import {isUnseen, unreadComments, useUnreadComments} from 'review/unreadComments'; import {renderHookWithReviewState} from 'support/renderWithReviewState'; describe('unreadComments', () => { @@ -104,6 +104,75 @@ describe('unreadComments', () => { expect(result).toEqual([]); }); + describe('isUnseen', () => { + it('is true for a comment created after the read timestamp', () => { + const result = isUnseen( + {creatorId: 43, createdAt: '2026-08-17T11:00:00.000Z'}, + {currentUser, readAt: '2026-08-17T10:00:00.000Z'} + ); + + expect(result).toBe(true); + }); + + it('is false for a comment created before the read timestamp', () => { + const result = isUnseen( + {creatorId: 43, createdAt: '2026-08-17T09:00:00.000Z'}, + {currentUser, readAt: '2026-08-17T10:00:00.000Z'} + ); + + expect(result).toBe(false); + }); + + it('is true for a comment in a never read thread', () => { + const result = isUnseen( + {creatorId: 43, createdAt: '2026-08-17T09:00:00.000Z'}, + {currentUser, readAt: undefined} + ); + + expect(result).toBe(true); + }); + + it('is false for an event of the current user', () => { + const result = isUnseen( + {creatorId: 42, createdAt: '2026-08-17T11:00:00.000Z'}, + {currentUser, readAt: undefined} + ); + + expect(result).toBe(false); + }); + + it('is false while the current user is unknown', () => { + const result = isUnseen( + {creatorId: 43, createdAt: '2026-08-17T11:00:00.000Z'}, + {currentUser: null, readAt: undefined} + ); + + expect(result).toBe(false); + }); + + describe('with a baseline on the current user', () => { + const joinedUser = {...currentUser, unreadCommentsSinceAt: '2026-08-17T10:00:00.000Z'}; + + it('is false for an event from before the baseline', () => { + const result = isUnseen( + {creatorId: 43, createdAt: '2026-08-17T09:00:00.000Z'}, + {currentUser: joinedUser, readAt: undefined} + ); + + expect(result).toBe(false); + }); + + it('is true for an event from after the baseline', () => { + const result = isUnseen( + {creatorId: 43, createdAt: '2026-08-17T11:00:00.000Z'}, + {currentUser: joinedUser, readAt: undefined} + ); + + expect(result).toBe(true); + }); + }); + }); + describe('useUnreadComments', () => { it('reads current user and read timestamp from review state', () => { const commentThread = { diff --git a/entry_types/scrolled/package/src/review/activityEntries.js b/entry_types/scrolled/package/src/review/activityEntries.js new file mode 100644 index 0000000000..e0d0d5cea4 --- /dev/null +++ b/entry_types/scrolled/package/src/review/activityEntries.js @@ -0,0 +1,88 @@ +import {useMemo} from 'react'; + +import {useCommentThreadReads, useCurrentUser} from './ReviewStateProvider'; +import {useDisplayedCommentThreadReads} from './commentThreadReadsSnapshot'; +import {useLocatedCommentThreads} from './useLocatedCommentThreads'; +import {isUnseen} from './unreadComments'; + +// Reads frozen read state, so that unseen markers hold still while the +// reviewer works through the list. +export function useActivityEntries() { + const {threads} = useLocatedCommentThreads(); + const currentUser = useCurrentUser(); + const commentThreadReads = useDisplayedCommentThreadReads(); + + return useMemo( + () => activityEntries({threads, currentUser, commentThreadReads}), + [threads, currentUser, commentThreadReads] + ); +} + +// For the control that opens the feed: reads live state, so that its +// indicator clears as threads are read. +export function useUnseenActivityCount() { + const {threads} = useLocatedCommentThreads(); + const currentUser = useCurrentUser(); + const commentThreadReads = useCommentThreadReads(); + + return useMemo( + () => activityEntries({threads, currentUser, commentThreadReads}) + .filter(entry => entry.unseenCount > 0).length, + [threads, currentUser, commentThreadReads] + ); +} + +export function activityEntries({threads, currentUser, commentThreadReads}) { + return threads + .map(thread => threadEntry(thread, { + currentUser, + readAt: commentThreadReads[thread.permaId] + })) + .filter(Boolean) + .sort(compareEntries); +} + +function threadEntry(thread, {currentUser, readAt}) { + const events = threadEvents(thread); + + if (!events.length) return null; + + const latest = events.reduce( + (result, event) => (new Date(event.at) >= new Date(result.at) ? event : result) + ); + const unseenEvents = events.filter(event => isUnseen(event, {currentUser, readAt})); + + return { + key: `thread-${thread.id}`, + thread, + threadId: thread.id, + threadPermaId: thread.permaId, + at: latest.at, + unseenCount: unseenEvents.length, + unseenCommentIds: unseenEvents.filter(event => event.id).map(event => event.id), + resolved: !!thread.resolvedAt + }; +} + +// A resolution leaves no read mark of its own, so it goes by the +// thread's: opening the thread clears it. +function threadEvents(thread) { + const events = thread.comments.map(comment => ({ + ...comment, + at: comment.createdAt + })); + + if (thread.resolvedAt) { + events.push({ + at: thread.resolvedAt, + createdAt: thread.resolvedAt, + creatorId: thread.resolvedById + }); + } + + return events; +} + +function compareEntries(a, b) { + return new Date(b.at) - new Date(a.at) || a.threadId - b.threadId; +} diff --git a/entry_types/scrolled/package/src/review/index.js b/entry_types/scrolled/package/src/review/index.js index 6a67404272..f57eeead55 100644 --- a/entry_types/scrolled/package/src/review/index.js +++ b/entry_types/scrolled/package/src/review/index.js @@ -8,6 +8,7 @@ export {useUnreadCommentCount} from './unreadComments'; export {ThreadsBadge} from './ThreadsBadge'; export {Badge} from './Badge'; export {ThreadList} from './ThreadList'; +export {activityEntries, useActivityEntries, useUnseenActivityCount} from './activityEntries'; export {CommentThreadReadsSnapshot} from './commentThreadReadsSnapshot'; export {Thread} from './Thread'; export {ScrollHighlightedThreadIntoViewProvider} from './scrollHighlightedThreadIntoView'; diff --git a/entry_types/scrolled/package/src/review/unreadComments.js b/entry_types/scrolled/package/src/review/unreadComments.js index 38c0f89c11..0525bd645e 100644 --- a/entry_types/scrolled/package/src/review/unreadComments.js +++ b/entry_types/scrolled/package/src/review/unreadComments.js @@ -49,26 +49,31 @@ export function useLiveUnreadComments(thread) { ); } -// Comments the reviewer has not seen yet. Own comments never count: the -// reviewer has read what they just wrote, and a thread would otherwise -// turn unread by replying to it. +// Comments the reviewer has not seen yet. +export function unreadComments(thread, {currentUser, readAt}) { + return thread.comments.filter(comment => isUnseen(comment, {currentUser, readAt})); +} + +// Whether an event in a thread is new to the reviewer. Own events never +// count: the reviewer has read what they just wrote, and a thread would +// otherwise turn unread by replying to it. // -// Comments from before the reviewer's baseline do not count either. It +// Events from before the reviewer's baseline do not count either. It // keeps the comments that were already there when read tracking started // - or when the reviewer joined - from all turning up as unread at once. // // Read state is only known once the current user has been fetched. Until -// then nothing counts as unread, so lists do not briefly show every +// then nothing counts as unseen, so lists do not briefly show every // thread as new. -export function unreadComments(thread, {currentUser, readAt}) { - if (!currentUser) return []; +// +// Kept in sync with Pageflow::EntryCommentSummary, which applies the same +// rule server side to summarize entries in the admin. +export function isUnseen({creatorId, createdAt}, {currentUser, readAt}) { + if (!currentUser || creatorId === currentUser.id) return false; const seenUpTo = latestTime([readAt, currentUser.unreadCommentsSinceAt]); - return thread.comments.filter( - comment => comment.creatorId !== currentUser.id && - (seenUpTo === null || new Date(comment.createdAt).getTime() > seenUpTo) - ); + return seenUpTo === null || new Date(createdAt).getTime() > seenUpTo; } function latestTime(timestamps) { From 6cbd5ba4b30f7cdb6cc3f320bd07a7d245cf88a8 Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Mon, 24 Aug 2026 21:31:18 +0200 Subject: [PATCH 09/29] Hold the activity feed's order while it is displayed Commenting on a thread or resolving one makes it the most recent again, which pushed it to the top of the feed while the reviewer was working in it - reordering the list under their own hands, for something they had just done themselves. Threads now keep the place they had when the list appeared, and coming back to the feed is what reflects the new order. Ordering by the latest activity of others instead would have made the rule depend on whether anyone else had touched a thread: a reply to your own thread would still reorder it, a reply to someone else's would not. --- .../spec/review/activityEntries-spec.js | 108 ++++++++++++++++++ .../package/src/review/activityEntries.js | 27 ++++- 2 files changed, 133 insertions(+), 2 deletions(-) diff --git a/entry_types/scrolled/package/spec/review/activityEntries-spec.js b/entry_types/scrolled/package/spec/review/activityEntries-spec.js index 6ba1e93658..c802b60efe 100644 --- a/entry_types/scrolled/package/spec/review/activityEntries-spec.js +++ b/entry_types/scrolled/package/spec/review/activityEntries-spec.js @@ -1,8 +1,11 @@ +import {act} from '@testing-library/react'; + import { activityEntries, useActivityEntries, useUnseenActivityCount } from 'review/activityEntries'; +import {postReviewStateThreadChangeMessage} from 'review/postMessage'; import {renderHookWithReviewState} from 'support/renderWithReviewState'; const currentUser = {id: 42, name: 'Alice'}; @@ -289,6 +292,111 @@ describe('activityEntries', () => { expect(result.current[0].thread.orphaned).toBe(true); }); + describe('while the feed is displayed', () => { + const older = { + id: 1, permaId: 5, + subjectType: 'ContentElement', subjectId: 10, + comments: [{id: 100, creatorId: 43, body: 'Older topic', + createdAt: '2026-08-17T09:00:00.000Z'}] + }; + + const newer = { + id: 2, permaId: 6, + subjectType: 'ContentElement', subjectId: 10, + comments: [{id: 200, creatorId: 43, body: 'Newer topic', + createdAt: '2026-08-17T11:00:00.000Z'}] + }; + + function renderEntries() { + return renderHookWithReviewState( + () => useActivityEntries(), + {seed, currentUser, commentThreads: [older, newer]} + ); + } + + // Posting crosses the window boundary, so it has to be flushed + // before the hook reflects it. + async function postThreadChange(thread) { + await act(async () => { + postReviewStateThreadChangeMessage(window, thread); + await new Promise(resolve => setTimeout(resolve, 0)); + }); + } + + function entryFor(entries, threadId) { + return entries.find(entry => entry.threadId === threadId); + } + + function withReply(thread, at) { + return { + ...thread, + comments: [...thread.comments, + {id: 300, creatorId: currentUser.id, body: 'A reply', createdAt: at}] + }; + } + + it('keeps threads in the place they had when it appeared', async () => { + const {result} = renderEntries(); + + await postThreadChange(withReply(older, '2026-08-17T12:00:00.000Z')); + + expect(result.current.map(entry => entry.threadId)).toEqual([2, 1]); + }); + + it('still reflects what was said', async () => { + const {result} = renderEntries(); + + await postThreadChange(withReply(older, '2026-08-17T12:00:00.000Z')); + + expect(entryFor(result.current, 1).thread.comments).toHaveLength(2); + }); + + it('keeps the time the place was taken', async () => { + const {result} = renderEntries(); + + await postThreadChange(withReply(older, '2026-08-17T12:00:00.000Z')); + + expect(entryFor(result.current, 1).at).toEqual('2026-08-17T09:00:00.000Z'); + }); + + it('orders threads that turn up later by their own time', async () => { + const {result} = renderEntries(); + + await postThreadChange({ + id: 3, permaId: 7, + subjectType: 'ContentElement', subjectId: 10, + comments: [{id: 400, creatorId: 43, body: 'Newest topic', + createdAt: '2026-08-17T13:00:00.000Z'}] + }); + + expect(result.current.map(entry => entry.threadId)).toEqual([3, 2, 1]); + }); + + it('reflects the new order on the next visit', async () => { + const {result: first} = renderEntries(); + + await postThreadChange(withReply(older, '2026-08-17T12:00:00.000Z')); + expect(first.current.map(entry => entry.threadId)).toEqual([2, 1]); + + const {result} = renderHookWithReviewState( + () => useActivityEntries(), + { + seed, + currentUser, + commentThreads: [ + {...older, + comments: [...older.comments, + {id: 300, creatorId: currentUser.id, body: 'A reply', + createdAt: '2026-08-17T12:00:00.000Z'}]}, + newer + ] + } + ); + + expect(result.current.map(entry => entry.threadId)).toEqual([1, 2]); + }); + }); + describe('useUnseenActivityCount', () => { it('counts the threads carrying something new', () => { const {result} = renderHookWithReviewState( diff --git a/entry_types/scrolled/package/src/review/activityEntries.js b/entry_types/scrolled/package/src/review/activityEntries.js index e0d0d5cea4..a6e9eec6dc 100644 --- a/entry_types/scrolled/package/src/review/activityEntries.js +++ b/entry_types/scrolled/package/src/review/activityEntries.js @@ -1,4 +1,4 @@ -import {useMemo} from 'react'; +import {useMemo, useRef} from 'react'; import {useCommentThreadReads, useCurrentUser} from './ReviewStateProvider'; import {useDisplayedCommentThreadReads} from './commentThreadReadsSnapshot'; @@ -12,10 +12,33 @@ export function useActivityEntries() { const currentUser = useCurrentUser(); const commentThreadReads = useDisplayedCommentThreadReads(); - return useMemo( + const entries = useMemo( () => activityEntries({threads, currentUser, commentThreadReads}), [threads, currentUser, commentThreadReads] ); + + return useHeldOrder(entries); +} + +// Replying to a thread or resolving one makes it the most recent again, +// which would shove it to the top under the reviewer's own hands. Threads +// keep the place they had when the list appeared, dated by when they took +// it so they do not change day either; the next visit reflects what +// happened last. +function useHeldOrder(entries) { + const takenAt = useRef(new Map()); + + return useMemo(() => { + entries.forEach(entry => { + if (!takenAt.current.has(entry.threadId)) { + takenAt.current.set(entry.threadId, entry.at); + } + }); + + return entries + .map(entry => ({...entry, at: takenAt.current.get(entry.threadId)})) + .sort(compareEntries); + }, [entries]); } // For the control that opens the feed: reads live state, so that its From 283134480758d60a231a2fe239763ed6b752eff2 Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Mon, 24 Aug 2026 13:19:26 +0200 Subject: [PATCH 10/29] Fold away earlier replies of a thread Lets a caller show the opening comment and the tail of a discussion with the replies in between folded into a marker, so a thread can be embedded where there is no room for all of it. A folded thread is a partial view, which is why it neither marks itself read nor offers the reply form. Read state is one timestamp per thread, so marking it would cover comments that were never shown; and replying to a discussion seen only in part invites talking past it. Expanding through the marker settles both. --- entry_types/scrolled/config/locales/de.yml | 3 + entry_types/scrolled/config/locales/en.yml | 3 + .../Thread/features/foldedReplies-spec.js | 214 ++++++++++++++++++ .../src/editor/views/ReviewView.module.css | 3 +- .../scrolled/package/src/review/Thread.js | 39 +++- .../package/src/review/Thread.module.css | 48 +++- 6 files changed, 301 insertions(+), 9 deletions(-) create mode 100644 entry_types/scrolled/package/spec/review/Thread/features/foldedReplies-spec.js diff --git a/entry_types/scrolled/config/locales/de.yml b/entry_types/scrolled/config/locales/de.yml index ed19a35be9..1a5c6f7689 100644 --- a/entry_types/scrolled/config/locales/de.yml +++ b/entry_types/scrolled/config/locales/de.yml @@ -2029,5 +2029,8 @@ de: reply_count: one: 1 Antwort other: '%{count} Antworten' + earlier_reply_count: + one: 1 weitere Antwort + other: '%{count} weitere Antworten' no_threads_yet: Noch keine Kommentare refers_to_deleted_element: Bezieht sich auf ein gelöschtes Element diff --git a/entry_types/scrolled/config/locales/en.yml b/entry_types/scrolled/config/locales/en.yml index 28c01986d8..61672a4d11 100644 --- a/entry_types/scrolled/config/locales/en.yml +++ b/entry_types/scrolled/config/locales/en.yml @@ -1857,5 +1857,8 @@ en: reply_count: one: 1 reply other: '%{count} replies' + earlier_reply_count: + one: 1 more + other: '%{count} more' no_threads_yet: No comments yet refers_to_deleted_element: Refers to a deleted element diff --git a/entry_types/scrolled/package/spec/review/Thread/features/foldedReplies-spec.js b/entry_types/scrolled/package/spec/review/Thread/features/foldedReplies-spec.js new file mode 100644 index 0000000000..48e5253fb6 --- /dev/null +++ b/entry_types/scrolled/package/spec/review/Thread/features/foldedReplies-spec.js @@ -0,0 +1,214 @@ +import React from 'react'; +import '@testing-library/jest-dom/extend-expect'; +import userEvent from '@testing-library/user-event'; +import {act} from '@testing-library/react'; +import {useFakeTranslations} from 'pageflow/testHelpers'; + +import {Thread} from 'review/Thread'; +import {renderWithReviewState} from 'support/renderWithReviewState'; +import { + simulateScrollingIntoView +} from 'support/fakeIntersectionObserver'; + +describe('Thread folded replies', () => { + useFakeTranslations({ + 'pageflow_scrolled.review.reply_placeholder': 'Reply...', + 'pageflow_scrolled.review.reply_count.one': '1 reply', + 'pageflow_scrolled.review.reply_count.other': '%{count} replies', + 'pageflow_scrolled.review.earlier_reply_count.one': '1 more', + 'pageflow_scrolled.review.earlier_reply_count.other': '%{count} more', + 'pageflow_scrolled.review.unread_replies': 'Unread replies' + }); + + const currentUser = {id: 42, name: 'Alice'}; + + function comment(attributes) { + return {creatorId: 43, creatorName: 'Bob', ...attributes}; + } + + const thread = { + id: 1, + permaId: 5, + subjectType: 'ContentElement', + subjectId: 10, + comments: [ + comment({id: 100, body: 'On the pull quote', createdAt: '2026-08-17T09:00:00.000Z'}), + comment({id: 101, body: 'First reply', createdAt: '2026-08-17T09:30:00.000Z'}), + comment({id: 102, body: 'Second reply', createdAt: '2026-08-17T10:00:00.000Z'}), + comment({id: 103, body: 'Third reply', createdAt: '2026-08-17T10:30:00.000Z'}) + ] + }; + + function render(ui, options = {}) { + return renderWithReviewState(ui, {currentUser, commentThreads: [thread], ...options}); + } + + it('shows the opening comment and the last replies asked for', () => { + const {getByText, queryByText} = render( + + ); + + expect(getByText('On the pull quote')).toBeInTheDocument(); + expect(getByText('Third reply')).toBeInTheDocument(); + expect(queryByText('First reply')).toBeNull(); + expect(queryByText('Second reply')).toBeNull(); + }); + + it('counts the folded replies between them', () => { + const {getByText} = render(); + + expect(getByText('2 more')).toBeInTheDocument(); + }); + + it('folds every reply away when none are asked for', () => { + const {getByText, queryByText} = render( + + ); + + expect(getByText('On the pull quote')).toBeInTheDocument(); + expect(getByText('3 more')).toBeInTheDocument(); + expect(queryByText('Third reply')).toBeNull(); + }); + + it('offers no reply count toggle while replies are folded', () => { + const {queryByRole} = render(); + + expect(queryByRole('button', {name: /3 replies/})).toBeNull(); + }); + + it('offers the reply count toggle once nothing is folded', () => { + const {getByRole} = render(); + + expect(getByRole('button', {name: /3 replies/})).toBeInTheDocument(); + }); + + it('offers the reply count toggle after expanding the folded replies', async () => { + const user = userEvent.setup(); + + function ExpandingThread() { + const [expanded, setExpanded] = React.useState(false); + + return ( + setExpanded(true)} /> + ); + } + + const {getByRole, queryByRole} = render(); + + expect(queryByRole('button', {name: /3 replies/})).toBeNull(); + + await user.click(getByRole('button', {name: '2 more'})); + + expect(getByRole('button', {name: /3 replies/})).toBeInTheDocument(); + }); + + it('offers the reply count toggle while collapsed with folded replies', () => { + const {getByRole} = render( + + ); + + expect(getByRole('button', {name: /3 replies/})).toBeInTheDocument(); + }); + + it('shows every reply without a visible count', () => { + const {getByText, queryByText} = render(); + + expect(getByText('First reply')).toBeInTheDocument(); + expect(getByText('Third reply')).toBeInTheDocument(); + expect(queryByText('2 more')).toBeNull(); + }); + + it('folds nothing when the count covers every reply', () => { + const {getByText, queryByText} = render( + + ); + + expect(getByText('First reply')).toBeInTheDocument(); + expect(queryByText('1 more')).toBeNull(); + }); + + it('reports the fold marker being clicked', async () => { + const user = userEvent.setup(); + const onExpandReplies = jest.fn(); + + const {getByRole} = render( + + ); + + await user.click(getByRole('button', {name: '2 more'})); + + expect(onExpandReplies).toHaveBeenCalled(); + }); + + it('does not offer the marker as a button without a handler', () => { + const {queryByRole, getByText} = render( + + ); + + expect(getByText('2 more')).toBeInTheDocument(); + expect(queryByRole('button', {name: '2 more'})).toBeNull(); + }); + + // Replying to a discussion you have only partly seen invites talking + // past what was already said. + it('hides the reply form while replies are folded away', () => { + const {queryByPlaceholderText} = render( + + ); + + expect(queryByPlaceholderText('Reply...')).toBeNull(); + }); + + it('offers the reply form once nothing is folded away', () => { + const {getByPlaceholderText} = render( + + ); + + expect(getByPlaceholderText('Reply...')).toBeInTheDocument(); + }); + + describe('marking read', () => { + beforeEach(() => { + jest.useFakeTimers(); + }); + + afterEach(() => { + jest.useRealTimers(); + jest.restoreAllMocks(); + }); + + function renderWithPostMessage(ui) { + const postMessage = jest.spyOn(window.top, 'postMessage').mockImplementation(() => {}); + + return {...render(ui), postMessage}; + } + + function markReadMessages(postMessage) { + return postMessage.mock.calls.filter(([message]) => message.type === 'MARK_THREADS_READ'); + } + + it('does not mark the thread read while replies are folded away', () => { + const {container, postMessage} = renderWithPostMessage( + + ); + + simulateScrollingIntoView(container); + act(() => jest.advanceTimersByTime(1000)); + + expect(markReadMessages(postMessage)).toEqual([]); + }); + + it('marks the thread read once nothing is folded away', () => { + const {container, postMessage} = renderWithPostMessage( + + ); + + simulateScrollingIntoView(container); + act(() => jest.advanceTimersByTime(1000)); + + expect(markReadMessages(postMessage)).toHaveLength(1); + }); + }); +}); 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 f1cf713588..cce413cd26 100644 --- a/entry_types/scrolled/package/src/editor/views/ReviewView.module.css +++ b/entry_types/scrolled/package/src/editor/views/ReviewView.module.css @@ -1,7 +1,8 @@ .container { position: relative; --review-thread-box-shadow: none; - --review-thread-border: 1px solid var(--ui-on-surface-color-lightest); + --review-thread-border-color: var(--ui-on-surface-color-lightest); + --review-thread-border: 1px solid var(--review-thread-border-color); --review-resolved-threads-pill-align: flex-end; --review-resolved-threads-pill-color: var(--ui-on-surface-color); --review-resolved-threads-pill-background-color: transparent; diff --git a/entry_types/scrolled/package/src/review/Thread.js b/entry_types/scrolled/package/src/review/Thread.js index eac4509c78..51f353139c 100644 --- a/entry_types/scrolled/package/src/review/Thread.js +++ b/entry_types/scrolled/package/src/review/Thread.js @@ -19,7 +19,7 @@ import ResolveIcon from './images/resolve.svg'; import UnresolveIcon from './images/unresolve.svg'; import styles from './Thread.module.css'; -export function Thread({thread, collapsed: collapsedProp, onToggle, onResolve, onClick, highlighted, showUnreadMarker, interactive = true}) { +export function Thread({thread, collapsed: collapsedProp, visibleReplyCount, onExpandReplies, onToggle, onResolve, onClick, highlighted, showUnreadMarker, interactive = true}) { const {t} = useI18n({locale: 'ui'}); const firstComment = thread.comments[0]; const replies = thread.comments.slice(1); @@ -31,6 +31,17 @@ export function Thread({thread, collapsed: collapsedProp, onToggle, onResolve, o const repliesCollapsed = collapsed && replies.length > 0; + // A partial view is not a read thread: read state is one timestamp per + // thread, so marking it read would cover comments never shown. A + // collapsed thread has nothing folded - hiding both the fold and the + // count would leave no way back into it. + const foldedReplyCount = visibleReplyCount === undefined || repliesCollapsed ? + 0 : + Math.max(replies.length - visibleReplyCount, 0); + const shownReplies = foldedReplyCount > 0 ? + replies.slice(foldedReplyCount) : + replies; + const unreadComments = useUnreadComments(thread); const unreadReplyCount = useMemo(() => { const ids = new Set(unreadComments.map(comment => comment.id)); @@ -75,7 +86,7 @@ export function Thread({thread, collapsed: collapsedProp, onToggle, onResolve, o const ref = useRef(); const scrollHighlightedIntoView = useScrollHighlightedThreadIntoView(); - useMarkThreadReadWhenSeen({thread, ref, enabled: !repliesCollapsed}); + useMarkThreadReadWhenSeen({thread, ref, enabled: !repliesCollapsed && !foldedReplyCount}); useEffect(() => { if (scrollHighlightedIntoView && highlighted && ref.current) { @@ -109,7 +120,7 @@ export function Thread({thread, collapsed: collapsedProp, onToggle, onResolve, o showQuote={outdatedQuotes.has(firstComment.id)} {...editProps(firstComment)} />} - {replies.length > 0 && + {replies.length > 0 && !foldedReplyCount && } - {!collapsed && replies.map(comment => ( + {!collapsed && foldedReplyCount > 0 && + } + + {!collapsed && shownReplies.map(comment => ( {comment.id === firstUnreadReplyId &&
@@ -141,7 +155,7 @@ export function Thread({thread, collapsed: collapsedProp, onToggle, onResolve, o ))} - {interactive && !thread.resolvedAt && !repliesCollapsed && !editing && + {interactive && !thread.resolvedAt && !repliesCollapsed && !foldedReplyCount && !editing && ); } + +function FoldedReplies({count, onExpand}) { + const {t} = useI18n({locale: 'ui'}); + const label = t('pageflow_scrolled.review.earlier_reply_count', {count}); + + if (!onExpand) { + return
{label}
; + } + + return ( + + ); +} diff --git a/entry_types/scrolled/package/src/review/Thread.module.css b/entry_types/scrolled/package/src/review/Thread.module.css index 28972dd9db..f7487b1a1e 100644 --- a/entry_types/scrolled/package/src/review/Thread.module.css +++ b/entry_types/scrolled/package/src/review/Thread.module.css @@ -7,7 +7,11 @@ background: var(--ui-surface-color); border-radius: rounded(lg); box-shadow: var(--review-thread-box-shadow, var(--ui-box-shadow)); - border: var(--review-thread-border, solid 1px var(--ui-surface-color)); + /* The folded seam is drawn in this too. Hosts restyling the border set + the color here as well, since a shorthand leaves nothing to read it + back out of. */ + --thread-border-color: var(--review-thread-border-color, var(--ui-surface-color)); + border: var(--review-thread-border, solid 1px var(--thread-border-color)); scroll-margin-top: space(11); } @@ -16,11 +20,13 @@ } .clickable:not(.highlighted):hover { - border-color: var(--review-thread-hover-border-color, var(--ui-accent-color-light)); + --thread-border-color: var(--review-thread-hover-border-color, var(--ui-accent-color-light)); + border-color: var(--thread-border-color); } .highlighted { - --review-thread-border: solid 1px var(--ui-accent-color); + --thread-border-color: var(--ui-accent-color); + --review-thread-border: solid 1px var(--thread-border-color); --review-thread-box-shadow: 0 0 0 space(1) var(--ui-accent-color-lighter); } @@ -192,4 +198,40 @@ color: var(--ui-on-surface-color); } +/* Drawn as a mask so the zig-zag takes the surrounding color, which a + data URI could not read from a custom property. */ +.foldedReplies { + display: flex; + align-items: center; + gap: space(2); + margin: space(1) space(-3); + font-size: space(3); + color: var(--ui-on-surface-color-light); +} + +.foldedReplies::before, +.foldedReplies::after { + content: ''; + flex: 1; + height: 10px; + background: var(--thread-border-color); + mask-image: url("data:image/svg+xml,%3Csvg xmlns='http://www.w3.org/2000/svg' width='16' height='10' viewBox='0 0 16 10'%3E%3Cpath d='M0 8L4 2L8 8L12 2L16 8' fill='none' stroke='%23000' stroke-width='1'/%3E%3C/svg%3E"); + mask-size: 16px 10px; + mask-repeat: repeat-x; + mask-position: center; +} +.foldedRepliesButton { + composes: foldedReplies; + font: inherit; + font-size: space(3); + background: none; + border: none; + padding: 0; + cursor: pointer; +} + +.foldedRepliesButton:hover, +.foldedRepliesButton:focus-visible { + color: var(--ui-primary-color); +} From 60c03e5580259ba6fbca0f5489e8239679d2fc0a Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Wed, 19 Aug 2026 16:12:59 +0200 Subject: [PATCH 11/29] Render a list of comment activity Each row says what happened and embeds the thread it happened in, with the earlier replies folded away. The latest reply stays visible: the headline announces it, so the row has to show it. Embedding the thread rather than a copy of its text puts both kinds of context a reviewer needs in one place: the row reveals the content the comments are about, and the fold marker reads the discussion without leaving the list. Where the thread sits and what it quotes are left out entirely - both are a click away, and saying them here only crowded the row. The thread carries the click and the highlight, as it does in the structural list, so a row needs no frame of its own. Only the actor's name is emphasized, the way a comment header does it. Shows 30 rows and extends on demand rather than capping the list silently. --- entry_types/scrolled/config/locales/de.yml | 3 + entry_types/scrolled/config/locales/en.yml | 3 + .../package/spec/review/ActivityList-spec.js | 268 ++++++++++++++++++ .../package/src/review/ActivityList.js | 73 +++++ .../src/review/ActivityList.module.css | 27 ++ .../scrolled/package/src/review/index.js | 1 + 6 files changed, 375 insertions(+) create mode 100644 entry_types/scrolled/package/spec/review/ActivityList-spec.js create mode 100644 entry_types/scrolled/package/src/review/ActivityList.js create mode 100644 entry_types/scrolled/package/src/review/ActivityList.module.css diff --git a/entry_types/scrolled/config/locales/de.yml b/entry_types/scrolled/config/locales/de.yml index 1a5c6f7689..0b05881c37 100644 --- a/entry_types/scrolled/config/locales/de.yml +++ b/entry_types/scrolled/config/locales/de.yml @@ -2034,3 +2034,6 @@ de: other: '%{count} weitere Antworten' no_threads_yet: Noch keine Kommentare refers_to_deleted_element: Bezieht sich auf ein gelöschtes Element + activity: + no_activity_yet: Noch keine Aktivität + show_more: Mehr anzeigen diff --git a/entry_types/scrolled/config/locales/en.yml b/entry_types/scrolled/config/locales/en.yml index 61672a4d11..71efe84323 100644 --- a/entry_types/scrolled/config/locales/en.yml +++ b/entry_types/scrolled/config/locales/en.yml @@ -1862,3 +1862,6 @@ en: other: '%{count} more' no_threads_yet: No comments yet refers_to_deleted_element: Refers to a deleted element + activity: + no_activity_yet: No activity yet + show_more: Show more diff --git a/entry_types/scrolled/package/spec/review/ActivityList-spec.js b/entry_types/scrolled/package/spec/review/ActivityList-spec.js new file mode 100644 index 0000000000..4476c31c36 --- /dev/null +++ b/entry_types/scrolled/package/spec/review/ActivityList-spec.js @@ -0,0 +1,268 @@ +import React from 'react'; +import '@testing-library/jest-dom/extend-expect'; +import userEvent from '@testing-library/user-event'; +import {useFakeTranslations} from 'pageflow/testHelpers'; + +import {ActivityList} from 'review/ActivityList'; +import {renderWithReviewState} from 'support/renderWithReviewState'; + +// ActivityList resolves its entries from the located threads, so the +// subjects the threads hang off have to exist in the entry structure. +const seed = { + sections: [{id: 1, permaId: 100}], + contentElements: [{id: 1, permaId: 10, sectionId: 1, typeName: 'textBlock'}] +}; + +const currentUser = {id: 42, name: 'Alice'}; + +function renderActivityList(ui, options = {}) { + return renderWithReviewState(ui, {seed, currentUser, ...options}); +} + +function thread({id = 1, comments = [], ...rest}) { + return { + id, + permaId: id + 4, + subjectType: 'ContentElement', + subjectId: 10, + comments, + ...rest + }; +} + +function comment({id = 100, creatorId = 43, creatorName = 'Bob', body = 'A comment', createdAt, ...rest}) { + return {id, creatorId, creatorName, body, createdAt, ...rest}; +} + +describe('ActivityList', () => { + useFakeTranslations({ + 'pageflow_scrolled.review.activity.no_activity_yet': 'No activity yet', + 'pageflow_scrolled.review.activity.show_more': 'Show more', + 'pageflow_scrolled.review.refers_to_deleted_element': 'Refers to a deleted element', + 'pageflow_scrolled.review.earlier_reply_count.one': '1 more', + 'pageflow_scrolled.review.earlier_reply_count.other': '%{count} more', + 'pageflow_scrolled.review.reply_count.one': '1 reply', + 'pageflow_scrolled.review.reply_count.other': '%{count} replies', + 'pageflow_scrolled.review.reply_placeholder': 'Reply...', + 'pageflow_scrolled.review.send': 'Send', + 'pageflow_scrolled.review.toggle_replies': 'Toggle replies', + 'pageflow_scrolled.review.resolution_by': 'Marked as resolved by', + 'pageflow_scrolled.review.resolve': 'Mark as resolved', + 'pageflow_scrolled.review.unresolve': 'Mark as unresolved' + }); + + it('shows who resolved a thread', () => { + const {getByText} = renderActivityList(, { + commentThreads: [thread({ + comments: [comment({body: 'A topic', createdAt: '2026-08-17T09:00:00.000Z'})], + resolvedAt: '2026-08-17T10:00:00.000Z', + resolvedById: 44, + resolverName: 'Carol' + })] + }); + + expect(getByText('Marked as resolved by')).toBeInTheDocument(); + expect(getByText('Carol')).toBeInTheDocument(); + }); + + it('lists threads with the most recent activity first', () => { + const {getAllByText} = renderActivityList(, { + commentThreads: [ + thread({ + id: 1, + comments: [comment({ + id: 100, creatorName: 'Bob', body: 'Older topic', + createdAt: '2026-08-17T09:00:00.000Z' + })] + }), + thread({ + id: 2, + comments: [comment({ + id: 200, creatorName: 'Carol', body: 'Newer topic', + createdAt: '2026-08-17T11:00:00.000Z' + })] + }) + ] + }); + + expect(getAllByText(/^(Newer|Older) topic$/).map(node => node.textContent)) + .toEqual(['Newer topic', 'Older topic']); + }); + + it('renders a blank slate without any activity', () => { + const {getByText} = renderActivityList(); + + expect(getByText('No activity yet')).toBeInTheDocument(); + }); + + it('leaves pointing out a deleted element to the thread', () => { + const {getByText} = renderActivityList(, { + commentThreads: [thread({ + subjectId: 999, + sectionPermaId: 100, + comments: [comment({body: 'A topic', createdAt: '2026-08-17T09:00:00.000Z'})] + })] + }); + + expect(getByText('Refers to a deleted element')).toBeInTheDocument(); + }); + + describe('embedded thread', () => { + const threadWithReplies = thread({ + comments: [ + comment({id: 100, body: 'A topic', createdAt: '2026-08-17T09:00:00.000Z'}), + comment({id: 101, body: 'First reply', createdAt: '2026-08-17T10:00:00.000Z'}), + comment({id: 102, body: 'Second reply', createdAt: '2026-08-17T11:00:00.000Z'}) + ] + }); + + // The headline names the latest event, so the row has to show it. + it('shows the opening comment and the latest reply', () => { + const {getByText, queryByText} = renderActivityList(, { + commentThreads: [threadWithReplies] + }); + + expect(getByText('A topic')).toBeInTheDocument(); + expect(getByText('Second reply')).toBeInTheDocument(); + expect(getByText('1 more')).toBeInTheDocument(); + expect(queryByText('First reply')).toBeNull(); + }); + + it('expands the discussion in place', async () => { + const user = userEvent.setup(); + + const {getByRole, getByText} = renderActivityList(, { + commentThreads: [threadWithReplies] + }); + + await user.click(getByRole('button', {name: '1 more'})); + + expect(getByText('First reply')).toBeInTheDocument(); + expect(getByText('Second reply')).toBeInTheDocument(); + }); + + it('offers the reply form only once expanded', async () => { + const user = userEvent.setup(); + + const {getByRole, getByPlaceholderText, queryByPlaceholderText} = + renderActivityList(, {commentThreads: [threadWithReplies]}); + + expect(queryByPlaceholderText('Reply...')).toBeNull(); + + await user.click(getByRole('button', {name: '1 more'})); + + expect(getByPlaceholderText('Reply...')).toBeInTheDocument(); + }); + + it('offers no reply count toggle while replies are folded', () => { + const {queryByRole} = renderActivityList(, { + commentThreads: [threadWithReplies] + }); + + expect(queryByRole('button', {name: /2 replies/})).toBeNull(); + }); + + it('collapses the row through the reply count once nothing is folded', async () => { + const user = userEvent.setup(); + + const {getByRole, getByText, queryByText} = renderActivityList(, { + commentThreads: [threadWithReplies] + }); + + await user.click(getByRole('button', {name: '1 more'})); + await user.click(getByRole('button', {name: /2 replies/})); + + expect(getByText('A topic')).toBeInTheDocument(); + expect(queryByText('First reply')).toBeNull(); + expect(queryByText('Second reply')).toBeNull(); + }); + + it('offers resolving without expanding', () => { + const {getByRole} = renderActivityList(, { + commentThreads: [threadWithReplies] + }); + + expect(getByRole('button', {name: 'Mark as resolved'})).toBeInTheDocument(); + }); + }); + + describe('interaction', () => { + function renderTwoEntries(ui) { + return renderActivityList(ui, { + commentThreads: [ + thread({ + id: 1, + comments: [comment({ + id: 100, creatorName: 'Bob', body: 'Older topic', + createdAt: '2026-08-17T09:00:00.000Z' + })] + }), + thread({ + id: 2, + comments: [comment({ + id: 200, creatorName: 'Carol', body: 'Newer topic', + createdAt: '2026-08-17T11:00:00.000Z' + })] + }) + ] + }); + } + + it('passes the entry of a clicked thread to onEntryClick', async () => { + const user = userEvent.setup(); + const onEntryClick = jest.fn(); + + const {getByText} = renderTwoEntries(); + + await user.click(getByText('Older topic')); + + expect(onEntryClick).toHaveBeenCalledWith( + expect.objectContaining({threadId: 1}) + ); + }); + + it('marks the thread of the highlighted entry as current', () => { + const {getByText} = renderTwoEntries(); + + expect(getByText('Older topic').closest('[aria-current]')).not.toBeNull(); + expect(getByText('Newer topic').closest('[aria-current]')).toBeNull(); + }); + }); + + describe('paging', () => { + function renderPagedEntries(ui) { + return renderActivityList(ui, { + commentThreads: [1, 2, 3].map(id => thread({ + id, + comments: [comment({ + id: id * 100, body: `Topic ${id}`, createdAt: `2026-08-1${id}T09:00:00.000Z` + })] + })) + }); + } + + it('shows only a first page of entries', () => { + const {getByText, queryByText} = renderPagedEntries(); + + expect(getByText('Topic 3')).toBeInTheDocument(); + expect(getByText('Topic 2')).toBeInTheDocument(); + expect(queryByText('Topic 1')).toBeNull(); + }); + + it('extends the list on show more', async () => { + const user = userEvent.setup(); + + const {getByRole, getByText} = renderPagedEntries(); + + await user.click(getByRole('button', {name: 'Show more'})); + + expect(getByText('Topic 1')).toBeInTheDocument(); + }); + + it('offers no show more button once everything is shown', () => { + const {queryByRole} = renderPagedEntries(); + + expect(queryByRole('button', {name: 'Show more'})).toBeNull(); + }); + }); +}); diff --git a/entry_types/scrolled/package/src/review/ActivityList.js b/entry_types/scrolled/package/src/review/ActivityList.js new file mode 100644 index 0000000000..b05f204734 --- /dev/null +++ b/entry_types/scrolled/package/src/review/ActivityList.js @@ -0,0 +1,73 @@ +import React, {useState} from 'react'; + +import {useI18n} from 'pageflow-scrolled/frontend'; +import {Thread} from './Thread'; +import {CommentThreadReadsSnapshot} from './commentThreadReadsSnapshot'; +import {useActivityEntries} from './activityEntries'; +import {postUpdateThreadMessage} from './postMessage'; + +import styles from './ActivityList.module.css'; + +export function ActivityList({onEntryClick, highlightedThreadId, pageSize = 30}) { + return ( + + + + ); +} + +// Reading the entries has to happen inside the snapshot for the freeze to +// apply. +function Entries({onEntryClick, highlightedThreadId, pageSize}) { + const {t} = useI18n({locale: 'ui'}); + const entries = useActivityEntries(); + const [pages, setPages] = useState(1); + + if (!entries.length) { + return ( +

+ {t('pageflow_scrolled.review.activity.no_activity_yet')} +

+ ); + } + + const shown = entries.slice(0, pages * pageSize); + + return ( +
+ {shown.map(entry => + onEntryClick(entry))} /> + )} + + {shown.length < entries.length && + } +
+ ); +} + +function Entry({entry, highlighted, onClick}) { + const [expanded, setExpanded] = useState(false); + const [collapsed, setCollapsed] = useState(false); + + return ( + setExpanded(true)} + collapsed={collapsed} + onToggle={() => setCollapsed(!collapsed)} + onClick={onClick} + highlighted={highlighted} + onResolve={() => postUpdateThreadMessage({ + threadId: entry.threadId, + resolved: !entry.resolved + })} /> + ); +} + diff --git a/entry_types/scrolled/package/src/review/ActivityList.module.css b/entry_types/scrolled/package/src/review/ActivityList.module.css new file mode 100644 index 0000000000..e6a8cced90 --- /dev/null +++ b/entry_types/scrolled/package/src/review/ActivityList.module.css @@ -0,0 +1,27 @@ +.list { + display: flex; + flex-direction: column; + gap: space(3); +} + +.showMore { + align-self: center; + font: inherit; + font-weight: 500; + color: var(--ui-on-surface-color-light); + background: none; + border: none; + padding: space(2); + cursor: pointer; +} + +.showMore:hover { + color: var(--ui-primary-color); +} + +.blankSlate { + margin: 0; + padding: space(4) space(2); + text-align: center; + color: var(--ui-on-surface-color-light); +} diff --git a/entry_types/scrolled/package/src/review/index.js b/entry_types/scrolled/package/src/review/index.js index f57eeead55..31a5fc621f 100644 --- a/entry_types/scrolled/package/src/review/index.js +++ b/entry_types/scrolled/package/src/review/index.js @@ -9,6 +9,7 @@ export {ThreadsBadge} from './ThreadsBadge'; export {Badge} from './Badge'; export {ThreadList} from './ThreadList'; export {activityEntries, useActivityEntries, useUnseenActivityCount} from './activityEntries'; +export {ActivityList} from './ActivityList'; export {CommentThreadReadsSnapshot} from './commentThreadReadsSnapshot'; export {Thread} from './Thread'; export {ScrollHighlightedThreadIntoViewProvider} from './scrollHighlightedThreadIntoView'; From 94c6ff23ee14870183b33d4603c56bdea97c0a7a Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Mon, 24 Aug 2026 14:50:07 +0200 Subject: [PATCH 12/29] Group comment activity by date Heads the rows of each day, with today and yesterday named rather than dated. Structure the list was missing: rows carried a timestamp each, which said less than a heading does and repeated what the discussion in every thread already dates. The heading also holds the machine readable date the rows gave up, and formats from a timestamp within the day rather than from the day itself - a bare date parses as UTC midnight, which reads as the day before west of it. Groups the shown slice rather than every entry, so a day spanning the show more boundary is not headed twice. --- entry_types/scrolled/config/locales/de.yml | 9 + entry_types/scrolled/config/locales/en.yml | 9 + .../package/spec/review/ActivityList-spec.js | 218 +++++++++++++++++- .../package/src/review/ActivityList.js | 154 +++++++++++-- .../src/review/ActivityList.module.css | 35 +++ 5 files changed, 399 insertions(+), 26 deletions(-) diff --git a/entry_types/scrolled/config/locales/de.yml b/entry_types/scrolled/config/locales/de.yml index 0b05881c37..16ac0a0e35 100644 --- a/entry_types/scrolled/config/locales/de.yml +++ b/entry_types/scrolled/config/locales/de.yml @@ -2035,5 +2035,14 @@ de: no_threads_yet: Noch keine Kommentare refers_to_deleted_element: Bezieht sich auf ein gelöschtes Element activity: + summary: + topic: Thema begonnen + reply_count: + one: 1 Antwort + other: '%{count} Antworten' + resolution: als gelöst markiert + and: ' und ' + today: Heute + yesterday: Gestern no_activity_yet: Noch keine Aktivität show_more: Mehr anzeigen diff --git a/entry_types/scrolled/config/locales/en.yml b/entry_types/scrolled/config/locales/en.yml index 71efe84323..2bb50d1c1b 100644 --- a/entry_types/scrolled/config/locales/en.yml +++ b/entry_types/scrolled/config/locales/en.yml @@ -1863,5 +1863,14 @@ en: no_threads_yet: No comments yet refers_to_deleted_element: Refers to a deleted element activity: + summary: + topic: topic started + reply_count: + one: 1 reply + other: '%{count} replies' + resolution: marked as resolved + and: ' and ' + today: Today + yesterday: Yesterday no_activity_yet: No activity yet show_more: Show more diff --git a/entry_types/scrolled/package/spec/review/ActivityList-spec.js b/entry_types/scrolled/package/spec/review/ActivityList-spec.js index 4476c31c36..8e94a3c87e 100644 --- a/entry_types/scrolled/package/spec/review/ActivityList-spec.js +++ b/entry_types/scrolled/package/spec/review/ActivityList-spec.js @@ -1,9 +1,11 @@ import React from 'react'; import '@testing-library/jest-dom/extend-expect'; import userEvent from '@testing-library/user-event'; +import I18n from 'i18n-js'; import {useFakeTranslations} from 'pageflow/testHelpers'; import {ActivityList} from 'review/ActivityList'; +import styles from 'review/ActivityList.module.css'; import {renderWithReviewState} from 'support/renderWithReviewState'; // ActivityList resolves its entries from the located threads, so the @@ -38,6 +40,13 @@ describe('ActivityList', () => { useFakeTranslations({ 'pageflow_scrolled.review.activity.no_activity_yet': 'No activity yet', 'pageflow_scrolled.review.activity.show_more': 'Show more', + 'pageflow_scrolled.review.activity.summary.topic': 'topic started', + 'pageflow_scrolled.review.activity.summary.reply_count.one': '1 reply', + 'pageflow_scrolled.review.activity.summary.reply_count.other': '%{count} replies', + 'pageflow_scrolled.review.activity.summary.resolution': 'marked as resolved', + 'pageflow_scrolled.review.activity.summary.and': ' and ', + 'pageflow_scrolled.review.activity.today': 'Today', + 'pageflow_scrolled.review.activity.yesterday': 'Yesterday', 'pageflow_scrolled.review.refers_to_deleted_element': 'Refers to a deleted element', 'pageflow_scrolled.review.earlier_reply_count.one': '1 more', 'pageflow_scrolled.review.earlier_reply_count.other': '%{count} more', @@ -107,32 +116,126 @@ describe('ActivityList', () => { expect(getByText('Refers to a deleted element')).toBeInTheDocument(); }); + describe('summary', () => { + function summaryOf(container) { + return container.querySelector(`.${styles.summary}`); + } + + it('names a topic started on the day it is listed under', () => { + const {container} = renderActivityList(, { + commentThreads: [thread({ + comments: [comment({body: 'A topic', createdAt: '2026-08-17T09:00:00.000Z'})] + })] + }); + + expect(summaryOf(container)).toHaveTextContent('topic started'); + }); + + it('counts the replies of that day', () => { + const {container} = renderActivityList(, { + commentThreads: [thread({ + comments: [ + comment({id: 100, body: 'A topic', createdAt: '2026-08-16T12:00:00.000Z'}), + comment({id: 101, body: 'First', createdAt: '2026-08-17T12:00:00.000Z'}), + comment({id: 102, body: 'Second', createdAt: '2026-08-17T13:00:00.000Z'}) + ] + })] + }); + + expect(summaryOf(container)).toHaveTextContent('2 replies'); + }); + + it('joins what happened on that day', () => { + const {container} = renderActivityList(, { + commentThreads: [thread({ + comments: [ + comment({id: 100, body: 'A topic', createdAt: '2026-08-17T09:00:00.000Z'}), + comment({id: 101, body: 'A reply', createdAt: '2026-08-17T12:00:00.000Z'}) + ], + resolvedAt: '2026-08-17T13:00:00.000Z', + resolvedById: 44, + resolverName: 'Carol' + })] + }); + + expect(summaryOf(container)) + .toHaveTextContent('topic started, 1 reply and marked as resolved'); + }); + + it('leaves out what happened on other days', () => { + const {container} = renderActivityList(, { + commentThreads: [thread({ + comments: [ + comment({id: 100, body: 'A topic', createdAt: '2026-08-16T12:00:00.000Z'}), + comment({id: 101, body: 'A reply', createdAt: '2026-08-17T12:00:00.000Z'}) + ] + })] + }); + + expect(summaryOf(container).textContent).toEqual('1 reply'); + }); + }); + describe('embedded thread', () => { const threadWithReplies = thread({ comments: [ comment({id: 100, body: 'A topic', createdAt: '2026-08-17T09:00:00.000Z'}), comment({id: 101, body: 'First reply', createdAt: '2026-08-17T10:00:00.000Z'}), - comment({id: 102, body: 'Second reply', createdAt: '2026-08-17T11:00:00.000Z'}) + comment({id: 102, body: 'Second reply', createdAt: '2026-08-17T12:00:00.000Z'}) ] }); - // The headline names the latest event, so the row has to show it. - it('shows the opening comment and the latest reply', () => { + const threadAcrossDays = thread({ + comments: [ + comment({id: 100, body: 'A topic', createdAt: '2026-08-16T12:00:00.000Z'}), + comment({id: 101, body: 'First reply', createdAt: '2026-08-16T13:00:00.000Z'}), + comment({id: 102, body: 'Second reply', createdAt: '2026-08-17T12:00:00.000Z'}) + ] + }); + + it('shows every reply made on the day it is listed under', () => { const {getByText, queryByText} = renderActivityList(, { commentThreads: [threadWithReplies] }); + expect(getByText('A topic')).toBeInTheDocument(); + expect(getByText('First reply')).toBeInTheDocument(); + expect(getByText('Second reply')).toBeInTheDocument(); + expect(queryByText('1 more')).toBeNull(); + }); + + it('folds away replies from earlier days', () => { + const {getByText, queryByText} = renderActivityList(, { + commentThreads: [threadAcrossDays] + }); + expect(getByText('A topic')).toBeInTheDocument(); expect(getByText('Second reply')).toBeInTheDocument(); expect(getByText('1 more')).toBeInTheDocument(); expect(queryByText('First reply')).toBeNull(); }); + it('shows the latest reply when the day contributed none', () => { + const {getByText} = renderActivityList(, { + commentThreads: [thread({ + comments: [ + comment({id: 100, body: 'A topic', createdAt: '2026-08-16T12:00:00.000Z'}), + comment({id: 101, body: 'Only reply', createdAt: '2026-08-16T13:00:00.000Z'}) + ], + resolvedAt: '2026-08-17T12:00:00.000Z', + resolvedById: 44, + resolverName: 'Carol' + })] + }); + + expect(getByText('Only reply')).toBeInTheDocument(); + }); + it('expands the discussion in place', async () => { const user = userEvent.setup(); const {getByRole, getByText} = renderActivityList(, { - commentThreads: [threadWithReplies] + commentThreads: [threadAcrossDays] }); await user.click(getByRole('button', {name: '1 more'})); @@ -145,7 +248,7 @@ describe('ActivityList', () => { const user = userEvent.setup(); const {getByRole, getByPlaceholderText, queryByPlaceholderText} = - renderActivityList(, {commentThreads: [threadWithReplies]}); + renderActivityList(, {commentThreads: [threadAcrossDays]}); expect(queryByPlaceholderText('Reply...')).toBeNull(); @@ -156,7 +259,7 @@ describe('ActivityList', () => { it('offers no reply count toggle while replies are folded', () => { const {queryByRole} = renderActivityList(, { - commentThreads: [threadWithReplies] + commentThreads: [threadAcrossDays] }); expect(queryByRole('button', {name: /2 replies/})).toBeNull(); @@ -166,7 +269,7 @@ describe('ActivityList', () => { const user = userEvent.setup(); const {getByRole, getByText, queryByText} = renderActivityList(, { - commentThreads: [threadWithReplies] + commentThreads: [threadAcrossDays] }); await user.click(getByRole('button', {name: '1 more'})); @@ -179,7 +282,7 @@ describe('ActivityList', () => { it('offers resolving without expanding', () => { const {getByRole} = renderActivityList(, { - commentThreads: [threadWithReplies] + commentThreads: [threadAcrossDays] }); expect(getByRole('button', {name: 'Mark as resolved'})).toBeInTheDocument(); @@ -229,6 +332,105 @@ describe('ActivityList', () => { }); }); + describe('date groups', () => { + // Relative labels need a fixed today to be worth asserting. + beforeEach(() => { + jest.useFakeTimers(); + jest.setSystemTime(new Date('2026-08-24T12:00:00.000Z')); + I18n.locale = 'en'; + }); + + afterEach(() => { + jest.useRealTimers(); + }); + + function dayThread({id, createdAt}) { + return thread({ + id, + comments: [comment({id: id * 100, body: `Topic ${id}`, createdAt})] + }); + } + + function renderDays(ui, days) { + return renderActivityList(ui, {commentThreads: days.map(dayThread)}); + } + + it('heads the rows of today with a relative label', () => { + const {getByRole} = renderDays(, [ + {id: 1, createdAt: '2026-08-24T09:00:00.000Z'} + ]); + + expect(getByRole('heading', {name: 'Today'})).toBeInTheDocument(); + }); + + it('heads the rows of the day before with a relative label', () => { + const {getByRole} = renderDays(, [ + {id: 1, createdAt: '2026-08-23T09:00:00.000Z'} + ]); + + expect(getByRole('heading', {name: 'Yesterday'})).toBeInTheDocument(); + }); + + it('heads earlier rows with their date', () => { + const {getByRole} = renderDays(, [ + {id: 1, createdAt: '2026-08-17T09:00:00.000Z'} + ]); + + expect(getByRole('heading', {name: 'Aug 17'})).toBeInTheDocument(); + }); + + it('includes the year for rows of previous years', () => { + const {getByRole} = renderDays(, [ + {id: 1, createdAt: '2025-08-17T09:00:00.000Z'} + ]); + + expect(getByRole('heading', {name: 'Aug 17, 2025'})).toBeInTheDocument(); + }); + + it('heads each day once, in the order of the rows', () => { + const {getAllByRole} = renderDays(, [ + {id: 1, createdAt: '2026-08-24T09:00:00.000Z'}, + {id: 2, createdAt: '2026-08-24T11:00:00.000Z'}, + {id: 3, createdAt: '2026-08-23T09:00:00.000Z'} + ]); + + expect(getAllByRole('heading').map(node => node.textContent)) + .toEqual(['Today', 'Yesterday']); + }); + + it('groups the rows under the heading of their day', () => { + const {getByRole, getByText} = renderDays(, [ + {id: 1, createdAt: '2026-08-24T09:00:00.000Z'}, + {id: 2, createdAt: '2026-08-23T09:00:00.000Z'} + ]); + + const yesterday = getByRole('heading', {name: 'Yesterday'}); + + expect(yesterday.compareDocumentPosition(getByText('Topic 1')) & + Node.DOCUMENT_POSITION_PRECEDING).toBeTruthy(); + expect(yesterday.compareDocumentPosition(getByText('Topic 2')) & + Node.DOCUMENT_POSITION_FOLLOWING).toBeTruthy(); + }); + + it('carries a machine readable date on the heading', () => { + const {getByRole} = renderDays(, [ + {id: 1, createdAt: '2026-08-17T09:00:00.000Z'} + ]); + + expect(getByRole('heading', {name: 'Aug 17'}).querySelector('time')) + .toHaveAttribute('dateTime', '2026-08-17'); + }); + + it('heads a day only once when it is split across pages', () => { + const {getAllByRole} = renderDays(, [ + {id: 1, createdAt: '2026-08-24T09:00:00.000Z'}, + {id: 2, createdAt: '2026-08-24T11:00:00.000Z'} + ]); + + expect(getAllByRole('heading').map(node => node.textContent)).toEqual(['Today']); + }); + }); + describe('paging', () => { function renderPagedEntries(ui) { return renderActivityList(ui, { diff --git a/entry_types/scrolled/package/src/review/ActivityList.js b/entry_types/scrolled/package/src/review/ActivityList.js index b05f204734..a4538fd997 100644 --- a/entry_types/scrolled/package/src/review/ActivityList.js +++ b/entry_types/scrolled/package/src/review/ActivityList.js @@ -1,9 +1,10 @@ import React, {useState} from 'react'; -import {useI18n} from 'pageflow-scrolled/frontend'; +import {useI18n, useLocale} from 'pageflow-scrolled/frontend'; import {Thread} from './Thread'; import {CommentThreadReadsSnapshot} from './commentThreadReadsSnapshot'; import {useActivityEntries} from './activityEntries'; +import {formatDate} from './formatDate'; import {postUpdateThreadMessage} from './postMessage'; import styles from './ActivityList.module.css'; @@ -37,11 +38,17 @@ function Entries({onEntryClick, highlightedThreadId, pageSize}) { return (
- {shown.map(entry => - onEntryClick(entry))} /> + {dayGroups(shown).map(group => + + + {group.entries.map(entry => + onEntryClick(entry))} /> + )} + )} {shown.length < entries.length && @@ -52,22 +59,133 @@ function Entries({onEntryClick, highlightedThreadId, pageSize}) { ); } -function Entry({entry, highlighted, onClick}) { +function Entry({entry, day, highlighted, onClick}) { + const {t} = useI18n({locale: 'ui'}); const [expanded, setExpanded] = useState(false); const [collapsed, setCollapsed] = useState(false); return ( - setExpanded(true)} - collapsed={collapsed} - onToggle={() => setCollapsed(!collapsed)} - onClick={onClick} - highlighted={highlighted} - onResolve={() => postUpdateThreadMessage({ - threadId: entry.threadId, - resolved: !entry.resolved - })} /> +
+

{summary(t, entry, day)}

+ setExpanded(true)} + collapsed={collapsed} + onToggle={() => setCollapsed(!collapsed)} + onClick={onClick} + highlighted={highlighted} + onResolve={() => postUpdateThreadMessage({ + threadId: entry.threadId, + resolved: !entry.resolved + })} /> +
); } +// What happened to the thread on the day the row is listed under, which +// is what puts it there. +function summary(t, {thread}, day) { + const replies = thread.comments.slice(1); + + const parts = [ + onDay(thread.comments[0], day) && t('pageflow_scrolled.review.activity.summary.topic'), + replyCountPart(t, replies.filter(reply => onDay(reply, day)).length), + thread.resolvedAt && dayOf(thread.resolvedAt) === day && + t('pageflow_scrolled.review.activity.summary.resolution') + ].filter(Boolean); + + return joinParts(t, parts); +} + +function replyCountPart(t, count) { + return count > 0 && + t('pageflow_scrolled.review.activity.summary.reply_count', {count}); +} + +function onDay(comment, day) { + return dayOf(comment.createdAt) === day; +} + +function joinParts(t, parts) { + if (parts.length < 2) { + return parts[0]; + } + + return [parts.slice(0, -1).join(', '), parts[parts.length - 1]] + .join(t('pageflow_scrolled.review.activity.summary.and')); +} + +// Everything said on the day the row is listed under, falling back to +// the latest reply so that a row never shows a thread without the comment +// it is listed for. +function visibleReplyCount({thread}, day) { + const replies = thread.comments.slice(1); + const firstOfDay = replies.findIndex(reply => dayOf(reply.createdAt) === day); + + return firstOfDay < 0 ? Math.min(replies.length, 1) : replies.length - firstOfDay; +} + +function DayHeading({day, at}) { + const {t} = useI18n({locale: 'ui'}); + const locale = useLocale({locale: 'ui'}); + + return ( +

+ +

+ ); +} + +// Grouping the shown slice rather than every entry keeps a day that spans +// the "show more" boundary from being headed twice. +function dayGroups(entries) { + const groups = []; + + entries.forEach(entry => { + const day = dayOf(entry.at); + const current = groups[groups.length - 1]; + + if (current && current.day === day) { + current.entries.push(entry); + } + else { + groups.push({day, at: entry.at, entries: [entry]}); + } + }); + + return groups; +} + +function dayOf(at) { + const date = new Date(at); + const month = String(date.getMonth() + 1).padStart(2, '0'); + const day = String(date.getDate()).padStart(2, '0'); + + return `${date.getFullYear()}-${month}-${day}`; +} + +// Formatted from a timestamp within the day: a bare date parses as UTC +// midnight, which reads as the day before west of it. +function dayLabel(t, {day, at}, locale) { + const days = daysSince(day); + + if (days === 0) { + return t('pageflow_scrolled.review.activity.today'); + } + + if (days === 1) { + return t('pageflow_scrolled.review.activity.yesterday'); + } + + return formatDate(at, locale); +} + +function daysSince(day) { + const [year, month, date] = day.split('-').map(Number); + const then = new Date(year, month - 1, date); + const now = new Date(); + + return Math.round( + (new Date(now.getFullYear(), now.getMonth(), now.getDate()) - then) / 86400000 + ); +} diff --git a/entry_types/scrolled/package/src/review/ActivityList.module.css b/entry_types/scrolled/package/src/review/ActivityList.module.css index e6a8cced90..3909848b70 100644 --- a/entry_types/scrolled/package/src/review/ActivityList.module.css +++ b/entry_types/scrolled/package/src/review/ActivityList.module.css @@ -25,3 +25,38 @@ text-align: center; color: var(--ui-on-surface-color-light); } + +.entry { + display: flex; + flex-direction: column; + gap: space(2); +} + +.summary { + margin: space(1) 0; + line-height: 1.4; + color: var(--ui-on-surface-color-light); +} + +/* The parts are fragments that get joined in whichever order the day + holds them, so the sentence is capitalized where it starts. */ +.summary::first-letter { + text-transform: uppercase; +} + +.dayHeading { + display: flex; + align-items: center; + gap: space(2); + margin: space(2) 0 0; + font-size: space(3); + font-weight: 500; + color: var(--ui-on-surface-color-light); +} + +.dayHeading::before, +.dayHeading::after { + content: ''; + flex: 1; + border-bottom: solid 1px var(--ui-on-surface-color-lightest); +} From f4497b56379b5e3249ac6670fb3ffb74966f9d02 Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Wed, 19 Aug 2026 16:16:31 +0200 Subject: [PATCH 13/29] Mark activity the reviewer has not seen Each row carries the dot that badges and threads already use one level up, and unfolds its thread from the first unread reply rather than showing only the latest one, so the unread part reads in the context of the comment it answers. No separator between seen and unseen rows: read marks are per thread, so a line drawn through a global time order lands somewhere arbitrary as soon as part of what is above it has been read. The date headings give the list its structure and the per row dots carry the exact signal. --- .../package/spec/review/ActivityList-spec.js | 108 ++++++++++++++++-- .../package/src/review/ActivityList.js | 22 ++-- 2 files changed, 114 insertions(+), 16 deletions(-) diff --git a/entry_types/scrolled/package/spec/review/ActivityList-spec.js b/entry_types/scrolled/package/spec/review/ActivityList-spec.js index 8e94a3c87e..33b87a9acf 100644 --- a/entry_types/scrolled/package/spec/review/ActivityList-spec.js +++ b/entry_types/scrolled/package/spec/review/ActivityList-spec.js @@ -55,6 +55,8 @@ describe('ActivityList', () => { 'pageflow_scrolled.review.reply_placeholder': 'Reply...', 'pageflow_scrolled.review.send': 'Send', 'pageflow_scrolled.review.toggle_replies': 'Toggle replies', + 'pageflow_scrolled.review.unread_comment_count.one': '1 unread comment', + 'pageflow_scrolled.review.unread_comment_count.other': '%{count} unread comments', 'pageflow_scrolled.review.resolution_by': 'Marked as resolved by', 'pageflow_scrolled.review.resolve': 'Mark as resolved', 'pageflow_scrolled.review.unresolve': 'Mark as unresolved' @@ -121,11 +123,16 @@ describe('ActivityList', () => { return container.querySelector(`.${styles.summary}`); } + // Everything counts as new without a read mark, so the tests about + // the plain wording have to say the thread was read. + const seen = {5: '2026-08-18T12:00:00.000Z'}; + it('names a topic started on the day it is listed under', () => { const {container} = renderActivityList(, { commentThreads: [thread({ comments: [comment({body: 'A topic', createdAt: '2026-08-17T09:00:00.000Z'})] - })] + })], + commentThreadReads: seen }); expect(summaryOf(container)).toHaveTextContent('topic started'); @@ -139,7 +146,8 @@ describe('ActivityList', () => { comment({id: 101, body: 'First', createdAt: '2026-08-17T12:00:00.000Z'}), comment({id: 102, body: 'Second', createdAt: '2026-08-17T13:00:00.000Z'}) ] - })] + })], + commentThreadReads: seen }); expect(summaryOf(container)).toHaveTextContent('2 replies'); @@ -155,13 +163,30 @@ describe('ActivityList', () => { resolvedAt: '2026-08-17T13:00:00.000Z', resolvedById: 44, resolverName: 'Carol' - })] + })], + commentThreadReads: seen }); expect(summaryOf(container)) .toHaveTextContent('topic started, 1 reply and marked as resolved'); }); + it('counts a day\'s replies whether or not they have been read', () => { + const {container} = renderActivityList(, { + commentThreads: [thread({ + permaId: 7, + comments: [ + comment({id: 100, body: 'A topic', createdAt: '2026-08-16T12:00:00.000Z'}), + comment({id: 101, body: 'First', createdAt: '2026-08-17T12:00:00.000Z'}), + comment({id: 102, body: 'Second', createdAt: '2026-08-17T13:00:00.000Z'}) + ] + })], + commentThreadReads: {7: '2026-08-17T12:30:00.000Z'} + }); + + expect(summaryOf(container).textContent).toEqual('2 replies'); + }); + it('leaves out what happened on other days', () => { const {container} = renderActivityList(, { commentThreads: [thread({ @@ -169,7 +194,8 @@ describe('ActivityList', () => { comment({id: 100, body: 'A topic', createdAt: '2026-08-16T12:00:00.000Z'}), comment({id: 101, body: 'A reply', createdAt: '2026-08-17T12:00:00.000Z'}) ] - })] + })], + commentThreadReads: seen }); expect(summaryOf(container).textContent).toEqual('1 reply'); @@ -185,6 +211,10 @@ describe('ActivityList', () => { ] }); + // Unseen comments show whatever day they are from, so the tests about + // folding have to say the thread was read. + const seenAll = {5: '2026-08-18T12:00:00.000Z'}; + const threadAcrossDays = thread({ comments: [ comment({id: 100, body: 'A topic', createdAt: '2026-08-16T12:00:00.000Z'}), @@ -206,7 +236,8 @@ describe('ActivityList', () => { it('folds away replies from earlier days', () => { const {getByText, queryByText} = renderActivityList(, { - commentThreads: [threadAcrossDays] + commentThreads: [threadAcrossDays], + commentThreadReads: seenAll }); expect(getByText('A topic')).toBeInTheDocument(); @@ -235,7 +266,8 @@ describe('ActivityList', () => { const user = userEvent.setup(); const {getByRole, getByText} = renderActivityList(, { - commentThreads: [threadAcrossDays] + commentThreads: [threadAcrossDays], + commentThreadReads: seenAll }); await user.click(getByRole('button', {name: '1 more'})); @@ -248,7 +280,10 @@ describe('ActivityList', () => { const user = userEvent.setup(); const {getByRole, getByPlaceholderText, queryByPlaceholderText} = - renderActivityList(, {commentThreads: [threadAcrossDays]}); + renderActivityList(, { + commentThreads: [threadAcrossDays], + commentThreadReads: seenAll + }); expect(queryByPlaceholderText('Reply...')).toBeNull(); @@ -259,7 +294,8 @@ describe('ActivityList', () => { it('offers no reply count toggle while replies are folded', () => { const {queryByRole} = renderActivityList(, { - commentThreads: [threadAcrossDays] + commentThreads: [threadAcrossDays], + commentThreadReads: seenAll }); expect(queryByRole('button', {name: /2 replies/})).toBeNull(); @@ -269,7 +305,8 @@ describe('ActivityList', () => { const user = userEvent.setup(); const {getByRole, getByText, queryByText} = renderActivityList(, { - commentThreads: [threadAcrossDays] + commentThreads: [threadAcrossDays], + commentThreadReads: seenAll }); await user.click(getByRole('button', {name: '1 more'})); @@ -289,6 +326,59 @@ describe('ActivityList', () => { }); }); + describe('unseen activity', () => { + // Read state is per thread, so a reply left unread days ago sits above + // the day the row is listed under. Folding it away would hide the very + // thing the feed exists to surface. + const threadWithUnseenReplyFromEarlierDay = thread({ + permaId: 7, + comments: [ + comment({id: 100, body: 'A topic', createdAt: '2026-08-15T12:00:00.000Z'}), + comment({id: 101, body: 'Unseen reply', createdAt: '2026-08-16T12:00:00.000Z'}), + comment({id: 102, body: 'Latest reply', createdAt: '2026-08-17T12:00:00.000Z'}) + ] + }); + + it('marks a thread with unseen comments', () => { + const {getByLabelText} = renderActivityList(, { + commentThreads: [threadWithUnseenReplyFromEarlierDay], + commentThreadReads: {} + }); + + expect(getByLabelText('3 unread comments')).toBeInTheDocument(); + }); + + it('does not mark a thread the reviewer has seen', () => { + const {queryByLabelText} = renderActivityList(, { + commentThreads: [threadWithUnseenReplyFromEarlierDay], + commentThreadReads: {7: '2026-08-18T12:00:00.000Z'} + }); + + expect(queryByLabelText('1 unread comment')).toBeNull(); + expect(queryByLabelText('2 unread comments')).toBeNull(); + }); + + it('shows unseen replies from earlier days', () => { + const {getByText} = renderActivityList(, { + commentThreads: [threadWithUnseenReplyFromEarlierDay], + commentThreadReads: {7: '2026-08-15T18:00:00.000Z'} + }); + + expect(getByText('Unseen reply')).toBeInTheDocument(); + expect(getByText('Latest reply')).toBeInTheDocument(); + }); + + it('folds away seen replies from earlier days', () => { + const {getByText, queryByText} = renderActivityList(, { + commentThreads: [threadWithUnseenReplyFromEarlierDay], + commentThreadReads: {7: '2026-08-18T12:00:00.000Z'} + }); + + expect(queryByText('Unseen reply')).toBeNull(); + expect(getByText('1 more')).toBeInTheDocument(); + }); + }); + describe('interaction', () => { function renderTwoEntries(ui) { return renderActivityList(ui, { diff --git a/entry_types/scrolled/package/src/review/ActivityList.js b/entry_types/scrolled/package/src/review/ActivityList.js index a4538fd997..369818d45b 100644 --- a/entry_types/scrolled/package/src/review/ActivityList.js +++ b/entry_types/scrolled/package/src/review/ActivityList.js @@ -72,6 +72,7 @@ function Entry({entry, day, highlighted, onClick}) { onExpandReplies={() => setExpanded(true)} collapsed={collapsed} onToggle={() => setCollapsed(!collapsed)} + showUnreadMarker onClick={onClick} highlighted={highlighted} onResolve={() => postUpdateThreadMessage({ @@ -88,7 +89,8 @@ function summary(t, {thread}, day) { const replies = thread.comments.slice(1); const parts = [ - onDay(thread.comments[0], day) && t('pageflow_scrolled.review.activity.summary.topic'), + onDay(thread.comments[0], day) && + t('pageflow_scrolled.review.activity.summary.topic'), replyCountPart(t, replies.filter(reply => onDay(reply, day)).length), thread.resolvedAt && dayOf(thread.resolvedAt) === day && t('pageflow_scrolled.review.activity.summary.resolution') @@ -115,14 +117,20 @@ function joinParts(t, parts) { .join(t('pageflow_scrolled.review.activity.summary.and')); } -// Everything said on the day the row is listed under, falling back to -// the latest reply so that a row never shows a thread without the comment -// it is listed for. -function visibleReplyCount({thread}, day) { +// Everything said on the day the row is listed under, plus anything +// unseen from before it - folding that away would hide what the feed +// exists to surface. Falls back to the latest reply, so that a row never +// shows a thread without the comment it is listed for. +function visibleReplyCount({thread, unseenCommentIds}, day) { const replies = thread.comments.slice(1); - const firstOfDay = replies.findIndex(reply => dayOf(reply.createdAt) === day); - return firstOfDay < 0 ? Math.min(replies.length, 1) : replies.length - firstOfDay; + const starts = [ + replies.findIndex(reply => unseenCommentIds.includes(reply.id)), + replies.findIndex(reply => dayOf(reply.createdAt) === day) + ].filter(index => index >= 0); + + return starts.length ? replies.length - Math.min(...starts) : + Math.min(replies.length, 1); } function DayHeading({day, at}) { From 0c1d136c12794bc12e1976736d59bcb4fd3ccfbf Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Wed, 19 Aug 2026 16:18:56 +0200 Subject: [PATCH 14/29] Show latest comment activity on its own sidebar route Puts the feed on scrolled/comments/activity with a back link, rather than into a third tab of the comments view: the feed is not a scope of the comments list but a different way of looking at all of them. Clicking a row selects its thread the same way the structural list does, so the preview scrolls to it and opens its popover. Since the preview reports selected section threads without a highlighted id, the view keeps the id of the clicked row to survive the round trip. --- entry_types/scrolled/config/locales/de.yml | 4 + entry_types/scrolled/config/locales/en.yml | 4 + .../controllers/SideBarController-spec.js | 16 ++ .../editor/views/CommentActivityView-spec.js | 199 ++++++++++++++++++ .../editor/controllers/SideBarController.js | 8 + .../src/editor/routers/SideBarRouter.js | 1 + .../src/editor/views/CommentActivityView.js | 87 ++++++++ .../views/CommentActivityView.module.css | 7 + 8 files changed, 326 insertions(+) create mode 100644 entry_types/scrolled/package/spec/editor/views/CommentActivityView-spec.js create mode 100644 entry_types/scrolled/package/src/editor/views/CommentActivityView.js create mode 100644 entry_types/scrolled/package/src/editor/views/CommentActivityView.module.css diff --git a/entry_types/scrolled/config/locales/de.yml b/entry_types/scrolled/config/locales/de.yml index 16ac0a0e35..70491547be 100644 --- a/entry_types/scrolled/config/locales/de.yml +++ b/entry_types/scrolled/config/locales/de.yml @@ -1647,6 +1647,10 @@ de: tabs: comments: Alle Kommentare selection: Für Auswahl + comment_activity_view: + back: Kommentare + tabs: + activity: Letzte Aktivität new_thread_view: back: Kommentare tabs: diff --git a/entry_types/scrolled/config/locales/en.yml b/entry_types/scrolled/config/locales/en.yml index 2bb50d1c1b..fcc88c9b81 100644 --- a/entry_types/scrolled/config/locales/en.yml +++ b/entry_types/scrolled/config/locales/en.yml @@ -1629,6 +1629,10 @@ en: tabs: comments: All comments selection: For selection + comment_activity_view: + back: Comments + tabs: + activity: Latest activity new_thread_view: back: Comments tabs: diff --git a/entry_types/scrolled/package/spec/editor/controllers/SideBarController-spec.js b/entry_types/scrolled/package/spec/editor/controllers/SideBarController-spec.js index b2a437ef85..09cbc2004f 100644 --- a/entry_types/scrolled/package/spec/editor/controllers/SideBarController-spec.js +++ b/entry_types/scrolled/package/spec/editor/controllers/SideBarController-spec.js @@ -2,6 +2,7 @@ import 'editor/config'; import {SideBarController} from 'editor/controllers/SideBarController'; import {CommentsView} from 'editor/views/CommentsView'; +import {CommentActivityView} from 'editor/views/CommentActivityView'; import {factories} from 'pageflow/testHelpers'; import {useEditorGlobals} from 'support'; @@ -36,4 +37,19 @@ describe('SideBarController', () => { expect(shown.options.defaultTab).toBe('selection'); }); }); + + describe('#commentActivity', () => { + it('shows a CommentActivityView in the region', () => { + const entry = createEntry({}); + entry.reviewSession = factories.reviewSession(); + + const region = {show: jest.fn()}; + const controller = new SideBarController({region, entry}); + + controller.commentActivity(); + + const shown = region.show.mock.calls[0][0]; + expect(shown).toBeInstanceOf(CommentActivityView); + }); + }); }); diff --git a/entry_types/scrolled/package/spec/editor/views/CommentActivityView-spec.js b/entry_types/scrolled/package/spec/editor/views/CommentActivityView-spec.js new file mode 100644 index 0000000000..08c937edbe --- /dev/null +++ b/entry_types/scrolled/package/spec/editor/views/CommentActivityView-spec.js @@ -0,0 +1,199 @@ +import '@testing-library/jest-dom/extend-expect'; +import {act} from '@testing-library/react'; +import {fireEvent} from '@testing-library/dom'; + +import {editor} from 'pageflow-scrolled/editor'; + +import {CommentActivityView} from 'editor/views/CommentActivityView'; + +import {factories, useFakeTranslations, renderBackboneView} from 'pageflow/testHelpers'; +import {useEditorGlobals} from 'support'; + +describe('CommentActivityView', () => { + const {createEntry} = useEditorGlobals(); + + useFakeTranslations({ + 'pageflow_scrolled.editor.comment_activity_view.back': 'Comments', + 'pageflow_scrolled.editor.comment_activity_view.tabs.activity': 'Latest activity', + 'pageflow_scrolled.review.activity.no_activity_yet': 'No activity yet', + 'pageflow_scrolled.review.activity.today': 'Today', + 'pageflow_scrolled.review.activity.yesterday': 'Yesterday', + 'pageflow_scrolled.review.earlier_reply_count.one': '1 more', + 'pageflow_scrolled.review.earlier_reply_count.other': '%{count} more', + 'pageflow_scrolled.review.reply_count.one': '1 reply', + 'pageflow_scrolled.review.reply_count.other': '%{count} replies', + 'pageflow_scrolled.review.reply_placeholder': 'Reply...', + 'pageflow_scrolled.review.send': 'Send', + 'pageflow_scrolled.review.toggle_replies': 'Toggle replies', + 'pageflow_scrolled.review.resolve': 'Mark as resolved', + 'pageflow_scrolled.review.unresolve': 'Mark as unresolved' + }); + + function comment({id, body, createdAt, creatorName = 'Bob'}) { + return {id, creatorId: 43, creatorName, body, createdAt}; + } + + function rowOf(getByText, body) { + return getByText(body).closest('[aria-current]'); + } + + function isFollowedBy(node, other) { + return !!(node.compareDocumentPosition(other) & Node.DOCUMENT_POSITION_FOLLOWING); + } + + function entryWithThreads(commentThreads) { + const entry = createEntry({ + chapters: [ + {id: 1, permaId: 10, storylineId: 1000, position: 0, configuration: {title: 'Intro'}}, + {id: 2, permaId: 20, storylineId: 1000, position: 1, configuration: {title: 'Middle'}} + ], + 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: 'textBlock'}, + {id: 2, permaId: 2000, sectionId: 2, typeName: 'textBlock'} + ] + }); + + entry.reviewSession = factories.reviewSession({commentThreads}); + return entry; + } + + it('lists activity of all chapters newest first', () => { + const entry = entryWithThreads([ + { + id: 1, permaId: 5, subjectType: 'ContentElement', subjectId: 1000, + comments: [comment({ + id: 100, body: 'Older topic', createdAt: '2026-08-17T09:00:00.000Z' + })] + }, + { + id: 2, permaId: 6, subjectType: 'ContentElement', subjectId: 2000, + comments: [comment({ + id: 200, body: 'Newer topic', createdAt: '2026-08-17T11:00:00.000Z' + })] + } + ]); + + const view = new CommentActivityView({entry, editor}); + const {getByText} = renderBackboneView(view); + + expect(isFollowedBy(getByText('Newer topic'), getByText('Older topic'))).toBe(true); + }); + + it('renders a tab label above the list', () => { + const entry = entryWithThreads([]); + + const view = new CommentActivityView({entry, editor}); + const {getByRole} = renderBackboneView(view); + + expect(getByRole('tab', {name: 'Latest activity'})).toBeInTheDocument(); + }); + + it('triggers selectCommentThread on entry when an entry is clicked', () => { + const entry = entryWithThreads([{ + id: 7, permaId: 5, subjectType: 'ContentElement', subjectId: 1000, + comments: [comment({id: 100, body: 'A topic', createdAt: '2026-08-17T09:00:00.000Z'})] + }]); + + const view = new CommentActivityView({entry, editor}); + const {getByText} = renderBackboneView(view); + + const listener = jest.fn(); + entry.on('selectCommentThread', listener); + + fireEvent.click(getByText('A topic')); + + expect(listener).toHaveBeenCalledWith(7); + }); + + it('marks the entry of the highlighted thread as current', () => { + const entry = entryWithThreads([{ + id: 7, permaId: 5, subjectType: 'ContentElement', subjectId: 1000, + comments: [comment({id: 100, body: 'A topic', createdAt: '2026-08-17T09:00:00.000Z'})] + }]); + + const view = new CommentActivityView({entry, editor}); + const {getByText} = renderBackboneView(view); + + act(() => { entry.set('highlightedThreadId', 7); }); + + expect(rowOf(getByText, 'A topic')).not.toBeNull(); + }); + + it('marks the entry of a thread already highlighted when it opens', () => { + const entry = entryWithThreads([{ + id: 7, permaId: 5, subjectType: 'ContentElement', subjectId: 1000, + comments: [comment({id: 100, body: 'A topic', createdAt: '2026-08-17T09:00:00.000Z'})] + }]); + + entry.set('highlightedThreadId', 7); + + const view = new CommentActivityView({entry, editor}); + const {getByText} = renderBackboneView(view); + + expect(rowOf(getByText, 'A topic')).not.toBeNull(); + }); + + it('keeps the clicked entry marked when the preview reports no highlight', () => { + const entry = entryWithThreads([ + { + id: 6, permaId: 5, subjectType: 'ContentElement', subjectId: 1000, + comments: [comment({ + id: 100, body: 'An element topic', creatorName: 'Bob', + createdAt: '2026-08-17T09:00:00.000Z' + })] + }, + { + id: 7, permaId: 6, subjectType: 'Section', subjectId: 200, + comments: [comment({ + id: 200, body: 'A section topic', creatorName: 'Carol', + createdAt: '2026-08-17T11:00:00.000Z' + })] + } + ]); + + const view = new CommentActivityView({entry, editor}); + const {getByText} = renderBackboneView(view); + + act(() => { entry.set('highlightedThreadId', 6); }); + + fireEvent.click(getByText('A section topic')); + + act(() => { + entry.set({ + highlightedThreadId: undefined, + selectedCommentsSubject: {subjectType: 'Section', id: 2} + }); + }); + + expect(rowOf(getByText, 'A section topic')).not.toBeNull(); + expect(rowOf(getByText, 'An element topic')).toBeNull(); + }); + + it('renders a back link', () => { + const entry = entryWithThreads([]); + + const view = new CommentActivityView({entry, editor}); + const {getByText} = renderBackboneView(view); + + expect(getByText('Comments')).toBeInTheDocument(); + }); + + it('navigates to the comments view when the back link is clicked', () => { + const entry = entryWithThreads([]); + + const view = new CommentActivityView({entry, editor}); + const {getByText} = renderBackboneView(view); + + const navigate = jest.spyOn(editor, 'navigate').mockImplementation(() => {}); + + fireEvent.click(getByText('Comments')); + + expect(navigate).toHaveBeenCalledWith('/scrolled/comments', {trigger: true}); + + navigate.mockRestore(); + }); +}); diff --git a/entry_types/scrolled/package/src/editor/controllers/SideBarController.js b/entry_types/scrolled/package/src/editor/controllers/SideBarController.js index 66fedbed1f..de911442ce 100644 --- a/entry_types/scrolled/package/src/editor/controllers/SideBarController.js +++ b/entry_types/scrolled/package/src/editor/controllers/SideBarController.js @@ -8,6 +8,7 @@ import {EditSectionTransitionView} from '../views/EditSectionTransitionView'; import {EditSectionPaddingsView} from '../views/EditSectionPaddingsView'; import {EditContentElementView} from '../views/EditContentElementView'; import {CommentsView} from '../views/CommentsView'; +import {CommentActivityView} from '../views/CommentActivityView'; import {NewThreadView} from '../views/NewThreadView'; export const SideBarController = Marionette.Controller.extend({ @@ -57,6 +58,13 @@ export const SideBarController = Marionette.Controller.extend({ })); }, + commentActivity: function() { + this.region.show(new CommentActivityView({ + entry: this.entry, + editor + })); + }, + contentElement: function(id, tab) { this.region.show(new EditContentElementView({ entry: this.entry, diff --git a/entry_types/scrolled/package/src/editor/routers/SideBarRouter.js b/entry_types/scrolled/package/src/editor/routers/SideBarRouter.js index 375e8db858..d71d779e6d 100644 --- a/entry_types/scrolled/package/src/editor/routers/SideBarRouter.js +++ b/entry_types/scrolled/package/src/editor/routers/SideBarRouter.js @@ -4,6 +4,7 @@ export const SideBarRouter = Marionette.AppRouter.extend({ appRoutes: { 'scrolled/comments?tab=:tab': 'comments', 'scrolled/comments': 'comments', + 'scrolled/comments/activity': 'commentActivity', 'scrolled/chapters/:id': 'chapter', 'scrolled/sections/:id/transition': 'sectionTransition', 'scrolled/sections/:id/paddings?position=:position': 'sectionPaddings', diff --git a/entry_types/scrolled/package/src/editor/views/CommentActivityView.js b/entry_types/scrolled/package/src/editor/views/CommentActivityView.js new file mode 100644 index 0000000000..d7e26078ef --- /dev/null +++ b/entry_types/scrolled/package/src/editor/views/CommentActivityView.js @@ -0,0 +1,87 @@ +import React from 'react'; +import I18n from 'i18n-js'; +import Marionette from 'backbone.marionette'; + +import {editor} from 'pageflow/editor'; +import {TabsView} from 'pageflow/ui'; +import {ActivityList} from 'pageflow-scrolled/review'; + +import {ReviewView} from './ReviewView'; + +import styles from './CommentActivityView.module.css'; + +export const CommentActivityView = Marionette.ItemView.extend({ + className: `comment_activity_view ${styles.root}`, + + template: () => ` + ${I18n.t('pageflow_scrolled.editor.comment_activity_view.back')} +
+ `, + + ui: { + tabs: '.tabs' + }, + + events: { + 'click a.back': 'goBack' + }, + + onRender: function() { + const {entry} = this.options; + + const tabsView = new TabsView({ + i18n: 'pageflow_scrolled.editor.comment_activity_view.tabs' + }); + + tabsView.tab('activity', () => new ActivityListView({entry})); + + this.appendSubview(tabsView, {to: this.ui.tabs}); + }, + + goBack: function() { + editor.navigate('/scrolled/comments', {trigger: true}); + } +}); + +const ActivityListView = ReviewView.extend({ + className: styles.list, + + initialize() { + this._trackHighlight(); + + // Structure changes reach React through WatchEntryCollections and + // review state through the ReviewMessageHandler, so only the + // highlight needs a rerender here. + this.listenTo(this.options.entry, 'change:highlightedThreadId', () => { + this._trackHighlight(); + this.rerender(); + }); + }, + + props() { + return { + highlightedThreadId: this.selectedThreadId, + onEntryClick: entry => this._selectThread(entry.threadId) + }; + }, + + renderContent(props) { + return ; + }, + + _selectThread(threadId) { + this.selectedThreadId = threadId; + this.options.entry.trigger('selectCommentThread', threadId); + this.rerender(); + }, + + // The preview reports selected section threads without a highlighted id, + // so the id of the clicked row is kept here to survive the round trip. + _trackHighlight() { + const highlightedThreadId = this.options.entry.get('highlightedThreadId'); + + if (highlightedThreadId) { + this.selectedThreadId = highlightedThreadId; + } + } +}); diff --git a/entry_types/scrolled/package/src/editor/views/CommentActivityView.module.css b/entry_types/scrolled/package/src/editor/views/CommentActivityView.module.css new file mode 100644 index 0000000000..d3e9437c26 --- /dev/null +++ b/entry_types/scrolled/package/src/editor/views/CommentActivityView.module.css @@ -0,0 +1,7 @@ +.root > div { + margin: space(2.5) 0; +} + +.list { + padding-top: space(2); +} From 1dcfc0da8e6b1ddcbcc015d8f2cd092aab3668a5 Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Wed, 19 Aug 2026 16:22:21 +0200 Subject: [PATCH 15/29] Link to latest activity from the comments view Adds a control strip above the tabs holding a link to the activity route, carrying the dot the comments main menu item already uses so that unseen comments are visible from within the comments view too. The strip is laid out with room on the left: the resolution filter the preview's floating toolbar offers is the intended next occupant. It stays outside the sticky tab bar until it actually holds filters. --- entry_types/scrolled/config/locales/de.yml | 3 +- entry_types/scrolled/config/locales/en.yml | 3 +- .../spec/editor/views/CommentsView-spec.js | 63 ++++++++++++++++++- .../package/src/editor/views/CommentsView.js | 39 +++++++++++- .../src/editor/views/CommentsView.module.css | 43 +++++++++++++ .../src/editor/views/images/activity.svg | 1 + 6 files changed, 147 insertions(+), 5 deletions(-) create mode 100644 entry_types/scrolled/package/src/editor/views/images/activity.svg diff --git a/entry_types/scrolled/config/locales/de.yml b/entry_types/scrolled/config/locales/de.yml index 70491547be..58d5513c1f 100644 --- a/entry_types/scrolled/config/locales/de.yml +++ b/entry_types/scrolled/config/locales/de.yml @@ -1642,10 +1642,11 @@ de: main_menu: comments: Kommentare comments_view: + activity: Letzte Aktivität new_thread: Neues Thema section: Abschnitt tabs: - comments: Alle Kommentare + comments: Alle selection: Für Auswahl comment_activity_view: back: Kommentare diff --git a/entry_types/scrolled/config/locales/en.yml b/entry_types/scrolled/config/locales/en.yml index fcc88c9b81..5b21f70e61 100644 --- a/entry_types/scrolled/config/locales/en.yml +++ b/entry_types/scrolled/config/locales/en.yml @@ -1624,10 +1624,11 @@ en: main_menu: comments: Comments comments_view: + activity: Latest activity new_thread: New topic section: Section tabs: - comments: All comments + comments: All selection: For selection comment_activity_view: back: Comments 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 f61bd3e9f0..f4b89e97c7 100644 --- a/entry_types/scrolled/package/spec/editor/views/CommentsView-spec.js +++ b/entry_types/scrolled/package/spec/editor/views/CommentsView-spec.js @@ -3,6 +3,7 @@ import '@testing-library/jest-dom/extend-expect'; import {editor} from 'pageflow-scrolled/editor'; import {CommentsView} from 'editor/views/CommentsView'; +import styles from 'editor/views/CommentsView.module.css'; import {factories, useFakeTranslations, renderBackboneView} from 'pageflow/testHelpers'; import {useEditorGlobals} from 'support'; @@ -25,9 +26,10 @@ describe('CommentsView', () => { 'pageflow_scrolled.review.add_comment_placeholder': 'Add a comment...', 'pageflow_scrolled.review.send': 'Send', 'pageflow_scrolled.editor.content_elements.textBlock.name': 'Text', - 'pageflow_scrolled.editor.comments_view.tabs.comments': 'All comments', + 'pageflow_scrolled.editor.comments_view.tabs.comments': 'All', 'pageflow_scrolled.editor.comments_view.tabs.selection': 'For selection', 'pageflow_scrolled.editor.comments_view.new_thread': 'New topic', + 'pageflow_scrolled.editor.comments_view.activity': 'Latest activity', 'pageflow.editor.templates.back_button_decorator.outline': 'Outline' }); @@ -54,7 +56,7 @@ describe('CommentsView', () => { const {getAllByRole} = renderBackboneView(view); const labels = getAllByRole('tab').map(t => t.textContent); - expect(labels).toEqual(['All comments', 'For selection']); + expect(labels).toEqual(['All', 'For selection']); }); it('shows the all-comments tab by default', () => { @@ -193,6 +195,63 @@ describe('CommentsView', () => { }); }); + describe('activity link', () => { + it('navigates to the activity route when clicked', () => { + const entry = setupEntry(); + + const view = new CommentsView({entry, editor}); + const {getByRole} = renderBackboneView(view); + + const navigate = jest.spyOn(editor, 'navigate').mockImplementation(() => {}); + + fireEvent.click(getByRole('button', {name: 'Latest activity'})); + + expect(navigate).toHaveBeenCalledWith( + '/scrolled/comments/activity', {trigger: true} + ); + + navigate.mockRestore(); + }); + + it('points at unseen comments while the entry holds some', () => { + const entry = setupEntry(); + entry.set('hasUnreadComments', true); + + const view = new CommentsView({entry, editor}); + const {getByRole} = renderBackboneView(view); + + expect(getByRole('button', {name: 'Latest activity'})) + .toHaveClass(styles.indicator); + }); + + it('does not point at unseen comments without any', () => { + const entry = setupEntry(); + + const view = new CommentsView({entry, editor}); + const {getByRole} = renderBackboneView(view); + + expect(getByRole('button', {name: 'Latest activity'})) + .not.toHaveClass(styles.indicator); + }); + + it('follows entry.hasUnreadComments while it changes', () => { + const entry = setupEntry(); + + const view = new CommentsView({entry, editor}); + const {getByRole} = renderBackboneView(view); + + act(() => { entry.set('hasUnreadComments', true); }); + + expect(getByRole('button', {name: 'Latest activity'})) + .toHaveClass(styles.indicator); + + act(() => { entry.set('hasUnreadComments', false); }); + + expect(getByRole('button', {name: 'Latest activity'})) + .not.toHaveClass(styles.indicator); + }); + }); + it('does not trigger selectNewThread when clicking the button while disabled', () => { const entry = unselectedEntry(createEntry); diff --git a/entry_types/scrolled/package/src/editor/views/CommentsView.js b/entry_types/scrolled/package/src/editor/views/CommentsView.js index 75ce799b48..9d175bd9ab 100644 --- a/entry_types/scrolled/package/src/editor/views/CommentsView.js +++ b/entry_types/scrolled/package/src/editor/views/CommentsView.js @@ -7,6 +7,8 @@ import {cssModulesUtils, TabsView} from 'pageflow/ui'; import {EntryCommentsView} from './EntryCommentsView'; import {SelectionCommentsView} from './SelectionCommentsView'; +import activityIcon from './images/activity.svg'; + import styles from './CommentsView.module.css'; export const CommentsView = Marionette.ItemView.extend({ @@ -25,7 +27,8 @@ export const CommentsView = Marionette.ItemView.extend({ events: { 'click a.back': 'goBack', ...cssModulesUtils.events(styles, { - 'click newThreadButton': 'startNewThread' + 'click newThreadButton': 'startNewThread', + 'click activityButton': 'showActivity' }) }, @@ -33,6 +36,9 @@ export const CommentsView = Marionette.ItemView.extend({ this.listenTo(this.options.entry, 'change:selectedCommentsSubject', this._updateNewThreadButton); + this.listenTo(this.options.entry, + 'change:hasUnreadComments', + this._updateActivityButton); }, onRender: function() { @@ -49,7 +55,13 @@ export const CommentsView = Marionette.ItemView.extend({ new SelectionCommentsView({entry, editor: editorApi})); this.appendSubview(tabsView, {to: this.ui.tabs}); + + // Beside the tab list rather than inside it, which is a tablist the + // link is not part of. + this.$('.tabs_view-scroller').append(activityButton()); + this._updateNewThreadButton(); + this._updateActivityButton(); }, startNewThread: function() { @@ -74,6 +86,10 @@ export const CommentsView = Marionette.ItemView.extend({ } }, + showActivity: function() { + editor.navigate('/scrolled/comments/activity', {trigger: true}); + }, + goBack: function() { editor.navigate('/', {trigger: true}); }, @@ -82,5 +98,26 @@ export const CommentsView = Marionette.ItemView.extend({ const enabled = !!this.options.entry.get('selectedCommentsSubject'); this.$(cssModulesUtils.selector(styles, 'newThreadButton')) .prop('disabled', !enabled); + }, + + _updateActivityButton: function() { + this.$(cssModulesUtils.selector(styles, 'activityButton')) + .toggleClass(styles.indicator, + !!this.options.entry.get('hasUnreadComments')); } }); + +function activityButton() { + const label = I18n.t('pageflow_scrolled.editor.comments_view.activity'); + + return ` + + `; +} + +function escapeCssUrl(url) { + return url.replace(/'/g, "\\'").replace(/\n/g, ''); +} diff --git a/entry_types/scrolled/package/src/editor/views/CommentsView.module.css b/entry_types/scrolled/package/src/editor/views/CommentsView.module.css index 68f370eb21..45a77f4563 100644 --- a/entry_types/scrolled/package/src/editor/views/CommentsView.module.css +++ b/entry_types/scrolled/package/src/editor/views/CommentsView.module.css @@ -23,3 +23,46 @@ composes: secondaryAddButton from './buttons.module.css'; float: right; } + +/* Rides the tab bar at its far end. The bar is the scroller's containing + block, and its tab list is absolutely positioned for the same reason. */ +.activityButton { + position: absolute; + top: 0; + right: 0; + display: flex; + align-items: center; + justify-content: center; + height: 100%; + background: none; + border: 0; + padding: 0 space(2); + cursor: pointer; +} + +.activityIcon { + width: 16px; + height: 16px; + background: var(--ui-on-surface-color-light); + mask-size: contain; + mask-repeat: no-repeat; + mask-position: center; +} + +.activityButton:hover .activityIcon { + background: var(--ui-primary-color); +} + +/* Matches a.indicator::after in app/assets/stylesheets/pageflow/editor/menu.scss, + which points at the same unseen comments from the sidebar root. */ +.indicator::after { + content: ""; + position: absolute; + top: space(1); + right: space(1); + width: space(1.5); + height: space(1.5); + border-radius: 50%; + background: var(--ui-warning-color); + box-shadow: 0 0 0 1px var(--ui-on-surface-color-lighter); +} diff --git a/entry_types/scrolled/package/src/editor/views/images/activity.svg b/entry_types/scrolled/package/src/editor/views/images/activity.svg new file mode 100644 index 0000000000..2e2bdd30dc --- /dev/null +++ b/entry_types/scrolled/package/src/editor/views/images/activity.svg @@ -0,0 +1 @@ + From eb4967724f62522818561f3cb9215056a7a16904 Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Wed, 26 Aug 2026 08:58:09 +0200 Subject: [PATCH 16/29] Leave the activity feed when a comment is picked in the preview Selecting a subject in the preview keeps the sidebar where it is once it is already showing comments, so that stepping through them does not tear the reviewer off the tab they chose. The activity feed sits under the same route but lists every subject at once, so a subject picked there had nowhere to appear, and the selection went unanswered. Threads clicked in the sidebar are the exception: they travel to the preview as a message and come back as a selection, which now says so, so that the view that asked is left alone. --- .../PreviewMessageController-spec.js | 52 +++++++++++++++++++ .../contentElementCommentBadges-spec.js | 4 +- .../features/sectionCommentBadges-spec.js | 4 +- .../controllers/PreviewMessageController.js | 18 +++++-- .../inlineEditing/ContentElementDecorator.js | 5 +- .../inlineEditing/useCommentSelection.js | 3 +- .../useSelectCommentThreadHandler.js | 7 ++- 7 files changed, 80 insertions(+), 13 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 1e5300948f..60477dc803 100644 --- a/entry_types/scrolled/package/spec/editor/controllers/PreviewMessageController-spec.js +++ b/entry_types/scrolled/package/spec/editor/controllers/PreviewMessageController-spec.js @@ -663,6 +663,58 @@ describe('PreviewMessageController', () => { })).resolves.toBe('/scrolled/comments?tab=selection'); }); + it('navigates to comments route with tab=selection on SELECTED while on the activity feed', async () => { + Backbone.history.fragment = 'scrolled/comments/activity'; + + const editor = factories.editorApi(); + const entry = factories.entry(ScrolledEntry, {}, { + entryTypeSeed: normalizeSeed({contentElements: [{id: 1}]}) + }); + const iframeWindow = createIframeWindow(); + controller = new PreviewMessageController({entry, iframeWindow, editor}); + + const path = await new Promise(resolve => { + editor.on('navigate', resolve); + window.postMessage({ + type: 'SELECTED', + payload: {id: 1, type: 'contentElementComments'} + }, '*'); + }); + + expect(path).toBe('/scrolled/comments?tab=selection'); + + Backbone.history.fragment = undefined; + }); + + it('does not navigate on SELECTED for a selection made from a message', async () => { + Backbone.history.fragment = 'scrolled/comments/activity'; + + const editor = factories.editorApi(); + const entry = factories.entry(ScrolledEntry, {}, { + entryTypeSeed: normalizeSeed({contentElements: [{id: 1}]}) + }); + const iframeWindow = createIframeWindow(); + controller = new PreviewMessageController({entry, iframeWindow, editor}); + + const navigate = jest.fn(); + editor.on('navigate', navigate); + + await new Promise(resolve => { + entry.once('change:highlightedThreadId', resolve); + window.postMessage({ + type: 'SELECTED', + payload: { + id: 1, type: 'contentElementComments', highlightedThreadId: 7, + source: 'editor' + } + }, '*'); + }); + + expect(navigate).not.toHaveBeenCalled(); + + Backbone.history.fragment = undefined; + }); + it('does not navigate on SELECTED contentElementComments while on the comments route', async () => { Backbone.history.fragment = 'scrolled/comments'; 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 6f38ba2493..0fd3284e17 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 @@ -129,7 +129,7 @@ describe('inline editing content element comment badges', () => { expect(window.parent.postMessage).toHaveBeenCalledWith({ type: 'SELECTED', - payload: {type: 'contentElementComments', id: 1, highlightedThreadId: 7} + payload: {type: 'contentElementComments', id: 1, highlightedThreadId: 7, source: 'editor'} }, expect.anything()); expect(scrollIntoView).toHaveBeenCalled(); @@ -182,7 +182,7 @@ describe('inline editing content element comment badges', () => { expect(window.parent.postMessage).toHaveBeenCalledWith({ type: 'SELECTED', - payload: {type: 'contentElementComments', id: 1, highlightedThreadId: 7} + payload: {type: 'contentElementComments', id: 1, highlightedThreadId: 7, source: 'editor'} }, expect.anything()); expect(scrollIntoView).toHaveBeenCalled(); 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 9efdfb7a02..4025a6d2d9 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 @@ -244,7 +244,7 @@ describe('inline editing section comment badges', () => { expect(section.scrollIntoView).toHaveBeenCalled(); expect(window.parent.postMessage).toHaveBeenCalledWith({ type: 'SELECTED', - payload: {type: 'sectionComments', id: 1, highlightedThreadId: 1} + payload: {type: 'sectionComments', id: 1, highlightedThreadId: 1, source: 'editor'} }, expect.anything()); }); @@ -289,7 +289,7 @@ describe('inline editing section comment badges', () => { expect(section.scrollIntoView).toHaveBeenCalled(); expect(window.parent.postMessage).toHaveBeenCalledWith({ type: 'SELECTED', - payload: {type: 'sectionComments', id: 1, highlightedThreadId: 3} + payload: {type: 'sectionComments', id: 1, highlightedThreadId: 3, source: 'editor'} }, expect.anything()); }); }); diff --git a/entry_types/scrolled/package/src/editor/controllers/PreviewMessageController.js b/entry_types/scrolled/package/src/editor/controllers/PreviewMessageController.js index 85b1fb990e..766b365897 100644 --- a/entry_types/scrolled/package/src/editor/controllers/PreviewMessageController.js +++ b/entry_types/scrolled/package/src/editor/controllers/PreviewMessageController.js @@ -200,10 +200,10 @@ export const PreviewMessageController = Object.extend({ }); 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. - if (!Backbone.history.fragment?.startsWith('scrolled/comments')) { + // Stay on the current tab when the user is already looking at + // the comments view — only force the selection tab when + // arriving there from elsewhere. + if (!onCommentsView() && message.data.payload.source !== 'editor') { this.editor.navigate('/scrolled/comments?tab=selection', {trigger: true}) } } @@ -344,3 +344,13 @@ function modelForSubject(entry, {subjectType, subjectId}) { const collection = subjectType === 'Section' ? entry.sections : entry.contentElements; return collection.findWhere({permaId: subjectId}); } + +// The comments view itself, but none of the routes below it: the +// activity feed lists every subject at once, so a subject picked in the +// preview has nowhere to appear there. +function onCommentsView() { + const fragment = Backbone.history.fragment || ''; + + return fragment === 'scrolled/comments' || + fragment.startsWith('scrolled/comments?'); +} diff --git a/entry_types/scrolled/package/src/frontend/inlineEditing/ContentElementDecorator.js b/entry_types/scrolled/package/src/frontend/inlineEditing/ContentElementDecorator.js index d1fcff2d53..4123facb12 100644 --- a/entry_types/scrolled/package/src/frontend/inlineEditing/ContentElementDecorator.js +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/ContentElementDecorator.js @@ -62,10 +62,11 @@ function DefaultSelectionRect(props) { subjectType: 'ContentElement', subjectId: props.permaId, getScrollTarget: useCallback(() => selectionRectRef.current, []), - selectThread: useCallback(threadId => selectComments({ + selectThread: useCallback((threadId, options) => selectComments({ type: 'contentElementComments', id: props.id, - highlightedThreadId: threadId + highlightedThreadId: threadId, + ...options }), [selectComments, props.id]) }); diff --git a/entry_types/scrolled/package/src/frontend/inlineEditing/useCommentSelection.js b/entry_types/scrolled/package/src/frontend/inlineEditing/useCommentSelection.js index b85385a72a..bbda782350 100644 --- a/entry_types/scrolled/package/src/frontend/inlineEditing/useCommentSelection.js +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/useCommentSelection.js @@ -27,7 +27,8 @@ export function useCommentSelection({type, id, subjectType, subjectId}) { )); const selectThread = useCallback( - threadId => selectComments({type, id, highlightedThreadId: threadId}), + (threadId, options) => + selectComments({type, id, highlightedThreadId: threadId, ...options}), [selectComments, type, id] ); diff --git a/entry_types/scrolled/package/src/frontend/inlineEditing/useSelectCommentThreadHandler.js b/entry_types/scrolled/package/src/frontend/inlineEditing/useSelectCommentThreadHandler.js index 78758254c9..a54eedfd9d 100644 --- a/entry_types/scrolled/package/src/frontend/inlineEditing/useSelectCommentThreadHandler.js +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/useSelectCommentThreadHandler.js @@ -12,7 +12,10 @@ import {usePostMessageListener} from '../../shared/usePostMessageListener'; // 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. +// place instead of inside each badge. The selection names the editor as +// its source, which tells the editor it is looking at its own request +// coming back rather than at the reviewer picking a subject in the +// preview. export function useSelectCommentThreadHandler({ subjectType, subjectId, getScrollTarget, beforeSelect, selectThread }) { @@ -33,6 +36,6 @@ export function useSelectCommentThreadHandler({ if (beforeSelect) beforeSelect(threadId); - selectThread(threadId); + selectThread(threadId, {source: 'editor'}); }, [threads, getScrollTarget, beforeSelect, selectThread])); } From 11793527a790555cd8170527569e0fcde8dc40d7 Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Wed, 26 Aug 2026 11:18:20 +0200 Subject: [PATCH 17/29] Say a thread is resolved even while it is collapsed A collapsed thread showed nothing of its resolution: the row carrying it was hidden along with the replies, so a list of resolved threads read as a list of ordinary ones. The state now stays whatever the thread shows of itself, undoing it included. Only the offer to resolve keeps waiting for the thread to be expanded. --- .../review/Thread/features/resolution-spec.js | 38 +++++++++++++++++++ .../scrolled/package/src/review/Thread.js | 2 +- 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/entry_types/scrolled/package/spec/review/Thread/features/resolution-spec.js b/entry_types/scrolled/package/spec/review/Thread/features/resolution-spec.js index f05c52b809..6cac7f81f7 100644 --- a/entry_types/scrolled/package/spec/review/Thread/features/resolution-spec.js +++ b/entry_types/scrolled/package/spec/review/Thread/features/resolution-spec.js @@ -23,6 +23,19 @@ describe('Thread resolution', () => { comments: [{id: 10, body: 'On the pull quote', creatorName: 'Bob', creatorId: 2}] }; + const withReply = { + ...thread, + comments: [...thread.comments, + {id: 11, body: 'A reply', creatorName: 'Carol', creatorId: 3}] + }; + + const resolvedWithReply = { + ...withReply, + resolvedAt: '2026-08-19T10:00:00.000Z', + resolvedById: 3, + resolverName: 'Ada' + }; + const resolved = { ...thread, resolvedAt: '2026-08-19T10:00:00.000Z', @@ -83,6 +96,31 @@ describe('Thread resolution', () => { expect(getByRole('button', {name: 'Mark as resolved'})).toBeInTheDocument(); }); + it('says a collapsed thread is resolved', () => { + const {getByText} = renderWithReviewState( + + ); + + expect(getByText('Marked as resolved by')).toBeInTheDocument(); + expect(getByText('Ada')).toBeInTheDocument(); + }); + + it('undoes the resolution of a collapsed thread as well', () => { + const {getByRole} = renderWithReviewState( + {}} /> + ); + + expect(getByRole('button', {name: 'Thread actions'})).toBeInTheDocument(); + }); + + it('offers no resolve button while collapsed', () => { + const {queryByRole} = renderWithReviewState( + {}} /> + ); + + expect(queryByRole('button', {name: 'Mark as resolved'})).toBeNull(); + }); + it('undoes the resolution through a menu', async () => { const user = userEvent.setup(); const onResolve = jest.fn(); diff --git a/entry_types/scrolled/package/src/review/Thread.js b/entry_types/scrolled/package/src/review/Thread.js index 51f353139c..8f4756fa26 100644 --- a/entry_types/scrolled/package/src/review/Thread.js +++ b/entry_types/scrolled/package/src/review/Thread.js @@ -161,7 +161,7 @@ export function Thread({thread, collapsed: collapsedProp, visibleReplyCount, onE subjectId={thread.subjectId} subjectRange={thread.subjectRange} />} - {(thread.resolvedAt || (interactive && onResolve)) && !repliesCollapsed && + {(thread.resolvedAt || (interactive && onResolve && !repliesCollapsed)) &&
{thread.resolvedAt ? Date: Wed, 26 Aug 2026 11:55:45 +0200 Subject: [PATCH 18/29] Extract selecting a comment navigation target Stepping to the next comment and jumping to a particular one differ only in how they pick the target, not in what selecting it involves. --- .../commenting/SelectedSubjectProvider.js | 28 ++++++++++--------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/entry_types/scrolled/package/src/frontend/commenting/SelectedSubjectProvider.js b/entry_types/scrolled/package/src/frontend/commenting/SelectedSubjectProvider.js index 5c6f340ab6..f587a5635b 100644 --- a/entry_types/scrolled/package/src/frontend/commenting/SelectedSubjectProvider.js +++ b/entry_types/scrolled/package/src/frontend/commenting/SelectedSubjectProvider.js @@ -33,18 +33,7 @@ export function SelectedSubjectProvider({children}) { setSelectedSubject(null); }, []); - const goTo = useCallback(step => { - if (targets.length === 0) { - return; - } - - const current = currentTargetIndex(targets, selectedSubject); - const next = current < 0 - ? (step > 0 ? 0 : targets.length - 1) - : (current + step + targets.length) % targets.length; - - const target = targets[next]; - + const selectTarget = useCallback(target => { // Activate the excursion the target lives in (or leave the current // one) before selecting it, so its popover can mount and open. Only // needed when moving to a different subject. @@ -63,7 +52,20 @@ export function SelectedSubjectProvider({children}) { subjectRange: target.subjectRange, highlightedThreadId: target.threadId }); - }, [targets, selectedSubject, activateExcursionOfSection, returnFromExcursion]); + }, [selectedSubject, activateExcursionOfSection, returnFromExcursion]); + + const goTo = useCallback(step => { + if (targets.length === 0) { + return; + } + + const current = currentTargetIndex(targets, selectedSubject); + const next = current < 0 + ? (step > 0 ? 0 : targets.length - 1) + : (current + step + targets.length) % targets.length; + + selectTarget(targets[next]); + }, [targets, selectedSubject, selectTarget]); const position = useMemo( () => currentTargetIndex(targets, selectedSubject) + 1, From 3f3e93cbc8bf84ada07bccc4c749108a8d2b335f Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Wed, 26 Aug 2026 12:00:00 +0200 Subject: [PATCH 19/29] Open the latest activity from the comment toolbar The feed the editor sidebar shows on its own route was out of reach while reviewing in the preview. A toolbar button now opens it as a panel above the toolbar, and clicking an entry reveals the thread in place - jumping to it the way the arrows do, excursions and all, and switching the filter where a resolved thread would otherwise stay behind the toolbar's own. The panel goes into the portal layer that sits above the navigation widgets, since it grows tall enough to reach them. That also keeps it out of the toolbar, whose collapse would otherwise snapshot it, and clear of the floating ui layer the popovers use, which is below the toolbar and would swallow it. --- entry_types/scrolled/config/locales/de.yml | 1 + entry_types/scrolled/config/locales/en.yml | 1 + .../editor/views/EntryCommentsView-spec.js | 22 +++ .../features/commentActivity-spec.js | 164 ++++++++++++++++++ .../package/spec/review/ThreadList-spec.js | 19 ++ .../spec/support/pageObjects/commenting.js | 20 ++- .../src/frontend/commenting/ActivityButton.js | 129 ++++++++++++++ .../commenting/ActivityButton.module.css | 48 +++++ .../src/frontend/commenting/EditableText.js | 6 +- .../frontend/commenting/FloatingToolbar.js | 2 + .../src/frontend/commenting/Popover.js | 2 + .../frontend/commenting/SectionDecorator.js | 5 +- .../commenting/SelectedSubjectProvider.js | 30 +++- .../frontend/commenting/images/activity.svg | 1 + .../src/review/ActivityList.module.css | 4 + .../package/src/review/ReviewStateProvider.js | 9 +- .../scrolled/package/src/review/ThreadList.js | 12 +- .../package/src/review/ThreadsBadge.js | 7 +- .../useLocatedCommentThreadsForSubject.js | 8 +- 19 files changed, 466 insertions(+), 24 deletions(-) create mode 100644 entry_types/scrolled/package/spec/frontend/commenting/features/commentActivity-spec.js create mode 100644 entry_types/scrolled/package/src/frontend/commenting/ActivityButton.js create mode 100644 entry_types/scrolled/package/src/frontend/commenting/ActivityButton.module.css create mode 100644 entry_types/scrolled/package/src/frontend/commenting/images/activity.svg diff --git a/entry_types/scrolled/config/locales/de.yml b/entry_types/scrolled/config/locales/de.yml index 58d5513c1f..b85187222f 100644 --- a/entry_types/scrolled/config/locales/de.yml +++ b/entry_types/scrolled/config/locales/de.yml @@ -2040,6 +2040,7 @@ de: no_threads_yet: Noch keine Kommentare refers_to_deleted_element: Bezieht sich auf ein gelöschtes Element activity: + toggle: Neueste Aktivität summary: topic: Thema begonnen reply_count: diff --git a/entry_types/scrolled/config/locales/en.yml b/entry_types/scrolled/config/locales/en.yml index 5b21f70e61..d9ab4cca86 100644 --- a/entry_types/scrolled/config/locales/en.yml +++ b/entry_types/scrolled/config/locales/en.yml @@ -1868,6 +1868,7 @@ en: no_threads_yet: No comments yet refers_to_deleted_element: Refers to a deleted element activity: + toggle: Latest activity summary: topic: topic started reply_count: 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 23bf4c47b0..ae1ec42ed3 100644 --- a/entry_types/scrolled/package/spec/editor/views/EntryCommentsView-spec.js +++ b/entry_types/scrolled/package/spec/editor/views/EntryCommentsView-spec.js @@ -336,6 +336,28 @@ describe('EntryCommentsView', () => { expect(getByText('on other').closest('[aria-current="true"]')).toBeNull(); }); + it('keeps resolved threads folded away when an element is selected', () => { + const entry = createEntry({ + contentElements: [{id: 1, permaId: 10, typeName: 'image'}] + }); + entry.set('selectedCommentsSubject', {subjectType: 'ContentElement', id: 1}); + entry.reviewSession = factories.reviewSession({ + commentThreads: [ + {id: 1, subjectType: 'ContentElement', subjectId: 10, + comments: [{id: 10, body: 'still open', creatorName: 'A'}]}, + {id: 2, subjectType: 'ContentElement', subjectId: 10, + resolvedAt: '2026-08-17T10:00:00.000Z', + comments: [{id: 20, body: 'already resolved', creatorName: 'B'}]} + ] + }); + + const view = new EntryCommentsView({entry, editor}); + const {getByText, queryByText} = renderBackboneView(view); + + expect(getByText('still open')).toBeInTheDocument(); + expect(queryByText('already resolved')).not.toBeInTheDocument(); + }); + it("falls back to highlightedThreadId when the selected element has commentThreadIdsAtSelection", () => { const entry = createEntry({ contentElements: [{id: 1, permaId: 10, typeName: 'textBlock'}] diff --git a/entry_types/scrolled/package/spec/frontend/commenting/features/commentActivity-spec.js b/entry_types/scrolled/package/spec/frontend/commenting/features/commentActivity-spec.js new file mode 100644 index 0000000000..b6e0371cf2 --- /dev/null +++ b/entry_types/scrolled/package/spec/frontend/commenting/features/commentActivity-spec.js @@ -0,0 +1,164 @@ +import '@testing-library/jest-dom/extend-expect'; +import {fireEvent} from '@testing-library/react'; + +import {renderEntry, useCommentingPageObjects} from 'support/pageObjects/commenting'; +import activityStyles from 'frontend/commenting/ActivityButton.module.css'; +import badgeStyles from 'review/Badge.module.css'; + +describe('comment activity', () => { + useCommentingPageObjects(); + + beforeEach(() => { + window.HTMLElement.prototype.scrollIntoView = jest.fn(); + }); + + const currentUser = {id: 42, name: 'Alice'}; + + function renderEntryWithThreads(commentThreads) { + return renderEntry({ + seed: { + contentElements: [ + {typeName: 'withTestId', configuration: {testId: 5}}, + {typeName: 'withTestId', configuration: {testId: 6}} + ] + }, + commenting: {currentUser, commentThreads} + }); + } + + function renderEntryWithTwoThreads() { + return renderEntryWithThreads([ + {id: 1, permaId: 5, subjectType: 'ContentElement', subjectId: 1, + comments: [{id: 10, body: 'First topic', creatorName: 'Bob', creatorId: 2, + createdAt: '2026-08-17T09:00:00.000Z'}]}, + {id: 2, permaId: 6, subjectType: 'ContentElement', subjectId: 2, + comments: [{id: 11, body: 'Second topic', creatorName: 'Bob', creatorId: 2, + createdAt: '2026-08-17T11:00:00.000Z'}]} + ]); + } + + it('opens the feed from the toolbar', () => { + const entry = renderEntryWithTwoThreads(); + + expect(entry.queryActivityPanel()).toBeNull(); + + fireEvent.click(entry.getActivityButton()); + + expect(entry.getActivityPanel()).toBeInTheDocument(); + }); + + it('renders the feed above the navigation widgets', () => { + const entry = renderEntryWithTwoThreads(); + + fireEvent.click(entry.getActivityButton()); + + expect(document.getElementById('floating-ui-above-navigation-widgets')) + .toContainElement(entry.getActivityPanel()); + }); + + it('lists the activity of every subject newest first', () => { + const entry = renderEntryWithTwoThreads(); + + fireEvent.click(entry.getActivityButton()); + + const panel = entry.getActivityPanel(); + + expect(panel).toHaveTextContent('First topic'); + expect(panel).toHaveTextContent('Second topic'); + }); + + it('closes an open thread popover when the feed opens', () => { + const entry = renderEntryWithTwoThreads(); + + fireEvent.click(entry.getAllCommentBadges()[0]); + expect(entry.getAllByText('First topic')).toHaveLength(1); + + fireEvent.click(entry.getActivityButton()); + + expect(entry.getAllByText('First topic')).toHaveLength(1); + expect(entry.getActivityPanel()).toHaveTextContent('First topic'); + }); + + it('closes the feed again from the toolbar', () => { + const entry = renderEntryWithTwoThreads(); + + fireEvent.click(entry.getActivityButton()); + fireEvent.click(entry.getActivityButton()); + + expect(entry.queryActivityPanel()).toBeNull(); + }); + + it('closes the feed on a click outside', () => { + const entry = renderEntryWithTwoThreads(); + + fireEvent.click(entry.getActivityButton()); + fireEvent.mouseDown(document.body); + + expect(entry.queryActivityPanel()).toBeNull(); + }); + + it('keeps the feed open while it is being used', () => { + const entry = renderEntryWithTwoThreads(); + + fireEvent.click(entry.getActivityButton()); + fireEvent.mouseDown(entry.getActivityPanel()); + + expect(entry.getActivityPanel()).toBeInTheDocument(); + }); + + it('closes the feed on escape', () => { + const entry = renderEntryWithTwoThreads(); + + fireEvent.click(entry.getActivityButton()); + fireEvent.keyDown(document, {key: 'Escape'}); + + expect(entry.queryActivityPanel()).toBeNull(); + }); + + it('reveals the thread of a clicked entry and closes the feed', () => { + const entry = renderEntryWithTwoThreads(); + + fireEvent.click(entry.getActivityButton()); + fireEvent.click(entry.getByText('First topic')); + + expect(entry.queryActivityPanel()).toBeNull(); + expect(window.HTMLElement.prototype.scrollIntoView).toHaveBeenCalled(); + expect(entry.getByText('First topic')).toBeInTheDocument(); + }); + + it('reveals a resolved thread without turning all of them on', () => { + const entry = renderEntryWithThreads([ + {id: 1, permaId: 5, subjectType: 'ContentElement', subjectId: 1, + resolvedAt: '2026-08-17T12:00:00.000Z', resolvedById: 2, resolverName: 'Bob', + comments: [{id: 10, body: 'Resolved topic', creatorName: 'Bob', creatorId: 2, + createdAt: '2026-08-17T09:00:00.000Z'}]} + ]); + + fireEvent.click(entry.getActivityButton()); + fireEvent.click(entry.getByText('Resolved topic')); + + expect(entry.getCommentFilterButton('unresolved')) + .toHaveAttribute('aria-pressed', 'true'); + expect(entry.getByText('Resolved topic')).toBeInTheDocument(); + expect(entry.getAllCommentBadges()[0]).toHaveClass(badgeStyles.resolved); + }); + + it('marks the button while activity is unseen', () => { + const entry = renderEntryWithTwoThreads(); + + expect(entry.getActivityButton().querySelector(`.${activityStyles.unseenDot}`)) + .not.toBeNull(); + }); + + it('leaves the button unmarked once everything has been seen', () => { + const entry = renderEntryWithThreads([ + {id: 1, permaId: 5, subjectType: 'ContentElement', subjectId: 1, + comments: [{id: 10, body: 'My own topic', creatorName: 'Alice', + creatorId: currentUser.id, + createdAt: '2026-08-17T09:00:00.000Z'}]} + ]); + + expect(entry.getActivityButton().querySelector(`.${activityStyles.unseenDot}`)) + .toBeNull(); + }); +}); diff --git a/entry_types/scrolled/package/spec/review/ThreadList-spec.js b/entry_types/scrolled/package/spec/review/ThreadList-spec.js index 5efdbd72d3..79b35fdfdb 100644 --- a/entry_types/scrolled/package/spec/review/ThreadList-spec.js +++ b/entry_types/scrolled/package/spec/review/ThreadList-spec.js @@ -129,6 +129,25 @@ describe('ThreadList', () => { expect(highlighted).not.toContainElement(getByText('first')); }); + // Its only thread is resolved, so the list would otherwise read as + // empty and offer to start a topic. + it('offers no new thread form while a resolved thread is highlighted', () => { + const {getByText, queryByPlaceholderText} = renderThreadList( + , + { + commentThreads: [ + {id: 2, subjectType: 'ContentElement', subjectId: 10, + resolvedAt: '2026-08-17T10:00:00.000Z', comments: [ + {id: 20, body: 'Resolved topic', creatorName: 'Bob', creatorId: 2} + ]} + ] + } + ); + + expect(getByText('Resolved topic')).toBeInTheDocument(); + expect(queryByPlaceholderText('Add a comment...')).toBeNull(); + }); + it('highlights every thread when highlightedThreadId is an array of ids', () => { const {container, getByText} = renderThreadList( , diff --git a/entry_types/scrolled/package/spec/support/pageObjects/commenting.js b/entry_types/scrolled/package/spec/support/pageObjects/commenting.js index bac9dd65f0..1b60fe07c7 100644 --- a/entry_types/scrolled/package/spec/support/pageObjects/commenting.js +++ b/entry_types/scrolled/package/spec/support/pageObjects/commenting.js @@ -42,7 +42,10 @@ export function renderEntry({ getCommentFilterButton: resolution => result.getByRole('button', {name: resolution === 'all' ? 'All' : 'Unresolved'}), getPreviousCommentButton: () => result.getByRole('button', {name: 'Previous comment'}), - getNextCommentButton: () => result.getByRole('button', {name: 'Next comment'}) + getNextCommentButton: () => result.getByRole('button', {name: 'Next comment'}), + getActivityButton: () => result.getByRole('button', {name: 'Latest activity'}), + getActivityPanel: () => result.getByRole('dialog', {name: 'Latest activity'}), + queryActivityPanel: () => result.queryByRole('dialog', {name: 'Latest activity'}) }; } @@ -74,7 +77,20 @@ export function useCommentingPageObjects() { 'pageflow_scrolled.review.filter.unresolved': 'Unresolved', 'pageflow_scrolled.review.filter.all': 'All', 'pageflow_scrolled.review.previous_comment': 'Previous comment', - 'pageflow_scrolled.review.next_comment': 'Next comment' + 'pageflow_scrolled.review.next_comment': 'Next comment', + 'pageflow_scrolled.review.activity.toggle': 'Latest activity', + 'pageflow_scrolled.review.activity.no_activity_yet': 'No activity yet', + 'pageflow_scrolled.review.activity.today': 'Today', + 'pageflow_scrolled.review.activity.yesterday': 'Yesterday', + 'pageflow_scrolled.review.reply_count.one': '1 reply', + 'pageflow_scrolled.review.reply_count.other': '%{count} replies', + 'pageflow_scrolled.review.earlier_reply_count.one': '1 more', + 'pageflow_scrolled.review.earlier_reply_count.other': '%{count} more', + 'pageflow_scrolled.review.resolve': 'Mark as resolved', + 'pageflow_scrolled.review.unresolve': 'Mark as unresolved', + 'pageflow_scrolled.review.thread_actions': 'Thread actions', + 'pageflow_scrolled.review.resolution_by': 'Marked as resolved by', + 'pageflow_scrolled.review.resolution': 'Marked as resolved' }); usePageObjects(); diff --git a/entry_types/scrolled/package/src/frontend/commenting/ActivityButton.js b/entry_types/scrolled/package/src/frontend/commenting/ActivityButton.js new file mode 100644 index 0000000000..8daf10b1a0 --- /dev/null +++ b/entry_types/scrolled/package/src/frontend/commenting/ActivityButton.js @@ -0,0 +1,129 @@ +import React, {useCallback, useEffect, useRef, useState} from 'react'; +import classNames from 'classnames'; +import { + useFloating, FloatingPortal, offset, shift, size, autoUpdate +} from '@floating-ui/react'; + +import {ActivityList, useUnseenActivityCount} from 'pageflow-scrolled/review'; +import {useI18n} from '../i18n'; +import {useFloatingPortalRoot} from '../FloatingPortalRootProvider'; +import {useCommentNavigation, useSelectedSubject} from './SelectedSubjectProvider'; + +import ActivityIcon from './images/activity.svg'; +import toolbarStyles from './FloatingToolbar.module.css'; +import styles from './ActivityButton.module.css'; + +// The default navigation sits across the top of the viewport - a 50px bar +// with an 8px progress bar under it - and the panel grows up into it. +const viewportPadding = {top: 74, right: 16, bottom: 16, left: 16}; + +export function ActivityButton() { + const {t} = useI18n({locale: 'ui'}); + const [open, setOpen] = useState(false); + + const unseenCount = useUnseenActivityCount(); + const label = t('pageflow_scrolled.review.activity.toggle'); + const {clearSelection} = useSelectedSubject(); + const portalRoot = useFloatingPortalRoot(); + + const {refs, floatingStyles} = useFloating({ + open, + strategy: 'fixed', + placement: 'top-end', + middleware: [ + offset(8), + shift({padding: viewportPadding}), + size({ + padding: viewportPadding, + apply({availableHeight, elements}) { + elements.floating.style.maxHeight = `${availableHeight}px`; + } + }) + ], + whileElementsMounted: autoUpdate + }); + + return ( + <> + + + {/* Out of the toolbar, whose view transition would snapshot the + panel along with it and whose clicks are exempt from dismissing + open popovers. */} + {open && + + setOpen(false)} /> + } + + ); +} + +const ActivityPanel = React.forwardRef(function ActivityPanel({style, onClose}, ref) { + const {t} = useI18n({locale: 'ui'}); + const {goToThread} = useCommentNavigation(); + + const panelRef = useRef(); + + const setRefs = useCallback(node => { + panelRef.current = node; + ref(node); + }, [ref]); + + useEffect(() => { + function handleClick(event) { + if (panelRef.current?.contains(event.target)) return; + if (event.target.closest('[data-comment-toolbar]')) return; + + onClose(); + } + + // Claimed in the capture phase so that an open popover does not close + // itself on the same key. + function handleKeyDown(event) { + if (event.key === 'Escape') { + event.stopPropagation(); + onClose(); + } + } + + document.addEventListener('mousedown', handleClick); + document.addEventListener('keydown', handleKeyDown, true); + + return () => { + document.removeEventListener('mousedown', handleClick); + document.removeEventListener('keydown', handleKeyDown, true); + }; + }, [onClose]); + + return ( +
+
+ { + goToThread(entry.threadId); + onClose(); + }} /> +
+
+ ); +}); diff --git a/entry_types/scrolled/package/src/frontend/commenting/ActivityButton.module.css b/entry_types/scrolled/package/src/frontend/commenting/ActivityButton.module.css new file mode 100644 index 0000000000..17c554de0a --- /dev/null +++ b/entry_types/scrolled/package/src/frontend/commenting/ActivityButton.module.css @@ -0,0 +1,48 @@ +.button { + position: relative; +} + +.unseenDot { + position: absolute; + top: space(0.5); + right: space(0.5); + width: space(2); + height: space(2); + border-radius: 50%; + background: var(--ui-warning-color); + box-shadow: 0 0 0 1px var(--ui-primary-color); +} + +/* Entry typography would otherwise reach in here, since this renders + inside the entry's own DOM. */ +.panel { + display: flex; + flex-direction: column; + width: space(96); + overflow: hidden; + font-family: var(--ui-font-family); + font-size: var(--ui-font-size); + line-height: normal; + text-align: left; + color: var(--ui-on-surface-color); + background: var(--ui-surface-color); + border-radius: rounded(lg); + box-shadow: var(--ui-box-shadow-md); + + --review-first-day-heading-margin-top: 0; + --review-thread-box-shadow: none; + --review-thread-border-color: var(--ui-on-surface-color-lightest); + --review-thread-border: 1px solid var(--review-thread-border-color); + --review-resolved-threads-pill-align: flex-end; + --review-resolved-threads-pill-color: var(--ui-on-surface-color); + --review-resolved-threads-pill-background-color: transparent; +} + +/* Scrolls inside the panel rather than the panel itself, so that the + scrollbar is clipped by the rounded corners instead of squaring them + off. */ +.scroller { + min-height: 0; + overflow-y: auto; + padding: space(3); +} diff --git a/entry_types/scrolled/package/src/frontend/commenting/EditableText.js b/entry_types/scrolled/package/src/frontend/commenting/EditableText.js index 0cb5451bce..34baaa2b9a 100644 --- a/entry_types/scrolled/package/src/frontend/commenting/EditableText.js +++ b/entry_types/scrolled/package/src/frontend/commenting/EditableText.js @@ -42,12 +42,14 @@ function CommentingEditableText({ const {anchors, registerAnchor} = useRangeAnchors(); const {contentElementPermaId} = useContentElementAttributes(); const {active, deactivate, preselect, clearPreselection} = useAddCommentMode(); - const {subjectRange, select} = useSelectedSubject('ContentElement', contentElementPermaId); + const {subjectRange, select, highlightedThreadId} = + useSelectedSubject('ContentElement', contentElementPermaId); const {resolution} = useCommentDisplayFilter(); const threads = useCommentThreads({ subjectType: 'ContentElement', subjectId: contentElementPermaId, - resolution + resolution, + revealedThreadId: highlightedThreadId }); const highlights = useCommentHighlights(threads, subjectRange); diff --git a/entry_types/scrolled/package/src/frontend/commenting/FloatingToolbar.js b/entry_types/scrolled/package/src/frontend/commenting/FloatingToolbar.js index e4483f85ab..fba0d1cc34 100644 --- a/entry_types/scrolled/package/src/frontend/commenting/FloatingToolbar.js +++ b/entry_types/scrolled/package/src/frontend/commenting/FloatingToolbar.js @@ -7,6 +7,7 @@ import {useAddCommentMode} from './AddCommentModeProvider'; import {useCommentDisplayFilter} from './CommentDisplayFilterProvider'; import {useCommentingVisibility} from './CommentingVisibilityProvider'; import {useCommentNavigation} from './SelectedSubjectProvider'; +import {ActivityButton} from './ActivityButton'; import AddCommentIcon from './images/addComment.svg'; import CancelCommentIcon from './images/cancelComment.svg'; @@ -38,6 +39,7 @@ export function FloatingToolbar() { +
diff --git a/entry_types/scrolled/package/src/frontend/commenting/Popover.js b/entry_types/scrolled/package/src/frontend/commenting/Popover.js index 4857d111a2..654d39b26e 100644 --- a/entry_types/scrolled/package/src/frontend/commenting/Popover.js +++ b/entry_types/scrolled/package/src/frontend/commenting/Popover.js @@ -48,6 +48,7 @@ export function Popover({ subjectId={subjectId} subjectRange={subjectRange} resolution={resolution} + revealedThreadId={highlightedThreadId} mode={isSelected ? 'active' : undefined} onClick={handleBadgeClick} /> {isSelected && @@ -96,6 +97,7 @@ function OpenThreadList({ if (event.target.closest('[data-comment-highlight]')) return; if (event.target.closest('[data-comment-toolbar]')) return; if (event.target.closest('[data-comment-menu]')) return; + if (event.target.closest('[data-comment-activity]')) return; onDismiss(); } diff --git a/entry_types/scrolled/package/src/frontend/commenting/SectionDecorator.js b/entry_types/scrolled/package/src/frontend/commenting/SectionDecorator.js index 4e2769dce1..ea62dbf402 100644 --- a/entry_types/scrolled/package/src/frontend/commenting/SectionDecorator.js +++ b/entry_types/scrolled/package/src/frontend/commenting/SectionDecorator.js @@ -15,12 +15,13 @@ import styles from './SectionDecorator.module.css'; export function SectionDecorator({section, children}) { const {visible} = useCommentingVisibility(); const {active} = useAddCommentMode(); - const {isSelected} = useSelectedSubject('Section', section.permaId); + const {isSelected, highlightedThreadId} = useSelectedSubject('Section', section.permaId); const {resolution} = useCommentDisplayFilter(); const threads = useLocatedCommentThreadsForSubject({ subjectType: 'Section', subjectId: section.permaId, - resolution + resolution, + revealedThreadId: highlightedThreadId }); const hasThreads = threads.length > 0; diff --git a/entry_types/scrolled/package/src/frontend/commenting/SelectedSubjectProvider.js b/entry_types/scrolled/package/src/frontend/commenting/SelectedSubjectProvider.js index f587a5635b..c324b28bb3 100644 --- a/entry_types/scrolled/package/src/frontend/commenting/SelectedSubjectProvider.js +++ b/entry_types/scrolled/package/src/frontend/commenting/SelectedSubjectProvider.js @@ -14,7 +14,8 @@ const CommentNavigationContext = createContext({ count: 0, position: 0, goToNext: () => {}, - goToPrevious: () => {} + goToPrevious: () => {}, + goToThread: () => {} }); export function SelectedSubjectProvider({children}) { @@ -24,9 +25,11 @@ export function SelectedSubjectProvider({children}) { const [selectedSubject, setSelectedSubject] = useState(null); + const allTargets = useMemo(() => navigableTargets(chapters), [chapters]); + const targets = useMemo( - () => navigableTargets(chapters, resolution), - [chapters, resolution] + () => allTargets.filter(target => matchesResolution(target, resolution)), + [allTargets, resolution] ); const clearSelection = useCallback(() => { @@ -67,6 +70,18 @@ export function SelectedSubjectProvider({children}) { selectTarget(targets[next]); }, [targets, selectedSubject, selectTarget]); + // Searched among all targets rather than the filtered ones, so that a + // resolved thread stays reachable from lists that show it whatever the + // toolbar filters. Showing it is left to the selection: turning all + // resolved threads on for the sake of one changes the whole preview. + const goToThread = useCallback(threadId => { + const target = allTargets.find(target => target.threadId === threadId); + + if (target) { + selectTarget(target); + } + }, [allTargets, selectTarget]); + const position = useMemo( () => currentTargetIndex(targets, selectedSubject) + 1, [targets, selectedSubject] @@ -82,8 +97,9 @@ export function SelectedSubjectProvider({children}) { count: targets.length, position, goToNext: () => goTo(1), - goToPrevious: () => goTo(-1) - }), [targets.length, position, goTo]); + goToPrevious: () => goTo(-1), + goToThread + }), [targets.length, position, goTo, goToThread]); return ( @@ -132,7 +148,7 @@ function currentTargetIndex(targets, selectedSubject) { return targets.findIndex(target => target.key === key); } -function navigableTargets(chapters, resolution) { +function navigableTargets(chapters) { const targets = []; chapters.forEach(chapter => { @@ -157,7 +173,7 @@ function navigableTargets(chapters, resolution) { }); }); - return targets.filter(target => matchesResolution(target, resolution)); + return targets; } function pushTargets(targets, threads, location) { diff --git a/entry_types/scrolled/package/src/frontend/commenting/images/activity.svg b/entry_types/scrolled/package/src/frontend/commenting/images/activity.svg new file mode 100644 index 0000000000..2e2bdd30dc --- /dev/null +++ b/entry_types/scrolled/package/src/frontend/commenting/images/activity.svg @@ -0,0 +1 @@ + diff --git a/entry_types/scrolled/package/src/review/ActivityList.module.css b/entry_types/scrolled/package/src/review/ActivityList.module.css index 3909848b70..4fb35b0c0b 100644 --- a/entry_types/scrolled/package/src/review/ActivityList.module.css +++ b/entry_types/scrolled/package/src/review/ActivityList.module.css @@ -54,6 +54,10 @@ color: var(--ui-on-surface-color-light); } +.dayHeading:first-child { + margin-top: var(--review-first-day-heading-margin-top, space(2)); +} + .dayHeading::before, .dayHeading::after { content: ''; diff --git a/entry_types/scrolled/package/src/review/ReviewStateProvider.js b/entry_types/scrolled/package/src/review/ReviewStateProvider.js index d20d7348ec..e89eee4dc7 100644 --- a/entry_types/scrolled/package/src/review/ReviewStateProvider.js +++ b/entry_types/scrolled/package/src/review/ReviewStateProvider.js @@ -182,7 +182,9 @@ export function useCommentThread(threadId) { return context?.commentThreads.find(t => t.id === threadId); } -export function useCommentThreads({subjectType, subjectId, subjectRange, resolution = 'all'} = {}) { +export function useCommentThreads({ + subjectType, subjectId, subjectRange, resolution = 'all', revealedThreadId +} = {}) { const context = useContext(ReviewStateContext); const commentThreads = context ? context.commentThreads : []; const hasSubject = subjectType !== undefined; @@ -196,9 +198,10 @@ export function useCommentThreads({subjectType, subjectId, subjectRange, resolut thread.subjectId === subjectId && (!rangeKey || JSON.stringify(thread.subjectRange) === rangeKey))) && - matchesResolution(thread, resolution) + (matchesResolution(thread, resolution) || thread.id === revealedThreadId) ); - }, [commentThreads, hasSubject, subjectType, subjectId, subjectRange, resolution]); + }, [commentThreads, hasSubject, subjectType, subjectId, subjectRange, resolution, + revealedThreadId]); } export function matchesResolution(thread, resolution) { diff --git a/entry_types/scrolled/package/src/review/ThreadList.js b/entry_types/scrolled/package/src/review/ThreadList.js index ff795b3c4c..d482923a7e 100644 --- a/entry_types/scrolled/package/src/review/ThreadList.js +++ b/entry_types/scrolled/package/src/review/ThreadList.js @@ -45,12 +45,20 @@ export function ThreadList({subjectType, subjectId, subjectRange, filter, highli (soleThread(activeThreads) || soleThread(resolvedThreads))?.id ); const [resolvedToggled, setResolvedToggled] = useState(null); + // A group highlight covers every thread of the subject; only one naming + // a single thread says the reviewer picked it out, which is what brings + // a resolved thread out of the fold. + const pickedThreadId = Array.isArray(highlightedThreadId) ? null : highlightedThreadId; + + const revealsResolved = + !!expandResolved || resolvedThreads.some(thread => thread.id === pickedThreadId); + const [formToggled, setFormToggled] = useState( showNewFormProp !== undefined ? showNewFormProp : - expandResolved ? noThreads : activeThreads.length === 0 + revealsResolved ? noThreads : activeThreads.length === 0 ); - const showResolved = resolvedToggled !== null ? resolvedToggled : !!expandResolved; + const showResolved = resolvedToggled !== null ? resolvedToggled : revealsResolved; // An unsent draft reopens the form and keeps it open while the thread is // being created. Callers passing showNewForm={false} suppress the form diff --git a/entry_types/scrolled/package/src/review/ThreadsBadge.js b/entry_types/scrolled/package/src/review/ThreadsBadge.js index c02fa60e87..7e0613275f 100644 --- a/entry_types/scrolled/package/src/review/ThreadsBadge.js +++ b/entry_types/scrolled/package/src/review/ThreadsBadge.js @@ -5,11 +5,12 @@ import {useLocatedCommentThreadsForSubject} from './useLocatedCommentThreadsForS import {useUnreadCommentCount} from './unreadComments'; import {Badge} from './Badge'; -export function ThreadsBadge({subjectType, subjectId, subjectRange, onClick, mode, resolution = 'unresolved'}) { +export function ThreadsBadge({subjectType, subjectId, subjectRange, onClick, mode, resolution = 'unresolved', revealedThreadId}) { const {t} = useI18n({locale: 'ui'}); - const threads = - useLocatedCommentThreadsForSubject({subjectType, subjectId, subjectRange, resolution}); + const threads = useLocatedCommentThreadsForSubject({ + subjectType, subjectId, subjectRange, resolution, revealedThreadId + }); const unresolvedThreads = useLocatedCommentThreadsForSubject({subjectType, subjectId, subjectRange, resolution: 'unresolved'}); diff --git a/entry_types/scrolled/package/src/review/useLocatedCommentThreadsForSubject.js b/entry_types/scrolled/package/src/review/useLocatedCommentThreadsForSubject.js index 77192c029f..e404744371 100644 --- a/entry_types/scrolled/package/src/review/useLocatedCommentThreadsForSubject.js +++ b/entry_types/scrolled/package/src/review/useLocatedCommentThreadsForSubject.js @@ -10,11 +10,13 @@ const NONE = []; // filters by resolution and range. Lets ThreadList/ThreadsBadge consume // threads by subject without re-scanning review state or re-running the // join, and keeps resolution/range filtering out of the components. +// `revealedThreadId` survives the resolution filter, so that a resolved +// thread can be shown on its own without turning all of them on. /** * @private */ export function useLocatedCommentThreadsForSubject({ - subjectType, subjectId, subjectRange, resolution = 'all' + subjectType, subjectId, subjectRange, resolution = 'all', revealedThreadId }) { const {bySubject} = useLocatedCommentThreads(); @@ -23,8 +25,8 @@ export function useLocatedCommentThreadsForSubject({ const rangeKey = subjectRange ? JSON.stringify(subjectRange) : undefined; return threads.filter(thread => - matchesResolution(thread, resolution) && + (matchesResolution(thread, resolution) || thread.id === revealedThreadId) && (!rangeKey || JSON.stringify(thread.subjectRange) === rangeKey) ); - }, [bySubject, subjectType, subjectId, subjectRange, resolution]); + }, [bySubject, subjectType, subjectId, subjectRange, resolution, revealedThreadId]); } From 1d158d5acb35fc50f53806781de615d125913855 Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Wed, 26 Aug 2026 12:01:39 +0200 Subject: [PATCH 20/29] Count the collapsed toolbar's unseen marker like the feed The puck counted unread comments while the activity button beside it counts topics carrying something new, so the same toolbar could show two markers that disagree - a thread someone resolved without commenting lit one and not the other. --- entry_types/scrolled/config/locales/de.yml | 4 +-- entry_types/scrolled/config/locales/en.yml | 4 +-- .../commenting/features/unreadToolbar-spec.js | 26 ++++++++++++++----- .../frontend/commenting/FloatingToolbar.js | 9 +++---- 4 files changed, 28 insertions(+), 15 deletions(-) diff --git a/entry_types/scrolled/config/locales/de.yml b/entry_types/scrolled/config/locales/de.yml index b85187222f..90e9016db8 100644 --- a/entry_types/scrolled/config/locales/de.yml +++ b/entry_types/scrolled/config/locales/de.yml @@ -1990,8 +1990,8 @@ de: hide_comments: Kommentare ausblenden show_comments: Kommentare einblenden show_comments_with_unread: - one: Kommentare einblenden (1 ungelesener Kommentar) - other: Kommentare einblenden (%{count} ungelesene Kommentare) + one: Kommentare einblenden (1 Thema mit neuer Aktivität) + other: Kommentare einblenden (%{count} Themen mit neuer Aktivität) comment_toolbar: Kommentare filter: label: Kommentare filtern diff --git a/entry_types/scrolled/config/locales/en.yml b/entry_types/scrolled/config/locales/en.yml index d9ab4cca86..5d5f7d14d5 100644 --- a/entry_types/scrolled/config/locales/en.yml +++ b/entry_types/scrolled/config/locales/en.yml @@ -1818,8 +1818,8 @@ en: hide_comments: Hide comments show_comments: Show comments show_comments_with_unread: - one: Show comments (1 unread comment) - other: Show comments (%{count} unread comments) + one: Show comments (1 topic with new activity) + other: Show comments (%{count} topics with new activity) comment_toolbar: Comments filter: label: Filter comments diff --git a/entry_types/scrolled/package/spec/frontend/commenting/features/unreadToolbar-spec.js b/entry_types/scrolled/package/spec/frontend/commenting/features/unreadToolbar-spec.js index 3d120d752d..1b7451d5bd 100644 --- a/entry_types/scrolled/package/spec/frontend/commenting/features/unreadToolbar-spec.js +++ b/entry_types/scrolled/package/spec/frontend/commenting/features/unreadToolbar-spec.js @@ -12,20 +12,21 @@ describe('unread comments on the collapsed toolbar', () => { useFakeTranslations({ 'pageflow_scrolled.review.show_comments': 'Show comments', 'pageflow_scrolled.review.show_comments_with_unread.one': - 'Show comments (1 unread comment)', + 'Show comments (1 topic with new activity)', 'pageflow_scrolled.review.show_comments_with_unread.other': - 'Show comments (%{count} unread comments)' + 'Show comments (%{count} topics with new activity)' }); const currentUser = {id: 42, name: 'Alice'}; - function renderCollapsedEntry({comments}) { + function renderCollapsedEntry({comments, ...threadAttributes}) { const entry = renderEntry({ seed: {contentElements: [{typeName: 'withTestId', configuration: {testId: 5}}]}, commenting: { currentUser, commentThreads: [ - {id: 1, permaId: 7, subjectType: 'ContentElement', subjectId: 1, comments} + {id: 1, permaId: 7, subjectType: 'ContentElement', subjectId: 1, comments, + ...threadAttributes} ], commentThreadReads: {} } @@ -54,13 +55,26 @@ describe('unread comments on the collapsed toolbar', () => { expect(unreadDot(entry)).not.toBeNull(); }); - it('names the unseen comments on the show button', () => { + // Counted the way the activity feed counts, so that the toolbar's two + // markers cannot disagree. + it('counts the topics rather than their comments on the show button', () => { const entry = renderCollapsedEntry({ comments: [comment(), comment({id: 11, creatorId: 44})] }); expect(entry.getShowCommentsButton()) - .toHaveAttribute('aria-label', 'Show comments (2 unread comments)'); + .toHaveAttribute('aria-label', 'Show comments (1 topic with new activity)'); + }); + + it('marks the show button for a resolution without new comments', () => { + const entry = renderCollapsedEntry({ + comments: [comment({creatorId: currentUser.id, creatorName: 'Alice'})], + resolvedAt: '2026-08-17T12:00:00.000Z', + resolvedById: 43, + resolverName: 'Bob' + }); + + expect(unreadDot(entry)).not.toBeNull(); }); it('leaves the show button unmarked without unseen comments', () => { diff --git a/entry_types/scrolled/package/src/frontend/commenting/FloatingToolbar.js b/entry_types/scrolled/package/src/frontend/commenting/FloatingToolbar.js index fba0d1cc34..421730b4ca 100644 --- a/entry_types/scrolled/package/src/frontend/commenting/FloatingToolbar.js +++ b/entry_types/scrolled/package/src/frontend/commenting/FloatingToolbar.js @@ -1,7 +1,7 @@ import React, {useEffect} from 'react'; import classNames from 'classnames'; -import {useLocatedCommentThreads, useUnreadCommentCount} from 'pageflow-scrolled/review'; +import {useLocatedCommentThreads, useUnseenActivityCount} from 'pageflow-scrolled/review'; import {useI18n} from '../i18n'; import {useAddCommentMode} from './AddCommentModeProvider'; import {useCommentDisplayFilter} from './CommentDisplayFilterProvider'; @@ -64,16 +64,15 @@ function HideCommentsButton() { function ShowCommentsButton() { const {t} = useI18n({locale: 'ui'}); const {toggle} = useCommentingVisibility(); - const {threads} = useLocatedCommentThreads(); - const unreadCommentCount = useUnreadCommentCount(threads); - const unread = unreadCommentCount > 0; + const unseenCount = useUnseenActivityCount(); + const unread = unseenCount > 0; // Named rather than only marked, since the dot is the sole cue that // comments are waiting behind the collapsed toolbar. const label = unread ? t('pageflow_scrolled.review.show_comments_with_unread', - {count: unreadCommentCount}) : + {count: unseenCount}) : t('pageflow_scrolled.review.show_comments'); return ( From f28cab987967402587ec7cede074df31c950b842 Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Wed, 26 Aug 2026 12:07:34 +0200 Subject: [PATCH 21/29] Reveal a thread from the activity feed without its popover A row in the feed shows its thread already, so opening the popover on top of it said everything twice and covered the content the click had just revealed. The selection now carries whether it only reveals: the badge goes active and the subject scrolls into view, while the thread list stays where the reviewer was reading it. Clicking the badge or the highlight of a revealed subject opens the popover after all, on the thread it was revealed for, rather than clearing the selection or doing nothing. --- .../features/commentActivity-spec.js | 27 ++++++++++++++-- .../package/spec/review/ThreadsBadge-spec.js | 31 +++++++++++++++++++ .../src/frontend/commenting/ActivityButton.js | 6 ++-- .../src/frontend/commenting/EditableText.js | 9 +++--- .../src/frontend/commenting/Popover.js | 13 ++++---- .../commenting/SelectedSubjectProvider.js | 12 ++++--- .../scrolled/package/src/review/Badge.js | 4 +-- .../package/src/review/ThreadsBadge.js | 8 ++++- 8 files changed, 86 insertions(+), 24 deletions(-) diff --git a/entry_types/scrolled/package/spec/frontend/commenting/features/commentActivity-spec.js b/entry_types/scrolled/package/spec/frontend/commenting/features/commentActivity-spec.js index b6e0371cf2..e20166fcb3 100644 --- a/entry_types/scrolled/package/spec/frontend/commenting/features/commentActivity-spec.js +++ b/entry_types/scrolled/package/spec/frontend/commenting/features/commentActivity-spec.js @@ -115,15 +115,36 @@ describe('comment activity', () => { expect(entry.queryActivityPanel()).toBeNull(); }); - it('reveals the thread of a clicked entry and closes the feed', () => { + it('reveals the subject of a clicked entry without leaving the feed', () => { const entry = renderEntryWithTwoThreads(); fireEvent.click(entry.getActivityButton()); fireEvent.click(entry.getByText('First topic')); - expect(entry.queryActivityPanel()).toBeNull(); + expect(entry.getActivityPanel()).toBeInTheDocument(); expect(window.HTMLElement.prototype.scrollIntoView).toHaveBeenCalled(); - expect(entry.getByText('First topic')).toBeInTheDocument(); + }); + + // The row shows the thread already, so a popover would only say it + // twice and cover the content just revealed. + it('opens no popover for a clicked entry', () => { + const entry = renderEntryWithTwoThreads(); + + fireEvent.click(entry.getActivityButton()); + fireEvent.click(entry.getByText('First topic')); + + expect(entry.getAllByText('First topic')).toHaveLength(1); + }); + + it('opens the popover from the badge of a revealed subject', () => { + const entry = renderEntryWithTwoThreads(); + + fireEvent.click(entry.getActivityButton()); + fireEvent.click(entry.getByText('First topic')); + + fireEvent.click(entry.getAllCommentBadges()[0]); + + expect(entry.getAllByText('First topic')).toHaveLength(2); }); it('reveals a resolved thread without turning all of them on', () => { diff --git a/entry_types/scrolled/package/spec/review/ThreadsBadge-spec.js b/entry_types/scrolled/package/spec/review/ThreadsBadge-spec.js index 1a9575fa2d..129d4f60cd 100644 --- a/entry_types/scrolled/package/spec/review/ThreadsBadge-spec.js +++ b/entry_types/scrolled/package/spec/review/ThreadsBadge-spec.js @@ -141,6 +141,37 @@ describe('ThreadsBadge', () => { expect(getByRole('status')).toHaveTextContent('2'); }); + it('does not count a revealed resolved thread', () => { + const {getByRole} = renderThreadsBadge( + , + { + commentThreads: [ + {id: 1, subjectType: 'ContentElement', subjectId: 10, resolvedAt: null, comments: []}, + {id: 2, subjectType: 'ContentElement', subjectId: 10, resolvedAt: null, comments: []}, + {id: 3, subjectType: 'ContentElement', subjectId: 10, + resolvedAt: '2026-04-09T10:00:00Z', comments: []} + ] + } + ); + + expect(getByRole('status')).toHaveTextContent('2'); + }); + + it('still shows a badge for a revealed resolved thread on its own', () => { + const {getByRole} = renderThreadsBadge( + , + { + commentThreads: [ + {id: 3, subjectType: 'ContentElement', subjectId: 10, + resolvedAt: '2026-04-09T10:00:00Z', comments: []} + ] + } + ); + + expect(getByRole('status')).toBeInTheDocument(); + expect(getByRole('status')).toHaveClass(badgeStyles.resolved); + }); + it('counts a section\'s orphaned threads too', () => { const {getByRole} = renderThreadsBadge( , diff --git a/entry_types/scrolled/package/src/frontend/commenting/ActivityButton.js b/entry_types/scrolled/package/src/frontend/commenting/ActivityButton.js index 8daf10b1a0..68f1bc854b 100644 --- a/entry_types/scrolled/package/src/frontend/commenting/ActivityButton.js +++ b/entry_types/scrolled/package/src/frontend/commenting/ActivityButton.js @@ -119,10 +119,8 @@ const ActivityPanel = React.forwardRef(function ActivityPanel({style, onClose}, aria-label={t('pageflow_scrolled.review.activity.toggle')} data-comment-activity>
- { - goToThread(entry.threadId); - onClose(); - }} /> + goToThread(entry.threadId, + {revealOnly: true})} />
); diff --git a/entry_types/scrolled/package/src/frontend/commenting/EditableText.js b/entry_types/scrolled/package/src/frontend/commenting/EditableText.js index 34baaa2b9a..ba93bea6a4 100644 --- a/entry_types/scrolled/package/src/frontend/commenting/EditableText.js +++ b/entry_types/scrolled/package/src/frontend/commenting/EditableText.js @@ -112,20 +112,21 @@ function CommentingEditableText({ function ClickableHighlight({subjectRange, children}) { const {contentElementPermaId} = useContentElementAttributes(); const {deactivate} = useAddCommentMode(); - const {isSelected, select} = useSelectedSubject('ContentElement', contentElementPermaId, subjectRange); + const {isSelected, revealOnly, select, highlightedThreadId} = + useSelectedSubject('ContentElement', contentElementPermaId, subjectRange); function handleClick(event) { if (event.target.closest('a')) return; - if (isSelected) return; + if (isSelected && !revealOnly) return; deactivate(); - select(); + select(revealOnly ? {highlightedThreadId} : undefined); } return ( {children} diff --git a/entry_types/scrolled/package/src/frontend/commenting/Popover.js b/entry_types/scrolled/package/src/frontend/commenting/Popover.js index 654d39b26e..3988c4ae21 100644 --- a/entry_types/scrolled/package/src/frontend/commenting/Popover.js +++ b/entry_types/scrolled/package/src/frontend/commenting/Popover.js @@ -15,7 +15,7 @@ export function Popover({ subjectType, subjectId, subjectRange, placement = 'bottom-start', strategy = 'absolute', hideNewTopicButton }) { - const {isSelected, showNewForm, select, clearSelection, highlightedThreadId} = + const {isSelected, revealOnly, showNewForm, select, clearSelection, highlightedThreadId} = useSelectedSubject(subjectType, subjectId, subjectRange); const {resolution} = useCommentDisplayFilter(); const [reference, setReference] = useState(null); @@ -33,17 +33,19 @@ export function Popover({ }, [isSelected, highlightedThreadId, reference]); function handleBadgeClick() { - if (isSelected) { + if (isSelected && !revealOnly) { clearSelection(); } else { - select(); + // A revealed subject opens its popover on the thread it was + // revealed for rather than starting over. + select(revealOnly ? {highlightedThreadId} : undefined); } } return ( - + - {isSelected && + {isSelected && !revealOnly && { + const selectTarget = useCallback((target, options) => { // Activate the excursion the target lives in (or leave the current // one) before selecting it, so its popover can mount and open. Only // needed when moving to a different subject. @@ -53,7 +53,8 @@ export function SelectedSubjectProvider({children}) { subjectType: target.subjectType, subjectId: target.subjectId, subjectRange: target.subjectRange, - highlightedThreadId: target.threadId + highlightedThreadId: target.threadId, + ...options }); }, [selectedSubject, activateExcursionOfSection, returnFromExcursion]); @@ -74,11 +75,11 @@ export function SelectedSubjectProvider({children}) { // resolved thread stays reachable from lists that show it whatever the // toolbar filters. Showing it is left to the selection: turning all // resolved threads on for the sake of one changes the whole preview. - const goToThread = useCallback(threadId => { + const goToThread = useCallback((threadId, options) => { const target = allTargets.find(target => target.threadId === threadId); if (target) { - selectTarget(target); + selectTarget(target, options); } }, [allTargets, selectTarget]); @@ -127,6 +128,9 @@ export function useSelectedSubject(subjectType, subjectId, subjectRange) { }, [setSelectedSubject, subjectType, subjectId, subjectRange]); return {isSelected, hasSelection: !!selectedSubject, select, clearSelection, + // Reveals the subject without opening its popover, for lists + // that show the thread themselves. + revealOnly: !!(isSelected && selectedSubject.revealOnly), showNewForm: isSelected && selectedSubject.showNewForm, subjectRange: isSelected ? selectedSubject.subjectRange : undefined, highlightedThreadId: isSelected ? selectedSubject.highlightedThreadId ?? null : null}; diff --git a/entry_types/scrolled/package/src/review/Badge.js b/entry_types/scrolled/package/src/review/Badge.js index ac3d2bc04d..10dc843bcd 100644 --- a/entry_types/scrolled/package/src/review/Badge.js +++ b/entry_types/scrolled/package/src/review/Badge.js @@ -5,9 +5,9 @@ import CommentIcon from './images/comment.svg'; import styles from './Badge.module.css'; export const Badge = forwardRef(function Badge({ - counter, mode, resolved, unread, label, onClick + counter, hasThreads = counter > 0, mode, resolved, unread, label, onClick }, ref) { - const variant = resolveVariant(mode, counter > 0, unread); + const variant = resolveVariant(mode, hasThreads, unread); if (!variant) { return null; diff --git a/entry_types/scrolled/package/src/review/ThreadsBadge.js b/entry_types/scrolled/package/src/review/ThreadsBadge.js index 7e0613275f..f69dde3f55 100644 --- a/entry_types/scrolled/package/src/review/ThreadsBadge.js +++ b/entry_types/scrolled/package/src/review/ThreadsBadge.js @@ -14,6 +14,11 @@ export function ThreadsBadge({subjectType, subjectId, subjectRange, onClick, mod const unresolvedThreads = useLocatedCommentThreadsForSubject({subjectType, subjectId, subjectRange, resolution: 'unresolved'}); + // A thread revealed from the feed is a guest: it brings the badge back + // where the filter hides every thread of the subject, but the count + // stands for what the filter itself holds. + const counted = threads.filter(thread => thread.id !== revealedThreadId); + const unreadCommentCount = useUnreadCommentCount(threads); const handleClick = useCallback(() => { @@ -22,7 +27,8 @@ export function ThreadsBadge({subjectType, subjectId, subjectRange, onClick, mod const resolved = threads.length > 0 && unresolvedThreads.length === 0; - return 0} mode={mode} resolved={resolved} unread={unreadCommentCount > 0} From a6bcd0571286b74ace52ea3c08060c49266e4807 Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Thu, 27 Aug 2026 08:41:13 +0200 Subject: [PATCH 22/29] Expand the thread a reply was sent to A thread with no replies yet shows its reply form even where the list keeps every thread summarized - there is nothing folded away to hide it. Sending the reply gave the thread its first one, and with it the fold that swallowed what had just been written; the draft held the thread open only until the session dropped it. The list now takes the reply as the reviewer picking that thread, the same way clicking its count does. --- .../package/spec/review/ThreadList-spec.js | 39 +++++++++++++++++++ .../scrolled/package/src/review/ReplyForm.js | 4 +- .../scrolled/package/src/review/Thread.js | 5 ++- .../scrolled/package/src/review/ThreadList.js | 2 + 4 files changed, 47 insertions(+), 3 deletions(-) diff --git a/entry_types/scrolled/package/spec/review/ThreadList-spec.js b/entry_types/scrolled/package/spec/review/ThreadList-spec.js index 79b35fdfdb..1d69f1a612 100644 --- a/entry_types/scrolled/package/spec/review/ThreadList-spec.js +++ b/entry_types/scrolled/package/spec/review/ThreadList-spec.js @@ -844,6 +844,45 @@ describe('ThreadList', () => { postMessage.mockRestore(); }); + it('expands the thread that was replied to', async () => { + const user = userEvent.setup(); + + const {getByPlaceholderText, getByRole} = renderThreadList( + , + { + commentThreads: [ + {id: 1, subjectType: 'ContentElement', subjectId: 10, comments: [ + {id: 10, body: 'First topic', creatorName: 'Bob', creatorId: 2}, + {id: 11, body: 'A reply', creatorName: 'Alice', creatorId: 1}, + {id: 12, body: 'Another reply', creatorName: 'Bob', creatorId: 2} + ]}, + {id: 2, subjectType: 'ContentElement', subjectId: 10, comments: [ + {id: 20, body: 'Second topic', creatorName: 'Eve', creatorId: 3} + ]} + ] + } + ); + + await user.type(getByPlaceholderText('Reply...'), 'My reply'); + await user.click(getByRole('button', {name: 'Send'})); + + postDraftsChange({}); + postThreadChange({ + id: 2, + subjectType: 'ContentElement', + subjectId: 10, + comments: [ + {id: 20, body: 'Second topic', creatorName: 'Eve', creatorId: 3}, + {id: 21, body: 'My reply', creatorName: 'Alice', creatorId: 1} + ] + }); + + await waitFor(() => + expect(getByRole('button', {name: /1 reply/})) + .toHaveAttribute('aria-expanded', 'true') + ); + }); + it('includes the quote of the thread range in create comment message', async () => { const user = userEvent.setup(); const postMessage = jest.spyOn(window.top, 'postMessage').mockImplementation(() => {}); diff --git a/entry_types/scrolled/package/src/review/ReplyForm.js b/entry_types/scrolled/package/src/review/ReplyForm.js index 66ed67d3be..53a6898af2 100644 --- a/entry_types/scrolled/package/src/review/ReplyForm.js +++ b/entry_types/scrolled/package/src/review/ReplyForm.js @@ -10,7 +10,7 @@ import SendIcon from './images/send.svg'; import SpinnerIcon from './images/spinner.svg'; import styles from './ReplyForm.module.css'; -export function ReplyForm({threadId, subjectType, subjectId, subjectRange}) { +export function ReplyForm({threadId, subjectType, subjectId, subjectRange, onSubmit}) { const {t} = useI18n({locale: 'ui'}); const {body, setBody, submitting} = useDraftedBody({threadId}); @@ -41,6 +41,8 @@ export function ReplyForm({threadId, subjectType, subjectId, subjectRange}) { if (!hasText || submitting) return; createComment(body); + + if (onSubmit) onSubmit(); } return ( diff --git a/entry_types/scrolled/package/src/review/Thread.js b/entry_types/scrolled/package/src/review/Thread.js index 8f4756fa26..b9e1ea4f0b 100644 --- a/entry_types/scrolled/package/src/review/Thread.js +++ b/entry_types/scrolled/package/src/review/Thread.js @@ -19,7 +19,7 @@ import ResolveIcon from './images/resolve.svg'; import UnresolveIcon from './images/unresolve.svg'; import styles from './Thread.module.css'; -export function Thread({thread, collapsed: collapsedProp, visibleReplyCount, onExpandReplies, onToggle, onResolve, onClick, highlighted, showUnreadMarker, interactive = true}) { +export function Thread({thread, collapsed: collapsedProp, visibleReplyCount, onExpandReplies, onToggle, onReply, onResolve, onClick, highlighted, showUnreadMarker, interactive = true}) { const {t} = useI18n({locale: 'ui'}); const firstComment = thread.comments[0]; const replies = thread.comments.slice(1); @@ -159,7 +159,8 @@ export function Thread({thread, collapsed: collapsedProp, visibleReplyCount, onE } + subjectRange={thread.subjectRange} + onSubmit={onReply} />} {(thread.resolvedAt || (interactive && onResolve && !repliesCollapsed)) &&
diff --git a/entry_types/scrolled/package/src/review/ThreadList.js b/entry_types/scrolled/package/src/review/ThreadList.js index d482923a7e..a629cf1d7b 100644 --- a/entry_types/scrolled/package/src/review/ThreadList.js +++ b/entry_types/scrolled/package/src/review/ThreadList.js @@ -99,6 +99,7 @@ export function ThreadList({subjectType, subjectId, subjectRange, filter, highli collapsed={expandedThreadId !== thread.id} showUnreadMarker={activeThreads.length > 1} onToggle={() => toggleThread(thread.id)} + onReply={() => setExpandedThreadId(thread.id)} onResolve={() => postUpdateThreadMessage({threadId: thread.id, resolved: true})} onClick={onThreadClick && (() => onThreadClick(thread))} highlighted={isHighlighted(thread)} @@ -120,6 +121,7 @@ export function ThreadList({subjectType, subjectId, subjectRange, filter, highli collapsed={expandedThreadId !== thread.id} showUnreadMarker={resolvedThreads.length > 1} onToggle={() => toggleThread(thread.id)} + onReply={() => setExpandedThreadId(thread.id)} onResolve={() => postUpdateThreadMessage({threadId: thread.id, resolved: false})} onClick={onThreadClick && (() => onThreadClick(thread))} highlighted={isHighlighted(thread)} From 6bb9e5de328ea64ec839aa5ba0a6b5eb46cae147 Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Thu, 27 Aug 2026 09:01:48 +0200 Subject: [PATCH 23/29] Read a thread only where the reviewer opened it Being on screen used to be enough, and anything the thread kept out of sight - collapsed or folded replies - switched it off. Both halves were wrong at the ends of this stack. The feed folds away exactly the replies the reviewer has already seen, so a row it could never clear was hiding nothing; and a whole entry list scrolled past clears markers the reviewer navigates by, spending them on threads nobody looked at. A thread is read once everything unread in it has been on screen, and, in the lists that survey many threads at once, once the reviewer has picked it out. That the entry list wants its threads expanded before they count, and the feed does not, falls out of the first half: the fold hides nothing unread, a collapsed thread hides its replies. --- .../editor/views/EntryCommentsView-spec.js | 58 +++++++++++++++++++ .../features/commentActivity-spec.js | 10 ++++ .../package/spec/review/ActivityList-spec.js | 51 ++++++++++++++++ .../Thread/features/foldedReplies-spec.js | 16 ++++- .../Thread/features/markingRead-spec.js | 22 +++++++ .../src/editor/views/EntryCommentsView.js | 2 + .../src/frontend/commenting/ActivityButton.js | 5 +- .../commenting/SelectedSubjectProvider.js | 3 +- .../package/src/review/ActivityList.js | 1 + .../scrolled/package/src/review/Thread.js | 32 ++++++---- .../scrolled/package/src/review/ThreadList.js | 4 +- .../src/review/markThreadReadWhenSeen.js | 5 +- 12 files changed, 189 insertions(+), 20 deletions(-) diff --git a/entry_types/scrolled/package/spec/editor/views/EntryCommentsView-spec.js b/entry_types/scrolled/package/spec/editor/views/EntryCommentsView-spec.js index ae1ec42ed3..8819388239 100644 --- a/entry_types/scrolled/package/spec/editor/views/EntryCommentsView-spec.js +++ b/entry_types/scrolled/package/spec/editor/views/EntryCommentsView-spec.js @@ -10,6 +10,7 @@ import styles from 'editor/views/EntryCommentsView.module.css'; import {factories, useFakeTranslations, renderBackboneView} from 'pageflow/testHelpers'; import {useEditorGlobals} from 'support'; +import {simulateScrollingIntoView} from 'support/fakeIntersectionObserver'; describe('EntryCommentsView', () => { const {createEntry} = useEditorGlobals(); @@ -35,6 +36,63 @@ describe('EntryCommentsView', () => { 'pageflow_scrolled.review.reply_count.other': '%{count} replies' }); + describe('marking read', () => { + beforeEach(() => { + jest.useFakeTimers(); + }); + + afterEach(() => { + jest.useRealTimers(); + jest.restoreAllMocks(); + }); + + function renderList({highlightedThreadId} = {}) { + 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({ + currentUser: {id: 42, name: 'Alice'}, + commentThreads: [{ + id: 1, permaId: 5, subjectType: 'ContentElement', subjectId: 1000, + comments: [{id: 100, body: 'A comment', creatorId: 43, creatorName: 'Bob', + createdAt: '2026-08-17T11:00:00.000Z'}] + }] + }); + entry.set('highlightedThreadId', highlightedThreadId); + + const postMessage = jest.spyOn(window.top, 'postMessage').mockImplementation(() => {}); + const view = new EntryCommentsView({entry, editor}); + + return {...renderBackboneView(view), element: view.el, postMessage}; + } + + function markReadMessages(postMessage) { + return postMessage.mock.calls.filter(([message]) => message.type === 'MARK_THREADS_READ'); + } + + it('leaves a thread the reviewer only scrolled past unread', () => { + const {element, postMessage} = renderList(); + + simulateScrollingIntoView(element); + act(() => jest.advanceTimersByTime(1000)); + + expect(markReadMessages(postMessage)).toEqual([]); + }); + + it('marks a highlighted thread read', () => { + const {element, postMessage} = renderList({highlightedThreadId: 1}); + + simulateScrollingIntoView(element); + act(() => jest.advanceTimersByTime(1000)); + + expect(markReadMessages(postMessage)).toHaveLength(1); + }); + }); + it('keeps replies of a lone thread collapsed', () => { const entry = createEntry({ chapters: [ diff --git a/entry_types/scrolled/package/spec/frontend/commenting/features/commentActivity-spec.js b/entry_types/scrolled/package/spec/frontend/commenting/features/commentActivity-spec.js index e20166fcb3..102ffae934 100644 --- a/entry_types/scrolled/package/spec/frontend/commenting/features/commentActivity-spec.js +++ b/entry_types/scrolled/package/spec/frontend/commenting/features/commentActivity-spec.js @@ -125,6 +125,16 @@ describe('comment activity', () => { expect(window.HTMLElement.prototype.scrollIntoView).toHaveBeenCalled(); }); + it('highlights the entry that was clicked', () => { + const entry = renderEntryWithTwoThreads(); + + fireEvent.click(entry.getActivityButton()); + fireEvent.click(entry.getByText('First topic')); + + expect(entry.getByText('First topic').closest('[aria-current="true"]')) + .not.toBeNull(); + }); + // The row shows the thread already, so a popover would only say it // twice and cover the content just revealed. it('opens no popover for a clicked entry', () => { diff --git a/entry_types/scrolled/package/spec/review/ActivityList-spec.js b/entry_types/scrolled/package/spec/review/ActivityList-spec.js index 33b87a9acf..b374ed173c 100644 --- a/entry_types/scrolled/package/spec/review/ActivityList-spec.js +++ b/entry_types/scrolled/package/spec/review/ActivityList-spec.js @@ -4,9 +4,12 @@ import userEvent from '@testing-library/user-event'; import I18n from 'i18n-js'; import {useFakeTranslations} from 'pageflow/testHelpers'; +import {act} from '@testing-library/react'; + import {ActivityList} from 'review/ActivityList'; import styles from 'review/ActivityList.module.css'; import {renderWithReviewState} from 'support/renderWithReviewState'; +import {simulateScrollingIntoView} from 'support/fakeIntersectionObserver'; // ActivityList resolves its entries from the located threads, so the // subjects the threads hang off have to exist in the entry structure. @@ -557,4 +560,52 @@ describe('ActivityList', () => { expect(queryByRole('button', {name: 'Show more'})).toBeNull(); }); }); + describe('marking read', () => { + beforeEach(() => { + jest.useFakeTimers(); + }); + + afterEach(() => { + jest.useRealTimers(); + jest.restoreAllMocks(); + }); + + const unreadThread = thread({ + id: 1, + comments: [comment({id: 100, body: 'A topic', createdAt: '2026-08-17T09:00:00.000Z'})] + }); + + function renderWithPostMessage(ui) { + const postMessage = jest.spyOn(window.top, 'postMessage').mockImplementation(() => {}); + + return { + ...renderActivityList(ui, {commentThreads: [unreadThread]}), + postMessage + }; + } + + function markReadMessages(postMessage) { + return postMessage.mock.calls.filter(([message]) => message.type === 'MARK_THREADS_READ'); + } + + it('leaves a row the reviewer only scrolled past unread', () => { + const {container, postMessage} = renderWithPostMessage(); + + simulateScrollingIntoView(container); + act(() => jest.advanceTimersByTime(1000)); + + expect(markReadMessages(postMessage)).toEqual([]); + }); + + it('marks a highlighted row read', () => { + const {container, postMessage} = renderWithPostMessage( + + ); + + simulateScrollingIntoView(container); + act(() => jest.advanceTimersByTime(1000)); + + expect(markReadMessages(postMessage)).toHaveLength(1); + }); + }); }); diff --git a/entry_types/scrolled/package/spec/review/Thread/features/foldedReplies-spec.js b/entry_types/scrolled/package/spec/review/Thread/features/foldedReplies-spec.js index 48e5253fb6..b3ea84012d 100644 --- a/entry_types/scrolled/package/spec/review/Thread/features/foldedReplies-spec.js +++ b/entry_types/scrolled/package/spec/review/Thread/features/foldedReplies-spec.js @@ -179,10 +179,10 @@ describe('Thread folded replies', () => { jest.restoreAllMocks(); }); - function renderWithPostMessage(ui) { + function renderWithPostMessage(ui, options) { const postMessage = jest.spyOn(window.top, 'postMessage').mockImplementation(() => {}); - return {...render(ui), postMessage}; + return {...render(ui, options), postMessage}; } function markReadMessages(postMessage) { @@ -200,6 +200,18 @@ describe('Thread folded replies', () => { expect(markReadMessages(postMessage)).toEqual([]); }); + it('marks the thread read when the replies folded away have been read', () => { + const {container, postMessage} = renderWithPostMessage( + , + {commentThreadReads: {5: '2026-08-17T10:15:00.000Z'}} + ); + + simulateScrollingIntoView(container); + act(() => jest.advanceTimersByTime(1000)); + + expect(markReadMessages(postMessage)).toHaveLength(1); + }); + it('marks the thread read once nothing is folded away', () => { const {container, postMessage} = renderWithPostMessage( diff --git a/entry_types/scrolled/package/spec/review/Thread/features/markingRead-spec.js b/entry_types/scrolled/package/spec/review/Thread/features/markingRead-spec.js index 20c449f761..f2c37db3f2 100644 --- a/entry_types/scrolled/package/spec/review/Thread/features/markingRead-spec.js +++ b/entry_types/scrolled/package/spec/review/Thread/features/markingRead-spec.js @@ -132,6 +132,28 @@ describe('Thread marking read', () => { expect(markReadMessages(postMessage)).toHaveLength(1); }); + it('does not mark thread read while it is not the highlighted one', () => { + const {container, postMessage} = render( + + ); + + simulateScrollingIntoView(container); + passTime(1000); + + expect(markReadMessages(postMessage)).toEqual([]); + }); + + it('marks thread read once it is the highlighted one', () => { + const {container, postMessage} = render( + + ); + + simulateScrollingIntoView(container); + passTime(1000); + + expect(markReadMessages(postMessage)).toHaveLength(1); + }); + it('does not mark thread read again once all comments have been read', () => { const {container, postMessage} = render( , diff --git a/entry_types/scrolled/package/src/editor/views/EntryCommentsView.js b/entry_types/scrolled/package/src/editor/views/EntryCommentsView.js index c8228aed2b..f059da1b6c 100644 --- a/entry_types/scrolled/package/src/editor/views/EntryCommentsView.js +++ b/entry_types/scrolled/package/src/editor/views/EntryCommentsView.js @@ -153,6 +153,7 @@ function ContentElementGroup({ onThreadClick={onThreadClick} restrictInteractionsToHighlighted startCollapsed + markReadWhenHighlighted showNewForm={false} hideNewTopicButton />
@@ -178,6 +179,7 @@ function SectionGroup({section, selectedSubject, highlightedThreadId, onThreadCl onThreadClick={onThreadClick} restrictInteractionsToHighlighted startCollapsed + markReadWhenHighlighted showNewForm={false} hideNewTopicButton />
diff --git a/entry_types/scrolled/package/src/frontend/commenting/ActivityButton.js b/entry_types/scrolled/package/src/frontend/commenting/ActivityButton.js index 68f1bc854b..c560384d23 100644 --- a/entry_types/scrolled/package/src/frontend/commenting/ActivityButton.js +++ b/entry_types/scrolled/package/src/frontend/commenting/ActivityButton.js @@ -76,7 +76,7 @@ export function ActivityButton() { const ActivityPanel = React.forwardRef(function ActivityPanel({style, onClose}, ref) { const {t} = useI18n({locale: 'ui'}); - const {goToThread} = useCommentNavigation(); + const {goToThread, highlightedThreadId} = useCommentNavigation(); const panelRef = useRef(); @@ -119,7 +119,8 @@ const ActivityPanel = React.forwardRef(function ActivityPanel({style, onClose}, aria-label={t('pageflow_scrolled.review.activity.toggle')} data-comment-activity>
- goToThread(entry.threadId, + goToThread(entry.threadId, {revealOnly: true})} />
diff --git a/entry_types/scrolled/package/src/frontend/commenting/SelectedSubjectProvider.js b/entry_types/scrolled/package/src/frontend/commenting/SelectedSubjectProvider.js index 8eb3038868..6b47793df5 100644 --- a/entry_types/scrolled/package/src/frontend/commenting/SelectedSubjectProvider.js +++ b/entry_types/scrolled/package/src/frontend/commenting/SelectedSubjectProvider.js @@ -97,10 +97,11 @@ export function SelectedSubjectProvider({children}) { const navigation = useMemo(() => ({ count: targets.length, position, + highlightedThreadId: selectedSubject?.highlightedThreadId ?? null, goToNext: () => goTo(1), goToPrevious: () => goTo(-1), goToThread - }), [targets.length, position, goTo, goToThread]); + }), [targets.length, position, selectedSubject, goTo, goToThread]); return ( diff --git a/entry_types/scrolled/package/src/review/ActivityList.js b/entry_types/scrolled/package/src/review/ActivityList.js index 369818d45b..233a596a7e 100644 --- a/entry_types/scrolled/package/src/review/ActivityList.js +++ b/entry_types/scrolled/package/src/review/ActivityList.js @@ -73,6 +73,7 @@ function Entry({entry, day, highlighted, onClick}) { collapsed={collapsed} onToggle={() => setCollapsed(!collapsed)} showUnreadMarker + markReadWhenHighlighted onClick={onClick} highlighted={highlighted} onResolve={() => postUpdateThreadMessage({ diff --git a/entry_types/scrolled/package/src/review/Thread.js b/entry_types/scrolled/package/src/review/Thread.js index b9e1ea4f0b..5fe058142c 100644 --- a/entry_types/scrolled/package/src/review/Thread.js +++ b/entry_types/scrolled/package/src/review/Thread.js @@ -19,7 +19,7 @@ import ResolveIcon from './images/resolve.svg'; import UnresolveIcon from './images/unresolve.svg'; import styles from './Thread.module.css'; -export function Thread({thread, collapsed: collapsedProp, visibleReplyCount, onExpandReplies, onToggle, onReply, onResolve, onClick, highlighted, showUnreadMarker, interactive = true}) { +export function Thread({thread, collapsed: collapsedProp, visibleReplyCount, onExpandReplies, onToggle, onReply, onResolve, onClick, highlighted, showUnreadMarker, markReadWhenHighlighted, interactive = true}) { const {t} = useI18n({locale: 'ui'}); const firstComment = thread.comments[0]; const replies = thread.comments.slice(1); @@ -31,9 +31,7 @@ export function Thread({thread, collapsed: collapsedProp, visibleReplyCount, onE const repliesCollapsed = collapsed && replies.length > 0; - // A partial view is not a read thread: read state is one timestamp per - // thread, so marking it read would cover comments never shown. A - // collapsed thread has nothing folded - hiding both the fold and the + // A collapsed thread has nothing folded - hiding both the fold and the // count would leave no way back into it. const foldedReplyCount = visibleReplyCount === undefined || repliesCollapsed ? 0 : @@ -43,11 +41,12 @@ export function Thread({thread, collapsed: collapsedProp, visibleReplyCount, onE replies; const unreadComments = useUnreadComments(thread); - const unreadReplyCount = useMemo(() => { - const ids = new Set(unreadComments.map(comment => comment.id)); - return replies.filter(reply => ids.has(reply.id)).length; - }, [unreadComments, replies]); + const unreadIds = useMemo( + () => new Set(unreadComments.map(comment => comment.id)), + [unreadComments] + ); + const unreadReplyCount = replies.filter(reply => unreadIds.has(reply.id)).length; const hidesUnreadReplies = repliesCollapsed && unreadReplyCount > 0; // Where the unseen part of the thread starts. Only meaningful with @@ -58,9 +57,8 @@ export function Thread({thread, collapsed: collapsedProp, visibleReplyCount, onE return null; } - const ids = new Set(unreadComments.map(comment => comment.id)); - return replies.find(reply => ids.has(reply.id))?.id; - }, [unreadComments, replies, firstComment]); + return replies.find(reply => unreadIds.has(reply.id))?.id; + }, [unreadComments, unreadIds, replies, firstComment]); // Kept here rather than per comment so that a thread never shows two // textareas at once: neither two comments being edited, nor an edit next @@ -86,7 +84,17 @@ export function Thread({thread, collapsed: collapsedProp, visibleReplyCount, onE const ref = useRef(); const scrollHighlightedIntoView = useScrollHighlightedThreadIntoView(); - useMarkThreadReadWhenSeen({thread, ref, enabled: !repliesCollapsed && !foldedReplyCount}); + // Read state is one timestamp per thread, so a thread still keeping an + // unread comment out of sight would be marked read over comments never + // shown. Replies folded away for having been seen hide nothing. + const hiddenReplies = repliesCollapsed ? replies : replies.slice(0, foldedReplyCount); + const hidesUnread = hiddenReplies.some(reply => unreadIds.has(reply.id)); + + useMarkThreadReadWhenSeen({ + thread, + ref, + enabled: !hidesUnread && (highlighted || !markReadWhenHighlighted) + }); useEffect(() => { if (scrollHighlightedIntoView && highlighted && ref.current) { diff --git a/entry_types/scrolled/package/src/review/ThreadList.js b/entry_types/scrolled/package/src/review/ThreadList.js index a629cf1d7b..55e7d50f42 100644 --- a/entry_types/scrolled/package/src/review/ThreadList.js +++ b/entry_types/scrolled/package/src/review/ThreadList.js @@ -13,7 +13,7 @@ import ChevronIcon from './images/chevron.svg'; import NewTopicIcon from './images/newTopic.svg'; import styles from './ThreadList.module.css'; -export function ThreadList({subjectType, subjectId, subjectRange, filter, highlightedThreadId, onThreadClick, restrictInteractionsToHighlighted, showNewForm: showNewFormProp, hideNewTopicButton, reversed, expandResolved, startCollapsed}) { +export function ThreadList({subjectType, subjectId, subjectRange, filter, highlightedThreadId, onThreadClick, restrictInteractionsToHighlighted, showNewForm: showNewFormProp, hideNewTopicButton, reversed, expandResolved, startCollapsed, markReadWhenHighlighted}) { const {t} = useI18n({locale: 'ui'}); // Threads arrive already located: in display order, with orphans of @@ -100,6 +100,7 @@ export function ThreadList({subjectType, subjectId, subjectRange, filter, highli showUnreadMarker={activeThreads.length > 1} onToggle={() => toggleThread(thread.id)} onReply={() => setExpandedThreadId(thread.id)} + markReadWhenHighlighted={markReadWhenHighlighted} onResolve={() => postUpdateThreadMessage({threadId: thread.id, resolved: true})} onClick={onThreadClick && (() => onThreadClick(thread))} highlighted={isHighlighted(thread)} @@ -122,6 +123,7 @@ export function ThreadList({subjectType, subjectId, subjectRange, filter, highli showUnreadMarker={resolvedThreads.length > 1} onToggle={() => toggleThread(thread.id)} onReply={() => setExpandedThreadId(thread.id)} + markReadWhenHighlighted={markReadWhenHighlighted} onResolve={() => postUpdateThreadMessage({threadId: thread.id, resolved: false})} onClick={onThreadClick && (() => onThreadClick(thread))} highlighted={isHighlighted(thread)} diff --git a/entry_types/scrolled/package/src/review/markThreadReadWhenSeen.js b/entry_types/scrolled/package/src/review/markThreadReadWhenSeen.js index 93a7b0860b..727980fb2a 100644 --- a/entry_types/scrolled/package/src/review/markThreadReadWhenSeen.js +++ b/entry_types/scrolled/package/src/review/markThreadReadWhenSeen.js @@ -14,8 +14,9 @@ const ROOT_MARGIN = '-10% 0px -10% 0px'; // A thread counts as read once it has been on screen long enough to // actually read it. Scrolling past it therefore leaves it unread. // -// Callers pass `enabled: false` while the thread hides part of itself, -// so collapsed replies do not get marked as read unseen. +// Callers pass `enabled: false` while the thread keeps an unread comment +// out of sight, and in lists that survey many threads at once, where +// being on screen is not the reviewer choosing to read one. export function useMarkThreadReadWhenSeen({thread, ref, enabled}) { const unreadComments = useLiveUnreadComments(thread); const markThreadRead = useMarkThreadRead(); From 8ef37b5a9b9d818b5f2517b18fe3c072bd98ad07 Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Thu, 27 Aug 2026 10:44:58 +0200 Subject: [PATCH 24/29] Outline a thread nobody has read yet The dot in the corner locates one unread thread among several, which is what it was for, but it takes finding. A thread whose topic has not been read is new all through, so the whole card can say so: its border takes a wash of the dot's colour, and the folded seam with it. A thread that only gained replies keeps its border - what is new there is the count and the divider, not the conversation. The warning colour gains the lighter and lightest variants every other colour in the palette already has. The outline outranks the hover and highlight borders. What is left to read is worth more than where the reviewer is, which the highlight halo goes on showing either way. --- .../stylesheets/pageflow/ui/properties.scss | 2 + .../Thread/features/unreadMarkers-spec.js | 39 +++++++++++++++++++ .../scrolled/package/src/review/Thread.js | 2 + .../package/src/review/Thread.module.css | 8 ++++ 4 files changed, 51 insertions(+) diff --git a/app/assets/stylesheets/pageflow/ui/properties.scss b/app/assets/stylesheets/pageflow/ui/properties.scss index 6b32f6fad3..9f791cf364 100644 --- a/app/assets/stylesheets/pageflow/ui/properties.scss +++ b/app/assets/stylesheets/pageflow/ui/properties.scss @@ -42,6 +42,8 @@ --ui-selection-color-lightest: hsla(197, 69%, 76%, 0.1); --ui-warning-color: #ff7400; + --ui-warning-color-lighter: hsla(27, 100%, 50%, 0.3); + --ui-warning-color-lightest: hsla(27, 100%, 50%, 0.1); --ui-error-color: #ff4d6d; --ui-error-color-light: hsla(349, 100%, 65%, 0.6); diff --git a/entry_types/scrolled/package/spec/review/Thread/features/unreadMarkers-spec.js b/entry_types/scrolled/package/spec/review/Thread/features/unreadMarkers-spec.js index 54bc926ecb..54ff4dc1b5 100644 --- a/entry_types/scrolled/package/spec/review/Thread/features/unreadMarkers-spec.js +++ b/entry_types/scrolled/package/spec/review/Thread/features/unreadMarkers-spec.js @@ -93,6 +93,45 @@ describe('Thread unread markers', () => { }); }); + describe('outline on the thread', () => { + function outlined(container) { + return container.querySelector(`.${styles.unreadTopic}`); + } + + it('outlines a thread whose topic is unseen', () => { + const {container} = render(); + + expect(outlined(container)).not.toBeNull(); + }); + + it('leaves a thread whose replies are unseen unoutlined', () => { + const {container} = render( + , + { + commentThreads: [threadWithReplies], + commentThreadReads: {5: '2026-08-17T10:00:00.000Z'} + } + ); + + expect(outlined(container)).toBeNull(); + }); + + it('leaves a thread without unseen comments unoutlined', () => { + const {container} = render( + , + {commentThreadReads: {5: '2026-08-17T12:00:00.000Z'}} + ); + + expect(outlined(container)).toBeNull(); + }); + + it('leaves a thread shown on its own unoutlined', () => { + const {container} = render(); + + expect(outlined(container)).toBeNull(); + }); + }); + describe('new reply count', () => { it('counts unseen replies hidden by collapsing', () => { const {getByText} = render( diff --git a/entry_types/scrolled/package/src/review/Thread.js b/entry_types/scrolled/package/src/review/Thread.js index 5fe058142c..754ade952f 100644 --- a/entry_types/scrolled/package/src/review/Thread.js +++ b/entry_types/scrolled/package/src/review/Thread.js @@ -48,6 +48,7 @@ export function Thread({thread, collapsed: collapsedProp, visibleReplyCount, onE const unreadReplyCount = replies.filter(reply => unreadIds.has(reply.id)).length; const hidesUnreadReplies = repliesCollapsed && unreadReplyCount > 0; + const unreadTopic = !!firstComment && unreadIds.has(firstComment.id); // Where the unseen part of the thread starts. Only meaningful with // seen comments above it: a thread that is new all through says so @@ -106,6 +107,7 @@ export function Thread({thread, collapsed: collapsedProp, visibleReplyCount, onE
Date: Thu, 27 Aug 2026 11:08:20 +0200 Subject: [PATCH 25/29] Show unseen comments on badges beside commented text The badges in the text column build their own, since they resolve a thread from a Slate range rather than from a subject, and so they never passed the badge an unread count. The dot stayed off, and worse, the escape that keeps a badge with unseen comments from shrinking to a dot never fired: unread text comments were the least visible ones in the editor. Badge takes the count now instead of a flag and the sentence naming it, which both callers were otherwise left to phrase alike. --- .../features/commentBadges-spec.js | 38 +++++++++++++++++++ .../spec/support/pageObjects/inlineEditing.js | 1 + .../inlineEditing/EditableText/BadgeColumn.js | 13 ++++++- .../scrolled/package/src/review/Badge.js | 11 +++++- .../package/src/review/ThreadsBadge.js | 9 +---- 5 files changed, 60 insertions(+), 12 deletions(-) diff --git a/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/features/commentBadges-spec.js b/entry_types/scrolled/package/spec/frontend/inlineEditing/EditableText/features/commentBadges-spec.js index 1de284f329..695db8c9db 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 @@ -45,6 +45,44 @@ describe('inline editing EditableText comment badges', () => { expect(badges[0].isInDotMode()).toBe(true); }); + function renderEntryWithUnreadThread() { + const value = [{type: 'paragraph', children: [{text: 'Some text to comment on'}]}]; + + return renderEntry({ + contentElement: { + ui: , + typeOptions: {inlineComments: true, customSelectionRect: true} + }, + commenting: { + currentUser: {id: 42, name: 'Alice'}, + commentThreads: [{ + id: 5, + permaId: 5, + subjectType: 'ContentElement', + subjectId: 10, + subjectRange: {anchor: {path: [0, 0], offset: 5}, focus: {path: [0, 0], offset: 9}}, + comments: [{ + id: 1, body: 'A comment', creatorName: 'Bob', creatorId: 43, + createdAt: '2026-08-17T11:00:00.000Z' + }] + }], + commentThreadReads: {} + } + }); + } + + it('marks a badge whose thread has unseen comments', () => { + const entry = renderEntryWithUnreadThread(); + + expect(entry.queryAllCommentBadges()[0].isUnread()).toBe(true); + }); + + it('keeps a badge with unseen comments out of dot mode', () => { + const entry = renderEntryWithUnreadThread(); + + expect(entry.queryAllCommentBadges()[0].isInDotMode()).toBe(false); + }); + it('renders only the highlighted thread badge in active mode', () => { const value = [ {type: 'paragraph', children: [{text: 'First paragraph thread here'}]}, diff --git a/entry_types/scrolled/package/spec/support/pageObjects/inlineEditing.js b/entry_types/scrolled/package/spec/support/pageObjects/inlineEditing.js index 5829320f4b..4bfef69af8 100644 --- a/entry_types/scrolled/package/spec/support/pageObjects/inlineEditing.js +++ b/entry_types/scrolled/package/spec/support/pageObjects/inlineEditing.js @@ -148,6 +148,7 @@ function createCommentBadgePageObject(el) { isInDotMode: () => el.classList.contains(badgeStyles.dot), isActive: () => el.classList.contains(badgeStyles.active), isResolved: () => el.classList.contains(badgeStyles.resolved), + isUnread: () => el.classList.contains(badgeStyles.unread), 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 cf4d27edb8..adde0ac889 100644 --- a/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/BadgeColumn.js +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/BadgeColumn.js @@ -1,14 +1,16 @@ -import React, {useCallback} from 'react'; +import React, {useCallback, useMemo} from 'react'; import {Range, Transforms} from 'slate'; import {useSlate, ReactEditor} from 'slate-react'; -import {Badge, useAnchoredFloating} from 'pageflow-scrolled/review'; +import {Badge, useAnchoredFloating, useUnreadCommentCount} from 'pageflow-scrolled/review'; import {useContentElementCommentSelection} from '../useCommentSelection'; import {highlightOverlapsSelection} from './highlightOverlapsSelection'; import styles from './BadgeColumn.module.css'; +const noThreads = []; + export function BadgeColumn({highlights, anchors}) { const editor = useSlate(); const {highlightedThreadId} = useContentElementCommentSelection(); @@ -52,6 +54,12 @@ function PositionedBadge({editor, highlight, overlapSelection, anchors}) { const {refs, floatingStyles, hasAnchor} = useAnchoredFloating(highlight.key, anchors, {placement: 'left-start'}); + const threads = useMemo( + () => (highlight.thread ? [highlight.thread] : noThreads), + [highlight.thread] + ); + const unreadCommentCount = useUnreadCommentCount(threads); + const handleClick = useCallback(() => { if (highlight.key === 'selection') { selectComments(); @@ -86,6 +94,7 @@ function PositionedBadge({editor, highlight, overlapSelection, anchors}) {
); diff --git a/entry_types/scrolled/package/src/review/Badge.js b/entry_types/scrolled/package/src/review/Badge.js index 10dc843bcd..8b7a58c956 100644 --- a/entry_types/scrolled/package/src/review/Badge.js +++ b/entry_types/scrolled/package/src/review/Badge.js @@ -1,12 +1,16 @@ import React, {forwardRef} from 'react'; import classNames from 'classnames'; +import {useI18n} from 'pageflow-scrolled/frontend'; import CommentIcon from './images/comment.svg'; import styles from './Badge.module.css'; export const Badge = forwardRef(function Badge({ - counter, hasThreads = counter > 0, mode, resolved, unread, label, onClick + counter, hasThreads = counter > 0, mode, resolved, unreadCount = 0, onClick }, ref) { + const {t} = useI18n({locale: 'ui'}); + + const unread = unreadCount > 0; const variant = resolveVariant(mode, hasThreads, unread); if (!variant) { @@ -16,7 +20,10 @@ export const Badge = forwardRef(function Badge({ return ( {/* Out of the toolbar, whose view transition would snapshot the diff --git a/entry_types/scrolled/package/src/frontend/commenting/FloatingToolbar.js b/entry_types/scrolled/package/src/frontend/commenting/FloatingToolbar.js index 421730b4ca..875332b447 100644 --- a/entry_types/scrolled/package/src/frontend/commenting/FloatingToolbar.js +++ b/entry_types/scrolled/package/src/frontend/commenting/FloatingToolbar.js @@ -1,7 +1,7 @@ import React, {useEffect} from 'react'; import classNames from 'classnames'; -import {useLocatedCommentThreads, useUnseenActivityCount} from 'pageflow-scrolled/review'; +import {useLocatedCommentThreads, useUnreadThreadCount} from 'pageflow-scrolled/review'; import {useI18n} from '../i18n'; import {useAddCommentMode} from './AddCommentModeProvider'; import {useCommentDisplayFilter} from './CommentDisplayFilterProvider'; @@ -65,14 +65,14 @@ function ShowCommentsButton() { const {t} = useI18n({locale: 'ui'}); const {toggle} = useCommentingVisibility(); - const unseenCount = useUnseenActivityCount(); - const unread = unseenCount > 0; + const unreadCount = useUnreadThreadCount(); + const unread = unreadCount > 0; // Named rather than only marked, since the dot is the sole cue that // comments are waiting behind the collapsed toolbar. const label = unread ? t('pageflow_scrolled.review.show_comments_with_unread', - {count: unseenCount}) : + {count: unreadCount}) : t('pageflow_scrolled.review.show_comments'); return ( 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 adde0ac889..b50d768311 100644 --- a/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/BadgeColumn.js +++ b/entry_types/scrolled/package/src/frontend/inlineEditing/EditableText/BadgeColumn.js @@ -3,7 +3,7 @@ import React, {useCallback, useMemo} from 'react'; import {Range, Transforms} from 'slate'; import {useSlate, ReactEditor} from 'slate-react'; -import {Badge, useAnchoredFloating, useUnreadCommentCount} from 'pageflow-scrolled/review'; +import {Badge, useAnchoredFloating, useUnreadActivityCount} from 'pageflow-scrolled/review'; import {useContentElementCommentSelection} from '../useCommentSelection'; import {highlightOverlapsSelection} from './highlightOverlapsSelection'; @@ -58,7 +58,7 @@ function PositionedBadge({editor, highlight, overlapSelection, anchors}) { () => (highlight.thread ? [highlight.thread] : noThreads), [highlight.thread] ); - const unreadCommentCount = useUnreadCommentCount(threads); + const unreadCount = useUnreadActivityCount(threads); const handleClick = useCallback(() => { if (highlight.key === 'selection') { @@ -94,7 +94,7 @@ function PositionedBadge({editor, highlight, overlapSelection, anchors}) { ); diff --git a/entry_types/scrolled/package/src/review/ActivityList.js b/entry_types/scrolled/package/src/review/ActivityList.js index 233a596a7e..113b67f83f 100644 --- a/entry_types/scrolled/package/src/review/ActivityList.js +++ b/entry_types/scrolled/package/src/review/ActivityList.js @@ -122,11 +122,11 @@ function joinParts(t, parts) { // unseen from before it - folding that away would hide what the feed // exists to surface. Falls back to the latest reply, so that a row never // shows a thread without the comment it is listed for. -function visibleReplyCount({thread, unseenCommentIds}, day) { +function visibleReplyCount({thread, unreadCommentIds}, day) { const replies = thread.comments.slice(1); const starts = [ - replies.findIndex(reply => unseenCommentIds.includes(reply.id)), + replies.findIndex(reply => unreadCommentIds.includes(reply.id)), replies.findIndex(reply => dayOf(reply.createdAt) === day) ].filter(index => index >= 0); diff --git a/entry_types/scrolled/package/src/review/Badge.js b/entry_types/scrolled/package/src/review/Badge.js index 8b7a58c956..5e478bdfec 100644 --- a/entry_types/scrolled/package/src/review/Badge.js +++ b/entry_types/scrolled/package/src/review/Badge.js @@ -21,7 +21,7 @@ export const Badge = forwardRef(function Badge({