From e743ce7135235b2d0def403fa330f663ea1a2667 Mon Sep 17 00:00:00 2001
From: Maxime Beauchemin <maximebeauchemin@gmail.com>
Date: Wed, 30 Sep 2020 23:12:51 -0700
Subject: [PATCH] fix: SpatialControl popover won't open (#11127)

Recently a change related to emotion styling not properly working through
react-bootstrap's popover broke the SpatialControl.

This PR makes SpatialControl use antd's equivalent, and addresses the
issue as a result: emotion's styling context is preserved through
this superior popover.
---
 .../src/components/Popover/index.tsx          |  23 ++++
 .../components/controls/SpatialControl.jsx    | 130 ++++++++----------
 2 files changed, 78 insertions(+), 75 deletions(-)
 create mode 100644 superset-frontend/src/components/Popover/index.tsx

diff --git a/superset-frontend/src/components/Popover/index.tsx b/superset-frontend/src/components/Popover/index.tsx
new file mode 100644
index 0000000000000..9cec64080b024
--- /dev/null
+++ b/superset-frontend/src/components/Popover/index.tsx
@@ -0,0 +1,23 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { Popover } from 'src/common/components';
+
+// Eventually Popover can be wrapped and customized in this file
+// for now we're just redirecting
+export default Popover;
diff --git a/superset-frontend/src/explore/components/controls/SpatialControl.jsx b/superset-frontend/src/explore/components/controls/SpatialControl.jsx
index 073148be9560b..5c34ec3ca685c 100644
--- a/superset-frontend/src/explore/components/controls/SpatialControl.jsx
+++ b/superset-frontend/src/explore/components/controls/SpatialControl.jsx
@@ -18,11 +18,11 @@
  */
 import React from 'react';
 import PropTypes from 'prop-types';
-import { Row, Col, OverlayTrigger, Popover } from 'react-bootstrap';
-import Button from 'src/components/Button';
+import { Row, Col } from 'react-bootstrap';
 import { t } from '@superset-ui/core';
 
 import Label from 'src/components/Label';
+import Popover from 'src/components/Popover';
 import PopoverSection from 'src/components/PopoverSection';
 import Checkbox from 'src/components/Checkbox';
 import ControlHeader from '../ControlHeader';
@@ -108,10 +108,6 @@ export default class SpatialControl extends React.Component {
     this.setState({ type }, this.onChange);
   }
 
-  close() {
-    this.refs.trigger.hide();
-  }
-
   toggleCheckbox() {
     this.setState(
       prevState => ({ reverseCheckbox: !prevState.reverseCheckbox }),
@@ -164,68 +160,56 @@ export default class SpatialControl extends React.Component {
     );
   }
 
-  renderPopover() {
+  renderPopoverContent() {
     return (
-      <Popover id="filter-popover">
-        <div style={{ width: '300px' }}>
-          <PopoverSection
-            title={t('Longitude & Latitude columns')}
-            isSelected={this.state.type === spatialTypes.latlong}
-            onSelect={this.setType.bind(this, spatialTypes.latlong)}
-          >
-            <Row>
-              <Col md={6}>
-                Longitude
-                {this.renderSelect('lonCol', spatialTypes.latlong)}
-              </Col>
-              <Col md={6}>
-                Latitude
-                {this.renderSelect('latCol', spatialTypes.latlong)}
-              </Col>
-            </Row>
-          </PopoverSection>
-          <PopoverSection
-            title={t('Delimited long & lat single column')}
-            info={t(
-              'Multiple formats accepted, look the geopy.points ' +
-                'Python library for more details',
-            )}
-            isSelected={this.state.type === spatialTypes.delimited}
-            onSelect={this.setType.bind(this, spatialTypes.delimited)}
-          >
-            <Row>
-              <Col md={6}>
-                {t('Column')}
-                {this.renderSelect('lonlatCol', spatialTypes.delimited)}
-              </Col>
-              <Col md={6}>{this.renderReverseCheckbox()}</Col>
-            </Row>
-          </PopoverSection>
-          <PopoverSection
-            title={t('Geohash')}
-            isSelected={this.state.type === spatialTypes.geohash}
-            onSelect={this.setType.bind(this, spatialTypes.geohash)}
-          >
-            <Row>
-              <Col md={6}>
-                Column
-                {this.renderSelect('geohashCol', spatialTypes.geohash)}
-              </Col>
-              <Col md={6}>{this.renderReverseCheckbox()}</Col>
-            </Row>
-          </PopoverSection>
-          <div className="clearfix">
-            <Button
-              buttonSize="small"
-              className="float-left ok"
-              buttonStyle="primary"
-              onClick={this.close.bind(this)}
-            >
-              Ok
-            </Button>
-          </div>
-        </div>
-      </Popover>
+      <div style={{ width: '300px' }}>
+        <PopoverSection
+          title={t('Longitude & Latitude columns')}
+          isSelected={this.state.type === spatialTypes.latlong}
+          onSelect={this.setType.bind(this, spatialTypes.latlong)}
+        >
+          <Row>
+            <Col md={6}>
+              Longitude
+              {this.renderSelect('lonCol', spatialTypes.latlong)}
+            </Col>
+            <Col md={6}>
+              Latitude
+              {this.renderSelect('latCol', spatialTypes.latlong)}
+            </Col>
+          </Row>
+        </PopoverSection>
+        <PopoverSection
+          title={t('Delimited long & lat single column')}
+          info={t(
+            'Multiple formats accepted, look the geopy.points ' +
+              'Python library for more details',
+          )}
+          isSelected={this.state.type === spatialTypes.delimited}
+          onSelect={this.setType.bind(this, spatialTypes.delimited)}
+        >
+          <Row>
+            <Col md={6}>
+              {t('Column')}
+              {this.renderSelect('lonlatCol', spatialTypes.delimited)}
+            </Col>
+            <Col md={6}>{this.renderReverseCheckbox()}</Col>
+          </Row>
+        </PopoverSection>
+        <PopoverSection
+          title={t('Geohash')}
+          isSelected={this.state.type === spatialTypes.geohash}
+          onSelect={this.setType.bind(this, spatialTypes.geohash)}
+        >
+          <Row>
+            <Col md={6}>
+              Column
+              {this.renderSelect('geohashCol', spatialTypes.geohash)}
+            </Col>
+            <Col md={6}>{this.renderReverseCheckbox()}</Col>
+          </Row>
+        </PopoverSection>
+      </div>
     );
   }
 
@@ -233,17 +217,13 @@ export default class SpatialControl extends React.Component {
     return (
       <div>
         <ControlHeader {...this.props} />
-        <OverlayTrigger
-          animation={this.props.animation}
-          container={document.body}
+        <Popover
+          content={this.renderPopoverContent()}
+          placement="topLeft" // so that popover doesn't move when label changes
           trigger="click"
-          rootClose
-          ref="trigger"
-          placement="right"
-          overlay={this.renderPopover()}
         >
           <Label className="pointer">{this.renderLabelContent()}</Label>
-        </OverlayTrigger>
+        </Popover>
       </div>
     );
   }