From 6690963ec2649feb5f86479231a170e9efbef609 Mon Sep 17 00:00:00 2001
From: Grace Guo <grace.guo@airbnb.com>
Date: Wed, 8 Jul 2020 16:21:47 -0700
Subject: [PATCH] feat: minor reorder SQL Lab Tab controls (#10257)

---
 .../cypress/integration/sqllab/tabs.test.js   |  6 ++-
 .../javascripts/sqllab/TabStatusIcon_spec.jsx | 14 ------
 .../src/SqlLab/components/TabStatusIcon.jsx   | 42 ++--------------
 .../SqlLab/components/TabbedSqlEditors.jsx    | 21 ++++----
 superset-frontend/src/SqlLab/main.less        | 49 +++++++++++++++++++
 5 files changed, 68 insertions(+), 64 deletions(-)

diff --git a/superset-frontend/cypress-base/cypress/integration/sqllab/tabs.test.js b/superset-frontend/cypress-base/cypress/integration/sqllab/tabs.test.js
index d876ef637..8f5294699 100644
--- a/superset-frontend/cypress-base/cypress/integration/sqllab/tabs.test.js
+++ b/superset-frontend/cypress-base/cypress/integration/sqllab/tabs.test.js
@@ -40,10 +40,12 @@ describe('SqlLab query tabs', () => {
       const initialTabCount = tabListA.length;
 
       // open the tab dropdown to remove
-      cy.get('.SqlEditorTabs > ul > li .dropdown-toggle').click();
+      cy.get('.SqlEditorTabs > ul > li .dropdown-toggle').click({
+        force: true,
+      });
 
       // first item is close
-      cy.get('.SqlEditorTabs .close-btn a').click();
+      cy.get('.SqlEditorTabs .ddbtn-tab .close').first().click();
 
       cy.get('.SqlEditorTabs > ul > li').should(
         'have.length',
diff --git a/superset-frontend/spec/javascripts/sqllab/TabStatusIcon_spec.jsx b/superset-frontend/spec/javascripts/sqllab/TabStatusIcon_spec.jsx
index 4db08f843..4241353dc 100644
--- a/superset-frontend/spec/javascripts/sqllab/TabStatusIcon_spec.jsx
+++ b/superset-frontend/spec/javascripts/sqllab/TabStatusIcon_spec.jsx
@@ -36,18 +36,4 @@ describe('TabStatusIcon', () => {
     expect(wrapper.find('div.circle')).toHaveLength(1);
     expect(wrapper.text()).toBe('');
   });
-
-  it('renders a circle with an x when hovered', () => {
-    const { wrapper } = setup();
-    wrapper.simulate('mouseOver');
-    expect(wrapper.find('div.circle')).toHaveLength(1);
-    expect(wrapper.text()).toBe('×');
-  });
-
-  it('calls onClose from props when clicked', () => {
-    const { wrapper, onClose } = setup();
-    wrapper.simulate('click');
-    // eslint-disable-next-line no-unused-expressions
-    expect(onClose.calledOnce).toBe(true);
-  });
 });
diff --git a/superset-frontend/src/SqlLab/components/TabStatusIcon.jsx b/superset-frontend/src/SqlLab/components/TabStatusIcon.jsx
index ec8a3f2f6..1eeedd1e1 100644
--- a/superset-frontend/src/SqlLab/components/TabStatusIcon.jsx
+++ b/superset-frontend/src/SqlLab/components/TabStatusIcon.jsx
@@ -19,42 +19,10 @@
 import React from 'react';
 import PropTypes from 'prop-types';
 
-const propTypes = {
-  onClose: PropTypes.func.isRequired,
-  tabState: PropTypes.string.isRequired,
-};
-
-class TabStatusIcon extends React.Component {
-  constructor(props) {
-    super(props);
-    this.onMouseOver = this.onMouseOver.bind(this);
-    this.onMouseOut = this.onMouseOut.bind(this);
-
-    this.state = { isHovered: false };
-  }
-
-  onMouseOver() {
-    this.setState({ isHovered: true });
-  }
-
-  onMouseOut() {
-    this.setState({ isHovered: false });
-  }
-
-  render() {
-    return (
-      <span
-        onMouseOver={this.onMouseOver}
-        onMouseOut={this.onMouseOut}
-        onClick={this.props.onClose}
-      >
-        <div className={'circle ' + this.props.tabState}>
-          {this.state.isHovered ? '×' : null}
-        </div>
-      </span>
-    );
-  }
+export default function TabStatusIcon(props) {
+  return <div className={'circle ' + props.tabState} />;
 }
 
-TabStatusIcon.propTypes = propTypes;
-export default TabStatusIcon;
+TabStatusIcon.propTypes = {
+  tabState: PropTypes.string.isRequired,
+};
diff --git a/superset-frontend/src/SqlLab/components/TabbedSqlEditors.jsx b/superset-frontend/src/SqlLab/components/TabbedSqlEditors.jsx
index 6fda4165a..ce1fbc5ef 100644
--- a/superset-frontend/src/SqlLab/components/TabbedSqlEditors.jsx
+++ b/superset-frontend/src/SqlLab/components/TabbedSqlEditors.jsx
@@ -18,7 +18,7 @@
  */
 import React from 'react';
 import PropTypes from 'prop-types';
-import { MenuItem, SplitButton, Tab, Tabs } from 'react-bootstrap';
+import { MenuItem, DropdownButton, Tab, Tabs } from 'react-bootstrap';
 import { connect } from 'react-redux';
 import { bindActionCreators } from 'redux';
 import URI from 'urijs';
@@ -288,22 +288,20 @@ class TabbedSqlEditors extends React.PureComponent {
 
       const title = (
         <>
-          <TabStatusIcon
-            onClose={() => this.removeQueryEditor(qe)}
-            tabState={state}
-          />{' '}
-          {qe.title}{' '}
+          {qe.title} <TabStatusIcon tabState={state} />{' '}
+          <span className="close" onClick={() => this.removeQueryEditor(qe)}>
+            {'×'}
+          </span>
         </>
       );
       const tabTitle = (
         <>
-          <span className="ddbtn-tab">{title}</span>
           {isSelected && (
-            <SplitButton
+            <DropdownButton
               bsSize="small"
               id={'ddbtn-tab-' + i}
-              className="ddbtn-tab"
-              title="&nbsp;"
+              title={' '}
+              noCaret
             >
               <MenuItem
                 className="close-btn"
@@ -347,8 +345,9 @@ class TabbedSqlEditors extends React.PureComponent {
                 </div>
                 {t('Duplicate tab')}
               </MenuItem>
-            </SplitButton>
+            </DropdownButton>
           )}
+          <span className="ddbtn-tab">{title}</span>
         </>
       );
       return (
diff --git a/superset-frontend/src/SqlLab/main.less b/superset-frontend/src/SqlLab/main.less
index de24ecea1..d6672e574 100644
--- a/superset-frontend/src/SqlLab/main.less
+++ b/superset-frontend/src/SqlLab/main.less
@@ -261,9 +261,56 @@ div.Workspace {
     }
   }
 
+  .dropdown.btn-group.btn-group-sm {
+    width: 3px;
+    height: 3px;
+    border-radius: 1.5px;
+    background: #bababa;
+    margin-right: 8px;
+    font-weight: @font-weight-normal;
+    display: inline-flex;
+
+    &:hover {
+      background-color: @primary-color;
+
+      &:before,
+      &:after {
+        background-color: @primary-color;
+      }
+    }
+
+    &:before,
+    &:after {
+      position: absolute;
+      content: ' ';
+      width: 3px;
+      height: 3px;
+      border-radius: 1.5px;
+      background-color: #bababa;
+    }
+    &:before {
+      transform: translateY(-5px);
+    }
+    &:after {
+      transform: translateY(5px);
+    }
+  }
+
+  ul.dropdown-menu {
+    margin-top: 10px;
+  }
+
   .dropdown-toggle {
     padding-top: 2px;
   }
+
+  .close {
+    opacity: 1;
+    color: @almost-black;
+    position: relative;
+    top: -2px;
+    right: -4px;
+  }
 }
 
 .SqlEditor {
@@ -462,6 +509,8 @@ a.Link {
   padding: 0;
   border: none;
   background: none;
+  position: relative;
+  top: 2px;
 
   &:focus {
     outline: 0;