From c5f29293732cc6698d28f1044d4e652eb9464c49 Mon Sep 17 00:00:00 2001
From: Tai Dupree <tdupreetan@gmail.com>
Date: Wed, 15 Jul 2020 09:10:47 -0700
Subject: [PATCH] address review feedback

---
 ...ForComponentToPaint.js => waitForComponentToPaint.ts} | 6 +++++-
 superset-frontend/src/components/ListView/ListView.tsx   | 9 ++++++---
 superset-frontend/src/components/Menu/SubMenu.tsx        | 7 ++++---
 superset-frontend/src/views/datasetList/DatasetList.tsx  | 8 +++++---
 4 files changed, 20 insertions(+), 10 deletions(-)
 rename superset-frontend/spec/helpers/{waitForComponentToPaint.js => waitForComponentToPaint.ts} (89%)

diff --git a/superset-frontend/spec/helpers/waitForComponentToPaint.js b/superset-frontend/spec/helpers/waitForComponentToPaint.ts
similarity index 89%
rename from superset-frontend/spec/helpers/waitForComponentToPaint.js
rename to superset-frontend/spec/helpers/waitForComponentToPaint.ts
index 1fdd4bc68fb46..2e57a80b413c8 100644
--- a/superset-frontend/spec/helpers/waitForComponentToPaint.js
+++ b/superset-frontend/spec/helpers/waitForComponentToPaint.ts
@@ -16,13 +16,17 @@
  * specific language governing permissions and limitations
  * under the License.
  */
+import { ReactWrapper } from 'enzyme';
 import { act } from 'react-dom/test-utils';
 
 // taken from: https://github.com/enzymejs/enzyme/issues/2073
 // There is currently and issue with enzyme and react-16's hooks
 // that results in a race condition between tests and react hook updates.
 // This function ensures tests run after all react updates are done.
-export default async function waitForComponentToPaint(wrapper, amount = 0) {
+export default async function waitForComponentToPaint<P = {}>(
+  wrapper: ReactWrapper<P>,
+  amount = 0,
+) {
   await act(async () => {
     await new Promise(resolve => setTimeout(resolve, amount));
     wrapper.update();
diff --git a/superset-frontend/src/components/ListView/ListView.tsx b/superset-frontend/src/components/ListView/ListView.tsx
index 7016bb5ecadd4..dc420cf7781f4 100644
--- a/superset-frontend/src/components/ListView/ListView.tsx
+++ b/superset-frontend/src/components/ListView/ListView.tsx
@@ -20,6 +20,7 @@ import { t } from '@superset-ui/translation';
 import React, { FunctionComponent } from 'react';
 import { Col, Row, Alert } from 'react-bootstrap';
 import styled from '@superset-ui/style';
+import cx from 'classnames';
 import Button from 'src/components/Button';
 import Loading from 'src/components/Loading';
 import IndeterminateCheckbox from 'src/components/IndeterminateCheckbox';
@@ -59,7 +60,6 @@ const BulkSelectWrapper = styled(Alert)`
   margin-bottom: 0;
   padding-top: 0;
   padding-bottom: 0;
-  padding-right: 16px;
   padding-right: 36px;
   color: #3d3d3d;
   background-color: ${({ theme }) => theme.colors.primary.light4};
@@ -78,7 +78,6 @@ const BulkSelectWrapper = styled(Alert)`
     margin: -8px 0 -8px 16px;
     width: 1px;
     height: 32px;
-    background: rgba(0, 0, 0, 0.0001);
     box-shadow: inset -1px 0px 0px #dadada;
     display: inline-flex;
     vertical-align: middle;
@@ -226,7 +225,11 @@ const ListView: FunctionComponent<ListViewProps> = ({
                     <Button
                       data-test="bulk-select-action"
                       key={action.key}
-                      className={`supersetButton ${action.type}`}
+                      className={cx('supersetButton', {
+                        danger: action.type === 'danger',
+                        primary: action.type === 'primary',
+                        secondary: action.type === 'secondary',
+                      })}
                       onClick={() =>
                         action.onSelect(selectedFlatRows.map(r => r.original))
                       }
diff --git a/superset-frontend/src/components/Menu/SubMenu.tsx b/superset-frontend/src/components/Menu/SubMenu.tsx
index c1f8feed86f18..e34fde210843d 100644
--- a/superset-frontend/src/components/Menu/SubMenu.tsx
+++ b/superset-frontend/src/components/Menu/SubMenu.tsx
@@ -28,9 +28,10 @@ const StyledHeader = styled.header`
   }
   .navbar-right {
     .supersetButton {
-      margin: ${({ theme }) => theme.gridUnit * 2}px
-        ${({ theme }) => theme.gridUnit * 4}px
-        ${({ theme }) => theme.gridUnit * 2}px 0;
+      margin: ${({ theme }) =>
+        `${theme.gridUnit * 2}px ${theme.gridUnit * 4}px ${
+          theme.gridUnit * 2
+        }px 0`};
     }
   }
   .navbar-nav {
diff --git a/superset-frontend/src/views/datasetList/DatasetList.tsx b/superset-frontend/src/views/datasetList/DatasetList.tsx
index bd1d93ae6486f..a144c2004b076 100644
--- a/superset-frontend/src/views/datasetList/DatasetList.tsx
+++ b/superset-frontend/src/views/datasetList/DatasetList.tsx
@@ -603,19 +603,21 @@ const DatasetList: FunctionComponent<DatasetListProps> = ({
                   { virtualCount: 0, physicalCount: 0 },
                 );
 
-                if (!selected.length) return t('0 Selected');
-                else if (virtualCount && !physicalCount)
+                if (!selected.length) {
+                  return t('0 Selected');
+                } else if (virtualCount && !physicalCount) {
                   return t(
                     '%s Selected (Virtual)',
                     selected.length,
                     virtualCount,
                   );
-                else if (physicalCount && !virtualCount)
+                } else if (physicalCount && !virtualCount) {
                   return t(
                     '%s Selected (Physical)',
                     selected.length,
                     physicalCount,
                   );
+                }
 
                 return t(
                   '%s Selected (%s Physical, %s Virtual)',