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

Remove empty space when resize datagrid table in Discover and Allow saving column width in both Discover and Dashboards Embeddable #5883

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- [BUG][Multiple Datasource] Fix data source filter bug and add tests ([#6152](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6152))
- [BUG][Multiple Datasource] Fix obsolete snapshots for test within data source management plugin ([#6185](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6185))
- [Workspace] Add base path when parse url in http service ([#6233](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6233))
- [Discover] Remove empty space when resize datagrid table in Discover and Allow saving column width in both Discover and Dashboards Embeddable ([#5883](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5883))

### 🚞 Infrastructure

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
- [BUG][Multiple Datasource] Add a migration function for datasource to add migrationVersion field ([#6025](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6025))
- [BUG][MD]Expose picker using function in data source management plugin setup([#6030](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6030))
- [BUG][Multiple Datasource] Fix data source filter bug and add tests ([#6152](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/6152))
- [Discover] Remove empty space when resize datagrid table in Discover and Allow saving column width in both Discover and Dashboards Embeddable ([#5883](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5883))

### 📝 Documentation

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import './_data_grid_table.scss';

import React, { useState, useMemo, useCallback } from 'react';
import { EuiDataGrid, EuiDataGridSorting, EuiPanel } from '@elastic/eui';
import { EuiDataGrid, EuiDataGridSorting, EuiPanel, EuiDataGridProps } from '@elastic/eui';
import { IndexPattern, getServices } from '../../../opensearch_dashboards_services';
import { fetchTableDataCell } from './data_grid_table_cell_value';
import { buildDataGridColumns, computeVisibleColumns } from './data_grid_table_columns';
Expand All @@ -26,9 +26,11 @@ import { LegacyDiscoverTable } from '../default_discover_table/default_discover_
import { getNewDiscoverSetting } from '../utils/local_storage';
import { SortDirection, SortOrder } from '../../../saved_searches/types';
import { useToolbarOptions } from './data_grid_toolbar';
import { ColumnWidths } from './data_grid_table_columns';

export interface DataGridTableProps {
columns: string[];
columnWidths?: ColumnWidths;
indexPattern: IndexPattern;
onAddColumn: (column: string) => void;
onFilter: DocViewFilterFn;
Expand All @@ -38,6 +40,7 @@ export interface DataGridTableProps {
onSort: (s: SortOrder[]) => void;
rows: OpenSearchSearchHit[];
onSetColumns: (columns: string[]) => void;
onColumnResize: ({ columnId, width }: { columnId: string; width: number }) => void;
sort: SortOrder[];
displayTimeColumn: boolean;
title?: string;
Expand All @@ -51,13 +54,15 @@ export interface DataGridTableProps {

export const DataGridTable = ({
columns,
columnWidths,
indexPattern,
onAddColumn,
onFilter,
onMoveColumn,
onRemoveColumn,
onSetColumns,
onSort,
onColumnResize,
sort,
hits,
rows,
Expand Down Expand Up @@ -125,9 +130,17 @@ export const DataGridTable = ({
indexPattern,
displayTimeColumn,
includeSourceInColumns,
isContextView
isContextView,
columnWidths
),
[adjustedColumns, indexPattern, displayTimeColumn, includeSourceInColumns, isContextView]
[
adjustedColumns,
indexPattern,
displayTimeColumn,
includeSourceInColumns,
isContextView,
columnWidths,
]
);

const dataGridTableColumnsVisibility = useMemo(
Expand Down Expand Up @@ -218,6 +231,7 @@ export const DataGridTable = ({
sorting={sorting}
toolbarVisibility={isToolbarVisible ? toolbarOptions : false}
rowHeightsOptions={rowHeightsOptions}
onColumnResize={onColumnResize}
className="discoverDataGrid"
/>
),
Expand All @@ -232,6 +246,7 @@ export const DataGridTable = ({
isToolbarVisible,
toolbarOptions,
rowHeightsOptions,
onColumnResize,
]
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,13 @@ const customizedIndexPatternMockWithTimeField = getMockedIndexPatternWithTimeFie

describe('Testing buildDataGridColumns function ', () => {
it('should return correct columns without time column when displayTimeColumn is false', () => {
const columns = buildDataGridColumns(['name', 'currency'], customizedIndexPatternMock, false);
const columns = buildDataGridColumns(
['name', 'currency'],
customizedIndexPatternMock,
false,
false,
false
);
expect(columns).toHaveLength(2);
expect(columns[0].id).toEqual('name');
expect(columns[1].id).toEqual('currency');
Expand Down Expand Up @@ -93,7 +99,9 @@ describe('Testing buildDataGridColumns function ', () => {
const columns = buildDataGridColumns(
['name', 'currency', '_source'],
customizedIndexPatternMockWithTimeField,
true
true,
false,
false
);
expect(columns).toHaveLength(4);
expect(columns[0].id).toEqual('order_date');
Expand Down Expand Up @@ -165,7 +173,9 @@ describe('Testing buildDataGridColumns function ', () => {
const columns = buildDataGridColumns(
['name', 'currency', 'order_date'],
customizedIndexPatternMockWithTimeField,
true
true,
false,
false
);
expect(columns).toHaveLength(3);
expect(columns[2].id).toEqual('order_date');
Expand Down Expand Up @@ -218,6 +228,32 @@ describe('Testing buildDataGridColumns function ', () => {
]
`);
});

it('should respect column widths when provided', () => {
const columnWidths = { name: { width: 100 }, currency: { width: 150 } };
const columns = buildDataGridColumns(
['name', 'currency'],
customizedIndexPatternMock,
false,
false,
false,
columnWidths
);
expect(columns[0].initialWidth).toEqual(100);
expect(columns[1].initialWidth).toEqual(150);
});

it('should set default column width when not specified', () => {
const columns = buildDataGridColumns(
['name', 'currency'],
customizedIndexPatternMock,
false,
false,
false
);
expect(columns[0].initialWidth).toBeUndefined();
expect(columns[1].initialWidth).toBeUndefined();
});
});

describe('Testing computeVisibleColumns function ', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,30 +8,47 @@ import { i18n } from '@osd/i18n';
import { IndexPattern } from '../../../opensearch_dashboards_services';
import { getCellActions } from './data_grid_table_cell_actions';

export interface ColumnWidth {
width?: number;
}

export interface ColumnWidths {
[columnName: string]: ColumnWidth;
}

export function buildDataGridColumns(
columnNames: string[],
idxPattern: IndexPattern,
displayTimeColumn: boolean,
includeSourceInColumns: boolean,
isContextView: boolean
isContextView: boolean,
columnWidths?: ColumnWidths
) {
const timeFieldName = idxPattern.timeFieldName;
let columnsToUse = columnNames;
const getColumnWidth = (columnName: string) => columnWidths?.[columnName]?.width ?? 0;

if (displayTimeColumn && timeFieldName && !columnNames.includes(timeFieldName)) {
columnsToUse = [timeFieldName, ...columnNames];
}

return columnsToUse.map((colName) =>
generateDataGridTableColumn(colName, idxPattern, includeSourceInColumns, isContextView)
generateDataGridTableColumn(
colName,
idxPattern,
includeSourceInColumns,
isContextView,
getColumnWidth(colName)
)
);
}

export function generateDataGridTableColumn(
colName: string,
idxPattern: IndexPattern,
includeSourceInColumns: boolean,
isContextView: boolean
isContextView: boolean,
columnWidth: number | undefined = 0
) {
const timeLabel = i18n.translate('discover.timeLabel', {
defaultMessage: 'Time',
Expand Down Expand Up @@ -69,6 +86,9 @@ export function generateDataGridTableColumn(
defaultMessage: 'Source',
});
}
if (columnWidth > 0) {
dataGridCol.initialWidth = Number(columnWidth);
}
return dataGridCol;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ export function ContextApp({
onRemoveColumn={() => {}}
onSetColumns={() => {}}
onSort={() => {}}
onColumnResize={() => {}}
sort={sort}
rows={rows}
displayTimeColumn={displayTimeColumn}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ export const getTopNavLinks = (

savedSearch.columns = state.columns;
savedSearch.sort = state.sort;
savedSearch.columnWidths = state.columnWidths;

try {
const id = await savedSearch.save(saveOptions);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,4 +134,35 @@ describe('discoverSlice', () => {
const result = discoverSlice.reducer(initialState, action);
expect(result.columns).toEqual(['column1', 'column2', 'column3']);
});

it('should handle setColumnWidths', () => {
// Add columnWidths to the state only in this test
const stateWithColumnWidths = { ...initialState, columnWidths: {} };
const action = {
type: 'discover/setColumnWidths',
payload: { columnName: 'column1', width: 100 },
};
const result = discoverSlice.reducer(stateWithColumnWidths, action);
expect(result.columnWidths).toEqual({ column1: { width: 100 } });
});

it('should update column width if already set', () => {
const stateWithColumnWidths = { ...initialState, columnWidths: { column1: { width: 150 } } };
const action = {
type: 'discover/setColumnWidths',
payload: { columnName: 'column1', width: 200 },
};
const result = discoverSlice.reducer(stateWithColumnWidths, action);
expect(result.columnWidths).toEqual({ column1: { width: 200 } });
});

it('should add new column width while maintaining existing ones', () => {
const stateWithColumnWidths = { ...initialState, columnWidths: { column1: { width: 150 } } };
const action = {
type: 'discover/setColumnWidths',
payload: { columnName: 'column2', width: 100 },
};
const result = discoverSlice.reducer(stateWithColumnWidths, action);
expect(result.columnWidths).toEqual({ column1: { width: 150 }, column2: { width: 100 } });
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import * as utils from './common';
import { SortOrder } from '../../../saved_searches/types';
import { DEFAULT_COLUMNS_SETTING, PLUGIN_ID } from '../../../../common';
import { ColumnWidths } from '../../components/data_grid/data_grid_table_columns';

export interface DiscoverState {
/**
Expand Down Expand Up @@ -52,6 +53,7 @@
*/
lineCount?: number;
};
columnWidths?: ColumnWidths;
}

export interface DiscoverRootState extends RootState {
Expand Down Expand Up @@ -86,6 +88,7 @@
preloadedState.state.columns = savedSearchInstance.columns;
preloadedState.state.sort = savedSearchInstance.sort;
preloadedState.state.savedSearch = savedSearchInstance.id;
preloadedState.state.columnWidths = savedSearchInstance.columnWidths;

Check warning on line 91 in src/plugins/discover/public/application/utils/state_management/discover_slice.tsx

View check run for this annotation

Codecov / codecov/patch

src/plugins/discover/public/application/utils/state_management/discover_slice.tsx#L91

Added line #L91 was not covered by tests
const indexPatternId = savedSearchInstance.searchSource.getField('index')?.id;
preloadedState.root = {
metadata: {
Expand Down Expand Up @@ -166,6 +169,11 @@
interval: action.payload,
};
},
setColumnWidths(state, action: PayloadAction<{ columnName: string; width: number }>) {
const { columnName, width } = action.payload;
if (!state.columnWidths) state.columnWidths = {};
state.columnWidths[columnName] = { width };
},
updateState(state, action: PayloadAction<Partial<DiscoverState>>) {
return {
...state,
Expand Down Expand Up @@ -200,6 +208,7 @@
setColumns,
setSort,
setInterval,
setColumnWidths,
setState,
updateState,
setSavedSearchId,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
*/

import React, { useCallback, useMemo } from 'react';
import { EuiDataGridProps } from '@elastic/eui';
import { DiscoverViewServices } from '../../../build_services';
import { useOpenSearchDashboards } from '../../../../../opensearch_dashboards_react/public';
import { DataGridTable } from '../../components/data_grid/data_grid_table';
Expand All @@ -14,6 +15,7 @@ import {
removeColumn,
setColumns,
setSort,
setColumnWidths,
useDispatch,
useSelector,
} from '../../utils/state_management';
Expand Down Expand Up @@ -41,7 +43,7 @@ export const DiscoverTable = ({ rows, scrollToTop }: Props) => {
} = services;

const { refetch$, indexPattern, savedSearch } = useDiscoverContext();
const { columns, sort } = useSelector((state) => state.discover);
const { columns, sort, columnWidths } = useSelector((state) => state.discover);
const dispatch = useDispatch();
const onAddColumn = (col: string) => {
if (indexPattern && capabilities.discover?.save) {
Expand Down Expand Up @@ -85,6 +87,14 @@ export const DiscoverTable = ({ rows, scrollToTop }: Props) => {
},
[filterManager, indexPattern]
);

const onColumnResize: EuiDataGridProps['onColumnResize'] = useCallback(
({ columnId, width }: { columnId: string; width: number }) => {
dispatch(setColumnWidths({ columnName: columnId, width }));
},
[dispatch]
);

const displayTimeColumn = useMemo(
() => !!(!uiSettings.get(DOC_HIDE_TIME_COLUMN_SETTING, false) && indexPattern?.isTimeBased()),
[indexPattern, uiSettings]
Expand All @@ -103,13 +113,15 @@ export const DiscoverTable = ({ rows, scrollToTop }: Props) => {
return (
<DataGridTable
columns={columns}
columnWidths={columnWidths}
indexPattern={indexPattern}
onAddColumn={onAddColumn}
onFilter={onAddFilter as DocViewFilterFn}
onMoveColumn={onMoveColumn}
onRemoveColumn={onRemoveColumn}
onSetColumns={onSetColumns}
onSort={onSetSort}
onColumnResize={onColumnResize}
sort={sort}
rows={rows}
displayTimeColumn={displayTimeColumn}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,9 @@ export type RefetchSubject = Subject<SearchRefetch>;
export const useSearch = (services: DiscoverViewServices) => {
const initalSearchComplete = useRef(false);
const [savedSearch, setSavedSearch] = useState<SavedSearch | undefined>(undefined);
const { savedSearch: savedSearchId, sort, interval } = useSelector((state) => state.discover);
const { savedSearch: savedSearchId, sort, interval, columnWidths } = useSelector(
(state) => state.discover
);
const { data, filterManager, getSavedSearchById, core, toastNotifications, chrome } = services;
const indexPattern = useIndexPattern(services);
const timefilter = data.query.timefilter.timefilter;
Expand Down
Loading
Loading