Skip to content

Conversation

@SychevAndrey
Copy link
Contributor

@SychevAndrey SychevAndrey commented Jan 21, 2026

Screen.Recording.2026-01-21.at.11.02.45.mov

@SychevAndrey SychevAndrey self-assigned this Jan 21, 2026
SychevAndrey and others added 6 commits January 21, 2026 10:37
…or' of github.com:dbeaver/cloudbeaver into 7023-cb-find-and-replace-functionality-in-the-data-editor
after some tests, added only debounce for user input, even on the big tables sync search doesn't show large degradation of ux/response time
@SychevAndrey SychevAndrey marked this pull request as ready for review January 21, 2026 12:14
Comment on lines 101 to 102
'rdg-cell-search-match': searchState.isMatch(rowIdx, colIdx),
'rdg-cell-search-active': searchState.isActiveMatch(rowIdx, colIdx),
Copy link
Member

Choose a reason for hiding this comment

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

This is a very heavy operation, you should put them in the computed or consider having a fast computed table on every search (so you can directly get information from this table without search operations, like Map<number, Map<number, SearchInfo>>

const SEARCH_DEBOUNCE_MS = 300;


export class DataGridSearchStore implements IDataGridSearchStateInternal {
Copy link
Member

Choose a reason for hiding this comment

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

This seems to be overcomplicated. Maybe it's not a good idea to complete this task right now, but better to decompose it into smaller tasks and plan its implementation again. We can keep transfer of the search panel to the ui-kit but it needs some improvements (localization)

Comment on lines 157 to 163
scrollToCell: (position: Partial<ICellPosition>) => {
innerGridRef.current?.scrollToCell({ idx: position.colIdx && dndHeaderContext.getDataColIdx(position.colIdx), rowIdx: position.rowIdx });
},
scrollToDataCell: (position: Partial<ICellPosition> & { colIdx?: number }) => {
const virtualIdx = position.colIdx !== undefined ? dndHeaderContext.getVirtualColIdx(position.colIdx) : undefined;
innerGridRef.current?.scrollToCell({ idx: virtualIdx, rowIdx: position.rowIdx });
},
Copy link
Member

Choose a reason for hiding this comment

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

scrollToCell should be enough, I think that we need to fix its behaviour because of dndHeaderContext.getVirtualColIdx

scrollToDataCell: (position: Partial<ICellPosition> & { colIdx?: number }) => void;
openEditor: (position: ICellPosition) => void;
getColumnsOrdered: () => readonly CalculatedColumn<IInnerRow, unknown>[];
getDataColIdxByKey: (key: string) => number | null;
Copy link
Member

Choose a reason for hiding this comment

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

getDataColIdxByKey should not be exposed

Comment on lines +94 to +111
getRowCount: () => tableData.rows.length,
getColumnCount: () => tableData.columns.length,
getCellText: (rowIdx: number, colIdx: number) => {
const row = tableData.rows[rowIdx];
const column = tableData.getColumn(colIdx)?.key;
if (!row || !column) {
return '';
}
return tableData.format.getText(tableData.format.get({ row, column }));
},
setCellValue: (rowIdx: number, colIdx: number, value: string) => {
const row = tableData.rows[rowIdx];
const column = tableData.getColumn(colIdx)?.key;
if (!row || !column) {
return;
}
tableData.editor?.set({ row, column }, value);
},
Copy link
Member

Choose a reason for hiding this comment

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

all this data is available in the common-react/@dbeaver/react-data-grid/src/DataGrid.tsx

Comment on lines +14 to +25
export class GridSearchService {
private readonly stores = new Map<string, GridSearchStore>();

getOrCreateStore(key: string): GridSearchStore {
if (this.stores.has(key)) {
return this.stores.get(key)!;
}

const store = new GridSearchStore(createDefaultSearchCache());
this.stores.set(key, store);
return store;
}
Copy link
Member

Choose a reason for hiding this comment

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

I don't think that we need a global service and store for this feature, because for different tables state can be stored differently

Comment on lines +123 to +124
this.reactionDisposers.push(
reaction(
Copy link
Member

Choose a reason for hiding this comment

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

You should not use MobX here; it's better to use imperative logic in such cases

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants