Add trash can icon to color picker

openstax · Sep 9, 2024 · f5e3260 · f5e3260
1 parent a394466
commit f5e3260
Show file tree

Hide file tree

Showing 6 changed files with 110 additions and 26 deletions.
diff --git a/src/app/content/highlights/components/ColorIndicator.tsx b/src/app/content/highlights/components/ColorIndicator.tsx
@@ -4,6 +4,7 @@ import { Check } from 'styled-icons/fa-solid/Check';
 import { isDefined } from '../../../guards';
 import { highlightStyles } from '../../constants';
 import { defaultFocusOutline } from '../../../theme';
+import trashIcon from '../../../../assets/trash-347.svg';
 
 interface StyleProps {
   style: typeof highlightStyles[number];
@@ -114,4 +115,31 @@ const ColorIndicator = styled(Hoc)`
   }
 `;
 
+function TB({
+  onClick,
+  className,
+}: {
+  onClick: React.MouseEventHandler<HTMLButtonElement> | undefined;
+  className: string;
+}) {
+  return (
+    <button
+      type='button'
+      className={className}
+      aria-label='remove highlight'
+      onClick={onClick}
+    >
+      <img src={trashIcon} alt='' />
+    </button>
+  );
+}
+
+// tslint:disable-next-line:variable-name
+export const TrashButton = styled(TB)`
+  img {
+    height: ${(props: Props) => indicatorSize(props) - 0.5}rem;
+    width: ${(props: Props) => indicatorSize(props) - 0.5}rem;
+  }
+`;
+
 export default ColorIndicator;
diff --git a/src/app/content/highlights/components/ColorPicker.spec.tsx b/src/app/content/highlights/components/ColorPicker.spec.tsx
@@ -4,7 +4,7 @@ import { renderToDom, dispatchKeyDownEvent } from '../../../../test/reactutils';
 import TestContainer from '../../../../test/TestContainer';
 import { highlightStyles } from '../../constants';
 import ColorPicker from './ColorPicker';
-import { HTMLElement, HTMLFieldSetElement, HTMLInputElement } from '@openstax/types/lib.dom';
+import { HTMLElement, HTMLFieldSetElement, HTMLInputElement, HTMLButtonElement } from '@openstax/types/lib.dom';
 import { assertDocument } from '../../../utils';
 
 describe('ColorPicker', () => {
@@ -84,6 +84,39 @@ describe('ColorPicker', () => {
     expect(onChange).not.toHaveBeenCalled();
   });
 
+  it('calls remove when trashcan is clicked', () => {
+    const onChange = jest.fn();
+    const onRemove = jest.fn();
+
+    const {root} = renderToDom(<TestContainer>
+      <ColorPicker color={highlightStyles[0].label} onChange={onChange} onRemove={onRemove} />
+    </TestContainer>);
+
+    const button = root.querySelector('button') as HTMLButtonElement;
+
+    // This should trigger the button, but does not in testing
+    dispatchKeyDownEvent({
+      element: button as HTMLElement,
+      key: 'Enter'
+    });
+
+    button.click();
+    expect(onRemove).toHaveBeenCalled();
+    expect(onChange).not.toHaveBeenCalled();
+  });
+
+  it('handles having no onRemove when trashcan is clicked', () => {
+    const onChange = jest.fn();
+    const component = renderer.create(<TestContainer>
+      <ColorPicker color={highlightStyles[0].label} onChange={onChange} />
+    </TestContainer>);
+
+    const button = component.root.findByType('button');
+
+    button.props.onClick();
+    expect(onChange).not.toHaveBeenCalled();
+  });
+
   it('operates as a radiogroup', () => {
     const onChange = jest.fn();
     const onRemove = jest.fn();

diff --git a/src/app/content/highlights/components/ColorPicker.tsx b/src/app/content/highlights/components/ColorPicker.tsx
@@ -5,7 +5,7 @@ import styled from 'styled-components/macro';
 import { match, not } from '../../../fpUtils';
 import { highlightStyles } from '../../constants';
 import { cardPadding } from '../constants';
-import ColorIndicator from './ColorIndicator';
+import ColorIndicator, { TrashButton } from './ColorIndicator';
 import { HTMLDivElement, HTMLInputElement } from '@openstax/types/lib.dom';
 
 interface SingleSelectProps {
@@ -48,7 +48,7 @@ const ColorButton = styled(({className, size, style, ...props}: ColorButtonProps
     component={<label />}
     className={className}
   >
-    <input type='checkbox' role='radio' aria-checked={props.checked} {...props} />
+    <input type='radio' aria-checked={props.checked} {...props} />
   </ColorIndicator>;
 })`
   cursor: pointer;
@@ -98,9 +98,11 @@ const ColorPicker = ({className, ...props}: Props) => {
       const destIdx = nextIdx(idx, inputs.length, event.key as NavKeys);
       const el = inputs[destIdx];
 
-      event.preventDefault();
-      el.focus();
-      el.click();
+      if (el) {
+        event.preventDefault();
+        el.focus();
+        el.click();
+      }
     },
     []
   );
@@ -140,6 +142,12 @@ const ColorPicker = ({className, ...props}: Props) => {
             ? props.onRemove ? props.onRemove() : null
             : props.onChange(style.label)}
       />)}
+      { props.size === 'small' ? null :
+        <TrashButton
+          size={props.size}
+          onClick={() => 'onRemove' in props && props.onRemove ? props.onRemove() : null}
+        />
+      }
     </fieldset>
   );
 };

diff --git a/src/app/content/highlights/components/SummaryPopup/__snapshots__/ContextMenu.spec.tsx.snap b/src/app/content/highlights/components/SummaryPopup/__snapshots__/ContextMenu.spec.tsx.snap
@@ -692,9 +692,8 @@ exports[`ContextMenu match snapshot when open 1`] = `
                 checked={false}
                 name="yellow"
                 onChange={[Function]}
-                role="radio"
                 tabIndex={-1}
-                type="checkbox"
+                type="radio"
               />
               <span
                 className="c20 c21"
@@ -755,9 +754,8 @@ exports[`ContextMenu match snapshot when open 1`] = `
                 checked={false}
                 name="green"
                 onChange={[Function]}
-                role="radio"
                 tabIndex={-1}
-                type="checkbox"
+                type="radio"
               />
               <span
                 className="c20 c21"
@@ -818,9 +816,8 @@ exports[`ContextMenu match snapshot when open 1`] = `
                 checked={true}
                 name="blue"
                 onChange={[Function]}
-                role="radio"
                 tabIndex={-1}
-                type="checkbox"
+                type="radio"
               />
               <span
                 className="c20 c21"
@@ -881,9 +878,8 @@ exports[`ContextMenu match snapshot when open 1`] = `
                 checked={false}
                 name="purple"
                 onChange={[Function]}
-                role="radio"
                 tabIndex={-1}
-                type="checkbox"
+                type="radio"
               />
               <span
                 className="c20 c21"
@@ -944,9 +940,8 @@ exports[`ContextMenu match snapshot when open 1`] = `
                 checked={false}
                 name="pink"
                 onChange={[Function]}
-                role="radio"
                 tabIndex={-1}
-                type="checkbox"
+                type="radio"
               />
               <span
                 className="c20 c21"

diff --git a/src/app/content/highlights/components/__snapshots__/ColorPicker.spec.tsx.snap b/src/app/content/highlights/components/__snapshots__/ColorPicker.spec.tsx.snap
@@ -266,6 +266,11 @@ exports[`ColorPicker matches snapshot no selection 1`] = `
   z-index: 1;
 }
 
+.c17 img {
+  height: 1.9rem;
+  width: 1.9rem;
+}
+
 .c2 {
   cursor: pointer;
   margin: 0;
@@ -346,9 +351,8 @@ exports[`ColorPicker matches snapshot no selection 1`] = `
       checked={false}
       name="yellow"
       onChange={[Function]}
-      role="radio"
       tabIndex={-1}
-      type="checkbox"
+      type="radio"
     />
     <span
       className="c6 c7"
@@ -406,9 +410,8 @@ exports[`ColorPicker matches snapshot no selection 1`] = `
       checked={false}
       name="green"
       onChange={[Function]}
-      role="radio"
       tabIndex={-1}
-      type="checkbox"
+      type="radio"
     />
     <span
       className="c6 c7"
@@ -466,9 +469,8 @@ exports[`ColorPicker matches snapshot no selection 1`] = `
       checked={false}
       name="blue"
       onChange={[Function]}
-      role="radio"
       tabIndex={-1}
-      type="checkbox"
+      type="radio"
     />
     <span
       className="c6 c7"
@@ -526,9 +528,8 @@ exports[`ColorPicker matches snapshot no selection 1`] = `
       checked={false}
       name="purple"
       onChange={[Function]}
-      role="radio"
       tabIndex={-1}
-      type="checkbox"
+      type="radio"
     />
     <span
       className="c6 c7"
@@ -586,9 +587,8 @@ exports[`ColorPicker matches snapshot no selection 1`] = `
       checked={false}
       name="pink"
       onChange={[Function]}
-      role="radio"
       tabIndex={-1}
-      type="checkbox"
+      type="radio"
     />
     <span
       className="c6 c7"
@@ -613,5 +613,16 @@ exports[`ColorPicker matches snapshot no selection 1`] = `
       }
     />
   </label>
+  <button
+    aria-label="remove highlight"
+    className="c17"
+    onClick={[Function]}
+    type="button"
+  >
+    <img
+      alt=""
+      src="trash-347.svg"
+    />
+  </button>
 </fieldset>
 `;
diff --git a/src/assets/trash-347.svg b/src/assets/trash-347.svg