Skip to content

Fix/desktop selection menu above - #1541

Merged
NanmiCoder merged 1 commit into
NanmiCoder:mainfrom
reeij:fix/desktop-selection-menu-above
Oct 15, 2026
Merged

NanmiCoder merged 1 commit into
NanmiCoder:mainfrom
reeij:fix/desktop-selection-menu-above

Conversation

@reeij

@reeij reeij commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

概要

多行选中文字后,操作气泡会出现在选区结束处的右侧。这是 #351(54f3c004)引入的
规则,63c81e63 又把它锚定到选区的 focus 行——而那正好是接下来要读的文字开始的地
方,所以顺着往下读的人,气泡就压在了下一段要看的内容上。

这次不是改规则,而是加一个偏好项。它单独存在 localStorage 的
cc-haha-selection-menu-placement 键下,默认 auto,即上游的右侧槽位,因此默认行
为逐行不变。另一个取值是 above:去掉右侧槽位,让气泡浮在整个选区上方。

这个偏好没有并入 cc-haha-chat-appearance 里的阅读偏好,因为气泡位置不是阅读样
式——改它不应该重写那条记录,「恢复聊天外观默认值」也不应该把它清掉。外观记录及其迁
移、settings.json(用 .strict() 解析)、以及 publicAccess /
remoteBrowserPolicy 两个白名单,全都没有变动。

这里有一个容易做错的点:preferAbove 切换的是锚点,而不只是候选顺序。默认档从
focus 行去量「上方」的位置,这是右侧槽位需要的;但如果用 focus 行去量上方,气泡会压
在刚选中的那几行上,所以该模式改为从整个选区矩形去量。

质量契约

  • 影响范围:desktop
  • 新增/更新的测试:
    • 单元 —— stores/selectionMenuPlacementStore.test.ts(新增):默认 auto、拒绝
      非法值、且只写自己的 key —— 这条断言直接证明「改气泡位置不会动
      cc-haha-chat-appearance」
    • 单元 —— hooks/useSelectionPopoverDismiss.test.ts:同一个两行选区,断言默认档是
      上游右侧槽位、preferAbove 是上方槽位;另有一条单行选区两种模式都在上方
    • 单元 —— components/workspace/surfaces/textSelection.test.ts:两种模式都走
      getTextSelectionFromContainer,即覆盖 store 接线
    • 组件 —— pages/settings/ChatAppearanceSettings.test.tsx:控件默认选中「自动」、
      切换能保存;另加一条证明「恢复聊天外观默认值」不会动气泡位置
    • 组件 —— components/chat/MessageList.test.tsx:新增一条设置偏好后断言上方槽位。
      既有的多行放置断言未改动,因为它锁定的本来就是上游槽位
  • 覆盖率证据:未跑 —— 本地 coverage lane 未完成;由 CI 执行
  • E2E / live model 证据:check:agent-e2e 通过(11 passed / 0 failed,28 次上游请求,
    走 mock upstream,不接 provider),产物在 artifacts/agent-e2e;live model 未跑
    (fork 无 secrets)
  • 已知风险 / 回滚:above 并非字面上的「永远在上方」。当选区贴近视口顶部、上方放不下
    时,两种模式都会退到右侧槽位。该兜底逻辑未改动,由既有的 top-edge 测试锁定。回滚就
    是 revert 这一个提交;独立 key 留在用户机器上也是惰性的、无影响。

验证

  • 我跑了相关的本地检查,或说明了为何不适用。
  • 每一个生产行为改动都补了同区域的测试。
  • 我跑了 bun run check:impact 选中的检查;若声明 PR-ready/完整验证,也跑了 bun run verify。
  • 新增或改动的可执行生产代码行达到改动行覆盖率阈值,或已记录阻塞原因/维护者豁免。
  • 我附上了或总结了质量报告路径、JUnit/日志产物路径,以及 pass/fail/skip 数量。
  • 对跨边界改动跑了确定性 E2E/契约检查。live model 证据在受信任维护者跑过时会附上,否则明确标注未跑。

bun run check:impact 对该 diff 选中 desktop、server、chat-contract、agent-flow、
agent-e2e、coverage。没有选中 persistence-upgrade:本次改动不再触及任何持久化表
面,chatAppearance.ts、它的迁移与相关测试与 main 逐字节相同(对这些路径执行
git diff origin/main 为空)。

实际观察到的确定性结果:

  • 覆盖本 diff 触及的全部文件的 9 个套件:316 passed。
  • 所有改动文件的 tsc --noEmit 与 eslint:干净。
  • check:agent-e2e:通过(11 / 0,28 次上游请求)。

