Wrap Icon component's SVG in a div to fix Safari rendering issue (#931)

* wrap icon in a wrapper element

* render icon wrapper as div

* update snapshots

* add changeset

* fix failing tests

* remove destructuring of imports

Author: joshfarrant

.changeset/grumpy-countries-fail.md

@@ -0,0 +1,5 @@
+---
+'@primer/react-brand': patch
+---
+
+Fixed a bug in Safari where an `Icon` component with `hasBackground={true}` would cut off the corners of the rendered SVG. To resolve this the `Icon` component now wraps the rendered SVG in a div.

packages/react/src/Card/Card.visual.spec.ts-snapshots/Visual-Comparison-Card-Card-Icon-native-1-linux.png

@@ -3,6 +3,41 @@
   padding: 0;
 }
 
+.Icon--size-20 {
+  width: var(--base-size-20);
+  height: var(--base-size-20);
+}
+
+.Icon--size-24 {
+  width: var(--base-size-24);
+  height: var(--base-size-24);
+}
+
+.Icon--size-28 {
+  width: var(--base-size-28);
+  height: var(--base-size-28);
+}
+
+.Icon--size-32 {
+  width: var(--base-size-32);
+  height: var(--base-size-32);
+}
+
+.Icon--size-36 {
+  width: var(--base-size-36);
+  height: var(--base-size-36);
+}
+
+.Icon--size-40 {
+  width: var(--base-size-40);
+  height: var(--base-size-40);
+}
+
+.Icon--size-44 {
+  width: var(--base-size-44);
+  height: var(--base-size-44);
+}
+
 .Icon--background {
   padding: var(--base-size-12);
   border-radius: var(--brand-borderRadius-full);

packages/react/src/Icon/Icon.module.css

@@ -29,6 +29,13 @@ declare const styles: {
   readonly "Icon--color-red": string;
   readonly "Icon--color-teal": string;
   readonly "Icon--color-yellow": string;
+  readonly "Icon--size-20": string;
+  readonly "Icon--size-24": string;
+  readonly "Icon--size-28": string;
+  readonly "Icon--size-32": string;
+  readonly "Icon--size-36": string;
+  readonly "Icon--size-40": string;
+  readonly "Icon--size-44": string;
 };
 export = styles;
 

packages/react/src/Icon/Icon.module.css.d.ts

@@ -22,14 +22,14 @@ describe('Icon', () => {
     expect(getByLabelText('Git merge icon')).toBeInTheDocument()
   })
 
-  it('forwards className to the icon', () => {
+  it('forwards className to the wrapper element', () => {
     const {getByLabelText} = render(<Icon icon={GitMergeIcon} aria-label="Git merge icon" className="custom-class" />)
-    expect(getByLabelText('Git merge icon')).toHaveClass('custom-class')
+    expect(getByLabelText('Git merge icon').parentElement).toHaveClass('custom-class')
   })
 
   it('sets the color of the icon', () => {
     const {getByLabelText} = render(<Icon icon={GitMergeIcon} aria-label="Git merge icon" color="blue" />)
-    expect(getByLabelText('Git merge icon')).toHaveClass('Icon--color-blue')
+    expect(getByLabelText('Git merge icon').parentElement).toHaveClass('Icon--color-blue')
   })
 
   it('sets the size of the icon when `size` is a number', () => {

packages/react/src/Icon/Icon.test.tsx

@@ -1,4 +1,4 @@
-import React, {type SVGAttributes} from 'react'
+import React, {type ReactElement, type SVGAttributes} from 'react'
 import styles from './Icon.module.css'
 import clsx from 'clsx'
 import {type Icon as OcticonProps} from '@primer/octicons-react'
@@ -37,7 +37,7 @@ export type IconColor = (typeof iconColors)[number]
 export const defaultIconColor = iconColors[0]
 
 export type IconProps = SVGAttributes<SVGElement> & {
-  icon: OcticonProps | React.ReactElement<OcticonProps>
+  icon: OcticonProps | ReactElement<OcticonProps>
   color?: IconColor
   hasBackground?: boolean
   size?: IconSize
@@ -54,25 +54,30 @@ export const Icon = ({
   const iconSize = getIconSize(size)
 
   const iconProps = {
-    className: clsx(
-      styles['Icon'],
-      styles[`Icon--color-${color}`],
-      hasBackground && [styles['Icon--background'], styles[`Icon--background-color-${color}`]],
-      className,
-    ),
     size: iconSize,
     ...rest,
   }
 
-  /**
-   * Ensures that instantiated JSX can continue to be passed as props
-   */
-  if (React.isValidElement(Octicon)) {
-    return React.cloneElement(Octicon as React.ReactElement<OcticonProps>, {
+  const iconComponent = React.isValidElement(Octicon) ? (
+    React.cloneElement(Octicon as ReactElement<OcticonProps>, {
       ...Octicon.props,
       ...iconProps,
     })
-  }
+  ) : (
+    <Octicon {...iconProps} />
+  )
 
-  return <Octicon {...iconProps} />
+  return (
+    <div
+      className={clsx(
+        styles['Icon'],
+        styles[`Icon--size-${iconSize}`],
+        styles[`Icon--color-${color}`],
+        hasBackground && [styles['Icon--background'], styles[`Icon--background-color-${color}`]],
+        className,
+      )}
+    >
+      {iconComponent}
+    </div>
+  )
 }