fix(box): css style properties passed as HTML attributes

removed the css properties from being passed directly to the styled component
Sage · Dec 5, 2024 · 2d47a16 · 2d47a16
1 parent 6675b86
commit 2d47a16
Show file tree

Hide file tree

Showing 5 changed files with 83 additions and 24 deletions.
diff --git a/src/components/box/box.component.tsx b/src/components/box/box.component.tsx
@@ -101,6 +101,8 @@ export const Box = React.forwardRef<HTMLDivElement, BoxProps>(
       borderRadius,
       color,
       opacity,
+      height,
+      width,
       "aria-hidden": ariaHidden,
       ...rest
     }: BoxProps,
@@ -113,6 +115,28 @@ export const Box = React.forwardRef<HTMLDivElement, BoxProps>(
       );
     }
 
+    let actualWidth = "";
+    if (typeof width === "number") {
+      actualWidth = width <= 1 ? `${(width * 100).toFixed(0)}%` : `${width}px`;
+    } else if (typeof width === "string") {
+      actualWidth = width;
+    }
+
+    let actualHeight = "";
+    if (typeof height === "number") {
+      actualHeight =
+        height <= 1 ? `${(height * 100).toFixed(0)}%` : `${height}px`;
+    } else if (typeof height === "string") {
+      actualHeight = height;
+    }
+
+    const cssProps = {
+      color,
+      opacity,
+      width: actualWidth,
+      height: actualHeight,
+    };
+
     return (
       <StyledBox
         as={as}
@@ -131,15 +155,14 @@ export const Box = React.forwardRef<HTMLDivElement, BoxProps>(
         backgroundColor={backgroundColor}
         boxShadow={boxShadow}
         borderRadius={borderRadius}
-        color={color}
-        opacity={opacity}
         aria-hidden={ariaHidden}
         {...tagComponent(dataComponent, rest)}
         {...filterStyledSystemMarginProps(rest)}
         {...filterStyledSystemPaddingProps(rest)}
         {...filterStyledSystemFlexboxProps(rest)}
         {...filterStyledSystemGridProps(rest)}
         {...filterStyledSystemLayoutProps(rest)}
+        cssProps={cssProps}
       >
         {children}
       </StyledBox>

diff --git a/src/components/box/box.pw.tsx b/src/components/box/box.pw.tsx
@@ -58,7 +58,6 @@ test.describe("should render Box component", () => {
     }) => {
       await mount(<Default color={color} />);
       const boxElement = await getDataElementByValue(page, "box");
-      await expect(boxElement).toHaveAttribute("color", color);
       await expect(boxElement).toHaveCSS("color", rgbValue);
     });
   });
@@ -70,7 +69,6 @@ test.describe("should render Box component", () => {
     }) => {
       await mount(<Default color={rgbValue} />);
       const boxElement = await getDataElementByValue(page, "box");
-      await expect(boxElement).toHaveAttribute("color", rgbValue);
       await expect(boxElement).toHaveCSS("color", rgbValue);
     });
   });
@@ -82,7 +80,6 @@ test.describe("should render Box component", () => {
     }) => {
       await mount(<Default color={hexValue} />);
       const boxElement = await getDataElementByValue(page, "box");
-      await expect(boxElement).toHaveAttribute("color", hexValue);
       await expect(boxElement).toHaveCSS("color", rgbValue);
     });
   });
@@ -168,7 +165,6 @@ test.describe("should render Box component", () => {
     }) => {
       await mount(<Default bg="primary" width={percentage} />);
       const boxElement = await getDataElementByValue(page, "box");
-      await expect(boxElement).toHaveAttribute("width", String(percentage));
       await assertCssValueIsApproximately(boxElement, "width", parseInt(width));
     });
   });
@@ -180,7 +176,6 @@ test.describe("should render Box component", () => {
     }) => {
       await mount(<Default bg="primary" width={number} />);
       const boxElement = await getDataElementByValue(page, "box");
-      await expect(boxElement).toHaveAttribute("width", String(number));
       await assertCssValueIsApproximately(boxElement, "width", parseInt(width));
     });
   });
@@ -192,7 +187,6 @@ test.describe("should render Box component", () => {
     }) => {
       await mount(<Default bg="primary" width={width} />);
       const boxElement = await getDataElementByValue(page, "box");
-      await expect(boxElement).toHaveAttribute("width", width);
       await assertCssValueIsApproximately(boxElement, "width", parseInt(width));
     });
   });
