From 7430ef53b8a7ee23beca3aeb39048d8219ecc4f9 Mon Sep 17 00:00:00 2001 From: Cee Chen <549407+cee-chen@users.noreply.github.com> Date: Mon, 20 Nov 2023 14:21:47 -0800 Subject: [PATCH 1/9] [EuiDataGrid] Cell actions redesign (#7343) --- changelogs/upcoming/7343.md | 6 + cypress/support/component-index.html | 1 + src/components/combo_box/combo_box.spec.tsx | 13 - .../__snapshots__/data_grid.test.tsx.snap | 780 ++++++++---------- .../datagrid/_data_grid_data_row.scss | 221 ++--- src/components/datagrid/_mixins.scss | 4 +- .../data_grid_body_custom.test.tsx.snap | 104 +-- .../data_grid_body_virtualized.test.tsx.snap | 60 +- .../data_grid_cell.test.tsx.snap | 50 +- .../datagrid/body/data_grid_cell.test.tsx | 14 +- .../datagrid/body/data_grid_cell.tsx | 101 ++- .../body/data_grid_cell_actions.test.tsx | 19 +- .../datagrid/body/data_grid_cell_actions.tsx | 23 +- .../body/data_grid_cell_popover.spec.tsx | 129 +-- .../body/data_grid_cell_popover.test.tsx | 20 +- .../datagrid/body/data_grid_cell_popover.tsx | 28 + .../body/footer/data_grid_footer_row.test.tsx | 4 + .../body/header/_data_grid_header_row.scss | 2 +- src/components/datagrid/data_grid.test.tsx | 58 +- src/components/datagrid/data_grid_types.ts | 11 +- .../datagrid/utils/row_heights.test.ts | 19 +- src/components/datagrid/utils/row_heights.ts | 4 +- .../datagrid/utils/scrolling.spec.tsx | 4 +- src/components/popover/popover.tsx | 7 +- .../text_truncate/text_truncate.spec.tsx | 13 - src/components/text_truncate/utils.spec.tsx | 12 - 26 files changed, 818 insertions(+), 889 deletions(-) create mode 100644 changelogs/upcoming/7343.md diff --git a/changelogs/upcoming/7343.md b/changelogs/upcoming/7343.md new file mode 100644 index 00000000000..300b86a1098 --- /dev/null +++ b/changelogs/upcoming/7343.md @@ -0,0 +1,6 @@ +- Updated `EuiDataGrid` cell actions to display above cells instead of within them, to avoid content clipping issues +- Updated `EuiDataGrid` cell expansion popovers to sit on top of cells instead of below/next to them + +**Bug fixes** + +- Fixed incorrect `EuiPopover` positioning calculations when `hasArrow` was set to false diff --git a/cypress/support/component-index.html b/cypress/support/component-index.html index ef0d3f7879b..2c3cd4b3d41 100644 --- a/cypress/support/component-index.html +++ b/cypress/support/component-index.html @@ -6,6 +6,7 @@ + Components App diff --git a/src/components/combo_box/combo_box.spec.tsx b/src/components/combo_box/combo_box.spec.tsx index dee351c7075..ca54e544d65 100644 --- a/src/components/combo_box/combo_box.spec.tsx +++ b/src/components/combo_box/combo_box.spec.tsx @@ -20,19 +20,6 @@ import { type EuiComboBoxOptionOption, } from './index'; -// CI doesn't have access to the Inter font, so we need to manually include it -// for truncation font calculations to work correctly -before(() => { - const linkElem = document.createElement('link'); - linkElem.setAttribute('rel', 'stylesheet'); - linkElem.setAttribute( - 'href', - 'https://fonts.googleapis.com/css2?family=Inter:wght@400;700&display=swap' - ); - document.head.appendChild(linkElem); - cy.wait(1000); // Wait a second to give the font time to load/swap in -}); - describe('EuiComboBox', () => { describe('focus management', () => { it('keeps focus on the input box when clicking a disabled item', () => { diff --git a/src/components/datagrid/__snapshots__/data_grid.test.tsx.snap b/src/components/datagrid/__snapshots__/data_grid.test.tsx.snap index b6dc51f0c29..cf9665dca56 100644 --- a/src/components/datagrid/__snapshots__/data_grid.test.tsx.snap +++ b/src/components/datagrid/__snapshots__/data_grid.test.tsx.snap @@ -689,7 +689,7 @@ exports[`EuiDataGrid rendering renders additional toolbar controls 1`] = `
-
- 0, A -
- + 0, A
+
-
- 0, B -
- + 0, B
+
-
- 1, A -
- + 1, A
+
-
- 1, B -
- + 1, B
+
-
- 2, A -
- + 2, A
+
-
- 2, B -
- + 2, B
+
@@ -1164,7 +1140,7 @@ exports[`EuiDataGrid rendering renders control columns 1`] = `
-
- 0 -
- + 0
+
-
- 0, A -
- + 0, A
+
-
- 0, B -
- + 0, B
+
-
- 0 -
- + 0
+
-
- 1 -
- + 1
+
-
- 1, A -
- + 1, A
+
-
- 1, B -
- + 1, B
+
-
- 1 -
- + 1
+
-
- 2 -
- + 2
+
-
- 2, A -
- + 2, A
+
-
- 2, B -
- + 2, B
+
-
- 2 -
- + 2
+
-
- 0, A -
- + 0, A
+
-
- 0, B -
- + 0, B
+
-
- 1, A -
- + 1, A
+
-
- 1, B -
- + 1, B
+
-
- 2, A -
- + 2, A
+
-
- 2, B -
- + 2, B
+
@@ -2298,7 +2202,7 @@ exports[`EuiDataGrid rendering renders with common and div attributes 1`] = `
-
- 0, A -
- + 0, A
+
-
- 0, B -
- + 0, B
+
-
- 1, A -
- + 1, A
+
-
- 1, B -
- + 1, B
+
-
- 2, A -
- + 2, A
+
-
- 2, B -
- + 2, B
+
diff --git a/src/components/datagrid/_data_grid_data_row.scss b/src/components/datagrid/_data_grid_data_row.scss index 6957f7f241e..0fdfde41139 100644 --- a/src/components/datagrid/_data_grid_data_row.scss +++ b/src/components/datagrid/_data_grid_data_row.scss @@ -3,15 +3,23 @@ } @include euiDataGridRowCell { - @include euiFontSizeS; - - padding: $euiDataGridCellPaddingM; + position: relative; // Needed for .euiDataGridRowCell__actions border-right: $euiDataGridVerticalBorder; border-bottom: $euiBorderThin; - overflow: hidden; + + .euiDataGridRowCell__content { + @include euiFontSizeS; + padding: $euiDataGridCellPaddingM; + height: 100%; + overflow: hidden; + + &--autoHeight { + height: auto; + } + } // Hack to allow focus trap to still stretch to full row height on defined heights - > * { + > [data-focus-lock-disabled] { height: 100%; } @@ -23,39 +31,42 @@ border-right-color: $euiBorderColor; } + &:hover, &:focus { - position: relative; + --euiDataGridCellOutlineColor: #{$euiColorPrimary}; @include euiDataGridCellFocus; } - // Only add the transition effect on hover, so that it is instantaneous on focus - // Long delays on hover to mitigate the accordion effect - &:hover { - .euiDataGridRowCell__actionButtonIcon { - animation-duration: $euiAnimSpeedExtraFast; - animation-name: euiDataGridCellActionsSlideIn; - animation-iteration-count: 1; - animation-delay: $euiAnimSpeedNormal; - animation-fill-mode: forwards; + // On hover + &:hover:not(:focus, :focus-within, .euiDataGridRowCell--open) { + // Color the actions and focus ring grayscale on hover + // (Actions look odd without the ring on grids without cell borders) + --euiDataGridCellOutlineColor: #{$euiColorDarkShade}; + + .euiDataGridRowCell__actions { + // Delay the actions showing on hover + // (Note: the focus ring show instantly on hover & the actions show instantly on focus) + animation-delay: $euiAnimSpeedSlow; } } - // On focus, directly show action buttons (without animation) + // On hover & focus, show cell action buttons + &:hover, &:focus, - // Prevent the animation from flickering after cell popover close when focus is restored the expansion button &:focus-within, // Always make the cell actions visible when the cell popover is open &.euiDataGridRowCell--open { - .euiDataGridRowCell__actionButtonIcon { - animation: none; - margin-left: $euiDataGridCellPaddingM; - width: $euiSizeM; + .euiDataGridRowCell__actions { + animation-duration: $euiAnimSpeedExtraFast; + animation-name: euiDataGridCellActionsSlideIn; + animation-iteration-count: 1; + animation-fill-mode: forwards; } } // if a cell is not hovered nor focused nor open via popover, don't show buttons in general &:not(:hover):not(:focus):not(.euiDataGridRowCell--open) { - .euiDataGridRowCell__actionButtonIcon { + .euiDataGridRowCell__actions { display: none; } } @@ -84,94 +95,115 @@ .euiDataGridRowCell__popover { @include euiScrollBar; overflow: auto; - // stylelint-disable declaration-no-important - max-width: 400px !important; - max-height: 400px !important; - z-index: $euiZDataGridCellPopover !important; - // stylelint-enable declaration-no-important + z-index: $euiZDataGridCellPopover !important; // stylelint-disable-line declaration-no-important // Workaround for a Safari CSS bug when using both `overflow: auto` & `filter: drop-shadow` // (see https://github.com/elastic/eui/issues/6151) // Disables the default EuiPopover filter drop-shadow and uses box-shadow instead, // since we don't use the popover arrow in any case for cell popovers filter: none; @include euiBottomShadow; // TODO: Convert to euiShadowMedium() in Emotion -} -.euiDataGridRowCell__contentWrapper { - position: relative; // Needed for .euiDataGridRowCell__actions--overlay - height: 100%; + // For some reason, the normal popover opacity transition doesn't work for datagrid popovers + // so we'll force it via an animation. If we don't, cells constrained by the inline max-width + // style that we set will see a flash of unwanted content before repositioning + animation-duration: $euiAnimSpeedNormal; + animation-name: euiDataGridCellPopover; } -.euiDataGridRowCell__defaultHeight { +.euiDataGridRowCell--controlColumn .euiDataGridRowCell__content { + max-height: 100%; + height: auto; display: flex; - align-items: baseline; - max-width: 100%; - - .euiDataGridRowCell__content { - flex-grow: 1; - } + align-items: center; +} - .euiDataGridRowCell__actions { - flex-grow: 0; - } +// Positioning for cell actions & the cell expansion popover +.euiDataGridRowCell__actions, +.euiDataGridRowCell__actions + [data-euiportal] > .euiPopover { + position: absolute; + bottom: 100%; - .euiDataGridRowCell--controlColumn & { - height: 100%; - align-items: center; + .euiDataGridRowCell--alignLeft & { + left: 0; } -} -.euiDataGridRowCell__numericalHeight { - // Without this rule, popover anchors content that overflows off the page - [data-euiportal], - .euiPopover { - height: 100%; + .euiDataGridRowCell--alignRight & { + right: 0; } } -// Cell actions .euiDataGridRowCell__actions { + z-index: $euiZDataGridCellPopover - 2; // Sit below sticky column headers + margin-bottom: -$euiBorderWidthThin; // Vertical alignment display: flex; + gap: $euiSizeXS / 2; + padding-inline: $euiSizeXS / 2; + background-color: var(--euiDataGridCellOutlineColor); + color: $euiColorEmptyShade; + border: $euiBorderWidthThin solid var(--euiDataGridCellOutlineColor); + border-top-left-radius: $euiBorderRadius / 2; + border-top-right-radius: $euiBorderRadius / 2; + transform: scaleY(0); + transform-origin: bottom; + + // The first row of cell actions need to be visible above the cell headers, + // but other cell actions that scroll past the sticky headers should not + .euiDataGridRowCell[data-gridcell-visible-row-index='0'] > & { + z-index: $euiZDataGridCellPopover - 1; + } - &--overlay { + // Visual trickery - fill in the gap between the cell outline border-radius & the actions + &::after { + content: ''; position: absolute; - right: 0; - top: 0; - padding: $euiDataGridCellPaddingM 0; - background-color: $euiColorEmptyShade; + top: 100%; + height: $euiBorderWidthThick; + width: $euiBorderWidthThick; + background-color: var(--euiDataGridCellOutlineColor); + + .euiDataGridRowCell--alignLeft & { + left: -$euiBorderWidthThin; + } + + .euiDataGridRowCell--alignRight & { + right: -$euiBorderWidthThin; + } } } .euiDataGridRowCell__actionButtonIcon { - height: $euiSizeM; - border-radius: $euiBorderRadius / 2; - width: 0; - overflow: hidden; - // Have to take out the generic transition so it is instaneous on focus - transition: none; - // Disable filled button box-shadows on legacy theme - they're unnecessary when this small in a datagrid - box-shadow: none !important; // stylelint-disable-line declaration-no-important - // Remove filled button borders on legacy theme - this way we don't need to animate the border - border: none; + height: $euiSize + $euiSizeXS; + width: $euiSize; + border-radius: 0; + + /* Force all cell action buttons to match EUI colors */ + &, + svg { + // stylelint-disable declaration-no-important + background-color: transparent !important; + color: currentColor !important; + fill: currentColor !important; + // stylelint-enable declaration-no-important + } + + /* Manually increase the size of the expand cell icon - it's a bit small by default */ + &.euiDataGridRowCell__expandCell .euiIcon { + width: 120%; + height: 100%; + } } // Row stripes @include euiDataGridStyles(stripes) { .euiDataGridRow--striped { - &, - .euiDataGridRowCell__actions--overlay { - background-color: $euiColorLightestShade; - } + background-color: $euiColorLightestShade; } } // Row highlights @include euiDataGridStyles(rowHoverHighlight) { .euiDataGridRow:hover { - &, - .euiDataGridRowCell__actions--overlay { - background-color: $euiColorHighlight; - } + background-color: $euiColorHighlight; } } @@ -192,48 +224,43 @@ // Font alternates @include euiDataGridStyles(fontSizeSmall) { @include euiDataGridRowCell { - @include euiFontSizeXS; + .euiDataGridRowCell__content { + @include euiFontSizeXS; + } } } @include euiDataGridStyles(fontSizeLarge) { @include euiDataGridRowCell { - @include euiFontSize; + .euiDataGridRowCell__content { + @include euiFontSize; + } } } // Padding alternates @include euiDataGridStyles(paddingSmall) { @include euiDataGridRowCell { - padding: $euiDataGridCellPaddingS; + .euiDataGridRowCell__content { + padding: $euiDataGridCellPaddingS; + } } } @include euiDataGridStyles(paddingLarge) { @include euiDataGridRowCell { - padding: $euiDataGridCellPaddingL; - } -} - -// Compressed density grids - height tweaks -@include euiDataGridStyles(fontSizeSmall, paddingSmall) { - .euiDataGridRowCell__actions--overlay { - padding: ($euiDataGridCellPaddingS / 2) 0; - } - - .euiDataGridRowCell__defaultHeight .euiDataGridRowCell__actions { - transform: translateY(1px); + .euiDataGridRowCell__content { + padding: $euiDataGridCellPaddingL; + } } } @keyframes euiDataGridCellActionsSlideIn { - from { - margin-left: 0; - width: 0; - } + from { transform: scaleY(0); } + to { transform: scaleY(1); } +} - to { - margin-left: $euiDataGridCellPaddingM; - width: $euiSizeM; - } +@keyframes euiDataGridCellPopover { + from { opacity: 0; } + to { opacity: 1; } } diff --git a/src/components/datagrid/_mixins.scss b/src/components/datagrid/_mixins.scss index 4303730770c..8952750a738 100644 --- a/src/components/datagrid/_mixins.scss +++ b/src/components/datagrid/_mixins.scss @@ -64,8 +64,8 @@ $euiDataGridStyles: ( position: absolute; top: 0; left: 0; - border: $euiBorderWidthThick solid $euiFocusRingColor; - border-radius: $euiBorderRadiusSmall; + border: $euiBorderWidthThick solid var(--euiDataGridCellOutlineColor, $euiFocusRingColor); + border-radius: $euiBorderRadius / 2; z-index: 2; // We want this to be on top of all the content pointer-events: none; // Because we put it with a higher z-index we don't want to make it clickable this way we allow selecting the content behind } diff --git a/src/components/datagrid/body/__snapshots__/data_grid_body_custom.test.tsx.snap b/src/components/datagrid/body/__snapshots__/data_grid_body_custom.test.tsx.snap index d51f94ba5d2..d17d29bfa94 100644 --- a/src/components/datagrid/body/__snapshots__/data_grid_body_custom.test.tsx.snap +++ b/src/components/datagrid/body/__snapshots__/data_grid_body_custom.test.tsx.snap @@ -115,7 +115,7 @@ exports[`EuiDataGridBodyCustomRender treats \`renderCustomGridBody\` as a render >
-
- hello -
- + hello
+
-
- world -
- + world
+
-
- lorem -
- + lorem
+
-
- ipsum -
- + ipsum
+
diff --git a/src/components/datagrid/body/__snapshots__/data_grid_body_virtualized.test.tsx.snap b/src/components/datagrid/body/__snapshots__/data_grid_body_virtualized.test.tsx.snap index c58ccaf130d..7452e75b2c3 100644 --- a/src/components/datagrid/body/__snapshots__/data_grid_body_virtualized.test.tsx.snap +++ b/src/components/datagrid/body/__snapshots__/data_grid_body_virtualized.test.tsx.snap @@ -116,7 +116,7 @@ exports[`EuiDataGridBodyVirtualized renders 1`] = `
-
- - cell content - -
- + + cell content +
+
-
- - cell content - -
- + + cell content +
+
diff --git a/src/components/datagrid/body/__snapshots__/data_grid_cell.test.tsx.snap b/src/components/datagrid/body/__snapshots__/data_grid_cell.test.tsx.snap index 019cda14347..d06990806c9 100644 --- a/src/components/datagrid/body/__snapshots__/data_grid_cell.test.tsx.snap +++ b/src/components/datagrid/body/__snapshots__/data_grid_cell.test.tsx.snap @@ -39,7 +39,7 @@ exports[`EuiDataGridCell componentDidUpdate handles the cell popover by forwardi exports[`EuiDataGridCell renders 1`] = `
-
-
- - -
+
+ +
-
+
`; diff --git a/src/components/datagrid/body/data_grid_cell.test.tsx b/src/components/datagrid/body/data_grid_cell.test.tsx index 0271566e937..da2eccd6284 100644 --- a/src/components/datagrid/body/data_grid_cell.test.tsx +++ b/src/components/datagrid/body/data_grid_cell.test.tsx @@ -26,6 +26,7 @@ describe('EuiDataGridCell', () => { closeCellPopover: jest.fn(), openCellPopover: jest.fn(), setPopoverAnchor: jest.fn(), + setPopoverAnchorPosition: jest.fn(), setPopoverContent: jest.fn(), setCellPopoverProps: () => {}, }; @@ -216,6 +217,7 @@ describe('EuiDataGridCell', () => { it('resets cell props when the cell is moved (columnId) or sorted (rowIndex)', () => { const setState = jest.spyOn(EuiDataGridCell.prototype, 'setState'); const component = mount(); + setState.mockClear(); component.setProps({ columnId: 'newColumnId' }); expect(setState).toHaveBeenCalledWith({ cellProps: {} }); @@ -727,7 +729,7 @@ describe('EuiDataGridCell', () => { ); expect( - component.find('.euiDataGridRowCell__defaultHeight').exists() + component.find('.euiDataGridRowCell__content--defaultHeight').exists() ).toBe(true); expect(component.find('.eui-textTruncate').exists()).toBe(true); }); @@ -740,9 +742,9 @@ describe('EuiDataGridCell', () => { /> ); - expect(component.find('.euiDataGridRowCell__autoHeight').exists()).toBe( - true - ); + expect( + component.find('.euiDataGridRowCell__content--autoHeight').exists() + ).toBe(true); expect(component.find('.eui-textBreakWord').exists()).toBe(true); }); @@ -755,7 +757,7 @@ describe('EuiDataGridCell', () => { ); expect( - component.find('.euiDataGridRowCell__numericalHeight').exists() + component.find('.euiDataGridRowCell__content--numericalHeight').exists() ).toBe(true); expect(component.find('.eui-textBreakWord').exists()).toBe(true); }); @@ -769,7 +771,7 @@ describe('EuiDataGridCell', () => { ); expect( - component.find('.euiDataGridRowCell__lineCountHeight').exists() + component.find('.euiDataGridRowCell__content--lineCountHeight').exists() ).toBe(true); expect(component.find('.eui-textBreakWord').exists()).toBe(true); expect(component.find('.euiTextBlockTruncate').exists()).toBe(true); diff --git a/src/components/datagrid/body/data_grid_cell.tsx b/src/components/datagrid/body/data_grid_cell.tsx index 23ec939abb3..0021a8cfc2f 100644 --- a/src/components/datagrid/body/data_grid_cell.tsx +++ b/src/components/datagrid/body/data_grid_cell.tsx @@ -49,13 +49,11 @@ const EuiDataGridCellContent: FunctionComponent< EuiDataGridCellValueProps & { setCellProps: EuiDataGridCellValueElementProps['setCellProps']; setCellContentsRef: EuiDataGridCell['setCellContentsRef']; - setPopoverAnchorRef: MutableRefObject; isExpanded: boolean; isControlColumn: boolean; isFocused: boolean; ariaRowIndex: number; rowHeight?: EuiDataGridRowHeightOption; - cellHeightType: string; cellActions?: ReactNode; } > = memo( @@ -63,7 +61,6 @@ const EuiDataGridCellContent: FunctionComponent< renderCellValue, column, setCellContentsRef, - setPopoverAnchorRef, rowIndex, colIndex, ariaRowIndex, @@ -71,7 +68,6 @@ const EuiDataGridCellContent: FunctionComponent< rowHeightUtils, isControlColumn, isFocused, - cellHeightType, cellActions, ...rest }) => { @@ -79,13 +75,12 @@ const EuiDataGridCellContent: FunctionComponent< const CellElement = renderCellValue as JSXElementConstructor; - const wrapperClasses = classNames( - 'euiDataGridRowCell__contentWrapper', - `euiDataGridRowCell__${cellHeightType}Height` - ); + const cellHeightType = + rowHeightUtils?.getHeightType(rowHeight) || 'default'; const classes = classNames( 'euiDataGridRowCell__content', + `euiDataGridRowCell__content--${cellHeightType}Height`, !isControlColumn && { 'eui-textBreakWord': cellHeightType !== 'default', 'eui-textTruncate': cellHeightType === 'default', @@ -94,18 +89,7 @@ const EuiDataGridCellContent: FunctionComponent< let cellContent = (
{ - setCellContentsRef(el); - setPopoverAnchorRef.current = - cellHeightType === 'default' - ? // Default height cells need the popover to be anchored on the wrapper, - // in order for the popover to centered on the full cell width (as content - // width is affected by the width of cell actions) - (el?.parentElement as HTMLDivElement) - : // Numerical height cells need the popover anchor to be below the wrapper - // class, in order to set height: 100% on the portalled popover div wrappers - el; - }} + ref={setCellContentsRef} data-datagrid-cellcontent className={classes} > @@ -146,11 +130,11 @@ const EuiDataGridCellContent: FunctionComponent< ); return ( -
+ <> {cellContent} {screenReaderText} {cellActions} -
+ ); } ); @@ -180,6 +164,7 @@ export class EuiDataGridCell extends Component< isEntered: false, enableInteractions: false, disableCellTabIndex: false, + cellTextAlign: 'Left', }; unsubscribeCell?: Function; focusTimeout: number | undefined; @@ -445,6 +430,7 @@ export class EuiDataGridCell extends Component< this.contentObserver.disconnect(); } this.preventTabbing(); + this.setCellTextAlign(); }; onFocus = (e: FocusEvent) => { @@ -503,6 +489,29 @@ export class EuiDataGridCell extends Component< } }; + setCellTextAlign = () => { + if (this.cellContentsRef) { + const { columnType } = this.props; + if (!columnType) { + // If no schema was set, this is likely a left aligned column + this.setState({ cellTextAlign: 'Left' }); + } else if (columnType === 'numeric' || columnType === 'currency') { + // Default EUI schemas that we know set right text align + this.setState({ cellTextAlign: 'Right' }); + } else { + // If the consumer is using a custom schema, it may have custom text alignment + const textAlign = window + .getComputedStyle(this.cellContentsRef) + .getPropertyValue('text-align'); + + this.setState({ + cellTextAlign: + textAlign === 'right' || textAlign === 'end' ? 'Right' : 'Left', + }); + } + } + }; + isExpandable = () => { // A cell must always show an expansion popover if it has cell actions, // otherwise keyboard and screen reader users have no way of accessing them @@ -526,12 +535,17 @@ export class EuiDataGridCell extends Component< handleCellPopover = () => { if (this.isPopoverOpen()) { - const { setPopoverAnchor, setPopoverContent, setCellPopoverProps } = - this.props.popoverContext; + const { + setPopoverAnchor, + setPopoverAnchorPosition, + setPopoverContent, + setCellPopoverProps, + } = this.props.popoverContext; // Set popover anchor const cellAnchorEl = this.popoverAnchorRef.current!; setPopoverAnchor(cellAnchorEl); + setPopoverAnchorPosition(`down${this.state.cellTextAlign}`); // Set popover contents with cell content const { @@ -603,6 +617,7 @@ export class EuiDataGridCell extends Component< const cellClasses = classNames( 'euiDataGridRowCell', + `euiDataGridRowCell--align${this.state.cellTextAlign}`, { [`euiDataGridRowCell--${columnType}`]: columnType, 'euiDataGridRowCell--open': popoverIsOpen, @@ -697,21 +712,17 @@ export class EuiDataGridCell extends Component< rowIndex, rowHeightsOptions ); - const cellHeightType = - rowHeightUtils?.getHeightType(rowHeight) || 'default'; const cellContentProps = { ...rest, setCellProps: this.setCellProps, column, columnType, - cellHeightType, isExpandable, isExpanded: popoverIsOpen, isDetails: false, isFocused: this.state.isFocused, setCellContentsRef: this.setCellContentsRef, - setPopoverAnchorRef: this.popoverAnchorRef, rowHeight, rowHeightUtils, isControlColumn: cellClasses.includes( @@ -721,19 +732,27 @@ export class EuiDataGridCell extends Component< }; const cellActions = showCellActions && ( - { - if (popoverIsOpen) { - closeCellPopover(); - } else { - openCellPopover({ rowIndex: visibleRowIndex, colIndex }); - } - }} - /> + <> + { + if (popoverIsOpen) { + closeCellPopover(); + } else { + openCellPopover({ rowIndex: visibleRowIndex, colIndex }); + } + }} + /> + {/* Give the cell expansion popover a separate div/ref - otherwise the + extra popover wrappers mess up the absolute positioning and cause + animation stuttering */} +
+ ); const cellContent = isExpandable ? ( diff --git a/src/components/datagrid/body/data_grid_cell_actions.test.tsx b/src/components/datagrid/body/data_grid_cell_actions.test.tsx index 151f3003814..6ab8a4f3e06 100644 --- a/src/components/datagrid/body/data_grid_cell_actions.test.tsx +++ b/src/components/datagrid/body/data_grid_cell_actions.test.tsx @@ -8,7 +8,6 @@ import React from 'react'; import { shallow } from 'enzyme'; -import { render } from '../../../test/rtl'; import { EuiDataGridColumnCellAction } from '../data_grid_types'; import { @@ -49,11 +48,11 @@ describe('EuiDataGridCellActions', () => { expect(button('expandButtonTitle')).toMatchInlineSnapshot(` { `); }); @@ -111,17 +113,6 @@ describe('EuiDataGridCellActions', () => { `); }); - it('renders with overlay positioning for non default height cells', () => { - const { container } = render( - - ); - - // TODO: Switch to `.toHaveStyle({ position: 'absolute' })` once on Emotion - expect(container.firstChild).toHaveClass( - 'euiDataGridRowCell__actions--overlay' - ); - }); - describe('visible cell actions limit', () => { it('by default, does not render more than the first two primary cell actions', () => { const component = shallow( diff --git a/src/components/datagrid/body/data_grid_cell_actions.tsx b/src/components/datagrid/body/data_grid_cell_actions.tsx index bc3ed204dfe..9c2f231e9e8 100644 --- a/src/components/datagrid/body/data_grid_cell_actions.tsx +++ b/src/components/datagrid/body/data_grid_cell_actions.tsx @@ -18,20 +18,17 @@ import { EuiButtonIcon, EuiButtonIconProps } from '../../button/button_icon'; import { EuiButtonEmpty, EuiButtonEmptyProps } from '../../button/button_empty'; import { EuiFlexGroup, EuiFlexItem } from '../../flex'; import { EuiPopoverFooter } from '../../popover'; -import classNames from 'classnames'; export const EuiDataGridCellActions = ({ onExpandClick, column, rowIndex, colIndex, - cellHeightType, }: { onExpandClick: () => void; column?: EuiDataGridColumn; rowIndex: number; colIndex: number; - cellHeightType: string; }) => { // Note: The cell expand button/expansion popover is *always* rendered if // column.cellActions is present (regardless of column.isExpandable). @@ -45,11 +42,11 @@ export const EuiDataGridCellActions = ({ > {(expandButtonTitle: string) => ( ); @@ -94,11 +95,11 @@ export const EuiDataGridCellActions = ({ ); }, [column, colIndex, rowIndex]); - const classes = classNames('euiDataGridRowCell__actions', { - 'euiDataGridRowCell__actions--overlay': cellHeightType !== 'default', - }); - - return
{[...additionalButtons, expandButton]}
; + return ( +
+ {[...additionalButtons, expandButton]} +
+ ); }; export const EuiDataGridCellPopoverActions = ({ diff --git a/src/components/datagrid/body/data_grid_cell_popover.spec.tsx b/src/components/datagrid/body/data_grid_cell_popover.spec.tsx index 0c4dd05fb04..657e542661a 100644 --- a/src/components/datagrid/body/data_grid_cell_popover.spec.tsx +++ b/src/components/datagrid/body/data_grid_cell_popover.spec.tsx @@ -16,12 +16,12 @@ import { EuiDataGrid, EuiDataGridProps } from '../'; const baseProps: EuiDataGridProps = { 'aria-label': 'Grid cell popover test', height: 300, - width: 300, - columns: [{ id: 'A' }, { id: 'B' }], + width: 400, + columns: [{ id: 'A' }, { id: 'B' }, { id: 'C', schema: 'numeric' }], rowCount: 2, renderCellValue: ({ rowIndex, columnId }) => `${columnId}, ${rowIndex}`, columnVisibility: { - visibleColumns: ['A', 'B'], + visibleColumns: ['A', 'B', 'C'], setVisibleColumns: () => {}, }, }; @@ -53,7 +53,7 @@ describe('EuiDataGridCellPopover', () => { '[data-gridcell-row-index="0"][data-gridcell-column-index="0"]' ).realClick(); - cy.get('[data-test-subj="euiDataGridCellExpandButton"]').realClick(); + cy.get('[data-test-subj="euiDataGridCellExpandButton"]').click(); cy.focused().should( 'have.attr', 'data-test-subj', @@ -73,7 +73,7 @@ describe('EuiDataGridCellPopover', () => { '[data-gridcell-row-index="1"][data-gridcell-column-index="1"]' ).realClick(); - cy.get('[data-test-subj="euiDataGridCellExpandButton"]').realClick(); + cy.get('[data-test-subj="euiDataGridCellExpandButton"]').click(); cy.focused().should( 'have.attr', 'data-test-subj', @@ -93,12 +93,12 @@ describe('EuiDataGridCellPopover', () => { '[data-gridcell-row-index="0"][data-gridcell-column-index="0"]' ).realClick(); - cy.get('[data-test-subj="euiDataGridCellExpandButton"]').realClick(); + cy.get('[data-test-subj="euiDataGridCellExpandButton"]').click(); cy.get('[data-test-subj="euiDataGridExpansionPopover"]').should('exist'); cy.get( '[data-gridcell-row-index="0"][data-gridcell-column-index="0"]' - ).realClick(); + ).realClick({ position: 'right' }); cy.get('[data-test-subj="euiDataGridExpansionPopover"]').should( 'not.exist' ); @@ -126,7 +126,7 @@ describe('EuiDataGridCellPopover', () => { cy.get( '[data-gridcell-row-index="0"][data-gridcell-column-index="0"]' ).realClick(); - cy.get('[data-test-subj="euiDataGridCellExpandButton"]').realClick(); + cy.get('[data-test-subj="euiDataGridCellExpandButton"]').click(); cy.get('.euiDataGridRowCell__popover.hello.world').should('exist'); }); @@ -136,10 +136,13 @@ describe('EuiDataGridCellPopover', () => { ...baseProps, rowCount: 1, renderCellValue: ({ columnId }) => { - if (columnId === 'A') { - return 'short text'; - } else { - return 'Very long text that should get cut off because it is so long'; + switch (columnId) { + case 'A': + return 'short text'; + case 'B': + return 'Very long text that should get cut off because it is so long, lorem ipsum dolor sit amet words words words'; + case 'C': + return 'right aligned text'; } }, }; @@ -147,65 +150,81 @@ describe('EuiDataGridCellPopover', () => { const openCellPopover = (id: string) => { cy.get( `[data-gridcell-row-index="0"][data-gridcell-column-id="${id}"]` - ).realClick(); + ).click(); cy.realPress('Enter'); }; - it('default row height', () => { + it('small popover', () => { cy.realMount(); - openCellPopover('B'); + openCellPopover('A'); cy.get('[data-test-subj="euiDataGridExpansionPopover"]') - .should('have.css', 'left', '24.5px') - .should('have.css', 'top') - .and('match', /^(104|103)px/); // CI is off by 1 px + .should('have.css', 'left', '1px') + .should('have.css', 'top', '73px') + .should('have.css', 'width', '112px'); }); - it('lineCount row height', () => { - cy.realMount( - - ); - openCellPopover('B'); - - cy.get('[data-test-subj="euiDataGridExpansionPopover"]') - .should('have.css', 'left', '24.5px') - .should('have.css', 'top') - .and('match', /^(127|126)px/); // CI is off by 1 px - }); + it('large popover', () => { + cy.realMount(); - it('numerical row height', () => { - cy.realMount( - - ); openCellPopover('B'); - - // Should not be anchored to the bottom of the overflowing text cy.get('[data-test-subj="euiDataGridExpansionPopover"]') - .should('have.css', 'left', '24.5px') - .should('have.css', 'top') - .and('match', /^(106|105)px/); // CI is off by 1 px + .should('have.css', 'left', '109px') + .should('have.css', 'top', '73px') + .should('have.css', 'width', '375px'); }); - it('auto row height', () => { - cy.realMount( - - ); + it('right aligned popover', () => { + cy.realMount(); - openCellPopover('B'); - cy.get('[data-test-subj="euiDataGridExpansionPopover"]') - .should('have.css', 'left', '24.5px') - .should('have.css', 'top') - .and('match', /^(151|150)px/); // CI is off by 1 px + openCellPopover('C'); - // The shorter cell content should not have the same top position - openCellPopover('A'); + // Matchers used due to subpixel rendering shenanigans + cy.get('[data-test-subj="euiDataGridExpansionPopover"]') + .should('have.css', 'top', '73px') + .should('have.css', 'left') + .and('match', /^254[.\d]+px$/); cy.get('[data-test-subj="euiDataGridExpansionPopover"]') - .should('have.css', 'left', '19px') - .should('have.css', 'top') - .and('match', /^(103|102)px/); // CI is off by 1 px + .should('have.css', 'width') + .and('match', /^144[.\d]+px$/); + }); + + describe('max popover dimensions', () => { + it('never exceeds 75% of the viewport width or 50% of the viewport height', () => { + cy.viewport(300, 200); + cy.realMount(); + + openCellPopover('B'); + cy.get('[data-test-subj="euiDataGridExpansionPopover"]') + .should('have.css', 'width', '225px') // 300 * .75 + .should('have.css', 'height', '100px'); // 200 * .5 + }); + + it('does not exceed 400px width if the column width is smaller than 400px', () => { + cy.viewport(1000, 500); + cy.realMount(); + + openCellPopover('B'); + cy.get('[data-test-subj="euiDataGridExpansionPopover"]') + .should('have.css', 'width', '400px') + .should('have.css', 'height', '88px'); + }); + + it('matches the width of the column if the column width is larger than 400px', () => { + cy.viewport(1000, 500); + cy.realMount( + + ); + + openCellPopover('B'); + cy.get('[data-test-subj="euiDataGridExpansionPopover"]') + .should('have.css', 'width', '500px') + .should('have.css', 'height', '64px'); + }); }); }); }); diff --git a/src/components/datagrid/body/data_grid_cell_popover.test.tsx b/src/components/datagrid/body/data_grid_cell_popover.test.tsx index f9272409669..2db3d3970fa 100644 --- a/src/components/datagrid/body/data_grid_cell_popover.test.tsx +++ b/src/components/datagrid/body/data_grid_cell_popover.test.tsx @@ -31,6 +31,9 @@ describe('useCellPopover', () => { }); it('does nothing if called again on a popover that is already open', () => { + const mockAnchor = document.createElement('div'); + document.body.appendChild(mockAnchor); + const { result } = renderHook(useCellPopover); expect(result.current.cellPopover).toBeFalsy(); @@ -39,9 +42,7 @@ describe('useCellPopover', () => { rowIndex: 0, colIndex: 0, }); - result.current.cellPopoverContext.setPopoverAnchor( - document.createElement('div') - ); + result.current.cellPopoverContext.setPopoverAnchor(mockAnchor); }); expect(result.current.cellPopover).not.toBeFalsy(); @@ -94,9 +95,15 @@ describe('useCellPopover', () => { populateCellPopover(result.current.cellPopoverContext); expect(result.current.cellPopover).toMatchInlineSnapshot(` } closePopover={[Function]} display="block" + focusTrapProps={ + Object { + "onClickOutside": [Function], + } + } hasArrow={false} isOpen={true} onKeyDown={[Function]} @@ -107,6 +114,13 @@ describe('useCellPopover', () => { "data-test-subj": "euiDataGridExpansionPopover", } } + panelStyle={ + Object { + "maxBlockSize": "50vh", + "maxInlineSize": "min(75vw, max(0px, 400px))", + } + } + repositionToCrossAxis={false} >
{}, closeCellPopover: () => {}, setPopoverAnchor: () => {}, + setPopoverAnchorPosition: () => {}, setPopoverContent: () => {}, setCellPopoverProps: () => {}, }); @@ -39,6 +40,9 @@ export const useCellPopover = (): { }); // Popover anchor & content are passed by individual `EuiDataGridCell`s const [popoverAnchor, setPopoverAnchor] = useState(null); + const [popoverAnchorPosition, setPopoverAnchorPosition] = useState< + 'downLeft' | 'downRight' + >('downLeft'); const [popoverContent, setPopoverContent] = useState(); // Allow customization of most (not all) popover props by consumers const [cellPopoverProps, setCellPopoverProps] = useState< @@ -74,10 +78,25 @@ export const useCellPopover = (): { openCellPopover, cellLocation, setPopoverAnchor, + setPopoverAnchorPosition, setPopoverContent, setCellPopoverProps, }; + // Override the default EuiPopover `onClickOutside` behavior, since the toggling + // popover button isn't actually the DOM node we pass to `button`. Otherwise, + // clicking the expansion cell action triggers an outside click + const onClickOutside = useCallback( + (event: Event) => { + if (!popoverAnchor) return; + const cellActions = popoverAnchor.previousElementSibling; + if (cellActions?.contains(event.target as Node) === false) { + closeCellPopover(); + } + }, + [popoverAnchor, closeCellPopover] + ); + // Note that this popover is rendered once at the top grid level, rather than one popover per cell const cellPopover = popoverIsOpen && popoverAnchor && ( { if (event.key === keys.F2 || event.key === keys.ESCAPE) { event.preventDefault(); diff --git a/src/components/datagrid/body/footer/data_grid_footer_row.test.tsx b/src/components/datagrid/body/footer/data_grid_footer_row.test.tsx index 92e93e3acea..8eae820cf20 100644 --- a/src/components/datagrid/body/footer/data_grid_footer_row.test.tsx +++ b/src/components/datagrid/body/footer/data_grid_footer_row.test.tsx @@ -52,6 +52,7 @@ describe('EuiDataGridFooterRow', () => { "popoverIsOpen": false, "setCellPopoverProps": [Function], "setPopoverAnchor": [Function], + "setPopoverAnchorPosition": [Function], "setPopoverContent": [Function], } } @@ -79,6 +80,7 @@ describe('EuiDataGridFooterRow', () => { "popoverIsOpen": false, "setCellPopoverProps": [Function], "setPopoverAnchor": [Function], + "setPopoverAnchorPosition": [Function], "setPopoverContent": [Function], } } @@ -132,6 +134,7 @@ describe('EuiDataGridFooterRow', () => { "popoverIsOpen": false, "setCellPopoverProps": [Function], "setPopoverAnchor": [Function], + "setPopoverAnchorPosition": [Function], "setPopoverContent": [Function], } } @@ -184,6 +187,7 @@ describe('EuiDataGridFooterRow', () => { "popoverIsOpen": false, "setCellPopoverProps": [Function], "setPopoverAnchor": [Function], + "setPopoverAnchorPosition": [Function], "setPopoverContent": [Function], } } diff --git a/src/components/datagrid/body/header/_data_grid_header_row.scss b/src/components/datagrid/body/header/_data_grid_header_row.scss index 8adaf9c4289..c17b5293aed 100644 --- a/src/components/datagrid/body/header/_data_grid_header_row.scss +++ b/src/components/datagrid/body/header/_data_grid_header_row.scss @@ -1,6 +1,6 @@ .euiDataGridHeader { display: flex; - z-index: 3; // Needs to sit above the content and focused cells + z-index: $euiZDataGridCellPopover - 1; // Needs to sit above the content and focused cells background: $euiColorEmptyShade; position: sticky; top: 0; diff --git a/src/components/datagrid/data_grid.test.tsx b/src/components/datagrid/data_grid.test.tsx index fac6ca20d3f..337c1a083e6 100644 --- a/src/components/datagrid/data_grid.test.tsx +++ b/src/components/datagrid/data_grid.test.tsx @@ -537,7 +537,7 @@ describe('EuiDataGrid', () => { Array [ Object { "aria-rowindex": 1, - "className": "euiDataGridRowCell euiDataGridRowCell--firstColumn customClass", + "className": "euiDataGridRowCell euiDataGridRowCell--alignLeft euiDataGridRowCell--firstColumn customClass", "data-gridcell-column-id": "A", "data-gridcell-column-index": 0, "data-gridcell-row-index": 0, @@ -563,7 +563,7 @@ describe('EuiDataGrid', () => { }, Object { "aria-rowindex": 1, - "className": "euiDataGridRowCell euiDataGridRowCell--lastColumn customClass", + "className": "euiDataGridRowCell euiDataGridRowCell--alignLeft euiDataGridRowCell--lastColumn customClass", "data-gridcell-column-id": "B", "data-gridcell-column-index": 1, "data-gridcell-row-index": 0, @@ -589,7 +589,7 @@ describe('EuiDataGrid', () => { }, Object { "aria-rowindex": 2, - "className": "euiDataGridRowCell euiDataGridRowCell--firstColumn customClass", + "className": "euiDataGridRowCell euiDataGridRowCell--alignLeft euiDataGridRowCell--firstColumn customClass", "data-gridcell-column-id": "A", "data-gridcell-column-index": 0, "data-gridcell-row-index": 1, @@ -615,7 +615,7 @@ describe('EuiDataGrid', () => { }, Object { "aria-rowindex": 2, - "className": "euiDataGridRowCell euiDataGridRowCell--lastColumn customClass", + "className": "euiDataGridRowCell euiDataGridRowCell--alignLeft euiDataGridRowCell--lastColumn customClass", "data-gridcell-column-id": "B", "data-gridcell-column-index": 1, "data-gridcell-row-index": 1, @@ -778,17 +778,17 @@ describe('EuiDataGrid', () => { expect(gridCellClassNames).toMatchInlineSnapshot(` Array [ "euiDataGridRowCell--firstColumn", - "euiDataGridRowCell euiDataGridRowCell--numeric euiDataGridRowCell--firstColumn", + "euiDataGridRowCell euiDataGridRowCell--alignRight euiDataGridRowCell--numeric euiDataGridRowCell--firstColumn", "euiDataGridRowCell--lastColumn", - "euiDataGridRowCell euiDataGridRowCell--customFormatName euiDataGridRowCell--lastColumn", + "euiDataGridRowCell euiDataGridRowCell--alignLeft euiDataGridRowCell--customFormatName euiDataGridRowCell--lastColumn", "euiDataGridRowCell--firstColumn", - "euiDataGridRowCell euiDataGridRowCell--numeric euiDataGridRowCell--firstColumn", + "euiDataGridRowCell euiDataGridRowCell--alignRight euiDataGridRowCell--numeric euiDataGridRowCell--firstColumn", "euiDataGridRowCell--lastColumn", - "euiDataGridRowCell euiDataGridRowCell--customFormatName euiDataGridRowCell--lastColumn", + "euiDataGridRowCell euiDataGridRowCell--alignLeft euiDataGridRowCell--customFormatName euiDataGridRowCell--lastColumn", "euiDataGridRowCell--firstColumn", - "euiDataGridRowCell euiDataGridRowCell--numeric euiDataGridRowCell--firstColumn", + "euiDataGridRowCell euiDataGridRowCell--alignRight euiDataGridRowCell--numeric euiDataGridRowCell--firstColumn", "euiDataGridRowCell--lastColumn", - "euiDataGridRowCell euiDataGridRowCell--customFormatName euiDataGridRowCell--lastColumn", + "euiDataGridRowCell euiDataGridRowCell--alignLeft euiDataGridRowCell--customFormatName euiDataGridRowCell--lastColumn", ] `); }); @@ -821,12 +821,12 @@ describe('EuiDataGrid', () => { .map((x) => x.props().className); expect(gridCellClassNames).toMatchInlineSnapshot(` Array [ - "euiDataGridRowCell euiDataGridRowCell--numeric euiDataGridRowCell--firstColumn", - "euiDataGridRowCell euiDataGridRowCell--boolean", - "euiDataGridRowCell euiDataGridRowCell--lastColumn", - "euiDataGridRowCell euiDataGridRowCell--numeric euiDataGridRowCell--firstColumn", - "euiDataGridRowCell euiDataGridRowCell--boolean", - "euiDataGridRowCell euiDataGridRowCell--lastColumn", + "euiDataGridRowCell euiDataGridRowCell--alignLeft euiDataGridRowCell--numeric euiDataGridRowCell--firstColumn", + "euiDataGridRowCell euiDataGridRowCell--alignLeft euiDataGridRowCell--boolean", + "euiDataGridRowCell euiDataGridRowCell--alignLeft euiDataGridRowCell--lastColumn", + "euiDataGridRowCell euiDataGridRowCell--alignLeft euiDataGridRowCell--numeric euiDataGridRowCell--firstColumn", + "euiDataGridRowCell euiDataGridRowCell--alignLeft euiDataGridRowCell--boolean", + "euiDataGridRowCell euiDataGridRowCell--alignLeft euiDataGridRowCell--lastColumn", ] `); }); @@ -853,10 +853,10 @@ describe('EuiDataGrid', () => { .map((x) => x.props().className); expect(gridCellClassNames).toMatchInlineSnapshot(` Array [ - "euiDataGridRowCell euiDataGridRowCell--numeric euiDataGridRowCell--firstColumn", - "euiDataGridRowCell euiDataGridRowCell--alphanumeric euiDataGridRowCell--lastColumn", - "euiDataGridRowCell euiDataGridRowCell--numeric euiDataGridRowCell--firstColumn", - "euiDataGridRowCell euiDataGridRowCell--alphanumeric euiDataGridRowCell--lastColumn", + "euiDataGridRowCell euiDataGridRowCell--alignLeft euiDataGridRowCell--numeric euiDataGridRowCell--firstColumn", + "euiDataGridRowCell euiDataGridRowCell--alignLeft euiDataGridRowCell--alphanumeric euiDataGridRowCell--lastColumn", + "euiDataGridRowCell euiDataGridRowCell--alignLeft euiDataGridRowCell--numeric euiDataGridRowCell--firstColumn", + "euiDataGridRowCell euiDataGridRowCell--alignLeft euiDataGridRowCell--alphanumeric euiDataGridRowCell--lastColumn", ] `); }); @@ -890,13 +890,13 @@ describe('EuiDataGrid', () => { .map((x) => x.props().className); expect(gridCellClassNames).toMatchInlineSnapshot(` Array [ - "euiDataGridRowCell euiDataGridRowCell--numeric euiDataGridRowCell--firstColumn", - "euiDataGridRowCell euiDataGridRowCell--boolean", - "euiDataGridRowCell euiDataGridRowCell--currency", - "euiDataGridRowCell euiDataGridRowCell--datetime", - "euiDataGridRowCell euiDataGridRowCell--datetime", - "euiDataGridRowCell euiDataGridRowCell--datetime", - "euiDataGridRowCell euiDataGridRowCell--datetime euiDataGridRowCell--lastColumn", + "euiDataGridRowCell euiDataGridRowCell--alignLeft euiDataGridRowCell--numeric euiDataGridRowCell--firstColumn", + "euiDataGridRowCell euiDataGridRowCell--alignLeft euiDataGridRowCell--boolean", + "euiDataGridRowCell euiDataGridRowCell--alignLeft euiDataGridRowCell--currency", + "euiDataGridRowCell euiDataGridRowCell--alignLeft euiDataGridRowCell--datetime", + "euiDataGridRowCell euiDataGridRowCell--alignLeft euiDataGridRowCell--datetime", + "euiDataGridRowCell euiDataGridRowCell--alignLeft euiDataGridRowCell--datetime", + "euiDataGridRowCell euiDataGridRowCell--alignLeft euiDataGridRowCell--datetime euiDataGridRowCell--lastColumn", ] `); }); @@ -939,8 +939,8 @@ describe('EuiDataGrid', () => { .map((x) => x.props().className); expect(gridCellClassNames).toMatchInlineSnapshot(` Array [ - "euiDataGridRowCell euiDataGridRowCell--numeric euiDataGridRowCell--firstColumn", - "euiDataGridRowCell euiDataGridRowCell--ipaddress euiDataGridRowCell--lastColumn", + "euiDataGridRowCell euiDataGridRowCell--alignLeft euiDataGridRowCell--numeric euiDataGridRowCell--firstColumn", + "euiDataGridRowCell euiDataGridRowCell--alignLeft euiDataGridRowCell--ipaddress euiDataGridRowCell--lastColumn", ] `); }); diff --git a/src/components/datagrid/data_grid_types.ts b/src/components/datagrid/data_grid_types.ts index 3935a7c47b8..07f891a05f2 100644 --- a/src/components/datagrid/data_grid_types.ts +++ b/src/components/datagrid/data_grid_types.ts @@ -222,6 +222,7 @@ export interface DataGridCellPopoverContextShape { openCellPopover(args: { rowIndex: number; colIndex: number }): void; closeCellPopover(): void; setPopoverAnchor(anchor: HTMLElement): void; + setPopoverAnchorPosition(position: 'downLeft' | 'downRight'): void; setPopoverContent(content: ReactNode): void; setCellPopoverProps: EuiDataGridCellPopoverElementProps['setCellPopoverProps']; } @@ -626,6 +627,7 @@ export interface EuiDataGridCellState { isEntered: boolean; // enables focus trap for non-expandable cells with multiple interactive elements enableInteractions: boolean; // cell got hovered at least once, so cell button and popover interactions are rendered disableCellTabIndex: boolean; // disables tabIndex on the wrapping cell, used for focus management of a single interactive child + cellTextAlign: 'Left' | 'Right'; // determines the cell actions and cell popover expansion position } export type EuiDataGridCellValueProps = Omit< @@ -773,9 +775,14 @@ export interface EuiDataGridColumnCellActionProps { */ columnId: string; /** - * React component representing the action displayed in the cell + * React component representing the action displayed in the cell. + * + * On cell hover/focus, an EuiButtonIcon will be displayed that cannot + * have its size or color customized, only its icon. + * + * On cell expand, an EuiButtonEmpty will be displayed in the cell popover + * that can have any sizing, color, or text. */ - // Component: ComponentType; Component: typeof EuiButtonEmpty | typeof EuiButtonIcon; /** * Determines whether the cell's action is displayed expanded (in the Popover) diff --git a/src/components/datagrid/utils/row_heights.test.ts b/src/components/datagrid/utils/row_heights.test.ts index a7b8c832a73..17c542a8212 100644 --- a/src/components/datagrid/utils/row_heights.test.ts +++ b/src/components/datagrid/utils/row_heights.test.ts @@ -296,17 +296,16 @@ describe('RowHeightUtils', () => { rowHeightUtils.setRowHeight(5, 'd', undefined, 0); // @ts-ignore this var is private, but we're inspecting it for the sake of the unit test - expect(rowHeightUtils.heightsCache.get(5)?.get('a')).toEqual(62); // @ts-ignore-line - expect(rowHeightUtils.heightsCache.get(5)?.get('b')).toEqual(46); // @ts-ignore-line - expect(rowHeightUtils.heightsCache.get(5)?.get('c')).toEqual(112); // @ts-ignore-line - expect(rowHeightUtils.heightsCache.get(5)?.get('d')).toEqual(46); // Falls back default row height - // NB: The cached heights have padding added to them + expect(rowHeightUtils.heightsCache.get(5)?.get('a')).toEqual(50); // @ts-ignore-line + expect(rowHeightUtils.heightsCache.get(5)?.get('b')).toEqual(34); // @ts-ignore-line + expect(rowHeightUtils.heightsCache.get(5)?.get('c')).toEqual(100); // @ts-ignore-line + expect(rowHeightUtils.heightsCache.get(5)?.get('d')).toEqual(34); // Falls back default row height }); }); describe('getRowHeight', () => { it('returns the highest height value stored for the specificed row', () => { - expect(rowHeightUtils.getRowHeight(5)).toEqual(112); // 100 + cell padding + expect(rowHeightUtils.getRowHeight(5)).toEqual(100); }); it('returns 0 if the passed row does not have any existing heights', () => { @@ -320,7 +319,7 @@ describe('RowHeightUtils', () => { { id: 'a' }, { id: 'b' }, ]); - expect(rowHeightUtils.getRowHeight(5)).toEqual(62); + expect(rowHeightUtils.getRowHeight(5)).toEqual(50); expect(didModify).toEqual(true); }); @@ -665,8 +664,8 @@ describe('useRowHeightUtils', () => { expect(result.current.heightsCache).toMatchInlineSnapshot(` Map { 0 => Map { - "A" => 42, - "B" => 62, + "A" => 30, + "B" => 50, }, } `); @@ -677,7 +676,7 @@ describe('useRowHeightUtils', () => { expect(result.current.heightsCache).toMatchInlineSnapshot(` Map { 0 => Map { - "A" => 42, + "A" => 30, }, } `); diff --git a/src/components/datagrid/utils/row_heights.ts b/src/components/datagrid/utils/row_heights.ts index 0c82650e390..caee8be8fa0 100644 --- a/src/components/datagrid/utils/row_heights.ts +++ b/src/components/datagrid/utils/row_heights.ts @@ -186,9 +186,7 @@ export class RowHeightUtils { ) { const rowHeights = this.heightsCache.get(rowIndex) || new Map(); - const adaptedHeight = Math.ceil( - height + this.styles.paddingTop + this.styles.paddingBottom - ); + const adaptedHeight = Math.ceil(height); if (rowHeights.get(colId) === adaptedHeight) { return false; diff --git a/src/components/datagrid/utils/scrolling.spec.tsx b/src/components/datagrid/utils/scrolling.spec.tsx index 6c9c1cbb517..2d0762e101a 100644 --- a/src/components/datagrid/utils/scrolling.spec.tsx +++ b/src/components/datagrid/utils/scrolling.spec.tsx @@ -65,7 +65,7 @@ describe('useScroll', () => { it('handles setFocusedCell being called manually on cells out of view', () => { const ref = createRef(); - cy.mount(); + cy.realMount(); // Wait for the grid to finish rendering and pass back the ref cy.get('[data-test-subj="euiDataGridBody"]').then(() => { @@ -80,7 +80,7 @@ describe('useScroll', () => { describe('cell popover', () => { it('handles openCellPopover being called manually on cells out of view', () => { const ref = createRef(); - cy.mount(); + cy.realMount(); // Wait for the grid to finish rendering and pass back the ref cy.get('[data-test-subj="euiDataGridBody"]').then(() => { diff --git a/src/components/popover/popover.tsx b/src/components/popover/popover.tsx index 14f6e7ba37f..e5eab7f6f15 100644 --- a/src/components/popover/popover.tsx +++ b/src/components/popover/popover.tsx @@ -534,10 +534,9 @@ export class EuiPopover extends Component { : this.props.hasArrow ? 16 + offset : 8 + offset, - arrowConfig: { - arrowWidth: 24, - arrowBuffer: 10, - }, + arrowConfig: this.props.hasArrow + ? { arrowWidth: 24, arrowBuffer: 10 } + : { arrowWidth: 0, arrowBuffer: 0 }, returnBoundingBox: this.props.attachToAnchor, allowCrossAxis: this.props.repositionToCrossAxis, buffer: this.props.buffer, diff --git a/src/components/text_truncate/text_truncate.spec.tsx b/src/components/text_truncate/text_truncate.spec.tsx index 55e186ea52f..e321071bee0 100644 --- a/src/components/text_truncate/text_truncate.spec.tsx +++ b/src/components/text_truncate/text_truncate.spec.tsx @@ -21,19 +21,6 @@ describe('EuiTextTruncate', () => { width: 200, }; - // CI doesn't have access to the Inter font, so we need to manually include it - // for font calculations to work correctly - before(() => { - const linkElem = document.createElement('link'); - linkElem.setAttribute('rel', 'stylesheet'); - linkElem.setAttribute( - 'href', - 'https://fonts.googleapis.com/css2?family=Inter:wght@400&display=swap' - ); - document.head.appendChild(linkElem); - cy.wait(1000); // Wait a second to give the font time to load/swap in - }); - const getTruncatedText = (selector = '#text') => cy.get(`${selector} [data-test-subj="truncatedText"]`); diff --git a/src/components/text_truncate/utils.spec.tsx b/src/components/text_truncate/utils.spec.tsx index ed3cc3769c6..aa8c01f2b25 100644 --- a/src/components/text_truncate/utils.spec.tsx +++ b/src/components/text_truncate/utils.spec.tsx @@ -12,19 +12,7 @@ import { TruncationUtils } from './utils'; -// CI doesn't have access to the Inter font, so we need to manually include it -// for font calculations to work correctly const font = '14px Inter'; -before(() => { - const linkElem = document.createElement('link'); - linkElem.setAttribute('rel', 'stylesheet'); - linkElem.setAttribute( - 'href', - 'https://fonts.googleapis.com/css2?family=Inter:wght@400&display=swap' - ); - document.head.appendChild(linkElem); - cy.wait(1000); // Wait a second to give the font time to load/swap in -}); describe('Truncation utils', () => { const params = { From d11ffc918df2458feb677425829498d46dd8cd3b Mon Sep 17 00:00:00 2001 From: Julia Rechkunova Date: Tue, 21 Nov 2023 18:39:30 +0100 Subject: [PATCH 2/9] [EuiDataGrid] Redesign grid header (#7371) Co-authored-by: Cee Chen --- changelogs/upcoming/7371.md | 6 + .../__snapshots__/data_grid.test.tsx.snap | 104 +++++++++++------- .../data_grid_body_custom.test.tsx.snap | 26 +++-- .../data_grid_body_virtualized.test.tsx.snap | 26 +++-- .../data_grid_header_cell.test.tsx.snap | 13 ++- .../body/header/_data_grid_header_row.scss | 64 ++++++----- .../body/header/data_grid_header_cell.tsx | 41 +++---- 7 files changed, 169 insertions(+), 111 deletions(-) create mode 100644 changelogs/upcoming/7371.md diff --git a/changelogs/upcoming/7371.md b/changelogs/upcoming/7371.md new file mode 100644 index 00000000000..96724c7ec76 --- /dev/null +++ b/changelogs/upcoming/7371.md @@ -0,0 +1,6 @@ +- Updated `EuiDataGrid` column header cells to show the sort arrow after the heading text, instead of before +- Updated `EuiDataGrid`'s column header actions icon from a chevron to `boxesVertical` + +**Bug fixes** + +- Fixed `EuiDataGrid`'s numeric and currency column heading cells to be correctly right-aligned diff --git a/src/components/datagrid/__snapshots__/data_grid.test.tsx.snap b/src/components/datagrid/__snapshots__/data_grid.test.tsx.snap index cf9665dca56..e35c3781496 100644 --- a/src/components/datagrid/__snapshots__/data_grid.test.tsx.snap +++ b/src/components/datagrid/__snapshots__/data_grid.test.tsx.snap @@ -617,12 +617,15 @@ exports[`EuiDataGrid rendering renders additional toolbar controls 1`] = ` > A
- + > + +

B

- + > + +

A - + > + +

B - + > + +

Column A - + > + +

- + > + +

A - + > + +

B - + > + +

columnA - + > + +

columnB - + > + +

columnA - + > + +

columnB - + > + +

someColumn - + > + +

+

+ {display || displayAsText || id} +
+ {sortingArrow} + + ); + return ( - {sortingArrow} -
- {display || displayAsText || id} -
+ {cellContent} {sortingScreenReaderText && (

{sortingScreenReaderText}

@@ -144,20 +150,15 @@ export const EuiDataGridHeaderCell: FunctionComponent< }} aria-describedby={`${sortingAriaId} ${actionsAriaId}`} > - {sortingArrow} -
- {display || displayAsText || id} + {cellContent} +
+
- } isOpen={isPopoverOpen} From 19b15180e43a83c36b45641f26058c262ee72573 Mon Sep 17 00:00:00 2001 From: Julia Rechkunova Date: Wed, 22 Nov 2023 00:35:16 +0100 Subject: [PATCH 3/9] [EuiDataGrid] Badges in grid toolbar (#7369) Co-authored-by: Cee Chen <549407+cee-chen@users.noreply.github.com> Co-authored-by: Cee Chen --- changelogs/upcoming/7369.md | 3 + .../datagrid/toolbar/additional_controls.tsx | 6 +- .../toolbar/datagrid_toolbar_example.js | 30 +++++-- .../toolbar/render_custom_toolbar.tsx | 12 ++- .../_button_display_content.test.tsx | 10 +++ .../_button_display_content.tsx | 20 +++-- .../button/button_empty/button_empty.test.tsx | 10 +++ .../button/button_empty/button_empty.tsx | 10 ++- .../__snapshots__/data_grid.test.tsx.snap | 72 ++++++++++----- .../column_selector.test.tsx.snap | 39 +++++++-- .../column_sorting.test.tsx.snap | 39 +++++++-- .../data_grid_toolbar_control.test.tsx.snap | 48 ++++++++++ .../display_selector.test.tsx.snap | 1 - .../datagrid/controls/_data_grid_toolbar.scss | 5 -- .../controls/column_selector.test.tsx | 18 ++-- .../datagrid/controls/column_selector.tsx | 62 +++++-------- .../datagrid/controls/column_sorting.test.tsx | 2 +- .../datagrid/controls/column_sorting.tsx | 29 ++----- .../data_grid_toolbar_control.test.tsx | 65 ++++++++++++++ .../controls/data_grid_toolbar_control.tsx | 87 +++++++++++++++++++ .../datagrid/controls/display_selector.tsx | 1 - .../datagrid/controls/fullscreen_selector.tsx | 9 +- src/components/datagrid/controls/index.ts | 4 + src/components/datagrid/data_grid.test.tsx | 20 +++-- src/components/datagrid/index.ts | 1 + .../super_date_picker/super_update_button.tsx | 2 +- src/components/filter_group/filter_button.tsx | 4 +- 27 files changed, 451 insertions(+), 158 deletions(-) create mode 100644 changelogs/upcoming/7369.md create mode 100644 src/components/datagrid/controls/__snapshots__/data_grid_toolbar_control.test.tsx.snap create mode 100644 src/components/datagrid/controls/data_grid_toolbar_control.test.tsx create mode 100644 src/components/datagrid/controls/data_grid_toolbar_control.tsx diff --git a/changelogs/upcoming/7369.md b/changelogs/upcoming/7369.md new file mode 100644 index 00000000000..f23fc30080f --- /dev/null +++ b/changelogs/upcoming/7369.md @@ -0,0 +1,3 @@ +- Added a new `EuiDataGridToolbarControl` subcomponent, which is useful for rendering your own custom `EuiDataGrid` toolbar buttons while matching the look of the default controls +- Updated `EuiDataGrid`'s toolbar controls to show active/current counts in badges, and updated the Columns button icon +- Updated `EuiButtonEmpty` to allow passing `false` to `textProps`, which allows rendering custom button content without an extra text wrapper diff --git a/src-docs/src/views/datagrid/toolbar/additional_controls.tsx b/src-docs/src/views/datagrid/toolbar/additional_controls.tsx index d91c17d43d8..b45ba919abc 100644 --- a/src-docs/src/views/datagrid/toolbar/additional_controls.tsx +++ b/src-docs/src/views/datagrid/toolbar/additional_controls.tsx @@ -3,6 +3,7 @@ import { faker } from '@faker-js/faker'; import { EuiDataGrid, + EuiDataGridToolbarControl, EuiButtonEmpty, EuiButtonIcon, EuiLink, @@ -149,13 +150,12 @@ export default () => { setPopover((open) => !open)} > Download - + } isOpen={isPopoverOpen} closePopover={() => setPopover(false)} diff --git a/src-docs/src/views/datagrid/toolbar/datagrid_toolbar_example.js b/src-docs/src/views/datagrid/toolbar/datagrid_toolbar_example.js index 9998f094434..fe92a60e883 100644 --- a/src-docs/src/views/datagrid/toolbar/datagrid_toolbar_example.js +++ b/src-docs/src/views/datagrid/toolbar/datagrid_toolbar_example.js @@ -1,7 +1,11 @@ import React, { Fragment } from 'react'; import { GuideSectionTypes } from '../../../components'; -import { EuiCode } from '../../../../../src'; +import { + EuiDataGridToolbarControl, + EuiCode, + EuiCallOut, +} from '../../../../../src'; import DataGridToolbarVisibility from './visibility'; const dataGridToolbarVisibilitySource = require('!!raw-loader!./_grid'); @@ -174,7 +178,7 @@ export const DataGridToolbarExample = {

Although any node is allowed, the recommendation is to use{' '} - {''} for the + {''} for the left-side of the toolbar and{' '} {''} for the right-side of the toolbar. @@ -186,6 +190,7 @@ export const DataGridToolbarExample = { EuiDataGridToolBarVisibilityOptions, EuiDataGridToolBarAdditionalControlsOptions, EuiDataGridToolBarAdditionalControlsLeftOptions, + EuiDataGridToolbarControl, }, demo: , }, @@ -211,18 +216,27 @@ export const DataGridToolbarExample = { renderCustomToolbar should only be used when a very custom layout (e.g. moving default buttons between sides, interspering custom controls between default controls, custom - responsive behavior, etc.) is required. We would caution you to keep - consistency in mind also when customizing the toolbar: if using - multiple datagrid instances across your app, users will typically - want to reach for the same controls for each grid. Changing the - available controls inconsistently across your app may result in user - frustration. + responsive behavior, etc.) is required. For consistent visuals, we + recommend using the{' '} + {''} subcomponent + when rendering custom controls.

+ + If using multiple datagrid instances across your app, users will + typically want to reach for the same controls for each grid. + Changing the available controls inconsistently across your app may + result in user frustration. + ), demo: , props: { EuiDataGridCustomToolbarProps, + EuiDataGridToolbarControl, }, snippet: ` {hasRoomForGridControls && ( - + {}} + > Custom left side - + )} 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 5f275e457c7..1a6f7cbcace 100644 --- a/src/components/button/button_display/_button_display_content.test.tsx +++ b/src/components/button/button_display/_button_display_content.test.tsx @@ -98,6 +98,16 @@ describe('EuiButtonDisplayContent', () => { expect(container.querySelector('.eui-textTruncate')).toBeTruthy(); }); + it('does not render a text span wrapper if textProps is explicitly set to false', () => { + const { container } = render( + + Text + + ); + + expect(container.querySelector('.eui-textTruncate')).toBeFalsy(); + }); + it('does not render a text span wrapper if custom child with no textProps are passed', () => { const { getByTestSubject, container } = render( diff --git a/src/components/button/button_display/_button_display_content.tsx b/src/components/button/button_display/_button_display_content.tsx index 88ca51b1df4..48c50e59bed 100644 --- a/src/components/button/button_display/_button_display_content.tsx +++ b/src/components/button/button_display/_button_display_content.tsx @@ -37,13 +37,17 @@ export interface EuiButtonDisplayContentProps extends CommonProps { iconSide?: ButtonContentIconSide; isLoading?: boolean; /** - * Object of props passed to the wrapping the content's text/children only (not icon) + * Object of props passed to the `` wrapping the content's text/children only (not icon) + * + * This span wrapper can be removed by passing `textProps={false}`. */ - textProps?: HTMLAttributes & - CommonProps & { - ref?: Ref; - 'data-text'?: string; - }; + textProps?: + | (HTMLAttributes & + CommonProps & { + ref?: Ref; + 'data-text'?: string; + }) + | false; iconSize?: ButtonContentIconSize; isDisabled?: boolean; } @@ -90,11 +94,13 @@ export const EuiButtonDisplayContent: FunctionComponent< } const isText = typeof children === 'string'; + const doNotRenderTextWrapper = textProps === false; + const renderTextWrapper = (isText || textProps) && !doNotRenderTextWrapper; return ( {iconSide === 'left' && icon} - {isText || textProps ? ( + {renderTextWrapper ? ( { expect(container.firstChild).toMatchSnapshot(); }); + + it('does not render the text wrapper when textProps is set to false', () => { + const { container } = render( + Content + ); + + expect( + container.querySelector('.euiButtonEmpty__text') + ).not.toBeInTheDocument(); + }); }); }); diff --git a/src/components/button/button_empty/button_empty.tsx b/src/components/button/button_empty/button_empty.tsx index 960544f848f..12db368b473 100644 --- a/src/components/button/button_empty/button_empty.tsx +++ b/src/components/button/button_empty/button_empty.tsx @@ -72,7 +72,7 @@ export interface CommonEuiButtonEmptyProps type?: 'button' | 'submit'; buttonRef?: Ref; /** - * Object of props passed to the wrapping the button's content + * Object of props passed to the `` wrapping the button's content */ contentProps?: CommonProps & EuiButtonDisplayContentType; } @@ -139,7 +139,7 @@ export const EuiButtonEmpty: FunctionComponent = ({ const textClassNames = classNames( 'euiButtonEmpty__text', - textProps?.className + textProps && textProps.className ); const innerNode = ( @@ -149,7 +149,11 @@ export const EuiButtonEmpty: FunctionComponent = ({ iconType={iconType} iconSide={iconSide} iconSize={size === 'xs' ? 's' : iconSize} - textProps={{ ...textProps, className: textClassNames }} + textProps={ + textProps === false + ? false + : { ...textProps, className: textClassNames } + } {...{ ...contentProps, className: contentClassNames }} > {children} diff --git a/src/components/datagrid/__snapshots__/data_grid.test.tsx.snap b/src/components/datagrid/__snapshots__/data_grid.test.tsx.snap index e35c3781496..e0433fb8303 100644 --- a/src/components/datagrid/__snapshots__/data_grid.test.tsx.snap +++ b/src/components/datagrid/__snapshots__/data_grid.test.tsx.snap @@ -476,7 +476,7 @@ exports[`EuiDataGrid rendering renders additional toolbar controls 1`] = ` data-test-subj="dataGridColumnSelectorPopover" >
@@ -533,7 +540,7 @@ exports[`EuiDataGrid rendering renders additional toolbar controls 1`] = ` > @@ -952,7 +967,7 @@ exports[`EuiDataGrid rendering renders control columns 1`] = ` > @@ -1646,7 +1669,7 @@ exports[`EuiDataGrid rendering renders custom column headers 1`] = ` > @@ -2064,7 +2095,7 @@ exports[`EuiDataGrid rendering renders with common and div attributes 1`] = ` > @@ -291,7 +298,7 @@ exports[`useDataGridColumnSelector columnSelector [React 17] renders a toolbar b data-test-subj="dataGridColumnSelectorPopover" > @@ -576,7 +590,7 @@ exports[`useDataGridColumnSelector columnSelector [React 18] renders a toolbar b data-test-subj="dataGridColumnSelectorPopover" > diff --git a/src/components/datagrid/controls/__snapshots__/column_sorting.test.tsx.snap b/src/components/datagrid/controls/__snapshots__/column_sorting.test.tsx.snap index b562f7a2182..cd7eee17d4d 100644 --- a/src/components/datagrid/controls/__snapshots__/column_sorting.test.tsx.snap +++ b/src/components/datagrid/controls/__snapshots__/column_sorting.test.tsx.snap @@ -6,7 +6,7 @@ exports[`useDataGridColumnSorting columnSorting [React 16] renders a toolbar but data-test-subj="dataGridColumnSortingPopover" > @@ -269,7 +276,7 @@ exports[`useDataGridColumnSorting columnSorting [React 17] renders a toolbar but data-test-subj="dataGridColumnSortingPopover" > @@ -532,7 +546,7 @@ exports[`useDataGridColumnSorting columnSorting [React 18] renders a toolbar but data-test-subj="dataGridColumnSortingPopover" > diff --git a/src/components/datagrid/controls/__snapshots__/data_grid_toolbar_control.test.tsx.snap b/src/components/datagrid/controls/__snapshots__/data_grid_toolbar_control.test.tsx.snap new file mode 100644 index 00000000000..64dd49abd84 --- /dev/null +++ b/src/components/datagrid/controls/__snapshots__/data_grid_toolbar_control.test.tsx.snap @@ -0,0 +1,48 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`euiDataGridToolbarControl passes props to the underlying EuiButtonEmpty 1`] = ` + +`; + +exports[`euiDataGridToolbarControl renders with a badge 1`] = ` + +`; diff --git a/src/components/datagrid/controls/__snapshots__/display_selector.test.tsx.snap b/src/components/datagrid/controls/__snapshots__/display_selector.test.tsx.snap index d806098b349..a690d2e23c4 100644 --- a/src/components/datagrid/controls/__snapshots__/display_selector.test.tsx.snap +++ b/src/components/datagrid/controls/__snapshots__/display_selector.test.tsx.snap @@ -13,7 +13,6 @@ exports[`useDataGridDisplaySelector displaySelector renders a toolbar button/pop > { describe('column visibility', () => { const showColumnSelector = { allowHide: true, allowReorder: false }; + const getButtonText = (component: ReactWrapper) => { + return component.find('span.euiDataGridToolbarControl__text').text(); + }; + const getBadgeText = (component: ReactWrapper) => { + return component.find('span.euiDataGridToolbarControl__badge').text(); + }; + it('shows the number of columns hidden as the toolbar button text', () => { const component = mount( { /> ); - expect(component.text()).toEqual('2 columns hidden'); + expect(getButtonText(component)).toEqual('Columns'); + expect(getBadgeText(component)).toEqual('0/2'); }); it('toggles column visibility on switch interaction', () => { @@ -207,7 +215,7 @@ describe('useDataGridColumnSelector', () => { ).simulate('click'); forceUpdate(component); - expect(component.text()).toEqual('1 column hidden'); + expect(getBadgeText(component)).toEqual('1/2'); findTestSubject( component, @@ -215,7 +223,7 @@ describe('useDataGridColumnSelector', () => { ).simulate('click'); forceUpdate(component); - expect(component.text()).not.toEqual('1 column hidden'); + expect(getBadgeText(component)).toEqual('2'); }); it('toggles all column visibility with the show/hide all buttons', () => { @@ -230,7 +238,7 @@ describe('useDataGridColumnSelector', () => { ).simulate('click'); forceUpdate(component); - expect(component.text()).toEqual('2 columns hidden'); + expect(getBadgeText(component)).toEqual('0/2'); findTestSubject( component, @@ -238,7 +246,7 @@ describe('useDataGridColumnSelector', () => { ).simulate('click'); forceUpdate(component); - expect(component.text()).toEqual('Columns'); + expect(getBadgeText(component)).toEqual('2'); }); }); }); diff --git a/src/components/datagrid/controls/column_selector.tsx b/src/components/datagrid/controls/column_selector.tsx index 0218f1f1560..e61de0c29c8 100644 --- a/src/components/datagrid/controls/column_selector.tsx +++ b/src/components/datagrid/controls/column_selector.tsx @@ -36,6 +36,7 @@ import { EuiDataGridToolBarVisibilityOptions, } from '../data_grid_types'; import { getNestedObjectOptions } from './data_grid_toolbar'; +import { EuiDataGridToolbarControl } from './data_grid_toolbar_control'; export const useDataGridColumnSelector = ( availableColumns: EuiDataGridColumn[], @@ -101,10 +102,6 @@ export const useDataGridColumnSelector = ( const [columnSearchText, setColumnSearchText] = useState(''); - const controlBtnClasses = classNames('euiDataGrid__controlBtn', { - 'euiDataGrid__controlBtn--active': numberOfHiddenFields > 0, - }); - const filteredColumns = useMemo( () => sortedColumns.filter( @@ -122,27 +119,22 @@ export const useDataGridColumnSelector = ( 'Drag handle' ); - let buttonText = ( + const buttonText = ( ); - if (numberOfHiddenFields === 1) { - buttonText = ( - - ); - } else if (numberOfHiddenFields > 1) { - buttonText = ( - - ); - } + const orderedVisibleColumns = useMemo( + () => + visibleColumns + .map( + (columnId) => + availableColumns.find( + ({ id }) => id === columnId + ) as EuiDataGridColumn // cast to avoid `undefined`, it filters those out next + ) + .filter((column) => column != null), + [availableColumns, visibleColumns] + ); const columnSelector = allowColumnHiding || allowColumnReorder ? ( @@ -154,16 +146,18 @@ export const useDataGridColumnSelector = ( panelPaddingSize="s" hasDragDrop button={ - 0 + ? `${orderedVisibleColumns.length}/${availableColumns.length}` + : availableColumns.length + } + iconType="tableDensityNormal" data-test-subj="dataGridColumnSelectorButton" onClick={() => setIsOpen(!isOpen)} > {buttonText} - + } > {allowColumnHiding && ( @@ -313,18 +307,6 @@ export const useDataGridColumnSelector = ( ) : null; - const orderedVisibleColumns = useMemo( - () => - visibleColumns - .map( - (columnId) => - availableColumns.find( - ({ id }) => id === columnId - ) as EuiDataGridColumn // cast to avoid `undefined`, it filters those out next - ) - .filter((column) => column != null), - [availableColumns, visibleColumns] - ); /** * Used for moving columns left/right, available in the headers actions menu */ diff --git a/src/components/datagrid/controls/column_sorting.test.tsx b/src/components/datagrid/controls/column_sorting.test.tsx index 2ce2ea61c1f..47d40e36005 100644 --- a/src/components/datagrid/controls/column_sorting.test.tsx +++ b/src/components/datagrid/controls/column_sorting.test.tsx @@ -84,7 +84,7 @@ describe('useDataGridColumnSorting', () => { component.find('[data-popover-panel]').first().render() ).toMatchSnapshot(); closePopover(component); - expect(component.text()).toEqual('1 field sorted'); + expect(component.text()).toEqual('Sort fields1'); } ); diff --git a/src/components/datagrid/controls/column_sorting.tsx b/src/components/datagrid/controls/column_sorting.tsx index 714b96e4f51..e2beb1845cb 100644 --- a/src/components/datagrid/controls/column_sorting.tsx +++ b/src/components/datagrid/controls/column_sorting.tsx @@ -6,7 +6,6 @@ * Side Public License, v 1. */ -import classNames from 'classnames'; import React, { ReactNode, useEffect, useState } from 'react'; import { DropResult } from '@hello-pangea/dnd'; import { EuiButtonEmpty } from '../../button'; @@ -20,6 +19,7 @@ import { EuiI18n, useEuiI18n } from '../../i18n'; import { EuiPopover, EuiPopoverFooter } from '../../popover'; import { EuiText } from '../../text'; import { EuiToken } from '../../token'; +import { EuiDataGridToolbarControl } from './data_grid_toolbar_control'; import { EuiDataGridColumnSortingDraggable } from './column_sorting_draggable'; import { getDetailsForSchema } from '../utils/data_grid_schema'; import { @@ -63,17 +63,6 @@ export const useDataGridColumnSorting = ( 'Sort fields' ); - const sortingButtonTextActive = useEuiI18n( - 'euiColumnSorting.buttonActive', - ({ numberOfSortedFields }) => - `${numberOfSortedFields} field${ - numberOfSortedFields === 1 ? '' : 's' - } sorted`, - { - numberOfSortedFields: sorting != null ? sorting.columns.length : 0, - } - ); - if (sorting == null) return null; const activeColumnIds = new Set(sorting.columns.map(({ id }) => id)); @@ -110,10 +99,6 @@ export const useDataGridColumnSorting = ( } }; - const controlBtnClasses = classNames('euiDataGrid__controlBtn', { - 'euiDataGrid__controlBtn--active': sorting.columns.length > 0, - }); - const schemaDetails = (id: string | number) => schema.hasOwnProperty(id) && schema[id].columnType != null ? getDetailsForSchema(schemaDetectors, schema[id].columnType) @@ -143,18 +128,14 @@ export const useDataGridColumnSorting = ( panelPaddingSize="s" hasDragDrop button={ - setIsOpen(!isOpen)} > - {sorting.columns.length > 0 - ? sortingButtonTextActive - : sortingButtonText} - + {sortingButtonText} + } > {sorting.columns.length > 0 ? ( diff --git a/src/components/datagrid/controls/data_grid_toolbar_control.test.tsx b/src/components/datagrid/controls/data_grid_toolbar_control.test.tsx new file mode 100644 index 00000000000..8fab0d361f1 --- /dev/null +++ b/src/components/datagrid/controls/data_grid_toolbar_control.test.tsx @@ -0,0 +1,65 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import React from 'react'; +import { render } from '../../../test/rtl'; +import { shouldRenderCustomStyles } from '../../../test/internal'; +import { requiredProps } from '../../../test'; + +import { EuiDataGridToolbarControl } from './data_grid_toolbar_control'; + +describe('euiDataGridToolbarControl', () => { + shouldRenderCustomStyles(); + + it('passes props to the underlying EuiButtonEmpty', () => { + const { container } = render( + + Test button text + + ); + + expect(container.firstChild).toMatchSnapshot(); + }); + + it('renders with a badge', () => { + const { container } = render( + + Test button text + + ); + + expect(container.firstChild).toMatchSnapshot(); + expect( + container.querySelector('.euiDataGridToolbarControl__badge') + ).toBeInTheDocument(); + }); + + it('renders textProps onto the custom text wrapper', () => { + const { container } = render( + + Test button text + + ); + + expect(container.querySelector('.euiDataGridToolbarControl__text')) + .toMatchInlineSnapshot(` + + Test button text + + `); + }); +}); diff --git a/src/components/datagrid/controls/data_grid_toolbar_control.tsx b/src/components/datagrid/controls/data_grid_toolbar_control.tsx new file mode 100644 index 00000000000..3bbddcf9bea --- /dev/null +++ b/src/components/datagrid/controls/data_grid_toolbar_control.tsx @@ -0,0 +1,87 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import React, { FunctionComponent } from 'react'; +import classNames from 'classnames'; +import { css } from '@emotion/react'; + +import { EuiButtonEmpty, EuiButtonEmptyProps } from '../../button'; +import { EuiNotificationBadge } from '../../badge'; +import { useEuiI18n } from '../../i18n'; + +export type EuiDataGridToolbarControlProps = EuiButtonEmptyProps & { + badgeContent?: number | string; +}; + +export const EuiDataGridToolbarControl: FunctionComponent< + EuiDataGridToolbarControlProps +> = ({ children, className, badgeContent, textProps, ...rest }) => { + const classes = classNames('euiDataGridToolbarControl', className); + + const badgeAriaLabel = useEuiI18n( + 'euiDataGridToolbarControl.badgeAriaLabel', + 'Active: {count}', + { + count: + typeof badgeContent === 'string' + ? betterScreenReaderSlashes(badgeContent) + : badgeContent, + } + ); + + return ( + + + {children} + + + {Boolean(badgeContent) && ( + + {badgeContent} + + )} + + ); +}; + +// The columns control specifically passes (e.g.) `5/10` when some columns +// are being hidden. We can make this a bit more legible to SRs with this quick util +const betterScreenReaderSlashes = (badgeContent: string) => + badgeContent.replaceAll('/', ' out of '); diff --git a/src/components/datagrid/controls/display_selector.tsx b/src/components/datagrid/controls/display_selector.tsx index 55b1c6f0984..1664ea7c9df 100644 --- a/src/components/datagrid/controls/display_selector.tsx +++ b/src/components/datagrid/controls/display_selector.tsx @@ -248,7 +248,6 @@ export const useDataGridDisplaySelector = ( setIsOpen(!isOpen)} diff --git a/src/components/datagrid/controls/fullscreen_selector.tsx b/src/components/datagrid/controls/fullscreen_selector.tsx index c76ba9931ee..d151e7f2f59 100644 --- a/src/components/datagrid/controls/fullscreen_selector.tsx +++ b/src/components/datagrid/controls/fullscreen_selector.tsx @@ -15,7 +15,7 @@ import React, { KeyboardEvent, KeyboardEventHandler, } from 'react'; -import classNames from 'classnames'; + import { keys } from '../../../services'; import { EuiToolTip } from '../../tool_tip'; import { EuiButtonIcon } from '../../button'; @@ -38,9 +38,6 @@ export const useDataGridFullScreenSelector = (): { ], ['Enter fullscreen', 'Exit fullscreen'] ); - const controlBtnClasses = classNames('euiDataGrid__controlBtn', { - 'euiDataGrid__controlBtn--active': isFullScreen, - }); const fullScreenSelector = useMemo( () => ( setIsFullScreen(!isFullScreen)} aria-label={isFullScreen ? fullScreenButtonActive : fullScreenButton} /> ), - [isFullScreen, controlBtnClasses, fullScreenButton, fullScreenButtonActive] + [isFullScreen, fullScreenButton, fullScreenButtonActive] ); const handleGridKeyDown = useCallback( diff --git a/src/components/datagrid/controls/index.ts b/src/components/datagrid/controls/index.ts index 5c4a98c398d..8b17b80bb86 100644 --- a/src/components/datagrid/controls/index.ts +++ b/src/components/datagrid/controls/index.ts @@ -15,3 +15,7 @@ export { checkOrDefaultToolBarDisplayOptions, EuiDataGridToolbar, } from './data_grid_toolbar'; +export { + EuiDataGridToolbarControl, + type EuiDataGridToolbarControlProps, +} from './data_grid_toolbar_control'; diff --git a/src/components/datagrid/data_grid.test.tsx b/src/components/datagrid/data_grid.test.tsx index 337c1a083e6..2eb963f13dd 100644 --- a/src/components/datagrid/data_grid.test.tsx +++ b/src/components/datagrid/data_grid.test.tsx @@ -1901,13 +1901,15 @@ describe('EuiDataGrid', () => { /> ); - // Get column sorting button - const sortColumn = component.find( - 'EuiButtonEmpty[data-test-subj="dataGridColumnSortingButton"]' - ); - const getButtonText = (): string => - sortColumn.find('span[className~="euiButtonEmpty__text"]').text(); - expect(getButtonText()).toEqual('Sort fields'); + // Get column sort count + const getBadgeText = () => { + const button = component.find( + 'EuiButtonEmpty[data-test-subj="dataGridColumnSortingButton"]' + ); + const badge = button.find('span.euiDataGridToolbarControl__badge'); + return badge.length ? badge.text() : false; + }; + expect(getBadgeText()).toBeFalsy(); // Update sorted columns component.setProps({ @@ -1916,7 +1918,7 @@ describe('EuiDataGrid', () => { onSort: () => {}, }, }); - expect(getButtonText()).toEqual('1 field sorted'); + expect(getBadgeText()).toEqual('1'); // Update sorted columns again component.setProps({ @@ -1928,7 +1930,7 @@ describe('EuiDataGrid', () => { onSort: () => {}, }, }); - expect(getButtonText()).toEqual('2 fields sorted'); + expect(getBadgeText()).toEqual('2'); }); }); diff --git a/src/components/datagrid/index.ts b/src/components/datagrid/index.ts index 7d32fd2d869..8a70ba76b17 100644 --- a/src/components/datagrid/index.ts +++ b/src/components/datagrid/index.ts @@ -11,6 +11,7 @@ export { useDataGridColumnSelector, useDataGridColumnSorting, useDataGridDisplaySelector, + EuiDataGridToolbarControl, } from './controls'; export * from './data_grid_types'; diff --git a/src/components/date_picker/super_date_picker/super_update_button.tsx b/src/components/date_picker/super_date_picker/super_update_button.tsx index dab442fe5ee..8c67f437ca8 100644 --- a/src/components/date_picker/super_date_picker/super_update_button.tsx +++ b/src/components/date_picker/super_date_picker/super_update_button.tsx @@ -185,7 +185,7 @@ export class EuiSuperUpdateButton extends Component< ...restTextProps, className: classNames( 'euiScreenReaderOnly', - restTextProps?.className + restTextProps && restTextProps.className ), }} {...rest} diff --git a/src/components/filter_group/filter_button.tsx b/src/components/filter_group/filter_button.tsx index fd2636dc4b9..3f7354a28da 100644 --- a/src/components/filter_group/filter_button.tsx +++ b/src/components/filter_group/filter_button.tsx @@ -119,7 +119,7 @@ export const EuiFilterButton: FunctionComponent = ({ const buttonTextClassNames = classNames( 'euiFilterButton__text', { 'euiFilterButton__text-hasNotification': showBadge }, - textProps?.className + textProps && textProps.className ); const badgeContent = showBadge && ( @@ -171,7 +171,7 @@ export const EuiFilterButton: FunctionComponent = ({ css: [ textStyles.euiFilterButton__text, showBadge && textStyles.hasNotification, - textProps?.css, + textProps && textProps.css, ], }} contentProps={{ From f58509d0d37c127f90bccf701078638dc27ce40f Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Wed, 22 Nov 2023 08:35:06 -0800 Subject: [PATCH 4/9] Fix bug with cell actions/outline CSS variable color - cell actions bg color was missing on popover open - always defaulting to color primary fixes the issue --- src/components/datagrid/_data_grid_data_row.scss | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/components/datagrid/_data_grid_data_row.scss b/src/components/datagrid/_data_grid_data_row.scss index 0fdfde41139..6bc2037e470 100644 --- a/src/components/datagrid/_data_grid_data_row.scss +++ b/src/components/datagrid/_data_grid_data_row.scss @@ -31,9 +31,10 @@ border-right-color: $euiBorderColor; } + --euiDataGridCellOutlineColor: #{$euiColorPrimary}; + &:hover, &:focus { - --euiDataGridCellOutlineColor: #{$euiColorPrimary}; @include euiDataGridCellFocus; } From 937a03344a28eb1dbec482d4b101be4e097e8fbc Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Wed, 22 Nov 2023 14:21:57 -0800 Subject: [PATCH 5/9] [design feedback] bottom border radius for when cell actions is wider than cell width --- src/components/datagrid/_data_grid_data_row.scss | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/components/datagrid/_data_grid_data_row.scss b/src/components/datagrid/_data_grid_data_row.scss index 6bc2037e470..75ee44d6879 100644 --- a/src/components/datagrid/_data_grid_data_row.scss +++ b/src/components/datagrid/_data_grid_data_row.scss @@ -153,6 +153,14 @@ z-index: $euiZDataGridCellPopover - 1; } + .euiDataGridRowCell--alignLeft & { + border-bottom-right-radius: $euiBorderRadius / 2; + } + + .euiDataGridRowCell--alignRight & { + border-bottom-left-radius: $euiBorderRadius / 2; + } + // Visual trickery - fill in the gap between the cell outline border-radius & the actions &::after { content: ''; From d04aabc7a7c43374cd2036fdf69dcaf0aaf68d8e Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Wed, 22 Nov 2023 14:22:45 -0800 Subject: [PATCH 6/9] [opinionated] show focus ring on popover expanion open - this prevents a visual bug when the popover is open *above* the cell instead of above - it also fills the gap that occurs between the cell actions and the popover --- src/components/datagrid/_data_grid_data_row.scss | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/components/datagrid/_data_grid_data_row.scss b/src/components/datagrid/_data_grid_data_row.scss index 75ee44d6879..f3287fd86f5 100644 --- a/src/components/datagrid/_data_grid_data_row.scss +++ b/src/components/datagrid/_data_grid_data_row.scss @@ -34,7 +34,8 @@ --euiDataGridCellOutlineColor: #{$euiColorPrimary}; &:hover, - &:focus { + &:focus, + &.euiDataGridRowCell--open { @include euiDataGridCellFocus; } From e1c802c1380d1c07def87451bfb9f827f2bd0677 Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Wed, 22 Nov 2023 14:31:41 -0800 Subject: [PATCH 7/9] [design feedback] Anchor column actions popover to icon instead of entire cell --- .../__snapshots__/data_grid.test.tsx.snap | 242 +++++++++--------- .../data_grid_body_custom.test.tsx.snap | 60 ++--- .../data_grid_body_virtualized.test.tsx.snap | 60 ++--- .../data_grid_header_cell.test.tsx.snap | 30 +-- .../body/header/data_grid_header_cell.tsx | 68 +++-- 5 files changed, 237 insertions(+), 223 deletions(-) diff --git a/src/components/datagrid/__snapshots__/data_grid.test.tsx.snap b/src/components/datagrid/__snapshots__/data_grid.test.tsx.snap index e0433fb8303..63fa99ebb2b 100644 --- a/src/components/datagrid/__snapshots__/data_grid.test.tsx.snap +++ b/src/components/datagrid/__snapshots__/data_grid.test.tsx.snap @@ -610,21 +610,21 @@ exports[`EuiDataGrid rendering renders additional toolbar controls 1`] = ` data-test-subj="dataGridColumnResizer" style="margin-right: 0px;" /> -
-
+
-
- A -
@@ -634,8 +634,8 @@ exports[`EuiDataGrid rendering renders additional toolbar controls 1`] = ` data-test-subj="dataGridHeaderCellActionButton-A" />
- -
+ +
-
+
-
- B -
@@ -687,8 +687,8 @@ exports[`EuiDataGrid rendering renders additional toolbar controls 1`] = ` data-test-subj="dataGridHeaderCellActionButton-B" />
- -
+ +
-
+
-
- A -
@@ -1080,8 +1080,8 @@ exports[`EuiDataGrid rendering renders control columns 1`] = ` data-test-subj="dataGridHeaderCellActionButton-A" />
- -
+ +
-
+
-
- B -
@@ -1133,8 +1133,8 @@ exports[`EuiDataGrid rendering renders control columns 1`] = ` data-test-subj="dataGridHeaderCellActionButton-B" />
- -
+ +
-
+
-
- Column A -
@@ -1763,8 +1763,8 @@ exports[`EuiDataGrid rendering renders custom column headers 1`] = ` data-test-subj="dataGridHeaderCellActionButton-A" />
- -
+ +
- -
+ +
-
+
-
- A -
@@ -2189,8 +2189,8 @@ exports[`EuiDataGrid rendering renders with common and div attributes 1`] = ` data-test-subj="dataGridHeaderCellActionButton-A" />
- -
+ +
-
+
-
- B -
@@ -2242,8 +2242,8 @@ exports[`EuiDataGrid rendering renders with common and div attributes 1`] = ` data-test-subj="dataGridHeaderCellActionButton-B" />
- -
+ +
-
+
-
- columnA -
@@ -49,8 +49,8 @@ exports[`EuiDataGridBodyCustomRender treats \`renderCustomGridBody\` as a render data-test-subj="dataGridHeaderCellActionButton-columnA" />
- -
+ +
-
+
-
- columnB -
@@ -102,8 +102,8 @@ exports[`EuiDataGridBodyCustomRender treats \`renderCustomGridBody\` as a render data-test-subj="dataGridHeaderCellActionButton-columnB" />
- -
+ +
-
+
-
- columnA -
@@ -53,8 +53,8 @@ exports[`EuiDataGridBodyVirtualized renders 1`] = ` data-test-subj="dataGridHeaderCellActionButton-columnA" />
- -
+ +
-
+
-
- columnB -
@@ -106,8 +106,8 @@ exports[`EuiDataGridBodyVirtualized renders 1`] = ` data-test-subj="dataGridHeaderCellActionButton-columnB" />
- -
+ +
-
+
-
- someColumn -
@@ -41,8 +41,8 @@ exports[`EuiDataGridHeaderCell renders 1`] = ` data-test-subj="dataGridHeaderCellActionButton-someColumn" />
- -
+ +