fix: restore text-select in the Markdown preview (re-vendor client v0.16.3) - #148
Merged
Conversation
….16.3) Selecting a phrase in the rendered-Markdown Preview view no longer showed the floating Comment button, so you couldn't comment on rendered text — only the raw/diff view worked. Root cause: the block-surface text-select logic (the `data-surface="block"` branch in the client's textOffsetsFromSelection, which resolves a selection to the touched [data-from]/[data-to] blocks' source line range) lived on an un-merged client branch. It was vendored in when rendered-Markdown comments shipped, then silently lost on a later re-vendor from a client `main` that never had it — so the directive fell through to the code-surface path, looked for [data-line] (which .md-rendered blocks don't have), returned null, and hid the button. Fix: the block-surface support was merged into the client and released as v0.16.3 (v0.16.2 + that one commit — no other client changes); this re-vendors it via `make sync-client`. Guard: the regression slipped because the browser e2e that catches it (TestE2E_TextSelectRenderedMarkdown) is skipped in CI. Added a plain `go test` (TestVendoredClientHasBlockTextSelect) that asserts the vendored bundle still carries the block-surface branch, so a future re-vendor that drops it fails the build in CI. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ey41dRwDpRgLtVHZjhYLKt
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.
The bug
Selecting a phrase in the rendered-Markdown Preview view no longer showed the floating Comment button, so you couldn't comment on rendered text — only the raw/diff view worked. Reported while testing the #110 preview work; it is not caused by #110 (it reproduces on
main).Root cause
The block-surface text-select logic — the
data-surface="block"branch in the client'stextOffsetsFromSelection, which resolves a native selection to the touched[data-from]/[data-to]blocks' source line range — lived on an un-merged client branch. It was vendored in when rendered-Markdown comments shipped (#84), then silently lost on a later re-vendor (the v0.16.x client bump for read-progress, #128) from a clientmainthat never had it. Without it, the directive fell through to the code-surface path, looked for[data-line](which.md-renderedblocks don't have), returnednull, and hid the button.Evidence: the #84-era vendored bundle had 4
data-surfacereferences; the current one had 2.Fix
make sync-client.Guard (why this slipped)
The browser e2e that catches it (
TestE2E_TextSelectRenderedMarkdown) does fail on the regression — but browser e2e are skipped in CI, so nothing gated the re-vendor. Added a plaingo test(TestVendoredClientHasBlockTextSelect) that asserts the vendored bundle still carries thedata-surface="block"branch. It runs in CI and would have caught this exact regression at re-vendor time. (Sentinel verified: present in v0.16.3, absent in the regressed bundle.)Verification
TestVendoredClientHasBlockTextSelect— pass (fails against the regressed bundle).TestE2E_TextSelectRenderedMarkdown— now passes (was failing onmain); full text-select e2e suite green.go build ./...clean.🤖 Generated with Claude Code