fix: remove state from ToggleButton (#1897)

contentful · Feb 22, 2022 · e3058e0 · e3058e0 · vercel · Feb 22, 2022
1 parent a537f6b
commit e3058e0
Show file tree

Hide file tree

Showing 3 changed files with 112 additions and 44 deletions.
diff --git a/packages/components/button/src/ToggleButton/ToggleButton.test.tsx b/packages/components/button/src/ToggleButton/ToggleButton.test.tsx
@@ -6,38 +6,50 @@ import { axe } from '@/scripts/test/axeHelper';
 
 import { ToggleButton } from '.';
 describe('ToggleButton', function () {
+  const mockOnToggle = jest.fn();
+
   it('renders the component', () => {
-    render(<ToggleButton>Toggle</ToggleButton>);
+    render(<ToggleButton onToggle={mockOnToggle}>Toggle</ToggleButton>);
 
     expect(screen.getByRole('button')).toBeTruthy();
   });
 
   it('renders the component with an additional class name', () => {
     const additionalClassName = 'my-extra-class';
-    render(<ToggleButton className={additionalClassName}>Toggle</ToggleButton>);
+    render(
+      <ToggleButton onToggle={mockOnToggle} className={additionalClassName}>
+        Toggle
+      </ToggleButton>,
+    );
 
     const button = screen.getByRole('button');
     expect(button.classList.contains(additionalClassName)).toBeTruthy();
   });
 
   it('renders the component active', () => {
-    render(<ToggleButton isActive>Toggle</ToggleButton>);
+    render(
+      <ToggleButton onToggle={mockOnToggle} isActive>
+        Toggle
+      </ToggleButton>,
+    );
 
     const button = screen.getByRole('button');
     expect(button.getAttribute('aria-pressed')).toBe('true');
     expect(button.getAttribute('data-state')).toBe('on');
   });
 
   it('renders the component with icon', () => {
-    render(<ToggleButton icon={<PreviewIcon />}>Toggle</ToggleButton>);
+    render(
+      <ToggleButton onToggle={mockOnToggle} icon={<PreviewIcon />}>
+        Toggle
+      </ToggleButton>,
+    );
 
     const button = screen.getByRole('button');
     expect(button.getElementsByTagName('svg')).toHaveLength(1);
   });
 
   it('should not dispatch onClick if disabled', () => {
-    const mockOnToggle = jest.fn();
-
     render(
       <ToggleButton onToggle={mockOnToggle} icon={<PreviewIcon />} isDisabled>
         Toggle
@@ -50,7 +62,9 @@ describe('ToggleButton', function () {
   });
 
   it('has no a11y issues', async () => {
-    const { container } = render(<ToggleButton>Toggle</ToggleButton>);
+    const { container } = render(
+      <ToggleButton onToggle={mockOnToggle}>Toggle</ToggleButton>,
+    );
     const results = await axe(container);
 
     expect(results).toHaveNoViolations();

diff --git a/packages/components/button/src/ToggleButton/ToggleButton.tsx b/packages/components/button/src/ToggleButton/ToggleButton.tsx
@@ -1,4 +1,4 @@
-import React, { useState } from 'react';
+import React from 'react';
 import { cx } from 'emotion';
 import { CommonProps, ExpandProps } from '@contentful/f36-core';
 import { Button } from '../Button';
@@ -23,7 +23,7 @@ export interface ToggleButtonProps extends CommonProps {
   /**
    * Function triggered when the toggle button is clicked.
    */
-  onToggle?: () => void;
+  onToggle: () => void;
 
   /**
    * Determines size variation of Button component
@@ -44,21 +44,18 @@ function _ToggleButton(props: ExpandProps<ToggleButtonProps>, ref) {
     testId = 'cf-ui-toggle-button',
     children,
     className,
-    isDisabled,
-    isActive,
+    isDisabled = false,
+    isActive = false,
     icon,
     onToggle,
-    size,
+    size = 'medium',
     ...otherProps
   } = props;
 
-  const [active, setActive] = useState(isActive);
-
-  const styles = getStyles({ isActive: active, isDisabled });
+  const styles = getStyles({ isActive, isDisabled });
 
   const handleToggle = () => {
     if (!isDisabled && onToggle) {
-      setActive(!active);
       onToggle();
     }
   };
@@ -73,8 +70,8 @@ function _ToggleButton(props: ExpandProps<ToggleButtonProps>, ref) {
       className={cx(styles.toggleButton, className)}
       startIcon={icon}
       isDisabled={isDisabled}
-      aria-pressed={active}
-      data-state={active ? 'on' : 'off'}
+      aria-pressed={isActive}
+      data-state={isActive ? 'on' : 'off'}
       {...otherProps}
     >
       {children}

diff --git a/packages/components/button/stories/ToggleButton.stories.tsx b/packages/components/button/stories/ToggleButton.stories.tsx
@@ -1,12 +1,13 @@
-import React from 'react';
+import React, { useState } from 'react';
 import type { Meta } from '@storybook/react/types-6-0';
 import { SectionHeading } from '@contentful/f36-typography';
 import { action } from '@storybook/addon-actions';
+
 import { Flex, Stack } from '@contentful/f36-core';
 import { Icon } from '@contentful/f36-icon';
 import * as icons from '@contentful/f36-icons';
-import { ButtonGroup } from '../src';
 
+import { ButtonGroup } from '../src';
 import { ToggleButton } from '../src/ToggleButton';
 
 export default {
@@ -27,39 +28,95 @@ export default {
   },
 } as Meta;
 
-export const basic = ({ icon, children, ...props }) => (
-  <div>
-    <ToggleButton icon={icon && <Icon as={icons[icon]} />} {...props}>
+export const Basic = ({ icon, children, isDisabled }) => {
+  const [isActive, setIsActive] = useState(false);
+
+  return (
+    <ToggleButton
+      isDisabled={isDisabled}
+      isActive={isActive}
+      onToggle={() => {
+        setIsActive(!isActive);
+      }}
+      icon={icon && <Icon as={icons[icon]} />}
+    >
       {children}
     </ToggleButton>
-  </div>
-);
+  );
+};
 
-basic.args = {
+Basic.args = {
   isDisabled: false,
-  isActive: false,
-  icon: undefined,
-  children: 'Single',
-  onToggle: action('toggled'),
+  icon: 'ThumbUpTrimmedIcon',
+  children: 'Like',
 };
 
-export const grouped = ({ icon }) => (
-  <div>
+export const Grouped = () => {
+  const [isItalic, setIsItalic] = useState(false);
+  const [isBold, setIsBold] = useState(true);
+  const [isUnderline, setIsUnderline] = useState(false);
+
+  return (
     <ButtonGroup>
-      <ToggleButton>Apples</ToggleButton>
-      <ToggleButton isActive>Pears</ToggleButton>
-      <ToggleButton>Peaches</ToggleButton>
-      <ToggleButton>Mangos</ToggleButton>
-      <ToggleButton isActive icon={icon && <Icon as={icons[icon]} />}>
-        Kiwis
-      </ToggleButton>
-      <ToggleButton isDisabled>Bananas</ToggleButton>
+      <ToggleButton
+        isActive={isItalic}
+        icon={<Icon as={icons.FormatItalicIcon} />}
+        aria-label="Italic"
+        size="small"
+        onToggle={() => {
+          setIsItalic(!isItalic);
+        }}
+      />
+      <ToggleButton
+        isActive={isBold}
+        icon={<Icon as={icons.FormatBoldIcon} />}
+        aria-label="Bold"
+        size="small"
+        onToggle={() => {
+          setIsBold(!isBold);
+        }}
+      />
+      <ToggleButton
+        isActive={isUnderline}
+        icon={<Icon as={icons.FormatUnderlinedIcon} />}
+        aria-label="Underline"
+        size="small"
+        onToggle={() => {
+          setIsUnderline(!isUnderline);
+        }}
+      />
     </ButtonGroup>
-  </div>
-);
+  );
+};
 
-grouped.args = {
-  icon: 'PreviewIcon',
+export const GroupedWithOnlyOneActive = () => {
+  const [isActive, setIsActive] = useState('bold');
+
+  return (
+    <ButtonGroup>
+      <ToggleButton
+        isActive={isActive === 'italic'}
+        icon={<Icon as={icons.FormatItalicIcon} />}
+        aria-label="Italic"
+        size="small"
+        onToggle={() => setIsActive('italic')}
+      />
+      <ToggleButton
+        isActive={isActive === 'bold'}
+        icon={<Icon as={icons.FormatBoldIcon} />}
+        aria-label="Bold"
+        size="small"
+        onToggle={() => setIsActive('bold')}
+      />
+      <ToggleButton
+        isActive={isActive === 'underline'}
+        icon={<Icon as={icons.FormatUnderlinedIcon} />}
+        aria-label="Underline"
+        size="small"
+        onToggle={() => setIsActive('underline')}
+      />
+    </ButtonGroup>
+  );
 };
 
 export const Overview = ({ icon, ...props }) => (