Stop the log moving under whoever is reading it

Two complaints, one cause: the height of the log changes after it is drawn,
and only one of the two cases anybody reads in was putting it back.

**A picture with no measurements had no box.** Where a service sends
dimensions - Discord, Matrix - the space was already reserved. Where it does
not, which is every link a message merely contains and therefore almost
everything on Sneedchat, the row was nothing at all until the image decoded
and then it was three hundred pixels. Now an unmeasured picture gets a shape
to sit in and measures itself on load, so the change is from a box to a box
rather than from nothing.

**And reading the past had nothing holding it.** The observer put the view
back when pinned to the bottom, and did nothing otherwise - while the
scroller sets `overflow-anchor: none`, which is us telling the browser not to
do it either. So an image loading three hundred lines back moved the reader,
and nobody was going to move them back. The row at the top of the view is now
noted as they scroll and put back where it was after anything resizes.

**The drawn tail was also sliding off them.** A message arriving pushes one
off the top of the window of rows that actually get built - which for
somebody parked in the past is the ground moving, and can unmount the very
row they are reading. While they are away from the bottom the tail grows
instead of sliding. It goes back to a tail when they return to the present,
which is where the button and the scroll-back-down already reset it.

Measured against live #fishtank, in the configuration this was reported from -
window in a split pane, member list open, message column 254px wide:

    60s pinned to the bottom: 0 of 120 samples off the bottom, worst gap 0px,
    the jump-to-present button never appeared.
    45s parked in the past: the log grew 661px underneath, and the row at the
    top of the view did not move once.

Before, the same test showed gaps of 320-600px while pinned, and a synthetic
400px growth above the reader moved them the whole 400.

Closes #143, #144

