Skip to content

Commit

Permalink
fix(explore): drag & drop column select component triggering onChange…
Browse files Browse the repository at this point in the history
… unnecessarily (apache#16073)

* fix(explore): avoid sync until after first render

* fix example
  • Loading branch information
suddjian authored Aug 5, 2021
1 parent 2307216 commit e6292a8
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
import React, { useCallback, useEffect, useMemo } from 'react';
import React, { useCallback, useMemo } from 'react';
import { tn } from '@superset-ui/core';
import { ColumnMeta } from '@superset-ui/chart-controls';
import { isEmpty } from 'lodash';
Expand All @@ -27,6 +27,7 @@ import { OptionSelector } from 'src/explore/components/controls/DndColumnSelectC
import { DatasourcePanelDndItem } from 'src/explore/components/DatasourcePanel/types';
import { DndItemType } from 'src/explore/components/DndItemType';
import { StyledColumnOption } from 'src/explore/components/optionRenderers';
import { useComponentDidUpdate } from 'src/common/hooks/useComponentDidUpdate';

export const DndColumnSelect = (props: LabelProps) => {
const {
Expand All @@ -45,7 +46,7 @@ export const DndColumnSelect = (props: LabelProps) => {
);

// synchronize values in case of dataset changes
useEffect(() => {
const handleOptionsChange = useCallback(() => {
const optionSelectorValues = optionSelector.getValues();
if (typeof value !== typeof optionSelectorValues) {
onChange(optionSelectorValues);
Expand All @@ -65,8 +66,13 @@ export const DndColumnSelect = (props: LabelProps) => {
) {
onChange(optionSelectorValues);
}
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [JSON.stringify(value), JSON.stringify(optionSelector.getValues())]);
// when options change, that means that the dataset has changed
// so we have to check if values are still applicable.
}, [options, value, optionSelector]);

// useComponentDidUpdate to avoid running this for the first render, to avoid
// calling onChange when the initial value is not valid for the dataset
useComponentDidUpdate(handleOptionsChange);

const onDrop = useCallback(
(item: DatasourcePanelDndItem) => {
Expand Down
2 changes: 1 addition & 1 deletion superset/examples/random_time_series.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ def load_random_time_series_data(
tbl = obj

slice_data = {
"granularity_sqla": "day",
"granularity_sqla": "ds",
"row_limit": app.config["ROW_LIMIT"],
"since": "2019-01-01",
"until": "2019-02-01",
Expand Down

0 comments on commit e6292a8

Please sign in to comment.