@@ -204,7 +198,6 @@ test.describe("should render Box component", () => {
     }) => {
       await mount(<Default bg="primary" height={number} />);
       const boxElement = await getDataElementByValue(page, "box");
-      await expect(boxElement).toHaveAttribute("height", String(number));
       await assertCssValueIsApproximately(
         boxElement,
         "height",
@@ -220,7 +213,6 @@ test.describe("should render Box component", () => {
     }) => {
       await mount(<Default bg="primary" height={height} />);
       const boxElement = await getDataElementByValue(page, "box");
-      await expect(boxElement).toHaveAttribute("height", height);
       await assertCssValueIsApproximately(
         boxElement,
         "height",

diff --git a/src/components/box/box.style.ts b/src/components/box/box.style.ts
@@ -23,7 +23,16 @@ const calculatePosition = (props: Omit<PositionProps, "zIndex">) => {
   };
 };
 
-const StyledBox = styled.div<BoxProps>`
+const StyledBox = styled.div<
+  BoxProps & {
+    cssProps?: {
+      color?: string;
+      opacity?: string;
+      height?: string;
+      width?: string;
+    };
+  }
+>`
   ${space}
   ${layout}
   ${flexbox}
@@ -35,26 +44,32 @@ const StyledBox = styled.div<BoxProps>`
     css`
       border-radius: var(--${borderRadius});
     `}
+  
+  ${({ cssProps, bg, backgroundColor, ...rest }) =>
+    styledColor({ color: cssProps?.color, bg, backgroundColor, ...rest })}
 
-  ${({ color, bg, backgroundColor, ...rest }) =>
-    styledColor({ color, bg, backgroundColor, ...rest })}
+  ${({ cssProps }) => css`
+    opacity: ${cssProps?.opacity};
+  `}
 
   ${({ overflowWrap }) =>
     overflowWrap &&
     css`
       overflow-wrap: ${overflowWrap};
     `}
   
-  ${({ height }) =>
-    height &&
+  ${({ cssProps, size }) =>
+    cssProps?.height &&
+    !size &&
     css`
-      height: ${height};
+      height: ${cssProps?.height};
     `}
 
-  ${({ width }) =>
-    width &&
+  ${({ cssProps, size }) =>
+    cssProps?.width &&
+    !size &&
     css`
-      width: ${width};
+      width: ${cssProps?.width};
     `}
 
   ${({ scrollVariant }) =>

diff --git a/src/components/box/box.test.tsx b/src/components/box/box.test.tsx
@@ -76,6 +76,40 @@ it("applies the boxShadow styling correctly when a design token is passed in", (
   expect(box).toHaveStyle(`box-shadow: var(--boxShadow100)`);
 });
 
+it("applies the correct styling from the cssProps", () => {
+  render(
+    <Box
+      width="100px"
+      height="100px"
+      opacity="25%"
+      color="yellow"
+      data-role="box"
+    />,
+  );
+
+  const box = screen.getByTestId("box");
+  expect(box).toHaveStyle(`width: 100px`);
+  expect(box).toHaveStyle(`height: 100px`);
+  expect(box).toHaveStyle(`opacity: 25%`);
+  expect(box).toHaveStyle(`color: yellow`);
+});
+
+it("applies the correct styling from the cssProps, when the width and height are numeric", () => {
+  render(<Box width={100} height={100} data-role="box" />);
+
+  const box = screen.getByTestId("box");
+  expect(box).toHaveStyle(`width: 100px`);
+  expect(box).toHaveStyle(`height: 100px`);
+});
+
+it("applies the correct styling from the cssProps, when the width and height are percentage", () => {
+  render(<Box width={0.5} height={0.5} data-role="box" />);
+
+  const box = screen.getByTestId("box");
+  expect(box).toHaveStyle(`width: 50%`);
+  expect(box).toHaveStyle(`height: 50%`);
+});
+
 test("logs a deprecation warning when the `tabIndex` prop is passed with a value", () => {
   const loggerSpy = jest
     .spyOn(Logger, "deprecate")

diff --git a/src/components/dismissible-box/dismissible-box.pw.tsx b/src/components/dismissible-box/dismissible-box.pw.tsx
@@ -46,11 +46,6 @@ test.describe("DismissibleBox component", () => {
     }) => {
       await mount(<DefaultDismissibleBox width={`${width}px`} />);
 
-      await expect(dismissibleBoxDataComponent(page)).toHaveAttribute(
-        "width",
-        `${width}px`,
-      );
-
       await assertCssValueIsApproximately(
         dismissibleBoxDataComponent(page),
         "width",