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

[EuiButtonIcon] change iconType prop to required #4106

Merged
merged 12 commits into from
Oct 14, 2020
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

**Bug fixes**

- Changed `iconType` prop to be `required` in `EuiButtonIcon` ([#4106](https://github.com/elastic/eui/pull/4106))
- Fixed `EuiFieldSearch` padding when `isClearable` but has no `value` ([#4089](https://github.com/elastic/eui/pull/4089))


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,79 +6,139 @@ exports[`EuiButtonIcon is rendered 1`] = `
class="euiButtonIcon euiButtonIcon--primary testClass1 testClass2"
data-test-subj="test subject string"
type="button"
/>
>
<span
aria-hidden="true"
class="euiButtonIcon__icon"
data-euiicon-type="user"
/>
</button>
`;

exports[`EuiButtonIcon props color accent is rendered 1`] = `
<button
aria-label="button"
class="euiButtonIcon euiButtonIcon--accent"
type="button"
/>
>
<span
aria-hidden="true"
class="euiButtonIcon__icon"
data-euiicon-type="user"
/>
</button>
`;

exports[`EuiButtonIcon props color danger is rendered 1`] = `
<button
aria-label="button"
class="euiButtonIcon euiButtonIcon--danger"
type="button"
/>
>
<span
aria-hidden="true"
class="euiButtonIcon__icon"
data-euiicon-type="user"
/>
</button>
`;

exports[`EuiButtonIcon props color disabled is rendered 1`] = `
<button
aria-label="button"
class="euiButtonIcon euiButtonIcon--disabled"
type="button"
/>
>
<span
aria-hidden="true"
class="euiButtonIcon__icon"
data-euiicon-type="user"
/>
</button>
`;

exports[`EuiButtonIcon props color ghost is rendered 1`] = `
<button
aria-label="button"
class="euiButtonIcon euiButtonIcon--ghost"
type="button"
/>
>
<span
aria-hidden="true"
class="euiButtonIcon__icon"
data-euiicon-type="user"
/>
</button>
`;

exports[`EuiButtonIcon props color primary is rendered 1`] = `
<button
aria-label="button"
class="euiButtonIcon euiButtonIcon--primary"
type="button"
/>
>
<span
aria-hidden="true"
class="euiButtonIcon__icon"
data-euiicon-type="user"
/>
</button>
`;

exports[`EuiButtonIcon props color subdued is rendered 1`] = `
<button
aria-label="button"
class="euiButtonIcon euiButtonIcon--subdued"
type="button"
/>
>
<span
aria-hidden="true"
class="euiButtonIcon__icon"
data-euiicon-type="user"
/>
</button>
`;

exports[`EuiButtonIcon props color success is rendered 1`] = `
<button
aria-label="button"
class="euiButtonIcon euiButtonIcon--success"
type="button"
/>
>
<span
aria-hidden="true"
class="euiButtonIcon__icon"
data-euiicon-type="user"
/>
</button>
`;

exports[`EuiButtonIcon props color text is rendered 1`] = `
<button
aria-label="button"
class="euiButtonIcon euiButtonIcon--text"
type="button"
/>
>
<span
aria-hidden="true"
class="euiButtonIcon__icon"
data-euiicon-type="user"
/>
</button>
`;

exports[`EuiButtonIcon props color warning is rendered 1`] = `
<button
aria-label="button"
class="euiButtonIcon euiButtonIcon--warning"
type="button"
/>
>
<span
aria-hidden="true"
class="euiButtonIcon__icon"
data-euiicon-type="user"
/>
</button>
`;

exports[`EuiButtonIcon props href secures the rel attribute when the target is _blank 1`] = `
Expand All @@ -88,7 +148,13 @@ exports[`EuiButtonIcon props href secures the rel attribute when the target is _
href="#"
rel="noopener noreferrer"
target="_blank"
/>
>
<span
aria-hidden="true"
class="euiButtonIcon__icon"
data-euiicon-type="user"
/>
</a>
`;

exports[`EuiButtonIcon props iconType is rendered 1`] = `
Expand All @@ -111,7 +177,13 @@ exports[`EuiButtonIcon props isDisabled is rendered 1`] = `
class="euiButtonIcon euiButtonIcon--primary"
disabled=""
type="button"
/>
>
<span
aria-hidden="true"
class="euiButtonIcon__icon"
data-euiicon-type="user"
/>
</button>
`;

exports[`EuiButtonIcon props isDisabled renders a button even when href is defined 1`] = `
Expand All @@ -120,5 +192,11 @@ exports[`EuiButtonIcon props isDisabled renders a button even when href is defin
class="euiButtonIcon euiButtonIcon--primary"
disabled=""
type="button"
/>
>
<span
aria-hidden="true"
class="euiButtonIcon__icon"
data-euiicon-type="user"
/>
</button>
`;
31 changes: 24 additions & 7 deletions src/components/button/button_icon/button_icon.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@ import { EuiButtonIcon, COLORS } from './button_icon';

describe('EuiButtonIcon', () => {
test('is rendered', () => {
const component = render(<EuiButtonIcon {...requiredProps} />);
const component = render(
<EuiButtonIcon iconType="user" {...requiredProps} />
);

expect(component).toMatchSnapshot();
});
Expand All @@ -34,15 +36,20 @@ describe('EuiButtonIcon', () => {
describe('isDisabled', () => {
it('is rendered', () => {
const component = render(
<EuiButtonIcon aria-label="button" isDisabled />
<EuiButtonIcon iconType="user" aria-label="button" isDisabled />
);

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

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

expect(component).toMatchSnapshot();
Expand All @@ -63,7 +70,7 @@ describe('EuiButtonIcon', () => {
COLORS.forEach(color => {
test(`${color} is rendered`, () => {
const component = render(
<EuiButtonIcon aria-label="button" color={color} />
<EuiButtonIcon iconType="user" aria-label="button" color={color} />
);

expect(component).toMatchSnapshot();
Expand All @@ -74,7 +81,12 @@ describe('EuiButtonIcon', () => {
describe('href', () => {
it('secures the rel attribute when the target is _blank', () => {
const component = render(
<EuiButtonIcon aria-label="button" href="#" target="_blank" />
<EuiButtonIcon
iconType="user"
aria-label="button"
href="#"
target="_blank"
/>
);

expect(component).toMatchSnapshot();
Expand All @@ -85,7 +97,12 @@ describe('EuiButtonIcon', () => {
it('supports onClick and href', () => {
const handler = jest.fn();
const component = mount(
<EuiButtonIcon aria-label="hoi" href="#" onClick={handler} />
<EuiButtonIcon
iconType="user"
aria-label="hoi"
href="#"
onClick={handler}
/>
);
component.find('a').simulate('click');
expect(handler.mock.calls.length).toEqual(1);
Expand All @@ -94,7 +111,7 @@ describe('EuiButtonIcon', () => {
it('supports onClick as a button', () => {
const handler = jest.fn();
const component = mount(
<EuiButtonIcon aria-label="hoi" onClick={handler} />
<EuiButtonIcon iconType="user" aria-label="hoi" onClick={handler} />
);
component.find('button').simulate('click');
expect(handler.mock.calls.length).toEqual(1);
Expand Down
2 changes: 1 addition & 1 deletion src/components/button/button_icon/button_icon.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export type EuiButtonIconColor =
| 'warning';

export interface EuiButtonIconProps extends CommonProps {
iconType?: IconType;
iconType: IconType;
color?: EuiButtonIconColor;
'aria-label'?: string;
'aria-labelledby'?: string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ exports[`EuiFieldPassword props dual dualToggleProps is rendered 1`] = `
</div>
</div>
<button
aria-label="Show password as plain text. Note: this will visually expose your password on the screen."
aria-label="aria-label"
class="euiButtonIcon euiButtonIcon--primary euiFormControlLayout__append testClass1 testClass2"
data-test-subj="test subject string"
title="Show password as plain text. Note: this will visually expose your password on the screen."
Expand Down
6 changes: 3 additions & 3 deletions src/components/form/field_password/field_password.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ export type EuiFieldPasswordProps = Omit<
/**
* Additional props to apply to the dual toggle. Extends EuiButtonIcon
*/
dualToggleProps?: EuiButtonIconProps;
dualToggleProps?: Omit<EuiButtonIconProps, 'iconType'>;
ashikmeerankutty marked this conversation as resolved.
Show resolved Hide resolved
};

export const EuiFieldPassword: FunctionComponent<EuiFieldPasswordProps> = ({
Expand Down Expand Up @@ -126,12 +126,12 @@ export const EuiFieldPassword: FunctionComponent<EuiFieldPasswordProps> = ({

const visibilityToggle = (
<EuiButtonIcon
{...dualToggleProps}
iconType={isVisible ? 'eyeClosed' : 'eye'}
onClick={() => handleToggle(isVisible)}
aria-label={isVisible ? maskPasswordLabel : showPasswordLabel}
title={isVisible ? maskPasswordLabel : showPasswordLabel}
disabled={rest.disabled}
{...dualToggleProps}
onClick={() => handleToggle(isVisible)}
ashikmeerankutty marked this conversation as resolved.
Show resolved Hide resolved
/>
);
appends = [...appends, visibilityToggle];
Expand Down