From 210e1d6918cf61c9f4912a288c13f4d9f6be0754 Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Fri, 28 Oct 2022 11:29:32 -0700 Subject: [PATCH 1/9] Fix text wrapper not being used for EuiI18n 'text' that pass textProps --- .../_button_display_content.test.tsx | 46 ++++++++++++++++++- .../_button_display_content.tsx | 2 +- 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/src/components/button/button_display/_button_display_content.test.tsx b/src/components/button/button_display/_button_display_content.test.tsx index 33417c3c5e5..5c957d3cbda 100644 --- a/src/components/button/button_display/_button_display_content.test.tsx +++ b/src/components/button/button_display/_button_display_content.test.tsx @@ -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'; @@ -28,4 +28,48 @@ describe('EuiButtonDisplayContent', () => { expect(container.firstChild).toMatchSnapshot(); }); + + describe('props', () => { + test('textProps', () => { + const { getByTestSubject } = render( + + Text + + ); + + expect(getByTestSubject('testing')).toBeTruthy(); + }); + }); + + describe('text span wrapper', () => { + it('renders a text span wrapper if the content is a string', () => { + const { container } = render( + Text + ); + + expect(container.querySelector('.eui-textTruncate')).toBeTruthy(); + }); + + it('renders a text span wrapper if textProps is passed', () => { + const { getByTestSubject, container } = render( + + <>Text + + ); + + expect(getByTestSubject('testing')).toBeTruthy(); + expect(container.querySelector('.eui-textTruncate')).toBeTruthy(); + }); + + it('does not render a text span wrapper if custom child with no textProps are passed', () => { + const { getByTestSubject, container } = render( + + Text + + ); + + expect(getByTestSubject('custom')).toBeTruthy(); + expect(container.querySelector('.eui-textTruncate')).toBeFalsy(); + }); + }); }); diff --git a/src/components/button/button_display/_button_display_content.tsx b/src/components/button/button_display/_button_display_content.tsx index 6b153740932..51a920a9379 100644 --- a/src/components/button/button_display/_button_display_content.tsx +++ b/src/components/button/button_display/_button_display_content.tsx @@ -111,7 +111,7 @@ export const EuiButtonDisplayContent: FunctionComponent< return ( {icon} - {isText ? ( + {isText || textProps ? ( Date: Fri, 28 Oct 2022 12:16:03 -0700 Subject: [PATCH 2/9] Fix unnecessary textProps spread causing false positive for undefined/empty objs --- src/components/button/button_display/_button_display.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/button/button_display/_button_display.tsx b/src/components/button/button_display/_button_display.tsx index 163afaeab6f..1d21e272a59 100644 --- a/src/components/button/button_display/_button_display.tsx +++ b/src/components/button/button_display/_button_display.tsx @@ -156,7 +156,7 @@ export const EuiButtonDisplay = forwardRef( iconType={iconType} iconSide={iconSide} iconSize={iconSize} - textProps={{ ...textProps }} + textProps={textProps} {...contentProps} > {children} From 8252261b479c2f06d69d0c7a409b2d482ba48c23 Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Fri, 28 Oct 2022 12:38:06 -0700 Subject: [PATCH 3/9] Fix minWidth inline style not applying for 0px + use logical property --- .../button/button_display/_button_display.test.tsx | 10 ++++++++++ .../button/button_display/_button_display.tsx | 2 +- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/components/button/button_display/_button_display.test.tsx b/src/components/button/button_display/_button_display.test.tsx index eb144ab028a..532b8a43a1b 100644 --- a/src/components/button/button_display/_button_display.test.tsx +++ b/src/components/button/button_display/_button_display.test.tsx @@ -25,4 +25,14 @@ describe('EuiButtonDisplay', () => { expect(container.firstChild).toMatchSnapshot(); }); + + describe('props', () => { + test('minWidth', () => { + const { container } = render(); + + expect(container.innerHTML).toEqual( + expect.stringContaining('style="min-inline-size: 0;"') + ); + }); + }); }); diff --git a/src/components/button/button_display/_button_display.tsx b/src/components/button/button_display/_button_display.tsx index 1d21e272a59..40257939fa6 100644 --- a/src/components/button/button_display/_button_display.tsx +++ b/src/components/button/button_display/_button_display.tsx @@ -193,7 +193,7 @@ export const EuiButtonDisplay = forwardRef( element, { css: cssStyles, - style: minWidth ? { ...style, minWidth } : style, + style: minWidth != null ? { ...style, minInlineSize: minWidth } : style, ref, ...elementProps, ...relObj, From a0c2e472d008ca5a8a6996f1183d94e68acbf8dd Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Fri, 28 Oct 2022 12:44:07 -0700 Subject: [PATCH 4/9] [opinionated] iconSide change: prefer DOM order over CSS order - more accessible to rtl --- .../button/__snapshots__/button.test.tsx.snap | 15 ++++++++------- .../button/button_display/_button_display.tsx | 2 +- .../_button_display_content.styles.ts | 5 ----- .../button_display/_button_display_content.tsx | 8 +++----- 4 files changed, 12 insertions(+), 18 deletions(-) diff --git a/src/components/button/__snapshots__/button.test.tsx.snap b/src/components/button/__snapshots__/button.test.tsx.snap index c44f284ef09..6057601aee7 100644 --- a/src/components/button/__snapshots__/button.test.tsx.snap +++ b/src/components/button/__snapshots__/button.test.tsx.snap @@ -156,7 +156,7 @@ exports[`EuiButton props iconSide left is rendered 1`] = ` type="button" > - Content + `; @@ -336,6 +336,7 @@ exports[`EuiButton props isSelected is rendered as true 1`] = ` exports[`EuiButton props minWidth is rendered 1`] = ` diff --git a/src/components/button/button_display/__snapshots__/_button_display_content.test.tsx.snap b/src/components/button/button_display/__snapshots__/_button_display_content.test.tsx.snap index 083dd8cbe7c..8a4fc9a9a02 100644 --- a/src/components/button/button_display/__snapshots__/_button_display_content.test.tsx.snap +++ b/src/components/button/button_display/__snapshots__/_button_display_content.test.tsx.snap @@ -1,5 +1,23 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`EuiButtonDisplayContent button icon loading icon renders disabled & loading spinners with custom border color 1`] = ` + + + + Loading + + +`; + exports[`EuiButtonDisplayContent button icon loading icon replaces existing icons 1`] = ` { expect(container.firstChild).toMatchSnapshot(); expect(container.querySelector('.euiLoadingSpinner')).toBeTruthy(); }); + + it('renders disabled & loading spinners with custom border color', () => { + const { container } = render( + + Loading + + ); + + expect(container.firstChild).toMatchSnapshot(); + }); }); }); diff --git a/src/components/button/button_display/_button_display_content.tsx b/src/components/button/button_display/_button_display_content.tsx index 5d3fac790ca..77963013aa6 100644 --- a/src/components/button/button_display/_button_display_content.tsx +++ b/src/components/button/button_display/_button_display_content.tsx @@ -9,7 +9,7 @@ import React, { HTMLAttributes, FunctionComponent, Ref } from 'react'; import { useEuiTheme } from '../../../services'; import { CommonProps } from '../../common'; -import { EuiLoadingSpinner, EuiLoadingSpinnerProps } from '../../loading'; +import { EuiLoadingSpinner } from '../../loading'; import { EuiIcon, IconType } from '../../icon'; import { euiButtonDisplayContentStyles } from './_button_display_content.styles'; import classNames from 'classnames'; @@ -74,9 +74,7 @@ export const EuiButtonDisplayContent: FunctionComponent< // to have the same color of the text. This way we ensure the borders // are always visible. The default spinner color could be very light. const loadingSpinnerColor = isDisabled - ? ({ - border: 'currentColor', - } as EuiLoadingSpinnerProps['color']) + ? { border: 'currentcolor' } : undefined; if (isLoading) { diff --git a/src/components/loading/__snapshots__/loading_spinner.test.tsx.snap b/src/components/loading/__snapshots__/loading_spinner.test.tsx.snap index 243c8c0c03e..707a5be5e88 100644 --- a/src/components/loading/__snapshots__/loading_spinner.test.tsx.snap +++ b/src/components/loading/__snapshots__/loading_spinner.test.tsx.snap @@ -1,5 +1,28 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`EuiLoadingSpinner custom colors 1`] = ` +Array [ + , + , + , +] +`; + exports[`EuiLoadingSpinner is rendered 1`] = ` { - return ` - border-color: ${highlight} ${border} ${border} ${border}; - `; +export const euiSpinnerBorderColorsCSS = ( + { euiTheme }: UseEuiTheme, + colors: EuiLoadingSpinnerColor = {} +): string => { + const { + border = euiTheme.colors.lightShade, + highlight = euiTheme.colors.primary, + } = colors; + return `${highlight} ${border} ${border} ${border}`; }; -export const euiLoadingSpinnerStyles = ( - { euiTheme }: UseEuiTheme, - color?: EuiLoadingSpinnerProps['color'] -) => { +export const euiLoadingSpinnerStyles = (euiThemeContext: UseEuiTheme) => { + const { euiTheme } = euiThemeContext; return { euiLoadingSpinner: css` flex-shrink: 0; // Ensures it never scales down below its intended size display: inline-block; border-radius: 50%; border: ${euiTheme.border.thick}; - ${spinnerColorsCSS( - color?.border || euiTheme.colors.lightShade, - color?.highlight || euiTheme.colors.primary - )}; + border-color: ${euiSpinnerBorderColorsCSS(euiThemeContext)}; ${euiCanAnimate} { animation: ${_loadingSpinner} 0.6s infinite linear; diff --git a/src/components/loading/loading_spinner.test.tsx b/src/components/loading/loading_spinner.test.tsx index f03ef48bb98..633cd4d23e3 100644 --- a/src/components/loading/loading_spinner.test.tsx +++ b/src/components/loading/loading_spinner.test.tsx @@ -28,4 +28,19 @@ describe('EuiLoadingSpinner', () => { }); }); }); + + test('custom colors', () => { + const component = render( + <> + + + + + ); + + expect(component).toMatchSnapshot(); + }); }); diff --git a/src/components/loading/loading_spinner.tsx b/src/components/loading/loading_spinner.tsx index 44445370c47..968305b9a14 100644 --- a/src/components/loading/loading_spinner.tsx +++ b/src/components/loading/loading_spinner.tsx @@ -11,7 +11,10 @@ import { CommonProps } from '../common'; import classNames from 'classnames'; import { useEuiTheme } from '../..//services'; import { useLoadingAriaLabel } from './_loading_strings'; -import { euiLoadingSpinnerStyles } from './loading_spinner.styles'; +import { + euiLoadingSpinnerStyles, + euiSpinnerBorderColorsCSS, +} from './loading_spinner.styles'; export const SIZES = ['s', 'm', 'l', 'xl', 'xxl'] as const; export type EuiLoadingSpinnerSize = typeof SIZES[number]; @@ -37,18 +40,23 @@ export const EuiLoadingSpinner: FunctionComponent = ({ className, 'aria-label': ariaLabel, color, + style, ...rest }) => { const euiTheme = useEuiTheme(); - const styles = euiLoadingSpinnerStyles(euiTheme, color); + const styles = euiLoadingSpinnerStyles(euiTheme); const cssStyles = [styles.euiLoadingSpinner, styles[size]]; const classes = classNames('euiLoadingSpinner', className); const defaultLabel = useLoadingAriaLabel(); + const customColorStyle = color + ? { ...style, borderColor: euiSpinnerBorderColorsCSS(euiTheme, color) } + : style; return ( Date: Fri, 28 Oct 2022 15:22:58 -0700 Subject: [PATCH 8/9] write more/missing unit tests for logic within internal EuiButtonDisplay --- .../button_display/_button_display.test.tsx | 127 +++++++++++++++++- 1 file changed, 126 insertions(+), 1 deletion(-) diff --git a/src/components/button/button_display/_button_display.test.tsx b/src/components/button/button_display/_button_display.test.tsx index 532b8a43a1b..d590c9d0018 100644 --- a/src/components/button/button_display/_button_display.test.tsx +++ b/src/components/button/button_display/_button_display.test.tsx @@ -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'; @@ -35,4 +35,129 @@ describe('EuiButtonDisplay', () => { ); }); }); + + 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 + + ); + expect(container.querySelector('button[disabled]')).toBeTruthy(); + }); + + it('disables buttons that are isLoading', () => { + const { container } = render(); + expect(container.querySelector('button[disabled]')).toBeTruthy(); + }); + + it('disables buttons that are isDisabled', () => { + const { container } = render(); + expect(container.querySelector('button[disabled]')).toBeTruthy(); + }); + + it('disables buttons that pass the native disabled attr', () => { + const { container } = render(); + expect(container.querySelector('button[disabled]')).toBeTruthy(); + }); + }); + + describe('link vs button behavior', () => { + describe('links', () => { + it('renders valid links as tags', () => { + const { container } = render(); + expect(container.querySelector('a')).toBeTruthy(); + expect(container.querySelector('button')).toBeFalsy(); + }); + + it('removes the `type` prop from links', () => { + const { container } = render( + + ); + expect(container.querySelector('a')?.getAttribute('type')).toBeNull(); + }); + + it('inserts `rel=noreferrer` for non-Elastic urls ', () => { + const { queryByTestSubject } = render( + <> + + + + + ); + + 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( + <> + + + + ); + + 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( + + ); + 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( + <> + + + + ); + expect(container.querySelectorAll('[disabled]')).toHaveLength(1); + expect(container.querySelectorAll('[aria-pressed]')).toHaveLength(1); + }); + }); + }); }); From 0fdfb69760575cba1aa4ab46d674158f53a64f65 Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Fri, 28 Oct 2022 15:34:09 -0700 Subject: [PATCH 9/9] changelog --- upcoming_changelogs/6332.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 upcoming_changelogs/6332.md diff --git a/upcoming_changelogs/6332.md b/upcoming_changelogs/6332.md new file mode 100644 index 00000000000..7cc1cbf7f0f --- /dev/null +++ b/upcoming_changelogs/6332.md @@ -0,0 +1,4 @@ +**Bug fixes** + +- Fixed `EuiButton` not correctly passing `textProps` for children inside fragments or i18n components +- Fixed `EuiButton` not correctly respecting `minWidth={0}`