Skip to content

fix(ui): restore shimmer for running trow summaries#883

Merged
Astro-Han merged 2 commits into
devfrom
codex/i882-trow-summary-shimmer
May 24, 2026
Merged

fix(ui): restore shimmer for running trow summaries#883
Astro-Han merged 2 commits into
devfrom
codex/i882-trow-summary-shimmer

Conversation

@Astro-Han
Copy link
Copy Markdown
Owner

@Astro-Han Astro-Han commented May 24, 2026

Summary

Restore the shared TextShimmer treatment for active trow summary text while keeping completed trow summaries stable.

Why

Issue #882 tracks the follow-up from PR #874: grouped trow summaries expose running state, but the summary text was still rendered as plain text. This makes active tool-group changes feel abrupt and leaves the existing shimmer contract unused.

Related Issue

Fixes #882

Human Review Status

Approved by @Astro-Han

Review Focus

Please check that the shimmer is scoped only to active trow summary text and that completed summaries do not keep an active animation node.

Risk Notes

The change adds one shared TextShimmer instance to active trow summaries, so the main risk is render/perf cost during tool-heavy sessions. Targeted snap and perf probe scenarios passed. Platform/packaging checklist item is unticked because this touches only web-rendered UI component code and snap coverage.

How To Verify

RED check: bun run snap session-trow failed before implementation because active running-current trow had no active TextShimmer node.
bun install --frozen-lockfile: installed dependencies in the isolated worktree.
bun run typecheck: passed.
bun test --preload ./happydom.ts ./src/components/session-turn-trow-block.reducer.test.ts: 19 passed.
bun run snap session-trow: passed; screenshot grid generated at docs/design/preview/screenshots/session-trow.png.
PAWWORK_PERF_SCENARIOS=tool-call-expand bun run test:e2e:local:perf: 1 passed, 9 skipped.
PAWWORK_PERF_PROFILE=low-end PAWWORK_PERF_SCENARIOS=concurrent-shimmer-extreme bun run test:e2e:local:perf: 1 passed, 9 skipped.
git diff --cached --check: passed.
Review follow-up for Gemini suggestion: removed keyed remount path for TextShimmer text updates.
Follow-up bun run typecheck: passed.
Follow-up bun test --preload ./happydom.ts ./src/components/session-turn-trow-block.reducer.test.ts: 19 passed.
Follow-up bun run snap session-trow: passed.
Follow-up git diff --check: passed.

Screenshots or Recordings

Updated snap target: session-trow, generated docs/design/preview/screenshots/session-trow.png.

Manual Electron check passed by @Astro-Han.

Checklist

How to use this checklist:

  • Tick a box by replacing [ ] with [x]. Do not edit, add, or remove items.
  • The bot-applied label items can only be honestly ticked AFTER the PR is opened and the labeler / priority-triage bots have run — return to the PR description and tick them then.
  • Most items are required. The few that are conditional are explicitly marked (conditional); for those, leave unticked if they truly do not apply and explain why in Risk Notes. All other items must be ticked before requesting human review.
  • Type label — this PR carries exactly one of bug, enhancement, task, documentation. Type labels are author-added; the labeler bot does NOT assign them. Add the label in the GitHub UI, then tick this.
  • Routing labels — this PR carries at least one of app, ui, platform, harness, ci. The labeler bot assigns these on PR open based on changed paths. Confirm the bot's choice (or override if wrong), then tick this.
  • Priority label — this PR carries exactly one of P0, P1, P2, P3. The priority-triage bot suggests one on PR open. Confirm or override, then tick this.
  • Human Review Status above is set to Pending, Approved by @<reviewer>, or Not required: <reason> (default is Pending; "not required" is restricted to bot-authored low-risk PRs).
  • I linked the related issue, or stated in Summary why there is no issue.
  • I described the review focus and any meaningful risks.
  • I replaced the example block in How To Verify with the real verification steps and the key result for each.
  • I did not introduce unrelated refactors, dependencies, generated files, or file changes beyond the stated scope.
  • (conditional) I manually checked visible UI or copy changes when needed, with screenshots or recordings. Leave unticked only if no visible UI or copy changed.
  • (conditional) I considered macOS and Windows impact for platform, packaging, updater, signing, paths, shell, or permissions changes. Leave unticked only if no platform/packaging surface was touched.
  • (conditional) I called out docs, release notes, dependencies, permissions, credentials, deletion behavior, generated content, or local file changes when relevant. Leave unticked only if none of those surfaces was touched.
  • I reviewed the final diff for unrelated changes and suspicious dependency changes.
  • I am targeting dev, and my PR title and commit messages use Conventional Commits in English.

