Conversation
|
PR governance checks passed. Awaiting human review. |
两处冲突,均因 main 的 #762(输入区宽度/遮挡系列修复)与本分支的 「移除 AI 头像」改到同一批代码: base-chat.css `.gateway-composer-layer` main 把输入框的列宽从已退役的 --gateway-chat-column-width 换成 --chat-transcript-content-width(输入框改为跟随可调正文宽),本分支 则给它减去了退役头像列的 40px。两者取并集:同一变量 + calc(-40px), 与 .gateway-transcript-shell 完全一致。原注释称「两者读不同变量」已 被 main 的统一作废,一并改写。 ChatComposerBar.tsx 层底实底条 取 main 的无条件版本。本分支保留的 surface === "desktop" 分支其实是 合并基线里的旧代码(本分支未曾改动过它),而 main 在 #762 明确将其 改为两端共用以修复正文从裙边下漏出的问题。列对齐注释取本分支的版本 ——头像已移除,80px/20px/4px 的描述不再成立。 同步更新两条互相矛盾的守卫测试: composer-width-follows-transcript 的意图是「读同一个变量」,正则放开 以允许 calc 包裹;measurements-lru 的第二条断言仍在期待被 main 退役的 --gateway-chat-column-width,改为按规则块分别断言,仍然验证两处都写了 40px 补偿。 验证:agent-gui 3032/3032、backend 10/10、gateway-web 714/714 通过, tsc 与 vite build 干净。合并后需 pnpm install 重链 web 的 jsdom。 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
yovinchen
left a comment
There was a problem hiding this comment.
审阅了整个分支(本地 tsc/biome/前端测试/网关测试都跑过)。焦点修复、提问订阅层、Switch 尺寸配对这些都没问题,DialogContent 默认宽度收窄的 34 个调用点也逐一核对过,只有 BackgroundTasksPanel 依赖默认值且已补上。
下面是需要处理的点,具体位置见行内评论:
建议合并前修复
- 转录宽度拖拽手柄没有跟着正文列一起扣减 40px,手柄悬在正文列外、读数比实际列宽大 40。
ToolApprovalBar的焦点守卫在最常见场景(发送后焦点仍在输入框)下会让 Enter/Escape 快捷键失效。
小问题
3. WorkspaceResourceSettingsPanel 的 role="radio" 按钮触发了新的 biome useSemanticElements 警告,缺一条 biome-ignore(AskUserQuestionCard 同写法有加)。
4. ComposerModelControls 里"供应商排序切换已移除"的注释与代码矛盾,按钮还在。
设计取舍,请确认是否接受
5. 侧栏两个列表 no-scrollbar:Windows/Linux 常驻滚动条用户失去拖动滚动条的方式;且 scroll-fade 依赖 animation-timeline,WKWebView 到 Safari 26 才支持,旧系统上既无滚动条也无渐隐。
6. 对话框 --zone-font-scale: 0.9 不覆盖 portal 弹层:Select/Dropdown 菜单仍是 1.0,会出现触发器 12.6px、菜单 14px 的不一致。
另外 PR 描述 checklist 里还留着"落后 main 7 个提交"的注释,实际已经合入。
| // transcript column gives that width back instead of widening the | ||
| // reading measure. Keeps assistant text at its original width and | ||
| // aligned with the composer, which is tuned off the same variable. | ||
| "mx-auto w-full max-w-[calc(var(--chat-transcript-content-width,768px)-2.5rem)] px-5 py-4 [overflow-anchor:none]", |
There was a problem hiding this comment.
正文列在这里(以及网关端 base-chat.css 两处)减了 40px,但 TranscriptWidthControls.tsx:337 的手柄轨道仍是 width: var(--chat-transcript-content-width),手柄定位在轨道的 left-0/right-0。结果两侧手柄各悬在正文列外 20px,拖拽时的像素读数(如 768 px)也比实际渲染列宽大 40,MIN/MAX_CHAT_TRANSCRIPT_WIDTH 的语义随之静默偏移。
40px 的扣减目前分散在三处(GUI 正文列、网关正文列、网关输入框列),手柄是漏掉的第四处。建议要么手柄轨道同样减 40,要么把扣减收到变量写入端、消费端保持纯读。
| if ( | ||
| active && | ||
| (active.tagName === "INPUT" || active.tagName === "TEXTAREA" || active.isContentEditable) | ||
| ) { |
There was a problem hiding this comment.
这个守卫的初衷是保护侧栏的 blur-to-commit 输入,但它在最常见场景下会误伤:审批栏出现时输入卡片是被 hidden 隐藏的(ChatComposerBar.tsx:1432),而发送后焦点通常仍留在输入框。React 提交时 document.activeElement 仍指向已隐藏的 textarea(浏览器的 focus fixup 晚于提交),守卫命中、不抓焦点,随后焦点落到 body,Enter/Escape 批准/拒绝快捷键就没了。
被审批栏替换掉的输入框本来就没有"正在输入"可保护。建议把条件收窄为"可编辑元素仍然可见",例如再加一个 active.getClientRects().length > 0。
| <button | ||
| key={value} | ||
| type="button" | ||
| role="radio" |
There was a problem hiding this comment.
这里的 role="radio" 按钮触发了新的 biome lint/a11y/useSemanticElements 警告。AskUserQuestionCard 同样写法前面加了 // biome-ignore lint/a11y/useSemanticElements: …,这里漏了。
| provider·model 副标题:模型名在触发器、副标题、列表勾选处重复 | ||
| 三次,且 11px 的标题比 12px 的模型行还小,标题反而是面板里最小 | ||
| 的粗体字。弹层自身的 aria-label 已覆盖无障碍命名。 | ||
| 供应商排序切换已移除:它的图标显示的是「目标模式」而非当前模式、 |
There was a problem hiding this comment.
注释说"供应商排序切换已移除",但下面第 501 行的按钮还在(而且这次还给它加了 aria-pressed)。这段注释应该删掉,不然下一个读代码的人会去找不存在的删除。
| ref={historyScrollRef} | ||
| aria-busy={listStatus === "loading" || listStatus === "syncing" || isLoadingMore} | ||
| className="chat-history-list min-h-0 flex-1 overflow-y-auto overflow-x-hidden px-2 pb-3" | ||
| className="chat-history-list no-scrollbar scroll-fade min-h-0 flex-1 overflow-y-auto overflow-x-hidden px-2 pb-3" |
There was a problem hiding this comment.
设计取舍,请确认:no-scrollbar 同时用在工作区列表和历史列表。macOS 覆盖式滚动条影响不大,但 Windows/Linux 常驻滚动条的用户会失去拖动滚动条这条路径,边缘渐隐只提示"有溢出"不提供操作。而且 scroll-fade 依赖 animation-timeline: scroll(self y),WKWebView 直到 Safari 26 才支持,旧系统上既没有滚动条也没有渐隐。
| data-has-close-button={showCloseButton ? "true" : undefined} | ||
| style={{ | ||
| ...({ "--zone-font-scale": DIALOG_FONT_SCALE } as React.CSSProperties), | ||
| ...style, |
There was a problem hiding this comment.
小问题:对话框自成 0.9 缩放区后,里面的 Select/Dropdown 弹层是渲染到 body 的,落在区外仍按 1.0 渲染,会出现触发器 12.6px、菜单 14px 的不一致。可以考虑让 SelectContent/DropdownMenuContent 继承触发器所在区的 --zone-font-scale。
正文列(GUI ChatTranscript、网关 transcript shell 与 composer layer)都从 --chat-transcript-content-width 里减掉了退役的头像列,手柄轨道仍按原变量取宽, 两侧手柄各悬在正文列外 20px,拖拽读数也比实际列宽大 40。轨道改为同样扣减, 并补一条源码守卫把三处扣减绑在一起。
审批栏出现时输入卡片被 hidden 隐藏,但提交时刻 activeElement 仍指向该 textarea(浏览器 focus fixup 晚于 React 提交),守卫只看标签名就会放弃抓 焦点,发送后紧接着到来的审批 Enter/Escape 快捷键失效。改为再要求元素有 渲染矩形,侧栏重命名与新建分组草稿仍受保护。
- WorkspaceResourceSettingsPanel 的 role="radio" 按钮触发 useSemanticElements 警告,按 AskUserQuestionCard 的写法加 biome-ignore 并说明原因。 - ComposerModelControls 注释称供应商排序切换已移除,但按钮仍在。
--zone-font-scale 是 CSS 自定义属性,到 portal 边界就断了:对话框里打开的 Select/Dropdown/Popover/Tooltip 渲染在 body 下退回 1.0,而触发器按对话框 的 0.9 绘制,出现触发器 12.6px、菜单 14px 的不一致。 对话框经 React context 发布档位(含调用方 style 覆盖),弹层 Positioner 把 它写回内联变量,.layer-popover 与 .zone-font-scale 共用同一条字号变量重声明。 无 zone 时变量不设、回退 1,弹层行为与之前一致。
no-scrollbar 与 scroll-fade 分开声明时,不支持 scroll(self y) 的引擎 (WKWebView 直到 Safari 26 才支持)既没有渐隐也没有滚动条,溢出没有任何 提示。把滚动条隐藏收进 scroll-fade 的 @supports 块,渐隐成为唯一的溢出 信号时才替换原生滚动条;删除已无使用点的 no-scrollbar 工具类。 补一条源码守卫,同时覆盖对话框弹层字号继承。
|
上面 review 里的 6 条我已在本分支补了 5 个提交修掉,对应关系:
第 5 条只解决了"旧引擎两者皆无"的情况;Windows/Linux 常驻滚动条用户在支持渐隐的引擎上仍看不到滚动条,这是设计取舍,保留你的原意。 验证:三端 |
Linked issue
Closes #765
Summary
Dialogs, the sidebar, and the transcript had each evolved separately, so the same control carried several mutually inconsistent implementations. The cost was not cosmetic: every new surface re-decided the same basic questions and usually answered them differently from what already existed.
DialogContentshipped a defaultp-6that twelve call sites each cancelled with their own mix ofpx-6/py-5/py-3.5; oneTabsprimitive appeared in four different guises, one of which was a hand-rolled<fieldset>lookalike; and section headings were one font weight away from field labels, so forms read as a flat pile.This PR establishes a single baseline and moves every affected surface onto it.
DialogContentloses its default padding;DialogHeader/DialogBody/DialogFooterown their own. The cancelling overrides are deleted from all twelve dialogs.TabsTriggerheight moves into the base component (four files were each carrying their ownh-7); dialogInput/SelectTriggerstandardise onh-8+shadow-none;Switchgains asizevariant where track and thumb travel are configured as a pair, since travel istrackWidth − thumbWidth − insetand overriding only the track pushes the thumb past the edge.text-muted-foregroundinstead of relying on a single weight step.role="radiogroup"; picking one of them never navigated anywhere, it only made the list below read-only.AskUserQuestion, mirroring the existing tool-approval marker.Focus defects fixed
The repo already had the correct pattern (
finalFocus+skipNextBlurCommitRefinChatHistorySidebarRows); it had simply never reached the group features added later.onBlur; the empty draft is silently discardedonBlurcommits the draft → the row unmounts before the click reaches the cancel handlerAlso:
ToolApprovalBargrabbed focus unconditionally when an approval arrived, interrupting whatever the user was typing. It now skips the grab when focus is already inside an editable element.Change scope
agent-gui,agent-uiFive commits:
1e56562d7dc88084542972c20af0dce537d384e0Screenshots / preview
Verification
Tests added or updated:
test/tools/ask-user-question-tools.test.mjs— new coverage for the per-conversation subscription layer: emit on register / settle / conversation teardown, isolation between conversations, and snapshot identity stability (the last one is whatuseSyncExternalStoretears on).test/chat/sidebar-selection.test.mjs— new guard for the new-group draft path: return-focus opt-out, effect-owned focus placement (notautoFocus), single-commit blur handling, andpreventDefaulton the confirm/cancel buttons.test/settings/workspace-resource-settings.test.mjs— the resource-mode guard now assertsrole="radiogroup"so the control cannot drift back to a tab strip.skills-category-tabs/mcp-hub-tabs/sidebar-selectionwere updated where the refactor moved a value into a base component. Each was reviewed individually against the guard's stated intent rather than bumped to make it pass; comments were updated to record the new intent.Pre-submit checklist