Skip to content
This repository was archived by the owner on Aug 6, 2026. It is now read-only.

trunk-merge/pr-3910/bc6be952-7e2a-45cd-9590-1a43de53b758 - #3915

Closed
trunk-io[bot] wants to merge 9 commits into
mainfrom
trunk-merge/pr-3910/bc6be952-7e2a-45cd-9590-1a43de53b758
Closed

trunk-merge/pr-3910/bc6be952-7e2a-45cd-9590-1a43de53b758#3915
trunk-io[bot] wants to merge 9 commits into
mainfrom
trunk-merge/pr-3910/bc6be952-7e2a-45cd-9590-1a43de53b758

Conversation

@trunk-io

@trunk-io trunk-io Bot commented Jul 29, 2026

Copy link
Copy Markdown
Trunk Merge Pull Request Banner

This pull request was created and is being managed by Trunk Merge.

This pull request is based on the main branch at SHA 7d1d9d7b0cb1e6f19531f7813d3c7b7a84adecc5.

See more details about each PR in the batch here:

When CI completes, this pull request will be closed automatically.

Pull Requests Being Tested

This pull request is testing a batch with the changes from pull requests 3910 and 3909 - batching documentation.

puemos and others added 9 commits July 29, 2026 08:12
…targets

The activity timeline mixed a `sm` avatar on the "created this task" row with
`lg` ones on every row below it, and quill's `ThreadItemAuthor`/`ThreadItemBody`
default to 14px while the pane's own copy is 13px — so a row's name read as a
heading over its own message.

- Every timeline node is now a `sm` avatar (or the matching `size-6` status
  bubble), centered in the 2.5rem gutter; the vertical line moves to 1.75rem to
  stay under them.
- Author names and body copy share one 13px size across the pane.
- PR cards in the timeline and the Artifacts tab now open the in-app review
  split when clicked, with a dedicated trailing button for GitHub. Previously
  the timeline card only ever left the app and the artifacts row only ever
  stayed in it.


Generated-By: PostHog Code
Task-Id: 3da5213f-90f2-4123-bbbd-c648c7b08e06
The pane had three typographic treatments and two row skeletons competing, so
nothing lined up. It now has exactly two row species:

- Lifecycle events (task created, run finished) are an icon bubble plus a single
  muted line. "Task created" gets its own icon rather than the creator's avatar,
  so it reads the same as "Task completed" instead of impersonating an authored
  row.
- Authored rows (messages, PR/canvas artifacts) keep the avatar and author line,
  with content on its own line below.

Also:
- Message rows preview the first non-empty line and truncate, rather than
  clamping four lines of wrapped text. The Comments tab still shows messages in
  full — that's where you read them.
- Content sits 0.375rem below its header, so the PR card is no longer jammed
  against its title.
- Timestamps ride a right-edge column via `ThreadTimestamp`'s className. The
  panel-level `[data-slot=thread-item-timestamp]` override this replaces never
  matched: quill's `TooltipTrigger` overwrites the wrapped element's `data-slot`
  with its own, which is why timestamps had been hugging the names.


Generated-By: PostHog Code
Task-Id: 3da5213f-90f2-4123-bbbd-c648c7b08e06
…pe pass

Typography:
- Timestamps drop to 11px, a step below the row's 13px copy.
- Event rows take the same text colour as every other row. Only the icon
  distinguishes them now, so the pane reads as one typographic system rather than
  greying out its own lifecycle markers.

Clicking a message row scrolls the transcript to that message. The Activity pane
is a sibling of the transcript, so it can't reach the scroller's context or the
windowed body's jump callback — a small `threadNavigationStore` carries the
request and each transcript serves it with its own jump, mirroring how
`reviewNavigationStore` already brokers scroll-to-file between the changes list
and the review pane.

Both transcripts are wired, since which one renders depends on the
`useNewChatThread` setting: `ConversationView` reuses its existing
`handleJumpToMessage` (grouped-row index), and `ChatThread` gets a bridge inside
`ChatMessageScrollerProvider` that prefers the windowed body's `jumpToMessage`
and falls back to the engine's `scrollToMessage`. Both panes derive items from
the same `useSessionViewState` events, so the conversation item ids match.

Only message rows are clickable — lifecycle events and artifacts have nothing to
scroll to. The row carries the button role itself: quill's `ThreadItem` renders an
<article>, which a <button> may not wrap and which takes no `render` prop.


Generated-By: PostHog Code
Task-Id: 3da5213f-90f2-4123-bbbd-c648c7b08e06
…ivity pane diff

Self-review pass. One bug, the rest is fat.

