Skip to content

Add Silk, Strata, and Echo thinking indicators - #40

Merged
TheGreatAxios merged 3 commits into
corbitsdev:mainfrom
pratikbuilds:add-thinking-indicator-variants
Sep 21, 2026
Merged

TheGreatAxios merged 3 commits into
corbitsdev:mainfrom
pratikbuilds:add-thinking-indicator-variants

Conversation

@pratikbuilds

@pratikbuilds pratikbuilds commented Sep 17, 2026 •

Copy link
Copy Markdown
Collaborator

Adds reusable Corbits thinking indicators so apps can use the Silk horizontal sweep, Strata upward scan, or Echo waves through a variant prop.

ThinkingIndicator pairs the animated mark with an accessible status label. ThinkingMark supports standalone use with class-based sizing. Both support pause, preserve the shared logo geometry, and show a static highlight for reduced motion. Includes package exports, usage documentation, and Ladle stories.

Validation:

  • bun run build
  • bun run typecheck
  • bun run lint
  • bun run dep-guard
  • bun test: 336 passed, zero failures
  • Browser checks of rendering, inline spacing, and pause/play

Clean-install validation passed using the npm tarball in two empty consumer projects:

  • React 19 with Vite: production build and browser rendering of all three variants
  • Next.js 16: production build and prerendering with a server component and a consumer-owned client boundary for the animated components
  • No browser console errors in either consumer
  • Packed exports, type declarations, animation CSS, and reduced-motion rules present
  • Tree-shaking: single-component bundle 62,251 bytes; all-exports bundle 466,635 bytes
  • Both consumers build without the optional React Query peer

@pratikbuilds

Copy link
Copy Markdown
Collaborator Author
Screenshot 2026-09-17 at 10 39 23 PM

@TheGreatAxios TheGreatAxios 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.

Thanks for this — the three variants look great and the useId-keyed masks are the right call. Two things before merge:

  1. Hardcoded hexes in thinking-mark.tsx. fill="#e98428" is light-mode --primary — under .dark, --primary is #bf6b20, so the sweep paints the wrong orange in dark mode. Same for fill-[#77716a] / dark:fill-[#aaa39a]. House rule (ARCHITECTURE.md standing patterns): theme.css is the only place a hex lives; components consume tokens (fill-primary, plus a token for the base gray). This is the same bug class the audit previously fixed in DitherCanvas.
  2. No test. Every sibling PR ships one — even a renderToStaticMarkup assert on role="status", aria-hidden on the mark, and the variant classes would cover the contract.

README section and the reduced-motion static frames in theme.css are good.

@pratikbuilds

Copy link
Copy Markdown
Collaborator Author

@TheGreatAxios The requested fixes are now in ebf13b2: the mark uses dark-aware theme tokens, and a focused rendering/accessibility test covers all variants. Could you re-review when you have a moment?

@pratikbuilds
pratikbuilds force-pushed the add-thinking-indicator-variants branch from ebf13b2 to 5ff5ac8 Compare September 21, 2026 17:54
@TheGreatAxios

Copy link
Copy Markdown
Contributor

Review pass — maintainer fixes applied on top of 5ff5ac8

The rebase onto post-prune main and the --thinking-base token migration were already in — the token work held up (no raw hex remains; fill-thinking-base/fill-primary drive everything).

Review findings — resolved in 83c0323

  • Fragile delay encoding (emil): the second echo circle encoded -1.8s as a separate animation-delay utility — the animation shorthand in the other class could reset it depending on emission order. The delay now lives inside the shorthand (corbits-thinking-wave_3.6s_..._-1.8s_infinite); verified in the built dist/styles.css.
  • paused untested (emil): test asserts the play-state utility renders.
  • Reduced-motion echo fallback untested (emil): test asserts motion-reduce:hidden on the second wave.
  • Geometry duplication (emil): the static reduced-motion poses in theme.css depend on the SVG geometry in thinking-mark.tsx — a comment now pins that coupling since CSS can't import TS constants.

Kept as designed: role="status" + aria-hidden mark, useId()-scoped defs, transform-origin, clean loop seams, and all three variants (Silk/Strata/Echo) — product decision is to ship the set.

Validation

227 tests (4 in this file), typecheck, lint, css build — all green.

@TheGreatAxios
TheGreatAxios merged commit a31c83f into corbitsdev:main Sep 21, 2026
1 check passed
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.

2 participants