Comment viewer fixes: rendering, dark treatment, composer UX#5
Merged
Conversation
Decouple the in-document highlight from the chrome toggle: the .jh-dark treatment now follows the document's sampled darkness, never the viewer's light/dark chrome choice. A highlight is painted ON the doc, so it must contrast with the page's real background — forcing light chrome on a dark doc previously flipped the highlight toward the light treatment and it stopped reading. This removes jh:setThemeMode entirely, so the "second jh:ready doesn't re-send the mode" gap no longer exists. Fix block-boundary anchors: an anchor whose first character starts a block (a heading, a paragraph) resolved its range to the block's leading edge, so wrapping pulled the whole element into an inline span whose background never paints — the highlight silently vanished. locateStart biases a boundary start into the next text node so we wrap the text, not the block. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
On dark documents, mark commented spans with a warm amber underline instead of a filled background wash — the fill read as muddy over dark backgrounds and fought the doc's own text. Hover and focus keep a faint transient wash for feedback only. Clicking "comment" in the selection toolbar now opens the rail, so the draft composer is visible even when the rail started closed (a doc with no comments on desktop, or any doc on mobile). Previously the draft landed in a hidden rail and the click looked like a no-op. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
AnnaXWang
changed the title
rgarcia
approved these changes
Jul 2, 2026
Cards are laid out at their highlight's absolute document Y, but the rail never
followed the document's scroll — so a comment anchored deep in a long doc left
its card stranded far below an otherwise-empty rail ("N comments" header with
nothing under it). The overlay now reports the document's scrollY, and on
desktop the rail scrolls in lockstep so each card sits next to its highlight;
the rail's content height is pinned to the document height so the sync has room
to move. On mobile (overlay drawer, no doc alongside) cards stack normally
instead of using absolute Y, so they're never stranded there either.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Three corrections to the desktop rail scroll-sync from review: - The cards list sits below the sticky header + doc-reaction/sign-in rows, but card Y is measured from the document top. Offset the rail scrollTop by the list's own offsetTop so a card lines up with its highlight instead of sitting that chrome-height below it. - Position the draft composer at the selection's document Y, so clicking Comment on a deep selection opens it next to the selected text instead of at the top of a list that's scrolled far away. - Reset the rail scrollTop to 0 when the viewport switches to mobile, so the drawer doesn't open scrolled past its stacked cards onto empty space. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
A late layout or resize can report a taller docHeight with unchanged scrollY. The rail's scrollHeight grows via minHeight but the sync effect didn't re-run, so a scrollTop the browser had clamped too low stayed there and cards drifted from their highlights until the next doc scroll. Add docHeight to the effect deps so scrollTop is reapplied. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit be66879. Configure here.
| // docHeight is a dep though unused above: when it grows (late layout / resize) the | ||
| // rail's scrollHeight grows with it, so a scrollTop the browser previously clamped | ||
| // too low must be reapplied — otherwise cards stay offset until the next doc scroll. | ||
| }, [docScrollY, docHeight, isMobile, railOpen]); |
There was a problem hiding this comment.
Rail sync ignores chrome height changes
Medium Severity
Desktop rail scroll is set from docScrollY plus [data-jh-cards] offsetTop, but that chrome height is only recomputed when docScrollY, docHeight, isMobile, or railOpen change. When rows above the cards list appear or disappear (e.g. the sign-in strip), offsetTop shifts while the document scroll offset is unchanged, so cards stay vertically misaligned with their highlights until the user scrolls the doc again.
Reviewed by Cursor Bugbot for commit be66879. Configure here.
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
2 participants
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.
Follow-up to #4 (merged). Fixes several comment-viewer issues found reviewing the merged theme toggle on real docs.
Fixes
Highlight decoupled from the chrome toggle. Add viewer theme toggle + fix washed-out dark-mode comment highlights #4 let the light/dark toggle drive the in-document highlight, but a highlight is painted on the document, so it must contrast with the document's background — forcing light chrome on a dark doc flipped it to the light treatment and it stopped reading.
.jh-darknow keys on the doc's sampled darkness only. Removesjh:setThemeModeentirely (no re-post gap on a secondjh:ready).Block-boundary anchors painted no highlight. An anchor starting at the beginning of a block resolved its range to the block's leading edge, so wrapping pulled the whole
<h3>into an inline<span>that paints no background — the highlight vanished (light mode too).locateStartbiases a boundary start into the next text node, so we wrap the text, not the block.Dark comments use an underline, not a wash. A filled amber background read as muddy over dark pages, so on dark docs the commented span is marked with a warm amber underline (depth = opacity), leaving the doc's own text untouched. Hover/focus keep a faint transient wash for feedback.
"Comment" now opens the rail. Clicking 💬 with the rail closed (a doc with no comments on desktop, or any doc on mobile) dropped the draft into a hidden rail. It now opens the rail so the composer is visible.
Rail scroll syncs to the document (fixes "empty rail / stranded card"). Cards are laid out at their highlight's absolute document Y, but the rail never followed the doc's scroll — a comment deep in a long doc left its card stranded below an empty rail. The overlay now reports
scrollY; on desktop the rail scrolls in lockstep. Specifically:offsetTop(the sticky header + doc-reaction/sign-in rows above it) so a card lines up with its highlight rather than sitting that chrome-height below it;scrollTop = 0on the switch to mobile, and on mobile cards stack normally (no absolute Y) so they're never stranded in the drawer.Presentation-only: overlay paint + shell UI. No API, DB, or anchoring-model changes.
/d/:slug/rawstays byte-pristine.Verification
tscclean ·vitest108/108 ·next buildcleanscrollY/docHeight).Note
The theme toggle themes the viewer chrome; it can't repaint the author's sandboxed HTML, so a fixed-dark doc stays dark (
autokeeps the chrome matched). Forwarding the choice toprefers-color-schemedocs is a possible follow-up.If the docs-style alignment keeps being finicky, the robust fallback is stacking cards in document order (no absolute Y) with click-to-scroll-into-view — happy to switch to that.
🤖 Generated with Claude Code
Note
Low Risk
Presentation-only overlay and shell UI; no API or persistence changes. Scroll-sync edge cases (e.g. highlights near the sticky header) may need manual QA.
Overview
In-document highlights no longer follow the viewer chrome light/dark toggle:
jh:setThemeModeis removed andhtml.jh-darkis driven only by the doc’s sampled background. On dark pages, highlights switch from a filled amber wash to underline-by-depth (hover/focus keep a light wash).Anchor painting uses
locateStartso range starts on text-node boundaries don’t wrap whole block elements (e.g. headings), which previously made highlights disappear.Comment rail (desktop) syncs scroll with the iframe: the overlay sends
scrollY(and existingdocHeight) injh:positions; the rail setsscrollTopwith a chrome offset so Y-aligned cards track their highlights. The draft composer is placed at the selection’s document Y; mobile stacks cards (aligned={false}), clears desktop margins, and resets rail scroll. 💬 Comment on the selection toolbar callssetRailOpen(true)so the composer isn’t hidden when the rail starts closed.Reviewed by Cursor Bugbot for commit be66879. Bugbot is set up for automated code reviews on this repo. Configure here.