feat!: Chip component redesign (#2836)

src/Chip/Chip.test.jsx

@@ -4,6 +4,7 @@ import { render, screen } from '@testing-library/react';
 import userEvent from '@testing-library/user-event';
 
 import { Close } from '../../icons';
+import { STYLE_VARIANTS } from './constants';
 import Chip from '.';
 
 function TestChip(props) {
@@ -24,58 +25,123 @@ describe('<Chip />', () => {
     });
     it('renders with props iconBefore', () => {
       const tree = renderer.create((
-        <TestChip iconBefore={Close} />
+        <TestChip iconBefore={Close} iconBeforeAlt="close icon" />
       )).toJSON();
       expect(tree).toMatchSnapshot();
     });
     it('renders with props iconAfter', () => {
       const tree = renderer.create((
-        <TestChip iconAfter={Close} />
+        <TestChip iconAfter={Close} iconAfterAlt="close icon" />
       )).toJSON();
       expect(tree).toMatchSnapshot();
     });
     it('renders with props iconBefore and iconAfter', () => {
       const tree = renderer.create((
-        <TestChip iconBefore={Close} iconAfter={Close}>Chip</TestChip>
+        <TestChip
+          iconBefore={Close}
+          iconBeforeAlt="close icon"
+          iconAfter={Close}
+          iconAfterAlt="close icon"
+        >
+          Chip
+        </TestChip>
+      )).toJSON();
+      expect(tree).toMatchSnapshot();
+    });
+    it('renders div with "button" role when onClick is provided', () => {
+      const tree = renderer.create((
+        <TestChip onClick={jest.fn}>Chip</TestChip>
       )).toJSON();
       expect(tree).toMatchSnapshot();
     });
   });
 
   describe('correct rendering', () => {
+    it('render a non-interactive element if onClick handlers are not provided', () => {
+      render(<TestChip />);
+      expect(screen.queryByRole('button')).not.toBeInTheDocument();
+    });
+    it('render an interactive element if onClick handler is provided', () => {
+      render(<TestChip onClick={jest.fn} />);
+      expect(screen.queryByRole('button')).toBeInTheDocument();
+    });
     it('renders with correct class when variant is added', () => {
-      render(<TestChip variant="dark" data-testid="chip" />);
-      const chip = screen.getByTestId('chip');
+      render(<TestChip variant={STYLE_VARIANTS.DARK} onClick={jest.fn} />);
+      const chip = screen.getByRole('button');
       expect(chip).toHaveClass('pgn__chip pgn__chip-dark');
     });
     it('renders with active class when disabled prop is added', () => {
-      render(<TestChip disabled data-testid="chip" />);
-      const chip = screen.getByTestId('chip');
+      render(<TestChip disabled onClick={jest.fn} />);
+      const chip = screen.getByRole('button');
       expect(chip).toHaveClass('disabled');
     });
     it('renders with the client\'s className', () => {
       const className = 'testClassName';
-      render(<TestChip className={className} data-testid="chip" />);
-      const chip = screen.getByTestId('chip');
+      render(<TestChip className={className} onClick={jest.fn} />);
+      const chip = screen.getByRole('button');
       expect(chip).toHaveClass(className);
     });
     it('onIconAfterClick is triggered', async () => {
       const func = jest.fn();
       render(
-        <TestChip iconAfter={Close} onIconAfterClick={func} />,
+        <TestChip
+          iconAfter={Close}
+          onIconAfterClick={func}
+          iconAfterAlt="icon-after"
+        />,
       );
-      const iconAfter = screen.getByTestId('icon-after');
+      const iconAfter = screen.getByLabelText('icon-after');
       await userEvent.click(iconAfter);
-      expect(func).toHaveBeenCalled();
+      expect(func).toHaveBeenCalledTimes(1);
     });
     it('onIconAfterKeyDown is triggered', async () => {
       const func = jest.fn();
       render(
-        <TestChip iconAfter={Close} onIconAfterClick={func} />,
+        <TestChip
+          iconAfter={Close}
+          onIconAfterClick={func}
+          iconAfterAlt="icon-after"
+        />,
+      );
+      const iconAfter = screen.getByLabelText('icon-after');
+      await userEvent.click(iconAfter, '{enter}', { skipClick: true });
+      expect(func).toHaveBeenCalledTimes(1);
+    });
+    it('onIconBeforeClick is triggered', async () => {
+      const func = jest.fn();
+      render(
+        <TestChip
+          iconBefore={Close}
+          onIconBeforeClick={func}
+          iconBeforeAlt="icon-before"
+        />,
+      );
+      const iconBefore = screen.getByLabelText('icon-before');
+      await userEvent.click(iconBefore);
+      expect(func).toHaveBeenCalledTimes(1);
+    });
+    it('onIconBeforeKeyDown is triggered', async () => {
+      const func = jest.fn();
+      render(
+        <TestChip
+          iconBefore={Close}
+          onIconBeforeClick={func}
+          iconBeforeAlt="icon-before"
+        />,
       );
-      const iconAfter = screen.getByTestId('icon-after');
-      await userEvent.type(iconAfter, '{enter}');
-      expect(func).toHaveBeenCalled();
+      const iconBefore = screen.getByLabelText('icon-before');
+      await userEvent.click(iconBefore, '{enter}', { skipClick: true });
+      expect(func).toHaveBeenCalledTimes(1);
+    });
+    it('checks the absence of the `selected` class in the chip', async () => {
+      render(<TestChip onClick={jest.fn} />);
+      const chip = screen.getByRole('button');
+      expect(chip).not.toHaveClass('selected');
+    });
+    it('checks the presence of the `selected` class in the chip', async () => {
+      render(<TestChip isSelected onClick={jest.fn} />);
+      const chip = screen.getByRole('button');
+      expect(chip).toHaveClass('selected');
     });
   });
 });

