refactor(platform): simplify ui components#476
Conversation
Greptile SummaryThis PR consolidates 10+ UI component libraries from multi-export "primitive wrapper" patterns (e.g.,
Confidence Score: 4/5
|
| Filename | Overview |
|---|---|
| services/platform/app/components/ui/overlays/tooltip.tsx | Simplified from multi-export primitives to a single Tooltip component with content/children props. Each tooltip now creates its own Provider (acceptable for small counts). Graceful null-content fallback added. |
| services/platform/app/components/ui/overlays/dropdown-menu.tsx | Major rewrite from ~15 exported primitives to a single config-driven DropdownMenu component with typed item variants (item, label, sub, radio-group, custom). Well-structured discriminated union types. |
| services/platform/app/components/ui/overlays/popover.tsx | Simplified from multi-export primitives to a single Popover component with trigger/children pattern. Adds onOpenAutoFocus prop. Clean refactor. |
| services/platform/app/components/ui/overlays/sheet.tsx | Simplified from 7+ exported primitives to a single Sheet component. Title/description are now sr-only by default for accessibility. Clean refactor. |
| services/platform/app/components/ui/feedback/toast.tsx | Reduced from full component file to pure type definitions (ToastVariant, ToastProps, ToastActionElement). All rendering logic moved to toaster.tsx. |
| services/platform/app/components/ui/feedback/toaster.tsx | Now contains all toast rendering logic (previously split across toast.tsx and toaster.tsx). Inlines primitives directly with proper variant icon rendering. z-index uses Tailwind v4 z-100. |
| services/platform/app/components/ui/feedback/alert.tsx | Refactored from 3 composable pieces (Alert, AlertTitle, AlertDescription) to a single Alert component with icon, title, description props. Cleaner API, retains accessibility attributes. |
| services/platform/app/components/ui/layout/card.tsx | Refactored from 6 composable pieces to a single Card with title, description, footer, and section className props. Conditional header/content padding logic is correct. |
| services/platform/app/components/ui/navigation/tabs.tsx | Refactored from 4 composable pieces to a single Tabs component with items array. Supports optional content per tab. |
| services/platform/app/components/user-button.tsx | Major refactor: inlined TeamFilterMenu into useMemo-based menu items config, converted to data-driven DropdownMenu API. Added useCallback for sign-out. Clean migration. |
| services/platform/app/components/ui/data-table/data-table-filters.tsx | Migrated to simplified Popover/RadioGroup APIs. Missing contentClassName="space-y-0" that was present in the old PopoverContent — potential visual regression. |
| services/platform/app/features/conversations/components/conversation-header.tsx | Migrated to config-driven DropdownMenu with useMemo. Custom contentClassName and per-item className from the old code are not carried over — visual regression. |
| services/platform/app/features/automations/components/automation-navigation.tsx | Complex migration to config-driven DropdownMenu with conditional item arrays using spread operators and satisfies. Correct but verbose. |
Flowchart
flowchart TD
subgraph Before["Before: Multi-export primitives"]
T1["Tooltip"] --> TP["TooltipProvider"]
T1 --> TT["TooltipTrigger"]
T1 --> TC["TooltipContent"]
D1["DropdownMenu"] --> DT["DropdownMenuTrigger"]
D1 --> DC["DropdownMenuContent"]
D1 --> DI["DropdownMenuItem"]
D1 --> DS["DropdownMenuSeparator"]
D1 --> DL["DropdownMenuLabel"]
D1 --> DSub["+ 9 more exports"]
C1["Card"] --> CH["CardHeader"]
C1 --> CT["CardTitle"]
C1 --> CD["CardDescription"]
C1 --> CC["CardContent"]
C1 --> CF["CardFooter"]
end
subgraph After["After: Single config-driven components"]
T2["Tooltip\n(content, children, side, ...)"]
D2["DropdownMenu\n(trigger, items: DropdownMenuGroup[], ...)"]
C2["Card\n(title, description, children, footer, ...)"]
end
Before -->|"Refactored to"| After
subgraph Consumers["40+ Consumer Files Updated"]
F1["chat/components/*"]
F2["conversations/components/*"]
F3["automations/components/*"]
F4["settings/integrations/*"]
F5["data-table/*"]
end
After --> Consumers
Last reviewed commit: f1c55f6
| <DropdownMenu | ||
| trigger={ | ||
| <Button | ||
| variant="ghost" | ||
| size="icon" | ||
| aria-label={tCommon('aria.actionsMenu')} | ||
| > | ||
| <MoreVertical className="text-muted-foreground size-5" /> | ||
| </Button> | ||
| </DropdownMenuTrigger> | ||
| <DropdownMenuContent | ||
| align="end" | ||
| className="border-border w-[14rem] rounded-xl border p-2 shadow-[0px_1px_2px_0px_rgba(0,0,0,0.05)]" | ||
| > | ||
| <DropdownMenuItem | ||
| onClick={() => setIsCustomerInfoOpen(true)} | ||
| disabled={isLoading} | ||
| className="hover:bg-muted flex cursor-pointer items-center gap-2 rounded-lg p-2" | ||
| > | ||
| <UserIcon className="text-muted-foreground size-5" /> | ||
| <span className="text-muted-foreground text-sm font-medium"> | ||
| {t('header.customerInfo')} | ||
| </span> | ||
| </DropdownMenuItem> | ||
| {conversation.status === 'open' && ( | ||
| <DropdownMenuItem | ||
| onClick={handleResolveConversation} | ||
| disabled={isLoading} | ||
| className="hover:bg-muted flex cursor-pointer items-center gap-2 rounded-lg p-2" | ||
| > | ||
| <MessageSquareOff className="text-muted-foreground size-5" /> | ||
| <span className="text-muted-foreground text-sm font-medium"> | ||
| {isClosing | ||
| ? t('header.closing') | ||
| : t('header.closeConversation')} | ||
| </span> | ||
| </DropdownMenuItem> | ||
| )} | ||
| {conversation.status !== 'open' && ( | ||
| <DropdownMenuItem | ||
| onClick={handleReopenConversation} | ||
| disabled={isLoading} | ||
| className="hover:bg-muted flex cursor-pointer items-center gap-2 rounded-lg p-2" | ||
| > | ||
| <MessageSquare className="text-muted-foreground size-5" /> | ||
| <span className="text-muted-foreground text-sm font-medium"> | ||
| {isReopening | ||
| ? t('header.reopening') | ||
| : t('header.reopenConversation')} | ||
| </span> | ||
| </DropdownMenuItem> | ||
| )} | ||
| {conversation.status === 'open' && ( | ||
| <DropdownMenuItem | ||
| onClick={handleMarkAsSpam} | ||
| disabled={isLoading} | ||
| className="hover:bg-muted flex cursor-pointer items-center gap-2 rounded-lg p-2" | ||
| > | ||
| <ShieldX className="text-muted-foreground size-5" /> | ||
| <span className="text-muted-foreground text-sm font-medium"> | ||
| {isMarkingSpam | ||
| ? t('header.markingAsSpam') | ||
| : t('header.markAsSpam')} | ||
| </span> | ||
| </DropdownMenuItem> | ||
| )} | ||
| </DropdownMenuContent> | ||
| </DropdownMenu> | ||
| } | ||
| items={[menuItems]} | ||
| align="end" | ||
| /> |
There was a problem hiding this comment.
Custom dropdown styling dropped
The previous DropdownMenuContent had custom styling: className="border-border w-[14rem] rounded-xl border p-2 shadow-[0px_1px_2px_0px_rgba(0,0,0,0.05)]". Each DropdownMenuItem also had className="hover:bg-muted flex cursor-pointer items-center gap-2 rounded-lg p-2".
These styles are not carried over to the new config-driven API. This will result in a visual regression for this dropdown (it'll use the generic dropdown appearance instead of the narrower, custom-styled one). Consider passing contentClassName and per-item className:
| <DropdownMenu | |
| trigger={ | |
| <Button | |
| variant="ghost" | |
| size="icon" | |
| aria-label={tCommon('aria.actionsMenu')} | |
| > | |
| <MoreVertical className="text-muted-foreground size-5" /> | |
| </Button> | |
| </DropdownMenuTrigger> | |
| <DropdownMenuContent | |
| align="end" | |
| className="border-border w-[14rem] rounded-xl border p-2 shadow-[0px_1px_2px_0px_rgba(0,0,0,0.05)]" | |
| > | |
| <DropdownMenuItem | |
| onClick={() => setIsCustomerInfoOpen(true)} | |
| disabled={isLoading} | |
| className="hover:bg-muted flex cursor-pointer items-center gap-2 rounded-lg p-2" | |
| > | |
| <UserIcon className="text-muted-foreground size-5" /> | |
| <span className="text-muted-foreground text-sm font-medium"> | |
| {t('header.customerInfo')} | |
| </span> | |
| </DropdownMenuItem> | |
| {conversation.status === 'open' && ( | |
| <DropdownMenuItem | |
| onClick={handleResolveConversation} | |
| disabled={isLoading} | |
| className="hover:bg-muted flex cursor-pointer items-center gap-2 rounded-lg p-2" | |
| > | |
| <MessageSquareOff className="text-muted-foreground size-5" /> | |
| <span className="text-muted-foreground text-sm font-medium"> | |
| {isClosing | |
| ? t('header.closing') | |
| : t('header.closeConversation')} | |
| </span> | |
| </DropdownMenuItem> | |
| )} | |
| {conversation.status !== 'open' && ( | |
| <DropdownMenuItem | |
| onClick={handleReopenConversation} | |
| disabled={isLoading} | |
| className="hover:bg-muted flex cursor-pointer items-center gap-2 rounded-lg p-2" | |
| > | |
| <MessageSquare className="text-muted-foreground size-5" /> | |
| <span className="text-muted-foreground text-sm font-medium"> | |
| {isReopening | |
| ? t('header.reopening') | |
| : t('header.reopenConversation')} | |
| </span> | |
| </DropdownMenuItem> | |
| )} | |
| {conversation.status === 'open' && ( | |
| <DropdownMenuItem | |
| onClick={handleMarkAsSpam} | |
| disabled={isLoading} | |
| className="hover:bg-muted flex cursor-pointer items-center gap-2 rounded-lg p-2" | |
| > | |
| <ShieldX className="text-muted-foreground size-5" /> | |
| <span className="text-muted-foreground text-sm font-medium"> | |
| {isMarkingSpam | |
| ? t('header.markingAsSpam') | |
| : t('header.markAsSpam')} | |
| </span> | |
| </DropdownMenuItem> | |
| )} | |
| </DropdownMenuContent> | |
| </DropdownMenu> | |
| } | |
| items={[menuItems]} | |
| align="end" | |
| /> | |
| <DropdownMenu | |
| trigger={ | |
| <Button | |
| variant="ghost" | |
| size="icon" | |
| aria-label={tCommon('aria.actionsMenu')} | |
| > | |
| <MoreVertical className="text-muted-foreground size-5" /> | |
| </Button> | |
| } | |
| items={[menuItems]} | |
| align="end" | |
| contentClassName="border-border w-[14rem] rounded-xl border p-2 shadow-[0px_1px_2px_0px_rgba(0,0,0,0.05)]" | |
| /> |
Additional Comments (1)
The previous |
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughThis pull request systematically refactors multiple UI component APIs across the platform from nested component composition patterns to simplified prop-based interfaces. Key changes include migrating Tooltip, Sheet, Card, Tabs, DropdownMenu, Popover, Alert, and RadioGroup from multi-part nested structures to single components accepting data and content via props. Additionally, stories and all consuming components are updated to use the new APIs. A new 'switch' skeleton type is introduced for data tables, and Toast component handling is reorganized. The refactoring applies consistently across core UI components, features, and routes. Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes ✨ 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 |
Summary by CodeRabbit
Release Notes
Refactor
Documentation