[LG-3416] feat: Icon - added title element

.changeset/puny-paws-yell.md

@@ -0,0 +1,7 @@
+---
+'@leafygreen-ui/icon': minor
+'@leafygreen-ui/select': minor
+'@leafygreen-ui/text-input': minor
+---
+
+Updated Icon to include <title> element when title is added. Deprecated the `createGlyphComponent`, and the design system is instead using glyphs from the `generated` folder

packages/emotion/src/version.ts

@@ -1 +1 @@
-export const VERSION = '5.0.3';
+export const VERSION = '5.0.4';

packages/icon-button/src/IconButton/IconButton.spec.tsx

@@ -4,7 +4,7 @@ import { axe } from 'jest-axe';
 
 import EllipsisIcon from '@leafygreen-ui/icon/dist/Ellipsis';
 
-import IconButton from '..';
+import { IconButton } from '..';
 
 const onClick = jest.fn();
 const className = 'test-icon-button-class';
@@ -84,13 +84,19 @@ describe('packages/icon-button', () => {
     expect(iconButton.tagName.toLowerCase()).toBe('a');
   });
 
-  test(`when '${titleText}' is set directly as the title child icon, the rendered icon includes the title attribute, '${titleText}'`, () => {
-    const iconWithTitle = (
-      <EllipsisIcon data-testid="icon-test-id" title={titleText} />
+  test(`shows a title element with '${titleText}' when the title prop is set`, () => {
+    const { container } = render(
+      <IconButton aria-label="Ellipsis">
+        <EllipsisIcon title={titleText} />
+      </IconButton>,
     );
-    const { icon } = renderIconButton({ children: iconWithTitle });
 
-    expect(icon.getAttribute('title')).toBe(titleText);
+    const icon = container.querySelector('[role="img"]');
+    expect(icon).toBeTruthy();
+
+    const titleElement = icon?.querySelector('title');
+    expect(titleElement).toBeTruthy();
+    expect(titleElement?.textContent).toBe(titleText);
   });
 
   /* eslint-disable jest/no-disabled-tests*/

packages/icon/package.json

@@ -36,6 +36,7 @@
   },
   "dependencies": {
     "@leafygreen-ui/emotion": "workspace:^",
+    "@leafygreen-ui/hooks": "workspace:^",
     "lodash": "^4.17.21"
   },
   "devDependencies": {

packages/icon/scripts/build/compare-checksum.ts

@@ -13,7 +13,8 @@ export function getAllIcons(): Array<string> {
   const iconNames = fsx
     .readdirSync(ICON_DIR)
     .filter(f => /\.tsx?$/.test(f))
-    .map(f => path.basename(f, path.extname(f)));
+    .map(f => path.basename(f, path.extname(f)))
+    .filter(name => name !== 'index'); // Exclude the index file
   return iconNames;
 }
 

packages/icon/scripts/prebuild/index.ts

@@ -7,7 +7,7 @@ import fs from 'fs';
 import path from 'path';
 
 import { getChecksum } from './checksum';
-import { indexTemplate } from './indexTemplate';
+import { generatedIndexTemplate, indexTemplate } from './indexTemplate';
 import { FileObject, PrebuildOptions } from './prebuild.types';
 import { svgrTemplate } from './svgrTemplate';
 
@@ -28,6 +28,7 @@ async function buildSvgFiles(options: PrebuildOptions): Promise<void> {
   options?.verbose && console.log('Processing SVG files...\n');
   await Promise.all(svgFiles.map(processFile));
   await createIndexFile(svgFiles, options);
+  await createGeneratedIndexFile(svgFiles, outputDir, options);
 }
 
 /**
@@ -91,6 +92,21 @@ async function createIndexFile(
   fs.writeFileSync(indexPath, formattedIndexContent, { encoding: 'utf8' });
 }
 
+/**
+ * Create the index file for the generated component exports
+ */
+async function createGeneratedIndexFile(
+  svgFiles: Array<FileObject>,
+  outputDir: string,
+  options?: PrebuildOptions,
+) {
+  const indexPath = path.resolve(outputDir, 'index.ts');
+  options?.verbose && console.log('Writing generated index file...', indexPath);
+  const indexContent = await generatedIndexTemplate(svgFiles);
+  const formattedIndexContent = await formatLG(indexContent, indexPath);
+  fs.writeFileSync(indexPath, formattedIndexContent, { encoding: 'utf8' });
+}
+
 /**
  * Annotate the generated file with script and checksum information
  */

packages/icon/scripts/prebuild/indexTemplate.ts

@@ -5,27 +5,31 @@ export async function indexTemplate(svgFiles: Array<FileObject>) {
     .map(({ name }) => `import ${name} from './${name}.svg';`)
     .join('\n');
 
-  const _glyphs = `{
-    ${svgFiles.map(({ name }) => `${name}`).join(',\n')}
-  }`;
+  const glyphsList = svgFiles.map(({ name }) => `${name}`).join(',\n    ');
 
   return `
-    import { createGlyphComponent } from '../createGlyphComponent';
-    import { LGGlyph } from '../types';
-
     // Glyphs
     ${imports}
 
-    const _glyphs = ${_glyphs} as const;
-
-    export type GlyphName = keyof typeof _glyphs;
-
-    const glyphKeys = Object.keys(_glyphs) as Array<GlyphName>;
-
-    export const glyphs = glyphKeys.reduce((acc, name) => {
-      acc[name] = createGlyphComponent(name, _glyphs[name]);
-
-      return acc;
-    }, {} as Record<GlyphName, LGGlyph.Component>);
+    export const glyphs = {
+      ${glyphsList}
+    } as const;
+
+    export type GlyphName = keyof typeof glyphs;
   `;
 }
+
+export async function generatedIndexTemplate(svgFiles: Array<FileObject>) {
+  const exports = svgFiles
+    .map(({ name }) => `export { default as ${name} } from './${name}';`)
+    .join('\n');
+
+  return `/**
+ * This is a generated file. Do not modify it manually.
+ *
+ * @script packages/icon/scripts/prebuild/index.ts
+ */
+
+${exports}
+`;
+}

packages/icon/scripts/prebuild/svgrTemplate.ts

@@ -20,10 +20,12 @@ interface ASTParts extends Record<string, any> {
 }
 
 export function svgrTemplate(
-  { template }: BabelAPI,
-  { state: { componentName } }: SVGROptions,
+  api: BabelAPI,
+  opts: SVGROptions,
   { imports, jsx, exports }: ASTParts,
 ) {
+  const { template, types: t } = api;
+  const { componentName } = opts.state;
   const typeScriptTpl = template.smart({ plugins: ['jsx', 'typescript'] });
 
   const jsxAttributes = typeScriptTpl.ast`
@@ -48,9 +50,40 @@ export function svgrTemplate(
       jsx.openingElement.attributes[2],
     );
 
+  // Convert self-closing svg to have children
+  jsx.openingElement.selfClosing = false;
+
+  // Create closing element using Babel types
+  jsx.closingElement = t.jsxClosingElement(
+    t.jsxIdentifier(jsx.openingElement.name.name),
+  );
+
+  // Create the title element JSX manually using Babel types
+  // {title && <title id={titleId}>{title}</title>}
+  const titleJSXElement = t.jsxElement(
+    t.jsxOpeningElement(t.jsxIdentifier('title'), [
+      t.jsxAttribute(
+        t.jsxIdentifier('id'),
+        t.jsxExpressionContainer(t.identifier('titleId')),
+      ),
+    ]),
+    t.jsxClosingElement(t.jsxIdentifier('title')),
+    [t.jsxExpressionContainer(t.identifier('title'))],
+    false,
+  );
+
+  // Wrap title in conditional expression: {title && <title>...</title>}
+  const conditionalTitleExpression = t.jsxExpressionContainer(
+    t.logicalExpression('&&', t.identifier('title'), titleJSXElement),
+  );
+
+  // Add the conditional title as a child, followed by the original children
+  jsx.children = [conditionalTitleExpression, ...jsx.children];
+
   return typeScriptTpl(`
     %%imports%%
     import { css, cx } from '@leafygreen-ui/emotion';
+    import { useIdAllocator } from '@leafygreen-ui/hooks';
     import { generateAccessibleProps, sizeMap } from '../glyphCommon';
     import { LGGlyph } from '../types';
 
@@ -66,6 +99,7 @@ export function svgrTemplate(
       role = 'img',
       ...props
     }: ${componentName}Props) => {
+      const titleId = useIdAllocator({ prefix: 'icon-title' });
       const fillStyle = css\`
         color: \${fill};
       \`;
@@ -74,7 +108,7 @@ export function svgrTemplate(
         flex-shrink: 0;
       \`;
 
-      const accessibleProps = generateAccessibleProps(role, '${componentName}', { title, ['aria-label']: ariaLabel, ['aria-labelledby']: ariaLabelledby })
+      const accessibleProps = generateAccessibleProps(role, '${componentName}', { title, titleId, ['aria-label']: ariaLabel, ['aria-labelledby']: ariaLabelledby })
 
       return %%jsx%%;
     }

packages/icon/src/Icon.spec.tsx

@@ -29,9 +29,11 @@ const glyphPaths = fs
 const generatedFilesDirectory = path.resolve(__dirname, './generated');
 const baseNameToGeneratedFilePath: Record<string, string> = {};
 
-fs.readdirSync(generatedFilesDirectory).forEach(filePath => {
-  baseNameToGeneratedFilePath[getBaseName(filePath)] = filePath;
-});
+fs.readdirSync(generatedFilesDirectory)
+  .filter(filePath => /.*\.tsx$/.test(filePath))
+  .forEach(filePath => {
+    baseNameToGeneratedFilePath[getBaseName(filePath)] = filePath;
+  });
 
 const MyTestSVGRGlyph: SVGR.Component = props => (
   <svg data-testid="my-glyph" {...props}></svg>
@@ -276,7 +278,7 @@ describe('Generated glyphs', () => {
   });
 
   describe('accessible props handled correctly', () => {
-    test('when no prop is supplied, aria-label is genereated', () => {
+    test('when no prop is supplied, aria-label is generated', () => {
       render(<EditIcon />);
       const editIcon = screen.getByRole('img');
       expect(editIcon.getAttribute('aria-label')).toBe('Edit Icon');
@@ -295,11 +297,32 @@ describe('Generated glyphs', () => {
       expect(editIcon.getAttribute('aria-labelledby')).toBe('Test label');
     });
 
-    test('when title is supplied it overrides default label', () => {
+    test('when title is supplied it renders a title element and aria-labelledby', () => {
       render(<EditIcon title="Test title" />);
       const editIcon = screen.getByRole('img');
       expect(editIcon.getAttribute('aria-label')).toBe(null);
-      expect(editIcon.getAttribute('title')).toBe('Test title');
+      // Should have aria-labelledby instead of title attribute
+      const ariaLabelledBy = editIcon.getAttribute('aria-labelledby');
+      expect(ariaLabelledBy).not.toBe(null);
+      // Should find a title element with matching ID containing the text
+      const titleElement = editIcon.querySelector('title');
+      expect(titleElement).not.toBe(null);
+      expect(titleElement?.textContent).toBe('Test title');
+      expect(titleElement?.id).toBe(ariaLabelledBy);
+    });
+
+    test('when both title and aria-labelledby are supplied they are combined', () => {
+      render(<EditIcon title="Test title" aria-labelledby="external-label" />);
+      const editIcon = screen.getByRole('img');
+      expect(editIcon.getAttribute('aria-label')).toBe(null);
+      const ariaLabelledBy = editIcon.getAttribute('aria-labelledby');
+      // Should contain both the title ID and the external label
+      expect(ariaLabelledBy).toContain('external-label');
+      const titleElement = editIcon.querySelector('title');
+      expect(titleElement).not.toBe(null);
+      expect(titleElement?.textContent).toBe('Test title');
+      // The aria-labelledby should reference both
+      expect(ariaLabelledBy).toBe(`${titleElement?.id} external-label`);
     });
 
     test('when role="presentation", aria-hidden is true', () => {

packages/icon/src/Icon.stories.tsx

@@ -17,7 +17,7 @@ const meta: StoryMetaType<typeof Icon> = {
   parameters: {
     default: 'LiveExample',
     controls: {
-      exclude: [...storybookExcludedControlParams, 'title', 'data-testid'],
+      exclude: [...storybookExcludedControlParams, 'data-testid'],
     },
   },
   args: {
@@ -33,6 +33,11 @@ const meta: StoryMetaType<typeof Icon> = {
     fill: {
       control: 'color',
     },
+    title: {
+      control: 'text',
+      description: 'The title of the icon for accessibility',
+      defaultValue: undefined,
+    },
   },
 };
 

packages/icon/src/Icon.tsx

@@ -1,5 +1,5 @@
 import { createIconComponent } from './createIconComponent';
-import { glyphs } from './glyphs';
+import * as glyphs from './generated';
 
 /**
  * The LeafyGreen system icons are a clear and concise set of glyphs designed to be minimal in form and express a variety of use-cases for all digital products.

packages/icon/src/createGlyphComponent.tsx

@@ -1,11 +1,13 @@
 import React from 'react';
 
 import { css, cx } from '@leafygreen-ui/emotion';
+import { useIdAllocator } from '@leafygreen-ui/hooks';
 
 import { generateAccessibleProps, Size, sizeMap } from './glyphCommon';
 import { LGGlyph, SVGR } from './types';
 
 /**
+ * @deprecated - No longer needed for icon generation. Keeping it for backwards compatibility.
  * Returns a single glyph component.
  * Process custom glyphs to ensure consistent behavior between custom and built-in icons
  * @param glyphName: string - the display name of the icon
@@ -26,6 +28,7 @@ export function createGlyphComponent(
     role = 'img',
     ...rest
   }: LGGlyph.ComponentProps) => {
+    const titleId = useIdAllocator({ prefix: 'icon-title' });
     const fillStyle = css`
       color: ${fill};
     `;
@@ -51,6 +54,7 @@ export function createGlyphComponent(
         role={role}
         {...generateAccessibleProps(role, glyphName, {
           title,
+          titleId,
           ['aria-label']: ariaLabel,
           ['aria-labelledby']: ariaLabelledby,
         })}