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 2 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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
## [`master`](https://github.com/elastic/eui/tree/master)

- 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
- Added props descriptions for `EuiComboBox` ([#3007](https://github.com/elastic/eui/pull/3007))
- Exported `dateFormatAliases` as a part of the public API ([#3043](https://github.com/elastic/eui/pull/3043))
- Exported `EuiTextProps` type definition ([#3039](https://github.com/elastic/eui/pull/3039))
Expand Down
4 changes: 2 additions & 2 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
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 @@ -48,11 +48,11 @@ exports[`EuiKeyPadMenuItem renders href 1`] = `
`;

exports[`EuiKeyPadMenuItemButton is rendered 1`] = `
<button
<a
aria-label="aria-label"
class="euiKeyPadMenuItem testClass1 testClass2"
data-test-subj="test subject string"
type="button"
role="menuitem"
>
<div
class="euiKeyPadMenuItem__inner"
Expand All @@ -68,5 +68,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';
17 changes: 7 additions & 10 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 @@ -32,9 +29,9 @@ describe('EuiKeyPadMenuItem', () => {
describe('EuiKeyPadMenuItemButton', () => {
anishagg17 marked this conversation as resolved.
Show resolved Hide resolved
test('is rendered', () => {
const component = render(
<EuiKeyPadMenuItemButton label="Label" {...requiredProps}>
<EuiKeyPadMenuItem label="Label" {...requiredProps}>
Icon
</EuiKeyPadMenuItemButton>
</EuiKeyPadMenuItem>
);

expect(component).toMatchSnapshot();
Expand All @@ -45,9 +42,9 @@ describe('EuiKeyPadMenuItemButton', () => {
const onClickHandler = jest.fn();

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

expect(onClickHandler).not.toBeCalled();
Expand All @@ -57,9 +54,9 @@ describe('EuiKeyPadMenuItemButton', () => {
const onClickHandler = jest.fn();

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

$button.simulate('click');
Expand Down
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>
);
};