fix(button): fix Button types and remove destructive type for the min…

…imal styled button
chanzuckerberg · Apr 30, 2024 · 36bce7f · 36bce7f
1 parent 3687567
commit 36bce7f
Show file tree

Hide file tree

Showing 8 changed files with 49 additions and 74 deletions.
diff --git a/packages/components/src/core/Button/__storybook__/constants.tsx b/packages/components/src/core/Button/__storybook__/constants.tsx
@@ -107,5 +107,5 @@ export const BUTTON_ICON_PSEUDO_STATES = [
   "default",
   "hover",
   "active",
-  "focus",
+  "focus-visible",
 ];
diff --git a/packages/components/src/core/Button/__storybook__/stories/default.tsx b/packages/components/src/core/Button/__storybook__/stories/default.tsx
@@ -15,8 +15,8 @@ const InvalidIconButtonPropsError = (
   <Callout intent="negative">
     <CalloutTitle>Invalid Props!</CalloutTitle>
     <p>
-      The icon styled Button must include an icon. Please choose an icon from
-      from the controls section.
+      The <strong>icon</strong> styled Button must include an icon. Please
+      choose an icon from from the controls section.
     </p>
   </Callout>
 );
@@ -25,8 +25,9 @@ const InvalidSdsTypeTertiaryError = (
   <Callout intent="negative">
     <CalloutTitle>Invalid Props!</CalloutTitle>
     <p>
-      Only buttons with the icon style can have the tertiary type. Please select
-      another type, either primary or secondary.
+      Only buttons with the <strong>icon</strong> style can have the{" "}
+      <strong>tertiary</strong> type. Please select another type, either{" "}
+      <strong>primary</strong> or <strong>secondary</strong>.
     </p>
   </Callout>
 );
@@ -35,9 +36,9 @@ const InvalidSdsTypeDestructiveError = (
   <Callout intent="negative">
     <CalloutTitle>Invalid Props!</CalloutTitle>
     <p>
-      Buttons with the &apos;icon&apos; style cannot have the
-      &apos;destructive&apos; type. Please choose another type, such as
-      &apos;square&apos;, &apos;rounded&apos;, or &apos;minimal&apos;.
+      Buttons with the <strong>icon</strong> or <strong>minimal</strong> styles
+      cannot have the <strong>destructive</strong> type. Please choose another
+      type, such as <strong>square</strong> or <strong>rounded</strong>.
     </p>
   </Callout>
 );
@@ -101,19 +102,15 @@ export const Button = (props: Args): JSX.Element => {
     return InvalidSdsTypeTertiaryError;
   }
 
