Merge pull request #33193 from backstage/rugvip/nfs-icon-sizing

frontend-app-api: align icon sizing in nfs

.changeset/nfs-icon-alpha-plugins.md

@@ -0,0 +1,12 @@
+---
+'@backstage/plugin-api-docs': patch
+'@backstage/plugin-catalog': patch
+'@backstage/plugin-catalog-unprocessed-entities': patch
+'@backstage/plugin-devtools': patch
+'@backstage/plugin-scaffolder': patch
+'@backstage/plugin-search': patch
+'@backstage/plugin-techdocs': patch
+'@backstage/plugin-user-settings': patch
+---
+
+Updated alpha plugin icons to follow the new frontend icon sizing rules when rendered in plugin and navigation surfaces.

.changeset/nfs-icon-foundations.md

@@ -0,0 +1,7 @@
+---
+'@backstage/core-components': patch
+'@backstage/frontend-app-api': patch
+'@backstage/frontend-plugin-api': patch
+---
+
+Clarified the `IconElement` sizing contract for the new frontend system and aligned legacy system icon rendering with the new icon API.

.changeset/nfs-icon-plugin-app.md

@@ -0,0 +1,5 @@
+---
+'@backstage/plugin-app': patch
+---
+
+The app nav now falls back to `plugin.icon` for navigation items that don't have an explicit icon set.

packages/core-components/src/layout/Sidebar/Items.tsx

@@ -116,6 +116,17 @@ const makeSidebarStyles = (sidebarConfig: SidebarConfig) =>
         font: 'inherit',
         textTransform: 'none',
       },
