add warnings, refactor, and test

Author: danrosenthal

src/components/Icon/Icon.tsx

@@ -16,12 +16,49 @@ const COLORS_WITH_BACKDROPS = [
   'inkLighter',
 ];
 
+const NEW_DESIGN_LANGUAGE_COLORS = [
+  'base',
+  'disabled',
+  'hovered',
+  'pressed',
+  'subdued',
+  'critical',
+  'warning',
+  'highlight',
+  'interactive',
+  'success',
+  'primary',
+  'primaryDisabled',
+  'primaryHovered',
+  'primaryPressed',
+];
+
 // This is needed for the polaris
 // styleguide to generate the props explorer
 interface Props extends IconProps {}
 
-export function Icon({source, color, backdrop, accessibilityLabel}: Props) {
+export function Icon({
+  source,
+  color: colorFromProps,
+  backdrop,
+  accessibilityLabel,
+}: Props) {
   const i18n = useI18n();
+  const {newDesignLanguage} = useFeatures();
+
+  const color =
+    colorFromProps == null && newDesignLanguage === true
+      ? 'base'
+      : colorFromProps;
+
+  let sourceType: 'function' | 'placeholder' | 'external';
+  if (typeof source === 'function') {
+    sourceType = 'function';
+  } else if (source === 'placeholder') {
+    sourceType = 'placeholder';
+  } else {
+    sourceType = 'external';
+  }
 
   if (color && backdrop && !COLORS_WITH_BACKDROPS.includes(color)) {
     // eslint-disable-next-line no-console
@@ -33,45 +70,49 @@ export function Icon({source, color, backdrop, accessibilityLabel}: Props) {
     );
   }
 
-  const {newDesignLanguage} = useFeatures();
+  if (
+    color &&
+    sourceType === 'external' &&
+    newDesignLanguage === true &&
+    NEW_DESIGN_LANGUAGE_COLORS.includes(color)
+  ) {
+    // eslint-disable-next-line no-console
+    console.warn(
+      'Recoloring external SVGs is not supported with colors in the new design langauge. Set the intended color on your SVG instead.',
+    );
+  }
 
   const className = classNames(
     styles.Icon,
     color && styles[variationName('color', color)],
-    color == null &&
-      newDesignLanguage &&
-      styles[variationName('color', 'base')],
     color && color !== 'white' && styles.isColored,
     backdrop && styles.hasBackdrop,
     newDesignLanguage && styles.newDesignLanguage,
   );
 
-  let contentMarkup: React.ReactNode;
-  if (typeof source === 'function') {
-    const SourceComponent = source;
-    contentMarkup = (
+  const SourceComponent = source;
+  const contentMarkup = {
+    function: (
       <SourceComponent
         className={styles.Svg}
         focusable="false"
         aria-hidden="true"
       />
-    );
-  } else if (source === 'placeholder') {
-    contentMarkup = <div className={styles.Placeholder} />;
-  } else {
-    contentMarkup = (
+    ),
+    placeholder: <div className={styles.Placeholder} />,
+    external: (
       <img
         className={styles.Img}
         src={`data:image/svg+xml;utf8,${source}`}
         alt=""
         aria-hidden="true"
       />
-    );
-  }
+    ),
+  };
 
   return (
     <span className={className} aria-label={accessibilityLabel}>
-      {contentMarkup}
+      {contentMarkup[sourceType]}
     </span>
   );
 }

src/components/Icon/tests/Icon.test.tsx

@@ -35,16 +35,42 @@ describe('<Icon />', () => {
   });
 
   describe('newDesignLanguage', () => {
-    it('adds a newDesignLanguage class when newDesignLanguage is enabled', () => {
+    it('adds a newDesignLanguage class when newDesignLanguage is enabled, and sets a default color', () => {
       const icon = mountWithApp(<Icon source={PlusMinor} />, {
         features: {newDesignLanguage: true},
       });
       expect(icon).toContainReactComponent('span', {
-        className: 'Icon newDesignLanguage',
+        className: 'Icon colorBase isColored newDesignLanguage',
       });
     });
 
-    it('does not add a newDesignLanguage class when newDesignLanguage is disabled', () => {
+    it('warns when an untrusted SVG is used with a color option from the new design langauge', () => {
+      const warningSpy = jest
+        .spyOn(console, 'warn')
+        .mockImplementation(() => {});
+      const svg =
+        "<svg><path d='M17 9h-6V3a1 1 0 1 0-2 0v6H3a1 1 0 1 0 0 2h6v6a1 1 0 1 0 2 0v-6h6a1 1 0 1 0 0-2'  fill-rule='evenodd'/></svg>";
+
+      mountWithApp(<Icon source={svg} color="subdued" />, {
+        features: {newDesignLanguage: true},
+      });
+
+      expect(warningSpy).toHaveBeenCalledWith(
+        'Recoloring external SVGs is not supported with colors in the new design langauge. Set the intended color on your SVG instead.',
+      );
+      warningSpy.mockRestore();
+    });
+
+    it('uses a specified color when newDesignLanguage is enabled', () => {
+      const icon = mountWithApp(<Icon source={PlusMinor} color="subdued" />, {
+        features: {newDesignLanguage: true},
+      });
+      expect(icon).toContainReactComponent('span', {
+        className: 'Icon colorSubdued isColored newDesignLanguage',
+      });
+    });
+
+    it('does not add a newDesignLanguage class when newDesignLanguage is disabled, and does not set a default color', () => {
       const icon = mountWithApp(<Icon source={PlusMinor} />, {
         features: {newDesignLanguage: false},
       });