Use CSS classes rather than a numeric size to set icon sizes

This provides more flexibility as it can be used to set color as well as
size and it is also more consistent with how everything else is styled in the
application.

Replace the existing use of the `size` prop and tweak menu item icon
sizes.

src/sidebar/components/menu-item.js

@@ -61,7 +61,7 @@ function MenuItem({
             (iconIsUrl ? (
               <img className={iconClass} alt={iconAlt} src={icon} />
             ) : (
-              <SvgIcon name={icon} size={10} />
+              <SvgIcon name={icon} className="menu-item__icon" />
             ))}
         </div>
       )}
@@ -86,7 +86,10 @@ function MenuItem({
           aria-label={`Show actions for ${label}`}
           {...onActivate('button', onToggleSubmenu)}
         >
-          <SvgIcon name={isSubmenuVisible ? 'collapse-menu' : 'expand-menu'} />
+          <SvgIcon
+            name={isSubmenuVisible ? 'collapse-menu' : 'expand-menu'}
+            className="menu-item__toggle-icon"
+          />
         </div>
       )}
     </div>

src/sidebar/components/menu.js

@@ -110,7 +110,7 @@ function Menu({
         {label}
         {menuIndicator && (
           <span className="menu__toggle-arrow">
-            <SvgIcon name="expand-menu" />
+            <SvgIcon name="expand-menu" className="menu__toggle-icon" />
           </span>
         )}
       </button>

src/sidebar/components/svg-icon.js

@@ -23,7 +23,7 @@ const icons = {
  * This matches the way we do icons on the website, see
  * https://github.com/hypothesis/h/pull/3675
  */
-function SvgIcon({ name, size }) {
+function SvgIcon({ name, className = '' }) {
   if (!icons[name]) {
     throw new Error(`Unknown icon ${name}`);
   }
@@ -32,11 +32,13 @@ function SvgIcon({ name, size }) {
   const element = useRef();
   useLayoutEffect(() => {
     const svg = element.current.querySelector('svg');
-    if (typeof size === 'number') {
-      svg.style.width = `${size}px`;
-      svg.style.height = `${size}px`;
-    }
-  }, [element, size]);
+    svg.setAttribute('class', className);
+  }, [
+    className,
+    // `markup` is a dependency of this effect because the SVG is replaced if
+    // it changes.
+    markup,
+  ]);
 
   return <span dangerouslySetInnerHTML={markup} ref={element} />;
 }
@@ -45,8 +47,8 @@ SvgIcon.propTypes = {
   /** The name of the icon to load. */
   name: propTypes.string,
 
-  /** The size of the icon in pixels. */
-  size: propTypes.number,
+  /** A CSS class to apply to the `<svg>` element. */
+  className: propTypes.string,
 };
 
 module.exports = SvgIcon;

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

@@ -22,19 +22,25 @@ describe('SvgIcon', () => {
     });
   });
 
-  it('sets the size of the SVG if provided', () => {
+  it('does not set the class of the SVG by default', () => {
     const container = document.createElement('div');
-    render(<SvgIcon name="refresh" size={16} />, container);
+    render(<SvgIcon name="refresh" />, container);
     const svg = container.querySelector('svg');
-    assert.equal(svg.style.width, '16px');
-    assert.equal(svg.style.height, '16px');
+    assert.equal(svg.getAttribute('class'), '');
   });
 
-  it("uses the icon's default size if no size is provided", () => {
+  it('sets the class of the SVG if provided', () => {
     const container = document.createElement('div');
-    render(<SvgIcon name="refresh" />, container);
+    render(<SvgIcon name="refresh" className="thing__icon" />, container);
+    const svg = container.querySelector('svg');
+    assert.equal(svg.getAttribute('class'), 'thing__icon');
+  });
+
+  it('retains the CSS class if the icon changes', () => {
+    const container = document.createElement('div');
+    render(<SvgIcon name="expand-menu" className="thing__icon" />, container);
+    render(<SvgIcon name="collapse-menu" className="thing__icon" />, container);
     const svg = container.querySelector('svg');
-    assert.equal(svg.style.width, '');
-    assert.equal(svg.style.height, '');
+    assert.equal(svg.getAttribute('class'), 'thing__icon');
   });
 });

src/styles/sidebar/components/menu-item.scss

@@ -111,10 +111,8 @@ $menu-item-padding: 10px;
     background-color: $grey-3;
   }
 
-  // The toggle icon is loaded from a string, so we can't apply a class to it
-  // directly.
-  svg {
-    width: 12px;
-    height: 12px;
+  &-icon {
+    width: 10px;
+    height: 10px;
   }
 }

src/styles/sidebar/components/menu.scss

@@ -12,6 +12,11 @@
   display: flex;
 }
 
+.menu__toggle-icon {
+  width: 10px;
+  height: 10px;
+}
+
 // Triangular indicator next to the toggle button indicating that there is
 // an associated drop-down menu.
 .menu__toggle-arrow {