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

[EuiTable EuiBasicTable] Fix nested content alignment when selection is enabled #7895

Merged
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
5 changes: 5 additions & 0 deletions packages/eui/changelogs/upcoming/7895.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
**Bug fixes**

- Fixed overlapping content in `EuiBasicTable` for expanded and selectable table rows
- Fixed the alignment of `EuiBasicTable` mobile actions

Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { action } from '@storybook/addon-actions';
import { faker } from '@faker-js/faker';
import { moveStorybookControlsToCategory } from '../../../.storybook/utils';

import { useEuiTheme } from '../../services';
import { EuiLink } from '../link';
import { EuiHealth } from '../health';

Expand All @@ -20,6 +21,7 @@ import type {
EuiBasicTableColumn,
} from './basic_table';
import { EuiBasicTable, EuiBasicTableProps } from './basic_table';
import { EuiIcon } from '../icon';

// Set static seed so that the generated faker data is consistent between page loads
faker.seed(8_02_2010);
Expand Down Expand Up @@ -148,7 +150,7 @@ const columns: Array<EuiBasicTableColumn<User>> = [
'data-test-subj': 'action-outboundlink',
},
{
name: <>Clone</>,
name: 'Clone',
description: 'Clone this user',
icon: 'copy',
type: 'icon',
Expand Down Expand Up @@ -263,6 +265,61 @@ export const ExpandedRow: Story = {
},
};

const NestedTable = ({
hasLeadingIcon = false,
}: {
hasLeadingIcon?: boolean;
}) => {
const { euiTheme } = useEuiTheme();

const _items = users.slice(0, 3);
const _columns = hasLeadingIcon
? [
{
name: '',
render: () => <EuiIcon type="warning" />,
width: euiTheme.size.xl,
},
...columns,
]
: columns;

return (
<EuiBasicTable
tableCaption="EuiBasicTable playground"
items={_items}
itemId="id"
rowHeader="firstName"
columns={_columns}
/>
);
};

export const ExpandedNestedTable: Story = {
parameters: {
controls: {
include: ['columns', 'items', 'itemIdToExpandedRowMap'],
},
},
args: {
tableCaption: 'EuiBasicTable playground',
items: users,
itemId: 'id',
rowHeader: 'firstName',
columns,
itemIdToExpandedRowMap: {
1: <NestedTable />,
3: <NestedTable hasLeadingIcon />,
},
selection: {
selectable: (user) => user.online,
selectableMessage: (selectable) =>
!selectable ? 'User is currently offline' : '',
onSelectionChange: action('onSelectionChange'),
},
},
};

const StatefulPlayground = ({
items,
pagination,
Expand Down
4 changes: 2 additions & 2 deletions packages/eui/src/components/table/table_row.styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ export const euiTableRowStyles = (euiThemeContext: UseEuiTheme) => {
// 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 {
Copy link
Member

@cee-chen cee-chen Jul 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm probably being a little nit-picky here, but wouldn't changing this to & > .euiTableRowCell:first-child { also have fixed the issue? I'm a little hesitant to apply new props to EuiTableRowCell mostly because it's a public top-level API change 🤷 (which would mean breaking changes or deprecations if they ever change again in the future)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh damn, yes you're absolutely right! I should have noticed that 🤦‍♀️
That is definitely a more straight forward solution. I'll update 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reverted the API changes here in favor of the suggested style solution.

& > .euiTableRowCell:first-child {
${logicalCSS('padding-left', checkboxSize)}
}
`,
Expand Down Expand Up @@ -124,7 +124,7 @@ export const euiTableRowStyles = (euiThemeContext: UseEuiTheme) => {
${logicalCSS('border-top-left-radius', 0)}
${logicalCSS('border-top-right-radius', 0)}

.euiTableRowCell {
> .euiTableRowCell {
${logicalCSS('width', '100%')}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@ const meta: Meta<EuiTableRowCellProps> = {
<EuiTable>
<EuiTableBody>
<EuiTableRow hasActions={args.hasActions}>
<EuiTableRowCell>Cell 1</EuiTableRowCell>
{(args.hasActions || args.isExpander) && (
<EuiTableRowCell>Cell 1</EuiTableRowCell>
)}
<Story {...args} />
</EuiTableRow>
</EuiTableBody>
Expand Down
Loading