From 14de7b4791174c7cfaa74732f7ac61ab1135cb1a Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Thu, 28 Jan 2021 07:58:39 +0100 Subject: [PATCH] feat(explore): Make metric title respond to changes immediately (#12747) * Make metric title respond to changes immediately * Bug fix * Change type to Metric * Bug fix --- .../AdhocMetricEditPopover_spec.jsx | 1 + .../MetricControl/AdhocMetricEditPopover.jsx | 34 +++++---- .../AdhocMetricPopoverTrigger.tsx | 73 ++++++++++++++++--- 3 files changed, 82 insertions(+), 26 deletions(-) diff --git a/superset-frontend/spec/javascripts/explore/components/AdhocMetricEditPopover_spec.jsx b/superset-frontend/spec/javascripts/explore/components/AdhocMetricEditPopover_spec.jsx index 3930de3231..2c7e276425 100644 --- a/superset-frontend/spec/javascripts/explore/components/AdhocMetricEditPopover_spec.jsx +++ b/superset-frontend/spec/javascripts/explore/components/AdhocMetricEditPopover_spec.jsx @@ -56,6 +56,7 @@ function setup(overrides) { onChange, onClose, onResize: () => {}, + getCurrentLabel: () => {}, columns, ...overrides, }; diff --git a/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover.jsx b/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover.jsx index f27f776c15..93135e689a 100644 --- a/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover.jsx +++ b/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover.jsx @@ -42,14 +42,11 @@ const propTypes = { onClose: PropTypes.func.isRequired, onResize: PropTypes.func.isRequired, getCurrentTab: PropTypes.func, + getCurrentLabel: PropTypes.func, columns: PropTypes.arrayOf(columnType), savedMetricsOptions: PropTypes.arrayOf(savedMetricType), savedMetric: savedMetricType, datasourceType: PropTypes.string, - title: PropTypes.shape({ - label: PropTypes.string, - hasCustomLabel: PropTypes.bool, - }), }; const defaultProps = { @@ -72,7 +69,7 @@ export const SAVED_TAB_KEY = 'SAVED'; const startingWidth = 320; const startingHeight = 240; -export default class AdhocMetricEditPopover extends React.Component { +export default class AdhocMetricEditPopover extends React.PureComponent { // "Saved" is a default tab unless there are no saved metrics for dataset defaultActiveTabKey = (this.props.savedMetric.metric_name || this.props.adhocMetric.isNew) && @@ -110,20 +107,31 @@ export default class AdhocMetricEditPopover extends React.Component { this.props.getCurrentTab(this.defaultActiveTabKey); } + componentDidUpdate(prevProps, prevState) { + if ( + prevState.adhocMetric?.sqlExpression !== + this.state.adhocMetric?.sqlExpression || + prevState.adhocMetric?.aggregate !== this.state.adhocMetric?.aggregate || + prevState.adhocMetric?.column?.column_name !== + this.state.adhocMetric?.column?.column_name || + prevState.savedMetric?.metric_name !== this.state.savedMetric?.metric_name + ) { + this.props.getCurrentLabel({ + savedMetricLabel: + this.state.savedMetric?.verbose_name || + this.state.savedMetric?.metric_name, + adhocMetricLabel: this.state.adhocMetric?.getDefaultLabel(), + }); + } + } + componentWillUnmount() { document.removeEventListener('mouseup', this.onMouseUp); document.removeEventListener('mousemove', this.onMouseMove); } onSave() { - const { title } = this.props; - const { hasCustomLabel } = title; - let { label } = title; const { adhocMetric, savedMetric } = this.state; - const metricLabel = adhocMetric.label; - if (!hasCustomLabel) { - label = metricLabel; - } const metric = savedMetric?.metric_name ? savedMetric : adhocMetric; const oldMetric = this.props.savedMetric?.metric_name @@ -132,8 +140,6 @@ export default class AdhocMetricEditPopover extends React.Component { this.props.onChange( { ...metric, - label, - hasCustomLabel, }, oldMetric, ); diff --git a/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricPopoverTrigger.tsx b/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricPopoverTrigger.tsx index 8080b2bff3..dd7b4bf894 100644 --- a/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricPopoverTrigger.tsx +++ b/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricPopoverTrigger.tsx @@ -17,6 +17,7 @@ * under the License. */ import React, { ReactNode } from 'react'; +import { Metric } from '@superset-ui/core'; import Popover from 'src/common/components/Popover'; import AdhocMetricEditPopoverTitle from 'src/explore/components/controls/MetricControl/AdhocMetricEditPopoverTitle'; import AdhocMetricEditPopover, { @@ -27,7 +28,7 @@ import { savedMetricType } from './types'; export type AdhocMetricPopoverTriggerProps = { adhocMetric: AdhocMetric; - onMetricEdit: () => void; + onMetricEdit(newMetric: Metric, oldMetric: Metric): void; columns: { column_name: string; type: string }[]; savedMetricsOptions: savedMetricType[]; savedMetric: savedMetricType; @@ -39,6 +40,7 @@ export type AdhocMetricPopoverTriggerProps = { export type AdhocMetricPopoverTriggerState = { popoverVisible: boolean; title: { label: string; hasCustomLabel: boolean }; + currentLabel: string; labelModified: boolean; isTitleEditDisabled: boolean; }; @@ -53,26 +55,38 @@ class AdhocMetricPopoverTrigger extends React.PureComponent< this.onLabelChange = this.onLabelChange.bind(this); this.closePopover = this.closePopover.bind(this); this.togglePopover = this.togglePopover.bind(this); + this.getCurrentTab = this.getCurrentTab.bind(this); + this.getCurrentLabel = this.getCurrentLabel.bind(this); + this.onChange = this.onChange.bind(this); + this.state = { popoverVisible: false, title: { label: props.adhocMetric.label, hasCustomLabel: props.adhocMetric.hasCustomLabel, }, + currentLabel: '', labelModified: false, isTitleEditDisabled: false, }; } onLabelChange(e: any) { + const { verbose_name, metric_name } = this.props.savedMetric; + const defaultMetricLabel = this.props.adhocMetric?.getDefaultLabel(); const label = e.target.value; - this.setState({ + this.setState(state => ({ title: { - label: label || this.props.adhocMetric.label, + label: + label || + state.currentLabel || + verbose_name || + metric_name || + defaultMetricLabel, hasCustomLabel: !!label, }, labelModified: true, - }); + })); } onPopoverResize() { @@ -92,13 +106,51 @@ class AdhocMetricPopoverTrigger extends React.PureComponent< }); } + getCurrentTab(tab: string) { + this.setState({ + isTitleEditDisabled: tab === SAVED_TAB_KEY, + }); + } + + getCurrentLabel({ + savedMetricLabel, + adhocMetricLabel, + }: { + savedMetricLabel: string; + adhocMetricLabel: string; + }) { + const currentLabel = savedMetricLabel || adhocMetricLabel; + this.setState({ + currentLabel, + labelModified: true, + }); + if (savedMetricLabel || !this.state.title.hasCustomLabel) { + this.setState({ + title: { + label: currentLabel, + hasCustomLabel: false, + }, + }); + } + } + + onChange(newMetric: Metric, oldMetric: Metric) { + this.props.onMetricEdit({ ...newMetric, ...this.state.title }, oldMetric); + } + render() { const { adhocMetric, savedMetric } = this.props; const { verbose_name, metric_name } = savedMetric; - const { label, hasCustomLabel } = adhocMetric; + const { hasCustomLabel, label } = adhocMetric; + const adhocMetricLabel = hasCustomLabel + ? label + : adhocMetric.getDefaultLabel(); const title = this.state.labelModified ? this.state.title - : { label: verbose_name || metric_name || label, hasCustomLabel }; + : { + label: verbose_name || metric_name || adhocMetricLabel, + hasCustomLabel, + }; const overlayContent = ( - this.setState({ - isTitleEditDisabled: tab === SAVED_TAB_KEY, - }) - } + onChange={this.onChange} + getCurrentTab={this.getCurrentTab} + getCurrentLabel={this.getCurrentLabel} /> );