feat(empty-state): new TEDI-Ready component #10#611
Conversation
|
Warning Rate limit exceeded
To continue reviewing without waiting, purchase usage credits in the billing tab. ⌛ 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. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThis PR introduces a complete ChangesEmptyState Component
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
…nly Icon related props #10
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/tedi/components/notifications/empty-state/empty-state.stories.tsx (1)
136-139: ⚡ Quick winStory comment is outdated:
iconno longer accepts arbitraryReactNode.Please align this note with the current prop type (
string | IconWithoutBackgroundProps | null) to avoid misleading usage guidance.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/tedi/components/notifications/empty-state/empty-state.stories.tsx` around lines 136 - 139, The story comment for the `icon` prop is outdated — it says "Any ReactNode" but the prop type is now `string | IconWithoutBackgroundProps | null`; update the comment in empty-state.stories.tsx to state that `icon` accepts either a string (icon name), an `IconWithoutBackgroundProps` object, or null, and remove the reference to arbitrary `ReactNode` so users are not misled when using the `icon` prop of the EmptyState story/component.src/tedi/components/notifications/empty-state/empty-state.spec.tsx (1)
13-42: ⚡ Quick winUse semantic RTL queries for icon/heading assertions instead of
querySelector.These tests can rely on
screen.getByRole(...)/screen.getByText(...)and avoid DOM selectors for behavior-level assertions.Suggested direction
- const { container } = render(<EmptyState heading="Choose new time">You have no data to display</EmptyState>); - const heading = container.querySelector('h3'); - expect(heading).toBeInTheDocument(); - expect(heading).toHaveTextContent('Choose new time'); + render(<EmptyState heading="Choose new time">You have no data to display</EmptyState>); + expect(screen.getByRole('heading', { level: 3, name: 'Choose new time' })).toBeInTheDocument();As per coding guidelines, "Use semantic queries in tests (
getByRole,getByLabelText) instead of non-semantic queries (getByTestId)."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/tedi/components/notifications/empty-state/empty-state.spec.tsx` around lines 13 - 42, Replace DOM querySelector checks with semantic RTL queries: for icon assertions in tests like "renders the default spa icon..." / "renders the icon named by a string icon prop" / "accepts a full IconProps object for the icon" use screen.getByText('spa' | 'event_busy' | 'inbox') or, if the icon renders with an accessible name/role, use screen.getByRole('img', { name: /spa|event_busy|inbox/i }) to assert presence and text; for the "hides the icon when icon is null" test use screen.queryByText(...) or screen.queryByRole('img') to assert absence; and for the heading test replace container.querySelector('h3') with screen.getByRole('heading', { level: 3, name: 'Choose new time' }) or screen.getByText('Choose new time'). Update imports to use screen from `@testing-library/react` if needed and adjust the four/five tests to use these semantic queries instead of container.querySelector('[data-name="icon"]') and container.querySelector('h3').
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/tedi/components/notifications/empty-state/empty-state.spec.tsx`:
- Around line 13-42: Replace DOM querySelector checks with semantic RTL queries:
for icon assertions in tests like "renders the default spa icon..." / "renders
the icon named by a string icon prop" / "accepts a full IconProps object for the
icon" use screen.getByText('spa' | 'event_busy' | 'inbox') or, if the icon
renders with an accessible name/role, use screen.getByRole('img', { name:
/spa|event_busy|inbox/i }) to assert presence and text; for the "hides the icon
when icon is null" test use screen.queryByText(...) or screen.queryByRole('img')
to assert absence; and for the heading test replace
container.querySelector('h3') with screen.getByRole('heading', { level: 3, name:
'Choose new time' }) or screen.getByText('Choose new time'). Update imports to
use screen from `@testing-library/react` if needed and adjust the four/five tests
to use these semantic queries instead of
container.querySelector('[data-name="icon"]') and container.querySelector('h3').
In `@src/tedi/components/notifications/empty-state/empty-state.stories.tsx`:
- Around line 136-139: The story comment for the `icon` prop is outdated — it
says "Any ReactNode" but the prop type is now `string |
IconWithoutBackgroundProps | null`; update the comment in
empty-state.stories.tsx to state that `icon` accepts either a string (icon
name), an `IconWithoutBackgroundProps` object, or null, and remove the reference
to arbitrary `ReactNode` so users are not misled when using the `icon` prop of
the EmptyState story/component.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9f0120e7-e713-4624-83b0-bf0897031fb4
📒 Files selected for processing (6)
src/tedi/components/notifications/empty-state/empty-state.module.scsssrc/tedi/components/notifications/empty-state/empty-state.spec.tsxsrc/tedi/components/notifications/empty-state/empty-state.stories.tsxsrc/tedi/components/notifications/empty-state/empty-state.tsxsrc/tedi/components/notifications/empty-state/index.tssrc/tedi/index.ts
…ht folder, fix export #10
2 participants
Summary by CodeRabbit
New Features
Tests
Documentation