Skip to content

Commit

Permalink
[EuiButton] Fix multiple issues around internal EuiButtonDisplay comp…
Browse files Browse the repository at this point in the history
…onents (elastic#6332)

* Fix text wrapper not being used for EuiI18n 'text' that pass textProps

* Fix unnecessary textProps spread causing false positive for undefined/empty objs

* Fix minWidth inline style not applying for 0px + use logical property

* [opinionated] iconSide change: prefer DOM order over CSS order

- more accessible to rtl

* [cleanup] remove unnecessary/unused CSS

- flex shrink is already set by EuiLoadingSpinner and EuiIcon respectively and does not need to be repeated

- size CSS isn't doing anything and isn't worth keeping

* Write more unit tests for EuiButtonDisplayContent

* Fix EuiLoadingSpinner to not pass wildcard color values into Emotion

- we should be using inline styles for non-enum props to avoid generating n/infinite Emotion classNames

+ add unit test for EuiButtonDisplayContent & remove unnecessary typing

* write more/missing unit tests for logic within internal EuiButtonDisplay

* changelog
  • Loading branch information
Constance authored and cee-chen committed Oct 31, 2022
1 parent b1c9d40 commit 2cfd088
Show file tree
Hide file tree
Showing 13 changed files with 366 additions and 71 deletions.
21 changes: 9 additions & 12 deletions src/components/button/__snapshots__/button.test.tsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -156,10 +156,9 @@ exports[`EuiButton props iconSide left is rendered 1`] = `
type="button"
>
<span
class="emotion-euiButtonDisplayContent-left"
class="emotion-euiButtonDisplayContent"
>
<span
class="emotion-euiButtonDisplayContent__icon-m"
color="inherit"
data-euiicon-type="user"
/>
Expand All @@ -178,18 +177,17 @@ exports[`EuiButton props iconSide right is rendered 1`] = `
type="button"
>
<span
class="emotion-euiButtonDisplayContent-right"
class="emotion-euiButtonDisplayContent"
>
<span
class="emotion-euiButtonDisplayContent__icon-m"
color="inherit"
data-euiicon-type="user"
/>
<span
class="eui-textTruncate"
>
Content
</span>
<span
color="inherit"
data-euiicon-type="user"
/>
</span>
</button>
`;
Expand All @@ -203,7 +201,6 @@ exports[`EuiButton props iconSize m is rendered 1`] = `
class="emotion-euiButtonDisplayContent"
>
<span
class="emotion-euiButtonDisplayContent__icon-m"
color="inherit"
data-euiicon-type="user"
/>
Expand All @@ -225,7 +222,6 @@ exports[`EuiButton props iconSize s is rendered 1`] = `
class="emotion-euiButtonDisplayContent"
>
<span
class="emotion-euiButtonDisplayContent__icon-s"
color="inherit"
data-euiicon-type="user"
/>
Expand All @@ -247,7 +243,6 @@ exports[`EuiButton props iconType is rendered 1`] = `
class="emotion-euiButtonDisplayContent"
>
<span
class="emotion-euiButtonDisplayContent__icon-m"
color="inherit"
data-euiicon-type="user"
/>
Expand Down Expand Up @@ -302,8 +297,9 @@ exports[`EuiButton props isLoading is rendered 1`] = `
>
<span
aria-label="Loading"
class="euiLoadingSpinner emotion-euiLoadingSpinner-m-euiButtonDisplayContent__spinner"
class="euiLoadingSpinner emotion-euiLoadingSpinner-m"
role="progressbar"
style="border-color:#07C currentcolor currentcolor currentcolor"
/>
</span>
</button>
Expand Down Expand Up @@ -336,6 +332,7 @@ exports[`EuiButton props isSelected is rendered as true 1`] = `
exports[`EuiButton props minWidth is rendered 1`] = `
<button
class="euiButton emotion-euiButtonDisplay-m-m-base-primary"
style="min-inline-size:0"
type="button"
>
<span
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,68 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`EuiButtonDisplayContent button icon loading icon renders disabled & loading spinners with custom border color 1`] = `
<span
class="emotion-euiButtonDisplayContent"
>
<span
aria-label="Loading"
class="euiLoadingSpinner emotion-euiLoadingSpinner-m"
role="progressbar"
style="border-color: #07c currentcolor currentcolor currentcolor;"
/>
<span
class="eui-textTruncate"
>
Loading
</span>
</span>
`;

exports[`EuiButtonDisplayContent button icon loading icon replaces existing icons 1`] = `
<span
class="emotion-euiButtonDisplayContent"
>
<span
aria-label="Loading"
class="euiLoadingSpinner emotion-euiLoadingSpinner-m"
role="progressbar"
/>
<span
class="eui-textTruncate"
>
Loading
</span>
</span>
`;

exports[`EuiButtonDisplayContent button icon renders icons on the left side by default 1`] = `
<span
class="emotion-euiButtonDisplayContent"
>
<span
color="inherit"
data-euiicon-type="user"
/>
<span
data-test-subj="content"
/>
</span>
`;

exports[`EuiButtonDisplayContent button icon renders icons on the right side 1`] = `
<span
class="emotion-euiButtonDisplayContent"
>
<span
data-test-subj="content"
/>
<span
color="inherit"
data-euiicon-type="user"
/>
</span>
`;

exports[`EuiButtonDisplayContent renders 1`] = `
<span
aria-label="aria-label"
Expand Down
137 changes: 136 additions & 1 deletion src/components/button/button_display/_button_display.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
*/

import React from 'react';
import { render } from '@testing-library/react';
import { render } from '../../../test/rtl';
import { requiredProps } from '../../../test/required_props';
import { shouldRenderCustomStyles } from '../../../test/internal';

Expand All @@ -25,4 +25,139 @@ describe('EuiButtonDisplay', () => {

expect(container.firstChild).toMatchSnapshot();
});

describe('props', () => {
test('minWidth', () => {
const { container } = render(<EuiButtonDisplay minWidth={0} />);

expect(container.innerHTML).toEqual(
expect.stringContaining('style="min-inline-size: 0;"')
);
});
});

describe('disabled behavior', () => {
it('disables hrefs with javascript in them and renders a button instead of a link', () => {
const { container } = render(
// eslint-disable-next-line no-script-url
<EuiButtonDisplay href="javascript:alert(0)" />
);
expect(container.querySelector('button[disabled]')).toBeTruthy();
});

it('disables buttons that are isLoading', () => {
const { container } = render(<EuiButtonDisplay isLoading />);
expect(container.querySelector('button[disabled]')).toBeTruthy();
});

it('disables buttons that are isDisabled', () => {
const { container } = render(<EuiButtonDisplay isDisabled />);
expect(container.querySelector('button[disabled]')).toBeTruthy();
});

it('disables buttons that pass the native disabled attr', () => {
const { container } = render(<EuiButtonDisplay disabled />);
expect(container.querySelector('button[disabled]')).toBeTruthy();
});
});

describe('link vs button behavior', () => {
describe('links', () => {
it('renders valid links as <a> tags', () => {
const { container } = render(<EuiButtonDisplay href="#" />);
expect(container.querySelector('a')).toBeTruthy();
expect(container.querySelector('button')).toBeFalsy();
});

it('removes the `type` prop from links', () => {
const { container } = render(
<EuiButtonDisplay href="#" type="button" />
);
expect(container.querySelector('a')?.getAttribute('type')).toBeNull();
});

it('inserts `rel=noreferrer` for non-Elastic urls ', () => {
const { queryByTestSubject } = render(
<>
<EuiButtonDisplay
href="https://elastic.co"
data-test-subj="norel"
rel="noreferrer" // Removes this
/>
<EuiButtonDisplay
href="ftp://elastic.co"
data-test-subj="badprotocol"
/>
<EuiButtonDisplay
href="https://hello.world"
data-test-subj="notelastic"
/>
</>
);

expect(queryByTestSubject('norel')?.getAttribute('rel')).toEqual('');
expect(queryByTestSubject('badprotocol')?.getAttribute('rel')).toEqual(
'noreferrer'
);
expect(queryByTestSubject('notelastic')?.getAttribute('rel')).toEqual(
'noreferrer'
);
});

it('inserts `rel=noopener` for all target=_blank links', () => {
const { queryByTestSubject } = render(
<>
<EuiButtonDisplay
href="https://elastic.co"
target="_blank"
data-test-subj="elastic"
/>
<EuiButtonDisplay
href="https://hello.world"
target="_blank"
data-test-subj="notelastic"
/>
</>
);

expect(queryByTestSubject('elastic')?.getAttribute('rel')).toEqual(
'noopener'
);
expect(queryByTestSubject('notelastic')?.getAttribute('rel')).toEqual(
'noopener noreferrer'
);
});
});

describe('buttons', () => {
it('removes the `href`, `rel` and `target` props from buttons', () => {
const { container } = render(
<EuiButtonDisplay
isDisabled
href="#"
rel="noopener"
target="_blank"
/>
);
expect(container.querySelector('a')).toBeFalsy();
const button = container.querySelector('button')!;

expect(button).toBeTruthy();
expect(button.getAttribute('href')).toBeNull();
expect(button.getAttribute('rel')).toBeNull();
expect(button.getAttribute('target')).toBeNull();
});

it('only sets [disabled] and [aria-pressed] on buttons', () => {
const { container } = render(
<>
<EuiButtonDisplay isDisabled isSelected />
<EuiButtonDisplay href="#" isSelected />
</>
);
expect(container.querySelectorAll('[disabled]')).toHaveLength(1);
expect(container.querySelectorAll('[aria-pressed]')).toHaveLength(1);
});
});
});
});
6 changes: 3 additions & 3 deletions src/components/button/button_display/_button_display.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ export const EuiButtonDisplay = forwardRef<HTMLElement, EuiButtonDisplayProps>(
{
children,
iconType,
iconSide,
iconSide = 'left',
iconSize,
size = 'm',
isDisabled,
Expand Down Expand Up @@ -156,7 +156,7 @@ export const EuiButtonDisplay = forwardRef<HTMLElement, EuiButtonDisplayProps>(
iconType={iconType}
iconSide={iconSide}
iconSize={iconSize}
textProps={{ ...textProps }}
textProps={textProps}
{...contentProps}
>
{children}
Expand Down Expand Up @@ -193,7 +193,7 @@ export const EuiButtonDisplay = forwardRef<HTMLElement, EuiButtonDisplayProps>(
element,
{
css: cssStyles,
style: minWidth ? { ...style, minWidth } : style,
style: minWidth != null ? { ...style, minInlineSize: minWidth } : style,
ref,
...elementProps,
...relObj,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,18 +20,4 @@ export const euiButtonDisplayContentStyles = ({ euiTheme }: UseEuiTheme) => ({
vertical-align: middle;
gap: ${euiTheme.size.s};
`,
// Icon side
left: css``,
right: css`
flex-direction: row-reverse;
`,
euiButtonDisplayContent__spinner: css`
flex-shrink: 0;
`,
euiButtonDisplayContent__icon: css`
flex-shrink: 0;
`,
// Icon size
s: css``,
m: css``,
});
Loading

0 comments on commit 2cfd088

Please sign in to comment.