From ab41b2896f86eb5e73823f0bfa8469d19fb7a311 Mon Sep 17 00:00:00 2001
From: Maxime Beauchemin <maximebeauchemin@gmail.com>
Date: Tue, 5 Dec 2017 11:37:13 -0800
Subject: [PATCH] [sql lab] fix position of 'save query' Popover (#3999)

* [sql lab] fix position of 'save query' Popover

The "Save Query" popover renders on the upper left corner as opposed to
where it should (relative to the Save Query button). After a bit of
research, it seems like Popover won't render in the right place when
parents are absolute.

I'm guessing this stopped working properly when I added the resizable
panes.

Anyhow, the solution here is to use a modal instead.

* Fixing tests
---
 .../SqlLab/components/SaveQuery.jsx           | 126 +++++++++---------
 .../javascripts/sqllab/SaveQuery_spec.jsx     |  22 ++-
 2 files changed, 71 insertions(+), 77 deletions(-)

diff --git a/superset/assets/javascripts/SqlLab/components/SaveQuery.jsx b/superset/assets/javascripts/SqlLab/components/SaveQuery.jsx
index 8ab1b22ad0c6..007fd57bd04c 100644
--- a/superset/assets/javascripts/SqlLab/components/SaveQuery.jsx
+++ b/superset/assets/javascripts/SqlLab/components/SaveQuery.jsx
@@ -1,7 +1,9 @@
 import React from 'react';
 import PropTypes from 'prop-types';
-import { FormControl, FormGroup, Overlay, Popover, Row, Col } from 'react-bootstrap';
+import { FormControl, FormGroup, Row, Col } from 'react-bootstrap';
+
 import Button from '../../components/Button';
+import ModalTrigger from '../../components/ModalTrigger';
 import { t } from '../../locales';
 
 const propTypes = {
@@ -41,10 +43,10 @@ class SaveQuery extends React.PureComponent {
       sql: this.props.sql,
     };
     this.props.onSave(query);
-    this.setState({ showSave: false });
+    this.saveModal.close();
   }
   onCancel() {
-    this.setState({ showSave: false });
+    this.saveModal.close();
   }
   onLabelChange(e) {
     this.setState({ label: e.target.value });
@@ -55,72 +57,70 @@ class SaveQuery extends React.PureComponent {
   toggleSave(e) {
     this.setState({ target: e.target, showSave: !this.state.showSave });
   }
-  renderPopover() {
+  renderModalBody() {
     return (
-      <Popover id="embed-code-popover">
-        <FormGroup bsSize="small" style={{ width: '350px' }}>
-          <Row>
-            <Col md={12}>
-              <small>
-                <label className="control-label" htmlFor="embed-height">
-                  {t('Label')}
-                </label>
-              </small>
-              <FormControl
-                type="text"
-                placeholder={t('Label for your query')}
-                value={this.state.label}
-                onChange={this.onLabelChange}
-              />
-            </Col>
-          </Row>
-          <br />
-          <Row>
-            <Col md={12}>
-              <small>
-                <label className="control-label" htmlFor="embed-height">{t('Description')}</label>
-              </small>
-              <FormControl
-                componentClass="textarea"
-                placeholder={t('Write a description for your query')}
-                value={this.state.description}
-                onChange={this.onDescriptionChange}
-              />
-            </Col>
-          </Row>
-          <br />
-          <Row>
-            <Col md={12}>
-              <Button
-                bsStyle="primary"
-                onClick={this.onSave}
-                className="m-r-3"
-              >
-                {t('Save')}
-              </Button>
-              <Button onClick={this.onCancel} className="cancelQuery">
-                {t('Cancel')}
-              </Button>
-            </Col>
-          </Row>
-        </FormGroup>
-      </Popover>
+      <FormGroup bsSize="small">
+        <Row>
+          <Col md={12}>
+            <small>
+              <label className="control-label" htmlFor="embed-height">
+                {t('Label')}
+              </label>
+            </small>
+            <FormControl
+              type="text"
+              placeholder={t('Label for your query')}
+              value={this.state.label}
+              onChange={this.onLabelChange}
+            />
+          </Col>
+        </Row>
+        <br />
+        <Row>
+          <Col md={12}>
+            <small>
+              <label className="control-label" htmlFor="embed-height">{t('Description')}</label>
+            </small>
+            <FormControl
+              componentClass="textarea"
+              placeholder={t('Write a description for your query')}
+              value={this.state.description}
+              onChange={this.onDescriptionChange}
+            />
+          </Col>
+        </Row>
+        <br />
+        <Row>
+          <Col md={12}>
+            <Button
+              bsStyle="primary"
+              onClick={this.onSave}
+              className="m-r-3"
+            >
+              {t('Save')}
+            </Button>
+            <Button onClick={this.onCancel} className="cancelQuery">
+              {t('Cancel')}
+            </Button>
+          </Col>
+        </Row>
+      </FormGroup>
     );
   }
   render() {
     return (
       <span className="SaveQuery">
-        <Overlay
-          trigger="click"
-          show={this.state.showSave}
-          placement="bottom"
-          animation={this.props.animation}
-        >
-          {this.renderPopover()}
-        </Overlay>
-        <Button bsSize="small" className="toggleSave" onClick={this.toggleSave}>
-          <i className="fa fa-save" /> {t('Save Query')}
-        </Button>
+        <ModalTrigger
+          ref={(ref) => { this.saveModal = ref; }}
+          modalTitle={t('Save Query')}
+          modalBody={this.renderModalBody()}
+          triggerNode={
+            <Button bsSize="small" className="toggleSave" onClick={this.toggleSave}>
+              <i className="fa fa-save" /> {t('Save Query')}
+            </Button>
+          }
+          bsSize="small"
+        />
       </span>
     );
   }
diff --git a/superset/assets/spec/javascripts/sqllab/SaveQuery_spec.jsx b/superset/assets/spec/javascripts/sqllab/SaveQuery_spec.jsx
index 98c8758a8db2..146a0746b833 100644
--- a/superset/assets/spec/javascripts/sqllab/SaveQuery_spec.jsx
+++ b/superset/assets/spec/javascripts/sqllab/SaveQuery_spec.jsx
@@ -1,9 +1,10 @@
 import React from 'react';
-import { Overlay, Popover, FormControl } from 'react-bootstrap';
+import { FormControl } from 'react-bootstrap';
 import { shallow } from 'enzyme';
 import { describe, it } from 'mocha';
 import { expect } from 'chai';
 import SaveQuery from '../../../javascripts/SqlLab/components/SaveQuery';
+import ModalTrigger from '../../../javascripts/components/ModalTrigger';
 
 describe('SavedQuery', () => {
   const mockedProps = {
@@ -23,25 +24,18 @@ describe('SavedQuery', () => {
       React.isValidElement(<SaveQuery {...mockedProps} />),
     ).to.equal(true);
   });
-  it('has an Overlay and a Popover', () => {
+  it('has a ModalTrigger', () => {
     const wrapper = shallow(<SaveQuery {...mockedProps} />);
-    expect(wrapper.find(Overlay)).to.have.length(1);
-    expect(wrapper.find(Popover)).to.have.length(1);
-  });
-  it('pops and hides', () => {
-    const wrapper = shallow(<SaveQuery {...mockedProps} />);
-    expect(wrapper.state().showSave).to.equal(false);
-    wrapper.find('.toggleSave').simulate('click', { target: { value: 'test' } });
-    expect(wrapper.state().showSave).to.equal(true);
-    wrapper.find('.toggleSave').simulate('click', { target: { value: 'test' } });
-    expect(wrapper.state().showSave).to.equal(false);
+    expect(wrapper.find(ModalTrigger)).to.have.length(1);
   });
   it('has a cancel button', () => {
     const wrapper = shallow(<SaveQuery {...mockedProps} />);
-    expect(wrapper.find('.cancelQuery')).to.have.length(1);
+    const modal = shallow(wrapper.instance().renderModalBody());
+    expect(modal.find('.cancelQuery')).to.have.length(1);
   });
   it('has 2 FormControls', () => {
     const wrapper = shallow(<SaveQuery {...mockedProps} />);
-    expect(wrapper.find(FormControl)).to.have.length(2);
+    const modal = shallow(wrapper.instance().renderModalBody());
+    expect(modal.find(FormControl)).to.have.length(2);
   });
 });