Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf(native-filters): reduce the re-rendering of native filter modal #21781

Merged
merged 4 commits into from
Oct 31, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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