From 004a6d9e54fe55cd497c1b5c74a097c467e4caba Mon Sep 17 00:00:00 2001 From: Corbin Robb <31329271+corbinrobb@users.noreply.github.com> Date: Thu, 3 Jun 2021 17:04:56 -0600 Subject: [PATCH] refactor: Convert TableElement.jsx component from class to functional with hooks (#14830) * Change TableElement from a class component to a functional component * Replace class state checks in TableElement_spec.jsx with checks testing elements they change * Refactor small bit of logic to use optional chaining * Add optional chaining to some logic * Fix IconTooltip and add IconTooltip to the collapse button * Fix custom icon using IconToolTip so it better matches the original * Update collapse/expand icon to use Icons component instead of importing from antdesign directly * Fix eslint errors * Clean up some code for readability Co-authored-by: Corbin Robb --- .../javascripts/sqllab/TableElement_spec.jsx | 24 ++- .../src/SqlLab/components/TableElement.jsx | 202 +++++++++--------- 2 files changed, 118 insertions(+), 108 deletions(-) diff --git a/superset-frontend/spec/javascripts/sqllab/TableElement_spec.jsx b/superset-frontend/spec/javascripts/sqllab/TableElement_spec.jsx index d55720637b..8c07008815 100644 --- a/superset-frontend/spec/javascripts/sqllab/TableElement_spec.jsx +++ b/superset-frontend/spec/javascripts/sqllab/TableElement_spec.jsx @@ -43,7 +43,7 @@ describe('TableElement', () => { it('renders with props', () => { expect(React.isValidElement()).toBe(true); }); - it('has 4 IconTooltip elements', () => { + it('has 5 IconTooltip elements', () => { const wrapper = mount( @@ -55,14 +55,14 @@ describe('TableElement', () => { }, }, ); - expect(wrapper.find(IconTooltip)).toHaveLength(4); + expect(wrapper.find(IconTooltip)).toHaveLength(5); }); it('has 14 columns', () => { const wrapper = shallow(); expect(wrapper.find(ColumnElement)).toHaveLength(14); }); it('mounts', () => { - mount( + const wrapper = mount( , @@ -73,6 +73,8 @@ describe('TableElement', () => { }, }, ); + + expect(wrapper.find(TableElement)).toHaveLength(1); }); it('fades table', async () => { const wrapper = mount( @@ -86,13 +88,11 @@ describe('TableElement', () => { }, }, ); - expect(wrapper.find(TableElement).state().hovered).toBe(false); expect(wrapper.find('[data-test="fade"]').first().props().hovered).toBe( false, ); wrapper.find('.header-container').hostNodes().simulate('mouseEnter'); await waitForComponentToPaint(wrapper, 300); - expect(wrapper.find(TableElement).state().hovered).toBe(true); expect(wrapper.find('[data-test="fade"]').first().props().hovered).toBe( true, ); @@ -111,12 +111,22 @@ describe('TableElement', () => { }, }, ); - expect(wrapper.find(TableElement).state().sortColumns).toBe(false); + expect( + wrapper.find(IconTooltip).at(2).hasClass('fa-sort-alpha-asc'), + ).toEqual(true); + expect( + wrapper.find(IconTooltip).at(2).hasClass('fa-sort-numeric-asc'), + ).toEqual(false); wrapper.find('.header-container').hostNodes().simulate('click'); expect(wrapper.find(ColumnElement).first().props().column.name).toBe('id'); wrapper.find('.header-container').simulate('mouseEnter'); wrapper.find('.sort-cols').hostNodes().simulate('click'); - expect(wrapper.find(TableElement).state().sortColumns).toBe(true); + expect( + wrapper.find(IconTooltip).at(2).hasClass('fa-sort-numeric-asc'), + ).toEqual(true); + expect( + wrapper.find(IconTooltip).at(2).hasClass('fa-sort-alpha-asc'), + ).toEqual(false); expect(wrapper.find(ColumnElement).first().props().column.name).toBe( 'active', ); diff --git a/superset-frontend/src/SqlLab/components/TableElement.jsx b/superset-frontend/src/SqlLab/components/TableElement.jsx index e44cccf0d6..384771a96e 100644 --- a/superset-frontend/src/SqlLab/components/TableElement.jsx +++ b/superset-frontend/src/SqlLab/components/TableElement.jsx @@ -16,16 +16,16 @@ * specific language governing permissions and limitations * under the License. */ -import React from 'react'; +import React, { useState } from 'react'; import PropTypes from 'prop-types'; import Collapse from 'src/components/Collapse'; import Card from 'src/components/Card'; import ButtonGroup from 'src/components/ButtonGroup'; -import shortid from 'shortid'; import { t, styled } from '@superset-ui/core'; import { debounce } from 'lodash'; import { Tooltip } from 'src/components/Tooltip'; +import Icons from 'src/components/Icons'; import CopyToClipboard from '../../components/CopyToClipboard'; import { IconTooltip } from '../../components/IconTooltip'; import ColumnElement from './ColumnElement'; @@ -56,44 +56,26 @@ const Fade = styled.div` opacity: ${props => (props.hovered ? 1 : 0)}; `; -class TableElement extends React.PureComponent { - constructor(props) { - super(props); - this.state = { - sortColumns: false, - hovered: false, - }; - this.toggleSortColumns = this.toggleSortColumns.bind(this); - this.removeTable = this.removeTable.bind(this); - this.setHover = debounce(this.setHover.bind(this), 100); - } +const TableElement = props => { + const [sortColumns, setSortColumns] = useState(false); + const [hovered, setHovered] = useState(false); - setHover(hovered) { - this.setState({ hovered }); - } + const { table, actions, isActive } = props; - popSelectStar() { - const qe = { - id: shortid.generate(), - title: this.props.table.name, - dbId: this.props.table.dbId, - autorun: true, - sql: this.props.table.selectStar, - }; - this.props.actions.addQueryEditor(qe); - } + const setHover = hovered => { + debounce(() => setHovered(hovered), 100)(); + }; - removeTable() { - this.props.actions.removeDataPreview(this.props.table); - this.props.actions.removeTable(this.props.table); - } + const removeTable = () => { + actions.removeDataPreview(table); + actions.removeTable(table); + }; - toggleSortColumns() { - this.setState(prevState => ({ sortColumns: !prevState.sortColumns })); - } + const toggleSortColumns = () => { + setSortColumns(prevState => !prevState); + }; - renderWell() { - const { table } = this.props; + const renderWell = () => { let header; if (table.partitions) { let partitionQuery; @@ -126,12 +108,11 @@ class TableElement extends React.PureComponent { ); } return header; - } + }; - renderControls() { + const renderControls = () => { let keyLink; - const { table } = this.props; - if (table.indexes && table.indexes.length > 0) { + if (table?.indexes?.length) { keyLink = ( + } text={table.selectStar} shouldShowText={false} - tooltipText={t('Copy SELECT statement to the clipboard')} /> )} {table.view && ( @@ -187,56 +170,52 @@ class TableElement extends React.PureComponent { )} ); - } + }; - renderHeader() { - const { table } = this.props; - return ( -
this.setHover(true)} - onMouseLeave={() => this.setHover(false)} + const renderHeader = () => ( +
setHover(true)} + onMouseLeave={() => setHover(false)} + > + - - - {table.name} - - + + {table.name} + + -
- {table.isMetadataLoading || table.isExtraMetadataLoading ? ( - - ) : ( - e.stopPropagation()} - > - {this.renderControls()} - - )} -
+
+ {table.isMetadataLoading || table.isExtraMetadataLoading ? ( + + ) : ( + e.stopPropagation()} + > + {renderControls()} + + )}
- ); - } +
+ ); - renderBody() { - const { table } = this.props; + const renderBody = () => { let cols; if (table.columns) { cols = table.columns.slice(); - if (this.state.sortColumns) { + if (sortColumns) { cols.sort((a, b) => { const colA = a.name.toUpperCase(); const colB = b.name.toUpperCase(); @@ -253,33 +232,54 @@ class TableElement extends React.PureComponent { const metadata = (
this.setHover(true)} - onMouseLeave={() => this.setHover(false)} + onMouseEnter={() => setHover(true)} + onMouseLeave={() => setHover(false)} css={{ paddingTop: 6 }} > - {this.renderWell()} + {renderWell()}
- {cols && - cols.map(col => )} + {cols?.map(col => ( + + ))}
); return metadata; - } + }; - render() { - return ( - - {this.renderBody()} - - ); - } -} + const collapseExpandIcon = () => ( + + + + ); + + return ( + + {renderBody()} + + ); +}; TableElement.propTypes = propTypes; TableElement.defaultProps = defaultProps;