From d61f098779ce6a0cfbf4ab5454cd69acb2b140fd Mon Sep 17 00:00:00 2001 From: witbrock Date: Wed, 16 Sep 2026 12:33:32 -0400 Subject: [PATCH] Jump to the first unread message across earlier pages --- .../static/js/components/messagePanel.js | 21 ++++--- tests/browser/messageUnreadNavigation.cjs | 14 ++--- tests/frontend/messageExchangeView.test.js | 60 ++++++++++++++----- 3 files changed, 62 insertions(+), 33 deletions(-) diff --git a/src/frontend/web/von_interface/static/js/components/messagePanel.js b/src/frontend/web/von_interface/static/js/components/messagePanel.js index 22d46826..559c9771 100644 --- a/src/frontend/web/von_interface/static/js/components/messagePanel.js +++ b/src/frontend/web/von_interface/static/js/components/messagePanel.js @@ -166,7 +166,7 @@ function updateUnreadMarkers() { updateMessageNavigation(); } -async function jumpToMostRecentUnread() { +async function jumpToFirstUnread() { const generation = _exchangeGeneration; const org = getSessionScopedOrgId(); if (_unreadJumpGeneration === generation || _isLoading) return; @@ -179,15 +179,14 @@ async function jumpToMostRecentUnread() { updateMessageNavigation(); try { while (stillCurrent()) { - const targets = content?.querySelectorAll('.is-unread[data-contribution-id]'); - const target = targets?.[targets.length - 1]; - if (target) { - target.scrollIntoView({ block: 'start' }); - focusConversationTarget(target); - return; - } + // Read markers can have gaps, and the catalogue count can be stale. + // Exhaust earlier pages before choosing the chronological first unread. if (!_olderCursor) { - showUnreadNavigationStatus('No unread messages remain in this conversation.'); + const target = content?.querySelector('.is-unread[data-contribution-id]'); + if (target) { + target.scrollIntoView({ block: 'start' }); + focusConversationTarget(target); + } else showUnreadNavigationStatus('No unread messages remain in this conversation.'); return; } const cursor = _olderCursor; @@ -228,7 +227,7 @@ function updateMessageNavigation() { const unread = document.createElement('button'); unread.type = 'button'; unread.className = 'message-jump-unread'; - unread.onclick = () => void jumpToMostRecentUnread(); + unread.onclick = () => void jumpToFirstUnread(); const latest = createLatestMessageButton(() => { content.scrollTop = content.scrollHeight; focusConversationTarget(content.querySelector('.message-list')?.lastElementChild || content); @@ -241,7 +240,7 @@ function updateMessageNavigation() { } const unread = navigation.querySelector('.message-jump-unread'); const jumping = _unreadJumpGeneration === _exchangeGeneration; - unread.textContent = jumping ? 'Loading unread messages…' : 'Jump to most recent unread'; + unread.textContent = jumping ? 'Loading unread messages…' : 'Jump to first unread'; unread.disabled = jumping || _isLoading; unread.hidden = !jumping && !content.querySelector('.is-unread') && !(_olderCursor && _exchange?.shared_unread_count > 0); diff --git a/tests/browser/messageUnreadNavigation.cjs b/tests/browser/messageUnreadNavigation.cjs index a73b6e4f..a075c362 100644 --- a/tests/browser/messageUnreadNavigation.cjs +++ b/tests/browser/messageUnreadNavigation.cjs @@ -39,7 +39,7 @@ const server = http.createServer(async (req, res) => { const { before } = JSON.parse(body); cursors.push(before); const initial = Array.from({ length: 20 }, (_, index) => message(`latest-${String(index).padStart(2, '0')}`, false, '13')); const result = !before ? { messages: background ? [...initial, message('Background unread', true, '14')] : initial, before: 'middle' } - : before === 'middle' ? { messages: Array.from({ length: 10 }, (_, index) => message(`middle-${index}`, false, '12')), before: 'older' } + : before === 'middle' ? { messages: Array.from({ length: 10 }, (_, index) => message(`middle-${index}`, index === 5, '12')), before: 'older' } : { messages: [message('Earlier unread', true, '10'), message('Most recent unread', true, '11')], before: null }; const finish = () => res.end(JSON.stringify({ ...result, current_user_id: '#V#alice' })); if (before || background) pendingPage = finish; @@ -68,7 +68,7 @@ const server = http.createServer(async (req, res) => { const { openMessageExchange } = await import('/static/js/components/messagePanel.js'); await openMessageExchange({ session_id: 'fixture', viewer_id: '#V#alice', participant_ids: ['#V#alice', '#V#bob'], other_participant_ids: ['#V#bob'], - session_name: 'Unread pagination fixture', shared_unread_count: 2 }); + session_name: 'Unread pagination fixture', shared_unread_count: 3 }); }); const content = page.locator('#messageViewContent'); assert.equal(await content.evaluate(el => el.scrollTop), 0, 'default loading preserves the top'); @@ -106,7 +106,7 @@ const server = http.createServer(async (req, res) => { const panel = await import('/static/js/components/messagePanel.js'); await panel.openMessageExchange({ session_id: 'fixture', viewer_id: '#V#alice', participant_ids: ['#V#alice', '#V#bob'], other_participant_ids: ['#V#bob'], - session_name: 'Unread pagination fixture', shared_unread_count: 2 }); + session_name: 'Unread pagination fixture', shared_unread_count: 3 }); }); cursors = []; await content.evaluate(el => { el.scrollTop = 450; }); @@ -117,7 +117,7 @@ const server = http.createServer(async (req, res) => { }); const offset = () => page.locator(`[data-contribution-id="${anchor.id}"]`).evaluate(el => el.getBoundingClientRect().top - document.getElementById('messageViewContent').getBoundingClientRect().top); - await page.getByRole('button', { name: 'Jump to most recent unread', exact: true }).click(); + await page.getByRole('button', { name: 'Jump to first unread', exact: true }).click(); await expect.poll(() => Boolean(pendingPage)).toBe(true); assert.equal(await offset(), anchor.offset, 'pending go-to-unread preserves the visible message'); pendingPage(); pendingPage = null; @@ -127,17 +127,17 @@ const server = http.createServer(async (req, res) => { await expect(page.getByRole('button', { name: 'Loading unread messages…', exact: true })).toBeVisible(); await page.screenshot({ path: path.join(evidence, `loading-${width}.png`) }); pendingPage(); pendingPage = null; - const target = page.locator('[data-contribution-id="Most recent unread"]'); + const target = page.locator('[data-contribution-id="Earlier unread"]'); await expect(target).toBeFocused(); await expect(target).toBeInViewport(); assert.deepEqual(cursors, ['middle', 'older']); + await page.screenshot({ path: path.join(evidence, `unread-${width}.png`) }); await send.click(); await expect(input).toHaveValue(''); assert.equal(sent.length, 1); assert.equal(sent[0].content, 'A reply from the up-arrow control.'); assert.deepEqual(errors, []); - await page.screenshot({ path: path.join(evidence, `unread-${width}.png`) }); - results.push({ width, cursors: [...cursors], sendArrow: true, sendTarget: box, sends: sent.length, labelledLatestNavigation: true, defaultTopPreserved: true, backgroundPosition: bottomPosition, intermediateAnchorOffset: anchor.offset, mostRecentUnreadFocused: true, targetInViewport: true }); + results.push({ width, cursors: [...cursors], sendArrow: true, sendTarget: box, sends: sent.length, labelledLatestNavigation: true, defaultTopPreserved: true, backgroundPosition: bottomPosition, intermediateAnchorOffset: anchor.offset, firstUnreadFocused: true, targetInViewport: true }); await page.close(); } fs.writeFileSync(path.join(evidence, 'result.json'), JSON.stringify({ fixture: true, results }, null, 2)); diff --git a/tests/frontend/messageExchangeView.test.js b/tests/frontend/messageExchangeView.test.js index dcf2d122..85395b57 100644 --- a/tests/frontend/messageExchangeView.test.js +++ b/tests/frontend/messageExchangeView.test.js @@ -190,17 +190,17 @@ test('refresh and pagination preserve scroll position and unread navigation with api.postJson.mockResolvedValueOnce(updated); await panel.refreshOpenMessageExchange(); expect(content.scrollTop).toBe(25); - const first = content.querySelector('.is-unread'); - first.scrollIntoView = jest.fn(); - document.querySelector('.message-jump-unread').click(); - expect(first.scrollIntoView).toHaveBeenCalledWith({ block: 'start' }); - expect(document.activeElement).toBe(first); const older = response('old'); older.messages[0].created_at = '2026-09-10T12:00:00Z'; api.postJson.mockResolvedValueOnce(older); [...content.querySelectorAll('button')].find(b => b.textContent === 'Load earlier messages').click(); await flush(); expect(content.scrollTop).toBe(25); expect(content.querySelectorAll('[data-contribution-id]')).toHaveLength(3); + const first = content.querySelector('.is-unread'); + first.scrollIntoView = jest.fn(); + document.querySelector('.message-jump-unread').click(); + expect(first.scrollIntoView).toHaveBeenCalledWith({ block: 'start' }); + expect(document.activeElement).toBe(first); expect(api.postJson.mock.calls.some(([url]) => url.includes('/read/'))).toBe(false); }); @@ -217,31 +217,61 @@ test('the visible-read observer excludes the fixed composer overlap', async () = expect(global.IntersectionObserver.mock.calls[1][1].rootMargin).toBe('0px 0px -150px 0px'); }); -function page(ids, before = null, unread = []) { +function page(ids, before = null, unread = [], firstDay = 1) { return { current_user_id: '#V#alice', before, messages: ids.map((id, index) => ({ - ...response(id).messages[0], created_at: `2026-09-${String(index + 1).padStart(2, '0')}T12:00:00Z`, + ...response(id).messages[0], created_at: `2026-09-${String(index + firstDay).padStart(2, '0')}T12:00:00Z`, relationships: { '#V#has_sender': ['#V#bob'], '#V#has_recipient': ['#V#alice'] }, concept_data: { content_fallback: id, read_by: unread.includes(id) ? [] : ['#V#alice'] } })) }; } -test('unloaded unread count exposes navigation that pages backwards to the most recent unread', async () => { +test('unloaded unread count exposes navigation that pages past loaded unread messages to the first unread', async () => { HTMLElement.prototype.scrollIntoView = jest.fn(); const api = require(base + 'apiService.js'); - api.postJson.mockResolvedValueOnce(page(['latest'], 'page2')) - .mockResolvedValueOnce(page(['middle'], 'page3')) - .mockResolvedValueOnce(page(['old-unread', 'recent-unread'], 'page4', ['old-unread', 'recent-unread'])); + api.postJson.mockResolvedValueOnce(page(['latest'], 'page2', [], 15)) + .mockResolvedValueOnce(page(['middle'], 'page3', [], 12)) + .mockResolvedValueOnce(page(['old-unread', 'recent-unread'], 'page4', ['old-unread', 'recent-unread'], 9)) + .mockResolvedValueOnce(page(['first-unread'], null, ['first-unread'])); await require(base + 'components/messagePanel.js').openMessageExchange({ ...row('#V#bob'), shared_unread_count: 2 }); const jump = document.querySelector('.message-jump-unread'); expect(jump.hidden).toBe(false); - expect(jump.textContent).toBe('Jump to most recent unread'); + expect(jump.textContent).toBe('Jump to first unread'); jump.click(); await flush(); - expect(document.activeElement.dataset.contributionId).toBe('recent-unread'); - expect(api.postJson.mock.calls.map(([, body]) => body.before)).toEqual([null, 'page2', 'page3']); - expect(document.querySelectorAll('.is-unread')).toHaveLength(2); + expect(document.activeElement.dataset.contributionId).toBe('first-unread'); + expect(api.postJson.mock.calls.map(([, body]) => body.before)).toEqual([null, 'page2', 'page3', 'page4']); + expect(document.querySelectorAll('.is-unread')).toHaveLength(3); expect(jump.disabled).toBe(false); }); +test('already loaded unread does not skip an older unread separated by a read page', async () => { + HTMLElement.prototype.scrollIntoView = jest.fn(); + const observers = mockReadObserver(); + const api = require(base + 'apiService.js'); + const latest = page(['recent-unread'], 'read-gap', ['recent-unread'], 15); + api.postJson.mockResolvedValueOnce(latest) + .mockResolvedValueOnce(page(['read-gap'], 'oldest', [], 10)) + .mockResolvedValueOnce(page(['first-unread'], null, ['first-unread'])); + // A stale low count must not cause an early stop. + await require(base + 'components/messagePanel.js').openMessageExchange({ ...row('#V#bob'), shared_unread_count: 1 }); + document.querySelector('.message-jump-unread').click(); await flush(); + expect(document.activeElement.dataset.contributionId).toBe('first-unread'); + expect(api.postJson.mock.calls.map(([, body]) => body.before)).toEqual([null, 'read-gap', 'oldest']); + expect(observers[0].disconnect).toHaveBeenCalled(); + expect(observers).toHaveLength(2); // Re-arm only after the final jump. + expect(document.querySelectorAll('.is-unread')).toHaveLength(2); +}); + +test('a fully loaded conversation focuses its first unread without a request', async () => { + HTMLElement.prototype.scrollIntoView = jest.fn(); + const api = require(base + 'apiService.js'); + api.postJson.mockResolvedValueOnce(page(['read', 'first', 'read-gap', 'last'], null, ['first', 'last'])); + await require(base + 'components/messagePanel.js').openMessageExchange(row('#V#bob')); + document.querySelector('.message-jump-unread').click(); + expect(document.activeElement.dataset.contributionId).toBe('first'); + expect(HTMLElement.prototype.scrollIntoView).toHaveBeenCalledWith({ block: 'start' }); + expect(api.postJson).toHaveBeenCalledTimes(1); +}); + test('failed unread pagination retains the action for retry', async () => { HTMLElement.prototype.scrollIntoView = jest.fn(); const api = require(base + 'apiService.js');