Refactor for consistency

superset-frontend/spec/javascripts/explore/components/AdhocMetricOption_spec.jsx

@@ -19,7 +19,7 @@
 /* eslint-disable no-unused-expressions */
 import React from 'react';
 import sinon from 'sinon';
-import { styledShallow as shallow } from 'spec/helpers/theming';
+import { shallow } from 'enzyme';
 
 import Popover from 'src/common/components/Popover';
 import Label from 'src/components/Label';
@@ -46,7 +46,7 @@ function setup(overrides) {
     columns,
     ...overrides,
   };
-  const wrapper = shallow(<AdhocMetricOption {...props} />).dive();
+  const wrapper = shallow(<AdhocMetricOption {...props} />);
   return { wrapper, onMetricEdit };
 }
 
@@ -73,11 +73,13 @@ describe('AdhocMetricOption', () => {
 
   it('returns to default labels when the custom label is cleared', () => {
     const { wrapper } = setup();
+    expect(wrapper.state('title').label).toBe('SUM(value)');
+
     wrapper.instance().onLabelChange({ target: { value: 'new label' } });
+    expect(wrapper.state('title').label).toBe('new label');
+
     wrapper.instance().onLabelChange({ target: { value: '' } });
-    // close and open the popover
-    wrapper.instance().closeMetricEditOverlay();
-    wrapper.instance().onOverlayEntered();
+
     expect(wrapper.state('title').label).toBe('SUM(value)');
     expect(wrapper.state('title').hasCustomLabel).toBe(false);
   });

superset-frontend/src/explore/components/AdhocFilterOption.jsx

@@ -45,7 +45,6 @@ class AdhocFilterOption extends React.PureComponent {
   constructor(props) {
     super(props);
     this.onPopoverResize = this.onPopoverResize.bind(this);
-    this.getContent = this.getContent.bind(this);
     this.openPopover = this.openPopover.bind(this);
     this.closePopover = this.closePopover.bind(this);
     this.togglePopover = this.togglePopover.bind(this);
@@ -55,30 +54,19 @@ class AdhocFilterOption extends React.PureComponent {
     };
   }
 
-  onPopoverResize() {
-    this.forceUpdate();
+  componentDidMount() {
+    const { adhocFilter } = this.props;
+    // isNew is used to auto-open the popup. Once popup is opened, it's not
+    // considered new anymore.
+    // put behind setTimeout so in case consequetive re-renderings are triggered
+    // for some reason, the popup can still show up.
+    setTimeout(() => {
+      adhocFilter.isNew = false;
+    });
   }
 
-  getContent() {
-    const {
-      adhocFilter,
-      onFilterEdit,
-      options,
-      datasource,
-      partitionColumn,
-    } = this.props;
-
-    return (
-      <AdhocFilterEditPopover
-        adhocFilter={adhocFilter}
-        options={options}
-        datasource={datasource}
-        partitionColumn={partitionColumn}
-        onResize={this.onPopoverResize}
-        onChange={onFilterEdit}
-        onClose={this.closePopover}
-      />
-    );
+  onPopoverResize() {
+    this.forceUpdate();
   }
 
   openPopover() {
@@ -99,10 +87,17 @@ class AdhocFilterOption extends React.PureComponent {
 
   render() {
     const { adhocFilter } = this.props;
-    const { isNew } = adhocFilter;
-    if (isNew) {
-      adhocFilter.isNew = false;
-    }
+    const overlayContent = (
+      <AdhocFilterEditPopover
+        adhocFilter={adhocFilter}
+        options={this.props.options}
+        datasource={this.props.datasource}
+        partitionColumn={this.props.partitionColumn}
+        onResize={this.onPopoverResize}
+        onClose={this.closePopover}
+        onChange={this.props.onFilterEdit}
+      />
+    );
 
     return (
       <div
@@ -125,8 +120,8 @@ class AdhocFilterOption extends React.PureComponent {
         <Popover
           placement="right"
           trigger="click"
-          content={this.getContent}
-          defaultVisible={isNew}
+          content={overlayContent}
+          defaultVisible={adhocFilter.isNew}
           visible={this.state.popoverVisible}
           onVisibleChange={visible => {
             this.setState({ popoverVisible: visible });

superset-frontend/src/explore/components/AdhocMetricOption.jsx

@@ -51,12 +51,23 @@ class AdhocMetricOption extends React.PureComponent {
     };
   }
 
+  componentDidMount() {
+    const { adhocMetric } = this.props;
+    // isNew is used to auto-open the popup. Once popup is opened, it's not
+    // considered new anymore.
+    // put behind setTimeout so in case consequetive re-renderings are triggered
+    // for some reason, the popup can still show up.
+    setTimeout(() => {
+      adhocMetric.isNew = false;
+    });
+  }
+
   onLabelChange(e) {
     const label = e.target.value;
     this.setState({
       title: {
-        label,
-        hasCustomLabel: true,
+        label: label || this.props.adhocMetric.label,
+        hasCustomLabel: !!label,
       },
     });
   }
@@ -83,14 +94,6 @@ class AdhocMetricOption extends React.PureComponent {
 
   render() {
     const { adhocMetric } = this.props;
-    const { isNew } = adhocMetric;
-    if (isNew) {
-      // new metrics automaticall open the popover
-      // once the metric is rendered, then it's not new.
-      // testing this by selecting multiple new columns to create multiple
-      // new adhoc metrics
-      adhocMetric.isNew = false;
-    }
 
     const overlayContent = (
       <AdhocMetricEditPopover
@@ -99,8 +102,8 @@ class AdhocMetricOption extends React.PureComponent {
         columns={this.props.columns}
         datasourceType={this.props.datasourceType}
         onResize={this.onPopoverResize}
-        onChange={this.props.onMetricEdit}
         onClose={this.closePopover}
+        onChange={this.props.onMetricEdit}
       />
     );
 
@@ -126,7 +129,7 @@ class AdhocMetricOption extends React.PureComponent {
           trigger="click"
           disabled
           content={overlayContent}
-          defaultVisible={isNew}
+          defaultVisible={adhocMetric.isNew}
           visible={this.state.popoverVisible}
           onVisibleChange={this.togglePopover}
           title={popoverTitle}