[WEB-5614] refactor: update styling and structure across various components #8388
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -441,8 +441,10 @@ export const SidebarProjectsListItem = observer(function SidebarProjectsListItem | |
| size="sm" | ||
| icon={ChevronRightIcon} | ||
| onClick={() => setIsProjectListOpen(!isProjectListOpen)} | ||
| className={cn("hidden group-hover/project-item:inline-flex text-placeholder", { | ||
| "inline-flex": isMenuActive, | ||
| })} | ||
| iconClassName={cn("transition-transform", { | ||
| "rotate-90": isProjectListOpen, | ||
| })} | ||
| aria-label={t( | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,6 +9,7 @@ import { Disclosure, Transition } from "@headlessui/react"; | |
| import { EUserPermissions, EUserPermissionsLevel, PROJECT_TRACKER_ELEMENTS } from "@plane/constants"; | ||
| import { useTranslation } from "@plane/i18n"; | ||
| import { ChevronRightIcon } from "@plane/propel/icons"; | ||
| import { IconButton } from "@plane/propel/icon-button"; | ||
| import { TOAST_TYPE, setToast } from "@plane/propel/toast"; | ||
| import { Tooltip } from "@plane/propel/tooltip"; | ||
| import { Loader } from "@plane/ui"; | ||
|
|
@@ -178,39 +179,37 @@ export const SidebarProjectsList = observer(function SidebarProjectsList() { | |
| > | ||
| <span className="text-13 font-semibold">{t("projects")}</span> | ||
| </Disclosure.Button> | ||
| <div className="flex items-center gap-1"> | ||
| {isAuthorizedUser && ( | ||
| <Tooltip tooltipHeading={t("create_project")} tooltipContent=""> | ||
| <IconButton | ||
| variant="ghost" | ||
| size="sm" | ||
| icon={Plus} | ||
| onClick={() => { | ||
| setIsProjectModalOpen(true); | ||
| }} | ||
| data-ph-element={PROJECT_TRACKER_ELEMENTS.SIDEBAR_CREATE_PROJECT_TOOLTIP} | ||
| className="hidden group-hover:inline-flex text-placeholder" | ||
| aria-label={t("aria_labels.projects_sidebar.create_new_project")} | ||
| /> | ||
| </Tooltip> | ||
| )} | ||
| <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" | ||
| )} | ||
| /> | ||
|
Comment on lines
+198
to
+212
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. |
||
| </div> | ||
| </div> | ||
| <Transition | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -112,7 +112,7 @@ export const AreaChart = React.memo(function AreaChart<K extends string, T exten | |
| left: margin?.left === undefined ? 20 : margin.left, | ||
| }} | ||
| > | ||
| <CartesianGrid stroke="var(--border-color-subtle)" vertical={false} /> | ||
| <XAxis | ||
| dataKey={xAxis.key} | ||
| tick={(props) => { | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -127,7 +127,7 @@ export const BarChart = React.memo(function BarChart<K extends string, T extends | |
| barSize={barSize} | ||
| className="recharts-wrapper" | ||
| > | ||
| <CartesianGrid stroke="var(--border-color-subtle)" vertical={false} /> | ||
| <XAxis | ||
| dataKey={xAxis.key} | ||
| tick={(props) => { | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -98,7 +98,7 @@ export const LineChart = React.memo(function LineChart<K extends string, T exten | |
| left: margin?.left === undefined ? 20 : margin.left, | ||
| }} | ||
| > | ||
| <CartesianGrid stroke="var(--border-color-subtle)" vertical={false} /> | ||
| <XAxis | ||
| dataKey={xAxis.key} | ||
| tick={(props) => { | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -82,7 +82,7 @@ export const ScatterChart = React.memo(function ScatterChart<K extends string, T | |
| left: margin?.left === undefined ? 20 : margin.left, | ||
| }} | ||
| > | ||
| <CartesianGrid stroke="var(--border-color-subtle)" vertical={false} /> | ||
| <XAxis | ||
| dataKey={xAxis.key} | ||
| tick={(props) => { | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,7 +16,7 @@ export const TreeMapChart = React.memo(function TreeMapChart(props: TreeMapChart | |
| data={data} | ||
| nameKey="name" | ||
| dataKey="value" | ||
| stroke="transparent" | ||
| className="bg-layer-1 cursor-pointer" | ||
| content={<CustomTreeMapContent />} | ||
| animationEasing="ease-out" | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -38,6 +38,7 @@ type IconButtonPropsWithChildren = React.ButtonHTMLAttributes<HTMLButtonElement> | |
| VariantProps<typeof iconButtonVariants> & { | ||
| icon: React.FC<{ className?: string }>; | ||
| loading?: boolean; | ||
| iconClassName?: string; | ||
| }; | ||
| export type IconButtonProps = Omit<IconButtonPropsWithChildren, "children">; | ||
|
|
||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,6 +15,7 @@ const IconButton = React.forwardRef(function IconButton( | |
| loading = false, | ||
| disabled = false, | ||
| icon: Icon, | ||
| iconClassName = "", | ||
| ...rest | ||
| } = props; | ||
|
|
||
|
|
@@ -27,11 +28,14 @@ const IconButton = React.forwardRef(function IconButton( | |
| {...rest} | ||
| > | ||
| <Icon | ||
| className={cn( | ||
| { | ||
| "size-3.5": size === "sm", | ||
| "size-4": size === "base" || size === "lg", | ||
| "size-5": size === "xl", | ||
| }, | ||
| iconClassName | ||
| )} | ||
| /> | ||
| </button> | ||
| ); | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -2,20 +2,29 @@ import * as React from "react"; | |||||
| import { Tabs as TabsPrimitive } from "@base-ui-components/react/tabs"; | ||||||
| import { cn } from "../utils/classname"; | ||||||
|
|
||||||
| type TabsVariant = "contained"; | ||||||
|
|
||||||
| type TabsContextType = { | ||||||
| variant?: TabsVariant; | ||||||
| }; | ||||||
|
|
||||||
| const TabsContext = React.createContext<TabsContextType | undefined>(undefined); | ||||||
|
Comment on lines
+7
to
+11
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. |
||||||
|
|
||||||
| type TabsCompound = React.ForwardRefExoticComponent< | ||||||
| React.ComponentProps<typeof TabsPrimitive.Root> & { | ||||||
| variant?: TabsVariant; | ||||||
| } & React.RefAttributes<React.ElementRef<typeof TabsPrimitive.Root>> | ||||||
| > & { | ||||||
| List: React.ForwardRefExoticComponent< | ||||||
| React.ComponentProps<typeof TabsPrimitive.List> & { | ||||||
| background?: TabsVariant; | ||||||
| } & React.RefAttributes<React.ElementRef<typeof TabsPrimitive.List>> | ||||||
| >; | ||||||
| Trigger: React.ForwardRefExoticComponent< | ||||||
| React.ComponentProps<typeof TabsPrimitive.Tab> & { | ||||||
| size?: "sm" | "md" | "lg"; | ||||||
| variant?: TabsVariant; | ||||||
| } & React.RefAttributes<React.ElementRef<typeof TabsPrimitive.Tab>> | ||||||
| >; | ||||||
| Content: React.ForwardRefExoticComponent< | ||||||
| React.ComponentProps<typeof TabsPrimitive.Panel> & React.RefAttributes<React.ElementRef<typeof TabsPrimitive.Panel>> | ||||||
|
|
@@ -24,39 +33,38 @@ type TabsCompound = React.ForwardRefExoticComponent< | |||||
| }; | ||||||
|
|
||||||
| const TabsRoot = React.forwardRef(function TabsRoot( | ||||||
| { className, variant, ...props }: React.ComponentProps<typeof TabsPrimitive.Root> & { variant?: TabsVariant }, | ||||||
| ref: React.ForwardedRef<React.ElementRef<typeof TabsPrimitive.Root>> | ||||||
| ) { | ||||||
| return ( | ||||||
| <TabsContext.Provider value={{ variant }}> | ||||||
| <TabsPrimitive.Root | ||||||
| data-slot="tabs" | ||||||
| className={cn("flex flex-col w-full h-full", className)} | ||||||
| {...props} | ||||||
| ref={ref} | ||||||
| /> | ||||||
| </TabsContext.Provider> | ||||||
| ); | ||||||
| }); | ||||||
|
|
||||||
| const TabsList = React.forwardRef(function TabsList( | ||||||
| { | ||||||
| className, | ||||||
| background = "contained", | ||||||
| ...props | ||||||
| }: React.ComponentProps<typeof TabsPrimitive.List> & { | ||||||
| background?: TabsVariant; | ||||||
| }, | ||||||
| ref: React.ForwardedRef<React.ElementRef<typeof TabsPrimitive.List>> | ||||||
| ) { | ||||||
| return ( | ||||||
| <TabsPrimitive.List | ||||||
| data-slot="tabs-list" | ||||||
| className={cn( | ||||||
| "flex w-full items-center justify-between gap-1.5 rounded-lg text-13 p-0.5 relative overflow-auto", | ||||||
| { | ||||||
| "bg-layer-3": background === "contained", | ||||||
| }, | ||||||
| className | ||||||
| )} | ||||||
|
|
@@ -67,16 +75,20 @@ const TabsList = React.forwardRef(function TabsList( | |||||
| }); | ||||||
|
|
||||||
| const TabsTrigger = React.forwardRef(function TabsTrigger( | ||||||
| { | ||||||
| className, | ||||||
| size = "md", | ||||||
| ...props | ||||||
| }: React.ComponentProps<typeof TabsPrimitive.Tab> & { size?: "sm" | "md" | "lg"; variant?: TabsVariant }, | ||||||
|
Comment on lines
+78
to
+82
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. |
||||||
| ref: React.ForwardedRef<React.ElementRef<typeof TabsPrimitive.Tab>> | ||||||
| ) { | ||||||
| return ( | ||||||
| <TabsPrimitive.Tab | ||||||
| data-slot="tabs-trigger" | ||||||
| className={cn( | ||||||
| "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.
Suggested change
|
||||||
| "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.
Suggested change
|
||||||
| "disabled:text-placeholder disabled:cursor-not-allowed", | ||||||
| { | ||||||
| "text-11": size === "sm", | ||||||
|
|
||||||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.