mirror of
https://github.com/nexu-io/open-design.git
synced 2026-09-28 05:22:59 +08:00
fix(chat): 把 .chat-log-viewport 的 grid 轨道换成确定值(滚动冻结候选修法)
`grid-template: minmax(0, 1fr) / minmax(0, 1fr)` → `100% / 100%`。
**这是假设,不是已证实的修复。** 产品拍板放进包里,理由是「免得到时候阻塞上线」。
## 假设
`1fr` 是**不确定轨道**:尺寸是剩余空间的一份,所以轨道要在拥有它的盒子之后的
另一趟里定尺寸,里面那个滚动盒再被拉伸到轨道最终定下的值。`100%` 直接从这个
viewport **已经确定**的内容盒解析(它自己的高度经 `.chat-log-wrap` 的 `flex: 1`
确定,再往上由 `.split` 的 grid 轨道确定),于是这一格第一趟就是对的,之后不再被
修订。
猜的是:滚动节点在轨道尺寸修订落地之前就建好了,而那次修订没有把绘制属性标脏。
**这条因果链没有建立。**
## 已知的缺陷形状(三次真实现场)
天花板被冻在一个早期的内容高度上(实测 91+583=674、824+583=1407、25+583=608 ——
**天花板 + 视口 = 冻结那一刻的内容高度**),程序性写 `scrollTop` 仍然有效,
只有销毁并重建那个布局盒才能解除。
## 怎么判它对不对
**读 `client_chat_scroll_frozen`**:如果带着这条规则的包仍然在报,假设就是错的,
这一行该换成下一个候选需要的写法。这也是为什么本次同时放开了探针的会话级上报
上限 —— 判据要靠事件量,不能报满三条就变瞎。
⚠️ **代价要说清**:如果冻结从此不再出现,我们**分不开「真的修好了」和「只是把
时序推开了」**。这是产品知情后的取舍。
## 等价性
代码注释声称「跨两种书写方向和五种布局状态,计算几何(含 used track sizes)与
`elementFromPoint` 探针逐字段与 `minmax(0, 1fr)` 相同」—— 那是**实测得来的**,
不是假设。`chat-log-viewport-definite-tracks.test.ts` 守的是 CSS 文本契约
(轨道保持确定、不退回 `1fr`/`minmax()`、`0e8bbdaa69` 的 rtl 滚动条沟槽验收
不被破坏、百分比轨道依赖的「祖先必须是确定包含块」这个前提仍然成立)。
**它证明不了修复有效** —— jsdom 不做布局,没有 used track size、没有滚动边界、
没有合成器,假装能量的测试就是假绿。假设的证伪路径是遥测事件,不是这个套件。
This commit is contained in:
@@ -4395,7 +4395,42 @@ body.entry-resizing { cursor: col-resize; user-select: none; }
|
||||
both logical edges with an explicit grid cell while the rail / jump /
|
||||
Plan controls remain positioned overlays. */
|
||||
display: grid;
|
||||
grid-template: minmax(0, 1fr) / minmax(0, 1fr);
|
||||
/* Definite tracks — `100%`, not `minmax(0, 1fr)`. This is not a style
|
||||
preference; it removes a resolution step.
|
||||
|
||||
`1fr` is an indefinite track: the size is a share of free space, so the
|
||||
track is sized in a later pass than the box that owns it, and the scroll
|
||||
box inside it then has to be stretched into whatever the track settled
|
||||
on. `100%` resolves straight off this viewport's already-definite content
|
||||
box (its own height is definite via `flex: 1` inside `.chat-log-wrap`,
|
||||
which is definite down from the `.split` grid track), so the cell is the
|
||||
right size on the first pass and nothing revises it afterwards.
|
||||
|
||||
HYPOTHESIS, NOT A CONFIRMED FIX. The defect being chased is the chat log
|
||||
going unscrollable while layout stays correct: the compositor keeps a
|
||||
maximum-scroll bound frozen at an early content height (measured live at
|
||||
91+583=674, 824+583=1407, 25+583=608 — bound + viewport = the content
|
||||
height at the instant it froze), programmatic `scrollTop` still works,
|
||||
and only destroying and rebuilding the layout box clears it. The guess is
|
||||
that the scroll node gets built before the revised track size lands and
|
||||
the revision never marks the paint property dirty. That causal chain is
|
||||
NOT established. Treat this as an experiment we shipped, and read the
|
||||
verdict off `client_chat_scroll_frozen` (see
|
||||
`apps/web/src/observability/chat-scroll-freeze.ts`): if that event keeps
|
||||
arriving from builds carrying this rule, the hypothesis is wrong and this
|
||||
line should go back to whatever the next candidate needs.
|
||||
|
||||
Equivalence to the previous writing is measured, not assumed: across both
|
||||
writing directions and five layout states, computed geometry (including
|
||||
the used track sizes) and the `elementFromPoint` probes that
|
||||
`e2e/ui/split-resize-scrollbar-hitbox.test.ts` asserts are field-for-field
|
||||
identical to `minmax(0, 1fr)`.
|
||||
|
||||
Precondition: a percentage track needs a definite containing block. If
|
||||
`.chat-log-wrap` / `.pane` ever stop handing this viewport a definite
|
||||
height, `100%` degrades to `auto` (content-sized) where `1fr` would still
|
||||
fill — so revisit this line if that chain changes. */
|
||||
grid-template: 100% / 100%;
|
||||
}
|
||||
.chat-log-viewport > .chat-log {
|
||||
grid-area: 1 / 1;
|
||||
|
||||
@@ -0,0 +1,136 @@
|
||||
import { readFileSync } from 'node:fs';
|
||||
import { describe, expect, it } from 'vitest';
|
||||
|
||||
/**
|
||||
* `client_chat_scroll_frozen` — the chat log stops responding to real wheel
|
||||
* input while layout stays correct, because the compositor holds a
|
||||
* maximum-scroll bound frozen at an early content height.
|
||||
*
|
||||
* The shipped candidate fix is to size `.chat-log-viewport`'s grid cell from a
|
||||
* DEFINITE track (`100% / 100%`) instead of an indefinite one
|
||||
* (`minmax(0, 1fr)`), so the scroll box's cell needs no free-space
|
||||
* distribution pass and is never revised after the scroll node is built.
|
||||
*
|
||||
* WHAT THIS FILE CAN AND CANNOT PROVE
|
||||
*
|
||||
* It cannot prove the fix works. jsdom performs no layout: there are no used
|
||||
* track sizes, no scroll bounds and no compositor here, and a test that
|
||||
* pretended otherwise would be a fake green. The falsification path for the
|
||||
* hypothesis itself is the telemetry event, not this suite.
|
||||
*
|
||||
* What it does hold is the CSS text contract, which is exactly what a careless
|
||||
* later edit would break silently:
|
||||
*
|
||||
* 1. the track stays definite (nobody reverts to `1fr` / `minmax()`);
|
||||
* 2. the acceptance of commit 0e8bbdaa69 ("fix(chat): own rtl scrollbar
|
||||
* gutter", asserted by e2e/ui/split-resize-scrollbar-hitbox.test.ts) is
|
||||
* preserved — the log still fills the cell to both logical edges and the
|
||||
* viewport is still the containing block for the absolute overlays;
|
||||
* 3. the precondition a percentage track depends on — a definite containing
|
||||
* block — is still handed down the `.pane` / `.chat-log-wrap` chain. If
|
||||
* that chain loses its definite height, `100%` degrades to `auto`
|
||||
* (content-sized) where `1fr` would still fill, which would be a silent
|
||||
* layout regression rather than a loud one.
|
||||
*/
|
||||
|
||||
const composioCss = readFileSync(
|
||||
new URL('../../src/styles/viewer/composio.css', import.meta.url),
|
||||
'utf8',
|
||||
);
|
||||
const shellCss = readFileSync(new URL('../../src/styles/shell.css', import.meta.url), 'utf8');
|
||||
const indexCss = readFileSync(new URL('../../src/index.css', import.meta.url), 'utf8');
|
||||
|
||||
function stripComments(css: string): string {
|
||||
return css.replace(/\/\*[\s\S]*?\*\//g, '');
|
||||
}
|
||||
|
||||
/** Declaration bodies of every rule whose selector list contains `selector` exactly. */
|
||||
function cssDeclarations(css: string, selector: string): string[] {
|
||||
const blocks: string[] = [];
|
||||
const rulePattern = /([^{}]+)\{([^}]*)\}/g;
|
||||
const source = stripComments(css);
|
||||
let match: RegExpExecArray | null;
|
||||
|
||||
while ((match = rulePattern.exec(source)) !== null) {
|
||||
const selectors = (match[1] ?? '').split(',').map((item) => item.trim());
|
||||
if (selectors.includes(selector)) blocks.push(match[2] ?? '');
|
||||
}
|
||||
|
||||
return blocks;
|
||||
}
|
||||
|
||||
/** Every stylesheet `index.css` pulls in, in its real cascade order. */
|
||||
function importedStylesheets(): { specifier: string; css: string }[] {
|
||||
const specifiers = [...indexCss.matchAll(/@import\s+'([^']+)'/g)].map((m) => m[1] as string);
|
||||
expect(specifiers.length).toBeGreaterThan(30);
|
||||
return specifiers.map((specifier) => ({
|
||||
specifier,
|
||||
css: readFileSync(new URL(specifier, new URL('../../src/index.css', import.meta.url)), 'utf8'),
|
||||
}));
|
||||
}
|
||||
|
||||
describe('client_chat_scroll_frozen — chat log viewport sizes its scroll box from a definite track', () => {
|
||||
it('sizes the grid cell with definite tracks, not free-space distribution', () => {
|
||||
const blocks = cssDeclarations(composioCss, '.chat-log-viewport');
|
||||
expect(blocks).toHaveLength(1);
|
||||
const viewport = blocks[0] as string;
|
||||
|
||||
expect(viewport).toMatch(/\bdisplay:\s*grid;/);
|
||||
expect(viewport).toMatch(/\bgrid-template:\s*100%\s*\/\s*100%;/);
|
||||
|
||||
// The whole point of the change: no indefinite track may come back in.
|
||||
// `1fr` is sized from leftover space in a later pass than the box that
|
||||
// owns it, which is the resolution step this rule exists to remove.
|
||||
expect(viewport).not.toMatch(/\bminmax\(/);
|
||||
expect(viewport).not.toMatch(/\d*\.?\d*fr\b/);
|
||||
});
|
||||
|
||||
it('keeps commit 0e8bbdaa69 intact: the log owns both logical edges of the viewport', () => {
|
||||
const viewport = cssDeclarations(composioCss, '.chat-log-viewport')[0] as string;
|
||||
|
||||
// `.chat-message-rail` and `.chat-bottom-float-slot` are absolutely
|
||||
// positioned against this box; dropping `position: relative` re-parents
|
||||
// them to the nearest ancestor and moves the overlays off the panel.
|
||||
expect(viewport).toMatch(/\bposition:\s*relative;/);
|
||||
expect(viewport).toMatch(/\bflex:\s*1;/);
|
||||
expect(viewport).toMatch(/\bmin-height:\s*0;/);
|
||||
expect(viewport).toMatch(/\bmin-width:\s*0;/);
|
||||
|
||||
// The scrollbar gutter is flush with the resize handle only while the log
|
||||
// is stretched across the single cell. e2e/ui/split-resize-scrollbar-hitbox
|
||||
// hit-tests 1px and 3px inside that edge, in LTR and RTL.
|
||||
const log = cssDeclarations(composioCss, '.chat-log-viewport > .chat-log');
|
||||
expect(log).toHaveLength(1);
|
||||
expect(log[0]).toMatch(/\bgrid-area:\s*1\s*\/\s*1;/);
|
||||
expect(log[0]).toMatch(/\bmin-height:\s*0;/);
|
||||
expect(log[0]).toMatch(/\bmin-width:\s*0;/);
|
||||
});
|
||||
|
||||
it('leaves composio.css the sole owner of the viewport, so no later sheet can re-introduce an indefinite track', () => {
|
||||
const owners = importedStylesheets()
|
||||
.filter(({ css }) => stripComments(css).includes('.chat-log-viewport'))
|
||||
.map(({ specifier }) => specifier);
|
||||
|
||||
// A second sheet later in the cascade — routines.css already wins over
|
||||
// chat.css elsewhere on source order alone — could restore `1fr` without
|
||||
// touching this rule at all.
|
||||
expect(owners).toEqual(['./styles/viewer/composio.css']);
|
||||
});
|
||||
|
||||
it('keeps the definite containing block a percentage track resolves against', () => {
|
||||
// `100%` needs a definite containing block. This is the chain that
|
||||
// supplies it: the split's chat track -> .pane -> .chat-log-wrap ->
|
||||
// .chat-log-viewport (`flex: 1`). Track ownership on `.split` itself is
|
||||
// covered by tests/styles/chatpane-grid-transition.test.ts.
|
||||
const pane = cssDeclarations(shellCss, '.pane').join('\n');
|
||||
expect(pane).toMatch(/\bdisplay:\s*flex;/);
|
||||
expect(pane).toMatch(/\bflex-direction:\s*column;/);
|
||||
expect(pane).toMatch(/\bmin-height:\s*0;/);
|
||||
|
||||
const wrap = cssDeclarations(composioCss, '.chat-log-wrap').join('\n');
|
||||
expect(wrap).toMatch(/\bdisplay:\s*flex;/);
|
||||
expect(wrap).toMatch(/\bflex-direction:\s*column;/);
|
||||
expect(wrap).toMatch(/\bflex:\s*1;/);
|
||||
expect(wrap).toMatch(/\bmin-height:\s*0;/);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user