check:desktop 是必选 lane 之一,在本地是红的,但并非本 diff 引起。它报告 12 项失
败,全部是 Test timed out in 5000ms,发生在一台被全量套件并行度压过头的机器上。失
败集合每轮都不同(一轮是 TabBar / hiddenSessionRelease / ActivitySettings /
diagnostics,下一轮变成 docxEngine / dataMigration / pdfEngine / computerUse),而每
个失败文件单独跑都通过 —— TabBar 106/106,唯一一个非超时的失败(RawRequestTab)
4/4。降低并行度后失败数 12 → 4 → 2 → 1。

风险

  • 本 PR 未触及 CLI 核心路径,或已获得 allow-cli-core-change 维护者批准。
  • 生产代码改动包含配套测试,或已获得 allow-missing-tests 维护者批准。
  • 覆盖率基线/阈值变更已获得 allow-coverage-baseline-change 维护者批准。
  • 任何 quarantine 策略变更均已获得维护者批准;确定性 provider/chat 契约测试未被隔离。
  • provider/runtime 改动已由离线 mock/fixture 契约测试覆盖;live smoke 由受信任维护者执行,或已明确推迟。
English

Summary

A multi-line text selection puts its action menu in the slot beside where the
selection ends. That is the upstream rule from 54f3c00, anchored on the
selection focus rect via 63c81e6 — which is exactly where the unread text
resumes, so the menu lands on the next thing the reader wants to look at.

This adds a preference rather than changing the rule. It lives under its own
localStorage key, cc-haha-selection-menu-placement, and defaults to auto,
the upstream slot, so default behaviour is unchanged line for line. The other
value is above: that drops the slot so the menu floats over the whole
selection. The preference does not join the reading preferences in
cc-haha-chat-appearance, because the bubble position is not a reading style —
changing it must not rewrite that record, and "Reset chat appearance" must not
clear it. Nothing in the appearance record, its migration, settings.json
(parsed with .strict()) or the publicAccess / remoteBrowserPolicy
allowlists moves.

One thing that is easy to get wrong here: preferAbove selects the anchor, it
does not only reorder the candidates. The default measures the above slot from
the focus line, which is what the right-side slot needs; measuring above from
there would drop the menu onto the earlier selected lines, so that mode measures
from the whole selection instead.

Feature Quality Contract

  • Changed surface: desktop
  • Tests added or updated:
    • unit — stores/selectionMenuPlacementStore.test.ts (new): defaults to auto,
      rejects anything else, and writes only its own key — the assertion that
      changing the bubble position leaves cc-haha-chat-appearance untouched
    • unit — hooks/useSelectionPopoverDismiss.test.ts: the default is asserted to
      be the upstream right-side slot and preferAbove the above slot, for the same
      two-line range, plus a single-line case that is above in either mode
    • unit — components/workspace/surfaces/textSelection.test.ts: both modes
      through getTextSelectionFromContainer, i.e. the store wiring
    • component — pages/settings/ChatAppearanceSettings.test.tsx: the control
      defaults to Automatic and saves opting out; plus a case proving "Reset chat
      appearance" leaves the bubble placement alone
    • component — components/chat/MessageList.test.tsx: a second case asserting
      the above slot once the preference is set. The existing multi-line assertion
      is unchanged, because it already pins the upstream slot
  • Coverage evidence:
    • not run — the local coverage lane did not complete; CI runs it
  • E2E / live-model evidence:
    • check:agent-e2e — passed (11 passed / 0 failed, 28 upstream requests, mock
      upstream, no provider); artifacts under artifacts/agent-e2e
    • live model: not run (untrusted fork / no provider)
  • Known risk / rollback:
    • above is not literally always above. When the selection sits close enough
      to the viewport top that the above slot cannot fit, both modes fall through to
      the right slot. That fallback is unchanged and is pinned by the existing
      top-edge test. Rollback is a revert of this one commit; the standalone key is
      simply orphaned and inert if it is left behind.

Verification

  • I ran the relevant local checks, or explained why they do not apply.
  • I added or updated same-area tests for every production behavior change.
  • I ran the checks selected by bun run check:impact; if I claim PR-ready/full validation, I also ran bun run verify.
  • New or changed executable production lines meet the changed-line coverage threshold, or the blocker/maintainer override is documented.
  • I attached or summarized the quality report path, JUnit/log artifact path, and pass/fail/skip counts.
  • I ran deterministic E2E/contract checks for cross-boundary changes. Live-model evidence is attached when a trusted maintainer ran it, otherwise it is explicitly marked not run.

bun run check:impact on this diff selects desktop, server, chat-contract,
agent-flow, agent-e2e and coverage. It does not select persistence-upgrade: the
change no longer touches any persistence surface, and chatAppearance.ts, its
migration and its tests are byte-identical to main (git diff origin/main on
those paths is empty).

