Navigation: Don't round app plugin icon images (#54543)

* Navigation: Don't round app plugin icon images

* round icon

.betterer.results

@@ -2740,12 +2740,10 @@ exports[`better eslint`] = {
     "public/app/core/components/NavBar/NavBarItemMenuTrigger.tsx:5381": [
       [0, 0, 0, "Do not use any type assertions.", "0"],
       [0, 0, 0, "Do not use any type assertions.", "1"],
-      [0, 0, 0, "Do not use any type assertions.", "2"],
+      [0, 0, 0, "Do not use any type assertions.", "2"]
-      [0, 0, 0, "Do not use any type assertions.", "3"]
     ],
     "public/app/core/components/NavBar/NavBarMenu.tsx:5381": [
-      [0, 0, 0, "Do not use any type assertions.", "0"],
+      [0, 0, 0, "Do not use any type assertions.", "0"]
-      [0, 0, 0, "Do not use any type assertions.", "1"]
     ],
     "public/app/core/components/OptionsUI/DashboardPicker.tsx:5381": [
       [0, 0, 0, "Unexpected any. Specify a different type.", "0"]

packages/grafana-data/src/types/navModel.ts

@@ -14,6 +14,8 @@ export interface NavLinkDTO {
   divider?: boolean;
   hideFromMenu?: boolean;
   hideFromTabs?: boolean;
+  showIconInNavbar?: boolean;
+  roundIcon?: boolean;
   children?: NavLinkDTO[];
   highlightText?: string;
   emptyMessageId?: string;
@@ -30,7 +32,6 @@ export interface NavModelItem extends NavLinkDTO {
   highlightText?: string;
   highlightId?: string;
   tabSuffix?: ComponentType<{ className?: string }>;
-  showIconInNavbar?: boolean;
   hideFromBreadcrumbs?: boolean;
 }
 

packages/grafana-ui/src/components/Icon/Icon.tsx

@@ -6,6 +6,7 @@ import { GrafanaTheme2 } from '@grafana/data';
 
 import { useStyles2 } from '../../themes/ThemeContext';
 import { IconName, IconType, IconSize } from '../../types/icon';
+export { toIconName } from '../../types/icon';
 
 import { cacheInitialized, initIconCache, iconRoot } from './iconBundle';
 import { getIconSubDir, getSvgSize } from './utils';

packages/grafana-ui/src/components/index.ts

@@ -6,7 +6,7 @@ import { AsyncSelect, Select } from './Forms/Legacy/Select/Select';
 import { Switch } from './Forms/Legacy/Switch/Switch';
 import { SecretFormField } from './SecretFormField/SecretFormField';
 
-export { Icon } from './Icon/Icon';
+export { Icon, toIconName } from './Icon/Icon';
 export { IconButton, type IconButtonVariant } from './IconButton/IconButton';
 export { ConfirmButton } from './ConfirmButton/ConfirmButton';
 export { DeleteButton } from './ConfirmButton/DeleteButton';

packages/grafana-ui/src/types/icon.ts

@@ -5,192 +5,202 @@ import { ComponentSize } from './size';
 export type IconType = 'mono' | 'default' | 'solid';
 export type IconSize = ComponentSize | 'xl' | 'xxl' | 'xxxl';
 
-export const getAvailableIcons = () =>
+const avaibleBrandIcons = [
-  [
+  'google' as const,
-    'anchor',
+  'microsoft' as const,
-    'angle-double-down',
+  'github' as const,
-    'angle-double-right',
+  'gitlab' as const,
-    'angle-double-up',
+  'okta' as const,
-    'angle-down',
+];
-    'angle-left',
-    'angle-right',
-    'angle-up',
-    'apps',
-    'arrow',
-    'arrow-down',
-    'arrow-from-right',
-    'arrow-left',
-    'arrow-random',
-    'arrow-right',
-    'arrow-up',
-    'arrows-h',
-    'arrows-v',
-    'backward',
-    'bars',
-    'bell',
-    'bell-slash',
-    'bolt',
-    'book',
-    'bookmark',
-    'book-open',
-    'brackets-curly',
-    'building',
-    'bug',
-    'building',
-    'calculator-alt',
-    'calendar-alt',
-    'camera',
-    'capture',
-    'channel-add',
-    'chart-line',
-    'check',
-    'check-circle',
-    'circle',
-    'clipboard-alt',
-    'clock-nine',
-    'cloud',
-    'cloud-download',
-    'cloud-upload',
-    'code-branch',
-    'cog',
-    'columns',
-    'comment-alt',
-    'comment-alt-message',
-    'comment-alt-share',
-    'comments-alt',
-    'compass',
-    'copy',
-    'credit-card',
-    'cube',
-    'dashboard',
-    'database',
-    'document-info',
-    'download-alt',
-    'draggabledots',
-    'edit',
-    'ellipsis-v',
-    'envelope',
-    'exchange-alt',
-    'exclamation-triangle',
-    'exclamation-circle',
-    'external-link-alt',
-    'eye',
-    'eye-slash',
-    'ellipsis-h',
-    'fa fa-spinner',
-    'favorite',
-    'file-alt',
-    'file-blank',
-    'file-copy-alt',
-    'filter',
-    'folder',
-    'font',
-    'fire',
-    'folder-open',
-    'folder-plus',
-    'folder-upload',
-    'forward',
-    'gf-bar-alignment-after',
-    'gf-bar-alignment-before',
-    'gf-bar-alignment-center',
-    'gf-glue',
-    'gf-grid',
-    'gf-interpolation-linear',
-    'gf-interpolation-smooth',
-    'gf-interpolation-step-after',
-    'gf-interpolation-step-before',
-    'gf-landscape',
-    'gf-layout-simple',
-    'gf-logs',
-    'gf-portrait',
-    'grafana',
-    'graph-bar',
-    'heart',
-    'heart-break',
-    'history',
-    'home',
-    'home-alt',
-    'horizontal-align-center',
-    'horizontal-align-left',
-    'horizontal-align-right',
-    'hourglass',
-    'import',
-    'info',
-    'info-circle',
-    'key-skeleton-alt',
-    'keyboard',
-    'layer-group',
-    'library-panel',
-    'line-alt',
-    'link',
-    'list-ui-alt',
-    'list-ul',
-    'lock',
-    'map-marker',
-    'message',
-    'minus',
-    'minus-circle',
-    'mobile-android',
-    'monitor',
-    'palette',
-    'panel-add',
-    'pause',
-    'pen',
-    'percentage',
-    'play',
-    'plug',
-    'plus',
-    'plus-circle',
-    'plus-square',
-    'power',
-    'presentation-play',
-    'process',
-    'question-circle',
-    'record-audio',
-    'repeat',
-    'rocket',
-    'ruler-combined',
-    'save',
-    'search',
-    'search-minus',
-    'search-plus',
-    'share-alt',
-    'shield',
-    'shield-exclamation',
-    'signal',
-    'signin',
-    'signout',
-    'sitemap',
-    'slack',
-    'sliders-v-alt',
-    'sort-amount-down',
-    'sort-amount-up',
-    'square-shape',
-    'star',
-    'step-backward',
-    'stopwatch-slash',
-    'sync',
-    'table',
-    'tag-alt',
-    'text-fields',
-    'times',
-    'toggle-on',
-    'trash-alt',
-    'unlock',
-    'upload',
-    'user',
-    'users-alt',
-    'vertical-align-bottom',
-    'vertical-align-center',
-    'vertical-align-top',
-    'wrap-text',
-    'rss',
-    'x',
-  ] as const;
 
-type BrandIconNames = 'google' | 'microsoft' | 'github' | 'gitlab' | 'okta';
+const availableIcons = [
+  'anchor' as const,
+  'angle-double-down' as const,
+  'angle-double-right' as const,
+  'angle-double-up' as const,
+  'angle-down' as const,
+  'angle-left' as const,
+  'angle-right' as const,
+  'angle-up' as const,
+  'apps' as const,
+  'arrow' as const,
+  'arrow-down' as const,
+  'arrow-from-right' as const,
+  'arrow-left' as const,
+  'arrow-random' as const,
+  'arrow-right' as const,
+  'arrow-up' as const,
+  'arrows-h' as const,
+  'arrows-v' as const,
+  'backward' as const,
+  'bars' as const,
+  'bell' as const,
+  'bell-slash' as const,
+  'bolt' as const,
+  'book' as const,
+  'bookmark' as const,
+  'book-open' as const,
+  'brackets-curly' as const,
+  'building' as const,
+  'bug' as const,
+  'building' as const,
+  'calculator-alt' as const,
+  'calendar-alt' as const,
+  'camera' as const,
+  'capture' as const,
+  'channel-add' as const,
+  'chart-line' as const,
+  'check' as const,
+  'check-circle' as const,
+  'circle' as const,
+  'clipboard-alt' as const,
+  'clock-nine' as const,
+  'cloud' as const,
+  'cloud-download' as const,
+  'cloud-upload' as const,
+  'code-branch' as const,
+  'cog' as const,
+  'columns' as const,
+  'comment-alt' as const,
+  'comment-alt-message' as const,
+  'comment-alt-share' as const,
+  'comments-alt' as const,
+  'compass' as const,
+  'copy' as const,
+  'credit-card' as const,
+  'cube' as const,
+  'dashboard' as const,
+  'database' as const,
+  'document-info' as const,
+  'download-alt' as const,
+  'draggabledots' as const,
+  'edit' as const,
+  'ellipsis-v' as const,
+  'envelope' as const,
+  'exchange-alt' as const,
+  'exclamation-triangle' as const,
+  'exclamation-circle' as const,
+  'external-link-alt' as const,
+  'eye' as const,
+  'eye-slash' as const,
+  'ellipsis-h' as const,
+  'fa fa-spinner' as const,
+  'favorite' as const,
+  'file-alt' as const,
+  'file-blank' as const,
+  'file-copy-alt' as const,
+  'filter' as const,
+  'folder' as const,
+  'font' as const,
+  'fire' as const,
+  'folder-open' as const,
+  'folder-plus' as const,
+  'folder-upload' as const,
+  'forward' as const,
+  'gf-bar-alignment-after' as const,
+  'gf-bar-alignment-before' as const,
+  'gf-bar-alignment-center' as const,
+  'gf-glue' as const,
+  'gf-grid' as const,
+  'gf-interpolation-linear' as const,
+  'gf-interpolation-smooth' as const,
+  'gf-interpolation-step-after' as const,
+  'gf-interpolation-step-before' as const,
+  'gf-landscape' as const,
+  'gf-layout-simple' as const,
+  'gf-logs' as const,
+  'gf-portrait' as const,
+  'grafana' as const,
+  'graph-bar' as const,
+  'heart' as const,
+  'heart-break' as const,
+  'history' as const,
+  'home' as const,
+  'home-alt' as const,
+  'horizontal-align-center' as const,
+  'horizontal-align-left' as const,
+  'horizontal-align-right' as const,
+  'hourglass' as const,
+  'import' as const,
+  'info' as const,
+  'info-circle' as const,
+  'key-skeleton-alt' as const,
+  'keyboard' as const,
+  'layer-group' as const,
+  'library-panel' as const,
+  'line-alt' as const,
+  'link' as const,
+  'list-ui-alt' as const,
+  'list-ul' as const,
+  'lock' as const,
+  'map-marker' as const,
+  'message' as const,
+  'minus' as const,
+  'minus-circle' as const,
+  'mobile-android' as const,
+  'monitor' as const,
+  'palette' as const,
+  'panel-add' as const,
+  'pause' as const,
+  'pen' as const,
+  'percentage' as const,
+  'play' as const,
+  'plug' as const,
+  'plus' as const,
+  'plus-circle' as const,
+  'plus-square' as const,
+  'power' as const,
+  'presentation-play' as const,
+  'process' as const,
+  'question-circle' as const,
+  'record-audio' as const,
+  'repeat' as const,
+  'rocket' as const,
+  'ruler-combined' as const,
+  'save' as const,
+  'search' as const,
+  'search-minus' as const,
+  'search-plus' as const,
+  'share-alt' as const,
+  'shield' as const,
+  'shield-exclamation' as const,
+  'signal' as const,
+  'signin' as const,
+  'signout' as const,
+  'sitemap' as const,
+  'slack' as const,
+  'sliders-v-alt' as const,
+  'sort-amount-down' as const,
+  'sort-amount-up' as const,
+  'square-shape' as const,
+  'star' as const,
+  'step-backward' as const,
+  'stopwatch-slash' as const,
+  'sync' as const,
+  'table' as const,
+  'tag-alt' as const,
+  'text-fields' as const,
+  'times' as const,
+  'toggle-on' as const,
+  'trash-alt' as const,
+  'unlock' as const,
+  'upload' as const,
+  'user' as const,
+  'users-alt' as const,
+  'vertical-align-bottom' as const,
+  'vertical-align-center' as const,
+  'vertical-align-top' as const,
+  'wrap-text' as const,
+  'rss' as const,
+  'x' as const,
+];
 
-export type IconName = ReturnType<typeof getAvailableIcons>[number] | BrandIconNames;
+// function remains for backwards compatibility
+export const getAvailableIcons = () => availableIcons;
+
+type BrandIconNames = typeof avaibleBrandIcons;
+
+export type IconName = ReturnType<typeof getAvailableIcons>[number] | BrandIconNames[number];
 
 /** Get the icon for a given field type */
 export function getFieldTypeIcon(field?: Field): IconName {
@@ -214,3 +224,18 @@ export function getFieldTypeIcon(field?: Field): IconName {
   }
   return 'question-circle';
 }
+
+function isValidIconName(iconName: string): iconName is IconName {
+  const namedIcons: string[] = availableIcons;
+  const brandIcons: string[] = avaibleBrandIcons;
+
+  return namedIcons.includes(iconName) || brandIcons.includes(iconName);
+}
+
+export function toIconName(iconName: string): IconName | undefined {
+  if (isValidIconName(iconName)) {
+    return iconName;
+  }
+
+  return undefined;
+}

pkg/api/dtos/index.go

@@ -71,6 +71,7 @@ type NavLink struct {
 	HideFromMenu     bool       `json:"hideFromMenu,omitempty"`
 	HideFromTabs     bool       `json:"hideFromTabs,omitempty"`
 	ShowIconInNavbar bool       `json:"showIconInNavbar,omitempty"`
+	RoundIcon        bool       `json:"roundIcon,omitempty"`
 	Children         []*NavLink `json:"children,omitempty"`
 	HighlightText    string     `json:"highlightText,omitempty"`
 	HighlightID      string     `json:"highlightId,omitempty"`

pkg/api/index.go

@@ -74,6 +74,7 @@ func (hs *HTTPServer) getProfileNode(c *models.ReqContext) *dtos.NavLink {
 		Section:    dtos.NavSectionConfig,
 		SortWeight: dtos.WeightProfile,
 		Children:   children,
+		RoundIcon:  true,
 	}
 }
 

public/app/core/components/NavBar/NavBar.tsx

@@ -8,13 +8,13 @@ import { useLocation } from 'react-router-dom';
 import { GrafanaTheme2, NavModelItem, NavSection } from '@grafana/data';
 import { config, locationService, reportInteraction } from '@grafana/runtime';
 import { Icon, useTheme2 } from '@grafana/ui';
-import { Branding } from 'app/core/components/Branding/Branding';
 import { getKioskMode } from 'app/core/navigation/kiosk';
 import { KioskMode, StoreState } from 'app/types';
 
 import { OrgSwitcher } from '../OrgSwitcher';
 
 import NavBarItem from './NavBarItem';
+import { NavBarItemIcon } from './NavBarItemIcon';
 import { NavBarItemWithoutMenu } from './NavBarItemWithoutMenu';
 import { NavBarMenu } from './NavBarMenu';
 import { NavBarMenuPortalContainer } from './NavBarMenuPortalContainer';
@@ -121,7 +121,7 @@ export const NavBar = React.memo(() => {
               url={homeItem.url}
               onClick={homeItem.onClick}
             >
-              <Branding.MenuLogo />
+              <NavBarItemIcon link={homeItem} />
             </NavBarItemWithoutMenu>
 
             <NavBarScrollContainer>

public/app/core/components/NavBar/NavBarItemIcon.tsx

@@ -0,0 +1,38 @@
+import { css, cx } from '@emotion/css';
+import React from 'react';
+
+import { GrafanaTheme2, NavModelItem } from '@grafana/data';
+import { Icon, toIconName, useTheme2 } from '@grafana/ui';
+
+import { Branding } from '../Branding/Branding';
+
+interface NavBarItemIconProps {
+  link: NavModelItem;
+}
+
+export function NavBarItemIcon({ link }: NavBarItemIconProps) {
+  const theme = useTheme2();
+  const styles = getStyles(theme);
+
+  if (link.icon === 'grafana') {
+    return <Branding.MenuLogo className={styles.img} />;
+  } else if (link.icon) {
+    const iconName = toIconName(link.icon);
+    return <Icon name={iconName ?? 'link'} size="xl" />;
+  } else {
+    // consumer of NavBarItemIcon gives enclosing element an appropriate label
+    return <img className={cx(styles.img, link.roundIcon && styles.round)} src={link.img} alt="" />;
+  }
+}
+
+function getStyles(theme: GrafanaTheme2) {
+  return {
+    img: css({
+      height: theme.spacing(3),
+      width: theme.spacing(3),
+    }),
+    round: css({
+      borderRadius: '50%',
+    }),
+  };
+}

public/app/core/components/NavBar/NavBarItemMenuTrigger.tsx

@@ -11,8 +11,9 @@ import React, { ReactElement, useEffect, useState } from 'react';
 
 import { GrafanaTheme2, NavModelItem } from '@grafana/data';
 import { reportExperimentView } from '@grafana/runtime';
-import { Icon, IconName, Link, useTheme2 } from '@grafana/ui';
+import { Link, useTheme2 } from '@grafana/ui';
 
+import { NavBarItemIcon } from './NavBarItemIcon';
 import { getNavMenuPortalContainer } from './NavBarMenuPortalContainer';
 import { NavFeatureHighlight } from './NavFeatureHighlight';
 import { NavBarItemMenuContext, useNavBarContext } from './context';
@@ -92,8 +93,7 @@ export function NavBarItemMenuTrigger(props: NavBarItemMenuTriggerProps): ReactE
   const itemContent = (
     <Wrapper>
       <span className={styles.icon}>
-        {item?.icon && <Icon name={item.icon as IconName} size="xl" />}
+        <NavBarItemIcon link={item} />
-        {item?.img && <img src={item.img} alt={`${item.text} logo`} />}
       </span>
     </Wrapper>
   );
@@ -250,11 +250,5 @@ const getStyles = (theme: GrafanaTheme2, isActive?: boolean) => ({
   icon: css({
     height: '100%',
     width: '100%',
-
-    img: {
-      borderRadius: '50%',
-      height: theme.spacing(3),
-      width: theme.spacing(3),
-    },
   }),
 });

public/app/core/components/NavBar/NavBarItemWithoutMenu.tsx

@@ -112,12 +112,6 @@ export function getNavBarItemWithoutMenuStyles(theme: GrafanaTheme2, isActive?:
     icon: css({
       height: '100%',
       width: '100%',
-
-      img: {
-        borderRadius: '50%',
-        height: theme.spacing(3),
-        width: theme.spacing(3),
-      },
     }),
   };
 }

public/app/core/components/NavBar/NavBarMenu.tsx

@@ -11,8 +11,7 @@ import { GrafanaTheme2, NavModelItem } from '@grafana/data';
 import { reportInteraction } from '@grafana/runtime';
 import { CollapsableSection, CustomScrollbar, Icon, IconButton, IconName, useStyles2, useTheme2 } from '@grafana/ui';
 
-import { Branding } from '../Branding/Branding';
+import { NavBarItemIcon } from './NavBarItemIcon';
-
 import { NavBarItemWithoutMenu } from './NavBarItemWithoutMenu';
 import { NavBarMenuItem } from './NavBarMenuItem';
 import { NavBarToggle } from './NavBarToggle';
@@ -286,7 +285,9 @@ export function NavItem({
         >
           <div className={styles.itemWithoutMenuContent}>
             <div className={styles.iconContainer}>
-              <FeatureHighlightWrapper>{getLinkIcon(link)}</FeatureHighlightWrapper>
+              <FeatureHighlightWrapper>
+                <NavBarItemIcon link={link} />
+              </FeatureHighlightWrapper>
             </div>
             <span className={styles.linkText}>{link.text}</span>
           </div>
@@ -377,7 +378,9 @@ function CollapsibleNavItem({
         className={styles.collapsibleMenuItem}
         elClassName={styles.collapsibleIcon}
       >
-        <FeatureHighlightWrapper>{getLinkIcon(link)}</FeatureHighlightWrapper>
+        <FeatureHighlightWrapper>
+          <NavBarItemIcon link={link} />
+        </FeatureHighlightWrapper>
       </NavBarItemWithoutMenu>
       <div className={styles.collapsibleSectionWrapper}>
         <CollapsableSection
@@ -456,13 +459,3 @@ const getCollapsibleStyles = (theme: GrafanaTheme2) => ({
 function linkHasChildren(link: NavModelItem): link is NavModelItem & { children: NavModelItem[] } {
   return Boolean(link.children && link.children.length > 0);
 }
-
-function getLinkIcon(link: NavModelItem) {
-  if (link.icon === 'grafana') {
-    return <Branding.MenuLogo />;
-  } else if (link.icon) {
-    return <Icon name={link.icon as IconName} size="xl" />;
-  } else {
-    return <img src={link.img} alt={`${link.text} logo`} height="24" width="24" style={{ borderRadius: '50%' }} />;
-  }
-}

public/app/core/components/NavBar/NavBarMenuItem.tsx

@@ -31,10 +31,13 @@ export function NavBarMenuItem({
   const theme = useTheme2();
   const styles = getStyles(theme, isActive);
   const elStyle = cx(styles.element, styleOverrides);
+
   const linkContent = (
     <div className={styles.linkContent}>
       {icon && <Icon data-testid="dropdown-child-icon" name={icon} />}
+
       <div className={styles.linkText}>{text}</div>
+
       {target === '_blank' && (
         <Icon data-testid="external-link-icon" name="external-link-alt" className={styles.externalLinkIcon} />
       )}