Skip to content

Commit

Permalink
fix: fix columns width on long rowset (#1384)
Browse files Browse the repository at this point in the history
  • Loading branch information
astandrik authored Oct 2, 2024
1 parent e63d22b commit f30a38f
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 22 deletions.
35 changes: 14 additions & 21 deletions src/components/QueryResultTable/QueryResultTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {ResizeableDataTable} from '../ResizeableDataTable/ResizeableDataTable';

import {Cell} from './Cell';
import i18n from './i18n';
import {getColumnWidth} from './utils/getColumnWidth';

import './QueryResultTable.scss';

Expand All @@ -21,30 +22,27 @@ const TABLE_SETTINGS: Settings = {
stripedRows: true,
dynamicRenderType: 'variable',
dynamicItemSizeGetter: () => 40,
sortable: false,
};

export const b = cn('ydb-query-result-table');

const prepareTypedColumns = (columns: ColumnType[]) => {
const WIDTH_PREDICTION_ROWS_COUNT = 100;

const prepareTypedColumns = (columns: ColumnType[], data?: KeyValueRow[]) => {
if (!columns.length) {
return [];
}

const dataSlice = data?.slice(0, WIDTH_PREDICTION_ROWS_COUNT);

return columns.map(({name, type}) => {
const columnType = getColumnType(type);

const column: Column<KeyValueRow> = {
name,
width: getColumnWidth({data: dataSlice, name}),
align: columnType === 'number' ? DataTable.RIGHT : DataTable.LEFT,
sortAccessor: (row) => {
const value = row[name];

if (value === undefined || value === null) {
return null;
}

return columnType === 'number' ? BigInt(value) : value;
},
render: ({row}) => <Cell value={String(row[name])} />,
};

Expand All @@ -57,11 +55,13 @@ const prepareGenericColumns = (data: KeyValueRow[]) => {
return [];
}

const dataSlice = data?.slice(0, WIDTH_PREDICTION_ROWS_COUNT);

return Object.keys(data[0]).map((name) => {
const column: Column<KeyValueRow> = {
name,
width: getColumnWidth({data: dataSlice, name}),
align: isNumeric(data[0][name]) ? DataTable.RIGHT : DataTable.LEFT,
sortAccessor: (row) => (isNumeric(row[name]) ? Number(row[name]) : row[name]),
render: ({row}) => <Cell value={String(row[name])} />,
};

Expand All @@ -78,19 +78,12 @@ interface QueryResultTableProps
}

export const QueryResultTable = (props: QueryResultTableProps) => {
const {columns: rawColumns, data: rawData, settings: settingsMix, ...restProps} = props;
const {columns: rawColumns, data: rawData, ...restProps} = props;

const data = React.useMemo(() => prepareQueryResponse(rawData), [rawData]);
const columns = React.useMemo(() => {
return rawColumns ? prepareTypedColumns(rawColumns) : prepareGenericColumns(data);
return rawColumns ? prepareTypedColumns(rawColumns, data) : prepareGenericColumns(data);
}, [data, rawColumns]);
const settings = React.useMemo(
() => ({
...TABLE_SETTINGS,
...settingsMix,
}),
[settingsMix],
);

// empty data is expected to be be an empty array
// undefined data is not rendered at all
Expand All @@ -106,7 +99,7 @@ export const QueryResultTable = (props: QueryResultTableProps) => {
<ResizeableDataTable
data={data}
columns={columns}
settings={settings}
settings={TABLE_SETTINGS}
// prevent accessing row.id in case it is present but is not the PK (i.e. may repeat)
rowKey={getRowIndex}
{...restProps}
Expand Down
37 changes: 37 additions & 0 deletions src/components/QueryResultTable/utils/getColumnWidth.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import {HEADER_PADDING, MAX_COLUMN_WIDTH, getColumnWidth} from './getColumnWidth';

describe('getColumnWidth', () => {
it('returns minimum width for empty data', () => {
const result = getColumnWidth({data: [], name: 'test'});
expect(result).toBe(HEADER_PADDING + 'test'.length * 10);
});

it('calculates correct width for string columns', () => {
const data = [{test: 'short'}, {test: 'medium length'}, {test: 'this is a longer string'}];
const result = getColumnWidth({data, name: 'test'});
expect(result).toBe(HEADER_PADDING + 'this is a longer string'.length * 10);
});

it('returns MAX_COLUMN_WIDTH when calculated width exceeds it', () => {
const data = [{test: 'a'.repeat(100)}];
const result = getColumnWidth({data, name: 'test'});
expect(result).toBe(MAX_COLUMN_WIDTH);
});

it('handles undefined data correctly', () => {
const result = getColumnWidth({name: 'test'});
expect(result).toBe(HEADER_PADDING + 'test'.length * 10);
});

it('handles missing values in data correctly', () => {
const data = [{test: 'short'}, {}, {test: 'longer string'}];
const result = getColumnWidth({data, name: 'test'});
expect(result).toBe(HEADER_PADDING + 'longer string'.length * 10);
});

it('uses column name length when all values are shorter', () => {
const data = [{longColumnName: 'a'}, {longColumnName: 'bb'}];
const result = getColumnWidth({data, name: 'longColumnName'});
expect(result).toBe(HEADER_PADDING + 'longColumnName'.length * 10);
});
});
21 changes: 21 additions & 0 deletions src/components/QueryResultTable/utils/getColumnWidth.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import type {KeyValueRow} from '../../../types/api/query';

export const MAX_COLUMN_WIDTH = 600;
export const HEADER_PADDING = 20;

export const getColumnWidth = ({data, name}: {data?: KeyValueRow[]; name: string}) => {
let maxColumnContentLength = name.length;

if (data) {
for (const row of data) {
const cellLength = row[name] ? String(row[name]).length : 0;
maxColumnContentLength = Math.max(maxColumnContentLength, cellLength);

if (maxColumnContentLength * 10 + HEADER_PADDING >= MAX_COLUMN_WIDTH) {
return MAX_COLUMN_WIDTH;
}
}
}

return maxColumnContentLength * 10 + HEADER_PADDING;
};
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,6 @@ export function ExecuteResult({
<QueryResultTable
data={currentResult?.result}
columns={currentResult?.columns}
settings={{sortable: false}}
/>
</div>
</div>
Expand Down

0 comments on commit f30a38f

Please sign in to comment.