fix: update interactive behaviour

src/Chip/Chip.test.jsx

@@ -25,38 +25,59 @@ 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={STYLE_VARIANTS[1]} />);
+      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 />);
+      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} />);
+      render(<TestChip className={className} onClick={jest.fn} />);
       const chip = screen.getByRole('button');
       expect(chip).toHaveClass(className);
     });
@@ -71,7 +92,7 @@ describe('<Chip />', () => {
       );
       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();
@@ -83,8 +104,8 @@ describe('<Chip />', () => {
         />,
       );
       const iconAfter = screen.getByLabelText('icon-after');
-      await userEvent.type(iconAfter, '{enter}');
-      expect(func).toHaveBeenCalled();
+      await userEvent.click(iconAfter, '{enter}', { skipClick: true });
+      expect(func).toHaveBeenCalledTimes(1);
     });
     it('onIconBeforeClick is triggered', async () => {
       const func = jest.fn();
@@ -97,7 +118,7 @@ describe('<Chip />', () => {
       );
       const iconBefore = screen.getByLabelText('icon-before');
       await userEvent.click(iconBefore);
-      expect(func).toHaveBeenCalled();
+      expect(func).toHaveBeenCalledTimes(1);
     });
     it('onIconBeforeKeyDown is triggered', async () => {
       const func = jest.fn();
@@ -109,16 +130,16 @@ describe('<Chip />', () => {
         />,
       );
       const iconBefore = screen.getByLabelText('icon-before');
-      await userEvent.type(iconBefore, '{enter}');
-      expect(func).toHaveBeenCalled();
+      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 />);
+      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 />);
+      render(<TestChip isSelected onClick={jest.fn} />);
       const chip = screen.getByRole('button');
       expect(chip).toHaveClass('selected');
     });

src/Chip/ChipIcon.tsx

@@ -23,15 +23,14 @@ function ChipIcon({
         className={className}
         src={src}
         onClick={onClick}
-        onKeyPress={onClick}
         iconAs={Icon}
         alt={alt}
-        invertColors={variant === STYLE_VARIANTS[1]}
+        invertColors={variant === STYLE_VARIANTS.DARK}
       />
     );
   }
 
-  return <Icon src={src} className={className} />;
+  return <Icon src={src} className={className} size="sm" />;
 }
 
 ChipIcon.propTypes = {
@@ -44,7 +43,7 @@ ChipIcon.propTypes = {
 
 ChipIcon.defaultProps = {
   onClick: undefined,
-  variant: STYLE_VARIANTS[0],
+  variant: STYLE_VARIANTS.LIGHT,
 };
 
 export default ChipIcon;

src/Chip/README.md

@@ -25,54 +25,105 @@ notes: |
 </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 have to provide
+accessible names for these icons for screen reader support via `iconBeforeAlt` and `iconAfterAlt` respectively. 
+
 ```jsx live
 <Stack
   gap={2}
   direction="horizontal"
 >
-  <Chip isSelected>New</Chip>
+  <Chip iconBefore={Person} iconBeforeAlt="icon-before">Person</Chip>
+  <Chip iconAfter={Close} iconAfterAlt="icon-after">Close</Chip>
   <Chip
-    isSelected
+    iconBefore={Person}
     iconAfter={Close}
-    onIconAfterClick={() => console.log('onIconAfterClick')}
     iconAfterAlt="icon-after"
+    iconBeforeAlt="icon-before"
   >
-    New
+    Both
   </Chip>
 </Stack>
 ```
 
-## With Icon Before and After
-### Basic Usage
+### Clickable variant
+
+Provide click handlers for icons via `onIconAfterClick` and `onIconBeforeClick` props. 
 
 ```jsx live
 <Stack
   gap={2}
   direction="horizontal"
 >
-  <Chip iconBefore={Person} iconBeforeAlt="icon-before">New</Chip>
+  <Chip
+    iconBefore={Person}
+    iconBeforeAlt="icon-before"
+    onIconBeforeClick={() => console.log('onIconBeforeClick')}
+  >
+    Person
+  </Chip>
   <Chip
     iconAfter={Close}
     onIconAfterClick={() => console.log('onIconAfterClick')}
     iconAfterAlt="icon-after"
   >
-    New 1
+    Close
   </Chip>
   <Chip
     iconBefore={Person}
     iconAfter={Close}
     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
   >
-    New
+    Both
   </Chip>
 </Stack>
 ```
 
+**Note**, however, that 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

src/Chip/__snapshots__/Chip.test.jsx.snap

@@ -1,18 +1,32 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
-exports[`<Chip /> snapshots renders with props iconAfter 1`] = `
+exports[`<Chip /> snapshots renders div with "button" role when onClick is provided 1`] = `
 <div
-  className="pgn__chip pgn__chip-light"
+  className="pgn__chip pgn__chip-light interactive"
+  onClick={[Function]}
+  onKeyPress={[Function]}
   role="button"
   tabIndex={0}
+>
+  <div
+    className="pgn__chip__label"
+  >
+    Test
+  </div>
+</div>
+`;
+
+exports[`<Chip /> snapshots renders with props iconAfter 1`] = `
+<div
+  className="pgn__chip pgn__chip-light"
 >
   <div
     className="pgn__chip__label p-after"
   >
     Test
   </div>
   <span
-    className="pgn__icon pgn__chip__icon-after"
+    className="pgn__icon pgn__icon__sm pgn__chip__icon-after"
   >
     <svg
       aria-hidden={true}
@@ -36,11 +50,9 @@ exports[`<Chip /> snapshots renders with props iconAfter 1`] = `
 exports[`<Chip /> snapshots renders with props iconBefore 1`] = `
 <div
   className="pgn__chip pgn__chip-light"
-  role="button"
-  tabIndex={0}
 >
   <span
-    className="pgn__icon pgn__chip__icon-before"
+    className="pgn__icon pgn__icon__sm pgn__chip__icon-before"
   >
     <svg
       aria-hidden={true}
@@ -69,11 +81,9 @@ exports[`<Chip /> snapshots renders with props iconBefore 1`] = `
 exports[`<Chip /> snapshots renders with props iconBefore and iconAfter 1`] = `
 <div
   className="pgn__chip pgn__chip-light"
-  role="button"
-  tabIndex={0}
 >
   <span
-    className="pgn__icon pgn__chip__icon-before"
+    className="pgn__icon pgn__icon__sm pgn__chip__icon-before"
   >
     <svg
       aria-hidden={true}
@@ -97,7 +107,7 @@ exports[`<Chip /> snapshots renders with props iconBefore and iconAfter 1`] = `
     Test
   </div>
   <span
-    className="pgn__icon pgn__chip__icon-after"
+    className="pgn__icon pgn__icon__sm pgn__chip__icon-after"
   >
     <svg
       aria-hidden={true}
@@ -121,8 +131,6 @@ exports[`<Chip /> snapshots renders with props iconBefore and iconAfter 1`] = `
 exports[`<Chip /> snapshots renders without props 1`] = `
 <div
   className="pgn__chip pgn__chip-light"
-  role="button"
-  tabIndex={0}
 >
   <div
     className="pgn__chip__label"

src/Chip/constants.js

@@ -1,2 +1,5 @@
 // eslint-disable-next-line import/prefer-default-export
-export const STYLE_VARIANTS = ['light', 'dark'];
+export const STYLE_VARIANTS = {
+  DARK: 'dark',
+  LIGHT: 'light',
+};