Skip to content

Commit 4b89eaf

Browse files
cee-chencee-chen
committed
🐛 Fix custom action mobile styling
- Fix 1: `flex-direction: column` and `padding: 0` was being applied to custom action cells when they should not have been - Fix 2: table rows with only custom actions were still showing an empty right column border/padding on mobile. We needed the `actions="custom"` logic on table rows as well as row cells + Add a few test assertions to try and capture regressions in the future
1 parent 39c965b commit 4b89eaf

5 files changed

Lines changed: 59 additions & 15 deletions

File tree

‎src/components/basic_table/__snapshots__/basic_table.test.tsx.snap‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,29 @@
11
// Jest Snapshot v1, https://goo.gl/fbAQLP
22

3+
exports[`EuiBasicTable actions custom item actions 1`] = `
4+
<tr
5+
class="euiTableRow euiTableRow-isSelectable euiTableRow-hasActions emotion-euiTableRow-mobile"
6+
>
7+
<td
8+
class="euiTableRowCell euiTableRowCell--hasActions emotion-euiTableRowCell-hasActions-middle-mobile-customActions"
9+
>
10+
<div
11+
class="euiTableCellContent emotion-euiTableCellContent-wrapText-actions"
12+
>
13+
<div
14+
class=""
15+
>
16+
<button
17+
data-test-subj="customAction-1"
18+
>
19+
Custom action
20+
</button>
21+
</div>
22+
</div>
23+
</td>
24+
</tr>
25+
`;
26+
327
exports[`EuiBasicTable renders (bare-bones) 1`] = `
428
<div
529
aria-label="aria-label"

‎src/components/basic_table/basic_table.test.tsx‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -704,7 +704,6 @@ describe('EuiBasicTable', () => {
704704
const props: EuiBasicTableProps<BasicItem> = {
705705
items: basicItems,
706706
columns: [
707-
...basicColumns,
708707
{
709708
name: 'Actions',
710709
actions: [
@@ -729,9 +728,21 @@ describe('EuiBasicTable', () => {
729728
expect(queryByTestSubject('customAction-2')).toBeInTheDocument();
730729
expect(queryByTestSubject('customAction-3')).not.toBeInTheDocument();
731730

731+
// TODO: These assertions should ideally be visual regression snapshots instead
732732
expect(
733733
container.querySelector('.euiTableRowCell--hasActions')!.className
734734
).toContain('-customActions');
735+
expect(
736+
container.querySelector(
737+
'.euiTableRowCell--hasActions .euiTableCellContent'
738+
)!.className
739+
).not.toContain('-actions-mobile');
740+
expect(
741+
container.querySelector('.euiTableRow-hasActions')!.className
742+
).not.toContain('-hasRightColumn');
743+
expect(
744+
container.querySelector('.euiTableRow-hasActions')
745+
).toMatchSnapshot();
735746
});
736747

737748
describe('are disabled on selection', () => {

‎src/components/basic_table/basic_table.tsx‎

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -987,18 +987,26 @@ export class EuiBasicTable<T extends object = any> extends Component<
987987
rowSelectionDisabled = !!isDisabled;
988988
}
989989

990-
let hasActions;
990+
let hasActions: 'custom' | boolean = false;
991991
columns.forEach((column: EuiBasicTableColumn<T>, columnIndex: number) => {
992-
if ((column as EuiTableActionsColumnType<T>).actions) {
992+
const columnActions = (column as EuiTableActionsColumnType<T>).actions;
993+
994+
if (columnActions) {
995+
const hasCustomActions = columnActions.some(
996+
(action) => !!(action as CustomItemAction<T>).render
997+
);
993998
cells.push(
994999
this.renderItemActionsCell(
9951000
itemId,
9961001
item,
9971002
column as EuiTableActionsColumnType<T>,
998-
columnIndex
1003+
columnIndex,
1004+
hasCustomActions
9991005
)
10001006
);
1001-
hasActions = true;
1007+
// A table theoretically could have both custom and default action items
1008+
// If it has both, default action mobile row styles take precedence over custom
1009+
hasActions = !hasActions && hasCustomActions ? 'custom' : true;
10021010
} else if ((column as EuiTableFieldDataColumnType<T>).field) {
10031011
const fieldDataColumn = column as EuiTableFieldDataColumnType<T>;
10041012
cells.push(
@@ -1122,15 +1130,12 @@ export class EuiBasicTable<T extends object = any> extends Component<
11221130
itemId: ItemIdResolved,
11231131
item: T,
11241132
column: EuiTableActionsColumnType<T>,
1125-
columnIndex: number
1133+
columnIndex: number,
1134+
hasCustomActions: boolean
11261135
) {
11271136
// Disable all actions if any row(s) are selected
11281137
const allDisabled = this.state.selection.length > 0;
11291138

1130-
const hasCustomActions = column.actions.some(
1131-
(action: Action<T>) => !!(action as CustomItemAction<T>).render
1132-
);
1133-
11341139
let actualActions = column.actions.filter(
11351140
(action: Action<T>) => !action.available || action.available(item)
11361141
);

‎src/components/table/_table_cell_content.tsx‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,9 +43,13 @@ export const EuiTableCellContent: FunctionComponent<
4343
!isResponsive && styles[align], // On mobile, always align cells to the left
4444
truncateText === true && styles.truncateText,
4545
truncateText === false && styles.wrapText,
46-
hasActions && styles.hasActions.actions,
47-
hasActions &&
48-
(isResponsive ? styles.hasActions.mobile : styles.hasActions.desktop),
46+
...(hasActions
47+
? [
48+
styles.hasActions.actions,
49+
!isResponsive && styles.hasActions.desktop,
50+
isResponsive && hasActions !== 'custom' && styles.hasActions.mobile,
51+
]
52+
: []),
4953
];
5054

5155
const classes = classNames('euiTableCellContent', className);

‎src/components/table/table_row.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ export interface EuiTableRowProps {
3838
* Indicates if the table has a dedicated column for actions
3939
* (used for mobile styling and desktop action hover behavior)
4040
*/
41-
hasActions?: boolean;
41+
hasActions?: boolean | 'custom';
4242
/**
4343
* Indicates if the row will have an expanded row
4444
*/
@@ -75,7 +75,7 @@ export const EuiTableRow: FunctionComponent<Props> = ({
7575
styles.mobile.mobile,
7676
isSelected && styles.mobile.selected,
7777
isExpandedRow && styles.mobile.expanded,
78-
(hasActions || isExpandable || isExpandedRow) &&
78+
(hasActions === true || isExpandable || isExpandedRow) &&
7979
styles.mobile.hasRightColumn,
8080
hasSelection && styles.mobile.hasLeftColumn,
8181
]

0 commit comments

Comments
 (0)