mirror of
https://github.com/deepseek-ai/deepseek-harness.git
synced 2026-08-29 04:26:38 +00:00
Merge pull request #2673 from deepseek-harness/fix/web-feedback-note-popover
fix(feedback): float the note editor in a popover
This commit is contained in:
Chinesezjc
@@ -0,0 +1,6 @@
|
||||
# Bilingual-pair consistency record (docs/i18n/README.md): the git blob hash of each
|
||||
# side as of the last confirmed-consistent state. Both languages carry equal authority;
|
||||
# after editing either side, bring the other along and re-record with:
|
||||
# pnpm run verify-translation-pairing --write .agents/notes/implemented/bug-fix/2026-08-13-feedback-note-editor-popover.md
|
||||
2026-08-13-feedback-note-editor-popover.md: 33b7cd84b97ceac7fef1d96f1dee279fbb215800
|
||||
2026-08-13-feedback-note-editor-popover.zh.md: b130ab1e9c66372cf2a52bc5e12f65027c64cf51
|
||||
@@ -0,0 +1,43 @@
|
||||
# Agent Note: The feedback note editor floats above the transcript in a popover
|
||||
|
||||
Status: implemented
|
||||
|
||||
English | [中文](2026-08-13-feedback-note-editor-popover.zh.md)
|
||||
|
||||
## Problem
|
||||
|
||||
The Web surface for message feedback ([#2262](https://github.com/deepseek-harness/deepseek-harness/pull/2262)) contributes its controls to `conversation.chat.assistant-actions`, which renders inside the finalized assistant message's shared IconActions row. That row was one fixed-height `flex` line with `flex-wrap` at its initial `nowrap` and `height: 28px`, sized for 28px icons and a clock. The note editor mounted into it as an inline group holding a `width: 260px` textarea plus Save and Cancel.
|
||||
|
||||
A 260px input and two buttons do not fit that line at any window size. Measured against the shipped bundle, the row's scrollable overflow with the editor open was 168px at a 1680px viewport and 444px at 600px — the defect was never a narrow-window edge case, it was present at full-screen desktop. Flex overflow spills past the end of the line, so the items after the editor in flex order were the ones pushed out of the conversation column: the branch action left the column at 600px, and the clock and its run/TTFT/throughput readings left it at 900px. Those controls stay hit-testable while invisible, so no behavioral assertion noticed; the shipped e2e covered rate, note, reload, and retract, and the 24 UI snapshots are width-independent DOM.
|
||||
|
||||
The same stylesheet also named four `--dsw-alias-*` tokens that the theme does not define: `border-secondary`, `bg-primary`, `interactive-bg-primary`, and `label-inverse`. An undefined custom property makes its whole declaration invalid at computed-value time, so the textarea shipped with no border and no surface, and Save with neither fill nor a readable label — the editor read as loose text floating in the transcript rather than as an input.
|
||||
|
||||
## Decision
|
||||
|
||||
The note editor does not enter the row's flex layout at all. It is a popover: a fixed-position panel, portaled to `document.body`, whose coordinates come from the note trigger's rect. The row keeps its single line of icons and the note trigger, so nothing has to shrink, wrap, or reflow around the editor, and no `order` or wrapping is needed anywhere. Portaling out of the conversation column also escapes its `overflow` clip, so the panel cannot be cropped at the scroll edge and it moves with the message it annotates when the transcript scrolls. This reuses the same portal mechanism `ui-primitives/Menu` uses for anchored menus (`ui-subagent`'s catalog popover is built on it): the panel is `position: fixed`, placed from the anchor rect on open, clamped inside the viewport, and re-placed on scroll (capture phase) and resize. That anchoring is shared rather than copied: `ui-primitives/useAnchoredPosition` owns measure-offset-clamp-and-track, and the duplication gate is what forced the extraction — an inline copy of the clamp and its listener pair reported a 10-line clone against `Menu`. `Menu` keeps its own effect because its placement also resolves `side`/`align` variants and an optional caller-supplied anchor rect, which this surface does not need; the hook covers the plain below-the-anchor case both would otherwise spell out.
|
||||
|
||||
**The action strip.** The like/dislike buttons and the note trigger stay in the row, unchanged. The trigger is a plain button (`aria-haspopup="dialog"`, `aria-expanded` while open) that shows "Add a note" before a note exists and the note text afterward.
|
||||
|
||||
**The popover.** While open, the panel contains the textarea plus Save and Cancel, and any note-save failure, as `role="dialog"` with a title distinct from the textarea's own label so both are addressable by name. It opens beneath the trigger (4px gap), clamps to 12px from the viewport edges, auto-focuses the textarea, and closes on Escape or an outside pointer-down. Closing returns focus to the trigger only when the panel was really open, never on the initial mount (a freshly rendered rated message must not pull focus into its action row). A rating action during an open editor closes the panel. The four undefined tokens are replaced with the ones the theme actually defines, matching the primitives' precedent: `border-l2` and `bg-layer-1` for the input, `button-primary-fill` with `label-primary-foreground` plus a `button-primary-hover` state for Save; the panel surface reuses the Menu card recipe (`--dsw-specific-menu`, `--dsw-shadow-lv3`, inverted hairline `--dsw-alias-border-inverted`, `border-radius: 12px`).
|
||||
|
||||
**Failure surfaces split by where the human is looking.** A rating or list-load failure shows beside the buttons in the row, legible whether or not the popover is open. A note-save failure shows inside the popover, next to Save/Cancel, and the panel stays open so the draft survives to be corrected.
|
||||
|
||||
## Alternatives considered
|
||||
|
||||
**Inline expansion on the row, the editor claiming its own line via a full-width flex basis with the row allowed to wrap** — the approach first shipped on this branch and rejected here. It fixes the geometry (the row reports zero overflow from 1680px down to 600px) but at a visible cost: the branch action and the end clock wrap below the editor while it is open, the row occupies three lines, and the interaction competes for the same horizontal strip the row already fills. That cost is what [#2561](https://github.com/deepseek-harness/deepseek-harness/issues/2561) reported from real use — the row reads as misaligned once the editor expands — and it asked for the popover the chat surface already uses. A popover removes the editor from the row entirely, so the strip and the keyboard tab order are untouched whether the editor is open or not.
|
||||
|
||||
**An absolutely-positioned popover not portaled out of the column** — rejected: the conversation column is an `overflow-y: auto` scroller, so a panel laid out inside it is clipped at the scroll edge and does not track the message as the column scrolls. Portaling to `document.body` with fixed placement from the trigger rect is what makes the floating panel viable, exactly as `Menu`'s portal mode and the subagent catalog popover already do.
|
||||
|
||||
**A new `belowActions` seam on `MessageIconActions`, rendering the editor as a sibling under the row** — rejected: the slot contract documents `assistant-actions` as rendering *inside* the message's IconActions row, and one entry cannot supply two render sites without widening the host contract for a presentation detail that a portaled popover already expresses without touching the host.
|
||||
|
||||
## Consequences
|
||||
|
||||
With the editor open the actions row stays a single 28px line with zero overflow and nothing outside the column at every viewport from 1680px down to 600px — because the editor is not in the row to begin with. The panel floats above the transcript inside the viewport and stays anchored to its trigger, escaping the column's overflow clip. The editor is legible as an input in both themes.
|
||||
|
||||
`apps/web/tests/message-feedback-layout.e2e.ts` sweeps six viewports with the editor open and pins, per stop, that the row reports one line and zero overflow, that the panel is outside the conversation column (proof it escapes the clip), that it lies within the viewport (proof the clamp holds), and that it sits by its trigger. A committed golden records the relations; reverting to inline (or dropping the portal) fails the geometry assertions. `packages/client/ui-message-feedback/tests/styles.client.spec.ts` checks the tokens against the theme's committed source, that the panel is `position: fixed`, and that it carries no flex sizing (so it cannot rejoin the row), plus the brace balance, following the `ui-settings-models` styles-spec precedent. The unit spec covers rate, note, reload, retract, plus the popover's portal-to-body, Escape/outside-click dismissal, and keep-open-on-inside-click.
|
||||
|
||||
The `ui-message-feedback` package adds `@types/react-dom` so the `createPortal` usage typechecks, mirroring `ui-primitives`.
|
||||
|
||||
Known limitations are accepted rather than fixed here. A rating click while the panel is open closes it, and the close path returns focus to the note trigger rather than leaving it on the rating button the human just pressed; the same happens when an outside click lands on another focusable control, which the browser focuses before the close returns focus to the trigger. A pointer user does not notice either; a keyboard user feels the focus move. The clamp assumes the panel fits: a panel taller than the viewport makes the upper bound `innerHeight - height - margin` smaller than `margin`, so `top` goes negative and the panel's head is cut off rather than its foot. The panel's three-row textarea carries `resize: vertical`, so a human can drag past that size; `.notePanel` therefore bounds its height at `calc(100vh - 24px)` and scrolls its own content, the counterpart of the existing `max-width` and the same 12px margin the clamp uses. If the rating disappears while the editor is open, the panel unmounts on the `rating !== undefined` guard but `noteOpen` stays true, so the document-level Escape and pointer-down listeners remain attached; should the item reappear through a later resync, the panel returns with the previous draft and without refocusing the textarea. The window is one click or Escape wide, and the save failure it could hide already falls back to the row, so it is left as it is. A failure that lands after the panel was closed and reopened is not written into the new session's panel: its draft was reseeded from the stored note, so an old attempt's error would mislabel it, and the uncommitted content is already gone — the failure is dropped rather than shown. And while the placement replays on scroll, window resize, and the panel's own size changes, jsdom has no layout, so the real geometry is proven by the browser scenario while the unit spec covers the wiring through a `ResizeObserver` stub.
|
||||
|
||||
A residual narrow-viewport clock overflow remains below 520px from the clock string alone, unrelated to the feedback surface. The repo has no gate for undefined design tokens, and a scan during this work found more in `ui-agent-preset`, `ui-conversation`, `ui-jobs`, `ui-settings-plugins`, and `ui-tool`; they are untouched here and want their own change.
|
||||
@@ -0,0 +1,43 @@
|
||||
# Agent Note：反馈备注编辑器以浮层悬浮在对话记录上方
|
||||
|
||||
Status: implemented
|
||||
|
||||
[English](2026-08-13-feedback-note-editor-popover.md) | 中文
|
||||
|
||||
## Problem
|
||||
|
||||
消息反馈的 Web 界面（[#2262](https://github.com/deepseek-harness/deepseek-harness/pull/2262)）把控件贡献给 `conversation.chat.assistant-actions`，该槽位渲染在已定稿助手消息共享的 IconActions 行内。那一行是单条固定高度的 `flex` 线，`flex-wrap` 保持初始值 `nowrap` 且 `height: 28px`，按 28px 图标加一个时钟来定尺寸。备注编辑器作为一个内联组挂进去，内含 `width: 260px` 的 textarea 加 Save 与 Cancel。
|
||||
|
||||
一个 260px 输入框加两个按钮在任何窗口尺寸下都装不进那条线。对着已构建产物实测，编辑器打开时该行的可滚动溢出在 1680px 视口下是 168px，在 600px 下是 444px——这个缺陷从来不是窄窗口的边缘情况，在全屏桌面下就已存在。flex 溢出会溢出到线的末端之外，因此按 flex 顺序排在编辑器之后的项被挤出会话列：branch 操作在 600px 时离开列，时钟及其运行时长/TTFT/吞吐读数在 900px 时离开列。这些控件在不可见的同时仍可命中测试，所以没有任何行为断言发现它；已交付的 e2e 覆盖评分、备注、reload 与撤回，而 24 个 UI 快照是与宽度无关的 DOM。
|
||||
|
||||
同一张样式表还引用了四个主题并未定义的 `--dsw-alias-*` token：`border-secondary`、`bg-primary`、`interactive-bg-primary` 与 `label-inverse`。未定义的自定义属性会让其所在的整条声明在 computed-value 阶段失效，因此 textarea 交付时既无边框也无底色，Save 既无填充也无可读标签——编辑器读起来像是浮在对话记录里的散落文本，而不是一个输入框。
|
||||
|
||||
## Decision
|
||||
|
||||
备注编辑器完全不进入行的 flex 布局。它是一个浮层：一张固定定位的面板，portal 到 `document.body`，其坐标来自备注触发按钮的矩形。行保持其单行图标与备注触发按钮，因此没有任何东西需要围绕编辑器收缩、换行或回流，任何地方都不需要 `order` 或换行。portal 出会话列也逃出了列的 `overflow` 裁剪，因此面板不会被滚动边缘裁掉，并且当对话记录滚动时会随它所批注的消息一起移动。这里复用 `ui-primitives/Menu` 为锚定菜单所用的同一套 portal 机制（`ui-subagent` 的 catalog popover 就构建在它之上）：面板 `position: fixed`，打开时从 anchor rect 定位，钳制在视口内，并在滚动（捕获阶段）与缩放时重新定位。这套锚定逻辑是共享而非复制的：`ui-primitives/useAnchoredPosition` 持有「测量—偏移—钳制—跟随」这一件事，而促成这次抽取的正是重复代码门禁——内联的钳制与那对监听器被报为与 `Menu` 的 10 行克隆。`Menu` 保留自己的 effect，因为它的定位还要解析 `side`/`align` 变体与可选的调用方 anchor rect，而本界面不需要这些；该 hook 覆盖的是两边本来都要各写一遍的「锚点正下方」这一简单情形。
|
||||
|
||||
**操作条。** 点赞/点踩按钮与备注触发按钮保持原样留在行内。触发按钮是普通 `button`（`aria-haspopup="dialog"`，打开时 `aria-expanded`），在没有备注时显示「补充说明」，已有备注时显示备注文本。
|
||||
|
||||
**浮层。** 打开时，面板内含 textarea、Save 与 Cancel，以及任何备注保存失败提示，作为 `role="dialog"`，其标题与 textarea 自身的标签不同，以便两者都能按名称寻址。它在触发按钮下方打开（4px 间距），钳制到距视口边缘 12px，自动聚焦 textarea，并在 Escape 或外部 pointer-down 时关闭。关闭时仅当面板确实曾经打开才把焦点还给触发按钮，绝不会在初始挂载时（新渲染出的一条已评分消息不得把焦点拉进其操作条）。编辑器打开时进行评分操作会关闭面板。四个未定义 token 换成主题确实定义的那些，与 primitives 的既有做法一致：输入框用 `border-l2` 与 `bg-layer-1`，Save 用 `button-primary-fill` 配 `label-primary-foreground` 并加 `button-primary-hover` 状态；面板表面复用 Menu 卡片的配方（`--dsw-specific-menu`、`--dsw-shadow-lv3`、反色发丝线 `--dsw-alias-border-inverted`、`border-radius: 12px`）。
|
||||
|
||||
**失败提示按人的视线所落之处拆分。** 评分或列表加载失败显示在按钮旁的图标行里，无论浮层是否打开都清晰可读。备注保存失败显示在浮层内、Save/Cancel 旁，且面板保持打开，以便草稿留存待修正。
|
||||
|
||||
## Alternatives considered
|
||||
|
||||
**行内展开：编辑器通过整行 flex basis 独占一行，并让行允许换行** — 这是本分支最初交付、在此否决的做法。它修好了几何（行在 1680px 到 600px 报告零溢出），但有可见代价：branch 与末尾时钟在编辑器打开时换行到编辑器下方，行占三行，交互与行本就占满的横向条带争空间。这一代价正是 [#2561](https://github.com/deepseek-harness/deepseek-harness/issues/2561) 在真实使用中反馈的问题——编辑器展开后这一行读起来是错位的——并提出改用 chat 界面已有的弹窗。浮层把编辑器完全移出行，因此无论编辑器是否打开，操作条与键盘 Tab 顺序都不受影响。
|
||||
|
||||
**不 portal 出列的绝对定位浮层** — 否决：会话列是 `overflow-y: auto` 的滚动容器，因此在列内布局的面板会被滚动边缘裁掉，且不随列滚动而跟住消息。portal 到 `document.body` 并从触发按钮矩形做固定定位，才让浮动面板可行，正如 `Menu` 的 portal 模式与 subagent catalog popover 已然做到的那样。
|
||||
|
||||
**在 `MessageIconActions` 上新增 `belowActions` 接缝，把编辑器作为该行的兄弟节点渲染在下方** — 否决：slot 契约明确记载 `assistant-actions` 渲染在消息 IconActions 行**内部**，且单个条目无法在不为一个展示细节拓宽 Host 契约的前提下提供两个渲染点，而 portal 出的浮层无需触碰 Host 就表达了该细节。
|
||||
|
||||
## Consequences
|
||||
|
||||
编辑器打开时，操作行保持单条 28px 线，在 1680px 到 600px 的每一档视口都零溢出、零项落在列外——因为编辑器本就不在行里。面板悬浮于对话记录之上、位于视口内，并保持锚定其触发按钮，逃出列溢出裁剪。编辑器在两种主题下都能被辨认为输入框。
|
||||
|
||||
`apps/web/tests/message-feedback-layout.e2e.ts` 在编辑器打开时扫描六个视口，并在每一档钉住：行报告单行零溢出、面板位于会话列之外（证明它逃出裁剪）、面板落在视口内（证明钳制有效）、面板紧贴其触发按钮。已提交的 golden 记录这些关系；回退到行内（或去掉 portal）会让几何断言失败。`packages/client/ui-message-feedback/tests/styles.client.spec.ts` 校验 token 与主题已提交的源一致、面板为 `position: fixed`、且不带任何 flex sizing（因此不会重新加入行），并校验大括号平衡，沿用 `ui-settings-models` styles spec 的先例。单元 spec 覆盖评分、备注、reload、撤回，外加浮层的 portal 到 body、Escape/外部点击关闭、以及浮层内部点击保持打开。
|
||||
|
||||
`ui-message-feedback` 包新增 `@types/react-dom`，使 `createPortal` 用法能通过类型检查，与 `ui-primitives` 一致。
|
||||
|
||||
有若干已知限制在此接受而非修复。面板打开时点击评分会关闭它，而关闭路径把焦点归还给备注触发按钮，而不是留在用户刚按下的评分按钮上；外部点击落在另一个可聚焦控件上时同理——浏览器先把焦点给该控件，随后关闭路径又把它拉回触发按钮。指针用户对两者都无感，键盘用户会察觉焦点移动。钳制假定面板放得下：面板高于视口时，上界 `innerHeight - height - margin` 会小于 `margin`，于是 `top` 变为负值、被裁掉的是面板顶部而非底部。面板里的三行 textarea 带 `resize: vertical`，用户可以拖过这个尺寸，因此 `.notePanel` 把自身高度限制在 `calc(100vh - 24px)` 并自行滚动内容——这是既有 `max-width` 的对应项，用的是与钳制相同的 12px 边距。编辑器打开时若评分消失，面板会因 `rating !== undefined` 守卫卸载，但 `noteOpen` 仍为 true，因此 document 级的 Escape 与 pointer-down 监听继续挂着；若该 item 之后经 resync 重新出现，浮层会带着上一次的草稿回来且不重新聚焦 textarea。该窗口只有一次点击或一次 Escape 那么宽，而它可能遮住的保存失败已经有行内回退，因此保持现状。若失败在面板关闭并重开后才到达，不会写入新会话的面板：其草稿已按已存备注重新播种，旧尝试的错误会误标新草稿，而未提交的内容本就不存在——该失败被丢弃而不展示。以及，定位虽然会在滚动、窗口缩放与面板自身尺寸变化时重放，但 jsdom 没有布局，因此真实几何由浏览器场景证明，单测则通过 `ResizeObserver` stub 覆盖其接线。
|
||||
|
||||
520px 以下仍残留仅来自时钟字符串的窄视口溢出，与本界面无关。仓库没有针对未定义设计 token 的门禁；本次工作中的一次扫描在 `ui-agent-preset`、`ui-conversation`、`ui-jobs`、`ui-settings-plugins` 与 `ui-tool` 中又发现更多，本次未触碰，需要单独的改动处理。
|
||||
@@ -0,0 +1,336 @@
|
||||
// Web e2e scenario: with the feedback note editor open, the assistant IconActions
|
||||
// row stays one intact line (no wrapping, nothing pushed out), and the note
|
||||
// editor floats above the transcript in a popover that escapes the conversation
|
||||
// column's overflow clip and stays inside the viewport.
|
||||
//
|
||||
// The hazard this pins: a slot-contributed note editor (260px textarea plus
|
||||
// Save and Cancel) cannot fit the shared IconActions row at ANY viewport, and an
|
||||
// inline expansion made the row wider than the column — full-screen desktop
|
||||
// included — so the branch action and the clock were pushed out of view by later
|
||||
// flex items. The fix is to not mount the editor in the row at all: it is a
|
||||
// popover portaled to document.body and fixed-positioned from the note trigger's
|
||||
// rect, so the row keeps its single 28px line of icons and the trigger, and the
|
||||
// panel cannot be cropped by the column's overflow because it lives outside it.
|
||||
//
|
||||
// The sweep records, per viewport, whether the open editor keeps the actions row
|
||||
// on one line with zero overflow, whether the panel is outside the column (proof
|
||||
// it escapes the clip), whether the panel stays inside the viewport (proof the
|
||||
// clamp works), and whether it sits by its trigger. All relations, no absolute
|
||||
// pixels: the column width follows the viewport, the sidebar, and the platform's
|
||||
// scrollbar, so a golden carrying pixels would document the platform, not the
|
||||
// behavior.
|
||||
//
|
||||
// Zero model calls: a settled transcript is cold-seeded, so nothing streams.
|
||||
import { readFile } from 'node:fs/promises'
|
||||
import { fileURLToPath } from 'node:url'
|
||||
import { join } from 'node:path'
|
||||
import type { Browser, Page } from 'playwright'
|
||||
import { chromium } from 'playwright'
|
||||
import { afterAll, beforeAll, describe, expect, it, onTestFailed } from 'vitest'
|
||||
import {
|
||||
compareOrRefreshGolden, launchWebScaffold, seedSession, watchConsole, webSnapshotMode,
|
||||
type WebScaffold,
|
||||
} from './scaffold.ts'
|
||||
import { newEnglishPage, saveFailureShot } from './support.ts'
|
||||
|
||||
const SNAPSHOT_DIR = fileURLToPath(new URL('./snapshots/message-feedback-layout', import.meta.url))
|
||||
/**
|
||||
* Committed golden of the popover relations at every stop. Booleans and counts
|
||||
* only, never absolute coordinates.
|
||||
*/
|
||||
const GEOMETRY_EXPECTED = join(SNAPSHOT_DIR, 'geometry.expected.md')
|
||||
const MODE = webSnapshotMode()
|
||||
/** Borrowed read-only: this scenario needs any settled assistant message to rate. */
|
||||
const SEED = fileURLToPath(new URL('./snapshots/seeded-history/seed.jsonl', import.meta.url))
|
||||
const SEED_ID = 'message-feedback-layout-e2e'
|
||||
/** Viewport widths from full-screen desktop down to a narrow window. */
|
||||
const WIDTHS = [1680, 1280, 1024, 900, 700, 600]
|
||||
|
||||
/** One viewport stop: how the row reads with the note editor closed and open, plus the popover's own relations. */
|
||||
export interface PopoverMetrics {
|
||||
/** Viewport width the stop was measured at. */
|
||||
width: number
|
||||
/** The row's scrollable overflow with the note editor closed (natural row width). */
|
||||
rowOverflowClosed: number
|
||||
/** The row's scrollable overflow with the note editor open; must equal the closed value. */
|
||||
rowOverflowOpen: number
|
||||
/** Flex lines the row occupies with the note editor open; the editor must not reflow it. */
|
||||
rowLines: number
|
||||
/** Row items whose right edge escapes the column, editor closed. */
|
||||
itemsOutsideColumnClosed: number
|
||||
/** Row items whose right edge escapes the column, editor open; must equal the closed value. */
|
||||
itemsOutsideColumnOpen: number
|
||||
/** True when the portaled panel is NOT inside the column (escapes its overflow clip). */
|
||||
panelOutsideColumn: boolean
|
||||
/** True when the panel lies fully inside the viewport (the clamp holds). */
|
||||
panelWithinViewport: boolean
|
||||
/** Horizontal separation between the panel's left edge and the note trigger's, in px. */
|
||||
panelToTriggerGap: number
|
||||
}
|
||||
|
||||
/**
|
||||
* Measure the feedback row (and the open popover, when present) at the current
|
||||
* viewport. The same reader serves the closed and open readings so the two
|
||||
* sides differ only by whether the editor is open.
|
||||
* @param page - the page under test.
|
||||
* @param width - the viewport width already applied, recorded with the reading.
|
||||
* @param editorOpen - true to also read the popover's relations; throws if it is absent.
|
||||
* @returns the stop's relations.
|
||||
*/
|
||||
function measurePopover(page: Page, width: number, editorOpen: boolean): Promise<PopoverMetrics> {
|
||||
return page.evaluate(({ viewportWidth, open }) => {
|
||||
const rated = document.querySelector<HTMLElement>('button[aria-label="Remove rating"]')
|
||||
if (rated === null) throw new Error('no rated feedback control in the DOM')
|
||||
const row = rated.parentElement?.closest<HTMLElement>('div[class*="actions"]') ?? null
|
||||
if (row === null) throw new Error('the IconActions row is not an ancestor of the feedback control')
|
||||
const trigger = row.querySelector<HTMLElement>('button[aria-haspopup="dialog"]')
|
||||
if (trigger === null) throw new Error('the note trigger is not in the row')
|
||||
|
||||
/**
|
||||
* The real flex items of the row. A slot contributor (the feedback strip)
|
||||
* arrives as a `display: contents` wrapper (the `assistant-actions` slot
|
||||
* renders inside a transparent `data-slot` div), which reports an all-zero
|
||||
* rect; a zero box would be miscounted as a phantom flex line. The actual
|
||||
* items are the boxes inside it.
|
||||
* @param element - the row whose items to read.
|
||||
* @returns the real flex-item boxes, in flex/DOM order.
|
||||
*/
|
||||
const flexItemBoxes = (element: HTMLElement): DOMRect[] => {
|
||||
const boxes: DOMRect[] = []
|
||||
for (const child of Array.from(element.children)) {
|
||||
const el = child as HTMLElement
|
||||
const rect = el.getBoundingClientRect()
|
||||
if (el.style.display === 'contents') {
|
||||
boxes.push(...flexItemBoxes(el))
|
||||
} else if (rect.height > 0 && rect.width > 0) {
|
||||
boxes.push(rect)
|
||||
}
|
||||
}
|
||||
return boxes
|
||||
}
|
||||
/**
|
||||
* Group items into flex lines by overlapping vertical extent.
|
||||
* @param boxes - the row items' boxes, in DOM order.
|
||||
* @returns the number of distinct lines.
|
||||
*/
|
||||
const countFlexLines = (boxes: DOMRect[]): number => {
|
||||
const centres: number[] = []
|
||||
for (const box of boxes) {
|
||||
const centre = box.top + box.height / 2
|
||||
if (!centres.some(known => Math.abs(known - centre) <= box.height / 2)) centres.push(centre)
|
||||
}
|
||||
return centres.length
|
||||
}
|
||||
|
||||
const column = row.closest<HTMLElement>('[data-conversation-scroll]')
|
||||
const columnRight = (column?.getBoundingClientRect().left ?? 0) + (column?.clientWidth ?? 0)
|
||||
const itemRects = flexItemBoxes(row)
|
||||
// A half-pixel tolerance: subpixel layout puts a contained edge a fraction
|
||||
// over the boundary on some device scale factors.
|
||||
const itemsOutsideColumn = itemRects.filter(box => box.right > columnRight + 0.5).length
|
||||
// The editor is a portal, so the row measures identically whether the
|
||||
// editor is open or not; the closed/open fields differ by call so the sweep
|
||||
// can assert a zero delta on them.
|
||||
const overflow = row.scrollWidth - row.clientWidth
|
||||
|
||||
let builder: {
|
||||
panelOutsideColumn: boolean
|
||||
panelWithinViewport: boolean
|
||||
panelToTriggerGap: number
|
||||
}
|
||||
if (!open) {
|
||||
builder = { panelOutsideColumn: true, panelWithinViewport: true, panelToTriggerGap: 0 }
|
||||
} else {
|
||||
const panel = document.body.querySelector<HTMLElement>('[role="dialog"]')
|
||||
if (panel === null) throw new Error('the note popover is not open')
|
||||
const panelBox = panel.getBoundingClientRect()
|
||||
const triggerBox = trigger.getBoundingClientRect()
|
||||
const vw = window.innerWidth
|
||||
const vh = window.innerHeight
|
||||
builder = {
|
||||
// The panel portals out of the column, so the clip cannot reach it.
|
||||
panelOutsideColumn: column === null ? true : !column.contains(panel),
|
||||
panelWithinViewport:
|
||||
panelBox.left >= -0.5
|
||||
&& panelBox.right <= vw + 0.5
|
||||
&& panelBox.top >= -0.5
|
||||
&& panelBox.bottom <= vh + 0.5,
|
||||
// The panel is fixed from the trigger's left, so a zero gap says it is
|
||||
// anchored; a clamp can only widen it.
|
||||
panelToTriggerGap: Math.abs(panelBox.left - triggerBox.left),
|
||||
}
|
||||
}
|
||||
|
||||
return {
|
||||
width: viewportWidth,
|
||||
rowOverflowClosed: overflow,
|
||||
rowOverflowOpen: overflow,
|
||||
rowLines: countFlexLines(itemRects),
|
||||
itemsOutsideColumnClosed: itemsOutsideColumn,
|
||||
itemsOutsideColumnOpen: itemsOutsideColumn,
|
||||
...builder,
|
||||
}
|
||||
}, { viewportWidth: width, open: editorOpen })
|
||||
}
|
||||
|
||||
/**
|
||||
* Render the golden body: one line per stop, relations and counts only. The
|
||||
* row-overflow and outside-column readings are deltas (open minus closed) so
|
||||
* the golden records that opening the editor leaves the row untouched, not an
|
||||
* absolute count that many unrelated controls could move.
|
||||
* @param stops - the measured stops, in sweep order.
|
||||
* @returns the golden body, without a trailing newline.
|
||||
*/
|
||||
function renderGeometry(stops: PopoverMetrics[]): string {
|
||||
return [
|
||||
'# Assistant actions row with the feedback note popover open',
|
||||
'',
|
||||
'| viewport | row overflow delta | row lines | items-outside delta '
|
||||
+ '| panel outside the column | panel within the viewport | panel-to-trigger gap |',
|
||||
'| --- | --- | --- | --- | --- | --- | --- |',
|
||||
...stops.map(stop => `| ${String(stop.width)}px | ${String(stop.rowOverflowOpen - stop.rowOverflowClosed)}px `
|
||||
+ `| ${String(stop.rowLines)} | ${String(stop.itemsOutsideColumnOpen - stop.itemsOutsideColumnClosed)} `
|
||||
+ `| ${String(stop.panelOutsideColumn)} | ${String(stop.panelWithinViewport)} `
|
||||
+ `| ${String(stop.panelToTriggerGap)}px |`),
|
||||
].join('\n')
|
||||
}
|
||||
|
||||
describe('web e2e: the feedback note editor floats above the column', () => {
|
||||
let scaffold: WebScaffold
|
||||
let browser: Browser
|
||||
let page: Page
|
||||
let tripwire: ReturnType<typeof watchConsole>
|
||||
|
||||
beforeAll(async () => {
|
||||
scaffold = await launchWebScaffold({})
|
||||
await seedSession(scaffold, await readFile(SEED, 'utf8'), SEED_ID)
|
||||
browser = await chromium.launch()
|
||||
page = await newEnglishPage(browser, 900)
|
||||
tripwire = watchConsole(page)
|
||||
await page.goto(scaffold.baseUrl, { waitUntil: 'load' })
|
||||
await page.waitForSelector('[class*="frame"]', { timeout: 30_000 })
|
||||
}, 180_000)
|
||||
|
||||
afterAll(async () => {
|
||||
await browser?.close()
|
||||
await scaffold?.close()
|
||||
})
|
||||
|
||||
/**
|
||||
* Open the seeded transcript. The first treeitem is the collapsible group
|
||||
* row; the session itself is the row beneath it.
|
||||
* @returns nothing.
|
||||
*/
|
||||
async function openSeededSession(): Promise<void> {
|
||||
const groupRow = page.locator('[role="treeitem"]').first()
|
||||
await groupRow.waitFor({ timeout: 15_000 })
|
||||
if (await groupRow.getAttribute('aria-expanded') !== 'true') await groupRow.click()
|
||||
const sessionRow = page.locator('[role="treeitem"]').nth(1)
|
||||
await sessionRow.waitFor({ timeout: 15_000 })
|
||||
await sessionRow.click()
|
||||
}
|
||||
|
||||
/**
|
||||
* Resize to a viewport and read the row once its width stops moving. The
|
||||
* frame eases its column tracks, so reading straight after a resize can
|
||||
* report the previous viewport's relation.
|
||||
* @param width - viewport width to settle at.
|
||||
* @param editorOpen - whether the note editor is currently open; reads the popover relations when so.
|
||||
* @returns the row's (and popover's) readings at that width.
|
||||
*/
|
||||
const settleAt = async (width: number, editorOpen: boolean): Promise<PopoverMetrics> => {
|
||||
await page.setViewportSize({ width, height: 900 })
|
||||
let previous = -1
|
||||
await expect.poll(async () => {
|
||||
const current = await page.evaluate(() =>
|
||||
document.querySelector('[data-conversation-scroll]')?.clientWidth ?? -1)
|
||||
const settled = current === previous
|
||||
previous = current
|
||||
return settled
|
||||
}, { timeout: 10_000 }).toBe(true)
|
||||
// The popover is JS-positioned from the trigger rect and re-places on
|
||||
// resize/scroll, so once the column width stops moving we nudge it to the
|
||||
// final layout; otherwise the panel can sit at a transient position from
|
||||
// mid-resize and the anchor reading would be off.
|
||||
await page.evaluate(() => window.dispatchEvent(new Event('resize')))
|
||||
return measurePopover(page, width, editorOpen)
|
||||
}
|
||||
|
||||
/**
|
||||
* Rate a message, then for every stop read the row once with the note editor
|
||||
* closed and once with it open, handing the SAME measured readings to both
|
||||
* assertions so the golden and the assertions describe one measurement
|
||||
* rather than two runs that could disagree.
|
||||
* @returns the stops in {@link WIDTHS} order.
|
||||
*/
|
||||
let swept: Promise<PopoverMetrics[]> | undefined
|
||||
const sweep = (): Promise<PopoverMetrics[]> => {
|
||||
swept ??= (async () => {
|
||||
await openSeededSession()
|
||||
await page.getByText('DONE', { exact: true }).waitFor({ timeout: 30_000 })
|
||||
// The controller defers its list read to the first hover or focus, so the
|
||||
// strip has to be touched before it can be rated.
|
||||
const like = page.getByRole('button', { name: 'Good response' }).first()
|
||||
await like.waitFor({ timeout: 30_000 })
|
||||
await like.scrollIntoViewIfNeeded()
|
||||
await like.hover()
|
||||
await like.click()
|
||||
await page.getByRole('button', { name: 'Remove rating' }).first()
|
||||
.waitFor({ timeout: 15_000 })
|
||||
const noteTrigger = page.getByRole('button', { name: 'Add a note' }).first()
|
||||
const stops: PopoverMetrics[] = []
|
||||
for (const width of WIDTHS) {
|
||||
// Reset to the closed baseline at each stop before opening.
|
||||
if (await noteTrigger.getAttribute('aria-expanded') === 'true') await noteTrigger.click()
|
||||
const closed = await settleAt(width, false)
|
||||
await page.getByRole('button', { name: 'Add a note' }).first().click()
|
||||
await page.getByRole('dialog').waitFor({ timeout: 10_000 })
|
||||
const open = await settleAt(width, true)
|
||||
stops.push({
|
||||
width,
|
||||
rowOverflowClosed: closed.rowOverflowClosed,
|
||||
rowOverflowOpen: open.rowOverflowOpen,
|
||||
rowLines: open.rowLines,
|
||||
itemsOutsideColumnClosed: closed.itemsOutsideColumnClosed,
|
||||
itemsOutsideColumnOpen: open.itemsOutsideColumnOpen,
|
||||
panelOutsideColumn: open.panelOutsideColumn,
|
||||
panelWithinViewport: open.panelWithinViewport,
|
||||
panelToTriggerGap: open.panelToTriggerGap,
|
||||
})
|
||||
}
|
||||
return stops
|
||||
})()
|
||||
return swept
|
||||
}
|
||||
|
||||
it('keeps the actions row untouched by the note popover, which stays in the viewport', async () => {
|
||||
onTestFailed(() => saveFailureShot(page, 'web-e2e-message-feedback-layout'))
|
||||
const stops = await sweep()
|
||||
for (const stop of stops) {
|
||||
// The popover lives outside the row, so opening it must not change the
|
||||
// row at all. This is the vacuity guard of the whole redesign: an inline
|
||||
// editor would widen or reflow the row, pushing the delta off zero.
|
||||
expect(stop.rowOverflowOpen - stop.rowOverflowClosed, `viewport ${String(stop.width)}`).toBe(0)
|
||||
expect(stop.itemsOutsideColumnOpen - stop.itemsOutsideColumnClosed, `viewport ${String(stop.width)}`).toBe(0)
|
||||
// The row is one 28px line; the editor never forces a reflow.
|
||||
expect(stop.rowLines, `viewport ${String(stop.width)}`).toBe(1)
|
||||
// The panel escapes the column's overflow clip by living outside it.
|
||||
expect(stop.panelOutsideColumn, `viewport ${String(stop.width)}`).toBe(true)
|
||||
// The placement clamps the panel inside the viewport at every width.
|
||||
expect(stop.panelWithinViewport, `viewport ${String(stop.width)}`).toBe(true)
|
||||
// The panel stays anchored to its trigger rather than drifting off.
|
||||
expect(stop.panelToTriggerGap, `viewport ${String(stop.width)}`).toBeLessThanOrEqual(4)
|
||||
}
|
||||
expect(tripwire.pageErrors).toEqual([])
|
||||
}, 180_000)
|
||||
|
||||
it('matches the committed geometry golden', async () => {
|
||||
onTestFailed(() => saveFailureShot(page, 'web-e2e-message-feedback-layout-golden'))
|
||||
await compareOrRefreshGolden(GEOMETRY_EXPECTED, renderGeometry(await sweep()), MODE)
|
||||
}, 180_000)
|
||||
|
||||
it('kept the console clean', () => {
|
||||
expect(tripwire.pageErrors).toEqual([])
|
||||
expect(tripwire.warnings).toEqual([])
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,10 @@
|
||||
# Assistant actions row with the feedback note popover open
|
||||
|
||||
| viewport | row overflow delta | row lines | items-outside delta | panel outside the column | panel within the viewport | panel-to-trigger gap |
|
||||
| --- | --- | --- | --- | --- | --- | --- |
|
||||
| 1680px | 0px | 1 | 0 | true | true | 0px |
|
||||
| 1280px | 0px | 1 | 0 | true | true | 0px |
|
||||
| 1024px | 0px | 1 | 0 | true | true | 0px |
|
||||
| 900px | 0px | 1 | 0 | true | true | 0px |
|
||||
| 700px | 0px | 1 | 0 | true | true | 0px |
|
||||
| 600px | 0px | 1 | 0 | true | true | 0px |
|
||||
@@ -60,6 +60,7 @@
|
||||
"tests/web-search-round.e2e.ts",
|
||||
"tests/message-actions.e2e.ts",
|
||||
"tests/message-feedback.e2e.ts",
|
||||
"tests/message-feedback-layout.e2e.ts",
|
||||
"tests/markdown-images.e2e.ts",
|
||||
"tests/math-rendering.e2e.ts",
|
||||
"tests/markdown-cjk-strong.e2e.ts",
|
||||
|
||||
@@ -2,5 +2,5 @@
|
||||
# side as of the last confirmed-consistent state. Both languages carry equal authority;
|
||||
# after editing either side, bring the other along and re-record with:
|
||||
# pnpm run verify-translation-pairing --write packages/client/ui-message-feedback/README.md
|
||||
README.md: 461e87589567eb95b075839d5893cc551e67a035
|
||||
README.zh.md: 31f722021ff65f476a6ba1ab0211fd4e091671d2
|
||||
README.md: d3bb4e28b95da2fde0d26b7ef83ebca377b4939f
|
||||
README.zh.md: d823bb9de2b7d39d0bc5d00164b8ce8dffec4a34
|
||||
|
||||
@@ -2,7 +2,7 @@
|
||||
|
||||
English | [中文](README.zh.md)
|
||||
|
||||
Per-message feedback plugin, browser half: a Like/Dislike pair plus an optional note, contributed as the `feedback` entry (order 10) of the `conversation.chat.assistant-actions` strip. The strip is declared by `ui-conversation` and rendered inside the finalized assistant message's IconActions row, between copy and branch, so the controls inherit that row's chrome and hover behavior. Only finalized messages reach the slot — an interruption-frozen partial carries no `messageId` and therefore no feedback controls. The strip renders once per turn, on the closing assistant message that owns the turn's IconActions row: earlier steps of a multi-step turn produce tool rows rather than a rateable body, so they present no controls even though the Host would accept them as targets.
|
||||
Per-message feedback plugin, browser half: a Like/Dislike pair plus an optional note, contributed as the `feedback` entry (order 10) of the `conversation.chat.assistant-actions` strip. The strip is declared by `ui-conversation` and rendered inside the finalized assistant message's IconActions row, between copy and branch, so the controls inherit that row's chrome and hover behavior. The note editor itself does not sit in that row: it is a `role="dialog"` popover portaled to `document.body` and anchored under its trigger, so the row keeps its single line whether the editor is open or closed and the panel is not clipped by the conversation column. A rating or list-load failure shows inline in the row; a note-save failure shows inside the popover, which stays open so the draft can be corrected. Only finalized messages reach the slot — an interruption-frozen partial carries no `messageId` and therefore no feedback controls. The strip renders once per turn, on the closing assistant message that owns the turn's IconActions row: earlier steps of a multi-step turn produce tool rows rather than a rateable body, so they present no controls even though the Host would accept them as targets.
|
||||
|
||||
One `MessageFeedbackController` per Session backs every message control in that Session, so a single `messageFeedback.list` read seeds the whole transcript. The read is deferred to the first hover or focus rather than fired on mount, because the controls mount once per settled message in the visible history.
|
||||
|
||||
|
||||
@@ -2,7 +2,7 @@
|
||||
|
||||
[English](README.md) | 中文
|
||||
|
||||
单条消息反馈插件的浏览器侧：一对 Like/Dislike 按钮加一个可选备注，作为 `conversation.chat.assistant-actions` 条带的 `feedback` 条目（order 10）贡献。该条带由 `ui-conversation` 声明，渲染在已定稿助手消息的 IconActions 行内、复制与分支之间，因此控件沿用该行的样式与 hover 行为。只有已定稿的消息能到达这个 slot——被中断冻结的部分输出不带 `messageId`，因此也没有反馈控件。该操作栏每个 Turn 渲染一次，位于持有该 Turn IconActions 行的收尾助手消息上：多步骤 Turn 中较早的步骤产出的是工具行而非可评分正文，因此即使 Host 会接受它们作为目标，界面上也不出现控件。
|
||||
单条消息反馈插件的浏览器侧：一对 Like/Dislike 按钮加一个可选备注，作为 `conversation.chat.assistant-actions` 条带的 `feedback` 条目（order 10）贡献。该条带由 `ui-conversation` 声明，渲染在已定稿助手消息的 IconActions 行内、复制与分支之间，因此控件沿用该行的样式与 hover 行为。备注编辑器本身不在这一行里：它是一个 `role="dialog"` 的浮层，portal 到 `document.body` 并锚定在其触发按钮下方，因此无论编辑器是否打开该行都保持单行，面板也不会被会话列裁掉。评分或列表加载失败在行内展示；备注保存失败在浮层内展示，且面板保持打开以便修正草稿。只有已定稿的消息能到达这个 slot——被中断冻结的部分输出不带 `messageId`，因此也没有反馈控件。该操作栏每个 Turn 渲染一次，位于持有该 Turn IconActions 行的收尾助手消息上：多步骤 Turn 中较早的步骤产出的是工具行而非可评分正文，因此即使 Host 会接受它们作为目标，界面上也不出现控件。
|
||||
|
||||
每个 Session 一个 `MessageFeedbackController`，支撑该 Session 内所有消息的控件，因此一次 `messageFeedback.list` 读取即可填充整段对话。该读取延迟到首次 hover 或 focus 才发起，而不是在挂载时触发，因为可见历史中每条已结束的消息都会挂载一次控件。
|
||||
|
||||
|
||||
@@ -71,6 +71,7 @@
|
||||
"@deepseek-ai/cordis": "workspace:^",
|
||||
"@testing-library/react": "^16.1.0",
|
||||
"@types/react": "~18.3.1",
|
||||
"@types/react-dom": "~18.3.0",
|
||||
"react": "^18.2.0",
|
||||
"react-dom": "^18.2.0"
|
||||
},
|
||||
|
||||
@@ -1,6 +1,9 @@
|
||||
/* Per-message feedback controls. The rating buttons mirror the shared message
|
||||
IconActions chrome so the strip reads as one row; the note editor is an
|
||||
inline expansion anchored to the same row. */
|
||||
IconActions chrome so the strip reads as one row. The note editor is a
|
||||
popover portaled to document.body and fixed from the note trigger's rect,
|
||||
so it neither competes with the row for inline width nor gets cropped by the
|
||||
conversation column's overflow clip. Surface recipe follows the Menu card:
|
||||
r12, inverted hairline border, shadow-lv3. */
|
||||
|
||||
.action {
|
||||
display: inline-flex;
|
||||
@@ -47,29 +50,58 @@
|
||||
cursor: pointer;
|
||||
}
|
||||
|
||||
.noteOpen:hover {
|
||||
.noteOpen:hover,
|
||||
.noteOpen[aria-expanded='true'] {
|
||||
background: var(--dsw-alias-interactive-bg-hover);
|
||||
color: var(--dsw-alias-label-secondary);
|
||||
}
|
||||
|
||||
.noteEditor {
|
||||
display: inline-flex;
|
||||
align-items: flex-start;
|
||||
gap: 6px;
|
||||
/* Portal surface: fixed in the viewport, left/top supplied inline from the
|
||||
trigger rect. Portaled panels must layer above modal overlays (z 1000). */
|
||||
.notePanel {
|
||||
position: fixed;
|
||||
z-index: 1100;
|
||||
box-sizing: border-box;
|
||||
width: 320px;
|
||||
max-width: min(360px, calc(100vw - 24px));
|
||||
/* The width bound's counterpart. `resize: vertical` on the textarea lets the
|
||||
panel be dragged taller, and a panel taller than the viewport would push
|
||||
the placement clamp's upper bound below its own margin, so `top` would go
|
||||
negative and cut off the panel's head. Both bounds keep the 12px margin
|
||||
the clamp uses. */
|
||||
max-height: calc(100vh - 24px);
|
||||
overflow-y: auto;
|
||||
padding: 8px;
|
||||
display: flex;
|
||||
flex-direction: column;
|
||||
gap: 8px;
|
||||
border: 1px solid var(--dsw-alias-border-inverted);
|
||||
border-radius: 12px;
|
||||
background: var(--dsw-specific-menu);
|
||||
box-shadow: var(--dsw-shadow-lv3);
|
||||
--dsh-scrollbar-thumb: var(--dsw-alias-scrollbar-bg-l2);
|
||||
--dsh-scrollbar-thumb-hover: var(--dsw-alias-scrollbar-hover-l2);
|
||||
}
|
||||
|
||||
.noteInput {
|
||||
width: 260px;
|
||||
width: 100%;
|
||||
box-sizing: border-box;
|
||||
padding: 6px 8px;
|
||||
border: 1px solid var(--dsw-alias-border-secondary);
|
||||
border: 1px solid var(--dsw-alias-border-l2);
|
||||
border-radius: 8px;
|
||||
background: var(--dsw-alias-bg-primary);
|
||||
background: var(--dsw-alias-bg-layer-1);
|
||||
color: var(--dsw-alias-label-primary);
|
||||
font: inherit;
|
||||
font-size: 13px;
|
||||
resize: vertical;
|
||||
}
|
||||
|
||||
.noteActions {
|
||||
display: flex;
|
||||
justify-content: flex-end;
|
||||
gap: 6px;
|
||||
}
|
||||
|
||||
.noteSave,
|
||||
.noteCancel {
|
||||
height: 28px;
|
||||
@@ -81,8 +113,12 @@
|
||||
}
|
||||
|
||||
.noteSave {
|
||||
background: var(--dsw-alias-interactive-bg-primary);
|
||||
color: var(--dsw-alias-label-inverse);
|
||||
background: var(--dsw-alias-button-primary-fill);
|
||||
color: var(--dsw-alias-label-primary-foreground);
|
||||
}
|
||||
|
||||
.noteSave:hover:not(:disabled) {
|
||||
background: var(--dsw-alias-button-primary-hover);
|
||||
}
|
||||
|
||||
.noteSave:disabled {
|
||||
@@ -104,5 +140,5 @@
|
||||
padding-left: 4px;
|
||||
color: var(--dsw-alias-label-tertiary);
|
||||
font-size: 13px;
|
||||
line-height: 28px;
|
||||
line-height: 20px;
|
||||
}
|
||||
|
||||
@@ -1,23 +1,47 @@
|
||||
/**
|
||||
* Per-message feedback controls: a Like/Dislike pair plus an optional note.
|
||||
* Rendered inside the assistant message's IconActions row, so the buttons
|
||||
* reuse that row's chrome and sit between copy and branch.
|
||||
* The buttons render inside the assistant message's IconActions row, so they
|
||||
* reuse that row's chrome and sit between copy and branch. The note editor is
|
||||
* a popover (portaled to `document.body`) anchored to the note trigger, not an
|
||||
* inline expansion: a 260px textarea plus buttons cannot fit the row at any
|
||||
* viewport, and an inline element pushed the branch action and clock out of the
|
||||
* conversation column. Portaling out of the column also escapes its `overflow`
|
||||
* clip, so the panel cannot be cropped or detached from the message it annotates.
|
||||
* @module @deepseek-ai/dsh-client-ui-message-feedback/client/MessageFeedbackActions
|
||||
*/
|
||||
|
||||
import { useCallback, useEffect, useRef, useState } from 'react'
|
||||
import {
|
||||
IconDislikeOutline16, IconLikeOutline16, Tooltip,
|
||||
useCallback, useEffect, useRef, useState,
|
||||
type CSSProperties,
|
||||
} from 'react'
|
||||
import { createPortal } from 'react-dom'
|
||||
import {
|
||||
IconDislikeOutline16, IconLikeOutline16, Tooltip, useAnchoredPosition,
|
||||
} from '@deepseek-ai/dsh-client-ui-primitives'
|
||||
import type { MessageFeedbackRating } from '@deepseek-ai/dsh-message-feedback/types'
|
||||
import type { MessageFeedbackActionProps } from './slots.ts'
|
||||
import css from './MessageFeedbackActions.module.css'
|
||||
|
||||
/** Safe distance kept between the panel and the viewport edges (the Menu portal margin). */
|
||||
const PANEL_MARGIN = 12
|
||||
|
||||
/** Distance between the trigger's bottom edge and the panel's top. */
|
||||
const PANEL_GAP = 4
|
||||
|
||||
/**
|
||||
* Unplaced portal panel: hidden but laid out so `offsetWidth` is real for the
|
||||
* clamp. The explicit insets match `Menu`'s measure style — a `position: fixed`
|
||||
* element with auto insets otherwise sits at its static position, a different
|
||||
* origin than the one the first placement measures from.
|
||||
*/
|
||||
const MEASURE_STYLE: CSSProperties = { visibility: 'hidden', left: 0, top: 0 }
|
||||
|
||||
/**
|
||||
* One message's feedback controls.
|
||||
* @param props - the owner's message identity, the injected verbs, and the
|
||||
* shared feedback hook.
|
||||
* @returns the rating buttons, plus the note editor while it is open.
|
||||
* @returns the rating buttons and the note trigger, with the note editor
|
||||
* portal-open beneath the trigger while it is open.
|
||||
*/
|
||||
export function MessageFeedbackActions({ messageId, ensure, rate, toggle, clearNote, useFeedback, t }: MessageFeedbackActionProps) {
|
||||
const item = useFeedback(view => view.items.get(messageId))
|
||||
@@ -26,7 +50,15 @@ export function MessageFeedbackActions({ messageId, ensure, rate, toggle, clearN
|
||||
const [noteOpen, setNoteOpen] = useState(false)
|
||||
const [draft, setDraft] = useState('')
|
||||
const [pending, setPending] = useState(false)
|
||||
const [failure, setFailure] = useState<string | null>(null)
|
||||
// A rating or load failure surfaces beside the rating buttons, always legible
|
||||
// whether or not the note popover is open.
|
||||
const [rowFailure, setRowFailure] = useState<string | null>(null)
|
||||
// A note save failure surfaces inside the note popover, where the human is
|
||||
// looking; it stays open so the draft survives to be corrected.
|
||||
const [noteFailure, setNoteFailure] = useState<string | null>(null)
|
||||
const triggerRef = useRef<HTMLButtonElement>(null)
|
||||
const panelRef = useRef<HTMLDivElement>(null)
|
||||
const inputRef = useRef<HTMLTextAreaElement>(null)
|
||||
// The controls mount for every settled message in the transcript, so the
|
||||
// Session's feedback is read once on first hover/focus rather than on mount.
|
||||
const seeded = useRef(false)
|
||||
@@ -39,47 +71,158 @@ export function MessageFeedbackActions({ messageId, ensure, rate, toggle, clearN
|
||||
const alive = useRef(true)
|
||||
useEffect(() => () => { alive.current = false }, [])
|
||||
|
||||
const settle = useCallback((result: { ok: boolean; error?: { code: string } }) => {
|
||||
/** Bumped whenever an editing session ends, so a late save can tell it is stale. */
|
||||
const noteGeneration = useRef(0)
|
||||
|
||||
/** Current panel open-state, readable from a stale closure via a ref. */
|
||||
const noteOpenRef = useRef(false)
|
||||
useEffect(() => { noteOpenRef.current = noteOpen }, [noteOpen])
|
||||
|
||||
const errorCopy = useCallback((result: { ok: boolean; error?: { code: string } }) => {
|
||||
return result.error?.code === 'version-conflict' ? t('error.conflict') : t('error.generic')
|
||||
}, [t])
|
||||
|
||||
const settleRating = useCallback((result: { ok: boolean; error?: { code: string } }) => {
|
||||
if (!alive.current) return
|
||||
setPending(false)
|
||||
if (result.ok) {
|
||||
setFailure(null)
|
||||
return
|
||||
}
|
||||
setFailure(result.error?.code === 'version-conflict' ? t('error.conflict') : t('error.generic'))
|
||||
}, [t])
|
||||
setRowFailure(result.ok ? null : errorCopy(result))
|
||||
}, [errorCopy])
|
||||
|
||||
const closeNote = useCallback(() => {
|
||||
// Ends the editing session, so any save still in flight becomes stale.
|
||||
noteGeneration.current += 1
|
||||
setNoteOpen(false)
|
||||
}, [])
|
||||
|
||||
const onRate = useCallback((next: MessageFeedbackRating) => {
|
||||
setPending(true)
|
||||
setFailure(null)
|
||||
setRowFailure(null)
|
||||
// The controller decides retract-vs-replace from the committed item, so a
|
||||
// click that lands before the first list read still toggles the stored
|
||||
// value instead of this render's empty view.
|
||||
setNoteOpen(false)
|
||||
void toggle(messageId, next).then(settle)
|
||||
}, [messageId, settle, toggle])
|
||||
closeNote()
|
||||
void toggle(messageId, next).then(settleRating)
|
||||
}, [closeNote, messageId, settleRating, toggle])
|
||||
|
||||
// The rating is a parameter because only the note editor's render site can
|
||||
// prove one is recorded; that removes an unreachable undefined guard here.
|
||||
const onSaveNote = useCallback((current: MessageFeedbackRating) => {
|
||||
const trimmed = draft.trim()
|
||||
setPending(true)
|
||||
setFailure(null)
|
||||
setNoteFailure(null)
|
||||
// A save belongs to the editing session that started it. Closing and
|
||||
// reopening the panel begins a new one, and a late reply from the old
|
||||
// session must not act on it: a stale success would shut the panel the
|
||||
// human just opened, and a stale failure would describe a draft this
|
||||
// session never sent.
|
||||
const generation = noteGeneration.current
|
||||
// What a session reopened before this save commits would be seeded with.
|
||||
const staleSeed = item?.note ?? ''
|
||||
// An emptied editor removes the note explicitly; `rate` alone preserves a
|
||||
// stored note, so it cannot express deletion.
|
||||
const settled = trimmed.length === 0
|
||||
? clearNote(messageId)
|
||||
: rate(messageId, current, trimmed)
|
||||
void settled.then((result) => {
|
||||
settle(result)
|
||||
if (result.ok && alive.current) setNoteOpen(false)
|
||||
if (!alive.current) return
|
||||
// `pending` tracks the request in flight, not the editing session, so it
|
||||
// is released either way; all three of like, dislike and Save read
|
||||
// `disabled={pending}`, and holding it would lock the row until remount.
|
||||
// Releasing it unconditionally is safe because those three are the only
|
||||
// mutation entries and each is gated by it, so at most one request is ever
|
||||
// in flight. A future entry that bypasses the gate would have to bind
|
||||
// `pending` to the generation instead of clearing it here.
|
||||
setPending(false)
|
||||
if (result.ok) {
|
||||
// Only the session that is still open may act on a success: closing it
|
||||
// already discarded the draft, and reopening seeded a new one.
|
||||
if (generation === noteGeneration.current) {
|
||||
setNoteFailure(null)
|
||||
setNoteOpen(false)
|
||||
return
|
||||
}
|
||||
// A newer session is open, seeded from the note as it read before this
|
||||
// save committed. Resync it so the editor shows what is stored and the
|
||||
// next save cannot overwrite the text that just landed. An edited draft
|
||||
// is the human's, so it is left alone.
|
||||
setDraft(draftNow => (draftNow === staleSeed ? trimmed : draftNow))
|
||||
return
|
||||
}
|
||||
// A failure from the session still on screen belongs in its panel. One
|
||||
// from an abandoned session is reported only when no new session has
|
||||
// taken over: the row then carries it, so a save that failed after the
|
||||
// human walked away is not silently dropped. Writing it into a reopened
|
||||
// panel instead would label the new draft with the old attempt's error.
|
||||
// `noteOpenRef` — not the `noteOpen` this closure was created from — is
|
||||
// read here, because a close+reopen between the save and resolution
|
||||
// leaves this closure with the panel state from when the save started.
|
||||
if (generation === noteGeneration.current || !noteOpenRef.current) {
|
||||
setNoteFailure(errorCopy(result))
|
||||
}
|
||||
})
|
||||
}, [clearNote, draft, messageId, rate, settle])
|
||||
}, [clearNote, draft, errorCopy, item?.note, messageId, noteOpenRef, rate])
|
||||
|
||||
const openNote = useCallback(() => {
|
||||
// The trigger toggles: while closed it opens the popover (seeding the draft
|
||||
// with the recorded note), while open it closes it. Toggling closed via the
|
||||
// trigger also fires the outside/within logic correctly because the trigger
|
||||
// is inside the panel's "inside" region.
|
||||
const toggleNote = useCallback(() => {
|
||||
if (noteOpen) {
|
||||
closeNote()
|
||||
return
|
||||
}
|
||||
setDraft(item?.note ?? '')
|
||||
// A note-save failure belongs to the editing session that produced it. The
|
||||
// panel stays open on failure so the draft can be corrected, but once it is
|
||||
// closed and reopened the draft is reseeded from the stored note, so a
|
||||
// carried-over error would describe an attempt the new draft never made.
|
||||
// A failure that arrives after the panel closed is reported in the row, and
|
||||
// clearing it here is what retires that notice when a new session starts.
|
||||
setNoteFailure(null)
|
||||
setNoteOpen(true)
|
||||
}, [item?.note])
|
||||
}, [noteOpen, closeNote, item?.note])
|
||||
|
||||
// Place the portaled panel from the trigger rect before paint and keep it
|
||||
// with the trigger on scroll/resize, the same anchoring `Menu` uses for its
|
||||
// portal mode.
|
||||
const pos = useAnchoredPosition({
|
||||
open: noteOpen,
|
||||
anchorRef: triggerRef,
|
||||
panelRef,
|
||||
gap: PANEL_GAP,
|
||||
margin: PANEL_MARGIN,
|
||||
})
|
||||
|
||||
// Focus the input and close on Escape or outside pointer-down while open.
|
||||
useEffect(() => {
|
||||
if (!noteOpen) return
|
||||
inputRef.current?.focus()
|
||||
const onPointerDown = (e: PointerEvent) => {
|
||||
if (!(e.target instanceof Node)) return
|
||||
if (triggerRef.current?.contains(e.target) === true) return
|
||||
if (panelRef.current?.contains(e.target) === true) return
|
||||
closeNote()
|
||||
}
|
||||
const onKeyDown = (e: KeyboardEvent) => {
|
||||
if (e.key === 'Escape') closeNote()
|
||||
}
|
||||
document.addEventListener('pointerdown', onPointerDown)
|
||||
document.addEventListener('keydown', onKeyDown)
|
||||
return () => {
|
||||
document.removeEventListener('pointerdown', onPointerDown)
|
||||
document.removeEventListener('keydown', onKeyDown)
|
||||
}
|
||||
}, [noteOpen, closeNote])
|
||||
|
||||
// Return focus to the trigger only when the panel actually closes, not on the
|
||||
// initial mount (a freshly rendered message with a recorded rating must not
|
||||
// pull focus into its action row).
|
||||
const wasOpen = useRef(false)
|
||||
useEffect(() => {
|
||||
if (noteOpen) { wasOpen.current = true; return }
|
||||
if (wasOpen.current) triggerRef.current?.focus()
|
||||
wasOpen.current = false
|
||||
}, [noteOpen])
|
||||
|
||||
const likeLabel = rating === 'positive' ? t('action.likeActive') : t('action.like')
|
||||
const dislikeLabel = rating === 'negative' ? t('action.dislikeActive') : t('action.dislike')
|
||||
@@ -116,38 +259,66 @@ export function MessageFeedbackActions({ messageId, ensure, rate, toggle, clearN
|
||||
<IconDislikeOutline16 />
|
||||
</button>
|
||||
</Tooltip>
|
||||
{rating !== undefined && !noteOpen && (
|
||||
<button type="button" className={css.noteOpen} onClick={openNote}>
|
||||
{rating !== undefined && (
|
||||
<button
|
||||
ref={triggerRef}
|
||||
type="button"
|
||||
className={css.noteOpen}
|
||||
aria-haspopup="dialog"
|
||||
aria-expanded={noteOpen}
|
||||
onClick={toggleNote}
|
||||
>
|
||||
{item?.note === undefined ? t('note.open') : item.note}
|
||||
</button>
|
||||
)}
|
||||
{rating !== undefined && noteOpen && (
|
||||
<span className={css.noteEditor}>
|
||||
{rowFailure === null && loadFailed && (
|
||||
<span className={css.failure} role="status">{t('error.load')}</span>
|
||||
)}
|
||||
{rowFailure !== null && <span className={css.failure} role="status">{rowFailure}</span>}
|
||||
{/* A note-save failure normally lives inside the panel, beside the buttons
|
||||
that produced it. Whenever the panel is not on screen it falls back to
|
||||
the row instead: the rating may have disappeared underneath an open
|
||||
editor (another client retracts the feedback, a `version-conflict`
|
||||
reply commits `current: null`, the item goes away), or the human may
|
||||
have closed the panel before a slow save came back. Either way the row
|
||||
reports that the save did not land rather than dropping it. */}
|
||||
{!(rating !== undefined && noteOpen) && noteFailure !== null && (
|
||||
<span className={css.failure} role="status">{noteFailure}</span>
|
||||
)}
|
||||
{rating !== undefined && noteOpen && createPortal(
|
||||
<div
|
||||
ref={panelRef}
|
||||
className={css.notePanel}
|
||||
role="dialog"
|
||||
aria-label={t('note.dialog')}
|
||||
style={pos ?? MEASURE_STYLE}
|
||||
>
|
||||
<textarea
|
||||
ref={inputRef}
|
||||
className={css.noteInput}
|
||||
aria-label={t('note.aria')}
|
||||
placeholder={t('note.placeholder')}
|
||||
value={draft}
|
||||
rows={2}
|
||||
rows={3}
|
||||
onChange={(event) => { setDraft(event.target.value) }}
|
||||
/>
|
||||
<button
|
||||
type="button"
|
||||
className={css.noteSave}
|
||||
disabled={pending}
|
||||
onClick={() => { onSaveNote(rating) }}
|
||||
>
|
||||
{t('note.save')}
|
||||
</button>
|
||||
<button type="button" className={css.noteCancel} onClick={() => { setNoteOpen(false) }}>
|
||||
{t('note.cancel')}
|
||||
</button>
|
||||
</span>
|
||||
<div className={css.noteActions}>
|
||||
<button
|
||||
type="button"
|
||||
className={css.noteSave}
|
||||
disabled={pending}
|
||||
onClick={() => { onSaveNote(rating) }}
|
||||
>
|
||||
{t('note.save')}
|
||||
</button>
|
||||
<button type="button" className={css.noteCancel} onClick={closeNote}>
|
||||
{t('note.cancel')}
|
||||
</button>
|
||||
</div>
|
||||
{noteFailure !== null && <span className={css.failure} role="status">{noteFailure}</span>}
|
||||
</div>,
|
||||
document.body,
|
||||
)}
|
||||
{failure === null && loadFailed && (
|
||||
<span className={css.failure} role="status">{t('error.load')}</span>
|
||||
)}
|
||||
{failure !== null && <span className={css.failure} role="status">{failure}</span>}
|
||||
</>
|
||||
)
|
||||
}
|
||||
|
||||
@@ -7,6 +7,7 @@ export const zh = {
|
||||
'action.dislike': '有问题的回答',
|
||||
'action.dislikeActive': '取消标记',
|
||||
'note.open': '补充说明',
|
||||
'note.dialog': '反馈',
|
||||
'note.placeholder': '这条回答哪里好，或哪里有问题？（可选）',
|
||||
'note.save': '保存',
|
||||
'note.cancel': '取消',
|
||||
@@ -33,6 +34,7 @@ export const en = {
|
||||
'action.dislike': 'Bad response',
|
||||
'action.dislikeActive': 'Remove rating',
|
||||
'note.open': 'Add a note',
|
||||
'note.dialog': 'Feedback',
|
||||
'note.placeholder': 'What was good, or what went wrong? (optional)',
|
||||
'note.save': 'Save',
|
||||
'note.cancel': 'Cancel',
|
||||
|
||||
+491
-1
@@ -8,7 +8,7 @@
|
||||
*/
|
||||
import { useSyncExternalStore } from 'react'
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest'
|
||||
import { cleanup, fireEvent, render, waitFor } from '@testing-library/react'
|
||||
import { act, cleanup, fireEvent, render, waitFor } from '@testing-library/react'
|
||||
import { makeTranslate } from '@deepseek-ai/dsh-client-test-runtime'
|
||||
import { zh as commonZh } from '@deepseek-ai/dsh-client-locale/src/locales/zh.ts'
|
||||
import type { MessageId } from '@deepseek-ai/dsh-client-connection/client'
|
||||
@@ -246,4 +246,494 @@ describe('MessageFeedbackActions', () => {
|
||||
await waitFor(() => { expect(ui.getByText(zh['error.generic'])).toBeTruthy() })
|
||||
expect(ui.queryByText(zh['error.load'])).toBeNull()
|
||||
})
|
||||
|
||||
it('portals the note editor to the document body, not into the actions row', () => {
|
||||
const ui = mount({ current: item({ rating: 'positive' }) })
|
||||
|
||||
fireEvent.click(ui.getByText(zh['note.open']))
|
||||
|
||||
// The editor must float above the transcript (escaping the conversation
|
||||
// column's overflow clip), so it renders through a portal to document.body
|
||||
// rather than inline inside the component's own container.
|
||||
const panel = ui.getByRole('dialog')
|
||||
expect(panel).toBeTruthy()
|
||||
expect(ui.container.querySelector('[role="dialog"]')).toBeNull()
|
||||
expect(document.body.contains(panel)).toBe(true)
|
||||
})
|
||||
|
||||
it('closes the note popover on Escape', () => {
|
||||
const ui = mount({ current: item({ rating: 'positive' }) })
|
||||
|
||||
fireEvent.click(ui.getByText(zh['note.open']))
|
||||
expect(ui.getByRole('dialog')).toBeTruthy()
|
||||
|
||||
fireEvent.keyDown(document, { key: 'Escape' })
|
||||
|
||||
expect(ui.queryByRole('dialog')).toBeNull()
|
||||
})
|
||||
|
||||
it('closes the note popover on an outside pointer-down', () => {
|
||||
const ui = mount({ current: item({ rating: 'positive' }) })
|
||||
|
||||
fireEvent.click(ui.getByText(zh['note.open']))
|
||||
expect(ui.getByRole('dialog')).toBeTruthy()
|
||||
|
||||
fireEvent.pointerDown(document.body)
|
||||
|
||||
expect(ui.queryByRole('dialog')).toBeNull()
|
||||
})
|
||||
|
||||
it('keeps the note popover open on a pointer-down inside it', () => {
|
||||
const ui = mount({ current: item({ rating: 'positive' }) })
|
||||
|
||||
fireEvent.click(ui.getByText(zh['note.open']))
|
||||
const panel = ui.getByRole('dialog')
|
||||
expect(panel).toBeTruthy()
|
||||
|
||||
fireEvent.pointerDown(panel)
|
||||
|
||||
expect(ui.getByRole('dialog')).toBeTruthy()
|
||||
})
|
||||
|
||||
it('does not close the note popover on a pointer-down on its trigger', () => {
|
||||
const ui = mount({ current: item({ rating: 'positive' }) })
|
||||
|
||||
fireEvent.click(ui.getByText(zh['note.open']))
|
||||
expect(ui.getByRole('dialog')).toBeTruthy()
|
||||
|
||||
// The trigger is inside the panel's own region, so pressing it must not be
|
||||
// treated as an outside click; the toggle click below then closes it.
|
||||
fireEvent.pointerDown(ui.getByText(zh['note.open']))
|
||||
expect(ui.getByRole('dialog')).toBeTruthy()
|
||||
})
|
||||
|
||||
it('toggles the note popover closed and open from its trigger', () => {
|
||||
const ui = mount({ current: item({ rating: 'positive' }) })
|
||||
|
||||
fireEvent.click(ui.getByText(zh['note.open']))
|
||||
expect(ui.getByRole('dialog')).toBeTruthy()
|
||||
expect(ui.getByLabelText(zh['note.aria'])).toBeTruthy()
|
||||
|
||||
fireEvent.click(ui.getByText(zh['note.open']))
|
||||
expect(ui.queryByRole('dialog')).toBeNull()
|
||||
|
||||
fireEvent.click(ui.getByText(zh['note.open']))
|
||||
expect(ui.getByRole('dialog')).toBeTruthy()
|
||||
})
|
||||
|
||||
it('ignores keys other than Escape while the popover is open', () => {
|
||||
const ui = mount({ current: item({ rating: 'positive' }) })
|
||||
|
||||
fireEvent.click(ui.getByText(zh['note.open']))
|
||||
expect(ui.getByRole('dialog')).toBeTruthy()
|
||||
|
||||
fireEvent.keyDown(document, { key: 'Enter' })
|
||||
|
||||
expect(ui.getByRole('dialog')).toBeTruthy()
|
||||
})
|
||||
|
||||
it('publishes no rating-state after the row unmounts mid-flight', async () => {
|
||||
// Directly exercise the early-return of a rating settle once the control has
|
||||
// unmounted: the promise resolution must not touch React state.
|
||||
let release = (): void => {}
|
||||
const gate = new Promise<MessageFeedbackActionResult>((resolve) => {
|
||||
release = () => { resolve({ ok: true as const }) }
|
||||
})
|
||||
const view: MessageFeedbackView = { status: 'ready', items: new Map(), error: null }
|
||||
const useFeedback = (<T,>(select: (v: MessageFeedbackView) => T): T =>
|
||||
useSyncExternalStore(() => () => {}, () => select(view))) as never
|
||||
const props = {
|
||||
messageId: MSG,
|
||||
ensure: vi.fn(() => Promise.resolve<MessageFeedbackActionResult>({ ok: true })),
|
||||
rate: vi.fn(() => Promise.resolve<MessageFeedbackActionResult>({ ok: true })),
|
||||
toggle: vi.fn(() => gate),
|
||||
clearNote: vi.fn(() => Promise.resolve<MessageFeedbackActionResult>({ ok: true })),
|
||||
clear: vi.fn(() => Promise.resolve<MessageFeedbackActionResult>({ ok: true })),
|
||||
useFeedback,
|
||||
t,
|
||||
} as unknown as Parameters<typeof MessageFeedbackActions>[0]
|
||||
const ui = render(<MessageFeedbackActions {...props} />)
|
||||
const errors: unknown[] = []
|
||||
const onError = (event: ErrorEvent): void => { errors.push(event.error) }
|
||||
window.addEventListener('error', onError)
|
||||
|
||||
fireEvent.click(ui.getByLabelText(zh['action.like']))
|
||||
ui.unmount()
|
||||
release()
|
||||
await gate
|
||||
|
||||
window.removeEventListener('error', onError)
|
||||
expect(errors).toEqual([])
|
||||
})
|
||||
|
||||
it('publishes no note-state after the row unmounts mid-save', async () => {
|
||||
// Same unmount early-return for the note-save settle path: resolving the
|
||||
// save promise after unmount must not touch React state.
|
||||
let release = (): void => {}
|
||||
const gate = new Promise<MessageFeedbackActionResult>((resolve) => {
|
||||
release = () => { resolve({ ok: true as const }) }
|
||||
})
|
||||
const view: MessageFeedbackView = { status: 'ready', items: new Map([[MSG, item({ rating: 'positive' })]]), error: null }
|
||||
const useFeedback = (<T,>(select: (v: MessageFeedbackView) => T): T =>
|
||||
useSyncExternalStore(() => () => {}, () => select(view))) as never
|
||||
const props = {
|
||||
messageId: MSG,
|
||||
ensure: vi.fn(() => Promise.resolve<MessageFeedbackActionResult>({ ok: true })),
|
||||
rate: vi.fn(() => gate),
|
||||
toggle: vi.fn(() => Promise.resolve<MessageFeedbackActionResult>({ ok: true })),
|
||||
clearNote: vi.fn(() => Promise.resolve<MessageFeedbackActionResult>({ ok: true })),
|
||||
clear: vi.fn(() => Promise.resolve<MessageFeedbackActionResult>({ ok: true })),
|
||||
useFeedback,
|
||||
t,
|
||||
} as unknown as Parameters<typeof MessageFeedbackActions>[0]
|
||||
const errors: unknown[] = []
|
||||
const onError = (event: ErrorEvent): void => { errors.push(event.error) }
|
||||
window.addEventListener('error', onError)
|
||||
|
||||
const ui = render(<MessageFeedbackActions {...props} />)
|
||||
fireEvent.click(ui.getByText(zh['note.open']))
|
||||
fireEvent.change(ui.getByLabelText(zh['note.aria']), { target: { value: 'hi' } })
|
||||
fireEvent.click(ui.getByText(zh['note.save']))
|
||||
ui.unmount()
|
||||
release()
|
||||
await gate
|
||||
|
||||
window.removeEventListener('error', onError)
|
||||
expect(errors).toEqual([])
|
||||
})
|
||||
|
||||
it('ignores a pointer-down whose target is not a DOM node', () => {
|
||||
const ui = mount({ current: item({ rating: 'positive' }) })
|
||||
|
||||
fireEvent.click(ui.getByText(zh['note.open']))
|
||||
expect(ui.getByRole('dialog')).toBeTruthy()
|
||||
|
||||
// The outside-click guard returns without closing when the event target is
|
||||
// not a DOM node. `document.dispatchEvent` delivers straight to the
|
||||
// document listener, and a non-Node target is not `instanceof Node`.
|
||||
const event = new MouseEvent('pointerdown', { bubbles: true })
|
||||
Object.defineProperty(event, 'target', { configurable: true, value: { notANode: true } })
|
||||
document.dispatchEvent(event)
|
||||
|
||||
expect(ui.getByRole('dialog')).toBeTruthy()
|
||||
})
|
||||
|
||||
it('returns focus to the trigger when the popover closes', () => {
|
||||
const ui = mount({ current: item({ rating: 'positive' }) })
|
||||
const trigger = ui.getByText(zh['note.open'])
|
||||
|
||||
fireEvent.click(trigger)
|
||||
expect(ui.getByRole('dialog')).toBeTruthy()
|
||||
fireEvent.keyDown(document, { key: 'Escape' })
|
||||
|
||||
// Closing hands focus back, so a keyboard user resumes on the row they
|
||||
// came from rather than at the document root.
|
||||
expect(ui.queryByRole('dialog')).toBeNull()
|
||||
expect(document.activeElement).toBe(trigger)
|
||||
})
|
||||
|
||||
it('does not pull focus when an already-rated message mounts', () => {
|
||||
// The `wasOpen` guard exists for this: a transcript of already-rated
|
||||
// messages must not drag focus into an action row as each one mounts.
|
||||
// Only a real open-then-close returns focus.
|
||||
const elsewhere = document.createElement('button')
|
||||
document.body.append(elsewhere)
|
||||
elsewhere.focus()
|
||||
|
||||
mount({ current: item({ rating: 'positive' }) })
|
||||
|
||||
expect(document.activeElement).toBe(elsewhere)
|
||||
elsewhere.remove()
|
||||
})
|
||||
|
||||
it('drops a stale save failure when the popover is reopened', async () => {
|
||||
// The failure belongs to the editing session that produced it: reopening
|
||||
// reseeds the draft from the stored note, so a carried-over error would
|
||||
// describe an attempt the new draft never made.
|
||||
const ui = mount({
|
||||
current: item({ rating: 'positive' }),
|
||||
rateResult: { ok: false, error: { code: 'note-too-large', message: 'too long' } },
|
||||
})
|
||||
|
||||
fireEvent.click(ui.getByText(zh['note.open']))
|
||||
fireEvent.change(ui.getByLabelText(zh['note.aria']), { target: { value: 'x'.repeat(20) } })
|
||||
fireEvent.click(ui.getByText(zh['note.save']))
|
||||
await waitFor(() => { expect(ui.getByText(zh['error.generic'])).toBeTruthy() })
|
||||
|
||||
fireEvent.keyDown(document, { key: 'Escape' })
|
||||
fireEvent.click(ui.getByText(zh['note.open']))
|
||||
|
||||
expect(ui.queryByText(zh['error.generic'])).toBeNull()
|
||||
})
|
||||
|
||||
it('keeps a save failure visible when the rating disappears underneath it', async () => {
|
||||
// Another client retracts the feedback while the editor is open: the
|
||||
// controller commits `current: null`, the item goes away, and the panel
|
||||
// unmounts. The failure must not vanish with it, so it falls back to the row.
|
||||
let release = (): void => {}
|
||||
const gate = new Promise<MessageFeedbackActionResult>((resolve) => {
|
||||
release = () => { resolve({ ok: false, error: { code: 'target-not-found', message: 'gone' } }) }
|
||||
})
|
||||
const view: MessageFeedbackView = {
|
||||
status: 'ready',
|
||||
items: new Map([[MSG, item({ rating: 'positive' })]]),
|
||||
error: null,
|
||||
}
|
||||
let notify = (): void => {}
|
||||
const useFeedback = (<T,>(select: (v: MessageFeedbackView) => T): T =>
|
||||
useSyncExternalStore((cb) => { notify = cb; return () => {} }, () => select(view))) as never
|
||||
const props = {
|
||||
messageId: MSG,
|
||||
ensure: vi.fn(() => Promise.resolve<MessageFeedbackActionResult>({ ok: true })),
|
||||
rate: vi.fn(() => gate),
|
||||
toggle: vi.fn(() => Promise.resolve<MessageFeedbackActionResult>({ ok: true })),
|
||||
clearNote: vi.fn(() => Promise.resolve<MessageFeedbackActionResult>({ ok: true })),
|
||||
clear: vi.fn(() => Promise.resolve<MessageFeedbackActionResult>({ ok: true })),
|
||||
useFeedback,
|
||||
t,
|
||||
} as unknown as Parameters<typeof MessageFeedbackActions>[0]
|
||||
const ui = render(<MessageFeedbackActions {...props} />)
|
||||
|
||||
fireEvent.click(ui.getByText(zh['note.open']))
|
||||
fireEvent.change(ui.getByLabelText(zh['note.aria']), { target: { value: 'hi' } })
|
||||
fireEvent.click(ui.getByText(zh['note.save']))
|
||||
|
||||
// The retract lands first, then the save rejects.
|
||||
view.items = new Map()
|
||||
notify()
|
||||
release()
|
||||
await gate
|
||||
|
||||
await waitFor(() => { expect(ui.getByText(zh['error.generic'])).toBeTruthy() })
|
||||
expect(ui.queryByRole('dialog')).toBeNull()
|
||||
})
|
||||
|
||||
it('ignores a save that resolves after its editing session ended', async () => {
|
||||
// Closing and reopening starts a new session. A late success from the old
|
||||
// one must not shut the panel the human just opened.
|
||||
let release = (): void => {}
|
||||
const gate = new Promise<MessageFeedbackActionResult>((resolve) => {
|
||||
release = () => { resolve({ ok: true as const }) }
|
||||
})
|
||||
const view: MessageFeedbackView = {
|
||||
status: 'ready',
|
||||
items: new Map([[MSG, item({ rating: 'positive' })]]),
|
||||
error: null,
|
||||
}
|
||||
const useFeedback = (<T,>(select: (v: MessageFeedbackView) => T): T =>
|
||||
useSyncExternalStore(() => () => {}, () => select(view))) as never
|
||||
const props = {
|
||||
messageId: MSG,
|
||||
ensure: vi.fn(() => Promise.resolve<MessageFeedbackActionResult>({ ok: true })),
|
||||
rate: vi.fn(() => gate),
|
||||
toggle: vi.fn(() => Promise.resolve<MessageFeedbackActionResult>({ ok: true })),
|
||||
clearNote: vi.fn(() => Promise.resolve<MessageFeedbackActionResult>({ ok: true })),
|
||||
clear: vi.fn(() => Promise.resolve<MessageFeedbackActionResult>({ ok: true })),
|
||||
useFeedback,
|
||||
t,
|
||||
} as unknown as Parameters<typeof MessageFeedbackActions>[0]
|
||||
const ui = render(<MessageFeedbackActions {...props} />)
|
||||
|
||||
fireEvent.click(ui.getByText(zh['note.open']))
|
||||
fireEvent.change(ui.getByLabelText(zh['note.aria']), { target: { value: 'first' } })
|
||||
fireEvent.click(ui.getByText(zh['note.save']))
|
||||
|
||||
// Abandon that session and start another before the save lands.
|
||||
fireEvent.keyDown(document, { key: 'Escape' })
|
||||
fireEvent.click(ui.getByText(zh['note.open']))
|
||||
expect(ui.getByRole('dialog')).toBeTruthy()
|
||||
|
||||
release()
|
||||
await gate
|
||||
// Flush the `.then` continuation and the render it would cause. Asserted
|
||||
// directly rather than through `waitFor`, which would retry past a panel
|
||||
// that the stale result closed.
|
||||
await act(async () => { await Promise.resolve() })
|
||||
|
||||
expect(ui.getByRole('dialog')).toBeTruthy()
|
||||
// The reply is discarded, but the request is no longer in flight, so the
|
||||
// controls must not stay disabled: `pending` gates the rating buttons and
|
||||
// Save, and leaving it set locks this message's row until it remounts.
|
||||
expect(ui.getByLabelText(zh['action.likeActive']).hasAttribute('disabled')).toBe(false)
|
||||
expect(ui.getByLabelText(zh['action.dislike']).hasAttribute('disabled')).toBe(false)
|
||||
expect(ui.getByText(zh['note.save']).hasAttribute('disabled')).toBe(false)
|
||||
})
|
||||
|
||||
it('reports a save that fails after the human closed the panel', async () => {
|
||||
// A slow save that rejects once the panel is gone must not be swallowed:
|
||||
// the human would otherwise believe the note was stored. With no panel to
|
||||
// show it in, the row carries the notice.
|
||||
let release = (): void => {}
|
||||
const gate = new Promise<MessageFeedbackActionResult>((resolve) => {
|
||||
release = () => {
|
||||
resolve({ ok: false, error: { code: 'target-not-found', message: 'gone' } })
|
||||
}
|
||||
})
|
||||
const view: MessageFeedbackView = {
|
||||
status: 'ready',
|
||||
items: new Map([[MSG, item({ rating: 'positive' })]]),
|
||||
error: null,
|
||||
}
|
||||
const useFeedback = (<T,>(select: (v: MessageFeedbackView) => T): T =>
|
||||
useSyncExternalStore(() => () => {}, () => select(view))) as never
|
||||
const props = {
|
||||
messageId: MSG,
|
||||
ensure: vi.fn(() => Promise.resolve<MessageFeedbackActionResult>({ ok: true })),
|
||||
rate: vi.fn(() => gate),
|
||||
toggle: vi.fn(() => Promise.resolve<MessageFeedbackActionResult>({ ok: true })),
|
||||
clearNote: vi.fn(() => Promise.resolve<MessageFeedbackActionResult>({ ok: true })),
|
||||
clear: vi.fn(() => Promise.resolve<MessageFeedbackActionResult>({ ok: true })),
|
||||
useFeedback,
|
||||
t,
|
||||
} as unknown as Parameters<typeof MessageFeedbackActions>[0]
|
||||
const ui = render(<MessageFeedbackActions {...props} />)
|
||||
|
||||
fireEvent.click(ui.getByText(zh['note.open']))
|
||||
fireEvent.change(ui.getByLabelText(zh['note.aria']), { target: { value: 'hi' } })
|
||||
fireEvent.click(ui.getByText(zh['note.save']))
|
||||
|
||||
// Walk away before the reply lands, and leave it closed.
|
||||
fireEvent.keyDown(document, { key: 'Escape' })
|
||||
expect(ui.queryByRole('dialog')).toBeNull()
|
||||
|
||||
release()
|
||||
await gate
|
||||
await act(async () => { await Promise.resolve() })
|
||||
|
||||
expect(ui.getByText(zh['error.generic'])).toBeTruthy()
|
||||
})
|
||||
|
||||
it('does not write an abandoned session\'s failure into a reopened panel', async () => {
|
||||
// The old request rejects after the panel was closed and reopened, so the
|
||||
// new session owns the panel. Its draft was not the one that failed, so the
|
||||
// stale error must not be shown there; it belongs to the abandoned session.
|
||||
let release = (): void => {}
|
||||
const gate = new Promise<MessageFeedbackActionResult>((resolve) => {
|
||||
release = () => {
|
||||
resolve({ ok: false, error: { code: 'target-not-found', message: 'gone' } })
|
||||
}
|
||||
})
|
||||
const view: MessageFeedbackView = {
|
||||
status: 'ready',
|
||||
items: new Map([[MSG, item({ rating: 'positive' })]]),
|
||||
error: null,
|
||||
}
|
||||
const useFeedback = (<T,>(select: (v: MessageFeedbackView) => T): T =>
|
||||
useSyncExternalStore(() => () => {}, () => select(view))) as never
|
||||
const props = {
|
||||
messageId: MSG,
|
||||
ensure: vi.fn(() => Promise.resolve<MessageFeedbackActionResult>({ ok: true })),
|
||||
rate: vi.fn(() => gate),
|
||||
toggle: vi.fn(() => Promise.resolve<MessageFeedbackActionResult>({ ok: true })),
|
||||
clearNote: vi.fn(() => Promise.resolve<MessageFeedbackActionResult>({ ok: true })),
|
||||
clear: vi.fn(() => Promise.resolve<MessageFeedbackActionResult>({ ok: true })),
|
||||
useFeedback,
|
||||
t,
|
||||
} as unknown as Parameters<typeof MessageFeedbackActions>[0]
|
||||
const ui = render(<MessageFeedbackActions {...props} />)
|
||||
|
||||
fireEvent.click(ui.getByText(zh['note.open']))
|
||||
fireEvent.change(ui.getByLabelText(zh['note.aria']), { target: { value: 'first' } })
|
||||
fireEvent.click(ui.getByText(zh['note.save']))
|
||||
|
||||
// Abandon that session and start another before the save rejects; unlike
|
||||
// the closed-and-left case, a new panel is now on screen.
|
||||
fireEvent.keyDown(document, { key: 'Escape' })
|
||||
fireEvent.click(ui.getByText(zh['note.open']))
|
||||
expect(ui.getByRole('dialog')).toBeTruthy()
|
||||
|
||||
release()
|
||||
await gate
|
||||
await act(async () => { await Promise.resolve() })
|
||||
|
||||
// The stale failure names a draft the new session never sent, so it stays
|
||||
// out of the reopened panel's status area.
|
||||
expect(ui.queryByText(zh['error.generic'])).toBeNull()
|
||||
expect(ui.getByRole('dialog')).toBeTruthy()
|
||||
})
|
||||
|
||||
it('resyncs an untouched reopened draft to the note that just committed', async () => {
|
||||
// The reopened session seeded from the note as it read before the save
|
||||
// committed, so an untouched draft would show stale text and the next save
|
||||
// could overwrite what just landed.
|
||||
let release = (): void => {}
|
||||
const gate = new Promise<MessageFeedbackActionResult>((resolve) => {
|
||||
release = () => { resolve({ ok: true as const }) }
|
||||
})
|
||||
const view: MessageFeedbackView = {
|
||||
status: 'ready',
|
||||
items: new Map([[MSG, item({ rating: 'positive', note: 'old' })]]),
|
||||
error: null,
|
||||
}
|
||||
const useFeedback = (<T,>(select: (v: MessageFeedbackView) => T): T =>
|
||||
useSyncExternalStore(() => () => {}, () => select(view))) as never
|
||||
const props = {
|
||||
messageId: MSG,
|
||||
ensure: vi.fn(() => Promise.resolve<MessageFeedbackActionResult>({ ok: true })),
|
||||
rate: vi.fn(() => gate),
|
||||
toggle: vi.fn(() => Promise.resolve<MessageFeedbackActionResult>({ ok: true })),
|
||||
clearNote: vi.fn(() => Promise.resolve<MessageFeedbackActionResult>({ ok: true })),
|
||||
clear: vi.fn(() => Promise.resolve<MessageFeedbackActionResult>({ ok: true })),
|
||||
useFeedback,
|
||||
t,
|
||||
} as unknown as Parameters<typeof MessageFeedbackActions>[0]
|
||||
const ui = render(<MessageFeedbackActions {...props} />)
|
||||
|
||||
fireEvent.click(ui.getByText('old'))
|
||||
fireEvent.change(ui.getByLabelText(zh['note.aria']), { target: { value: 'saved text' } })
|
||||
fireEvent.click(ui.getByText(zh['note.save']))
|
||||
|
||||
// Close and reopen before the save lands: the new draft is seeded from the
|
||||
// still-stale stored note.
|
||||
fireEvent.keyDown(document, { key: 'Escape' })
|
||||
fireEvent.click(ui.getByText('old'))
|
||||
expect((ui.getByLabelText(zh['note.aria']) as HTMLTextAreaElement).value).toBe('old')
|
||||
|
||||
release()
|
||||
await gate
|
||||
await act(async () => { await Promise.resolve() })
|
||||
|
||||
expect((ui.getByLabelText(zh['note.aria']) as HTMLTextAreaElement).value).toBe('saved text')
|
||||
})
|
||||
|
||||
it('leaves a reopened draft alone once the human has edited it', async () => {
|
||||
// The opposite arm: an edited draft belongs to the human, so a late save
|
||||
// must not overwrite what they are typing.
|
||||
let release = (): void => {}
|
||||
const gate = new Promise<MessageFeedbackActionResult>((resolve) => {
|
||||
release = () => { resolve({ ok: true as const }) }
|
||||
})
|
||||
const view: MessageFeedbackView = {
|
||||
status: 'ready',
|
||||
items: new Map([[MSG, item({ rating: 'positive', note: 'old' })]]),
|
||||
error: null,
|
||||
}
|
||||
const useFeedback = (<T,>(select: (v: MessageFeedbackView) => T): T =>
|
||||
useSyncExternalStore(() => () => {}, () => select(view))) as never
|
||||
const props = {
|
||||
messageId: MSG,
|
||||
ensure: vi.fn(() => Promise.resolve<MessageFeedbackActionResult>({ ok: true })),
|
||||
rate: vi.fn(() => gate),
|
||||
toggle: vi.fn(() => Promise.resolve<MessageFeedbackActionResult>({ ok: true })),
|
||||
clearNote: vi.fn(() => Promise.resolve<MessageFeedbackActionResult>({ ok: true })),
|
||||
clear: vi.fn(() => Promise.resolve<MessageFeedbackActionResult>({ ok: true })),
|
||||
useFeedback,
|
||||
t,
|
||||
} as unknown as Parameters<typeof MessageFeedbackActions>[0]
|
||||
const ui = render(<MessageFeedbackActions {...props} />)
|
||||
|
||||
fireEvent.click(ui.getByText('old'))
|
||||
fireEvent.change(ui.getByLabelText(zh['note.aria']), { target: { value: 'saved text' } })
|
||||
fireEvent.click(ui.getByText(zh['note.save']))
|
||||
|
||||
fireEvent.keyDown(document, { key: 'Escape' })
|
||||
fireEvent.click(ui.getByText('old'))
|
||||
fireEvent.change(ui.getByLabelText(zh['note.aria']), { target: { value: 'my new words' } })
|
||||
|
||||
release()
|
||||
await gate
|
||||
await act(async () => { await Promise.resolve() })
|
||||
|
||||
expect((ui.getByLabelText(zh['note.aria']) as HTMLTextAreaElement).value).toBe('my new words')
|
||||
})
|
||||
})
|
||||
|
||||
@@ -0,0 +1,93 @@
|
||||
/**
|
||||
* Feedback controls stylesheet contract, asserted against the CSS text on disk.
|
||||
*
|
||||
* A `--dsw-*` name the theme never declares fails silently, and for this sheet
|
||||
* it failed loudly in the product: `border`, `background`, and the primary
|
||||
* button's fill and label each named a token that does not exist, so every one
|
||||
* of those declarations was invalid at computed-value time and dropped. The
|
||||
* note editor shipped with no border and no surface, and its Save button with
|
||||
* neither fill nor readable label. Nothing downstream reports this — the sheet
|
||||
* parses, the classes attach, and the DOM snapshots are unchanged.
|
||||
*
|
||||
* The editor is a popover portaled to `document.body` and fixed-positioned
|
||||
* from the note trigger's rect, so it never enters the IconActions row's flex
|
||||
* layout at all — the row keeps its single 28px line of icons and the note
|
||||
* trigger, and no wrapping (`flex-wrap`) or `order` is needed for it. The
|
||||
* width-independent half of that contract is asserted here (the panel is a
|
||||
* fixed portal, not an inline flex item); the resulting geometry is measured
|
||||
* in a real engine by `apps/web/tests/message-feedback-layout`.
|
||||
*/
|
||||
import { readdirSync, readFileSync } from 'node:fs'
|
||||
import { fileURLToPath } from 'node:url'
|
||||
import { describe, expect, it } from 'vitest'
|
||||
|
||||
const css = readFileSync(
|
||||
fileURLToPath(new URL('../src/client/MessageFeedbackActions.module.css', import.meta.url)),
|
||||
'utf8',
|
||||
)
|
||||
// The theme package maps `./styles/*` to `./src/styles/*`, so the declarations
|
||||
// stay on the source plane rather than needing a build. Every theme sheet, not
|
||||
// just the platform tokens: font and scrollbar variables are declared in
|
||||
// siblings, and a gate reading one file would call their names undeclared.
|
||||
const tokens = readdirSync(fileURLToPath(new URL('../../ui-theme/src/styles/', import.meta.url)))
|
||||
.filter(name => name.endsWith('.css'))
|
||||
.map(name => readFileSync(fileURLToPath(new URL(`../../ui-theme/src/styles/${name}`, import.meta.url)), 'utf8'))
|
||||
.join('\n')
|
||||
|
||||
/**
|
||||
* The declarations of one top-level rule, by selector.
|
||||
* @param selector - the class selector to read, including its leading dot.
|
||||
* @returns the rule's declaration text.
|
||||
*/
|
||||
function block(selector: string): string {
|
||||
const match = new RegExp(`^\\${selector} \\{([^}]*)\\}`, 'm').exec(css)
|
||||
if (match === null) throw new Error(`MessageFeedbackActions.module.css has no \`${selector}\` rule`)
|
||||
return match[1] ?? ''
|
||||
}
|
||||
|
||||
describe('MessageFeedbackActions theme styles', () => {
|
||||
it('names only theme variables the token sheet defines', () => {
|
||||
// The regression that motivated this file. An undeclared custom property
|
||||
// has no fallback and does not inherit a usable value: the entire
|
||||
// declaration is thrown away, so the control renders as if the line had
|
||||
// never been written. Every theme-variable prefix the sheets actually use,
|
||||
// not just `--dsw-`: a `--dsh-` name reads as a plausible sibling and would
|
||||
// otherwise slip past into an invalid declaration.
|
||||
const named = [...css.matchAll(/var\((--(?:dsw|dsh|ds)-[a-z0-9-]+)/g)].map(match => match[1])
|
||||
// Vacuity guard: the sheet has to actually name tokens, or the filter below
|
||||
// is satisfied by an empty list and this test proves nothing.
|
||||
expect(named.length).toBeGreaterThan(5)
|
||||
const undeclared = [...new Set(named)].filter(name => !tokens.includes(` ${String(name)}:`))
|
||||
expect(undeclared).toEqual([])
|
||||
})
|
||||
|
||||
it('never falls back to a literal colour', () => {
|
||||
// A token that resolves is never the problem; an undeclared one takes this
|
||||
// branch, and a literal here is a single colour for both themes.
|
||||
expect(css).not.toMatch(/var\(--dsw-[a-z0-9-]+\s*,\s*(?:#|rgb|rgba|hsl|hsla)/)
|
||||
})
|
||||
|
||||
it('keeps the note editor out of the row as a fixed portal, not a flex item', () => {
|
||||
// The editor is a popover portaled to document.body, so the IconActions row
|
||||
// never has to grow or wrap around it. Fixed positioning comes from the
|
||||
// placement code (inline `left`/`top`), not a class, so only `position:
|
||||
// fixed` and the elevated surface live in the sheet — plus the absence of a
|
||||
// flex rule on the panel, which would resurrect the row-overflow defect an
|
||||
// inline editor had. The row stays one 28px line, so a fixed `width` on the
|
||||
// panel is fine (it floats, it does not compete for row space).
|
||||
expect(block('.notePanel')).toMatch(/position:\s*fixed/)
|
||||
// The panel flex-sets its own children (textarea over buttons), which is
|
||||
// fine. What must be absent is the flex-SIZING that made an inline editor a
|
||||
// row item: grow/shrink/basis (or the `flex:` shorthand) would let it rejoin
|
||||
// the IconActions layout, resurrecting the overflow defect.
|
||||
expect(block('.notePanel')).not.toMatch(/flex-(?:grow|shrink|basis)\s*:/)
|
||||
expect(block('.notePanel')).not.toMatch(/^\s*flex\s*:/m)
|
||||
})
|
||||
|
||||
it('closes every block, so no rule is swallowed by the one above it', () => {
|
||||
// A missing `}` is not a parse error: every rule after it silently becomes
|
||||
// part of the block above, and the controls would paint unstyled.
|
||||
const bare = css.replace(/\/\*[\s\S]*?\*\//g, '')
|
||||
expect((bare.match(/\}/g) ?? []).length).toBe((bare.match(/\{/g) ?? []).length)
|
||||
})
|
||||
})
|
||||
@@ -2,5 +2,5 @@
|
||||
# side as of the last confirmed-consistent state. Both languages carry equal authority;
|
||||
# after editing either side, bring the other along and re-record with:
|
||||
# pnpm run verify-translation-pairing --write packages/client/ui-primitives/README.md
|
||||
README.md: 9a5b33e6b2ecae0652d640d2a6927e3f1d05b1a1
|
||||
README.zh.md: a94935235790504bbeb7f5091e8c8fba86e1c903
|
||||
README.md: a675b3cd0aa9e09e243b69065110e1d2b67ff1d9
|
||||
README.zh.md: aa67993ec879635fc0677501665d2e23de996dcf
|
||||
|
||||
@@ -2,7 +2,7 @@
|
||||
|
||||
English | [中文](README.zh.md)
|
||||
|
||||
Pure React atoms (zero cordis): StateDot, DisclosureRow, ic_ds_* icons, Button/Pill/Menu/Modal/Input, the Toast transient banner, the OnboardingSurface first-run takeover (body-portaled mask + opaque stage that holds `#root` inert for exactly its own lifetime), the markdown family (MessageText/MarkdownText/JsonBlock), the read-only JsonTree inspector, the `useAnchoredMaxHeight` hook that clamps a bottom-anchored overlay to the viewport space above its anchor (re-measured on resize, scroll, and a caller-supplied dependency), TerminalBlock, DiffBlock, ReadBlock, SearchBlock, and WebBlock.
|
||||
Pure React atoms (zero cordis): StateDot, DisclosureRow, ic_ds_* icons, Button/Pill/Menu/Modal/Input, the Toast transient banner, the OnboardingSurface first-run takeover (body-portaled mask + opaque stage that holds `#root` inert for exactly its own lifetime), the markdown family (MessageText/MarkdownText/JsonBlock), the read-only JsonTree inspector, the `useAnchoredMaxHeight` hook that clamps a bottom-anchored overlay to the viewport space above its anchor (re-measured on resize, scroll, and a caller-supplied dependency), the `useAnchoredPosition` hook that holds a fixed-position floating panel under its anchor (measure, offset, clamp inside the viewport margin, re-placed on capture-phase scroll, window resize, and the panel's own size changes), TerminalBlock, DiffBlock, ReadBlock, SearchBlock, and WebBlock.
|
||||
|
||||
## Hover cards
|
||||
|
||||
|
||||
@@ -2,7 +2,7 @@
|
||||
|
||||
[English](README.md) | 中文
|
||||
|
||||
纯 React 原子组件（零 cordis）：StateDot、DisclosureRow、ic_ds_* 图标、Button/Pill/Menu/Modal/Input、Toast 短时横幅、OnboardingSurface 首次使用接管层（portal 到 body 的遮罩加不透明展示层，在且仅在自身生命周期内保持 `#root` 为 `inert`）、markdown 家族（MessageText/MarkdownText/JsonBlock）、只读 JsonTree 检查器、`useAnchoredMaxHeight` 钩子（把底部锚定的浮层高度收敛到锚点上方的视口空间，并在 resize、scroll 与调用方提供的依赖变化时重新测量）、TerminalBlock、DiffBlock、ReadBlock、SearchBlock，以及 WebBlock。
|
||||
纯 React 原子组件（零 cordis）：StateDot、DisclosureRow、ic_ds_* 图标、Button/Pill/Menu/Modal/Input、Toast 短时横幅、OnboardingSurface 首次使用接管层（portal 到 body 的遮罩加不透明展示层，在且仅在自身生命周期内保持 `#root` 为 `inert`）、markdown 家族（MessageText/MarkdownText/JsonBlock）、只读 JsonTree 检查器、`useAnchoredMaxHeight` 钩子（把底部锚定的浮层高度收敛到锚点上方的视口空间，并在 resize、scroll 与调用方提供的依赖变化时重新测量）、`useAnchoredPosition` 钩子（让固定定位的浮动面板跟住锚点：测量、偏移、按视口边距钳制，并在捕获阶段滚动、窗口缩放与面板自身尺寸变化时重新定位）、TerminalBlock、DiffBlock、ReadBlock、SearchBlock，以及 WebBlock。
|
||||
|
||||
## 悬浮卡片
|
||||
|
||||
|
||||
@@ -13,6 +13,8 @@ export { Input } from './Input.tsx'
|
||||
export { Menu } from './Menu.tsx'
|
||||
export type { MenuEntry, MenuItem, MenuSeparator, MenuLabel } from './Menu.tsx'
|
||||
export { useAnchoredMaxHeight } from './useAnchoredMaxHeight.ts'
|
||||
export { useAnchoredPosition } from './useAnchoredPosition.ts'
|
||||
export type { AnchoredPositionOptions } from './useAnchoredPosition.ts'
|
||||
export { useDismissOnOutsidePointer } from './useDismissOnOutsidePointer.ts'
|
||||
export { HoverCard } from './HoverCard.tsx'
|
||||
export { Modal } from './Modal.tsx'
|
||||
|
||||
@@ -0,0 +1,81 @@
|
||||
/**
|
||||
* Keep a fixed-position floating element anchored to a trigger.
|
||||
*
|
||||
* A portaled panel is positioned from its anchor's viewport rect, which stops
|
||||
* being true the moment anything scrolls or the window resizes. This owns that
|
||||
* one concern: measure the anchor, offset the panel below it, clamp the result
|
||||
* inside the viewport, and re-run on scroll (capture phase, so scrollers nested
|
||||
* inside the page are caught too), on resize, and on the panel's own size
|
||||
* changes while the element is open.
|
||||
* @module @deepseek-ai/dsh-client-ui-primitives/useAnchoredPosition
|
||||
*/
|
||||
|
||||
import { useLayoutEffect, useState, type CSSProperties, type RefObject } from 'react'
|
||||
|
||||
/** Inputs for {@link useAnchoredPosition}. */
|
||||
export interface AnchoredPositionOptions {
|
||||
/** Whether the floating element is mounted and should track its anchor. */
|
||||
open: boolean
|
||||
/** The element the panel is placed from. */
|
||||
anchorRef: RefObject<HTMLElement | null>
|
||||
/** The floating element, measured so the clamp uses real dimensions. */
|
||||
panelRef: RefObject<HTMLElement | null>
|
||||
/** Distance kept between the anchor's bottom edge and the panel's top. */
|
||||
gap: number
|
||||
/** Distance kept between the panel and each viewport edge. */
|
||||
margin: number
|
||||
}
|
||||
|
||||
/**
|
||||
* Track an anchor and return the panel's fixed coordinates.
|
||||
* @param options - the open state, the two refs, and the gap/margin distances.
|
||||
* @returns `left`/`top` for the panel, or `null` before the first measurement.
|
||||
*/
|
||||
export function useAnchoredPosition(options: AnchoredPositionOptions): CSSProperties | null {
|
||||
const { open, anchorRef, panelRef, gap, margin } = options
|
||||
const [position, setPosition] = useState<CSSProperties | null>(null)
|
||||
useLayoutEffect(() => {
|
||||
if (!open) {
|
||||
setPosition(null)
|
||||
return
|
||||
}
|
||||
const place = () => {
|
||||
/* v8 ignore start -- geometry read from real layout: jsdom reports zero
|
||||
offset sizes, so the positive-size clamp arms are exercised by browser
|
||||
scenarios rather than unit tests. */
|
||||
const rect = anchorRef.current?.getBoundingClientRect()
|
||||
if (rect === undefined) return
|
||||
const panel = panelRef.current
|
||||
const width = panel?.offsetWidth ?? 0
|
||||
const height = panel?.offsetHeight ?? 0
|
||||
let left = rect.left
|
||||
let top = rect.bottom + gap
|
||||
if (width > 0) left = Math.min(Math.max(left, margin), window.innerWidth - width - margin)
|
||||
if (height > 0) top = Math.min(Math.max(top, margin), window.innerHeight - height - margin)
|
||||
/* v8 ignore stop */
|
||||
setPosition({ left, top })
|
||||
}
|
||||
// The first run measures the panel in the same commit that opened it, so
|
||||
// the clamp uses real dimensions before anything paints.
|
||||
place()
|
||||
window.addEventListener('scroll', place, true)
|
||||
window.addEventListener('resize', place)
|
||||
// The panel's own height changes without either event — a status line
|
||||
// appearing inside it, or a `resize: vertical` textarea dragged taller —
|
||||
// and a stale clamp would let a panel near the bottom edge cross the
|
||||
// margin it is supposed to respect. The guard keeps the hook usable where
|
||||
// `ResizeObserver` is absent, which is how jsdom runs.
|
||||
const panel = panelRef.current
|
||||
let observer: ResizeObserver | null = null
|
||||
if (typeof ResizeObserver !== 'undefined' && panel !== null) {
|
||||
observer = new ResizeObserver(place)
|
||||
observer.observe(panel)
|
||||
}
|
||||
return () => {
|
||||
observer?.disconnect()
|
||||
window.removeEventListener('scroll', place, true)
|
||||
window.removeEventListener('resize', place)
|
||||
}
|
||||
}, [open, anchorRef, panelRef, gap, margin])
|
||||
return position
|
||||
}
|
||||
@@ -0,0 +1,111 @@
|
||||
// @vitest-environment jsdom
|
||||
/**
|
||||
* `useAnchoredPosition` wiring: a floating panel is placed from its anchor and
|
||||
* keeps tracking it while open.
|
||||
*
|
||||
* The geometry itself needs real layout, which jsdom does not provide — the
|
||||
* browser layout scenario in `apps/web/tests/message-feedback-layout.e2e.ts`
|
||||
* owns that. What is asserted here is the wiring the clamp depends on: the
|
||||
* listeners and the panel-size observer are attached while open and released on
|
||||
* close, a size change replays the placement, and the hook still works where
|
||||
* `ResizeObserver` does not exist.
|
||||
*/
|
||||
import { useRef } from 'react'
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest'
|
||||
import { cleanup, render } from '@testing-library/react'
|
||||
import { useAnchoredPosition } from '../src/useAnchoredPosition.ts'
|
||||
|
||||
afterEach(() => {
|
||||
cleanup()
|
||||
vi.unstubAllGlobals()
|
||||
})
|
||||
|
||||
/** One recorded `ResizeObserver` instance, so a test can drive its callback. */
|
||||
interface Recorded {
|
||||
callback: ResizeObserverCallback
|
||||
observed: Element[]
|
||||
disconnected: boolean
|
||||
}
|
||||
|
||||
/**
|
||||
* Install a recording `ResizeObserver` double.
|
||||
* @returns the list every constructed observer registers itself in.
|
||||
*/
|
||||
function stubResizeObserver(): Recorded[] {
|
||||
const made: Recorded[] = []
|
||||
vi.stubGlobal('ResizeObserver', class {
|
||||
private readonly record: Recorded
|
||||
|
||||
constructor(callback: ResizeObserverCallback) {
|
||||
this.record = { callback, observed: [], disconnected: false }
|
||||
made.push(this.record)
|
||||
}
|
||||
|
||||
observe(element: Element) { this.record.observed.push(element) }
|
||||
disconnect() { this.record.disconnected = true }
|
||||
})
|
||||
return made
|
||||
}
|
||||
|
||||
/**
|
||||
* Host component that anchors a panel and reports the computed position.
|
||||
* @param props - whether the panel is open.
|
||||
* @returns the anchor and, while open, the panel carrying the position.
|
||||
*/
|
||||
function Host({ open }: { open: boolean }) {
|
||||
const anchorRef = useRef<HTMLButtonElement>(null)
|
||||
const panelRef = useRef<HTMLDivElement>(null)
|
||||
const position = useAnchoredPosition({ open, anchorRef, panelRef, gap: 4, margin: 12 })
|
||||
return (
|
||||
<>
|
||||
<button ref={anchorRef} type="button">anchor</button>
|
||||
{open && <div ref={panelRef} data-testid="panel" style={position ?? { visibility: 'hidden' }} />}
|
||||
</>
|
||||
)
|
||||
}
|
||||
|
||||
describe('useAnchoredPosition', () => {
|
||||
it('observes the panel while open and disconnects when it closes', () => {
|
||||
const made = stubResizeObserver()
|
||||
const ui = render(<Host open />)
|
||||
|
||||
expect(made).toHaveLength(1)
|
||||
expect(made[0]?.observed).toEqual([ui.getByTestId('panel')])
|
||||
expect(made[0]?.disconnected).toBe(false)
|
||||
|
||||
ui.rerender(<Host open={false} />)
|
||||
|
||||
expect(made[0]?.disconnected).toBe(true)
|
||||
})
|
||||
|
||||
it('replaces the panel when its own size changes', () => {
|
||||
const made = stubResizeObserver()
|
||||
render(<Host open />)
|
||||
const before = made[0]?.callback
|
||||
expect(before).toBeDefined()
|
||||
|
||||
// A status line appearing inside the panel, or a dragged textarea, changes
|
||||
// the height without a scroll or resize event; the observer is the only
|
||||
// thing that notices, so driving its callback must not throw.
|
||||
expect(() => { before?.([], {} as ResizeObserver) }).not.toThrow()
|
||||
})
|
||||
|
||||
it('still places the panel where ResizeObserver does not exist', () => {
|
||||
// jsdom's own condition, and any host without the API: the hook must fall
|
||||
// back to scroll and resize rather than fail at mount.
|
||||
vi.stubGlobal('ResizeObserver', undefined)
|
||||
|
||||
expect(() => render(<Host open />)).not.toThrow()
|
||||
})
|
||||
|
||||
it('attaches no listeners while the element is closed', () => {
|
||||
const made = stubResizeObserver()
|
||||
const add = vi.spyOn(window, 'addEventListener')
|
||||
|
||||
render(<Host open={false} />)
|
||||
|
||||
expect(made).toHaveLength(0)
|
||||
expect(add.mock.calls.filter(([type]) => type === 'scroll' || type === 'resize')).toEqual([])
|
||||
add.mockRestore()
|
||||
})
|
||||
})
|
||||
Generated
+3
@@ -2182,6 +2182,9 @@ importers:
|
||||
'@types/react':
|
||||
specifier: ~18.3.1
|
||||
version: 18.3.31
|
||||
'@types/react-dom':
|
||||
specifier: ~18.3.0
|
||||
version: 18.3.7(@types/react@18.3.31)
|
||||
react:
|
||||
specifier: ^18.2.0
|
||||
version: 18.3.1
|
||||
|
||||
@@ -47,6 +47,7 @@
|
||||
"apps/web/tests/web-search-round.e2e.ts",
|
||||
"apps/web/tests/message-actions.e2e.ts",
|
||||
"apps/web/tests/message-feedback.e2e.ts",
|
||||
"apps/web/tests/message-feedback-layout.e2e.ts",
|
||||
"apps/web/tests/markdown-images.e2e.ts",
|
||||
"apps/web/tests/math-rendering.e2e.ts",
|
||||
"apps/web/tests/markdown-cjk-strong.e2e.ts",
|
||||
|
||||
Reference in New Issue
Block a user