-  if (sdsStyle === "icon" && sdsType === "destructive") {
+  if (
+    (sdsStyle === "icon" || sdsStyle === "minimal") &&
+    sdsType === "destructive"
+  ) {
     return InvalidSdsTypeDestructiveError;
   }
 
   return (
-    <RawButton
-      sdsType={sdsType}
-      sdsStyle={sdsStyle}
-      sdsSize={sdsSize}
-      {...props}
-      startIcon={startIconFinal}
-      endIcon={endIconFinal}
-    >
+    <RawButton {...props} startIcon={startIconFinal} endIcon={endIconFinal}>
       {text}
     </RawButton>
   );

diff --git a/packages/components/src/core/Button/__storybook__/stories/screenshot.tsx b/packages/components/src/core/Button/__storybook__/stories/screenshot.tsx
@@ -105,6 +105,9 @@ export const ScreenshotTestDemo = (props: Args): JSX.Element => {
           Style: <b>{sdsStyle}</b>
         </h3>
         {SCREENSHOT_BUTTON_SDS_TYPES.map((type) => {
+          // (masoudmanson): The minimal style does not have the destructive type.
+          if (sdsStyle === "minimal" && type === "destructive") return null;
+
           return (
             <ButtonTypeOption sdsStyle={sdsStyle} type={type} key={type} />
           );
@@ -113,7 +116,7 @@ export const ScreenshotTestDemo = (props: Args): JSX.Element => {
     );
   }
 
-  // loop through all BUTTON_ICON_OPTIONS_2 + create headers for SCREENSHOT_BUTTON_SDS_TYPES
+  // loop through all SCREENSHOT_BUTTON_ICON_OPTIONS + create headers for button types
   function ButtonTypeOption({
     sdsStyle,
     type,
@@ -168,7 +171,7 @@ export const ScreenshotTestDemo = (props: Args): JSX.Element => {
     );
   }
 
-  // loop through all BUTTON_DISABLED_OPTIONS + create headers for BUTTON_ICON_OPTIONS_2
+  // loop through all BUTTON_DISABLED_OPTIONS + create headers for BUTTON_ICON_OPTIONS
   function ButtonIconOption({
     sdsStyle,
     type,
@@ -247,7 +250,7 @@ export const ScreenshotTestDemo = (props: Args): JSX.Element => {
 
   // Icon Style
 
-  // loop through all SDS_SIZES + create headers for SDS_TYPES
+  // loop through all BUTTON_ICON_SDS_SIZES + create headers for SDS_TYPES
   function ButtonIconTypeOption({ sdsType }: { sdsType: SDSTypes[number] }) {
     const LEVEL_STYLE: React.CSSProperties = {
       columnGap: "20px",
@@ -278,7 +281,7 @@ export const ScreenshotTestDemo = (props: Args): JSX.Element => {
     );
   }
 
-  // loop through all ON_OPTIONS + create headers for SDS_SIZES
+  // loop through all BUTTON_ICON_DISABLED_OPTIONS + create headers for SDS_SIZES
   function ButtonIconSizeOption({
     sdsType,
     sdsSize,

diff --git a/packages/components/src/core/Button/index.tsx b/packages/components/src/core/Button/index.tsx
@@ -9,19 +9,31 @@ import ButtonIcon from "../ButtonIcon";
 import { IconNameToSizes, IconProps } from "../Icon";
 import { filterProps } from "src/common/utils";
 
-type ButtonStyle = "rounded" | "square" | "minimal" | "icon";
 type ButtonType = "primary" | "secondary" | "tertiary" | "destructive";
 type ButtonSize = "small" | "medium" | "large";
 
-type SdsProps = {
-  sdsStyle?: ButtonStyle;
-  sdsType?: ButtonType;
-  isAllCaps?: boolean;
-  isRounded?: boolean;
-  sdsSize?: ButtonSize;
-  icon?: keyof IconNameToSizes | React.ReactElement<CustomSVGProps>;
-  sdsIconProps?: Partial<IconProps<keyof IconNameToSizes>>;
-};
+type SdsProps =
+  | {
+      sdsStyle?: "icon";
+      sdsType?: "primary" | "secondary" | "tertiary";
+      isAllCaps?: boolean;
+      isRounded?: boolean;
+      sdsSize?: ButtonSize;
+      icon?: keyof IconNameToSizes | React.ReactElement<CustomSVGProps>;
+      sdsIconProps?: Partial<IconProps<keyof IconNameToSizes>>;
+    }
+  | {
+      sdsStyle?: "minimal";
+      sdsType?: "primary" | "secondary";
+      isAllCaps?: boolean;
+      isRounded?: boolean;
+    }
+  | {
+      sdsStyle?: "rounded" | "square";
+      sdsType?: "primary" | "secondary" | "destructive";
+      isAllCaps?: boolean;
+      isRounded?: boolean;
+    };
 
 export type ButtonProps = RawButtonProps & SdsProps;
 
@@ -43,7 +55,7 @@ const Button = React.forwardRef(
     props: ButtonProps,
     ref: ForwardedRef<HTMLButtonElement>
   ): JSX.Element | null => {
-    const { sdsStyle, sdsType, icon } = props;
+    const { sdsStyle, sdsType } = props;
 
     if (!sdsStyle || !sdsType) {
       showWarningIfFirstOccurence(SDSWarningTypes.ButtonMissingSDSProps);
@@ -62,7 +74,7 @@ const Button = React.forwardRef(
 
     switch (true) {
       case sdsStyle === "icon":
-        if (icon !== undefined) {
+        if (props?.icon !== undefined) {
           // (masoudmanson): We need to remove the props that are not supported by
           // the ButtonIcon component.
           const excludedProps: (keyof PropsWithDefaultsType)[] = [
@@ -76,7 +88,7 @@ const Button = React.forwardRef(
 
           return (
             <ButtonIcon
-              icon={icon}
+              icon={props?.icon}
               {...finalProps}
               sdsType={sdsType as Exclude<ButtonType, "destructive">}
               ref={ref}

diff --git a/packages/components/src/core/Button/style.ts b/packages/components/src/core/Button/style.ts
@@ -287,42 +287,6 @@ const SecondaryMinimalButton = (props: ButtonExtraProps): SerializedStyles => {
   `;
 };
 
-const DestructiveMinimalButton = (
-  props: ButtonExtraProps
-): SerializedStyles => {
-  const semanticTextColors = getSemanticTextColors(props);
-  const semanticComponentColors = getSemanticComponentColors(props);
-
-  return css`
-    ${Minimal(props)}
-    color: ${semanticComponentColors?.negative?.fill};
-
-    &:hover,
-    &:focus-visible {
-      background-color: transparent;
-      color: ${semanticComponentColors?.negative?.fillHover};
-
-      svg {
-        color: ${semanticComponentColors?.negative?.fillHover};
-      }
-    }
-    &:active {
-      color: ${semanticComponentColors?.negative?.fillPressed};
-
-      svg {
-        color: ${semanticComponentColors?.negative?.fillPressed};
-      }
-    }
-    &:disabled {
-      color: ${semanticTextColors?.base?.onFillDisabled};
-
-      svg {
-        color: ${semanticTextColors?.base?.onFillDisabled};
-      }
-    }
-  `;
-};
-
 export const StyledMinimalButton = styled(Button, {
   shouldForwardProp: (prop: string) => !doNotForwardProps.includes(prop),
 })`
@@ -332,7 +296,6 @@ export const StyledMinimalButton = styled(Button, {
     return css`
       ${sdsType === "primary" && PrimaryMinimalButton(props)}
       ${sdsType === "secondary" && SecondaryMinimalButton(props)}
-      ${sdsType === "destructive" && DestructiveMinimalButton(props)}
     `;
   }}
 `;

diff --git a/packages/components/src/core/Dialog/__tests__/__snapshots__/index.test.tsx.snap b/packages/components/src/core/Dialog/__tests__/__snapshots__/index.test.tsx.snap
@@ -2,7 +2,7 @@
 
 exports[`<Dialog /> Default story renders snapshot 1`] = `
 <button
-  class="MuiButtonBase-root MuiButton-root MuiButton-text MuiButton-textPrimary MuiButton-sizeMedium MuiButton-textSizeMedium MuiButton-root MuiButton-text MuiButton-textPrimary MuiButton-sizeMedium MuiButton-textSizeMedium css-1evm0vy-MuiButtonBase-root-MuiButton-root"
+  class="MuiButtonBase-root MuiButton-root MuiButton-text MuiButton-textPrimary MuiButton-sizeMedium MuiButton-textSizeMedium MuiButton-root MuiButton-text MuiButton-textPrimary MuiButton-sizeMedium MuiButton-textSizeMedium css-1nbscw4-MuiButtonBase-root-MuiButton-root"
   tabindex="0"
   type="button"
 >

diff --git a/packages/components/src/core/TagFilter/__tests__/__snapshots__/index.test.tsx.snap b/packages/components/src/core/TagFilter/__tests__/__snapshots__/index.test.tsx.snap
@@ -6,7 +6,7 @@ exports[`<TagFilter /> Default story renders snapshot 1`] = `
 >
   <div>
     <button
-      class="MuiButtonBase-root MuiButton-root MuiButton-text MuiButton-textPrimary MuiButton-sizeMedium MuiButton-textSizeMedium Mui-disabled MuiButton-root MuiButton-text MuiButton-textPrimary MuiButton-sizeMedium MuiButton-textSizeMedium css-g6h7xk-MuiButtonBase-root-MuiButton-root"
+      class="MuiButtonBase-root MuiButton-root MuiButton-text MuiButton-textPrimary MuiButton-sizeMedium MuiButton-textSizeMedium Mui-disabled MuiButton-root MuiButton-text MuiButton-textPrimary MuiButton-sizeMedium MuiButton-textSizeMedium css-77d05w-MuiButtonBase-root-MuiButton-root"
       disabled=""
       tabindex="-1"
       type="button"

diff --git a/...ta-viz/src/core/HeatmapChart/__storybook__/stories/heatmapDemo/components/Header/style.ts b/...ta-viz/src/core/HeatmapChart/__storybook__/stories/heatmapDemo/components/Header/style.ts
@@ -25,7 +25,7 @@ export const HeaderLeft = styled("div")`
 `;
 
 export const HeaderRight = styled("div")`
-  display: flex;
+  display: block;
 `;
 
 export const StyledButton = styled(Button)`