Button group fixes (#1444)

elastic · Jan 16, 2019 · e2b98b3 · e2b98b3
1 parent 5a34b78
commit e2b98b3
Show file tree

Hide file tree

Showing 7 changed files with 86 additions and 45 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,6 +1,9 @@
 ## [`master`](https://github.com/elastic/eui/tree/master)
 
-No public interface changes since `6.4.0`.
+**Bug fixes**
+
+- Added `legend` for accessibility of `EuiButtonGroup` and fixed opacity of disabled input ([#1444](https://github.com/elastic/eui/pull/1444))
+
 
 ## [`6.4.0`](https://github.com/elastic/eui/tree/v6.4.0)
 

diff --git a/src-docs/src/views/button/button_example.js b/src-docs/src/views/button/button_example.js
@@ -15,6 +15,7 @@ import {
   EuiCode,
   EuiButtonGroup,
   EuiButtonToggle,
+  EuiCallOut,
 } from '../../../../src/components';
 
 import Button from './button';
@@ -251,6 +252,12 @@ export const ButtonExample = {
             (default) or <EuiCode>&quot;primary&quot;</EuiCode>. If your just displaying a group of
             icons, add the prop <EuiCode>isIconOnly</EuiCode>.
           </p>
+          <EuiCallOut title="Accessibility">
+            <p>
+              In order for groups to be properly read as groups with a title, add the <EuiCode>legend</EuiCode> prop.
+              This is only for accessiblity, however, so it will be visibly hidden.
+            </p>
+          </EuiCallOut>
         </div>
       ),
       demo: <ButtonGroup />,

diff --git a/src-docs/src/views/button/button_group.js b/src-docs/src/views/button/button_group.js
@@ -58,18 +58,22 @@ export default class extends Component {
     this.toggleButtonsIconsMulti = [{
       id: `${idPrefix3}3`,
       label: 'Bold',
+      name: 'bold',
       iconType: 'editorBold',
     }, {
       id: `${idPrefix3}4`,
       label: 'Italic',
+      name: 'italic',
       iconType: 'editorItalic',
     }, {
       id: `${idPrefix3}5`,
       label: 'Underline',
+      name: 'underline',
       iconType: 'editorUnderline',
     }, {
       id: `${idPrefix3}6`,
       label: 'Strikethrough',
+      name: 'strikethrough',
       iconType: 'editorStrike',
     }];
 
@@ -119,7 +123,7 @@ export default class extends Component {
     return (
       <Fragment>
         <EuiButtonGroup
-          name="Basic"
+          legend="This is a basic group"
           options={this.toggleButtons}
           idSelected={this.state.toggleIdSelected}
           onChange={this.onChange}
@@ -132,7 +136,8 @@ export default class extends Component {
         <EuiSpacer size="s" />
 
         <EuiButtonGroup
-          name="Primary"
+          legend="This is a primary group"
+          name="primary"
           options={this.toggleButtonsMulti}
           idToSelectedMap={this.state.toggleIdToSelectedMap}
           onChange={this.onChangeMulti}
@@ -147,7 +152,8 @@ export default class extends Component {
         <EuiSpacer size="s" />
 
         <EuiButtonGroup
-          name="Disabled"
+          legend="This is a disabled group"
+          name="disabledGroup"
           options={this.toggleButtons}
           idSelected={this.state.toggleIdSelected}
           onChange={this.onChange}
@@ -162,7 +168,8 @@ export default class extends Component {
         <EuiSpacer size="s" />
 
         <EuiButtonGroup
-          name="Text align"
+          legend="Text align"
+          name="textAlign"
           className="eui-displayInlineBlock"
           options={this.toggleButtonsIcons}
           idSelected={this.state.toggleIconIdSelected}
@@ -173,7 +180,7 @@ export default class extends Component {
         &nbsp;&nbsp;
 
         <EuiButtonGroup
-          name="Text style"
+          legend="Text style"
           className="eui-displayInlineBlock"
           options={this.toggleButtonsIconsMulti}
           idToSelectedMap={this.state.toggleIconIdToSelectedMap}

diff --git a/src/components/button/button_group/__snapshots__/button_group.test.js.snap b/src/components/button/button_group/__snapshots__/button_group.test.js.snap
@@ -1,9 +1,11 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
 exports[`EuiButtonGroup is rendered 1`] = `
-<div
-  aria-label="aria-label"
-  class="euiButtonGroup testClass1 testClass2"
-  data-test-subj="test subject string"
-/>
+<fieldset>
+  <div
+    aria-label="aria-label"
+    class="euiButtonGroup testClass1 testClass2"
+    data-test-subj="test subject string"
+  />
+</fieldset>
 `;
diff --git a/src/components/button/button_group/button_group.js b/src/components/button/button_group/button_group.js
@@ -2,6 +2,7 @@ import React from 'react';
 import PropTypes from 'prop-types';
 import classNames from 'classnames';
 
+import { EuiScreenReaderOnly } from '../../accessibility';
 import { EuiButtonToggle } from '../button_toggle';
 import { TOGGLE_TYPES } from '../../toggle';
 
@@ -15,6 +16,7 @@ export const EuiButtonGroup = ({
   isFullWidth,
   isIconOnly,
   name,
+  legend,
   onChange,
   options,
   type,
@@ -29,40 +31,50 @@ export const EuiButtonGroup = ({
     className,
   );
 
+  let legendNode;
+  if (legend) {
+    legendNode = (
+      <EuiScreenReaderOnly><legend>{legend}</legend></EuiScreenReaderOnly>
+    );
+  }
+
   return (
-    <div className={classes} {...rest}>
-      {options.map((option, index) => {
-
-        let isSelectedState;
-        if (type === 'multi') {
-          isSelectedState = idToSelectedMap[option.id] || false;
-        } else {
-          isSelectedState = option.id === idSelected;
-        }
-
-        return (
-          <EuiButtonToggle
-            className="euiButtonGroup__button"
-            color={color}
-            fill={isSelectedState}
-            iconSide={option.iconSide}
-            iconType={option.iconType}
-            id={option.id}
-            isDisabled={isDisabled || option.isDisabled}
-            isIconOnly={isIconOnly}
-            isSelected={isSelectedState}
-            key={index}
-            label={option.label}
-            name={name}
-            onChange={onChange.bind(null, option.id, option.value)}
-            size={buttonSize}
-            toggleClassName="euiButtonGroup__toggle"
-            type={type}
-            value={option.value}
-          />
-        );
-      })}
-    </div>
+    <fieldset>
+      {legendNode}
+
+      <div className={classes} {...rest}>
+        {options.map((option, index) => {
+          let isSelectedState;
+          if (type === 'multi') {
+            isSelectedState = idToSelectedMap[option.id] || false;
+          } else {
+            isSelectedState = option.id === idSelected;
+          }
+
+          return (
+            <EuiButtonToggle
+              className="euiButtonGroup__button"
+              color={color}
+              fill={isSelectedState}
+              iconSide={option.iconSide}
+              iconType={option.iconType}
+              id={option.id}
+              isDisabled={isDisabled || option.isDisabled}
+              isIconOnly={isIconOnly}
+              isSelected={isSelectedState}
+              key={index}
+              label={option.label}
+              name={option.name || name}
+              onChange={onChange.bind(null, option.id, option.value)}
+              size={buttonSize}
+              toggleClassName="euiButtonGroup__toggle"
+              type={type}
+              value={option.value}
+            />
+          );
+        })}
+      </div>
+    </fieldset>
   );
 };
 
@@ -112,6 +124,11 @@ EuiButtonGroup.propTypes = {
    * Map of ids of selected options for `type="multi"`
    */
   idToSelectedMap: PropTypes.objectOf(PropTypes.bool),
+
+  /**
+   * Adds a hidden legend to the group for accessiblity
+   */
+  legend: PropTypes.string,
 };
 
 EuiButtonGroup.defaultProps = {

diff --git a/src/components/form/switch/_mixins.scss b/src/components/form/switch/_mixins.scss
@@ -1,6 +1,7 @@
 @mixin euiHiddenSelectableInput {
   position: absolute;
-  opacity: 0; /* 1 */
+  // sass-lint:disable no-important
+  opacity: 0 !important; // Make sure it's still hidden when :disabled
   width: 100%;
   height: 100%;
   cursor: pointer;

diff --git a/src/global_styling/reset/_reset.scss b/src/global_styling/reset/_reset.scss
@@ -133,3 +133,7 @@ table {
 hr {
   margin: 0;
 }
+
+fieldset {
+  min-inline-size: auto;
+}