refactor: extract PanelCard from ChartCard for reusability#23360
refactor: extract PanelCard from ChartCard for reusability#23360eunjae-lee merged 8 commits intomainfrom
Conversation
- Create base PanelCard and PanelCardItem components in UI package - Refactor ChartCard to use PanelCard as foundation while keeping legend functionality - Add headerActions prop to PanelCard for extensibility - Maintain backward compatibility for existing ChartCard usage - Export PanelCard and PanelCardItem from UI card components Co-Authored-By: eunjae@cal.com <hey@eunjae.dev>
Co-Authored-By: eunjae@cal.com <hey@eunjae.dev>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
Walkthrough
Possibly related PRs
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. 📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ 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
Status, Documentation and Community
|
- Rename 'actions' prop to 'headerContent' in PanelCard component - Update ChartCard to use new 'headerContent' prop name - More accurate naming since content can be text or other elements, not just buttons Co-Authored-By: eunjae@cal.com <hey@eunjae.dev>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
eunjae-lee
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
packages/ui/components/card/PanelCard.tsx (4)
12-16: Export a shared CTA type to eliminate duplication across consumersPanelCard defines the CTA shape inline. Exporting a named CTA type improves reuse (e.g., ChartCard) and prevents drift.
Apply:
export function PanelCard({ @@ -}: { - title: string | ReactNode; - subtitle?: string; - cta?: { label: string; onClick: () => void }; - headerContent?: ReactNode; - children: ReactNode; -}) { +}: PanelCardProps) { return ( @@ ); } + +export type PanelCardCTA = { label: string; onClick: () => void }; +export type PanelCardProps = { + title: string | ReactNode; + subtitle?: string; + cta?: PanelCardCTA; + headerContent?: ReactNode; + children: ReactNode; +};
18-43: Expose styling hooks for broader reuseSince PanelCard is meant to be general-purpose, consider
className,headerClassName, andcontentClassNameprops to allow consumers to tweak spacing/borders without overriding via deep selectors.Apply:
-export function PanelCard({ - title, - subtitle, - cta, - headerContent, - children, -}: PanelCardProps) { +export function PanelCard({ + title, + subtitle, + cta, + headerContent, + children, + className, + headerClassName, + contentClassName, +}: PanelCardProps & { + className?: string; + headerClassName?: string; + contentClassName?: string; +}) { return ( - <div className="bg-muted group relative flex w-full flex-col items-center rounded-2xl px-1 pb-1"> - <div className="flex h-11 w-full shrink-0 items-center justify-between gap-2 px-4"> + <div className={classNames("bg-muted group relative flex w-full flex-col items-center rounded-2xl px-1 pb-1", className)}> + <div className={classNames("flex h-11 w-full shrink-0 items-center justify-between gap-2 px-4", headerClassName)}> @@ - <div className="bg-default border-muted w-full grow gap-3 rounded-xl border"> + <div className={classNames("bg-default border-muted w-full grow gap-3 rounded-xl border", contentClassName)}>Note: Add
import classNames from "@calcom/ui/classNames";at the top.
26-33: CTA flexibility: allow disabled/aria/tooltip or a custom nodeTo keep PanelCard broadly reusable, consider either (a) extending CTA with optional
disabled,ariaLabel, andicon, or (b) supporting actaNode?: ReactNodefor full control (links, menus, popovers).Example (non-breaking):
-export type PanelCardCTA = { label: string; onClick: () => void }; +export type PanelCardCTA = { + label: string; + onClick: () => void; + disabled?: boolean; + ariaLabel?: string; +}; @@ - {cta && ( - <Button className="shrink-0" color="secondary" onClick={cta.onClick}> + {cta && ( + <Button + className="shrink-0" + color="secondary" + onClick={cta.onClick} + aria-label={cta.ariaLabel ?? cta.label} + disabled={cta.disabled} + > {cta.label} </Button> )}
21-25: Heading semantics when title is a custom nodeWhen
titleis a ReactNode, heading semantics can be lost. Consider an optionalheadingLevelprop (e.g., 2|3|4) and render the children inside that tag for consistent a11y.packages/features/insights/components/ChartCard.tsx (2)
31-31: Minor: compute legend inline or memoize if legend is stableCurrent approach is fine. If Legend is heavy or legend arrays are stable, consider
useMemoto avoid re-renders on unrelated state changes. Otherwise, inlineheaderContent={legend?.length ? <Legend ... /> : null}is sufficient.
34-36: Prop type reuse from PanelCard for ctaTo avoid duplicating CTA shape, consider importing a shared type from PanelCard (e.g.,
PanelCardCTA) and usingcta?: PanelCardCTAhere. Keeps API drift-free.Example:
-import { PanelCard } from "@calcom/ui/components/card"; +import { PanelCard, type PanelCardCTA } from "@calcom/ui/components/card"; @@ - cta?: { label: string; onClick: () => void }; + cta?: PanelCardCTA;
📜 Review details
Configuration used: Path: .coderabbit.yaml
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 (3)
packages/features/insights/components/ChartCard.tsx(2 hunks)packages/ui/components/card/PanelCard.tsx(1 hunks)packages/ui/components/card/index.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/ui/components/card/index.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
packages/ui/components/card/index.tspackages/ui/components/card/PanelCard.tsxpackages/features/insights/components/ChartCard.tsx
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/ui/components/card/index.tspackages/ui/components/card/PanelCard.tsxpackages/features/insights/components/ChartCard.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Always use
t()for text localization in frontend code; direct text embedding should trigger a warning
Files:
packages/ui/components/card/PanelCard.tsxpackages/features/insights/components/ChartCard.tsx
🧬 Code graph analysis (2)
packages/ui/components/card/PanelCard.tsx (1)
packages/ui/components/card/index.ts (1)
PanelCard(5-5)
packages/features/insights/components/ChartCard.tsx (3)
packages/features/insights/components/booking/EventTrendsChart.tsx (1)
legend(23-30)packages/features/insights/components/routing/RoutingFunnelContent.tsx (1)
legend(25-29)packages/ui/components/card/PanelCard.tsx (1)
PanelCard(5-45)
🔇 Additional comments (6)
packages/ui/components/card/index.ts (1)
5-5: Good addition: named export aligns with our guidelinesRe-exporting PanelCard as a named export via the barrel is correct and consistent with our preference for named exports. No further action needed.
packages/ui/components/card/PanelCard.tsx (1)
36-40: i18n: Verify PanelCard callers localizesubtitleandcta.label
PanelCard should remain presentation-only. Confirm every consumer of<PanelCard>passes already-localized strings (e.g.t("…")) for both props:• For each
<PanelCard subtitle={…} />, ensure the value is a translation call (such ast("…")), not a raw string literal.
• For each<PanelCard cta={{ label: … }} />, ensurelabelis supplied viat("…"), not a hard-coded string.To audit all usages, you can run something like:
rg -n "PanelCard" -g '*.tsx'and inspect the
subtitle=andcta={{ label: }}props.
If you locate any raw literals, update them to use your i18n function.packages/features/insights/components/ChartCard.tsx (4)
6-6: Good pivot to reusable layout via PanelCardReplacing bespoke header markup with PanelCard improves consistency and reuse. Import via the barrel keeps the public API clean.
24-24: Type cleanup: ReactNode alias is preferredSwitching to
ReactNode(imported as a type) avoids namespace references and matches our TS style.
29-29: Children’s type narrowed to ReactNodeThis aligns with React’s standard typings and is appropriate here.
34-36: No brittle selectors found for ChartCard headerI’ve searched the codebase for any references to the old ChartCard header structure (e.g. direct
ChartCardusages in tests or CSS/SCSS selectors targeting the previoush2,flex h-11, oroverflow-x-autoclasses) and found none. There are no remaining selector or test references that would break due to this header DOM change. Feel free to merge.
E2E results are ready! |
3 participants
What does this PR do?
This PR extracts a general-purpose
PanelCardcomponent from the existingChartCardcomponent to enable reusability across the application. The refactoring maintains backward compatibility while creating a flexible base component that can be used in contexts beyond charts.Motivation: The
ChartCardcomponent contains useful general-purpose card functionality (title, subtitle, CTA button, collapsible potential) that could benefit other parts of the application.Visual Demo (For contributors especially)
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
Visit /insights and all the charts look the same as before.