mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
feat(desktop): wire measured column width into row estimates + anchor-contract guard
Two of the deferred scroll-jitter seams, on top of the integrated fix train
(T1.1 gate -> T2 writer -> T3 estimator @ 1fa4551a):
- columnWidthPx: measure the timeline row column once (useElementWidth) and
thread it into timelineRowReserveStyle so row-height reserves use the real
wrap width instead of the 64-char fallback. Pre-measure zero passes undefined
to preserve the estimator's own fallback rather than tripping its min-chars
floor. On jitter-corpus this shrinks estimate-error R from 75 -> 55 peak
(better reserves = smaller realized-vs-reserved delta) while the motion gate
stays GREEN 0.00/0.00 -- R moves only in the estimator lane, the gate only in
the writer lane, cross-lane invariant intact.
- Wren #4 anchor-contract guard: a companion perf test that reads the PRODUCTION
computed overflow-anchor on the real conversation scroller (no test override)
and asserts it resolves to none. The jitter gate forces the property for its
measurement; this proves the shipped stylesheet actually ships it, so a
regression that hands correction back to Chromium's native anchoring (masking
a WKWebView-only on-device break) fails loudly.
The onRealizedHeight seam is intentionally NOT wired: T4 ground truth
(RESEARCH/GUI_SCROLL_RECLAIM_T4.md) shows the upscroll path is append-only with
no unmount, so it needs no JS height cache; a per-event-id cache is only the
deferred, narrowly-scoped T4b for the head-refetch case. Adding an unused
callback now would be dead abstraction.
Validation:
- pnpm --dir desktop typecheck: clean
- biome check on both changed files: clean
- rowHeightEstimate.test.mjs 16/16, useAnchoredScroll.test.mjs 2/2
- perf: upscroll-jitter GREEN 0.00/0.00 (R 55), anchor-contract PASS
Co-authored-by: Tyler Longwell <tlongwell@block.xyz>
Signed-off-by: Tyler Longwell <tlongwell@block.xyz>
This commit is contained in:
co-authored by
Tyler Longwell
parent
1fa4551a70
commit
963133b6a9
@@ -21,6 +21,7 @@ import type { TimelineMessage } from "@/features/messages/types";
|
||||
import { canManageMessageForCurrentUser } from "@/features/messages/lib/canManageMessage";
|
||||
import type { UserProfileLookup } from "@/features/profile/lib/identity";
|
||||
import type { ChannelType } from "@/shared/api/types";
|
||||
import { useElementWidth } from "@/shared/hooks/use-mobile";
|
||||
import { cn } from "@/shared/lib/cn";
|
||||
import { DayDivider } from "./DayDivider";
|
||||
import { MessageRow } from "./MessageRow";
|
||||
@@ -178,6 +179,14 @@ export const TimelineMessageList = React.memo(function TimelineMessageList({
|
||||
[itemsResult.items],
|
||||
);
|
||||
|
||||
// Measure the row column once so row-height estimates reserve credible space
|
||||
// for the *actual* wrap width instead of the 64-char fallback. The rows are
|
||||
// `w-full` inside this wrapper, so its width is the row box the estimator
|
||||
// subtracts chrome from. Zero (pre-measure) passes `undefined` to preserve
|
||||
// the estimator's own fallback rather than tripping its min-chars floor.
|
||||
const [columnRef, columnWidthPx] = useElementWidth<HTMLDivElement>();
|
||||
const reserveColumnWidthPx = columnWidthPx > 0 ? columnWidthPx : undefined;
|
||||
|
||||
const renderItem = React.useCallback(
|
||||
(item: TimelineNonDayItem) => {
|
||||
switch (item.kind) {
|
||||
@@ -256,7 +265,7 @@ export const TimelineMessageList = React.memo(function TimelineMessageList({
|
||||
);
|
||||
|
||||
return (
|
||||
<div className="flex flex-col">
|
||||
<div className="flex flex-col" ref={columnRef}>
|
||||
{dayGroups.map((group) => (
|
||||
<section
|
||||
className={cn(
|
||||
@@ -279,7 +288,9 @@ export const TimelineMessageList = React.memo(function TimelineMessageList({
|
||||
<div
|
||||
className="timeline-row-cv"
|
||||
key={getTimelineItemKey(item)}
|
||||
style={timelineRowReserveStyle(item)}
|
||||
style={timelineRowReserveStyle(item, {
|
||||
columnWidthPx: reserveColumnWidthPx,
|
||||
})}
|
||||
>
|
||||
{renderItem(item)}
|
||||
</div>
|
||||
|
||||
@@ -0,0 +1,51 @@
|
||||
import { expect, test } from "@playwright/test";
|
||||
|
||||
import { installMockBridge } from "../helpers/bridge";
|
||||
|
||||
/**
|
||||
* ANCHOR-CONTRACT GUARD (Wren #4) — companion to `upscroll-jitter.perf.ts`.
|
||||
*
|
||||
* The jitter gate *forces* `overflow-anchor: none` on the scroller before it
|
||||
* measures, to reproduce shipped WKWebView (which has no `overflow-anchor` and
|
||||
* so corrects nothing) on anchoring Chromium. That force is a test convenience,
|
||||
* not a proof: it would stay green even if production ever stopped shipping the
|
||||
* property, silently handing correction back to Chromium's native anchoring and
|
||||
* masking a WKWebView-only regression on device.
|
||||
*
|
||||
* This test closes that gap by reading the PRODUCTION computed style — no test
|
||||
* override — on the real conversation scroller and asserting it resolves to
|
||||
* `none`. We run on Chromium, whose default `overflow-anchor` is `auto`, so a
|
||||
* resolved value of `none` can only have come from the shipped stylesheet
|
||||
* (`utilities.css` `[data-buzz-conversation-scroll]`). The engine-support
|
||||
* assert keeps the check honest: if it ever runs somewhere without the property
|
||||
* at all, we want the loud failure, not a vacuous pass on an empty string.
|
||||
*/
|
||||
test("CONTRACT: production ships overflow-anchor:none on the conversation scroller", async ({
|
||||
page,
|
||||
}) => {
|
||||
await installMockBridge(page);
|
||||
await page.goto("/");
|
||||
await page.waitForFunction(
|
||||
() => typeof window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__ === "function",
|
||||
);
|
||||
|
||||
await page.getByTestId("channel-jitter-corpus").click();
|
||||
await expect(page.getByTestId("chat-title")).toHaveText("jitter-corpus");
|
||||
const timeline = page.getByTestId("message-timeline");
|
||||
await expect(timeline.locator("[data-message-id]").first()).toBeVisible();
|
||||
|
||||
// Sanity: Chromium DOES support overflow-anchor (default `auto`), so a
|
||||
// resolved `none` below is the production stylesheet's doing, not the engine
|
||||
// returning an empty/unsupported value.
|
||||
const anchorSupported = await page.evaluate(() =>
|
||||
typeof CSS !== "undefined" && typeof CSS.supports === "function"
|
||||
? CSS.supports("overflow-anchor", "auto")
|
||||
: false,
|
||||
);
|
||||
expect(anchorSupported).toBe(true);
|
||||
|
||||
const resolvedOverflowAnchor = await timeline.evaluate(
|
||||
(element) => getComputedStyle(element).overflowAnchor,
|
||||
);
|
||||
expect(resolvedOverflowAnchor).toBe("none");
|
||||
});
|
||||
Reference in New Issue
Block a user