src/Chip/ChipIcon.tsx

@@ -0,0 +1,54 @@
+import React, { KeyboardEventHandler, MouseEventHandler } from 'react';
+import PropTypes from 'prop-types';
+import Icon from '../Icon';
+// @ts-ignore
+import IconButton from '../IconButton';
+// @ts-ignore
+import { STYLE_VARIANTS } from './constants';
+
+export interface ChipIconProps {
+  className: string,
+  src: React.ReactElement | Function,
+  onClick?: KeyboardEventHandler & MouseEventHandler,
+  alt?: string,
+  variant: string,
+  disabled?: boolean,
+}
+
+function ChipIcon({
+  className, src, onClick, alt, variant, disabled,
+}: ChipIconProps) {
+  if (onClick) {
+    return (
+      <IconButton
+        className={className}
+        src={src}
+        onClick={onClick}
+        iconAs={Icon}
+        alt={alt}
+        invertColors={variant === STYLE_VARIANTS.DARK}
+        tabIndex={disabled ? -1 : 0}
+      />
+    );
+  }
+
+  return <Icon src={src} className={className} size="sm" />;
+}
+
+ChipIcon.propTypes = {
+  className: PropTypes.string.isRequired,
+  src: PropTypes.oneOfType([PropTypes.element, PropTypes.func]).isRequired,
+  onClick: PropTypes.func,
+  alt: PropTypes.string,
+  variant: PropTypes.string,
+  disabled: PropTypes.bool,
+};
+
+ChipIcon.defaultProps = {
+  onClick: undefined,
+  alt: undefined,
+  variant: STYLE_VARIANTS.LIGHT,
+  disabled: false,
+};
+
+export default ChipIcon;

src/Chip/README.md

@@ -16,34 +16,139 @@ notes: |
 ## Basic Usage
 
 ```jsx live
-<div>
+<Stack
+  gap={2}
+  direction="horizontal"
+>
   <Chip>New</Chip>
   <Chip disabled>New</Chip>
-  <Chip variant="dark">New</Chip>
-</div>
+</Stack>
+```
+
+## Clickable Variant
+
+Use `onClick` prop to make the whole `Chip` clickable, this will also add appropriate styles to make `Chip` interactive.
+
+```jsx live
+<Chip onClick={() => console.log('Click!')}>Click Me</Chip>
+```
+
+## With isSelected prop
+
+```jsx live
+<Chip isSelected>New</Chip>
 ```
 
 ## With Icon Before and After
