From c611dd4056bf805432e36de03ffbe0394302d17a Mon Sep 17 00:00:00 2001
From: Temidayo32 <temidayoazeez032@gmail.com>
Date: Tue, 24 Oct 2023 17:12:13 +0100
Subject: [PATCH] Ensure the Icon react component can support custom icon paths
 #11122

- When provided with children (e.g. custom paths), render these instead of the `use` symbol reference
- Allow any valid SVG attribute to be passed to the component to render on the `svg` element
- Clean up rendering of className to avoid extra whitespace
- Clean up ordering of the props to be alphabetically sorted
- Update unit tests to be focused more on testing and less on snapshots
---
 client/src/components/Icon/Icon.test.js       | 52 ++++++++++++++++---
 client/src/components/Icon/Icon.tsx           | 20 +++++--
 .../Icon/__snapshots__/Icon.test.js.snap      | 41 +++------------
 .../__snapshots__/FieldBlock.test.js.snap     |  2 +-
 .../__snapshots__/StreamBlock.test.js.snap    |  4 +-
 5 files changed, 69 insertions(+), 50 deletions(-)

diff --git a/client/src/components/Icon/Icon.test.js b/client/src/components/Icon/Icon.test.js
index a4d4a6ee9e..f3820ac91c 100644
--- a/client/src/components/Icon/Icon.test.js
+++ b/client/src/components/Icon/Icon.test.js
@@ -1,5 +1,5 @@
 import React from 'react';
