fix(explore): fix undefined error when using dnd (#16020)

2026-04-23 18:14:56 +00:00 · 2021-08-02 15:22:32 +02:00
parent c77bf264d1
commit 3061b6ad09
9 changed files with 395 additions and 277 deletions
--- a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndColumnSelect.tsx
+++ b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndColumnSelect.tsx
@@ -16,7 +16,7 @@
 * specific language governing permissions and limitations
 * under the License.
 */
-import React, { useEffect } from 'react';
+import React, { useCallback, useEffect, useMemo } from 'react';
 import { tn } from '@superset-ui/core';
 import { ColumnMeta } from '@superset-ui/chart-controls';
 import { isEmpty } from 'lodash';
@@ -36,8 +36,13 @@ export const DndColumnSelect = (props: LabelProps) => {
    onChange,
    canDelete = true,
    ghostButtonText,
+    name,
+    label,
  } = props;
-  const optionSelector = new OptionSelector(options, multi, value);
+  const optionSelector = useMemo(
+    () => new OptionSelector(options, multi, value),
+    [multi, options, value],
+  );

  // synchronize values in case of dataset changes
  useEffect(() => {
@@ -63,46 +68,68 @@ export const DndColumnSelect = (props: LabelProps) => {
    // eslint-disable-next-line react-hooks/exhaustive-deps
  }, [JSON.stringify(value), JSON.stringify(optionSelector.getValues())]);

-  const onDrop = (item: DatasourcePanelDndItem) => {
-    const column = item.value as ColumnMeta;
-    if (!optionSelector.multi && !isEmpty(optionSelector.values)) {
-      optionSelector.replace(0, column.column_name);
-    } else {
-      optionSelector.add(column.column_name);
-    }
-    onChange(optionSelector.getValues());
-  };
+  const onDrop = useCallback(
+    (item: DatasourcePanelDndItem) => {
+      const column = item.value as ColumnMeta;
+      if (!optionSelector.multi && !isEmpty(optionSelector.values)) {
+        optionSelector.replace(0, column.column_name);
+      } else {
+        optionSelector.add(column.column_name);
+      }
+      onChange(optionSelector.getValues());
+    },
+    [onChange, optionSelector],
+  );

-  const canDrop = (item: DatasourcePanelDndItem) => {
-    const columnName = (item.value as ColumnMeta).column_name;
-    return (
-      columnName in optionSelector.options && !optionSelector.has(columnName)
-    );
-  };
+  const canDrop = useCallback(
+    (item: DatasourcePanelDndItem) => {
+      const columnName = (item.value as ColumnMeta).column_name;
+      return (
+        columnName in optionSelector.options && !optionSelector.has(columnName)
+      );
+    },
+    [optionSelector],
+  );

-  const onClickClose = (index: number) => {
-    optionSelector.del(index);
-    onChange(optionSelector.getValues());
-  };
+  const onClickClose = useCallback(
+    (index: number) => {
+      optionSelector.del(index);
+      onChange(optionSelector.getValues());
+    },
+    [onChange, optionSelector],
+  );

-  const onShiftOptions = (dragIndex: number, hoverIndex: number) => {
-    optionSelector.swap(dragIndex, hoverIndex);
-    onChange(optionSelector.getValues());
-  };
+  const onShiftOptions = useCallback(
+    (dragIndex: number, hoverIndex: number) => {
+      optionSelector.swap(dragIndex, hoverIndex);
+      onChange(optionSelector.getValues());
+    },
+    [onChange, optionSelector],
+  );

-  const valuesRenderer = () =>
-    optionSelector.values.map((column, idx) => (
-      <OptionWrapper
-        key={idx}
-        index={idx}
-        clickClose={onClickClose}
-        onShiftOptions={onShiftOptions}
-        type={DndItemType.ColumnOption}
-        canDelete={canDelete}
-      >
-        <StyledColumnOption column={column} showType />
-      </OptionWrapper>
-    ));
+  const valuesRenderer = useCallback(
+    () =>
+      optionSelector.values.map((column, idx) => (
+        <OptionWrapper
+          key={idx}
+          index={idx}
+          clickClose={onClickClose}
+          onShiftOptions={onShiftOptions}
+          type={`${DndItemType.ColumnOption}_${name}_${label}`}
+          canDelete={canDelete}
+        >
+          <StyledColumnOption column={column} showType />
+        </OptionWrapper>
+      )),
+    [
+      canDelete,
+      label,
+      name,
+      onClickClose,
+      onShiftOptions,
+      optionSelector.values,
+    ],
+  );

  return (
    <DndSelectLabel<string | string[], ColumnMeta[]>
--- a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndFilterSelect.tsx
+++ b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndFilterSelect.tsx
@@ -16,7 +16,7 @@
 * specific language governing permissions and limitations
 * under the License.
 */
-import React, { useEffect, useMemo, useState } from 'react';
+import React, { useCallback, useEffect, useMemo, useState } from 'react';
 import {
  FeatureFlag,
  isFeatureEnabled,
@@ -50,10 +50,19 @@ import {
 } from 'src/explore/components/DatasourcePanel/types';
 import { DndItemType } from 'src/explore/components/DndItemType';

+const DND_ACCEPTED_TYPES = [
+  DndItemType.Column,
+  DndItemType.Metric,
+  DndItemType.MetricOption,
+  DndItemType.AdhocMetricOption,
+];
+
 const isDictionaryForAdhocFilter = (value: OptionValueType) =>
  !(value instanceof AdhocFilter) && value?.expressionType;

 export const DndFilterSelect = (props: DndFilterSelectProps) => {
+  const { datasource, onChange } = props;
+
  const propsValues = Array.from(props.value ?? []);
  const [values, setValues] = useState(
    propsValues.map((filter: OptionValueType) =>
@@ -117,7 +126,6 @@ export const DndFilterSelect = (props: DndFilterSelectProps) => {
  );

  useEffect(() => {
-    const { datasource } = props;
    if (datasource && datasource.type === 'table') {
      const dbId = datasource.database?.id;
      const {
@@ -149,7 +157,7 @@ export const DndFilterSelect = (props: DndFilterSelectProps) => {
          });
      }
    }
-  }, []);
+  }, [datasource]);

  useEffect(() => {
    setOptions(optionsForSelect(props.columns, props.formData));
@@ -163,138 +171,167 @@ export const DndFilterSelect = (props: DndFilterSelectProps) => {
    );
  }, [props.value]);

-  const onClickClose = (index: number) => {
-    const valuesCopy = [...values];
-    valuesCopy.splice(index, 1);
-    setValues(valuesCopy);
-    props.onChange(valuesCopy);
-  };
+  const onClickClose = useCallback(
+    (index: number) => {
+      const valuesCopy = [...values];
+      valuesCopy.splice(index, 1);
+      setValues(valuesCopy);
+      onChange(valuesCopy);
+    },
+    [onChange, values],
+  );

-  const onShiftOptions = (dragIndex: number, hoverIndex: number) => {
-    const newValues = [...values];
-    [newValues[hoverIndex], newValues[dragIndex]] = [
-      newValues[dragIndex],
-      newValues[hoverIndex],
-    ];
-    setValues(newValues);
-  };
-
-  const getMetricExpression = (savedMetricName: string) =>
-    props.savedMetrics.find(
-      (savedMetric: Metric) => savedMetric.metric_name === savedMetricName,
-    )?.expression;
-
-  const mapOption = (option: OptionValueType) => {
-    // already a AdhocFilter, skip
-    if (option instanceof AdhocFilter) {
-      return option;
-    }
-    const filterOptions = option as Record<string, any>;
-    // via datasource saved metric
-    if (filterOptions.saved_metric_name) {
-      return new AdhocFilter({
-        expressionType:
-          props.datasource.type === 'druid'
-            ? EXPRESSION_TYPES.SIMPLE
-            : EXPRESSION_TYPES.SQL,
-        subject:
-          props.datasource.type === 'druid'
-            ? filterOptions.saved_metric_name
-            : getMetricExpression(filterOptions.saved_metric_name),
-        operator:
-          OPERATOR_ENUM_TO_OPERATOR_TYPE[Operators.GREATER_THAN].operation,
-        operatorId: Operators.GREATER_THAN,
-        comparator: 0,
-        clause: CLAUSES.HAVING,
-      });
-    }
-    // has a custom label, meaning it's custom column
-    if (filterOptions.label) {
-      return new AdhocFilter({
-        expressionType:
-          props.datasource.type === 'druid'
-            ? EXPRESSION_TYPES.SIMPLE
-            : EXPRESSION_TYPES.SQL,
-        subject:
-          props.datasource.type === 'druid'
-            ? filterOptions.label
-            : new AdhocMetric(option).translateToSql(),
-        operator:
-          OPERATOR_ENUM_TO_OPERATOR_TYPE[Operators.GREATER_THAN].operation,
-        operatorId: Operators.GREATER_THAN,
-        comparator: 0,
-        clause: CLAUSES.HAVING,
-      });
-    }
-    // add a new filter item
-    if (filterOptions.column_name) {
-      return new AdhocFilter({
-        expressionType: EXPRESSION_TYPES.SIMPLE,
-        subject: filterOptions.column_name,
-        operator: OPERATOR_ENUM_TO_OPERATOR_TYPE[Operators.EQUALS].operation,
-        operatorId: Operators.EQUALS,
-        comparator: '',
-        clause: CLAUSES.WHERE,
-        isNew: true,
-      });
-    }
-    return null;
-  };
-
-  const onFilterEdit = (changedFilter: AdhocFilter) => {
-    props.onChange(
-      values.map((value: AdhocFilter) => {
-        if (value.filterOptionName === changedFilter.filterOptionName) {
-          return changedFilter;
-        }
-        return value;
-      }),
-    );
-  };
-
-  const onNewFilter = (newFilter: AdhocFilter) => {
-    const mappedOption = mapOption(newFilter);
-    if (mappedOption) {
-      const newValues = [...values, mappedOption];
+  const onShiftOptions = useCallback(
+    (dragIndex: number, hoverIndex: number) => {
+      const newValues = [...values];
+      [newValues[hoverIndex], newValues[dragIndex]] = [
+        newValues[dragIndex],
+        newValues[hoverIndex],
+      ];
      setValues(newValues);
-      props.onChange(newValues);
-    }
-  };
+    },
+    [values],
+  );

-  const togglePopover = (visible: boolean) => {
-    setNewFilterPopoverVisible(visible);
-  };
+  const getMetricExpression = useCallback(
+    (savedMetricName: string) =>
+      props.savedMetrics.find(
+        (savedMetric: Metric) => savedMetric.metric_name === savedMetricName,
+      )?.expression,
+    [props.savedMetrics],
+  );

-  const closePopover = () => {
-    togglePopover(false);
-  };
+  const mapOption = useCallback(
+    (option: OptionValueType) => {
+      // already a AdhocFilter, skip
+      if (option instanceof AdhocFilter) {
+        return option;
+      }
+      const filterOptions = option as Record<string, any>;
+      // via datasource saved metric
+      if (filterOptions.saved_metric_name) {
+        return new AdhocFilter({
+          expressionType:
+            datasource.type === 'druid'
+              ? EXPRESSION_TYPES.SIMPLE
+              : EXPRESSION_TYPES.SQL,
+          subject:
+            datasource.type === 'druid'
+              ? filterOptions.saved_metric_name
+              : getMetricExpression(filterOptions.saved_metric_name),
+          operator:
+            OPERATOR_ENUM_TO_OPERATOR_TYPE[Operators.GREATER_THAN].operation,
+          operatorId: Operators.GREATER_THAN,
+          comparator: 0,
+          clause: CLAUSES.HAVING,
+        });
+      }
+      // has a custom label, meaning it's custom column
+      if (filterOptions.label) {
+        return new AdhocFilter({
+          expressionType:
+            datasource.type === 'druid'
+              ? EXPRESSION_TYPES.SIMPLE
+              : EXPRESSION_TYPES.SQL,
+          subject:
+            datasource.type === 'druid'
+              ? filterOptions.label
+              : new AdhocMetric(option).translateToSql(),
+          operator:
+            OPERATOR_ENUM_TO_OPERATOR_TYPE[Operators.GREATER_THAN].operation,
+          operatorId: Operators.GREATER_THAN,
+          comparator: 0,
+          clause: CLAUSES.HAVING,
+        });
+      }
+      // add a new filter item
+      if (filterOptions.column_name) {
+        return new AdhocFilter({
+          expressionType: EXPRESSION_TYPES.SIMPLE,
+          subject: filterOptions.column_name,
+          operator: OPERATOR_ENUM_TO_OPERATOR_TYPE[Operators.EQUALS].operation,
+          operatorId: Operators.EQUALS,
+          comparator: '',
+          clause: CLAUSES.WHERE,
+          isNew: true,
+        });
+      }
+      return null;
+    },
+    [datasource.type, getMetricExpression],
+  );

-  const valuesRenderer = () =>
-    values.map((adhocFilter: AdhocFilter, index: number) => {
-      const label = adhocFilter.getDefaultLabel();
-      return (
-        <AdhocFilterPopoverTrigger
-          key={index}
-          adhocFilter={adhocFilter}
-          options={options}
-          datasource={props.datasource}
-          onFilterEdit={onFilterEdit}
-          partitionColumn={partitionColumn}
-        >
-          <OptionWrapper
-            key={index}
-            index={index}
-            clickClose={onClickClose}
-            onShiftOptions={onShiftOptions}
-            type={DndItemType.FilterOption}
-            withCaret
-            isExtra={adhocFilter.isExtra}
-          >
-            <Tooltip title={label}>{label}</Tooltip>
-          </OptionWrapper>
-        </AdhocFilterPopoverTrigger>
+  const onFilterEdit = useCallback(
+    (changedFilter: AdhocFilter) => {
+      onChange(
+        values.map((value: AdhocFilter) => {
+          if (value.filterOptionName === changedFilter.filterOptionName) {
+            return changedFilter;
+          }
+          return value;
+        }),
      );
-    });
+    },
+    [onChange, values],
+  );
+
+  const onNewFilter = useCallback(
+    (newFilter: AdhocFilter) => {
+      const mappedOption = mapOption(newFilter);
+      if (mappedOption) {
+        const newValues = [...values, mappedOption];
+        setValues(newValues);
+        onChange(newValues);
+      }
+    },
+    [mapOption, onChange, values],
+  );
+
+  const togglePopover = useCallback((visible: boolean) => {
+    setNewFilterPopoverVisible(visible);
+  }, []);
+
+  const closePopover = useCallback(() => {
+    togglePopover(false);
+  }, [togglePopover]);
+
+  const valuesRenderer = useCallback(
+    () =>
+      values.map((adhocFilter: AdhocFilter, index: number) => {
+        const label = adhocFilter.getDefaultLabel();
+        return (
+          <AdhocFilterPopoverTrigger
+            key={index}
+            adhocFilter={adhocFilter}
+            options={options}
+            datasource={datasource}
+            onFilterEdit={onFilterEdit}
+            partitionColumn={partitionColumn}
+          >
+            <OptionWrapper
+              key={index}
+              index={index}
+              clickClose={onClickClose}
+              onShiftOptions={onShiftOptions}
+              type={DndItemType.FilterOption}
+              withCaret
+              isExtra={adhocFilter.isExtra}
+            >
+              <Tooltip title={label}>{label}</Tooltip>
+            </OptionWrapper>
+          </AdhocFilterPopoverTrigger>
+        );
+      }),
+    [
+      onClickClose,
+      onFilterEdit,
+      onShiftOptions,
+      options,
+      partitionColumn,
+      datasource,
+      values,
+    ],
+  );

  const adhocFilter = useMemo(() => {
    if (droppedItem?.metric_name) {
@@ -321,28 +358,29 @@ export const DndFilterSelect = (props: DndFilterSelectProps) => {
    return new AdhocFilter(config);
  }, [droppedItem]);

+  const canDrop = useCallback(() => true, []);
+  const handleDrop = useCallback(
+    (item: DatasourcePanelDndItem) => {
+      setDroppedItem(item.value);
+      togglePopover(true);
+    },
+    [togglePopover],
+  );
+
  return (
    <>
      <DndSelectLabel<OptionValueType, OptionValueType[]>
-        onDrop={(item: DatasourcePanelDndItem) => {
-          setDroppedItem(item.value);
-          togglePopover(true);
-        }}
-        canDrop={() => true}
+        onDrop={handleDrop}
+        canDrop={canDrop}
        valuesRenderer={valuesRenderer}
-        accept={[
-          DndItemType.Column,
-          DndItemType.Metric,
-          DndItemType.MetricOption,
-          DndItemType.AdhocMetricOption,
-        ]}
+        accept={DND_ACCEPTED_TYPES}
        ghostButtonText={t('Drop columns or metrics')}
        {...props}
      />
      <AdhocFilterPopoverTrigger
        adhocFilter={adhocFilter}
        options={options}
-        datasource={props.datasource}
+        datasource={datasource}
        onFilterEdit={onNewFilter}
        partitionColumn={partitionColumn}
        isControlledComponent
--- a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndMetricSelect.tsx
+++ b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndMetricSelect.tsx
@@ -39,6 +39,9 @@ import DndSelectLabel from 'src/explore/components/controls/DndColumnSelectContr
 import { savedMetricType } from 'src/explore/components/controls/MetricControl/types';
 import { AGGREGATES } from 'src/explore/constants';

+const EMPTY_OBJECT = {};
+const DND_ACCEPTED_TYPES = [DndItemType.Column, DndItemType.Metric];
+
 const isDictionaryForAdhocMetric = (value: any) =>
  value && !(value instanceof AdhocMetric) && value.expressionType;

@@ -162,102 +165,144 @@ export const DndMetricSelect = (props: any) => {
    onChange,
  ]);

-  const canDrop = (item: DatasourcePanelDndItem) => {
-    const isMetricAlreadyInValues =
-      item.type === 'metric' ? value.includes(item.value.metric_name) : false;
-    return !isMetricAlreadyInValues;
-  };
+  const canDrop = useCallback(
+    (item: DatasourcePanelDndItem) => {
+      const isMetricAlreadyInValues =
+        item.type === 'metric' ? value.includes(item.value.metric_name) : false;
+      return !isMetricAlreadyInValues;
+    },
+    [value],
+  );

-  const onNewMetric = (newMetric: Metric) => {
-    const newValue = props.multi ? [...value, newMetric] : [newMetric];
-    setValue(newValue);
-    handleChange(newValue);
-  };
+  const onNewMetric = useCallback(
+    (newMetric: Metric) => {
+      const newValue = props.multi ? [...value, newMetric] : [newMetric];
+      setValue(newValue);
+      handleChange(newValue);
+    },
+    [handleChange, props.multi, value],
+  );

-  const onMetricEdit = (
-    changedMetric: Metric | AdhocMetric,
-    oldMetric: Metric | AdhocMetric,
-  ) => {
-    const newValue = value.map(value => {
-      if (
-        // compare saved metrics
-        value === (oldMetric as Metric).metric_name ||
-        // compare adhoc metrics
-        typeof (value as AdhocMetric).optionName !== 'undefined'
-          ? (value as AdhocMetric).optionName ===
-            (oldMetric as AdhocMetric).optionName
-          : false
-      ) {
-        return changedMetric;
+  const onMetricEdit = useCallback(
+    (changedMetric: Metric | AdhocMetric, oldMetric: Metric | AdhocMetric) => {
+      const newValue = value.map(value => {
+        if (
+          // compare saved metrics
+          value === (oldMetric as Metric).metric_name ||
+          // compare adhoc metrics
+          typeof (value as AdhocMetric).optionName !== 'undefined'
+            ? (value as AdhocMetric).optionName ===
+              (oldMetric as AdhocMetric).optionName
+            : false
+        ) {
+          return changedMetric;
+        }
+        return value;
+      });
+      setValue(newValue);
+      handleChange(newValue);
+    },
+    [handleChange, value],
+  );
+
+  const onRemoveMetric = useCallback(
+    (index: number) => {
+      if (!Array.isArray(value)) {
+        return;
      }
-      return value;
-    });
-    setValue(newValue);
-    handleChange(newValue);
-  };
+      const valuesCopy = [...value];
+      valuesCopy.splice(index, 1);
+      setValue(valuesCopy);
+      onChange(valuesCopy);
+    },
+    [onChange, value],
+  );

-  const onRemoveMetric = (index: number) => {
-    if (!Array.isArray(value)) {
-      return;
-    }
-    const valuesCopy = [...value];
-    valuesCopy.splice(index, 1);
-    setValue(valuesCopy);
-    onChange(valuesCopy);
-  };
+  const moveLabel = useCallback(
+    (dragIndex: number, hoverIndex: number) => {
+      const newValues = [...value];
+      [newValues[hoverIndex], newValues[dragIndex]] = [
+        newValues[dragIndex],
+        newValues[hoverIndex],
+      ];
+      setValue(newValues);
+    },
+    [value],
+  );

-  const moveLabel = (dragIndex: number, hoverIndex: number) => {
-    const newValues = [...value];
-    [newValues[hoverIndex], newValues[dragIndex]] = [
-      newValues[dragIndex],
-      newValues[hoverIndex],
-    ];
-    setValue(newValues);
-  };
+  const newSavedMetricOptions = useMemo(
+    () => getOptionsForSavedMetrics(props.savedMetrics, props.value),
+    [props.savedMetrics, props.value],
+  );

-  const valueRenderer = (
-    option: Metric | AdhocMetric | string,
-    index: number,
-  ) => (
-    <MetricDefinitionValue
-      key={index}
-      index={index}
-      option={option}
-      onMetricEdit={onMetricEdit}
-      onRemoveMetric={() => onRemoveMetric(index)}
-      columns={props.columns}
-      savedMetrics={props.savedMetrics}
-      savedMetricsOptions={getOptionsForSavedMetrics(
+  const getSavedMetricOptionsForMetric = useCallback(
+    index =>
+      getOptionsForSavedMetrics(
        props.savedMetrics,
        props.value,
        props.value?.[index],
-      )}
-      datasourceType={props.datasourceType}
-      onMoveLabel={moveLabel}
-      onDropLabel={() => onChange(value)}
-    />
+      ),
+    [props.savedMetrics, props.value],
  );

-  const valuesRenderer = () =>
-    value.map((value, index) => valueRenderer(value, index));
+  const handleDropLabel = useCallback(() => onChange(value), [onChange, value]);

-  const togglePopover = (visible: boolean) => {
+  const valueRenderer = useCallback(
+    (option: Metric | AdhocMetric | string, index: number) => (
+      <MetricDefinitionValue
+        key={index}
+        index={index}
+        option={option}
+        onMetricEdit={onMetricEdit}
+        onRemoveMetric={onRemoveMetric}
+        columns={props.columns}
+        savedMetrics={props.savedMetrics}
+        savedMetricsOptions={getSavedMetricOptionsForMetric(index)}
+        datasourceType={props.datasourceType}
+        onMoveLabel={moveLabel}
+        onDropLabel={handleDropLabel}
+        type={`${DndItemType.AdhocMetricOption}_${props.name}_${props.label}`}
+      />
+    ),
+    [
+      getSavedMetricOptionsForMetric,
+      handleDropLabel,
+      moveLabel,
+      onMetricEdit,
+      onRemoveMetric,
+      props.columns,
+      props.datasourceType,
+      props.label,
+      props.name,
+      props.savedMetrics,
+    ],
+  );
+
+  const valuesRenderer = useCallback(
+    () => value.map((value, index) => valueRenderer(value, index)),
+    [value, valueRenderer],
+  );
+
+  const togglePopover = useCallback((visible: boolean) => {
    setNewMetricPopoverVisible(visible);
-  };
+  }, []);

-  const closePopover = () => {
+  const closePopover = useCallback(() => {
    togglePopover(false);
-  };
+  }, [togglePopover]);

-  const handleDrop = (item: DatasourcePanelDndItem) => {
-    if (item.type === DndItemType.Metric) {
-      onNewMetric(item.value as Metric);
-    }
-    if (item.type === DndItemType.Column) {
-      setDroppedItem(item);
-      togglePopover(true);
-    }
-  };
+  const handleDrop = useCallback(
+    (item: DatasourcePanelDndItem) => {
+      if (item.type === DndItemType.Metric) {
+        onNewMetric(item.value as Metric);
+      }
+      if (item.type === DndItemType.Column) {
+        setDroppedItem(item);
+        togglePopover(true);
+      }
+    },
+    [onNewMetric, togglePopover],
+  );

  const adhocMetric = useMemo(() => {
    if (droppedItem?.type === DndItemType.Column) {
@@ -287,7 +332,7 @@ export const DndMetricSelect = (props: any) => {
        onDrop={handleDrop}
        canDrop={canDrop}
        valuesRenderer={valuesRenderer}
-        accept={[DndItemType.Column, DndItemType.Metric]}
+        accept={DND_ACCEPTED_TYPES}
        ghostButtonText={tn(
          'Drop column or metric',
          'Drop columns or metrics',
@@ -300,11 +345,8 @@ export const DndMetricSelect = (props: any) => {
        adhocMetric={adhocMetric}
        onMetricEdit={onNewMetric}
        columns={props.columns}
-        savedMetricsOptions={getOptionsForSavedMetrics(
-          props.savedMetrics,
-          props.value,
-        )}
-        savedMetric={{} as savedMetricType}
+        savedMetricsOptions={newSavedMetricOptions}
+        savedMetric={EMPTY_OBJECT as savedMetricType}
        datasourceType={props.datasourceType}
        isControlledComponent
        visible={newMetricPopoverVisible}
--- a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/Option.tsx
+++ b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/Option.tsx
@@ -16,7 +16,7 @@
 * specific language governing permissions and limitations
 * under the License.
 */
-import React from 'react';
+import React, { useCallback } from 'react';
 import { styled, t, useTheme } from '@superset-ui/core';
 import Icons from 'src/components/Icons';
 import {
@@ -41,13 +41,17 @@ export default function Option({
  canDelete = true,
 }: OptionProps) {
  const theme = useTheme();
+  const onClickClose = useCallback(() => clickClose(index), [
+    clickClose,
+    index,
+  ]);
  return (
    <OptionControlContainer data-test="option-label" withCaret={withCaret}>
      {canDelete && (
        <CloseContainer
          role="button"
          data-test="remove-control-button"
-          onClick={() => clickClose(index)}
+          onClick={onClickClose}
        >
          <Icons.XSmall iconColor={theme.colors.grayscale.light1} />
        </CloseContainer>
--- a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/OptionWrapper.tsx
+++ b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/OptionWrapper.tsx
@@ -16,7 +16,7 @@
 * specific language governing permissions and limitations
 * under the License.
 */
-import React, { useRef } from 'react';
+import React, { useMemo, useRef } from 'react';
 import {
  useDrag,
  useDrop,
@@ -24,7 +24,6 @@ import {
  DragSourceMonitor,
 } from 'react-dnd';
 import { DragContainer } from 'src/explore/components/controls/OptionControls';
-import { DndItemType } from 'src/explore/components/DndItemType';
 import {
  OptionProps,
  OptionItemInterface,
@@ -33,7 +32,7 @@ import Option from './Option';

 export default function OptionWrapper(
  props: OptionProps & {
-    type: DndItemType;
+    type: string;
    onShiftOptions: (dragIndex: number, hoverIndex: number) => void;
  },
 ) {
@@ -50,10 +49,13 @@ export default function OptionWrapper(
  } = props;
  const ref = useRef<HTMLDivElement>(null);

-  const item: OptionItemInterface = {
-    dragIndex: index,
-    type,
-  };
+  const item: OptionItemInterface = useMemo(
+    () => ({
+      dragIndex: index,
+      type,
+    }),
+    [index, type],
+  );
  const [, drag] = useDrag({
    item,
    collect: (monitor: DragSourceMonitor) => ({
--- a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/types.ts
+++ b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/types.ts
@@ -44,6 +44,7 @@ export interface LabelProps<T = string[] | string> {
  multi?: boolean;
  canDelete?: boolean;
  ghostButtonText?: string;
+  label?: string;
 }

 export interface DndColumnSelectProps<
--- a/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricOption.jsx
+++ b/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricOption.jsx
@@ -36,6 +36,7 @@ const propTypes = {
  onMoveLabel: PropTypes.func,
  onDropLabel: PropTypes.func,
  index: PropTypes.number,
+  type: PropTypes.string,
 };

 class AdhocMetricOption extends React.PureComponent {
@@ -46,7 +47,7 @@ class AdhocMetricOption extends React.PureComponent {

  onRemoveMetric(e) {
    e.stopPropagation();
-    this.props.onRemoveMetric();
+    this.props.onRemoveMetric(this.props.index);
  }

  render() {
@@ -60,6 +61,7 @@ class AdhocMetricOption extends React.PureComponent {
      onMoveLabel,
      onDropLabel,
      index,
+      type,
    } = this.props;

    return (
@@ -79,7 +81,7 @@ class AdhocMetricOption extends React.PureComponent {
          onMoveLabel={onMoveLabel}
          onDropLabel={onDropLabel}
          index={index}
-          type={DndItemType.AdhocMetricOption}
+          type={type ?? DndItemType.AdhocMetricOption}
          withCaret
          isFunction
        />
--- a/superset-frontend/src/explore/components/controls/MetricControl/MetricDefinitionValue.jsx
+++ b/superset-frontend/src/explore/components/controls/MetricControl/MetricDefinitionValue.jsx
@@ -48,6 +48,7 @@ export default function MetricDefinitionValue({
  onMoveLabel,
  onDropLabel,
  index,
+  type,
 }) {
  const getSavedMetricByName = metricName =>
    savedMetrics.find(metric => metric.metric_name === metricName);
@@ -74,6 +75,7 @@ export default function MetricDefinitionValue({
      onDropLabel,
      index,
      savedMetric: savedMetric ?? {},
+      type,
    };

    return <AdhocMetricOption {...metricOptionProps} />;
--- a/superset-frontend/src/explore/components/controls/MetricControl/MetricsControl.jsx
+++ b/superset-frontend/src/explore/components/controls/MetricControl/MetricsControl.jsx
@@ -142,7 +142,7 @@ class MetricsControl extends React.PureComponent {
        index={index}
        option={option}
        onMetricEdit={this.onMetricEdit}
-        onRemoveMetric={() => this.onRemoveMetric(index)}
+        onRemoveMetric={this.onRemoveMetric}
        columns={this.props.columns}
        datasource={this.props.datasource}
        savedMetrics={this.props.savedMetrics}