Skip to content

Commit

Permalink
perf(native-filters): improve native filter modal form performance (#…
Browse files Browse the repository at this point in the history 
…21821)
  • Loading branch information
@stephenLYZ
stephenLYZ committed Oct 30, 2022
1 parent 3ea8f20 commit bf00193
Show file tree
Hide file tree
Showing 5 changed files with 148 additions and 106 deletions.
8 changes: 0 additions & 8 deletions ...ntend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterConfigPane.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ const scrollMock = jest.fn();
Element.prototype.scroll = scrollMock;

const defaultProps = {
children: jest.fn(),
getFilterTitle: (id: string) => id,
onChange: jest.fn(),
onAdd: jest.fn(),
Expand Down Expand Up @@ -63,13 +62,6 @@ beforeEach(() => {
scrollMock.mockClear();
});

test('renders form', async () => {
await act(async () => {
defaultRender();
});
expect(defaultProps.children).toHaveBeenCalledTimes(3);
});

test('drag and drop', async () => {
defaultRender();
// Drag the state and country filter above the product filter
Expand Down
56 changes: 20 additions & 36 deletions ...rontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterConfigurePane.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import FilterTitlePane from './FilterTitlePane';
import { FilterRemoval } from './types';

interface Props {
children: (filterId: string) => React.ReactNode;
children?: React.ReactNode;
getFilterTitle: (filterId: string) => string;
onChange: (activeKey: string) => void;
onAdd: (type: NativeFilterType) => void;
Expand Down Expand Up @@ -62,40 +62,24 @@ const FilterConfigurePane: React.FC<Props> = ({
currentFilterId,
filters,
removedFilters,
}) => {
const active = filters.filter(id => id === currentFilterId)[0];
return (
<Container>
<TitlesContainer>
<FilterTitlePane
currentFilterId={currentFilterId}
filters={filters}
removedFilters={removedFilters}
erroredFilters={erroredFilters}
getFilterTitle={getFilterTitle}
onChange={onChange}
onAdd={(type: NativeFilterType) => onAdd(type)}
onRearrange={onRearrange}
onRemove={(id: string) => onRemove(id)}
restoreFilter={restoreFilter}
/>
</TitlesContainer>
<ContentHolder>
{filters.map(id => (
<div
key={id}
style={{
height: '100%',
overflowY: 'auto',
display: id === active ? '' : 'none',
}}
>
{children(id)}
</div>
))}
</ContentHolder>
</Container>
);
};
}) => (
<Container>
<TitlesContainer>
<FilterTitlePane
currentFilterId={currentFilterId}
filters={filters}
removedFilters={removedFilters}
erroredFilters={erroredFilters}
getFilterTitle={getFilterTitle}
onChange={onChange}
onAdd={(type: NativeFilterType) => onAdd(type)}
onRearrange={onRearrange}
onRemove={(id: string) => onRemove(id)}
restoreFilter={restoreFilter}
/>
</TitlesContainer>
<ContentHolder>{children}</ContentHolder>
</Container>
);

export default FilterConfigurePane;
9 changes: 6 additions & 3 deletions ...board/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,7 @@ const FiltersConfigForm = (
setErroredFilters,
validateDependencies,
getDependencySuggestion,
isActive,
}: FiltersConfigFormProps,
ref: React.RefObject<any>,
) => {
Expand All @@ -346,7 +347,7 @@ const FiltersConfigForm = (
string,
any
> | null>(null);
const forceUpdate = useForceUpdate();
const forceUpdate = useForceUpdate(isActive);
const [datasetDetails, setDatasetDetails] = useState<Record<string, any>>();
const defaultFormFilter = useMemo(() => ({}), []);
const filters = form.getFieldValue('filters');
Expand Down Expand Up @@ -1240,6 +1241,8 @@ const FiltersConfigForm = (
);
};

export default forwardRef<typeof FiltersConfigForm, FiltersConfigFormProps>(
FiltersConfigForm,
export default React.memo(
forwardRef<typeof FiltersConfigForm, FiltersConfigFormProps>(
FiltersConfigForm,
),
);
8 changes: 6 additions & 2 deletions ...tend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,13 @@ export const FILTER_SUPPORTED_TYPES = {
filter_range: [GenericDataType.NUMERIC],
};

export const useForceUpdate = () => {
export const useForceUpdate = (isActive = true) => {
const [, updateState] = React.useState({});
return React.useCallback(() => updateState({}), []);
return React.useCallback(() => {
if (isActive) {
updateState({});
}
}, [isActive]);
};

export const setNativeFilterFieldValues = (
Expand Down
173 changes: 116 additions & 57 deletions ...frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,19 +153,27 @@ export function FiltersConfigModal({

const unsavedFiltersIds = newFilterIds.filter(id => !removedFilters[id]);
// brings back a filter that was previously removed ("Undo")
const restoreFilter = (id: string) => {
const removal = removedFilters[id];
// gotta clear the removal timeout to prevent the filter from getting deleted
if (removal?.isPending) clearTimeout(removal.timerId);
setRemovedFilters(current => ({ ...current, [id]: null }));
};
const restoreFilter = useCallback(
(id: string) => {
const removal = removedFilters[id];
// gotta clear the removal timeout to prevent the filter from getting deleted
if (removal?.isPending) clearTimeout(removal.timerId);
setRemovedFilters(current => ({ ...current, [id]: null }));
},
[removedFilters],
);
const getInitialFilterOrder = () => Object.keys(filterConfigMap);

// State for tracking the re-ordering of filters
const [orderedFilters, setOrderedFilters] = useState<string[]>(
getInitialFilterOrder(),
);

// State for rendered filter to improve performance
const [renderedFilters, setRenderedFilters] = useState<string[]>([
initialCurrentFilterId,
]);

const getActiveFilterPanelKey = (filterId: string) => [
`${filterId}-${FilterPanels.configuration.key}`,
`${filterId}-${FilterPanels.settings.key}`,
Expand All @@ -175,7 +183,7 @@ export function FiltersConfigModal({
string | string[]
>(getActiveFilterPanelKey(initialCurrentFilterId));

const onTabChange = (filterId: string) => {
const handleTabChange = (filterId: string) => {
setCurrentFilterId(filterId);
setActiveFilterPanelKey(getActiveFilterPanelKey(filterId));
};
Expand Down Expand Up @@ -229,6 +237,7 @@ export function FiltersConfigModal({
if (!isSaving) {
setOrderedFilters(getInitialFilterOrder());
}
setRenderedFilters([initialCurrentFilterId]);
form.resetFields(['filters']);
form.setFieldsValue({ changed: false });
};
Expand Down Expand Up @@ -379,7 +388,7 @@ export function FiltersConfigModal({
handleConfirmCancel();
}
};
const onRearrange = (dragIndex: number, targetIndex: number) => {
const handleRearrange = (dragIndex: number, targetIndex: number) => {
const newOrderedFilter = [...orderedFilters];
const removed = newOrderedFilter.splice(dragIndex, 1)[0];
newOrderedFilter.splice(targetIndex, 0, removed);
Expand All @@ -405,7 +414,7 @@ export function FiltersConfigModal({
return dependencyMap;
}, [filterConfigMap, form]);

const validateDependencies = () => {
const validateDependencies = useCallback(() => {
const dependencyMap = buildDependencyMap();
filterIds
.filter(id => !removedFilters[id])
Expand All @@ -418,26 +427,35 @@ export function FiltersConfigModal({
form.setFields([field]);
});
handleErroredFilters();
};
}, [
buildDependencyMap,
filterIds,
form,
handleErroredFilters,
removedFilters,
]);

const getDependencySuggestion = (filterId: string) => {
const dependencyMap = buildDependencyMap();
const possibleDependencies = orderedFilters.filter(
key => key !== filterId && canBeUsedAsDependency(key),
);
const found = possibleDependencies.find(filter => {
const dependencies = dependencyMap.get(filterId) || [];
dependencies.push(filter);
if (hasCircularDependency(dependencyMap, filterId)) {
dependencies.pop();
return false;
}
return true;
});
return found || possibleDependencies[0];
};
const getDependencySuggestion = useCallback(
(filterId: string) => {
const dependencyMap = buildDependencyMap();
const possibleDependencies = orderedFilters.filter(
key => key !== filterId && canBeUsedAsDependency(key),
);
const found = possibleDependencies.find(filter => {
const dependencies = dependencyMap.get(filterId) || [];
dependencies.push(filter);
if (hasCircularDependency(dependencyMap, filterId)) {
dependencies.pop();
return false;
}
return true;
});
return found || possibleDependencies[0];
},
[buildDependencyMap, canBeUsedAsDependency, orderedFilters],
);

const onValuesChange = useMemo(
const handleValuesChange = useMemo(
() =>
debounce((changes: any, values: NativeFiltersForm) => {
const didChangeFilterName =
Expand Down Expand Up @@ -466,32 +484,73 @@ export function FiltersConfigModal({
);
}, [removedFilters]);

const getForm = (id: string) => {
const isDivider = id.startsWith(NATIVE_FILTER_DIVIDER_PREFIX);
return isDivider ? (
<DividerConfigForm
componentId={id}
divider={filterConfigMap[id] as Divider}
/>
) : (
<FiltersConfigForm
ref={configFormRef}
form={form}
filterId={id}
filterToEdit={filterConfigMap[id] as Filter}
removedFilters={removedFilters}
restoreFilter={restoreFilter}
getAvailableFilters={getAvailableFilters}
key={id}
activeFilterPanelKeys={activeFilterPanelKey}
handleActiveFilterPanelChange={key => setActiveFilterPanelKey(key)}
isActive={currentFilterId === id}
setErroredFilters={setErroredFilters}
validateDependencies={validateDependencies}
getDependencySuggestion={getDependencySuggestion}
/>
);
};
useEffect(() => {
if (!renderedFilters.includes(currentFilterId)) {
setRenderedFilters([...renderedFilters, currentFilterId]);
}
}, [currentFilterId]);

const handleActiveFilterPanelChange = useCallback(
key => setActiveFilterPanelKey(key),
[setActiveFilterPanelKey],
);

const formList = useMemo(
() =>
orderedFilters.map(id => {
if (!renderedFilters.includes(id)) return null;
const isDivider = id.startsWith(NATIVE_FILTER_DIVIDER_PREFIX);
const isActive = currentFilterId === id;
return (
<div
key={id}
style={{
height: '100%',
overflowY: 'auto',
display: isActive ? '' : 'none',
}}
>
{isDivider ? (
<DividerConfigForm
componentId={id}
divider={filterConfigMap[id] as Divider}
/>
) : (
<FiltersConfigForm
ref={configFormRef}
form={form}
filterId={id}
filterToEdit={filterConfigMap[id] as Filter}
removedFilters={removedFilters}
restoreFilter={restoreFilter}
getAvailableFilters={getAvailableFilters}
key={id}
activeFilterPanelKeys={activeFilterPanelKey}
handleActiveFilterPanelChange={handleActiveFilterPanelChange}
isActive={isActive}
setErroredFilters={setErroredFilters}
validateDependencies={validateDependencies}
getDependencySuggestion={getDependencySuggestion}
/>
)}
</div>
);
}),
[
renderedFilters,
orderedFilters,
currentFilterId,
filterConfigMap,
form,
removedFilters,
restoreFilter,
getAvailableFilters,
activeFilterPanelKey,
validateDependencies,
getDependencySuggestion,
handleActiveFilterPanelChange,
],
);

return (
<StyledModalWrapper
Expand Down Expand Up @@ -519,22 +578,22 @@ export function FiltersConfigModal({
<StyledModalBody>
<StyledForm
form={form}
onValuesChange={onValuesChange}
onValuesChange={handleValuesChange}
layout="vertical"
>
<FilterConfigurePane
erroredFilters={erroredFilters}
onRemove={handleRemoveItem}
onAdd={addFilter}
onChange={onTabChange}
onChange={handleTabChange}
getFilterTitle={getFilterTitle}
currentFilterId={currentFilterId}
removedFilters={removedFilters}
restoreFilter={restoreFilter}
onRearrange={onRearrange}
onRearrange={handleRearrange}
filters={orderedFilters}
>
{(id: string) => getForm(id)}
{formList}
</FilterConfigurePane>
</StyledForm>
</StyledModalBody>
Expand Down

0 comments on commit bf00193

Please sign in to comment.