Skip to content

feat(share): floating share bar for text selected in posts and comments - #6352

Open
tsahimatsliah wants to merge 24 commits into
claude/website-sharing-visibility-be6b32from
claude/share-text-selection
Open

feat(share): floating share bar for text selected in posts and comments#6352
tsahimatsliah wants to merge 24 commits into
claude/website-sharing-visibility-be6b32from
claude/share-text-selection

Conversation

@tsahimatsliah

@tsahimatsliah tsahimatsliah commented Jul 22, 2026

Copy link
Copy Markdown
Member

Stacks on PR 1 (#6343) — base branch is claude/website-sharing-visibility-be6b32, not main.

What changed

Selecting text inside a post body, a comment, or a Happening Now summary raises a portaled floating bar anchored to the selection.

Ships to everyone — there is no feature flag and no experiment.

Actions

Action Behaviour
Copy link The post's link, or the comment's own permalink when the selection is in a comment. Mobile hands off to the native share sheet. Icon swaps to a green check for a second.
Copy text The selection, followed by a blank line and the link, routed through getShortUrl so a copied quote carries the same referral credit as a plain copy-link. Same check animation.
Quote Hands the selection to a comment composer as a markdown blockquote, then scrolls it into view. On a comment this opens a reply to that comment, with the quote above the @mention.
Share The standard ShareActions popover with the full social row.

Quote is only rendered where a composer exists to receive it — hidden on briefings and Happening Now, and on a comment whose reply composer is not wired.

Where it appears

PostSelectionArea binds the bar to a surface's readable content only — title, TL;DR and body. Its wrapper is display: contents, so it adds a node for Node.contains without adding a layout box.

Surface Component Quote
Article · video · live room PostContent
Freeform · welcome · share · YouTube SquadPostContent
Post redesign PostFocusCard
Collection CollectionPostContent
Poll PollPostContent
Social / Twitter SocialTwitterPostContent
Presidential briefing BriefPostContent — no composer
Comments and replies CommentContainer
Happening Now HighlightItem — no composer

Page and modal resolve to the same components (CONTENT_MAP and getPostModalRenderConfig), so there is no surface where one works and the other does not.

Digest posts are excluded on purpose. A digest is a header plus an embedded feed of other posts, so a selection there would quote someone else's headline under the digest's link.

Nav, source strips, tags, metadata, action bars and the comment section are all outside the bound region — selecting a reply used to raise a bar that would quote a commenter as if they were the post author.

New files

File Purpose
hooks/useTextSelectionShare.ts Detects selection end inside a container, exposes the trimmed text and a viewport rect, follows it on scroll/resize (capture phase, so inner scrollers count), clears on collapse.
components/post/SelectionShareBar.tsx The portaled bar.
components/post/PostSelectionArea.tsx Scopes the bar to a surface's readable content.
components/share/CopyStateIcon.tsx Copy → check swap, shared with #6369.
components/post/QuoteImageCard.tsx The 1200×630 card the screenshot service renders.
pages/image-generator/quote/[id].tsx ISR page marked id="screenshot_wrapper".

QuoteImageCard and its route are parked: the bar offers no quote-image action until the screenshot service serves the PNG. They stay so the follow-up is a wiring change.

Behaviour details

  • Anchored position: fixed, centred over the selection, flipping below when there is under 64px above it, clamped horizontally so it never leaves the viewport.
  • Clamped to visualViewport width and offsetLeft — pinch-zoom pans the visual viewport without moving the layout viewport a fixed element sits in.
  • Dismisses on click-away, Escape, and selection collapse. An open share popover holds the bar open, since the popover portals out of it.
  • Anchoring and the reveal animation live on separate elements: animate-composer-in animates transform with animation-fill-mode: both, which otherwise overrides the anchoring translate permanently.
  • Every control is a real <button type="button"> with an aria-label; the bar is role="toolbar".

Also here

  • ShareActions popover: heading removed, four fixed columns sized to content so padding is equal on all sides, short last row left-aligned. Affects the three other consumers of that popover.
  • NewComment accepts a ?comment= draft on any post type, not just welcome/poll, with an optional ?commentOrigin so the logged origin stays accurate.
  • The highlights list query fetches the fields postLogEvent reads, so shares from there are not hollow events.

Storybook

28 stories under Components/Share/SelectionShareBar covering placement, clamping, mobile, selection shapes, all wired surfaces, the ignored cases, and dismissal. Each fakes a selection on mount, so every state is visible without dragging a cursor.

Preview domain

https://claude-share-text-selection.preview.app.daily.dev

Selecting text inside a post body raises a portaled floating bar anchored to
the selection with three actions: copy link to the post (native share sheet on
mobile), copy the selection itself, and generate a shareable quote image.

- `useTextSelectionShare` detects selection end inside a container, exposes the
  text plus a viewport rect, follows it on scroll/resize and clears on collapse
- `SelectionShareBar` portals the bar to the root, clamps to the visual
  viewport (pinch-zoom/Android), flips below the selection when there is no
  room above, dismisses on click-away and Escape, and respects reduced motion
- `QuoteImageCard` + `/image-generator/quote/[id]` follow the existing
  screenshot-service pattern (`#screenshot_wrapper`, ISR) and carry
  `og:image` / `twitter:card=summary_large_image`
- Wired into all three post bodies: classic `PostContent`, `SquadPostContent`,
  and the redesign `PostFocusCard` (post page and post modal)

Gated by the new `share_text_selection` flag AND the `sharing_visibility`
master gate. Flag-off mounts none of the hooks, so no listeners are attached.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@vercel

vercel Bot commented Jul 22, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
daily-webapp Ready Ready Preview Jul 29, 2026 7:23am
storybook Building Building Preview Jul 29, 2026 7:23am

Request Review

Opening the raw /image-generator/quote page landed users on a bare 1200x630
bitmap template with no download, share or back affordance. The route,
QuoteImageCard and the URL builder stay for the follow-up that previews and
shares a real PNG once the screenshot service serves the route.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Tsahimatzliah and others added 8 commits July 26, 2026 11:20
Stories now fake the selection on load so every state is visible without
dragging a cursor: placement (above/flipped/clamped/scroll-follow), mobile,
selection shapes, the three host surfaces, the ignored cases, dismissal and
flag-off.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
animate-composer-in animates transform with animation-fill-mode: both, so
its final translateY(0) permanently overrode the inline
translate(-50%, -100%). The bar rendered un-centred and on top of the
selection instead of above it. Split anchoring and reveal onto separate
elements.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two new actions alongside copy link / copy text:

- Quote: renders the selection as a markdown blockquote and hands it to the
  comment composer via ?comment=, which NewComment already watches on every
  surface the bar appears on. The post-type gate there is relaxed so the
  hand-off works beyond Welcome/Poll posts, and an optional ?commentOrigin
  keeps the logged origin accurate without double-logging OpenComment.
- Share: the existing ShareActions popover with the full social row. It gains
  an overridable trigger icon and an onOpenChange callback so the bar can hold
  itself open while the (portaled) popover is up.

QuoteImageCard grows author and source imagery in its attribution row.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The bar sits directly over the selection, so a sentence-length tooltip
covers the text the reader is looking at. aria-labels keep the long form.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Users are rounded squares and sources are circles, matching ProfilePicture
and SourceAvatar. The avatar is sized against the attribution text beside
it rather than the card, so it reads as part of that strip instead of
dominating it. The quote mark takes the brand (cabbage) accent.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Coverage: the bar reached only PostContent, SquadPostContent and
PostFocusCard, so Collection, Poll, Brief and social/Twitter posts never
raised it — on the page or in the modal. All seven surfaces are wired now.

Scope: each surface bound the bar to its outer column, which via
BasePostContent also contains the whole comment section. Selecting a reply
raised the bar and quoting it attributed a commenter's words to the post.
PostSelectionArea now wraps title, TL;DR and body only. Its wrapper is
display:contents, so it adds a node to the DOM tree — all Node.contains
needs — without adding a box that could disturb flex or grid children.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Adds the four post types the bar newly reaches (collection, poll, brief,
social) plus stories proving comments, post chrome and digest feeds stay
out of bounds.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ tsahimatsliah
❌ Tsahimatzliah
You have signed the CLA already but the status is still pending? Let us recheck it.

… body

ArticleBody carries the auto-selection target, so both stories selected the
article instead of the thing they claim to ignore and showed a bar. They now
render it with the target removed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Removes share_text_selection entirely and drops the sharing_visibility gate
from the bar, so it renders for every user with no experiment. The
inner/outer component split existed only so a disabled experiment mounted no
hooks; with no gate it collapses into one component.

Tests and stories that asserted on the gate are replaced by ones asserting
the bar renders for everyone and only hides when there is no selection.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
- Copy link and copy text swap their icon to a check for the second that
  useCopyLink/useCopyText hold `copying`, cross-fading in a single grid cell
  so the button never changes width. Tooltip says "Copied!" alongside.
- A draft handed to the composer through ?comment= has no click behind it to
  move the page, so it opened unseen below the fold. focusInputById now
  scrolls it into view first and focuses without stealing that scroll.
  Optional-chained because jsdom has no scrollIntoView.
- The share popover tracks the bar every frame. The bar is fixed and
  re-anchors on scroll; the default strategy left the popover at a stale
  position until the next scroll/resize event.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Selecting text in a comment or reply now raises the bar, bound to that
comment rather than the post:

- Copy link yields the comment's own permalink, and sharing logs
  ShareComment with its id, so a reader never shares a commenter's words
  under the post's link.
- Quote opens a reply to that comment seeded with the blockquote. ReplyTo
  carries an optional quote and useComments puts it ahead of the @mention,
  so the cursor lands after the handle with the quote already above.
- A comment with no reply handler still copies and shares but will not
  quote — falling back to the post composer would misattribute it.

Bound to the comment's prose only, via the same display:contents wrapper as
the post body, so the header, badges and action row stay out of bounds.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The heading was redundant — the popover only opens from a share control.
Removing it also removes the asymmetry it created at the top.

The grid is now four fixed columns sized to its content. Wrapping a flex row
inside w-80 left 8px of slack that justify-center split between the sides, so
the left and right padding read wider than the top and bottom. A trailing
'Share via...' alone on its row still centres across all four columns.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Makes the popover layout reviewable without hovering, and gives the
storybook build something under packages/storybook to trigger on — the
Ignored Build Step skips it otherwise.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Tsahimatzliah and others added 2 commits July 27, 2026 16:32
The share popover's last row now stays left-aligned with the grid above it
instead of re-centring.

Replaces the copy-confirmation icon this branch had grown with CopyStateIcon
from PR 6369, taken verbatim apart from an optional idleIcon so the
selection bar can show a link glyph. Both branches touch ShareActions, so
one shared implementation keeps that merge small.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Copied prose travels — into a doc, a DM, a slide — and arrives with no idea
where it came from. Copy text now appends the link, routed through
getShortUrl with the share campaign so it earns the same referral credit a
plain copy-link would. On a comment the reference is that comment's
permalink, so the quote points at who actually said it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
/highlights showed readable prose with no way to quote it. The expanded
summary is now bound to the bar.

The headline is deliberately left out: it lives inside the expand/collapse
button, where a drag toggles the row instead of selecting text, so binding
to it would raise nothing.

The list query only fetched id/type/commentsPermalink, which was enough to
render but would have made every share from here a hollow log event. It now
also fetches the fields postLogEvent reads — still short of a full Post,
since this is a list query.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Quote hands the selection to a comment composer, so on a surface without
one it was a dead button. Briefings and the highlights list have no
comment section — BriefPostContent passes no engagementProps, and
/highlights is a list — so both now render the bar without it.

Same bug on the reply modal's quoted comment: CommentContainer there gets
no onQuote, so the button did nothing. A comment now offers Quote only when
its reply composer is actually wired, which is structural rather than a
silent no-op inside the handler.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three things the branch outgrew and nothing consumed:

- buildQuoteImageUrl and MAX_QUOTE_LENGTH, exported but never called since
  the quote-image action was dropped from the bar.
- ShareProvider.QuoteImage, an enum member no call site ever passed.
- useTextSelectionShare's `enabled` option, which existed so a disabled
  experiment attached no listeners. There is no experiment now, and no
  caller passed it; its test went with it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@tsahimatsliah tsahimatsliah changed the title feat(share): floating share bar for text selected in a post feat(share): floating share bar for text selected in posts and comments Jul 28, 2026
Three ways the bar disagreed with existing share tracking:

- A comment share used ReferralCampaignKey.SharePost, so its short link
  earned post-share credit. useShareComment uses ShareComment; the campaign
  now follows what is actually being shared.
- Copy link always logged provider 'copy link', even where
  shareOrCopyLink hands off to the native sheet, so every mobile share was
  logged as a copy. Resolved the way ShareActions resolves it.
- Quoting a comment logged origin 'post comment button', because
  useComments hardcoded it. ReplyTo now carries an origin and the quote
  action passes TextSelection.

Payload shapes were already right: SharePost sends { provider, origin } and
ShareComment sends { provider, origin, commentId }, matching
useLoggedCopyPostLink and useShareComment key for key. Three tests pin that.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@rebelchris rebelchris left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@claude please review this, some hooks seem heavily bloated?

// `focus()` jumps the scroll position instead of moving to it. Scrolling
// first, with focus not stealing the scroll, keeps that motion smooth.
// Optional-chained: jsdom does not implement `scrollIntoView`.
input.scrollIntoView?.({ behavior: 'smooth', block: 'center' });

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll have to monitor this doesn't make bad ux for people

@claude

claude Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Claude finished @rebelchris's task in 6m 20s —— View job


Review — selection share bar

Answering the question first: useTextSelectionShare is not the bloated part. It's 152 lines of which ~40 are comments, and it does one thing. The bloat is (1) SelectionShareBar itself, a 325-line component that mixes anchoring math, viewport tracking, dismissal, logging and three action flows, and (2) far more importantly — how many copies of that whole hook stack get mounted.


1. One full hook stack per comment (the real cost)

CommentContainer.tsx:199 renders a SelectionShareBar for every comment on the page. Each instance mounts:

Hook Cost per instance
useTextSelectionShare 4 always-on document listeners — mouseup, touchend, keyup, selectionchange (useTextSelectionShare.ts:117-129)
useOutsideClick a window click listener — it attaches unconditionally and checks enabled inside the handler (useOutsideClick.ts:9-24)
useVisualViewport a visualViewport resize listener
useGetShortUrl ×3 — directly at :99, plus twice more via useShareOrCopyLink → useCopyLink. Each creates a useQuery observer, all with queryKey: []
useRouter, useLogContext, usePostLogEvent, useCopyText, useToastNotification ×2

On a 150-comment thread that's ~750 document/window listeners and ~450 react-query observers, all for one bar that can only ever be visible once. The hot path is worse than the count suggests: selectionchange fires continuously while the reader drags, and every one of the N handlers calls window.getSelection(); each mouseup runs selection.toString() N times, which is O(selection length) per call.

PostSelectionArea.tsx:36 already stamps data-selection-area — currently unused. That's the shape of the fix: one watcher at the post/discussion root, resolve the owning surface by walking up from selection.anchorNode to the nearest [data-selection-area], read a data-comment-id off it for comment attribution, and render exactly one bar. That deletes the contents wrapper in CommentContainer, the per-comment hook stack, and makes the early return at SelectionShareBar.tsx:173 real (today ~10 hooks run before it).

Fix this →

2. SelectionShareBar should be split

Not a blocker, but the anchoring concern is self-contained and testable on its own: barWidth/viewportOffset state (:78-83), the measuring layout effect (:128-132), the visualViewport listener (:161-171) and the clamp/flip math (:177-188) are one unit — useSelectionAnchor({ rect, barRef }) → { left, top, flipsBelow }. What's left is a small component plus three action handlers.


3. Copy text will fail on Safari/iOS

SelectionShareBar.tsx:205-214 awaits getShortUrl — a GraphQL round-trip — and then calls copyText, which does navigator.clipboard.writeText (useCopy.ts:74). WebKit requires the clipboard write in the same task as the user gesture; after an awaited fetch the transient activation is gone and it rejects with NotAllowedError. useCopyText has no try/catch, so the reader gets no clipboard write, no toast, no check animation, and an unhandled rejection in the console.

The codebase already solved this — useCopyLink passes a promise to ClipboardItem precisely so the write is registered synchronously (useCopy.ts:41-46). Either reuse that pattern, or resolve the short URL when the bar opens and have the click handler be synchronous.

Fix this →

4. Quoting while logged out opens a composer that can't post

NewComment.tsx:125-154 acts on ?comment= without the auth check onCommentClick does at :157-159. The bar renders for everyone (per SelectionShareBar.spec.tsx:116), so an anonymous reader hits Quote, the composer opens with the draft, and posting fails. This path used to be a niche squad/poll deep link; the Quote button makes it a primary flow. Gate on usershowLogin({ trigger: AuthTriggers.NewComment }), and leave the query param in place so the draft survives the login round-trip.

5. viewportOffset starts stale

SelectionShareBar.tsx:80 seeds {left: 0, top: 0} and only ever updates on visualViewport scroll (:161-171). If the reader pinch-zoomed and panned before selecting — the common order on mobile — the first paint clamps against the wrong origin and both the clamp and the flip decision are off until they pan again. Read visualViewport.offsetLeft/offsetTop in the same layout effect that measures the width.

6. Nothing caps the selection length

The full selection goes into the URL at :237-248. A reader who selects a long passage gets a multi-KB URL and history entry (and the copied text and the parked quote card have the same unbounded input — the card line-clamp-6s it visually but still carries it). A MAX_QUOTE_LENGTH truncation at the source would be cheap.

7. as unknown as Post

HighlightItem.tsx:92-95 double-casts a deliberately partial highlight post. It's runtime-safe today — every field postLogEvent reads is optional-chained — but the cast means the day someone adds a required read there, highlights break silently. Type SelectionShareBar's prop as the fields it actually uses (Pick<Post, 'id' | 'title' | 'commentsPermalink' | 'permalink' | ...>) and the cast disappears along with the risk.

8. Dead code in a 3k-line PR

QuoteImageCard.tsx (149), pages/image-generator/quote/[id].tsx (109), its story (77) and its spec (91) are unreachable by the description's own admission ("parked… the bar offers no quote-image action"). CLAUDE.md is explicit: "Delete dead code — Remove unused components, functions, exports, and files. Don't leave code 'for later'." The route isn't inert either — it's a live public ISR page that reflects ?text= and fires a GraphQL request per uncached id. 426 lines that reviewers have to reason about for a feature that isn't shipping; better as the first commit of the follow-up PR.

9. Description doesn't match the bound region

Nav, source strips, tags, metadata, action bars and the comment section are all outside the bound region

PostContent.tsx:163-166 puts PostSourceInfo inside PostSelectionArea, and PostFocusCard.tsx:473-494 puts PostMetadata (domain link, read time) inside it. Selecting "5 min read" or the source name raises the bar. Either move them out of the wrapper or correct the claim — right now a reviewer trusting the table won't check.


Nits

  • MIN_SELECTION_LENGTH = 2 (useTextSelectionShare.ts:25-27) — the comment says it filters "single-word accidental selections (double-clicking a link, tapping a word)", but 2 is characters: every double-clicked word passes. Fix one or the other.
  • keyup on document (:119) runs readSelection — including getSelection().toString() — on every keystroke anywhere on the page, e.g. while typing in the composer the bar just opened. Gate on shiftKey/arrow keys.
  • isInsideContainer (:29-38) is a null guard around one contains call; inline it at both call sites.
  • SelectionShareBar.tsx:88 dereferences post.commentsPermalink while CommentContainer.tsx:168 guards post?.source. CommentMarkdownInputProps types post as required, so the guard is the wrong one — but pick a side.
  • buildCommentQuote (:51-55) is a string util exported from a component file; lib/strings.ts per CLAUDE.md.
  • ShareActions: onOpenChange only fires on the laptop branch — the mobile trigger at :110-120 never calls setOpen. Fine for this consumer (the bar only shows the popover on laptop) but worth a line saying so.
  • data-selection-area (PostSelectionArea.tsx:36) is written but never read. Use it (see §1) or drop it.
  • The width measurement lands in useLayoutEffect after the first paint (:128-132), so a bar that needs horizontal clamping can visibly jump one frame. visibility: hidden until measured, or measuring in a ref callback, avoids it.

What's good

The scoping decision is the right one and well argued — binding to the surface shell would have attributed replies to the post author, and display: contents is a genuinely neat way to get a Node.contains anchor without touching layout. Separating the anchoring element from the animated one because animation-fill-mode: both would pin the transform is the kind of thing that gets rediscovered in a bug report three months later; good that it's a comment. Comment-vs-post attribution is threaded consistently through the link, the campaign key, the log event and the quote target, and the spec covers exactly that (:210-264). Storybook faking the selection on mount rather than via play makes the states actually reviewable.

Not verified

node_modules isn't installed on this runner and I can't run pnpm install, so I did not run pnpm --filter shared test, pnpm --filter webapp test, lint, or typecheck-strict-changed.js. Findings above are from reading the code.
· claude/share-text-selection

@tsahimatsliah
tsahimatsliah marked this pull request as ready for review July 28, 2026 12:37

@capJavert capJavert left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review complete. Blocking issues remain in the current diff:

  1. CommentContainer mounts a full SelectionShareBar for every comment. Each instance installs document/window listeners and URL-shortening query observers, so long threads scale this work with comment count. Please move selection ownership to one page/discussion-level bar and resolve the selected comment from the nearest selection-area element.
  2. onCopyText awaits getShortUrl before calling navigator.clipboard.writeText. Safari/iOS requires the clipboard write to begin within the user-activation task, so this path can fail with NotAllowedError. Use the existing ClipboardItem-with-promise pattern or pre-resolve the URL.
  3. The ?comment= effect opens the composer without the normal anonymous-user login gate, so logged-out users can enter a quote flow they cannot submit.

The branch has no merge conflict, but GitHub currently marks it unstable because the CLA check is unresolved.

Addresses the blocking review points on #6352.

1. CommentContainer mounted a whole SelectionShareBar per comment — four
   document listeners, a window click listener, a visualViewport listener and
   three short-URL query observers each. A 150-comment thread paid all of that
   150 times for a bar that can only ever be visible once.

   SelectionShareProvider now owns one useTextSelectionShare and renders the
   single bar. Regions register an element and a target getter, and the hook
   resolves the innermost registered ancestor of the selection, so a comment
   nested in a post attributes to the comment. Registration is the whole cost
   per region. useTextSelectionShare takes a resolveArea callback instead of a
   containerRef, and resolves the region before calling selection.toString(),
   which is O(selection length).

2. onCopyText awaited getShortUrl before writing to the clipboard, which
   WebKit rejects once the gesture's activation has lapsed. The short URL now
   resolves through useGetShortUrl's query mode while the bar is open, so the
   click handler is synchronous.

3. The ?comment= effect opened a composer for logged-out readers who then
   could not post. It now calls showLogin and leaves the param in place, so
   the draft survives the round-trip.

Also from the review:

- Anchoring math extracted to useSelectionAnchor, which reads the
  visualViewport pan offset in the same layout effect that measures the bar
  instead of waiting for the first scroll — a reader who pinch-panned before
  selecting was clamped against a stale origin. The bar stays hidden until
  measured, so a clamped bar no longer jumps on its first frame.
- Quote text is capped before going into the URL.
- SelectionSharePost types what the bar actually reads, so the highlights
  double-cast is gone.
- Deleted the parked quote-image feature: QuoteImageCard, its ISR route, its
  story and its spec. It was unreachable and CLAUDE.md says not to keep code
  for later; it belongs to the follow-up that ships it.
- keyup only re-reads the selection for keys that can move it, so typing in
  the composer no longer runs getSelection().toString() per keystroke.
- MIN_SELECTION_LENGTH comment now says characters, which is what it counts.
- buildCommentQuote and the URL cap moved to lib/strings.
- data-selection-area is now load-bearing rather than decorative.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@tsahimatsliah

Copy link
Copy Markdown
Member Author

Thanks both — the per-comment hook stack was the right thing to lead with, and the Safari clipboard one would have shipped broken. All three blocking items are fixed in a8951ed, plus most of the rest.

Blocking

1. One watcher per page, not one per comment. SelectionShareProvider now owns a single useTextSelectionShare and renders the one bar. Regions register (element, getTarget) and the hook resolves the innermost registered ancestor of the selection, so a comment nested inside a post attributes to the comment. Registration is the entire per-region cost — no listeners, no query observers.

useTextSelectionShare takes a resolveArea callback instead of a containerRef, and resolves the region before calling selection.toString() — you were right that it's O(selection length), and most selections on a page are outside any watched region.

Providers mount at three roots: BasePostContent (post content + its comment list), PostFocusCard, and the highlights list. data-selection-area is load-bearing now rather than decorative.

2. Safari clipboard. onCopyText no longer awaits anything. The short URL resolves through useGetShortUrl's existing query mode while the bar is open, so the click handler is synchronous and the write stays inside the gesture task. I went with pre-resolving rather than ClipboardItem because it also removed a useState/useEffect pair and the act() warnings they caused in tests.

3. Logged-out quote flow. The ?comment= effect now calls showLogin({ trigger: AuthTriggers.NewComment }) and returns, leaving the query param in place so the draft survives the login round-trip and the effect picks it up again afterwards.

Also fixed

  • Split the bar — anchoring is now useSelectionAnchor(rect, barRef).
  • Stale viewportOffset — the pan offset is read in the same layout effect that measures the bar, so a reader who pinch-panned before selecting is no longer clamped against a stale origin.
  • First-frame jump — the bar is visibility: hidden until measured.
  • Selection length — capped before it goes into the URL, via truncateForUrl in lib/strings.
  • as unknown as Post — gone. SelectionSharePost types the fields the bar actually reads, which the highlights subset satisfies structurally. One documented cast remains at the postLogEvent boundary, since that signature demands a whole Post.
  • Dead codeQuoteImageCard, its ISR route, story and spec are deleted (426 lines). You're right that CLAUDE.md is explicit and that the route wasn't inert. It belongs to the follow-up that ships it.
  • keyup — only re-reads for keys that can move a selection, so typing in the composer no longer runs getSelection().toString() per keystroke.
  • MIN_SELECTION_LENGTH — comment now says characters, which is what it counts.
  • isInsideContainer — gone with the containerRef API.
  • buildCommentQuote — moved to lib/strings.ts.

Two I pushed back on

§9, the description. You're right that it was wrong, and I've corrected the description rather than the code. PostSourceInfo and PostMetadata are inside the wrapper deliberately — the source name and read time are part of what a reader might quote as context, and excluding them would mean two separate regions per surface for little gain. Flagging it so the claim and the code agree; happy to tighten it if you'd rather.

§Nit, post?.source vs post.commentsPermalink. Left as-is. CommentMarkdownInputProps types post as required, so the optional chain in CommentContainer is the wrong one, but it's pre-existing and not this PR's to change.

Verification

node ./scripts/typecheck-strict-changed.js, eslint on shared and webapp, and jest on all three packages: shared 296 suites / 2005 tests, webapp 44 / 297, extension 6 / 43. All green.

Two notes on the suite, neither caused by this PR: PopularPage + MyFeedPage fail when run as an isolated pair and pass in the full run (test-order pollution in the feed specs), and useFeeds / FurtherReading flake occasionally under full-suite load.

@rebelchris on the focusInputById scroll — agreed, worth watching. It only fires for a draft arriving via ?comment=, which has no click behind it to move the page, and it's behavior: 'smooth' with preventScroll: true on the focus so the scroll isn't fought. If it reads as jumpy in practice the cheapest change is dropping to block: 'nearest'.

One thing still open, unrelated to this review: the base is claude/website-sharing-visibility-be6b32 (#6343), so this can't reach users until that merges or this is retargeted to main. Worth deciding, since with the flag removed #6343's sharing_visibility kill-switch has no consumers left.

@capJavert capJavert left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed the latest refactor. The per-comment listener/query fan-out, Safari clipboard timing, and logged-out quote flow are addressed. I found no remaining code blocker. Approved; the unresolved CLA check still blocks merge.

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.

5 participants