Co-Authored-By: Claude Opus 5 <[email protected]>
This commit is contained in:
2026-09-09 17:47:12 -04:00
co-authored by Claude Opus 5
parent 3ba8bde3c6
commit 7d5e3777f4
2 changed files with 112 additions and 10 deletions
+41 -6
View File
@@ -60,6 +60,16 @@ function humanSize(bytes: number): string {
return `${(bytes / (1024 * 1024)).toFixed(1)} MB`
}
/**
* The shape an unmeasured picture is given to sit in.
*
* Four by three, because it has to be something and something wrong in the
* ordinary direction is less jarring than a tall box round a wide photograph.
* It is a starting size rather than a final one - see `measured` below - so
* being wrong costs one small adjustment rather than a permanent letterbox.
*/
const UNKNOWN_RATIO = '4 / 3'
export function MediaEmbed({
item,
attachment,
@@ -82,6 +92,8 @@ export function MediaEmbed({
* picture that could be fetched.
*/
const [previewGone, setPreviewGone] = useState(false)
/** What the picture turned out to be, once it had loaded enough to say. */
const [seen, setSeen] = useState<{ width: number; height: number } | null>(null)
const [playing, setPlaying] = useState(false)
const [expanded, setExpanded] = useState(false)
const [refreshing, setRefreshing] = useState(false)
@@ -101,12 +113,24 @@ export function MediaEmbed({
const open = (): void => void window.moho.openExternal(openTarget)
// Reserve the right box before anything loads. Without this the list
// reflows under the reader as each image arrives.
const ratio =
attachment?.width && attachment?.height
? { aspectRatio: `${attachment.width} / ${attachment.height}` }
: undefined
/**
* The size the picture is going to be, decided before it arrives.
*
* Where the service told us - Discord and Matrix both send dimensions with
* an attachment - this is exact. Where it did not, which is every link a
* message merely contains, the box was previously nothing at all: a row of
* no height that became three hundred pixels the moment the image decoded,
* shoving everything below it. On a channel where most messages are
* pictures that is not an occasional jolt, it is the reading experience.
*
* So an unmeasured picture gets a placeholder shape to sit in until it can
* be measured, and measures itself on load - one change, at a size the list
* has already made room for, instead of one from nothing.
*/
const measured = seen ?? (attachment?.width && attachment?.height
? { width: attachment.width, height: attachment.height }
: null)
const ratio = { aspectRatio: measured ? `${measured.width} / ${measured.height}` : UNKNOWN_RATIO }
const canRefresh = !!(isDiscordAttachment(openTarget) && bufferId && messageId && onRefresh)
@@ -218,6 +242,17 @@ export function MediaEmbed({
alt={attachment?.filename || ''}
title={refreshError || attachment?.filename}
loading="lazy"
// Its real shape, the first moment there is one to have. Only for
// pictures nobody told us about: where the service sent dimensions
// they are already right, and re-measuring would swap an exact box
// for a rounded one.
onLoad={(e) => {
if (measured) return
const img = e.currentTarget
if (img.naturalWidth && img.naturalHeight) {
setSeen({ width: img.naturalWidth, height: img.naturalHeight })
}
}}
// A failed preview means the cached file went away, which says
// nothing about the link. Only a failure of the original itself is
// the attachment actually being unreachable.
+71 -4
View File
@@ -282,6 +282,20 @@ export function MessageList(): JSX.Element {
const preLoadHeightRef = useRef(0)
/** A jumped-to message to keep in view while the page settles around it. */
const holdRef = useRef('')
/**
* The row the reader is looking at, and where on screen it sits.
*
* Kept so that anything which changes the height of the log *above* them -
* a picture arriving three hundred lines back, an embed unfurling - can be
* undone by putting that row back where it was. Without it the reader is
* shoved by media they cannot see loading, which is the whole of what
* reading old messages in a busy channel felt like.
*
* Recorded on scroll rather than computed when it is needed, because by the
* time a resize is observed the layout has already moved and there is
* nothing left to measure against.
*/
const placeRef = useRef<{ id: string; offset: number } | null>(null)
/** Where the view was at the last scroll, to tell moving up from growing. */
const lastTopRef = useRef(0)
/** And how tall it was, to tell moving up from the page losing content. */
@@ -295,6 +309,41 @@ export function MessageList(): JSX.Element {
el.scrollTo({ top: el.scrollHeight, behavior })
}, [])
/**
* Which row is at the top of the view, and how far into it we are.
*
* By hit test rather than by walking the rows: this runs on every scroll,
* and a list of two thousand is not a thing to scan that often.
*/
const notePlace = useCallback(() => {
const el = scrollRef.current
if (!el) return
const box = el.getBoundingClientRect()
const hit = document.elementFromPoint(box.left + 8, box.top + 1)
const row = hit?.closest?.('[data-msg-id]') as HTMLElement | null
if (!row) return
placeRef.current = {
id: row.dataset.msgId ?? '',
offset: row.getBoundingClientRect().top - box.top
}
}, [])
/** And putting it back after something changed size. */
const keepPlace = useCallback(() => {
const el = scrollRef.current
const place = placeRef.current
if (!el || !place?.id) return
const row = el.querySelector(`[data-msg-id="${CSS.escape(place.id)}"]`)
if (!row) return
const drift = row.getBoundingClientRect().top - el.getBoundingClientRect().top - place.offset
// Whole pixels only, and never for a hair: sub-pixel corrections chase
// rounding for ever and are not a movement anybody could see.
if (Math.abs(drift) < 1) return
el.scrollTop += drift
lastTopRef.current = el.scrollTop
lastHeightRef.current = el.scrollHeight
}, [])
const anchor = useCallback((on: boolean) => {
anchoredRef.current = on
setAnchored(on)
@@ -325,7 +374,16 @@ export function MessageList(): JSX.Element {
?.scrollIntoView({ block: 'center' })
return
}
if (anchoredRef.current) scrollToBottom()
if (anchoredRef.current) {
scrollToBottom()
return
}
// Reading the past. Nothing here wants to move, so the height changing
// under them has to be taken back out: the row that was at the top of
// the view goes back to where it was, whether the growth was above it
// (which would have shoved them) or below it (which would not, and
// this then costs nothing).
keepPlace()
})
ro.observe(content)
// And the window onto it. Making the pane taller does not necessarily
@@ -334,7 +392,7 @@ export function MessageList(): JSX.Element {
// was the case nothing put back.
if (scrollRef.current) ro.observe(scrollRef.current)
return () => ro.disconnect()
}, [scrollToBottom])
}, [keepPlace, scrollToBottom])
/**
@@ -428,7 +486,15 @@ export function MessageList(): JSX.Element {
const seen = previous ? messages.findIndex((m) => m.id === previous) : -1
// Not found means the message the count was last taken from has been
// trimmed off the top; one is the honest floor rather than a guess.
setMissedCount((n) => n + (seen >= 0 ? messages.length - 1 - seen : 1))
const arrived = seen >= 0 ? messages.length - 1 - seen : 1
setMissedCount((n) => n + arrived)
// The drawn part of the log is a tail, so a message arriving at the bottom
// pushes one off the top of it - which for a reader parked in the past is
// the ground moving under them, and can take the very row they are reading
// out of the page. So while they are away from the bottom the tail grows
// instead of sliding: the top of it stays where it was, and what arrives
// is simply added.
if (seen >= 0) setShown((n) => n + arrived)
// `shown` alongside `messages`: rows appearing above the reader move the
// page under them whether they came from the daemon or from the list this
// window already held, and both need the position putting back.
@@ -452,6 +518,7 @@ export function MessageList(): JSX.Element {
lastTopRef.current = top
lastHeightRef.current = height
lastViewportRef.current = viewport
notePlace()
// Reading away un-pins the view, and reading back pins it again; nothing
// else moves it. Both halves of that have been got wrong here before, and
@@ -504,7 +571,7 @@ export function MessageList(): JSX.Element {
void store.loadMoreHistory(bufferId)
}
}
}, [anchor, bufferId, isLoadingMore, messages.length, shown, store])
}, [anchor, bufferId, isLoadingMore, messages.length, notePlace, shown, store])
// Reset alongside the buffer: the position in one conversation says nothing
// about the next, and a stale one would read the first scroll there as a