Skip to content

refactor(web): replace RunningBorder SVG arc with CSS body-shimmer - #2274

Open
lsm wants to merge 2 commits into
devfrom
space/replace-runningborder-svg-arc-with-css-body-shimmer-then
Open

refactor(web): replace RunningBorder SVG arc with CSS body-shimmer#2274
lsm wants to merge 2 commits into
devfrom
space/replace-runningborder-svg-arc-with-css-body-shimmer-then

Conversation

@lsm

@lsm lsm commented Jul 27, 2026

Copy link
Copy Markdown
Owner

Replaces the heavy SVG "chasing glow" running indicator with a lightweight pure-CSS shimmer sweep across the card body (the "option 4" mockup the user approved). No border arc, no SVG, no ResizeObserver.

Changes

Visual — while a block is running, a faint white left→right gradient sweeps across the card surface (~1.8s loop). Motion-only signal per the user's decision (no static accent pairing).

New CSS (packages/web/src/styles.css):

  • @keyframes runningShimmer + .running-shimmer overlay — position:absolute; inset:0; pointer-events:none, white gradient, background-size:200%, left→right sweep.
  • Loop seam is invisible: with background-size:200%, animating background-position 130% → -30% sweeps the band fully off the left edge, across the card, and fully off the right edge.
  • prefers-reduced-motion hides it entirely.

Blocks (keep existing isRunning gating intact, only swap the visual):

  • ThinkingBlock, SubagentBlock, ToolResultCard — drop the <RunningBorder> wrapper; render the overlay as a card child while running and add relative to the card when running (overflow-hidden clips it to the rounded surface).

Cleanup:

  • Delete packages/web/src/components/sdk/RunningBorder.tsx and its 3 imports.
  • Update doc/comment references that mentioned the SVG arc / RunningBorder to describe the shimmer (SDKMessageRenderer, SDKAssistantMessage, SubagentBlock, ThinkingBlock, ToolResultCard, tool-types, styles.css).

Tests:

  • No existing tests asserted on the arc markup (verified — all svg queries were for icons).
  • Added ThinkingBlock test: .running-shimmer is present while running, absent when not.

Verification

  • `bun run check` — lint + typecheck + knip + guard scripts all clean (no dangling imports / dead exports).
  • `vitest run` on ThinkingBlock / SubagentBlock / ToolResultCard — 180 passed.

Replace the heavy SVG 'chasing glow' running indicator (inline SVG +
animateMotion + ResizeObserver) with a lightweight pure-CSS shimmer sweep
across the card body — the 'option 4' mockup the user approved.

- Add .running-shimmer overlay + @Keyframes runningShimmer to styles.css:
  faint white left->right gradient, background-size 200%, ~1.8s loop,
  position:absolute inset-0 pointer-events:none, honors prefers-reduced-motion.
- ThinkingBlock / SubagentBlock / ToolResultCard: drop the <RunningBorder>
  wrapper; render the overlay as a child while isRunning and add 'relative'
  to the card when running. Existing isRunning gating is unchanged.
- Delete packages/web/src/components/sdk/RunningBorder.tsx.
- Update doc/comment references to describe the shimmer instead of the SVG arc.
- Add ThinkingBlock test that .running-shimmer is present while running and
  absent when not.

@lsm lsm left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

🤖 Review by glm-5.1 (GLM)

Model: glm-5.1 | Client: NeoKai | Provider: GLM

Recommendation: REQUEST_CHANGES — 1 P2 finding (one missed stale comment). The refactor itself is correct and well-scoped; all verification passed.

