From ea3bc6e9331b384bc0ac79a0edeef5c825ebb956 Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Mon, 17 Aug 2026 15:40:22 +0200 Subject: [PATCH 01/18] Add comment thread read model Records per user and entry when a comment thread was last read. Keyed by comment thread perma id rather than record id so read state survives comment threads being copied to a new revision. --- app/models/pageflow/comment_thread_read.rb | 18 ++++ app/models/pageflow/entry.rb | 2 + ...60817000000_create_comment_thread_reads.rb | 16 +++ lib/pageflow/user_mixin.rb | 4 + spec/factories/comment_thread_reads.rb | 10 ++ .../pageflow/comment_thread_read_spec.rb | 98 +++++++++++++++++++ 6 files changed, 148 insertions(+) create mode 100644 app/models/pageflow/comment_thread_read.rb create mode 100644 db/migrate/20260817000000_create_comment_thread_reads.rb create mode 100644 spec/factories/comment_thread_reads.rb create mode 100644 spec/models/pageflow/comment_thread_read_spec.rb diff --git a/app/models/pageflow/comment_thread_read.rb b/app/models/pageflow/comment_thread_read.rb new file mode 100644 index 0000000000..e24d8161e6 --- /dev/null +++ b/app/models/pageflow/comment_thread_read.rb @@ -0,0 +1,18 @@ +module Pageflow + # Records when a user last read a comment thread. Keyed by perma id + # so read state survives comment threads being copied to a new + # revision. + # + # @api private + class CommentThreadRead < ApplicationRecord + belongs_to :entry + belongs_to :user + + def self.mark(entry:, user:, comment_thread_perma_ids:, read_at: Time.current) + comment_thread_perma_ids.each do |perma_id| + find_or_initialize_by(entry:, user:, comment_thread_perma_id: perma_id) + .update!(read_at:) + end + end + end +end diff --git a/app/models/pageflow/entry.rb b/app/models/pageflow/entry.rb index 551146ed1b..b7e7a3f28a 100644 --- a/app/models/pageflow/entry.rb +++ b/app/models/pageflow/entry.rb @@ -33,6 +33,8 @@ class PasswordMissingError < StandardError has_many :imports, class_name: 'Pageflow::FileImport', dependent: :destroy + has_many :comment_thread_reads, dependent: :destroy + has_one :draft, -> { editable }, class_name: 'Revision', inverse_of: :entry has_one :published_revision, -> { published }, class_name: 'Revision', inverse_of: :entry diff --git a/db/migrate/20260817000000_create_comment_thread_reads.rb b/db/migrate/20260817000000_create_comment_thread_reads.rb new file mode 100644 index 0000000000..6acb6b79e5 --- /dev/null +++ b/db/migrate/20260817000000_create_comment_thread_reads.rb @@ -0,0 +1,16 @@ +class CreateCommentThreadReads < ActiveRecord::Migration[6.0] + def change + create_table :pageflow_comment_thread_reads do |t| + t.integer :entry_id, null: false + t.integer :user_id, null: false + t.integer :comment_thread_perma_id, null: false + t.datetime :read_at, null: false + end + + add_index :pageflow_comment_thread_reads, + [:user_id, :entry_id, :comment_thread_perma_id], + unique: true, + name: 'index_comment_thread_reads_on_user_and_entry_and_thread' + add_index :pageflow_comment_thread_reads, :entry_id + end +end diff --git a/lib/pageflow/user_mixin.rb b/lib/pageflow/user_mixin.rb index 5ea782ad46..f1629698c3 100644 --- a/lib/pageflow/user_mixin.rb +++ b/lib/pageflow/user_mixin.rb @@ -23,6 +23,10 @@ module UserMixin has_many :revisions, class_name: 'Pageflow::Revision', foreign_key: :creator_id + has_many :comment_thread_reads, + dependent: :destroy, + class_name: 'Pageflow::CommentThreadRead' + validates :first_name, :last_name, presence: true validates_inclusion_of :locale, in: Pageflow.config.available_locales.map(&:to_s) diff --git a/spec/factories/comment_thread_reads.rb b/spec/factories/comment_thread_reads.rb new file mode 100644 index 0000000000..5993a29c44 --- /dev/null +++ b/spec/factories/comment_thread_reads.rb @@ -0,0 +1,10 @@ +module Pageflow + FactoryBot.define do + factory :comment_thread_read, class: CommentThreadRead do + entry + user + comment_thread_perma_id { 1 } + read_at { Time.current } + end + end +end diff --git a/spec/models/pageflow/comment_thread_read_spec.rb b/spec/models/pageflow/comment_thread_read_spec.rb new file mode 100644 index 0000000000..a85ae55c2d --- /dev/null +++ b/spec/models/pageflow/comment_thread_read_spec.rb @@ -0,0 +1,98 @@ +require 'spec_helper' + +module Pageflow + describe CommentThreadRead do + describe '.mark' do + it 'creates records for the given comment thread perma ids' do + entry = create(:entry) + user = create(:user) + + CommentThreadRead.mark(entry:, user:, comment_thread_perma_ids: [5, 6]) + + expect(CommentThreadRead.where(entry:, user:).pluck(:comment_thread_perma_id)) + .to contain_exactly(5, 6) + end + + it 'records the given read at timestamp' do + entry = create(:entry) + user = create(:user) + read_at = 2.hours.ago + + CommentThreadRead.mark(entry:, user:, comment_thread_perma_ids: [5], read_at:) + + expect(CommentThreadRead.last.read_at).to eq(read_at) + end + + it 'moves read at timestamp of existing record forward' do + read = create(:comment_thread_read, + comment_thread_perma_id: 5, + read_at: 2.hours.ago) + + Timecop.freeze(1.hour.from_now) do + CommentThreadRead.mark(entry: read.entry, + user: read.user, + comment_thread_perma_ids: [5]) + + expect(read.reload.read_at).to eq(Time.current) + end + + expect(CommentThreadRead.count).to eq(1) + end + + it 'keeps records of other users separate' do + entry = create(:entry) + other_read = create(:comment_thread_read, + entry:, + comment_thread_perma_id: 5, + read_at: 2.hours.ago) + + CommentThreadRead.mark(entry:, + user: create(:user), + comment_thread_perma_ids: [5]) + + expect(other_read.reload.read_at).to eq(2.hours.ago) + expect(CommentThreadRead.count).to eq(2) + end + + it 'keeps records of other entries separate' do + user = create(:user) + other_read = create(:comment_thread_read, + user:, + comment_thread_perma_id: 5, + read_at: 2.hours.ago) + + CommentThreadRead.mark(entry: create(:entry), + user:, + comment_thread_perma_ids: [5]) + + expect(other_read.reload.read_at).to eq(2.hours.ago) + expect(CommentThreadRead.count).to eq(2) + end + + it 'does nothing for blank list of perma ids' do + entry = create(:entry) + user = create(:user) + + CommentThreadRead.mark(entry:, user:, comment_thread_perma_ids: []) + + expect(CommentThreadRead.count).to eq(0) + end + end + + it 'is destroyed together with entry' do + read = create(:comment_thread_read) + + read.entry.destroy + + expect(CommentThreadRead.count).to eq(0) + end + + it 'is destroyed together with user' do + read = create(:comment_thread_read) + + read.user.destroy + + expect(CommentThreadRead.count).to eq(0) + end + end +end From 8803fadfc8349643ecfe70888f26be8fe3393dd3 Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Mon, 17 Aug 2026 15:40:27 +0200 Subject: [PATCH 02/18] Add endpoint to mark comment threads as read Accepts a batch of comment thread perma ids so the client can coalesce read marks into a single request. Perma ids that do not belong to the entry are ignored. --- .../review/comment_thread_reads_controller.rb | 30 ++++++ config/routes.rb | 2 + .../comment_thread_reads_controller_spec.rb | 94 +++++++++++++++++++ 3 files changed, 126 insertions(+) create mode 100644 app/controllers/pageflow/review/comment_thread_reads_controller.rb create mode 100644 spec/controllers/pageflow/review/comment_thread_reads_controller_spec.rb diff --git a/app/controllers/pageflow/review/comment_thread_reads_controller.rb b/app/controllers/pageflow/review/comment_thread_reads_controller.rb new file mode 100644 index 0000000000..2c0c29dd99 --- /dev/null +++ b/app/controllers/pageflow/review/comment_thread_reads_controller.rb @@ -0,0 +1,30 @@ +module Pageflow + module Review + # @api private + class CommentThreadReadsController < Pageflow::ApplicationController + respond_to :json + before_action :authenticate_user! + + def create + entry = DraftEntry.find(params[:entry_id]) + authorize!(:read, entry.to_model) + + CommentThreadRead.mark(entry: entry.to_model, + user: current_user, + comment_thread_perma_ids: known_perma_ids(entry)) + + head :no_content + end + + private + + # Guards against read records piling up for comment threads that + # do not exist in the entry. + def known_perma_ids(entry) + entry.comment_threads + .where(perma_id: params.fetch(:comment_thread_perma_ids, [])) + .pluck(:perma_id) + end + end + end +end diff --git a/config/routes.rb b/config/routes.rb index 9215836b88..a4a4e998e6 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -84,6 +84,8 @@ resources :comment_threads, only: [:index, :create, :update] do resources :comments, only: [:create, :update] end + + resources :comment_thread_reads, only: [:create] end end diff --git a/spec/controllers/pageflow/review/comment_thread_reads_controller_spec.rb b/spec/controllers/pageflow/review/comment_thread_reads_controller_spec.rb new file mode 100644 index 0000000000..62e046f0a4 --- /dev/null +++ b/spec/controllers/pageflow/review/comment_thread_reads_controller_spec.rb @@ -0,0 +1,94 @@ +require 'spec_helper' + +module Pageflow + describe Review::CommentThreadReadsController do + routes { Engine.routes } + + describe '#create' do + it 'marks comment threads as read for current user' do + user = create(:user) + entry = create(:entry, with_previewer: user) + thread = create(:comment_thread, revision: entry.draft) + + sign_in(user, scope: :user) + post(:create, params: { + entry_id: entry.id, + comment_thread_perma_ids: [thread.perma_id] + }, format: 'json') + + expect(response.status).to eq(204) + expect(CommentThreadRead.where(entry:, user:).pluck(:comment_thread_perma_id)) + .to eq([thread.perma_id]) + end + + it 'moves read at timestamp of already read thread forward' do + user = create(:user) + entry = create(:entry, with_previewer: user) + thread = create(:comment_thread, revision: entry.draft) + read = create(:comment_thread_read, + entry:, + user:, + comment_thread_perma_id: thread.perma_id, + read_at: 2.hours.ago) + + sign_in(user, scope: :user) + + Timecop.freeze(1.hour.from_now) do + post(:create, params: { + entry_id: entry.id, + comment_thread_perma_ids: [thread.perma_id] + }, format: 'json') + + expect(read.reload.read_at).to eq(Time.current) + end + + expect(CommentThreadRead.count).to eq(1) + end + + it 'ignores perma ids of threads not belonging to entry' do + user = create(:user) + entry = create(:entry, with_previewer: user) + other_entry = create(:entry, with_previewer: user) + other_thread = create(:comment_thread, revision: other_entry.draft) + + sign_in(user, scope: :user) + post(:create, params: { + entry_id: entry.id, + comment_thread_perma_ids: [other_thread.perma_id] + }, format: 'json') + + expect(response.status).to eq(204) + expect(CommentThreadRead.count).to eq(0) + end + + it 'ignores blank list of perma ids' do + user = create(:user) + entry = create(:entry, with_previewer: user) + + sign_in(user, scope: :user) + post(:create, params: {entry_id: entry.id}, format: 'json') + + expect(response.status).to eq(204) + expect(CommentThreadRead.count).to eq(0) + end + + it 'requires user to be signed in' do + entry = create(:entry) + + post(:create, params: {entry_id: entry.id}, format: 'json') + + expect(response.status).to eq(401) + end + + it 'requires read permission on entry' do + user = create(:user) + entry = create(:entry) + + sign_in(user, scope: :user) + post(:create, params: {entry_id: entry.id}, format: 'json') + + expect(response.status).to eq(403) + end + end + end +end From 1e6b164c54e8ebdf75dc955d94b0c105c97d6d15 Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Mon, 17 Aug 2026 15:43:38 +0200 Subject: [PATCH 03/18] Expose comment thread read timestamps in review index Delivered as a separate map keyed by thread perma id instead of a field on each thread, so responses that render a single thread cannot clobber read state in the client. --- .../review/comment_threads_controller.rb | 2 ++ app/models/pageflow/comment_thread_read.rb | 4 +++ .../comment_threads/index.json.jbuilder | 2 ++ .../review/comment_threads_controller_spec.rb | 34 +++++++++++++++++++ .../pageflow/comment_thread_read_spec.rb | 21 ++++++++++++ 5 files changed, 63 insertions(+) diff --git a/app/controllers/pageflow/review/comment_threads_controller.rb b/app/controllers/pageflow/review/comment_threads_controller.rb index b579f968ee..59e0d62f2f 100644 --- a/app/controllers/pageflow/review/comment_threads_controller.rb +++ b/app/controllers/pageflow/review/comment_threads_controller.rb @@ -10,6 +10,8 @@ def index authorize!(:read, entry.to_model) @comment_threads = entry.comment_threads.includes(comments: :creator) + @read_at_by_perma_id = + CommentThreadRead.read_at_by_perma_id(entry: entry.to_model, user: current_user) end def create diff --git a/app/models/pageflow/comment_thread_read.rb b/app/models/pageflow/comment_thread_read.rb index e24d8161e6..a414fbf4ba 100644 --- a/app/models/pageflow/comment_thread_read.rb +++ b/app/models/pageflow/comment_thread_read.rb @@ -8,6 +8,10 @@ class CommentThreadRead < ApplicationRecord belongs_to :entry belongs_to :user + def self.read_at_by_perma_id(entry:, user:) + where(entry:, user:).pluck(:comment_thread_perma_id, :read_at).to_h + end + def self.mark(entry:, user:, comment_thread_perma_ids:, read_at: Time.current) comment_thread_perma_ids.each do |perma_id| find_or_initialize_by(entry:, user:, comment_thread_perma_id: perma_id) diff --git a/app/views/pageflow/review/comment_threads/index.json.jbuilder b/app/views/pageflow/review/comment_threads/index.json.jbuilder index bdd65bcfe4..5e948c26c8 100644 --- a/app/views/pageflow/review/comment_threads/index.json.jbuilder +++ b/app/views/pageflow/review/comment_threads/index.json.jbuilder @@ -8,3 +8,5 @@ end json.comment_threads(@comment_threads) do |comment_thread| json.partial!('pageflow/review/comment_threads/comment_thread', comment_thread:) end + +json.comment_thread_reads(@read_at_by_perma_id.transform_keys(&:to_s)) diff --git a/spec/controllers/pageflow/review/comment_threads_controller_spec.rb b/spec/controllers/pageflow/review/comment_threads_controller_spec.rb index 416a4ea2cf..dded8b67b7 100644 --- a/spec/controllers/pageflow/review/comment_threads_controller_spec.rb +++ b/spec/controllers/pageflow/review/comment_threads_controller_spec.rb @@ -113,6 +113,40 @@ module Pageflow end end + it 'includes read timestamps of current user by thread perma id' do + user = create(:user) + entry = create(:entry, with_previewer: user) + thread = create(:comment_thread, revision: entry.draft, creator: user) + read_at = 2.hours.ago + create(:comment_thread_read, + entry:, + user:, + comment_thread_perma_id: thread.perma_id, + read_at:) + + sign_in(user, scope: :user) + get(:index, params: {entry_id: entry.id}, format: 'json') + + reads = JSON.parse(response.body)['commentThreadReads'] + + expect(Time.zone.parse(reads[thread.perma_id.to_s])).to eq(read_at) + end + + it 'does not include read timestamps of other users' do + user = create(:user) + entry = create(:entry, with_previewer: user) + thread = create(:comment_thread, revision: entry.draft, creator: user) + create(:comment_thread_read, + entry:, + user: create(:user), + comment_thread_perma_id: thread.perma_id) + + sign_in(user, scope: :user) + get(:index, params: {entry_id: entry.id}, format: 'json') + + expect(JSON.parse(response.body)['commentThreadReads']).to eq({}) + end + it 'requires user to be signed in' do entry = create(:entry) diff --git a/spec/models/pageflow/comment_thread_read_spec.rb b/spec/models/pageflow/comment_thread_read_spec.rb index a85ae55c2d..7ebf746cc5 100644 --- a/spec/models/pageflow/comment_thread_read_spec.rb +++ b/spec/models/pageflow/comment_thread_read_spec.rb @@ -79,6 +79,27 @@ module Pageflow end end + describe '.read_at_by_perma_id' do + it 'returns read timestamps of user in entry keyed by thread perma id' do + read = create(:comment_thread_read, comment_thread_perma_id: 5) + + result = CommentThreadRead.read_at_by_perma_id(entry: read.entry, user: read.user) + + expect(result.keys).to eq([5]) + expect(result[5]).to eq(read.read_at) + end + + it 'ignores records of other users and entries' do + read = create(:comment_thread_read, comment_thread_perma_id: 5) + create(:comment_thread_read, entry: read.entry, comment_thread_perma_id: 6) + create(:comment_thread_read, user: read.user, comment_thread_perma_id: 7) + + result = CommentThreadRead.read_at_by_perma_id(entry: read.entry, user: read.user) + + expect(result.keys).to eq([5]) + end + end + it 'is destroyed together with entry' do read = create(:comment_thread_read) From 6fc1d7d464fd5ad902e9eef39ce7062905868d10 Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Mon, 17 Aug 2026 15:46:15 +0200 Subject: [PATCH 04/18] Track comment thread read marks in review session Read marks are collected and sent as a single debounced request instead of one request per thread. Failed requests keep their perma ids pending so the next flush retries them. --- package/spec/review/ReviewSession-spec.js | 142 +++++++++++++++++++++- package/spec/review/index-spec.js | 32 +++++ package/spec/review/request-spec.js | 12 ++ package/src/review/ReviewSession.js | 55 ++++++++- package/src/review/index.js | 8 +- package/src/review/request.js | 4 + 6 files changed, 249 insertions(+), 4 deletions(-) create mode 100644 package/spec/review/index-spec.js diff --git a/package/spec/review/ReviewSession-spec.js b/package/spec/review/ReviewSession-spec.js index d8fc8ebe59..40eaf166f7 100644 --- a/package/spec/review/ReviewSession-spec.js +++ b/package/spec/review/ReviewSession-spec.js @@ -29,7 +29,8 @@ describe('ReviewSession', () => { id: 1, comments: [expect.objectContaining({body: 'Hello'})] }) - ] + ], + commentThreadReads: {} }); }); @@ -66,7 +67,8 @@ describe('ReviewSession', () => { currentUser: {id: 42, name: 'Alice'}, commentThreads: [ expect.objectContaining({id: 1}) - ] + ], + commentThreadReads: {} }); }); @@ -1081,4 +1083,140 @@ describe('ReviewSession', () => { expect(session.state.commentThreads).toEqual([]); }); }); + + describe('#markThreadsRead', () => { + beforeEach(() => { + jest.useFakeTimers(); + }); + + afterEach(() => { + jest.useRealTimers(); + }); + + async function createFetchedSession({commentThreadReads = {}} = {}) { + const request = jest.fn().mockResolvedValue({ + currentUser: {id: 42, name: 'Alice'}, + commentThreads: [{id: 1, permaId: 5, comments: []}], + commentThreadReads + }); + + const session = new ReviewSession({entryId: 5, request}); + await session.fetch(); + request.mockClear(); + request.mockResolvedValue(null); + + return {session, request}; + } + + it('exposes read timestamps from fetch in state', async () => { + const {session} = await createFetchedSession({ + commentThreadReads: {5: '2026-08-17T10:00:00.000Z'} + }); + + expect(session.state.commentThreadReads).toEqual({5: '2026-08-17T10:00:00.000Z'}); + }); + + it('emits change:reads with read timestamp for marked threads', async () => { + const {session} = await createFetchedSession(); + const listener = jest.fn(); + session.on('change:reads', listener); + + session.markThreadsRead([5]); + + expect(listener).toHaveBeenCalledWith({5: expect.any(String)}); + expect(session.state.commentThreadReads[5]).toEqual(expect.any(String)); + }); + + it('keeps read timestamps of other threads', async () => { + const {session} = await createFetchedSession({ + commentThreadReads: {7: '2026-08-17T10:00:00.000Z'} + }); + + session.markThreadsRead([5]); + + expect(session.state.commentThreadReads[7]).toEqual('2026-08-17T10:00:00.000Z'); + }); + + it('sends marked perma ids in single request after delay', async () => { + const {session, request} = await createFetchedSession(); + + session.markThreadsRead([5]); + session.markThreadsRead([6]); + + expect(request).not.toHaveBeenCalled(); + + jest.runAllTimers(); + + expect(request).toHaveBeenCalledTimes(1); + expect(request).toHaveBeenCalledWith({ + url: '/review/entries/5/comment_thread_reads', + method: 'POST', + payload: {comment_thread_perma_ids: [5, 6]} + }); + }); + + it('does not send perma id twice', async () => { + const {session, request} = await createFetchedSession(); + + session.markThreadsRead([5]); + session.markThreadsRead([5]); + jest.runAllTimers(); + + expect(request).toHaveBeenCalledWith( + expect.objectContaining({payload: {comment_thread_perma_ids: [5]}}) + ); + }); + + it('does not send request for empty list of perma ids', async () => { + const {session, request} = await createFetchedSession(); + + session.markThreadsRead([]); + jest.runAllTimers(); + + expect(request).not.toHaveBeenCalled(); + }); + + it('ignores marks before state has been fetched', () => { + const request = jest.fn(); + const session = new ReviewSession({entryId: 5, request}); + + session.markThreadsRead([5]); + jest.runAllTimers(); + + expect(request).not.toHaveBeenCalled(); + }); + + it('sends pending perma ids on flushReads', async () => { + const {session, request} = await createFetchedSession(); + + session.markThreadsRead([5]); + await session.flushReads(); + + expect(request).toHaveBeenCalledTimes(1); + }); + + it('does not send request again when nothing is pending', async () => { + const {session, request} = await createFetchedSession(); + + session.markThreadsRead([5]); + await session.flushReads(); + await session.flushReads(); + + expect(request).toHaveBeenCalledTimes(1); + }); + + it('retries perma ids of failed request on next flush', async () => { + const {session, request} = await createFetchedSession(); + request.mockRejectedValueOnce(new Error('Network down')); + + session.markThreadsRead([5]); + await session.flushReads(); + await session.flushReads(); + + expect(request).toHaveBeenCalledTimes(2); + expect(request).toHaveBeenLastCalledWith( + expect.objectContaining({payload: {comment_thread_perma_ids: [5]}}) + ); + }); + }); }); diff --git a/package/spec/review/index-spec.js b/package/spec/review/index-spec.js new file mode 100644 index 0000000000..faa135f3b0 --- /dev/null +++ b/package/spec/review/index-spec.js @@ -0,0 +1,32 @@ +import {createReviewSession} from 'review'; + +describe('createReviewSession', () => { + afterEach(() => { + jest.restoreAllMocks(); + }); + + it('sends pending read marks when page is hidden', async () => { + window.fetch = jest.fn().mockResolvedValue({ok: true, status: 204}); + + const session = createReviewSession({ + entryId: 5, + initialState: { + currentUser: {id: 42, name: 'Alice'}, + commentThreads: [{id: 1, permaId: 7, comments: []}], + commentThreadReads: {} + } + }); + + session.markThreadsRead([7]); + window.dispatchEvent(new window.Event('pagehide')); + await Promise.resolve(); + + expect(window.fetch).toHaveBeenCalledWith( + '/review/entries/5/comment_thread_reads', + expect.objectContaining({ + method: 'POST', + body: JSON.stringify({comment_thread_perma_ids: [7]}) + }) + ); + }); +}); diff --git a/package/spec/review/request-spec.js b/package/spec/review/request-spec.js index 8cbda1277d..de0e61c378 100644 --- a/package/spec/review/request-spec.js +++ b/package/spec/review/request-spec.js @@ -23,6 +23,18 @@ describe('request', () => { expect(result).toEqual({some: 'data'}); }); + it('returns null for responses without content', async () => { + window.fetch = jest.fn().mockResolvedValue({ + ok: true, + status: 204, + json: () => Promise.reject(new Error('Unexpected end of JSON input')) + }); + + const result = await request({url: '/some/path', method: 'POST', payload: {}}); + + expect(result).toBeNull(); + }); + it('sends JSON body with CSRF token for POST requests', async () => { window.fetch = jest.fn().mockResolvedValue({ ok: true, diff --git a/package/src/review/ReviewSession.js b/package/src/review/ReviewSession.js index 3fba870ee8..4172393b02 100644 --- a/package/src/review/ReviewSession.js +++ b/package/src/review/ReviewSession.js @@ -1,11 +1,15 @@ import BackboneEvents from 'backbone-events-standalone'; +const FLUSH_READS_DELAY = 1000; + export class ReviewSession { constructor({entryId, request, initialState = null}) { this._entryId = entryId; this._request = request; this._state = initialState; this._drafts = {}; + this._pendingReads = new Set(); + this._flushReadsTimeout = null; } get state() { @@ -181,12 +185,61 @@ export class ReviewSession { this._state = { currentUser: data.currentUser, - commentThreads: data.commentThreads + commentThreads: data.commentThreads, + commentThreadReads: data.commentThreadReads || {} }; this.trigger('reset', this._state); } + // Read marks are frequent and individually unimportant, so they are + // collected and sent as one request instead of one request per thread. + markThreadsRead(permaIds) { + if (!this._state || !permaIds.length) return; + + const readAt = new Date().toISOString(); + + this._state = { + ...this._state, + commentThreadReads: { + ...this._state.commentThreadReads, + ...Object.fromEntries(permaIds.map(permaId => [permaId, readAt])) + } + }; + + permaIds.forEach(permaId => this._pendingReads.add(permaId)); + + this.trigger('change:reads', this._state.commentThreadReads); + this._scheduleFlushReads(); + } + + async flushReads() { + clearTimeout(this._flushReadsTimeout); + this._flushReadsTimeout = null; + + if (!this._pendingReads.size) return; + + const permaIds = [...this._pendingReads]; + this._pendingReads.clear(); + + await this._request({ + url: `/review/entries/${this._entryId}/comment_thread_reads`, + method: 'POST', + payload: {comment_thread_perma_ids: permaIds} + }).catch(() => { + // Local state already counts the threads as read. Keeping the + // perma ids pending lets the next flush try again instead of + // leaving them unread until the next page load. + permaIds.forEach(permaId => this._pendingReads.add(permaId)); + }); + } + + _scheduleFlushReads() { + if (this._flushReadsTimeout) return; + + this._flushReadsTimeout = setTimeout(() => this.flushReads(), FLUSH_READS_DELAY); + } + _writeDraft({body, pending = false, ...of}) { this._drafts = { ...this._drafts, diff --git a/package/src/review/index.js b/package/src/review/index.js index 33563650b3..d969fcf986 100644 --- a/package/src/review/index.js +++ b/package/src/review/index.js @@ -4,5 +4,11 @@ import {request} from './request'; export {ReviewSession}; export function createReviewSession({entryId, initialState}) { - return new ReviewSession({entryId, request, initialState}); + const session = new ReviewSession({entryId, request, initialState}); + + // Read marks are sent with a delay, which a tab closing right after + // reading would otherwise cut short. + window.addEventListener('pagehide', () => session.flushReads()); + + return session; } diff --git a/package/src/review/request.js b/package/src/review/request.js index 502c229b26..8b4ae09cf2 100644 --- a/package/src/review/request.js +++ b/package/src/review/request.js @@ -21,6 +21,10 @@ export async function request({url, method, payload}) { throw new Error(`${response.status} ${response.statusText}`); } + if (response.status === 204) { + return null; + } + return response.json(); } From 5c3c4a173f47d26f848ed2cd0478b2dc6f893a5e Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Mon, 17 Aug 2026 15:49:15 +0200 Subject: [PATCH 05/18] Deliver comment thread read marks across window boundary Read marks flow from the UI to the session like other mutations and come back as state changes. Reads live in their own context so that marking a thread read does not invalidate the thread lists. --- .../spec/review/ReviewMessageHandler-spec.js | 40 +++++++- .../spec/review/ReviewStateProvider-spec.js | 91 ++++++++++++++++++- .../src/review/ReviewMessageHandler.js | 12 ++- .../package/src/review/ReviewStateProvider.js | 49 +++++++++- .../package/src/review/postMessage.js | 14 +++ 5 files changed, 200 insertions(+), 6 deletions(-) diff --git a/entry_types/scrolled/package/spec/review/ReviewMessageHandler-spec.js b/entry_types/scrolled/package/spec/review/ReviewMessageHandler-spec.js index a46c80d338..f5d9c0dd72 100644 --- a/entry_types/scrolled/package/spec/review/ReviewMessageHandler-spec.js +++ b/entry_types/scrolled/package/spec/review/ReviewMessageHandler-spec.js @@ -8,7 +8,8 @@ function fakeReviewSession() { createComment: jest.fn().mockResolvedValue(), updateThread: jest.fn().mockResolvedValue(), updateComment: jest.fn().mockResolvedValue(), - setDraft: jest.fn() + setDraft: jest.fn(), + markThreadsRead: jest.fn() }; Object.assign(session, BackboneEvents); @@ -98,6 +99,43 @@ describe('ReviewMessageHandler', () => { window.postMessage.mockRestore(); }); + it('calls session.markThreadsRead on MARK_THREADS_READ message from targetWindow', async () => { + const session = fakeReviewSession(); + + ReviewMessageHandler.create({session, targetWindow: window}); + + window.dispatchEvent(new MessageEvent('message', { + data: { + type: 'MARK_THREADS_READ', + payload: {permaIds: [5, 6]} + }, + origin: window.location.origin, + source: window + })); + + await new Promise(resolve => setTimeout(resolve, 0)); + + expect(session.markThreadsRead).toHaveBeenCalledWith([5, 6]); + }); + + it('posts REVIEW_STATE_READS_CHANGE to target window on session change:reads', () => { + const session = fakeReviewSession(); + const postMessage = jest.fn(); + jest.spyOn(window, 'postMessage').mockImplementation(postMessage); + + ReviewMessageHandler.create({session, targetWindow: window}); + + const reads = {5: '2026-08-17T10:00:00.000Z'}; + session.trigger('change:reads', reads); + + expect(postMessage).toHaveBeenCalledWith( + {type: 'REVIEW_STATE_READS_CHANGE', payload: reads}, + window.location.origin + ); + + window.postMessage.mockRestore(); + }); + it('posts REVIEW_STATE_THREAD_CHANGE to target window on session change:thread', () => { const session = fakeReviewSession(); const postMessage = jest.fn(); diff --git a/entry_types/scrolled/package/spec/review/ReviewStateProvider-spec.js b/entry_types/scrolled/package/spec/review/ReviewStateProvider-spec.js index 08282207cc..ab37455edb 100644 --- a/entry_types/scrolled/package/spec/review/ReviewStateProvider-spec.js +++ b/entry_types/scrolled/package/spec/review/ReviewStateProvider-spec.js @@ -7,16 +7,19 @@ import { ReviewStateProvider, useCommentDraft, useCommentThread, + useCommentThreadReadAt, useCommentThreads, useCreateComment, useCreateCommentThread, useCurrentUser, + useMarkThreadRead, useUpdateComment } from 'review/ReviewStateProvider'; import { postReviewStateResetMessage, postReviewStateThreadChangeMessage, - postReviewStateDraftsChangeMessage + postReviewStateDraftsChangeMessage, + postReviewStateReadsChangeMessage } from 'review/postMessage'; import {renderHookWithReviewState} from 'support/renderWithReviewState'; @@ -549,4 +552,90 @@ describe('ReviewStateProvider', () => { expect(result.current.other).toMatchObject({body: 'Elsewhere'}); }); }); + + describe('comment thread reads', () => { + function postReadsChange(payload) { + act(() => { + postReviewStateReadsChangeMessage(window, payload); + }); + } + + it('provides no read timestamp initially', () => { + const {result} = renderHook(() => useCommentThreadReadAt(5), {wrapper}); + + expect(result.current).toBeUndefined(); + }); + + it('provides read timestamp from reset message', async () => { + const {result, waitForNextUpdate} = renderHook( + () => useCommentThreadReadAt(5), + {wrapper} + ); + + postReset({ + currentUser: {id: 42, name: 'Alice'}, + commentThreads: [], + commentThreadReads: {5: '2026-08-17T10:00:00.000Z'} + }); + await waitForNextUpdate(); + + expect(result.current).toEqual('2026-08-17T10:00:00.000Z'); + }); + + it('updates read timestamp on reads change message', async () => { + const {result, waitForNextUpdate} = renderHook( + () => useCommentThreadReadAt(5), + {wrapper} + ); + + postReadsChange({5: '2026-08-17T12:00:00.000Z'}); + await waitForNextUpdate(); + + expect(result.current).toEqual('2026-08-17T12:00:00.000Z'); + }); + + it('marks thread read by posting a message', () => { + const postMessage = jest.spyOn(window.top, 'postMessage').mockImplementation(() => {}); + + const {result} = renderHook(() => useMarkThreadRead(), {wrapper}); + + result.current(5); + + expect(postMessage).toHaveBeenCalledWith( + {type: 'MARK_THREADS_READ', payload: {permaIds: [5]}}, + window.location.origin + ); + + postMessage.mockRestore(); + }); + + it('keeps mark callback stable when reads change', async () => { + const {result, waitForNextUpdate} = renderHook(() => useMarkThreadRead(), {wrapper}); + + const markThreadRead = result.current; + postReadsChange({5: '2026-08-17T12:00:00.000Z'}); + await waitForNextUpdate(); + + expect(result.current).toBe(markThreadRead); + }); + + it('does not invalidate thread state when reads change', async () => { + const {result, waitForNextUpdate} = renderHook( + () => useCommentThreads(), + {wrapper} + ); + + postReset({ + currentUser: {id: 42, name: 'Alice'}, + commentThreads: [{id: 1, permaId: 5, subjectType: 'CE', subjectId: 10, comments: []}], + commentThreadReads: {} + }); + await waitForNextUpdate(); + + const threadsBefore = result.current; + postReadsChange({5: '2026-08-17T12:00:00.000Z'}); + + expect(result.current).toBe(threadsBefore); + }); + }); }); diff --git a/entry_types/scrolled/package/src/review/ReviewMessageHandler.js b/entry_types/scrolled/package/src/review/ReviewMessageHandler.js index baff9d3754..9c6e395113 100644 --- a/entry_types/scrolled/package/src/review/ReviewMessageHandler.js +++ b/entry_types/scrolled/package/src/review/ReviewMessageHandler.js @@ -1,7 +1,8 @@ import { postReviewStateResetMessage, postReviewStateThreadChangeMessage, - postReviewStateDraftsChangeMessage + postReviewStateDraftsChangeMessage, + postReviewStateReadsChangeMessage } from './postMessage'; export const ReviewMessageHandler = { @@ -27,6 +28,9 @@ export const ReviewMessageHandler = { else if (type === 'SET_COMMENT_DRAFT') { session.setDraft(payload); } + else if (type === 'MARK_THREADS_READ') { + session.markThreadsRead(payload.permaIds); + } } function handleReset(state) { @@ -41,10 +45,15 @@ export const ReviewMessageHandler = { postReviewStateDraftsChangeMessage(targetWindow, drafts); } + function handleReadsChange(reads) { + postReviewStateReadsChangeMessage(targetWindow, reads); + } + window.addEventListener('message', handleMessage); session.on('reset', handleReset); session.on('change:thread', handleThreadChange); session.on('change:drafts', handleDraftsChange); + session.on('change:reads', handleReadsChange); return { dispose() { @@ -52,6 +61,7 @@ export const ReviewMessageHandler = { session.off('reset', handleReset); session.off('change:thread', handleThreadChange); session.off('change:drafts', handleDraftsChange); + session.off('change:reads', handleReadsChange); } }; } diff --git a/entry_types/scrolled/package/src/review/ReviewStateProvider.js b/entry_types/scrolled/package/src/review/ReviewStateProvider.js index 245f5ba084..8f23b83418 100644 --- a/entry_types/scrolled/package/src/review/ReviewStateProvider.js +++ b/entry_types/scrolled/package/src/review/ReviewStateProvider.js @@ -7,6 +7,7 @@ import {useSectionPermaIdOfSubject} from 'pageflow-scrolled/entryState'; import { postCreateCommentMessage, postCreateCommentThreadMessage, + postMarkThreadsReadMessage, postSetCommentDraftMessage, postUpdateCommentMessage } from './postMessage'; @@ -14,6 +15,7 @@ import {useSubjectQuote} from './subjectQuote'; const ReviewStateContext = createContext(null); const CommentDraftsContext = createContext(null); +const CommentThreadReadsContext = createContext(null); export function ReviewStateProvider({initialState, initialDrafts, setDraft, children}) { const [state, dispatch] = useReducer( @@ -37,6 +39,9 @@ export function ReviewStateProvider({initialState, initialDrafts, setDraft, chil else if (type === 'REVIEW_STATE_DRAFTS_CHANGE') { dispatch({type: 'SET_DRAFTS', payload}); } + else if (type === 'REVIEW_STATE_READS_CHANGE') { + dispatch({type: 'SET_READS', payload}); + } } window.addEventListener('message', handleMessage); @@ -81,10 +86,27 @@ export function ReviewStateProvider({initialState, initialDrafts, setDraft, chil createComment }), [state.drafts, setDraft, createThread, createComment]); + // Kept stable across read state changes: consumers wait for a thread + // to stay visible before marking it read, which a changing callback + // would start over. + const markThreadRead = useCallback( + permaId => postMarkThreadsReadMessage([permaId]), + [] + ); + + // Read marks arrive whenever a thread is displayed, so they get their + // own context to keep thread lists from rerendering along with them. + const readsValue = useMemo(() => ({ + commentThreadReads: state.commentThreadReads, + markThreadRead + }), [state.commentThreadReads, markThreadRead]); + return ( - {children} + + {children} + ); @@ -141,6 +163,16 @@ export function useCurrentUser() { return context ? context.currentUser : null; } +export function useCommentThreadReadAt(permaId) { + const context = useContext(CommentThreadReadsContext); + return context?.commentThreadReads[permaId]; +} + +export function useMarkThreadRead() { + const context = useContext(CommentThreadReadsContext); + return context?.markThreadRead; +} + export function useCommentThread(threadId) { const context = useContext(ReviewStateContext); return context?.commentThreads.find(t => t.id === threadId); @@ -172,7 +204,12 @@ export function matchesResolution(thread, resolution) { } function initState({initialState, initialDrafts}) { - const empty = {currentUser: null, threads: {}, drafts: initialDrafts || {}}; + const empty = { + currentUser: null, + threads: {}, + drafts: initialDrafts || {}, + commentThreadReads: {} + }; if (initialState) { return reducer(empty, {type: 'RESET', payload: initialState}); @@ -193,9 +230,15 @@ function reducer(state, action) { return { ...state, currentUser: action.payload.currentUser, - threads + threads, + commentThreadReads: action.payload.commentThreadReads || {} }; } + case 'SET_READS': + return { + ...state, + commentThreadReads: action.payload + }; case 'SET_DRAFTS': return { ...state, diff --git a/entry_types/scrolled/package/src/review/postMessage.js b/entry_types/scrolled/package/src/review/postMessage.js index 31c2f51477..df54fde3be 100644 --- a/entry_types/scrolled/package/src/review/postMessage.js +++ b/entry_types/scrolled/package/src/review/postMessage.js @@ -31,6 +31,13 @@ export function postSetCommentDraftMessage(draft) { ); } +export function postMarkThreadsReadMessage(permaIds) { + window.top.postMessage( + {type: 'MARK_THREADS_READ', payload: {permaIds}}, + window.location.origin + ); +} + export function postUpdateThreadMessage({threadId, resolved}) { window.top.postMessage( {type: 'UPDATE_THREAD', payload: {threadId, resolved}}, @@ -58,3 +65,10 @@ export function postReviewStateDraftsChangeMessage(targetWindow, drafts) { window.location.origin ); } + +export function postReviewStateReadsChangeMessage(targetWindow, reads) { + targetWindow.postMessage( + {type: 'REVIEW_STATE_READS_CHANGE', payload: reads}, + window.location.origin + ); +} From 858699da219902795d0395d3f68c69e0da8be929 Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Mon, 17 Aug 2026 15:50:34 +0200 Subject: [PATCH 06/18] Derive unread comments of a thread Comments written by the reviewer never count as unread, and nothing counts as unread while the current user is still unknown, so lists do not briefly show every thread as new. --- .../spec/review/unreadComments-spec.js | 107 ++++++++++++++++++ .../spec/support/renderWithReviewState.js | 12 +- .../package/src/review/unreadComments.js | 31 +++++ 3 files changed, 144 insertions(+), 6 deletions(-) create mode 100644 entry_types/scrolled/package/spec/review/unreadComments-spec.js create mode 100644 entry_types/scrolled/package/src/review/unreadComments.js diff --git a/entry_types/scrolled/package/spec/review/unreadComments-spec.js b/entry_types/scrolled/package/spec/review/unreadComments-spec.js new file mode 100644 index 0000000000..3cac27867c --- /dev/null +++ b/entry_types/scrolled/package/spec/review/unreadComments-spec.js @@ -0,0 +1,107 @@ +import {unreadComments, useUnreadComments} from 'review/unreadComments'; +import {renderHookWithReviewState} from 'support/renderWithReviewState'; + +describe('unreadComments', () => { + const currentUser = {id: 42, name: 'Alice'}; + + function thread(comments) { + return {id: 1, permaId: 5, comments}; + } + + it('returns comments created after read timestamp', () => { + const result = unreadComments( + thread([ + {id: 100, creatorId: 43, createdAt: '2026-08-17T09:00:00.000Z'}, + {id: 101, creatorId: 43, createdAt: '2026-08-17T11:00:00.000Z'} + ]), + {currentUser, readAt: '2026-08-17T10:00:00.000Z'} + ); + + expect(result.map(comment => comment.id)).toEqual([101]); + }); + + it('returns all comments of never read thread', () => { + const result = unreadComments( + thread([ + {id: 100, creatorId: 43, createdAt: '2026-08-17T09:00:00.000Z'}, + {id: 101, creatorId: 43, createdAt: '2026-08-17T11:00:00.000Z'} + ]), + {currentUser, readAt: undefined} + ); + + expect(result.map(comment => comment.id)).toEqual([100, 101]); + }); + + it('excludes comments of current user', () => { + const result = unreadComments( + thread([ + {id: 100, creatorId: 42, createdAt: '2026-08-17T11:00:00.000Z'}, + {id: 101, creatorId: 43, createdAt: '2026-08-17T11:00:00.000Z'} + ]), + {currentUser, readAt: undefined} + ); + + expect(result.map(comment => comment.id)).toEqual([101]); + }); + + it('returns nothing while current user is unknown', () => { + const result = unreadComments( + thread([{id: 100, creatorId: 43, createdAt: '2026-08-17T11:00:00.000Z'}]), + {currentUser: null, readAt: undefined} + ); + + expect(result).toEqual([]); + }); + + it('compares timestamps of different time zone offsets', () => { + const result = unreadComments( + thread([{id: 100, creatorId: 43, createdAt: '2026-08-17T12:00:00.000+02:00'}]), + {currentUser, readAt: '2026-08-17T11:00:00.000Z'} + ); + + expect(result).toEqual([]); + }); + + describe('useUnreadComments', () => { + it('reads current user and read timestamp from review state', () => { + const commentThread = { + id: 1, + permaId: 5, + subjectType: 'ContentElement', + subjectId: 10, + comments: [ + {id: 100, creatorId: 43, createdAt: '2026-08-17T09:00:00.000Z'}, + {id: 101, creatorId: 43, createdAt: '2026-08-17T11:00:00.000Z'} + ] + }; + + const {result} = renderHookWithReviewState( + () => useUnreadComments(commentThread), + { + currentUser, + commentThreads: [commentThread], + commentThreadReads: {5: '2026-08-17T10:00:00.000Z'} + } + ); + + expect(result.current.map(comment => comment.id)).toEqual([101]); + }); + + it('returns nothing before review state has been fetched', () => { + const commentThread = { + id: 1, + permaId: 5, + subjectType: 'ContentElement', + subjectId: 10, + comments: [{id: 100, creatorId: 43, createdAt: '2026-08-17T11:00:00.000Z'}] + }; + + const {result} = renderHookWithReviewState( + () => useUnreadComments(commentThread), + {commentThreads: [commentThread]} + ); + + expect(result.current).toEqual([]); + }); + }); +}); diff --git a/entry_types/scrolled/package/spec/support/renderWithReviewState.js b/entry_types/scrolled/package/spec/support/renderWithReviewState.js index ad31f6435c..1035e58ace 100644 --- a/entry_types/scrolled/package/spec/support/renderWithReviewState.js +++ b/entry_types/scrolled/package/spec/support/renderWithReviewState.js @@ -9,25 +9,25 @@ import {LocatedCommentThreadsProvider} from 'review/useLocatedCommentThreads'; // lives in), so it needs the entry scope just like any other frontend // component — review state is layered on top via the entry helper's // `wrapper` option. -export function renderWithReviewState(ui, {commentThreads = [], currentUser = null, drafts, setDraft, seed = {}} = {}) { +export function renderWithReviewState(ui, {commentThreads = [], commentThreadReads = {}, currentUser = null, drafts, setDraft, seed = {}} = {}) { return renderInEntry(ui, { seed, - wrapper: reviewStateWrapper({commentThreads, currentUser, drafts, setDraft}) + wrapper: reviewStateWrapper({commentThreads, commentThreadReads, currentUser, drafts, setDraft}) }); } // Counterpart for selector hooks that read entry state and review state. -export function renderHookWithReviewState(hook, {commentThreads = [], currentUser = null, drafts, setDraft, seed = {}} = {}) { +export function renderHookWithReviewState(hook, {commentThreads = [], commentThreadReads = {}, currentUser = null, drafts, setDraft, seed = {}} = {}) { return renderHookInEntry(hook, { seed, - wrapper: reviewStateWrapper({commentThreads, currentUser, drafts, setDraft}) + wrapper: reviewStateWrapper({commentThreads, commentThreadReads, currentUser, drafts, setDraft}) }); } -function reviewStateWrapper({commentThreads = [], currentUser = null, drafts, setDraft} = {}) { +function reviewStateWrapper({commentThreads = [], commentThreadReads = {}, currentUser = null, drafts, setDraft} = {}) { return function ReviewStateWrapper({children}) { return ( - diff --git a/entry_types/scrolled/package/src/review/unreadComments.js b/entry_types/scrolled/package/src/review/unreadComments.js new file mode 100644 index 0000000000..547b99d1ef --- /dev/null +++ b/entry_types/scrolled/package/src/review/unreadComments.js @@ -0,0 +1,31 @@ +import {useMemo} from 'react'; + +import {useCommentThreadReadAt, useCurrentUser} from './ReviewStateProvider'; + +export function useUnreadComments(thread) { + const currentUser = useCurrentUser(); + const readAt = useCommentThreadReadAt(thread.permaId); + + return useMemo( + () => unreadComments(thread, {currentUser, readAt}), + [thread, currentUser, readAt] + ); +} + +// 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. +// +// 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 +// thread as new. +export function unreadComments(thread, {currentUser, readAt}) { + if (!currentUser) return []; + + const readAtTime = readAt ? new Date(readAt).getTime() : null; + + return thread.comments.filter( + comment => comment.creatorId !== currentUser.id && + (readAtTime === null || new Date(comment.createdAt).getTime() > readAtTime) + ); +} From 3c2c60fc7d59f4afb5cb645c9b94dc2fe851664d Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Mon, 17 Aug 2026 15:54:27 +0200 Subject: [PATCH 07/18] Mark comment threads read once they have been seen A thread counts as read after staying in the middle of the viewport long enough to read it, so scrolling past leaves it unread. Threads hiding their replies are left alone until expanded. Splits Thread-spec into topic specs under Thread/features, since a flat spec file and a features directory for the same unit must not coexist. --- .../package/spec/review/Thread-spec.js | 267 ------------------ .../features/deletedElementHint-spec.js | 45 +++ .../Thread/features/markingRead-spec.js | 176 ++++++++++++ .../review/Thread/features/quotes-spec.js | 138 +++++++++ .../review/Thread/features/replyForm-spec.js | 110 ++++++++ .../scrolled/package/src/review/Thread.js | 3 + .../src/review/markThreadReadWhenSeen.js | 48 ++++ 7 files changed, 520 insertions(+), 267 deletions(-) delete mode 100644 entry_types/scrolled/package/spec/review/Thread-spec.js create mode 100644 entry_types/scrolled/package/spec/review/Thread/features/deletedElementHint-spec.js create mode 100644 entry_types/scrolled/package/spec/review/Thread/features/markingRead-spec.js create mode 100644 entry_types/scrolled/package/spec/review/Thread/features/quotes-spec.js create mode 100644 entry_types/scrolled/package/spec/review/Thread/features/replyForm-spec.js create mode 100644 entry_types/scrolled/package/src/review/markThreadReadWhenSeen.js diff --git a/entry_types/scrolled/package/spec/review/Thread-spec.js b/entry_types/scrolled/package/spec/review/Thread-spec.js deleted file mode 100644 index dfa048419b..0000000000 --- a/entry_types/scrolled/package/spec/review/Thread-spec.js +++ /dev/null @@ -1,267 +0,0 @@ -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 {review} from 'review/api'; -import {renderWithReviewState} from 'support/renderWithReviewState'; - -describe('Thread', () => { - useFakeTranslations({ - 'pageflow_scrolled.review.refers_to_deleted_element': 'Refers to a deleted element', - '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' - }); - - const thread = { - id: 1, - comments: [{id: 10, body: 'On the pull quote', creatorName: 'Bob', creatorId: 2}] - }; - - // The reply form is hidden while a thread with replies is collapsed, - // which would leave a drafted reply out of reach. - it('expands a collapsed thread that has a drafted reply', () => { - const threadWithReply = { - ...thread, - comments: [ - ...thread.comments, - {id: 11, body: 'A first reply', creatorName: 'Alice', creatorId: 1} - ] - }; - - const {getByPlaceholderText} = renderWithReviewState( - , - { - drafts: { - 'Thread:1': {threadId: 1, body: 'Half a reply', pending: false} - } - } - ); - - expect(getByPlaceholderText('Reply...')).toHaveValue('Half a reply'); - }); - - // Two textareas in one card leave it ambiguous which one typing lands in. - describe('while editing a comment', () => { - useFakeTranslations({ - 'pageflow_scrolled.review.comment_actions': 'Comment actions', - 'pageflow_scrolled.review.edit_comment': 'Edit', - 'pageflow_scrolled.review.cancel': 'Cancel' - }); - - const ownThread = { - id: 1, - comments: [ - {id: 10, body: 'On the pull quote', creatorName: 'Bob', creatorId: 2}, - {id: 11, body: 'A first reply', creatorName: 'Bob', creatorId: 2} - ] - }; - - async function startEditing(user, getAllByRole, index) { - await user.click(getAllByRole('button', {name: 'Comment actions'})[index]); - await user.click(getAllByRole('menuitem', {name: 'Edit'})[0]); - } - - it('hides the reply form', async () => { - const user = userEvent.setup(); - - const {getAllByRole, queryByPlaceholderText} = renderWithReviewState( - , - {currentUser: {id: 2, name: 'Bob'}} - ); - - expect(queryByPlaceholderText('Reply...')).toBeInTheDocument(); - - await startEditing(user, getAllByRole, 0); - - expect(queryByPlaceholderText('Reply...')).toBeNull(); - }); - - it('restores the reply form when editing ends', async () => { - const user = userEvent.setup(); - - const {getAllByRole, getByRole, queryByPlaceholderText} = renderWithReviewState( - , - {currentUser: {id: 2, name: 'Bob'}} - ); - - await startEditing(user, getAllByRole, 0); - await user.click(getByRole('button', {name: 'Cancel'})); - - expect(queryByPlaceholderText('Reply...')).toBeInTheDocument(); - }); - - it('keeps only one comment in edit mode', async () => { - const user = userEvent.setup(); - - const {getAllByRole} = renderWithReviewState( - , - {currentUser: {id: 2, name: 'Bob'}} - ); - - await startEditing(user, getAllByRole, 0); - await startEditing(user, getAllByRole, 1); - - expect(getAllByRole('textbox')).toHaveLength(1); - expect(getAllByRole('textbox')[0]).toHaveValue('A first reply'); - }); - }); - - it('renders a deleted-element hint when the thread is orphaned', () => { - const {getByText} = renderWithReviewState( - - ); - - expect(getByText('Refers to a deleted element')).toBeInTheDocument(); - }); - - it('does not render the hint for a normal thread', () => { - const {queryByText} = renderWithReviewState( - - ); - - expect(queryByText('Refers to a deleted element')).not.toBeInTheDocument(); - }); - - it('renders the hint above the first comment', () => { - const {getByText} = renderWithReviewState( - - ); - - const hint = getByText('Refers to a deleted element'); - const comment = getByText('On the pull quote'); - - expect(hint.compareDocumentPosition(comment) & Node.DOCUMENT_POSITION_FOLLOWING) - .toBeTruthy(); - }); - - describe('comment quotes', () => { - const seed = { - sections: [{id: 1, permaId: 1}], - contentElements: [ - { - id: 1, permaId: 10, sectionId: 1, typeName: 'textBlock', - configuration: {value: 'Current wording'} - } - ] - }; - - const quotingThread = { - ...thread, - subjectType: 'ContentElement', - subjectId: 10, - subjectRange: {anchor: {path: [0, 0], offset: 0}, focus: {path: [0, 0], offset: 7}} - }; - - function threadWithQuotes(...quotes) { - return { - ...quotingThread, - comments: quotes.map((quote, index) => ({ - id: 10 + index, - body: `Comment ${index + 1}`, - creatorName: 'Bob', - creatorId: 2, - quote - })) - }; - } - - beforeEach(() => { - review.contentElementTypes.register('textBlock', { - extractQuote: configuration => configuration.value - }); - }); - - afterEach(() => { - review.contentElementTypes.types = {}; - }); - - it('renders a quote for a comment whose text has since changed', () => { - const {getByText} = renderWithReviewState( - , - {seed} - ); - - expect(getByText('Original wording')).toBeInTheDocument(); - }); - - it('renders no quote while the text still reads the same', () => { - const {container} = renderWithReviewState( - , - {seed} - ); - - expect(container.querySelector('blockquote')).toBeNull(); - }); - - it('renders the quote inside the comment, above its body', () => { - const {getByText} = renderWithReviewState( - , - {seed} - ); - - const quote = getByText('Original wording'); - const body = getByText('Comment 1'); - - expect(quote.parentNode).toBe(body.parentNode); - expect(quote.compareDocumentPosition(body) & Node.DOCUMENT_POSITION_FOLLOWING) - .toBeTruthy(); - }); - - it('renders a quote per version once the text changes mid thread', () => { - const {getByText} = renderWithReviewState( - , - {seed} - ); - - expect(getByText('First wording')).toBeInTheDocument(); - expect(getByText('Second wording')).toBeInTheDocument(); - }); - - it('renders the quote once for a run of replies about the same wording', () => { - const {queryAllByText} = renderWithReviewState( - , - {seed} - ); - - expect(queryAllByText('Same wording')).toHaveLength(1); - }); - - it('renders no quote for the last comment when it matches the current text', () => { - const {getByText, queryByText} = renderWithReviewState( - , - {seed} - ); - - expect(getByText('Original wording')).toBeInTheDocument(); - expect(queryByText('Current wording')).not.toBeInTheDocument(); - }); - - it('renders quotes for threads whose content element was deleted', () => { - const {getByText} = renderWithReviewState( - , - {seed} - ); - - expect(getByText('Original wording')).toBeInTheDocument(); - }); - - it('renders no quote for comments recorded without one', () => { - const {container} = renderWithReviewState( - , - {seed} - ); - - expect(container.querySelector('blockquote')).toBeNull(); - }); - }); -}); diff --git a/entry_types/scrolled/package/spec/review/Thread/features/deletedElementHint-spec.js b/entry_types/scrolled/package/spec/review/Thread/features/deletedElementHint-spec.js new file mode 100644 index 0000000000..5431778304 --- /dev/null +++ b/entry_types/scrolled/package/spec/review/Thread/features/deletedElementHint-spec.js @@ -0,0 +1,45 @@ +import React from 'react'; +import '@testing-library/jest-dom/extend-expect'; +import {useFakeTranslations} from 'pageflow/testHelpers'; + +import {Thread} from 'review/Thread'; +import {renderWithReviewState} from 'support/renderWithReviewState'; + +describe('Thread deleted element hint', () => { + useFakeTranslations({ + 'pageflow_scrolled.review.refers_to_deleted_element': 'Refers to a deleted element' + }); + + const thread = { + id: 1, + comments: [{id: 10, body: 'On the pull quote', creatorName: 'Bob', creatorId: 2}] + }; + + it('renders a deleted-element hint when the thread is orphaned', () => { + const {getByText} = renderWithReviewState( + + ); + + expect(getByText('Refers to a deleted element')).toBeInTheDocument(); + }); + + it('does not render the hint for a normal thread', () => { + const {queryByText} = renderWithReviewState( + + ); + + expect(queryByText('Refers to a deleted element')).not.toBeInTheDocument(); + }); + + it('renders the hint above the first comment', () => { + const {getByText} = renderWithReviewState( + + ); + + const hint = getByText('Refers to a deleted element'); + const comment = getByText('On the pull quote'); + + expect(hint.compareDocumentPosition(comment) & Node.DOCUMENT_POSITION_FOLLOWING) + .toBeTruthy(); + }); +}); 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 new file mode 100644 index 0000000000..6f9b3bff2c --- /dev/null +++ b/entry_types/scrolled/package/spec/review/Thread/features/markingRead-spec.js @@ -0,0 +1,176 @@ +import React from 'react'; +import '@testing-library/jest-dom/extend-expect'; +import {act} from '@testing-library/react'; +import {useFakeTranslations} from 'pageflow/testHelpers'; + +import {Thread} from 'review/Thread'; +import {renderWithReviewState} from 'support/renderWithReviewState'; +import { + simulateScrollingIntoView, + simulateScrollingOutOfView +} from 'support/fakeIntersectionObserver'; + +describe('Thread marking read', () => { + 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' + }); + + const currentUser = {id: 42, name: 'Alice'}; + + const thread = { + id: 1, + permaId: 5, + subjectType: 'ContentElement', + subjectId: 10, + comments: [ + { + id: 100, + body: 'On the pull quote', + creatorId: 43, + creatorName: 'Bob', + createdAt: '2026-08-17T11:00:00.000Z' + } + ] + }; + + const threadWithReply = { + ...thread, + comments: [ + ...thread.comments, + { + id: 101, + body: 'Agreed', + creatorId: 44, + creatorName: 'Carol', + createdAt: '2026-08-17T12:00:00.000Z' + } + ] + }; + + beforeEach(() => { + jest.useFakeTimers(); + }); + + afterEach(() => { + jest.useRealTimers(); + jest.restoreAllMocks(); + }); + + function render(ui, options) { + const postMessage = jest.spyOn(window.top, 'postMessage').mockImplementation(() => {}); + + return { + ...renderWithReviewState(ui, {currentUser, commentThreads: [thread], ...options}), + postMessage + }; + } + + function passTime(ms) { + act(() => { + jest.advanceTimersByTime(ms); + }); + } + + function markReadMessages(postMessage) { + return postMessage.mock.calls.filter(([message]) => message.type === 'MARK_THREADS_READ'); + } + + it('marks thread read once it has been on screen long enough', () => { + const {container, postMessage} = render(); + + simulateScrollingIntoView(container); + passTime(1000); + + expect(postMessage).toHaveBeenCalledWith( + {type: 'MARK_THREADS_READ', payload: {permaIds: [5]}}, + window.location.origin + ); + }); + + it('does not mark thread read while it is off screen', () => { + const {postMessage} = render(); + + passTime(1000); + + expect(markReadMessages(postMessage)).toEqual([]); + }); + + it('does not mark thread read when it scrolls by', () => { + const {container, postMessage} = render(); + + simulateScrollingIntoView(container); + passTime(400); + simulateScrollingOutOfView(container); + passTime(1000); + + expect(markReadMessages(postMessage)).toEqual([]); + }); + + it('does not mark thread read while replies are collapsed', () => { + const {container, postMessage} = render( + , + {commentThreads: [threadWithReply]} + ); + + simulateScrollingIntoView(container); + passTime(1000); + + expect(markReadMessages(postMessage)).toEqual([]); + }); + + it('marks thread read once replies are expanded', () => { + const {container, postMessage} = render( + , + {commentThreads: [threadWithReply]} + ); + + 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( + , + {commentThreadReads: {5: '2026-08-17T13:00:00.000Z'}} + ); + + simulateScrollingIntoView(container); + passTime(1000); + + expect(markReadMessages(postMessage)).toEqual([]); + }); + + it('does not mark thread read that only contains own comments', () => { + const ownThread = { + ...thread, + comments: [{...thread.comments[0], creatorId: currentUser.id, creatorName: 'Alice'}] + }; + + const {container, postMessage} = render( + , + {commentThreads: [ownThread]} + ); + + simulateScrollingIntoView(container); + passTime(1000); + + expect(markReadMessages(postMessage)).toEqual([]); + }); + + it('does not mark thread read before current user is known', () => { + const {container, postMessage} = render( + , + {currentUser: null} + ); + + simulateScrollingIntoView(container); + passTime(1000); + + expect(markReadMessages(postMessage)).toEqual([]); + }); +}); diff --git a/entry_types/scrolled/package/spec/review/Thread/features/quotes-spec.js b/entry_types/scrolled/package/spec/review/Thread/features/quotes-spec.js new file mode 100644 index 0000000000..585a3cd525 --- /dev/null +++ b/entry_types/scrolled/package/spec/review/Thread/features/quotes-spec.js @@ -0,0 +1,138 @@ +import React from 'react'; +import '@testing-library/jest-dom/extend-expect'; + +import {Thread} from 'review/Thread'; +import {review} from 'review/api'; +import {renderWithReviewState} from 'support/renderWithReviewState'; + +describe('Thread comment quotes', () => { + const thread = { + id: 1, + comments: [{id: 10, body: 'On the pull quote', creatorName: 'Bob', creatorId: 2}] + }; + + const seed = { + sections: [{id: 1, permaId: 1}], + contentElements: [ + { + id: 1, permaId: 10, sectionId: 1, typeName: 'textBlock', + configuration: {value: 'Current wording'} + } + ] + }; + + const quotingThread = { + ...thread, + subjectType: 'ContentElement', + subjectId: 10, + subjectRange: {anchor: {path: [0, 0], offset: 0}, focus: {path: [0, 0], offset: 7}} + }; + + function threadWithQuotes(...quotes) { + return { + ...quotingThread, + comments: quotes.map((quote, index) => ({ + id: 10 + index, + body: `Comment ${index + 1}`, + creatorName: 'Bob', + creatorId: 2, + quote + })) + }; + } + + beforeEach(() => { + review.contentElementTypes.register('textBlock', { + extractQuote: configuration => configuration.value + }); + }); + + afterEach(() => { + review.contentElementTypes.types = {}; + }); + + it('renders a quote for a comment whose text has since changed', () => { + const {getByText} = renderWithReviewState( + , + {seed} + ); + + expect(getByText('Original wording')).toBeInTheDocument(); + }); + + it('renders no quote while the text still reads the same', () => { + const {container} = renderWithReviewState( + , + {seed} + ); + + expect(container.querySelector('blockquote')).toBeNull(); + }); + + it('renders the quote inside the comment, above its body', () => { + const {getByText} = renderWithReviewState( + , + {seed} + ); + + const quote = getByText('Original wording'); + const body = getByText('Comment 1'); + + expect(quote.parentNode).toBe(body.parentNode); + expect(quote.compareDocumentPosition(body) & Node.DOCUMENT_POSITION_FOLLOWING) + .toBeTruthy(); + }); + + it('renders a quote per version once the text changes mid thread', () => { + const {getByText} = renderWithReviewState( + , + {seed} + ); + + expect(getByText('First wording')).toBeInTheDocument(); + expect(getByText('Second wording')).toBeInTheDocument(); + }); + + it('renders the quote once for a run of replies about the same wording', () => { + const {queryAllByText} = renderWithReviewState( + , + {seed} + ); + + expect(queryAllByText('Same wording')).toHaveLength(1); + }); + + it('renders no quote for the last comment when it matches the current text', () => { + const {getByText, queryByText} = renderWithReviewState( + , + {seed} + ); + + expect(getByText('Original wording')).toBeInTheDocument(); + expect(queryByText('Current wording')).not.toBeInTheDocument(); + }); + + it('renders quotes for threads whose content element was deleted', () => { + const {getByText} = renderWithReviewState( + , + {seed} + ); + + expect(getByText('Original wording')).toBeInTheDocument(); + }); + + it('renders no quote for comments recorded without one', () => { + const {container} = renderWithReviewState( + , + {seed} + ); + + expect(container.querySelector('blockquote')).toBeNull(); + }); +}); diff --git a/entry_types/scrolled/package/spec/review/Thread/features/replyForm-spec.js b/entry_types/scrolled/package/spec/review/Thread/features/replyForm-spec.js new file mode 100644 index 0000000000..abeccf5bf8 --- /dev/null +++ b/entry_types/scrolled/package/spec/review/Thread/features/replyForm-spec.js @@ -0,0 +1,110 @@ +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 reply form', () => { + 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' + }); + + const thread = { + id: 1, + comments: [{id: 10, body: 'On the pull quote', creatorName: 'Bob', creatorId: 2}] + }; + + // The reply form is hidden while a thread with replies is collapsed, + // which would leave a drafted reply out of reach. + it('expands a collapsed thread that has a drafted reply', () => { + const threadWithReply = { + ...thread, + comments: [ + ...thread.comments, + {id: 11, body: 'A first reply', creatorName: 'Alice', creatorId: 1} + ] + }; + + const {getByPlaceholderText} = renderWithReviewState( + , + { + drafts: { + 'Thread:1': {threadId: 1, body: 'Half a reply', pending: false} + } + } + ); + + expect(getByPlaceholderText('Reply...')).toHaveValue('Half a reply'); + }); + + // Two textareas in one card leave it ambiguous which one typing lands in. + describe('while editing a comment', () => { + useFakeTranslations({ + 'pageflow_scrolled.review.comment_actions': 'Comment actions', + 'pageflow_scrolled.review.edit_comment': 'Edit', + 'pageflow_scrolled.review.cancel': 'Cancel' + }); + + const ownThread = { + id: 1, + comments: [ + {id: 10, body: 'On the pull quote', creatorName: 'Bob', creatorId: 2}, + {id: 11, body: 'A first reply', creatorName: 'Bob', creatorId: 2} + ] + }; + + async function startEditing(user, getAllByRole, index) { + await user.click(getAllByRole('button', {name: 'Comment actions'})[index]); + await user.click(getAllByRole('menuitem', {name: 'Edit'})[0]); + } + + it('hides the reply form', async () => { + const user = userEvent.setup(); + + const {getAllByRole, queryByPlaceholderText} = renderWithReviewState( + , + {currentUser: {id: 2, name: 'Bob'}} + ); + + expect(queryByPlaceholderText('Reply...')).toBeInTheDocument(); + + await startEditing(user, getAllByRole, 0); + + expect(queryByPlaceholderText('Reply...')).toBeNull(); + }); + + it('restores the reply form when editing ends', async () => { + const user = userEvent.setup(); + + const {getAllByRole, getByRole, queryByPlaceholderText} = renderWithReviewState( + , + {currentUser: {id: 2, name: 'Bob'}} + ); + + await startEditing(user, getAllByRole, 0); + await user.click(getByRole('button', {name: 'Cancel'})); + + expect(queryByPlaceholderText('Reply...')).toBeInTheDocument(); + }); + + it('keeps only one comment in edit mode', async () => { + const user = userEvent.setup(); + + const {getAllByRole} = renderWithReviewState( + , + {currentUser: {id: 2, name: 'Bob'}} + ); + + await startEditing(user, getAllByRole, 0); + await startEditing(user, getAllByRole, 1); + + expect(getAllByRole('textbox')).toHaveLength(1); + expect(getAllByRole('textbox')[0]).toHaveValue('A first reply'); + }); + }); +}); diff --git a/entry_types/scrolled/package/src/review/Thread.js b/entry_types/scrolled/package/src/review/Thread.js index cde662ec32..d0941449be 100644 --- a/entry_types/scrolled/package/src/review/Thread.js +++ b/entry_types/scrolled/package/src/review/Thread.js @@ -8,6 +8,7 @@ import {ReplyForm} from './ReplyForm'; import {useCommentDraft} from './ReviewStateProvider'; import {useSubjectQuote} from './subjectQuote'; import {commentsWithOutdatedQuote} from './outdatedQuotes'; +import {useMarkThreadReadWhenSeen} from './markThreadReadWhenSeen'; import {useScrollHighlightedThreadIntoView} from './scrollHighlightedThreadIntoView'; import ChevronIcon from './images/chevron.svg'; @@ -51,6 +52,8 @@ export function Thread({thread, collapsed: collapsedProp, onToggle, onResolve, o const ref = useRef(); const scrollHighlightedIntoView = useScrollHighlightedThreadIntoView(); + useMarkThreadReadWhenSeen({thread, ref, enabled: !repliesCollapsed}); + useEffect(() => { if (scrollHighlightedIntoView && highlighted && ref.current) { ref.current.scrollIntoView({block: 'nearest', behavior: 'smooth'}); diff --git a/entry_types/scrolled/package/src/review/markThreadReadWhenSeen.js b/entry_types/scrolled/package/src/review/markThreadReadWhenSeen.js new file mode 100644 index 0000000000..a81fa46e02 --- /dev/null +++ b/entry_types/scrolled/package/src/review/markThreadReadWhenSeen.js @@ -0,0 +1,48 @@ +import {useEffect} from 'react'; + +import {useMarkThreadRead} from './ReviewStateProvider'; +import {useUnreadComments} from './unreadComments'; + +const DWELL_TIME = 800; + +// Threads that only peek in at the very edge of the viewport have not +// been read, but anything substantially on screen has. Expressed as a +// margin rather than a visibility ratio, which a thread taller than the +// viewport could never reach. +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. +export function useMarkThreadReadWhenSeen({thread, ref, enabled}) { + const unreadComments = useUnreadComments(thread); + const markThreadRead = useMarkThreadRead(); + + const {permaId} = thread; + const hasUnreadComments = unreadComments.length > 0; + + useEffect(() => { + const element = ref.current; + + if (!enabled || !hasUnreadComments || !markThreadRead || !element) return; + + let timeout; + + const observer = new IntersectionObserver(entries => { + clearTimeout(timeout); + + if (entries[entries.length - 1].isIntersecting) { + timeout = setTimeout(() => markThreadRead(permaId), DWELL_TIME); + } + }, {rootMargin: ROOT_MARGIN}); + + observer.observe(element); + + return () => { + clearTimeout(timeout); + observer.disconnect(); + }; + }, [enabled, hasUnreadComments, markThreadRead, permaId, ref]); +} From 9f923ac2451781bf6558cb4d5f902636f4e194f8 Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Mon, 17 Aug 2026 15:58:40 +0200 Subject: [PATCH 08/18] Mark badges of subjects with unseen comments Adds a dot to the thread badge and names the unread count for screen readers, so subjects carrying comments the reviewer has not seen stand out before their threads are opened. --- entry_types/scrolled/config/locales/de.yml | 3 + entry_types/scrolled/config/locales/en.yml | 3 + .../package/spec/review/ThreadsBadge-spec.js | 86 ++++++++++++++++++- .../scrolled/package/src/review/Badge.js | 8 +- .../package/src/review/Badge.module.css | 17 ++++ .../package/src/review/ReviewStateProvider.js | 12 ++- .../package/src/review/ThreadsBadge.js | 16 +++- .../package/src/review/unreadComments.js | 20 ++++- 8 files changed, 157 insertions(+), 8 deletions(-) diff --git a/entry_types/scrolled/config/locales/de.yml b/entry_types/scrolled/config/locales/de.yml index ab86ffdf4d..c398e68b12 100644 --- a/entry_types/scrolled/config/locales/de.yml +++ b/entry_types/scrolled/config/locales/de.yml @@ -1995,6 +1995,9 @@ de: zero: Keine Kommentare one: 1 Kommentar other: '%{count} Kommentare' + unread_comment_count: + one: 1 ungelesener Kommentar + other: '%{count} ungelesene Kommentare' select_content_element: Zum Kommentieren auswählen select_section: Abschnitt zum Kommentieren auswählen select_text_to_comment: Text zum Kommentieren auswählen diff --git a/entry_types/scrolled/config/locales/en.yml b/entry_types/scrolled/config/locales/en.yml index 8d4bca3d63..61d1cb41e7 100644 --- a/entry_types/scrolled/config/locales/en.yml +++ b/entry_types/scrolled/config/locales/en.yml @@ -1823,6 +1823,9 @@ en: zero: No comments one: 1 comment other: '%{count} comments' + unread_comment_count: + one: 1 unread comment + other: '%{count} unread comments' select_content_element: Select to comment select_section: Select section to comment select_text_to_comment: Select text to comment diff --git a/entry_types/scrolled/package/spec/review/ThreadsBadge-spec.js b/entry_types/scrolled/package/spec/review/ThreadsBadge-spec.js index f992922aa9..5a14c2989d 100644 --- a/entry_types/scrolled/package/spec/review/ThreadsBadge-spec.js +++ b/entry_types/scrolled/package/spec/review/ThreadsBadge-spec.js @@ -1,5 +1,6 @@ import React from 'react'; import '@testing-library/jest-dom/extend-expect'; +import {useFakeTranslations} from 'pageflow/testHelpers'; import {ThreadsBadge} from 'review/ThreadsBadge'; import {renderWithReviewState} from 'support/renderWithReviewState'; @@ -12,11 +13,92 @@ const seed = { contentElements: [{id: 1, permaId: 10, sectionId: 1, typeName: 'textBlock'}] }; -function renderThreadsBadge(ui, {commentThreads = []} = {}) { - return renderWithReviewState(ui, {seed, commentThreads}); +function renderThreadsBadge(ui, {commentThreads = [], ...options} = {}) { + return renderWithReviewState(ui, {seed, commentThreads, ...options}); } describe('ThreadsBadge', () => { + describe('unread comments', () => { + useFakeTranslations({ + 'pageflow_scrolled.review.unread_comment_count.one': '1 unread comment', + 'pageflow_scrolled.review.unread_comment_count.other': '%{count} unread comments' + }); + + const currentUser = {id: 42, name: 'Alice'}; + + function threadWithComment(attributes) { + return { + id: 1, + permaId: 5, + subjectType: 'ContentElement', + subjectId: 10, + comments: [{ + id: 100, + creatorId: 43, + createdAt: '2026-08-17T11:00:00.000Z', + ...attributes + }] + }; + } + + it('marks badge as unread and names the count', () => { + const {getByRole} = renderThreadsBadge( + , + {currentUser, commentThreads: [threadWithComment()]} + ); + + expect(getByRole('status')).toHaveClass(badgeStyles.unread); + expect(getByRole('status')).toHaveAttribute('aria-label', '1 unread comment'); + }); + + it('counts unread comments across threads of the subject', () => { + const {getByRole} = renderThreadsBadge( + , + { + currentUser, + commentThreads: [ + threadWithComment(), + {...threadWithComment({id: 101}), id: 2, permaId: 6} + ] + } + ); + + expect(getByRole('status')).toHaveAttribute('aria-label', '2 unread comments'); + }); + + it('does not mark badge as unread once comments have been read', () => { + const {getByRole} = renderThreadsBadge( + , + { + currentUser, + commentThreads: [threadWithComment()], + commentThreadReads: {5: '2026-08-17T12:00:00.000Z'} + } + ); + + expect(getByRole('status')).not.toHaveClass(badgeStyles.unread); + expect(getByRole('status')).not.toHaveAttribute('aria-label'); + }); + + it('does not mark badge as unread for own comments', () => { + const {getByRole} = renderThreadsBadge( + , + {currentUser, commentThreads: [threadWithComment({creatorId: currentUser.id})]} + ); + + expect(getByRole('status')).not.toHaveClass(badgeStyles.unread); + }); + + it('does not mark badge as unread before current user is known', () => { + const {getByRole} = renderThreadsBadge( + , + {commentThreads: [threadWithComment()]} + ); + + expect(getByRole('status')).not.toHaveClass(badgeStyles.unread); + }); + }); + it('does not display count for single thread', () => { const {getByRole} = renderThreadsBadge( , diff --git a/entry_types/scrolled/package/src/review/Badge.js b/entry_types/scrolled/package/src/review/Badge.js index 0cc0f23295..58121aae3f 100644 --- a/entry_types/scrolled/package/src/review/Badge.js +++ b/entry_types/scrolled/package/src/review/Badge.js @@ -4,7 +4,9 @@ import classNames from 'classnames'; import CommentIcon from './images/comment.svg'; import styles from './Badge.module.css'; -export const Badge = forwardRef(function Badge({counter, mode, resolved, onClick}, ref) { +export const Badge = forwardRef(function Badge({ + counter, mode, resolved, unread, label, onClick +}, ref) { const variant = resolveVariant(mode, counter > 0); if (!variant) { @@ -14,8 +16,10 @@ export const Badge = forwardRef(function Badge({counter, mode, resolved, onClick return ( } - - {showNewForm && - setFormToggled(false)} />} - - {noThreads && !showNewForm && -

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

} - - {activeThreads.map(thread => ( - 1 && expandedThreadId !== thread.id} - onToggle={() => toggleThread(thread.id)} - onResolve={() => postUpdateThreadMessage({threadId: thread.id, resolved: true})} - onClick={onThreadClick && (() => onThreadClick(thread))} - highlighted={isHighlighted(thread)} - interactive={!restrictInteractionsToHighlighted || isHighlighted(thread)} /> - ))} - - {resolvedThreads.length > 0 && -
- - - {showResolved && resolvedThreads.map(thread => ( - 1 && expandedThreadId !== thread.id} - onToggle={() => toggleThread(thread.id)} - onResolve={() => postUpdateThreadMessage({threadId: thread.id, resolved: false})} - onClick={onThreadClick && (() => onThreadClick(thread))} - highlighted={isHighlighted(thread)} - interactive={!restrictInteractionsToHighlighted || isHighlighted(thread)} /> - ))} -
} - + +
+ {!showNewForm && !hideNewTopicButton && + } + + {showNewForm && + setFormToggled(false)} />} + + {noThreads && !showNewForm && +

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

} + + {activeThreads.map(thread => ( + 1 && expandedThreadId !== thread.id} + onToggle={() => toggleThread(thread.id)} + onResolve={() => postUpdateThreadMessage({threadId: thread.id, resolved: true})} + onClick={onThreadClick && (() => onThreadClick(thread))} + highlighted={isHighlighted(thread)} + interactive={!restrictInteractionsToHighlighted || isHighlighted(thread)} /> + ))} + + {resolvedThreads.length > 0 && +
+ + + {showResolved && resolvedThreads.map(thread => ( + 1 && expandedThreadId !== thread.id} + onToggle={() => toggleThread(thread.id)} + onResolve={() => postUpdateThreadMessage({threadId: thread.id, resolved: false})} + onClick={onThreadClick && (() => onThreadClick(thread))} + highlighted={isHighlighted(thread)} + interactive={!restrictInteractionsToHighlighted || isHighlighted(thread)} /> + ))} +
} +
+ ); } diff --git a/entry_types/scrolled/package/src/review/commentThreadReadsSnapshot.js b/entry_types/scrolled/package/src/review/commentThreadReadsSnapshot.js new file mode 100644 index 0000000000..87f1825c96 --- /dev/null +++ b/entry_types/scrolled/package/src/review/commentThreadReadsSnapshot.js @@ -0,0 +1,45 @@ +import React, {createContext, useContext, useRef} from 'react'; + +import {useCommentThreadReads, useCurrentUser} from './ReviewStateProvider'; + +const CommentThreadReadsSnapshotContext = createContext(null); + +// Threads mark themselves read while they are on screen, so markers +// derived from live read state would disappear from under the reviewer +// mid-read. Displaying threads therefore freezes read state for as long +// as they are shown: markers hold still until the list goes away, and +// the next visit reflects what was read. +// +// Nesting reuses the outermost snapshot, so a list rendered inside an +// already frozen scope keeps that scope's idea of what is new. +export function CommentThreadReadsSnapshot({enabled = true, children}) { + const outerSnapshot = useContext(CommentThreadReadsSnapshotContext); + const liveReads = useCommentThreadReads(); + + // Read state only means anything once the current user is known. + // Freezing before that would keep every thread marked new. + const currentUser = useCurrentUser(); + const snapshot = useRef(null); + + if (!enabled) { + snapshot.current = null; + } + else if (!snapshot.current && currentUser) { + snapshot.current = liveReads; + } + + return ( + + {children} + + ); +} + +// Read state to derive markers from. Falls back to live state where +// nothing has been frozen. +export function useDisplayedCommentThreadReads() { + const snapshot = useContext(CommentThreadReadsSnapshotContext); + const liveReads = useCommentThreadReads(); + + return snapshot || liveReads; +} diff --git a/entry_types/scrolled/package/src/review/index.js b/entry_types/scrolled/package/src/review/index.js index bf53c4da55..dcdd2bb1ae 100644 --- a/entry_types/scrolled/package/src/review/index.js +++ b/entry_types/scrolled/package/src/review/index.js @@ -6,6 +6,7 @@ export {ReviewMessageHandler} from './ReviewMessageHandler'; export {ThreadsBadge} from './ThreadsBadge'; export {Badge} from './Badge'; export {ThreadList} from './ThreadList'; +export {CommentThreadReadsSnapshot} from './commentThreadReadsSnapshot'; export {Thread} from './Thread'; export {ScrollHighlightedThreadIntoViewProvider} from './scrollHighlightedThreadIntoView'; export {NewThreadForm} from './NewThreadForm'; diff --git a/entry_types/scrolled/package/src/review/markThreadReadWhenSeen.js b/entry_types/scrolled/package/src/review/markThreadReadWhenSeen.js index a81fa46e02..93a7b0860b 100644 --- a/entry_types/scrolled/package/src/review/markThreadReadWhenSeen.js +++ b/entry_types/scrolled/package/src/review/markThreadReadWhenSeen.js @@ -1,7 +1,7 @@ import {useEffect} from 'react'; import {useMarkThreadRead} from './ReviewStateProvider'; -import {useUnreadComments} from './unreadComments'; +import {useLiveUnreadComments} from './unreadComments'; const DWELL_TIME = 800; @@ -17,7 +17,7 @@ const ROOT_MARGIN = '-10% 0px -10% 0px'; // Callers pass `enabled: false` while the thread hides part of itself, // so collapsed replies do not get marked as read unseen. export function useMarkThreadReadWhenSeen({thread, ref, enabled}) { - const unreadComments = useUnreadComments(thread); + const unreadComments = useLiveUnreadComments(thread); const markThreadRead = useMarkThreadRead(); const {permaId} = thread; diff --git a/entry_types/scrolled/package/src/review/unreadComments.js b/entry_types/scrolled/package/src/review/unreadComments.js index bfd1fa8ebf..be1e278098 100644 --- a/entry_types/scrolled/package/src/review/unreadComments.js +++ b/entry_types/scrolled/package/src/review/unreadComments.js @@ -1,12 +1,11 @@ import {useMemo} from 'react'; -import { - useCommentThreadReadAt, useCommentThreadReads, useCurrentUser -} from './ReviewStateProvider'; +import {useCommentThreadReads, useCurrentUser} from './ReviewStateProvider'; +import {useDisplayedCommentThreadReads} from './commentThreadReadsSnapshot'; export function useUnreadCommentCount(threads) { const currentUser = useCurrentUser(); - const commentThreadReads = useCommentThreadReads(); + const commentThreadReads = useDisplayedCommentThreadReads(); return useMemo( () => threads.reduce( @@ -22,11 +21,31 @@ export function useUnreadCommentCount(threads) { export function useUnreadComments(thread) { const currentUser = useCurrentUser(); - const readAt = useCommentThreadReadAt(thread.permaId); + const commentThreadReads = useDisplayedCommentThreadReads(); + + return useMemo( + () => unreadComments(thread, { + currentUser, + readAt: commentThreadReads[thread.permaId] + }), + [thread, currentUser, commentThreadReads] + ); +} + +// Counterpart of useUnreadComments for deciding whether a thread still +// needs to be marked read. Reading frozen state here would keep the +// thread unread no matter how often it was marked, leaving the read +// signal firing forever. +export function useLiveUnreadComments(thread) { + const currentUser = useCurrentUser(); + const commentThreadReads = useCommentThreadReads(); return useMemo( - () => unreadComments(thread, {currentUser, readAt}), - [thread, currentUser, readAt] + () => unreadComments(thread, { + currentUser, + readAt: commentThreadReads[thread.permaId] + }), + [thread, currentUser, commentThreadReads] ); } From 2968bc1c9ecdbdc3edf45f5e56c30d4365b03c0f Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Wed, 19 Aug 2026 13:37:37 +0200 Subject: [PATCH 10/18] Mark threads and collapsed replies that are new A dot locates the thread carrying unseen comments, and the expand control names how many of the replies it hides are new. A thread shown on its own gets no dot: the badge that opened the list already says the same thing next to it. --- entry_types/scrolled/config/locales/de.yml | 3 + entry_types/scrolled/config/locales/en.yml | 3 + .../review/Thread/features/newMarkers-spec.js | 144 ++++++++++++++++++ .../scrolled/package/src/review/Thread.js | 27 +++- .../package/src/review/Thread.module.css | 35 ++++- .../scrolled/package/src/review/ThreadList.js | 2 + 6 files changed, 210 insertions(+), 4 deletions(-) create mode 100644 entry_types/scrolled/package/spec/review/Thread/features/newMarkers-spec.js diff --git a/entry_types/scrolled/config/locales/de.yml b/entry_types/scrolled/config/locales/de.yml index c398e68b12..e7ea0afe5d 100644 --- a/entry_types/scrolled/config/locales/de.yml +++ b/entry_types/scrolled/config/locales/de.yml @@ -1998,6 +1998,9 @@ de: unread_comment_count: one: 1 ungelesener Kommentar other: '%{count} ungelesene Kommentare' + new_reply_count: + one: 1 neu + other: '%{count} neu' 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 61d1cb41e7..d5d8bb478e 100644 --- a/entry_types/scrolled/config/locales/en.yml +++ b/entry_types/scrolled/config/locales/en.yml @@ -1826,6 +1826,9 @@ en: unread_comment_count: one: 1 unread comment other: '%{count} unread comments' + new_reply_count: + one: 1 new + other: '%{count} new' 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/newMarkers-spec.js new file mode 100644 index 0000000000..ac944b9c97 --- /dev/null +++ b/entry_types/scrolled/package/spec/review/Thread/features/newMarkers-spec.js @@ -0,0 +1,144 @@ +import React from 'react'; +import '@testing-library/jest-dom/extend-expect'; +import {useFakeTranslations} from 'pageflow/testHelpers'; + +import {Thread} from 'review/Thread'; +import {renderWithReviewState} from 'support/renderWithReviewState'; +import styles from 'review/Thread.module.css'; + +describe('Thread new 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.toggle_replies': 'Toggle replies', + 'pageflow_scrolled.review.unread_comment_count.one': '1 unread comment', + 'pageflow_scrolled.review.unread_comment_count.other': '%{count} unread comments' + }); + + const currentUser = {id: 42, name: 'Alice'}; + + function comment(attributes) { + return { + creatorId: 43, + creatorName: 'Bob', + createdAt: '2026-08-17T11:00:00.000Z', + ...attributes + }; + } + + const thread = { + id: 1, + permaId: 5, + subjectType: 'ContentElement', + subjectId: 10, + comments: [comment({id: 100, body: 'On the pull quote'})] + }; + + const threadWithReplies = { + ...thread, + comments: [ + comment({ + id: 100, body: 'On the pull quote', createdAt: '2026-08-17T09:00:00.000Z' + }), + comment({id: 101, body: 'Agreed', creatorId: 44, creatorName: 'Carol'}) + ] + }; + + function render(ui, options = {}) { + return renderWithReviewState(ui, {currentUser, commentThreads: [thread], ...options}); + } + + function newDot(container) { + return container.querySelector(`.${styles.newDot}`); + } + + describe('dot on the thread', () => { + it('marks a thread with unseen comments', () => { + const {container, getByLabelText} = render( + + ); + + expect(newDot(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(); + }); + + it('does not mark a thread without unseen comments', () => { + const {container} = render( + , + {commentThreadReads: {5: '2026-08-17T12:00:00.000Z'}} + ); + + expect(newDot(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(); + }); + }); + + describe('new reply count', () => { + it('counts unseen replies hidden by collapsing', () => { + const {getByText} = render( + , + { + commentThreads: [threadWithReplies], + commentThreadReads: {5: '2026-08-17T10:00:00.000Z'} + } + ); + + expect(getByText('1 reply')).toBeInTheDocument(); + expect(getByText('1 new')).toBeInTheDocument(); + }); + + // An unread first comment must not be counted among the replies it + // is collapsed above, and own replies are never new. + it('counts neither the first comment nor own replies', () => { + const threadWithOwnReply = { + ...thread, + comments: [ + comment({id: 100, body: 'On the pull quote'}), + comment({id: 101, body: 'Will fix', creatorId: currentUser.id, creatorName: 'Alice'}) + ] + }; + + const {getByText, queryByText} = render( + , + {commentThreads: [threadWithOwnReply]} + ); + + expect(getByText('1 reply')).toBeInTheDocument(); + expect(queryByText('1 new')).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'} + } + ); + + expect(queryByText('1 reply')).toBeInTheDocument(); + expect(queryByText('1 new')).toBeNull(); + }); + }); +}); diff --git a/entry_types/scrolled/package/src/review/Thread.js b/entry_types/scrolled/package/src/review/Thread.js index d0941449be..acab197278 100644 --- a/entry_types/scrolled/package/src/review/Thread.js +++ b/entry_types/scrolled/package/src/review/Thread.js @@ -9,6 +9,7 @@ import {useCommentDraft} from './ReviewStateProvider'; import {useSubjectQuote} from './subjectQuote'; import {commentsWithOutdatedQuote} from './outdatedQuotes'; import {useMarkThreadReadWhenSeen} from './markThreadReadWhenSeen'; +import {useUnreadComments} from './unreadComments'; import {useScrollHighlightedThreadIntoView} from './scrollHighlightedThreadIntoView'; import ChevronIcon from './images/chevron.svg'; @@ -16,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, interactive = true}) { +export function Thread({thread, collapsed: collapsedProp, onToggle, onResolve, onClick, highlighted, showNewMarker, interactive = true}) { const {t} = useI18n({locale: 'ui'}); const firstComment = thread.comments[0]; const replies = thread.comments.slice(1); @@ -28,6 +29,14 @@ 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)); + return replies.filter(reply => ids.has(reply.id)).length; + }, [newComments, replies]); + + const hidesNewReplies = repliesCollapsed && newReplyCount > 0; + // 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 // to the reply form. @@ -69,6 +78,14 @@ export function Thread({thread, collapsed: collapsedProp, onToggle, onResolve, o })} onClick={onClick} 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 && + } + {replies.length > 0 && } diff --git a/entry_types/scrolled/package/src/review/Thread.module.css b/entry_types/scrolled/package/src/review/Thread.module.css index 4334579c34..a98b46545b 100644 --- a/entry_types/scrolled/package/src/review/Thread.module.css +++ b/entry_types/scrolled/package/src/review/Thread.module.css @@ -29,6 +29,37 @@ 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 chevron button inside it. */ +.newDot { + position: absolute; + top: space(-1); + right: space(-1); + width: space(2); + height: space(2); + border-radius: 50%; + background: var(--ui-warning-color); + box-shadow: 0 0 0 1px var(--ui-on-surface-color-lighter); +} + +.replyCount { + display: flex; + align-items: center; + gap: space(2); +} + +.newReplyCount::before { + content: '·'; + margin-right: space(2); + color: var(--ui-on-surface-color-light); +} + +.newReplyCount { + color: var(--ui-warning-color); + font-weight: 600; +} + .deletedHint { margin: 0; font-size: space(3); @@ -69,14 +100,14 @@ padding: 0 0 0 space(8); font: inherit; font-weight: 500; - color: var(--ui-primary-color); + color: var(--ui-on-surface-color-light); background: none; border: none; cursor: pointer; } .expandButton:hover { - text-decoration: underline; + color: var(--ui-primary-color); } .resolveRow { diff --git a/entry_types/scrolled/package/src/review/ThreadList.js b/entry_types/scrolled/package/src/review/ThreadList.js index c735bbc4d2..e9b8dfff5c 100644 --- a/entry_types/scrolled/package/src/review/ThreadList.js +++ b/entry_types/scrolled/package/src/review/ThreadList.js @@ -86,6 +86,7 @@ export function ThreadList({subjectType, subjectId, subjectRange, filter, highli 1 && expandedThreadId !== thread.id} + showNewMarker={activeThreads.length > 1} onToggle={() => toggleThread(thread.id)} onResolve={() => postUpdateThreadMessage({threadId: thread.id, resolved: true})} onClick={onThreadClick && (() => onThreadClick(thread))} @@ -106,6 +107,7 @@ export function ThreadList({subjectType, subjectId, subjectRange, filter, highli 1 && expandedThreadId !== thread.id} + showNewMarker={resolvedThreads.length > 1} onToggle={() => toggleThread(thread.id)} onResolve={() => postUpdateThreadMessage({threadId: thread.id, resolved: false})} onClick={onThreadClick && (() => onThreadClick(thread))} From 31ac45c8dc5d376ae8fc8d4646e1f2474cbe8da2 Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Wed, 19 Aug 2026 13:39:42 +0200 Subject: [PATCH 11/18] Separate unseen replies with a divider Marks where the unseen part of an expanded thread starts. Left out when the thread is new all through, since the thread marker already says so and a divider at the very top would only repeat it. --- entry_types/scrolled/config/locales/de.yml | 1 + entry_types/scrolled/config/locales/en.yml | 1 + .../Thread/features/newRepliesDivider-spec.js | 108 ++++++++++++++++++ .../scrolled/package/src/review/Thread.js | 25 +++- .../package/src/review/Thread.module.css | 17 +++ 5 files changed, 148 insertions(+), 4 deletions(-) create mode 100644 entry_types/scrolled/package/spec/review/Thread/features/newRepliesDivider-spec.js diff --git a/entry_types/scrolled/config/locales/de.yml b/entry_types/scrolled/config/locales/de.yml index e7ea0afe5d..e72cfb83b6 100644 --- a/entry_types/scrolled/config/locales/de.yml +++ b/entry_types/scrolled/config/locales/de.yml @@ -2001,6 +2001,7 @@ de: new_reply_count: one: 1 neu other: '%{count} neu' + new_replies: Neue 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 d5d8bb478e..c6ce56ebf8 100644 --- a/entry_types/scrolled/config/locales/en.yml +++ b/entry_types/scrolled/config/locales/en.yml @@ -1829,6 +1829,7 @@ en: new_reply_count: one: 1 new other: '%{count} new' + new_replies: New 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/newRepliesDivider-spec.js b/entry_types/scrolled/package/spec/review/Thread/features/newRepliesDivider-spec.js new file mode 100644 index 0000000000..c91f953f0c --- /dev/null +++ b/entry_types/scrolled/package/spec/review/Thread/features/newRepliesDivider-spec.js @@ -0,0 +1,108 @@ +import React from 'react'; +import '@testing-library/jest-dom/extend-expect'; +import {useFakeTranslations} from 'pageflow/testHelpers'; + +import {Thread} from 'review/Thread'; +import {renderWithReviewState} from 'support/renderWithReviewState'; + +describe('Thread new 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' + }); + + 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: 'Older reply', createdAt: '2026-08-17T09:30:00.000Z'}), + comment({ + id: 102, body: 'Newer reply', creatorId: 44, creatorName: 'Carol', + createdAt: '2026-08-17T11:00:00.000Z' + }) + ] + }; + + function render(options = {}) { + return renderWithReviewState( + , + {currentUser, commentThreads: [thread], ...options} + ); + } + + 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 seen = getByText('Older reply'); + const unseen = getByText('Newer reply'); + + expect(divider.compareDocumentPosition(seen) & Node.DOCUMENT_POSITION_PRECEDING) + .toBeTruthy(); + expect(divider.compareDocumentPosition(unseen) & Node.DOCUMENT_POSITION_FOLLOWING) + .toBeTruthy(); + }); + + 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(); + }); + + // 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(); + }); + + it('renders no divider while replies are collapsed', () => { + const {queryByText} = renderWithReviewState( + , + { + currentUser, + commentThreads: [thread], + commentThreadReads: {5: '2026-08-17T10:00:00.000Z'} + } + ); + + expect(queryByText('New replies')).toBeNull(); + }); + + it('renders no divider before the current user own replies', () => { + const threadWithOwnReply = { + ...thread, + comments: [ + thread.comments[0], + thread.comments[1], + comment({ + id: 102, body: 'Will fix', creatorId: currentUser.id, creatorName: 'Alice', + createdAt: '2026-08-17T11:00:00.000Z' + }) + ] + }; + + const {queryByText} = renderWithReviewState( + , + { + currentUser, + commentThreads: [threadWithOwnReply], + commentThreadReads: {5: '2026-08-17T10:00:00.000Z'} + } + ); + + expect(queryByText('New replies')).toBeNull(); + }); +}); diff --git a/entry_types/scrolled/package/src/review/Thread.js b/entry_types/scrolled/package/src/review/Thread.js index acab197278..1d21c33664 100644 --- a/entry_types/scrolled/package/src/review/Thread.js +++ b/entry_types/scrolled/package/src/review/Thread.js @@ -37,6 +37,18 @@ export function Thread({thread, collapsed: collapsedProp, onToggle, onResolve, o const hidesNewReplies = repliesCollapsed && newReplyCount > 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) { + return null; + } + + const ids = new Set(newComments.map(comment => comment.id)); + return replies.find(reply => ids.has(reply.id))?.id; + }, [newComments, 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 // to the reply form. @@ -116,10 +128,15 @@ export function Thread({thread, collapsed: collapsedProp, onToggle, onResolve, o } {!collapsed && replies.map(comment => ( - + + {comment.id === firstNewReplyId && +
+ {t('pageflow_scrolled.review.new_replies')} +
} + + ))} {interactive && !thread.resolvedAt && !repliesCollapsed && !editing && diff --git a/entry_types/scrolled/package/src/review/Thread.module.css b/entry_types/scrolled/package/src/review/Thread.module.css index a98b46545b..b4d83849ae 100644 --- a/entry_types/scrolled/package/src/review/Thread.module.css +++ b/entry_types/scrolled/package/src/review/Thread.module.css @@ -60,6 +60,23 @@ font-weight: 600; } +.newRepliesDivider { + display: flex; + align-items: center; + gap: space(2); + font-weight: 500; + color: var(--ui-warning-color); +} + +.newRepliesDivider::before, +.newRepliesDivider::after { + content: ''; + flex: 1; + height: 1px; + background: var(--ui-warning-color); + opacity: 0.4; +} + .deletedHint { margin: 0; font-size: space(3); From d8cf798a0c17180abe478ad4e9ff6cfd4f10b17f Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Wed, 19 Aug 2026 13:58:38 +0200 Subject: [PATCH 12/18] Keep badges with unseen comments from collapsing Badges in the editor preview collapse to a bare dot when their element is not the current one. Carrying the unread dot on top of that would show two dots, and a subject with unseen comments is worth the space of the full badge. --- .../package/spec/review/ThreadsBadge-spec.js | 22 +++++++++++++++++++ .../scrolled/package/src/review/Badge.js | 8 ++++--- 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/entry_types/scrolled/package/spec/review/ThreadsBadge-spec.js b/entry_types/scrolled/package/spec/review/ThreadsBadge-spec.js index 5a14c2989d..1a9575fa2d 100644 --- a/entry_types/scrolled/package/spec/review/ThreadsBadge-spec.js +++ b/entry_types/scrolled/package/spec/review/ThreadsBadge-spec.js @@ -267,6 +267,28 @@ describe('ThreadsBadge', () => { expect(getByRole('status')).toBeInTheDocument(); expect(getByRole('status')).not.toHaveTextContent(/\d/); }); + + // Two dots on one badge say less than one, so unseen comments keep + // the badge from collapsing. + it('renders full badge when comments are unread', () => { + const {container, getByRole} = renderThreadsBadge( + , + { + currentUser: {id: 42, name: 'Alice'}, + commentThreads: [ + { + id: 1, permaId: 5, subjectType: 'ContentElement', subjectId: 10, + comments: [{ + id: 100, creatorId: 43, createdAt: '2026-08-17T11:00:00.000Z' + }] + } + ] + } + ); + + expect(getByRole('status')).not.toHaveClass(badgeStyles.dot); + expect(container.querySelector(`.${badgeStyles.icon}`)).not.toBeNull(); + }); }); describe('mode active', () => { diff --git a/entry_types/scrolled/package/src/review/Badge.js b/entry_types/scrolled/package/src/review/Badge.js index 58121aae3f..ac3d2bc04d 100644 --- a/entry_types/scrolled/package/src/review/Badge.js +++ b/entry_types/scrolled/package/src/review/Badge.js @@ -7,7 +7,7 @@ import styles from './Badge.module.css'; export const Badge = forwardRef(function Badge({ counter, mode, resolved, unread, label, onClick }, ref) { - const variant = resolveVariant(mode, counter > 0); + const variant = resolveVariant(mode, counter > 0, unread); if (!variant) { return null; @@ -27,14 +27,16 @@ export const Badge = forwardRef(function Badge({ ); }); -function resolveVariant(mode, hasThreads) { +function resolveVariant(mode, hasThreads, unread) { switch (mode) { case 'active': return 'active'; case 'icon': return hasThreads ? 'expanded' : 'iconOnly'; case 'dot': - return hasThreads ? 'dot' : null; + // Collapsing to a dot would leave the unread dot sitting on a dot. + // Unseen comments are worth the space of the full badge anyway. + return hasThreads ? (unread ? 'expanded' : 'dot') : null; default: return hasThreads ? 'expanded' : null; } From 538d2c6a0ffc5abe0741df1c28b589194ed9799a Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Wed, 19 Aug 2026 14:49:55 +0200 Subject: [PATCH 13/18] Count comments as unread only from a baseline per user Threads carry no read records until someone reads them, so every comment that already existed would show up as unread the moment read tracking starts. Users get a baseline instead: the migration sets it to rollout time for existing users, and creation time for everyone after, which keeps the backlog from turning unread for people joining later too. --- .../comment_threads/index.json.jbuilder | 1 + ...0_add_unread_comments_since_at_to_users.rb | 18 ++++++++ .../spec/review/unreadComments-spec.js | 42 +++++++++++++++++++ .../package/src/review/unreadComments.js | 14 ++++++- lib/pageflow/user_mixin.rb | 8 ++++ .../review/comment_threads_controller_spec.rb | 13 ++++++ spec/models/user_spec.rb | 29 +++++++++++++ 7 files changed, 123 insertions(+), 2 deletions(-) create mode 100644 db/migrate/20260819000000_add_unread_comments_since_at_to_users.rb create mode 100644 spec/models/user_spec.rb diff --git a/app/views/pageflow/review/comment_threads/index.json.jbuilder b/app/views/pageflow/review/comment_threads/index.json.jbuilder index 5e948c26c8..ad38046c37 100644 --- a/app/views/pageflow/review/comment_threads/index.json.jbuilder +++ b/app/views/pageflow/review/comment_threads/index.json.jbuilder @@ -3,6 +3,7 @@ json.key_format!(camelize: :lower) json.current_user do json.id current_user.id json.name current_user.full_name + json.unread_comments_since_at current_user.unread_comments_since_at end json.comment_threads(@comment_threads) do |comment_thread| diff --git a/db/migrate/20260819000000_add_unread_comments_since_at_to_users.rb b/db/migrate/20260819000000_add_unread_comments_since_at_to_users.rb new file mode 100644 index 0000000000..dfba5a9491 --- /dev/null +++ b/db/migrate/20260819000000_add_unread_comments_since_at_to_users.rb @@ -0,0 +1,18 @@ +class AddUnreadCommentsSinceAtToUsers < ActiveRecord::Migration[7.1] + class MigratedUser < ActiveRecord::Base + self.table_name = 'users' + end + + def up + add_column :users, :unread_comments_since_at, :datetime + + # Comments written before the feature existed have not gone unread: + # without a baseline, every one of them would turn up as unread for + # every user at once. + MigratedUser.update_all(unread_comments_since_at: Time.current) + end + + def down + remove_column :users, :unread_comments_since_at + end +end diff --git a/entry_types/scrolled/package/spec/review/unreadComments-spec.js b/entry_types/scrolled/package/spec/review/unreadComments-spec.js index 3cac27867c..b02437e3b6 100644 --- a/entry_types/scrolled/package/spec/review/unreadComments-spec.js +++ b/entry_types/scrolled/package/spec/review/unreadComments-spec.js @@ -53,6 +53,48 @@ describe('unreadComments', () => { expect(result).toEqual([]); }); + describe('with a baseline on the current user', () => { + const joinedUser = {...currentUser, unreadCommentsSinceAt: '2026-08-17T10:00:00.000Z'}; + + it('ignores comments from before the baseline', () => { + const result = unreadComments( + thread([{id: 100, creatorId: 43, createdAt: '2026-08-17T09:00:00.000Z'}]), + {currentUser: joinedUser, readAt: undefined} + ); + + expect(result).toEqual([]); + }); + + it('returns comments from after the baseline', () => { + const result = unreadComments( + thread([{id: 100, creatorId: 43, createdAt: '2026-08-17T11:00:00.000Z'}]), + {currentUser: joinedUser, readAt: undefined} + ); + + expect(result.map(comment => comment.id)).toEqual([100]); + }); + + // A thread read after the baseline has moved past it. + it('prefers a later read timestamp over the baseline', () => { + const result = unreadComments( + thread([{id: 100, creatorId: 43, createdAt: '2026-08-17T11:00:00.000Z'}]), + {currentUser: joinedUser, readAt: '2026-08-17T12:00:00.000Z'} + ); + + expect(result).toEqual([]); + }); + + // A thread last read before the baseline says nothing newer than it. + it('prefers the baseline over an earlier read timestamp', () => { + const result = unreadComments( + thread([{id: 100, creatorId: 43, createdAt: '2026-08-17T09:30:00.000Z'}]), + {currentUser: joinedUser, readAt: '2026-08-17T09:00:00.000Z'} + ); + + expect(result).toEqual([]); + }); + }); + it('compares timestamps of different time zone offsets', () => { const result = unreadComments( thread([{id: 100, creatorId: 43, createdAt: '2026-08-17T12:00:00.000+02:00'}]), diff --git a/entry_types/scrolled/package/src/review/unreadComments.js b/entry_types/scrolled/package/src/review/unreadComments.js index be1e278098..38c0f89c11 100644 --- a/entry_types/scrolled/package/src/review/unreadComments.js +++ b/entry_types/scrolled/package/src/review/unreadComments.js @@ -53,16 +53,26 @@ export function useLiveUnreadComments(thread) { // 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 +// 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 // thread as new. export function unreadComments(thread, {currentUser, readAt}) { if (!currentUser) return []; - const readAtTime = readAt ? new Date(readAt).getTime() : null; + const seenUpTo = latestTime([readAt, currentUser.unreadCommentsSinceAt]); return thread.comments.filter( comment => comment.creatorId !== currentUser.id && - (readAtTime === null || new Date(comment.createdAt).getTime() > readAtTime) + (seenUpTo === null || new Date(comment.createdAt).getTime() > seenUpTo) ); } + +function latestTime(timestamps) { + const times = timestamps.filter(Boolean).map(timestamp => new Date(timestamp).getTime()); + + return times.length ? Math.max(...times) : null; +} diff --git a/lib/pageflow/user_mixin.rb b/lib/pageflow/user_mixin.rb index f1629698c3..ab6702a321 100644 --- a/lib/pageflow/user_mixin.rb +++ b/lib/pageflow/user_mixin.rb @@ -31,6 +31,14 @@ module UserMixin validates_inclusion_of :locale, in: Pageflow.config.available_locales.map(&:to_s) scope :admins, -> { where(admin: true) } + + before_create :ensure_unread_comments_since_at + end + + # Comments predating a user are not new to them. Without a baseline, + # joining would mean facing every comment ever written as unread. + def ensure_unread_comments_since_at + self.unread_comments_since_at ||= Time.current end def admin? diff --git a/spec/controllers/pageflow/review/comment_threads_controller_spec.rb b/spec/controllers/pageflow/review/comment_threads_controller_spec.rb index dded8b67b7..a6879ea04b 100644 --- a/spec/controllers/pageflow/review/comment_threads_controller_spec.rb +++ b/spec/controllers/pageflow/review/comment_threads_controller_spec.rb @@ -113,6 +113,19 @@ module Pageflow end end + it 'includes the unread baseline of the current user' do + unread_comments_since_at = 2.hours.ago + user = create(:user, unread_comments_since_at:) + entry = create(:entry, with_previewer: user) + + sign_in(user, scope: :user) + get(:index, params: {entry_id: entry.id}, format: 'json') + + baseline = JSON.parse(response.body)['currentUser']['unreadCommentsSinceAt'] + + expect(Time.zone.parse(baseline)).to eq(unread_comments_since_at) + end + it 'includes read timestamps of current user by thread perma id' do user = create(:user) entry = create(:entry, with_previewer: user) diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb new file mode 100644 index 0000000000..d09b371326 --- /dev/null +++ b/spec/models/user_spec.rb @@ -0,0 +1,29 @@ +require 'spec_helper' + +describe User do + describe 'unread_comments_since_at' do + it 'is set to creation time' do + user = create(:user) + + expect(user.unread_comments_since_at).to eq(Time.current) + end + + it 'is not overwritten when given' do + unread_comments_since_at = 2.hours.ago + + user = create(:user, unread_comments_since_at:) + + expect(user.unread_comments_since_at).to eq(unread_comments_since_at) + end + + it 'stays put when the user is updated' do + user = create(:user) + + Timecop.freeze(1.hour.from_now) do + user.update!(first_name: 'Renamed') + end + + expect(user.reload.unread_comments_since_at).to eq(user.created_at) + end + end +end From e22af0ab8278a6fbe87e2b621ec747e9c7af6e65 Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Wed, 19 Aug 2026 15:03:02 +0200 Subject: [PATCH 14/18] Point at unseen comments from the main menu The comments item at the sidebar root shows an indicator dot while the entry holds comments the user has not seen, so they do not have to open the comments view to find out. Adds an indicatorAttribute option to registerMainMenuItem rather than a comment specific hook, and keeps an entry attribute in sync with the review session to drive it. --- .../stylesheets/pageflow/editor/menu.scss | 14 ++ .../spec/review/watchUnreadComments-spec.js | 120 ++++++++++++++++++ .../scrolled/package/src/editor/config.js | 3 +- .../src/editor/models/ScrolledEntry/index.js | 2 + .../scrolled/package/src/review/index.js | 1 + .../package/src/review/watchUnreadComments.js | 31 +++++ .../spec/editor/views/EditEntryView-spec.js | 40 ++++++ package/src/editor/api/index.js | 2 + package/src/editor/views/EditEntryView.js | 15 +++ 9 files changed, 227 insertions(+), 1 deletion(-) create mode 100644 entry_types/scrolled/package/spec/review/watchUnreadComments-spec.js create mode 100644 entry_types/scrolled/package/src/review/watchUnreadComments.js diff --git a/app/assets/stylesheets/pageflow/editor/menu.scss b/app/assets/stylesheets/pageflow/editor/menu.scss index 394895758a..392036aeee 100644 --- a/app/assets/stylesheets/pageflow/editor/menu.scss +++ b/app/assets/stylesheets/pageflow/editor/menu.scss @@ -15,4 +15,18 @@ ul.menu { display: block; padding: space(2.5); } + + // Set right after the label rather than at the edge of the item, so + // it reads as belonging to the label it marks. + a.indicator::after { + content: ""; + display: inline-block; + position: relative; + top: space(-2); + margin-left: space(0.5); + width: space(1); + height: space(1); + border-radius: 50%; + background: var(--ui-warning-color); + } } diff --git a/entry_types/scrolled/package/spec/review/watchUnreadComments-spec.js b/entry_types/scrolled/package/spec/review/watchUnreadComments-spec.js new file mode 100644 index 0000000000..d77df4eb11 --- /dev/null +++ b/entry_types/scrolled/package/spec/review/watchUnreadComments-spec.js @@ -0,0 +1,120 @@ +import BackboneEvents from 'backbone-events-standalone'; +import {Model} from 'backbone'; + +import {watchUnreadComments} from 'review/watchUnreadComments'; + +describe('watchUnreadComments', () => { + const currentUser = {id: 42, name: 'Alice'}; + + function fakeSession(state = null) { + const session = {state}; + Object.assign(session, BackboneEvents); + return session; + } + + function thread(attributes = {}) { + return { + id: 1, + permaId: 5, + comments: [{ + id: 100, creatorId: 43, createdAt: '2026-08-17T11:00:00.000Z' + }], + ...attributes + }; + } + + function watch(state) { + const entry = new Model(); + const session = fakeSession(state); + + watchUnreadComments({entry, session}); + + return {entry, session}; + } + + it('is false before the session has state', () => { + const {entry} = watch(null); + + expect(entry.get('hasUnreadComments')).toBe(false); + }); + + it('is true while a thread holds unseen comments', () => { + const {entry} = watch({ + currentUser, + commentThreads: [thread()], + commentThreadReads: {} + }); + + expect(entry.get('hasUnreadComments')).toBe(true); + }); + + it('is false once every comment has been read', () => { + const {entry} = watch({ + currentUser, + commentThreads: [thread()], + commentThreadReads: {5: '2026-08-17T12:00:00.000Z'} + }); + + expect(entry.get('hasUnreadComments')).toBe(false); + }); + + it('is false for own comments', () => { + const {entry} = watch({ + currentUser, + commentThreads: [thread({ + comments: [{id: 100, creatorId: currentUser.id, createdAt: '2026-08-17T11:00:00.000Z'}] + })], + commentThreadReads: {} + }); + + expect(entry.get('hasUnreadComments')).toBe(false); + }); + + it('follows threads arriving with a fetch', () => { + const {entry, session} = watch(null); + + session.state = { + currentUser, + commentThreads: [thread()], + commentThreadReads: {} + }; + session.trigger('reset', session.state); + + expect(entry.get('hasUnreadComments')).toBe(true); + }); + + it('follows threads being read', () => { + const {entry, session} = watch({ + currentUser, + commentThreads: [thread()], + commentThreadReads: {} + }); + + session.state = { + ...session.state, + commentThreadReads: {5: '2026-08-17T12:00:00.000Z'} + }; + session.trigger('change:reads', session.state.commentThreadReads); + + expect(entry.get('hasUnreadComments')).toBe(false); + }); + + it('follows replies being added', () => { + const {entry, session} = watch({ + currentUser, + commentThreads: [thread()], + commentThreadReads: {5: '2026-08-17T12:00:00.000Z'} + }); + + const updatedThread = thread({ + comments: [ + ...thread().comments, + {id: 101, creatorId: 44, createdAt: '2026-08-17T13:00:00.000Z'} + ] + }); + session.state = {...session.state, commentThreads: [updatedThread]}; + session.trigger('change:thread', updatedThread); + + expect(entry.get('hasUnreadComments')).toBe(true); + }); +}); diff --git a/entry_types/scrolled/package/src/editor/config.js b/entry_types/scrolled/package/src/editor/config.js index 4a258fd210..056eb53672 100644 --- a/entry_types/scrolled/package/src/editor/config.js +++ b/entry_types/scrolled/package/src/editor/config.js @@ -63,7 +63,8 @@ editor.addInitializer(() => { editor.registerMainMenuItem({ translationKey: 'pageflow_scrolled.editor.main_menu.comments', path: '/scrolled/comments', - id: 'comments' + id: 'comments', + indicatorAttribute: 'hasUnreadComments' }); } }); diff --git a/entry_types/scrolled/package/src/editor/models/ScrolledEntry/index.js b/entry_types/scrolled/package/src/editor/models/ScrolledEntry/index.js index 63a49cff12..41f8b9d00e 100644 --- a/entry_types/scrolled/package/src/editor/models/ScrolledEntry/index.js +++ b/entry_types/scrolled/package/src/editor/models/ScrolledEntry/index.js @@ -2,6 +2,7 @@ import {Entry, editor} from 'pageflow/editor'; import I18n from 'i18n-js'; import {features} from 'pageflow/frontend'; import {createReviewSession} from 'pageflow/review'; +import {watchUnreadComments} from 'pageflow-scrolled/review'; import {ConsentVendors} from '../ConsentVendors'; @@ -81,6 +82,7 @@ export const ScrolledEntry = Entry.extend({ if (features.isEnabled('commenting')) { this.reviewSession = createReviewSession({entryId: this.id}); + watchUnreadComments({entry: this, session: this.reviewSession}); } }, diff --git a/entry_types/scrolled/package/src/review/index.js b/entry_types/scrolled/package/src/review/index.js index dcdd2bb1ae..ee9f81f35b 100644 --- a/entry_types/scrolled/package/src/review/index.js +++ b/entry_types/scrolled/package/src/review/index.js @@ -3,6 +3,7 @@ export {ReviewStateProvider, useCommentThreads, useCommentThread} from './Review export {LocatedCommentThreadsProvider, useLocatedCommentThreads} from './useLocatedCommentThreads'; export {useLocatedCommentThreadsForSubject} from './useLocatedCommentThreadsForSubject'; export {ReviewMessageHandler} from './ReviewMessageHandler'; +export {watchUnreadComments} from './watchUnreadComments'; export {ThreadsBadge} from './ThreadsBadge'; export {Badge} from './Badge'; export {ThreadList} from './ThreadList'; diff --git a/entry_types/scrolled/package/src/review/watchUnreadComments.js b/entry_types/scrolled/package/src/review/watchUnreadComments.js new file mode 100644 index 0000000000..fdc3a3f9ea --- /dev/null +++ b/entry_types/scrolled/package/src/review/watchUnreadComments.js @@ -0,0 +1,31 @@ +import {unreadComments} from './unreadComments'; + +// Keeps an entry attribute in sync with whether the review session holds +// comments the user has not seen, so that the comments main menu item can +// point at them from the root of the sidebar. +export function watchUnreadComments({entry, session}) { + function update() { + entry.set('hasUnreadComments', hasUnreadComments(session.state)); + } + + session.on('reset', update); + session.on('change:thread', update); + session.on('change:reads', update); + + update(); +} + +function hasUnreadComments(state) { + if (!state) { + return false; + } + + const {currentUser, commentThreads, commentThreadReads = {}} = state; + + return commentThreads.some( + thread => unreadComments(thread, { + currentUser, + readAt: commentThreadReads[thread.permaId] + }).length > 0 + ); +} diff --git a/package/spec/editor/views/EditEntryView-spec.js b/package/spec/editor/views/EditEntryView-spec.js index 8cfe2294e7..c11c9ea330 100644 --- a/package/spec/editor/views/EditEntryView-spec.js +++ b/package/spec/editor/views/EditEntryView-spec.js @@ -36,4 +36,44 @@ describe('EditEntryView', () => { expect(item).toHaveText('some translation'); }); + + describe('menu item indicator', () => { + function renderMenuItem(entry) { + editor.registerMainMenuItem({ + translationKey: 'some.key', + id: 'some-id', + indicatorAttribute: 'somethingUnseen' + }); + const view = new EditEntryView({model: entry}); + + view.render(); + + return view.$el.find('[data-main-menu-item="some-id"]'); + } + + it('is absent while the entry attribute is falsy', () => { + const item = renderMenuItem(factories.entry()); + + expect(item).not.toHaveClass('indicator'); + }); + + it('is present while the entry attribute is truthy', () => { + const item = renderMenuItem(factories.entry({somethingUnseen: true})); + + expect(item).toHaveClass('indicator'); + }); + + it('follows changes of the entry attribute', () => { + const entry = factories.entry(); + const item = renderMenuItem(entry); + + entry.set('somethingUnseen', true); + + expect(item).toHaveClass('indicator'); + + entry.set('somethingUnseen', false); + + expect(item).not.toHaveClass('indicator'); + }); + }); }); diff --git a/package/src/editor/api/index.js b/package/src/editor/api/index.js index 07f6304977..b59060f09b 100644 --- a/package/src/editor/api/index.js +++ b/package/src/editor/api/index.js @@ -190,6 +190,8 @@ export const EditorApi = Object.extend( * - translationKey: for the label * - path: route to link to * - click: click handler + * - indicatorAttribute: name of an entry attribute. While it is + * truthy, the item displays an indicator dot. */ registerMainMenuItem: function(options) { this.mainMenuItems.push(options); diff --git a/package/src/editor/views/EditEntryView.js b/package/src/editor/views/EditEntryView.js index b97b1e90fb..21b1ade964 100644 --- a/package/src/editor/views/EditEntryView.js +++ b/package/src/editor/views/EditEntryView.js @@ -98,7 +98,22 @@ export const EditEntryView = Marionette.Layout.extend({ $(link).click(options.click); } + if (options.indicatorAttribute) { + view._bindMenuItemIndicator(link, options.indicatorAttribute); + } + view.ui.menu.append(item); }); + }, + + _bindMenuItemIndicator: function(link, attribute) { + var view = this; + + var update = function() { + link.toggleClass('indicator', !!view.model.get(attribute)); + }; + + this.listenTo(this.model, 'change:' + attribute, update); + update(); } }); From 86f5a9ef570efbe855ed8c34f77ec9bcf6aa77af Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Wed, 19 Aug 2026 15:38:09 +0200 Subject: [PATCH 15/18] Restrict relative imports of review code The review interface is bundled separately just like the frontend and entry state, so importing it relatively inlines a second copy along with its React contexts. The rule listed only the other two, which is how such an import reached the editor unnoticed. --- entry_types/scrolled/package/.eslintrc.js | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/entry_types/scrolled/package/.eslintrc.js b/entry_types/scrolled/package/.eslintrc.js index 52e75ca2cb..18d41ca91b 100644 --- a/entry_types/scrolled/package/.eslintrc.js +++ b/entry_types/scrolled/package/.eslintrc.js @@ -34,10 +34,10 @@ module.exports = { { // Content elements, widgets, the editor and the review interface // are bundled separately and need to import from - // 'pageflow-scrolled/frontend' and 'pageflow-scrolled/entryState' - // to keep that code external. Importing relatively would inline a - // second copy of the module, giving the bundle its own React - // contexts. + // 'pageflow-scrolled/frontend', 'pageflow-scrolled/entryState' and + // 'pageflow-scrolled/review' to keep that code external. Importing + // relatively would inline a second copy of the module, giving the + // bundle its own React contexts. // Stories are excluded because they legitimately import the // content element's sibling './frontend' aggregator via // '../frontend'. @@ -52,7 +52,8 @@ module.exports = { "no-restricted-imports": ["error", { "patterns": [ "**/frontend/**", "../**/frontend", - "**/entryState/**", "../**/entryState" + "**/entryState/**", "../**/entryState", + "**/review/**", "../**/review" ] }] } From 2fb921e4a793176ef3d5f455354a458062ecb8b7 Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Wed, 19 Aug 2026 16:42:54 +0200 Subject: [PATCH 16/18] Summarize comment activity of an entry Counts unresolved topics and the comments the user has not seen for a whole page of entries at once, so a list can show an indicator without querying per row. --- app/models/pageflow/entry_comment_summary.rb | 88 ++++++++++ .../pageflow/entry_comment_summary_spec.rb | 155 ++++++++++++++++++ 2 files changed, 243 insertions(+) create mode 100644 app/models/pageflow/entry_comment_summary.rb create mode 100644 spec/models/pageflow/entry_comment_summary_spec.rb diff --git a/app/models/pageflow/entry_comment_summary.rb b/app/models/pageflow/entry_comment_summary.rb new file mode 100644 index 0000000000..10e1674f85 --- /dev/null +++ b/app/models/pageflow/entry_comment_summary.rb @@ -0,0 +1,88 @@ +module Pageflow + # Counts of comment topics and comments the user has not seen, for + # displaying an indicator next to an entry in lists of entries. + # + # Built for a whole page of entries at once: rendering a list must not + # query per row. + # + # @api private + class EntryCommentSummary + attr_reader :topic_count, :new_topic_count, :new_reply_count + + def self.for_entries(entries, user:) + entries = entries.to_a + return {} if entries.empty? + + threads = unresolved_threads_by_entry_id(entries) + read_at = read_at_by_entry_id(entries, user) + + entries.to_h do |entry| + [entry.id, build(threads.fetch(entry.id, []), + read_at: read_at.fetch(entry.id, {}), + user:)] + end + end + + def initialize(topic_count:, new_topic_count:, new_reply_count:) + @topic_count = topic_count + @new_topic_count = new_topic_count + @new_reply_count = new_reply_count + end + + def any? + topic_count.positive? + end + + def new? + new_topic_count.positive? || new_reply_count.positive? + end + + # Comment threads live on the draft revision, so entries are reached + # through their editable revision rather than directly. + def self.unresolved_threads_by_entry_id(entries) + entry_id_by_revision_id = + Revision.editable.where(entry_id: entries.map(&:id)).pluck(:id, :entry_id).to_h + + CommentThread + .where(revision_id: entry_id_by_revision_id.keys, resolved_at: nil) + .includes(:comments) + .group_by { |thread| entry_id_by_revision_id[thread.revision_id] } + end + private_class_method :unresolved_threads_by_entry_id + + def self.read_at_by_entry_id(entries, user) + CommentThreadRead + .where(user:, entry_id: entries.map(&:id)) + .pluck(:entry_id, :comment_thread_perma_id, :read_at) + .group_by(&:first) + .transform_values do |rows| + rows.to_h { |(_entry_id, perma_id, read_at)| [perma_id, read_at] } + end + end + private_class_method :read_at_by_entry_id + + def self.build(threads, read_at:, user:) + new_topics = 0 + new_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) } + end + + new(topic_count: threads.size, new_topic_count: new_topics, new_reply_count: new_replies) + end + private_class_method :build + + # Mirrors the unread rule of the review interface: own comments never + # count, and neither do comments from before the user's baseline. + def self.unread?(comment, seen_up_to, user) + comment.creator_id != user.id && + (seen_up_to.nil? || comment.created_at > seen_up_to) + end + private_class_method :unread? + end +end diff --git a/spec/models/pageflow/entry_comment_summary_spec.rb b/spec/models/pageflow/entry_comment_summary_spec.rb new file mode 100644 index 0000000000..6f03d1fddb --- /dev/null +++ b/spec/models/pageflow/entry_comment_summary_spec.rb @@ -0,0 +1,155 @@ +require 'spec_helper' + +module Pageflow + describe EntryCommentSummary do + describe '.for_entries' do + def summary_for(entry, user) + described_class.for_entries([entry], user:)[entry.id] + end + + it 'is empty without entries' do + expect(described_class.for_entries([], user: create(:user))).to eq({}) + end + + it 'counts unresolved threads of the draft revision as topics' do + user = create(:user) + entry = create(:entry) + create(:comment_thread, revision: entry.draft) + create(:comment_thread, revision: entry.draft) + + expect(summary_for(entry, user).topic_count).to eq(2) + end + + it 'ignores resolved threads' do + user = create(:user) + entry = create(:entry) + create(:comment_thread, revision: entry.draft, resolved_at: Time.current) + + expect(summary_for(entry, user).topic_count).to eq(0) + end + + it 'ignores threads of other entries' do + user = create(:user) + entry = create(:entry) + create(:comment_thread, revision: create(:entry).draft) + + expect(summary_for(entry, user).topic_count).to eq(0) + end + + it 'counts threads whose first comment is unseen as new topics' do + user = create(:user, unread_comments_since_at: 3.hours.ago) + entry = create(:entry) + thread = create(:comment_thread, revision: entry.draft) + create(:comment, comment_thread: thread, creator: create(:user)) + + summary = summary_for(entry, user) + + expect(summary.new_topic_count).to eq(1) + expect(summary.new_reply_count).to eq(0) + end + + it 'counts unseen comments after the first as new replies' do + user = create(:user, unread_comments_since_at: 3.hours.ago) + entry = create(:entry) + thread = create(:comment_thread, revision: entry.draft) + create(:comment, comment_thread: thread, creator: create(:user)) + create(:comment, comment_thread: thread, creator: create(:user)) + + summary = summary_for(entry, user) + + expect(summary.new_topic_count).to eq(1) + expect(summary.new_reply_count).to eq(1) + end + + it 'ignores comments the user wrote' do + user = create(:user, unread_comments_since_at: 3.hours.ago) + entry = create(:entry) + thread = create(:comment_thread, revision: entry.draft) + create(:comment, comment_thread: thread, creator: user) + + expect(summary_for(entry, user).new_topic_count).to eq(0) + end + + it 'ignores comments from before the users baseline' do + user = create(:user, unread_comments_since_at: Time.current) + entry = create(:entry) + thread = create(:comment_thread, revision: entry.draft) + + Timecop.freeze(2.hours.ago) do + create(:comment, comment_thread: thread, creator: create(:user)) + end + + expect(summary_for(entry, user).new_topic_count).to eq(0) + end + + it 'ignores comments from before the thread was read' do + user = create(:user, unread_comments_since_at: 3.hours.ago) + entry = create(:entry) + thread = create(:comment_thread, revision: entry.draft) + + Timecop.freeze(2.hours.ago) do + create(:comment, comment_thread: thread, creator: create(:user)) + end + + create(:comment_thread_read, + entry:, + user:, + comment_thread_perma_id: thread.perma_id, + read_at: 1.hour.ago) + + expect(summary_for(entry, user).new_topic_count).to eq(0) + end + + it 'keeps read state of other users apart' do + user = create(:user, unread_comments_since_at: 3.hours.ago) + entry = create(:entry) + thread = create(:comment_thread, revision: entry.draft) + create(:comment, comment_thread: thread, creator: create(:user)) + create(:comment_thread_read, + entry:, + user: create(:user), + comment_thread_perma_id: thread.perma_id) + + expect(summary_for(entry, user).new_topic_count).to eq(1) + end + + it 'returns a summary per entry' do + user = create(:user, unread_comments_since_at: 3.hours.ago) + entry = create(:entry) + other_entry = create(:entry) + create(:comment_thread, revision: entry.draft) + + result = described_class.for_entries([entry, other_entry], user:) + + expect(result[entry.id].topic_count).to eq(1) + expect(result[other_entry.id].topic_count).to eq(0) + end + + it 'does not have N+1 queries' do + user = create(:user) + entries = Array.new(3) { create(:entry) } + entries.each do |entry| + thread = create(:comment_thread, revision: entry.draft) + create(:comment, comment_thread: thread, creator: create(:user)) + create(:comment, comment_thread: thread, creator: create(:user)) + end + + detect_n_plus_one_queries do + described_class.for_entries(entries, user:) + end + end + end + + describe '#new?' do + it 'is true with new topics or new replies' do + expect(build_summary(new_topic_count: 1)).to be_new + expect(build_summary(new_reply_count: 1)).to be_new + expect(build_summary).not_to be_new + end + + def build_summary(new_topic_count: 0, new_reply_count: 0) + described_class.new(topic_count: 1, new_topic_count:, new_reply_count:) + end + end + end +end From f68150064951d6f4ac89045d74cae2e24e35d423 Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Wed, 19 Aug 2026 16:50:52 +0200 Subject: [PATCH 17/18] Show comment activity next to entries in the admin Marks entries carrying unresolved topics with the comment icon of the review interface, and marks the indicator when comments have gone unseen. The tooltip names topics, new topics and new replies. The title cell now holds more than the link, so the index table domino reads the title from the link itself. --- admins/pageflow/entry.rb | 3 +- .../images/pageflow/admin/icons/comment.svg | 1 + app/assets/stylesheets/pageflow/admin.scss | 1 + .../admin/entry_comments_indicator.scss | 45 ++++++++++ app/helpers/pageflow/admin/entries_helper.rb | 36 ++++++++ config/locales/de.yml | 11 +++ config/locales/en.yml | 11 +++ ...seeing_comment_activity_of_entries_spec.rb | 50 +++++++++++ .../pageflow/admin/entries_helper_spec.rb | 84 +++++++++++++++++++ .../dominos/admin/entry_in_index_table.rb | 8 +- 10 files changed, 248 insertions(+), 2 deletions(-) create mode 100644 app/assets/images/pageflow/admin/icons/comment.svg create mode 100644 app/assets/stylesheets/pageflow/admin/entry_comments_indicator.scss create mode 100644 spec/features/account_previewer/seeing_comment_activity_of_entries_spec.rb diff --git a/admins/pageflow/entry.rb b/admins/pageflow/entry.rb index faccd6d9da..1f7dd6f8a3 100644 --- a/admins/pageflow/entry.rb +++ b/admins/pageflow/entry.rb @@ -9,7 +9,8 @@ module Pageflow entry_publication_state_indicator(entry) end column :title, sortable: 'title' do |entry| - link_to(entry.title, admin_entry_path(entry)) + safe_join([link_to(entry.title, admin_entry_path(entry)), + entry_comments_indicator(entry)].compact) end column I18n.t('pageflow.admin.entries.members'), class: 'members' do |entry| entry_user_badge_list(entry) diff --git a/app/assets/images/pageflow/admin/icons/comment.svg b/app/assets/images/pageflow/admin/icons/comment.svg new file mode 100644 index 0000000000..ae5ba7b3a9 --- /dev/null +++ b/app/assets/images/pageflow/admin/icons/comment.svg @@ -0,0 +1 @@ + diff --git a/app/assets/stylesheets/pageflow/admin.scss b/app/assets/stylesheets/pageflow/admin.scss index 14cb08cc3f..4ce83be388 100644 --- a/app/assets/stylesheets/pageflow/admin.scss +++ b/app/assets/stylesheets/pageflow/admin.scss @@ -25,6 +25,7 @@ $pageflow-hint-color: #666 !default; @import "pageflow/admin/embed_code"; @import "pageflow/admin/embedded_index_table"; @import "pageflow/admin/entries"; +@import "pageflow/admin/entry_comments_indicator"; @import "pageflow/admin/features"; @import "pageflow/admin/filters"; @import "pageflow/admin/forms"; diff --git a/app/assets/stylesheets/pageflow/admin/entry_comments_indicator.scss b/app/assets/stylesheets/pageflow/admin/entry_comments_indicator.scss new file mode 100644 index 0000000000..415915feb8 --- /dev/null +++ b/app/assets/stylesheets/pageflow/admin/entry_comments_indicator.scss @@ -0,0 +1,45 @@ +$pageflow-entry-comments-indicator-color: #8a5a00 !default; + +$pageflow-entry-comments-indicator-background-color: #fdf1dc !default; + +$pageflow-entry-comments-indicator-dot-color: #ff7400 !default; + +$pageflow-entry-comments-indicator-icon-directory: "pageflow/admin/icons" !default; + +.entry_comments_indicator { + $dir: $pageflow-entry-comments-indicator-icon-directory; + + position: relative; + display: inline-block; + margin-left: 8px; + padding: 1px 6px 1px 22px; + vertical-align: middle; + border-radius: 10px; + background: $pageflow-entry-comments-indicator-background-color + image-url("#{$dir}/comment.svg") no-repeat 5px center / 13px 13px; + color: $pageflow-entry-comments-indicator-color; + font-size: 0.7rem; + line-height: 1.6; + + // The tooltip claims both pseudo elements of the indicator, so the + // dot has to be an element of its own. + // + // Straddles the corner like the unread dot inside the editor, so both + // read as the same signal. + .unread_dot { + position: absolute; + top: -2px; + right: -2px; + width: 6px; + height: 6px; + border-radius: 50%; + background-color: $pageflow-entry-comments-indicator-dot-color; + } + + // The bubble is placed by its static position, which on an inline + // element sits at the far end of the content and thus away from the + // arrow anchored to the left. + &::after { + left: 0; + } +} diff --git a/app/helpers/pageflow/admin/entries_helper.rb b/app/helpers/pageflow/admin/entries_helper.rb index 4a54acf3b4..9b85aa1851 100644 --- a/app/helpers/pageflow/admin/entries_helper.rb +++ b/app/helpers/pageflow/admin/entries_helper.rb @@ -19,6 +19,42 @@ def collection_for_entry_publication_states end end + def entry_comments_indicator(entry, summaries: entry_comment_summaries) + summary = summaries[entry.id] + return unless summary&.any? + + content_tag(:span, + 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) + end + end + + # Built for the whole page at once, so that rendering a row does + # not query. Views without an index table collection pass their own + # summaries instead. + def entry_comment_summaries + @entry_comment_summaries ||= + EntryCommentSummary.for_entries(collection, user: current_user) + end + + def entry_comments_tooltip(summary) + scope = 'pageflow.admin.entries.comments' + + 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) + end + + if summary.new_reply_count.positive? + parts << t("#{scope}.new_reply_count", count: summary.new_reply_count) + end + + t("#{scope}.tooltip", summary: parts.join(', ')) + end + def entry_type_collection(entry_types = Pageflow.config.entry_types) entry_types.map(&:name).index_by do |type| I18n.t(type, scope: 'activerecord.values.pageflow/entry.type_names') diff --git a/config/locales/de.yml b/config/locales/de.yml index a093ab93c1..c8c77702ff 100644 --- a/config/locales/de.yml +++ b/config/locales/de.yml @@ -828,6 +828,17 @@ de: site_defaults_inline_help: Die folgenden Einstellungen werden als Standard für neue Beiträge des Kontos verwendet. Änderungen wirken sich nicht auf existierende Beiträge aus. 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" 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 530f2b7861..14672e825a 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -827,6 +827,17 @@ en: site_defaults_inline_help: The following settings will be used as defaults for new stories in this account. Changes do not affect existing stories. 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" 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/spec/features/account_previewer/seeing_comment_activity_of_entries_spec.rb b/spec/features/account_previewer/seeing_comment_activity_of_entries_spec.rb new file mode 100644 index 0000000000..ba62f63f73 --- /dev/null +++ b/spec/features/account_previewer/seeing_comment_activity_of_entries_spec.rb @@ -0,0 +1,50 @@ +require 'spec_helper' + +feature 'as account previewer, seeing comment activity in the entries table' do + scenario 'entry without comments shows no indicator' do + entry = create(:entry, title: 'Quiet Entry') + Dom::Admin::Page.sign_in_as(:previewer, on: entry.account) + + visit(admin_entries_path) + + expect(Dom::Admin::EntryInIndexTable.find_by_title('Quiet Entry').comments_indicator) + .to be_nil + end + + scenario 'entry with unseen comments shows marked indicator naming them' do + entry = create(:entry, title: 'Discussed Entry') + user = Dom::Admin::Page.sign_in_as(:previewer, on: entry.account) + user.update!(unread_comments_since_at: 1.day.ago) + + thread = create(:comment_thread, revision: entry.draft) + create(:comment, comment_thread: thread, creator: create(:user)) + create(:comment, comment_thread: thread, creator: create(:user)) + + visit(admin_entries_path) + indicator = Dom::Admin::EntryInIndexTable.find_by_title('Discussed Entry') + .comments_indicator + + expect(indicator).to have_text('1') + expect(indicator).to have_selector('.unread_dot') + expect(indicator['data-tooltip']) + .to eq('Comments: 1 unresolved topic, 1 new topic, 1 new reply') + end + + scenario 'entry with comments predating the user shows unmarked indicator' do + entry = create(:entry, title: 'Settled Entry') + Dom::Admin::Page.sign_in_as(:previewer, on: entry.account) + + Timecop.travel(2.days.ago) do + thread = create(:comment_thread, revision: entry.draft) + create(:comment, comment_thread: thread, creator: create(:user)) + end + + visit(admin_entries_path) + indicator = Dom::Admin::EntryInIndexTable.find_by_title('Settled Entry') + .comments_indicator + + expect(indicator).to have_text('1') + expect(indicator).to have_no_selector('.unread_dot') + expect(indicator['data-tooltip']).to eq('Comments: 1 unresolved topic') + end +end diff --git a/spec/helpers/pageflow/admin/entries_helper_spec.rb b/spec/helpers/pageflow/admin/entries_helper_spec.rb index 4e09231963..82aa17ab0e 100644 --- a/spec/helpers/pageflow/admin/entries_helper_spec.rb +++ b/spec/helpers/pageflow/admin/entries_helper_spec.rb @@ -4,6 +4,90 @@ module Pageflow module Admin describe EntriesHelper do + describe '#entry_comments_indicator' do + def render_indicator(entry, user) + allow(helper).to receive(:collection).and_return([entry]) + allow(helper).to receive(:current_user).and_return(user) + + helper.entry_comments_indicator(entry) + end + + it 'renders nothing without unresolved threads' do + user = create(:user) + entry = create(:entry) + + expect(render_indicator(entry, user)).to be_nil + end + + it 'renders the number of unresolved topics' do + user = create(:user) + entry = create(:entry) + create(:comment_thread, revision: entry.draft) + create(:comment_thread, revision: entry.draft) + + result = render_indicator(entry, user) + + expect(result).to have_selector('span.entry_comments_indicator', text: '2') + end + + it 'marks the indicator when comments are unseen' do + user = create(:user, unread_comments_since_at: 3.hours.ago) + entry = create(:entry) + thread = create(:comment_thread, revision: entry.draft) + create(:comment, comment_thread: thread, creator: create(:user)) + + result = render_indicator(entry, user) + + expect(result).to have_selector('span.entry_comments_indicator .unread_dot') + end + + it 'does not mark the indicator when everything has been seen' do + user = create(:user, unread_comments_since_at: Time.current) + entry = create(:entry) + create(:comment_thread, revision: entry.draft) + + result = render_indicator(entry, user) + + expect(result).to have_selector('span.entry_comments_indicator') + expect(result).not_to have_selector('span.entry_comments_indicator .unread_dot') + end + + it 'names the topic count in the tooltip' do + user = create(:user, unread_comments_since_at: Time.current) + entry = create(:entry) + create(:comment_thread, revision: entry.draft) + + result = render_indicator(entry, user) + + expect(result).to have_selector("[data-tooltip='Comments: 1 unresolved topic']") + end + + it 'renders summaries passed in instead of querying the collection' do + user = create(:user) + entry = create(:entry) + create(:comment_thread, revision: entry.draft) + summaries = EntryCommentSummary.for_entries([entry], user:) + + result = helper.entry_comments_indicator(entry, summaries:) + + expect(result).to have_selector('span.entry_comments_indicator', text: '1') + end + + it 'names new topics and replies in the tooltip' do + user = create(:user, unread_comments_since_at: 3.hours.ago) + entry = create(:entry) + thread = create(:comment_thread, revision: entry.draft) + create(:comment, comment_thread: thread, creator: create(:user)) + create(:comment, comment_thread: thread, creator: create(:user)) + + result = render_indicator(entry, user) + + expect(result).to have_selector( + "[data-tooltip='Comments: 1 unresolved topic, 1 new topic, 1 new reply']" + ) + end + end + describe '#entry_type_collection' do include_context 'fake translations' diff --git a/spec/support/dominos/admin/entry_in_index_table.rb b/spec/support/dominos/admin/entry_in_index_table.rb index 2669a7c48d..6b9cb02c2e 100644 --- a/spec/support/dominos/admin/entry_in_index_table.rb +++ b/spec/support/dominos/admin/entry_in_index_table.rb @@ -3,8 +3,14 @@ module Admin class EntryInIndexTable < Domino selector '.admin_entries.index .index_table tbody tr' - attribute :title, 'td.col-title' + # Scoped to the link since the title cell also carries the comment + # indicator. + attribute :title, 'td.col-title a' attribute :account_name, 'td.col-account' + + def comments_indicator + node.first('.entry_comments_indicator', minimum: 0) + end end end end From 67e5175143bc8c0dcb5c57a1e2ff97956e050d72 Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Wed, 19 Aug 2026 18:04:21 +0200 Subject: [PATCH 18/18] Mark the collapsed comment toolbar when comments are unseen Hiding the toolbar leaves nothing pointing at comments that arrived since, so the puck carries a dot in its corner and names the count. --- entry_types/scrolled/config/locales/de.yml | 3 + entry_types/scrolled/config/locales/en.yml | 3 + .../commenting/features/unreadToolbar-spec.js | 75 +++++++++++++++++++ .../spec/support/pageObjects/commenting.js | 5 +- .../frontend/commenting/FloatingToolbar.js | 15 +++- .../commenting/FloatingToolbar.module.css | 23 +++++- .../scrolled/package/src/review/index.js | 1 + 7 files changed, 117 insertions(+), 8 deletions(-) create mode 100644 entry_types/scrolled/package/spec/frontend/commenting/features/unreadToolbar-spec.js diff --git a/entry_types/scrolled/config/locales/de.yml b/entry_types/scrolled/config/locales/de.yml index e72cfb83b6..314888e115 100644 --- a/entry_types/scrolled/config/locales/de.yml +++ b/entry_types/scrolled/config/locales/de.yml @@ -1984,6 +1984,9 @@ de: cancel_add_comment: Abbrechen hide_comments: Kommentare ausblenden show_comments: Kommentare einblenden + show_comments_with_unread: + one: Kommentare einblenden (1 ungelesener Kommentar) + other: Kommentare einblenden (%{count} ungelesene Kommentare) 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 c6ce56ebf8..99c00d6ae1 100644 --- a/entry_types/scrolled/config/locales/en.yml +++ b/entry_types/scrolled/config/locales/en.yml @@ -1812,6 +1812,9 @@ en: cancel_add_comment: Cancel hide_comments: Hide comments show_comments: Show comments + show_comments_with_unread: + one: Show comments (1 unread comment) + other: Show comments (%{count} unread comments) 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 new file mode 100644 index 0000000000..3d120d752d --- /dev/null +++ b/entry_types/scrolled/package/spec/frontend/commenting/features/unreadToolbar-spec.js @@ -0,0 +1,75 @@ +import '@testing-library/jest-dom/extend-expect'; +import {fireEvent} from '@testing-library/react'; +import {useFakeTranslations} from 'pageflow/testHelpers'; + +import toolbarStyles from 'frontend/commenting/FloatingToolbar.module.css'; + +import {renderEntry, useCommentingPageObjects} from 'support/pageObjects/commenting'; + +describe('unread comments on the collapsed toolbar', () => { + useCommentingPageObjects(); + + useFakeTranslations({ + 'pageflow_scrolled.review.show_comments': 'Show comments', + 'pageflow_scrolled.review.show_comments_with_unread.one': + 'Show comments (1 unread comment)', + 'pageflow_scrolled.review.show_comments_with_unread.other': + 'Show comments (%{count} unread comments)' + }); + + const currentUser = {id: 42, name: 'Alice'}; + + function renderCollapsedEntry({comments}) { + const entry = renderEntry({ + seed: {contentElements: [{typeName: 'withTestId', configuration: {testId: 5}}]}, + commenting: { + currentUser, + commentThreads: [ + {id: 1, permaId: 7, subjectType: 'ContentElement', subjectId: 1, comments} + ], + commentThreadReads: {} + } + }); + + fireEvent.click(entry.getHideCommentsButton()); + + return entry; + } + + function comment(attributes) { + return { + id: 10, body: 'Nice work', creatorName: 'Bob', creatorId: 43, + createdAt: '2026-08-17T11:00:00.000Z', + ...attributes + }; + } + + function unreadDot(entry) { + return entry.getShowCommentsButton().querySelector(`.${toolbarStyles.unreadDot}`); + } + + it('marks the show button while comments are unseen', () => { + const entry = renderCollapsedEntry({comments: [comment()]}); + + expect(unreadDot(entry)).not.toBeNull(); + }); + + it('names the unseen 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)'); + }); + + it('leaves the show button unmarked without unseen comments', () => { + const entry = renderCollapsedEntry({ + comments: [comment({creatorId: currentUser.id, creatorName: 'Alice'})] + }); + + expect(unreadDot(entry)).toBeNull(); + expect(entry.getShowCommentsButton()) + .toHaveAttribute('aria-label', 'Show comments'); + }); +}); diff --git a/entry_types/scrolled/package/spec/support/pageObjects/commenting.js b/entry_types/scrolled/package/spec/support/pageObjects/commenting.js index 7ebac9740e..bac9dd65f0 100644 --- a/entry_types/scrolled/package/spec/support/pageObjects/commenting.js +++ b/entry_types/scrolled/package/spec/support/pageObjects/commenting.js @@ -27,8 +27,9 @@ export function renderEntry({ getCommentToolbar: () => result.getByRole('group', {name: 'Comments'}), queryCommentToolbar: () => result.queryByRole('group', {name: 'Comments'}), getHideCommentsButton: () => result.getByRole('button', {name: 'Hide comments'}), - getShowCommentsButton: () => result.getByRole('button', {name: 'Show comments'}), - queryShowCommentsButton: () => result.queryByRole('button', {name: 'Show comments'}), + // Matched by prefix since the button also names unread comments. + getShowCommentsButton: () => result.getByRole('button', {name: /^Show comments/}), + queryShowCommentsButton: () => result.queryByRole('button', {name: /^Show comments/}), getAddCommentButton: () => result.getByRole('button', {name: 'Add comment'}), getCancelAddCommentButton: () => result.getByRole('button', {name: 'Cancel add comment'}), getNewThreadInput: () => result.getByPlaceholderText('Add a comment...'), diff --git a/entry_types/scrolled/package/src/frontend/commenting/FloatingToolbar.js b/entry_types/scrolled/package/src/frontend/commenting/FloatingToolbar.js index 41c401ad77..e4483f85ab 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} from 'pageflow-scrolled/review'; +import {useLocatedCommentThreads, useUnreadCommentCount} from 'pageflow-scrolled/review'; import {useI18n} from '../i18n'; import {useAddCommentMode} from './AddCommentModeProvider'; import {useCommentDisplayFilter} from './CommentDisplayFilterProvider'; @@ -62,7 +62,17 @@ function HideCommentsButton() { function ShowCommentsButton() { const {t} = useI18n({locale: 'ui'}); const {toggle} = useCommentingVisibility(); - const label = t('pageflow_scrolled.review.show_comments'); + const {threads} = useLocatedCommentThreads(); + + const unreadCommentCount = useUnreadCommentCount(threads); + const unread = unreadCommentCount > 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}) : + t('pageflow_scrolled.review.show_comments'); return ( ); } diff --git a/entry_types/scrolled/package/src/frontend/commenting/FloatingToolbar.module.css b/entry_types/scrolled/package/src/frontend/commenting/FloatingToolbar.module.css index 9add2f8ee8..814224bded 100644 --- a/entry_types/scrolled/package/src/frontend/commenting/FloatingToolbar.module.css +++ b/entry_types/scrolled/package/src/frontend/commenting/FloatingToolbar.module.css @@ -4,10 +4,10 @@ puck together (the content deforms and cross-fades along with the rect, having no counterpart in the puck). The toggle icons (eye and bubble) share `comment-toolbar-icon` and morph on their own — naming them lifts them out - of the surface snapshot so the icon swap stays crisp. The - ::view-transition old/new overrides below stretch the surface snapshots to - fill the animating box, so it reshapes into the puck rather than scaling - down with a fixed aspect ratio. */ + of the surface snapshot so the icon swap stays crisp, and the unread dot is + named for the same reason. The ::view-transition old/new overrides below + stretch the surface snapshots to fill the animating box, so it reshapes into + the puck rather than scaling down with a fixed aspect ratio. */ .toolbar { position: fixed; bottom: space(3); @@ -74,6 +74,21 @@ animation: none; } +.unreadDot { + position: absolute; + top: space(-1); + right: space(-1); + width: space(2); + height: space(2); + border-radius: 50%; + background: var(--ui-warning-color); + box-shadow: 0 0 0 1px var(--ui-on-surface-color-lighter); + + /* Naming the dot lifts it out of the puck's surface snapshot, so it + does not deform along with the rect while the toolbar expands. */ + view-transition-name: comment-toolbar-unread-dot; +} + @keyframes puckIdle { from { opacity: 1; diff --git a/entry_types/scrolled/package/src/review/index.js b/entry_types/scrolled/package/src/review/index.js index ee9f81f35b..6a67404272 100644 --- a/entry_types/scrolled/package/src/review/index.js +++ b/entry_types/scrolled/package/src/review/index.js @@ -4,6 +4,7 @@ export {LocatedCommentThreadsProvider, useLocatedCommentThreads} from './useLoca export {useLocatedCommentThreadsForSubject} from './useLocatedCommentThreadsForSubject'; export {ReviewMessageHandler} from './ReviewMessageHandler'; export {watchUnreadComments} from './watchUnreadComments'; +export {useUnreadCommentCount} from './unreadComments'; export {ThreadsBadge} from './ThreadsBadge'; export {Badge} from './Badge'; export {ThreadList} from './ThreadList';