-
- Notifications
You must be signed in to change notification settings - Fork 43
Add Loader Component with Customizable Square Animations #75
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@OmarAlithawi is attempting to deploy a commit to the Retro UI Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughAdds a new Loader UI component with variants and sizes, exposes it via public exports, registers it in component config, adds documentation and preview examples, and applies minor formatting changes to an existing file. Changes
Sequence Diagram(s)sequenceDiagram autonumber actor Dev as Developer participant App as App/Docs participant Loader as Loader Component participant DOM as DOM/Screen Reader Dev->>App: Import { Loader } from components/retroui App->>Loader: Render with props (variant, size, count, duration, delayStep) Loader->>Loader: Compute classes via cva and map count Loader->>DOM: Output div[role="status" aria-label="Loading..."] with animated items Note over DOM: Screen readers announce "Loading..." Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (13)
preview/components/loader-style-custom.tsx (1)
1-1
: Prefer importing from the public APIImporting from the top-level retroui index makes previews mirror consumer usage and reduces coupling to internal file paths.
Apply this diff:
-import { Loader } from "@/components/retroui/newComponent/Loader" +import { Loader } from "@/components/retroui"components/retroui/newComponent/Loader.tsx (3)
25-33
: Remove unusedasChild
prop to avoid leaking unknown attributes to the DOM
asChild
is declared but unused; it will get spread to the DOM via{...props}
as an unknown attribute. Either implement the pattern or drop it. Given the current structure, removing it is simplest.Apply this diff:
-interface LoaderProps - extends Omit<React.HTMLAttributes<HTMLDivElement>, "color">, - VariantProps<typeof loaderVariants> { - asChild?: boolean; - count?: number; // number of bouncing dots - duration?: number; // animation duration in seconds - delayStep?: number; // delay in ms -} +interface LoaderProps + extends React.ComponentPropsWithoutRef<"div">, + VariantProps<typeof loaderVariants> { + count?: number; // number of squares + duration?: number; // animation duration in seconds + delayStep?: number; // delay in ms +}Notes:
- Using
React.ComponentPropsWithoutRef<"div">
is idiomatic and avoids the oddOmit<..., "color">
(div doesn’t meaningfully support acolor
attribute).
55-66
: Respect reduced motion and drop redundant inline style
- Add
motion-reduce:animate-none
so users with reduced motion preferences don’t get bounce animations.animationIterationCount: "infinite"
is already set by Tailwind’sanimate-bounce
; remove the redundant inline style.Apply this diff:
{Array.from({ length: count }).map((_, i) => ( <div key={i} - className="border-2 animate-bounce" + className="border-2 animate-bounce motion-reduce:animate-none" style={{ animationDuration: `${duration}s`, - animationIterationCount: "infinite", animationDelay: `${i * delayStep}ms`, }} /> ))}
25-33
: Optional: keep the public surface minimalIf you don’t intend to support polymorphic rendering soon, consider omitting polymorphism-related props entirely (e.g.,
asChild
,as
) for now to keep the API lean. You can reintroduce them later with a semver-minor change.components/retroui/index.ts (1)
27-27
: Minor: re-export from the directory index to reduce file couplingSince
components/retroui/newComponent/index.ts
already re-exports Loader, you can target the directory to avoid direct file coupling.Apply this diff:
-export * from "./newComponent/Loader"; +export * from "./newComponent";preview/components/loader-style-solid.tsx (1)
1-1
: Prefer importing from the public APISame as the custom preview: use the top-level retroui export for consistency with consumer usage.
Apply this diff:
-import { Loader } from "@/components/retroui/newComponent/Loader" +import { Loader } from "@/components/retroui"preview/components/loader-style-outline.tsx (1)
1-5
: Prefer importing from the public barrel to reduce coupling.Importing from the barrel keeps previews resilient to internal file moves.
-import { Loader } from "@/components/retroui/newComponent/Loader" +import { Loader } from "@/components/retroui"preview/components/loader-style-sizes.tsx (1)
1-11
: Import from the public barrel for stability.Same Loader import note as other previews—prefer the public surface.
-import { Loader } from "@/components/retroui/newComponent/Loader" +import { Loader } from "@/components/retroui"preview/components/loader-style-default.tsx (1)
1-5
: Use the public export path for consistency.Keeps imports aligned with the library’s public API and avoids reaching into internals.
-import { Loader } from "@/components/retroui/newComponent/Loader" +import { Loader } from "@/components/retroui"config/components.ts (2)
1-1
: Remove unused import.
import { table } from "console";
appears unused and can break builds with noUnusedLocals.-import { table } from "console";
599-623
: Loader preview entries added correctly. Optional naming tweak.Entries wire up fine. Consider renaming “loader-style-solid” → “loader-style-secondary” to match the
variant="secondary"
terminology used in code/docs (optional).content/docs/components/loader.mdx (2)
40-44
: Section headline says “Secondary” while example key is “loader-style-solid”.For clarity, consider aligning naming across docs/config (e.g., “secondary” instead of “solid”), or add a note that “solid” maps to the secondary variant.
130-137
: A11y: Consider documenting aria-live and reduced motion.Optionally mention
aria-live="polite"
(orassertive
if appropriate) and respectingprefers-reduced-motion
for animations.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (10)
components/retroui/index.ts
(1 hunks)components/retroui/newComponent/Loader.tsx
(1 hunks)components/retroui/newComponent/index.ts
(1 hunks)config/components.ts
(4 hunks)content/docs/components/loader.mdx
(1 hunks)preview/components/loader-style-custom.tsx
(1 hunks)preview/components/loader-style-default.tsx
(1 hunks)preview/components/loader-style-outline.tsx
(1 hunks)preview/components/loader-style-sizes.tsx
(1 hunks)preview/components/loader-style-solid.tsx
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (7)
preview/components/loader-style-default.tsx (5)
preview/components/loader-style-outline.tsx (1)
LoaderPreview
(3-5)preview/components/loader-style-custom.tsx (1)
LoaderPreview
(3-16)preview/components/loader-style-sizes.tsx (1)
LoaderPreview
(3-11)preview/components/loader-style-solid.tsx (1)
LoaderPreview
(3-5)components/retroui/newComponent/Loader.tsx (1)
Loader
(72-72)
preview/components/loader-style-outline.tsx (4)
preview/components/loader-style-default.tsx (1)
LoaderPreview
(3-5)preview/components/loader-style-custom.tsx (1)
LoaderPreview
(3-16)preview/components/loader-style-sizes.tsx (1)
LoaderPreview
(3-11)preview/components/loader-style-solid.tsx (1)
LoaderPreview
(3-5)
content/docs/components/loader.mdx (1)
components/MDX.tsx (6)
MDX
(111-121)type
(16-109)props
(17-19)props
(32-32)props
(34-39)props
(20-25)
preview/components/loader-style-solid.tsx (4)
preview/components/loader-style-default.tsx (1)
LoaderPreview
(3-5)preview/components/loader-style-outline.tsx (1)
LoaderPreview
(3-5)preview/components/loader-style-custom.tsx (1)
LoaderPreview
(3-16)preview/components/loader-style-sizes.tsx (1)
LoaderPreview
(3-11)
preview/components/loader-style-custom.tsx (4)
preview/components/loader-style-default.tsx (1)
LoaderPreview
(3-5)preview/components/loader-style-outline.tsx (1)
LoaderPreview
(3-5)preview/components/loader-style-sizes.tsx (1)
LoaderPreview
(3-11)preview/components/loader-style-solid.tsx (1)
LoaderPreview
(3-5)
components/retroui/newComponent/Loader.tsx (1)
lib/utils.ts (1)
cn
(4-6)
preview/components/loader-style-sizes.tsx (4)
preview/components/loader-style-default.tsx (1)
LoaderPreview
(3-5)preview/components/loader-style-outline.tsx (1)
LoaderPreview
(3-5)preview/components/loader-style-custom.tsx (1)
LoaderPreview
(3-16)preview/components/loader-style-solid.tsx (1)
LoaderPreview
(3-5)
🪛 LanguageTool
content/docs/components/loader.mdx
[grammar] ~22-~22: There might be a mistake here.
Context: ...# 2. Copy the code 👇 into your project: </ComponentInstall.Manual> </ComponentIns...
(QB_NEW_EN)
[grammar] ~96-~96: There might be a mistake here.
Context: ...t - Type:
default|
secondary |
outline - Default:
default` - Description: Defi...
(QB_NEW_EN)
[grammar] ~97-~97: There might be a mistake here.
Context: ...| secondary
| outline
- Default: default
- Description: Defines the style variant f...
(QB_NEW_EN)
[grammar] ~101-~101: There might be a mistake here.
Context: ...ent - * size
- Type: sm
| md
| lg
- Default: md
- Description: Controls ...
(QB_NEW_EN)
[grammar] ~102-~102: There might be a mistake here.
Context: ...- Type: sm
| md
| lg
- Default: md
- Description: Controls the size of the lo...
(QB_NEW_EN)
[grammar] ~106-~106: There might be a mistake here.
Context: ...e loader squares - * count
- Type: number
- Default: 3
- Description: Sets the n...
(QB_NEW_EN)
[grammar] ~107-~107: There might be a mistake here.
Context: ...count
- Type: number
- Default: 3
- Description: Sets the number of bouncing...
(QB_NEW_EN)
[grammar] ~111-~111: There might be a mistake here.
Context: ...ncing squares - * duration
- Type: number
- Default: 0.5
- Description: Controls...
(QB_NEW_EN)
[grammar] ~112-~112: There might be a mistake here.
Context: ...ration - Type:
number - Default:
0.5` - Description: Controls the animation dura...
(QB_NEW_EN)
[grammar] ~116-~116: There might be a mistake here.
Context: ...n in seconds - * delayStep
- Type: number
- Default: 100
- Description: Defines ...
(QB_NEW_EN)
[grammar] ~117-~117: There might be a mistake here.
Context: ...ayStep - Type:
number - Default:
100` - Description: Defines the delay between s...
(QB_NEW_EN)
[grammar] ~121-~121: There might be a mistake here.
Context: ...milliseconds - * className
- Type: string
- Default: undefined
- Description: Al...
(QB_NEW_EN)
[grammar] ~122-~122: There might be a mistake here.
Context: ...ssName - Type:
string - Default:
undefined` - Description: Allows for adding additiona...
(QB_NEW_EN)
[grammar] ~126-~126: There might be a mistake here.
Context: ...custom styling - * asChild
- Type: boolean
- Default: false
- Description: When `...
(QB_NEW_EN)
[grammar] ~127-~127: There might be a mistake here.
Context: ...Child - Type:
boolean - Default:
false - Description: When
true`, merges the com...
(QB_NEW_EN)
🔇 Additional comments (6)
components/retroui/newComponent/index.ts (1)
1-1
: LGTM: simple and correct re-exportThis cleanly exposes Loader from the submodule. No issues.
components/retroui/index.ts (1)
27-27
: LGTM: public export addedRe-exporting Loader from the library root is correct and matches the intended public surface.
config/components.ts (2)
239-241
: Formatting-only change looks good.Multiline lazy import improves readability without functional impact.
427-429
: Formatting-only change looks good.Consistent with other multiline lazy imports.
content/docs/components/loader.mdx (2)
66-91
: Verify the import path shown in docs matches the public API.Docs show
import { Loader } from "@/components/ui/loader"
. Previews import from retroui. If the public surface for consumers is@/components/ui/loader
, keep as-is; otherwise, update docs to the correct public path.Please confirm the intended public import path for consumers.
93-119
: All documented props match implementation
Verified inLoader.tsx
that:
- Default props:
count = 3
,duration = 0.5
,delayStep = 100
- CVA defaults:
variant: "default"
,size: "md"
- Variant options:
default | secondary | outline
- Size options:
sm | md | lg
<ComponentSource name="loader" /> | ||
| ||
</ComponentInstall.Manual> | ||
</ComponentInstall> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Docs will fail to show source until loader is in core config.
<ComponentSource name="loader" />
relies on componentConfig.core.loader
, which is absent. Add it in config/components.ts (see suggested diff there).
🤖 Prompt for AI Agents
In content/docs/components/loader.mdx around lines 24 to 27, the docs include <ComponentSource name="loader" /> which depends on componentConfig.core.loader but that key is missing from config/components.ts; open config/components.ts and add a core.loader entry (matching other core component shapes) tied to the loader implementation so componentConfig.core.loader exists, export it alongside the existing componentConfig, and ensure the loader name matches the value used in the MDX file so the <ComponentSource name="loader" /> can retrieve and render the source.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (7)
preview/components/loader-style-default.tsx (1)
1-1
: Prefer barrel import for public API stability.Import from the retroui index to avoid deep path coupling and keep previews aligned with the public surface.
-import { Loader } from "@/components/retroui/Loader"; +import { Loader } from "@/components/retroui";components/retroui/Loader.tsx (6)
11-11
: Border color in outline variant — consider theme-friendly color.Hardcoding
border-black
can clash with dark themes. If your design system exposes a semantic color (e.g.,border-foreground
) or if you want inheritance,border-current
may be safer.- outline: "[&>div]:bg-transparent [&>div]:border-black [&>div]:shadow-md", + outline: "[&>div]:bg-transparent [&>div]:border-current [&>div]:shadow-md",
25-27
:Omit<..., "color">
is unnecessary for a div.
color
isn’t a standard HTML attribute you need to protect against here. Dropping theOmit
simplifies the public type surface.-interface LoaderProps - extends Omit<React.HTMLAttributes<HTMLDivElement>, "color">, - VariantProps<typeof loaderVariants> { +interface LoaderProps + extends React.HTMLAttributes<HTMLDivElement>, + VariantProps<typeof loaderVariants> {
28-28
:asChild
prop is declared but unused — either wire it or remove it.Given RetroUI patterns, supporting
asChild
via Radix Slot is common. Wiring it keeps parity with other components; otherwise, remove it to avoid API confusion.Wire up
asChild
with Slot:import * as React from "react"; import { cva, type VariantProps } from "class-variance-authority"; import { cn } from "@/lib/utils"; +import { Slot } from "@radix-ui/react-slot"; @@ ( { className, variant, size, + asChild = false, count = 3, duration = 0.5, delayStep = 100, ...props }, ref, ) => { - return ( - <div + const Component = asChild ? Slot : "div"; + return ( + <Component className={cn(loaderVariants({ variant, size }), className)} ref={ref} role="status" aria-label="Loading..." {...props} > @@ - </div> + </Component> ); }, );If you prefer to keep it simple now, remove the prop from the type:
- asChild?: boolean;
56-63
:animationIterationCount: "infinite"
is redundant withanimate-bounce
.Tailwind’s
animate-bounce
already sets infinite iteration. Keeping onlyanimationDuration
andanimationDelay
reduces inline style overrides.style={{ animationDuration: `${duration}s`, - animationIterationCount: "infinite", animationDelay: `${i * delayStep}ms`, }}
40-43
: Consider clampingcount
to prevent accidental perf issues.A very large
count
will render many bouncing elements and can bog down the UI. A soft clamp (e.g., 1–12) is a safe default while still allowing overrides if you expose an escape hatch later.- count = 3, + count = 3, @@ - {Array.from({ length: count }).map((_, i) => ( + {Array.from({ length: Math.max(1, Math.min(count, 12)) }).map((_, i) => (
51-53
: A11y note —role="status"
is good; ensure it’s not over-announced.
role="status"
announces “Loading...” when mounted. If the loader is long-lived or frequently toggled, consider making the label customizable or allow opting out via a prop (e.g.,ariaLabel
orsrOnlyLabel={false}
) to avoid repetitive announcements.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (9)
components/SideNav.tsx
(1 hunks)components/retroui/Loader.tsx
(1 hunks)components/retroui/index.ts
(1 hunks)config/components.ts
(5 hunks)preview/components/loader-style-custom.tsx
(1 hunks)preview/components/loader-style-default.tsx
(1 hunks)preview/components/loader-style-outline.tsx
(1 hunks)preview/components/loader-style-sizes.tsx
(1 hunks)preview/components/loader-style-solid.tsx
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- components/SideNav.tsx
🚧 Files skipped from review as they are similar to previous changes (6)
- preview/components/loader-style-solid.tsx
- preview/components/loader-style-sizes.tsx
- components/retroui/index.ts
- preview/components/loader-style-outline.tsx
- preview/components/loader-style-custom.tsx
- config/components.ts
🧰 Additional context used
🧬 Code graph analysis (2)
components/retroui/Loader.tsx (1)
lib/utils.ts (1)
cn
(4-6)
preview/components/loader-style-default.tsx (4)
preview/components/loader-style-custom.tsx (1)
LoaderPreview
(3-16)preview/components/loader-style-outline.tsx (1)
LoaderPreview
(3-5)preview/components/loader-style-sizes.tsx (1)
LoaderPreview
(3-11)preview/components/loader-style-solid.tsx (1)
LoaderPreview
(3-5)
🔇 Additional comments (2)
preview/components/loader-style-default.tsx (1)
3-5
: LGTM — minimal, clear preview.Straightforward example that mirrors the default variant and keeps preview noise low.
components/retroui/Loader.tsx (1)
71-72
: LGTM — named export and displayName set correctly.Good API shape and devtools friendliness.
secondary: | ||
"[&>div]:bg-secondary [&>div]:border-black [&>div]:shadow-md [&>div]:shadow-primary", | ||
outline: "[&>div]:bg-transparent [&>div]:border-black [&>div]:shadow-md", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Verify Tailwind support for shadow-primary
(may compile to no-op).
Tailwind doesn’t provide shadow-{color}
out of the box in older configs. Unless your Tailwind config adds colored shadow utilities, shadow-primary
won’t have any effect. If you intend a colored shadow, either ensure the config supports it or switch to an arbitrary property/class that’s guaranteed to compile.
Run this to confirm support and usage:
If unsupported, consider:
- "[&>div]:bg-secondary [&>div]:border-black [&>div]:shadow-md [&>div]:shadow-primary", + // Keep a visible shadow and tint via arbitrary value or CSS var + "[&>div]:bg-secondary [&>div]:border-black [&>div]:shadow-md [&>div]:shadow-[color:var(--primary)]",
🏁 Script executed:
#!/bin/bash # 1) Check for 'shadow-primary' usage rg -n --no-ignore 'shadow-primary' -g '!**/dist/**' -g '!**/.next/**' # 2) Inspect Tailwind config for primary color and any shadow color plugin/util fd -a -t f 'tailwind.config.*' | xargs -I{} sh -c "echo '--- {} ---'; sed -n '1,200p' {} | rg -n 'colors|boxShadow|shadowColor|plugin'"
Length of output: 2397
shadow-primary is not supported – it compiles to a no-op
Our search confirmed that there is no Tailwind config (e.g. tailwind.config.js
/ts
) defining custom boxShadow
, shadowColor
, or any plugin for colored shadows. As a result, every occurrence of shadow-primary
will produce no CSS and fall back to the default shadow-md
only.
Instances of shadow-primary
in the codebase:
- components/retroui/Loader.tsx (line 10)
- components/retroui/Button.tsx (line 14)
- app/(marketing)/page.tsx (line 85)
- public/r/button.json (secondary variant)
Action items:
- Either extend your Tailwind config to add a colored-shadow utility (via
boxShadow
in your config or ashadowColor
plugin), - Or replace
shadow-primary
with an arbitrary-value class, for example:
- "[&>div]:bg-secondary [&>div]:border-black [&>div]:shadow-md [&>div]:shadow-primary", + "[&>div]:bg-secondary [&>div]:border-black [&>div]:shadow-md [&>div]:shadow-[color:var(--primary)]",
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
secondary: | |
"[&>div]:bg-secondary [&>div]:border-black [&>div]:shadow-md [&>div]:shadow-primary", | |
outline: "[&>div]:bg-transparent [&>div]:border-black [&>div]:shadow-md", | |
secondary: | |
"[&>div]:bg-secondary [&>div]:border-black [&>div]:shadow-md [&>div]:shadow-[color:var(--primary)]", | |
outline: "[&>div]:bg-transparent [&>div]:border-black [&>div]:shadow-md", |
🤖 Prompt for AI Agents
In components/retroui/Loader.tsx around lines 9 to 11, the class "shadow-primary" is unsupported and compiles to a no-op; fix by either (A) replacing "shadow-primary" with a valid Tailwind arbitrary shadow/color class (e.g. use an arbitrary box-shadow or shadow + shadow-color arbitrary value) in the variant strings so the shadow is actually rendered, or (B) add a custom entry in tailwind.config.js to define a colored shadow utility (extend theme.boxShadow or install/configure a shadowColor plugin) and rebuild so "shadow-primary" produces CSS.
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Thank you for your contribution 🙏 |
This PR introduces a new Loader component to RetroUI with the following features:
Features
Summary by CodeRabbit
New Features
Documentation
Style