+### Basic Usage
+
+Use `iconBefore` and `iconAfter` props to provide icons for the `Chip`, note that you also can provide
+accessible names for these icons for screen reader support via `iconBeforeAlt` and `iconAfterAlt` respectively. 
 
 ```jsx live
-<div>
-  <Chip iconBefore={Person}>New</Chip>
+<Stack
+  gap={2}
+  direction="horizontal"
+>
+  <Chip iconBefore={Person} iconBeforeAlt="icon-before">Person</Chip>
+  <Chip iconAfter={Close} iconAfterAlt="icon-after">Close</Chip>
   <Chip
-    variant="dark"
     iconBefore={Person}
     iconAfter={Close}
-    onIconAfterClick={() => console.log('Remove Chip')}
+    iconAfterAlt="icon-after"
+    iconBeforeAlt="icon-before"
   >
-    New
+    Both
+  </Chip>
+</Stack>
+```
+
+### Clickable icon variant
+
+Provide click handlers for icons via `onIconAfterClick` and `onIconBeforeClick` props. 
+
+```jsx live
+<Stack
+  gap={2}
+  direction="horizontal"
+>
+  <Chip
+    iconBefore={Person}
+    iconBeforeAlt="icon-before"
+    onIconBeforeClick={() => console.log('onIconBeforeClick')}
+  >
+    Person
+  </Chip>
+  <Chip
+    iconAfter={Close}
+    onIconAfterClick={() => console.log('onIconAfterClick')}
+    iconAfterAlt="icon-after"
+  >
+    Close
   </Chip>
   <Chip
-    variant="dark"
     iconBefore={Person}
     iconAfter={Close}
-    onIconAfterClick={() => console.log('Remove Chip')}
+    onIconAfterClick={() => console.log('onIconAfterClick')}
+    onIconBeforeClick={() => console.log('onIconBeforeClick')}
+    iconAfterAlt="icon-after"
+    iconBeforeAlt="icon-before"
+  >
+    Both
+  </Chip>
+  <Chip
+    iconBefore={Person}
+    iconAfter={Close}
+    onIconAfterClick={() => console.log('onIconAfterClick')}
+    onIconBeforeClick={() => console.log('onIconBeforeClick')}
+    iconAfterAlt="icon-after"
+    iconBeforeAlt="icon-before"
+    disabled
+  >
+    Both
+  </Chip>
+</Stack>
+```
+
+**Note**: both `Chip` and its icons cannot be made interactive at the same time, e.g. if you provide both `onClick` and `onIconAfterClick` props,
+`onClick` will be ignored and only the icon will get interactive behaviour, see example below (this is done to avoid usability issues where users might click on the `Chip` itself by mistake when they meant to click the icon instead).
+
+```jsx live
+<Chip
+  iconBefore={Person}
+  iconBeforeAlt="icon-before"
+  onIconBeforeClick={() => console.log('onIconBeforeClick')}
+  onClick={() => console.log('onClick')}
+>
+  Person
+</Chip>
+```
+
+### Inverse Pallete
+
+```jsx live
+<Stack
+  className="bg-dark-700 p-4"
+  gap={2}
+  direction="horizontal"
+>
+  <Chip variant="dark" iconBefore={Person} iconBeforeAlt="icon-before">New</Chip>
+  <Chip
+    variant="dark"
+    iconAfter={Close}
+    onIconAfterClick={() => console.log('onIconAfterClick')}
+    iconAfterAlt="icon-after"
+  >
+    New 1
+  </Chip>
+  <Chip
+    variant="dark"
+    iconAfter={Close}
+    onIconAfterClick={() => console.log('onIconAfterClick')}
+    iconAfterAlt="icon-after"
     disabled
   >
     New
   </Chip>
-</div>
+</Stack>
 ```