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

[Discover][UnifiedDataTable] Enable drag&drop for grid columns #197832

Merged
merged 23 commits into from
Nov 6, 2024
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
a1cc540
[Discover][UnifiedDataTable] Enable drag&drop for grid columns
jughosta Oct 25, 2024
cbdc05e
Merge branch 'main' into 195769-draggable-columns
jughosta Oct 25, 2024
6dfd46c
Merge branch 'main' into 195769-draggable-columns
jughosta Oct 25, 2024
af6b57a
Merge branch 'main' into 195769-draggable-columns
jughosta Oct 28, 2024
483d40a
[Discover] Add a test
jughosta Oct 28, 2024
ab766d4
[Discover] Update tests
jughosta Oct 28, 2024
6f51907
Merge remote-tracking branch 'origin/195769-draggable-columns' into 1…
jughosta Oct 28, 2024
3fbaecf
[Discover] Update styles
jughosta Oct 28, 2024
7c71840
[Discover] Update code style
jughosta Oct 28, 2024
a22034d
[Discover] Update code style
jughosta Oct 28, 2024
4fa0a4e
[Discover] Update styles
jughosta Oct 28, 2024
63acdb1
[Discover] Update styles
jughosta Oct 28, 2024
047e955
Merge branch 'main' into 195769-draggable-columns
jughosta Oct 30, 2024
7946301
[Discover] Update styles
jughosta Oct 30, 2024
979fa69
Merge branch 'main' into 195769-draggable-columns
jughosta Oct 31, 2024
6247504
Merge branch 'main' into 195769-draggable-columns
jughosta Nov 1, 2024
7967c37
[Discover] Update code style
jughosta Nov 1, 2024
a8eaa1b
Merge branch 'main' into 195769-draggable-columns
jughosta Nov 4, 2024
ae44093
Merge branch 'main' into 195769-draggable-columns
jughosta Nov 4, 2024
7978524
[Discover] Switch to opt-in prop
jughosta Nov 5, 2024
940b492
Merge branch 'main' into 195769-draggable-columns
jughosta Nov 5, 2024
2e61677
[Discover] Update tests
jughosta Nov 5, 2024
4008bd8
Merge branch 'main' into 195769-draggable-columns
jughosta Nov 6, 2024
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

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

22 changes: 14 additions & 8 deletions packages/kbn-unified-data-table/src/components/data_table.scss
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,6 @@
background: transparent;
}

.euiDataGridHeaderCell {
align-items: start;

.euiPopover[class*='euiDataGridHeaderCell__popover'] {
align-self: center;
}
}

.euiDataGrid--bordersHorizontal .euiDataGridHeader {
border-top: none;
}
Expand Down Expand Up @@ -101,6 +93,20 @@
}
}

// Custom styles for data grid header cell.
// It can also be inside a portal (outside of `unifiedDataTable__inner`) when dragged.
.unifiedDataTable__headerCell {
align-items: start;

.euiDataGridHeaderCell__draggableIcon {
padding-block: $euiSizeXS / 2; // to align with a token height
}

.euiDataGridHeaderCell__button {
margin-block: -$euiSizeXS; // to override Eui value for Density "Expanded"
}
}