+      itemIcon: {
+        display: 'inline-flex',
+        fontSize: theme.typography.fontSize,
+        lineHeight: 0,
+        '& svg': {
+          width: '1.5em',
+          height: '1.5em',
+          fontSize: 'inherit',
+          flexShrink: 0,
+        },
+      },
       closed: {
         width: sidebarConfig.drawerWidthClosed,
         justifyContent: 'center',
@@ -401,7 +412,9 @@ const SidebarItemBase = forwardRef<
 
   const displayItemIcon = (
     <Box style={divStyle}>
-      <Icon fontSize="small" />
+      <Box component="span" className={classes.itemIcon}>
+        <Icon fontSize="inherit" />
+      </Box>
       {!isOpen && hasSubmenu ? <ArrowRightIcon fontSize="small" /> : <></>}
     </Box>
   );

packages/frontend-app-api/src/apis/implementations/IconsApi/DefaultIconsApi.test.ts

@@ -59,6 +59,8 @@ describe('DefaultIconsApi', () => {
     expect(result).toBeTruthy();
     // @ts-expect-error accessing internal React element structure
     expect(result.type).toBe(MyIcon);
+    // @ts-expect-error accessing internal React element structure
+    expect(result.props.fontSize).toBe('inherit');
   });
 
   it('should wrap IconElement values in a component for getIcon()', () => {
@@ -69,10 +71,43 @@ describe('DefaultIconsApi', () => {
     expect(icon).toBeDefined();
     expect(typeof icon).toBe('function');
     // @ts-expect-error testing runtime behavior
-    expect(icon({})).toBe(element);
+    const result = icon({});
+    expect(result.type).toBe('span');
+    expect(result.props.style).toEqual({ fontSize: '1.5rem' });
+    expect(result.props.children).toBe(element.props.children);
     expect(api.getIcon('myIcon')).toBe(icon);
   });
 
+  it('should honor fontSize for getIcon()', () => {
+    const element = createElement('svg');
+    const api = new DefaultIconsApi({ myIcon: element });
+    const icon = api.getIcon('myIcon');
+
+    // @ts-expect-error testing runtime behavior
+    const result = icon({ fontSize: 'small' });
+    expect(result.props.style.fontSize).toBe('1.25rem');
+  });
+
+  it('should forward runtime props to the original icon element', () => {
+    const element = createElement('svg', {
+      className: 'existing',
+      style: { color: 'red' },
+    });
+    const api = new DefaultIconsApi({ myIcon: element });
+    const icon = api.getIcon('myIcon');
+
+    // @ts-expect-error testing runtime behavior
+    const result = icon({ className: 'extra', style: { width: '2em' } });
+
+    expect(result.type).toBe('svg');
+    expect(result.props.className).toBe('existing extra');
+    expect(result.props.style).toEqual({
+      color: 'red',
+      fontSize: '1.5rem',
+      width: '2em',
+    });
+  });
+
   it('should wrap null IconElement in a component for getIcon()', () => {
     const api = new DefaultIconsApi({ empty: null });
 

packages/frontend-app-api/src/apis/implementations/IconsApi/DefaultIconsApi.ts

@@ -19,7 +19,22 @@ import {
   IconElement,
   IconsApi,
 } from '@backstage/frontend-plugin-api';
-import { createElement, isValidElement } from 'react';
+import { cloneElement, createElement, isValidElement } from 'react';
+
+const legacyFontSizeMap = {
+  inherit: 'inherit',
+  small: '1.25rem',
+  medium: '1.5rem',
+  large: '2.1875rem',
+} as const;
+
+function mergeClassNames(...classNames: Array<string | undefined>) {
+  const merged = classNames.filter(Boolean).join(' ');
+  if (merged) {
+    return merged;
+  }
+  return undefined;
+}
 
 /**
  * Implementation for the {@link IconsApi}
@@ -39,7 +54,10 @@ export class DefaultIconsApi implements IconsApi {
           return [key, value];
         }
         deprecatedKeys.push(key);
-        return [key, createElement(value as IconComponent)];
+        return [
+          key,
+          createElement(value as IconComponent, { fontSize: 'inherit' }),
+        ];
       }),
     );
 
@@ -65,7 +83,37 @@ export class DefaultIconsApi implements IconsApi {
     if (el === undefined) {
       return undefined;
     }
-    component = () => el;
+    component = props => {
+      if (el === null) {
+        return null;
+      }
+
+      const {
+        fontSize = 'medium',
+        className,
+        style,
+        ...rest
+      } = props as {
+        fontSize?: keyof typeof legacyFontSizeMap;
+        className?: string;
+        style?: Record<string, unknown>;
+      } & Record<string, unknown>;
+
+      const elementProps = el.props as {
+        className?: string;
+        style?: Record<string, unknown>;
+      };
+
+      return cloneElement(el, {
+        ...rest,
+        className: mergeClassNames(elementProps.className, className),
+        style: {
+          ...elementProps.style,
+          fontSize: legacyFontSizeMap[fontSize],
+          ...style,
+        },
+      });
+    };
     this.#components.set(key, component);
     return component;
   }

packages/frontend-plugin-api/src/icons/types.ts

@@ -42,7 +42,14 @@ export type IconComponent = ComponentType<{
  *
  * @remarks
  *
- * Icons should be exactly 24x24 pixels in size.
+ * Icon elements should behave like rendering a plain icon directly, for example
+ * from `@remixicon/react`, and are expected to be sized by the surrounding UI.
+ * Icons should be exactly 24x24 pixels in size by default.
+ *
+ * Using icons from `@remixicon/react` is preferred. Using icons from
+ * `@material-ui/icons` or `AppIcon` and its variants from
+ * `@backstage/core-components` is supported while migrating, but deprecated.
+ * When using those icons, you must set `fontSize="inherit"` on the element.
  *
  * @public
  */

plugins/api-docs/src/alpha.tsx

@@ -43,7 +43,7 @@ const apiDocsNavItem = NavItemBlueprint.make({
   params: {
     title: 'APIs',
     routeRef: rootRoute,
-    icon: () => <AppIcon id="kind:api" />,
+    icon: () => <AppIcon fontSize="inherit" id="kind:api" />,
   },
 });
 
@@ -211,7 +211,7 @@ const apiDocsApisEntityContent = EntityContentBlueprint.make({
 export default createFrontendPlugin({
   pluginId: 'api-docs',
   title: 'APIs',
-  icon: <AppIcon id="kind:api" />,
+  icon: <AppIcon fontSize="inherit" id="kind:api" />,
   info: { packageJson: () => import('../package.json') },
   routes: {
     root: rootRoute,

plugins/app/src/extensions/AppNav.tsx

@@ -172,6 +172,7 @@ function NavContentRenderer(props: {
       // We want the priority: page (config/params) -> nav item -> plugin -> pluginId
       const resolvedTitle = node.instance.getData(coreExtensionData.title);
       const pluginTitle = node.spec.plugin.title;
+      const pluginIcon = node.spec.plugin.icon;
       const pluginId = node.spec.plugin.pluginId;
       const hasExplicitPageTitle =
         resolvedTitle !== undefined &&
@@ -194,6 +195,8 @@ function NavContentRenderer(props: {
         icon = <NavItemIcon />;
       } else if (resolvedIcon) {
         icon = resolvedIcon;
+      } else if (pluginIcon) {
+        icon = pluginIcon;
       }
 
       if (!title || !icon) {

plugins/catalog-unprocessed-entities/src/alpha/plugin.tsx

@@ -69,7 +69,7 @@ export const catalogUnprocessedEntitiesNavItem = NavItemBlueprint.make({
 export default createFrontendPlugin({
   pluginId: 'catalog-unprocessed-entities',
   title: 'Unprocessed Entities',
-  icon: <QueueIcon />,
+  icon: <QueueIcon fontSize="inherit" />,
   info: { packageJson: () => import('../../package.json') },
   routes: {
     root: rootRouteRef,

plugins/catalog/src/alpha/pages.tsx

@@ -59,7 +59,7 @@ export const catalogPage = PageBlueprint.makeWithOverrides({
     return originalFactory({
       path: '/catalog',
       routeRef: rootRouteRef,
-      icon: <CategoryIcon />,
+      icon: <CategoryIcon fontSize="inherit" />,
       title: 'Catalog',
       loader: async () => {
         const { BaseCatalogPage } = await import('../components/CatalogPage');

plugins/catalog/src/alpha/plugin.tsx

@@ -40,7 +40,7 @@ import contextMenuItems from './contextMenuItems';
 export default createFrontendPlugin({
   pluginId: 'catalog',
   title: 'Catalog',
-  icon: <CategoryIcon />,
+  icon: <CategoryIcon fontSize="inherit" />,
   info: {
     packageJson: () => import('../../package.json'),
   },

plugins/devtools/src/alpha/plugin.tsx

@@ -89,7 +89,7 @@ export const devToolsNavItem = NavItemBlueprint.make({
 export default createFrontendPlugin({
   pluginId: 'devtools',
   title: 'DevTools',
-  icon: <BuildIcon />,
+  icon: <BuildIcon fontSize="inherit" />,
   info: { packageJson: () => import('../../package.json') },
   routes: {
     root: rootRouteRef,

plugins/scaffolder/src/alpha/plugin.tsx

@@ -61,7 +61,7 @@ const scaffolderEntityIconLink = EntityIconLinkBlueprint.make({
 export default createFrontendPlugin({
   pluginId: 'scaffolder',
   title: 'Create',
-  icon: <CreateComponentIcon />,
+  icon: <CreateComponentIcon fontSize="inherit" />,
   info: { packageJson: () => import('../../package.json') },
   routes: {
     root: rootRouteRef,

plugins/search/src/alpha.tsx

@@ -277,7 +277,7 @@ export const searchNavItem = NavItemBlueprint.make({
 export default createFrontendPlugin({
   pluginId: 'search',
   title: 'Search',
-  icon: <SearchIcon />,
+  icon: <SearchIcon fontSize="inherit" />,
   info: { packageJson: () => import('../package.json') },
   extensions: [searchApi, searchPage, searchNavItem],
   routes: {

plugins/techdocs/src/alpha/index.tsx

@@ -280,7 +280,7 @@ const techDocsNavItem = NavItemBlueprint.make({
 export default createFrontendPlugin({
   pluginId: 'techdocs',
   title: 'Docs',
-  icon: <LibraryBooks />,
+  icon: <LibraryBooks fontSize="inherit" />,
   info: { packageJson: () => import('../../package.json') },
   extensions: [
     techDocsClientApi,

plugins/user-settings/src/alpha.tsx

@@ -63,7 +63,7 @@ export const settingsNavItem = NavItemBlueprint.make({
 export default createFrontendPlugin({
   pluginId: 'user-settings',
   title: 'Settings',
-  icon: <SettingsIcon />,
+  icon: <SettingsIcon fontSize="inherit" />,
   info: { packageJson: () => import('../package.json') },
   extensions: [userSettingsPage, settingsNavItem],
   routes: {