From ab083b86f35c6f01dfa16bf84c8c212bf21743cf Mon Sep 17 00:00:00 2001
From: Maxime Beauchemin <maximebeauchemin@gmail.com>
Date: Sat, 29 Oct 2016 07:48:17 -0700
Subject: [PATCH] [sqllab] slide animations when adding/removing/toggling
 TableElement (#1472)

* [sqllab] slide animations when adding/removing/toggling TableElement

* Adressing comments
---
 .../SqlLab/components/CopyQueryTabUrl.jsx     |   9 +-
 .../SqlLab/components/TabbedSqlEditors.jsx    |   8 +-
 .../SqlLab/components/TableElement.jsx        | 181 ++++++++++--------
 .../sqllab/CopyQueryTabUrl_spec.jsx           |  11 +-
 .../javascripts/sqllab/TableElement_spec.jsx  |  19 +-
 .../spec/javascripts/sqllab/fixtures.js       |  10 +
 6 files changed, 135 insertions(+), 103 deletions(-)

diff --git a/caravel/assets/javascripts/SqlLab/components/CopyQueryTabUrl.jsx b/caravel/assets/javascripts/SqlLab/components/CopyQueryTabUrl.jsx
index b011631406..4b5cc0c7dd 100644
--- a/caravel/assets/javascripts/SqlLab/components/CopyQueryTabUrl.jsx
+++ b/caravel/assets/javascripts/SqlLab/components/CopyQueryTabUrl.jsx
@@ -3,11 +3,7 @@ import CopyToClipboard from '../../components/CopyToClipboard';
 import { getShortUrl } from '../../../utils/common';
 
 const propTypes = {
-  queryEditor: React.PropTypes.object,
-};
-
-const defaultProps = {
-  queryEditor: null,
+  queryEditor: React.PropTypes.object.isRequired,
 };
 
 export default class CopyQueryTabUrl extends React.PureComponent {
@@ -19,8 +15,8 @@ export default class CopyQueryTabUrl extends React.PureComponent {
   }
 
   componentWillMount() {
-    const params = [];
     const qe = this.props.queryEditor;
+    const params = [];
     if (qe.dbId) params.push('dbid=' + qe.dbId);
     if (qe.title) params.push('title=' + encodeURIComponent(qe.title));
     if (qe.schema) params.push('schema=' + encodeURIComponent(qe.schema));
@@ -52,4 +48,3 @@ export default class CopyQueryTabUrl extends React.PureComponent {
 }
 
 CopyQueryTabUrl.propTypes = propTypes;
-CopyQueryTabUrl.defaultProps = defaultProps;
diff --git a/caravel/assets/javascripts/SqlLab/components/TabbedSqlEditors.jsx b/caravel/assets/javascripts/SqlLab/components/TabbedSqlEditors.jsx
index 43d126b119..0a722cb22c 100644
--- a/caravel/assets/javascripts/SqlLab/components/TabbedSqlEditors.jsx
+++ b/caravel/assets/javascripts/SqlLab/components/TabbedSqlEditors.jsx
@@ -122,9 +122,11 @@ class TabbedSqlEditors extends React.PureComponent {
             <MenuItem eventKey="2" onClick={this.renameTab.bind(this, qe)}>
               <i className="fa fa-i-cursor" /> rename tab
             </MenuItem>
-            <MenuItem eventKey="3">
-              <i className="fa fa-clipboard" /> <CopyQueryTabUrl queryEditor={qe} />
-            </MenuItem>
+            {qe &&
+              <MenuItem eventKey="3">
+                <i className="fa fa-clipboard" /> <CopyQueryTabUrl queryEditor={qe} />
+              </MenuItem>
+            }
           </DropdownButton>
         </div>
       );
diff --git a/caravel/assets/javascripts/SqlLab/components/TableElement.jsx b/caravel/assets/javascripts/SqlLab/components/TableElement.jsx
index a4b0c2a201..3bad4b72b8 100644
--- a/caravel/assets/javascripts/SqlLab/components/TableElement.jsx
+++ b/caravel/assets/javascripts/SqlLab/components/TableElement.jsx
@@ -1,6 +1,6 @@
 import React from 'react';
 
-import { ButtonGroup, Well } from 'react-bootstrap';
+import { ButtonGroup, Collapse, Well } from 'react-bootstrap';
 import shortid from 'shortid';
 
 import CopyToClipboard from '../../components/CopyToClipboard';
@@ -10,11 +10,13 @@ import ModalTrigger from '../../components/ModalTrigger';
 const propTypes = {
   table: React.PropTypes.object,
   actions: React.PropTypes.object,
+  timeout: React.PropTypes.integer,  // used for tests
 };
 
 const defaultProps = {
-  table: null,
   actions: {},
+  table: null,
+  timeout: 500,
 };
 
 class TableElement extends React.PureComponent {
@@ -22,6 +24,7 @@ class TableElement extends React.PureComponent {
     super(props);
     this.state = {
       sortColumns: false,
+      expanded: true,
     };
   }
 
@@ -36,18 +39,17 @@ class TableElement extends React.PureComponent {
     this.props.actions.addQueryEditor(qe);
   }
 
-  collapseTable(e) {
+  toggleTable(e) {
     e.preventDefault();
-    this.props.actions.collapseTable(this.props.table);
-  }
-
-  expandTable(e) {
-    e.preventDefault();
-    this.props.actions.expandTable(this.props.table);
+    if (this.props.table.expanded) {
+      this.props.actions.collapseTable(this.props.table);
+    } else {
+      this.props.actions.expandTable(this.props.table);
+    }
   }
 
   removeTable() {
-    this.props.actions.removeTable(this.props.table);
+    this.setState({ expanded: false });
   }
   dataPreviewModal() {
     const query = {
@@ -65,11 +67,8 @@ class TableElement extends React.PureComponent {
     this.setState({ sortColumns: !this.state.sortColumns });
   }
 
-  render() {
+  renderHeader() {
     const table = this.props.table;
-    let metadata = null;
-    let buttonToggle;
-
     let header;
     if (table.partitions) {
       let partitionQuery;
@@ -101,25 +100,26 @@ class TableElement extends React.PureComponent {
         </Well>
       );
     }
-    if (table.expanded) {
-      buttonToggle = (
-        <a
-          href="#"
-          onClick={(e) => { this.collapseTable(e); }}
-        >
-          <strong>{table.name}</strong>
-          <small className="m-l-5"><i className="fa fa-minus" /></small>
-        </a>
-      );
-      const cols = table.columns.slice();
+    return header;
+  }
+  renderMetadata() {
+    const table = this.props.table;
+    let cols;
+    if (table.columns) {
+      cols = table.columns.slice();
       if (this.state.sortColumns) {
         cols.sort((a, b) => a.name.toUpperCase() > b.name.toUpperCase());
       }
-      metadata = (
+    }
+    const metadata = (
+      <Collapse
+        in={table.expanded}
+        timeout={this.props.timeout}
+      >
         <div>
-          {header}
+          {this.renderHeader()}
           <div className="table-columns">
-            {cols.map((col) => {
+            {cols && cols.map((col) => {
               let name = col.name;
               if (col.indexed) {
                 name = <strong>{col.name}</strong>;
@@ -137,18 +137,17 @@ class TableElement extends React.PureComponent {
             <hr />
           </div>
         </div>
-      );
-    } else {
-      buttonToggle = (
-        <a
-          href="#"
-          onClick={(e) => { this.expandTable(e); }}
-        >
-          {table.name}
-          <small className="m-l-5"><i className="fa fa-plus" /></small>
-        </a>
-      );
-    }
+      </Collapse>
+    );
+    return metadata;
+  }
+  removeFromStore() {
+    this.props.actions.removeTable(this.props.table);
+  }
+
+  render() {
+    const table = this.props.table;
+
     let keyLink;
     if (table.indexes && table.indexes.length > 0) {
       keyLink = (
@@ -171,52 +170,70 @@ class TableElement extends React.PureComponent {
       );
     }
     return (
-      <div className="TableElement">
-        <div className="clearfix">
-          <div className="pull-left">
-            {buttonToggle}
-          </div>
-          <div className="pull-right">
-            <ButtonGroup className="ws-el-controls pull-right">
-              {keyLink}
-              <Link
-                className={
-                  `fa fa-sort-${!this.state.sortColumns ? 'alpha' : 'numeric'}-asc ` +
-                  'pull-left sort-cols m-l-2'}
-                onClick={this.toggleSortColumns.bind(this)}
-                tooltip={
-                  !this.state.sortColumns ?
-                  'Sort columns alphabetically' :
-                  'Original table column order'}
+      <Collapse
+        in={this.state.expanded}
+        timeout={this.props.timeout}
+        transitionAppear
+        onExited={this.removeFromStore.bind(this)}
+      >
+        <div className="TableElement">
+          <div className="clearfix">
+            <div className="pull-left">
+              <a
                 href="#"
-              />
-              <Link
-                className="fa fa-search-plus pull-left m-l-2"
-                onClick={this.dataPreviewModal.bind(this)}
-                tooltip="Data preview"
-                href="#"
-              />
-              <CopyToClipboard
-                copyNode={
-                  <a className="fa fa-clipboard pull-left m-l-2" />
+                className="table-name"
+                onClick={(e) => { this.toggleTable(e); }}
+              >
+                <strong>{table.name}</strong>
+                <small className="m-l-5">
+                  <i className={`fa fa-${table.expanded ? 'minus' : 'plus'}-square-o`} />
+                </small>
+              </a>
+            </div>
+            <div className="pull-right">
+              <ButtonGroup className="ws-el-controls pull-right">
+                {keyLink}
+                <Link
+                  className={
+                    `fa fa-sort-${!this.state.sortColumns ? 'alpha' : 'numeric'}-asc ` +
+                    'pull-left sort-cols m-l-2'}
+                  onClick={this.toggleSortColumns.bind(this)}
+                  tooltip={
+                    !this.state.sortColumns ?
+                    'Sort columns alphabetically' :
+                    'Original table column order'}
+                  href="#"
+                />
+                <Link
+                  className="fa fa-search-plus pull-left m-l-2"
+                  onClick={this.dataPreviewModal.bind(this)}
+                  tooltip="Data preview"
+                  href="#"
+                />
+                {table.selectStar &&
+                  <CopyToClipboard
+                    copyNode={
+                      <a className="fa fa-clipboard pull-left m-l-2" />
+                    }
+                    text={table.selectStar}
+                    shouldShowText={false}
+                    tooltipText="Copy SELECT statement to clipboard"
+                  />
                 }
-                text={table.selectStar}
-                shouldShowText={false}
-                tooltipText="Copy SELECT statement to clipboard"
-              />
-              <Link
-                className="fa fa-trash pull-left m-l-2"
-                onClick={this.removeTable.bind(this)}
-                tooltip="Remove from panel"
-                href="#"
-              />
-            </ButtonGroup>
+                <Link
+                  className="fa fa-trash table-remove pull-left m-l-2"
+                  onClick={this.removeTable.bind(this)}
+                  tooltip="Remove from panel"
+                  href="#"
+                />
+              </ButtonGroup>
+            </div>
+          </div>
+          <div>
+            {this.renderMetadata()}
           </div>
         </div>
-        <div>
-          {metadata}
-        </div>
-      </div>
+      </Collapse>
     );
   }
 }
diff --git a/caravel/assets/spec/javascripts/sqllab/CopyQueryTabUrl_spec.jsx b/caravel/assets/spec/javascripts/sqllab/CopyQueryTabUrl_spec.jsx
index 2cb4ed81d7..909faffe37 100644
--- a/caravel/assets/spec/javascripts/sqllab/CopyQueryTabUrl_spec.jsx
+++ b/caravel/assets/spec/javascripts/sqllab/CopyQueryTabUrl_spec.jsx
@@ -1,7 +1,5 @@
 import React from 'react';
 import CopyQueryTabUrl from '../../../javascripts/SqlLab/components/CopyQueryTabUrl';
-import CopyToClipboard from '../../../javascripts/components/CopyToClipboard';
-import { shallow } from 'enzyme';
 import { describe, it } from 'mocha';
 import { expect } from 'chai';
 import { initialState } from './fixtures';
@@ -10,16 +8,9 @@ describe('CopyQueryTabUrl', () => {
   const mockedProps = {
     queryEditor: initialState.queryEditors[0],
   };
-  it('should be valid', () => {
-    expect(React.isValidElement(<CopyQueryTabUrl />)).to.equal(true);
-  });
-  it('renders with props', () => {
+  it('is valid with props', () => {
     expect(
       React.isValidElement(<CopyQueryTabUrl {...mockedProps} />)
     ).to.equal(true);
   });
-  it('renders a CopyToClipboard', () => {
-    const wrapper = shallow(<CopyQueryTabUrl {...mockedProps} />);
-    expect(wrapper.find(CopyToClipboard)).to.have.length(1);
-  });
 });
diff --git a/caravel/assets/spec/javascripts/sqllab/TableElement_spec.jsx b/caravel/assets/spec/javascripts/sqllab/TableElement_spec.jsx
index e4abd859d5..90a73b9afd 100644
--- a/caravel/assets/spec/javascripts/sqllab/TableElement_spec.jsx
+++ b/caravel/assets/spec/javascripts/sqllab/TableElement_spec.jsx
@@ -1,7 +1,7 @@
 import React from 'react';
 import Link from '../../../javascripts/SqlLab/components/Link';
 import TableElement from '../../../javascripts/SqlLab/components/TableElement';
-import { table } from './fixtures';
+import { mockedActions, table } from './fixtures';
 import { mount, shallow } from 'enzyme';
 import { describe, it } from 'mocha';
 import { expect } from 'chai';
@@ -9,7 +9,9 @@ import { expect } from 'chai';
 
 describe('TableElement', () => {
   const mockedProps = {
+    actions: mockedActions,
     table,
+    timeout: 0,
   };
   it('renders', () => {
     expect(
@@ -40,4 +42,19 @@ describe('TableElement', () => {
     expect(wrapper.state().sortColumns).to.equal(true);
     expect(wrapper.find('.col-name').first().text()).to.equal('last_login');
   });
+  it('calls the collapseTable action', () => {
+    const wrapper = mount(<TableElement {...mockedProps} />);
+    expect(mockedActions.collapseTable.called).to.equal(false);
+    wrapper.find('.table-name').simulate('click');
+    expect(mockedActions.collapseTable.called).to.equal(true);
+  });
+  it('removes the table', () => {
+    const wrapper = mount(<TableElement {...mockedProps} />);
+    expect(wrapper.state().expanded).to.equal(true);
+    wrapper.find('.table-remove').simulate('click');
+    expect(wrapper.state().expanded).to.equal(false);
+    setTimeout(() => {
+      expect(mockedActions.removeTable.called).to.equal(true);
+    }, 10);
+  });
 });
diff --git a/caravel/assets/spec/javascripts/sqllab/fixtures.js b/caravel/assets/spec/javascripts/sqllab/fixtures.js
index 0440f81f10..2580e774ea 100644
--- a/caravel/assets/spec/javascripts/sqllab/fixtures.js
+++ b/caravel/assets/spec/javascripts/sqllab/fixtures.js
@@ -1,3 +1,8 @@
+import * as actions from '../../../javascripts/SqlLab/actions';
+import sinon from 'sinon';
+
+export const mockedActions = sinon.stub(Object.assign({}, actions));
+
 export const alert = { bsStyle: 'danger', msg: 'Ooops', id: 'lksvmcx32' };
 export const table = {
   dbId: 1,
@@ -6,6 +11,11 @@ export const table = {
   schema: 'caravel',
   name: 'ab_user',
   id: 'r11Vgt60',
+  partitions: {
+    cols: ['username'],
+    latest: 'bob',
+    partitionQuery: 'SHOW PARTITIONS FROM ab_user',
+  },
   indexes: [
     {
       unique: true,