[WEB-4860] dev: propel animated counter component #7740
Conversation
anmolsinghbhatia
requested review from
prateekshourya29 and
vamsikrishnamathala
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Warning Rate limit exceeded@anmolsinghbhatia has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 14 minutes and 16 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughAdds a new AnimatedCounter React component, Storybook stories, CSS keyframe animations, package exports and build entries to expose the component from the Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant App as Consumer
participant Counter as AnimatedCounter
participant CSS as CSS/DOM
participant Timer as setTimeout(250ms)
User->>App: triggers count change (increment/decrement)
App->>Counter: props { count, size }
Counter->>Counter: compute direction (up/down), set displayCount, isAnimating, animationKey++
Counter->>CSS: render prev/current spans with direction classes
CSS-->>Counter: visual slide in/out (250ms)
Counter->>Timer: start 250ms timeout
Timer-->>Counter: timeout fires
Counter->>Counter: clear isAnimating, commit state
Counter->>CSS: render stable number
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks (2 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. Comment |
|
Pull Request Linked with Plane Work Items
Comment Automatically Generated by Plane |
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a new AnimatedCounter component to the propel package that provides smooth visual transitions when a numeric count value changes.
- Adds a new AnimatedCounter React component with configurable size variants
- Includes animation logic for sliding effects when count values increase or decrease
- Provides Storybook documentation and interactive demos for the component
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/propel/tsdown.config.ts | Adds build entry point for animated-counter component |
| packages/propel/src/animated-counter/index.ts | Exports AnimatedCounter component and types |
| packages/propel/src/animated-counter/animated-counter.tsx | Core component implementation with animation logic |
| packages/propel/src/animated-counter/animated-counter.stories.tsx | Storybook stories for component documentation |
| packages/propel/package.json | Adds package export path for animated-counter |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (11)
packages/propel/src/animated-counter/animated-counter.tsx (6)
24-42: Avoid timer drift: end animation via onAnimationEnd instead of setTimeout(250)Hard-coding 250 ms can desync if CSS timing changes. Prefer onAnimationEnd.
Apply:
useEffect(() => { if (count !== prevCount) { setDirection(count > prevCount ? "up" : "down"); setIsAnimating(true); setAnimationKey((prev) => prev + 1); // Update the display count immediately, animation will show the transition setDisplayCount(count); - // End animation after CSS transition - const timer = setTimeout(() => { - setIsAnimating(false); - setDirection(null); - setPrevCount(count); - }, 250); - - return () => clearTimeout(timer); + // End handled by onAnimationEnd on the current span. + return; } }, [count, prevCount]); + + const handleAnimationEnd = () => { + setIsAnimating(false); + setDirection(null); + setPrevCount(count); + };And wire the handler as shown in a later comment.
52-66: Duplicate animation sources (class vs inline style) — consolidate to oneYou set animation via both Tailwind's arbitrary animate-[...] and inline style; inline style wins, making the class redundant and confusing.
Apply:
- "animate-[slideOut_0.25s_ease-out_forwards]", + // animation driven via style only- isAnimating && "animate-[slideIn_0.25s_ease-out_forwards]", + // animation driven via style onlyAnd add onAnimationEnd on the current span:
- <span + <span + onAnimationEnd={isAnimating ? handleAnimationEnd : undefined}Also applies to: 75-76
10-14: Type-safety nit: lock keys with satisfiesPrevents accidental key typos and preserves literal types.
-const sizeClasses = { +const sizeClasses = { sm: "text-xs h-4 w-4", md: "text-sm h-5 w-5", lg: "text-base h-6 w-6", -}; +} as const satisfies Record<"sm" | "md" | "lg", string>;
44-48: Reduce duplication: apply sizing to container only; move className to containerSize is set on container; repeating sizeClass on children is redundant. Also pass className to the container for flexibility.
- const sizeClass = sizeClasses[size]; + const sizeClass = sizeClasses[size]; return ( - <div className={cn("relative inline-flex items-center justify-center overflow-hidden", sizeClass)}> + <div + role="status" + aria-live="polite" + aria-atomic="true" + className={cn("relative inline-flex items-center justify-center overflow-hidden tabular-nums", sizeClass, className)} + > ... - sizeClass, + // size from container ... - sizeClass, - className + // size from containerAlso applies to: 57-58, 74-79
47-47: Multi-digit counts risk clipping with fixed w-/h-; confirm usage or enable auto widthWith w-4/5/6 the container will clip 2+ digits (or negative values). If counts can exceed 9, consider auto width + tabular-nums or a prop to switch sizing modes.
I can propose an auto-width variant (min-w based on digits) if needed.
4-8: Optional: expose duration and reduced-motion handlingConsider a durationMs prop and honoring prefers-reduced-motion to disable animations for accessibility.
I can supply a small hook (usePrefersReducedMotion) and wire it to skip animation styles when true.
packages/propel/src/animated-counter/animated-counter.stories.tsx (5)
23-25: Sync local state with Controls; avoid stale args.count.Changing count via Storybook Controls won’t reflect after first render. Also prefer nullish coalescing over
||.-const AnimatedCounterDemo = (args: React.ComponentProps<typeof AnimatedCounter>) => { - const [count, setCount] = useState(args.count || 0); +const AnimatedCounterDemo = (args: React.ComponentProps<typeof AnimatedCounter>) => { + const [count, setCount] = useState<number>(args.count ?? 0); + // Keep in sync with Controls + useEffect(() => { + setCount(args.count ?? 0); + }, [args.count]);
1-1: Import useEffect for the sync fix.-import { useState } from "react"; +import { useState, useEffect } from "react";
35-37: Announce count updates for screen readers.Expose the live-updating value via
role="status"andaria-live="polite". If AnimatedCounter already handles this, skip.- <div className="flex items-center justify-center min-w-[60px] h-12 bg-gray-50 border border-gray-200 rounded-lg"> + <div + className="flex items-center justify-center min-w-[60px] h-12 bg-gray-50 border border-gray-200 rounded-lg" + role="status" + aria-live="polite" + >
3-3: Prefer importing from the barrel to exercise the published API.-import { AnimatedCounter } from "./animated-counter"; +import { AnimatedCounter } from ".";
5-7: Group AnimatedCounter under "Components/". Change thetitleinpackages/propel/src/animated-counter/animated-counter.stories.tsxtotitle: "Components/AnimatedCounter",to match other stories (Skeleton, Separator, Tabs).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/propel/package.json(1 hunks)packages/propel/src/animated-counter/animated-counter.stories.tsx(1 hunks)packages/propel/src/animated-counter/animated-counter.tsx(1 hunks)packages/propel/src/animated-counter/index.ts(1 hunks)packages/propel/tsdown.config.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Build and lint web apps
🔇 Additional comments (4)
packages/propel/src/animated-counter/index.ts (1)
1-2: LGTM — correct re-exportsPublic API shape is clear and consistent.
packages/propel/tsdown.config.ts (1)
6-6: Export mapping wired correctly for animated-counter
Source file exists at packages/propel/src/animated-counter/index.ts and package.json exports “./animated-counter” → “./dist/animated-counter/index.js”.packages/propel/src/animated-counter/animated-counter.stories.tsx (2)
49-55: Default story looks good.Interactive demo with increment/decrement and initial args is clear.
5-20: Stronger typing withsatisfies Meta<>(optional)
TypeScript 5.8.3 supportssatisfies, so you can preserve literal types and catch config drift at compile-time:-const meta: Meta<typeof AnimatedCounter> = { +const meta = { title: "AnimatedCounter", component: AnimatedCounter, parameters: { layout: "centered", }, tags: ["autodocs"], argTypes: { size: { control: { type: "select" }, options: ["sm", "md", "lg"], }, }, -}; +} satisfies Meta<typeof AnimatedCounter>; export default meta;
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/tailwind-config/global.css (3)
721-741: Naming symmetry: add slideOutUp alias for clarity.We have slideOutDown but slideOut (up) is implicit. Adding an explicit slideOutUp alias improves readability without breaking existing names.
@keyframes slideOut { 0% { transform: translateY(0); opacity: 1; } 100% { transform: translateY(-100%); opacity: 0; } } + +@keyframes slideOutUp { + 0% { + transform: translateY(0); + opacity: 1; + } + 100% { + transform: translateY(-100%); + opacity: 0; + } +}
699-719: Minor perf nit: consider translate3d for smoother compositing on older/low-end GPUs.Swapping translateY(...) for translate3d(0, ... , 0) can help avoid occasional subpixel jitter.
699-750: Scope keyframe names or register in Tailwind theme for consistency.To avoid global name collisions and to standardize usage, either:
- Prefix names (e.g., propel-slide-in-from-bottom), or
- Define these in tailwind config (theme.extend.keyframes/animation) and consume via Tailwind utilities.
Low-risk, optional tidy-up.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/tailwind-config/global.css(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build and lint web apps
- GitHub Check: Analyze (javascript)
🔇 Additional comments (1)
packages/tailwind-config/global.css (1)
743-750: LGTM on fadeOut.Simple, reusable, and complements the slide variants.
* dev: animated counter added to propel * chore: animated counter story added * chore: propel config updated * chore: code refactor * chore: code refactor * fix: format error
Description
This PR introduces the animated counter component in Propel.
Media
Summary by CodeRabbit
New Features
Documentation
Style
Chores