Skip to content

feat(widget): show a New reply button while reading earlier messages - #447

Open
srujankaleru2007 wants to merge 2 commits into
codercops:devfrom
srujankaleru2007:issue-420-widget-new-reply-pill
Open

srujankaleru2007 wants to merge 2 commits into
codercops:devfrom
srujankaleru2007:issue-420-widget-new-reply-pill

Conversation

@srujankaleru2007

Copy link
Copy Markdown
Contributor

feat(widget): show a New reply pill while reading earlier messages

What does this change?

Show a New reply ↓ button at the bottom of the log when a reply arrives while the visitor reads earlier messages. Clicking it scrolls to the latest reply smoothly, or instantly with reduced motion; returning to the bottom by hand also hides it.

The button is outside the live log, reuses theme and focus styles, and uses logical positioning for RTL. Adds the newReply locale string with English fallback, support for updating that label, documentation, the API snapshot, regression tests, and a minor widget changeset. Extends private-property minification to keep the existing bundle budgets unchanged.

Closes #420

How I tested this

  • Widget coverage: 476 tests passed, with 97.46% lines and 90.02% branches.
  • Both acceptance Chromium regressions pass: the existing scroll-preservation test now checks pill visibility; the new keyboard test checks the focus ring, overlay placement, scroll-to-bottom, and hiding.
  • Confirmed the new pill test fails against the original upstream renderer; restored the exact feature source afterward.
  • pnpm -r build and pnpm -r typecheck pass. On Windows, the existing build script needs Git's Unix utilities on PATH for rm.
  • pnpm size passes: panel 16.92 kB / 17 kB, single-file widget 18.99 kB / 19 kB gzipped.
  • pnpm lint exits 0. Its package globs are unsupported on Windows, so Biome was additionally run with explicit package directories: no errors, existing warnings only.
  • The widget API snapshot was regenerated. The snapshot generator needed a temporary Windows path normalization; the generator itself is unchanged in this PR.
  • Full coverage passes: pnpm test:coverage --maxWorkers=4 --testTimeout=60000 --coverage.reportsDirectory=coverage/full (1,794 passed, 65 skipped; all thresholds met). The timeout accommodates slow Cloudflare concurrent-append conformance cases on this Windows host.
  • The existing 4,000-word performance test exceeds its limits on this host with both the feature and the original upstream renderer. Its assertions remain unchanged.

Screenshot: attach the adjacent new-reply.png, captured by the final Chromium regression with the keyboard focus ring visible.

AI assistance

OpenAI Codex implemented the button, locale wiring, styles, tests, docs, API snapshot, and changeset, and ran validation.

Checklist

  • This PR targets dev
  • I was assigned the linked issue
  • pnpm -r build, unit tests with coverage, and pnpm -r typecheck pass locally (coverage uses the Windows timeout override above)
  • Tests added or updated where it makes sense
  • For a bug fix: the new test fails without my change
  • Changeset added
  • Docs updated

Local branch: codex/widget-new-reply-pill
Local commit: aa74ce7
Worktree: C:\Users\sruja\.codex\worktrees\widget-new-reply\chatcops

Add an accessible, localized jump-to-latest button with reduced-motion support and preserve the existing stream follow threshold. Include regression coverage, API snapshot, documentation, and a minor changeset.

@anurag629 anurag629 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks @srujankaleru2007. The behavior is right: the pill only shows when you're scrolled up, it hides on a manual scroll or a click, and sending still jumps to the bottom. Build, typecheck, test:coverage, lint (same as dev), api:check and the full e2e suite pass locally. A few things before it can go in:

  1. The focus ring from the issue is missing. .cc-new-reply isn't in the :focus-visible list at packages/widget/src/styles/widget.css:591-598, so the focused pill gets the browser's default outline instead of --cc-focus-ring. The e2e check at e2e/tests/stream-render.spec.ts:128 can't catch it: the pill always has box-shadow: var(--cc-shadow), so it's never none. Please add .cc-new-reply:focus-visible to that list, and have the test compare the focused box-shadow with the unfocused one.

  2. Clicking or pressing Enter on the pill hides the button while it still has focus (packages/widget/src/dom/messages.ts:81-87), so focus falls to document.body. In the popup that breaks Escape-to-close and the Tab trap, since both listen on the panel (packages/widget/src/dom/panel.ts:55-80). If the pill had focus, move it to the message input before hiding it, and add a line to the keyboard e2e test that checks where focus ends up after Enter.

  3. The arrow is part of the accessible name, so screen readers say "New reply down arrow". Keep the label as just the newReply string and draw the arrow in CSS, for example .cc-new-reply::after { content: " ↓" / ""; }. That also makes refreshReplyLabel simpler, and the locators at stream-render.spec.ts:87 and :119 become name: 'New reply'.

  4. Please drop the changes to the mangle regex in packages/widget/vite.config.ts:24 and packages/widget/vite.panel.config.ts:30. They rename internals across the whole bundle, which needs its own review: a name that later becomes a config field would get renamed in the CDN build only, and the unit tests run against src, so they wouldn't notice. The panel being this close to its budget isn't on you. I'll make room in a separate PR, and you can rebase on it. Without the mangle change this PR is 17.11 kB on the panel and 19.19 kB on the single file, so anything you can trim from the feature itself helps too.

Small one: the test at packages/widget/tests/dom/stream.test.ts:260 registers an old locale that stays in the global registry for the rest of the file. A cleanup in afterEach would keep the tests independent.

@anurag629

Copy link
Copy Markdown
Member

@srujankaleru2007 the room is there now: #453 freed about 240 B on the panel (16.6 kB) and 300 B on the single file (18.62 kB), so this PR fits without its mangle changes. Rebase on dev and drop the edits to both vite*.config.ts files. If you want to rename a private member of your own, add it to CHECKED in packages/widget/mangle.ts, and tests/mangle.test.ts will tell you whether it's safe.

@anurag629

Copy link
Copy Markdown
Member

@srujankaleru2007 a heads up: #469 raised the panel budget to 18 kB and the single file to 20 kB, so the New reply button has room. The other points from my review still stand.

This branch has not been deployed

No deployments
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.

Widget: a "New reply" button when the visitor has scrolled up

2 participants