Merge pull request #1984 from hypothesis/separate-svg-component-and-icons

Separate out icon definitions from `SvgIcon` component

src/sidebar/components/svg-icon.js

@@ -3,54 +3,18 @@ import { createElement } from 'preact';
 import { useLayoutEffect, useRef } from 'preact/hooks';
 import propTypes from 'prop-types';
 
-// The list of supported icons
-const icons = {
-  add: require('../../images/icons/add.svg'),
-  annotate: require('../../images/icons/annotate.svg'),
-  'arrow-left': require('../../images/icons/arrow-left.svg'),
-  'arrow-right': require('../../images/icons/arrow-right.svg'),
-  cancel: require('../../images/icons/cancel.svg'),
-  'caret-right': require('../../images/icons/caret-right.svg'),
-  'cc-std': require('../../images/icons/cc-std.svg'),
-  'cc-zero': require('../../images/icons/cc-zero.svg'),
-  'collapse-menu': require('../../images/icons/collapse-menu.svg'),
-  copy: require('../../images/icons/copy.svg'),
-  cursor: require('../../images/icons/cursor.svg'),
-  edit: require('../../images/icons/edit.svg'),
-  email: require('../../images/icons/email.svg'),
-  'expand-menu': require('../../images/icons/expand-menu.svg'),
-  error: require('../../images/icons/cancel.svg'),
-  external: require('../../images/icons/external.svg'),
-  facebook: require('../../images/icons/facebook.svg'),
-  flag: require('../../images/icons/flag.svg'),
-  'flag--active': require('../../images/icons/flag--active.svg'),
-  'format-bold': require('../../images/icons/format-bold.svg'),
-  'format-functions': require('../../images/icons/format-functions.svg'),
-  'format-italic': require('../../images/icons/format-italic.svg'),
-  'format-list-numbered': require('../../images/icons/format-list-numbered.svg'),
-  'format-list-unordered': require('../../images/icons/format-list-unordered.svg'),
-  'format-quote': require('../../images/icons/format-quote.svg'),
-  groups: require('../../images/icons/groups.svg'),
-  help: require('../../images/icons/help.svg'),
-  highlight: require('../../images/icons/highlight.svg'),
-  image: require('../../images/icons/image.svg'),
-  leave: require('../../images/icons/leave.svg'),
-  link: require('../../images/icons/link.svg'),
-  lock: require('../../images/icons/lock.svg'),
-  logo: require('../../images/icons/logo.svg'),
-  pointer: require('../../images/icons/pointer.svg'),
-  profile: require('../../images/icons/profile.svg'),
-  public: require('../../images/icons/public.svg'),
-  refresh: require('../../images/icons/refresh.svg'),
-  restricted: require('../../images/icons/restricted.svg'),
-  reply: require('../../images/icons/reply.svg'),
-  search: require('../../images/icons/search.svg'),
-  share: require('../../images/icons/share.svg'),
-  success: require('../../images/icons/check.svg'),
-  sort: require('../../images/icons/sort.svg'),
-  trash: require('../../images/icons/trash.svg'),
-  twitter: require('../../images/icons/twitter.svg'),
-};
+/**
+ * Object mapping icon names to SVG markup.
+ *
+ * @typedef {{[name: string]: string}} IconMap
+ */
+
+/**
+ * Map of icon name to SVG data.
+ *
+ * @type {IconMap}
+ */
+let iconRegistry = {};
 
 /**
  * Component that renders icons using inline `<svg>` elements.
@@ -65,10 +29,10 @@ export default function SvgIcon({
   inline = false,
   title = '',
 }) {
-  if (!icons[name]) {
-    throw new Error(`Unknown icon ${name}`);
+  if (!iconRegistry[name]) {
+    throw new Error(`Icon name "${name}" is not registered`);
   }
-  const markup = { __html: icons[name] };
+  const markup = { __html: iconRegistry[name] };
 
   const element = useRef();
   useLayoutEffect(() => {
@@ -97,7 +61,12 @@ export default function SvgIcon({
 }
 
 SvgIcon.propTypes = {
-  /** The name of the icon to load. */
+  /**
+   * The name of the icon to display.
+   *
+   * The name must match a name that has already been registered using the
+   * `registerIcons` function.
+   */
   name: propTypes.string,
 
   /** A CSS class to apply to the `<svg>` element. */
@@ -109,3 +78,28 @@ SvgIcon.propTypes = {
   /** Optional title attribute to apply to the SVG's containing `span` */
   title: propTypes.string,
 };
+
+/**
+ * Register icons for use with the `SvgIcon` component.
+ *
+ * @param {IconMap} icons - Object mapping icon names to SVG data.
+ * @param {boolean} [options.reset] - If `true`, remove existing registered icons.
+ */
+export function registerIcons(icons, { reset = false } = {}) {
+  if (reset) {
+    iconRegistry = {};
+  }
+  Object.assign(iconRegistry, icons);
+}
+
+/**
+ * Return the currently available icons.
+ *
+ * To register icons, don't mutate this directly but call `registerIcons`
+ * instead.
+ *
+ * @return {IconMap}
+ */
+export function availableIcons() {
+  return iconRegistry;
+}

src/sidebar/components/test/svg-icon-test.js

 import { createElement, render } from 'preact';
 
