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 <[email protected]>
apache · Jun 3, 2021 · 004a6d9 · 004a6d9
1 parent f652908
commit 004a6d9
Show file tree

Hide file tree

Showing 2 changed files with 118 additions and 108 deletions.
diff --git 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(<TableElement {...mockedProps} />)).toBe(true);
   });
-  it('has 4 IconTooltip elements', () => {
+  it('has 5 IconTooltip elements', () => {
     const wrapper = mount(
       <Provider store={store}>
         <TableElement {...mockedProps} />
@@ -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(<TableElement {...mockedProps} />);
     expect(wrapper.find(ColumnElement)).toHaveLength(14);
   });
   it('mounts', () => {
-    mount(
+    const wrapper = mount(
       <Provider store={store}>
         <TableElement {...mockedProps} />
       </Provider>,
@@ -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
@@ -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 = (
         <ModalTrigger
           modalTitle={
@@ -156,26 +137,28 @@ class TableElement extends React.PureComponent {
         {keyLink}
         <IconTooltip
           className={
-            `fa fa-sort-${!this.state.sortColumns ? 'alpha' : 'numeric'}-asc ` +
+            `fa fa-sort-${sortColumns ? 'numeric' : 'alpha'}-asc ` +
             'pull-left sort-cols m-l-2 pointer'
           }
-          onClick={this.toggleSortColumns}
+          onClick={toggleSortColumns}
           tooltip={
-            !this.state.sortColumns
+            !sortColumns
               ? t('Sort columns alphabetically')
               : t('Original table column order')
           }
         />
         {table.selectStar && (
           <CopyToClipboard
             copyNode={
-              <IconTooltip aria-label="Copy">
+              <IconTooltip
+                aria-label="Copy"
+                tooltip={t('Copy SELECT statement to the clipboard')}
+              >
                 <i aria-hidden className="fa fa-clipboard pull-left m-l-2" />
               </IconTooltip>
             }
             text={table.selectStar}
             shouldShowText={false}
-            tooltipText={t('Copy SELECT statement to the clipboard')}
           />
         )}
         {table.view && (
@@ -187,56 +170,52 @@ class TableElement extends React.PureComponent {
         )}
         <IconTooltip
           className="fa fa-times table-remove pull-left m-l-2 pointer"
-          onClick={this.removeTable}
+          onClick={removeTable}
           tooltip={t('Remove table preview')}
         />
       </ButtonGroup>
     );
-  }
+  };
 
-  renderHeader() {
-    const { table } = this.props;
-    return (
-      <div
-        className="clearfix header-container"
-        onMouseEnter={() => this.setHover(true)}
-        onMouseLeave={() => this.setHover(false)}
+  const renderHeader = () => (
+    <div
+      className="clearfix header-container"
+      onMouseEnter={() => setHover(true)}
+      onMouseLeave={() => setHover(false)}
+    >
+      <Tooltip
+        id="copy-to-clipboard-tooltip"
+        placement="topLeft"
+        style={{ cursor: 'pointer' }}
+        title={table.name}
+        trigger={['hover']}
       >
-        <Tooltip
-          id="copy-to-clipboard-tooltip"
-          placement="topLeft"
-          style={{ cursor: 'pointer' }}
-          title={table.name}
-          trigger={['hover']}
-        >
-          <StyledSpan data-test="collapse" className="table-name">
-            <strong>{table.name}</strong>
-          </StyledSpan>
-        </Tooltip>
+        <StyledSpan data-test="collapse" className="table-name">
+          <strong>{table.name}</strong>
+        </StyledSpan>
+      </Tooltip>
 
-        <div className="pull-right header-right-side">
-          {table.isMetadataLoading || table.isExtraMetadataLoading ? (
-            <Loading position="inline" />
-          ) : (
-            <Fade
-              data-test="fade"
-              hovered={this.state.hovered}
-              onClick={e => e.stopPropagation()}
-            >
-              {this.renderControls()}
-            </Fade>
-          )}
-        </div>
+      <div className="pull-right header-right-side">
+        {table.isMetadataLoading || table.isExtraMetadataLoading ? (
+          <Loading position="inline" />
+        ) : (
+          <Fade
+            data-test="fade"
+            hovered={hovered}
+            onClick={e => e.stopPropagation()}
+          >
+            {renderControls()}
+          </Fade>
+        )}
       </div>
-    );
-  }
+    </div>
+  );
 
-  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 = (
       <div
-        onMouseEnter={() => this.setHover(true)}
-        onMouseLeave={() => this.setHover(false)}
+        onMouseEnter={() => setHover(true)}
+        onMouseLeave={() => setHover(false)}
         css={{ paddingTop: 6 }}
       >
-        {this.renderWell()}
+        {renderWell()}
         <div>
-          {cols &&
-            cols.map(col => <ColumnElement column={col} key={col.name} />)}
+          {cols?.map(col => (
+            <ColumnElement column={col} key={col.name} />
+          ))}
         </div>
       </div>
     );
     return metadata;
-  }
+  };
 
-  render() {
-    return (
-      <Collapse.Panel
-        {...this.props}
-        header={this.renderHeader()}
-        className="TableElement"
-        forceRender="true"
-      >
-        {this.renderBody()}
-      </Collapse.Panel>
-    );
-  }
-}
+  const collapseExpandIcon = () => (
+    <IconTooltip
+      style={{
+        position: 'fixed',
+        right: '16px',
+        left: 'auto',
+        fontSize: '12px',
+        transform: 'rotate(90deg)',
+        display: 'flex',
+        alignItems: 'center',
+      }}
+      aria-label="Collapse"
+      tooltip={t(`${isActive ? 'Collapse' : 'Expand'} table preview`)}
+    >
+      <Icons.RightOutlined
+        iconSize="s"
+        style={isActive ? { transform: 'rotateY(180deg)' } : null}
+      />
+    </IconTooltip>
+  );
+
+  return (
+    <Collapse.Panel
+      {...props}
+      header={renderHeader()}
+      className="TableElement"
+      forceRender
+      expandIcon={collapseExpandIcon}
+    >
+      {renderBody()}
+    </Collapse.Panel>
+  );
+};
 
 TableElement.propTypes = propTypes;
 TableElement.defaultProps = defaultProps;