refactor: styles and components on PR pages (#679)

* refactor: styles and components on PR pages * fix: lint * fix: prettier
harness · Jan 17, 2025 · 17df592 · 17df592
1 parent a24b86e
commit 17df592
Show file tree

Hide file tree

Showing 10 changed files with 169 additions and 122 deletions.
diff --git a/packages/ui/src/components/button-with-options.tsx b/packages/ui/src/components/button-with-options.tsx
@@ -1,6 +1,6 @@
-import { MouseEvent } from 'react'
+import { MouseEvent, ReactNode } from 'react'
 
-import { Button } from '@components/button'
+import { Button, buttonVariants } from '@/components/button'
 import {
   DropdownMenu,
   DropdownMenuContent,
@@ -12,6 +12,26 @@ import { Icon } from '@components/icon'
 import { Option } from '@components/option'
 import { RadioButton, RadioGroup } from '@components/radio'
 import { cn } from '@utils/cn'
+import { type VariantProps } from 'class-variance-authority'
+
+type ButtonWithOptionsSizes = Extract<VariantProps<typeof buttonVariants>['size'], 'default' | 'md'>
+type ButtonWithOptionsTheme = Exclude<NonNullable<VariantProps<typeof buttonVariants>['theme']>, null | undefined>
+
+const buttonPaddings: Record<ButtonWithOptionsSizes, string> = {
+  default: 'pl-4 pr-2.5',
+  md: 'pl-5 pr-2.5'
+}
+
+const separatorThemes: Record<ButtonWithOptionsTheme, string> = {
+  default: 'after:bg-inherit',
+  primary: 'after:bg-borders-7',
+  error: 'after:bg-button-border-danger-1',
+  success: 'after:bg-button-border-success-1',
+  disabled: 'after:bg-button-border-disabled-1',
+  // TODO: Add warning and muted themes
+  warning: '',
+  muted: ''
+}
 
 export interface ButtonWithOptionsOptionType<T extends string> {
   value: T
@@ -22,45 +42,54 @@ export interface ButtonWithOptionsOptionType<T extends string> {
 export interface ButtonWithOptionsProps<T extends string> {
   id: string
   handleButtonClick: (e: MouseEvent) => void
-  isLoading?: boolean
-  buttonText: string
+  loading?: boolean
   selectedValue: T
   options: ButtonWithOptionsOptionType<T>[]
   handleOptionChange: (val: T) => void
   className?: string
-  buttonSizeClassName?: 'h-8' | 'h-9'
+  size?: ButtonWithOptionsSizes
+  theme?: ButtonWithOptionsTheme
+  disabled?: boolean
+  children: ReactNode
 }
 
 export const ButtonWithOptions = <T extends string>({
   id,
   handleButtonClick,
-  isLoading = false,
-  buttonText,
+  loading = false,
   selectedValue,
   options,
   handleOptionChange,
   className,
-  buttonSizeClassName = 'h-8'
+  size = 'default',
+  theme = 'primary',
+  disabled = false,
+  children
 }: ButtonWithOptionsProps<T>) => {
   return (
-    <div className={cn('flex rounded bg-background-5', className)}>
+    <div className={cn('flex', className)}>
       <Button
-        className={cn('rounded-r-none pr-2.5 pl-5', buttonSizeClassName)}
-        theme="primary"
+        className={cn('rounded-r-none', theme !== 'primary' && 'border-y border-l', buttonPaddings[size])}
+        theme={theme}
+        size={size}
         onClick={handleButtonClick}
         type="button"
-        disabled={!!isLoading}
+        disabled={disabled}
+        loading={loading}
       >
-        {buttonText}
+        {children}
       </Button>
       <DropdownMenu>
         <DropdownMenuTrigger
           className={cn(
-            'relative flex size-8 items-center justify-center rounded-r after:absolute after:inset-y-0 after:left-0 after:my-auto after:h-6 after:w-px after:bg-borders-7 hover:bg-background-10',
-            buttonSizeClassName
+            buttonVariants({ theme }),
+            'relative h-[inherit] w-8 p-0 rounded-l-none after:absolute after:inset-y-0 after:left-0 after:my-auto after:h-3.5 after:w-px',
+            theme !== 'primary' && 'border-y border-r',
+            (disabled || loading) && 'pointer-events-none',
+            separatorThemes[theme || 'default']
           )}
         >
-          <Icon name="chevron-down" size={12} className="text-icons-10" />
+          <Icon name="chevron-down" size={12} className="chevron-down" />
         </DropdownMenuTrigger>
         <DropdownMenuContent
           className="mt-1 max-w-80"
@@ -73,7 +102,7 @@ export const ButtonWithOptions = <T extends string>({
                 <DropdownMenuItem
                   key={String(option.value)}
                   onClick={() => handleOptionChange(option.value)}
-                  disabled={!!isLoading}
+                  disabled={!!loading}
                 >
                   <Option
                     control={<RadioButton className="mt-px" value={String(option.value)} id={String(option.value)} />}

diff --git a/packages/ui/src/components/button.tsx b/packages/ui/src/components/button.tsx
@@ -6,13 +6,14 @@ import { cn } from '@utils/cn'
 import { cva, type VariantProps } from 'class-variance-authority'
 
 const buttonVariants = cva(
-  'inline-flex items-center justify-center whitespace-nowrap rounded text-sm font-medium transition-colors disabled:cursor-not-allowed',
+  'inline-flex items-center justify-center whitespace-nowrap rounded text-sm font-medium transition-colors disabled:pointer-events-none disabled:cursor-not-allowed',
   {
     variants: {
       variant: {
         default:
           'bg-background-5 text-foreground-6 hover:bg-background-10 disabled:bg-background-6 disabled:text-foreground-9',
-        destructive: 'bg-background-button-danger-1 text-foreground-button-danger hover:bg-background-button-danger-3',
+        destructive:
+          'bg-button-background-danger-1 text-button-foreground-danger-1 hover:bg-button-background-danger-3',
         outline:
           'border border-borders-2 bg-transparent text-foreground-2 hover:border-borders-6 hover:text-foreground-8',
         secondary: 'bg-secondary text-secondary-foreground shadow-sm hover:bg-secondary/80',
@@ -45,9 +46,12 @@ const buttonVariants = cva(
         default: '',
         error: 'border-borders-danger/30 bg-background-danger text-error',
         warning: 'border-borders-danger/30 bg-background-danger text-warning',
-        success: 'border-borders-success/30 bg-background-success text-success',
+        success:
+          'border-button-border-success-1 bg-button-background-success-1 text-button-foreground-success-1 hover:bg-button-background-success-2',
         muted: 'border-tertiary-background/20 bg-tertiary-background/10 text-tertiary-background',
-        primary: 'border-primary-foreground/20 bg-background-5 text-foreground-6'
+        primary: 'border-primary-foreground/20 bg-background-5 text-foreground-6',
+        disabled:
+          'border-button-border-disabled-1 bg-button-background-disabled-1 text-button-foreground-disabled-1 disabled:bg-button-background-disabled-1 disabled:text-button-foreground-disabled-1'
       },
       padding: {
         default: '',

diff --git a/packages/ui/src/shared-style-variables.css b/packages/ui/src/shared-style-variables.css
@@ -153,7 +153,7 @@
     --canary-foreground-alert: var(--canary-orange-55);
     --canary-foreground-success: var(--canary-green-65);
     --canary-foreground-accent: var(--canary-blue-60);
-    --canary-button-foreground-danger: var(--canary-red-65);
+    --canary-button-foreground-danger-01: var(--canary-red-65);
     /* Backgrounds */
     --canary-background-01: var(--canary-grey-6);
     --canary-background-02: var(--canary-grey-8);
@@ -171,8 +171,8 @@
     --canary-background-danger: var(--canary-red-65) / 0.1; /* red65/10 */
     --canary-background-success: 154 13% 10% / 0.4;
     --canary-background-accent: var(--canary-blue-70);
-    --canary-background-button-danger-1: var(--canary-red-65) / 0.1; /* red65/10 */
-    --canary-background-button-danger-3: var(--canary-red-65) / 0.16; /* red65/16 */
+    --canary-button-background-danger-01: var(--canary-red-65) / 0.1; /* red65/10 */
+    --canary-button-background-danger-03: var(--canary-red-65) / 0.16; /* red65/16 */
     /* Borders */
     --canary-border-01: var(--canary-grey-15);
     --canary-border-02: var(--canary-grey-20);
@@ -363,8 +363,8 @@
     --canary-background-danger: var(--canary-harness-red-350) / 0.1;
     /* I can’t find the usage of this token in Figma*/
     --canary-background-success: var(--canary-harness-green-300) / 0.1;
-    --canary-background-button-danger-1: var(--canary-harness-red-350) / 0.1;
-    --canary-background-button-danger-3: var(--canary-harness-red-350) / 0.2;
+    --canary-button-background-danger-01: var(--canary-harness-red-350) / 0.1;
+    --canary-button-background-danger-03: var(--canary-harness-red-350) / 0.2;
     --canary-toast-background-danger: var(--canary-harness-red-350);
     /* Exist in Figma, but not in code. I added it just in case */
     /* Borders */
@@ -553,7 +553,10 @@
     --canary-foreground-alert: var(--canary-orange-55);
     --canary-foreground-success: var(--canary-green-65);
     --canary-foreground-accent: var(--canary-blue-60);
-    --canary-button-foreground-danger: var(--canary-red-65);
+    /* Button Foregrounds */
+    --canary-button-foreground-danger-01: var(--canary-red-65);
+    --canary-button-foreground-disabled-01: var(--canary-grey-40);
+    --canary-button-foreground-success-01: var(--canary-green-65);
     /* Backgrounds */
     --canary-background-01: var(--canary-grey-6);
     --canary-background-02: var(--canary-grey-8);
@@ -571,8 +574,12 @@
     --canary-background-danger: var(--canary-red-65) / 0.1; /* red65/10 */
     --canary-background-success: 154 13% 10% / 0.4;
     --canary-background-accent: var(--canary-blue-70);
-    --canary-background-button-danger-1: var(--canary-red-65) / 0.1; /* red65/10 */
-    --canary-background-button-danger-3: var(--canary-red-65) / 0.16; /* red65/16 */
+    /* Button Backgrounds */
+    --canary-button-background-danger-01: var(--canary-red-65) / 0.1; /* red65/10 */
+    --canary-button-background-danger-03: var(--canary-red-65) / 0.16; /* red65/16 */
+    --canary-button-background-disabled-01: var(--canary-grey-40) / 0.1; /* grey70/10 */
+    --canary-button-background-success-01: var(--canary-green-65) / 0.1; /* green65/10 */
+    --canary-button-background-success-02: 155 26% 14%;
     /* Borders */
     --canary-border-01: var(--canary-grey-15);
     --canary-border-02: var(--canary-grey-20);
@@ -587,6 +594,9 @@
     --canary-border-danger: var(--canary-red-65);
     --canary-border-success: var(--canary-green-65);
     --canary-border-accent: var(--canary-blue-60);
+    /* Button Borders */
+    --canary-button-border-disabled-01: var(--canary-grey-40) / 0.15; /* grey70/15 */
+    --canary-button-border-success-01: var(--canary-green-65) / 0.15; /* green65/15 */
     /* Icons */
     --canary-icon-01: var(--canary-grey-60);
     --canary-icon-02: var(--canary-white);
@@ -620,10 +630,10 @@
     --canary-tag-background-blue-01: var(--canary-blue-60) / 0.1; /* blue60/10 */
     --canary-tag-background-blue-02: var(--canary-blue-60) / 0.15; /* blue60/15 */
     /* --canary-mint */
-    --canary-tag-foreground-mint-01: var(--canary-mint);
-    --canary-tag-border-mint-01: var(--canary-mint) / 0.12; /* mint/12 */
-    --canary-tag-background-mint-01: var(--canary-mint) / 0.1; /* mint/10 */
-    --canary-tag-background-mint-02: var(--canary-mint) / 0.15; /* mint/15 */
+    --canary-tag-foreground-mint-01: var(--canary-green-65);
+    --canary-tag-border-mint-01: var(--canary-green-65) / 0.12; /* mint/12 */
+    --canary-tag-background-mint-01: var(--canary-green-65) / 0.1; /* mint/10 */
+    --canary-tag-background-mint-02: var(--canary-green-65) / 0.15; /* mint/15 */
     /* --canary-amber */
     --canary-tag-foreground-amber-01: var(--canary-amber);
     --canary-tag-border-amber-01: var(--canary-amber) / 0.12; /* amber/12 */

diff --git a/packages/ui/src/styles.css b/packages/ui/src/styles.css
@@ -100,7 +100,7 @@
     --canary-foreground-alert: var(--canary-orange-55);
     --canary-foreground-success: var(--canary-green-65);
     --canary-foreground-accent: var(--canary-blue-60);
-    --canary-button-foreground-danger: var(--canary-harness-red-700);
+    --canary-button-foreground-danger-01: var(--canary-harness-red-700);
     /* Backgrounds */
     --canary-background-01: var(--canary-white);
     --canary-background-02: var(--canary-grey-8);
@@ -118,8 +118,8 @@
     --canary-background-danger: var(--canary-red-65) / 0.1; /* red65/10 */
     --canary-background-success: 154 13% 10% / 0.4;
     --canary-background-accent: var(--canary-blue-70);
-    --canary-background-button-danger-1: var(--canary-harness-red-350) / 0.1;
-    --canary-background-button-danger-3: var(--canary-harness-red-350) / 0.2;
+    --canary-button-background-danger-01: var(--canary-harness-red-350) / 0.1;
+    --canary-button-background-danger-03: var(--canary-harness-red-350) / 0.2;
     /* Borders */
     --canary-border-01: var(--canary-grey-15);
     --canary-border-02: var(--canary-grey-20);
@@ -279,8 +279,8 @@
     --canary-background-danger: var(--canary-red-65) / 0.1; /* red65/10 */
     --canary-background-success: 154 13% 10% / 0.4;
     --canary-background-accent: var(--canary-blue-70);
-    --canary-background-button-danger-1: var(--canary-red-65) / 0.1; /* red65/10 */
-    --canary-background-button-danger-3: var(--canary-red-65) / 0.16; /* red65/16 */
+    --canary-button-background-danger-01: var(--canary-red-65) / 0.1; /* red65/10 */
+    --canary-button-background-danger-03: var(--canary-red-65) / 0.16; /* red65/16 */
     /* Borders */
     --canary-border-01: var(--canary-grey-15);
     --canary-border-02: var(--canary-grey-20);
@@ -460,8 +460,8 @@
     --canary-background-danger: var(--canary-red-65) / 0.1; /* red65/10 */
     --canary-background-success: 154 13% 10% / 0.4;
     --canary-background-accent: var(--canary-blue-70);
-    --canary-background-button-danger-1: var(--canary-harness-red-350) / 0.1;
-    --canary-background-button-danger-3: var(--canary-harness-red-350) / 0.2;
+    --canary-button-background-danger-01: var(--canary-harness-red-350) / 0.1;
+    --canary-button-background-danger-03: var(--canary-harness-red-350) / 0.2;
     /* Borders */
     --canary-border-01: var(--canary-grey-15);
     --canary-border-02: var(--canary-grey-20);
@@ -621,8 +621,8 @@
     --canary-background-danger: var(--canary-red-65) / 0.1; /* red65/10 */
     --canary-background-success: 154 13% 10% / 0.4;
     --canary-background-accent: var(--canary-blue-70);
-    --canary-background-button-danger-1: var(--canary-red-65) / 0.1; /* red65/10 */
-    --canary-background-button-danger-3: var(--canary-red-65) / 0.16; /* red65/16 */
+    --canary-button-background-danger-01: var(--canary-red-65) / 0.1; /* red65/10 */
+    --canary-button-background-danger-03: var(--canary-red-65) / 0.16; /* red65/16 */
     /* Borders */
     --canary-border-01: var(--canary-grey-15);
     --canary-border-02: var(--canary-grey-20);

diff --git a/packages/ui/src/views/repo/pull-request/compare/components/pull-request-compare-button.tsx b/packages/ui/src/views/repo/pull-request/compare/components/pull-request-compare-button.tsx
@@ -61,11 +61,8 @@ const PullRequestCompareButton: FC<PullRequestCompareButtonProps> = ({
         <ButtonWithOptions<PR_TYPE>
           id="pr-type"
           handleButtonClick={handleButtonClick}
-          isLoading={isLoading}
-          buttonText={t(
-            `views:pullRequests.compareChanges${prType}Button${isLoading ? 'Loading' : ''}`,
-            `${prType}${isLoading ? 'ing' : ''} pull request${isLoading ? '...' : ''}`
-          )}
+          loading={isLoading}
+          size="md"
           selectedValue={prType}
           handleOptionChange={handlePrTypeChange}
           options={[
@@ -86,7 +83,12 @@ const PullRequestCompareButton: FC<PullRequestCompareButtonProps> = ({
               )
             }
           ]}
-        />
+        >
+          {t(
+            `views:pullRequests.compareChanges${prType}Button${isLoading ? 'Loading' : ''}`,
+            `${prType}${isLoading ? 'ing' : ''} pull request${isLoading ? '...' : ''}`
+          )}
+        </ButtonWithOptions>
       ) : (
         <Button variant="ghost" type="button" size="sm" theme="success" className="pointer-events-none">
           {t(`views:pullRequests.compareChangesCreatedButton`)}&nbsp;&nbsp;

diff --git a/packages/ui/src/views/repo/pull-request/components/pull-request-header.tsx b/packages/ui/src/views/repo/pull-request/components/pull-request-header.tsx
@@ -7,7 +7,7 @@ import { timeAgo } from '@utils/utils'
 import { IconType } from '../pull-request.types'
 import { getPrState } from '../utils'
 
-type ThemeType = 'default' | 'destructive' | 'warning' | 'success' | 'emphasis' | 'muted' | null | undefined
+type ThemeType = 'default' | 'destructive' | 'success' | 'emphasis' | 'muted' | null | undefined
 interface PullRequestTitleProps {
   data: {
     title?: string