[WEB-5614] refactor: update styling and structure across various components#8388
[WEB-5614] refactor: update styling and structure across various components#8388
Conversation
…consistency and improved UI
JayashTripathy
changed the title
|
Linked to Plane Work Item(s) This comment was auto-generated by Plane |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThis PR applies multiple presentational/styling updates across web app UI and Propel components, adds an Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ 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 |
There was a problem hiding this comment.
Pull request overview
This PR refactors styling and structure across various UI components, focusing on visual consistency and component architecture improvements. The changes update CSS utility classes, component styling patterns, and introduce new props for enhanced customization.
- Refactored tabs component to introduce a variant system with context provider (though incomplete implementation)
- Updated chart components to use consistent border styling across CartesianGrid
- Enhanced IconButton component with iconClassName prop for better icon customization
- Modernized UI styling patterns across sidebar, layout switcher, and analytics components
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/propel/src/tabs/tabs.tsx | Introduces TabsContext and variant system, updates styling with new border radius and layer backgrounds |
| packages/propel/src/icon-button/icon-button.tsx | Adds iconClassName prop for icon customization |
| packages/propel/src/icon-button/helper.tsx | Adds iconClassName type definition to IconButtonProps |
| packages/propel/src/charts/tree-map/root.tsx | Changes stroke from "currentColor" to "transparent" for cleaner appearance |
| packages/propel/src/charts/scatter-chart/root.tsx | Updates CartesianGrid stroke from alpha-modified variable to direct CSS variable |
| packages/propel/src/charts/line-chart/root.tsx | Updates CartesianGrid stroke styling for consistency |
| packages/propel/src/charts/bar-chart/root.tsx | Updates CartesianGrid stroke styling for consistency |
| packages/propel/src/charts/area-chart/root.tsx | Updates CartesianGrid stroke styling for consistency |
| apps/web/core/components/workspace/sidebar/projects-list.tsx | Refactors buttons to use IconButton component with improved hover states |
| apps/web/core/components/workspace/sidebar/projects-list-item.tsx | Moves transition-transform to iconClassName prop for cleaner separation |
| apps/web/core/components/base-layouts/layout-switcher.tsx | Updates styling patterns and switches from label to i18n_title for translations |
| apps/web/core/components/base-layouts/kanban/group.tsx | Changes background colors from surface-2 to layer-1 for consistency |
| apps/web/core/components/analytics/select/project.tsx | Refactors to use button styling utilities with customButton pattern |
| apps/web/app/(all)/[workspaceSlug]/(projects)/analytics/[tabId]/page.tsx | Updates tabs styling and removes explicit background prop |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| type TabsContextType = { | ||
| variant?: TabsVariant; | ||
| }; | ||
|
|
||
| const TabsContext = React.createContext<TabsContextType | undefined>(undefined); |
There was a problem hiding this comment.
The TabsContext is created and provided but never consumed. The variant prop is accepted by TabsTrigger but not used in the component implementation. Either the context should be consumed to use the variant from the root component, or it should be removed if not needed.
| { | ||
| className, | ||
| size = "md", | ||
| ...props | ||
| }: React.ComponentProps<typeof TabsPrimitive.Tab> & { size?: "sm" | "md" | "lg"; variant?: TabsVariant }, |
There was a problem hiding this comment.
The variant prop is accepted in the TabsTrigger type signature but is never destructured or used in the component implementation. Either implement the variant functionality or remove it from the type signature.
| "data-[selected]:bg-layer-transparent-active data-[selected]:text-primary data-[selected]:shadow-sm", | ||
| "text-placeholder hover:text-tertiary hover:bg-layer-transparent-hover", | ||
| "flex items-center justify-center p-1 min-w-fit w-full font-medium text-primary outline-none focus:outline-none cursor-pointer transition-all duration-200 ease-in-out rounded-md border border-transparent", | ||
| " data-[selected]:text-primary data-[selected]:shadow-sm data-[selected]:bg-layer-2 data-[selected]:border data-[selected]:border-subtle-1 data-[selected]:raised-200", |
There was a problem hiding this comment.
There are extra spaces at the beginning of this line. Remove the leading space for consistency.
| " data-[selected]:text-primary data-[selected]:shadow-sm data-[selected]:bg-layer-2 data-[selected]:border data-[selected]:border-subtle-1 data-[selected]:raised-200", | |
| "data-[selected]:text-primary data-[selected]:shadow-sm data-[selected]:bg-layer-2 data-[selected]:border data-[selected]:border-subtle-1 data-[selected]:raised-200", |
| "text-placeholder hover:text-tertiary hover:bg-layer-transparent-hover", | ||
| "flex items-center justify-center p-1 min-w-fit w-full font-medium text-primary outline-none focus:outline-none cursor-pointer transition-all duration-200 ease-in-out rounded-md border border-transparent", | ||
| " data-[selected]:text-primary data-[selected]:shadow-sm data-[selected]:bg-layer-2 data-[selected]:border data-[selected]:border-subtle-1 data-[selected]:raised-200", | ||
| "text-placeholder hover:text-tertiary hover:bg-layer-transparent-hover", |
There was a problem hiding this comment.
There are extra spaces in this class string. Remove the double space after 'text-placeholder' for consistency.
| "text-placeholder hover:text-tertiary hover:bg-layer-transparent-hover", | |
| "text-placeholder hover:text-tertiary hover:bg-layer-transparent-hover", |
| const Icon = layout.icon; | ||
| return ( | ||
| <Tooltip key={layout.key} tooltipContent={layout.label} isMobile={isMobile}> | ||
| <Tooltip key={layout.key} tooltipContent={t(layout.i18n_title)} isMobile={isMobile}> |
There was a problem hiding this comment.
The code is accessing layout.i18n_title but this property doesn't exist in the IBaseLayoutConfig interface, which only has key, icon, and label properties. This will cause a runtime error. Use layout.label instead or add the i18n_title property to the BASE_LAYOUTS configuration.
| <Tooltip key={layout.key} tooltipContent={t(layout.i18n_title)} isMobile={isMobile}> | |
| <Tooltip key={layout.key} tooltipContent={t(layout.label)} isMobile={isMobile}> |
| <IconButton | ||
| variant="ghost" | ||
| size="sm" | ||
| icon={ChevronRightIcon} | ||
| onClick={() => toggleListDisclosure(!isAllProjectsListOpen)} | ||
| className="text-placeholder" | ||
| iconClassName={cn("transition-transform", { | ||
| "rotate-90": isAllProjectsListOpen, | ||
| })} | ||
| aria-label={t( | ||
| isAllProjectsListOpen | ||
| ? "aria_labels.projects_sidebar.close_projects_menu" | ||
| : "aria_labels.projects_sidebar.open_projects_menu" | ||
| )} | ||
| > | ||
| <ChevronRightIcon | ||
| className={cn("flex-shrink-0 size-3 transition-all", { | ||
| "rotate-90": isAllProjectsListOpen, | ||
| })} | ||
| /> | ||
| </Disclosure.Button> | ||
| /> |
There was a problem hiding this comment.
The IconButton component is not wrapped with Disclosure.Button. The previous implementation had this button as a Disclosure.Button which is necessary for the Headless UI Disclosure component to function properly. The button needs to be a Disclosure.Button to trigger the disclosure state.
| )} | ||
| > | ||
| <Tabs.List background="layer-2" className={"my-2 overflow-x-auto flex w-fit"}> | ||
| <Tabs.List className={"overflow-x-auto flex w-fit h-7"}> |
There was a problem hiding this comment.
The background prop was removed from Tabs.List, but based on the tabs.tsx implementation, the default background is now 'contained' which applies 'bg-layer-3'. Since this was previously 'layer-2', this change will alter the visual appearance. Verify this is intentional or explicitly set background="contained" for clarity.
| <Tabs.List className={"overflow-x-auto flex w-fit h-7"}> | |
| <Tabs.List background="contained" className={"overflow-x-auto flex w-fit h-7"}> |
![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)
packages/propel/src/icon-button/icon-button.tsx (1)
31-38: Improved class composition with customization support.The refactoring to use
cn()with object-based conditional classes is cleaner and more maintainable. TheiconClassNameis properly merged after the size-based classes, allowing external overrides while maintaining sensible defaults.Optional: Verify icon size mapping for "lg"
Both
size === "base"andsize === "lg"map tosize-4. If "lg" should render a larger icon than "base", consider updating line 34:- "size-4": size === "base" || size === "lg", + "size-4": size === "base", + "size-4.5": size === "lg",If the current behavior is intentional (where "lg" affects button padding but not icon size), then no change is needed.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
apps/web/app/(all)/[workspaceSlug]/(projects)/analytics/[tabId]/page.tsx(1 hunks)apps/web/core/components/analytics/select/project.tsx(3 hunks)apps/web/core/components/base-layouts/kanban/group.tsx(1 hunks)apps/web/core/components/base-layouts/layout-switcher.tsx(3 hunks)apps/web/core/components/workspace/sidebar/projects-list-item.tsx(1 hunks)apps/web/core/components/workspace/sidebar/projects-list.tsx(2 hunks)packages/propel/src/charts/area-chart/root.tsx(1 hunks)packages/propel/src/charts/bar-chart/root.tsx(1 hunks)packages/propel/src/charts/line-chart/root.tsx(1 hunks)packages/propel/src/charts/scatter-chart/root.tsx(1 hunks)packages/propel/src/charts/tree-map/root.tsx(1 hunks)packages/propel/src/icon-button/helper.tsx(1 hunks)packages/propel/src/icon-button/icon-button.tsx(2 hunks)packages/propel/src/tabs/tabs.tsx(3 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{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:
packages/propel/src/charts/line-chart/root.tsxpackages/propel/src/icon-button/helper.tsxpackages/propel/src/charts/scatter-chart/root.tsxpackages/propel/src/icon-button/icon-button.tsxapps/web/core/components/workspace/sidebar/projects-list.tsxpackages/propel/src/charts/area-chart/root.tsxapps/web/core/components/analytics/select/project.tsxapps/web/core/components/base-layouts/kanban/group.tsxapps/web/app/(all)/[workspaceSlug]/(projects)/analytics/[tabId]/page.tsxapps/web/core/components/base-layouts/layout-switcher.tsxpackages/propel/src/charts/bar-chart/root.tsxpackages/propel/src/tabs/tabs.tsxapps/web/core/components/workspace/sidebar/projects-list-item.tsxpackages/propel/src/charts/tree-map/root.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Enable TypeScript strict mode and ensure all files are fully typed
Files:
packages/propel/src/charts/line-chart/root.tsxpackages/propel/src/icon-button/helper.tsxpackages/propel/src/charts/scatter-chart/root.tsxpackages/propel/src/icon-button/icon-button.tsxapps/web/core/components/workspace/sidebar/projects-list.tsxpackages/propel/src/charts/area-chart/root.tsxapps/web/core/components/analytics/select/project.tsxapps/web/core/components/base-layouts/kanban/group.tsxapps/web/app/(all)/[workspaceSlug]/(projects)/analytics/[tabId]/page.tsxapps/web/core/components/base-layouts/layout-switcher.tsxpackages/propel/src/charts/bar-chart/root.tsxpackages/propel/src/tabs/tabs.tsxapps/web/core/components/workspace/sidebar/projects-list-item.tsxpackages/propel/src/charts/tree-map/root.tsx
**/*.{js,jsx,ts,tsx,json,css}
📄 CodeRabbit inference engine (AGENTS.md)
Use Prettier with Tailwind plugin for code formatting, run
pnpm fix:format
Files:
packages/propel/src/charts/line-chart/root.tsxpackages/propel/src/icon-button/helper.tsxpackages/propel/src/charts/scatter-chart/root.tsxpackages/propel/src/icon-button/icon-button.tsxapps/web/core/components/workspace/sidebar/projects-list.tsxpackages/propel/src/charts/area-chart/root.tsxapps/web/core/components/analytics/select/project.tsxapps/web/core/components/base-layouts/kanban/group.tsxapps/web/app/(all)/[workspaceSlug]/(projects)/analytics/[tabId]/page.tsxapps/web/core/components/base-layouts/layout-switcher.tsxpackages/propel/src/charts/bar-chart/root.tsxpackages/propel/src/tabs/tabs.tsxapps/web/core/components/workspace/sidebar/projects-list-item.tsxpackages/propel/src/charts/tree-map/root.tsx
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,jsx,ts,tsx}: Use ESLint with shared config across packages, adhering to max warnings limits per package
Use camelCase for variable and function names, PascalCase for components and types
Use try-catch with proper error types and log errors appropriately
Files:
packages/propel/src/charts/line-chart/root.tsxpackages/propel/src/icon-button/helper.tsxpackages/propel/src/charts/scatter-chart/root.tsxpackages/propel/src/icon-button/icon-button.tsxapps/web/core/components/workspace/sidebar/projects-list.tsxpackages/propel/src/charts/area-chart/root.tsxapps/web/core/components/analytics/select/project.tsxapps/web/core/components/base-layouts/kanban/group.tsxapps/web/app/(all)/[workspaceSlug]/(projects)/analytics/[tabId]/page.tsxapps/web/core/components/base-layouts/layout-switcher.tsxpackages/propel/src/charts/bar-chart/root.tsxpackages/propel/src/tabs/tabs.tsxapps/web/core/components/workspace/sidebar/projects-list-item.tsxpackages/propel/src/charts/tree-map/root.tsx
🧠 Learnings (20)
📚 Learning: 2025-12-17T10:58:59.591Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: packages/tailwind-config/AGENTS.md:0-0
Timestamp: 2025-12-17T10:58:59.591Z
Learning: Applies to packages/tailwind-config/**/*.{ts,tsx} : Use semantic border colors: `border-subtle` for subtle borders and dividers, `border-subtle-1` for slightly more visible borders, `border-strong` for strong emphasis borders, and `border-strong-1` for very strong borders
Applied to files:
packages/propel/src/charts/line-chart/root.tsxpackages/propel/src/charts/scatter-chart/root.tsxpackages/propel/src/charts/area-chart/root.tsxpackages/propel/src/tabs/tabs.tsx
📚 Learning: 2025-12-17T10:58:59.591Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: packages/tailwind-config/AGENTS.md:0-0
Timestamp: 2025-12-17T10:58:59.591Z
Learning: Applies to packages/tailwind-config/**/*.{ts,tsx} : Sidebar menu items should use transparent backgrounds with hover states like `hover:bg-layer-1-hover` rather than base layer backgrounds, maintaining visual lightness while providing hover feedback
Applied to files:
packages/propel/src/charts/line-chart/root.tsxpackages/propel/src/charts/scatter-chart/root.tsxapps/web/core/components/workspace/sidebar/projects-list.tsxapps/web/core/components/base-layouts/kanban/group.tsxapps/web/app/(all)/[workspaceSlug]/(projects)/analytics/[tabId]/page.tsxpackages/propel/src/tabs/tabs.tsxpackages/propel/src/charts/tree-map/root.tsx
📚 Learning: 2025-12-17T10:58:59.591Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: packages/tailwind-config/AGENTS.md:0-0
Timestamp: 2025-12-17T10:58:59.591Z
Learning: Applies to packages/tailwind-config/**/*.{ts,tsx} : Use `-selected` state variants (e.g., `bg-layer-1-selected`) only when there is actual selection logic implemented, or use data attributes like `data-[selected]:bg-layer-1-selected`
Applied to files:
apps/web/core/components/analytics/select/project.tsxapps/web/core/components/base-layouts/kanban/group.tsxapps/web/core/components/base-layouts/layout-switcher.tsxpackages/propel/src/tabs/tabs.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/analytics/select/project.tsxapps/web/core/components/base-layouts/layout-switcher.tsxpackages/propel/src/tabs/tabs.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/analytics/select/project.tsx
📚 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/analytics/select/project.tsxapps/web/core/components/base-layouts/layout-switcher.tsx
📚 Learning: 2025-12-12T15:20:36.542Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-12T15:20:36.542Z
Learning: Applies to packages/plane/ui/**/*.{ts,tsx} : Build components in `plane/ui` package with Storybook for isolated development
Applied to files:
apps/web/core/components/analytics/select/project.tsxapps/web/core/components/base-layouts/layout-switcher.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/analytics/select/project.tsxapps/web/core/components/base-layouts/layout-switcher.tsx
📚 Learning: 2025-12-17T10:58:59.591Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: packages/tailwind-config/AGENTS.md:0-0
Timestamp: 2025-12-17T10:58:59.591Z
Learning: Applies to packages/tailwind-config/**/*.{ts,tsx} : Use `bg-surface-1`, `bg-surface-2`, or `bg-surface-3` for top-level containers that sit directly on the canvas, such as main content areas, page sections, or primary containers
Applied to files:
apps/web/core/components/base-layouts/kanban/group.tsxapps/web/app/(all)/[workspaceSlug]/(projects)/analytics/[tabId]/page.tsxpackages/propel/src/tabs/tabs.tsx
📚 Learning: 2025-12-17T10:58:59.591Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: packages/tailwind-config/AGENTS.md:0-0
Timestamp: 2025-12-17T10:58:59.591Z
Learning: Applies to packages/tailwind-config/**/*.{ts,tsx} : Never use `bg-layer-2` or higher for content boxes and cards - always use the matching layer number; the exception for going one level above applies only to interactive form elements
Applied to files:
apps/web/core/components/base-layouts/kanban/group.tsxpackages/propel/src/tabs/tabs.tsx
📚 Learning: 2025-12-17T10:58:59.591Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: packages/tailwind-config/AGENTS.md:0-0
Timestamp: 2025-12-17T10:58:59.591Z
Learning: Applies to packages/tailwind-config/**/*.{ts,tsx} : Pages and routes should use `bg-surface-1` (or appropriate surface variant) as their root container, not `bg-canvas`
Applied to files:
apps/web/core/components/base-layouts/kanban/group.tsxapps/web/app/(all)/[workspaceSlug]/(projects)/analytics/[tabId]/page.tsxpackages/propel/src/tabs/tabs.tsx
📚 Learning: 2025-12-17T10:58:59.591Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: packages/tailwind-config/AGENTS.md:0-0
Timestamp: 2025-12-17T10:58:59.591Z
Learning: Applies to packages/tailwind-config/**/*.{ts,tsx} : Use `bg-layer-1`, `bg-layer-2`, and `bg-layer-3` for nested elements within a surface to create depth, with layer-1 being the closest to the surface, layer-2 on top of layer-1, and layer-3 on top of layer-2
Applied to files:
apps/web/core/components/base-layouts/kanban/group.tsx
📚 Learning: 2025-12-17T10:58:59.591Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: packages/tailwind-config/AGENTS.md:0-0
Timestamp: 2025-12-17T10:58:59.591Z
Learning: Applies to packages/tailwind-config/**/*.{ts,tsx} : Do not use `bg-canvas` for page-level backgrounds, nested containers, cards, components, modals, dropdowns, sidebars, or panels
Applied to files:
apps/web/core/components/base-layouts/kanban/group.tsxpackages/propel/src/tabs/tabs.tsx
📚 Learning: 2025-12-17T10:58:59.591Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: packages/tailwind-config/AGENTS.md:0-0
Timestamp: 2025-12-17T10:58:59.591Z
Learning: Applies to packages/tailwind-config/**/*.{ts,tsx} : Headers and main content sections within a page should be part of the same surface container (using semantic borders for visual separation if needed) rather than using separate surface backgrounds
Applied to files:
apps/web/core/components/base-layouts/kanban/group.tsxapps/web/app/(all)/[workspaceSlug]/(projects)/analytics/[tabId]/page.tsxpackages/propel/src/tabs/tabs.tsx
📚 Learning: 2025-12-17T10:58:59.591Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: packages/tailwind-config/AGENTS.md:0-0
Timestamp: 2025-12-17T10:58:59.591Z
Learning: Applies to packages/tailwind-config/**/*.{ts,tsx} : Match layers to their parent surface: use `bg-layer-1` within `bg-surface-1`, use `bg-layer-2` within `bg-surface-2`, and use `bg-layer-3` within `bg-surface-3`
Applied to files:
apps/web/core/components/base-layouts/kanban/group.tsx
📚 Learning: 2025-12-17T10:58:59.591Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: packages/tailwind-config/AGENTS.md:0-0
Timestamp: 2025-12-17T10:58:59.591Z
Learning: Applies to packages/tailwind-config/**/*.{ts,tsx} : Do not use hover state classes (e.g., `bg-layer-1-hover`) as base backgrounds - hover states should only be applied with the `hover:` prefix alongside the base background class
Applied to files:
apps/web/core/components/base-layouts/kanban/group.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/base-layouts/layout-switcher.tsx
📚 Learning: 2025-03-11T19:42:41.769Z
Learnt from: janreges
Repo: makeplane/plane PR: 6743
File: packages/i18n/src/store/index.ts:160-161
Timestamp: 2025-03-11T19:42:41.769Z
Learning: In the Plane project, the file 'packages/i18n/src/store/index.ts' already includes support for Polish language translations with the case "pl".
Applied to files:
apps/web/core/components/base-layouts/layout-switcher.tsx
📚 Learning: 2025-12-17T10:58:59.591Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: packages/tailwind-config/AGENTS.md:0-0
Timestamp: 2025-12-17T10:58:59.591Z
Learning: Applies to packages/tailwind-config/**/*.{ts,tsx} : Use `-active` state variants (e.g., `bg-layer-1-active`) when an element is in a pressed or active state
Applied to files:
packages/propel/src/tabs/tabs.tsx
📚 Learning: 2025-12-17T10:58:59.591Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: packages/tailwind-config/AGENTS.md:0-0
Timestamp: 2025-12-17T10:58:59.591Z
Learning: Applies to packages/tailwind-config/**/*.{ts,tsx} : Use the rare exception to go one level above for visual separation only in interactive form elements (inputs, buttons, switches) within modals - for example, a modal with `bg-surface-1` can use `bg-layer-2` for form inputs to achieve visual distinction
Applied to files:
packages/propel/src/tabs/tabs.tsx
🧬 Code graph analysis (3)
apps/web/core/components/analytics/select/project.tsx (1)
packages/propel/src/icons/arrows/chevron-down.tsx (1)
ChevronDownIcon(6-15)
apps/web/app/(all)/[workspaceSlug]/(projects)/analytics/[tabId]/page.tsx (1)
packages/propel/src/tabs/tabs.tsx (1)
Tabs(135-140)
packages/propel/src/tabs/tabs.tsx (1)
packages/propel/src/utils/classname.tsx (1)
cn(63-63)
🪛 GitHub Actions: Build and lint web apps
apps/web/core/components/base-layouts/layout-switcher.tsx
[error] 30-30: TS2339: Property 'i18n_title' does not exist on type 'IBaseLayoutConfig'.
⏰ 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). (1)
- GitHub Check: check:format
🔇 Additional comments (18)
packages/propel/src/charts/bar-chart/root.tsx (1)
130-130: LGTM! Consistent grid styling update.The CartesianGrid stroke has been simplified from a semi-transparent alpha-based color to a direct CSS variable reference, improving consistency across chart components.
packages/propel/src/charts/tree-map/root.tsx (1)
19-19: LGTM! Clean treemap borders removed.Setting the stroke to transparent removes cell borders for a cleaner treemap visualization. This is a purely visual enhancement with no functional impact.
packages/propel/src/charts/area-chart/root.tsx (1)
115-115: LGTM! Consistent grid styling.The CartesianGrid stroke has been updated to use a direct CSS variable reference, consistent with the styling improvements applied across other chart components in this PR.
packages/propel/src/charts/scatter-chart/root.tsx (1)
85-85: LGTM! Grid styling aligned.The CartesianGrid stroke has been updated consistently with the other chart components, simplifying the color application to use a direct CSS variable.
packages/propel/src/charts/line-chart/root.tsx (1)
101-101: LGTM! Final grid styling update.The CartesianGrid stroke has been updated to complete the consistent styling improvements across all chart components in the propel package.
apps/web/core/components/base-layouts/kanban/group.tsx (1)
38-46: LGTM! Background styling updates align with design system.The changes from
bg-surface-2tobg-layer-1for the kanban group container and the removal ofbg-surface-2from the header correctly follow the background layering guidelines. Kanban groups are nested content that should use layer backgrounds rather than surface backgrounds.Based on learnings, headers and content within the same container should share the same surface background with semantic borders for separation, rather than using separate surface backgrounds.
apps/web/app/(all)/[workspaceSlug]/(projects)/analytics/[tabId]/page.tsx (2)
75-75: LGTM! Correct page-level background styling.The change from
bg-layer-1tobg-surface-1for the analytics page header is correct. Page-level containers should use surface backgrounds rather than layer backgrounds.Based on learnings, pages and routes should use
bg-surface-1(or appropriate surface variant) as their root container.
78-95: Tabs styling updates align with new variant system.The removal of the
backgroundprop and addition of explicit className for the Tabs.List, along with the height adjustment for Tabs.Trigger, aligns with the new variant-based styling system introduced in packages/propel/src/tabs/tabs.tsx.packages/propel/src/tabs/tabs.tsx (4)
5-11: LGTM! TabsContext pattern for variant propagation.The introduction of
TabsVarianttype andTabsContextprovides a clean way to propagate the variant through the component tree. The context pattern is appropriate for this use case.
36-49: LGTM! TabsRoot correctly provides variant context.The TabsRoot component properly wraps the primitive with the TabsContext.Provider, making the variant available to child components. The className construction is clean and maintainable.
51-75: LGTM! TabsList background handling updated correctly.The background prop has been simplified to use the
TabsVarianttype, defaulting to "contained". The conditional className application forbg-layer-3when background is "contained" is straightforward.
77-104: LGTM! TabsTrigger styling uses appropriate state variants.The TabsTrigger styling correctly uses
data-[selected]state variants withbg-layer-2for the selected state, which aligns with the design system guidelines. The use of-selectedvariants is appropriate here since the actual selection logic is implemented via thedata-[selected]attribute.Based on learnings, use
-selectedstate variants only when there is actual selection logic implemented, or use data attributes likedata-[selected]:bg-layer-1-selected.packages/propel/src/icon-button/helper.tsx (1)
41-41: LGTM! IconButton API extended for icon-specific styling.The addition of the optional
iconClassNameprop enables consumers to apply styling directly to the icon element independently from the button container. This provides greater styling flexibility, as demonstrated in the workspace sidebar components where it's used for icon rotation transitions.apps/web/core/components/workspace/sidebar/projects-list-item.tsx (1)
439-456: LGTM! Clean separation of button and icon styling.The use of the new
iconClassNameprop to control the chevron rotation is a cleaner approach than embedding transition-related classes in the button's className. This maintains the existing behavior while improving the styling organization.apps/web/core/components/workspace/sidebar/projects-list.tsx (1)
182-213: LGTM! Consistent migration to IconButton components.The replacement of native button elements with IconButton components provides a more consistent UI. The use of
iconClassNamefor the chevron rotation, along with preserved aria-labels and data attributes, maintains both functionality and accessibility.apps/web/core/components/analytics/select/project.tsx (1)
45-60: LGTM! Improved select trigger styling with button helpers.The migration from the
labelprop to acustomButtonwithgetButtonStylingand dedicated icon rendering provides more consistent styling across the application. The use of styling helpers ensures the select trigger aligns with the broader button design system.apps/web/core/components/base-layouts/layout-switcher.tsx (1)
26-26: Verify background layer is appropriate for parent container context.The container background changed from
bg-layer-1tobg-layer-3. Per the design guidelines, layers must match their parent surface: usebg-layer-1withinbg-surface-1,bg-layer-2withinbg-surface-2, andbg-layer-3withinbg-surface-3. Since this is a reusable component with varying parent contexts, ensure the parent container usesbg-surface-3, or adjust the layer to match the actual parent surface.packages/propel/src/icon-button/icon-button.tsx (1)
18-18: LGTM! Clean prop addition.The
iconClassNameprop with a safe default empty string follows the component's established pattern and enables icon-specific styling customization.
| const Icon = layout.icon; | ||
| return ( | ||
| <Tooltip key={layout.key} tooltipContent={layout.label} isMobile={isMobile}> | ||
| <Tooltip key={layout.key} tooltipContent={t(layout.i18n_title)} isMobile={isMobile}> |
There was a problem hiding this comment.
Fix TypeScript compilation error.
The pipeline failure indicates that i18n_title does not exist on type IBaseLayoutConfig. This is a critical issue that breaks the build.
🔎 Verify the correct property name:
#!/bin/bash
# Search for IBaseLayoutConfig type definition and available properties
ast-grep --pattern $'interface IBaseLayoutConfig {
$$$
}'
# Search for BASE_LAYOUTS constant definition
ast-grep --pattern $'BASE_LAYOUTS = $$$'🧰 Tools
🪛 GitHub Actions: Build and lint web apps
[error] 30-30: TS2339: Property 'i18n_title' does not exist on type 'IBaseLayoutConfig'.
🤖 Prompt for AI Agents
In apps/web/core/components/base-layouts/layout-switcher.tsx around line 30, the
code references layout.i18n_title which doesn't exist on IBaseLayoutConfig; open
the IBaseLayoutConfig definition and the BASE_LAYOUTS constant to determine the
correct property name (e.g., i18nTitle, title, or key) and replace
layout.i18n_title with that property; if the property can be undefined, provide
a safe fallback like layout.title || layout.key (or a localized default) and
keep the access type-safe (narrow or cast) so TypeScript compiles cleanly.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/web/core/components/base-layouts/layout-switcher.tsx (1)
42-48: Icon styling updated correctly.The explicit
widthandheightprops (14px) and the conditional color classes are correctly applied. Note that thesize-3.5class in the className also sets dimensions to 14px, making the explicit props somewhat redundant, but this doesn't cause issues.🔎 Optional: Remove redundant size props
Since
size-3.5already sets the icon to 14×14px, you could remove the explicitwidthandheightprops:<Icon - width={14} - height={14} strokeWidth={2} className={cn("size-3.5", {However, keeping both provides explicit documentation and works fine if the icon component expects these props.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/core/components/base-layouts/layout-switcher.tsx(3 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{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/base-layouts/layout-switcher.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Enable TypeScript strict mode and ensure all files are fully typed
Files:
apps/web/core/components/base-layouts/layout-switcher.tsx
**/*.{js,jsx,ts,tsx,json,css}
📄 CodeRabbit inference engine (AGENTS.md)
Use Prettier with Tailwind plugin for code formatting, run
pnpm fix:format
Files:
apps/web/core/components/base-layouts/layout-switcher.tsx
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,jsx,ts,tsx}: Use ESLint with shared config across packages, adhering to max warnings limits per package
Use camelCase for variable and function names, PascalCase for components and types
Use try-catch with proper error types and log errors appropriately
Files:
apps/web/core/components/base-layouts/layout-switcher.tsx
🧠 Learnings (8)
📚 Learning: 2025-12-17T10:58:59.591Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: packages/tailwind-config/AGENTS.md:0-0
Timestamp: 2025-12-17T10:58:59.591Z
Learning: Applies to packages/tailwind-config/**/*.{ts,tsx} : Use `-selected` state variants (e.g., `bg-layer-1-selected`) only when there is actual selection logic implemented, or use data attributes like `data-[selected]:bg-layer-1-selected`
Applied to files:
apps/web/core/components/base-layouts/layout-switcher.tsx
📚 Learning: 2025-12-12T15:20:36.542Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-12T15:20:36.542Z
Learning: Applies to packages/plane/ui/**/*.{ts,tsx} : Build components in `plane/ui` package with Storybook for isolated development
Applied to files:
apps/web/core/components/base-layouts/layout-switcher.tsx
📚 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/base-layouts/layout-switcher.tsx
📚 Learning: 2025-11-25T10:18:05.172Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: .github/instructions/typescript.instructions.md:0-0
Timestamp: 2025-11-25T10:18:05.172Z
Learning: Applies to **/*.{ts,tsx,mts,cts} : Ensure variables are initialized before use to avoid errors with never-initialized variables (TypeScript 5.7+)
Applied to files:
apps/web/core/components/base-layouts/layout-switcher.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/base-layouts/layout-switcher.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/base-layouts/layout-switcher.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/base-layouts/layout-switcher.tsx
📚 Learning: 2025-03-11T19:42:41.769Z
Learnt from: janreges
Repo: makeplane/plane PR: 6743
File: packages/i18n/src/store/index.ts:160-161
Timestamp: 2025-03-11T19:42:41.769Z
Learning: In the Plane project, the file 'packages/i18n/src/store/index.ts' already includes support for Polish language translations with the case "pl".
Applied to files:
apps/web/core/components/base-layouts/layout-switcher.tsx
🧬 Code graph analysis (1)
apps/web/core/components/base-layouts/layout-switcher.tsx (3)
packages/i18n/src/hooks/use-translation.ts (1)
useTranslation(23-35)apps/web/core/components/base-layouts/constants.ts (1)
BASE_LAYOUTS(4-20)packages/i18n/src/store/index.ts (1)
t(222-243)
⏰ 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). (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Build packages
🔇 Additional comments (4)
apps/web/core/components/base-layouts/layout-switcher.tsx (4)
1-1: LGTM: Imports added correctly.The
useTranslationandcnimports are properly structured and both are utilized in the component.Also applies to: 4-4
17-17: LGTM: useTranslation hook integrated correctly.The hook is properly initialized and the
tfunction is correctly destructured.
26-26: LGTM: Container styling updated appropriately.The styling changes (border radius, background layer, padding) align with the PR's refactoring objectives and improve the visual presentation.
33-38: LGTM: Button styling refactored properly.The
cnutility is correctly used to apply conditional classes based on the selected layout state. The active state styling appropriately uses-activevariants rather than-selectedvariants (as per learnings about state variant usage).
| const Icon = layout.icon; | ||
| return ( | ||
| <Tooltip key={layout.key} tooltipContent={layout.label} isMobile={isMobile}> | ||
| <Tooltip key={layout.key} tooltipContent={t(layout.label)} isMobile={isMobile}> |
There was a problem hiding this comment.
Critical: Handle missing label for gantt layout.
From the BASE_LAYOUTS constant, the gantt layout doesn't have a label property, which means layout.label is undefined for that layout. Passing undefined to t(key: string) violates the type signature and causes a TypeScript compilation error (similar to the previous i18n_title issue flagged in past reviews).
This is a blocking issue that prevents the build from passing.
🔎 Apply this diff to add a fallback:
- <Tooltip key={layout.key} tooltipContent={t(layout.label)} isMobile={isMobile}>
+ <Tooltip key={layout.key} tooltipContent={t(layout.label || layout.key)} isMobile={isMobile}>Alternative solution: Add a label property to the gantt layout in BASE_LAYOUTS constant:
{
key: "gantt",
icon: TimelineLayoutIcon,
label: "Timeline Layout", // Add this
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Tooltip key={layout.key} tooltipContent={t(layout.label)} isMobile={isMobile}> | |
| <Tooltip key={layout.key} tooltipContent={t(layout.label || layout.key)} isMobile={isMobile}> |
🤖 Prompt for AI Agents
In apps/web/core/components/base-layouts/layout-switcher.tsx around line 30,
layout.label can be undefined for the "gantt" layout which causes
t(layout.label) to violate the i18n function signature; update the call to pass
a guaranteed string by using a fallback (e.g., t(layout.label ?? layout.key) or
t(layout.label || 'gantt') ), or alternatively add a label property to the gantt
entry in BASE_LAYOUTS so layout.label is always defined.
Description
This PR update styling and structure across various components.
Type of Change
Summary by CodeRabbit
Style
UI
✏️ Tip: You can customize this high-level summary in your review settings.