From 0a9cb7a7899cf328bc001100028d36e68f77a386 Mon Sep 17 00:00:00 2001 From: Caroline Horn <549577+cchaos@users.noreply.github.com> Date: Wed, 15 Jul 2020 13:22:18 -0400 Subject: [PATCH] Chore/decouple button content (#3730) * [New] EuiButtonContent For rendering the contents of buttons, icon (loading spinner) and text wrapper for children * Use EuiButtonContent in EuiButtonEmpty * Fixing classNames and disabled states * Create EuiButtonDisplay for internal usage * Fix snaps * ts gymnastics * Added tests for EuiButtonContent * More optimization - Extend EuiButtonContentProps - Content styles are in button_content.scss * Restricted list of `element`s --- CHANGELOG.md | 1 + .../__snapshots__/skip_link.test.tsx.snap | 12 +- .../in_memory_table.test.tsx.snap | 95 ++++-- .../button/__snapshots__/button.test.tsx.snap | 109 +++++-- .../button_content.test.tsx.snap | 79 +++++ src/components/button/_button.scss | 19 +- src/components/button/_button_content.scss | 7 + src/components/button/_index.scss | 1 + src/components/button/button.test.tsx | 25 +- src/components/button/button.tsx | 222 +++++++------ src/components/button/button_content.test.tsx | 72 +++++ src/components/button/button_content.tsx | 88 ++++++ .../__snapshots__/button_empty.test.tsx.snap | 122 ++++++-- .../button/button_empty/_button_empty.scss | 18 +- .../button/button_empty/button_empty.test.tsx | 39 ++- .../button/button_empty/button_empty.tsx | 96 +++--- src/components/button/button_empty/index.ts | 1 - .../__snapshots__/button_group.test.tsx.snap | 116 +++---- .../button/button_group/button_group.tsx | 5 +- .../__snapshots__/button_toggle.test.tsx.snap | 2 +- src/components/button/index.ts | 1 - .../card/__snapshots__/card.test.tsx.snap | 2 +- .../__snapshots__/card_select.test.tsx.snap | 15 +- .../collapsible_nav.test.tsx.snap | 40 +-- .../__snapshots__/control_bar.test.tsx.snap | 294 +++++++++++++----- .../__snapshots__/data_grid.test.tsx.snap | 73 ++--- .../__snapshots__/filter_button.test.tsx.snap | 43 ++- .../__snapshots__/header_link.test.tsx.snap | 4 +- .../__snapshots__/confirm_modal.test.tsx.snap | 8 +- .../__snapshots__/pagination.test.tsx.snap | 2 +- .../pagination_button.test.tsx.snap | 2 +- .../table_sort_mobile.test.tsx.snap | 7 +- .../table_pagination.test.tsx.snap | 31 +- src/global_styling/mixins/_button.scss | 17 +- 34 files changed, 1115 insertions(+), 553 deletions(-) create mode 100644 src/components/button/__snapshots__/button_content.test.tsx.snap create mode 100644 src/components/button/_button_content.scss create mode 100644 src/components/button/button_content.test.tsx create mode 100644 src/components/button/button_content.tsx diff --git a/CHANGELOG.md b/CHANGELOG.md index 28e9b4178af..c326eee1f35 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ No public interface changes since `27.1.0`. - Added `index.d.ts` file to `lib/test` and `es/test` ([#3715](https://github.com/elastic/eui/pull/3715)) - Added `descriptionFlexItemProps` and `fieldFlexItemProps` props to `EuiDescribedFormGroup` ([#3717](https://github.com/elastic/eui/pull/3717)) - Expanded `EuiBasicTable`'s default action's name configuration to accept a function that returns a React node ([#3739](https://github.com/elastic/eui/pull/3739)) +- Added internal use only button building blocks for reusability in other button components ([#3730](https://github.com/elastic/eui/pull/3730)) ## [`27.0.0`](https://github.com/elastic/eui/tree/v27.0.0) - Added `paddingSize` prop to `EuiCard` ([#3638](https://github.com/elastic/eui/pull/3638)) diff --git a/src/components/accessibility/__snapshots__/skip_link.test.tsx.snap b/src/components/accessibility/__snapshots__/skip_link.test.tsx.snap index 8d73fa7dd02..5a12b3e9c5c 100644 --- a/src/components/accessibility/__snapshots__/skip_link.test.tsx.snap +++ b/src/components/accessibility/__snapshots__/skip_link.test.tsx.snap @@ -9,7 +9,7 @@ exports[`EuiSkipLink is rendered 1`] = ` rel="noreferrer" > @@ -545,15 +554,25 @@ exports[`EuiInMemoryTable behavior pagination 1`] = ` onClick={[Function]} rel="noreferrer" > - - 1 + + 1 + - + @@ -611,21 +630,31 @@ exports[`EuiInMemoryTable behavior pagination 1`] = ` aria-controls="generated-id" aria-current={true} aria-label="Page 2 of 2" - className="euiButtonEmpty euiButtonEmpty--text euiButtonEmpty--xSmall euiPaginationButton euiPaginationButton-isActive euiPaginationButton--hideOnMobile" + className="euiButtonEmpty euiButtonEmpty--text euiButtonEmpty--xSmall euiButtonEmpty-isDisabled euiPaginationButton euiPaginationButton-isActive euiPaginationButton--hideOnMobile" data-test-subj="pagination-button-1" disabled={true} onClick={[Function]} type="button" > - - 2 + + 2 + - + diff --git a/src/components/button/__snapshots__/button.test.tsx.snap b/src/components/button/__snapshots__/button.test.tsx.snap index 4df2e87c8ca..8e699c46818 100644 --- a/src/components/button/__snapshots__/button.test.tsx.snap +++ b/src/components/button/__snapshots__/button.test.tsx.snap @@ -8,7 +8,7 @@ exports[`EuiButton is rendered 1`] = ` type="button" > `; +exports[`EuiButton props contentProps is rendered 1`] = ` + +`; + exports[`EuiButton props fill is rendered 1`] = ` +`; + +exports[`EuiButton props isDisabled renders if passed as disabled 1`] = ` + `; + +exports[`EuiButton props textProps is rendered 1`] = ` + +`; diff --git a/src/components/button/__snapshots__/button_content.test.tsx.snap b/src/components/button/__snapshots__/button_content.test.tsx.snap new file mode 100644 index 00000000000..0625a6a37f6 --- /dev/null +++ b/src/components/button/__snapshots__/button_content.test.tsx.snap @@ -0,0 +1,79 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`EuiButtonContent is rendered 1`] = ` + + + +`; + +exports[`EuiButtonContent props children is rendered 1`] = ` + + + Content + + +`; + +exports[`EuiButtonContent props iconSide is rendered 1`] = ` + +
+ + +`; + +exports[`EuiButtonContent props iconType is rendered 1`] = ` + +
+ + +`; + +exports[`EuiButtonContent props isLoading is rendered 1`] = ` + + + + +`; + +exports[`EuiButtonContent props isLoading replaces iconType with spinner 1`] = ` + + + + +`; + +exports[`EuiButtonContent props textProps is rendered 1`] = ` + + + +`; diff --git a/src/components/button/_button.scss b/src/components/button/_button.scss index 2551d35f3f5..d77c41f9db6 100644 --- a/src/components/button/_button.scss +++ b/src/components/button/_button.scss @@ -10,8 +10,6 @@ min-width: $euiButtonMinWidth; .euiButton__content { - @include euiButtonContent; - padding: 0 ($euiSize - $euiSizeXS); } @@ -25,12 +23,6 @@ line-height: $euiButtonHeightSmall; // prevents descenders from getting cut off } - &.euiButton--iconRight { - .euiButton__content { - @include euiButtonContent($isReverse: true); - } - } - &:hover, &:active { @include euiSlightShadowHover; @@ -44,19 +36,12 @@ } &:disabled { + @include euiButtonContentDisabled; + color: $euiButtonColorDisabledText; border-color: $euiButtonColorDisabled; pointer-events: none; - .euiButton__content { - pointer-events: auto; - cursor: not-allowed; - } - - .euiButton__spinner { - border-color: euiLoadingSpinnerBorderColors(currentColor); - } - &.euiButton--fill { // Only increase the contrast of background color to text to 2.0 for disabled color: makeHighContrastColor($euiButtonColorDisabled, $euiButtonColorDisabled, 2); diff --git a/src/components/button/_button_content.scss b/src/components/button/_button_content.scss new file mode 100644 index 00000000000..e1933a0cbf7 --- /dev/null +++ b/src/components/button/_button_content.scss @@ -0,0 +1,7 @@ +.euiButtonContent { + @include euiButtonContent; +} + +.euiButtonContent--iconRight { + @include euiButtonContent($isReverse: true); +} diff --git a/src/components/button/_index.scss b/src/components/button/_index.scss index 2815f912feb..61a1784e7f4 100644 --- a/src/components/button/_index.scss +++ b/src/components/button/_index.scss @@ -1,4 +1,5 @@ @import 'button'; +@import 'button_content'; @import 'button_empty/index'; @import 'button_icon/index'; @import 'button_toggle/index'; diff --git a/src/components/button/button.test.tsx b/src/components/button/button.test.tsx index 69f0ba0af97..bcc7c3cd655 100644 --- a/src/components/button/button.test.tsx +++ b/src/components/button/button.test.tsx @@ -21,7 +21,8 @@ import React from 'react'; import { render, mount } from 'enzyme'; import { requiredProps } from '../../test/required_props'; -import { EuiButton, COLORS, SIZES, ICON_SIDES } from './button'; +import { EuiButton, COLORS, SIZES } from './button'; +import { ICON_SIDES } from './button_content'; describe('EuiButton', () => { test('is rendered', () => { @@ -51,6 +52,12 @@ describe('EuiButton', () => { expect(component).toMatchSnapshot(); }); + + it('renders if passed as disabled', () => { + const component = render(); + + expect(component).toMatchSnapshot(); + }); }); describe('isLoading', () => { @@ -134,5 +141,21 @@ describe('EuiButton', () => { expect(handler.mock.calls.length).toEqual(1); }); }); + + test('contentProps is rendered', () => { + const component = render( + Content + ); + + expect(component).toMatchSnapshot(); + }); + + test('textProps is rendered', () => { + const component = render( + Content + ); + + expect(component).toMatchSnapshot(); + }); }); }); diff --git a/src/components/button/button.tsx b/src/components/button/button.tsx index 7d01b5f9e44..c230b0d2214 100644 --- a/src/components/button/button.tsx +++ b/src/components/button/button.tsx @@ -17,12 +17,7 @@ * under the License. */ -import React, { - FunctionComponent, - HTMLAttributes, - Ref, - ButtonHTMLAttributes, -} from 'react'; +import React, { FunctionComponent, Ref, ButtonHTMLAttributes } from 'react'; import classNames from 'classnames'; import { @@ -32,13 +27,14 @@ import { PropsForButton, keysOf, } from '../common'; -import { EuiLoadingSpinner } from '../loading'; import { getSecureRelForTarget } from '../../services'; -import { IconType, EuiIcon } from '../icon'; - -export type ButtonIconSide = 'left' | 'right'; +import { + EuiButtonContentProps, + EuiButtonContentType, + EuiButtonContent, +} from './button_content'; export type ButtonColor = | 'primary' @@ -72,35 +68,121 @@ const sizeToClassNameMap: { [size in ButtonSize]: string | null } = { export const SIZES = keysOf(sizeToClassNameMap); -const iconSideToClassNameMap: { [side in ButtonIconSide]: string | null } = { - left: null, - right: 'euiButton--iconRight', -}; - -export const ICON_SIDES = keysOf(iconSideToClassNameMap); - -export interface EuiButtonProps extends CommonProps { - iconType?: IconType; - iconSide?: ButtonIconSide; +/** + * Extends EuiButtonContentProps which provides + * `iconType`, `iconSide`, and `textProps` + */ +export interface EuiButtonProps extends EuiButtonContentProps, CommonProps { + /** + * Make button a solid color for prominence + */ fill?: boolean; /** - * `text` color is set for deprecation + * Any of our named colors. `text` color is set for deprecation */ color?: ButtonColor; + /** + * Use size `s` in confined spaces + */ size?: ButtonSize; - isLoading?: boolean; + /** + * `disabled` is also allowed + */ isDisabled?: boolean; + /** + * Extends the button to 100% width + */ fullWidth?: boolean; /** - * Object of props passed to the wrapping the button's content + * Force disables the button and changes the icon to a loading spinner */ - contentProps?: HTMLAttributes; + isLoading?: boolean; /** - * Object of props passed to the wrapping the component's {children} + * Object of props passed to the wrapping the button's content */ - textProps?: HTMLAttributes; + contentProps?: EuiButtonContentType; } +export interface EuiButtonDisplayProps extends EuiButtonProps { + element: 'a' | 'button' | 'span' | 'label'; +} + +/** + * *INTERNAL ONLY* + * Component for displaying any element as a button + * EuiButton is largely responsible for providing relevant props + * and the logic for element-specific attributes + */ +const EuiButtonDisplay = React.forwardRef( + ( + { + children, + className, + iconType, + iconSide = 'left', + color = 'primary', + size = 'm', + fill = false, + isDisabled, + isLoading, + contentProps, + textProps, + fullWidth, + element = 'button', + ...rest + }, + ref + ) => { + const classes = classNames( + 'euiButton', + color ? colorToClassNameMap[color] : null, + size ? sizeToClassNameMap[size] : null, + className, + { + 'euiButton--fill': fill, + 'euiButton--fullWidth': fullWidth, + 'euiButton-isDisabled': isDisabled, + } + ); + + const contentClassNames = classNames( + 'euiButton__content', + contentProps && contentProps.className + ); + + const textClassNames = classNames( + 'euiButton__text', + textProps && textProps.className + ); + + const innerNode = ( + + {children} + + ); + + return React.createElement( + element, + { + className: classes, + ref, + ...rest, + }, + innerNode + ); + } +); + +EuiButtonDisplay.displayName = 'EuiButtonDisplay'; +export { EuiButtonDisplay }; + type EuiButtonPropsForAnchor = PropsForAnchor< EuiButtonProps, { @@ -121,79 +203,26 @@ export type Props = ExclusiveUnion< >; export const EuiButton: FunctionComponent = ({ - children, - className, - iconType, - iconSide = 'left', - color = 'primary', - size = 'm', - fill = false, isDisabled, - isLoading, + disabled, href, target, rel, type = 'button', buttonRef, - contentProps, - textProps, - fullWidth, ...rest }) => { - // If in the loading state, force disabled to true - isDisabled = isLoading ? true : isDisabled; - - const classes = classNames( - 'euiButton', - color ? colorToClassNameMap[color] : null, - size ? sizeToClassNameMap[size] : null, - iconSide ? iconSideToClassNameMap[iconSide] : null, - className, - { - 'euiButton--fill': fill, - 'euiButton--fullWidth': fullWidth, - } - ); - - const contentClassNames = classNames( - 'euiButton__content', - contentProps && contentProps.className - ); - - const textClassNames = classNames( - 'euiButton__text', - textProps && textProps.className - ); - - // Add an icon to the button if one exists. - let buttonIcon; - - if (isLoading) { - buttonIcon = ; - } else if (iconType) { - buttonIcon = ( -
- + + + Sound the Alarm + + + + +
- + + + Sound the Alarm + + + + +
- + + + Sound the Alarm + + + + +
- + + + Sound the Alarm + + + + +
- + + + Sound the Alarm + + + + +
- + + + Sound the Alarm + + + + +