Deterministic results actually observed:

  • 9 suites covering every file this diff touches: 316 passed.
  • tsc --noEmit and eslint on all changed files: clean.
  • check:agent-e2e: passed (11 / 0, 28 upstream requests).

check:desktop is one required lane that is red locally, for a reason this diff
does not cause. It reports 12 failures, every one Test timed out in 5000ms on a
machine where the full suite oversubscribes itself. The failing set is different
on each run (TabBar / hiddenSessionRelease / ActivitySettings / diagnostics on one
run; docxEngine / dataMigration / pdfEngine / computerUse on the next) and every
failing file passes on its own — TabBar 106/106, and the one non-timeout failure
(RawRequestTab) 4/4. Failure count fell 12 → 4 → 2 → 1 as parallelism was reduced.

Risk

  • This PR does not touch CLI core paths, or it has maintainer approval for allow-cli-core-change.
  • Production code changes include matching tests, or have maintainer approval for allow-missing-tests.
  • Coverage baseline/threshold changes have maintainer approval for allow-coverage-baseline-change.
  • Any quarantine-policy change has maintainer approval; deterministic provider/chat contract tests were not quarantined.
  • Provider/runtime changes are covered by offline mock/fixture contract tests; live smoke was run by a trusted maintainer or explicitly deferred.

@github-actions

Copy link
Copy Markdown

PR quality triage

Changed areas: area:desktop

CLI core policy: No CLI-core policy block detected.

Missing-test policy: No missing-test policy block detected.

Coverage baseline policy: No coverage-baseline policy block detected.

CLI core files:

  • none

Coverage policy files:

  • none

Required check plan:

  • See PR Quality / scope-plan; it is the single source of truth for selected jobs.

Test coverage signals:

  • No obvious missing-test signal from changed paths.

Risk notes:

  • Desktop state/API layer changed: verify store persistence, WebSocket behavior, and startup errors.

Hard merge gates come from the deterministic GitHub Actions contract lanes above.

@reeij
reeij force-pushed the fix/desktop-selection-menu-above branch from ff6c06e to 7bab3c2 Compare October 13, 2026 09:04
A multi-line text selection puts its action menu in the slot beside where the
selection ends. That is the upstream rule from 54f3c00, anchored on the
selection focus rect via 63c81e6 — which is exactly where the unread text
resumes, so the menu lands on the next thing the reader wants to look at.

Add a preference instead of changing the rule. `cc-haha-selection-menu-placement`
defaults to `auto`, the upstream slot, and `above` drops the slot so the menu
floats over the selection. It keys itself rather than joining the reading
preferences in `cc-haha-chat-appearance`: the bubble position is not a reading
style, so changing it must not rewrite that record and "Reset chat appearance"
must not clear it. Nothing in the appearance record, its migration, or the
settings.json schema changes.

`preferAbove` selects the anchor, it does not only reorder the candidates. The
default measures the above slot from the focus line, which is what the
right-side slot needs; measuring above from there would drop the menu onto the
earlier selected lines, so that mode measures from the whole selection.

Tested: bun run test -- --run src/stores/selectionMenuPlacementStore.test.ts src/hooks/useSelectionPopoverDismiss.test.ts src/components/workspace/surfaces/textSelection.test.ts src/components/chat/MessageList.test.tsx src/pages/settings/ChatAppearanceSettings.test.tsx
@reeij
reeij force-pushed the fix/desktop-selection-menu-above branch from 7bab3c2 to 4d6b04a Compare October 15, 2026 01:45
@reeij

reeij commented Oct 15, 2026

Copy link
Copy Markdown
Contributor Author

已 rebase 到最新 main(755a18ed),冲突已解决。

  • 冲突只有 desktop/src/components/chat/MessageList.tsx 的导入区:上游 9fd74c66 引入了 t as translateNow,本 PR 在同一位置插入 prefersSelectionMenuAbove,两边都保留,生产逻辑未改动
  • rebase 后重跑受影响的套件:285 passed / 0 failed;eslint 与 tsc --noEmit 干净
  • 顺带修正 useSelectionPopoverDismiss.ts 中一处指向 lib/chatAppearance.ts 的过期注释(该偏好项实际在 stores/selectionMenuPlacementStore.ts)
  • 提示:PR Quality 目前仍是 action_required,需要维护者批准 fork PR 的 workflow 之后才会开始运行检查

麻烦 review,谢谢。

@NanmiCoder
NanmiCoder merged commit c3183b3 into NanmiCoder:main Oct 15, 2026
1 check passed
@NanmiCoder NanmiCoder mentioned this pull request Oct 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants