From 754f3d83b8b43df9e2e73a1d051b0b574b9b7207 Mon Sep 17 00:00:00 2001
From: "github-actions[bot]" <github-actions[bot]@users.noreply.github.com>
Date: Fri, 8 Dec 2023 02:07:30 +0000
Subject: [PATCH] [Enhance] Implement Validation for Icon Input Source & Set
 Default Icon to `Beaker` (#1137)

* Revise the logic to handle default icon

Signed-off-by: Willie Hung <willie880201044@gmail.com>

* Categorize different input icon types

Signed-off-by: Willie Hung <willie880201044@gmail.com>

* Remove unused code

Signed-off-by: Willie Hung <willie880201044@gmail.com>

* Update CHANGELOG.md

Signed-off-by: Willie Hung <willie880201044@gmail.com>

* Use readable parameter name

Signed-off-by: Willie Hung <willie880201044@gmail.com>

* Handle different input cases and add tests

Signed-off-by: Willie Hung <willie880201044@gmail.com>

---------

Signed-off-by: Willie Hung <willie880201044@gmail.com>
Co-authored-by: Josh Romero <rmerqg@amazon.com>
(cherry picked from commit 0d3b6ccf04f514c26bf3eeca085f286bd85b8800)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

# Conflicts:
#	CHANGELOG.md
---
 .../icon/__snapshots__/icon.test.tsx.snap     | 52 +++++++++++++++++-
 src/components/icon/icon.test.tsx             | 55 ++++++++++++++-----
 src/components/icon/icon.tsx                  | 52 ++++++++++--------
 3 files changed, 121 insertions(+), 38 deletions(-)

diff --git a/src/components/icon/__snapshots__/icon.test.tsx.snap b/src/components/icon/__snapshots__/icon.test.tsx.snap
index 8ae3b0f871..2480d0d500 100644
--- a/src/components/icon/__snapshots__/icon.test.tsx.snap
+++ b/src/components/icon/__snapshots__/icon.test.tsx.snap
@@ -10184,7 +10184,7 @@ exports[`OuiIcon props type wrench is rendered 1`] = `
 </svg>
 `;
 
-exports[`OuiIcon renders custom components 1`] = `
+exports[`OuiIcon render different types of icons renders custom components 1`] = `
 <span
   aria-hidden="true"
   aria-label="heart"
@@ -10195,3 +10195,53 @@ exports[`OuiIcon renders custom components 1`] = `
   ❤️
 </span>
 `;
+
+exports[`OuiIcon render different types of icons renders custom svg from absolute url 1`] = `
+<img
+  alt=""
+  class="ouiIcon ouiIcon--medium"
+  src="https://upload.wikimedia.org/wikipedia/commons/0/02/SVG_logo.svg"
+/>
+`;
+
+exports[`OuiIcon render different types of icons renders custom svg from relative url 1`] = `
+<img
+  alt=""
+  class="ouiIcon ouiIcon--medium"
+  src="./assets/beaker.svg"
+/>
+`;
+
+exports[`OuiIcon render different types of icons renders default icon when type is not in OuiIconType 1`] = `
+<svg
+  aria-hidden="true"
+  class="ouiIcon ouiIcon--medium ouiIcon-isLoading"
+  focusable="false"
+  height="16"
+  role="img"
+  viewBox="0 0 16 16"
+  width="16"
+  xmlns="http://www.w3.org/2000/svg"
+>
+  <path
+    d="M5.277 10.088c.02.014.04.03.057.047.582.55 1.134.812 1.666.812.586 0 1.84-.293 3.713-.88L9 6.212V2H7v4.212l-1.723 3.876Zm-.438.987L3.539 14h8.922l-1.32-2.969C9.096 11.677 7.733 12 7 12c-.74 0-1.463-.315-2.161-.925ZM6 2H5V1h6v1h-1v4l3.375 7.594A1 1 0 0 1 12.461 15H3.54a1 1 0 0 1-.914-1.406L6 6V2Z"
+  />
+</svg>
+`;
+
+exports[`OuiIcon render different types of icons renders icon when type is in cache 1`] = `
+<svg
+  aria-hidden="true"
+  class="ouiIcon ouiIcon--medium"
+  focusable="false"
+  height="16"
+  role="img"
+  viewBox="0 0 16 16"
+  width="16"
+  xmlns="http://www.w3.org/2000/svg"
+>
+  <path
+    d="M0 1.994C0 .893.895 0 1.994 0h12.012C15.107 0 16 .895 16 1.994v12.012A1.995 1.995 0 0 1 14.006 16H1.994A1.995 1.995 0 0 1 0 14.006V1.994Zm1 0v12.012c0 .548.446.994.994.994h12.012a.995.995 0 0 0 .994-.994V1.994A.995.995 0 0 0 14.006 1H1.994A.995.995 0 0 0 1 1.994ZM1 4h14v1H1V4Zm1.5-1a.5.5 0 0 1 0-1h1a.5.5 0 0 1 0 1h-1Zm3 0a.5.5 0 0 1 0-1h1a.5.5 0 0 1 0 1h-1Zm4.947 6.106a1 1 0 0 1 0 1.788l-3 2A1 1 0 0 1 6 12V8a1 1 0 0 1 1.447-.894l3 2ZM10 10 7 8v4l3-2Z"
+  />
+</svg>
+`;
diff --git a/src/components/icon/icon.test.tsx b/src/components/icon/icon.test.tsx
index 03faf02305..3ed24098be 100644
--- a/src/components/icon/icon.test.tsx
+++ b/src/components/icon/icon.test.tsx
@@ -135,23 +135,11 @@ describe('OuiIcon', () => {
     });
   });
 
