Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix EuiButtonGroup a11y and remove EuiToggle/EuiButtonToggle #4056

Merged
merged 36 commits into from
Oct 16, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
34e804f
Moved changes over from old PR
Aug 20, 2020
e4bf21e
Moved changes over from old PR
Aug 20, 2020
fb5fd51
Cleanup multi button
Aug 20, 2020
24a9623
Fix up single button
Aug 20, 2020
0c3f397
Clean slate with styles
Aug 20, 2020
ed50d7a
Clean slate with styles
Aug 20, 2020
7875eba
[EuiButton] add `minWidth` prop
Aug 20, 2020
4a0f285
Semi-fix disabled states
Aug 20, 2020
c7fb7e3
Trying to fix styles as EuiButton
Aug 20, 2020
f433aaa
Styles are working for sizes small & medium in EUI theme
Aug 21, 2020
9ca2a35
Fix flex and ghost
Aug 21, 2020
2305f81
Finish up styles for EUI theme
Aug 22, 2020
38dcaee
Fixed Amsterdam styles
Aug 22, 2020
812e5da
Fixing disabled states
Aug 24, 2020
bfccd91
Update existing tests for button group
Sep 17, 2020
972ec07
Merging to a single button compontent
Sep 17, 2020
f5f5fc2
Merging to a single button compontent. Part II and fixing types
Sep 17, 2020
87fb166
Fixing styles and a11y
Sep 17, 2020
98969d8
Adding text shift to button and fix DataGrid usage
Sep 19, 2020
1fdaf8c
[Breaking] Removing EuiToggle
Sep 19, 2020
5edd2fc
[Break] Change functionality of EuiButtonToggle
Sep 19, 2020
29dff4c
Cleanup
Oct 12, 2020
469fd15
Fix some EuiButtonToggle instances
Oct 13, 2020
b2ac204
Removing more extraneous EuiButtonToggle stuff
Oct 13, 2020
2d5994f
Switch to EuiButton
Oct 13, 2020
3b9666c
Make `name` optional
Oct 13, 2020
cda5ff8
[Breaking] Removed EuiButtonToggle in favor of EuiButton.isSelected
Oct 14, 2020
e098807
Merge remote-tracking branch 'upstream/master' into a11y/button_group
Oct 14, 2020
22b97f5
Fix button_group SASS import
Oct 14, 2020
90cbd81
Added `isSelected` to PropsForButton
Oct 14, 2020
79c7400
Add `isSelected` individually not to `PropsForButton`
Oct 15, 2020
dca6377
Some docs stuff
Oct 15, 2020
dd0445a
Merge remote-tracking branch 'upstream/master' into a11y/button_group
Oct 15, 2020
6c1f946
cl
Oct 15, 2020
d8b6f1e
typo
cchaos Oct 15, 2020
49a78c7
Simplify `isSelected` on EuiButtonGroupButton
Oct 16, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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))
Expand Down
6 changes: 2 additions & 4 deletions packages/eslint-plugin/rules/href_or_on_click.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
const componentNames = ['EuiButton', 'EuiButtonEmpty', 'EuiLink'];
const componentNames = ['EuiButton', 'EuiButtonEmpty', 'EuiLink', 'EuiBadge'];

module.exports = {
meta: {
Expand All @@ -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.`
);
}
},
Expand Down
3 changes: 0 additions & 3 deletions src-docs/src/routes.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -468,7 +466,6 @@ const navigation = [
ResizeObserverExample,
ResponsiveExample,
TextDiffExample,
ToggleExample,
WindowEventExample,
].map((example) => createExample(example)),
},
Expand Down
132 changes: 86 additions & 46 deletions src-docs/src/views/button/button_example.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
import React from 'react';

import { Link } from 'react-router-dom';

import { renderToHtml } from '../../services';

import { GuideSectionTypes } from '../../components';

import {
Expand All @@ -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');
Expand Down Expand Up @@ -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) => {}}
/>`,
];

Expand Down Expand Up @@ -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',
Expand All @@ -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
Expand All @@ -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',
Expand Down Expand Up @@ -390,5 +430,5 @@ export const ButtonExample = {
},
],
guidelines: <Guidelines />,
playground: buttonConfig,
playground: Playground,
};
14 changes: 7 additions & 7 deletions src-docs/src/views/button/button_ghost.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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>
Expand Down
8 changes: 3 additions & 5 deletions src-docs/src/views/button/button_group.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()();
Expand Down Expand Up @@ -89,6 +89,7 @@ export default () => {
id: `${idPrefix3}2`,
label: 'Align right',
iconType: 'editorAlignRight',
isDisabled: true,
},
];

Expand All @@ -104,6 +105,7 @@ export default () => {
label: 'Italic',
name: 'italic',
iconType: 'editorItalic',
isDisabled: true,
},
{
id: `${idPrefix3}5`,
Expand Down Expand Up @@ -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)}
Expand All @@ -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)}
Expand All @@ -230,7 +230,6 @@ export default () => {
<EuiSpacer size="s" />
<EuiButtonGroup
legend="Text align"
name="textAlign"
options={toggleButtonsIcons}
idSelected={toggleIconIdSelected}
onChange={(id) => onChangeIcons(id)}
Expand Down Expand Up @@ -269,7 +268,6 @@ export default () => {
</EuiTitle>
<EuiSpacer size="s" />
<EuiButtonGroup
name="textStyleCompressed"
legend="Text style"
className="eui-displayInlineBlock"
options={toggleButtonsIconsMulti}
Expand Down
Loading