Skip to content

Commit

Permalink
Fix bug in which disabled EuiButton, EuiButtonEmpty, and EuiButtonIco…
Browse files Browse the repository at this point in the history
…n didn't exhibit disabled behavior and visual state when href was defined. (elastic#862)
  • Loading branch information
cjcenizal authored and chandlerprall committed May 24, 2018
1 parent 4a57654 commit 90d0904
Show file tree
Hide file tree
Showing 12 changed files with 122 additions and 21 deletions.
6 changes: 5 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,14 +1,18 @@
## [`master`](https://github.com/elastic/eui/tree/master)

No public interface changes since `0.0.49`.
**Bug fixes**

- `EuiButton`, `EuiButtonEmpty`, and `EuiButtonIcon` now look and behave disabled when `isDisabled={true}` ([#862](https://github.com/elastic/eui/pull/862))

## [`0.0.49`](https://github.com/elastic/eui/tree/v0.0.49)

**Breaking changes**

- EUI requires React `16.3` or higher ([#849](https://github.com/elastic/eui/pull/849))
- `EuiHeaderBreadcrumbs` refactored to use `EuiBreadcrumbs`. This removed all child components of `EuiHeaderBreadcrumbs`. ([#844](https://github.com/elastic/eui/pull/844))

**Bug fixes**

- `EuiInMemoryTable` now applies its search filter ([#851](https://github.com/elastic/eui/pull/851))
- `EuiInMemoryTable` and `EuiBasicTable` now pass unknown props through to their child ([#836](https://github.com/elastic/eui/pull/836))
- Added `EuiHeaderLinks` which allow you to construct navigation in the header in place of the app menu. ([#844](https://github.com/elastic/eui/pull/844))
Expand Down
53 changes: 37 additions & 16 deletions src-docs/src/views/button/button_as_link.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React from 'react';
import React, { Fragment } from 'react';

import {
EuiButton,
Expand All @@ -9,20 +9,41 @@ import {
} from '../../../../src/components';

export default () => (
<EuiFlexGroup gutterSize="s" alignItems="center">
<EuiFlexItem grow={false}>
<EuiButton href="http://www.elastic.co">
Link to elastic.co
</EuiButton>
</EuiFlexItem>
<Fragment>
<EuiFlexGroup gutterSize="s" alignItems="center">
<EuiFlexItem grow={false}>
<EuiButton href="http://www.elastic.co">
Link to elastic.co
</EuiButton>
</EuiFlexItem>

<EuiFlexItem grow={false}>
<EuiButtonEmpty href="http://www.elastic.co">
Link to elastic.co
</EuiButtonEmpty>
</EuiFlexItem>
<EuiFlexItem grow={false}>
<EuiButtonIcon href="http://www.elastic.co" iconType="link" aria-label="This is a link" />
</EuiFlexItem>
</EuiFlexGroup>
<EuiFlexItem grow={false}>
<EuiButtonEmpty href="http://www.elastic.co">
Link to elastic.co
</EuiButtonEmpty>
</EuiFlexItem>

<EuiFlexItem grow={false}>
<EuiButtonIcon href="http://www.elastic.co" iconType="link" aria-label="This is a link" />
</EuiFlexItem>
</EuiFlexGroup>

<EuiFlexGroup gutterSize="s" alignItems="center">
<EuiFlexItem grow={false}>
<EuiButton href="http://www.elastic.co" isDisabled>
Disabled link
</EuiButton>
</EuiFlexItem>

<EuiFlexItem grow={false}>
<EuiButtonEmpty href="http://www.elastic.co" isDisabled>
Disabled empty link
</EuiButtonEmpty>
</EuiFlexItem>

<EuiFlexItem grow={false}>
<EuiButtonIcon href="http://www.elastic.co" iconType="link" aria-label="This is a link" isDisabled />
</EuiFlexItem>
</EuiFlexGroup>
</Fragment>
);
16 changes: 16 additions & 0 deletions src/components/button/__snapshots__/button.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,22 @@ exports[`EuiButton props isDisabled is rendered 1`] = `
</button>
`;

exports[`EuiButton props isDisabled renders a button even when href is defined 1`] = `
<button
class="euiButton euiButton--primary"
disabled=""
type="button"
>
<span
class="euiButton__content"
>
<span
class="euiButton__text"
/>
</span>
</button>
`;

exports[`EuiButton props isLoading is rendered 1`] = `
<button
class="euiButton euiButton--primary"
Expand Down
4 changes: 3 additions & 1 deletion src/components/button/button.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,9 @@ export const EuiButton = ({
);
}

if (href) {
// <a> elements don't respect the `disabled` attribute. So if we're disabled, we'll just pretend
// this is a button and piggyback off its disabled styles.
if (href && !isDisabled) {
const secureRel = getSecureRelForTarget(target, rel);

return (
Expand Down
9 changes: 9 additions & 0 deletions src/components/button/button.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,15 @@ describe('EuiButton', () => {
expect(component)
.toMatchSnapshot();
});

it('renders a button even when href is defined', () => {
const component = render(
<EuiButton href="#" isDisabled />
);

expect(component)
.toMatchSnapshot();
});
});

describe('isLoading', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,20 @@ exports[`EuiButtonEmpty props isDisabled is rendered 1`] = `
</button>
`;

exports[`EuiButtonEmpty props isDisabled renders a button even when href is defined 1`] = `
<button
class="euiButtonEmpty euiButtonEmpty--primary"
disabled=""
type="button"
>
<span
class="euiButtonEmpty__content"
>
<span />
</span>
</button>
`;

exports[`EuiButtonEmpty props size l is rendered 1`] = `
<button
class="euiButtonEmpty euiButtonEmpty--primary euiButtonEmpty--large"
Expand Down
4 changes: 3 additions & 1 deletion src/components/button/button_empty/button_empty.js
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,9 @@ export const EuiButtonEmpty = ({
);
}

if (href) {
// <a> elements don't respect the `disabled` attribute. So if we're disabled, we'll just pretend
// this is a button and piggyback off its disabled styles.
if (href && !isDisabled) {
const secureRel = getSecureRelForTarget(target, rel);

return (
Expand Down
9 changes: 9 additions & 0 deletions src/components/button/button_empty/button_empty.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,15 @@ describe('EuiButtonEmpty', () => {
expect(component)
.toMatchSnapshot();
});

it('renders a button even when href is defined', () => {
const component = render(
<EuiButtonEmpty href="#" isDisabled />
);

expect(component)
.toMatchSnapshot();
});
});

describe('iconType', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,3 +95,12 @@ exports[`EuiButtonIcon props isDisabled is rendered 1`] = `
type="button"
/>
`;

exports[`EuiButtonIcon props isDisabled renders a button even when href is defined 1`] = `
<button
aria-label="button"
class="euiButtonIcon euiButtonIcon--primary"
disabled=""
type="button"
/>
`;
6 changes: 5 additions & 1 deletion src/components/button/button_icon/_button_icon.scss
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@
color: $euiButtonColorDisabled;
pointer-events: none;

.euiButtonIcon__icon {
pointer-events: auto;
cursor: not-allowed;
}

&:hover, &:focus {
background-color: $euiColorEmptyShade;
text-decoration: none;
Expand All @@ -44,7 +49,6 @@ $buttonTypes: (
}

&:hover, &:focus {

@if ($name == 'disabled') {
cursor: not-allowed;
}
Expand Down
4 changes: 3 additions & 1 deletion src/components/button/button_icon/button_icon.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,9 @@ export const EuiButtonIcon = ({
);
}

if (href) {
// <a> elements don't respect the `disabled` attribute. So if we're disabled, we'll just pretend
// this is a button and piggyback off its disabled styles.
if (href && !isDisabled) {
const secureRel = getSecureRelForTarget(target, rel);

return (
Expand Down
9 changes: 9 additions & 0 deletions src/components/button/button_icon/button_icon.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,15 @@ describe('EuiButtonIcon', () => {
expect(component)
.toMatchSnapshot();
});

it('renders a button even when href is defined', () => {
const component = render(
<EuiButtonIcon aria-label="button" href="#" isDisabled />
);

expect(component)
.toMatchSnapshot();
});
});

describe('iconType', () => {
Expand Down

0 comments on commit 90d0904

Please sign in to comment.