-  it('renders custom components', () => {
-    const CustomIcon = ({ ...props }) => {
-      return (
-        <span role="img" aria-label="heart" {...props}>
-          ❤️
-        </span>
-      );
-    };
-    const component = mount(<OuiIcon type={CustomIcon} />);
-    expect(prettyHtml(component.html())).toMatchSnapshot();
-  });
-
   describe('appendIconComponentCache', () => {
     it('does nothing if not called', () => {
       const component = mount(<OuiIcon type="videoPlayer" />);
       expect(
-        component.find('OuiIcon[type="videoPlayer"] > OuiIconEmpty').length
+        component.find('OuiIcon[type="videoPlayer"] > OuiIconBeaker').length
       ).toBe(1);
     });
 
@@ -172,8 +160,47 @@ describe('OuiIcon', () => {
       });
       const component = mount(<OuiIcon type="accessibility" />);
       expect(
-        component.find('OuiIcon[type="accessibility"] > OuiIconEmpty').length
+        component.find('OuiIcon[type="accessibility"] > OuiIconBeaker').length
       ).toBe(1);
     });
   });
+
+  describe('render different types of icons', () => {
+    it('renders icon when type is in cache', () => {
+      appendIconComponentCache({
+        videoPlayer: OuiIconVideoPlayer,
+      });
+      const component = mount(<OuiIcon type="videoPlayer" />);
+      expect(prettyHtml(component.html())).toMatchSnapshot();
+    });
+
+    it('renders custom svg from absolute url', () => {
+      const component = mount(
+        <OuiIcon type="https://upload.wikimedia.org/wikipedia/commons/0/02/SVG_logo.svg" />
+      );
+      expect(prettyHtml(component.html())).toMatchSnapshot();
+    });
+
+    it('renders custom svg from relative url', () => {
+      const component = mount(<OuiIcon type="./assets/beaker.svg" />);
+      expect(prettyHtml(component.html())).toMatchSnapshot();
+    });
+
+    it('renders default icon when type is not in OuiIconType', () => {
+      const component = mount(<OuiIcon type="foo" />);
+      expect(prettyHtml(component.html())).toMatchSnapshot();
+    });
+
+    it('renders custom components', () => {
+      const CustomIcon = ({ ...props }) => {
+        return (
+          <span role="img" aria-label="heart" {...props}>
+            ❤️
+          </span>
+        );
+      };
+      const component = mount(<OuiIcon type={CustomIcon} />);
+      expect(prettyHtml(component.html())).toMatchSnapshot();
+    });
+  });
 });
diff --git a/src/components/icon/icon.tsx b/src/components/icon/icon.tsx
index d45ee3baa7..45a09d3078 100644
--- a/src/components/icon/icon.tsx
+++ b/src/components/icon/icon.tsx
@@ -42,7 +42,7 @@ import { CommonProps, keysOf } from '../common';
 // because we'd need to dynamically know if we're importing the
 // TS file (dev/docs) or the JS file (distributed), and it's more effort than worth
 // to generate & git track a TS module definition for each icon component
-import { icon as empty } from './assets/empty.js';
+import { icon as defaultIcon } from './assets/beaker.js';
 import { enqueueStateChange } from '../../services/react';
 
 import { htmlIdGenerator } from '../../services';
