Skip to content

Fix triple-click highlight bleed into the next line - #1408

Draft
dca123 wants to merge 2 commits into
backnotprop:mainfrom
dca123:fix/triple-click-boundary-whitespace
Draft

Fix triple-click highlight bleed into the next line#1408
dca123 wants to merge 2 commits into
backnotprop:mainfrom
dca123:fix/triple-click-boundary-whitespace

Conversation

@dca123

@dca123 dca123 commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Summary

  • trim whitespace-only boundary nodes before web-highlighter paints a selection
  • preserve whitespace inside genuine multi-node selections
  • prevent Chromium triple-click ranges from producing a padded mark on the next line

Validation

  • pnpm exec tsc --noEmit -p packages/ui/tsconfig.json
  • pnpm --dir apps/hook run build
  • headless Chromium reproduction: baseline wrapped trailing indentation; fixed selection did not

@backnotprop

Copy link
Copy Markdown
Owner

Thanks @dca123, the root-cause analysis here is exactly right. A triple-click leaves the range's end boundary on the next block's text node at offset 0, web-highlighter's splitText(0) hands the painter an empty text node, and wrapping that in a styled mark is the sliver on the following line. Trimming only the boundary nodes (and leaving whitespace between real nodes alone) is the right shape for the fix.

Since maintainer edits are allowed, we pushed three small changes straight to your branch rather than sending you round again:

  1. HookCallback signature. The tap callback was typed (_id, selectedNodes: SelectedNode[]), but HookCallback<T> is (...args: unknown[]) => T, which is what failed Test and Release with TS2345. It now taps with a rest-args callback and reads args[1].

  2. Local structural type instead of the deep import. @plannotator/web-highlighter/dist/types resolves today only because the fork ships no exports map. We have a fork patch queued that may add one, and this file is on the published @plannotator/ui consumer surface (packages/ui/tsconfig.strict-consumer.json), so a republish could have broken consumers compiling from source. The helper moved to packages/ui/utils/selectionBoundary.ts with a minimal SelectedNodeLike covering just the field it reads, same precedent as packages/ui/webmcp/modelContext.ts declaring local structural types instead of depending on webmcp-types.

  3. A test that actually runs in CI. useAnnotationHighlighter.test.tsx is DOM-gated (skipped unless DOM_TESTS=1), so a DOM test there would have guarded nothing on PRs. The helper being its own pure module made a plain unit test possible: packages/ui/utils/selectionBoundary.test.ts covers the empty boundary node, pure indentation, leading and trailing trim, whitespace preserved in the middle, an all-whitespace list, and an ordinary selection left untouched.

Your behavior and naming are unchanged, the trim logic is byte-identical to what you wrote.

Heads-up on something adjacent: an independent fork patch handles a different boundary shape, where the end container is an element and the offset equals childNodes.length. That lands separately and does not overlap with this change.

Typecheck and the ui suite are green locally; CI is running on the updated head.

AI-assisted (Claude) under maintainer direction.

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.

2 participants