feat: new empty placeholder component #2792
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
📝 WalkthroughWalkthroughThis pull request introduces a comprehensive replacement of the custom Changes
Suggested labels
Suggested reviewers
Possibly related PRs
📜 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 (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (
|
|
Thank you for following the naming conventions for pull request titles! 🙏 |
…-overhaul-emptyplaceholder
MichaelUnkey
requested review from
perkinsjr,
chronark,
mcstepp and
ogzhanolguncu
as code owners
|
MichaelUnkey (GitHub user) ▸ What does this PR do? Added an new component to use when no data is available. Includes Top icon, title, description, and action element. eng-1606 Type of change
How should this be tested?
Required
Appreciated
|
![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
🔭 Outside diff range comments (1)
apps/dashboard/components/opt-in.tsx (1)
Line range hint
22-35: Remove duplicate analytics event.The same PostHogEvent is being triggered in both
onMutateandonSuccess. Consider keeping only theonSuccessevent to avoid duplicate tracking.const optIn = trpc.workspace.optIntoBeta.useMutation({ - onMutate() { - PostHogEvent({ - name: "self-serve-opt-in", - properties: { feature }, - }); - }, onSuccess() { PostHogEvent({ name: "self-serve-opt-in", properties: { feature }, }); toast.success("Successfully opted in"); router.refresh(); }, onError(err) { toast.error(err.message); }, });
🧹 Nitpick comments (19)
apps/engineering/content/design/components/empty.example.tsx (2)
18-20: Consider using variant prop instead of direct background class.The Button component likely supports variants through props rather than requiring direct background color classes.
- <Button className="bg-gray-2"> + <Button variant="secondary"> <BookOpen /> Example action button with icon </Button>
15-16: Consider using more meaningful example text.The current example text is generic. Consider using text that better represents real-world usage scenarios.
- <Empty.Title>Example Title Text</Empty.Title> - <Empty.Description>Example of Description Text.</Empty.Description> + <Empty.Title>No Results Found</Empty.Title> + <Empty.Description>Try adjusting your search filters to find what you're looking for.</Empty.Description>internal/ui/src/components/empty.tsx (2)
5-7: Add type for children prop in EmptyRootProps interfaceThe interface should explicitly define the children prop type for better type safety.
interface EmptyRootProps extends React.HTMLAttributes<HTMLDivElement> { fill: boolean | undefined; + children: React.ReactNode; }
40-61: Use consistent type definitionsFor consistency, consider using interfaces instead of type aliases for EmptyTitleProps, EmptyDescriptionProps, and EmptyActionProps as they represent object types.
-type EmptyTitleProps = React.HTMLAttributes<HTMLHeadingElement>; +interface EmptyTitleProps extends React.HTMLAttributes<HTMLHeadingElement> {} -type EmptyDescriptionProps = React.HTMLAttributes<HTMLParagraphElement>; +interface EmptyDescriptionProps extends React.HTMLAttributes<HTMLParagraphElement> {} -type EmptyActionProps = React.HTMLAttributes<HTMLDivElement>; +interface EmptyActionProps extends React.HTMLAttributes<HTMLDivElement> {}apps/dashboard/app/(app)/settings/billing/stripe/page.tsx (1)
34-39: Consider adding an icon for consistencyOther instances of the Empty component across the application include an icon. Consider adding an appropriate icon (e.g., AlertTriangle or Settings) to maintain consistency.
<Empty fill={true}> + <Empty.Icon> + <AlertTriangle className="w-8 h-8" /> + </Empty.Icon> <Empty.Title>Stripe is not configured</Empty.Title> <Empty.Description> If you are selfhosting Unkey, you need to configure Stripe in your environment variables. </Empty.Description> </Empty>apps/dashboard/app/integrations/vercel/callback/page.tsx (1)
Line range hint
69-84: Consider improving error message readabilityWhile the error handling is comprehensive, consider making the error message more user-friendly by formatting the technical details in a collapsible section.
<Empty className="m-8" fill={true}> <Empty.Title>Error</Empty.Title> <Empty.Description> We couldn't load your projects from Vercel. Please try again or contact [email protected] </Empty.Description> + <details className="mt-4"> + <summary className="cursor-pointer">Technical Details</summary> <Code className="text-left"> {JSON.stringify( { message: projects.err.message, context: projects.err.context, }, null, 2, )} </Code> + </details> </Empty>apps/dashboard/app/(app)/apis/client.tsx (1)
79-96: Add consistent button stylingThe implementation looks good, but the documentation button is missing className for consistent styling with other buttons.
<Empty.Action> <CreateApiButton key="createApi" /> <Link href="/docs" target="_blank"> - <Button> + <Button className="items-center w-full gap-2"> <BookOpen /> Read the docs </Button> </Link> </Empty.Action>apps/dashboard/app/(app)/settings/billing/stripe/success/page.tsx (1)
39-44: Standardize error message structure across empty states.While the empty states are well-implemented, consider standardizing the error message structure for consistency:
- Title: Clear error message
- Description: Technical details (IDs)
- Action: Support contact information
Example structure for the session not found case:
<Empty fill={true}> <Empty.Title>Stripe session not found</Empty.Title> - <Empty.Description>The Stripe session</Empty.Description> - <Code>{session_id}</Code> - <Empty.Description> - you are trying to access does not exist. Please contact [email protected]. - </Empty.Description> + <Empty.Description> + The Stripe session {session_id} does not exist. + </Empty.Description> + <Empty.Action> + <Button onClick={() => window.location.href = 'mailto:[email protected]'}> + Contact Support + </Button> + </Empty.Action> </Empty>Also applies to: 56-63, 69-76
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/keys.tsx (1)
73-73: Remove commented-out code.The commented-out code for
CreateNewRoleshould be removed as it appears to be unused.- {/* <CreateNewRole trigger={<Button variant="primary">Create New Role</Button>} /> */}apps/dashboard/app/(app)/settings/vercel/page.tsx (1)
86-94: Consider consolidating error descriptionsInstead of using two separate Empty.Description components, consider consolidating them into a single description with better formatting:
<Empty fill={true}> <Empty.Title>Error</Empty.Title> <Empty.Description> We couldn't load your projects from Vercel. Please try again or contact support. - </Empty.Description> - <Empty.Description> <Code className="text-left mt-4">{JSON.stringify(err, null, 2)}</Code> </Empty.Description> </Empty>apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/page.tsx (1)
159-175: Fix inconsistent JSX spacingWhile the Empty component implementation is correct, there's inconsistent spacing in the JSX structure. Consider fixing the indentation:
<Empty fill={true}> <Empty.Icon> <Box /> </Empty.Icon> <Empty.Title>No logs found</Empty.Title> {isFiltered ? ( <div className="flex flex-col items-center gap-2"> <Empty.Description> No events matched these filters, try changing them.{" "} </Empty.Description> </div> ) : ( <Empty.Description> Create, update or delete something and come back again. </Empty.Description> )} </Empty>apps/dashboard/app/(app)/settings/team/page.tsx (1)
217-221: Wrap the InviteButton in Empty.Action.For consistency with other empty states and to follow the component's API, consider wrapping the
InviteButtoninEmpty.Action.<Empty fill={true}> <Empty.Title>No pending invitations</Empty.Title> <Empty.Description>Invite members to your team</Empty.Description> - <InviteButton /> + <Empty.Action> + <InviteButton /> + </Empty.Action> </Empty>apps/dashboard/app/(app)/ratelimits/[namespaceId]/page.tsx (1)
227-237: Wrap the code snippet in Empty.Action.For consistency with other empty states and to follow the component's API, consider wrapping the code snippet in
Empty.Action.<Empty fill={true}> <Empty.Icon> <BarChart /> </Empty.Icon> <Empty.Title>No usage</Empty.Title> <Empty.Description>Ratelimit something or change the range</Empty.Description> - <Code className="flex items-start gap-0 sm:gap-8 p-4 my-8 text-xs sm:text-xxs text-start overflow-x-auto max-w-full"> - {snippet} - <CopyButton value={snippet} /> - </Code> + <Empty.Action> + <Code className="flex items-start gap-0 sm:gap-8 p-4 my-8 text-xs sm:text-xxs text-start overflow-x-auto max-w-full"> + {snippet} + <CopyButton value={snippet} /> + </Code> + </Empty.Action> </Empty>apps/engineering/content/design/components/empty.mdx (2)
11-11: Consider expanding the overview section.The overview could benefit from more detailed guidance on when to use this component, including specific use cases and best practices.
29-31: Add code snippet for the full example.While the example is rendered via
<EmptyExample/>, it would be helpful to include the actual code snippet in the documentation for direct reference.Consider adding:
## Full Example <EmptyExample/> + +```tsx +import { Empty } from "@unkey/ui"; + +export function EmptyExample() { + return ( + <Empty> + <Empty.Icon><YourIcon /></Empty.Icon> + <Empty.Title>No Data Available</Empty.Title> + <Empty.Description>Add some data to get started</Empty.Description> + <Empty.Action> + <Button>Add Data</Button> + </Empty.Action> + </Empty> + ); +} +```apps/dashboard/app/(app)/apis/[apiId]/page.tsx (2)
249-255: Consider differentiating the empty states.While the implementation is correct, using identical empty states for both verifications and active keys might not provide the best user experience. Consider customizing the messages and actions for each case.
For verifications, you could add:
<Empty fill={true}> <Empty.Icon> <BarChart /> </Empty.Icon> <Empty.Title>No usage</Empty.Title> <Empty.Description>Verify a key or change the range</Empty.Description> + <Empty.Action> + <Button onClick={() => setInterval("24h")}>Try Last 24 Hours</Button> + </Empty.Action> </Empty>
291-297: Add an action to guide users.The empty state for active keys could benefit from an action that helps users understand how to get started.
<Empty fill={true}> <Empty.Icon> <BarChart /> </Empty.Icon> <Empty.Title>No usage</Empty.Title> <Empty.Description>Verify a key or change the range</Empty.Description> + <Empty.Action> + <CreateKeyButton apiId={api.id} keyAuthId={key.keyAuthId} /> + </Empty.Action> </Empty>apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/page.tsx (1)
355-361: Add an action to help users get started.The empty state could be more helpful by providing an action that guides users on how to start using the key.
<Empty fill={true}> <Empty.Icon> <BarChart /> </Empty.Icon> <Empty.Title>Not used</Empty.Title> <Empty.Description>This key was not used in the last {interval}</Empty.Description> + <Empty.Action> + <Button asChild> + <a href="/docs/api-reference/verify" target="_blank" rel="noopener"> + View API Documentation + </a> + </Button> + </Empty.Action> </Empty>apps/dashboard/app/(app)/settings/vercel/client.tsx (1)
67-86: Consider extracting the SVG icon and enhancing the action button.The Empty component implementation is good, but there are a few suggestions for improvement:
- The SVG icon is duplicated (also used in the project list). Consider extracting it to a shared component.
- The hardcoded black color in the SVG might not work well with different themes.
- The ghost variant for the action button might not provide enough emphasis for the primary action.
- The external link should indicate that it opens in a new tab.
Here's a suggested improvement:
<Empty fill={true}> <Empty.Icon> - <svg - width="76" - height="65" - viewBox="0 0 76 65" - fill="none" - xmlns="http://www.w3.org/2000/svg" - > - <path d="M37.5274 0L75.0548 65H0L37.5274 0Z" fill="#000000" /> - </svg> + <VercelIcon className="w-[76px] h-[65px]" /> </Empty.Icon> <Empty.Title>No connected projects found</Empty.Title> <Empty.Description>Connect a Vercel project now</Empty.Description> <Empty.Action> <Link href="https://vercel.com/integrations/unkey" target="_blank"> - <Button variant="ghost">Vercel Integration</Button> + <Button> + Vercel Integration + <ExternalLink className="w-4 h-4 ml-2" /> + </Button> </Link> </Empty.Action> </Empty>Create a new shared component for the Vercel icon:
// components/icons/vercel-icon.tsx export const VercelIcon: React.FC<{ className?: string }> = ({ className }) => ( <svg className={className} viewBox="0 0 76 65" fill="none" xmlns="http://www.w3.org/2000/svg" > <path d="M37.5274 0L75.0548 65H0L37.5274 0Z" fill="currentColor" /> </svg> );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (31)
apps/dashboard/app/(app)/[...not-found]/page.tsx(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/page.tsx(2 hunks)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/keys.tsx(2 hunks)apps/dashboard/app/(app)/apis/[apiId]/page.tsx(3 hunks)apps/dashboard/app/(app)/apis/client.tsx(2 hunks)apps/dashboard/app/(app)/audit/components/table/audit-log-table-client.tsx(2 hunks)apps/dashboard/app/(app)/audit/page.tsx(2 hunks)apps/dashboard/app/(app)/authorization/permissions/page.tsx(2 hunks)apps/dashboard/app/(app)/authorization/roles/page.tsx(2 hunks)apps/dashboard/app/(app)/identities/page.tsx(2 hunks)apps/dashboard/app/(app)/layout.tsx(2 hunks)apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/page.tsx(3 hunks)apps/dashboard/app/(app)/ratelimits/[namespaceId]/overrides/page.tsx(2 hunks)apps/dashboard/app/(app)/ratelimits/[namespaceId]/page.tsx(2 hunks)apps/dashboard/app/(app)/ratelimits/page.tsx(2 hunks)apps/dashboard/app/(app)/settings/billing/stripe/page.tsx(2 hunks)apps/dashboard/app/(app)/settings/billing/stripe/success/page.tsx(3 hunks)apps/dashboard/app/(app)/settings/team/page.tsx(3 hunks)apps/dashboard/app/(app)/settings/user/update-user-email.tsx(2 hunks)apps/dashboard/app/(app)/settings/user/update-user-name.tsx(2 hunks)apps/dashboard/app/(app)/settings/vercel/client.tsx(2 hunks)apps/dashboard/app/(app)/settings/vercel/page.tsx(3 hunks)apps/dashboard/app/integrations/vercel/callback/page.tsx(3 hunks)apps/dashboard/app/new/keys.tsx(2 hunks)apps/dashboard/components/dashboard/empty-placeholder.tsx(0 hunks)apps/dashboard/components/opt-in.tsx(3 hunks)apps/dashboard/components/virtual-table.tsx(2 hunks)apps/engineering/content/design/components/empty.example.tsx(1 hunks)apps/engineering/content/design/components/empty.mdx(1 hunks)internal/ui/src/components/empty.tsx(1 hunks)internal/ui/src/index.ts(1 hunks)
💤 Files with no reviewable changes (1)
- apps/dashboard/components/dashboard/empty-placeholder.tsx
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Test Packages / Test ./internal/clickhouse
- GitHub Check: Test API / API Test Local
- GitHub Check: Test Agent Local / test_agent_local
- GitHub Check: Build / Build
🔇 Additional comments (25)
internal/ui/src/index.ts (1)
4-4: LGTM! Export statement follows consistent pattern.The export statement for the new Empty component follows the established pattern in the file.
apps/dashboard/app/(app)/[...not-found]/page.tsx (1)
6-14: LGTM! Clean implementation of the Empty component.The Empty component is appropriately used for the 404 page, maintaining the same user experience while using the new component structure.
apps/dashboard/components/opt-in.tsx (1)
Line range hint
43-59: LGTM! Clean implementation of the Empty component.The Empty component is appropriately used for the opt-in UI, maintaining the same user experience while using the new component structure.
apps/dashboard/app/(app)/audit/page.tsx (1)
41-50: LGTM! Clean implementation of the Empty componentThe Empty component is properly implemented with all necessary subcomponents and the fill prop is correctly set.
apps/dashboard/app/(app)/layout.tsx (1)
53-67: Verify the need for border-0 classNameThe Empty component is used with
className="border-0". Please clarify if this is intentional and why the default border needs to be removed in this specific instance.apps/dashboard/app/(app)/settings/user/update-user-name.tsx (1)
47-49: LGTM! Good use of Empty component for loading stateThe Empty component is appropriately used with correct props:
min-h-[200px]ensures consistent height during loadingfill={false}prevents unnecessary expansionapps/dashboard/app/integrations/vercel/callback/page.tsx (1)
90-96: LGTM! Clear empty state messagingGood implementation of the Empty component for the no-projects scenario with clear user guidance.
apps/dashboard/app/(app)/ratelimits/page.tsx (1)
73-94: LGTM! Excellent empty state implementationGreat implementation with:
- Clear messaging
- Helpful code snippet with copy functionality
- Documentation link for further guidance
apps/dashboard/app/(app)/ratelimits/[namespaceId]/overrides/page.tsx (1)
100-106: LGTM! Clean implementation of the empty state.The empty state implementation is clear, consistent, and provides good user guidance.
apps/dashboard/app/(app)/identities/page.tsx (1)
54-58: LGTM! Clean implementation of the loading state.The loading state implementation is simple, clean, and consistent with the application's patterns.
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/keys.tsx (1)
62-71: LGTM! Well-structured empty state with clear actions.The empty state implementation is clear, actionable, and follows the component's best practices.
apps/dashboard/app/(app)/authorization/roles/page.tsx (1)
8-8: Implementation of Empty component looks good!The migration from EmptyPlaceholder to Empty is well-structured with proper hierarchy of Empty.Icon, Empty.Title, Empty.Description, and Empty.Action components. The fill prop is correctly set to maintain consistent styling.
Also applies to: 86-98
apps/dashboard/app/(app)/authorization/permissions/page.tsx (1)
9-9: Implementation of Empty component looks good!The migration from EmptyPlaceholder to Empty is well-structured. Good addition of
className="w-full"to ensure proper width rendering.Also applies to: 83-94
apps/dashboard/app/(app)/settings/vercel/page.tsx (1)
64-71: Implementation of Empty component for disconnected state looks good!The Empty component is well-structured with proper Title and Action elements. The Button is correctly wrapped in a Link component for external navigation.
apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/page.tsx (1)
105-109: Loading state implementation looks good!The Empty component is appropriately used as a loading state with the Loading component as the icon.
apps/dashboard/app/new/keys.tsx (2)
18-18: LGTM!The import statement for the new
Emptycomponent is correctly placed.
124-133: LGTM!The
Emptycomponent is correctly implemented with proper nesting of description and button elements. The migration fromEmptyPlaceholdermaintains the same functionality while following the new component's API.apps/dashboard/app/(app)/settings/team/page.tsx (2)
3-3: LGTM!The import statement for the new
Emptycomponent is correctly placed.
64-72: LGTM!The
Emptycomponent is correctly implemented with proper nesting of title, description, and action elements. The migration maintains the same functionality while following the new component's API.apps/dashboard/app/(app)/ratelimits/[namespaceId]/page.tsx (1)
16-16: LGTM!The import statement for the new
Emptycomponent is correctly placed.apps/dashboard/components/virtual-table.tsx (2)
3-6: LGTM!The import statements for the new components and icons are correctly placed.
206-220: LGTM!The
Emptycomponent is correctly implemented with proper nesting of all elements. The migration enhances the user experience by providing a clear action to reset filters.apps/dashboard/app/(app)/audit/components/table/audit-log-table-client.tsx (2)
5-5: LGTM!Clean import of the new Empty component.
81-87: LGTM! Clean implementation of the Empty component.The error state implementation is clear, actionable, and follows the new component structure correctly.
apps/dashboard/app/(app)/settings/vercel/client.tsx (1)
27-27: LGTM!Clean import of the new Empty component.
|
|
…-overhaul-emptyplaceholder
What does this PR do?
Added an new component to use when no data is available. Includes Top icon, title, description, and action element.
Added the component to the @unkey/ui package and Engineering docs.
added the component to all places previous was. Everything worked well with no issues noticed.
Fixes # (issue)
eng-1606
Type of change
How should this be tested?
Required
pnpm buildpnpm fmtconsole.logsgit pull origin mainAppreciated
Light Mode
Dark Mode
Summary by CodeRabbit
Release Notes: UI Component Refactoring
New Features
Emptycomponent from@unkey/uilibrary to replaceEmptyPlaceholderUI Improvements
Technical Updates
EmptyPlaceholdercomponentEmptycomponentStyling Changes
This release focuses on improving the user interface's empty state representations with a more modular and consistent approach.