refactor(octicons_react): add support for className, use presentation…

…al attributes over inline styles (#1037) * Concat className with default * join space * update snap * tests * Refactor className concatenation method * Set default value for className variable * Update snapshot size in tree-shaking test * Move to logical properties and deprecate verticalAlign * Fix verticalAlign style property formatting * test: update snapshots, add support for style * chore: update snapshots * chore: add changeset --------- Co-authored-by: Katie Langerman <[email protected]> Co-authored-by: Josh Black <[email protected]>
primer · Dec 9, 2024 · 7402e69 · 7402e69
1 parent 4819803
commit 7402e69
Show file tree

Hide file tree

Showing 6 changed files with 15 additions and 8 deletions.
diff --git a/.changeset/brown-buttons-search.md b/.changeset/brown-buttons-search.md
@@ -0,0 +1,5 @@
+---
+'@primer/octicons': minor
+---
+
+Update octicons-react to use presentational attributes over inline styles for base styles
diff --git a/lib/octicons_react/__tests__/tree-shaking.test.js b/lib/octicons_react/__tests__/tree-shaking.test.js
@@ -50,5 +50,5 @@ test('tree shaking single export', async () => {
   })
 
   const bundleSize = Buffer.byteLength(output[0].code.trim()) / 1000
-  expect(`${bundleSize}kB`).toMatchInlineSnapshot(`"6.29kB"`)
+  expect(`${bundleSize}kB`).toMatchInlineSnapshot(`"6.292kB"`)
 })
diff --git a/lib/octicons_react/src/__tests__/__snapshots__/octicon.js.snap b/lib/octicons_react/src/__tests__/__snapshots__/octicon.js.snap
@@ -4,10 +4,12 @@ exports[`An icon component matches snapshot 1`] = `
 <svg
   aria-hidden="true"
   class="octicon octicon-alert"
+  display="inline-block"
   fill="currentColor"
   focusable="false"
   height="16"
-  style="display: inline-block; user-select: none; vertical-align: text-bottom; overflow: visible;"
+  overflow="visible"
+  style="vertical-align: text-bottom;"
   viewBox="0 0 16 16"
   width="16"
 >

diff --git a/lib/octicons_react/src/__tests__/octicon.js b/lib/octicons_react/src/__tests__/octicon.js
@@ -54,7 +54,7 @@ describe('An icon component', () => {
 
   it('respects the className prop', () => {
     const {container} = render(<AlertIcon className="foo" />)
-    expect(container.querySelector('svg')).toHaveAttribute('class', 'foo')
+    expect(container.querySelector('svg')).toHaveAttribute('class', 'octicon octicon-alert foo')
   })
 
   it('respects the fill prop', () => {

diff --git a/lib/octicons_react/src/createIconComponent.js b/lib/octicons_react/src/createIconComponent.js
@@ -16,7 +16,7 @@ export function createIconComponent(name, defaultClassName, getSVGData) {
         'aria-label': ariaLabel,
         'aria-labelledby': arialabelledby,
         tabIndex,
-        className = defaultClassName,
+        className = '',
         fill = 'currentColor',
         size = 16,
         verticalAlign = 'text-bottom',
@@ -44,18 +44,17 @@ export function createIconComponent(name, defaultClassName, getSVGData) {
           focusable={tabIndex >= 0 ? 'true' : 'false'}
           aria-label={ariaLabel}
           aria-labelledby={arialabelledby}
-          className={className}
+          className={`${defaultClassName} ${className}`.trim()}
           role={role}
           viewBox={`0 0 ${naturalWidth} ${naturalHeight}`}
           width={width}
           height={height}
           fill={fill}
           id={id}
+          display="inline-block"
+          overflow="visible"
           style={{
-            display: 'inline-block',
-            userSelect: 'none',
             verticalAlign,
-            overflow: 'visible',
             ...style
           }}
         >

diff --git a/lib/octicons_react/src/index.d.ts b/lib/octicons_react/src/index.d.ts
@@ -17,6 +17,7 @@ export interface OcticonProps extends React.ComponentPropsWithoutRef<'svg'> {
   fill?: string
   icon?: Icon | React.ReactNode
   size?: number | Size
+  /** @deprecated use v-align utilities instead */
   verticalAlign?: 'middle' | 'text-bottom' | 'text-top' | 'top' | 'unset'
 }