Summary by CodeRabbit

Release Notes

  • New Features

    • Added shimmer loading animations to session summary text during active operations and various summary states
  • Tests

    • Extended snapshot tests to verify shimmer animation visibility and behavior across different session summary display states

Review Change Stack

@Astro-Han Astro-Han added enhancement New feature or request P2 Medium priority ui Design system and user interface labels May 24, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 24, 2026

📝 Walkthrough

Walkthrough

Session-turn-trow-block now wraps active summary text with the TextShimmer component. The CSS documentation describes the shimmer behavior, and e2e snapshot tests validate that running trows display active shimmer while collapsed states remain still.

Changes

Trow Summary Shimmer

Layer / File(s) Summary
TextShimmer component integration
packages/ui/src/components/session-turn-trow-block.tsx, packages/ui/src/components/session-turn-trow-block.css
TextShimmer is imported and wrapped around the summary text in the single-row details/summary path with active={!!activeTool()}. CSS documentation is updated to describe the shimmer behavior and reduced-motion handling, replacing the prior caller-wired shimmer model.
E2E shimmer state assertions
packages/app/e2e/snap/session-trow.snap.ts
ACTIVE_SHIMMER selector constant is added and assertions are extended to validate that running-current shows an active shimmer while both collapsed states (activity-summary-collapsed and failed-summary-collapsed) show zero active shimmers.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested labels

bug

Poem

🐰 A shimmer dance for tools in flight,
Active summaries now glow just right—
Subtle motion, respectful and lean,
The shimmer returns to the trow scene!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: restoring shimmer UI treatment for active (running) trow summary text, which aligns with the PR's primary objective.
Linked Issues check ✅ Passed The PR fully addresses #882 objectives: TextShimmer is applied to active trow summaries via updated session-turn-trow-block.tsx [in raw_summary], completed summaries remain stable, reduced-motion is respected, snapshot coverage was added, and perf probes passed.
Out of Scope Changes check ✅ Passed All changes are in-scope: three files modified (snapshot test assertions, CSS documentation, and TSX component logic) directly implement the shimmer restoration without altering grouping model, summary copy, or unrelated surfaces.
Description check ✅ Passed The pull request description is comprehensive and follows the required template with all major sections completed, including summary, context, issue link, human review status, review focus, risk notes, verification steps, and a fully-ticked checklist.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/i882-trow-summary-shimmer

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions Bot added the app Application behavior and product flows label May 24, 2026
Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

Suggested priority: P2 (includes non-doc, non-test paths outside the low-risk bucket).

P1/P0 are reserved for maintainer confirmation. Please relabel manually if this is a release blocker, security issue, data-loss risk, or updater/runtime failure.

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request integrates the TextShimmer component into the TrowBlock to handle loading states for summary text and updates the end-to-end tests to verify the shimmer's visibility. A review comment identifies that using the keyed attribute on the <Show> component causes unnecessary remounting, which triggers unwanted entry animations and breaks the internal transition logic of TextShimmer. It is recommended to remove the keyed attribute and treat the text as an accessor to ensure smoother visual transitions.

Comment thread packages/ui/src/components/session-turn-trow-block.tsx Outdated
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 24, 2026

Perf delta summary

Comparator: pass

Profile / Scenario interaction median interaction worst long task max tbt frame gap p95 frame gap max jank count cls status
default / tool-default-open-heavy-bash 16 -> 16 (0) 16 -> 16 (0) 77 -> 66 (-11) 41 -> 16 (-25) 50 -> 50 (0) 133.3 -> 116.6 (-16.7) 3 -> 3 (0) 0.004 -> 0.004 (0) pass

@Astro-Han Astro-Han merged commit dcc8b56 into dev May 24, 2026
31 checks passed
@Astro-Han Astro-Han deleted the codex/i882-trow-summary-shimmer branch May 24, 2026 12:05
Astro-Han added a commit that referenced this pull request May 24, 2026
Bump the desktop Electron package version to v2026.5.25 for the release train.

Included release-readiness checks:
- Local app and desktop Electron typechecks passed.
- Focused UI component tests for grouped tool-call summaries passed.
- PR CI passed, including app/ui/opencode/desktop units, typecheck, CodeQL, desktop smoke, and e2e-artifacts.
- Computer Use manual E2E covered new session creation, an actual execute-command tool call, right-panel tabs and collapse, #880 sidebar/right-panel toggle affordances, and pinned session drag reordering.

This release includes the current dev branch work through #880 and the later fixes #881 and #883.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

app Application behavior and product flows enhancement New feature or request P2 Medium priority ui Design system and user interface

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature] Restore text shimmer for active trow summaries

1 participant