-import { shallow } from 'enzyme';
+import { shallow, mount } from 'enzyme';
 import Icon from './Icon';
 
 describe('Icon', () => {
@@ -7,15 +7,53 @@ describe('Icon', () => {
     expect(Icon).toBeDefined();
   });
 
-  it('#name', () => {
-    expect(shallow(<Icon name="test" />)).toMatchSnapshot();
+  it('should support icons with a name', () => {
+    const wrapper = mount(<Icon name="test" />);
+
+    expect(wrapper.find('.icon.icon-test')).toHaveLength(1);
+    expect(wrapper.find('use[href="#icon-test"]')).toHaveLength(1);
+
+    expect(wrapper).toMatchSnapshot();
   });
 
-  it('#className', () => {
-    expect(shallow(<Icon name="test" className="u-test" />)).toMatchSnapshot();
+  it('should support children in place of the icon use#name', () => {
+    const wrapper = shallow(
+      <icon name="example">
+        <rect
+          x="10"
+          y="10"
+          width="30"
+          height="30"
+          stroke="black"
+          fill="transparent"
+          strokeWidth="5"
+        />
+      </icon>,
+    );
+
+    expect(wrapper.find('use')).toHaveLength(0);
+    expect(wrapper.find('rect')).toHaveLength(1);
   });
 
-  it('#title', () => {
-    expect(shallow(<Icon name="test" title="Test title" />)).toMatchSnapshot();
+  it('should support a className prop', () => {
+    const wrapper = mount(<Icon name="test" className="u-test" />);
+
+    expect(wrapper.find('.icon.u-test')).toHaveLength(1);
+  });
+
+  it('should support other svg attributes', () => {
+    const wrapper = mount(<Icon name="test" viewBox="0 0 1024 1024" />);
+
+    expect(wrapper.find('svg').prop('viewBox')).toBe('0 0 1024 1024');
+  });
+
+  it('should support a title that is output as a sibling of the title', () => {
+    const wrapper = mount(<Icon name="test" title="Test title" />);
+
+    const title = wrapper.find('svg.icon ~ span');
+    expect(title).toHaveLength(1);
+
+    expect(title.text()).toBe('Test title');
+    expect(title.hasClass('w-sr-only')).toBe(true);
   });
 });
diff --git a/client/src/components/Icon/Icon.tsx b/client/src/components/Icon/Icon.tsx
index bf9e87f8dd..9ff672a007 100644
--- a/client/src/components/Icon/Icon.tsx
+++ b/client/src/components/Icon/Icon.tsx
@@ -1,8 +1,10 @@
 import * as React from 'react';
 
-export interface IconProps {
-  name: string;
+export interface IconProps extends React.SVGProps<SVGSVGElement> {
+  /** Optional svg `path` instead of the `use` based on the icon name. */
+  children?: React.ReactNode;
   className?: string;
+  name: string;
   title?: string;
 }
 
@@ -10,13 +12,21 @@ export interface IconProps {
  * Provide a `title` as an accessible label intended for screen readers.
  */
 const Icon: React.FunctionComponent<IconProps> = ({
-  name,
+  children,
   className,
+  name,
   title,
+  ...props
 }) => (
   <>
-    <svg className={`icon icon-${name} ${className || ''}`} aria-hidden="true">
-      <use href={`#icon-${name}`} />
+    <svg
+      {...props}
+      className={['icon', `icon-${name}`, className || '']
+        .filter(Boolean)
+        .join(' ')}
+      aria-hidden="true"
+    >
+      {children || <use href={`#icon-${name}`} />}
     </svg>
     {title && <span className="w-sr-only">{title}</span>}
   </>
diff --git a/client/src/components/Icon/__snapshots__/Icon.test.js.snap b/client/src/components/Icon/__snapshots__/Icon.test.js.snap
index 9796195804..8cd3a07ac4 100644
--- a/client/src/components/Icon/__snapshots__/Icon.test.js.snap
+++ b/client/src/components/Icon/__snapshots__/Icon.test.js.snap
@@ -1,45 +1,16 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
-exports[`Icon #className 1`] = `
-<Fragment>
+exports[`Icon should support icons with a name 1`] = `
+<Icon
+  name="test"
+>
   <svg
     aria-hidden="true"
-    className="icon icon-test u-test"
+    className="icon icon-test"
   >
     <use
       href="#icon-test"
     />
   </svg>
-</Fragment>
-`;
-
-exports[`Icon #name 1`] = `
-<Fragment>
-  <svg
-    aria-hidden="true"
-    className="icon icon-test "
-  >
-    <use
-      href="#icon-test"
-    />
-  </svg>
-</Fragment>
-`;
-
-exports[`Icon #title 1`] = `
-<Fragment>
-  <svg
-    aria-hidden="true"
-    className="icon icon-test "
-  >
-    <use
-      href="#icon-test"
-    />
-  </svg>
-  <span
-    className="w-sr-only"
-  >
-    Test title
-  </span>
-</Fragment>
+</Icon>
 `;
diff --git a/client/src/components/StreamField/blocks/__snapshots__/FieldBlock.test.js.snap b/client/src/components/StreamField/blocks/__snapshots__/FieldBlock.test.js.snap
index 4d1377d0b0..d324f94507 100644
--- a/client/src/components/StreamField/blocks/__snapshots__/FieldBlock.test.js.snap
+++ b/client/src/components/StreamField/blocks/__snapshots__/FieldBlock.test.js.snap
@@ -51,7 +51,7 @@ exports[`telepath: wagtail.blocks.FieldBlock with comments enabled it renders co
           <div class="w-field__help" id="the-prefix-helptext" data-field-help=""><p class="help">drink <em>more</em> water</p></div>
           <div class="w-field__input" data-field-input="">
             <p name="the-prefix" id="the-prefix">The widget</p>
-          <button type="button" aria-label="Add Comment" data-comment-add="" class="w-field__comment-button w-field__comment-button--add"><svg class="icon icon-comment-add " aria-hidden="true"><use href="#icon-comment-add"></use></svg><svg class="icon icon-comment-add-reversed " aria-hidden="true"><use href="#icon-comment-add-reversed"></use></svg></button></div>
+          <button type="button" aria-label="Add Comment" data-comment-add="" class="w-field__comment-button w-field__comment-button--add"><svg class="icon icon-comment-add" aria-hidden="true"><use href="#icon-comment-add"></use></svg><svg class="icon icon-comment-add-reversed" aria-hidden="true"><use href="#icon-comment-add-reversed"></use></svg></button></div>
         </div>
       </div>"
 `;
diff --git a/client/src/components/StreamField/blocks/__snapshots__/StreamBlock.test.js.snap b/client/src/components/StreamField/blocks/__snapshots__/StreamBlock.test.js.snap
index 15f1029f00..67e9c29336 100644
--- a/client/src/components/StreamField/blocks/__snapshots__/StreamBlock.test.js.snap
+++ b/client/src/components/StreamField/blocks/__snapshots__/StreamBlock.test.js.snap
@@ -711,7 +711,7 @@ exports[`telepath: wagtail.blocks.StreamBlock it renders menus on opening 1`] =
         <button type="button" title="Insert a block" class="c-sf-add-button" aria-expanded="true">
           <svg class="icon icon-plus" aria-hidden="true"><use href="#icon-plus"></use></svg>
         </button>
-      <div data-tippy-root="" id="tippy-5" style="z-index: 9999; visibility: visible; transition: none; position: absolute; left: 0px; top: 0px; margin: 0px;"><div class="tippy-box" data-state="hidden" tabindex="-1" data-theme="dropdown" data-animation="fade" style="max-width: 350px; transition-duration: 0ms;" role="tooltip"><div class="tippy-content" data-state="hidden" style="transition-duration: 0ms;"><div><div class="w-combobox"><label id="downshift-0-label" for="downshift-0-input" class="w-sr-only">Search options…</label><div class="w-combobox__field"><input aria-activedescendant="" aria-autocomplete="list" aria-controls="downshift-0-menu" aria-expanded="false" aria-labelledby="downshift-0-label" autocomplete="off" id="downshift-0-input" role="combobox" type="text" placeholder="Search options…" value=""></div><div id="downshift-0-menu" role="listbox" aria-labelledby="downshift-0-label" class="w-combobox__menu"><div class="w-combobox__optgroup"><div role="option" aria-selected="false" id="downshift-0-item-0" class="w-combobox__option w-combobox__option--col1"><div class="w-combobox__option-icon"><svg class="icon icon-placeholder " aria-hidden="true"><use href="#icon-placeholder"></use></svg></div><div class="w-combobox__option-text">Test Block A</div></div><div role="option" aria-selected="false" id="downshift-0-item-1" class="w-combobox__option w-combobox__option--col2"><div class="w-combobox__option-icon"><svg class="icon icon-pilcrow " aria-hidden="true"><use href="#icon-pilcrow"></use></svg></div><div class="w-combobox__option-text">Test Block B</div></div></div></div></div></div></div></div></div></div><div data-contentpath="2">
+      <div data-tippy-root="" id="tippy-5" style="z-index: 9999; visibility: visible; transition: none; position: absolute; left: 0px; top: 0px; margin: 0px;"><div class="tippy-box" data-state="hidden" tabindex="-1" data-theme="dropdown" data-animation="fade" style="max-width: 350px; transition-duration: 0ms;" role="tooltip"><div class="tippy-content" data-state="hidden" style="transition-duration: 0ms;"><div><div class="w-combobox"><label id="downshift-0-label" for="downshift-0-input" class="w-sr-only">Search options…</label><div class="w-combobox__field"><input aria-activedescendant="" aria-autocomplete="list" aria-controls="downshift-0-menu" aria-expanded="false" aria-labelledby="downshift-0-label" autocomplete="off" id="downshift-0-input" role="combobox" type="text" placeholder="Search options…" value=""></div><div id="downshift-0-menu" role="listbox" aria-labelledby="downshift-0-label" class="w-combobox__menu"><div class="w-combobox__optgroup"><div role="option" aria-selected="false" id="downshift-0-item-0" class="w-combobox__option w-combobox__option--col1"><div class="w-combobox__option-icon"><svg class="icon icon-placeholder" aria-hidden="true"><use href="#icon-placeholder"></use></svg></div><div class="w-combobox__option-text">Test Block A</div></div><div role="option" aria-selected="false" id="downshift-0-item-1" class="w-combobox__option w-combobox__option--col2"><div class="w-combobox__option-icon"><svg class="icon icon-pilcrow" aria-hidden="true"><use href="#icon-pilcrow"></use></svg></div><div class="w-combobox__option-text">Test Block B</div></div></div></div></div></div></div></div></div></div><div data-contentpath="2">
         <input type="hidden" name="the-prefix-1-deleted" value="">
         <input type="hidden" name="the-prefix-1-order" value="1">
         <input type="hidden" name="the-prefix-1-type" value="test_block_b">
@@ -922,7 +922,7 @@ exports[`telepath: wagtail.blocks.StreamBlock setError renders error messages 1`
       </div>"
 `;
 
-exports[`telepath: wagtail.blocks.StreamBlock with labels that need escaping it renders correctly 1`] = `"<div class="w-combobox__optgroup"><div role="option" aria-selected="false" id="downshift-2-item-0" class="w-combobox__option w-combobox__option--col1"><div class="w-combobox__option-icon"><svg class="icon icon-placeholder " aria-hidden="true"><use href="#icon-placeholder"></use></svg></div><div class="w-combobox__option-text">Test Block &lt;A&gt;</div></div><div role="option" aria-selected="false" id="downshift-2-item-1" class="w-combobox__option w-combobox__option--col2"><div class="w-combobox__option-icon"><svg class="icon icon-pilcrow " aria-hidden="true"><use href="#icon-pilcrow"></use></svg></div><div class="w-combobox__option-text">Test Block &lt;B&gt;</div></div></div>"`;
+exports[`telepath: wagtail.blocks.StreamBlock with labels that need escaping it renders correctly 1`] = `"<div class="w-combobox__optgroup"><div role="option" aria-selected="false" id="downshift-2-item-0" class="w-combobox__option w-combobox__option--col1"><div class="w-combobox__option-icon"><svg class="icon icon-placeholder" aria-hidden="true"><use href="#icon-placeholder"></use></svg></div><div class="w-combobox__option-text">Test Block &lt;A&gt;</div></div><div role="option" aria-selected="false" id="downshift-2-item-1" class="w-combobox__option w-combobox__option--col2"><div class="w-combobox__option-icon"><svg class="icon icon-pilcrow" aria-hidden="true"><use href="#icon-pilcrow"></use></svg></div><div class="w-combobox__option-text">Test Block &lt;B&gt;</div></div></div>"`;
 
 exports[`telepath: wagtail.blocks.StreamBlock with unique block type it can add block 1`] = `
 "<div class="c-sf-help">