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

Removed EuiKeyPadMenuItemButton in favor of just EuiKeyPadMenuItem #3062

Merged
merged 9 commits into from
Mar 19, 2020
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@
- Added `useResizeObserver` hook ([#2991](https://github.com/elastic/eui/pull/2991))
- Added `showColumnSelector.allowHide` and `showColumnSelector.allowReorder` props to `EuiDataGrid` UI configuration ([#2993](https://github.com/elastic/eui/pull/2993))

**Breaking changes**

- Removed `EuiKeyPadMenuItemButton` in favor of just `EuiKeyPadMenuItem` that can also accept an `onClick` ([#3062](https://github.com/elastic/eui/pull/3062))
anishagg17 marked this conversation as resolved.
Show resolved Hide resolved

**Bug Fixes**

- Fixed `EuiFieldNumber` so values of type `number` are now allowed ([#3020](https://github.com/elastic/eui/pull/3020))
Expand Down
10 changes: 5 additions & 5 deletions src-docs/src/views/key_pad_menu/key_pad_menu_example.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,11 @@ const keyPadMenuSnippet = `<EuiKeyPadMenu>
import KeyPadMenuItemButton from './key_pad_menu_item_button';
const keyPadMenuItemButtonSource = require('!!raw-loader!./key_pad_menu_item_button');
const keyPadMenuItemButtonHtml = renderToHtml(KeyPadMenuItemButton);
const keyPadMenuItemButtonSnippet = `<EuiKeyPadMenuItemButton
const keyPadMenuItemButtonSnippet = `<EuiKeyPadMenuItem
label={label}
onClick={this.handleClick}>
<EuiIcon type={icon} size="l" />
</EuiKeyPadMenuItemButton>
</EuiKeyPadMenuItem>
`;

import KeyPadBeta from './key_pad_beta';
Expand Down Expand Up @@ -85,9 +85,9 @@ export const KeyPadMenuExample = {
],
text: (
<p>
The KeyPadMenuItem component is a link by default, but you can swap it
out for a KeyPadMenuItemButton if you want <EuiCode>onClick</EuiCode>{' '}
behavior.
The KeyPadMenuItem component can act both as an anchor as well as a
button by specifying <EuiCode>href</EuiCode> or
<EuiCode>onClick</EuiCode> attribute respectively.
anishagg17 marked this conversation as resolved.
Show resolved Hide resolved
</p>
),
snippet: keyPadMenuItemButtonSnippet,
Expand Down
10 changes: 5 additions & 5 deletions src-docs/src/views/key_pad_menu/key_pad_menu_item_button.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,21 +3,21 @@ import React from 'react';
import {
EuiIcon,
EuiKeyPadMenu,
EuiKeyPadMenuItemButton,
EuiKeyPadMenuItem,
} from '../../../../src/components';

export default () => (
<EuiKeyPadMenu>
<EuiKeyPadMenuItemButton
<EuiKeyPadMenuItem
label="Dashboard"
onClick={() => window.alert('Clicked')}>
<EuiIcon type="dashboardApp" size="l" />
</EuiKeyPadMenuItemButton>
<EuiKeyPadMenuItemButton
</EuiKeyPadMenuItem>
<EuiKeyPadMenuItem
label="Dashboard"
isDisabled
onClick={() => window.alert('Clicked')}>
<EuiIcon type="dashboardApp" size="l" />
</EuiKeyPadMenuItemButton>
</EuiKeyPadMenuItem>
</EuiKeyPadMenu>
);
6 changes: 1 addition & 5 deletions src/components/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -176,11 +176,7 @@ export {
EuiLoadingSpinner,
} from './loading';

export {
EuiKeyPadMenu,
EuiKeyPadMenuItem,
EuiKeyPadMenuItemButton,
} from './key_pad_menu';
export { EuiKeyPadMenu, EuiKeyPadMenuItem } from './key_pad_menu';

export { EuiLink } from './link';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,9 @@ exports[`EuiKeyPadMenuItem is rendered 1`] = `
</a>
`;

exports[`EuiKeyPadMenuItem renders href 1`] = `
exports[`EuiKeyPadMenuItem renders button 1`] = `
<a
class="euiKeyPadMenuItem"
href="#"
role="menuitem"
>
<div
Expand All @@ -47,12 +46,11 @@ exports[`EuiKeyPadMenuItem renders href 1`] = `
</a>
`;

exports[`EuiKeyPadMenuItemButton is rendered 1`] = `
<button
aria-label="aria-label"
class="euiKeyPadMenuItem testClass1 testClass2"
data-test-subj="test subject string"
type="button"
exports[`EuiKeyPadMenuItem renders href 1`] = `
<a
class="euiKeyPadMenuItem"
href="#"
role="menuitem"
>
<div
class="euiKeyPadMenuItem__inner"
Expand All @@ -68,5 +66,5 @@ exports[`EuiKeyPadMenuItemButton is rendered 1`] = `
Label
</p>
</div>
</button>
</a>
`;
5 changes: 1 addition & 4 deletions src/components/key_pad_menu/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
export { EuiKeyPadMenu } from './key_pad_menu';

export {
EuiKeyPadMenuItem,
EuiKeyPadMenuItemButton,
} from './key_pad_menu_item';
export { EuiKeyPadMenuItem } from './key_pad_menu_item';
53 changes: 24 additions & 29 deletions src/components/key_pad_menu/key_pad_menu_item.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,7 @@ import React from 'react';
import { render, shallow } from 'enzyme';
import { requiredProps } from '../../test';

import {
EuiKeyPadMenuItem,
EuiKeyPadMenuItemButton,
} from './key_pad_menu_item';
import { EuiKeyPadMenuItem } from './key_pad_menu_item';

describe('EuiKeyPadMenuItem', () => {
test('is rendered', () => {
Expand All @@ -27,44 +24,42 @@ describe('EuiKeyPadMenuItem', () => {

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

describe('EuiKeyPadMenuItemButton', () => {
test('is rendered', () => {
test('renders button', () => {
const onClickHandler = jest.fn();

const component = render(
<EuiKeyPadMenuItemButton label="Label" {...requiredProps}>
<EuiKeyPadMenuItem label="Label" onClick={onClickHandler}>
Icon
</EuiKeyPadMenuItemButton>
</EuiKeyPadMenuItem>
);

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

describe('onClick', () => {
test("isn't called upon instantiation", () => {
const onClickHandler = jest.fn();
test("onClick isn't called upon instantiation", () => {
const onClickHandler = jest.fn();

shallow(
<EuiKeyPadMenuItemButton label="Label" onClick={onClickHandler}>
Icon
</EuiKeyPadMenuItemButton>
);
shallow(
<EuiKeyPadMenuItem label="Label" onClick={onClickHandler}>
Icon
</EuiKeyPadMenuItem>
);

expect(onClickHandler).not.toBeCalled();
});
expect(onClickHandler).not.toBeCalled();
});

test('is called when the button is clicked', () => {
const onClickHandler = jest.fn();
test('onClick is called when the button is clicked', () => {
const onClickHandler = jest.fn();

const $button = shallow(
<EuiKeyPadMenuItemButton label="Label" onClick={onClickHandler}>
Icon
</EuiKeyPadMenuItemButton>
);
const $button = shallow(
<EuiKeyPadMenuItem label="Label" onClick={onClickHandler}>
Icon
</EuiKeyPadMenuItem>
);

$button.simulate('click');
$button.simulate('click');

expect(onClickHandler).toBeCalledTimes(1);
});
expect(onClickHandler).toBeCalledTimes(1);
});
});
85 changes: 21 additions & 64 deletions src/components/key_pad_menu/key_pad_menu_item.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,11 @@ import React, {
ButtonHTMLAttributes,
FunctionComponent,
ReactNode,
HTMLAttributes,
} from 'react';
import classNames from 'classnames';

import { CommonProps } from '../common';
import { CommonProps, ExclusiveUnion } from '../common';

import { EuiBetaBadge } from '../badge/beta_badge';

Expand Down Expand Up @@ -59,11 +60,13 @@ interface EuiKeyPadMenuItemCommonProps {
}

export type EuiKeyPadMenuItemProps = CommonProps &
AnchorHTMLAttributes<HTMLAnchorElement> &
ExclusiveUnion<
AnchorHTMLAttributes<HTMLAnchorElement>,
ButtonHTMLAttributes<HTMLButtonElement>
> &
EuiKeyPadMenuItemCommonProps;

export const EuiKeyPadMenuItem: FunctionComponent<EuiKeyPadMenuItemProps> = ({
href,
isDisabled,
label,
children,
Expand All @@ -81,77 +84,31 @@ export const EuiKeyPadMenuItem: FunctionComponent<EuiKeyPadMenuItemProps> = ({
className
);

if (!isDisabled) {
return (
<a href={href} className={classes} role="menuitem" {...rest}>
{renderContent(
children,
label,
betaBadgeLabel,
betaBadgeTooltipContent,
betaBadgeIconType
)}
</a>
);
const Element = !isDisabled ? 'a' : 'button';
anishagg17 marked this conversation as resolved.
Show resolved Hide resolved
const relObj: {
role?: string;
disabled?: boolean;
type?: string;
} = {};

if (!isDisabled) relObj.role = 'menuitem';
else {
relObj.type = 'button';
relObj.disabled = isDisabled;
}

return (
<button
type="button"
disabled={isDisabled}
<Element
className={classes}
// Type case needed due to how the props are defined
{...rest as ButtonHTMLAttributes<HTMLButtonElement>}>
{...relObj as HTMLAttributes<HTMLElement>}
{...rest as HTMLAttributes<HTMLElement>}>
{renderContent(
children,
label,
betaBadgeLabel,
betaBadgeTooltipContent,
betaBadgeIconType
)}
</button>
);
};

export type EuiKeyPadMenuItemButtonProps = CommonProps &
ButtonHTMLAttributes<HTMLButtonElement> &
EuiKeyPadMenuItemCommonProps;

export const EuiKeyPadMenuItemButton: FunctionComponent<
EuiKeyPadMenuItemButtonProps
> = ({
onClick,
label,
children,
className,
betaBadgeLabel,
betaBadgeTooltipContent,
betaBadgeIconType,
isDisabled,
...rest
}) => {
const classes = classNames(
'euiKeyPadMenuItem',
{
'euiKeyPadMenuItem--hasBetaBadge': betaBadgeLabel,
},
className
);

return (
<button
type="button"
onClick={onClick}
disabled={isDisabled}
className={classes}
{...rest}>
{renderContent(
children,
label,
betaBadgeLabel,
betaBadgeTooltipContent,
betaBadgeIconType
)}
</button>
</Element>
);
};