Skip to content

fix: restore text-select in the Markdown preview (re-vendor client v0.16.3) - #148

Merged
adnaan merged 1 commit into
mainfrom
fix/preview-textselect
Jul 9, 2026
Merged

fix: restore text-select in the Markdown preview (re-vendor client v0.16.3)#148
adnaan merged 1 commit into
mainfrom
fix/preview-textselect

Conversation

@adnaan

@adnaan adnaan commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

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's textOffsetsFromSelection, 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 client main that never had it. Without it, 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.

Evidence: the #84-era vendored bundle had 4 data-surface references; the current one had 2.

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 PR re-vendors it via make sync-client.
  • The only file delta is the client JS bundle (CSS was byte-identical).

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 plain go test (TestVendoredClientHasBlockTextSelect) that asserts the vendored bundle still carries the data-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).
  • e2e TestE2E_TextSelectRenderedMarkdown — now passes (was failing on main); full text-select e2e suite green.
  • Full non-browser suite green; go build ./... clean.
  • The client delta is exactly the block-surface addition, so read-progress / heartbeat / other client behaviour is unchanged.

🤖 Generated with Claude Code

….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
@adnaan
adnaan merged commit 40d1c62 into main Jul 9, 2026
7 checks passed
@adnaan
adnaan deleted the fix/preview-textselect branch July 9, 2026 18:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant