Skip to content

Commit

Permalink
perf(weave): speed up string editor close in dataset editor
Browse files Browse the repository at this point in the history
  • Loading branch information
@bcsherma
bcsherma committed Mar 4, 2025
1 parent 60b0926 commit 2c0e296
Show file tree
Hide file tree
Showing 3 changed files with 99 additions and 52 deletions.
59 changes: 49 additions & 10 deletions weave-js/src/components/PagePanelComponents/Home/Browse3/datasets/CellRenderers.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ export const CellViewingRenderer: React.FC<
serverValue,
}) => {
const [isHovered, setIsHovered] = useState(false);
const {setEditedRows, setAddedRows} = useDatasetEditContext();
const {setEditedRows, setAddedRows, setFieldEdited} = useDatasetEditContext();

const isWeaveUrl = isRefPrefixedString(value);
const isEditable =
Expand All @@ -77,6 +77,7 @@ export const CellViewingRenderer: React.FC<
event.stopPropagation();
const existingRow = api.getRow(id);
const updatedRow = {...existingRow};

set(updatedRow, field, serverValue);
api.updateRows([{id, ...updatedRow}]);
api.setEditCellValue({id, field, value: serverValue});
Expand All @@ -85,6 +86,9 @@ export const CellViewingRenderer: React.FC<
newMap.set(existingRow.___weave?.index, updatedRow);
return newMap;
});
if (existingRow.___weave?.index !== undefined) {
setFieldEdited(existingRow.___weave.index, field, false);
}
};

const getBackgroundColor = () => {
Expand All @@ -107,16 +111,22 @@ export const CellViewingRenderer: React.FC<
const existingRow = api.getRow(id);
const updatedRow = {...existingRow, [field]: !value};
api.updateRows([{id, ...updatedRow}]);
const rowToUpdate = {...updatedRow};

if (existingRow.___weave?.isNew) {
setAddedRows(prev => {
const newMap = new Map(prev);
newMap.set(existingRow.___weave?.id, updatedRow);
newMap.set(existingRow.___weave?.id, rowToUpdate);
return newMap;
});
} else {
if (!rowToUpdate.___weave.editedFields) {
rowToUpdate.___weave.editedFields = new Set<string>();
}
rowToUpdate.___weave.editedFields.add(field);
setEditedRows(prev => {
const newMap = new Map(prev);
newMap.set(existingRow.___weave?.index, updatedRow);
newMap.set(existingRow.___weave?.index, rowToUpdate);
return newMap;
});
}
Expand Down Expand Up @@ -311,9 +321,10 @@ const NumberEditor: React.FC<{
api: any;
id: string | number;
field: string;
}> = ({value, onClose, api, id, field}) => {
serverValue?: any;
}> = ({value, onClose, api, id, field, serverValue}) => {
const [inputValue, setInputValue] = useState(value.toString());
const {setEditedRows, setAddedRows} = useDatasetEditContext();
const {setEditedRows, setAddedRows, setFieldEdited} = useDatasetEditContext();

const handleValueUpdate = (newValue: string) => {
setInputValue(newValue);
Expand All @@ -327,6 +338,8 @@ const NumberEditor: React.FC<{
if (inputValue !== '') {
const numValue = Number(inputValue);
const existingRow = api.getRow(id);
const isValueChanged = numValue !== serverValue;

if (existingRow.___weave?.isNew) {
setAddedRows((prev: Map<string, DatasetRow>) => {
const newMap = new Map(prev);
Expand All @@ -343,6 +356,9 @@ const NumberEditor: React.FC<{
newMap.set(existingRow.___weave?.index, updatedRow);
return newMap;
});
if (isValueChanged && existingRow.___weave?.index !== undefined) {
setFieldEdited(existingRow.___weave.index, field, isValueChanged);
}
}
}
onClose();
Expand Down Expand Up @@ -524,13 +540,13 @@ const StringEditor: React.FC<{

export const CellEditingRenderer: React.FC<
CellEditingRendererProps
> = params => {
> = props => {
const {setEditedRows, setAddedRows} = useDatasetEditContext();
const {id, value, field, api, serverValue, preserveFieldOrder} = params;
const {id, value, field, api, serverValue, preserveFieldOrder} = props;

// Convert edit params to render params
const renderParams: GridRenderCellParams = {
...params,
...props,
value,
};

Expand All @@ -557,6 +573,7 @@ export const CellEditingRenderer: React.FC<
api={api}
id={id}
field={field}
serverValue={serverValue}
/>
);
}
Expand All @@ -569,16 +586,38 @@ export const CellEditingRenderer: React.FC<
onClose={() => {
const existingRow = api.getRow(id);
const updatedRow = updateRow(existingRow, value);

const isValueChanged = value !== serverValue;
const rowToUpdate = {...updatedRow};

if (existingRow.___weave?.isNew) {
setAddedRows(prev => {
const newMap = new Map(prev);
newMap.set(existingRow.___weave?.id, updatedRow);
newMap.set(existingRow.___weave?.id, rowToUpdate);
return newMap;
});
} else {
if (!rowToUpdate.___weave.editedFields) {
rowToUpdate.___weave.editedFields = new Set<string>();
}

if (isValueChanged) {
rowToUpdate.___weave.editedFields.add(field);
} else {
rowToUpdate.___weave.editedFields.delete(field);
}

setEditedRows(prev => {
const newMap = new Map(prev);
newMap.set(existingRow.___weave?.index, updatedRow);

// If we don't have any edited fields and it's not a new row,
// don't add it to the editedRows map
if (rowToUpdate.___weave.editedFields.size === 0) {
newMap.delete(existingRow.___weave?.index);
} else {
newMap.set(existingRow.___weave?.index, rowToUpdate);
}

return newMap;
});
}
Expand Down
78 changes: 43 additions & 35 deletions weave-js/src/components/PagePanelComponents/Home/Browse3/datasets/DatasetEditorContext.tsx
View file
Original file line number Diff line number Diff line change
@@ -1,24 +1,28 @@
import isEqual from 'lodash/isEqual';
import React, {createContext, useCallback, useContext, useState} from 'react';

import {flattenObjectPreservingWeaveTypes} from '../flattenObject';

export interface DatasetRow {
[key: string]: any;
___weave: {
id: string;
index?: number;
isNew?: boolean;
serverValue?: any;
editedFields?: Set<string>; // Set of field paths that have been edited
};
}

interface DatasetEditContextType {
/** Map of complete edited rows, keyed by row absolute index */
editedRows: Map<number, DatasetRow>;
setEditedRows: React.Dispatch<React.SetStateAction<Map<number, DatasetRow>>>;
/** Get edited fields for a row */
getEditedFields: (rowIndex: number) => {[fieldName: string]: unknown};
/** Check if a specific field in a row has been edited */
isFieldEdited: (rowIndex: number, fieldName: string) => boolean;
/** Mark a field as edited or not edited within the row object */
setFieldEdited: (
rowIndex: number,
fieldName: string,
isEdited: boolean
) => void;
/** Array of row indices that have been marked for deletion */
deletedRows: number[];
setDeletedRows: React.Dispatch<React.SetStateAction<number[]>>;
Expand Down Expand Up @@ -66,46 +70,49 @@ export const DatasetEditProvider: React.FC<DatasetEditProviderProps> = ({
initialAddedRows || new Map()
);

const getEditedFields = useCallback(
(rowIndex: number) => {
const isFieldEdited = useCallback(
(rowIndex: number, fieldName: string): boolean => {
const editedRow = editedRows.get(rowIndex);
const originalRow = editedRow?.___weave?.serverValue ?? editedRow;
if (!editedRow) {
return {};
return false;
}
const flattenedOriginalRow =
flattenObjectPreservingWeaveTypes(originalRow);
const flattenedEditedRow = flattenObjectPreservingWeaveTypes(editedRow);
return Object.fromEntries(
Object.entries(flattenedEditedRow).filter(
([key, value]) =>
!key.startsWith('___weave') &&
!isEqual(value, flattenedOriginalRow[key])
)
);

return editedRow.___weave?.editedFields?.has(fieldName) ?? false;
},
[editedRows]
);

// Cleanup effect to remove rows that no longer have any edits
// from the editedRows map.
React.useEffect(() => {
const rowsToRemove: number[] = [];
editedRows.forEach((editedRow, rowIndex) => {
const fields = getEditedFields(rowIndex);
if (Object.keys(fields).length === 0) {
rowsToRemove.push(rowIndex);
}
});

if (rowsToRemove.length > 0) {
const setFieldEdited = useCallback(
(rowIndex: number, fieldName: string, isEdited: boolean) => {
setEditedRows(prev => {
const newMap = new Map(prev);
rowsToRemove.forEach(index => newMap.delete(index));
const row = newMap.get(rowIndex);

if (!row) {
return newMap;
}

if (!row.___weave.editedFields) {
row.___weave.editedFields = new Set<string>();
}

if (isEdited) {
row.___weave.editedFields.add(fieldName);
} else {
row.___weave.editedFields.delete(fieldName);
}

if (row.___weave.editedFields.size === 0 && !row.___weave.isNew) {
newMap.delete(rowIndex);
} else {
newMap.set(rowIndex, row);
}

return newMap;
});
}
}, [editedRows, getEditedFields]);
},
[setEditedRows]
);

const reset = useCallback(() => {
setEditedRows(new Map());
Expand Down Expand Up @@ -172,7 +179,8 @@ export const DatasetEditProvider: React.FC<DatasetEditProviderProps> = ({
value={{
editedRows,
setEditedRows,
getEditedFields,
isFieldEdited,
setFieldEdited,
deletedRows,
setDeletedRows,
addedRows,
Expand Down
14 changes: 7 additions & 7 deletions weave-js/src/components/PagePanelComponents/Home/Browse3/datasets/EditableDatasetView.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,11 +91,11 @@ export const EditableDatasetView: React.FC<EditableDatasetViewProps> = ({

const {
editedRows,
getEditedFields,
deletedRows,
setDeletedRows,
setAddedRows,
addedRows,
isFieldEdited,
} = useDatasetEditContext();

const [paginationModel, setPaginationModel] = useState<GridPaginationModel>({
Expand Down Expand Up @@ -428,14 +428,14 @@ export const EditableDatasetView: React.FC<EditableDatasetViewProps> = ({
}
const rowIndex = params.row.___weave?.index;

const editedFields =
rowIndex != null && !params.row.___weave?.isNew
? getEditedFields(rowIndex)
: {};
return (
<CellViewingRenderer
{...params}
isEdited={editedFields[field as string] !== undefined}
isEdited={
rowIndex != null && !params.row.___weave?.isNew
? isFieldEdited(rowIndex, field as string)
: false
}
isDeleted={deletedRows.includes(params.row.___weave?.index)}
isNew={params.row.___weave?.isNew}
serverValue={get(
Expand Down Expand Up @@ -464,7 +464,6 @@ export const EditableDatasetView: React.FC<EditableDatasetViewProps> = ({
return [...baseColumns, ...fieldColumns];
}, [
combinedRows,
getEditedFields,
deleteRow,
restoreRow,
deletedRows,
Expand All @@ -477,6 +476,7 @@ export const EditableDatasetView: React.FC<EditableDatasetViewProps> = ({
columnWidths,
preserveFieldOrder,
hideRemoveForAddedRows,
isFieldEdited,
]);

const handleColumnWidthChange = useCallback((params: any) => {
Expand Down

0 comments on commit 2c0e296

Please sign in to comment.