From 7f8b06ace742aa7e3c111a2275aa32496d492c99 Mon Sep 17 00:00:00 2001 From: Maxime Beauchemin Date: Fri, 28 Oct 2016 00:30:35 -0700 Subject: [PATCH 1/2] [sqllab] slide animations when adding/removing/toggling TableElement --- .../SqlLab/components/CopyQueryTabUrl.jsx | 9 +- .../SqlLab/components/TabbedSqlEditors.jsx | 3 +- .../SqlLab/components/TableElement.jsx | 180 ++++++++++-------- caravel/assets/package.json | 5 +- .../sqllab/CopyQueryTabUrl_spec.jsx | 11 +- .../javascripts/sqllab/TableElement_spec.jsx | 19 +- .../spec/javascripts/sqllab/fixtures.js | 10 + 7 files changed, 134 insertions(+), 103 deletions(-) diff --git a/caravel/assets/javascripts/SqlLab/components/CopyQueryTabUrl.jsx b/caravel/assets/javascripts/SqlLab/components/CopyQueryTabUrl.jsx index b011631406a11..4b5cc0c7dd9f6 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 43d126b119237..2c61ba6a5af9d 100644 --- a/caravel/assets/javascripts/SqlLab/components/TabbedSqlEditors.jsx +++ b/caravel/assets/javascripts/SqlLab/components/TabbedSqlEditors.jsx @@ -122,9 +122,10 @@ class TabbedSqlEditors extends React.PureComponent { rename tab - + {qe && + } ); diff --git a/caravel/assets/javascripts/SqlLab/components/TableElement.jsx b/caravel/assets/javascripts/SqlLab/components/TableElement.jsx index a4b0c2a20139d..acfbc424fbf08 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) { - e.preventDefault(); - this.props.actions.collapseTable(this.props.table); - } - - expandTable(e) { + toggleTable(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() { + getHeader() { 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 { ); } - if (table.expanded) { - buttonToggle = ( - { this.collapseTable(e); }} - > - {table.name} - - - ); - const cols = table.columns.slice(); + return header; + } + getMetadata() { + 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 = ( +
- {header} + {this.getHeader()}
- {cols.map((col) => { + {cols && cols.map((col) => { let name = col.name; if (col.indexed) { name = {col.name}; @@ -137,18 +137,17 @@ class TableElement extends React.PureComponent {
- ); - } else { - buttonToggle = ( - { this.expandTable(e); }} - > - {table.name} - - - ); - } + + ); + 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,69 @@ class TableElement extends React.PureComponent { ); } return ( -
-
-
- {buttonToggle} -
-
- - {keyLink} - +
+
+ +
+ + {keyLink} + + + {table.selectStar && + } + text={table.selectStar} + shouldShowText={false} + tooltipText="Copy SELECT statement to clipboard" + /> } - text={table.selectStar} - shouldShowText={false} - tooltipText="Copy SELECT statement to clipboard" - /> - - + + +
+
+
+ {this.getMetadata()}
-
- {metadata} -
-
+ ); } } diff --git a/caravel/assets/package.json b/caravel/assets/package.json index 433c3f4367abe..7ed5e3f3be6fa 100644 --- a/caravel/assets/package.json +++ b/caravel/assets/package.json @@ -90,11 +90,11 @@ "devDependencies": { "babel": "^6.3.26", "babel-cli": "^6.14.0", - "babel-polyfill": "^6.14.0", - "babel-preset-es2015": "^6.14.0", "babel-core": "^6.10.4", "babel-loader": "^6.2.4", + "babel-polyfill": "^6.14.0", "babel-preset-airbnb": "^2.1.1", + "babel-preset-es2015": "^6.14.0", "babel-preset-react": "^6.11.1", "chai": "^3.5.0", "codeclimate-test-reporter": "^0.3.3", @@ -115,6 +115,7 @@ "less-loader": "^2.2.2", "mocha": "^2.4.5", "react-addons-test-utils": "^15.3.2", + "sinon": "^1.17.6", "style-loader": "^0.13.0", "transform-loader": "^0.2.3", "url-loader": "^0.5.7", diff --git a/caravel/assets/spec/javascripts/sqllab/CopyQueryTabUrl_spec.jsx b/caravel/assets/spec/javascripts/sqllab/CopyQueryTabUrl_spec.jsx index 2cb4ed81d76ad..ccd00c41e6be0 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()).to.equal(true); - }); - it('renders with props', () => { + it('be valid with props', () => { expect( React.isValidElement() ).to.equal(true); }); - it('renders a CopyToClipboard', () => { - const wrapper = shallow(); - 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 e4abd859d57e5..90a73b9afdd8c 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(); + 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(); + 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 0440f81f101a5..2580e774ea3eb 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, From 86303b8668dcfaf7b0237d6710e71f8a4d642e3f Mon Sep 17 00:00:00 2001 From: Maxime Beauchemin Date: Fri, 28 Oct 2016 16:17:58 -0700 Subject: [PATCH 2/2] Adressing comments --- .../SqlLab/components/TabbedSqlEditors.jsx | 7 +++--- .../SqlLab/components/TableElement.jsx | 25 ++++++++++--------- .../sqllab/CopyQueryTabUrl_spec.jsx | 2 +- 3 files changed, 18 insertions(+), 16 deletions(-) diff --git a/caravel/assets/javascripts/SqlLab/components/TabbedSqlEditors.jsx b/caravel/assets/javascripts/SqlLab/components/TabbedSqlEditors.jsx index 2c61ba6a5af9d..0a722cb22c390 100644 --- a/caravel/assets/javascripts/SqlLab/components/TabbedSqlEditors.jsx +++ b/caravel/assets/javascripts/SqlLab/components/TabbedSqlEditors.jsx @@ -122,9 +122,10 @@ class TabbedSqlEditors extends React.PureComponent { rename tab - {qe && - - + {qe && + + + }
diff --git a/caravel/assets/javascripts/SqlLab/components/TableElement.jsx b/caravel/assets/javascripts/SqlLab/components/TableElement.jsx index acfbc424fbf08..3bad4b72b8d13 100644 --- a/caravel/assets/javascripts/SqlLab/components/TableElement.jsx +++ b/caravel/assets/javascripts/SqlLab/components/TableElement.jsx @@ -67,7 +67,7 @@ class TableElement extends React.PureComponent { this.setState({ sortColumns: !this.state.sortColumns }); } - getHeader() { + renderHeader() { const table = this.props.table; let header; if (table.partitions) { @@ -102,7 +102,7 @@ class TableElement extends React.PureComponent { } return header; } - getMetadata() { + renderMetadata() { const table = this.props.table; let cols; if (table.columns) { @@ -117,7 +117,7 @@ class TableElement extends React.PureComponent { timeout={this.props.timeout} >
- {this.getHeader()} + {this.renderHeader()}
{cols && cols.map((col) => { let name = col.name; @@ -210,14 +210,15 @@ class TableElement extends React.PureComponent { tooltip="Data preview" href="#" /> - {table.selectStar && - } - text={table.selectStar} - shouldShowText={false} - tooltipText="Copy SELECT statement to clipboard" - /> + {table.selectStar && + + } + text={table.selectStar} + shouldShowText={false} + tooltipText="Copy SELECT statement to clipboard" + /> }
- {this.getMetadata()} + {this.renderMetadata()}
diff --git a/caravel/assets/spec/javascripts/sqllab/CopyQueryTabUrl_spec.jsx b/caravel/assets/spec/javascripts/sqllab/CopyQueryTabUrl_spec.jsx index ccd00c41e6be0..909faffe373c6 100644 --- a/caravel/assets/spec/javascripts/sqllab/CopyQueryTabUrl_spec.jsx +++ b/caravel/assets/spec/javascripts/sqllab/CopyQueryTabUrl_spec.jsx @@ -8,7 +8,7 @@ describe('CopyQueryTabUrl', () => { const mockedProps = { queryEditor: initialState.queryEditors[0], }; - it('be valid with props', () => { + it('is valid with props', () => { expect( React.isValidElement() ).to.equal(true);