fix(docs): keep the original quote visible on an orphaned comment - #1183
fix(docs): keep the original quote visible on an orphaned comment#1183211-lee wants to merge 1 commit into
Conversation
Superseded: reposting with repo-relative references (removed absolute local paths).
Jerry-Xin
left a comment
There was a problem hiding this comment.
The change correctly keeps orphaned-comment context visible without altering anchor resolution behavior. Verified at head 29c881d1: the quote is rendered as an auto-escaped React text child ({thread.anchorText || '…'}) — no dangerouslySetInnerHTML/HTML injection — and anchorText is a plain-text doc.textBetween snapshot, so no stored-XSS surface is introduced. The orphan state is derived read-only (orphaned = ready && anchorStart != null && range == null) with no destructive write, and healthy comments (resolvable range) keep the normal quote styling.
💬 Non-blocking
- 🔵 Suggestion:
packages/docs/src/comments/CommentPanel.tsx:210has a very detailed JSX comment with exact line-number references to other files. The explanation is useful, but line numbers drift quickly; consider trimming it or referencing symbols likeanchorTextSnapshot/resolveAnchorRangeinstead. - 🔵 Suggestion:
packages/docs/src/editor/styles.css:2788changes the orphan visual state but relies on manual verification. A focused visual/component test for orphaned + resolved + long-quote would reduce regression risk, though not blocking for this small display-only fix.
✅ Highlights
- 🔵
packages/docs/src/comments/CommentPanel.tsx:216now renders the orphan label and the quote together, preserving the storedanchorTextsnapshot so the reader can still see what was commented on after the anchored text is deleted. - 🔵
packages/docs/src/comments/anchor.ts:172only adds explanatory comments; the null-resolve and collapsed (from === to) orphan branches both still route through the same orphan path — resolution behavior is unchanged. - 🔵
packages/docs/src/editor/styles.css:2780correctly addsmin-width: 0so the existing ellipsis behavior works for the quote inside the flex row, andflex-shrink: 0keeps the status label legible.
mochashanyao
left a comment
There was a problem hiding this comment.
[Octo-Q · automated review]
Verdict: Approve — no blocking findings; notes below (data-flow traced).
Code Review — PR #1183 (octo-web)
Summary
This PR fixes orphaned comment rendering in the docs panel: instead of replacing the original quote text with an "orphaned" label, the label is now shown alongside the quote — which is struck through and dimmed to signal that the anchored text no longer exists in the document. A supporting CSS fix adds min-width: 0 to the quote flex-child so text-overflow: ellipsis actually works when both the label and a long quote share the row. The anchor.ts change is comment-only, documenting why the null-resolve branch is reachable (version restore) despite TipTap's UndoManager keeping it cold in normal editing.
The change is small (3 files, +29/−6), well-scoped, and the data flow from stored anchorText snapshot through to the rendered <span> is correct.
Verification
- ✅ Data flow:
thread.anchorText— populated fromapi.ts:79(input.anchorText ?? ''), originally set byanchorTextSnapshot()atanchor.ts:149(capped at 512 chars). The|| '…'fallback atCommentPanel.tsx:219handles the empty-string edge. Display-only, never used for location. - ✅ Orphan detection invariant —
orphaned = ready && thread.anchorStart != null && range == nullatCommentPanel.tsx:197is unchanged.resolveAnchorRangeatanchor.ts:166–190returnsnullfor both the null-resolve (version restore) and collapsed-range (normal delete) orphan paths. - ✅ Decoration layer consistency —
CommentDecorations.ts:66skips collapsed ranges (if (lo >= hi) continue), matching the panel's orphan state. No highlight painted for orphans in either layer. - ✅ Flex layout correctness —
.octo-comment-anchorisdisplay: flex; align-items: center; gap: 8pxatstyles.css:2763. Addingmin-width: 0to the quote child andflex-shrink: 0to the orphan label is the standard pattern to allow ellipsis truncation in flex containers. - ✅ Click behavior preserved —
scrollToHighlight()atCommentPanel.tsx:200early-returns whenrangeis null (orphan case).onSelect()still fires for thread selection; no scroll for orphan is correct.
Static analysis only at head 29c881d160e8adcff2aa130926180a70a26c285d; build and tests not executed in this environment.
Findings
No P0/P1 issues. No correctness, security, build, or functional concerns.
Things I checked that are fine
- CSS specificity:
.octo-comment-quote.is-orphaned(two-class selector) correctly overrides the base.octo-comment-quoteforopacityandtext-decorationwithout ambiguity. anchor.tscomment accuracy: the null-resolve branch documentation (version restore scenario, UndoManager GC-protection) is consistent with Yjs relative-position semantics and the collapsed-range comment below it.scrollToHighlightfor orphans: early-return on null range means clicking an orphan selects the thread but doesn't scroll — same as before this PR.- i18n key
docs.comment.orphaned: pre-existing, loaded from@octo/basei18n — not introduced by this PR. - No cross-file contract changes: the PR is purely a rendering change; no API, persistence, or shared-type modifications.
Verdict: APPROVED
Clean, well-documented UI fix. The orphan label + struck-through quote gives readers both status and context at a glance, and the min-width: 0 fix is the correct flex-layout pattern. No issues found.
lml2468
left a comment
There was a problem hiding this comment.
Review + Verdict: #1183 — fix(docs): keep the original quote visible on an orphaned comment @ 29c881d1
锚定 head 29c881d160e8(rev-parse == live head)/ blocked。3 文件 +29/−6,纯展示层修复(Closes #1181)。逐处审阅 + 实测。
结论:APPROVE ✅(可合并)—— 低风险展示修复:孤儿评论现「状态标签 + 引用」并存(引用置灰删除线),锚点解析行为零变更;flex 省略号布局正确;构建 / 31 评论测试 / i18n / FULL CI 全绿。
规格符合 ✅(Closes #1181)
- 问题:锚定文本被删的孤儿评论,旧代码用
(orphaned)标签替换引用行 → 读者知道评论已失效,却看不到它原本在评论什么。 - 修复:
CommentPanel.tsx由「二选一三元」改为{orphaned && <标签>}+ 始终渲染<引用span>(孤儿时加is-orphaned类)→ 标签与引用并存;引用来自thread.anchorText(schema.sql:193 明确为孤儿态展示快照、从不作定位源)。
代码质量 ✅(逐处核实)
- 锚点解析零行为变更:
anchor.ts仅新增注释,解释 null-resolve 孤儿分支非死代码——普通编辑因 TipTap UndoManager 保 GC 而两 relpos 均可解析,但版本恢复等整档重写 + 锚点字节留存时会命中(与 #1161 版本恢复路径一致,可信)。if (from==null||to==null) return null逐字未改,解析返回值与旧完全一致。 - flex 省略号布局正确:
.octo-comment-anchor实为display:flex(:2763),故.octo-comment-quote { min-width:0 }(允许收缩到内在宽度以下 → 省略号生效)+.octo-comment-orphan { flex-shrink:0 }(标签不缩、引用缩)的推理成立;引用已有overflow:hidden/ellipsis/nowrap。孤儿is-orphaned= opacity .65 + line-through,一眼可辨。 - 始终挂载引用 span 无副作用:全仓无任何代码/测试以
.octo-comment-quote/.octo-comment-orphan的存在与否推断孤儿态 → 孤儿判定不受影响。 - 安全:
anchorText作 React 文本子节点渲染(“{anchorText || '…'}”,自动转义),无innerHTML、无注入面;快照展示专用、不参与定位。
验证(实测 @ 29c881d1)
- 构建 ✅
✓ built in 4.29s;i18n:check✅(docs.comment.orphaneden/zh 均已存在,无需新 key)。 - 评论测试 ✅ 5 文件 / 31 通过。
- FULL CI:Build / install-build / dependency-review / osv-scan / secret-scan / block-removed-enterprise / history / label / pr-title-lint 均 success;
e2e-p0in_progress(此前各 head 均绿);check-sprint/satisfy-code-reviewskipped(流程门)。
🟡 非阻断(Jerry-Xin 的两条 🟡,可 fast-follow)
- JSX/CSS 注释里的
anchor.ts:148/:180、schema.sql:193行号引用可能随代码漂移——建议改引符号名。非缺陷。 - 样式变更依赖人工验证——可补一个轻量组件测试断言「孤儿时标签+引用并存且引用带
is-orphaned」以防回归。非阻断。
明确判断:APPROVE(可合并)。 纯展示层修复:孤儿评论保留原引用(置灰删除线)并与状态标签并存,补齐「评论在说什么」的信息;锚点解析仅加注释、行为零变更;flex 省略号布局推理成立(父为 flex)、始终挂载引用无副作用、无注入面。构建 / 31 评论测试 / i18n / FULL CI 全绿。文档质量优秀。感谢 @211-lee。Jerry-Xin 的两条 🟡(注释行号、样式测试)建议 fast-follow,不阻断合并。
yujiawei
left a comment
There was a problem hiding this comment.
Code Review — PR #1183 (octo-web)
Reviewed at head SHA 29c881d160e8adcff2aa130926180a70a26c285d against main (merge-base b5f38b9f). Diff is 3 files / +29 −6. I read the linked issue #1181, the full diff, both comment panels that share the touched CSS, and comments/anchor.test.ts; layout and contrast claims below were measured in headless Chromium at the real drawer width, not eyeballed.
1. Spec compliance
Spec: ✅
Issue #1181 asks for one behaviour change: on an orphaned thread, show the (orphaned) label and the original quote, with the quote visually marked as a historical snapshot. CommentPanel.tsx:216-219 does exactly that.
- Missing: none. Both required elements render; the orphan decision itself (
CommentPanel.tsx:191) is untouched, andresolveAnchorRangereturns exactly what it returned before. - Out of scope, correctly untouched: sheet row/column drift and the "content changed but position survived" intermediate state — the issue explicitly excluded both, and neither is touched.
- Unrequested riders: two, both declared in the PR body.
anchor.ts:172-176is a comment-only hunk;styles.css:2780-2783addsmin-width: 0. Neither adds a feature, flag, field or interface, so I am not failing the spec gate on them — but both make claims that do not hold (P1-2, P1-3 below), and the CSS line is dead today. - Verified, not assumed:
docs.comment.orphanedalready exists in both locales (i18n/zh-CN.json:409,i18n/en-US.json:409), so the "no new i18n key" claim holds.
2. Code quality
Quality: Changes-Requested
P1-1 — styles.css:2789: the dim drops the snapshot to ~2.0:1 contrast, on the exact text this PR exists to reveal
.octo-comment-quote.is-orphaned {
opacity: 0.65;
text-decoration: line-through;
}The quote is color: var(--octo-muted, #86909c) at font-size: 12px; font-style: italic (styles.css:2773-2776). Tokens resolve to --octo-muted: #8a919e on --octo-bg: #ffffff (styles.css:3-5), and the drawer paints background: var(--octo-bg, #fff) (styles.css:1921).
| theme | before | composited at opacity: .65 |
WCAG AA (4.5:1, 12px is normal text) |
|---|---|---|---|
light #ffffff |
#8a919e → 3.17:1 |
≈#b3b8c0 → 1.99:1 |
fails, and materially worse |
dark #1b1c1e |
#8a919e → 5.38:1 |
≈#636871 → 3.04:1 |
passed before, fails after |
The baseline was already under AA in light mode; this halves it. Net effect: the row's least legible element becomes the one thing #1181 was filed to make visible — struck-through 12px italic at 2:1. The PR body already flags this as unverified ("the strike-through/dim rendering has not been eyeballed in light and dark"); this is that check.
Suggested fix: line-through plus the adjacent (orphaned) label already carry the "historical" signal unambiguously. Drop opacity and let colour stay at the token, or introduce a dedicated token that stays ≥4.5:1 in both themes.
P1-2 — anchor.ts:172-176: the new comment's mechanism is false, and a test 100 lines away contradicts it
// editing never hits it — TipTap keeps an UndoManager, so deleted items stay GC-protected and both
// relpos still resolve ...
// restore. Verified empirically: no UndoManager → resolve returns null; with one → non-null.
comments/anchor.test.ts:76-91 ('anchored selection deleted => both endpoints collapse to the SAME index (not null)') constructs a bare new Y.Doc() — no UndoManager, gc: true by default — deletes the entire anchored range, and asserts expect(from).not.toBeNull() / expect(to).not.toBeNull(). That is the precise configuration the comment predicts returns null.
The reason the comment gets right conclusion / wrong mechanism: Yjs' Item.gc() only replaces an item with a GC struct when its parent is also deleted. While the parent Y.Text survives, a deleted range stays an Item whose content is swapped for ContentDeleted — length preserved, so the relative position still resolves. The gc flag and the UndoManager are not what protects it.
Worse, the branch's actual reachability condition is already pinned by a test six lines above the one that contradicts the comment: anchor.test.ts:58-71, 'returns null (orphan) when the referenced item is unknown to the doc', resolves a relpos from docA against a fresh docB and gets null. That is the state-vector condition (getState(store, id.client) <= id.clock) — the same thing a wholesale document rewrite produces. The hunk's own thesis is defensible; it just cites the wrong cause and the wrong evidence.
Suggested fix: point the comment at anchor.test.ts:58-71 and state the condition as "the referenced client/clock is absent from this doc's state vector (fresh or wholesale-rewritten doc — e.g. version restore)". Remove the UndoManager/GC-protection claim and the "Verified empirically" wording, which currently reads as authority for something a sibling test disproves.
P1-3 — styles.css:2780-2783: min-width: 0 is a no-op here, and the 3-line rationale states a false CSS rule
/* Flex child inside .octo-comment-anchor: default min-width:auto would refuse to shrink below
the text's intrinsic width, so the ellipsis never kicks in ... */
min-width: 0;Per css-flexbox-1 §4.5, the content-based automatic minimum size applies only to a flex item that is not a scroll container. .octo-comment-quote already sets overflow: hidden (styles.css:2777), which makes it a scroll container, so its automatic minimum size is already 0 and the ellipsis already worked.
Measured in headless Chromium with the exact declarations from this file, at the real 306px thread content width (drawer 360px − 16px×2 padding − thread 10px×2 padding − 1px borders), long snapshot, (orphaned) label present:
A overflow:hidden, no min-width quote clientW=233 scrollW=671 row scrollW=300 == clientW=300
B overflow:hidden + min-width:0 quote clientW=233 scrollW=671 row scrollW=300 == clientW=300 <- identical
C overflow:visible, no min-width quote clientW=671 scrollW=671 row scrollW=738 > clientW=300 <- only here does it matter
A and B are byte-identical. The described symptom ("a long quote pushes the orphan/resolved labels out of the row") is only reproducible in case C, which this stylesheet is not in. The PR body elevates this line to "the one line with reach beyond the orphan case"; it currently has no reach at all.
Suggested fix: delete the line, or keep it as defence-in-depth against a future overflow change and rewrite the comment to say that (overflow: hidden already zeroes the automatic minimum size; this guards the day that changes).
P2-1 — styles.css:2792-2797: the flex-shrink comment's guarantee does not cover the resolved badge
/* Status label must stay fully legible; the quote shrinks instead (see min-width above). */
flex-shrink: 0;True for .octo-comment-orphan. Not true for .octo-comment-resolved-badge (styles.css:2798-2806), which is a sibling flex item with neither flex-shrink: 0 nor white-space: nowrap. Measured at 306px, zh-CN, orphaned and resolved, long snapshot:
orphan w=60.0 h=17.0 | quote w=193.7 h=17.0 | resolved-badge w=30.3 h=49.0 | row h=55.0
The badge wraps to three lines and the row triples in height (22px → 55px). This is pre-existing — the same wrap already occurs today on a live long quote plus a resolved badge (measured rowH=55 with HEAD~1 CSS) and also reaches sheet/SheetCommentPanel.tsx:227 — but this PR makes it reachable in the orphan state, which previously rendered cleanly at rowH=25 because there was no quote in the row.
Suggested fix: flex-shrink: 0; white-space: nowrap on .octo-comment-resolved-badge, then the comment's claim becomes true as written.
Related, and it checks out: .octo-comment-quote is shared with sheet/SheetCommentPanel.tsx:227, so the min-width addition lands there too — I verified it is a no-op there for the same overflow: hidden reason, so there is no sheet regression.
P2-2 — no component regression test for the changed lines
packages/docs already has @testing-library/react 16.1.0 + jsdom (packages/docs/package.json:83,90) and sibling panel tests: versions/VersionPanel.test.tsx, html/HtmlDocCommentPanel.test.tsx, share/ShareScopePanel.test.tsx. There is no CommentPanel test, and grepping packages/docs/src test files for orphan returns only the pure-Yjs cases in comments/anchor.test.ts.
So the 31 passing tests cited in the PR body do not execute a single changed line. The precise regression this PR fixes — label rendered instead of quote — stays unguarded, as do anchorText === '' (renders a struck-through “…” next to the label), the is-orphaned class, and label/quote/badge ordering.
Suggested fix: one CommentPanel.test.tsx with three cases: orphaned + non-empty snapshot asserts both label and quote present; orphaned + empty snapshot; orphaned + resolved.
P2-3 — the snapshot is clipped to one line with no way to read the rest
ANCHOR_TEXT_MAX = 512 (anchor.ts:47), so a snapshot can be 512 characters. Measured box width for the quote at the 360px drawer: 193.7px orphaned + resolved, 232.0px orphaned only — roughly 16–19 CJK glyphs or ~27 Latin characters at 12px. Everything past that is text-overflow: ellipsis with no tooltip and no expansion.
For a live thread that is fine — the full text is in the document. For an orphaned thread the snapshot is the only surviving record of what was commented on, which is the entire premise of #1181. Clipping ~95% of it leaves the reader close to where the issue found them.
The sibling panel in this repo already solves exactly this: html/HtmlDocCommentPanel.tsx:180 sets title={quoteText} on its quote.
Suggested fix: title={thread.anchorText} on the quote span (one line, matches existing practice), or allow two lines via -webkit-line-clamp: 2 in the .is-orphaned case.
P2-4 — citation rot, self-inflicted inside this commit
CommentPanel.tsx:212citesanchor.ts:180for"rendered from the anchorText snapshot". That phrase was atanchor.ts:180before this commit; this commit's ownanchor.tshunk (+6/−1) moved it to:185.anchor.ts:180now reads// RelativePositions collapsed onto the same index (Yjs does NOT return null in this. The citation was stale the moment it was written.CommentPanel.tsx:215citesmigrations/schema.sql:193. This repository contains no.sqlfile —find . -name '*.sql'outsidenode_modulesreturns nothing. It is a cross-repo pointer with no local resolution and no repo named.
Suggested fix: cite symbols and block names (anchorTextSnapshot, the ORPHAN (collapsed) branch) rather than line numbers, and name the repository for cross-repo references.
3. What I checked that holds up
Recording these so the next reviewer does not redo them:
- Nothing keys off the quote's absence.
.octo-comment-quoteappears in exactly five places repo-wide:CommentPanel.tsx:217,SheetCommentPanel.tsx:227, and the three CSS rules. No test selector, decoration, or query depends on the orphan branch omitting it. The PR body's claim is correct. - The two now-adjacent spans are spaced. JSX strips the newline between
:216and:217, so no text node separates them — but.octo-comment-anchoralready setsgap: 8px(styles.css:2765), which covers it. flex-shrink: 0on the orphan label is genuinely required, not cargo-cult. Without it, zh-CN(已失效)shrinks 60.0px → 32.5px and wraps, because CJK min-content is ~1 glyph. Measured.- No falsy-render hazard.
orphanedis a real boolean (CommentPanel.tsx:191), so{orphaned && …}cannot leak a0. - The anchor path is behaviourally untouched, as claimed:
resolveAnchorRange's only edit is the comment, and the orphan predicate is unchanged.
4. Coverage — what I did not verify
- Did not run the test suite,
i18n:check, ortsc. This checkout has nonode_modulesand I did not install. The PR body's "31 passed / 0 candidates / no new errors" claims are therefore unverified by me — not disputed. - The version-restore reachability claim is unverified against a live TipTap+Yjs binding. My P1-2 reasoning comes from Yjs
Item.gc()semantics plus this repo's own tests, not from executing a restore. - Layout and contrast numbers are Chromium-only (headless 1223), with rules reconstructed from
styles.cssat this SHA. Not checked on Safari or Firefox, not on a real device, and not at the drawer'smax-width: 92vwmobile width, where the quote box shrinks further and P2-3 gets worse. - No screenshots — the same gap the author flagged. Light/dark captures of an orphaned thread, ideally one orphaned and resolved with a long zh-CN snapshot, would settle P1-1 and P2-1 in one image each.
- Did not review
sheet/SheetCommentPanel.tsxorhtml/HtmlDocCommentPanel.tsxbeyond the shared-CSS blast radius. Worth noting thatHtmlDocCommentPanel.tsx:179-186has the same mutually-exclusivequote ? … : labelshape this PR is fixing here; the semantics differ (its quote is resolved live, its label is a positional descriptor, so it is not the same defect), but it is the place to look if #1181's pattern is suspected elsewhere.
5. Verdict
CHANGES_REQUESTED.
The user-facing behaviour is right and the fix is the correct shape — the orphan branch should never have thrown the snapshot away, and flex-shrink: 0 on the label is a real catch. What blocks is narrow and cheap to fix:
opacity: 0.65renders the snapshot at ~2:1 in light theme, defeating the readability this PR is for (P1-1).- Two of the three hunks are comments, and both assert mechanisms that are false — one contradicted by a test in the same directory, one by browser measurement — with one asserting "Verified empirically" (P1-2, P1-3).
None of these changes runtime behaviour, so all four P1/P2 code fixes are a handful of lines. Add the CommentPanel test (P2-2) and the title attribute (P2-3) and this is a clean merge.
|
Correction to my review above — two line citations in it are wrong, and since one of my own findings (P2-4) is about citation accuracy, it would be poor form to leave them standing. I twice cited the orphan predicate as
No change to the verdict or to any finding. |
Jerry-Xin
left a comment
There was a problem hiding this comment.
Correcting my earlier APPROVE — I under-called an accessibility contrast failure. Credit to yujiawei.
🔴 Blocking — packages/docs/src/editor/styles.css (.octo-comment-quote.is-orphaned): opacity: 0.65 drops the orphaned quote to failing contrast on the very text this PR exists to reveal. The quote is color: var(--octo-muted) ≈ #8a919e at 12px; composited at opacity 0.65 over the panel background it renders ≈ 1.99:1 (light, #fff) and ≈ 3.04:1 (dark, #1b1c1e) — both below WCAG AA (4.5:1 for normal text). The light-mode baseline already sat at ~3.17:1 and the dim roughly halves it. Fix: drop opacity and keep text-decoration: line-through — the strike plus the adjacent status label already signal "historical" unambiguously; or use a dedicated token that stays ≥ 4.5:1 in both themes.
🟡 Non-blocking — packages/docs/src/comments/anchor.ts (:172-176): the new comment's mechanism is factually wrong and contradicted by a test in the same file. It says ordinary editing never hits the null branch because "TipTap keeps an UndoManager, so deleted items stay GC-protected and both relpos still resolve," but comments/anchor.test.ts (:76-91) uses a bare new Y.Doc() (no UndoManager, default GC), deletes the whole anchored range, and asserts both endpoints resolve non-null and collapse — the exact config the comment predicts returns null. The range stays resolvable because Yjs swaps deleted content to ContentDeleted (length preserved) while the parent Y.Text survives, not because of the UndoManager. The conclusion (reachable via wholesale rewrite / version restore) is fine; the cited cause and the "verified empirically" wording are not. Fix: point the comment at anchor.test.ts (~:58-68, fresh-doc null resolve) and state the condition as "the referenced client/clock is absent from this doc's state vector (fresh or wholesale-rewritten doc, e.g. version restore)."
The core feature (keeping the quote visible + orphaned label/strike) is good — the only blocker is the contrast; the comment is a same-pass cleanup.
Superseded — correcting to request-changes after confirming a WCAG AA contrast failure on the orphaned quote text.
lml2468
left a comment
There was a problem hiding this comment.
校准 / 更正结论:#1183 @ 29c881d1 —— 我上轮 APPROVE 低估了 @yujiawei 的两条,更正为 REQUEST_CHANGES
我更正上一条 APPROVE。 感谢 @yujiawei 指出两条我审阅时低估的问题。经我在 checkout 逐行/逐值核实,两条均属实,且都由本 PR 引入或与本 PR 的注释直接相关。认领。
🟡→🔴(应修)Claim 1:孤儿引用透明度导致对比度不足(a11y)
- 核实:
.octo-comment-quote基色var(--octo-muted, #86909c),在白底上对比约 2.9:1(本就临界);本 PR 新增.octo-comment-quote.is-orphaned { opacity: 0.65 }把有效前景色混向背景(≈#b1b7bf),对比降到 ~1.9:1 —— 对有意义的正文(评论原本在说什么的历史快照)未达 WCAG AA(4.5:1)。 - 归因:2.9:1 的基色是既存(非本 PR),但额外的 0.65 opacity 是本 PR 引入、专施于孤儿引用 → 这是本 PR 造成的增量对比下降。我上轮只夸「一眼可辨」,漏评对比度——认领。
- 修(采纳 yujiawei 建议):去掉
opacity: 0.65,仅保留text-decoration: line-through+ muted 基色 + 相邻橙色(orphaned)标签(#d4691e,对比良好)区分历史态。既不再进一步压对比,又保留可辨识度。
🟡 Claim 2:anchor.ts 的 UndoManager 注释机制描述有误、与测试矛盾
- 核实:注释称「TipTap 有 UndoManager → 删除项被 GC 保护 → 两 relpos 仍可解析;无 UndoManager → 解析返回 null」。但
anchor.test.ts:76「anchored selection deleted => both endpoints collapse to the SAME index (not null)」证明:同档内普通删除(不依赖 UndoManager)relpos 仍非 null、靠 collapse 语义(from===to)判孤儿;而真正的 null-resolve 由:58「referenced item is unknown to the doc」(跨档/整档重写)触发,并非「UndoManager 缺席」。 - 结论:注释「不要当死代码删」的核心主张正确(null 分支确被
:58触达),但机制解释错误——把「relpos 仍解析」归因于 UndoManager 的 GC 保护,与 collapse 语义的测试事实矛盾。属可维护性缺陷:会误导后续维护者(如误以为「关掉 UndoManager 就会破坏孤儿处理」)。 - 修:改写注释——null-resolve 的真实触发是「item 对当前 doc 未知(整档重写/版本恢复)」;同档删除走 collapse(from===to)非 null;两者都路由到孤儿路径。去掉对 UndoManager GC 保护的因果归因。
不受影响(保持)
运行时行为正确:两种孤儿态(null-resolve、collapsed)都正确路由到孤儿路径并渲染 anchorText 快照;无注入面;anchor.ts 仅注释、解析逻辑零变更;构建 / 31 评论测试 / i18n / FULL CI 仍绿。规格意图(孤儿评论保留原引用)本身正确——问题在呈现对比与注释准确性。
更正后判断:需修改 —— REQUEST_CHANGES。 两处应修(均由本 PR 引入/相关,fix 都很小):(1) 去掉孤儿引用的 opacity:0.65(WCAG AA:~1.9:1),保留删除线+muted+橙标区分;(2) 订正 anchor.ts 的 UndoManager 机制注释(真实为 collapse 语义 + unknown-item,非 GC 保护),与 anchor.test.ts:76/:58 对齐。我上轮 APPROVE 低估了这两点,认领并致谢 @yujiawei。改后即可复审放行。
An orphaned thread replaced its quote row with the "(orphaned)" label, so a reader could see that the comment was dead but not WHAT it had been commenting on — the comment body lost its referent entirely. The anchorText snapshot exists precisely for this case: anchor.ts:148 documents it as "for orphaned comments" and anchor.ts:180 documents the orphan path as "rendered from the anchorText snapshot", while migrations/schema.sql:193 marks the field display-only, never a location source. The renderer was discarding the one thing the data layer had stored for it. Now the label and the quote render together, the quote struck through and dimmed so a historical snapshot is distinguishable from a live one at a glance. Also documents why the null-resolve orphan branch in resolveAnchorRange is not dead code (TipTap's UndoManager keeps deleted items GC-protected, so ordinary editing never reaches it; a wholesale document rewrite such as a version restore does), and fixes the flex min-width that stopped the quote from ellipsizing and pushed the status labels out of the row.
29c881d to
863c391
Compare
lml2468
left a comment
There was a problem hiding this comment.
Re-review (rr2) + Verdict: #1183 — fix(docs): keep the original quote visible on an orphaned comment @ 863c3911
锚定 live head 863c39110f4f(rev-parse == live head)/ blocked。单提交 PR(squash)。逐处核实我上轮 REQUEST_CHANGES 的两项。
结论:REQUEST_CHANGES → APPROVE ✅(可合并)—— 我上轮 CR 的两项(a11y 对比 🔴 + UndoManager 注释 🟡)均已精准修复并实测验证。构建 / 31 评论测试 / i18n / CI 依赖面全绿。
🔴→✅ a11y 对比:去掉孤儿引用的透明度
.octo-comment-quote.is-orphaned现仅text-decoration: line-through——opacity: 0.65已移除。孤儿引用回到基色var(--octo-muted, #86909c)(~2.9:1,与非孤儿引用同,无本 PR 引入的增量下降),历史态由删除线 + 相邻橙色(orphaned)标签承载。正是我与 @yujiawei 建议的修法。新增注释也如实记录该权衡(「dimming → ~1.9:1 well under WCAG AA」)。阻塞项解除。
🟡→✅ UndoManager 注释:机制描述订正、与测试对齐
- 旧注释错误归因(「UndoManager GC 保护 → relpos 仍解析」)已删除。新注释准确:
- 非 null:删除锚定文本后两 relpos 仍可解析并 collapse 到同一 index(collapse 语义,pinned by
anchor.test.ts:76)。 - null-resolve:「referenced client/clock 不在本 doc 的 state vector 中」(fresh/整档重写,如版本恢复)而锚点字节留存(pinned by
anchor.test.ts:58-68)。
- 非 null:删除锚定文本后两 relpos 仍可解析并 collapse 到同一 index(collapse 语义,pinned by
- 引用行号实测准确:
:58= 「returns null (orphan) when the referenced item is unknown to the doc」;:76= 「anchored selection deleted => both endpoints collapse to the SAME index (not null)」——与注释描述完全对应。分支非死代码的核心主张保留且现机制解释与测试一致。
其余保持(不受影响)
特性主体不变:孤儿评论「状态标签 + 引用」并存、引用 min-width:0(flex 省略号)、标签 flex-shrink:0(不缩);anchorText 作转义 React 文本节点渲染、无注入面;锚点解析逻辑零行为变更(仅注释)。
验证(实测 @ 863c3911)
- 构建 ✅
✓ built in 4.20s;i18n:check✅。 - 评论测试 ✅ 5 文件 / 31 通过。
- CI:dependency-review / osv-scan / secret-scan / block-removed-enterprise / history / label / pr-title-lint 均 success;Build / install-build 本 head 在跑(本地 build 已通过替代验证)。请合并前确认 Build 转绿。
明确判断:REQUEST_CHANGES → APPROVE(可合并)。 我上轮 CR 的两项均已正确处理:孤儿引用去掉 opacity:0.65(消除 ~1.9:1 的 WCAG AA 违规,保留删除线+橙标区分);anchor.ts 的 UndoManager 机制注释订正为 collapse 语义 + state-vector 缺失,并准确引用 anchor.test.ts:76 / :58-68。构建 / 31 测试 / i18n / CI 依赖面全绿。感谢 @211-lee 快速收敛,谢 @yujiawei 抓到对比度这条。
Jerry-Xin
left a comment
There was a problem hiding this comment.
This PR is relevant to Mininglamp-OSS/octo-web and correctly resolves both blocking items from the previous round. The orphan display now keeps the quote visible without altering anchor resolution behavior.
✅ Blocking items resolved (were CHANGES_REQUESTED @ 29c881d)
- Contrast:
.octo-comment-quote.is-orphanedinpackages/docs/src/editor/styles.cssnow applies onlytext-decoration: line-through— theopacity: 0.65that composited the quote text to ~1.9:1 (light) / ~3.0:1 (dark), below WCAG AA 4.5:1, has been removed. The historical state is signaled by the strike-through plus the adjacent orange orphaned label. The PR-introduced opacity regression is gone; the quote is back at parity with the non-orphaned muted baseline (the basevar(--octo-muted, #86909c)~2.9:1 is pre-existing and used for all quotes, not this PR's burden). - Anchor comment: the comment at
packages/docs/src/comments/anchor.ts(~:169-176) no longer claims an UndoManager GC mechanism. It now correctly states the null resolve occurs when the referenced client/clock is absent from this doc's state vector (a fresh or wholesale-rewritten doc, e.g. version restore) and cites the right tests —anchor.test.ts:58-68for the null case and the collapsed hot path pinned byanchor.test.ts:76. Both citations verified accurate against the test file.
✅ Core behavior intact
packages/docs/src/comments/CommentPanel.tsxrenders the orphaned label ALONGSIDE the quote (label conditional, quote always shown), matching theanchorTextsnapshot contract; anchor decision at :181 is unchanged so anchoring semantics are untouched.{thread.anchorText || '…'}is JSX text — React auto-escapes it; nodangerouslySetInnerHTML, no XSS surface.- Orphan path in
anchor.tsis a display-onlyreturn null; no destructive orphan-branch write. min-width: 0on.octo-comment-quoteis the correct flexbox fix so the ellipsis engages and the quote shrinks instead of pushing labels out of the row.
💬 Non-blocking
- 🔵 Consider a small component-level regression test asserting an orphaned thread renders both
.octo-comment-orphanand.octo-comment-quote.is-orphaned; the anchor unit tests cover orphan detection but not this specific rendering contract. - 🔵 The JSX/CSS explanatory comments are accurate but long for UI code; consider trimming if this area is touched again, and note the embedded line-number citations can drift over time.
mochashanyao
left a comment
There was a problem hiding this comment.
[Octo-Q · automated review]
Verdict: Approve — no blocking findings; notes below (data-flow traced).
Code Review — PR #1183 (octo-web)
Summary
This PR fixes the orphaned-comment display in the docs comment panel: previously, when a comment's anchored text was deleted, the panel showed only an "orphaned" status label and threw away the original quote. Now the orphaned label appears alongside the historical anchorText snapshot (struck through), so readers can still see what was commented on even after the anchored content is gone. The CSS adds flex-shrink handling so the quote truncates properly instead of pushing labels off-screen.
The change is well-motivated, the data flow is sound, and the implementation is clean.
Verification
- ✅ Data flow:
thread.anchorTextalways available for orphaned comments —anchorTextis astringfield on theCommentinterface (packages/docs/src/comments/api.ts:24), populated at creation time byanchorTextSnapshot()(packages/docs/src/comments/anchor.ts:149–151), and persisted via the backend REST contract. The fallback|| '…'atpackages/docs/src/comments/CommentPanel.tsx:218handles the empty-string edge case. - ✅ Orphan detection unchanged —
orphaned = ready && thread.anchorStart != null && range == nullat:181.resolveAnchorRangereturns null for both null-resolve (absent state vector) and collapsed (deleted text) cases. The PR'sanchor.tschanges are comment-only — no logic change. - ✅ Click-to-scroll safe on orphaned comments —
scrollToHighlightat:189checksif (!range) returnbefore scrolling. Orphaned comments haverange == null, so clicking selects the thread but correctly skips the impossible scroll. - ✅ Flex layout correct —
.octo-comment-anchorisdisplay: flex; align-items: center; gap: 8px(packages/docs/src/editor/styles.css:2762). Addingmin-width: 0to the quote enables proper text-overflow ellipsis;flex-shrink: 0on the orphan label prevents it from being crushed. CSS specificity of.octo-comment-quote.is-orphaned(0,2,0) correctly overrides the base class. - ✅ Resolved + orphaned coexistence — A comment can be both resolved and orphaned. The new layout shows all three indicators simultaneously:
[orphaned label] [strikethrough quote] [resolved badge]. No state is lost. - ✅ Test coverage for anchor logic —
anchor.test.tspins both orphan paths: null-resolve at:58–68and collapsed-selection at:76–92. The rendering change is UI-only and not regression-tested, which is acceptable for this scope.
Findings
No P0/P1 issues. One P2 nit on comment accuracy.
P2 — Stale line-number cross-reference (packages/docs/src/comments/CommentPanel.tsx:212)
The JSX comment cites anchor.ts:180 for the phrase "rendered from the anchorText snapshot", but that phrase actually lives at line 187 in the post-PR file (inside the collapsed-case comment block). The reference is off by 7 lines — likely because it was written against a different intermediate state of the file.
Similarly, migrations/schema.sql:193 points to a file that does not exist in this repo (the schema lives in octo-server). The claim it supports — "The snapshot is display-only — never a location source" — is plausible, but the reference cannot be verified from this checkout.
Consider dropping bare line numbers in favor of stable anchors (function name, comment heading, or a keyword search string) so these cross-references survive the next edit without drifting.
Verdict: APPROVED
Clean, well-documented UI fix. The data flow is sound — anchorText is always populated at comment creation and persisted, so the orphaned-quote display will always have content to render. The CSS correctly handles flex shrinking and visual differentiation. The only finding is a minor line-number inaccuracy in a comment, not a merge blocker.
yujiawei
left a comment
There was a problem hiding this comment.
Code Review — PR #1183 (octo-web)
Reviewed at head 863c39110f4f7d0dde852d358479272a167367bb against main (merge-base b5f38b9f). 3 files, +33 −6. I read issue #1181, the full diff, both panels that share the touched CSS, and comments/anchor.test.ts. Layout and contrast numbers below were measured in headless Chromium using the exact declarations from this file at the real drawer width — not eyeballed. Build, the comments suite, i18n:check and tsc were executed.
This is the third round on this PR. Two of the three previous blockers are genuinely fixed; one is unchanged.
1. Spec compliance
Spec: ✅
Issue #1181 asks for one behaviour change: on an orphaned thread show the (orphaned) label and the original quote, with the quote marked as a historical snapshot. CommentPanel.tsx:216-219 does exactly that.
- Missing: none. Both elements render; the orphan decision (
CommentPanel.tsx:181) is untouched. - Out of scope, correctly untouched: sheet row/column drift and the "content changed but position survived" intermediate state — both explicitly excluded by the issue.
- Unrequested riders: two, both declared in the PR body — the comment-only
anchor.tshunk andmin-width: 0. Neither adds a feature, flag, field or interface, so I am not failing the spec gate on them.min-width: 0is still inert, which is P1 below, not a spec failure. - Verified rather than assumed:
docs.comment.orphanedexists in both locales (i18n/en-US.json:409,i18n/zh-CN.json:409), so the "no new i18n key" claim holds.
2. Code quality
Quality: Changes-Requested
✅ Confirmed fixed since the previous head
- Contrast.
opacity: 0.65is gone. Measured computed style on the orphaned quote:opacity: 1,color: rgb(138, 145, 158)=#8a919e— i.e. the token, uncomposited. That is 3.17:1 on#ffffffand 5.38:1 on#1b1c1e. This PR now makes zero incremental change to contrast, which was the ask. The residual sub-AA light-mode value belongs to the repo-wide--octo-mutedtoken (styles.css:5), is shared by every muted string in the product, and is correctly out of scope here. anchor.tscomment. Rewritten accurately, and both new citations resolve:anchor.test.ts:76is'anchored selection deleted => both endpoints collapse to the SAME index (not null)', andanchor.test.ts:58-68is'returns null (orphan) when the referenced item is unknown to the doc'. The UndoManager/GC-protection attribution and the "verified empirically" wording are gone. The state-vector framing is correct.
🔴 P1 — styles.css:2780-2783: min-width: 0 is inert, and its 3-line rationale states a false CSS rule
/* Flex child inside .octo-comment-anchor: default min-width:auto would refuse to shrink below
the text's intrinsic width, so the ellipsis never kicks in and a long quote pushes the
orphan/resolved labels out of the row. The quote is the only element here that may shrink. */
min-width: 0;This block is byte-identical to the previous head — diff against 29c881d1 reports no change. It was raised last round and neither fixed nor contested.
The content-based automatic minimum size applies only to a flex item that is not a scroll container. .octo-comment-quote already sets overflow: hidden (styles.css:2777), which makes it one, so its automatic minimum was already 0 and the ellipsis already worked.
Ablation — same stylesheet, min-width: 0 removed from .octo-comment-quote and nothing else, at the real 305px anchor width (drawer 360px − 16×2 padding − thread 10×2 − borders):
| case | computed min-width |
quote clientW | quote scrollW | row overflow |
|---|---|---|---|---|
| en, orphaned + resolved | 0px |
155 | 517 | 0 |
| en, orphaned + resolved | auto |
155 | 517 | 0 |
| zh, orphaned + resolved | 0px |
199 | 416 | 0 |
| zh, orphaned + resolved | auto |
199 | 416 | 0 |
All six configurations tested (en/zh × orphaned+resolved / orphaned-only / resolved-only) are metric-identical. Each of the comment's three claims is refuted by the auto rows: the quote did shrink (155px against a 517px intrinsic width), the ellipsis did engage (362px clipped), and nothing was ever pushed out of the row (row overflow = 0 everywhere).
The PR body still elevates this to "the one line with reach beyond the orphan case." It has no reach at all.
Fix: delete the declaration and the comment; or keep it as defence-in-depth and reword to say that overflow: hidden already zeroes the automatic minimum size and this guards the day that changes. Four lines either way, no behaviour change.
🟡 P2-1 — styles.css:2794-2799: the flex-shrink guarantee does not cover the resolved badge
flex-shrink: 0 is right for .octo-comment-orphan, and the comment at :2797 claims the quote is what shrinks. .octo-comment-resolved-badge (styles.css:2800-2808) is a sibling flex item with neither flex-shrink: 0 nor white-space: nowrap. Measured, zh-CN, orphaned and resolved, long snapshot:
base (HEAD~1 markup+CSS): row h=26 badge 48.0 x 20.0 ← label only, no quote in the row
head: row h=58 badge 30.3 x 52.0 ← badge wraps to 3 lines
CJK min-content is ~1 glyph, so the badge absorbs its proportional share of the shrink and collapses. This is pre-existing on the live-quote path — base with a non-orphan long zh quote also measures row h=58 / badge 35.4 x 52.0 — but this PR newly reaches it in the orphan state, which rendered cleanly at h=26 before because there was no quote in that row.
Fix: flex-shrink: 0; white-space: nowrap on .octo-comment-resolved-badge. The comment at :2797 then becomes true as written.
🟡 P2-2 — CommentPanel.tsx:212,215: citation rot, one instance self-inflicted by this commit
:212citesanchor.ts:180for"rendered from the anchorText snapshot". That phrase was at:180before this commit; this commit's ownanchor.tshunk (+7) moved it to:187.anchor.ts:180now reads// ORPHAN (collapsed): a root comment is always created from a NON-empty selection,. The citation was stale the moment it was written.:215citesmigrations/schema.sql:193.find . -name '*.sql'outsidenode_modulesreturns nothing — this repo contains no SQL file. It is a cross-repo pointer with no repo named and no local resolution.
anchor.ts:148 in the same comment does check out.
Fix: cite symbols and block names (anchorTextSnapshot, the ORPHAN (collapsed) branch) instead of line numbers, and name the repository for cross-repo references.
🟡 P2-3 — the commit message and PR description now contradict the shipped code
The code comments were corrected; the commit body was not, and it is what squash-merges into main.
- Commit body: "the quote struck through and dimmed" —
opacitywas deliberately removed. Nothing is dimmed. The PR summary says the same. - Commit body: "TipTap's UndoManager keeps deleted items GC-protected, so ordinary editing never reaches it" — this is exactly the mechanism
anchor.tswas corrected away from. Fixing the comment while leaving the refuted explanation in permanent history preserves the thing a future maintainer will actually find viagit log/git blame. - PR body, How verified: "no new errors under
packages/docs/src/(baseline is zero there)". The baseline is 2, not zero —src/editor/EditorShell.test.tsx(106,58)andsrc/members/sort.ts(26,9), both present at merge-baseb5f38b9fand both still present at this head. The delta is genuinely zero, which is the claim that matters; the stated baseline is not right.
Fix: amend the commit body on the next push.
🟡 P2-4 — no test executes a changed line
The cited 31 passing tests are anchor / api / roles / the two hooks; there is no CommentPanel.test.tsx. packages/docs already has @testing-library/react and sibling panel tests (html/HtmlDocCommentPanel.test.tsx, html/HtmlMemberPanel.test.tsx), so the precedent and the harness both exist. The precise regression this PR fixes — label rendered instead of the quote — stays unguarded, as do anchorText === '' (renders a struck-through “…” beside the label) and label/quote/badge ordering.
Fix: three assertions — orphaned + snapshot asserts both spans present; orphaned + empty snapshot; orphaned + resolved ordering.
🟡 P2-5 — the snapshot is clipped to one line with no way to read the rest
ANCHOR_TEXT_MAX = 512 (anchor.ts:47), but the measured quote box is 199px orphaned+resolved and 237px orphaned-only — roughly 16–19 CJK glyphs, or ~27 Latin characters at 12px. Everything past that is text-overflow: ellipsis with no tooltip and no expansion.
For a live thread that is fine, because the full text is in the document. For an orphaned thread the snapshot is the only surviving record of what was commented on, which is the entire premise of #1181 — so clipping the large majority of it leaves the reader close to where the issue found them. The sibling panel in this repo already solves this: html/HtmlDocCommentPanel.tsx:180 sets title={quoteText} on its quote.
Fix: title={thread.anchorText} on the quote span, or two lines via -webkit-line-clamp: 2 in the .is-orphaned case.
3. What I checked that holds up
Recorded so the next round does not redo it.
- Anchor resolution is behaviourally untouched, as claimed. Only comment lines changed;
if (from == null || to == null) return nullandif (from === to) return nullare byte-identical, and the orphan predicate is unchanged. - No injection surface.
anchorTextrenders as a React text child (auto-escaped) and originates from a plain-textdoc.textBetweensnapshot. NodangerouslySetInnerHTML. - Nothing keys off the quote span's absence.
.octo-comment-quoteoccurs in exactly five places repo-wide:CommentPanel.tsx:217,SheetCommentPanel.tsx:227, and three CSS rules. No test selector or decoration depends on the orphan branch omitting it. - Shared-CSS blast radius is safe.
SheetCommentPanel.tsx:221,227also uses.octo-comment-anchor/.octo-comment-quote, somin-widthlands there too — and is a no-op there for the sameoverflow: hiddenreason. No sheet regression. flex-shrink: 0on the orphan label is genuinely required, not cargo-cult: zh-CN(已失效)would otherwise shrink and wrap, CJK min-content being ~1 glyph. Measured at 60.0px intact.- No falsy-render hazard.
orphanedis a real boolean (CommentPanel.tsx:181), so{orphaned && …}cannot leak a0. - Spacing between the two now-adjacent spans is covered by
gap: 8pxon.octo-comment-anchor(styles.css:2765); JSX strips the newline between them. - Accessibility shape is right: state is carried by the visible text label, with the strike-through as redundant decoration — not the reverse.
- Executed at this head:
pnpm run build✅; comments suite 31 passed / 5 files ✅ (run frompackages/docs; the repo-root vitest invocation fails on an unrelated@douyinfe/semi-iconsCJS parse error);pnpm -w run i18n:check✅0 candidates within baseline;tsc --noEmitdelta vs merge-base 0.
4. Coverage — what I did not verify
- Chromium only. Not Safari or Firefox, not a real device, and not at the drawer's
max-width: 92vwmobile width, where the quote box shrinks further and P2-5 gets worse. - No screenshots. Light/dark captures of an orphaned thread, and one orphaned and resolved with a long zh-CN snapshot, would settle P2-1 in a single image each.
- The version-restore reachability of the null branch is reasoned, not executed — from Yjs relative-position semantics plus this repo's own tests, not from performing a restore against a live TipTap binding. The
anchor.tscomment's conclusion is well supported; I did not reproduce the scenario end to end. - e2e not run in this environment.
- Did not review
html/HtmlDocCommentPanel.tsxbeyond the shared-CSS check. Worth noting:179-186has the same mutually-exclusivequote ? … : labelshape this PR is fixing here. The semantics differ — its quote resolves live and its label is a positional descriptor — so it is not the same defect, but it is where to look if #1181's pattern is suspected elsewhere.
5. Verdict
CHANGES_REQUESTED.
The user-facing behaviour is right, the fix is the correct shape, and the two blockers from last round were addressed properly — the contrast fix is verified, and the anchor.ts rewrite is accurate down to its citations. Thanks for both.
What blocks is a single carried-forward P1: min-width: 0 and its three-line rationale (styles.css:2780-2783) are unchanged from the previous head, and browser measurement refutes all three claims the comment makes. In a PR where two of three hunks are documentation, a comment that confidently states a false CSS rule is the defect — the inert declaration itself is harmless. The fix is a four-line delete or reword with no behaviour change.
P2-1 (one CSS line) and P2-3 (amend the commit body) are cheap enough to fold into the same push. P2-2, P2-4 and P2-5 are fine as fast-follow.
For the record, this is round 3. The remaining blocking work is about six lines and needs no further behavioural review — a re-read of the CSS block and the commit body should be enough to land it.
Closes #1181
Summary
An orphaned comment (its anchored text deleted) replaced the quote row with the
(orphaned)label, so the reader learned the comment was dead but not what ithad been commenting on. Label and quote now render together; the quote is struck
through and dimmed when orphaned.
Two supporting fixes ride along: a comment documenting why the null-resolve
orphan branch in
resolveAnchorRangeis not dead code, and the flexmin-widththat prevented the quote from ellipsizing.
Linked Spec
Issue #1181 (filed with the code-level evidence:
anchor.ts:148/:180andmigrations/schema.sql:193all designateanchorTextas the orphan-casedisplay snapshot).
How verified
npx vitest run src/comments/→ 31 passed (5 files)pnpm -w run i18n:check→0 candidates within baseline; locale keys healthynpx tsc --noEmit→ no new errors underpackages/docs/src/(baseline iszero there; the ~3.9k lines of pre-existing output are all dmworkbase's
untyped react@17)
comment.orphanedalready exists in both locales(zh-CN.json:409 / en-US.json:409)
COMPREHENSION
What this does to the load-bearing path. Nothing in the anchor resolution
path changes behaviourally.
resolveAnchorRangereturns exactly what itreturned before — the only edit there is a comment. The orphan decision
(
orphaned = ready && thread.anchorStart != null && range == null) isuntouched. What changes is purely what the panel renders once that decision is
made: previously one of two mutually exclusive spans, now both.
What it could break. The quote span is now always mounted, so anything that
keyed off its absence to detect an orphan would break. Nothing does — the
orphan class name
.octo-comment-orphanis still the marker and is stillrendered.
anchorTextis read only for display here; it is never fed back intopositioning, so a stale snapshot cannot mis-locate anything. The CSS additions
are scoped to
.octo-comment-quote.is-orphanedand amin-width: 0on.octo-comment-quote— the latter is the one line with reach beyond the orphancase, and its effect is to permit the ellipsis that the rule already asked for
(
overflow: hidden; text-overflow: ellipsis) but could not deliver as a flexchild.
What verified it. The comments suite (31 tests, including the 6 anchor
tests that lock the orphan-detection contract) plus the typecheck delta against
a recorded baseline. Not verified by screenshot: this is a display-layer change
and the strike-through/dim rendering has not been eyeballed in light and dark on
a real device. Say so and I will attach both.