From 3fc86695ce294897eb702e3b8f1d55c18c214dff Mon Sep 17 00:00:00 2001 From: Adam Rasheed Date: Wed, 10 Dec 2025 20:22:38 -0800 Subject: [PATCH 1/9] [LG-5766] fix: Icon fill support --- packages/icon/src/Icon.spec.tsx | 11 +++++++++++ packages/icon/src/createIconComponent.tsx | 4 ++-- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/packages/icon/src/Icon.spec.tsx b/packages/icon/src/Icon.spec.tsx index 30246f9971..1e229c849d 100644 --- a/packages/icon/src/Icon.spec.tsx +++ b/packages/icon/src/Icon.spec.tsx @@ -9,6 +9,7 @@ import { typeIs } from '@leafygreen-ui/lib'; import EditIcon from './generated/Edit'; import { Size } from './glyphCommon'; +import { Icon } from './Icon'; import { isComponentGlyph } from './isComponentGlyph'; import { SVGR } from './types'; import { createGlyphComponent, createIconComponent, glyphs } from '.'; @@ -245,6 +246,16 @@ describe('packages/Icon/createIconComponent', () => { const glyph = getByTestId('my-glyph'); expect(glyph).toHaveAttribute('role', 'presentation'); }); + + test('`fill` prop applies CSS color correctly', () => { + const { container } = render(); + const svg = container.querySelector('svg'); + expect(svg).toBeTruthy(); + // The fill prop should be applied as a CSS color via emotion + // We check that the SVG has a className (from emotion) and the computed style + const computedStyle = window.getComputedStyle(svg!); + expect(computedStyle.color).toBe('red'); + }); }); test('returned Icon function logs an error when glyph does not exist', () => { diff --git a/packages/icon/src/createIconComponent.tsx b/packages/icon/src/createIconComponent.tsx index 7c76f7e53c..057f25fabf 100644 --- a/packages/icon/src/createIconComponent.tsx +++ b/packages/icon/src/createIconComponent.tsx @@ -27,11 +27,11 @@ type GlyphObject = Record; export function createIconComponent( glyphs: G, ) { - const Icon = ({ glyph, ...rest }: IconProps) => { + const Icon = ({ glyph, fill, ...rest }: IconProps) => { const SVGComponent = glyphs[glyph]; if (SVGComponent) { - return ; + return ; } else { // TODO: improve fuzzy match // Suggest the proper icon casing if there's a near match From d30a4d668f7e854be90d29ba58cf78932917297c Mon Sep 17 00:00:00 2001 From: Adam Rasheed Date: Wed, 10 Dec 2025 22:59:58 -0800 Subject: [PATCH 2/9] cleanup test --- packages/icon/src/Icon.spec.tsx | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/packages/icon/src/Icon.spec.tsx b/packages/icon/src/Icon.spec.tsx index 1e229c849d..b04965dd36 100644 --- a/packages/icon/src/Icon.spec.tsx +++ b/packages/icon/src/Icon.spec.tsx @@ -246,16 +246,6 @@ describe('packages/Icon/createIconComponent', () => { const glyph = getByTestId('my-glyph'); expect(glyph).toHaveAttribute('role', 'presentation'); }); - - test('`fill` prop applies CSS color correctly', () => { - const { container } = render(); - const svg = container.querySelector('svg'); - expect(svg).toBeTruthy(); - // The fill prop should be applied as a CSS color via emotion - // We check that the SVG has a className (from emotion) and the computed style - const computedStyle = window.getComputedStyle(svg!); - expect(computedStyle.color).toBe('red'); - }); }); test('returned Icon function logs an error when glyph does not exist', () => { @@ -267,6 +257,18 @@ describe('packages/Icon/createIconComponent', () => { }); }); +describe('packages/Icon/Icon', () => { + test('`fill` prop applies CSS color correctly', () => { + const { container } = render(); + const svg = container.querySelector('svg'); + expect(svg).toBeTruthy(); + // The fill prop should be applied as a CSS color via emotion + // We check that the computed style has the correct color + const computedStyle = window.getComputedStyle(svg!); + expect(computedStyle.color).toBe('red'); + }); +}); + describe('Generated glyphs', () => { test('Edit icon has displayName: "Edit"', () => { expect(EditIcon.displayName).toBe('Edit'); From 88a510f89be176cf21e63631f67a8c98b098f16c Mon Sep 17 00:00:00 2001 From: Adam Rasheed Date: Wed, 10 Dec 2025 23:01:25 -0800 Subject: [PATCH 3/9] more cleanup --- packages/icon/src/createIconComponent.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/icon/src/createIconComponent.tsx b/packages/icon/src/createIconComponent.tsx index 057f25fabf..c12b660a58 100644 --- a/packages/icon/src/createIconComponent.tsx +++ b/packages/icon/src/createIconComponent.tsx @@ -31,7 +31,7 @@ export function createIconComponent( const SVGComponent = glyphs[glyph]; if (SVGComponent) { - return ; + return ; } else { // TODO: improve fuzzy match // Suggest the proper icon casing if there's a near match From 24253c60173839869d8c600f02af1c72e74c2408 Mon Sep 17 00:00:00 2001 From: Adam Rasheed Date: Thu, 11 Dec 2025 09:28:56 -0800 Subject: [PATCH 4/9] added changeset --- .changeset/polite-turkeys-run.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/polite-turkeys-run.md diff --git a/.changeset/polite-turkeys-run.md b/.changeset/polite-turkeys-run.md new file mode 100644 index 0000000000..1157293e2e --- /dev/null +++ b/.changeset/polite-turkeys-run.md @@ -0,0 +1,5 @@ +--- +'@leafygreen-ui/icon': patch +--- + +Fixed an issue where icons generated through createIconComponent were not passing in fill value correctly From bdb66a3976574b251f7cf6ae0aae428c7e920e00 Mon Sep 17 00:00:00 2001 From: Adam Rasheed Date: Thu, 11 Dec 2025 17:37:33 -0800 Subject: [PATCH 5/9] updated createIconComponent, added tests, added story --- packages/icon/src/Icon.stories.tsx | 24 +- .../icon/src/createIconComponent.spec.tsx | 486 ++++++++++++++++++ packages/icon/src/createIconComponent.tsx | 16 +- 3 files changed, 523 insertions(+), 3 deletions(-) create mode 100644 packages/icon/src/createIconComponent.spec.tsx diff --git a/packages/icon/src/Icon.stories.tsx b/packages/icon/src/Icon.stories.tsx index aebdfbbd6e..c235a3751c 100644 --- a/packages/icon/src/Icon.stories.tsx +++ b/packages/icon/src/Icon.stories.tsx @@ -9,7 +9,13 @@ import { css } from '@leafygreen-ui/emotion'; import { palette } from '@leafygreen-ui/palette'; import { GlyphName } from './glyphs'; -import Icon, { glyphs, IconProps, Size } from '.'; +import Icon, { + createGlyphComponent, + createIconComponent, + glyphs, + IconProps, + Size, +} from '.'; const meta: StoryMetaType = { title: 'Components/Display/Icon', @@ -109,6 +115,22 @@ export const LiveExample: StoryObj = { ), }; +const customGlyphs = { + CustomGlyph: createGlyphComponent('CustomGlyph', props => ), +}; + +export const Custom: StoryObj = { + parameters: { + controls: { + exclude: [...meta.parameters.controls!.exclude!, 'glyph'], + }, + }, + render: (args: Omit) => { + const IconComponent = createIconComponent(customGlyphs); + return ; + }, +}; + export const Generated: StoryObj = { parameters: { generate: { diff --git a/packages/icon/src/createIconComponent.spec.tsx b/packages/icon/src/createIconComponent.spec.tsx new file mode 100644 index 0000000000..90097e174e --- /dev/null +++ b/packages/icon/src/createIconComponent.spec.tsx @@ -0,0 +1,486 @@ +import React from 'react'; +import { render, screen } from '@testing-library/react'; + +import { createGlyphComponent } from './createGlyphComponent'; +import { createIconComponent } from './createIconComponent'; +import * as generatedGlyphs from './generated'; +import { Size } from './glyphCommon'; +import { isComponentGlyph } from './isComponentGlyph'; +import { SVGR } from './types'; + +// Mock SVGR component representing a custom SVG +const CustomSVGRGlyph: SVGR.Component = ({ children, ...props }) => ( + + {children} + +); + +// Another mock glyph for multiple glyph testing +const AnotherCustomGlyph: SVGR.Component = ({ children, ...props }) => ( + + {children} + +); + +// Create glyph components from the SVGR components +const customGlyphs = { + CustomGlyph: createGlyphComponent('CustomGlyph', CustomSVGRGlyph), + AnotherGlyph: createGlyphComponent('AnotherGlyph', AnotherCustomGlyph), +}; + +describe('packages/Icon/createIconComponent', () => { + describe('basic functionality', () => { + const IconComponent = createIconComponent(customGlyphs); + + test('returns a function', () => { + expect(typeof IconComponent).toBe('function'); + }); + + test('returned function has the displayName: "Icon"', () => { + expect(IconComponent.displayName).toBe('Icon'); + }); + + test('returned function has the property: `isGlyph`', () => { + expect(IconComponent).toHaveProperty('isGlyph'); + expect(IconComponent.isGlyph).toBeTruthy(); + }); + + test('returned function passes `isComponentGlyph`', () => { + expect(isComponentGlyph(IconComponent)).toBeTruthy(); + }); + }); + + describe('rendering glyphs', () => { + const IconComponent = createIconComponent(customGlyphs); + + test('renders the correct glyph when passed a valid glyph name', () => { + render(); + const glyph = screen.getByTestId('custom-svgr-glyph'); + expect(glyph).toBeInTheDocument(); + expect(glyph.nodeName.toLowerCase()).toBe('svg'); + }); + + test('renders different glyphs based on glyph prop', () => { + const { rerender } = render(); + expect(screen.getByTestId('custom-svgr-glyph')).toBeInTheDocument(); + + rerender(); + expect(screen.getByTestId('another-custom-glyph')).toBeInTheDocument(); + }); + + test('logs an error and renders nothing when glyph does not exist', () => { + const consoleSpy = jest + .spyOn(console, 'error') + .mockImplementation(() => {}); + + const { container } = render(); + + // Should log an error + expect(consoleSpy).toHaveBeenCalledWith( + 'Error in Icon', + 'Could not find glyph named "NonExistentGlyph" in the icon set.', + undefined, + ); + + // Should not render an SVG + const svg = container.querySelector('svg'); + expect(svg).not.toBeInTheDocument(); + + consoleSpy.mockRestore(); + }); + + test('suggests near match when glyph name has incorrect casing', () => { + const consoleSpy = jest + .spyOn(console, 'error') + .mockImplementation(() => {}); + + render(); + + expect(consoleSpy).toHaveBeenCalledWith( + 'Error in Icon', + 'Could not find glyph named "custom-glyph" in the icon set.', + 'Did you mean "CustomGlyph?"', + ); + + consoleSpy.mockRestore(); + }); + }); + + describe('custom SVG support', () => { + // Create an Icon component that uses raw SVGR components (custom SVGs) + const RawSVGGlyph: SVGR.Component = props => ( + + ); + + const customSVGGlyphs = { + RawSVG: createGlyphComponent('RawSVG', RawSVGGlyph), + }; + + const IconComponent = createIconComponent(customSVGGlyphs); + + test('renders custom SVG components correctly', () => { + render(); + const glyph = screen.getByTestId('raw-svg-glyph'); + expect(glyph).toBeInTheDocument(); + expect(glyph.nodeName.toLowerCase()).toBe('svg'); + }); + + test('applies size prop to custom SVGs', () => { + render(); + const glyph = screen.getByTestId('raw-svg-glyph'); + expect(glyph).toHaveAttribute('height', '32'); + expect(glyph).toHaveAttribute('width', '32'); + }); + + test('applies Size enum to custom SVGs', () => { + render(); + const glyph = screen.getByTestId('raw-svg-glyph'); + expect(glyph).toHaveAttribute('height', '20'); + expect(glyph).toHaveAttribute('width', '20'); + }); + }); + + describe('title prop with generated glyphs', () => { + // Use real generated glyphs to test title functionality + const IconComponent = createIconComponent(generatedGlyphs); + + test('renders a title element when title prop is provided', () => { + render(); + const glyph = screen.getByRole('img'); + const titleElement = glyph.querySelector('title'); + expect(titleElement).toBeInTheDocument(); + expect(titleElement?.textContent).toBe('Edit Icon Title'); + }); + + test('sets aria-labelledby to title element ID when title is provided', () => { + render(); + const glyph = screen.getByRole('img'); + const titleElement = glyph.querySelector('title'); + const ariaLabelledBy = glyph.getAttribute('aria-labelledby'); + expect(ariaLabelledBy).toBe(titleElement?.id); + }); + + test('combines title ID with aria-labelledby when both are provided', () => { + render( + , + ); + const glyph = screen.getByRole('img'); + const titleElement = glyph.querySelector('title'); + const ariaLabelledBy = glyph.getAttribute('aria-labelledby'); + expect(ariaLabelledBy).toContain('external-label'); + expect(ariaLabelledBy).toBe(`${titleElement?.id} external-label`); + }); + + test('does not render title when title prop is not provided', () => { + render(); + const glyph = screen.getByRole('img'); + const titleElement = glyph.querySelector('title'); + expect(titleElement).not.toBeInTheDocument(); + }); + }); + + describe('title prop with custom SVGR glyphs', () => { + // Custom SVGR component that renders title similar to generated glyphs + const CustomGlyphWithTitle: SVGR.Component = ({ children, ...props }) => ( + + {children} + + ); + + const customGlyphsWithTitle = { + CustomWithTitle: createGlyphComponent( + 'CustomWithTitle', + CustomGlyphWithTitle, + ), + }; + + const IconComponent = createIconComponent(customGlyphsWithTitle); + + test('passes title through createGlyphComponent which sets aria-labelledby', () => { + render(); + const glyph = screen.getByTestId('custom-glyph-with-title'); + // createGlyphComponent sets aria-labelledby when title is provided + const ariaLabelledBy = glyph.getAttribute('aria-labelledby'); + expect(ariaLabelledBy).not.toBeNull(); + expect(ariaLabelledBy).toContain('icon-title'); + }); + + test('sets aria-label when no title or aria-labelledby provided', () => { + render(); + const glyph = screen.getByTestId('custom-glyph-with-title'); + // Default aria-label is generated from glyph name + expect(glyph).toHaveAttribute('aria-label', 'Custom With Title Icon'); + }); + }); + + describe('className prop', () => { + const IconComponent = createIconComponent(customGlyphs); + + test('applies className to glyph SVG element', () => { + render(); + const glyph = screen.getByTestId('custom-svgr-glyph'); + expect(glyph).toHaveClass('custom-class'); + }); + + test('applies multiple classNames to glyph SVG element', () => { + render( + , + ); + const glyph = screen.getByTestId('custom-svgr-glyph'); + expect(glyph).toHaveClass('class-one'); + expect(glyph).toHaveClass('class-two'); + }); + + test('applies className alongside fill style', () => { + render( + , + ); + const glyph = screen.getByTestId('custom-svgr-glyph'); + expect(glyph).toHaveClass('custom-class'); + // fill applies a CSS class for the color style + const classList = Array.from(glyph.classList); + expect(classList.length).toBeGreaterThan(1); + }); + }); + + describe('className prop with generated glyphs', () => { + const IconComponent = createIconComponent(generatedGlyphs); + + test('applies className to generated glyph SVG element', () => { + render(); + const glyph = screen.getByRole('img'); + expect(glyph).toHaveClass('generated-glyph-class'); + }); + + test('applies multiple classNames to generated glyph', () => { + render(); + const glyph = screen.getByRole('img'); + expect(glyph).toHaveClass('class-one'); + expect(glyph).toHaveClass('class-two'); + }); + }); + + describe('className prop with custom SVGs', () => { + const RawSVGGlyph: SVGR.Component = props => ( + + ); + + const customSVGGlyphs = { + RawSVG: createGlyphComponent('RawSVG', RawSVGGlyph), + }; + + const IconComponent = createIconComponent(customSVGGlyphs); + + test('applies className to custom SVG components', () => { + render(); + const glyph = screen.getByTestId('raw-svg-for-class'); + expect(glyph).toHaveClass('my-custom-class'); + }); + }); + + describe('size prop', () => { + const IconComponent = createIconComponent(customGlyphs); + + test('applies numeric size to glyph', () => { + render(); + const glyph = screen.getByTestId('custom-svgr-glyph'); + expect(glyph).toHaveAttribute('height', '24'); + expect(glyph).toHaveAttribute('width', '24'); + }); + + test('applies Size.Small correctly', () => { + render(); + const glyph = screen.getByTestId('custom-svgr-glyph'); + expect(glyph).toHaveAttribute('height', '14'); + expect(glyph).toHaveAttribute('width', '14'); + }); + + test('applies Size.Default correctly', () => { + render(); + const glyph = screen.getByTestId('custom-svgr-glyph'); + expect(glyph).toHaveAttribute('height', '16'); + expect(glyph).toHaveAttribute('width', '16'); + }); + + test('applies Size.Large correctly', () => { + render(); + const glyph = screen.getByTestId('custom-svgr-glyph'); + expect(glyph).toHaveAttribute('height', '20'); + expect(glyph).toHaveAttribute('width', '20'); + }); + + test('applies Size.XLarge correctly', () => { + render(); + const glyph = screen.getByTestId('custom-svgr-glyph'); + expect(glyph).toHaveAttribute('height', '24'); + expect(glyph).toHaveAttribute('width', '24'); + }); + + test('uses default size when size prop is not provided', () => { + render(); + const glyph = screen.getByTestId('custom-svgr-glyph'); + expect(glyph).toHaveAttribute('height', '16'); + expect(glyph).toHaveAttribute('width', '16'); + }); + }); + + describe('accessibility props', () => { + const IconComponent = createIconComponent(customGlyphs); + + test('applies role="img" by default', () => { + render(); + const glyph = screen.getByTestId('custom-svgr-glyph'); + expect(glyph).toHaveAttribute('role', 'img'); + }); + + test('applies role="presentation" when specified', () => { + render(); + const glyph = screen.getByTestId('custom-svgr-glyph'); + expect(glyph).toHaveAttribute('role', 'presentation'); + expect(glyph).toHaveAttribute('aria-hidden', 'true'); + }); + + test('generates default aria-label when no accessibility props provided', () => { + render(); + const glyph = screen.getByTestId('custom-svgr-glyph'); + expect(glyph).toHaveAttribute('aria-label', 'Custom Glyph Icon'); + }); + + test('applies custom aria-label when provided', () => { + render( + , + ); + const glyph = screen.getByTestId('custom-svgr-glyph'); + expect(glyph).toHaveAttribute('aria-label', 'My Custom Label'); + }); + + test('applies aria-labelledby when provided', () => { + render( + , + ); + const glyph = screen.getByTestId('custom-svgr-glyph'); + expect(glyph).toHaveAttribute('aria-labelledby', 'my-label-id'); + }); + }); + + describe('fill prop', () => { + const IconComponent = createIconComponent(customGlyphs); + + test('applies fill as CSS color', () => { + render(); + const glyph = screen.getByTestId('custom-svgr-glyph'); + const computedStyle = window.getComputedStyle(glyph); + expect(computedStyle.color).toBe('red'); + }); + + test('applies fill alongside className', () => { + render( + , + ); + const glyph = screen.getByTestId('custom-svgr-glyph'); + expect(glyph).toHaveClass('custom-class'); + const computedStyle = window.getComputedStyle(glyph); + expect(computedStyle.color).toBe('blue'); + }); + }); + + describe('fill prop with generated glyphs', () => { + const IconComponent = createIconComponent(generatedGlyphs); + + test('applies fill as CSS color to generated glyph', () => { + render(); + const glyph = screen.getByRole('img'); + const computedStyle = window.getComputedStyle(glyph); + expect(computedStyle.color).toBe('purple'); + }); + }); + + describe('combined props with generated glyphs', () => { + const IconComponent = createIconComponent(generatedGlyphs); + + test('applies all props correctly together', () => { + render( + , + ); + const glyph = screen.getByRole('img'); + + // Check size + expect(glyph).toHaveAttribute('height', '24'); + expect(glyph).toHaveAttribute('width', '24'); + + // Check className + expect(glyph).toHaveClass('combined-class'); + + // Check title + const titleElement = glyph.querySelector('title'); + expect(titleElement).toBeInTheDocument(); + expect(titleElement?.textContent).toBe('Combined Title'); + + // Check aria-labelledby points to title + expect(glyph.getAttribute('aria-labelledby')).toBe(titleElement?.id); + + // Check fill (as CSS color) + const computedStyle = window.getComputedStyle(glyph); + expect(computedStyle.color).toBe('green'); + }); + }); + + describe('combined props with custom SVGR glyphs', () => { + const IconComponent = createIconComponent(customGlyphs); + + test('applies size, className, and fill together', () => { + render( + , + ); + const glyph = screen.getByTestId('custom-svgr-glyph'); + + // Check size + expect(glyph).toHaveAttribute('height', '32'); + expect(glyph).toHaveAttribute('width', '32'); + + // Check className + expect(glyph).toHaveClass('combined-custom-class'); + + // Check fill (as CSS color) + const computedStyle = window.getComputedStyle(glyph); + expect(computedStyle.color).toBe('orange'); + }); + + test('applies accessibility props with className', () => { + render( + , + ); + const glyph = screen.getByTestId('custom-svgr-glyph'); + + expect(glyph).toHaveClass('accessible-class'); + expect(glyph).toHaveAttribute('aria-label', 'Accessible Custom Icon'); + }); + }); +}); diff --git a/packages/icon/src/createIconComponent.tsx b/packages/icon/src/createIconComponent.tsx index c12b660a58..9be9273c8b 100644 --- a/packages/icon/src/createIconComponent.tsx +++ b/packages/icon/src/createIconComponent.tsx @@ -1,6 +1,8 @@ import React from 'react'; import kebabCase from 'lodash/kebabCase'; +import mapValues from 'lodash/mapValues'; +import { createGlyphComponent } from './createGlyphComponent'; import { Size } from './glyphCommon'; import { LGGlyph } from './types'; @@ -27,12 +29,22 @@ type GlyphObject = Record; export function createIconComponent( glyphs: G, ) { - const Icon = ({ glyph, fill, ...rest }: IconProps) => { + const Icon = ({ glyph, ...rest }: IconProps) => { const SVGComponent = glyphs[glyph]; if (SVGComponent) { - return ; + return ; } else { + const generatedGlyphs = mapValues(glyphs, (val, key) => { + return createGlyphComponent(key, val); + }); + + const GeneratedSVGComponent = generatedGlyphs[glyph]; + + if (GeneratedSVGComponent) { + return ; + } + // TODO: improve fuzzy match // Suggest the proper icon casing if there's a near match const nearMatch = Object.keys(glyphs).find( From 996ee977093222f908392305ca8a489d4a55e1bf Mon Sep 17 00:00:00 2001 From: Adam Rasheed Date: Fri, 12 Dec 2025 11:04:17 -0800 Subject: [PATCH 6/9] updated createIconComponent, added tests --- .../icon/src/createGlyphComponent.spec.tsx | 295 ++++++++++++++++++ .../icon/src/createIconComponent.spec.tsx | 92 ++---- packages/icon/src/createIconComponent.tsx | 24 +- packages/icon/src/testUtils.tsx | 52 +++ 4 files changed, 386 insertions(+), 77 deletions(-) create mode 100644 packages/icon/src/createGlyphComponent.spec.tsx create mode 100644 packages/icon/src/testUtils.tsx diff --git a/packages/icon/src/createGlyphComponent.spec.tsx b/packages/icon/src/createGlyphComponent.spec.tsx new file mode 100644 index 0000000000..6694ac5c2d --- /dev/null +++ b/packages/icon/src/createGlyphComponent.spec.tsx @@ -0,0 +1,295 @@ +import React from 'react'; +import { render, screen } from '@testing-library/react'; + +import { createGlyphComponent } from './createGlyphComponent'; +import { Size } from './glyphCommon'; +import { isComponentGlyph } from './isComponentGlyph'; +import { + expectFillColor, + expectSize, + MockSVGRGlyph, + MockSVGRGlyphWithChildren, +} from './testUtils'; + +describe('packages/Icon/createGlyphComponent', () => { + describe('basic functionality', () => { + const GlyphComponent = createGlyphComponent('TestGlyph', MockSVGRGlyph); + + test('returns a function', () => { + expect(typeof GlyphComponent).toBe('function'); + }); + + test('returned component has the correct displayName', () => { + expect(GlyphComponent.displayName).toBe('TestGlyph'); + }); + + test('returned component has the property `isGlyph`', () => { + expect(GlyphComponent).toHaveProperty('isGlyph'); + expect(GlyphComponent.isGlyph).toBe(true); + }); + + test('returned component passes `isComponentGlyph`', () => { + expect(isComponentGlyph(GlyphComponent)).toBe(true); + }); + }); + + describe('rendering', () => { + const GlyphComponent = createGlyphComponent('TestGlyph', MockSVGRGlyph); + + test('renders an SVG element', () => { + render(); + const glyph = screen.getByTestId('mock-glyph'); + expect(glyph).toBeInTheDocument(); + expect(glyph.nodeName.toLowerCase()).toBe('svg'); + }); + + test('passes through additional props to the SVG element', () => { + render(); + const glyph = screen.getByTestId('mock-glyph'); + expect(glyph).toHaveAttribute('data-custom', 'custom-value'); + }); + }); + + describe('size prop', () => { + const GlyphComponent = createGlyphComponent('TestGlyph', MockSVGRGlyph); + + test('applies numeric size to height and width', () => { + render(); + const glyph = screen.getByTestId('mock-glyph'); + expectSize(glyph, '24'); + }); + + test('applies Size.Small correctly (14px)', () => { + render(); + const glyph = screen.getByTestId('mock-glyph'); + expectSize(glyph, '14'); + }); + + test('applies Size.Default correctly (16px)', () => { + render(); + const glyph = screen.getByTestId('mock-glyph'); + expectSize(glyph, '16'); + }); + + test('applies Size.Large correctly (20px)', () => { + render(); + const glyph = screen.getByTestId('mock-glyph'); + expectSize(glyph, '20'); + }); + + test('applies Size.XLarge correctly (24px)', () => { + render(); + const glyph = screen.getByTestId('mock-glyph'); + expectSize(glyph, '24'); + }); + + test('uses Size.Default (16px) when size prop is not provided', () => { + render(); + const glyph = screen.getByTestId('mock-glyph'); + expectSize(glyph, '16'); + }); + }); + + describe('fill prop', () => { + const GlyphComponent = createGlyphComponent('TestGlyph', MockSVGRGlyph); + + test('applies fill as CSS color', () => { + render(); + const glyph = screen.getByTestId('mock-glyph'); + expectFillColor(glyph, 'red'); + }); + + test('does not apply fill style when fill is not provided', () => { + render(); + const glyph = screen.getByTestId('mock-glyph'); + // When no fill is provided, no fill-related class should be applied + // The glyph should still render without error + expect(glyph).toBeInTheDocument(); + }); + + test('applies fill alongside other props', () => { + render(); + const glyph = screen.getByTestId('mock-glyph'); + expect(glyph).toHaveClass('custom-class'); + expectSize(glyph, '32'); + expectFillColor(glyph, 'blue'); + }); + }); + + describe('className prop', () => { + const GlyphComponent = createGlyphComponent('TestGlyph', MockSVGRGlyph); + + test('applies className to the SVG element', () => { + render(); + const glyph = screen.getByTestId('mock-glyph'); + expect(glyph).toHaveClass('my-custom-class'); + }); + + test('applies multiple classNames to the SVG element', () => { + render(); + const glyph = screen.getByTestId('mock-glyph'); + expect(glyph).toHaveClass('class-one'); + expect(glyph).toHaveClass('class-two'); + }); + + test('applies className alongside fill style', () => { + render(); + const glyph = screen.getByTestId('mock-glyph'); + expect(glyph).toHaveClass('custom-class'); + // fill applies a CSS class for the color style + const classList = Array.from(glyph.classList); + expect(classList.length).toBeGreaterThan(1); + }); + }); + + describe('role prop', () => { + const GlyphComponent = createGlyphComponent('TestGlyph', MockSVGRGlyph); + + test('applies role="img" by default', () => { + render(); + const glyph = screen.getByTestId('mock-glyph'); + expect(glyph).toHaveAttribute('role', 'img'); + }); + + test('applies role="presentation" when specified', () => { + render(); + const glyph = screen.getByTestId('mock-glyph'); + expect(glyph).toHaveAttribute('role', 'presentation'); + expect(glyph).toHaveAttribute('aria-hidden', 'true'); + }); + + test('logs a warning when an invalid role is provided', () => { + const consoleSpy = jest + .spyOn(console, 'warn') + .mockImplementation(() => {}); + + // @ts-expect-error - intentionally passing invalid role for testing + // eslint-disable-next-line jsx-a11y/aria-role + render(); + + expect(consoleSpy).toHaveBeenCalledWith( + "Please provide a valid role to this component. Valid options are 'img' and 'presentation'. If you'd like the Icon to be accessible to screen readers please use 'img', otherwise set the role to 'presentation'.", + ); + + consoleSpy.mockRestore(); + }); + }); + + describe('accessibility props', () => { + const GlyphComponent = createGlyphComponent('TestGlyph', MockSVGRGlyph); + + test('generates default aria-label from glyph name when no accessibility props provided', () => { + render(); + const glyph = screen.getByTestId('mock-glyph'); + expect(glyph).toHaveAttribute('aria-label', 'Test Glyph Icon'); + }); + + test('applies custom aria-label when provided', () => { + render(); + const glyph = screen.getByTestId('mock-glyph'); + expect(glyph).toHaveAttribute('aria-label', 'My Custom Label'); + }); + + test('applies aria-labelledby when provided', () => { + render(); + const glyph = screen.getByTestId('mock-glyph'); + expect(glyph).toHaveAttribute('aria-labelledby', 'my-label-id'); + }); + + test('sets aria-labelledby when title is provided', () => { + render(); + const glyph = screen.getByTestId('mock-glyph'); + const ariaLabelledBy = glyph.getAttribute('aria-labelledby'); + expect(ariaLabelledBy).not.toBeNull(); + expect(ariaLabelledBy).toContain('icon-title'); + }); + + test('combines title ID with aria-labelledby when both are provided', () => { + render( + , + ); + const glyph = screen.getByTestId('mock-glyph'); + const ariaLabelledBy = glyph.getAttribute('aria-labelledby'); + expect(ariaLabelledBy).toContain('external-label'); + expect(ariaLabelledBy).toContain('icon-title'); + }); + + test('sets aria-hidden to true when role is presentation', () => { + render(); + const glyph = screen.getByTestId('mock-glyph'); + expect(glyph).toHaveAttribute('aria-hidden', 'true'); + }); + }); + + describe('title prop', () => { + const GlyphComponent = createGlyphComponent( + 'TestGlyph', + MockSVGRGlyphWithChildren, + ); + + test('does not include title in children when title is not provided', () => { + render(); + const glyph = screen.getByTestId('mock-glyph-with-children'); + const titleElement = glyph.querySelector('title'); + expect(titleElement).not.toBeInTheDocument(); + }); + }); + + describe('combined props', () => { + const GlyphComponent = createGlyphComponent('TestGlyph', MockSVGRGlyph); + + test('applies all props correctly together', () => { + render( + , + ); + const glyph = screen.getByTestId('mock-glyph'); + + expectSize(glyph, '32'); + expect(glyph).toHaveClass('combined-class'); + expect(glyph).toHaveAttribute('aria-label', 'Combined Icon'); + expectFillColor(glyph, 'purple'); + }); + + test('applies size enum with className and role', () => { + render( + , + ); + const glyph = screen.getByTestId('mock-glyph'); + + expectSize(glyph, '20'); + expect(glyph).toHaveClass('accessible-class'); + expect(glyph).toHaveAttribute('role', 'presentation'); + expect(glyph).toHaveAttribute('aria-hidden', 'true'); + }); + }); + + describe('different glyph names', () => { + test('handles PascalCase glyph names correctly', () => { + const GlyphComponent = createGlyphComponent( + 'MyCustomGlyph', + MockSVGRGlyph, + ); + expect(GlyphComponent.displayName).toBe('MyCustomGlyph'); + render(); + const glyph = screen.getByTestId('mock-glyph'); + expect(glyph).toHaveAttribute('aria-label', 'My Custom Glyph Icon'); + }); + + test('handles single word glyph names correctly', () => { + const GlyphComponent = createGlyphComponent('Edit', MockSVGRGlyph); + expect(GlyphComponent.displayName).toBe('Edit'); + render(); + const glyph = screen.getByTestId('mock-glyph'); + expect(glyph).toHaveAttribute('aria-label', 'Edit Icon'); + }); + }); +}); diff --git a/packages/icon/src/createIconComponent.spec.tsx b/packages/icon/src/createIconComponent.spec.tsx index 90097e174e..15a775cf79 100644 --- a/packages/icon/src/createIconComponent.spec.tsx +++ b/packages/icon/src/createIconComponent.spec.tsx @@ -6,21 +6,13 @@ import { createIconComponent } from './createIconComponent'; import * as generatedGlyphs from './generated'; import { Size } from './glyphCommon'; import { isComponentGlyph } from './isComponentGlyph'; -import { SVGR } from './types'; - -// Mock SVGR component representing a custom SVG -const CustomSVGRGlyph: SVGR.Component = ({ children, ...props }) => ( - - {children} - -); - -// Another mock glyph for multiple glyph testing -const AnotherCustomGlyph: SVGR.Component = ({ children, ...props }) => ( - - {children} - -); +import { + AnotherCustomGlyph, + createMockSVGRComponent, + CustomSVGRGlyph, + expectFillColor, + expectSize, +} from './testUtils'; // Create glyph components from the SVGR components const customGlyphs = { @@ -107,10 +99,7 @@ describe('packages/Icon/createIconComponent', () => { }); describe('custom SVG support', () => { - // Create an Icon component that uses raw SVGR components (custom SVGs) - const RawSVGGlyph: SVGR.Component = props => ( - - ); + const RawSVGGlyph = createMockSVGRComponent('raw-svg-glyph'); const customSVGGlyphs = { RawSVG: createGlyphComponent('RawSVG', RawSVGGlyph), @@ -128,15 +117,13 @@ describe('packages/Icon/createIconComponent', () => { test('applies size prop to custom SVGs', () => { render(); const glyph = screen.getByTestId('raw-svg-glyph'); - expect(glyph).toHaveAttribute('height', '32'); - expect(glyph).toHaveAttribute('width', '32'); + expectSize(glyph, '32'); }); test('applies Size enum to custom SVGs', () => { render(); const glyph = screen.getByTestId('raw-svg-glyph'); - expect(glyph).toHaveAttribute('height', '20'); - expect(glyph).toHaveAttribute('width', '20'); + expectSize(glyph, '20'); }); }); @@ -184,11 +171,8 @@ describe('packages/Icon/createIconComponent', () => { }); describe('title prop with custom SVGR glyphs', () => { - // Custom SVGR component that renders title similar to generated glyphs - const CustomGlyphWithTitle: SVGR.Component = ({ children, ...props }) => ( - - {children} - + const CustomGlyphWithTitle = createMockSVGRComponent( + 'custom-glyph-with-title', ); const customGlyphsWithTitle = { @@ -269,9 +253,7 @@ describe('packages/Icon/createIconComponent', () => { }); describe('className prop with custom SVGs', () => { - const RawSVGGlyph: SVGR.Component = props => ( - - ); + const RawSVGGlyph = createMockSVGRComponent('raw-svg-for-class'); const customSVGGlyphs = { RawSVG: createGlyphComponent('RawSVG', RawSVGGlyph), @@ -292,43 +274,37 @@ describe('packages/Icon/createIconComponent', () => { test('applies numeric size to glyph', () => { render(); const glyph = screen.getByTestId('custom-svgr-glyph'); - expect(glyph).toHaveAttribute('height', '24'); - expect(glyph).toHaveAttribute('width', '24'); + expectSize(glyph, '24'); }); test('applies Size.Small correctly', () => { render(); const glyph = screen.getByTestId('custom-svgr-glyph'); - expect(glyph).toHaveAttribute('height', '14'); - expect(glyph).toHaveAttribute('width', '14'); + expectSize(glyph, '14'); }); test('applies Size.Default correctly', () => { render(); const glyph = screen.getByTestId('custom-svgr-glyph'); - expect(glyph).toHaveAttribute('height', '16'); - expect(glyph).toHaveAttribute('width', '16'); + expectSize(glyph, '16'); }); test('applies Size.Large correctly', () => { render(); const glyph = screen.getByTestId('custom-svgr-glyph'); - expect(glyph).toHaveAttribute('height', '20'); - expect(glyph).toHaveAttribute('width', '20'); + expectSize(glyph, '20'); }); test('applies Size.XLarge correctly', () => { render(); const glyph = screen.getByTestId('custom-svgr-glyph'); - expect(glyph).toHaveAttribute('height', '24'); - expect(glyph).toHaveAttribute('width', '24'); + expectSize(glyph, '24'); }); test('uses default size when size prop is not provided', () => { render(); const glyph = screen.getByTestId('custom-svgr-glyph'); - expect(glyph).toHaveAttribute('height', '16'); - expect(glyph).toHaveAttribute('width', '16'); + expectSize(glyph, '16'); }); }); @@ -377,8 +353,7 @@ describe('packages/Icon/createIconComponent', () => { test('applies fill as CSS color', () => { render(); const glyph = screen.getByTestId('custom-svgr-glyph'); - const computedStyle = window.getComputedStyle(glyph); - expect(computedStyle.color).toBe('red'); + expectFillColor(glyph, 'red'); }); test('applies fill alongside className', () => { @@ -391,8 +366,7 @@ describe('packages/Icon/createIconComponent', () => { ); const glyph = screen.getByTestId('custom-svgr-glyph'); expect(glyph).toHaveClass('custom-class'); - const computedStyle = window.getComputedStyle(glyph); - expect(computedStyle.color).toBe('blue'); + expectFillColor(glyph, 'blue'); }); }); @@ -402,8 +376,7 @@ describe('packages/Icon/createIconComponent', () => { test('applies fill as CSS color to generated glyph', () => { render(); const glyph = screen.getByRole('img'); - const computedStyle = window.getComputedStyle(glyph); - expect(computedStyle.color).toBe('purple'); + expectFillColor(glyph, 'purple'); }); }); @@ -422,11 +395,7 @@ describe('packages/Icon/createIconComponent', () => { ); const glyph = screen.getByRole('img'); - // Check size - expect(glyph).toHaveAttribute('height', '24'); - expect(glyph).toHaveAttribute('width', '24'); - - // Check className + expectSize(glyph, '24'); expect(glyph).toHaveClass('combined-class'); // Check title @@ -437,9 +406,7 @@ describe('packages/Icon/createIconComponent', () => { // Check aria-labelledby points to title expect(glyph.getAttribute('aria-labelledby')).toBe(titleElement?.id); - // Check fill (as CSS color) - const computedStyle = window.getComputedStyle(glyph); - expect(computedStyle.color).toBe('green'); + expectFillColor(glyph, 'green'); }); }); @@ -457,16 +424,9 @@ describe('packages/Icon/createIconComponent', () => { ); const glyph = screen.getByTestId('custom-svgr-glyph'); - // Check size - expect(glyph).toHaveAttribute('height', '32'); - expect(glyph).toHaveAttribute('width', '32'); - - // Check className + expectSize(glyph, '32'); expect(glyph).toHaveClass('combined-custom-class'); - - // Check fill (as CSS color) - const computedStyle = window.getComputedStyle(glyph); - expect(computedStyle.color).toBe('orange'); + expectFillColor(glyph, 'orange'); }); test('applies accessibility props with className', () => { diff --git a/packages/icon/src/createIconComponent.tsx b/packages/icon/src/createIconComponent.tsx index 9be9273c8b..eb2917892d 100644 --- a/packages/icon/src/createIconComponent.tsx +++ b/packages/icon/src/createIconComponent.tsx @@ -4,6 +4,7 @@ import mapValues from 'lodash/mapValues'; import { createGlyphComponent } from './createGlyphComponent'; import { Size } from './glyphCommon'; +import { isComponentGlyph } from './isComponentGlyph'; import { LGGlyph } from './types'; // We omit size here because we map string values for size to numbers in this component. @@ -29,22 +30,23 @@ type GlyphObject = Record; export function createIconComponent( glyphs: G, ) { + const allGlyphsAreComponents = Object.values(glyphs).every(isComponentGlyph); + const glyphDict = allGlyphsAreComponents + ? glyphs + : mapValues(glyphs, (val, key) => { + if (isComponentGlyph(val)) return val; + + const isValSVG = typeof val === 'string'; + + return isValSVG ? createGlyphComponent(key, val) : val; + }); + const Icon = ({ glyph, ...rest }: IconProps) => { - const SVGComponent = glyphs[glyph]; + const SVGComponent = glyphDict[glyph]; if (SVGComponent) { return ; } else { - const generatedGlyphs = mapValues(glyphs, (val, key) => { - return createGlyphComponent(key, val); - }); - - const GeneratedSVGComponent = generatedGlyphs[glyph]; - - if (GeneratedSVGComponent) { - return ; - } - // TODO: improve fuzzy match // Suggest the proper icon casing if there's a near match const nearMatch = Object.keys(glyphs).find( diff --git a/packages/icon/src/testUtils.tsx b/packages/icon/src/testUtils.tsx new file mode 100644 index 0000000000..c73c50febe --- /dev/null +++ b/packages/icon/src/testUtils.tsx @@ -0,0 +1,52 @@ +import React from 'react'; + +import { SVGR } from './types'; + +/** + * Creates a mock SVGR component for testing purposes + * @param testId - The data-testid to apply to the SVG element + */ +export const createMockSVGRComponent = (testId: string): SVGR.Component => { + const MockComponent: SVGR.Component = ({ children, ...props }) => ( + + {children} + + ); + return MockComponent; +}; + +// Pre-built mock components for common test scenarios +export const MockSVGRGlyph = createMockSVGRComponent('mock-glyph'); +export const MockSVGRGlyphWithChildren = createMockSVGRComponent( + 'mock-glyph-with-children', +); +export const CustomSVGRGlyph = createMockSVGRComponent('custom-svgr-glyph'); +export const AnotherCustomGlyph = createMockSVGRComponent( + 'another-custom-glyph', +); + +/** + * Size enum values and their expected pixel values for testing + */ +export const sizeTestCases = [ + { size: 'small', enumValue: 'Small', expected: '14' }, + { size: 'default', enumValue: 'Default', expected: '16' }, + { size: 'large', enumValue: 'Large', expected: '20' }, + { size: 'xlarge', enumValue: 'XLarge', expected: '24' }, +] as const; + +/** + * Asserts that an element has the expected height and width attributes + */ +export const expectSize = (element: HTMLElement, size: string) => { + expect(element).toHaveAttribute('height', size); + expect(element).toHaveAttribute('width', size); +}; + +/** + * Asserts that an element has the expected fill color applied via CSS + */ +export const expectFillColor = (element: HTMLElement, color: string) => { + const computedStyle = window.getComputedStyle(element); + expect(computedStyle.color).toBe(color); +}; From d5b9ade1661792f3ac5c5b29423acad48ab46be4 Mon Sep 17 00:00:00 2001 From: Adam Rasheed Date: Fri, 12 Dec 2025 11:35:49 -0800 Subject: [PATCH 7/9] updated story --- packages/icon/src/Icon.stories.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/icon/src/Icon.stories.tsx b/packages/icon/src/Icon.stories.tsx index c235a3751c..075f770371 100644 --- a/packages/icon/src/Icon.stories.tsx +++ b/packages/icon/src/Icon.stories.tsx @@ -116,7 +116,7 @@ export const LiveExample: StoryObj = { }; const customGlyphs = { - CustomGlyph: createGlyphComponent('CustomGlyph', props => ), + ...glyphs, }; export const Custom: StoryObj = { @@ -126,8 +126,8 @@ export const Custom: StoryObj = { }, }, render: (args: Omit) => { - const IconComponent = createIconComponent(customGlyphs); - return ; + const CustomIcon = createIconComponent(customGlyphs); + return ; }, }; From b7843adb8541b7d4f001bc24ddcf9cee2b1bf79b Mon Sep 17 00:00:00 2001 From: Adam Rasheed Date: Fri, 12 Dec 2025 13:57:46 -0800 Subject: [PATCH 8/9] updated createIconComponent, cleaned up story --- packages/icon/src/Icon.stories.tsx | 14 ++------------ packages/icon/src/createIconComponent.tsx | 4 +--- 2 files changed, 3 insertions(+), 15 deletions(-) diff --git a/packages/icon/src/Icon.stories.tsx b/packages/icon/src/Icon.stories.tsx index 075f770371..d4ecea4e55 100644 --- a/packages/icon/src/Icon.stories.tsx +++ b/packages/icon/src/Icon.stories.tsx @@ -9,13 +9,7 @@ import { css } from '@leafygreen-ui/emotion'; import { palette } from '@leafygreen-ui/palette'; import { GlyphName } from './glyphs'; -import Icon, { - createGlyphComponent, - createIconComponent, - glyphs, - IconProps, - Size, -} from '.'; +import Icon, { createIconComponent, glyphs, IconProps, Size } from '.'; const meta: StoryMetaType = { title: 'Components/Display/Icon', @@ -115,10 +109,6 @@ export const LiveExample: StoryObj = { ), }; -const customGlyphs = { - ...glyphs, -}; - export const Custom: StoryObj = { parameters: { controls: { @@ -126,7 +116,7 @@ export const Custom: StoryObj = { }, }, render: (args: Omit) => { - const CustomIcon = createIconComponent(customGlyphs); + const CustomIcon = createIconComponent(glyphs); return ; }, }; diff --git a/packages/icon/src/createIconComponent.tsx b/packages/icon/src/createIconComponent.tsx index eb2917892d..c189d5d78c 100644 --- a/packages/icon/src/createIconComponent.tsx +++ b/packages/icon/src/createIconComponent.tsx @@ -36,9 +36,7 @@ export function createIconComponent( : mapValues(glyphs, (val, key) => { if (isComponentGlyph(val)) return val; - const isValSVG = typeof val === 'string'; - - return isValSVG ? createGlyphComponent(key, val) : val; + return createGlyphComponent(key, val); }); const Icon = ({ glyph, ...rest }: IconProps) => { From f5b344ac078f8ba190b3a430d0cf7509a638c9b8 Mon Sep 17 00:00:00 2001 From: Adam Rasheed Date: Wed, 17 Dec 2025 18:43:21 -0800 Subject: [PATCH 9/9] refined tests --- .../icon/src/createGlyphComponent.spec.tsx | 116 +++++++----------- .../icon/src/createIconComponent.spec.tsx | 59 +++++---- packages/icon/src/testUtils.tsx | 52 ++++---- 3 files changed, 105 insertions(+), 122 deletions(-) diff --git a/packages/icon/src/createGlyphComponent.spec.tsx b/packages/icon/src/createGlyphComponent.spec.tsx index 6694ac5c2d..2ec57f3cb7 100644 --- a/packages/icon/src/createGlyphComponent.spec.tsx +++ b/packages/icon/src/createGlyphComponent.spec.tsx @@ -4,16 +4,11 @@ import { render, screen } from '@testing-library/react'; import { createGlyphComponent } from './createGlyphComponent'; import { Size } from './glyphCommon'; import { isComponentGlyph } from './isComponentGlyph'; -import { - expectFillColor, - expectSize, - MockSVGRGlyph, - MockSVGRGlyphWithChildren, -} from './testUtils'; +import { TestSVGRGlyph, TestSVGRGlyphWithChildren } from './testUtils'; describe('packages/Icon/createGlyphComponent', () => { describe('basic functionality', () => { - const GlyphComponent = createGlyphComponent('TestGlyph', MockSVGRGlyph); + const GlyphComponent = createGlyphComponent('TestGlyph', TestSVGRGlyph); test('returns a function', () => { expect(typeof GlyphComponent).toBe('function'); @@ -34,7 +29,7 @@ describe('packages/Icon/createGlyphComponent', () => { }); describe('rendering', () => { - const GlyphComponent = createGlyphComponent('TestGlyph', MockSVGRGlyph); + const GlyphComponent = createGlyphComponent('TestGlyph', TestSVGRGlyph); test('renders an SVG element', () => { render(); @@ -51,99 +46,92 @@ describe('packages/Icon/createGlyphComponent', () => { }); describe('size prop', () => { - const GlyphComponent = createGlyphComponent('TestGlyph', MockSVGRGlyph); + const GlyphComponent = createGlyphComponent('TestGlyph', TestSVGRGlyph); test('applies numeric size to height and width', () => { render(); const glyph = screen.getByTestId('mock-glyph'); - expectSize(glyph, '24'); + expect(glyph).toHaveAttribute('height', '24'); + expect(glyph).toHaveAttribute('width', '24'); }); test('applies Size.Small correctly (14px)', () => { render(); const glyph = screen.getByTestId('mock-glyph'); - expectSize(glyph, '14'); + expect(glyph).toHaveAttribute('height', '14'); + expect(glyph).toHaveAttribute('width', '14'); }); test('applies Size.Default correctly (16px)', () => { render(); const glyph = screen.getByTestId('mock-glyph'); - expectSize(glyph, '16'); + expect(glyph).toHaveAttribute('height', '16'); + expect(glyph).toHaveAttribute('width', '16'); }); test('applies Size.Large correctly (20px)', () => { render(); const glyph = screen.getByTestId('mock-glyph'); - expectSize(glyph, '20'); + expect(glyph).toHaveAttribute('height', '20'); + expect(glyph).toHaveAttribute('width', '20'); }); test('applies Size.XLarge correctly (24px)', () => { render(); const glyph = screen.getByTestId('mock-glyph'); - expectSize(glyph, '24'); + expect(glyph).toHaveAttribute('height', '24'); + expect(glyph).toHaveAttribute('width', '24'); }); test('uses Size.Default (16px) when size prop is not provided', () => { render(); const glyph = screen.getByTestId('mock-glyph'); - expectSize(glyph, '16'); + expect(glyph).toHaveAttribute('height', '16'); + expect(glyph).toHaveAttribute('width', '16'); }); }); describe('fill prop', () => { - const GlyphComponent = createGlyphComponent('TestGlyph', MockSVGRGlyph); + const GlyphComponent = createGlyphComponent('TestGlyph', TestSVGRGlyph); - test('applies fill as CSS color', () => { + test('applies fill as CSS color via className', () => { render(); const glyph = screen.getByTestId('mock-glyph'); - expectFillColor(glyph, 'red'); + // Fill is applied as a CSS color via an emotion-generated class + expect(glyph).toHaveAttribute('class'); + expect(glyph.className).not.toBe(''); }); - test('does not apply fill style when fill is not provided', () => { + test('does not apply fill style class when fill is not provided', () => { render(); const glyph = screen.getByTestId('mock-glyph'); - // When no fill is provided, no fill-related class should be applied - // The glyph should still render without error - expect(glyph).toBeInTheDocument(); + // When no fill is provided, no emotion class should be applied + expect(glyph.classList.length).toBe(0); }); test('applies fill alongside other props', () => { render(); const glyph = screen.getByTestId('mock-glyph'); expect(glyph).toHaveClass('custom-class'); - expectSize(glyph, '32'); - expectFillColor(glyph, 'blue'); + expect(glyph).toHaveAttribute('height', '32'); + expect(glyph).toHaveAttribute('width', '32'); + // Fill adds an emotion class in addition to custom-class + expect(glyph.classList.length).toBeGreaterThan(1); }); }); describe('className prop', () => { - const GlyphComponent = createGlyphComponent('TestGlyph', MockSVGRGlyph); + const GlyphComponent = createGlyphComponent('TestGlyph', TestSVGRGlyph); test('applies className to the SVG element', () => { render(); const glyph = screen.getByTestId('mock-glyph'); expect(glyph).toHaveClass('my-custom-class'); }); - - test('applies multiple classNames to the SVG element', () => { - render(); - const glyph = screen.getByTestId('mock-glyph'); - expect(glyph).toHaveClass('class-one'); - expect(glyph).toHaveClass('class-two'); - }); - - test('applies className alongside fill style', () => { - render(); - const glyph = screen.getByTestId('mock-glyph'); - expect(glyph).toHaveClass('custom-class'); - // fill applies a CSS class for the color style - const classList = Array.from(glyph.classList); - expect(classList.length).toBeGreaterThan(1); - }); }); describe('role prop', () => { - const GlyphComponent = createGlyphComponent('TestGlyph', MockSVGRGlyph); + const GlyphComponent = createGlyphComponent('TestGlyph', TestSVGRGlyph); test('applies role="img" by default', () => { render(); @@ -176,7 +164,7 @@ describe('packages/Icon/createGlyphComponent', () => { }); describe('accessibility props', () => { - const GlyphComponent = createGlyphComponent('TestGlyph', MockSVGRGlyph); + const GlyphComponent = createGlyphComponent('TestGlyph', TestSVGRGlyph); test('generates default aria-label from glyph name when no accessibility props provided', () => { render(); @@ -214,6 +202,14 @@ describe('packages/Icon/createGlyphComponent', () => { expect(ariaLabelledBy).toContain('icon-title'); }); + test('sets the title ID when title is provided', () => { + render(); + const glyph = screen.getByTestId('mock-glyph'); + const titleId = glyph.getAttribute('aria-labelledby'); + expect(titleId).not.toBeNull(); + expect(titleId).toContain('icon-title'); + }); + test('sets aria-hidden to true when role is presentation', () => { render(); const glyph = screen.getByTestId('mock-glyph'); @@ -224,7 +220,7 @@ describe('packages/Icon/createGlyphComponent', () => { describe('title prop', () => { const GlyphComponent = createGlyphComponent( 'TestGlyph', - MockSVGRGlyphWithChildren, + TestSVGRGlyphWithChildren, ); test('does not include title in children when title is not provided', () => { @@ -236,7 +232,7 @@ describe('packages/Icon/createGlyphComponent', () => { }); describe('combined props', () => { - const GlyphComponent = createGlyphComponent('TestGlyph', MockSVGRGlyph); + const GlyphComponent = createGlyphComponent('TestGlyph', TestSVGRGlyph); test('applies all props correctly together', () => { render( @@ -249,10 +245,12 @@ describe('packages/Icon/createGlyphComponent', () => { ); const glyph = screen.getByTestId('mock-glyph'); - expectSize(glyph, '32'); + expect(glyph).toHaveAttribute('height', '32'); + expect(glyph).toHaveAttribute('width', '32'); expect(glyph).toHaveClass('combined-class'); expect(glyph).toHaveAttribute('aria-label', 'Combined Icon'); - expectFillColor(glyph, 'purple'); + // Fill adds an emotion class in addition to combined-class + expect(glyph.classList.length).toBeGreaterThan(1); }); test('applies size enum with className and role', () => { @@ -265,31 +263,11 @@ describe('packages/Icon/createGlyphComponent', () => { ); const glyph = screen.getByTestId('mock-glyph'); - expectSize(glyph, '20'); + expect(glyph).toHaveAttribute('height', '20'); + expect(glyph).toHaveAttribute('width', '20'); expect(glyph).toHaveClass('accessible-class'); expect(glyph).toHaveAttribute('role', 'presentation'); expect(glyph).toHaveAttribute('aria-hidden', 'true'); }); }); - - describe('different glyph names', () => { - test('handles PascalCase glyph names correctly', () => { - const GlyphComponent = createGlyphComponent( - 'MyCustomGlyph', - MockSVGRGlyph, - ); - expect(GlyphComponent.displayName).toBe('MyCustomGlyph'); - render(); - const glyph = screen.getByTestId('mock-glyph'); - expect(glyph).toHaveAttribute('aria-label', 'My Custom Glyph Icon'); - }); - - test('handles single word glyph names correctly', () => { - const GlyphComponent = createGlyphComponent('Edit', MockSVGRGlyph); - expect(GlyphComponent.displayName).toBe('Edit'); - render(); - const glyph = screen.getByTestId('mock-glyph'); - expect(glyph).toHaveAttribute('aria-label', 'Edit Icon'); - }); - }); }); diff --git a/packages/icon/src/createIconComponent.spec.tsx b/packages/icon/src/createIconComponent.spec.tsx index 15a775cf79..42dffe7d77 100644 --- a/packages/icon/src/createIconComponent.spec.tsx +++ b/packages/icon/src/createIconComponent.spec.tsx @@ -8,10 +8,8 @@ import { Size } from './glyphCommon'; import { isComponentGlyph } from './isComponentGlyph'; import { AnotherCustomGlyph, - createMockSVGRComponent, + createTestSVGRComponent, CustomSVGRGlyph, - expectFillColor, - expectSize, } from './testUtils'; // Create glyph components from the SVGR components @@ -99,7 +97,7 @@ describe('packages/Icon/createIconComponent', () => { }); describe('custom SVG support', () => { - const RawSVGGlyph = createMockSVGRComponent('raw-svg-glyph'); + const RawSVGGlyph = createTestSVGRComponent('raw-svg-glyph'); const customSVGGlyphs = { RawSVG: createGlyphComponent('RawSVG', RawSVGGlyph), @@ -117,13 +115,15 @@ describe('packages/Icon/createIconComponent', () => { test('applies size prop to custom SVGs', () => { render(); const glyph = screen.getByTestId('raw-svg-glyph'); - expectSize(glyph, '32'); + expect(glyph).toHaveAttribute('height', '32'); + expect(glyph).toHaveAttribute('width', '32'); }); test('applies Size enum to custom SVGs', () => { render(); const glyph = screen.getByTestId('raw-svg-glyph'); - expectSize(glyph, '20'); + expect(glyph).toHaveAttribute('height', '20'); + expect(glyph).toHaveAttribute('width', '20'); }); }); @@ -171,7 +171,7 @@ describe('packages/Icon/createIconComponent', () => { }); describe('title prop with custom SVGR glyphs', () => { - const CustomGlyphWithTitle = createMockSVGRComponent( + const CustomGlyphWithTitle = createTestSVGRComponent( 'custom-glyph-with-title', ); @@ -253,7 +253,7 @@ describe('packages/Icon/createIconComponent', () => { }); describe('className prop with custom SVGs', () => { - const RawSVGGlyph = createMockSVGRComponent('raw-svg-for-class'); + const RawSVGGlyph = createTestSVGRComponent('raw-svg-for-class'); const customSVGGlyphs = { RawSVG: createGlyphComponent('RawSVG', RawSVGGlyph), @@ -274,37 +274,43 @@ describe('packages/Icon/createIconComponent', () => { test('applies numeric size to glyph', () => { render(); const glyph = screen.getByTestId('custom-svgr-glyph'); - expectSize(glyph, '24'); + expect(glyph).toHaveAttribute('height', '24'); + expect(glyph).toHaveAttribute('width', '24'); }); test('applies Size.Small correctly', () => { render(); const glyph = screen.getByTestId('custom-svgr-glyph'); - expectSize(glyph, '14'); + expect(glyph).toHaveAttribute('height', '14'); + expect(glyph).toHaveAttribute('width', '14'); }); test('applies Size.Default correctly', () => { render(); const glyph = screen.getByTestId('custom-svgr-glyph'); - expectSize(glyph, '16'); + expect(glyph).toHaveAttribute('height', '16'); + expect(glyph).toHaveAttribute('width', '16'); }); test('applies Size.Large correctly', () => { render(); const glyph = screen.getByTestId('custom-svgr-glyph'); - expectSize(glyph, '20'); + expect(glyph).toHaveAttribute('height', '20'); + expect(glyph).toHaveAttribute('width', '20'); }); test('applies Size.XLarge correctly', () => { render(); const glyph = screen.getByTestId('custom-svgr-glyph'); - expectSize(glyph, '24'); + expect(glyph).toHaveAttribute('height', '24'); + expect(glyph).toHaveAttribute('width', '24'); }); test('uses default size when size prop is not provided', () => { render(); const glyph = screen.getByTestId('custom-svgr-glyph'); - expectSize(glyph, '16'); + expect(glyph).toHaveAttribute('height', '16'); + expect(glyph).toHaveAttribute('width', '16'); }); }); @@ -350,10 +356,12 @@ describe('packages/Icon/createIconComponent', () => { describe('fill prop', () => { const IconComponent = createIconComponent(customGlyphs); - test('applies fill as CSS color', () => { + test('applies fill as CSS color via className', () => { render(); const glyph = screen.getByTestId('custom-svgr-glyph'); - expectFillColor(glyph, 'red'); + // Fill is applied as a CSS color via an emotion-generated class + expect(glyph).toHaveAttribute('class'); + expect(glyph.classList.length).toBeGreaterThan(0); }); test('applies fill alongside className', () => { @@ -366,7 +374,8 @@ describe('packages/Icon/createIconComponent', () => { ); const glyph = screen.getByTestId('custom-svgr-glyph'); expect(glyph).toHaveClass('custom-class'); - expectFillColor(glyph, 'blue'); + // Fill adds an emotion class in addition to custom-class + expect(glyph.classList.length).toBeGreaterThan(1); }); }); @@ -376,7 +385,9 @@ describe('packages/Icon/createIconComponent', () => { test('applies fill as CSS color to generated glyph', () => { render(); const glyph = screen.getByRole('img'); - expectFillColor(glyph, 'purple'); + // Fill is applied as a CSS color via an emotion-generated class + expect(glyph).toHaveAttribute('class'); + expect(glyph.classList.length).toBeGreaterThan(0); }); }); @@ -395,7 +406,8 @@ describe('packages/Icon/createIconComponent', () => { ); const glyph = screen.getByRole('img'); - expectSize(glyph, '24'); + expect(glyph).toHaveAttribute('height', '24'); + expect(glyph).toHaveAttribute('width', '24'); expect(glyph).toHaveClass('combined-class'); // Check title @@ -406,7 +418,8 @@ describe('packages/Icon/createIconComponent', () => { // Check aria-labelledby points to title expect(glyph.getAttribute('aria-labelledby')).toBe(titleElement?.id); - expectFillColor(glyph, 'green'); + // Fill adds an emotion class in addition to combined-class + expect(glyph.classList.length).toBeGreaterThan(1); }); }); @@ -424,9 +437,11 @@ describe('packages/Icon/createIconComponent', () => { ); const glyph = screen.getByTestId('custom-svgr-glyph'); - expectSize(glyph, '32'); + expect(glyph).toHaveAttribute('height', '32'); + expect(glyph).toHaveAttribute('width', '32'); expect(glyph).toHaveClass('combined-custom-class'); - expectFillColor(glyph, 'orange'); + // Fill adds an emotion class in addition to combined-custom-class + expect(glyph.classList.length).toBeGreaterThan(1); }); test('applies accessibility props with className', () => { diff --git a/packages/icon/src/testUtils.tsx b/packages/icon/src/testUtils.tsx index c73c50febe..a5746783b4 100644 --- a/packages/icon/src/testUtils.tsx +++ b/packages/icon/src/testUtils.tsx @@ -6,47 +6,37 @@ import { SVGR } from './types'; * Creates a mock SVGR component for testing purposes * @param testId - The data-testid to apply to the SVG element */ -export const createMockSVGRComponent = (testId: string): SVGR.Component => { - const MockComponent: SVGR.Component = ({ children, ...props }) => ( +export const createTestSVGRComponent = (testId: string): SVGR.Component => { + const TestComponent: SVGR.Component = ({ children, ...props }) => ( {children} ); - return MockComponent; + return TestComponent; }; // Pre-built mock components for common test scenarios -export const MockSVGRGlyph = createMockSVGRComponent('mock-glyph'); -export const MockSVGRGlyphWithChildren = createMockSVGRComponent( +export const TestSVGRGlyph = createTestSVGRComponent('mock-glyph'); +export const TestSVGRGlyphWithChildren = createTestSVGRComponent( 'mock-glyph-with-children', ); -export const CustomSVGRGlyph = createMockSVGRComponent('custom-svgr-glyph'); -export const AnotherCustomGlyph = createMockSVGRComponent( +export const CustomSVGRGlyph = createTestSVGRComponent('custom-svgr-glyph'); +export const AnotherCustomGlyph = createTestSVGRComponent( 'another-custom-glyph', ); -/** - * Size enum values and their expected pixel values for testing - */ -export const sizeTestCases = [ - { size: 'small', enumValue: 'Small', expected: '14' }, - { size: 'default', enumValue: 'Default', expected: '16' }, - { size: 'large', enumValue: 'Large', expected: '20' }, - { size: 'xlarge', enumValue: 'XLarge', expected: '24' }, -] as const; +// /** +// * Asserts that an element has the expected height and width attributes +// */ +// export const expectSize = (element: HTMLElement, size: string) => { +// expect(element).toHaveAttribute('height', size); +// expect(element).toHaveAttribute('width', size); +// }; -/** - * Asserts that an element has the expected height and width attributes - */ -export const expectSize = (element: HTMLElement, size: string) => { - expect(element).toHaveAttribute('height', size); - expect(element).toHaveAttribute('width', size); -}; - -/** - * Asserts that an element has the expected fill color applied via CSS - */ -export const expectFillColor = (element: HTMLElement, color: string) => { - const computedStyle = window.getComputedStyle(element); - expect(computedStyle.color).toBe(color); -}; +// /** +// * Asserts that an element has the expected fill color applied via CSS +// */ +// export const expectFillColor = (element: HTMLElement, color: string) => { +// const computedStyle = window.getComputedStyle(element); +// expect(computedStyle.color).toBe(color); +// };