.unifiedDataTable__table {
flex-grow: 1;
flex-shrink: 1;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1354,5 +1354,49 @@ describe('UnifiedDataTable', () => {
},
EXTENDED_JEST_TIMEOUT
);

it(
'should have columnVisibility configuration',
async () => {
const component = await getComponent({
...getProps(),
columns: ['message'],
canDragAndDropColumns: true,
});
expect(component.find(EuiDataGrid).last().prop('columnVisibility')).toMatchInlineSnapshot(`
Object {
"canDragAndDropColumns": true,
"setVisibleColumns": [Function],
"visibleColumns": Array [
"@timestamp",
"message",
],
}
`);
},
EXTENDED_JEST_TIMEOUT
);

it(
'should disable drag&drop if Summary is present',
async () => {
const component = await getComponent({
...getProps(),
columns: [],
canDragAndDropColumns: true,
});
expect(component.find(EuiDataGrid).last().prop('columnVisibility')).toMatchInlineSnapshot(`
Object {
"canDragAndDropColumns": false,
"setVisibleColumns": [Function],
"visibleColumns": Array [
"@timestamp",
"_source",
],
}
`);
},
EXTENDED_JEST_TIMEOUT
);
});
});
14 changes: 13 additions & 1 deletion packages/kbn-unified-data-table/src/components/data_table.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,10 @@ export interface UnifiedDataTableProps {
* Field tokens could be rendered in column header next to the field name.
*/
showColumnTokens?: boolean;
/**
* Set to true to allow users to drag and drop columns for reordering
*/
canDragAndDropColumns?: boolean;
/**
* Optional value for providing configuration setting for UnifiedDataTable header row height
*/
Expand Down Expand Up @@ -425,6 +429,7 @@ export const UnifiedDataTable = ({
columns,
columnsMeta,
showColumnTokens,
canDragAndDropColumns,
configHeaderRowHeight,
headerRowHeightState,
onUpdateHeaderRowHeight,
Expand Down Expand Up @@ -870,13 +875,20 @@ export const UnifiedDataTable = ({
const schemaDetectors = useMemo(() => getSchemaDetectors(), []);
const columnsVisibility = useMemo(
() => ({
canDragAndDropColumns: defaultColumns ? false : canDragAndDropColumns,
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the role of defaultColumns here. Does this condition covers the scenario when Summary column is visible?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@logeekal Yes, I noticed that columns are not movable right now when Summary is present. So I disabled the drag&drop too.

visibleColumns,
setVisibleColumns: (newColumns: string[]) => {
const dontModifyColumns = !shouldPrependTimeFieldColumn(newColumns);
onSetColumns(newColumns, dontModifyColumns);
},
}),
[visibleColumns, onSetColumns, shouldPrependTimeFieldColumn]
[
visibleColumns,
onSetColumns,
shouldPrependTimeFieldColumn,
canDragAndDropColumns,
defaultColumns,
]
);

const canSetExpandedDoc = Boolean(setExpandedDoc && !!renderDocumentView);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,7 @@ function buildEuiGridColumn({
},
cellActions,
visibleCellActions,
displayHeaderCellProps: { className: 'unifiedDataTable__headerCell' },
};

if (column.id === dataView.timeFieldName) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ export const DiscoverGrid: React.FC<DiscoverGridProps> = ({
return (
<UnifiedDataTable
showColumnTokens
canDragAndDropColumns
enableComparisonMode
renderCustomToolbar={renderCustomToolbar}
getRowIndicator={getRowIndicator}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,15 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {

it('should not show reset width button for auto width column', async () => {
await unifiedFieldList.clickFieldListItemAdd('@message');
await header.waitUntilLoadingHasFinished();
await discover.waitUntilSearchingHasFinished();
expect(await dataGrid.resetColumnWidthExists('@message')).to.be(false);
});

it('should show reset width button for absolute width column, and allow resetting to auto width', async () => {
await unifiedFieldList.clickFieldListItemAdd('@message');
await header.waitUntilLoadingHasFinished();
await discover.waitUntilSearchingHasFinished();
await testResizeColumn('@message');
});

Expand Down
20 changes: 18 additions & 2 deletions test/functional/services/data_grid.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,9 +102,25 @@ export class DataGridService extends FtrService {
public async resizeColumn(field: string, delta: number) {
const header = await this.getHeaderElement(field);
const originalWidth = (await header.getSize()).width;
const resizer = await header.findByCssSelector(
this.testSubjects.getCssSelector('dataGridColumnResizer')
const headerDraggableColumns = await this.find.allByCssSelector(
'[data-test-subj="euiDataGridHeaderDroppable"] > div'
);
// searching for a common parent of the field column header and its resizer
const fieldHeader: WebElementWrapper | null | undefined = (
await Promise.all(
headerDraggableColumns.map(async (column) => {
const hasFieldColumn =
(await column.findAllByCssSelector(`[data-gridcell-column-id="${field}"]`)).length > 0;
return hasFieldColumn ? column : null;
})
)
).find(Boolean);
const resizer = await fieldHeader?.findByTestSubject('dataGridColumnResizer');

if (!fieldHeader || !resizer) {
throw new Error(`Unable to find column resizer for field ${field}`);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Wow, maybe we should ask EUI for a test subj on the draggable wrappers as a followup 😅


await this.browser.dragAndDrop({ location: resizer }, { location: { x: delta, y: 0 } });
return { originalWidth, newWidth: (await header.getSize()).width };
}
Expand Down