feat(EmptyState): add EmptyState component #1491
Conversation
WalkthroughThe pull request introduces a new EmptyState component for React, including TypeScript definitions, styling, documentation in Storybook format, and unit tests. It provides various configurations for displaying content when no data is available, such as inline, centered, and full-width layouts. The implementation features SCSS modules for styling, comprehensive Storybook stories to illustrate usage, and tests to ensure proper rendering behavior. Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (2)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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
CodeRabbit Configuration File (
|
…image size in stories
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (12)
packages/react-components/src/components/EmptyState/types.ts (1)
3-21: Add JSDoc documentation for better DXThe type definition is well-structured but would benefit from documentation explaining the different variants and prop combinations.
Add JSDoc comments like this:
+/** + * Props for the EmptyState component + * @property {('full'|'inline')} type - Layout type. 'full' supports description and image, 'inline' is compact + * @property {string} title - Main message to display + * @property {ReactNode} [actions] - Optional action buttons/links + * @property {boolean} [centered] - Whether to center the content + * @property {string} [image] - Optional image URL (mutually exclusive with icon) + * @property {ReactNode} [icon] - Optional icon element (mutually exclusive with image) + */ export type IEmptyStateProps = (packages/react-components/src/components/EmptyState/EmptyState.tsx (2)
21-27: Add aria-label for accessibilityThe empty state should be labeled for screen readers.
<div className={cx( styles[baseClass], styles[`${baseClass}--${type}`], centered && styles[`${baseClass}--centered`] )} + aria-label={`Empty state: ${title}`} >
11-73: Consider memoizing the componentFor performance optimization, especially with complex children in actions or icon props.
-export const EmptyState = ({ +export const EmptyState = React.memo(({ type = 'full', // ... other props -}: IEmptyStateProps) => { +}: IEmptyStateProps) => { // component implementation -}; +});🧰 Tools
🪛 Biome (1.9.4)
[error] 54-60: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment(lint/complexity/noUselessFragments)
packages/react-components/src/components/EmptyState/EmptyState.stories.tsx (3)
26-42: Consider adding aria-label to buttonsThe Default story's buttons lack accessibility labels, which could impact screen reader users.
- <Button kind="primary">Primary action</Button> + <Button kind="primary" aria-label="Primary action">Primary action</Button> - <Button kind="secondary">Secondary action</Button> + <Button kind="secondary" aria-label="Secondary action">Secondary action</Button> - <Button icon={<Icon source={InfoIcon} />} kind="secondary"> + <Button icon={<Icon source={InfoIcon} />} kind="secondary" aria-label="More information"> Tell me more </Button> - <Button kind="link">Link action</Button> + <Button kind="link" aria-label="Link action">Link action</Button>
44-80: Add loading state to ActionMenuThe Inline story's ActionMenu might benefit from a loading state for better UX.
<ActionMenu + isLoading={false} options={[
130-151: Fix typo in Facebook MessengerThere's a typo in the social media platform name.
- <Text bold>Facebook Messanger</Text> + <Text bold>Facebook Messenger</Text>packages/react-components/src/components/EmptyState/EmptyState.stories.module.scss (2)
16-18: Use consistent media query breakpointsConsider extracting the breakpoint value to a variable for better maintainability.
+$mobile-breakpoint: 600px; + @media (width <= 600px) {Also applies to: 28-30
21-27: Use design tokens for measurementsReplace hardcoded values with design system tokens for consistency.
- border-radius: 8px; + border-radius: var(--radius-m); - width: 220px; + width: var(--width-box-default);packages/react-components/src/components/EmptyState/EmptyState.mdx (1)
21-28: Add accessibility guidelinesConsider adding a section about accessibility best practices and ARIA attributes.
#### Example implementation + +#### Accessibility + +The EmptyState component follows WCAG guidelines: +- Use semantic HTML elements +- Ensure proper heading hierarchy +- Provide descriptive button labelspackages/react-components/src/components/EmptyState/EmptyState.module.scss (3)
23-25: Extract hardcoded breakpoint value into a variable.Consider defining the breakpoint value (600px) as a design system variable for consistency and maintainability.
+$breakpoint-mobile: 600px; + .#{$base-class} { // ... - @media (width <= 600px) { + @media (width <= $breakpoint-mobile) {
42-58: Extract repeated max-width value into a variable.The max-width value of 600px is used multiple times. Consider extracting it into a variable for better maintainability.
+$content-max-width: 600px; + .#{$base-class} { // ... &__description { - max-width: 600px; + max-width: $content-max-width; text-align: center; } // ... &__title { margin: 0; margin-bottom: var(--spacing-2); - max-width: 600px; + max-width: $content-max-width; text-align: center; }
69-76: Use the same breakpoint variable here.Once the breakpoint variable is extracted, update this media query to use it as well.
- @media (width <= 600px) { + @media (width <= $breakpoint-mobile) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
packages/react-components/src/components/EmptyState/EmptyState.mdx(1 hunks)packages/react-components/src/components/EmptyState/EmptyState.module.scss(1 hunks)packages/react-components/src/components/EmptyState/EmptyState.spec.tsx(1 hunks)packages/react-components/src/components/EmptyState/EmptyState.stories.module.scss(1 hunks)packages/react-components/src/components/EmptyState/EmptyState.stories.tsx(1 hunks)packages/react-components/src/components/EmptyState/EmptyState.tsx(1 hunks)packages/react-components/src/components/EmptyState/index.ts(1 hunks)packages/react-components/src/components/EmptyState/types.ts(1 hunks)packages/react-components/src/index.ts(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
packages/react-components/src/components/EmptyState/EmptyState.tsx
[error] 54-60: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
🔇 Additional comments (4)
packages/react-components/src/components/EmptyState/index.ts (1)
1-2: Clean exports following barrel pattern!packages/react-components/src/index.ts (1)
24-24: LGTM!The EmptyState component export follows the alphabetical ordering convention.
packages/react-components/src/components/EmptyState/EmptyState.stories.tsx (1)
1-24: LGTM!Clean imports organization and proper Storybook setup with Meta and Story types.
packages/react-components/src/components/EmptyState/EmptyState.module.scss (1)
28-40: LGTM! Well-structured image and icon styles.The styles handle responsive sizing appropriately and use design system tokens consistently.
packages/react-components/src/components/EmptyState/EmptyState.tsx
Outdated
Show resolved
Hide resolved
JoannaSikora
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
packages/react-components/src/components/EmptyState/EmptyState.stories.tsx (6)
26-42: Consider using a more reliable image source.Replace the placeholder URL with a local asset or a more reliable CDN to prevent potential story rendering issues.
- image: 'https://placehold.co/600x300', + image: '/assets/empty-state-default.png',
64-64: Document the purpose of openedOnInit.Add a comment explaining why the menu needs to be initially opened in this story.
85-85: Document the purpose of the custom icon class.Add a comment explaining what styles
empty-state-story__iconapplies and why they're needed.
122-128: Use a reliable image source.Replace the placeholder URL with a local asset for reliability.
- image: 'https://placehold.co/250x200', + image: '/assets/empty-state-small.png',
136-146: Consider more generic class names for reusability.The current class names are tightly coupled to the story. Consider more generic names if these styles might be reused.
160-160: Replace inline styles with CSS modules.Move the margin style to the CSS module for consistency with the rest of the stories.
- <Text style={{ margin: 0 }}> + <Text className={styles['empty-state-story__description']}>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/react-components/src/components/EmptyState/EmptyState.stories.tsx(1 hunks)packages/react-components/src/components/EmptyState/types.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/react-components/src/components/EmptyState/types.ts
🧰 Additional context used
📓 Learnings (1)
📓 Common learnings
Learnt from: JoannaSikora
PR: livechat/design-system#1491
File: packages/react-components/src/components/EmptyState/EmptyState.spec.tsx:15-50
Timestamp: 2025-01-22T12:18:56.927Z
Learning: For React components, styling and layout aspects (like centering, inline vs block display, etc.) should be verified through visual testing tools rather than React Testing Library (RTL) tests. RTL tests should focus on behavior and functionality instead.
🔇 Additional comments (2)
packages/react-components/src/components/EmptyState/EmptyState.stories.tsx (2)
1-24: LGTM! Well-organized imports and proper Storybook setup.The imports are logically grouped and the story configuration follows Storybook best practices.
113-119: Verify if the decorator is necessary.Since the component has a
centeredprop, the decorator might be redundant. Consider removing it if the prop handles centering.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/react-components/src/components/EmptyState/EmptyState.stories.tsx (3)
28-28: Consider using local assets instead of external placeholder service.Replace
placehold.cowith a local asset to ensure reliability in the stories.
140-142: Consider enforcing character limits in the component.The story mentions character limits (50 for title, 100 for description) but these aren't enforced. Consider adding prop validation in the component itself.
168-168: Move inline styles to CSS module.Replace
style={{ margin: 0 }}with a class in the CSS module for better maintainability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/react-components/src/components/EmptyState/EmptyState.module.scss(1 hunks)packages/react-components/src/components/EmptyState/EmptyState.stories.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/react-components/src/components/EmptyState/EmptyState.module.scss
🧰 Additional context used
📓 Learnings (1)
📓 Common learnings
Learnt from: JoannaSikora
PR: livechat/design-system#1491
File: packages/react-components/src/components/EmptyState/EmptyState.spec.tsx:15-50
Timestamp: 2025-01-22T12:18:56.927Z
Learning: For React components, styling and layout aspects (like centering, inline vs block display, etc.) should be verified through visual testing tools rather than React Testing Library (RTL) tests. RTL tests should focus on behavior and functionality instead.
🔇 Additional comments (4)
packages/react-components/src/components/EmptyState/EmptyState.stories.tsx (4)
1-24: LGTM! Clean imports and proper Storybook setup.
64-64: Verify visual regression tests with openedOnInit.The
openedOnInitprop might affect visual regression tests. Consider adding a specific test case for this scenario.
82-96: LGTM! Good demonstration of icon usage.
98-120: Verify centered layout in visual tests.As per our learnings, ensure the centered layout is verified through visual testing tools rather than RTL tests.
Resolves: #1379
Description
This pull request introduces the new
EmptyStatecomponent.New
EmptyStateComponent:EmptyStatecomponent implementation, which supports different types (fullandinline), and includes properties for:Figma: https://www.figma.com/design/9FDwjR8lYvincseDkKypC4/%5BDS%5D-Component-Documentations?node-id=24836-20224&m=dev
Storybook
https://feature-1379--613a8e945a5665003a05113b.chromatic.com
Checklist
Obligatory:
Summary by CodeRabbit
New Features
EmptyStatecomponent for displaying content when no data is available.Documentation
EmptyStatecomponent with multiple variations and a Figma link for design specifications.Tests
EmptyStatecomponent.