-import SvgIcon from '../svg-icon';
+import SvgIcon, { availableIcons, registerIcons } from '../svg-icon';
 
 describe('SvgIcon', () => {
   // Tests here use DOM APIs rather than Enzyme because SvgIcon uses
   // `dangerouslySetInnerHTML` for its content, and that is not visible in the
   // Enzyme tree.
 
+  // Global icon set that is registered with `SvgIcon` outside of these tests.
+  let savedIconSet;
+
+  beforeEach(() => {
+    savedIconSet = availableIcons();
+
+    registerIcons(
+      {
+        'collapse-menu': require('../../../images/icons/collapse-menu.svg'),
+        'expand-menu': require('../../../images/icons/expand-menu.svg'),
+        refresh: require('../../../images/icons/refresh.svg'),
+      },
+      { reset: true }
+    );
+  });
+
+  afterEach(() => {
+    registerIcons(savedIconSet, { reset: true });
+  });
+
   it("sets the element's content to the content of the SVG", () => {
     const container = document.createElement('div');
     render(<SvgIcon name="refresh" />, container);
     assert.ok(container.querySelector('svg'));
   });
 
-  it('throws an error if the icon is unknown', () => {
+  it('throws an error if the icon name is not registered', () => {
     assert.throws(() => {
       const container = document.createElement('div');
       render(<SvgIcon name="unknown" />, container);
-    });
+    }, 'Icon name "unknown" is not registered');
   });
 
   it('does not set the class of the SVG by default', () => {

src/sidebar/icons.js

+/**
+ * Set of icons used by the sidebar application for use with the `SvgIcon`
+ * component.
+ */
+export default {
+  add: require('../images/icons/add.svg'),
+  annotate: require('../images/icons/annotate.svg'),
+  'arrow-left': require('../images/icons/arrow-left.svg'),
+  'arrow-right': require('../images/icons/arrow-right.svg'),
+  cancel: require('../images/icons/cancel.svg'),
+  'caret-right': require('../images/icons/caret-right.svg'),
+  'cc-std': require('../images/icons/cc-std.svg'),
+  'cc-zero': require('../images/icons/cc-zero.svg'),
+  'collapse-menu': require('../images/icons/collapse-menu.svg'),
+  copy: require('../images/icons/copy.svg'),
+  cursor: require('../images/icons/cursor.svg'),
+  edit: require('../images/icons/edit.svg'),
+  email: require('../images/icons/email.svg'),
+  'expand-menu': require('../images/icons/expand-menu.svg'),
+  error: require('../images/icons/cancel.svg'),
+  external: require('../images/icons/external.svg'),
+  facebook: require('../images/icons/facebook.svg'),
+  flag: require('../images/icons/flag.svg'),
+  'flag--active': require('../images/icons/flag--active.svg'),
+  'format-bold': require('../images/icons/format-bold.svg'),
+  'format-functions': require('../images/icons/format-functions.svg'),
+  'format-italic': require('../images/icons/format-italic.svg'),
+  'format-list-numbered': require('../images/icons/format-list-numbered.svg'),
+  'format-list-unordered': require('../images/icons/format-list-unordered.svg'),
+  'format-quote': require('../images/icons/format-quote.svg'),
+  groups: require('../images/icons/groups.svg'),
+  help: require('../images/icons/help.svg'),
+  highlight: require('../images/icons/highlight.svg'),
+  image: require('../images/icons/image.svg'),
+  leave: require('../images/icons/leave.svg'),
+  link: require('../images/icons/link.svg'),
+  lock: require('../images/icons/lock.svg'),
+  logo: require('../images/icons/logo.svg'),
+  pointer: require('../images/icons/pointer.svg'),
+  profile: require('../images/icons/profile.svg'),
+  public: require('../images/icons/public.svg'),
+  refresh: require('../images/icons/refresh.svg'),
+  restricted: require('../images/icons/restricted.svg'),
+  reply: require('../images/icons/reply.svg'),
+  search: require('../images/icons/search.svg'),
+  share: require('../images/icons/share.svg'),
+  success: require('../images/icons/check.svg'),
+  sort: require('../images/icons/sort.svg'),
+  trash: require('../images/icons/trash.svg'),
+  twitter: require('../images/icons/twitter.svg'),
+};

src/sidebar/index.js

@@ -107,6 +107,11 @@ function autosave(autosaveService) {
   autosaveService.init();
 }
 
+// Register icons used by the sidebar app (and maybe other assets in future).
+import { registerIcons } from './components/svg-icon';
+import iconSet from './icons';
+registerIcons(iconSet);
+
 // Preact UI components that are wrapped for use within Angular templates.
 
 import Annotation from './components/annotation';

src/sidebar/test/bootstrap.js

@@ -11,3 +11,10 @@ import { configure } from 'enzyme';
 import { Adapter } from 'enzyme-adapter-preact-pure';
 
 configure({ adapter: new Adapter() });
+
+// Make all the icons that are available for use with `SvgIcon` in the actual
+// app available in the tests. This enables validation of icon names passed to
+// `SvgIcon`.
+import iconSet from '../icons';
+import { registerIcons } from '../components/svg-icon';
+registerIcons(iconSet);