Repository navigation
Fix/desktop selection menu above - #1541
Merged
NanmiCoder merged 1 commit intoOct 15, 2026
Merged
Conversation
PR quality triageChanged 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:
Coverage policy files:
Required check plan:
Test coverage signals:
Risk notes:
Hard merge gates come from the deterministic GitHub Actions contract lanes above. |
reeij
force-pushed
the
fix/desktop-selection-menu-above
branch
from
October 13, 2026 09:04
ff6c06e to
7bab3c2
Compare
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
force-pushed
the
fix/desktop-selection-menu-above
branch
from
October 15, 2026 01:45
7bab3c2 to
4d6b04a
Compare
Contributor
Author
|
已 rebase 到最新
麻烦 review,谢谢。 |
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
概要
多行选中文字后,操作气泡会出现在选区结束处的右侧。这是 #351(
54f3c004)引入的规则,
63c81e63又把它锚定到选区的 focus 行——而那正好是接下来要读的文字开始的地方,所以顺着往下读的人,气泡就压在了下一段要看的内容上。
这次不是改规则,而是加一个偏好项。它单独存在 localStorage 的
cc-haha-selection-menu-placement键下,默认auto,即上游的右侧槽位,因此默认行为逐行不变。另一个取值是
above:去掉右侧槽位,让气泡浮在整个选区上方。这个偏好没有并入
cc-haha-chat-appearance里的阅读偏好,因为气泡位置不是阅读样式——改它不应该重写那条记录,「恢复聊天外观默认值」也不应该把它清掉。外观记录及其迁
移、
settings.json(用.strict()解析)、以及publicAccess/remoteBrowserPolicy两个白名单,全都没有变动。这里有一个容易做错的点:
preferAbove切换的是锚点,而不只是候选顺序。默认档从focus 行去量「上方」的位置,这是右侧槽位需要的;但如果用 focus 行去量上方,气泡会压
在刚选中的那几行上,所以该模式改为从整个选区矩形去量。
质量契约
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:新增一条设置偏好后断言上方槽位。既有的多行放置断言未改动,因为它锁定的本来就是上游槽位
coveragelane 未完成;由 CI 执行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。bun run check:impact对该 diff 选中 desktop、server、chat-contract、agent-flow、agent-e2e、coverage。没有选中 persistence-upgrade:本次改动不再触及任何持久化表
面,
chatAppearance.ts、它的迁移与相关测试与main逐字节相同(对这些路径执行git diff origin/main为空)。实际观察到的确定性结果:
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。
风险
allow-cli-core-change维护者批准。allow-missing-tests维护者批准。allow-coverage-baseline-change维护者批准。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 toauto,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 wholeselection. 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 thepublicAccess/remoteBrowserPolicyallowlists moves.
One thing that is easy to get wrong here:
preferAboveselects the anchor, itdoes 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
stores/selectionMenuPlacementStore.test.ts(new): defaults toauto,rejects anything else, and writes only its own key — the assertion that
changing the bubble position leaves
cc-haha-chat-appearanceuntouchedhooks/useSelectionPopoverDismiss.test.ts: the default is asserted tobe the upstream right-side slot and
preferAbovethe above slot, for the sametwo-line range, plus a single-line case that is above in either mode
components/workspace/surfaces/textSelection.test.ts: both modesthrough
getTextSelectionFromContainer, i.e. the store wiringpages/settings/ChatAppearanceSettings.test.tsx: the controldefaults to Automatic and saves opting out; plus a case proving "Reset chat
appearance" leaves the bubble placement alone
components/chat/MessageList.test.tsx: a second case assertingthe above slot once the preference is set. The existing multi-line assertion
is unchanged, because it already pins the upstream slot
coveragelane did not complete; CI runs itcheck:agent-e2e— passed (11 passed / 0 failed, 28 upstream requests, mockupstream, no provider); artifacts under
artifacts/agent-e2eaboveis not literally always above. When the selection sits close enoughto 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
bun run check:impact; if I claim PR-ready/full validation, I also ranbun run verify.bun run check:impacton 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, itsmigration and its tests are byte-identical to
main(git diff origin/mainonthose paths is empty).
Deterministic results actually observed:
tsc --noEmitandeslinton all changed files: clean.check:agent-e2e: passed (11 / 0, 28 upstream requests).check:desktopis one required lane that is red locally, for a reason this diffdoes not cause. It reports 12 failures, every one
Test timed out in 5000mson amachine 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
allow-cli-core-change.allow-missing-tests.allow-coverage-baseline-change.