Merge pull request #1958 from NYPL/ISW-5431/icon-styles-bug

ISW-5431: double style bug for custom svgs

Author: bigfishdesign13

CHANGELOG.md

@@ -8,6 +8,10 @@ Currently, this repo is in Prerelease. When it is released, this project will ad
 
 ## Prerelease
 
+### Fixes
+
+- Fixed the double application of styles when a custom svg is passed into the `Icon` component.
+
 ## 4.1.4 (February 26, 2026)
 
 ### Updates

src/components/Icons/Icon.mdx

@@ -19,7 +19,7 @@ import { changelogData } from "./iconChangelogData";
   componentName="Icon"
   summary="Renders commonly used icons in SVG format"
   versionAdded="0.0.4"
-  versionLatest="4.1.1"
+  versionLatest="Prerelease"
 />
 
 <Canvas of={IconStories.WithControls} />

src/components/Icons/Icon.test.tsx

@@ -50,7 +50,7 @@ describe("Icon", () => {
     const warn = jest.spyOn(console, "warn");
     render(<Icon>Not an SVG</Icon>);
     expect(warn).toHaveBeenCalledWith(
-      "NYPL Reservoir Icon: An `svg` element must be passed to the `Icon` " +
+      "NYPL Reservoir Icon: Only an `svg` element can be passed to the `Icon` " +
         "component as its child."
     );
   });

src/components/Icons/Icon.tsx

@@ -76,9 +76,11 @@ export const Icon: ChakraComponent<
       variant = "default",
       ...rest
     } = props;
+    const hasChildSVG = children && (children as JSX.Element).type === "svg";
     const styles = useStyleConfig("ReservoirIcon", {
       align,
       color,
+      hasChildSVG,
       iconRotation,
       size,
       variant,
@@ -90,16 +92,21 @@ export const Icon: ChakraComponent<
       title,
       ...rest,
     };
-    let childSVG: any = null;
 
     // Component prop validation
-    if (name && children) {
+    if (name && hasChildSVG) {
       console.warn(
         "NYPL Reservoir Icon: Pass in either a `name` prop or an `svg` element " +
           "child. Do not pass both."
       );
       return null;
-    } else if (!name && !children) {
+    } else if (children && !hasChildSVG) {
+      console.warn(
+        "NYPL Reservoir Icon: Only an `svg` element can be passed to the `Icon` " +
+          "component as its child."
+      );
+      return null;
+    } else if (!name && !hasChildSVG) {
       console.warn(
         "NYPL Reservoir Icon: Pass an icon `name` prop or an SVG child to " +
           "ensure an icon appears."
@@ -117,22 +124,13 @@ export const Icon: ChakraComponent<
       );
     }
 
-    // If no `name` prop was passed, we expect a child SVG element to be passed.
+    // If all prop validation passed and no `name` prop was passed, we expect a
+    // child SVG element was passed and render it accordingly.
     // Apply icon props to the SVG child.
-    if (
-      (children as JSX.Element).type === "svg" ||
-      (children as JSX.Element).props?.type === "svg"
-    ) {
-      childSVG = React.cloneElement(children as JSX.Element, {
-        ...iconProps,
-        ref,
-      });
-    } else {
-      console.warn(
-        "NYPL Reservoir Icon: An `svg` element must be passed to the `Icon` " +
-          "component as its child."
-      );
-    }
+    const childSVG = React.cloneElement(children as JSX.Element, {
+      ...iconProps,
+      ref,
+    });
 
     return (
       <Box ref={ref} __css={styles}>

src/components/Icons/iconChangelogData.ts

@@ -9,6 +9,15 @@
 import { ChangelogData } from "../../utils/ComponentChangelogTable";
 
 export const changelogData: ChangelogData[] = [
+  {
+    date: "Prerelease",
+    version: "Prerelease",
+    type: "Bug Fix",
+    affects: ["Functionality", "Styles"],
+    notes: [
+      "Fixed the double application of styles when a custom svg is passed.",
+    ],
+  },
   {
     date: "2025-12-11",
     version: "4.1.1",

src/theme/components/icon.ts

@@ -38,6 +38,7 @@ const iconRotation: Record<string, { transform: string }> = {
 interface IconBaseStyle extends StyleFunctionProps {
   align: keyof typeof align;
   color: string;
+  hasChildSVG: boolean;
   iconRotation: keyof typeof iconRotation;
   size: keyof typeof iconSizeStyles;
 }
@@ -53,7 +54,10 @@ const Icon = defineStyleConfig({
     };
 
     return {
-      ...allStyles,
+      // Apply styles to the root element if a custom svg is not passed
+      ...(!props.hasChildSVG ? allStyles : {}),
+
+      // Apply styles to a child svg element
       svg: {
         ...allStyles,
       },