Verified ✅

  • bun run check (root: oxlint + tsc --build --noEmit + knip + session-guards + space-task-handler-tests + db-schema-parity + test-quality) — clean, exit 0.
  • Block tests green: ThinkingBlock (incl. 2 new shimmer tests), SubagentBlock, SDKAssistantMessage — 130 passed.
  • No remaining RunningBorder references anywhere in the repo; RunningBorder.tsx fully deleted; no test imported or asserted on it.
  • Shimmer loop math verified: with background-size:200%, the band center sits at W·(1 − P/100); at P=130% → −0.30W (fully off-left) and at P=−30% → +1.30W (fully off-right). Off-screen clearance (0.30W) exceeds the band half-width (0.10W), so the infinite-loop seam is invisible. Sweep is left→right, 1.8s linear. Correct.
  • prefers-reduced-motion fully disables it (animation:none + display:none). pointer-events:none + aria-hidden="true" on all three usages.
  • Containment: all three card roots are always overflow-hidden and gain relative exactly when isRunning.
  • ConfirmModal (ToolResultCard) renders via Modal<Portal into="body"> + fixed inset-0 z-50, so the in-card shimmer cannot occlude it.
  • isRunning plumbing is byte-for-byte identical to before (doc-comment changes only); parent containers don't depend on the old wrapper structure.

Finding

P2 — packages/web/src/components/sdk/SDKAssistantMessage.tsx:261-264 (stale comment, in task scope)

The inline comment still describes the deleted mechanism verbatim:

// When isRunning, ALL blocks in this message receive the animated arc so every block type is visible for debugging/verification. Each component applies the arc via a wrapper div (not directly on its overflow:hidden root) so the inset:-2px extension isn't clipped.

This is now wrong on three counts: there is no "animated arc" (it's a body shimmer), no "wrapper div" (the shimmer is a direct child of the overflow-hidden root), and no "inset:-2px extension". The task explicitly scoped updating arc/RunningBorder doc references in this file — lines 62-66 and 359-366 were updated, but this paragraph was missed. Suggest rewriting it to describe the in-card .running-shimmer overlay (contained by the card's overflow-hidden). Doc-only; no behavioral impact.

Non-blocking notes

  • New @keyframes runningShimmer rather than reusing the existing shimmer (skeleton) keyframe: the existing one is px-locked to dark skeleton colors and isn't reusable for a white percentage sweep, so a separate keyframe is the right call.
  • Task text said the shimmer should sit "below interactive children"; it's rendered as the last child (paints above content) but pointer-events:none fully preserves interaction, satisfying the intent. No change needed.

The inline comment at SDKAssistantMessage.tsx:261-264 still described the
deleted SVG-arc mechanism (animated arc, wrapper div, inset:-2px extension).
Rewrite it to describe the in-card .running-shimmer overlay, matching the
updates already made elsewhere in the file.

@lsm lsm left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

🤖 Review by glm-5.1 (GLM)

Model: glm-5.1 | Client: NeoKai | Provider: GLM

Recommendation: APPROVE — zero findings. The P2 doc-comment from the prior round is fixed; all checks and tests pass.

Round-2 verification (fresh, not relying on prior conclusions):

  • Fix commit 8bb46f3fe is surgical: only SDKAssistantMessage.tsx (3+/3−). The stale inline comment now correctly describes the in-card .running-shimmer overlay contained by the card's overflow-hidden.
  • Independent residual-vocabulary grep across packages/web/src (animated arc / border-light / animateMotion / mpath / radial-gradient mask / inset:-2px / wrapper-div arc / RunningBorder) → only false positives (router *FromPath helpers, unrelated "wrapper div" comments). No real leftovers.
  • bun run check (root: oxlint + tsc --build --noEmit + knip + session-guards + space-task-handler-tests + db-schema-parity + test-quality) — exit 0.
  • ThinkingBlock / SubagentBlock / SDKAssistantMessage tests — 130 passed.
  • PR OPEN, MERGEABLE, head == fix commit (8bb46f3fe), zero unresolved review threads.

The refactor itself (verified in round 1): correct CSS loop-seam math, prefers-reduced-motion, pointer-events:none + aria-hidden, overflow-hidden/relative containment on all three blocks, and ConfirmModal portal isolation. No blocking issues.

(Self-review fallback: GitHub rejects self-APPROVE, so posted as COMMENT with an explicit APPROVE recommendation.)

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.

1 participant