Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[EuiBasicTable][EuiInMemoryTable] Remove need for isSelectable, isExpandable, and hasActions props #7632

Merged
merged 7 commits into from
Apr 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions changelogs/upcoming/7632.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
**Breaking changes**

- The following props are no longer needed by `EuiBasicTable` or `EuiInMemoryTable` for responsive table behavior to work correctly, and can be removed:
- `isSelectable`
- `isExpandable`
- `hasActions`

**DOM changes**

- `EuiTableRow`s rendered by basic and memory tables now only render a `.euiTableRow-isSelectable` className if the selection checkbox is not disabled
3 changes: 3 additions & 0 deletions changelogs/upcoming/TBD.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
**CSS-in-JS conversions**

- Removed the following `EuiTable` Sass variables:
- `$euiTableCellContentPadding`
- `$euiTableCellContentPaddingCompressed`
- `$euiTableCellCheckboxWidth`
- `$euiTableHoverColor`
- `$euiTableSelectedColor`
- `$euiTableHoverSelectedColor`
Expand Down
1 change: 0 additions & 1 deletion src-docs/src/views/tables/actions/actions.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -396,7 +396,6 @@ export default () => {
pagination={pagination}
sorting={sorting}
selection={selection}
hasActions={customAction ? false : true}
onChange={onTableChange}
/>
</>
Expand Down
2 changes: 1 addition & 1 deletion src-docs/src/views/tables/custom/custom.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -686,7 +686,7 @@ export default class extends Component<{}, State> {
<EuiTableRow
key={item.id}
isSelected={this.isItemSelected(item.id)}
isSelectable={true}
hasSelection={true}
hasActions={true}
>
{cells}
Expand Down
3 changes: 0 additions & 3 deletions src-docs/src/views/tables/expanding_rows/expanding_rows.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -260,12 +260,9 @@ export default () => {
items={pageOfItems}
itemId="id"
itemIdToExpandedRowMap={itemIdToExpandedRowMap}
isExpandable={true}
hasActions={true}
columns={columnsWithExpandingRowToggle}
pagination={pagination}
sorting={sorting}
isSelectable={true}
selection={selection}
onChange={onTableChange}
/>
Expand Down
1 change: 0 additions & 1 deletion src-docs/src/views/tables/footer/footer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,6 @@ export default () => {
columns={columns}
pagination={pagination}
sorting={sorting}
isSelectable={true}
selection={selection}
onChange={onTableChange}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,6 @@ export default () => {
pagination={pagination}
sorting={true}
selection={selectionValue}
isSelectable={true}
/>
</>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,6 @@ export default () => {
pagination={pagination}
sorting={true}
selection={selectionValue}
isSelectable={true}
/>
);
};
2 changes: 0 additions & 2 deletions src-docs/src/views/tables/mobile/mobile.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -300,8 +300,6 @@ export default () => {
pagination={pagination}
sorting={sorting}
selection={selection}
isSelectable={true}
hasActions={true}
responsiveBreakpoint={isResponsive}
onChange={onTableChange}
/>
Expand Down
21 changes: 0 additions & 21 deletions src-docs/src/views/tables/mobile/mobile_section.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import { Link } from 'react-router-dom';
import { GuideSectionTypes } from '../../../components';

import Table from './mobile';
import { EuiTextColor } from '../../../../../src/components/text';
import { EuiCode, EuiCodeBlock } from '../../../../../src/components/code';
const source = require('!!raw-loader!./mobile');
import { EuiTableRowCellMobileOptionsShape } from '../props/props';
Expand Down Expand Up @@ -51,26 +50,6 @@ export const section = {
Inversely, if you always want your table to render in a mobile-friendly
manner, pass <EuiCode>true</EuiCode>.
</p>
<p>
{/* TODO: This shouldn't be true by the end of the Emotion conversion */}
To make your table work responsively, please make sure you add the
following <EuiTextColor color="danger">additional</EuiTextColor> props
to the top level table component (<strong>EuiBasicTable</strong> or{' '}
<strong>EuiInMemoryTable</strong>):
</p>
<ul>
<li>
<EuiCode>isSelectable</EuiCode>: if the table has a single column of
checkboxes for selecting rows
</li>
<li>
<EuiCode>isExpandable</EuiCode>: if the table has rows that can expand
</li>
<li>
<EuiCode>hasActions</EuiCode>: if the table has a column for actions
which may/may not be hidden in hover
</li>
</ul>
<p>
The <EuiCode>mobileOptions</EuiCode> object can be passed to the{' '}
<strong>EuiTableRowCell</strong> directly or with each column item
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,6 @@ export default () => {
columns={columns}
pagination={pagination}
sorting={sorting}
isSelectable={true}
selection={selection}
onChange={onTableChange}
rowHeader="firstName"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,6 @@ export default () => {
columns={columns}
pagination={pagination}
sorting={sorting}
isSelectable={true}
selection={selection}
onChange={onTableChange}
rowHeader="firstName"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ exports[`EuiBasicTable renders (bare-bones) 1`] = `
class="css-0"
>
<tr
class="euiTableRow emotion-euiTableRow-desktop"
class="euiTableRow euiTableRow-isSelectable emotion-euiTableRow-desktop"
>
<td
class="euiTableRowCell emotion-euiTableRowCell-middle-desktop"
Expand All @@ -66,7 +66,7 @@ exports[`EuiBasicTable renders (bare-bones) 1`] = `
</td>
</tr>
<tr
class="euiTableRow emotion-euiTableRow-desktop"
class="euiTableRow euiTableRow-isSelectable emotion-euiTableRow-desktop"
>
<td
class="euiTableRowCell emotion-euiTableRowCell-middle-desktop"
Expand All @@ -88,7 +88,7 @@ exports[`EuiBasicTable renders (bare-bones) 1`] = `
</td>
</tr>
<tr
class="euiTableRow emotion-euiTableRow-desktop"
class="euiTableRow euiTableRow-isSelectable emotion-euiTableRow-desktop"
>
<td
class="euiTableRowCell emotion-euiTableRowCell-middle-desktop"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ exports[`EuiInMemoryTable with items 1`] = `
class="css-0"
>
<tr
class="euiTableRow emotion-euiTableRow-desktop"
class="euiTableRow euiTableRow-isSelectable emotion-euiTableRow-desktop"
>
<td
class="euiTableRowCell emotion-euiTableRowCell-middle-desktop"
Expand All @@ -306,7 +306,7 @@ exports[`EuiInMemoryTable with items 1`] = `
</td>
</tr>
<tr
class="euiTableRow emotion-euiTableRow-desktop"
class="euiTableRow euiTableRow-isSelectable emotion-euiTableRow-desktop"
>
<td
class="euiTableRowCell emotion-euiTableRowCell-middle-desktop"
Expand All @@ -328,7 +328,7 @@ exports[`EuiInMemoryTable with items 1`] = `
</td>
</tr>
<tr
class="euiTableRow emotion-euiTableRow-desktop"
class="euiTableRow euiTableRow-isSelectable emotion-euiTableRow-desktop"
>
<td
class="euiTableRowCell emotion-euiTableRowCell-middle-desktop"
Expand Down
1 change: 0 additions & 1 deletion src/components/basic_table/basic_table.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,6 @@ describe('EuiBasicTable', () => {
itemIdToExpandedRowMap: {
'1': <div>Expanded row</div>,
},
isExpandable: true,
};
const { getByText } = render(<EuiBasicTable {...props} />);

Expand Down
54 changes: 23 additions & 31 deletions src/components/basic_table/basic_table.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -241,9 +241,6 @@ interface BasicTableProps<T extends object>
* Indicates which column should be used as the identifying cell in each row. Should match a "field" prop in FieldDataColumn
*/
rowHeader?: string;
hasActions?: boolean;
isExpandable?: boolean;
isSelectable?: boolean;
/**
* Provides an infinite loading indicator
*/
Expand Down Expand Up @@ -523,9 +520,6 @@ export class EuiBasicTable<T extends object = any> extends Component<
compressed,
itemIdToExpandedRowMap,
responsiveBreakpoint,
isSelectable,
isExpandable,
hasActions,
rowProps,
cellProps,
tableCaption,
Expand Down Expand Up @@ -965,15 +959,8 @@ export class EuiBasicTable<T extends object = any> extends Component<
}

renderItemRow(item: T, rowIndex: number) {
const {
columns,
selection,
isSelectable,
hasActions,
rowHeader,
itemIdToExpandedRowMap = {},
isExpandable,
} = this.props;
const { columns, selection, rowHeader, itemIdToExpandedRowMap } =
this.props;

const cells = [];

Expand All @@ -990,13 +977,18 @@ export class EuiBasicTable<T extends object = any> extends Component<
getItemId(selectedItem, itemIdCallback) === itemId
);

let calculatedHasSelection;
let rowSelectionDisabled = false;
if (selection) {
cells.push(this.renderItemSelectionCell(itemId, item, selected));
calculatedHasSelection = true;
const [checkboxCell, isDisabled] = this.renderItemSelectionCell(
itemId,
item,
selected
);
cells.push(checkboxCell);
rowSelectionDisabled = !!isDisabled;
}

let calculatedHasActions;
let hasActions;
columns.forEach((column: EuiBasicTableColumn<T>, columnIndex: number) => {
if ((column as EuiTableActionsColumnType<T>).actions) {
cells.push(
Expand All @@ -1007,7 +999,7 @@ export class EuiBasicTable<T extends object = any> extends Component<
columnIndex
)
);
calculatedHasActions = true;
hasActions = true;
} else if ((column as EuiTableFieldDataColumnType<T>).field) {
const fieldDataColumn = column as EuiTableFieldDataColumnType<T>;
cells.push(
Expand Down Expand Up @@ -1043,18 +1035,18 @@ export class EuiBasicTable<T extends object = any> extends Component<
expandedRowColSpan = expandedRowColSpan - mobileOnlyCols;

// We'll use the ID to associate the expanded row with the original.
const hasExpandedRow = itemIdToExpandedRowMap.hasOwnProperty(itemId);
const hasExpandedRow = itemIdToExpandedRowMap?.hasOwnProperty(itemId);
const expandedRowId = hasExpandedRow
? `row_${itemId}_expansion`
: undefined;
const expandedRow = hasExpandedRow ? (
<EuiTableRow
id={expandedRowId}
isExpandedRow={true}
isSelectable={isSelectable}
hasSelection={!!selection}
>
<EuiTableRowCell colSpan={expandedRowColSpan} textOnly={false}>
{itemIdToExpandedRowMap[itemId]}
{itemIdToExpandedRowMap![itemId]}
</EuiTableRowCell>
</EuiTableRow>
) : undefined;
Expand All @@ -1064,12 +1056,11 @@ export class EuiBasicTable<T extends object = any> extends Component<
const row = (
<EuiTableRow
aria-owns={expandedRowId}
isSelectable={
isSelectable == null ? calculatedHasSelection : isSelectable
}
hasSelection={!!selection}
isSelectable={!rowSelectionDisabled}
isSelected={selected}
hasActions={hasActions == null ? calculatedHasActions : hasActions}
isExpandable={isExpandable}
hasActions={hasActions}
isExpandable={hasExpandedRow}
{...rowProps}
>
{cells}
Expand Down Expand Up @@ -1107,7 +1098,7 @@ export class EuiBasicTable<T extends object = any> extends Component<
);
}
};
return (
return [
<EuiTableRowCellCheckbox key={key}>
<EuiI18n token="euiBasicTable.selectThisRow" default="Select this row">
{(selectThisRow: string) => (
Expand All @@ -1123,8 +1114,9 @@ export class EuiBasicTable<T extends object = any> extends Component<
/>
)}
</EuiI18n>
</EuiTableRowCellCheckbox>
);
</EuiTableRowCellCheckbox>,
disabled,
];
}

renderItemActionsCell(
Expand Down
4 changes: 0 additions & 4 deletions src/components/basic_table/in_memory_table.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -677,8 +677,6 @@ export class EuiInMemoryTable<T extends object = object> extends Component<
message,
error,
selection,
isSelectable,
hasActions,
compressed,
pagination: hasPagination,
sorting: hasSorting,
Expand Down Expand Up @@ -747,8 +745,6 @@ export class EuiInMemoryTable<T extends object = object> extends Component<
pagination={pagination}
sorting={sorting}
selection={selection}
isSelectable={isSelectable}
hasActions={hasActions}
onChange={this.onTableChange}
error={error}
loading={loading}
Expand Down
2 changes: 0 additions & 2 deletions src/components/table/_index.scss
Original file line number Diff line number Diff line change
@@ -1,4 +1,2 @@
@import 'variables';

@import 'table';
@import 'responsive';
6 changes: 0 additions & 6 deletions src/components/table/_table.scss
Original file line number Diff line number Diff line change
@@ -1,9 +1,3 @@
.euiTableRow-isExpandedRow {
&.euiTableRow-isSelectable .euiTableCellContent {
padding-left: $euiTableCellCheckboxWidth + $euiTableCellContentPadding;
}
}

/**
* 1. Vertically align all children.
* 4. Prevent very long single words (e.g. the name of a field in a document) from overflowing
Expand Down
6 changes: 0 additions & 6 deletions src/components/table/_variables.scss

This file was deleted.

11 changes: 9 additions & 2 deletions src/components/table/table_row.styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export const euiTableRowStyles = (euiThemeContext: UseEuiTheme) => {
const rowColors = _rowColorVariables(euiThemeContext);
const expandedAnimationCss = _expandedRowAnimation(euiThemeContext);

const { cellContentPadding, mobileSizes } =
const { cellContentPadding, mobileSizes, checkboxSize } =
euiTableVariables(euiThemeContext);

return {
Expand Down Expand Up @@ -57,6 +57,13 @@ export const euiTableRowStyles = (euiThemeContext: UseEuiTheme) => {
background-color: ${rowColors.selected.hover};
}
`,
// Offset expanded & selectable rows by the checkbox width to line up content with the 2nd column
// Set on the `<td>` because padding can't be applied to `<tr>` elements directly
checkboxOffset: css`
.euiTableRowCell:first-child {
${logicalCSS('padding-left', checkboxSize)}
}
`,
},

mobile: {
Expand All @@ -82,7 +89,7 @@ export const euiTableRowStyles = (euiThemeContext: UseEuiTheme) => {
* Left column offset (no border)
* Used for selection checkbox, which will be absolutely positioned
*/
selectable: css`
hasLeftColumn: css`
${logicalCSS('padding-left', mobileSizes.checkbox.width)}
`,
/**
Expand Down
Loading