Skip to content

Commit

Permalink
refactor: Refactor QueryTable to use react-table (#11216)
Browse files Browse the repository at this point in the history
* Refactor QueryTable to use react-table

* Fix lodash import

* Fix tests

* Fix imports and QuerySearch styles

* Update superset-frontend/src/SqlLab/components/QuerySearch.jsx

Co-authored-by: Evan Rusackas <[email protected]>

* Update superset-frontend/src/SqlLab/components/QuerySearch.jsx

Co-authored-by: Evan Rusackas <[email protected]>

* Lint fix

* Refactored QueryTable into functional component

* Remove calculating content height

* Lint fix

Co-authored-by: Evan Rusackas <[email protected]>
  • Loading branch information
kgabryje and rusackas authored Oct 19, 2020
1 parent a1f8429 commit 735123d
Show file tree
Hide file tree
Showing 5 changed files with 90 additions and 112 deletions.
9 changes: 0 additions & 9 deletions superset-frontend/spec/javascripts/sqllab/App_spec.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import configureStore from 'redux-mock-store';
import thunk from 'redux-thunk';

import { shallow } from 'enzyme';
import sinon from 'sinon';

import App from 'src/SqlLab/components/App';
import TabbedSqlEditors from 'src/SqlLab/components/TabbedSqlEditors';
Expand All @@ -41,14 +40,6 @@ describe('SqlLab App', () => {
expect(React.isValidElement(<App />)).toBe(true);
});

it('should handler resize', () => {
const inner = wrapper.dive();
sinon.spy(inner.instance(), 'getHeight');
inner.instance().handleResize();
expect(inner.instance().getHeight.callCount).toBe(1);
inner.instance().getHeight.restore();
});

it('should render', () => {
const inner = wrapper.dive();
expect(inner.find('.SqlLab')).toHaveLength(1);
Expand Down
18 changes: 11 additions & 7 deletions superset-frontend/spec/javascripts/sqllab/QueryTable_spec.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@
*/
import React from 'react';
import { shallow } from 'enzyme';
import { Table } from 'reactable-arc';
import QueryTable from 'src/SqlLab/components/QueryTable';

import TableView from 'src/components/TableView';
import { TableCollection } from 'src/components/dataViewCommon';
import { queries } from './fixtures';

describe('QueryTable', () => {
Expand All @@ -35,10 +35,14 @@ describe('QueryTable', () => {
});
it('renders a proper table', () => {
const wrapper = shallow(<QueryTable {...mockedProps} />);
expect(wrapper.find(Table)).toExist();
expect(wrapper.find(Table).shallow().find('table')).toExist();
expect(wrapper.find(Table).shallow().find('table').find('Tr')).toHaveLength(
2,
);
const tableWrapper = wrapper
.find(TableView)
.shallow()
.find(TableCollection)
.shallow();
expect(wrapper.find(TableView)).toExist();
expect(tableWrapper.find('table')).toExist();
expect(tableWrapper.find('table').find('thead').find('tr')).toHaveLength(1);
expect(tableWrapper.find('table').find('tbody').find('tr')).toHaveLength(2);
});
});
37 changes: 0 additions & 37 deletions superset-frontend/src/SqlLab/components/App.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import React from 'react';
import PropTypes from 'prop-types';
import { bindActionCreators } from 'redux';
import { connect } from 'react-redux';
import $ from 'jquery';
import { t, supersetTheme, ThemeProvider } from '@superset-ui/core';
import throttle from 'lodash/throttle';
import TabbedSqlEditors from './TabbedSqlEditors';
Expand All @@ -39,7 +38,6 @@ class App extends React.PureComponent {
super(props);
this.state = {
hash: window.location.hash,
contentHeight: '0px',
};

this.showLocalStorageUsageWarning = throttle(
Expand All @@ -50,10 +48,7 @@ class App extends React.PureComponent {
}

componentDidMount() {
/* eslint-disable react/no-did-mount-set-state */
this.setState({ contentHeight: this.getHeight() });
window.addEventListener('hashchange', this.onHashChanged.bind(this));
window.addEventListener('resize', this.handleResize.bind(this));
}

componentDidUpdate() {
Expand All @@ -69,39 +64,12 @@ class App extends React.PureComponent {

componentWillUnmount() {
window.removeEventListener('hashchange', this.onHashChanged.bind(this));
window.removeEventListener('resize', this.handleResize.bind(this));
}

onHashChanged() {
this.setState({ hash: window.location.hash });
}

getHeight() {
const warningEl = $('#navbar-warning');
const tabsEl = $('.nav-tabs');
const searchHeaderEl = $('#search-header');
const alertEl = $('#sqllab-alerts');
const headerEl = $('header .navbar');
const headerHeight =
headerEl.outerHeight() + parseInt(headerEl.css('marginBottom'), 10);
const searchHeaderHeight =
searchHeaderEl.length > 0
? searchHeaderEl.outerHeight() +
parseInt(searchHeaderEl.css('marginBottom'), 10)
: 0;
const tabsHeight =
tabsEl.length > 0 ? tabsEl.outerHeight() : searchHeaderHeight;
const warningHeight = warningEl.length > 0 ? warningEl.outerHeight() : 0;
const alertHeight = alertEl.length > 0 ? alertEl.outerHeight() : 0;
return `${
window.innerHeight -
headerHeight -
tabsHeight -
warningHeight -
alertHeight
}px`;
}

showLocalStorageUsageWarning(currentUsage) {
this.props.actions.addDangerToast(
t(
Expand All @@ -116,16 +84,11 @@ class App extends React.PureComponent {
);
}

handleResize() {
this.setState({ contentHeight: this.getHeight() });
}

render() {
let content;
if (this.state.hash) {
content = (
<QuerySearch
height={this.state.contentHeight}
actions={this.props.actions}
displayLimit={this.props.common.conf.DISPLAY_MAX_ROW}
/>
Expand Down
28 changes: 20 additions & 8 deletions superset-frontend/src/SqlLab/components/QuerySearch.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import React from 'react';
import PropTypes from 'prop-types';
import Button from 'src/components/Button';
import Select from 'src/components/Select';
import { t, SupersetClient } from '@superset-ui/core';
import { styled, t, SupersetClient } from '@superset-ui/core';

import Loading from '../../components/Loading';
import QueryTable from './QueryTable';
Expand All @@ -39,6 +39,21 @@ const propTypes = {
displayLimit: PropTypes.number.isRequired,
};

const TableWrapper = styled.div`
display: flex;
flex-direction: column;
flex: 1;
height: 100%;
`;

const TableStyles = styled.div`
.table > thead > tr > th {
border-bottom: ${({ theme }) => theme.gridUnit / 2}px solid
${({ theme }) => theme.colors.grayscale.light2};
background: ${({ theme }) => theme.colors.grayscale.light4};
}
`;

class QuerySearch extends React.PureComponent {
constructor(props) {
super(props);
Expand Down Expand Up @@ -201,7 +216,7 @@ class QuerySearch extends React.PureComponent {

render() {
return (
<div>
<TableWrapper>
<div id="search-header" className="row space-1">
<div className="col-sm-2">
<AsyncSelect
Expand Down Expand Up @@ -278,10 +293,7 @@ class QuerySearch extends React.PureComponent {
{this.state.queriesLoading ? (
<Loading />
) : (
<div
className="scrollbar-content"
style={{ height: this.props.height }}
>
<TableStyles className="scrollbar-content">
<QueryTable
columns={[
'state',
Expand All @@ -299,10 +311,10 @@ class QuerySearch extends React.PureComponent {
actions={this.props.actions}
displayLimit={this.props.displayLimit}
/>
</div>
</TableStyles>
)}
</div>
</div>
</TableWrapper>
);
}
}
Expand Down
Loading

0 comments on commit 735123d

Please sign in to comment.