@@ -582,22 +582,16 @@ interface State {
   neededLoading: boolean; // controls the fade-in animation, cached icons are immediately rendered
 }
 
-function isOuiIconType(x: OuiIconProps['type']): x is OuiIconType {
-  return typeof x === 'string' && typeToPathMap.hasOwnProperty(x);
+function isOuiIconType(type: OuiIconProps['type']): type is OuiIconType {
+  return typeof type === 'string' && typeToPathMap.hasOwnProperty(type);
 }
 
-function getInitialIcon(icon: OuiIconProps['type']) {
-  if (icon == null) {
-    return undefined;
-  }
-  if (isOuiIconType(icon)) {
-    if (iconComponentCache.hasOwnProperty(icon)) {
-      return iconComponentCache[icon];
-    }
-    return undefined;
-  }
+function isUrl(type: OuiIconProps['type']) {
+  return typeof type === 'string' && (type.includes('.') || type.includes('/'));
+}
 
-  return icon;
+function isCachedIcon(type: OuiIconProps['type']) {
+  return isOuiIconType(type) && iconComponentCache.hasOwnProperty(type);
 }
 
 const generateId = htmlIdGenerator();
@@ -628,13 +622,22 @@ export class OuiIcon extends PureComponent<OuiIconProps, State> {
     super(props);
 
     const { type } = props;
-    const initialIcon = getInitialIcon(type);
+    let initialIcon;
     let isLoading = false;
 
-    if (isOuiIconType(type) && initialIcon == null) {
+    // Category 1: cached oui icons
+    if (isCachedIcon(type)) {
+      initialIcon = iconComponentCache[type as string];
+      // Category 2: URL (relative, absolute)
+    } else if (isUrl(type)) {
+      initialIcon = type;
+      // Category 3: non-cached oui icon or new icon
+    } else if (typeof type === 'string') {
       isLoading = true;
-      this.loadIconComponent(type);
+      this.loadIconComponent(type as OuiIconType);
     } else {
+      // Category 4: custom icon component
+      initialIcon = type;
       this.onIconLoad();
     }
 
@@ -683,12 +686,14 @@ export class OuiIcon extends PureComponent<OuiIconProps, State> {
       return;
     }
 
+    const iconPath = typeToPathMap[iconType] || 'beaker';
+
     import(
       /* webpackChunkName: "icon.[request]" */
       // It's important that we don't use a template string here, it
       // stops webpack from building a dynamic require context.
       // eslint-disable-next-line prefer-template
-      './assets/' + typeToPathMap[iconType] + '.js'
+      './assets/' + iconPath + '.js'
     ).then(({ icon }) => {
       iconComponentCache[iconType] = icon;
       enqueueStateChange(() => {
@@ -761,7 +766,7 @@ export class OuiIcon extends PureComponent<OuiIconProps, State> {
       className
     );
 
-    const icon = this.state.icon || (empty as ComponentType);
+    const icon = this.state.icon || (defaultIcon as ComponentType);
 
     // This is a fix for IE and Edge, which ignores tabindex="-1" on an SVG, but respects
     // focusable="false".
@@ -771,6 +776,7 @@ export class OuiIcon extends PureComponent<OuiIconProps, State> {
     //   - For all other values, the consumer wants the icon to be focusable.
     const focusable = tabIndex == null || tabIndex === -1 ? 'false' : 'true';
 
+    // relative, absolute path
     if (typeof icon === 'string') {
       return (
         <img
@@ -784,15 +790,15 @@ export class OuiIcon extends PureComponent<OuiIconProps, State> {
     } else {
       const Svg = icon;
 
-      // If it's an empty icon, or if there is no aria-label, aria-labelledby, or title it gets aria-hidden true
+      // If it's a default icon, or if there is no aria-label, aria-labelledby, or title it gets aria-hidden true
       const isAriaHidden =
-        icon === empty ||
+        icon === defaultIcon ||
         !(
           this.props['aria-label'] ||
           this.props['aria-labelledby'] ||
           this.props.title
         );
-      const hideIconEmpty = isAriaHidden && { 'aria-hidden': true };
+      const hideDefaultIcon = isAriaHidden && { 'aria-hidden': true };
 
       let titleId: any;
 
@@ -817,7 +823,7 @@ export class OuiIcon extends PureComponent<OuiIconProps, State> {
           title={title}
           {...titleId}
           {...rest}
-          {...hideIconEmpty}
+          {...hideDefaultIcon}
         />
       );
     }