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 = {