From 142bb1204450834476a8d5aebbf9867ebaf20652 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Thu, 1 Oct 2026 18:47:51 +0200 Subject: [PATCH 01/25] Add stable message keys and dedupe messages Give messages a stable `key` for list rendering and scroll anchoring: the transport id when present, otherwise a composite of the timestamp and a hash of the author and content (remoteStorage IRC logs have no message ids). `BaseChannel.addMessage` assigns the key and ignores duplicates. --- app/models/base_channel.js | 12 ++++ app/utils/message-key.js | 27 +++++++++ tests/unit/models/base-channel-test.js | 81 ++++++++++++++++++++++++++ tests/unit/utils/message-key-test.js | 61 +++++++++++++++++++ 4 files changed, 181 insertions(+) create mode 100644 app/utils/message-key.js create mode 100644 tests/unit/utils/message-key-test.js diff --git a/app/models/base_channel.js b/app/models/base_channel.js index 3af79dde..7a7a2acd 100644 --- a/app/models/base_channel.js +++ b/app/models/base_channel.js @@ -3,6 +3,7 @@ import { tracked, cached } from '@glimmer/tracking'; import { TrackedArray } from 'tracked-built-ins'; import Message from 'hyperchannel/models/message'; import domainFromId from 'hyperchannel/utils/domain-from-id'; +import { messageKey, dateHeadlineKey } from 'hyperchannel/utils/message-key'; import moment from 'moment'; export default class BaseChannel { @@ -124,6 +125,7 @@ export default class BaseChannel { if (existingDateHeadline) { return; } let dateMessage = new Message({ type: 'date-headline', date: headlineDate }); + dateMessage.key = dateHeadlineKey(headlineDate); this.messages.push(dateMessage); } @@ -133,6 +135,16 @@ export default class BaseChannel { return; } + if (!message.key) { + message.key = messageKey(message); + } + + // Ignore duplicates: the same transport message delivered twice, or an + // id-less archive message whose derived key matches an existing one. + if (this.messages.some(m => m.key === message.key)) { + return; + } + this.addDateHeadline(message); // Find the last non-date-headline message for grouping check diff --git a/app/utils/message-key.js b/app/utils/message-key.js new file mode 100644 index 00000000..37cc901a --- /dev/null +++ b/app/utils/message-key.js @@ -0,0 +1,27 @@ +import { sha1 } from '@noble/hashes/legacy.js'; +import { bytesToHex, utf8ToBytes } from '@noble/hashes/utils.js'; + +/** + * Stable identity for a message, used for list keys and scroll anchoring. + * + * Messages that come from a transport with real ids use them. Older archive + * messages (e.g. remoteStorage IRC logs) have no id, so we derive one from the + * timestamp plus a hash of the author and content. + * + * Note: two byte-identical messages in the same timestamp tick will produce the + * same key and are treated as duplicates when added (`BaseChannel.addMessage`). + * That is intentional; distinguishing such messages is not a supported feature. + */ +export function messageKey (message) { + if (message.id) return String(message.id); + + const timestamp = message.date ? new Date(message.date).getTime() : 0; + const identity = `${message.nickname ?? ''}\u0000${message.content ?? ''}`; + const hash = bytesToHex(sha1(utf8ToBytes(identity))).slice(0, 10); + + return `${timestamp}-${hash}`; +} + +export function dateHeadlineKey (date) { + return `date-${new Date(date).getTime()}`; +} diff --git a/tests/unit/models/base-channel-test.js b/tests/unit/models/base-channel-test.js index 47b980aa..d931c1d3 100644 --- a/tests/unit/models/base-channel-test.js +++ b/tests/unit/models/base-channel-test.js @@ -270,6 +270,87 @@ module('Unit | Model | base-channel', function (hooks) { assert.notOk(addDateHeadlineStub.calledOnce); }); + test('#addMessage assigns a stable key', function (assert) { + const channel = new BaseChannel({ account: xmppAccount }); + const message = new Message({ + type: 'message-chat', date: new Date(), nickname: 'alice', + content: 'hello', id: 'key-1' + }); + + channel.addMessage(message); + + assert.strictEqual(message.key, 'key-1', 'message gets the transport id as key'); + const stored = channel.messages.find(item => item.type === 'message-chat'); + assert.strictEqual(stored.key, 'key-1'); + }); + + test('#addMessage derives a key for id-less messages', function (assert) { + const channel = new BaseChannel({ account: xmppAccount }); + const message = new Message({ + type: 'message-chat', date: new Date('2024-01-02T03:04:05.678Z'), + nickname: 'alice', content: 'hello' + }); + + channel.addMessage(message); + + assert.ok(message.key.startsWith('1704164645678-'), `derives a composite key (${message.key})`); + }); + + test('#addMessage ignores duplicates with the same id', function (assert) { + const channel = new BaseChannel({ account: xmppAccount }); + const date = new Date(); + + channel.addMessage(new Message({ + type: 'message-chat', date, nickname: 'alice', content: 'hello', id: 'dup-1' + })); + channel.addMessage(new Message({ + type: 'message-chat', date, nickname: 'alice', content: 'hello', id: 'dup-1' + })); + + assert.strictEqual( + channel.messages.filter(item => item.type === 'message-chat').length, + 1, + 'only adds the message once' + ); + }); + + test('#addMessage dedupes id-less messages with identical timestamp and content', function (assert) { + const channel = new BaseChannel({ account: xmppAccount }); + const date = new Date('2024-01-02T03:04:05.678Z'); + + channel.addMessage(new Message({ + type: 'message-chat', date, nickname: 'alice', content: 'hello' + })); + channel.addMessage(new Message({ + type: 'message-chat', date, nickname: 'alice', content: 'hello' + })); + + assert.strictEqual( + channel.messages.filter(item => item.type === 'message-chat').length, + 1, + 'only adds the message once' + ); + }); + + test('#addMessage keeps id-less messages with the same content at different times', function (assert) { + const channel = new BaseChannel({ account: xmppAccount }); + + channel.addMessage(new Message({ + type: 'message-chat', date: new Date('2024-01-02T03:04:05.678Z'), + nickname: 'alice', content: 'hello' + })); + channel.addMessage(new Message({ + type: 'message-chat', date: new Date('2024-01-02T03:04:06.678Z'), + nickname: 'alice', content: 'hello' + })); + + assert.strictEqual( + channel.messages.filter(item => item.type === 'message-chat').length, + 2, + 'keeps both messages' + ); + }); + // // replaceMessage // diff --git a/tests/unit/utils/message-key-test.js b/tests/unit/utils/message-key-test.js new file mode 100644 index 00000000..f1d66a3f --- /dev/null +++ b/tests/unit/utils/message-key-test.js @@ -0,0 +1,61 @@ +import { module, test } from 'qunit'; +import { messageKey, dateHeadlineKey } from 'hyperchannel/utils/message-key'; +import Message from 'hyperchannel/models/message'; + +function chatMessage (props) { + return new Message({ + type: 'message-chat', + date: new Date('2024-01-02T03:04:05.678Z'), + nickname: 'alice', + content: 'hello world', + ...props + }); +} + +module('Unit | Utility | message key', function () { + test('uses the id when the message has one', function (assert) { + const message = chatMessage({ id: 'abc123' }); + + assert.strictEqual(messageKey(message), 'abc123'); + }); + + test('derives a key from the timestamp and content when there is no id', function (assert) { + const message = chatMessage(); + + const key = messageKey(message); + + assert.ok(key.startsWith('1704164645678-'), `includes the timestamp (${key})`); + assert.notStrictEqual(key, '1704164645678-', 'includes a content hash'); + }); + + test('derives the same key for identical id-less messages', function (assert) { + assert.strictEqual(messageKey(chatMessage()), messageKey(chatMessage())); + }); + + test('derives different keys for different content', function (assert) { + const first = messageKey(chatMessage()); + const second = messageKey(chatMessage({ content: 'goodbye world' })); + + assert.notStrictEqual(first, second); + }); + + test('derives different keys for different timestamps', function (assert) { + const first = messageKey(chatMessage()); + const second = messageKey(chatMessage({ date: new Date('2024-01-02T03:04:06.678Z') })); + + assert.notStrictEqual(first, second); + }); + + test('derives different keys for different authors', function (assert) { + const first = messageKey(chatMessage()); + const second = messageKey(chatMessage({ nickname: 'bob' })); + + assert.notStrictEqual(first, second); + }); + + test('date headlines get a date- prefixed key', function (assert) { + const date = new Date('2024-01-02T00:00:00.000Z'); + + assert.strictEqual(dateHeadlineKey(date), `date-${date.getTime()}`); + }); +}); From a1e260e326a4557ffc6632854b976aeacd527e25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Thu, 1 Oct 2026 18:47:51 +0200 Subject: [PATCH 02/25] Fetch older archive messages on demand Add `coms.loadOlderMessages`, which fetches the next archive page and reports `{ added, hasMore }`. `loadArchiveMessages` now returns that summary and sets `channel.hasOlderMessages` when history is exhausted. The channel tracks `hasOlderMessages` / `loadingOlderMessages` for the UI. --- app/models/base_channel.js | 2 ++ app/services/coms.js | 27 ++++++++++++++++++++++++++- 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/app/models/base_channel.js b/app/models/base_channel.js index 7a7a2acd..9cbb2085 100644 --- a/app/models/base_channel.js +++ b/app/models/base_channel.js @@ -19,6 +19,8 @@ export default class BaseChannel { @tracked messages = new TrackedArray([]); @tracked unreadMessages = false; @tracked unreadMentions = false; + @tracked hasOlderMessages = false; + @tracked loadingOlderMessages = false; @tracked visible = false; // Current/active channel @tracked roomInfoLoaded = false; @tracked description = null; diff --git a/app/services/coms.js b/app/services/coms.js index 26039f07..f017ef08 100644 --- a/app/services/coms.js +++ b/app/services/coms.js @@ -386,6 +386,7 @@ export default class ComsService extends Service { let messagesCount = 0; let archivesCount = 0; + let added = 0; while ((messagesCount < options.minMessages) && (archivesCount < options.maxDays)) { @@ -401,15 +402,39 @@ export default class ComsService extends Service { }); channel.addMessage(channelMessage); + added += 1; }); messagesCount += archive.today?.messages?.length || 0; archivesCount += 1; dateStr = archive.today?.previous; - channel.searchedPreviousLogsUntilDate = dateStr; if (isEmpty(dateStr)) break; } + + channel.searchedPreviousLogsUntilDate = dateStr; + channel.hasOlderMessages = !isEmpty(dateStr); + + return { added, hasMore: channel.hasOlderMessages }; + } + + /** + * Fetches an additional page of older messages into the channel. + * + * Returns `{ added, hasMore }` so the caller can reveal them and know whether + * more history is available. + */ + async loadOlderMessages (channel, options = {}) { + if (isEmpty(channel.searchedPreviousLogsUntilDate)) { + channel.hasOlderMessages = false; + return { added: 0, hasMore: false }; + } + + return this.loadArchiveMessages(channel, channel.searchedPreviousLogsUntilDate, { + minMessages: 30, + maxDays: 3, + ...options + }); } createUserChannel (account, name) { From af129c30623b78c1ffbffd9615d36c01dea6bdaf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Thu, 1 Oct 2026 18:47:51 +0200 Subject: [PATCH 03/25] Rework chat scrolling and automatic history loading Fixes #285. Replace the ad-hoc, insertion-triggered scroll handling in the channel container with a three-layer design: - a pure scroll policy (app/utils/chat-scroll-state.js) with unit tests; - a ChatScroller component that adapts it to the DOM (stick to bottom, detach on scroll, anchor-preserving history loads, auto-loading); - a chat-scroll modifier that wires up the ResizeObserver and listeners. Auto-loading is level-triggered and single-flight (near the top, or filling a short list) with a no-progress guard, and is re-checked after each page, so reaching the top while a page is still loading loads the next one without scrolling down and back up. Scroll position is preserved when older messages are prepended by restoring the anchored message in the same turn as the DOM update (no one-frame jump), and prepends/appends that happen together are handled independently. Also: - add a bottom-right "scroll to bottom" button (grey chevron when idle, blue with a new-message count when there are new messages) with a smooth scroll that honors prefers-reduced-motion; - fix on-change to depend only on its value (run the callback in untrack) so callbacks that read tracked state don't retrigger it; - remove the now-unused on-channel-change/on-users-change modifiers; - show "Beginning of history" only once history has actually been loaded. Covered by new unit and integration tests. --- app/components/channel-container.hbs | 55 ++- app/components/channel-container.js | 135 +++--- app/components/chat-scroller.hbs | 15 + app/components/chat-scroller.js | 436 ++++++++++++++++++ app/components/user-list.hbs | 2 +- app/components/user-list.js | 5 +- app/models/message.js | 4 + app/modifiers/chat-scroll.js | 38 ++ app/modifiers/on-change.js | 22 + app/modifiers/on-channel-change.js | 8 - app/modifiers/on-users-change.js | 9 - app/styles/components/channel-container.scss | 48 ++ app/utils/chat-scroll-state.js | 150 ++++++ .../components/chat-scroller-test.js | 318 +++++++++++++ tests/integration/modifiers/on-change-test.js | 61 +++ tests/unit/utils/chat-scroll-state-test.js | 172 +++++++ 16 files changed, 1372 insertions(+), 106 deletions(-) create mode 100644 app/components/chat-scroller.hbs create mode 100644 app/components/chat-scroller.js create mode 100644 app/modifiers/chat-scroll.js create mode 100644 app/modifiers/on-change.js delete mode 100644 app/modifiers/on-channel-change.js delete mode 100644 app/modifiers/on-users-change.js create mode 100644 app/utils/chat-scroll-state.js create mode 100644 tests/integration/components/chat-scroller-test.js create mode 100644 tests/integration/modifiers/on-change-test.js create mode 100644 tests/unit/utils/chat-scroll-state-test.js diff --git a/app/components/channel-container.hbs b/app/components/channel-container.hbs index c1f22dfa..443c4ad1 100644 --- a/app/components/channel-container.hbs +++ b/app/components/channel-container.hbs @@ -1,5 +1,5 @@
+ {{on-change @channel this.channelChanged}}>

-
- {{#if @channel.isLogged}} - - Load previous messages - + + {{#if this.showHistoryStart}} +
+ Beginning of history +
{{/if}}
    -
  • - -
  • - {{#each this.renderedMessages as |message|}} -
  • + {{#each this.renderedMessages key="key" as |message|}} +
  • {{#if (eq message.type "message-chat-me")}} {{/each}} -
  • - -
-
+ + {{#unless scroll.isAtBottom}} + + {{/unless}} +
0 || this.args.channel.hasOlderMessages; + } - get renderedMessages () { - if (this.partialRenderingEnabled) { - return this.args.channel.sortedMessages.slice(-this.renderedMessagesCount); - } else { - return this.args.channel.sortedMessages; - } + get isLoadingOlderMessages () { + return this.args.channel.loadingOlderMessages; + } + + // Only show the "beginning of history" marker once older history has + // actually been loaded/revealed for this channel. + get showHistoryStart () { + return this.historyLoaded && !this.canLoadOlderMessages; } @action channelChanged () { - this.renderedMessagesCount = this.renderedMessagesAddendumAmount; - this.partialRenderingEnabled = true; - this.automaticScrollingEnabled = true; + const messageCount = this.args.channel.sortedMessages.length; + this.renderedStartIndex = Math.max(0, messageCount - INITIAL_RENDERED_MESSAGES); + this.historyLoaded = false; + setTimeout(() => { if (this.isDestroyed || this.isDestroying) return; this.menu('global', 'hide'); @@ -44,49 +60,54 @@ export default class ChannelContainerComponent extends Component { } @action - messagesUpdated () { - if (this.automaticScrollingEnabled) { - requestAnimationFrame(() => { - if (this.isDestroyed || this.isDestroying) return; - scrollToBottom(); - }); - } - } - - @action - onAfterRender (element) { - // TODO update the config when window is resized - this.partialRenderingObserverMargin = `${element.clientHeight/3}px`; - + onAfterRender () { // We need to define an empty handler for swipe events on the // #channel-content element, so that the actual handler of the app container // component gets triggered Hammer(document.getElementById('channel-content')).on('swipe', function (){}); } - focusMessageInputField () { - const inputEl = document.querySelector('input#message-field'); - inputEl.focus(); - } + /** + * Reveals the next page of older messages. Older messages already in memory + * are revealed first; a new archive page is fetched once the window reaches + * the beginning of what we have. + */ + @action + async loadOlderMessages () { + const channel = this.args.channel; + if (channel.loadingOlderMessages) return; + + if (this.renderedStartIndex > 0) { + this.renderedStartIndex = Math.max(0, this.renderedStartIndex - RENDERED_MESSAGES_INCREMENT); + this.historyLoaded = true; + return; + } - @tracked loadingPreviousMessages = false; + if (!channel.hasOlderMessages) return; - @action - async loadPreviousMessages () { - if (this.loadingPreviousMessages) return; - this.loadingPreviousMessages = true; - this.automaticScrollingEnabled = false; + const lengthBefore = channel.sortedMessages.length; + + channel.loadingOlderMessages = true; + this.historyLoaded = true; try { - await this.coms.loadArchiveMessages( - this.args.channel, - this.args.channel.searchedPreviousLogsUntilDate, - { minMessages: 1, maxDays: 1 } - ); + await this.coms.loadOlderMessages(channel); } finally { - this.loadingPreviousMessages = false; + channel.loadingOlderMessages = false; + } + + // Keep the currently rendered messages visible and reveal a page of the + // newly loaded ones. `ChatScroller` restores the scroll position around it. + const added = channel.sortedMessages.length - lengthBefore; + if (added > 0) { + this.renderedStartIndex = Math.max(0, added - RENDERED_MESSAGES_INCREMENT); } } + focusMessageInputField () { + const inputEl = document.querySelector('input#message-field'); + inputEl.focus(); + } + // TODO make dynamic based on active sidebar content get headerNavButtonUsersActive () { if (window.innerWidth > 900) return true; @@ -116,18 +137,6 @@ export default class ChannelContainerComponent extends Component { this.focusMessageInputField(); } - @action - increaseRenderedMessagesCount () { - let newMessagesCount = this.renderedMessagesCount + this.renderedMessagesAddendumAmount; - this.renderedMessagesCount =newMessagesCount; - this.partialRenderingEnabled = newMessagesCount < this.args.channel.sortedMessages.length; - } - - @action - setAutomaticScrolling (state) { - this.automaticScrollingEnabled = state; - } - @action leaveChannel (channel) { this.args.onLeaveChannel(channel); diff --git a/app/components/chat-scroller.hbs b/app/components/chat-scroller.hbs new file mode 100644 index 00000000..547a0b21 --- /dev/null +++ b/app/components/chat-scroller.hbs @@ -0,0 +1,15 @@ +
+
+ {{yield this.scrollApi}} +
+
diff --git a/app/components/chat-scroller.js b/app/components/chat-scroller.js new file mode 100644 index 00000000..407057dd --- /dev/null +++ b/app/components/chat-scroller.js @@ -0,0 +1,436 @@ +import Component from '@glimmer/component'; +import { tracked } from '@glimmer/tracking'; +import { action } from '@ember/object'; +import { + ScrollAction, + createScrollState, + reduceScrollState, + isNearTop, +} from 'hyperchannel/utils/chat-scroll-state'; + +const MAINTENANCE_TIMEOUT = 150; // ms +const SMOOTH_SCROLL_TIMEOUT = 1000; // ms; upper bound for a smooth scroll +const FILL_MAX_NO_PROGRESS = 2; + +function itemKey (item) { + if (!item) return ''; + return item.key ?? item.id ?? ''; +} + +function escapeAttributeValue (value) { + const stringValue = String(value ?? ''); + if (typeof CSS !== 'undefined' && CSS.escape) { + return CSS.escape(stringValue); + } + return stringValue.replace(/["\\]/g, '\\$&'); +} + +/** + * Scroll container for a chat message list. + * + * Owns all scroll behavior for the channel: sticking to the bottom, detaching + * when the user scrolls up, restoring the visual position when older messages + * are revealed/loaded, and triggering automatic history loading near the top. + * + * Auto-loading is level-triggered and single-flight (near the top, or filling + * a short list), with a separate no-progress guard. All programmatic position + * changes happen under a maintenance lock so they are never mistaken for user + * intent (which caused feedback loops in the past). Layout corrections are + * applied synchronously with the DOM update so the viewport never jumps. + * + * The decision making lives in `hyperchannel/utils/chat-scroll-state`; this + * component is the DOM adapter that applies the resulting effects. + * + * Yields a small API to the surrounding template: + * scroll.isAtBottom, scroll.newMessageCount, scroll.showJumpToLatest, scroll.scrollToBottom + */ +export default class ChatScrollerComponent extends Component { + @tracked isAtBottom = true; + @tracked newMessageCount = 0; + @tracked showJumpToLatest = false; + + scrollState = createScrollState(); + scrollElement = null; + itemKeys = null; + + // Programmatic scroll maintenance lock (does not trigger reactive updates). + maintainingScroll = false; + maintenanceTimer = null; + maintenanceTarget = null; + maintenanceObservedTop = 0; + smoothScrollRequested = false; + + // Auto-load bookkeeping. + olderRequestPending = false; + fillNoProgress = 0; + lastFillHeight = 0; + resizeScheduled = false; + + get scrollApi () { + return { + isAtBottom: this.isAtBottom, + newMessageCount: this.newMessageCount, + showJumpToLatest: this.showJumpToLatest, + scrollToBottom: this.scrollToBottom, + }; + } + + getContentElement () { + return this.scrollElement?.firstElementChild ?? null; + } + + metricsFrom (element) { + if (!element) return { scrollTop: 0, scrollHeight: 0, clientHeight: 0 }; + return { + scrollTop: element.scrollTop, + scrollHeight: element.scrollHeight, + clientHeight: element.clientHeight, + }; + } + + metrics () { + return this.metricsFrom(this.scrollElement); + } + + contentFits (metrics = this.metrics()) { + return metrics.clientHeight > 0 && metrics.scrollHeight <= metrics.clientHeight + 1; + } + + reduceAndApply (event) { + const { state, effects } = reduceScrollState(this.scrollState, event); + this.scrollState = state; + this.syncTrackedState(); + this.applyEffects(effects); + } + + // Modifier callbacks (`on-change`) run during the render transaction, so we + // defer state changes and DOM effects to the next frame. Setting tracked + // state during render would invalidate and re-run the modifier recursively. + scheduleReduce (event) { + requestAnimationFrame(() => { + if (this.isDestroyed || this.isDestroying) return; + this.reduceAndApply(event); + }); + } + + syncTrackedState () { + const { atBottom, newMessageCount, showJumpToLatest } = this.scrollState; + if (this.isAtBottom !== atBottom) this.isAtBottom = atBottom; + if (this.newMessageCount !== newMessageCount) this.newMessageCount = newMessageCount; + if (this.showJumpToLatest !== showJumpToLatest) this.showJumpToLatest = showJumpToLatest; + } + + applyEffects (effects) { + for (const effect of effects) { + switch (effect) { + case ScrollAction.ScrollToBottom: + this.applyScrollToBottom(); + break; + case ScrollAction.CaptureAnchor: + this.captureAnchor(); + break; + case ScrollAction.RestoreAnchor: + this.restoreAnchor(); + break; + case ScrollAction.LoadOlder: + this.args.onLoadOlder?.(); + break; + } + } + } + + // --- Programmatic scroll maintenance -------------------------------------- + + beginMaintenance (timeout = MAINTENANCE_TIMEOUT) { + if (this.maintenanceTimer) { + clearTimeout(this.maintenanceTimer); + } + this.maintainingScroll = true; + this.maintenanceObservedTop = this.scrollElement?.scrollTop ?? 0; + this.maintenanceTimer = setTimeout(() => { + this.maintenanceTimer = null; + this.endMaintenance(); + }, timeout); + } + + endMaintenance () { + if (this.maintenanceTimer) { + clearTimeout(this.maintenanceTimer); + this.maintenanceTimer = null; + } + this.maintainingScroll = false; + this.maintenanceTarget = null; + } + + prefersReducedMotion () { + return typeof window !== 'undefined' && + typeof window.matchMedia === 'function' && + window.matchMedia('(prefers-reduced-motion: reduce)').matches; + } + + applyScrollToBottom () { + const element = this.scrollElement; + if (!element) return; + + const smooth = this.smoothScrollRequested && !this.prefersReducedMotion(); + this.smoothScrollRequested = false; + + const maxTop = element.scrollHeight - element.clientHeight; + + if (smooth) { + this.beginMaintenance(SMOOTH_SCROLL_TIMEOUT); + element.scrollTo({ top: maxTop, behavior: 'smooth' }); + this.maintenanceTarget = maxTop; + } else { + this.beginMaintenance(); + element.scrollTop = element.scrollHeight; + this.maintenanceTarget = element.scrollTop; + } + } + + // --- Anchoring ------------------------------------------------------------ + + captureAnchor () { + const element = this.scrollElement; + const content = this.getContentElement(); + if (!element || !content) return; + + const containerTop = element.getBoundingClientRect().top; + const nodes = content.querySelectorAll('[data-message-key]'); + + for (const node of nodes) { + const rect = node.getBoundingClientRect(); + if (rect.bottom > containerTop) { + this.scrollState = { + ...this.scrollState, + anchor: { key: node.dataset.messageKey, offset: rect.top - containerTop }, + }; + return; + } + } + + this.scrollState = { ...this.scrollState, anchor: null }; + } + + restoreAnchor () { + const anchor = this.scrollState.anchor; + const element = this.scrollElement; + const content = this.getContentElement(); + if (!anchor || !element || !content) return; + + const node = content.querySelector( + `[data-message-key="${escapeAttributeValue(anchor.key)}"]` + ); + if (!node) return; + + const containerTop = element.getBoundingClientRect().top; + const rect = node.getBoundingClientRect(); + const delta = (rect.top - containerTop) - anchor.offset; + if (Math.abs(delta) <= 1) return; // tolerate sub-pixel/fractional scroll values + + this.beginMaintenance(); + element.scrollTop += delta; + this.maintenanceTarget = element.scrollTop; + } + + // --- Auto-loading --------------------------------------------------------- + + /** + * Genuine user scroll: update stickiness and request older messages when the + * user is near the top. + */ + handleUserScroll (metrics) { + this.reduceAndApply({ type: 'user-scroll', metrics }); + this.requestOlderIfNeeded(); + } + + /** + * Requests the next page of older messages when appropriate. + * + * Level-triggered (not a one-shot edge) but single-flight via + * `olderRequestPending`, so it can be safely re-checked after a load + * completes, on resize, and on scroll end. This ensures that reaching the top + * while a page is already loading still loads the next page once the current + * one finishes, instead of requiring the user to scroll down and back up. + */ + requestOlderIfNeeded () { + if (this.isDestroyed || this.isDestroying) return; + if (this.args.isLoadingOlder || !this.args.hasOlder) return; + if (this.olderRequestPending) return; + + const metrics = this.metrics(); + if (metrics.clientHeight === 0) return; + + const nearTop = isNearTop(metrics); + const fits = this.contentFits(metrics); + if (!nearTop && !fits) return; + + if (fits) { + // Fill the viewport: keep loading while the content is shorter than the + // window, but stop if loading makes no progress. + if (metrics.scrollHeight === this.lastFillHeight) { + this.fillNoProgress += 1; + } else { + this.fillNoProgress = 0; + this.lastFillHeight = metrics.scrollHeight; + } + if (this.fillNoProgress > FILL_MAX_NO_PROGRESS) return; + } else if (this.scrollState.stickToBottom) { + // Pinned to the bottom of a long list: this is not history reading. + return; + } + + this.olderRequestPending = true; + this.reduceAndApply({ type: 'reached-top', hasOlder: true, isLoadingOlder: false }); + } + + // --- DOM event handlers --------------------------------------------------- + + @action + handleReady (element) { + this.scrollElement = element; + } + + @action + handleReset () { + this.endMaintenance(); + this.olderRequestPending = false; + this.fillNoProgress = 0; + this.lastFillHeight = 0; + this.scheduleReduce({ type: 'reset' }); + } + + @action + handleItemsChanged (items) { + // Content changed, so the previous request has been satisfied. + this.olderRequestPending = false; + + const keys = (items ?? []).map(itemKey); + const previousKeys = this.itemKeys; + this.itemKeys = keys; + + if (!previousKeys) return; // initial render; reset handled separately + + const previousSet = new Set(previousKeys); + const firstKey = keys[0]; + const lastKey = keys[keys.length - 1]; + + // Detect both ends independently: a live message can arrive at the same time + // as a history page, and the prepend must still be handled. + const prepended = firstKey !== undefined && !previousSet.has(firstKey); + const appended = lastKey !== undefined && !previousSet.has(lastKey); + + if (prepended) { + // `older-prepended` only restores the scroll position (no tracked writes), + // so apply it synchronously. Doing this in the same turn as the DOM update + // (before paint) prevents a visible jump and stops a concurrent user + // scroll from capturing a shifted anchor. + this.reduceAndApply({ type: 'older-prepended' }); + } + + if (appended) { + let count = 0; + for (let i = keys.length - 1; i >= 0 && !previousSet.has(keys[i]); i--) { + count += 1; + } + if (this.scrollState.stickToBottom) { + // The sticky branch only scrolls to the bottom (no tracked writes). + this.reduceAndApply({ type: 'content-appended', count }); + } else { + // Detached: this updates newMessageCount/showJumpToLatest (tracked), so + // defer it to avoid writing tracked state during render. + this.scheduleReduce({ type: 'content-appended', count }); + } + } + + if (!prepended && !appended) { + // No tracked writes either; apply synchronously for the same reason. + this.reduceAndApply({ type: 'content-resized' }); + } + } + + @action + handleScroll (event) { + const metrics = this.metricsFrom(event.currentTarget); + + if (this.maintainingScroll) { + const target = this.maintenanceTarget; + + if (target === null) { + this.endMaintenance(); + } else { + const distance = Math.abs(metrics.scrollTop - target); + const previousDistance = Math.abs(this.maintenanceObservedTop - target); + const reachedTarget = distance <= 1; + // Moving away from the target means the user interrupted (e.g. scrolled + // up mid-animation); everything else is our own scroll in progress. + const movingAway = distance > previousDistance; + + this.maintenanceObservedTop = metrics.scrollTop; + + if (!reachedTarget && !movingAway) { + return; + } + + this.endMaintenance(); + if (reachedTarget) return; + // Otherwise fall through and treat it as a user scroll. + } + } + + this.handleUserScroll(metrics); + } + + @action + handleScrollEnd () { + // Do not re-apply scroll here: re-correcting from scrollend caused runaway + // feedback loops. Layout corrections are driven by ResizeObserver instead. + this.endMaintenance(); + // Coalesced scroll events can skip the exact position where the top zone was + // entered, so re-check here as well. + this.requestOlderIfNeeded(); + } + + @action + handleContentResize () { + // ResizeObserver callbacks run after layout but before paint, so applying + // the layout correction here keeps the scroll position stable without a + // one-frame jump. `content-resized` performs no tracked writes (it only + // adjusts scrollTop), and setting scrollTop does not resize the observed + // element, so this is safe inside the callback. + this.reduceAndApply({ type: 'content-resized' }); + + // Defer the auto-load request: it can add content, which must not happen + // inside the ResizeObserver callback (that causes the "ResizeObserver loop + // completed with undelivered notifications" error). + if (this.resizeScheduled) return; + this.resizeScheduled = true; + + requestAnimationFrame(() => { + this.resizeScheduled = false; + if (this.isDestroyed || this.isDestroying) return; + this.requestOlderIfNeeded(); + }); + } + + @action + handleLoadingOlderChanged (isLoading) { + if (isLoading) return; + + // The in-flight page finished; allow requesting the next one if we are + // still near the top (e.g. the user reached the top while it was loading). + this.olderRequestPending = false; + requestAnimationFrame(() => { + if (this.isDestroyed || this.isDestroying) return; + this.requestOlderIfNeeded(); + }); + } + + @action + scrollToBottom () { + // Request a smooth scroll only for this explicit, user-initiated action. + this.smoothScrollRequested = true; + this.reduceAndApply({ type: 'scroll-to-bottom' }); + this.smoothScrollRequested = false; + } +} diff --git a/app/components/user-list.hbs b/app/components/user-list.hbs index 443ab781..34a322a4 100644 --- a/app/components/user-list.hbs +++ b/app/components/user-list.hbs @@ -1,4 +1,4 @@ -
+
{{#if @users}}

{{@users.length}} Online

{{/if}} diff --git a/app/components/user-list.js b/app/components/user-list.js index bf319e2e..041500c1 100644 --- a/app/components/user-list.js +++ b/app/components/user-list.js @@ -22,13 +22,14 @@ export default class UserListComponent extends Component { // called when changing list of users (i.e. when switching channels) @action - usersChanged (element) { + usersChanged () { this.renderedUsersCount = this.renderedUsersAddendumAmount; this.partialRenderingEnabled = true; requestAnimationFrame(() => { if (this.isDestroyed || this.isDestroying) return; - this.scrollToTop(element); + const element = document.getElementById('user-list'); + if (element) this.scrollToTop(element); }); } diff --git a/app/models/message.js b/app/models/message.js index 567a058e..5465a83e 100644 --- a/app/models/message.js +++ b/app/models/message.js @@ -10,6 +10,10 @@ export default class Message { @tracked edited = false; @tracked grouped = false; + // Stable identity used for list keys and scroll anchoring. Assigned by + // `BaseChannel.addMessage` / `addDateHeadline`; see `utils/message-key`. + key = null; + constructor (props) { Object.assign(this, props); } diff --git a/app/modifiers/chat-scroll.js b/app/modifiers/chat-scroll.js new file mode 100644 index 00000000..ea9c23da --- /dev/null +++ b/app/modifiers/chat-scroll.js @@ -0,0 +1,38 @@ +import { modifier } from 'ember-modifier'; + +/** + * Sets up DOM observation for a chat scroll container. + * + * Observes both the scroll container and its content wrapper with a + * `ResizeObserver` (so content growth/reflow is detected even when the + * container itself doesn't change size), and forwards scroll and `scrollend` + * events. All scroll policy lives in the component; this modifier only wires + * up and tears down observers/listeners. + */ +export default modifier((element, _positional, { onReady, onScroll, onScrollEnd, onContentResize }) => { + onReady?.(element); + + // Observe the content wrapper, not the scroll container itself: the container + // resizes when a scrollbar appears/disappears, which would otherwise cause a + // resize/scroll feedback loop. Window resizes are handled separately. + const content = element.firstElementChild ?? element; + + const resizeObserver = new ResizeObserver(() => onContentResize?.()); + resizeObserver.observe(content); + + const windowResizeHandler = () => onContentResize?.(); + window.addEventListener('resize', windowResizeHandler); + + const scrollHandler = (event) => onScroll?.(event); + element.addEventListener('scroll', scrollHandler, { passive: true }); + + const scrollEndHandler = () => onScrollEnd?.(); + element.addEventListener('scrollend', scrollEndHandler); + + return () => { + resizeObserver.disconnect(); + window.removeEventListener('resize', windowResizeHandler); + element.removeEventListener('scroll', scrollHandler); + element.removeEventListener('scrollend', scrollEndHandler); + }; +}, { eager: false }); diff --git a/app/modifiers/on-change.js b/app/modifiers/on-change.js new file mode 100644 index 00000000..a978fdf8 --- /dev/null +++ b/app/modifiers/on-change.js @@ -0,0 +1,22 @@ +import { modifier } from 'ember-modifier'; +import { untrack } from '@glimmer/validator'; + +/** + * Runs `callback` immediately and again whenever `value` changes. + * + * This is the `did-update` semantics that `ember-render-modifiers` used to + * provide. Unlike a plain `on-render` (install-only) modifier, it autotracks + * `value`, so it is safe to use for reacting to changed arguments such as the + * current channel or rendered messages. + * + * The callback is run inside `untrack`: ember-modifier autotracks every tracked + * value the modifier function accesses, including values read inside the + * callback. Callbacks often read other tracked state (e.g. `channelChanged` + * reads `channel.sortedMessages`), which must not become a dependency of this + * modifier, otherwise it re-runs on unrelated changes. + */ +export default modifier((element, [value, callback]) => { + untrack(() => callback(value)); + + return () => {}; +}, { eager: false }); diff --git a/app/modifiers/on-channel-change.js b/app/modifiers/on-channel-change.js deleted file mode 100644 index 4489275f..00000000 --- a/app/modifiers/on-channel-change.js +++ /dev/null @@ -1,8 +0,0 @@ -import { modifier } from 'ember-modifier'; - -export default modifier((element, [onChannelChange]) => { - // Call immediately on mount - onChannelChange(); - - return () => {}; -}, { eager: false }); diff --git a/app/modifiers/on-users-change.js b/app/modifiers/on-users-change.js deleted file mode 100644 index a719f43e..00000000 --- a/app/modifiers/on-users-change.js +++ /dev/null @@ -1,9 +0,0 @@ -import { modifier } from 'ember-modifier'; - -export default modifier((element, [onUsersChange]) => { - // Call immediately on mount - onUsersChange(element); - - // No cleanup needed, but we return a cleanup function anyway - return () => {}; -}, { eager: false }); diff --git a/app/styles/components/channel-container.scss b/app/styles/components/channel-container.scss index fb9e0177..54059028 100644 --- a/app/styles/components/channel-container.scss +++ b/app/styles/components/channel-container.scss @@ -147,6 +147,13 @@ } #channel-content { + // We manage scroll anchoring ourselves (see ChatScrollerComponent), so opt + // out of the browser's anchoring to avoid double-correction. + overflow-anchor: none; + // Reserve space for the scrollbar so its appearance/disappearance doesn't + // reflow the message list (which fed the resize/scroll feedback loop). + scrollbar-gutter: stable; + a { color: #aaa; text-decoration: underline; @@ -169,6 +176,47 @@ } } } + + .jump-to-latest { + position: sticky; + bottom: 0; + z-index: 10; + display: flex; + align-items: center; + gap: 0.35rem; + width: fit-content; + // Pin to the bottom-right corner. The content already has bottom padding, + // so no extra offset is needed. + margin: 0 1rem 0 auto; + padding: 0.5rem; + border: none; + border-radius: 9999px; + // Neutral "scroll to bottom" state: filled grey circle, white chevron. + background-color: #9ca3af; + color: #fff; + font-size: 0.875rem; + box-shadow: 0 2px 8px rgba(0, 0, 0, 0.25); + cursor: pointer; + + &:hover { + background-color: #6b7280; + } + + svg { + width: 1.1rem; + height: 1.1rem; + } + + // With new messages: blue pill, chevron followed by the count. + &.has-new-messages { + padding: 0.5rem 1rem; + background-color: #1d4ed8; + + &:hover { + background-color: #1e40af; + } + } + } } } diff --git a/app/utils/chat-scroll-state.js b/app/utils/chat-scroll-state.js new file mode 100644 index 00000000..64c6cc02 --- /dev/null +++ b/app/utils/chat-scroll-state.js @@ -0,0 +1,150 @@ +/** + * Pure scroll policy for chat message lists. + * + * This module contains no DOM access: given the current state and an event it + * returns the next state plus a list of effects for the DOM adapter + * (`ChatScrollerComponent`) to apply. Keeping the policy pure makes the tricky + * parts (stickiness, anchoring, history loading) straightforward to unit test. + */ + +export const AT_BOTTOM_THRESHOLD = 50; // px +export const AT_TOP_THRESHOLD = 200; // px + +export const ScrollAction = Object.freeze({ + ScrollToBottom: 'ScrollToBottom', + CaptureAnchor: 'CaptureAnchor', + RestoreAnchor: 'RestoreAnchor', + LoadOlder: 'LoadOlder', +}); + +export function isAtBottom ( + { scrollTop, scrollHeight, clientHeight }, + threshold = AT_BOTTOM_THRESHOLD +) { + // When the content fits entirely, we are always at the bottom. + if (scrollHeight <= clientHeight) return true; + // Use a small margin so fractional/DPI-related scroll values don't cause flicker. + return scrollHeight - scrollTop - clientHeight <= threshold; +} + +export function isNearTop ( + { scrollTop, scrollHeight, clientHeight }, + threshold = AT_TOP_THRESHOLD +) { + // Only meaningful when there is actually something to scroll. This prevents + // short channels without a scrollbar from auto-loading their entire history. + if (scrollHeight <= clientHeight) return false; + return scrollTop <= threshold; +} + +export function createScrollState () { + return { + atBottom: true, + stickToBottom: true, + anchor: null, + programmaticScroll: false, + newMessageCount: 0, + showJumpToLatest: false, + }; +} + +/** + * @param {object} state current scroll state (see createScrollState) + * @param {object} event one of: + * { type: 'reset' } + * { type: 'scroll-to-bottom' } + * { type: 'programmatic-scroll-started' } + * { type: 'programmatic-scroll-ended' } + * { type: 'user-scroll', metrics } + * { type: 'content-appended', count } + * { type: 'content-resized' } + * { type: 'older-prepended' } + * { type: 'reached-top', hasOlder, isLoadingOlder } + * @returns {{ state: object, effects: string[] }} + */ +export function reduceScrollState (state, event) { + switch (event.type) { + case 'reset': + // TODO: When read status is cached/persisted, scroll to the first unread + // message instead of the bottom (see issue #285 and the unread divider plan). + return { + state: createScrollState(), + effects: [ScrollAction.ScrollToBottom], + }; + + case 'scroll-to-bottom': + return { + state: { + ...state, + atBottom: true, + stickToBottom: true, + newMessageCount: 0, + showJumpToLatest: false, + }, + effects: [ScrollAction.ScrollToBottom], + }; + + case 'programmatic-scroll-started': + return { state: { ...state, programmaticScroll: true }, effects: [] }; + + case 'programmatic-scroll-ended': + return { state: { ...state, programmaticScroll: false }, effects: [] }; + + case 'user-scroll': { + if (isAtBottom(event.metrics)) { + return { + state: { + ...state, + atBottom: true, + stickToBottom: true, + newMessageCount: 0, + showJumpToLatest: false, + }, + effects: [], + }; + } + return { + state: { ...state, atBottom: false, stickToBottom: false }, + effects: [ScrollAction.CaptureAnchor], + }; + } + + case 'content-appended': { + if (state.stickToBottom) { + return { state, effects: [ScrollAction.ScrollToBottom] }; + } + const newMessageCount = state.newMessageCount + Math.max(1, event.count ?? 1); + return { + state: { ...state, newMessageCount, showJumpToLatest: true }, + effects: [], + }; + } + + case 'content-resized': { + if (state.stickToBottom) { + return { state, effects: [ScrollAction.ScrollToBottom] }; + } + return { state, effects: [ScrollAction.RestoreAnchor] }; + } + + case 'older-prepended': + return { state, effects: [ScrollAction.RestoreAnchor] }; + + case 'reached-top': { + if (!event.hasOlder || event.isLoadingOlder) { + return { state, effects: [] }; + } + // While sticking to the bottom (e.g. filling a short history), don't + // anchor: the new page should keep the newest message in view. When the + // user is reading history, capture the anchor so their position is kept + // while older messages are revealed/loaded above. + const effects = state.stickToBottom + ? [ScrollAction.LoadOlder] + : [ScrollAction.CaptureAnchor, ScrollAction.LoadOlder]; + return { state, effects }; + } + + default: + return { state, effects: [] }; + } +} diff --git a/tests/integration/components/chat-scroller-test.js b/tests/integration/components/chat-scroller-test.js new file mode 100644 index 00000000..63a5ed8b --- /dev/null +++ b/tests/integration/components/chat-scroller-test.js @@ -0,0 +1,318 @@ +import { module, test } from 'qunit'; +import { setupRenderingTest } from 'ember-qunit'; +import { render, settled, waitUntil } from '@ember/test-helpers'; +import { hbs } from 'ember-cli-htmlbars'; + +function items (count, startAt = 0) { + return Array.from({ length: count }, (_, index) => ({ id: `m${startAt + index}` })); +} + +function isScrolledToBottom (element) { + return element.scrollTop + element.clientHeight >= element.scrollHeight - 2; +} + +function anchoredNode (scroller) { + const containerTop = scroller.getBoundingClientRect().top; + for (const node of scroller.querySelectorAll('[data-message-key]')) { + if (node.getBoundingClientRect().bottom > containerTop) return node; + } + return null; +} + +function offsetWithin (scroller, node) { + return node.getBoundingClientRect().top - scroller.getBoundingClientRect().top; +} + +function wait (ms) { + return new Promise((resolve) => setTimeout(resolve, ms)); +} + +const TEMPLATE = hbs` + {{! template-lint-disable no-inline-styles }} + + {{#unless scroll.isAtBottom}} + + {{/unless}} + {{#each this.items key="id" as |item|}} +
{{item.id}}
+ {{/each}} +
+`; + +module('Integration | Component | chat-scroller', function (hooks) { + setupRenderingTest(hooks); + + hooks.beforeEach(function () { + this.resetKey = 'channel-a'; + this.hasOlder = false; + this.isLoadingOlder = false; + this.onLoadOlder = () => {}; + }); + + async function renderScroller () { + await render(TEMPLATE); + const scroller = document.getElementById('scroller'); + scroller.style.height = '200px'; + scroller.style.overflowY = 'auto'; + await settled(); + return scroller; + } + + test('scrolls to the bottom on initial render', async function (assert) { + this.items = items(20); + + const scroller = await renderScroller(); + await waitUntil(() => isScrolledToBottom(scroller), { timeout: 2000 }); + + assert.true(isScrolledToBottom(scroller), 'is scrolled to the bottom'); + }); + + test('stays at the bottom when items are appended while at the bottom', async function (assert) { + this.items = items(20); + + const scroller = await renderScroller(); + await waitUntil(() => isScrolledToBottom(scroller), { timeout: 2000 }); + + this.set('items', items(25)); + await settled(); + await waitUntil(() => isScrolledToBottom(scroller), { timeout: 2000 }); + + assert.true(isScrolledToBottom(scroller), 'still at the bottom after appending'); + assert.dom('.jump-to-latest').doesNotExist('does not show the jump-to-latest button'); + }); + + test('shows the scroll-to-bottom button when scrolled up without new messages', async function (assert) { + this.items = items(20); + + const scroller = await renderScroller(); + await waitUntil(() => isScrolledToBottom(scroller), { timeout: 2000 }); + assert.dom('.jump-to-latest').doesNotExist('hidden while at the bottom'); + + scroller.scrollTop = 0; + scroller.dispatchEvent(new Event('scroll')); + await settled(); + + assert.dom('.jump-to-latest').exists('shows the button when scrolled up'); + assert.dom('.jump-to-latest').doesNotHaveClass( + 'has-new-messages', + 'is in the neutral (no new messages) state' + ); + }); + + test('detaches when scrolled up and offers a way back for new messages', async function (assert) { + this.items = items(20); + + const scroller = await renderScroller(); + await waitUntil(() => isScrolledToBottom(scroller), { timeout: 2000 }); + + // Simulate a user scrolling away from the bottom. + scroller.scrollTop = 0; + scroller.dispatchEvent(new Event('scroll')); + await settled(); + + assert.dom('.jump-to-latest').exists('shows the scroll-to-bottom button when scrolled up'); + assert.dom('.jump-to-latest').doesNotHaveClass( + 'has-new-messages', + 'neutral until new messages arrive' + ); + + // A new message arriving while detached should not yank the viewport down, + // but should offer a way back. + this.set('items', items(21)); + await settled(); + await waitUntil(() => document.querySelector('.jump-to-latest.has-new-messages'), { timeout: 2000 }); + + assert.dom('.jump-to-latest').hasClass('has-new-messages', 'switches to the new-messages state'); + assert.dom('.jump-to-latest .count').hasText('1 new', 'shows the new message count'); + assert.false(isScrolledToBottom(scroller), 'did not jump to the bottom automatically'); + + await document.querySelector('.jump-to-latest').click(); + await settled(); + await waitUntil(() => isScrolledToBottom(scroller), { timeout: 2000 }); + + assert.true(isScrolledToBottom(scroller), 'jumped to the bottom when clicked'); + assert.dom('.jump-to-latest').doesNotExist('hides the button after jumping'); + }); + + test('loads older history until the viewport is filled', async function (assert) { + this.items = items(2); // 80px in a 200px window + + let calls = 0; + this.onLoadOlder = () => { + calls += 1; + const older = [{ id: `old${calls}a` }, { id: `old${calls}b` }]; + this.set('items', [...older, ...this.items]); + }; + + const scroller = await renderScroller(); + + // Enable history loading only once the container is constrained to a + // fixed height, otherwise the content trivially "fits" during layout. + this.set('hasOlder', true); + await settled(); + window.dispatchEvent(new Event('resize')); + + await waitUntil(() => scroller.scrollHeight > scroller.clientHeight, { timeout: 3000 }); + + assert.true(scroller.scrollHeight > scroller.clientHeight, 'viewport is filled'); + assert.ok(calls >= 2, `loaded multiple pages to fill the viewport (${calls})`); + + const callsWhenFilled = calls; + window.dispatchEvent(new Event('resize')); + await settled(); + await wait(200); + + assert.strictEqual(calls, callsWhenFilled, 'does not keep loading once the viewport is filled'); + }); + + test('requests older history once when entering the top zone', async function (assert) { + this.items = items(20); + + let calls = 0; + this.onLoadOlder = () => { + calls += 1; + }; + + const scroller = await renderScroller(); + this.set('hasOlder', true); + await settled(); + + // Enter the top zone. + scroller.scrollTop = 100; + scroller.dispatchEvent(new Event('scroll')); + await waitUntil(() => calls >= 1, { timeout: 2000 }); + + // Keep firing scroll events while "holding" in the top zone. + for (const scrollTop of [120, 90]) { + scroller.scrollTop = scrollTop; + scroller.dispatchEvent(new Event('scroll')); + await settled(); + } + await wait(100); + + assert.strictEqual(calls, 1, 'only requests history once while within the top zone'); + }); + + test('does not request older history while a load is in flight', async function (assert) { + this.items = items(20); + + let calls = 0; + this.onLoadOlder = () => { + calls += 1; + }; + + const scroller = await renderScroller(); + this.set('hasOlder', true); + this.set('isLoadingOlder', true); + await settled(); + + scroller.scrollTop = 100; + scroller.dispatchEvent(new Event('scroll')); + await settled(); + await wait(100); + + assert.strictEqual(calls, 0, 'does not request a new page while loading'); + }); + + test('loads the next page after the in-flight load finishes when at the top', async function (assert) { + this.items = items(20); + + let calls = 0; + this.onLoadOlder = () => { + calls += 1; + }; + + const scroller = await renderScroller(); + this.set('hasOlder', true); + this.set('isLoadingOlder', true); + await settled(); + + // Reach the top while a page is still loading; the request is deferred. + scroller.scrollTop = 100; + scroller.dispatchEvent(new Event('scroll')); + await settled(); + await wait(200); // let native scroll/scrollend events fully settle + assert.strictEqual(calls, 0, 'does not start a concurrent load'); + + // When the in-flight load finishes, the next page is requested automatically + // without having to scroll down and back up. + this.set('isLoadingOlder', false); + await settled(); + await waitUntil(() => calls >= 1, { timeout: 2000 }); + + assert.strictEqual(calls, 1, 'requests the next page once loading finishes'); + }); + + test('keeps the anchored message in place when older messages are prepended', async function (assert) { + this.items = items(20); + + const scroller = await renderScroller(); + await waitUntil(() => isScrolledToBottom(scroller), { timeout: 2000 }); + await wait(200); + + scroller.scrollTop = 120; + scroller.dispatchEvent(new Event('scroll')); + await settled(); + await wait(50); + + const before = anchoredNode(scroller); + assert.ok(before, 'has an anchored message'); + const beforeKey = before.dataset.messageKey; + const beforeOffset = offsetWithin(scroller, before); + + const older = [{ id: 'old1' }, { id: 'old2' }, { id: 'old3' }]; + this.set('items', [...older, ...this.items]); + await settled(); + await wait(100); + + const after = scroller.querySelector(`[data-message-key="${beforeKey}"]`); + assert.ok(after, 'the anchored message is still rendered'); + assert.ok( + Math.abs(offsetWithin(scroller, after) - beforeOffset) <= 2, + `anchor offset preserved (before=${beforeOffset}, after=${offsetWithin(scroller, after)})` + ); + }); + + test('keeps the anchored message in place when older messages are prepended and a new message is appended', async function (assert) { + this.items = items(20); + + const scroller = await renderScroller(); + await waitUntil(() => isScrolledToBottom(scroller), { timeout: 2000 }); + await wait(200); + + scroller.scrollTop = 120; + scroller.dispatchEvent(new Event('scroll')); + await settled(); + await wait(50); + + const before = anchoredNode(scroller); + assert.ok(before, 'has an anchored message'); + const beforeKey = before.dataset.messageKey; + const beforeOffset = offsetWithin(scroller, before); + + const older = [{ id: 'old1' }, { id: 'old2' }, { id: 'old3' }]; + this.set('items', [...older, ...this.items, { id: 'new1' }]); + await settled(); + await wait(100); + + const after = scroller.querySelector(`[data-message-key="${beforeKey}"]`); + assert.ok(after, 'the anchored message is still rendered'); + assert.ok( + Math.abs(offsetWithin(scroller, after) - beforeOffset) <= 2, + `anchor offset preserved (before=${beforeOffset}, after=${offsetWithin(scroller, after)})` + ); + }); +}); diff --git a/tests/integration/modifiers/on-change-test.js b/tests/integration/modifiers/on-change-test.js new file mode 100644 index 00000000..061e49f4 --- /dev/null +++ b/tests/integration/modifiers/on-change-test.js @@ -0,0 +1,61 @@ +import { module, test } from 'qunit'; +import { setupRenderingTest } from 'ember-qunit'; +import { render, settled } from '@ember/test-helpers'; +import { hbs } from 'ember-cli-htmlbars'; +import { tracked } from '@glimmer/tracking'; + +class State { + @tracked counter = 0; +} + +module('Integration | Modifier | on-change', function (hooks) { + setupRenderingTest(hooks); + + test('runs on install and when the value changes, but not on unrelated rerenders', async function (assert) { + this.calls = []; + this.value = 'a'; + this.other = 0; + this.handleChange = (value) => this.calls.push(value); + + await render(hbs` +
+ `); + + assert.deepEqual(this.calls, ['a'], 'runs once on install'); + + this.set('other', 1); + await settled(); + + assert.deepEqual(this.calls, ['a'], 'does not rerun on an unrelated rerender'); + + this.set('value', 'b'); + await settled(); + + assert.deepEqual(this.calls, ['a', 'b'], 'reruns when the value changes'); + }); + + test('does not rerun when tracked state read by the callback changes', async function (assert) { + this.calls = []; + this.value = 'a'; + this.state = new State(); + this.handleChange = (value) => this.calls.push(`${value}:${this.state.counter}`); + + await render(hbs` +
+ `); + + assert.deepEqual(this.calls, ['a:0'], 'runs once on install'); + + // The callback reads `state.counter`; changing it must not retrigger the + // modifier (which would have reset unrelated state, e.g. the render window). + this.state.counter = 1; + await settled(); + + assert.deepEqual(this.calls, ['a:0'], 'does not rerun when the callback reads changed tracked state'); + + this.set('value', 'b'); + await settled(); + + assert.deepEqual(this.calls, ['a:0', 'b:1'], 'reruns when the value changes'); + }); +}); diff --git a/tests/unit/utils/chat-scroll-state-test.js b/tests/unit/utils/chat-scroll-state-test.js new file mode 100644 index 00000000..77aa80b3 --- /dev/null +++ b/tests/unit/utils/chat-scroll-state-test.js @@ -0,0 +1,172 @@ +import { module, test } from 'qunit'; +import { + ScrollAction, + createScrollState, + isAtBottom, + isNearTop, + reduceScrollState, +} from 'hyperchannel/utils/chat-scroll-state'; + +const metrics = (scrollTop, scrollHeight, clientHeight) => ({ + scrollTop, + scrollHeight, + clientHeight, +}); + +module('Unit | Utility | chat scroll state', function () { + module('isAtBottom', function () { + test('is true when the content fits entirely', function (assert) { + assert.true(isAtBottom(metrics(0, 300, 500))); + }); + + test('is true within the threshold of the bottom', function (assert) { + assert.true(isAtBottom(metrics(700, 1250, 500))); // 50px remaining + }); + + test('is false when scrolled away from the bottom', function (assert) { + assert.false(isAtBottom(metrics(400, 1250, 500))); // 350px remaining + }); + }); + + module('isNearTop', function () { + test('is false when the content fits entirely', function (assert) { + assert.false(isNearTop(metrics(0, 300, 500))); + }); + + test('is true within the threshold of the top', function (assert) { + assert.true(isNearTop(metrics(150, 1250, 500))); + }); + + test('is false away from the top', function (assert) { + assert.false(isNearTop(metrics(600, 1250, 500))); + }); + }); + + module('reduceScrollState', function () { + test('reset returns to a sticky bottom state and scrolls down', function (assert) { + const dirty = { + ...createScrollState(), + atBottom: false, + stickToBottom: false, + newMessageCount: 4, + showJumpToLatest: true, + }; + + const { state, effects } = reduceScrollState(dirty, { type: 'reset' }); + + assert.true(state.stickToBottom); + assert.strictEqual(state.newMessageCount, 0); + assert.strictEqual(state.showJumpToLatest, false); + assert.deepEqual(effects, [ScrollAction.ScrollToBottom]); + }); + + test('a user scroll to the bottom re-enables stickiness and clears the pill', function (assert) { + const detached = { + ...createScrollState(), + stickToBottom: false, + newMessageCount: 3, + showJumpToLatest: true, + }; + + const { state, effects } = reduceScrollState(detached, { + type: 'user-scroll', + metrics: metrics(750, 1250, 500), + }); + + assert.true(state.stickToBottom); + assert.true(state.atBottom); + assert.strictEqual(state.newMessageCount, 0); + assert.strictEqual(state.showJumpToLatest, false); + assert.deepEqual(effects, []); + }); + + test('a user scroll away from the bottom detaches and captures an anchor', function (assert) { + const { state, effects } = reduceScrollState(createScrollState(), { + type: 'user-scroll', + metrics: metrics(300, 1250, 500), + }); + + assert.false(state.stickToBottom); + assert.false(state.atBottom); + assert.deepEqual(effects, [ScrollAction.CaptureAnchor]); + }); + + test('appended content scrolls to the bottom while sticky', function (assert) { + const { effects } = reduceScrollState(createScrollState(), { + type: 'content-appended', + count: 1, + }); + + assert.deepEqual(effects, [ScrollAction.ScrollToBottom]); + }); + + test('appended content while detached counts new messages', function (assert) { + const detached = { ...createScrollState(), stickToBottom: false }; + + const first = reduceScrollState(detached, { type: 'content-appended', count: 2 }); + const second = reduceScrollState(first.state, { type: 'content-appended', count: 1 }); + + assert.strictEqual(second.state.newMessageCount, 3); + assert.true(second.state.showJumpToLatest); + assert.deepEqual(second.effects, []); + }); + + test('resized content keeps the anchor while detached', function (assert) { + const detached = { ...createScrollState(), stickToBottom: false }; + + const { effects } = reduceScrollState(detached, { type: 'content-resized' }); + + assert.deepEqual(effects, [ScrollAction.RestoreAnchor]); + }); + + test('prepended older messages restore the anchor', function (assert) { + const detached = { ...createScrollState(), stickToBottom: false }; + + const { effects } = reduceScrollState(detached, { type: 'older-prepended' }); + + assert.deepEqual(effects, [ScrollAction.RestoreAnchor]); + }); + + test('reaching the top loads older messages without anchoring while stuck to the bottom', function (assert) { + const { effects } = reduceScrollState(createScrollState(), { + type: 'reached-top', + hasOlder: true, + isLoadingOlder: false, + }); + + assert.deepEqual(effects, [ScrollAction.LoadOlder]); + }); + + test('reaching the top captures an anchor and loads older messages while reading history', function (assert) { + const detached = { ...createScrollState(), stickToBottom: false }; + + const { effects } = reduceScrollState(detached, { + type: 'reached-top', + hasOlder: true, + isLoadingOlder: false, + }); + + assert.deepEqual(effects, [ScrollAction.CaptureAnchor, ScrollAction.LoadOlder]); + }); + + test('reaching the top does not load when there is nothing older', function (assert) { + const { effects } = reduceScrollState(createScrollState(), { + type: 'reached-top', + hasOlder: false, + isLoadingOlder: false, + }); + + assert.deepEqual(effects, []); + }); + + test('reaching the top does not load while a load is in flight', function (assert) { + const { effects } = reduceScrollState(createScrollState(), { + type: 'reached-top', + hasOlder: true, + isLoadingOlder: true, + }); + + assert.deepEqual(effects, []); + }); + }); +}); From d3ba5a303b5395d2576373771545569b1ac92883 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Thu, 1 Oct 2026 18:58:19 +0200 Subject: [PATCH 04/25] Release the scroll maintenance lock on user intent for instant scrolls For instant programmatic scrolls (scroll to bottom, anchor restore), record the maintenance lock's observed position as the post-scroll target, so any deviating scroll event is treated as the user interrupting and releases the lock immediately. Previously such an event could be misread as our own scroll in progress (moving toward the target), leaving stickToBottom stuck true and preventing auto-loading after the user reached the top. Also make the in-flight auto-load test wait for the initial programmatic scroll to settle before scrolling, removing a timing dependence that surfaced on Chrome 154. --- app/components/chat-scroller.js | 6 ++++++ tests/integration/components/chat-scroller-test.js | 3 +++ 2 files changed, 9 insertions(+) diff --git a/app/components/chat-scroller.js b/app/components/chat-scroller.js index 407057dd..b20ff3b4 100644 --- a/app/components/chat-scroller.js +++ b/app/components/chat-scroller.js @@ -185,6 +185,10 @@ export default class ChatScrollerComponent extends Component { this.beginMaintenance(); element.scrollTop = element.scrollHeight; this.maintenanceTarget = element.scrollTop; + // For an instant scroll the position is already at the target, so any + // deviating scroll event means the user interrupted: treat it as such + // immediately instead of waiting for the maintenance timeout. + this.maintenanceObservedTop = element.scrollTop; } } @@ -231,6 +235,8 @@ export default class ChatScrollerComponent extends Component { this.beginMaintenance(); element.scrollTop += delta; this.maintenanceTarget = element.scrollTop; + // Instant scroll: see applyScrollToBottom. + this.maintenanceObservedTop = element.scrollTop; } // --- Auto-loading --------------------------------------------------------- diff --git a/tests/integration/components/chat-scroller-test.js b/tests/integration/components/chat-scroller-test.js index 63a5ed8b..72c51823 100644 --- a/tests/integration/components/chat-scroller-test.js +++ b/tests/integration/components/chat-scroller-test.js @@ -236,6 +236,9 @@ module('Integration | Component | chat-scroller', function (hooks) { }; const scroller = await renderScroller(); + await waitUntil(() => isScrolledToBottom(scroller), { timeout: 2000 }); + await wait(200); // let the initial programmatic scroll release its lock + this.set('hasOlder', true); this.set('isLoadingOlder', true); await settled(); From 978109171d332ccd8eb2a61e805d39d6f6777a22 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= <842+raucao@users.noreply.github.com> Date: Thu, 1 Oct 2026 19:02:54 +0200 Subject: [PATCH 05/25] Prevent rendering if channel is destroyed or changed Add early return condition for channel updates Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- app/components/channel-container.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/components/channel-container.js b/app/components/channel-container.js index c2c94c09..0e4fe951 100644 --- a/app/components/channel-container.js +++ b/app/components/channel-container.js @@ -97,6 +97,8 @@ export default class ChannelContainerComponent extends Component { // Keep the currently rendered messages visible and reveal a page of the // newly loaded ones. `ChatScroller` restores the scroll position around it. + if (this.args.channel !== channel || this.isDestroyed || this.isDestroying) return; + const added = channel.sortedMessages.length - lengthBefore; if (added > 0) { this.renderedStartIndex = Math.max(0, added - RENDERED_MESSAGES_INCREMENT); From 6b00e5e1ffc8b0964a794e3384ec4fec977d00d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= <842+raucao@users.noreply.github.com> Date: Thu, 1 Oct 2026 19:03:41 +0200 Subject: [PATCH 06/25] Enhance scroll button for better accessibility Updated button accessibility by adding screen reader text. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- app/components/channel-container.hbs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/components/channel-container.hbs b/app/components/channel-container.hbs index 443c4ad1..5041b13a 100644 --- a/app/components/channel-container.hbs +++ b/app/components/channel-container.hbs @@ -75,10 +75,10 @@ {{#unless scroll.isAtBottom}} +
+ {{else if this.showHistoryStart}}
Beginning of history
diff --git a/app/components/channel-container.js b/app/components/channel-container.js index 0e4fe951..edf1cbd4 100644 --- a/app/components/channel-container.js +++ b/app/components/channel-container.js @@ -21,6 +21,7 @@ export default class ChannelContainerComponent extends Component { // message volumes are currently small enough that this is not yet needed. @tracked renderedStartIndex = 0; @tracked historyLoaded = false; + @tracked historyLoadFailed = false; @cached get renderedMessages () { @@ -32,9 +33,12 @@ export default class ChannelContainerComponent extends Component { } // Whether there is anything older to reveal or fetch: either older messages - // already in memory (window not at the start) or more archive pages. + // already in memory (window not at the start) or more archive pages. A failed + // page stops auto-loading (see `historyLoadFailed`) so we don't retry the same + // failing request in a loop; the user can retry explicitly. get canLoadOlderMessages () { - return this.renderedStartIndex > 0 || this.args.channel.hasOlderMessages; + return !this.historyLoadFailed && + (this.renderedStartIndex > 0 || this.args.channel.hasOlderMessages); } get isLoadingOlderMessages () { @@ -44,7 +48,7 @@ export default class ChannelContainerComponent extends Component { // Only show the "beginning of history" marker once older history has // actually been loaded/revealed for this channel. get showHistoryStart () { - return this.historyLoaded && !this.canLoadOlderMessages; + return this.historyLoaded && !this.historyLoadFailed && !this.canLoadOlderMessages; } @action @@ -52,6 +56,7 @@ export default class ChannelContainerComponent extends Component { const messageCount = this.args.channel.sortedMessages.length; this.renderedStartIndex = Math.max(0, messageCount - INITIAL_RENDERED_MESSAGES); this.historyLoaded = false; + this.historyLoadFailed = false; setTimeout(() => { if (this.isDestroyed || this.isDestroying) return; @@ -91,6 +96,12 @@ export default class ChannelContainerComponent extends Component { this.historyLoaded = true; try { await this.coms.loadOlderMessages(channel); + } catch (error) { + // Stop auto-loading so we don't retry the same failing page in a loop or + // leave an unhandled rejection; the user can retry explicitly. + this.historyLoadFailed = true; + console.error('[channel] Failed to load older messages', error); + return; } finally { channel.loadingOlderMessages = false; } @@ -105,6 +116,12 @@ export default class ChannelContainerComponent extends Component { } } + @action + retryLoadOlderMessages () { + this.historyLoadFailed = false; + this.loadOlderMessages(); + } + focusMessageInputField () { const inputEl = document.querySelector('input#message-field'); inputEl.focus(); diff --git a/app/styles/components/channel-container.scss b/app/styles/components/channel-container.scss index 54059028..96487739 100644 --- a/app/styles/components/channel-container.scss +++ b/app/styles/components/channel-container.scss @@ -177,6 +177,22 @@ } } + .history-load-error { + button.retry { + padding: 0; + border: none; + background: none; + color: inherit; + font: inherit; + text-decoration: underline; + cursor: pointer; + + &:hover { + color: #333; + } + } + } + .jump-to-latest { position: sticky; bottom: 0; diff --git a/tests/integration/components/channel-container-test.js b/tests/integration/components/channel-container-test.js index f8a244bc..5f63170d 100644 --- a/tests/integration/components/channel-container-test.js +++ b/tests/integration/components/channel-container-test.js @@ -1,6 +1,6 @@ import { module, test } from 'qunit'; import { setupRenderingTest } from 'ember-qunit'; -import { render } from '@ember/test-helpers'; +import { render, waitUntil, click } from '@ember/test-helpers'; import { hbs } from 'ember-cli-htmlbars'; import Service from '@ember/service'; import Channel from 'hyperchannel/models/channel'; @@ -11,6 +11,19 @@ class comsStub extends Service { loadArchiveMessages () {} } +const TEMPLATE = hbs` + +`; + +function wait (ms) { + return new Promise((resolve) => setTimeout(resolve, ms)); +} + module('Integration | Component | channel-container', function (hooks) { setupRenderingTest(hooks); @@ -51,4 +64,38 @@ module('Integration | Component | channel-container', function (hooks) { assert.strictEqual(messageContents.length, 1, 'renders only the chat message with MessageChat'); assert.strictEqual(messageContents[0].innerText.trim(), 'hello world', 'renders the chat message content'); }); + + test('stops auto-loading and offers a retry when loading older messages fails', async function (assert) { + let calls = 0; + class FailingComs extends Service { + async loadOlderMessages () { + calls += 1; + throw new Error('boom'); + } + } + this.owner.register('service:coms', FailingComs); + + this.channel.hasOlderMessages = true; + + await render(TEMPLATE); + + // Constrain the scroller so the "fill the viewport" auto-load path runs. + const scroller = document.getElementById('channel-content'); + scroller.style.height = '400px'; + scroller.style.overflowY = 'auto'; + window.dispatchEvent(new Event('resize')); + + await waitUntil(() => calls >= 1, { timeout: 2000 }); + await wait(300); + + assert.strictEqual(calls, 1, 'does not retry the failing request in a loop'); + assert.dom('.history-load-error').exists('shows a retry affordance'); + assert.dom('.history-start').doesNotExist('does not claim the beginning of history was reached'); + + await click('.history-load-error button'); + await waitUntil(() => calls >= 2, { timeout: 2000 }); + + assert.strictEqual(calls, 2, 'retry calls the loader again'); + }); }); + From 264db07fdc02c50b068c3a7482f11db376e7da7a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Thu, 1 Oct 2026 19:29:41 +0200 Subject: [PATCH 10/25] Address review feedback on history loading and scrolling - Guard the history-load failure flag against channel changes/destruction, so a late rejection from a previous channel doesn't show the retry error on the newly active channel or hide its history loading. - Announce the history-load error with role="alert". - Reset the user list on channel changes, not on every user-list recomputation, so presence updates no longer reset the render window and scroll to the top. - Mark the list at bottom only once a programmatic scroll settles (new `scroll-settled` event) instead of immediately in `scroll-to-bottom`. This keeps the jump button rendered during a smooth scroll; removing it mid-flight resized the observed content wrapper and turned the animation into an instant jump. --- app/components/channel-container.hbs | 2 +- app/components/channel-container.js | 4 +++ app/components/chat-scroller.js | 9 ++++++ app/components/user-list.hbs | 2 +- app/utils/chat-scroll-state.js | 12 +++++++- tests/unit/utils/chat-scroll-state-test.js | 35 ++++++++++++++++++++++ 6 files changed, 61 insertions(+), 3 deletions(-) diff --git a/app/components/channel-container.hbs b/app/components/channel-container.hbs index 12c661c8..cdb11631 100644 --- a/app/components/channel-container.hbs +++ b/app/components/channel-container.hbs @@ -37,7 +37,7 @@ @onLoadOlder={{this.loadOlderMessages}} as |scroll|> {{#if this.historyLoadFailed}} -
+ diff --git a/app/components/channel-container.js b/app/components/channel-container.js index edf1cbd4..76fb87cf 100644 --- a/app/components/channel-container.js +++ b/app/components/channel-container.js @@ -97,6 +97,10 @@ export default class ChannelContainerComponent extends Component { try { await this.coms.loadOlderMessages(channel); } catch (error) { + // Guard against the channel having changed or the component being torn + // down while the request was in flight, so we don't show the new + // channel's retry error or hide its history loading. + if (this.args.channel !== channel || this.isDestroyed || this.isDestroying) return; // Stop auto-loading so we don't retry the same failing page in a loop or // leave an unhandled rejection; the user can retry explicitly. this.historyLoadFailed = true; diff --git a/app/components/chat-scroller.js b/app/components/chat-scroller.js index d2f0a533..c73a00ed 100644 --- a/app/components/chat-scroller.js +++ b/app/components/chat-scroller.js @@ -158,8 +158,17 @@ export default class ChatScrollerComponent extends Component { clearTimeout(this.maintenanceTimer); this.maintenanceTimer = null; } + const wasMaintaining = this.maintainingScroll; this.maintainingScroll = false; this.maintenanceTarget = null; + + // Transition to "at bottom" only once a programmatic scroll has finished. + // Doing it earlier removes the jump button mid-animation, and its removal + // resizes the observed content wrapper, whose correction would turn a smooth + // scroll into an instant jump. + if (wasMaintaining && this.scrollState.stickToBottom) { + this.reduceAndApply({ type: 'scroll-settled' }); + } } prefersReducedMotion () { diff --git a/app/components/user-list.hbs b/app/components/user-list.hbs index 34a322a4..3267157e 100644 --- a/app/components/user-list.hbs +++ b/app/components/user-list.hbs @@ -1,4 +1,4 @@ -
+
{{#if @users}}

{{@users.length}} Online

{{/if}} diff --git a/app/utils/chat-scroll-state.js b/app/utils/chat-scroll-state.js index 64c6cc02..26b32554 100644 --- a/app/utils/chat-scroll-state.js +++ b/app/utils/chat-scroll-state.js @@ -53,6 +53,7 @@ export function createScrollState () { * @param {object} event one of: * { type: 'reset' } * { type: 'scroll-to-bottom' } + * { type: 'scroll-settled' } * { type: 'programmatic-scroll-started' } * { type: 'programmatic-scroll-ended' } * { type: 'user-scroll', metrics } @@ -73,10 +74,13 @@ export function reduceScrollState (state, event) { }; case 'scroll-to-bottom': + // Don't mark `atBottom` yet: the jump button lives inside the observed + // content wrapper, so removing it immediately would resize the wrapper and + // trigger a non-smooth correction that cancels a smooth scroll. Settle it + // in `scroll-settled` once the scroll has finished. return { state: { ...state, - atBottom: true, stickToBottom: true, newMessageCount: 0, showJumpToLatest: false, @@ -84,6 +88,12 @@ export function reduceScrollState (state, event) { effects: [ScrollAction.ScrollToBottom], }; + case 'scroll-settled': + if (!state.stickToBottom) { + return { state, effects: [] }; + } + return { state: { ...state, atBottom: true }, effects: [] }; + case 'programmatic-scroll-started': return { state: { ...state, programmaticScroll: true }, effects: [] }; diff --git a/tests/unit/utils/chat-scroll-state-test.js b/tests/unit/utils/chat-scroll-state-test.js index 77aa80b3..b7a291d4 100644 --- a/tests/unit/utils/chat-scroll-state-test.js +++ b/tests/unit/utils/chat-scroll-state-test.js @@ -60,6 +60,41 @@ module('Unit | Utility | chat scroll state', function () { assert.deepEqual(effects, [ScrollAction.ScrollToBottom]); }); + test('scroll-to-bottom keeps the detached state until the scroll settles', function (assert) { + const detached = { + ...createScrollState(), + atBottom: false, + stickToBottom: false, + newMessageCount: 2, + showJumpToLatest: true, + }; + + const { state, effects } = reduceScrollState(detached, { type: 'scroll-to-bottom' }); + + assert.true(state.stickToBottom); + assert.false(state.atBottom, 'stays detached until the scroll finishes'); + assert.strictEqual(state.newMessageCount, 0); + assert.strictEqual(state.showJumpToLatest, false); + assert.deepEqual(effects, [ScrollAction.ScrollToBottom]); + }); + + test('scroll-settled marks the list at the bottom while sticking', function (assert) { + const detached = { ...createScrollState(), atBottom: false, stickToBottom: true }; + + const { state, effects } = reduceScrollState(detached, { type: 'scroll-settled' }); + + assert.true(state.atBottom); + assert.deepEqual(effects, []); + }); + + test('scroll-settled does nothing when not sticking to the bottom', function (assert) { + const detached = { ...createScrollState(), atBottom: false, stickToBottom: false }; + + const { state } = reduceScrollState(detached, { type: 'scroll-settled' }); + + assert.false(state.atBottom); + }); + test('a user scroll to the bottom re-enables stickiness and clears the pill', function (assert) { const detached = { ...createScrollState(), From f311a66db731711534c590da363d2b785a9b7aef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Thu, 1 Oct 2026 20:00:36 +0200 Subject: [PATCH 11/25] Address the latest review: message scope, windowing, async safety - Scope transport ids by author and prefer server stanza ids in `messageKey`, so two senders using the same (sender-scoped) id no longer collapse into one message when `BaseChannel.addMessage` dedupes channel-wide. - Initialize the render window as an "auto" window (latest N) derived from the live message list, so the asynchronously loaded IRC archive is windowed correctly instead of rendering the entire batch. Revealing older history materializes a fixed start index. - Guard and cancel the scroll maintenance timer on destruction, and instrument the scroller's deferred async work (maintenance timer and frame callbacks) with an Ember test waiter so `settled()` synchronizes deterministically. - Replace fixed sleeps in the scroller tests with `settled()` / observable waits, and add regression tests for the message-key scoping and initial render window. --- app/components/channel-container.js | 38 ++++++++++++--- app/components/chat-scroller.js | 48 +++++++++++++++++-- app/utils/message-key.js | 13 +++-- .../components/channel-container-test.js | 41 +++++++++++++--- .../components/chat-scroller-test.js | 19 +------- tests/unit/models/base-channel-test.js | 35 +++++++++++++- tests/unit/utils/message-key-test.js | 17 ++++++- 7 files changed, 171 insertions(+), 40 deletions(-) diff --git a/app/components/channel-container.js b/app/components/channel-container.js index 76fb87cf..d1adbb9f 100644 --- a/app/components/channel-container.js +++ b/app/components/channel-container.js @@ -16,16 +16,27 @@ export default class ChannelContainerComponent extends Component { // We render from here to the end, so appending new messages at the bottom // never shifts the content the user is looking at. // + // `null` means "auto": render the latest `INITIAL_RENDERED_MESSAGES`. This is + // the initial state and is important because the IRC archive is loaded + // asynchronously after the component mounts; deriving the window from the live + // message list (rather than sizing it once at mount) means the initial batch + // is windowed correctly. It becomes a fixed index once the user starts reading + // older history. + // // TODO: bound the window size while detached and shed older DOM nodes // (Element-style, with height reservation) for very deep history. Session // message volumes are currently small enough that this is not yet needed. - @tracked renderedStartIndex = 0; + @tracked renderedStartIndex = null; @tracked historyLoaded = false; @tracked historyLoadFailed = false; @cached get renderedMessages () { - return this.args.channel.sortedMessages.slice(this.renderedStartIndex); + const all = this.args.channel.sortedMessages; + if (this.renderedStartIndex === null) { + return all.slice(Math.max(0, all.length - INITIAL_RENDERED_MESSAGES)); + } + return all.slice(this.renderedStartIndex); } get hasOlderMessages () { @@ -37,8 +48,12 @@ export default class ChannelContainerComponent extends Component { // page stops auto-loading (see `historyLoadFailed`) so we don't retry the same // failing request in a loop; the user can retry explicitly. get canLoadOlderMessages () { - return !this.historyLoadFailed && - (this.renderedStartIndex > 0 || this.args.channel.hasOlderMessages); + if (this.historyLoadFailed) return false; + const channel = this.args.channel; + if (this.renderedStartIndex === null) { + return channel.hasOlderMessages || channel.sortedMessages.length > INITIAL_RENDERED_MESSAGES; + } + return this.renderedStartIndex > 0 || channel.hasOlderMessages; } get isLoadingOlderMessages () { @@ -53,8 +68,8 @@ export default class ChannelContainerComponent extends Component { @action channelChanged () { - const messageCount = this.args.channel.sortedMessages.length; - this.renderedStartIndex = Math.max(0, messageCount - INITIAL_RENDERED_MESSAGES); + // `null` = auto-window the latest messages (see `renderedStartIndex`). + this.renderedStartIndex = null; this.historyLoaded = false; this.historyLoadFailed = false; @@ -82,6 +97,17 @@ export default class ChannelContainerComponent extends Component { const channel = this.args.channel; if (channel.loadingOlderMessages) return; + if (this.renderedStartIndex === null) { + // Materialize the auto-window and reveal one increment of older messages. + const length = channel.sortedMessages.length; + this.renderedStartIndex = Math.max( + 0, + length - INITIAL_RENDERED_MESSAGES - RENDERED_MESSAGES_INCREMENT + ); + this.historyLoaded = true; + return; + } + if (this.renderedStartIndex > 0) { this.renderedStartIndex = Math.max(0, this.renderedStartIndex - RENDERED_MESSAGES_INCREMENT); this.historyLoaded = true; diff --git a/app/components/chat-scroller.js b/app/components/chat-scroller.js index c73a00ed..691d1fe2 100644 --- a/app/components/chat-scroller.js +++ b/app/components/chat-scroller.js @@ -1,6 +1,8 @@ import Component from '@glimmer/component'; import { tracked } from '@glimmer/tracking'; import { action } from '@ember/object'; +import { registerDestructor } from '@ember/destroyable'; +import { buildWaiter } from '@ember/test-waiters'; import { ScrollAction, createScrollState, @@ -12,6 +14,10 @@ const MAINTENANCE_TIMEOUT = 150; // ms const SMOOTH_SCROLL_TIMEOUT = 1000; // ms; upper bound for a smooth scroll const FILL_MAX_NO_PROGRESS = 2; +// Tracks the component's deferred async work (maintenance timer, frame +// callbacks) so `settled()` can synchronize deterministically in tests. +const scrollerWaiter = buildWaiter('chat-scroller'); + function itemKey (item) { if (!item) return ''; return item.key ?? item.id ?? ''; @@ -58,6 +64,7 @@ export default class ChatScrollerComponent extends Component { maintenanceTimer = null; maintenanceTarget = null; maintenanceObservedTop = 0; + maintenanceWaiterToken = null; smoothScrollRequested = false; // Auto-load bookkeeping. @@ -66,6 +73,37 @@ export default class ChatScrollerComponent extends Component { lastFillHeight = 0; resizeScheduled = false; + constructor () { + super(...arguments); + registerDestructor(this, () => { + if (this.maintenanceTimer) { + clearTimeout(this.maintenanceTimer); + this.maintenanceTimer = null; + } + this.endMaintenanceWaiter(); + }); + } + + // Runs `callback` on the next frame, tracked by a test waiter so `settled()` + // can synchronize deterministically instead of tests relying on sleeps. + runInFrame (callback) { + const token = scrollerWaiter.beginAsync(); + requestAnimationFrame(() => { + try { + callback(); + } finally { + scrollerWaiter.endAsync(token); + } + }); + } + + endMaintenanceWaiter () { + if (this.maintenanceWaiterToken) { + scrollerWaiter.endAsync(this.maintenanceWaiterToken); + this.maintenanceWaiterToken = null; + } + } + get scrollApi () { return { isAtBottom: this.isAtBottom, @@ -107,7 +145,7 @@ export default class ChatScrollerComponent extends Component { // defer state changes and DOM effects to the next frame. Setting tracked // state during render would invalidate and re-run the modifier recursively. scheduleReduce (event) { - requestAnimationFrame(() => { + this.runInFrame(() => { if (this.isDestroyed || this.isDestroying) return; this.reduceAndApply(event); }); @@ -145,10 +183,13 @@ export default class ChatScrollerComponent extends Component { if (this.maintenanceTimer) { clearTimeout(this.maintenanceTimer); } + this.endMaintenanceWaiter(); + this.maintenanceWaiterToken = scrollerWaiter.beginAsync(); this.maintainingScroll = true; this.maintenanceObservedTop = this.scrollElement?.scrollTop ?? 0; this.maintenanceTimer = setTimeout(() => { this.maintenanceTimer = null; + if (this.isDestroyed || this.isDestroying) return; this.endMaintenance(); }, timeout); } @@ -158,6 +199,7 @@ export default class ChatScrollerComponent extends Component { clearTimeout(this.maintenanceTimer); this.maintenanceTimer = null; } + this.endMaintenanceWaiter(); const wasMaintaining = this.maintainingScroll; this.maintainingScroll = false; this.maintenanceTarget = null; @@ -421,7 +463,7 @@ export default class ChatScrollerComponent extends Component { if (this.resizeScheduled) return; this.resizeScheduled = true; - requestAnimationFrame(() => { + this.runInFrame(() => { this.resizeScheduled = false; if (this.isDestroyed || this.isDestroying) return; this.requestOlderIfNeeded(); @@ -435,7 +477,7 @@ export default class ChatScrollerComponent extends Component { // The in-flight page finished; allow requesting the next one if we are // still near the top (e.g. the user reached the top while it was loading). this.olderRequestPending = false; - requestAnimationFrame(() => { + this.runInFrame(() => { if (this.isDestroyed || this.isDestroying) return; this.requestOlderIfNeeded(); }); diff --git a/app/utils/message-key.js b/app/utils/message-key.js index 37cc901a..483c261e 100644 --- a/app/utils/message-key.js +++ b/app/utils/message-key.js @@ -4,16 +4,21 @@ import { bytesToHex, utf8ToBytes } from '@noble/hashes/utils.js'; /** * Stable identity for a message, used for list keys and scroll anchoring. * - * Messages that come from a transport with real ids use them. Older archive - * messages (e.g. remoteStorage IRC logs) have no id, so we derive one from the - * timestamp plus a hash of the author and content. + * - A server-assigned stanza id (`sid`) is unique within a room, so it is used + * as-is. + * - Other transport ids (e.g. XMPP stanza `id`s) are sender/stream scoped, so + * they are combined with the author. Otherwise two senders using the same id + * would collapse into one when `BaseChannel.addMessage` dedupes channel-wide. + * - Older archive messages (e.g. remoteStorage IRC logs) have no id, so one is + * derived from the timestamp plus a hash of the author and content. * * Note: two byte-identical messages in the same timestamp tick will produce the * same key and are treated as duplicates when added (`BaseChannel.addMessage`). * That is intentional; distinguishing such messages is not a supported feature. */ export function messageKey (message) { - if (message.id) return String(message.id); + if (message.sid) return `sid:${message.sid}`; + if (message.id) return `${message.nickname ?? ''}\u0000${message.id}`; const timestamp = message.date ? new Date(message.date).getTime() : 0; const identity = `${message.nickname ?? ''}\u0000${message.content ?? ''}`; diff --git a/tests/integration/components/channel-container-test.js b/tests/integration/components/channel-container-test.js index 5f63170d..04eab597 100644 --- a/tests/integration/components/channel-container-test.js +++ b/tests/integration/components/channel-container-test.js @@ -1,6 +1,6 @@ import { module, test } from 'qunit'; import { setupRenderingTest } from 'ember-qunit'; -import { render, waitUntil, click } from '@ember/test-helpers'; +import { render, settled, waitUntil, click } from '@ember/test-helpers'; import { hbs } from 'ember-cli-htmlbars'; import Service from '@ember/service'; import Channel from 'hyperchannel/models/channel'; @@ -20,10 +20,6 @@ const TEMPLATE = hbs` @addUsernameMentionToMessage={{this.noop}} /> `; -function wait (ms) { - return new Promise((resolve) => setTimeout(resolve, ms)); -} - module('Integration | Component | channel-container', function (hooks) { setupRenderingTest(hooks); @@ -86,7 +82,7 @@ module('Integration | Component | channel-container', function (hooks) { window.dispatchEvent(new Event('resize')); await waitUntil(() => calls >= 1, { timeout: 2000 }); - await wait(300); + await settled(); assert.strictEqual(calls, 1, 'does not retry the failing request in a loop'); assert.dom('.history-load-error').exists('shows a retry affordance'); @@ -97,5 +93,38 @@ module('Integration | Component | channel-container', function (hooks) { assert.strictEqual(calls, 2, 'retry calls the loader again'); }); + + test('applies the initial render window when the async archive batch arrives', async function (assert) { + // A freshly joined channel has no messages yet; the archive is loaded + // asynchronously after the component has sized its window. + this.channel = new Channel({ account: ircAccount, name: '#big', displayName: '#big' }); + + await render(TEMPLATE); + + // Constrain the scroller so the fill path doesn't reveal the whole batch. + const scroller = document.getElementById('channel-content'); + scroller.style.flex = '0 0 400px'; + scroller.style.height = '400px'; + scroller.style.overflowY = 'auto'; + scroller.style.overflowAnchor = 'none'; + await settled(); + + for (let i = 0; i < 120; i++) { + this.channel.addMessage(new Message({ + type: 'message-chat', + id: `m${i}`, + date: new Date(Date.now() + i * 1000), + nickname: 'alice', + content: `message ${i}` + })); + } + await settled(); + + assert.strictEqual( + this.element.querySelectorAll('#channel-content .msg-content').length, + 50, + 'renders the initial window of 50 messages, not the whole batch' + ); + }); }); diff --git a/tests/integration/components/chat-scroller-test.js b/tests/integration/components/chat-scroller-test.js index 72c51823..65b703f5 100644 --- a/tests/integration/components/chat-scroller-test.js +++ b/tests/integration/components/chat-scroller-test.js @@ -23,10 +23,6 @@ function offsetWithin (scroller, node) { return node.getBoundingClientRect().top - scroller.getBoundingClientRect().top; } -function wait (ms) { - return new Promise((resolve) => setTimeout(resolve, ms)); -} - const TEMPLATE = hbs` {{! template-lint-disable no-inline-styles }} isScrolledToBottom(scroller), { timeout: 2000 }); - await wait(200); // let the initial programmatic scroll release its lock + await waitUntil(() => isScrolledToBottom(scroller), { timeout: 2000 }); // let the initial programmatic scroll release its lock this.set('hasOlder', true); this.set('isLoadingOlder', true); @@ -246,8 +238,7 @@ module('Integration | Component | chat-scroller', function (hooks) { // Reach the top while a page is still loading; the request is deferred. scroller.scrollTop = 100; scroller.dispatchEvent(new Event('scroll')); - await settled(); - await wait(200); // let native scroll/scrollend events fully settle + await settled(); // let native scroll/scrollend events fully settle assert.strictEqual(calls, 0, 'does not start a concurrent load'); // When the in-flight load finishes, the next page is requested automatically @@ -264,12 +255,10 @@ module('Integration | Component | chat-scroller', function (hooks) { const scroller = await renderScroller(); await waitUntil(() => isScrolledToBottom(scroller), { timeout: 2000 }); - await wait(200); scroller.scrollTop = 120; scroller.dispatchEvent(new Event('scroll')); await settled(); - await wait(50); const before = anchoredNode(scroller); assert.ok(before, 'has an anchored message'); @@ -279,7 +268,6 @@ module('Integration | Component | chat-scroller', function (hooks) { const older = [{ id: 'old1' }, { id: 'old2' }, { id: 'old3' }]; this.set('items', [...older, ...this.items]); await settled(); - await wait(100); const after = scroller.querySelector(`[data-message-key="${beforeKey}"]`); assert.ok(after, 'the anchored message is still rendered'); @@ -294,12 +282,10 @@ module('Integration | Component | chat-scroller', function (hooks) { const scroller = await renderScroller(); await waitUntil(() => isScrolledToBottom(scroller), { timeout: 2000 }); - await wait(200); scroller.scrollTop = 120; scroller.dispatchEvent(new Event('scroll')); await settled(); - await wait(50); const before = anchoredNode(scroller); assert.ok(before, 'has an anchored message'); @@ -309,7 +295,6 @@ module('Integration | Component | chat-scroller', function (hooks) { const older = [{ id: 'old1' }, { id: 'old2' }, { id: 'old3' }]; this.set('items', [...older, ...this.items, { id: 'new1' }]); await settled(); - await wait(100); const after = scroller.querySelector(`[data-message-key="${beforeKey}"]`); assert.ok(after, 'the anchored message is still rendered'); diff --git a/tests/unit/models/base-channel-test.js b/tests/unit/models/base-channel-test.js index d931c1d3..7d8dd041 100644 --- a/tests/unit/models/base-channel-test.js +++ b/tests/unit/models/base-channel-test.js @@ -279,9 +279,40 @@ module('Unit | Model | base-channel', function (hooks) { channel.addMessage(message); - assert.strictEqual(message.key, 'key-1', 'message gets the transport id as key'); + assert.strictEqual(message.key, 'alice\u0000key-1', 'keys the transport id by author'); const stored = channel.messages.find(item => item.type === 'message-chat'); - assert.strictEqual(stored.key, 'key-1'); + assert.strictEqual(stored.key, 'alice\u0000key-1'); + }); + + test('#addMessage uses a server stanza id as the key', function (assert) { + const channel = new BaseChannel({ account: xmppAccount }); + const message = new Message({ + type: 'message-chat', date: new Date(), nickname: 'alice', + content: 'hello', id: 'client-id', sid: 'stanza-42' + }); + + channel.addMessage(message); + + assert.strictEqual(message.key, 'sid:stanza-42'); + }); + + test('#addMessage keeps the same transport id from different authors', function (assert) { + const channel = new BaseChannel({ account: xmppAccount }); + + channel.addMessage(new Message({ + type: 'message-chat', date: new Date(), nickname: 'alice', + content: 'from alice', id: 'shared' + })); + channel.addMessage(new Message({ + type: 'message-chat', date: new Date(), nickname: 'bob', + content: 'from bob', id: 'shared' + })); + + assert.strictEqual( + channel.messages.filter(item => item.type === 'message-chat').length, + 2, + 'keeps both messages despite the shared id' + ); }); test('#addMessage derives a key for id-less messages', function (assert) { diff --git a/tests/unit/utils/message-key-test.js b/tests/unit/utils/message-key-test.js index f1d66a3f..67a29fd9 100644 --- a/tests/unit/utils/message-key-test.js +++ b/tests/unit/utils/message-key-test.js @@ -13,10 +13,23 @@ function chatMessage (props) { } module('Unit | Utility | message key', function () { - test('uses the id when the message has one', function (assert) { + test('scopes a transport id by author', function (assert) { const message = chatMessage({ id: 'abc123' }); - assert.strictEqual(messageKey(message), 'abc123'); + assert.strictEqual(messageKey(message), 'alice\u0000abc123'); + }); + + test('uses a server stanza id as-is', function (assert) { + const message = chatMessage({ id: 'abc123', sid: 'stanza-9' }); + + assert.strictEqual(messageKey(message), 'sid:stanza-9'); + }); + + test('does not collapse the same id sent by different authors', function (assert) { + const alice = chatMessage({ id: 'shared' }); + const bob = chatMessage({ nickname: 'bob', id: 'shared' }); + + assert.notStrictEqual(messageKey(alice), messageKey(bob)); }); test('derives a key from the timestamp and content when there is no id', function (assert) { From 5f1a530743aca8f71fbc2be9abf499441ef2caba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Thu, 1 Oct 2026 20:13:11 +0200 Subject: [PATCH 12/25] Animate scroll-to-bottom in JS instead of native smooth scroll The native `scrollTo({ behavior: 'smooth' })` engaged the browser's overscroll/chaining animation when it reached the container edge, pulling the content up and settling it again (janky), because the chat scroller sits inside other `overflow: auto` ancestors. Drive the user-initiated scroll-to-bottom with a short rAF animation that sets `scrollTop` directly: monotonic, ease-out, never overshoots. Native overscroll is left untouched for user scrolls. The animation stays under the maintenance lock and test waiter, and is cancelled on destruction or user interruption. --- app/components/chat-scroller.js | 65 +++++++++++++++++++++++++++++++-- 1 file changed, 61 insertions(+), 4 deletions(-) diff --git a/app/components/chat-scroller.js b/app/components/chat-scroller.js index 691d1fe2..0b291d86 100644 --- a/app/components/chat-scroller.js +++ b/app/components/chat-scroller.js @@ -11,7 +11,9 @@ import { } from 'hyperchannel/utils/chat-scroll-state'; const MAINTENANCE_TIMEOUT = 150; // ms -const SMOOTH_SCROLL_TIMEOUT = 1000; // ms; upper bound for a smooth scroll +// Duration bounds for the user-initiated smooth scroll to the bottom. +const SMOOTH_SCROLL_MIN_DURATION = 200; // ms +const SMOOTH_SCROLL_MAX_DURATION = 600; // ms const FILL_MAX_NO_PROGRESS = 2; // Tracks the component's deferred async work (maintenance timer, frame @@ -65,6 +67,7 @@ export default class ChatScrollerComponent extends Component { maintenanceTarget = null; maintenanceObservedTop = 0; maintenanceWaiterToken = null; + smoothAnimationFrame = null; smoothScrollRequested = false; // Auto-load bookkeeping. @@ -80,6 +83,10 @@ export default class ChatScrollerComponent extends Component { clearTimeout(this.maintenanceTimer); this.maintenanceTimer = null; } + if (this.smoothAnimationFrame) { + cancelAnimationFrame(this.smoothAnimationFrame); + this.smoothAnimationFrame = null; + } this.endMaintenanceWaiter(); }); } @@ -199,6 +206,10 @@ export default class ChatScrollerComponent extends Component { clearTimeout(this.maintenanceTimer); this.maintenanceTimer = null; } + if (this.smoothAnimationFrame) { + cancelAnimationFrame(this.smoothAnimationFrame); + this.smoothAnimationFrame = null; + } this.endMaintenanceWaiter(); const wasMaintaining = this.maintainingScroll; this.maintainingScroll = false; @@ -229,9 +240,7 @@ export default class ChatScrollerComponent extends Component { const maxTop = element.scrollHeight - element.clientHeight; if (smooth) { - this.beginMaintenance(SMOOTH_SCROLL_TIMEOUT); - element.scrollTo({ top: maxTop, behavior: 'smooth' }); - this.maintenanceTarget = maxTop; + this.startSmoothScrollToBottom(element, maxTop); } else { this.beginMaintenance(); element.scrollTop = element.scrollHeight; @@ -243,6 +252,54 @@ export default class ChatScrollerComponent extends Component { } } + // Drives the user-initiated scroll-to-bottom ourselves rather than using the + // browser's `scrollTo({ behavior: 'smooth' })`. A native smooth scroll to the + // container edge engages the browser's overscroll/chaining animation (which + // pulls the content and then settles) and looks janky. Setting `scrollTop` + // directly each frame never overshoots and leaves native overscroll for user + // scrolls untouched. + startSmoothScrollToBottom (element, end) { + const start = element.scrollTop; + const distance = end - start; + + if (distance <= 1) { + this.beginMaintenance(); + element.scrollTop = end; + this.maintenanceTarget = element.scrollTop; + this.maintenanceObservedTop = element.scrollTop; + return; + } + + const duration = Math.min( + SMOOTH_SCROLL_MAX_DURATION, + Math.max(SMOOTH_SCROLL_MIN_DURATION, distance * 0.5) + ); + + this.beginMaintenance(duration + 100); + this.maintenanceTarget = end; + this.maintenanceObservedTop = start; + + const startTime = performance.now(); + + const step = (now) => { + this.smoothAnimationFrame = null; + if (this.isDestroyed || this.isDestroying || !this.maintainingScroll) return; + + const progress = Math.min(1, (now - startTime) / duration); + const eased = 1 - Math.pow(1 - progress, 3); // easeOutCubic + element.scrollTop = start + distance * eased; + + if (progress < 1) { + this.smoothAnimationFrame = requestAnimationFrame(step); + } else { + element.scrollTop = end; + this.endMaintenance(); + } + }; + + this.smoothAnimationFrame = requestAnimationFrame(step); + } + // --- Anchoring ------------------------------------------------------------ captureAnchor () { From 184519a43fa683674cb884d151fe7ab082dfa9f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Thu, 1 Oct 2026 21:28:10 +0200 Subject: [PATCH 13/25] Fix scroll-to-bottom button reliability, jump, and ease-out The user-initiated scroll-to-bottom had three problems: - A scroll event queued from a previous programmatic scroll could arrive before the first animation frame. maintenanceTarget was still null at that point, so handleScroll ended maintenance immediately and the animation was cancelled: the button often did not scroll at all after loading history. Seed the target synchronously and ignore the scrollend events our own frames produce. - The animation moved a fixed fraction of the remaining distance in the first frame, which read as a jump when starting from far up. Replace it with a capped, time-normalized exponential decay: a gentle start followed by a long, monotonic ease-out. - The jump-to-latest button was an in-flow sticky child of the scroll content, so adding/removing it changed scrollHeight and shifted the view by the button's height. Move it into a zero-height sticky zone so toggling it never affects the scrollable height. Also clamp every programmatic scroll to the real maximum (scrollHeight - clientHeight) instead of assigning scrollHeight, which overshoots. --- app/components/channel-container.hbs | 36 +++--- app/components/chat-scroller.js | 118 +++++++++++++----- app/styles/components/channel-container.scss | 20 ++- .../components/chat-scroller-test.js | 86 +++++++++++++ 4 files changed, 208 insertions(+), 52 deletions(-) diff --git a/app/components/channel-container.hbs b/app/components/channel-container.hbs index cdb11631..ac4074db 100644 --- a/app/components/channel-container.hbs +++ b/app/components/channel-container.hbs @@ -77,21 +77,27 @@ {{/each}} - {{#unless scroll.isAtBottom}} - - {{/unless}} + {{! Zero-height sticky zone so the button is pinned to the viewport edge + without ever contributing to the scroll content's height. If the + button were in flow, adding/removing it would change scrollHeight + and shift the view by the button's height. }} +
+ {{#unless scroll.isAtBottom}} + + {{/unless}} +