Fix EuiButtonGroup a11y and remove EuiToggle/EuiButtonToggle (#4056)

* [EuiButton] add `minWidth` prop * Adding text shift to button and fix DataGrid usage and other tests * [Breaking] Removing EuiToggle * [Breaking] Removed EuiButtonToggle in favor of EuiButton.isSelected
elastic · Oct 16, 2020 · 9e55637 · 9e55637
1 parent cfd1dcc
commit 9e55637
Show file tree

Hide file tree

Showing 60 changed files with 3,291 additions and 2,799 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,14 @@
 ## [`master`](https://github.com/elastic/eui/tree/master)
 
+- Added `minWidth` prop to `EuiButton` ([4056](https://github.com/elastic/eui/pull/4056))
+- Added `isSelected` prop to easily turn `EuiButton`, `EuiButtonEmpty`, and `EuiButtonIcon` into toggle buttons ([4056](https://github.com/elastic/eui/pull/4056))
+- Updated `EuiButtonGroup` props and render for better accessibility ([4056](https://github.com/elastic/eui/pull/4056))
+
+**Breaking changes**
+
+- Removed `EuiToggle` and `EuiButtonToggle` in favor of `aria-pressed` ([4056](https://github.com/elastic/eui/pull/4056))
+- Updated `legend` and `idSelected` props of `EuiButtonGroup` to be required ([4056](https://github.com/elastic/eui/pull/4056))
+
 **Theme: Amsterdam**
 
 - Tightened `line-height` for some `EuiTitle` sizes ([4133](https://github.com/elastic/eui/pull/4133))

diff --git a/packages/eslint-plugin/rules/href_or_on_click.js b/packages/eslint-plugin/rules/href_or_on_click.js
@@ -1,4 +1,4 @@
-const componentNames = ['EuiButton', 'EuiButtonEmpty', 'EuiLink'];
+const componentNames = ['EuiButton', 'EuiButtonEmpty', 'EuiLink', 'EuiBadge'];
 
 module.exports = {
   meta: {
@@ -24,9 +24,7 @@ module.exports = {
         if (hasHref && hasOnClick) {
           context.report(
             node,
-            `<${
-              node.name.name
-            }> accepts either \`href\` or \`onClick\`, not both.`
+            `<${node.name.name}> accepts either \`href\` or \`onClick\`, not both.`
           );
         }
       },

diff --git a/src-docs/src/routes.js b/src-docs/src/routes.js
@@ -204,8 +204,6 @@ import { ToastExample } from './views/toast/toast_example';
 
 import { ToolTipExample } from './views/tool_tip/tool_tip_example';
 
-import { ToggleExample } from './views/toggle/toggle_example';
-
 import { TourExample } from './views/tour/tour_example';
 
 import { WindowEventExample } from './views/window_event/window_event_example';
@@ -468,7 +466,6 @@ const navigation = [
       ResizeObserverExample,
       ResponsiveExample,
       TextDiffExample,
-      ToggleExample,
       WindowEventExample,
     ].map((example) => createExample(example)),
   },

diff --git a/src-docs/src/views/button/button_example.js b/src-docs/src/views/button/button_example.js
@@ -1,9 +1,7 @@
 import React from 'react';
-
 import { Link } from 'react-router-dom';
 
 import { renderToHtml } from '../../services';
-
 import { GuideSectionTypes } from '../../components';
 
 import {
@@ -12,12 +10,14 @@ import {
   EuiButtonIcon,
   EuiCode,
   EuiButtonGroup,
-  EuiButtonToggle,
   EuiCallOut,
   EuiText,
 } from '../../../../src/components';
+
+import { EuiButtonGroupOptionProps } from '!!prop-loader!../../../../src/components/button/button_group/button_group';
+
 import Guidelines from './guidelines';
-import buttonConfig from './playground';
+import Playground from './playground';
 
 import Button from './button';
 const buttonSource = require('!!raw-loader!./button');
@@ -82,30 +82,60 @@ const buttonLoadingSnippet = `<EuiButton isLoading={true}>
 import ButtonToggle from './button_toggle';
 const buttonToggleSource = require('!!raw-loader!./button_toggle');
 const buttonToggleHtml = renderToHtml(ButtonToggle);
-const buttonToggleSnippet = `<EuiButtonToggle
-  label={label}
+const buttonToggleSnippet = [
+  `<EuiButton
   iconType={toggleOn ? onIcon : offIcon}
-  onChange={onToggleChange}
+  onClick={onToggleChange}
+>
+  {toggleOn ? onLabel : offLabel}
+</EuiButton>
+`,
+  `<EuiButton
   isSelected={toggleOn}
-/>`;
+  fill={toggleOn}
+  onClick={onToggleChange}
+  >
+  <!-- Button text -->
+</EuiButton>`,
+  `<EuiButton
+  aria-pressed={toggleOn}
+  fill={toggleOn}
+  onClick={onToggleChange}
+>
+  <!-- Button text -->
+</EuiButton>`,
+];
 
 import ButtonGroup from './button_group';
 const buttonGroupSource = require('!!raw-loader!./button_group');
 const buttonGroupHtml = renderToHtml(ButtonGroup);
 const buttonGroupSnippet = [
   `<EuiButtonGroup
+  type="single"
   legend={legend}
-  options={toggleButtons}
-  idSelected={toggleIdSelected}
-  onChange={onChange}
+  name={name}
+  options={[
+    {
+      id,
+      label'
+    }
+  ]}
+  idSelected={idSelected}
+  onChange={(optionId) => {}}
 />`,
   `<EuiButtonGroup
-  legend={legend}
-  options={toggleButtonsIconsMulti}
-  idToSelectedMap={toggleIconIdToSelectedMap}
-  onChange={onChangeIconsMulti}
   type="multi"
   isIconOnly
+  legend={legend}
+  options={[
+    {
+      id,
+      label,
+      iconType,
+    }
+  ]}
+  idToSelectedMap={{ optionId: true }}
+  onChange={(optionId, optionValue) => {}}
 />`,
 ];
 
@@ -292,26 +322,43 @@ export const ButtonExample = {
         },
       ],
       text: (
-        <div>
+        <>
           <p>
-            This is a specialized component that combines{' '}
-            <strong>EuiButton</strong> and <strong>EuiToggle</strong> to create
-            a button with an on/off state. You can pass all the same parameters
-            to it as you can to <strong>EuiButton</strong>. The main difference
-            is that, it does not accept any children, but a{' '}
-            <EuiCode>label</EuiCode> prop instead. This is for the handling of
-            accessibility with the <strong>EuiToggle</strong>.
+            You can create a toggle style button with any button type like the
+            standard <strong>EuiButton</strong>, <strong>EuiButtonEmpty</strong>
+            , or <strong>EuiButtonIcon</strong>. Use state management to handle
+            the visual differences for on and off. Though there are two{' '}
+            <strong>exclusive</strong> situations to consider.
           </p>
-          <p>
-            The <strong>EuiButtonToggle</strong> does not have any inherit
-            visual state differences. These you must apply in your
-            implementation.
-          </p>
-        </div>
+          <ol>
+            <li>
+              If your button changes its readable <strong>text</strong>, via
+              children or <EuiCode>aria-label</EuiCode>, then there is no
+              additional accessibility concern.
+            </li>
+            <li>
+              If your button only changes the <strong>visual</strong>{' '}
+              appearance, you must add <EuiCode>aria-pressed</EuiCode> passing a
+              boolean for the on and off states. All EUI button types provide a
+              helper prop for this called <EuiCode>isSelected</EuiCode>.
+            </li>
+          </ol>
+          <EuiCallOut
+            iconType="accessibility"
+            color="warning"
+            title={
+              <span>
+                Do not add <EuiCode>aria-pressed</EuiCode> or{' '}
+                <EuiCode>isSelected</EuiCode> if you also change the readable
+                text.
+              </span>
+            }
+          />
+        </>
       ),
       demo: <ButtonToggle />,
       snippet: buttonToggleSnippet,
-      props: { EuiButtonToggle },
+      props: { EuiButton, EuiButtonIcon },
     },
     {
       title: 'Button groups',
@@ -328,19 +375,11 @@ export const ButtonExample = {
       text: (
         <div>
           <p>
-            <strong>EuiButtonGroups</strong> are handled similarly to the way
-            checkbox and radio groups are handled but made to look like buttons.
-            They group multiple <strong>EuiButtonToggles</strong> and utilize
-            the <EuiCode language="js">type=&quot;single&quot;</EuiCode> or{' '}
+            <strong>EuiButtonGroups</strong> utilize the{' '}
+            <EuiCode language="js">type=&quot;single&quot;</EuiCode> or{' '}
             <EuiCode language="js">&quot;multi&quot;</EuiCode> prop to determine
-            whether multiple or only single selections are allowed per group.
-          </p>
-          <p>
-            Stylistically, all button groups are the size of small buttons, do
-            not stretch to fill the container, and typically should only be{' '}
-            <EuiCode language="js">color=&quot;text&quot;</EuiCode> (default) or{' '}
-            <EuiCode language="js">&quot;primary&quot;</EuiCode>. If you&apos;re
-            just displaying a group of icons, add the prop{' '}
+            whether multiple or only single selections are allowed per group. If
+            you&apos;re just displaying a group of icons, add the prop{' '}
             <EuiCode>isIconOnly</EuiCode>.
           </p>
           <EuiCallOut
@@ -349,16 +388,17 @@ export const ButtonExample = {
             title={
               <span>
                 In order for groups to be properly read as groups with a title,
-                add the <EuiCode>legend</EuiCode> prop. This is only for
-                accessibility, however, so it will be visibly hidden.
+                the <EuiCode>legend</EuiCode> prop is <strong>required</strong>.
+                This is only for accessibility, however, so it will be visibly
+                hidden.
               </span>
             }
           />
         </div>
       ),
       demo: <ButtonGroup />,
       snippet: buttonGroupSnippet,
-      props: { EuiButtonGroup },
+      props: { EuiButtonGroup, EuiButtonGroupOptionProps },
     },
     {
       title: 'Ghost',
@@ -390,5 +430,5 @@ export const ButtonExample = {
     },
   ],
   guidelines: <Guidelines />,
-  playground: buttonConfig,
+  playground: Playground,
 };
diff --git a/src-docs/src/views/button/button_ghost.js b/src-docs/src/views/button/button_ghost.js
@@ -6,15 +6,14 @@ import {
   EuiButtonIcon,
   EuiFlexGroup,
   EuiFlexItem,
-  EuiButtonToggle,
   EuiPanel,
 } from '../../../../src/components';
 
 export default () => {
   const [toggle0On, setToggle0On] = useState(false);
 
-  const onToggle0Change = (e) => {
-    setToggle0On(e.target.checked);
+  const onToggle0Change = () => {
+    setToggle0On((isOn) => !isOn);
   };
 
   return (
@@ -71,12 +70,13 @@ export default () => {
         </EuiFlexItem>
 
         <EuiFlexItem grow={false}>
-          <EuiButtonToggle
+          <EuiButton
             color="ghost"
-            label="Toggle Me"
+            isSelected={toggle0On}
             fill={toggle0On}
-            onChange={onToggle0Change}
-          />
+            onClick={onToggle0Change}>
+            Toggle me
+          </EuiButton>
         </EuiFlexItem>
       </EuiFlexGroup>
     </EuiPanel>

diff --git a/src-docs/src/views/button/button_group.js b/src-docs/src/views/button/button_group.js
@@ -4,10 +4,10 @@ import {
   EuiButtonGroup,
   EuiSpacer,
   EuiTitle,
+  EuiPanel,
 } from '../../../../src/components';
 
 import { htmlIdGenerator } from '../../../../src/services';
-import { EuiPanel } from '../../../../src/components/panel/panel';
 
 const idPrefix = htmlIdGenerator()();
 const idPrefix2 = htmlIdGenerator()();
@@ -89,6 +89,7 @@ export default () => {
       id: `${idPrefix3}2`,
       label: 'Align right',
       iconType: 'editorAlignRight',
+      isDisabled: true,
     },
   ];
 
@@ -104,6 +105,7 @@ export default () => {
       label: 'Italic',
       name: 'italic',
       iconType: 'editorItalic',
+      isDisabled: true,
     },
     {
       id: `${idPrefix3}5`,
@@ -201,7 +203,6 @@ export default () => {
       <EuiSpacer size="s" />
       <EuiButtonGroup
         legend="This is a primary group"
-        name="primary"
         options={toggleButtonsMulti}
         idToSelectedMap={toggleIdToSelectedMap}
         onChange={(id) => onChangeMulti(id)}
@@ -215,7 +216,6 @@ export default () => {
       <EuiSpacer size="s" />
       <EuiButtonGroup
         legend="This is a disabled group"
-        name="disabledGroup"
         options={toggleButtonsDisabled}
         idSelected={toggleIdDisabled}
         onChange={(id) => onChangeDisabled(id)}
@@ -230,7 +230,6 @@ export default () => {
       <EuiSpacer size="s" />
       <EuiButtonGroup
         legend="Text align"
-        name="textAlign"
         options={toggleButtonsIcons}
         idSelected={toggleIconIdSelected}
         onChange={(id) => onChangeIcons(id)}
@@ -269,7 +268,6 @@ export default () => {
         </EuiTitle>
         <EuiSpacer size="s" />
         <EuiButtonGroup
-          name="textStyleCompressed"
           legend="Text style"
           className="eui-displayInlineBlock"
           options={toggleButtonsIconsMulti}