The bug: `ActivityTimeline` renders in two surfaces, and only one of them mounts
the task's own view. In the channel-home sidebar (`WebsiteChannelHome`) there is
no `TaskDetail`, so opening a PR "in place" set review state nobody reads and a
scroll request nobody serves — both silently did nothing, and the PR card used to
open GitHub. A `canOpenInPlace` flag, set only by the task route where the
transcript and review pane sit alongside, now gates both: without it message rows
stay inert and PRs open externally. That also fixes the same pre-existing dead
click on the Artifacts tab's PR row, which is where the pattern came from.

Trimmed:
- Dropped four exported `THREAD_*_CLASS` constants in favour of the literal
  Tailwind classes. Each cost a docblock, an export and a cross-file import to
  avoid repeating one class; the timestamp's alignment moved into
  `ThreadTimestamp`, which owns timestamps, so no caller passes classes at all.
- Replaced `messagePreview()` and its tests with `line-clamp-1
  whitespace-pre-wrap`. The file already clamped at four lines; changing the
  number is the whole edit.
- `ThreadTimeline` no longer takes a `taskId` it only forwarded, and
  `ArtifactCardButton`/`ArtifactListRow` derive the external button's label from
  the title instead of taking a prop for it.
- Folded the scroll-request hook into `threadNavigationStore` (never used apart)
  and cut its test from four cases to two.
- `ThreadScrollRequestBridge` loses a `useCallback` and a branch: the hook's
  effect no-ops unless a request is pending, so `jumpToMessage ?? scrollToMessage`
  is enough. Kept the bridge rather than dropping it — `useNewChatThread` is
  off by default, but cutting it would reintroduce exactly the dead click above
  for anyone who flips that setting.

485+/145- down from 562+/140-, with the bug fix included.


Generated-By: PostHog Code
Task-Id: 3da5213f-90f2-4123-bbbd-c648c7b08e06
…ts contents

ReviewHog caught a real a11y regression in this PR. The row's `aria-label` was
`Jump to message from ${name}`, and an explicit label on a widget role replaces
the name that would otherwise be composed from the row's children — so a screen
reader announced neither the timestamp nor the content preview a sighted user
sees. Worse, every conversation row is attributed to the task's creator, so that
label was byte-identical on every row and gave no way to tell them apart.

Dropping the label is the fix: `role="button"` takes its name from its contents,
so each row now announces its own author, time and preview. The avatar gutter is
marked `aria-hidden` so the initials don't double up on the name written beside
them.

Adds `ActivityTimeline.test.tsx`, which had no coverage before: it asserts the
computed accessible names are distinct and carry each row's content (the old label
fails it), that clicking files a scroll request for that item, and that rows stay
inert without a transcript alongside.


Generated-By: PostHog Code
Task-Id: 3da5213f-90f2-4123-bbbd-c648c7b08e06
…ass tests

Per Shy and Adam: the timestamp stays at 11px but sits next to the actor again
rather than being pushed to the row's right edge. `ml-auto`/`pl-2` come off; the
header's own gap handles the spacing, which is where the timestamp sat before this
PR touched it. Everything else about the row stays as it is.

Also deletes three tests of mine that were asserting Tailwind classes rather than
behaviour — including an `expect(time).not.toHaveClass("ml-auto")`, which pinned
the absence of an arbitrary class and so had no failure mode matching a real bug.
jsdom computes no layout, so a clamp or an alignment simply cannot be verified
there; those assertions only restated the source, and would have failed the moment
someone renamed a utility without changing what a user sees.

What's left in `ThreadMessageRow` is the one behavioural fact worth holding: the
full message text renders, so no future change quietly reintroduces slicing the
string. Presentation is left to review and to the visual-regression suite.


Generated-By: PostHog Code
Task-Id: 3da5213f-90f2-4123-bbbd-c648c7b08e06
The list's render branches were independent conditions rather than a mutually
exclusive chain, so a cold load with a filter active drew the skeleton and the
Recent section's "No matches" empty state at the same time.

The list now has one loading state covering pinned and recent together: until
items arrive, a single skeleton speaks for the whole list, and a zero-length
result is only reported as empty once loading settles. Also fixes "No matches"
flashing while items are still arriving from the other queries, and the two
empty states stacking when a filter or search is set on an empty space.

Generated-By: PostHog Code
Task-Id: 3bebbb5d-6e22-46c9-bacc-1c6d60bc9539
@github-actions

Copy link
Copy Markdown

React Doctor found no issues in the changed files. 🎉

Reviewed by React Doctor for commit 3aecd7b.

@trunk-io trunk-io Bot closed this Jul 29, 2026
@trunk-io
trunk-io Bot deleted the trunk-merge/pr-3910/bc6be952-7e2a-45cd-9590-1a43de53b758 branch July 29, 2026 09:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant