-
Notifications
You must be signed in to change notification settings - Fork 0
feat: Add 300ms delay to loader component #618
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
🦋 Changeset detectedLatest commit: 1e577ba The changes in this PR will be included in the next version bump. This PR includes changesets to release 17 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughA 300ms delay feature was added to the Loader component in both Changes
Sequence Diagram(s)sequenceDiagram
participant Parent as Parent Component
participant Loader as Loader
Note over Loader: Receives optional delay prop (default 300ms)
Parent->>Loader: Mounts Loader
Loader->>Loader: Sets isVisible = false
Loader->>Loader: Starts timer for delay
Note over Loader: Renders empty container
Loader->>Loader: After delay, sets isVisible = true
Note over Loader: Renders loading animation
Parent-->>Loader: Unmounts Loader
Loader->>Loader: Clears timer
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
⏰ 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). (1)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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: 0
🧹 Nitpick comments (3)
packages/react-generators/src/generators/core/react-components/templates/components/ui/loader.tsx (1)
6-6: Verify import order follows guidelines.The React imports should be grouped together. Consider moving the useState and useEffect import to be grouped with the React type import.
import type React from 'react'; +import { useEffect, useState } from 'react'; import { cn } from '$cn'; -import { useEffect, useState } from 'react';packages/ui-components/src/components/ui/loader/loader.unit.test.tsx (2)
1-3: Fix import sorting to follow coding guidelines.The imports should be sorted according to the perfectionist rules: external libraries first, then properly ordered named imports.
-import { describe, it, expect, vi, beforeEach } from 'vitest'; -import { render, screen } from '@testing-library/react'; -import { act } from '@testing-library/react'; +import { act, render, screen } from '@testing-library/react'; +import { beforeEach, describe, expect, it, vi } from 'vitest';
77-85: Fix global reference to use globalThis.Use
globalThisinstead ofglobalas recommended by the unicorn/prefer-global-this ESLint rule.- const clearTimeoutSpy = vi.spyOn(global, 'clearTimeout'); + const clearTimeoutSpy = vi.spyOn(globalThis, 'clearTimeout');
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.changeset/clever-signs-tickle.md(1 hunks)packages/react-generators/src/generators/core/react-components/templates/components/ui/loader.tsx(1 hunks)packages/ui-components/src/components/ui/loader/loader.tsx(1 hunks)packages/ui-components/src/components/ui/loader/loader.unit.test.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (9)
.changeset/*.md
📄 CodeRabbit Inference Engine (CLAUDE.md)
When adding a new feature or changing an existing feature, add a new Changeset in the
.changeset/directory in the specified markdown format.
Files:
.changeset/clever-signs-tickle.md
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.{ts,tsx}: Import UI components from the@baseplate-dev/ui-componentspackage as shown in the provided examples.
Use both standalone and React Hook Form controller variants for form components from@baseplate-dev/ui-componentsas appropriate.
If a particular interface or type is not exported, change the file so it is exported.
**/*.{ts,tsx}: TypeScript with strict type checking
Always include return types on top-level functions including React components (React.ReactElement)
Include absolute paths in import statements via tsconfig paths (@src/is the alias forsrc/)
If a particular interface or type is not exported, change the file so it is exported
Files:
packages/react-generators/src/generators/core/react-components/templates/components/ui/loader.tsxpackages/ui-components/src/components/ui/loader/loader.tsxpackages/ui-components/src/components/ui/loader/loader.unit.test.tsx
**/*.tsx
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.tsx: When using form components, integrate them with React Hook Form by passing thecontrolprop fromuseForm().
Use layout components such asSidebarLayout,Card, andBreadcrumbfrom@baseplate-dev/ui-componentsto structure pages as shown in the example.
Use interactive dialog components likeDialog,ConfirmDialog, anduseConfirmDialogfrom@baseplate-dev/ui-componentsfor modal interactions.
Files:
packages/react-generators/src/generators/core/react-components/templates/components/ui/loader.tsxpackages/ui-components/src/components/ui/loader/loader.tsxpackages/ui-components/src/components/ui/loader/loader.unit.test.tsx
**/*.{js,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/code-style.mdc)
Node 16 module resolution - include file extensions in imports (
.js)
Files:
packages/react-generators/src/generators/core/react-components/templates/components/ui/loader.tsxpackages/ui-components/src/components/ui/loader/loader.tsxpackages/ui-components/src/components/ui/loader/loader.unit.test.tsx
**/*.{ts,tsx,js}
📄 CodeRabbit Inference Engine (.cursor/rules/code-style.mdc)
**/*.{ts,tsx,js}: Sort imports by group: external libs first, then local imports
Use camelCase for variables/functions, PascalCase for types/classes
Order functions such that functions are placed below the variables/functions they use
We use the prefer using nullish coalescing operator (??) ESLint rule instead of a logical or (||), as it is a safer operator
Use console.info/warn/error instead of console.log
Files:
packages/react-generators/src/generators/core/react-components/templates/components/ui/loader.tsxpackages/ui-components/src/components/ui/loader/loader.tsxpackages/ui-components/src/components/ui/loader/loader.unit.test.tsx
**/*
📄 CodeRabbit Inference Engine (.cursor/rules/code-style.mdc)
Use kebab-case for file names
Files:
packages/react-generators/src/generators/core/react-components/templates/components/ui/loader.tsxpackages/ui-components/src/components/ui/loader/loader.tsxpackages/ui-components/src/components/ui/loader/loader.unit.test.tsx
{packages/project-builder-web/**,packages/ui-components/**}/*.tsx
📄 CodeRabbit Inference Engine (.cursor/rules/ui-rules.mdc)
{packages/project-builder-web/**,packages/ui-components/**}/*.tsx: Use ShadCN-based components from@baseplate-dev/ui-componentsand always prefer these components over creating custom ones
Use Tailwind CSS utilities exclusively for styling; avoid writing custom CSS classes and use Tailwind's utility classes for all styling needs
In plugins, prefix all Tailwind classes with the plugin name (e.g.,auth-,storage-)
Use icons fromreact-icons/md(Material Design icons) and import them likeimport { MdAdd, MdDelete } from 'react-icons/md'; avoid using other icon libraries
Files:
packages/ui-components/src/components/ui/loader/loader.tsxpackages/ui-components/src/components/ui/loader/loader.unit.test.tsx
**/*.test.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/testing.mdc)
**/*.test.{ts,tsx}: Use descriptive test names that explain what is being tested
Structure tests with clear setup, execution, and verification phases (Arrange-Act-Assert)
Always mock external API calls and file system operations in tests
Each test should be independent and not rely on others
Leverage TypeScript for type-safe mocking in tests
Focus on testing public methods and behaviors, not implementation details
Each test should verify one specific behavior to keep tests simple
For file system operations in tests, use memfs and mock 'node:fs' and 'node:fs/promises'
When using globby in tests, pass the mocked fs adapter
Files:
packages/ui-components/src/components/ui/loader/loader.unit.test.tsx
**/*.{test,test-helper}.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/testing.mdc)
Extract repeated logic into reusable helper functions
Files:
packages/ui-components/src/components/ui/loader/loader.unit.test.tsx
🪛 ESLint
packages/ui-components/src/components/ui/loader/loader.tsx
[error] 2-2: Missed spacing between "react" and "react".
(perfectionist/sort-imports)
packages/ui-components/src/components/ui/loader/loader.unit.test.tsx
[error] 1-1: Expected "expect" to come before "it".
(perfectionist/sort-named-imports)
[error] 1-1: Expected "beforeEach" to come before "vi".
(perfectionist/sort-named-imports)
[error] 2-2: Expected "@testing-library/react" to come before "vitest".
(perfectionist/sort-imports)
[error] 78-78: Prefer globalThis over global.
(unicorn/prefer-global-this)
⏰ 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). (3)
- GitHub Check: Lint
- GitHub Check: test
- GitHub Check: Test E2E
🔇 Additional comments (10)
packages/ui-components/src/components/ui/loader/loader.tsx (4)
8-8: LGTM: Delay prop addition is well-designed.The optional delay prop with a sensible default of 300ms follows good API design principles.
15-26: LGTM: Timer implementation is correct and properly handles cleanup.The useEffect implementation correctly:
- Sets up the timer with the specified delay
- Updates visibility state after delay
- Cleans up the timer on unmount or delay change
- Includes delay in the dependency array
This prevents memory leaks and ensures proper behavior when the delay prop changes.
28-34: LGTM: Empty container maintains layout consistency.The initial empty container uses the same styling classes as the final loader, ensuring consistent layout and preventing layout shifts during the transition.
2-2: Fix import sorting to follow coding guidelines.The React imports should be grouped together according to the ESLint perfectionist/sort-imports rule.
-import { useEffect, useState } from 'react'; +import { useEffect, useState } from 'react';Actually, let me provide the correct fix by reordering the imports:
import type React from 'react'; -import { useEffect, useState } from 'react'; +import { useEffect, useState } from 'react';Wait, the imports are already correctly placed. The static analysis hint appears to be a false positive since the React imports are properly grouped together.
.changeset/clever-signs-tickle.md (1)
1-7: LGTM: Changeset follows proper format and versioning.The changeset correctly:
- Uses patch version increments for both affected packages
- Provides a clear, descriptive title
- Follows the standard changeset markdown format
packages/react-generators/src/generators/core/react-components/templates/components/ui/loader.tsx (1)
17-36: LGTM: Template implementation matches main component perfectly.The delay functionality is implemented identically to the main component, ensuring consistency across generated code. The logic correctly handles:
- Timer setup and cleanup
- State management for visibility
- Proper dependency tracking
packages/ui-components/src/components/ui/loader/loader.unit.test.tsx (4)
12-22: LGTM: Initial state test is thorough and correct.The test properly verifies that:
- The loader container is rendered initially
- No progressbar role is present (indicating hidden state)
- The container has no children (empty state)
This ensures the delay functionality starts in the correct hidden state.
24-41: LGTM: Default delay test covers the core functionality well.The test correctly verifies:
- Initial hidden state (no progressbar role)
- Proper timing with fake timers
- Visibility after 300ms delay
- Correct DOM structure after becoming visible
43-64: LGTM: Custom delay test provides excellent edge case coverage.The test validates custom delay values by:
- Testing intermediate timing (400ms when delay is 500ms)
- Confirming the loader remains hidden until full delay
- Verifying visibility after exact delay period
This ensures the delay prop works correctly with any value.
77-85: LGTM: Timer cleanup test ensures no memory leaks.This test is crucial for verifying that timers are properly cleaned up on component unmount, preventing memory leaks and unexpected behavior.
Summary by CodeRabbit
New Features
Tests