From 401eac875f3ac04488db0b072711fc100b9204b6 Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Wed, 14 Jun 2023 14:17:03 -0700 Subject: [PATCH 01/11] [Setup] Clean up and DRY out shared button sizes - Remove `useEuiButtonRadiusCSS` in favor of a more agnostic `euiButtonSizeMap` - using the raw data is less strict and generates fewer classNames - `EuiButtonIcon` will shortly need the height/radius map and does not need to use all of `EuiButtonDisplay`'s logic (e.g. doesn't need fullWidth, or a content/text wrapper) - Move `_buttonSize` to internal fn to make it unexportable / clearer that its CSS is specific to `EuiButtonDisplay` --- .../button/__snapshots__/button.test.tsx.snap | 54 +++--- .../_button_display.test.tsx.snap | 2 +- .../button_display/_button_display.styles.ts | 27 +-- .../button/button_display/_button_display.tsx | 3 - .../__snapshots__/button_group.test.tsx.snap | 168 +++++++++--------- .../amsterdam/global_styling/mixins/button.ts | 42 ++--- 6 files changed, 150 insertions(+), 146 deletions(-) diff --git a/src/components/button/__snapshots__/button.test.tsx.snap b/src/components/button/__snapshots__/button.test.tsx.snap index 89fa202379a..a843fdb478e 100644 --- a/src/components/button/__snapshots__/button.test.tsx.snap +++ b/src/components/button/__snapshots__/button.test.tsx.snap @@ -3,7 +3,7 @@ exports[`EuiButton is rendered 1`] = ` `; diff --git a/src/components/button/button_icon/_button_icon.scss b/src/components/button/button_icon/_button_icon.scss index f6fffa90742..90111021526 100644 --- a/src/components/button/button_icon/_button_icon.scss +++ b/src/components/button/button_icon/_button_icon.scss @@ -1,5 +1,2 @@ .euiButtonIcon { - &:disabled { - @include euiButtonContentDisabled; - } } diff --git a/src/components/button/button_icon/button_icon.styles.ts b/src/components/button/button_icon/button_icon.styles.ts index e054eff68df..38ea22118da 100644 --- a/src/components/button/button_icon/button_icon.styles.ts +++ b/src/components/button/button_icon/button_icon.styles.ts @@ -36,6 +36,9 @@ export const euiButtonIconStyles = (euiThemeContext: UseEuiTheme) => { pointer-events: none; } `, + isDisabled: css` + cursor: not-allowed; + `, // Sizes xs: css` ${logicalSizeCSS(sizes.xs.height)} diff --git a/src/components/button/button_icon/button_icon.tsx b/src/components/button/button_icon/button_icon.tsx index d6a95a1be18..dc24dc7a2a7 100644 --- a/src/components/button/button_icon/button_icon.tsx +++ b/src/components/button/button_icon/button_icon.tsx @@ -170,6 +170,7 @@ export const EuiButtonIcon: FunctionComponent = (props) => { buttonColorStyles[color], buttonFocusStyle, display === 'empty' && emptyHoverStyles, + isDisabled && styles.isDisabled, ]; const classes = classNames( @@ -202,13 +203,24 @@ export const EuiButtonIcon: FunctionComponent = (props) => { ); } - // `original` size doesn't exist in `EuiLoadingSpinner` - // when the `iconSize` is `original` we don't pass any size to the `EuiLoadingSpinner` - // so it gets the default size - const loadingSize = iconSize === 'original' ? undefined : iconSize; - if (iconType && isLoading) { - buttonIcon = ; + // `original` size doesn't exist in `EuiLoadingSpinner` + // when the `iconSize` is `original` we don't pass any size to the `EuiLoadingSpinner` + // so it gets the default size + const loadingSize = iconSize === 'original' ? undefined : iconSize; + + // When the button is disabled the text gets gray + // and in some buttons the background gets a light gray + // for better contrast we want to change the border of the spinner + // 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' } + : undefined; + + buttonIcon = ( + + ); } // elements don't respect the `disabled` attribute. So if we're disabled, we'll just pretend From 9a3567e8359caf91cb4a7a00f308b7ed15ce7c41 Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Wed, 14 Jun 2023 22:56:49 -0700 Subject: [PATCH 05/11] [EuiButtonIcon] Remove unused size modifiers/maps + convert enum types to new preferred syntax + move type enums to top of file --- .../__snapshots__/button_icon.test.tsx.snap | 44 +++++++++---------- .../button/button_icon/button_icon.tsx | 29 +++--------- 2 files changed, 27 insertions(+), 46 deletions(-) diff --git a/src/components/button/button_icon/__snapshots__/button_icon.test.tsx.snap b/src/components/button/button_icon/__snapshots__/button_icon.test.tsx.snap index 5b98af268e8..2c3a4896687 100644 --- a/src/components/button/button_icon/__snapshots__/button_icon.test.tsx.snap +++ b/src/components/button/button_icon/__snapshots__/button_icon.test.tsx.snap @@ -3,7 +3,7 @@ exports[`EuiButtonIcon is rendered 1`] = ` `; diff --git a/src/components/button/button_icon/button_icon.test.tsx b/src/components/button/button_icon/button_icon.test.tsx index ba6c6a93f56..30424343b1f 100644 --- a/src/components/button/button_icon/button_icon.test.tsx +++ b/src/components/button/button_icon/button_icon.test.tsx @@ -7,7 +7,8 @@ */ import React from 'react'; -import { render, mount } from 'enzyme'; +import { fireEvent } from '@testing-library/dom'; +import { render } from '../../../test/rtl'; import { requiredProps } from '../../../test/required_props'; import { shouldRenderCustomStyles } from '../../../test/internal'; @@ -15,38 +16,38 @@ import { EuiButtonIcon, DISPLAYS, SIZES } from './button_icon'; import { BUTTON_COLORS } from '../../../themes/amsterdam/global_styling/mixins'; describe('EuiButtonIcon', () => { + shouldRenderCustomStyles( + + ); + test('is rendered', () => { - const component = render( + const { container } = render( ); - expect(component).toMatchSnapshot(); + expect(container.firstChild).toMatchSnapshot(); }); - shouldRenderCustomStyles( - - ); - describe('props', () => { describe('isDisabled', () => { it('is rendered', () => { - const component = render( + const { container } = render( ); - expect(component).toMatchSnapshot(); + expect(container.firstChild).toMatchSnapshot(); }); it('or disabled is rendered', () => { - const component = render( + const { container } = render( ); - expect(component).toMatchSnapshot(); + expect(container.firstChild).toMatchSnapshot(); }); it('renders a button even when href is defined', () => { - const component = render( + const { container } = render( { /> ); - expect(component).toMatchSnapshot(); + expect(container.firstChild).toMatchSnapshot(); }); }); describe('iconType', () => { it('is rendered', () => { - const component = render( + const { container } = render( ); - expect(component).toMatchSnapshot(); + expect(container.firstChild).toMatchSnapshot(); }); }); describe('color', () => { BUTTON_COLORS.forEach((color) => { test(`${color} is rendered`, () => { - const component = render( + const { container } = render( ); - expect(component).toMatchSnapshot(); + expect(container.firstChild).toMatchSnapshot(); }); }); test('ghost is rendered', () => { - const component = render( + const { container } = render( ); - expect(component).toMatchSnapshot(); + expect(container.firstChild).toMatchSnapshot(); }); }); describe('display', () => { DISPLAYS.forEach((display) => { test(`${display} is rendered`, () => { - const component = render( + const { container } = render( { /> ); - expect(component).toMatchSnapshot(); + expect(container.firstChild).toMatchSnapshot(); }); }); }); @@ -108,26 +109,26 @@ describe('EuiButtonIcon', () => { describe('size', () => { SIZES.forEach((size) => { test(`${size} is rendered`, () => { - const component = render( + const { container } = render( ); - expect(component).toMatchSnapshot(); + expect(container.firstChild).toMatchSnapshot(); }); }); }); describe('isSelected', () => { it('is rendered as true', () => { - const component = render( + const { container } = render( ); - expect(component).toMatchSnapshot(); + expect(container.firstChild).toMatchSnapshot(); }); it('is rendered as false', () => { - const component = render( + const { container } = render( { /> ); - expect(component).toMatchSnapshot(); + expect(container.firstChild).toMatchSnapshot(); }); }); describe('href', () => { it('secures the rel attribute when the target is _blank', () => { - const component = render( + const { container } = render( { /> ); - expect(component).toMatchSnapshot(); + expect(container.firstChild).toMatchSnapshot(); }); }); describe('onClick', () => { it('supports onClick and href', () => { - const handler = jest.fn(); - const component = mount( + const onClick = jest.fn(); + const { container } = render( ); - component.find('a').simulate('click'); - expect(handler.mock.calls.length).toEqual(1); + fireEvent.click(container.querySelector('a')!); + expect(onClick).toHaveBeenCalledTimes(1); }); it('supports onClick as a button', () => { - const handler = jest.fn(); - const component = mount( - + const onClick = jest.fn(); + const { container } = render( + ); - component.find('button').simulate('click'); - expect(handler.mock.calls.length).toEqual(1); + fireEvent.click(container.querySelector('button')!); + expect(onClick).toHaveBeenCalledTimes(1); }); }); describe('isLoading', () => { it('is rendered', () => { - const component = render( + const { container } = render( ); - expect(component).toMatchSnapshot(); + expect(container.firstChild).toMatchSnapshot(); }); }); }); From 8c1b3b01dd8960f325e404bd2671b213d327c06f Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Tue, 20 Jun 2023 08:15:42 -0700 Subject: [PATCH 09/11] changelog --- upcoming_changelogs/6844.md | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 upcoming_changelogs/6844.md diff --git a/upcoming_changelogs/6844.md b/upcoming_changelogs/6844.md new file mode 100644 index 00000000000..e9b0516181b --- /dev/null +++ b/upcoming_changelogs/6844.md @@ -0,0 +1,3 @@ +**CSS-in-JS conversions** + +- Converted `EuiButtonIcon` to Emotion From 48064892a5284135e2c7ec9b945fdcefcbe709d4 Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Tue, 20 Jun 2023 09:02:39 -0700 Subject: [PATCH 10/11] Update downstream snapshots --- .../__snapshots__/skip_link.test.tsx.snap | 10 +- .../__snapshots__/accordion.test.tsx.snap | 38 ++--- .../__snapshots__/basic_table.test.tsx.snap | 4 +- .../collapsed_item_actions.test.tsx.snap | 2 +- .../in_memory_table.test.tsx.snap | 4 +- .../card/__snapshots__/card.test.tsx.snap | 4 +- .../__snapshots__/card_select.test.tsx.snap | 10 +- .../__snapshots__/code_block.test.tsx.snap | 6 +- .../collapsible_nav.test.tsx.snap | 14 +- .../collapsible_nav_group.test.tsx.snap | 4 +- .../__snapshots__/control_bar.test.tsx.snap | 16 +- .../__snapshots__/data_grid.test.tsx.snap | 28 ++-- .../column_sorting.test.tsx.snap | 6 +- .../keyboard_shortcuts.test.tsx.snap | 2 +- .../super_date_picker.test.tsx.snap | 18 +-- .../quick_select_popover.test.tsx.snap | 6 +- .../__snapshots__/facet_button.test.tsx.snap | 12 +- .../flyout/__snapshots__/flyout.test.tsx.snap | 46 +++--- .../field_password.test.tsx.snap | 6 +- .../inline_edit_form.test.tsx.snap | 24 +-- .../__snapshots__/list_group.test.tsx.snap | 4 +- .../list_group_item.test.tsx.snap | 4 +- ...list_group_item_extra_action.test.tsx.snap | 10 +- .../pinnable_list_group.test.tsx.snap | 20 +-- .../markdown_editor.test.tsx.snap | 138 +++++++++--------- .../__snapshots__/confirm_modal.test.tsx.snap | 8 +- .../modal/__snapshots__/modal.test.tsx.snap | 2 +- .../notification_event.test.tsx.snap | 2 +- ...tification_event_read_button.test.tsx.snap | 4 +- .../__snapshots__/pagination.test.tsx.snap | 48 +++--- .../table_pagination.test.tsx.snap | 8 +- .../global_toast_list.test.tsx.snap | 8 +- 32 files changed, 258 insertions(+), 258 deletions(-) diff --git a/src/components/accessibility/skip_link/__snapshots__/skip_link.test.tsx.snap b/src/components/accessibility/skip_link/__snapshots__/skip_link.test.tsx.snap index 53af988e3e1..9f034b50274 100644 --- a/src/components/accessibility/skip_link/__snapshots__/skip_link.test.tsx.snap +++ b/src/components/accessibility/skip_link/__snapshots__/skip_link.test.tsx.snap @@ -3,7 +3,7 @@ exports[`EuiSkipLink is rendered 1`] = ` @@ -34,7 +34,7 @@ exports[`EuiSkipLink props position absolute is rendered 1`] = ` exports[`EuiSkipLink props position fixed is rendered 1`] = ` @@ -59,7 +59,7 @@ exports[`EuiSkipLink props position static is rendered 1`] = ` exports[`EuiSkipLink props tabIndex is rendered 1`] = `