mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
fix(desktop): key timeline day sections by calendar day, not first message
Each day group renders as a `<section>` whose React key was the exact `createdAt` of its first message. Scrolling up prepends older messages, and when a batch landed on a calendar day already on screen, the first message of that day changed — flipping the section key. React can't diff a changed key, so it unmounted and remounted the entire day section, all its rows torn down and rebuilt above the reader's eye. That whole-section remount is the residual flicker on scroll-up: the anchor restore was correcting a full teardown instead of a clean prepend. Key the section by the local start-of-day of its messages, which is stable across same-day prepends, so an older message grows an existing section's children (stably-keyed rows reorder, not remount) instead of replacing the section. Fold the duplicate key derivation in the render loop into the lib boundary's `key`, which was already documented as "stable" but wasn't. Pure helper `startOfLocalDaySeconds` plus lib tests for day-key stability across a prepend and separation across calendar days. tsc, biome, 40 lib unit, scroll-history e2e 6/6 green. Co-authored-by: Tyler Longwell <tlongwell@squareup.com> Signed-off-by: Tyler Longwell <tlongwell@squareup.com>
This commit is contained in:
co-authored by
Tyler Longwell
parent
02efffc956
commit
53c580871e
@@ -5,6 +5,7 @@ import {
|
||||
formatDayHeading,
|
||||
formatShortMonthDayOrdinal,
|
||||
formatThreadSummaryLastReplyTime,
|
||||
startOfLocalDaySeconds,
|
||||
} from "./dateFormatters.ts";
|
||||
|
||||
function localUnixSeconds(year, monthIndex, day) {
|
||||
@@ -126,3 +127,22 @@ test("formatDayHeading includes the year for other years", () => {
|
||||
`${weekday(date)}, May 19th, ${year}`,
|
||||
);
|
||||
});
|
||||
|
||||
test("startOfLocalDaySeconds collapses a day's timestamps to one value", () => {
|
||||
const morning = new Date(2026, 5, 14, 8, 30, 15).getTime() / 1_000;
|
||||
const evening = new Date(2026, 5, 14, 23, 59, 59).getTime() / 1_000;
|
||||
const midnight = new Date(2026, 5, 14, 0, 0, 0).getTime() / 1_000;
|
||||
|
||||
assert.equal(startOfLocalDaySeconds(morning), midnight);
|
||||
assert.equal(startOfLocalDaySeconds(evening), midnight);
|
||||
});
|
||||
|
||||
test("startOfLocalDaySeconds separates adjacent calendar days", () => {
|
||||
const lateOn14 = new Date(2026, 5, 14, 23, 0, 0).getTime() / 1_000;
|
||||
const earlyOn15 = new Date(2026, 5, 15, 1, 0, 0).getTime() / 1_000;
|
||||
|
||||
assert.notEqual(
|
||||
startOfLocalDaySeconds(lateOn14),
|
||||
startOfLocalDaySeconds(earlyOn15),
|
||||
);
|
||||
});
|
||||
|
||||
@@ -78,6 +78,18 @@ export function isSameDay(a: number, b: number): boolean {
|
||||
return isSameDayDate(new Date(a * 1_000), new Date(b * 1_000));
|
||||
}
|
||||
|
||||
/**
|
||||
* Unix-seconds timestamp of local midnight for the calendar day containing
|
||||
* `unixSeconds`. Two timestamps on the same calendar day map to the same value,
|
||||
* so it is a stable identifier for a day group that does not shift when an
|
||||
* older message is prepended into that day.
|
||||
*/
|
||||
export function startOfLocalDaySeconds(unixSeconds: number): number {
|
||||
const date = new Date(unixSeconds * 1_000);
|
||||
date.setHours(0, 0, 0, 0);
|
||||
return Math.floor(date.getTime() / 1_000);
|
||||
}
|
||||
|
||||
/** Short month + ordinal day, e.g. "May 19th". */
|
||||
export function formatShortMonthDayOrdinal(unixSeconds: number): string {
|
||||
return formatMonthDayOrdinal(
|
||||
|
||||
@@ -212,6 +212,31 @@ test("buildDayGroupBoundaries: group counts always sum to message count", () =>
|
||||
assert.equal(total, messages.length);
|
||||
});
|
||||
|
||||
test("buildDayGroupBoundaries: same-day group key is stable across a prepend", () => {
|
||||
// The day section is keyed by this value; if it changes when an older
|
||||
// message lands on the same calendar day, React remounts the whole section
|
||||
// on every scroll-up prepend — the timeline flicker. The key must depend on
|
||||
// the calendar day, not the first message's exact timestamp.
|
||||
const before = buildDayGroupBoundaries([
|
||||
message({ id: "b", createdAt: dayAt(2026, 6, 14, 9) }),
|
||||
message({ id: "c", createdAt: dayAt(2026, 6, 14, 10) }),
|
||||
]);
|
||||
const afterPrepend = buildDayGroupBoundaries([
|
||||
message({ id: "a", createdAt: dayAt(2026, 6, 14, 8) }),
|
||||
message({ id: "b", createdAt: dayAt(2026, 6, 14, 9) }),
|
||||
message({ id: "c", createdAt: dayAt(2026, 6, 14, 10) }),
|
||||
]);
|
||||
assert.equal(before[0].key, afterPrepend[0].key);
|
||||
});
|
||||
|
||||
test("buildDayGroupBoundaries: distinct calendar days get distinct keys", () => {
|
||||
const groups = buildDayGroupBoundaries([
|
||||
message({ id: "a", createdAt: dayAt(2026, 6, 13, 12) }),
|
||||
message({ id: "b", createdAt: dayAt(2026, 6, 14, 12) }),
|
||||
]);
|
||||
assert.notEqual(groups[0].key, groups[1].key);
|
||||
});
|
||||
|
||||
// --- jump-to-message deep links ----------------------------------------------
|
||||
|
||||
test("resolveDeepLinkTarget: unresolved with no target", () => {
|
||||
|
||||
@@ -11,7 +11,7 @@
|
||||
*/
|
||||
|
||||
import type { TimelineMessage } from "@/features/messages/types";
|
||||
import { isSameDay } from "./dateFormatters";
|
||||
import { isSameDay, startOfLocalDaySeconds } from "./dateFormatters";
|
||||
|
||||
/** Distance (px) from the bottom within which the timeline counts as "at bottom". */
|
||||
export const BOTTOM_THRESHOLD_PX = 72;
|
||||
@@ -88,7 +88,11 @@ export function selectLatestMessageAutoScrollBehavior({
|
||||
|
||||
/** A single day boundary in the timeline: where it starts and how many messages it covers. */
|
||||
export type DayGroupBoundary = {
|
||||
/** Stable key for the day section. */
|
||||
/**
|
||||
* Stable key for the day section: the local start-of-day of the messages it
|
||||
* covers, so prepending an older message into an already-rendered day reuses
|
||||
* the same key instead of remounting the whole `<section>`.
|
||||
*/
|
||||
key: string;
|
||||
/** Index into `messages` of the first message in this day. */
|
||||
startIndex: number;
|
||||
@@ -114,7 +118,7 @@ export function buildDayGroupBoundaries(
|
||||
|
||||
if (!prev || !isSameDay(prev.createdAt, message.createdAt)) {
|
||||
boundaries.push({
|
||||
key: `day-${message.createdAt}`,
|
||||
key: `day-${startOfLocalDaySeconds(message.createdAt)}`,
|
||||
startIndex: i,
|
||||
count: 1,
|
||||
headingTimestamp: message.createdAt,
|
||||
|
||||
@@ -146,11 +146,12 @@ export const TimelineMessageList = React.memo(function TimelineMessageList({
|
||||
|
||||
// Day-divider decision delegated to a pure, lib-tested helper: a new group
|
||||
// starts at index 0 and whenever a message falls on a different calendar day
|
||||
// than the one before it. We index the boundary start positions so the render
|
||||
// loop below stays a straight walk while the grouping logic lives in `lib/`.
|
||||
const dayGroupStartIndices = new Set(
|
||||
// than the one before it. We index the boundaries by start position so the
|
||||
// render loop below stays a straight walk while the grouping logic — and the
|
||||
// prepend-stable section key — lives in `lib/`.
|
||||
const dayGroupBoundariesByStartIndex = new Map(
|
||||
buildDayGroupBoundaries(entries.map((entry) => entry.message)).map(
|
||||
(boundary) => boundary.startIndex,
|
||||
(boundary) => [boundary.startIndex, boundary],
|
||||
),
|
||||
);
|
||||
|
||||
@@ -158,9 +159,10 @@ export const TimelineMessageList = React.memo(function TimelineMessageList({
|
||||
const { message, summary } = entries[i];
|
||||
const messageRenderKey = message.renderKey ?? message.id;
|
||||
|
||||
if (dayGroupStartIndices.has(i)) {
|
||||
const dayBoundary = dayGroupBoundariesByStartIndex.get(i);
|
||||
if (dayBoundary) {
|
||||
currentDayGroup = {
|
||||
key: `day-${message.createdAt}`,
|
||||
key: dayBoundary.key,
|
||||
label: formatDayHeading(message.createdAt),
|
||||
elements: [],
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user