[WEB-4921] refactor: implement propel tabs everywhere #8235
Conversation
- Replaced instances of Headless UI's Tab component with a new custom Tabs component across various components, including analytics modals, image pickers, and navigation panes. - Updated tab handling logic to align with the new Tabs API, ensuring consistent behavior and styling. - Removed unused Tab imports and cleaned up related code for improved maintainability. - This refactor enhances the overall structure and consistency of tab navigation within the application.
|
Linked to Plane Work Item(s) This comment was auto-generated by Plane |
WalkthroughMigrated tab components across the codebase from Headless UI to a new custom Tabs component from Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ 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 |
![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 (1)
apps/web/core/components/analytics/work-items/modal/content.tsx (1)
3-3: Remove unusedTabimport and dependency on Headless UI tabs
Tabfrom"@headlessui/react"is no longer used in this component, and the PR’s goal is to eliminate tab usage from Headless UI. Clean up this import (and any related dependency if still present elsewhere) to avoid dead code and keep the migration consistent.Also applies to: 72-77
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
apps/web/core/components/analytics/work-items/modal/content.tsx(1 hunks)apps/web/core/components/core/image-picker-popover.tsx(4 hunks)apps/web/core/components/cycles/analytics-sidebar/progress-stats.tsx(3 hunks)apps/web/core/components/license/modal/card/base-paid-plan-card.tsx(3 hunks)apps/web/core/components/license/modal/card/plan-upgrade.tsx(1 hunks)apps/web/core/components/modules/analytics-sidebar/issue-progress.tsx(1 hunks)apps/web/core/components/modules/analytics-sidebar/progress-stats.tsx(3 hunks)apps/web/core/components/pages/navigation-pane/root.tsx(3 hunks)apps/web/core/components/pages/navigation-pane/tab-panels/root.tsx(2 hunks)apps/web/core/components/pages/navigation-pane/tabs-list.tsx(2 hunks)packages/ui/src/index.ts(0 hunks)packages/ui/src/tabs/index.ts(0 hunks)packages/ui/src/tabs/tab-list.tsx(0 hunks)packages/ui/src/tabs/tabs.tsx(0 hunks)
💤 Files with no reviewable changes (4)
- packages/ui/src/tabs/index.ts
- packages/ui/src/tabs/tab-list.tsx
- packages/ui/src/index.ts
- packages/ui/src/tabs/tabs.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,mts,cts}
📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)
**/*.{ts,tsx,mts,cts}: Useconsttype parameters for more precise literal inference in TypeScript 5.0+
Use thesatisfiesoperator to validate types without widening them
Leverage inferred type predicates to reduce the need for explicitisreturn types in filter/check functions
UseNoInfer<T>utility to block inference for specific type arguments when they should be determined by other arguments
Utilize narrowing inswitch(true)blocks for control flow analysis (TypeScript 5.3+)
Rely on narrowing from direct boolean comparisons for type guards
Trust preserved narrowing in closures when variables aren't modified after the check (TypeScript 5.4+)
Use constant indices to narrow object/array properties (TypeScript 5.5+)
Use standard ECMAScript decorators (Stage 3) instead of legacyexperimentalDecorators
Useusingdeclarations for explicit resource management with Disposable pattern instead of manual cleanup (TypeScript 5.2+)
Usewith { type: "json" }for import attributes; avoid deprecatedassertsyntax (TypeScript 5.3/5.8+)
Useimport typeexplicitly when importing types to ensure they are erased during compilation, respectingverbatimModuleSyntaxflag
Use.ts,.mts,.ctsextensions inimport typestatements (TypeScript 5.2+)
Useimport type { Type } from "mod" with { "resolution-mode": "import" }for specific module resolution contexts (TypeScript 5.3+)
Use new iterator methods (map, filter, etc.) if targeting modern environments (TypeScript 5.6+)
Utilize newSetmethods likeunion,intersection, etc., when available (TypeScript 5.5+)
UseObject.groupBy/Map.groupBystandard methods for grouping instead of external libraries (TypeScript 5.4+)
UsePromise.withResolvers()for creating promises with exposed resolve/reject functions (TypeScript 5.7+)
Use copying array methods (toSorted,toSpliced,with) for immutable array operations (TypeScript 5.2+)
Avoid accessing instance fields viasuperin classes (TypeScript 5....
Files:
apps/web/core/components/license/modal/card/plan-upgrade.tsxapps/web/core/components/analytics/work-items/modal/content.tsxapps/web/core/components/modules/analytics-sidebar/issue-progress.tsxapps/web/core/components/pages/navigation-pane/tab-panels/root.tsxapps/web/core/components/license/modal/card/base-paid-plan-card.tsxapps/web/core/components/cycles/analytics-sidebar/progress-stats.tsxapps/web/core/components/modules/analytics-sidebar/progress-stats.tsxapps/web/core/components/core/image-picker-popover.tsxapps/web/core/components/pages/navigation-pane/tabs-list.tsxapps/web/core/components/pages/navigation-pane/root.tsx
🧠 Learnings (6)
📓 Common learnings
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7989
File: apps/web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(detail)/[pageId]/page.tsx:45-46
Timestamp: 2025-10-21T17:22:05.204Z
Learning: In the makeplane/plane repository, the refactor from useParams() to params prop is specifically scoped to page.tsx and layout.tsx files in apps/web/app (Next.js App Router pattern). Other components (hooks, regular client components, utilities) should continue using the useParams() hook as that is the correct pattern for non-route components.
📚 Learning: 2025-10-21T17:22:05.204Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7989
File: apps/web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(detail)/[pageId]/page.tsx:45-46
Timestamp: 2025-10-21T17:22:05.204Z
Learning: In the makeplane/plane repository, the refactor from useParams() to params prop is specifically scoped to page.tsx and layout.tsx files in apps/web/app (Next.js App Router pattern). Other components (hooks, regular client components, utilities) should continue using the useParams() hook as that is the correct pattern for non-route components.
Applied to files:
apps/web/core/components/pages/navigation-pane/tab-panels/root.tsxapps/web/core/components/license/modal/card/base-paid-plan-card.tsxapps/web/core/components/cycles/analytics-sidebar/progress-stats.tsxapps/web/core/components/modules/analytics-sidebar/progress-stats.tsxapps/web/core/components/core/image-picker-popover.tsxapps/web/core/components/pages/navigation-pane/tabs-list.tsxapps/web/core/components/pages/navigation-pane/root.tsx
📚 Learning: 2025-10-09T20:42:31.843Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7922
File: apps/admin/app/(all)/(dashboard)/ai/form.tsx:19-19
Timestamp: 2025-10-09T20:42:31.843Z
Learning: In the makeplane/plane repository, React types are globally available through TypeScript configuration. Type annotations like React.FC, React.ReactNode, etc. can be used without explicitly importing the React namespace. The codebase uses the modern JSX transform, so React imports are not required for JSX or type references.
Applied to files:
apps/web/core/components/pages/navigation-pane/tab-panels/root.tsxapps/web/core/components/license/modal/card/base-paid-plan-card.tsxapps/web/core/components/cycles/analytics-sidebar/progress-stats.tsxapps/web/core/components/modules/analytics-sidebar/progress-stats.tsxapps/web/core/components/core/image-picker-popover.tsxapps/web/core/components/pages/navigation-pane/tabs-list.tsxapps/web/core/components/pages/navigation-pane/root.tsx
📚 Learning: 2025-10-01T15:30:17.605Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7888
File: packages/propel/src/avatar/avatar.stories.tsx:2-3
Timestamp: 2025-10-01T15:30:17.605Z
Learning: In the makeplane/plane repository, avoid suggesting inline type imports (e.g., `import { Avatar, type TAvatarSize }`) due to bundler compatibility issues. Keep type imports and value imports as separate statements.
Applied to files:
apps/web/core/components/pages/navigation-pane/tab-panels/root.tsxapps/web/core/components/cycles/analytics-sidebar/progress-stats.tsxapps/web/core/components/modules/analytics-sidebar/progress-stats.tsxapps/web/core/components/pages/navigation-pane/tabs-list.tsx
📚 Learning: 2025-10-09T22:12:26.424Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7922
File: apps/admin/app/(all)/(dashboard)/ai/form.tsx:19-19
Timestamp: 2025-10-09T22:12:26.424Z
Learning: When `types/react` is installed in a TypeScript project (which is standard for React + TypeScript codebases), React types (React.FC, React.ReactNode, React.ComponentProps, etc.) are globally available by design. These type annotations can and should be used without explicitly importing the React namespace. This is a TypeScript/DefinitelyTyped feature, not codebase-specific configuration.
Applied to files:
apps/web/core/components/pages/navigation-pane/tab-panels/root.tsxapps/web/core/components/cycles/analytics-sidebar/progress-stats.tsxapps/web/core/components/modules/analytics-sidebar/progress-stats.tsx
📚 Learning: 2025-05-14T13:16:23.323Z
Learnt from: vamsikrishnamathala
Repo: makeplane/plane PR: 7061
File: web/core/components/workspace-notifications/root.tsx:18-18
Timestamp: 2025-05-14T13:16:23.323Z
Learning: In the Plane project codebase, the path alias `@/plane-web` resolves to the `ce` directory, making imports like `@/plane-web/hooks/use-notification-preview` correctly resolve to files in `web/ce/hooks/`.
Applied to files:
apps/web/core/components/pages/navigation-pane/tab-panels/root.tsx
🧬 Code graph analysis (6)
apps/web/core/components/license/modal/card/plan-upgrade.tsx (1)
packages/ui/src/billing/subscription.ts (1)
getDiscountPillStyle(208-209)
apps/web/core/components/pages/navigation-pane/tab-panels/root.tsx (2)
packages/propel/src/tabs/tabs.tsx (1)
Tabs(107-112)apps/web/core/components/pages/navigation-pane/tab-panels/assets.tsx (1)
PageNavigationPaneAssetsTabPanel(104-120)
apps/web/core/components/license/modal/card/base-paid-plan-card.tsx (2)
packages/propel/src/tabs/tabs.tsx (1)
Tabs(107-112)packages/ui/src/billing/subscription.ts (1)
getSubscriptionBackgroundColor(35-73)
apps/web/core/components/cycles/analytics-sidebar/progress-stats.tsx (1)
packages/propel/src/tabs/tabs.tsx (1)
Tabs(107-112)
apps/web/core/components/modules/analytics-sidebar/progress-stats.tsx (1)
packages/propel/src/tabs/tabs.tsx (1)
Tabs(107-112)
apps/web/core/components/pages/navigation-pane/root.tsx (3)
packages/propel/src/tabs/tabs.tsx (1)
Tabs(107-112)apps/web/core/components/pages/navigation-pane/tabs-list.tsx (1)
PageNavigationPaneTabsList(7-27)apps/web/core/components/pages/navigation-pane/tab-panels/root.tsx (1)
PageNavigationPaneTabPanelsRoot(21-40)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Agent
- GitHub Check: CodeQL analysis (javascript-typescript)
- GitHub Check: Analyze (javascript)
- GitHub Check: Build and lint web apps
🔇 Additional comments (13)
apps/web/core/components/modules/analytics-sidebar/issue-progress.tsx (1)
117-117: CSS spacing adjustment aligns with refactoring goals.The removal of horizontal padding (
px-3) and retention of vertical padding (py-4) appears intentional as part of the broader analytics sidebar layout refinement. The component's disclosure structure and child rendering remain unchanged and functional.Verify that this CSS adjustment is consistent across other analytics-sidebar components (e.g.,
progress-stats.tsx) to ensure uniform spacing after the propel tabs migration.apps/web/core/components/analytics/work-items/modal/content.tsx (1)
72-77: Layout wrapper migration looks correctSwitching from the tab wrapper to a simple
<div>while preserving the children and props (peekView={!fullScreen},isEpic) keeps behavior and data flow intact. No issues from this change alone.apps/web/core/components/license/modal/card/plan-upgrade.tsx (1)
76-83: LGTM! Styling improvement enhances badge layout.The addition of
flex items-center justify-centerproperly centers the discount percentage within the badge, and the fixed heighth-5withleading-tightensures consistent dimensions across different discount values.apps/web/core/components/cycles/analytics-sidebar/progress-stats.tsx (1)
117-169: LGTM! Clean migration to the new Tabs API.The refactoring correctly migrates from Headless UI to
@plane/propel/tabs:
Tab.Group→Tabswith value-based controlTab.List→Tabs.ListTab→Tabs.TriggerwithvaluepropTab.Panel→Tabs.Contentwith correspondingvalueThe active state styling correctly uses
data-[selected]attributes, and localStorage integration is preserved.apps/web/core/components/modules/analytics-sidebar/progress-stats.tsx (1)
115-167: LGTM! Consistent migration pattern.The migration follows the same pattern as the cycles component, correctly adopting the new Tabs API with value-based control and
data-[selected]styling.apps/web/core/components/license/modal/card/base-paid-plan-card.tsx (1)
42-89: LGTM! Controlled tabs implementation is correct.The migration properly uses controlled state with
valueandonValueChange, correctly mappingprice.recurringas the tab value to link triggers with content panels.apps/web/core/components/core/image-picker-popover.tsx (3)
215-280: LGTM on Unsplash content structure.The Unsplash tab content correctly implements search functionality with Controller/Input integration and proper image grid rendering with loading states.
282-298: LGTM on Images tab content.The static images tab correctly renders the grid with click-to-select behavior.
300-367: LGTM on Upload tab content.The upload tab correctly preserves the drag-and-drop functionality, file validation, preview, and upload flow with proper error handling.
apps/web/core/components/pages/navigation-pane/tab-panels/root.tsx (1)
25-39: LGTM! Panel structure correctly migrated.The migration properly replaces
Tab.Panelswith a fragment wrapper and individualTabs.Contentblocks, correctly usingtab.keyas thevalueprop to link with the corresponding triggers.apps/web/core/components/pages/navigation-pane/tabs-list.tsx (1)
12-26: LGTM! Excellent simplification with Tabs.Indicator.The migration removes manual active state calculations and positioning logic in favor of
Tabs.Indicator, which handles the visual active state automatically. This is cleaner and more maintainable.apps/web/core/components/pages/navigation-pane/root.tsx (2)
69-80: LGTM! Clean migration to value-based tab control.The change from index-based to value-based tab handling is more semantic and maintainable. The type cast to
TPageNavigationPaneTabis appropriate for ensuring type safety with the query parameter.
107-110: LGTM! Root Tabs structure is correct.The migration correctly wraps the tabs list and panels in the new
Tabscomponent with propervalueandonValueChangehandlers.
| <Tabs defaultValue={tabOptions[0].key} className={"h-full overflow-hidden"}> | ||
| <Tabs.List className="p-1 w-fit"> | ||
| {tabOptions.map((tab) => ( | ||
| <Tab | ||
| <Tabs.Trigger | ||
| key={tab.key} | ||
| className={({ selected }) => | ||
| `rounded px-4 py-1 text-center text-sm outline-none transition-colors ${ | ||
| selected ? "bg-custom-primary text-white" : "text-custom-text-100" | ||
| }` | ||
| } | ||
| value={tab.key} | ||
| className="rounded px-4 py-1 text-center text-sm outline-none transition-colors data-[selected]:bg-custom-primary data-[selected]:text-white text-custom-text-100" | ||
| > | ||
| {tab.title} | ||
| </Tab> | ||
| </Tabs.Trigger> | ||
| ))} | ||
| </Tab.List> | ||
| <Tab.Panels className="vertical-scrollbar scrollbar-md h-full w-full flex-1 overflow-y-auto overflow-x-hidden"> | ||
| <Tab.Panel className="mt-4 h-full w-full space-y-4"> | ||
| {(unsplashImages || !unsplashError) && ( | ||
| <> | ||
| <div className="flex gap-x-2"> | ||
| <Controller | ||
| control={control} | ||
| name="search" | ||
| render={({ field: { value, ref } }) => ( | ||
| <Input | ||
| id="search" | ||
| name="search" | ||
| type="text" | ||
| onKeyDown={(e) => { | ||
| if (e.key === "Enter") { | ||
| e.preventDefault(); | ||
| setSearchParams(formData.search); | ||
| } | ||
| </Tabs.List> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n apps/web/core/components/core/image-picker-popover.tsx | head -100Repository: makeplane/plane
Length of output: 3873
🏁 Script executed:
sed -n '190,220p' apps/web/core/components/core/image-picker-popover.tsxRepository: makeplane/plane
Length of output: 1486
🏁 Script executed:
sed -n '220,280p' apps/web/core/components/core/image-picker-popover.tsxRepository: makeplane/plane
Length of output: 3067
🏁 Script executed:
sed -n '275,350p' apps/web/core/components/core/image-picker-popover.tsxRepository: makeplane/plane
Length of output: 3911
🏁 Script executed:
sed -n '360,380p' apps/web/core/components/core/image-picker-popover.tsxRepository: makeplane/plane
Length of output: 485
Filter disabled tabs and set default to first enabled tab.
The tabOptions array includes an isEnabled field (set on line 68 based on hasUnsplashConfigured), but this is not being used when rendering tabs. All tabs render regardless of enabled state (lines 203-212), and defaultValue always uses tabOptions[0].key ("unsplash" at line 201), which may be disabled.
If hasUnsplashConfigured is false, the Unsplash tab button displays even though it shouldn't be available, and the component attempts to set a disabled tab as the default.
Filter tabs by isEnabled and calculate the default value from the first enabled tab:
- <Tabs defaultValue={tabOptions[0].key} className={"h-full overflow-hidden"}>
+ <Tabs defaultValue={tabOptions.find(tab => tab.isEnabled)?.key ?? "images"} className={"h-full overflow-hidden"}>
<Tabs.List className="p-1 w-fit">
- {tabOptions.map((tab) => (
+ {tabOptions.filter(tab => tab.isEnabled).map((tab) => (🤖 Prompt for AI Agents
In apps/web/core/components/core/image-picker-popover.tsx around lines 201 to
212, the Tabs are rendered from tabOptions regardless of each tab's isEnabled
flag and defaultValue uses tabOptions[0].key which may be disabled; filter
tabOptions to only include tabs where isEnabled is true when computing the list
to render and when computing the default tab key (use the first enabled tab's
key), and handle the case where no tabs are enabled (e.g., fall back to
undefined or a safe default) so the Tabs component never tries to select a
disabled entry.
There was a problem hiding this comment.
Pull request overview
This PR migrates the codebase from using @headlessui/react Tab components to @plane/propel/tabs, and removes the old tabs implementation from the ui package. The migration involves updating all tab component usages to the new propel Tabs API which uses a value-based approach instead of index-based selection.
- Removed the old Tabs and TabList components from the
@plane/uipackage - Migrated all tab implementations to use the propel Tabs API with value-based selection
- Updated styling to use data attributes (
data-[selected]) instead of render props
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/ui/src/tabs/tabs.tsx | Deleted old Tabs component implementation |
| packages/ui/src/tabs/tab-list.tsx | Deleted old TabList component implementation |
| packages/ui/src/tabs/index.ts | Deleted tabs index file |
| packages/ui/src/index.ts | Removed tabs export from ui package |
| apps/web/core/components/pages/navigation-pane/tabs-list.tsx | Migrated to propel Tabs with Tabs.List, Tabs.Trigger, and Tabs.Indicator |
| apps/web/core/components/pages/navigation-pane/tab-panels/root.tsx | Replaced Tab.Panel with Tabs.Content |
| apps/web/core/components/pages/navigation-pane/root.tsx | Migrated Tab.Group to Tabs with value-based selection API |
| apps/web/core/components/modules/analytics-sidebar/progress-stats.tsx | Migrated to propel Tabs with data-[selected] styling pattern |
| apps/web/core/components/modules/analytics-sidebar/issue-progress.tsx | Removed horizontal padding for layout consistency |
| apps/web/core/components/license/modal/card/plan-upgrade.tsx | Updated discount badge styling with improved vertical alignment |
| apps/web/core/components/license/modal/card/base-paid-plan-card.tsx | Migrated pricing tabs to propel Tabs with simplified structure |
| apps/web/core/components/cycles/analytics-sidebar/progress-stats.tsx | Migrated to propel Tabs with value-based API and data attribute styling |
| apps/web/core/components/core/image-picker-popover.tsx | Migrated image picker tabs to propel Tabs with restructured content panels |
| apps/web/core/components/analytics/work-items/modal/content.tsx | Removed unnecessary Tab.Group wrapper |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| onClick={() => setSelectedPlan(price.recurring)} | ||
| value={price.recurring} | ||
| className={cn( | ||
| "w-full rounded text-sm font-medium leading-5 py-2", |
There was a problem hiding this comment.
Extra whitespace detected: There are two spaces between "rounded" and "text-sm". This should be a single space for consistency.
| "w-full rounded text-sm font-medium leading-5 py-2", | |
| "w-full rounded text-sm font-medium leading-5 py-2", |
| > | ||
| <Tab.Group> | ||
| <Tab.List as="span" className="inline-block rounded bg-custom-background-80 p-1"> | ||
| <Tabs defaultValue={tabOptions[0].key} className={"h-full overflow-hidden"}> |
There was a problem hiding this comment.
Unnecessary curly braces: The className value is a static string and doesn't need to be wrapped in curly braces. Use className="h-full overflow-hidden" instead of className={"h-full overflow-hidden"}.
| <Tabs defaultValue={tabOptions[0].key} className={"h-full overflow-hidden"}> | |
| <Tabs defaultValue={tabOptions[0].key} className="h-full overflow-hidden"> |
Description
This PR replaces all the instances of tabs with propel tabs. And removes tabs from the ui package.
Type of Change
Summary by CodeRabbit
Refactor
Style
✏️ Tip: You can customize this high-level summary in your review settings.