Skip to content

Commit

Permalink
perf(native-filters): reduce the re-rendering of native filter modal (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
stephenLYZ committed Oct 31, 2022
1 parent bf00193 commit 66f166b
Show file tree
Hide file tree
Showing 5 changed files with 61 additions and 40 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,18 @@
* specific language governing permissions and limitations
* under the License.
*/
import React, { useState } from 'react';
import React, { useCallback, useState } from 'react';
import { useDispatch } from 'react-redux';
import { setFilterConfiguration } from 'src/dashboard/actions/nativeFilters';
import Button from 'src/components/Button';
import { FilterConfiguration, styled } from '@superset-ui/core';
import { FiltersConfigModal } from 'src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal';
import FiltersConfigModal from 'src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal';
import { getFilterBarTestId } from '..';

export interface FCBProps {
createNewOnOpen?: boolean;
dashboardId?: number;
children?: React.ReactNode;
}

const HeaderButton = styled(Button)`
Expand All @@ -41,14 +42,21 @@ export const FilterConfigurationLink: React.FC<FCBProps> = ({
const dispatch = useDispatch();
const [isOpen, setOpen] = useState(false);

function close() {
const close = useCallback(() => {
setOpen(false);
}
}, [setOpen]);

async function submit(filterConfig: FilterConfiguration) {
dispatch(await setFilterConfiguration(filterConfig));
close();
}
const submit = useCallback(
async (filterConfig: FilterConfiguration) => {
dispatch(await setFilterConfiguration(filterConfig));
close();
},
[dispatch, close],
);

const handleClick = useCallback(() => {
setOpen(true);
}, [setOpen]);

return (
<>
Expand All @@ -57,7 +65,7 @@ export const FilterConfigurationLink: React.FC<FCBProps> = ({
{...getFilterBarTestId('create-filter')}
buttonStyle="link"
buttonSize="xsmall"
onClick={() => setOpen(true)}
onClick={handleClick}
>
{children}
</HeaderButton>
Expand All @@ -72,4 +80,4 @@ export const FilterConfigurationLink: React.FC<FCBProps> = ({
);
};

export default FilterConfigurationLink;
export default React.memo(FilterConfigurationLink);
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
*/
/* eslint-disable no-param-reassign */
import { css, styled, t, useTheme } from '@superset-ui/core';
import React, { FC } from 'react';
import React, { FC, useMemo } from 'react';
import Icons from 'src/components/Icons';
import Button from 'src/components/Button';
import { useSelector } from 'react-redux';
Expand Down Expand Up @@ -74,7 +74,7 @@ const AddFiltersButtonContainer = styled.div`
const Header: FC<HeaderProps> = ({ toggleFiltersBar }) => {
const theme = useTheme();
const filters = useFilters();
const filterValues = Object.values(filters);
const filterValues = useMemo(() => Object.values(filters), [filters]);
const canEdit = useSelector<RootState, boolean>(
({ dashboardInfo }) => dashboardInfo.dash_edit_perm,
);
Expand Down Expand Up @@ -109,4 +109,4 @@ const Header: FC<HeaderProps> = ({ toggleFiltersBar }) => {
);
};

export default Header;
export default React.memo(Header);
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,7 @@ import {
TimeFilterPlugin,
TimeGrainFilterPlugin,
} from 'src/filters/components';
import {
FiltersConfigModal,
import FiltersConfigModal, {
FiltersConfigModalProps,
} from './FiltersConfigModal';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import React, {
useState,
useRef,
} from 'react';
import { uniq, isEqual, sortBy, debounce } from 'lodash';
import { uniq, isEqual, sortBy, debounce, isEmpty } from 'lodash';
import {
Filter,
FilterConfiguration,
Expand Down Expand Up @@ -91,6 +91,12 @@ export interface FiltersConfigModalProps {
}
export const ALLOW_DEPENDENCIES = ['filter_select'];

const DEFAULT_EMPTY_FILTERS: string[] = [];
const DEFAULT_REMOVED_FILTERS: Record<string, FilterRemoval> = {};
const DEFAULT_FORM_VALUES: NativeFiltersForm = {
filters: {},
};

/**
* This is the modal to configure all the dashboard-native filters.
* Manages modal-level state, such as what filters are in the list,
Expand All @@ -99,7 +105,7 @@ export const ALLOW_DEPENDENCIES = ['filter_select'];
* Calls the `save` callback with the new FilterConfiguration object
* when the user saves the filters.
*/
export function FiltersConfigModal({
function FiltersConfigModal({
isOpen,
initialFilterId,
createNewOnOpen,
Expand All @@ -116,14 +122,16 @@ export function FiltersConfigModal({

// new filter ids belong to filters have been added during
// this configuration session, and only exist in the form state until we submit.
const [newFilterIds, setNewFilterIds] = useState<string[]>([]);
const [newFilterIds, setNewFilterIds] = useState<string[]>(
DEFAULT_EMPTY_FILTERS,
);

// store ids of filters that have been removed with the time they were removed
// so that we can disappear them after a few secs.
// filters are still kept in state until form is submitted.
const [removedFilters, setRemovedFilters] = useState<
Record<string, FilterRemoval>
>({});
>(DEFAULT_REMOVED_FILTERS);

const [saveAlertVisible, setSaveAlertVisible] = useState<boolean>(false);

Expand All @@ -143,13 +151,14 @@ export function FiltersConfigModal({
const [currentFilterId, setCurrentFilterId] = useState(
initialCurrentFilterId,
);
const [erroredFilters, setErroredFilters] = useState<string[]>([]);
const [erroredFilters, setErroredFilters] = useState<string[]>(
DEFAULT_EMPTY_FILTERS,
);

// the form values are managed by the antd form, but we copy them to here
// so that we can display them (e.g. filter titles in the tab headers)
const [formValues, setFormValues] = useState<NativeFiltersForm>({
filters: {},
});
const [formValues, setFormValues] =
useState<NativeFiltersForm>(DEFAULT_FORM_VALUES);

const unsavedFiltersIds = newFilterIds.filter(id => !removedFilters[id]);
// brings back a filter that was previously removed ("Undo")
Expand All @@ -162,12 +171,14 @@ export function FiltersConfigModal({
},
[removedFilters],
);
const getInitialFilterOrder = () => Object.keys(filterConfigMap);
const initialFilterOrder = useMemo(
() => Object.keys(filterConfigMap),
[filterConfigMap],
);

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

// State for rendered filter to improve performance
const [renderedFilters, setRenderedFilters] = useState<string[]>([
Expand Down Expand Up @@ -225,17 +236,17 @@ export function FiltersConfigModal({
// After this, it should be as if the modal was just opened fresh.
// Called when the modal is closed.
const resetForm = (isSaving = false) => {
setNewFilterIds([]);
setNewFilterIds(DEFAULT_EMPTY_FILTERS);
setCurrentFilterId(initialCurrentFilterId);
setRemovedFilters({});
setRemovedFilters(DEFAULT_REMOVED_FILTERS);
setSaveAlertVisible(false);
setFormValues({ filters: {} });
setErroredFilters([]);
setFormValues(DEFAULT_FORM_VALUES);
setErroredFilters(DEFAULT_EMPTY_FILTERS);
if (filterIds.length > 0) {
setActiveFilterPanelKey(getActiveFilterPanelKey(filterIds[0]));
}
if (!isSaving) {
setOrderedFilters(getInitialFilterOrder());
setOrderedFilters(initialFilterOrder);
}
setRenderedFilters([initialCurrentFilterId]);
form.resetFields(['filters']);
Expand Down Expand Up @@ -330,7 +341,7 @@ export function FiltersConfigModal({

// no form validation issues found, resets errored filters
if (!erroredFiltersIds.length && erroredFilters.length > 0) {
setErroredFilters([]);
setErroredFilters(DEFAULT_EMPTY_FILTERS);
return;
}
// form validation issues found, sets errored filters
Expand Down Expand Up @@ -373,10 +384,9 @@ export function FiltersConfigModal({

const handleCancel = () => {
const changed = form.getFieldValue('changed');
const initialOrder = getInitialFilterOrder();
const didChangeOrder =
orderedFilters.length !== initialOrder.length ||
orderedFilters.some((val, index) => val !== initialOrder[index]);
orderedFilters.length !== initialFilterOrder.length ||
orderedFilters.some((val, index) => val !== initialFilterOrder[index]);
if (
unsavedFiltersIds.length > 0 ||
form.isFieldsTouched() ||
Expand Down Expand Up @@ -479,9 +489,11 @@ export function FiltersConfigModal({
);

useEffect(() => {
setErroredFilters(prevErroredFilters =>
prevErroredFilters.filter(f => !removedFilters[f]),
);
if (!isEmpty(removedFilters)) {
setErroredFilters(prevErroredFilters =>
prevErroredFilters.filter(f => !removedFilters[f]),
);
}
}, [removedFilters]);

useEffect(() => {
Expand Down Expand Up @@ -601,3 +613,5 @@ export function FiltersConfigModal({
</StyledModalWrapper>
);
}

export default React.memo(FiltersConfigModal);
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import waitForComponentToPaint from 'spec/helpers/waitForComponentToPaint';
import { AntdDropdown } from 'src/components';
import { Menu } from 'src/components/Menu';
import Alert from 'src/components/Alert';
import { FiltersConfigModal } from 'src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal';
import FiltersConfigModal from 'src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal';

Object.defineProperty(window, 'matchMedia', {
writable: true,
Expand Down

0 comments on commit 66f166b

Please sign in to comment.