fix: improve import flow UI/UX (#12070)

* fix: improve import flow UI/UX * Reset modal after import * Adjust margin before overwrite text
apache · Dec 16, 2020 · 69185ee · 69185ee
1 parent e299dbf
commit 69185ee
Show file tree

Hide file tree

Showing 8 changed files with 61 additions and 62 deletions.
diff --git a/superset-frontend/src/common/components/Modal/Modal.tsx b/superset-frontend/src/common/components/Modal/Modal.tsx
@@ -61,6 +61,8 @@ const StyledModal = styled(BaseModal)<StyledModalProps>`
     background-color: ${({ theme }) => theme.colors.grayscale.light4};
     border-radius: ${({ theme }) => theme.borderRadius}px
       ${({ theme }) => theme.borderRadius}px 0 0;
+    padding-left: ${({ theme }) => theme.gridUnit * 4}px;
+    padding-right: ${({ theme }) => theme.gridUnit * 4}px;
 
     .ant-modal-title h4 {
       display: flex;

diff --git a/superset-frontend/src/components/ImportModal/ImportModal.test.tsx b/superset-frontend/src/components/ImportModal/ImportModal.test.tsx
@@ -23,7 +23,7 @@ import { styledMount as mount } from 'spec/helpers/theming';
 import { ReactWrapper } from 'enzyme';
 
 import { ImportResourceName } from 'src/views/CRUD/types';
-import ImportModelsModal, { StyledIcon } from 'src/components/ImportModal';
+import ImportModelsModal from 'src/components/ImportModal';
 import Modal from 'src/common/components/Modal';
 
 const mockStore = configureStore([thunk]);
@@ -32,7 +32,6 @@ const store = mockStore({});
 const requiredProps = {
   resourceName: 'database' as ImportResourceName,
   resourceLabel: 'database',
-  icon: <StyledIcon name="database" />,
   passwordsNeededMessage: 'Passwords are needed',
   confirmOverwriteMessage: 'Database exists',
   addDangerToast: () => {},

diff --git a/superset-frontend/src/components/ImportModal/index.tsx b/superset-frontend/src/components/ImportModal/index.tsx
@@ -20,32 +20,34 @@ import React, { FunctionComponent, useEffect, useRef, useState } from 'react';
 import { styled, t } from '@superset-ui/core';
 
 import Icon from 'src//components/Icon';
-import Modal from 'src/common/components/Modal';
+import StyledModal from 'src/common/components/Modal';
 import { useImportResource } from 'src/views/CRUD/hooks';
 import { ImportResourceName } from 'src/views/CRUD/types';
 
 export const StyledIcon = styled(Icon)`
   margin: auto ${({ theme }) => theme.gridUnit * 2}px auto 0;
 `;
 
+const HelperMessage = styled.div`
+  display: block;
+  color: ${({ theme }) => theme.colors.grayscale.base};
+  font-size: ${({ theme }) => theme.typography.sizes.s - 1}px;
+`;
+
 const StyledInputContainer = styled.div`
-  margin-bottom: ${({ theme }) => theme.gridUnit * 2}px;
+  padding-bottom: ${({ theme }) => theme.gridUnit * 2}px;
+  padding-top: ${({ theme }) => theme.gridUnit * 2}px;
+
+  & > div {
+    margin: ${({ theme }) => theme.gridUnit}px 0;
+  }
 
   &.extra-container {
     padding-top: 8px;
   }
 
-  .helper {
-    display: block;
-    padding: ${({ theme }) => theme.gridUnit}px 0;
-    color: ${({ theme }) => theme.colors.grayscale.base};
-    font-size: ${({ theme }) => theme.typography.sizes.s - 1}px;
-    text-align: left;
-
-    .required {
-      margin-left: ${({ theme }) => theme.gridUnit / 2}px;
-      color: ${({ theme }) => theme.colors.error.base};
-    }
+  .confirm-overwrite {
+    margin-bottom: ${({ theme }) => theme.gridUnit * 2}px;
   }
 
   .input-container {
@@ -100,7 +102,6 @@ const StyledInputContainer = styled.div`
 export interface ImportModelsModalProps {
   resourceName: ImportResourceName;
   resourceLabel: string;
-  icon: React.ReactNode;
   passwordsNeededMessage: string;
   confirmOverwriteMessage: string;
   addDangerToast: (msg: string) => void;
@@ -115,7 +116,6 @@ export interface ImportModelsModalProps {
 const ImportModelsModal: FunctionComponent<ImportModelsModalProps> = ({
   resourceName,
   resourceLabel,
-  icon,
   passwordsNeededMessage,
   confirmOverwriteMessage,
   addDangerToast,
@@ -140,6 +140,8 @@ const ImportModelsModal: FunctionComponent<ImportModelsModalProps> = ({
     setUploadFile(null);
     setPasswordFields([]);
     setPasswords({});
+    setNeedsOverwriteConfirm(false);
+    setConfirmedOverwrite(false);
     if (fileInputRef && fileInputRef.current) {
       fileInputRef.current.value = '';
     }
@@ -161,12 +163,13 @@ const ImportModelsModal: FunctionComponent<ImportModelsModalProps> = ({
 
   useEffect(() => {
     setNeedsOverwriteConfirm(alreadyExists.length > 0);
-  }, [alreadyExists]);
+  }, [alreadyExists, setNeedsOverwriteConfirm]);
 
   // Functions
   const hide = () => {
     setIsHidden(true);
     onHide();
+    clearModal();
   };
 
   const onUpload = () => {
@@ -201,9 +204,7 @@ const ImportModelsModal: FunctionComponent<ImportModelsModalProps> = ({
     return (
       <>
         <h5>Database passwords</h5>
-        <StyledInputContainer>
-          <div className="helper">{passwordsNeededMessage}</div>
-        </StyledInputContainer>
+        <HelperMessage>{passwordsNeededMessage}</HelperMessage>
         {passwordFields.map(fileName => (
           <StyledInputContainer key={`password-for-${fileName}`}>
             <div className="control-label">
@@ -226,18 +227,16 @@ const ImportModelsModal: FunctionComponent<ImportModelsModalProps> = ({
   };
 
   const renderOverwriteConfirmation = () => {
-    if (alreadyExists.length === 0) {
+    if (!needsOverwriteConfirm) {
       return null;
     }
 
     return (
       <>
         <StyledInputContainer>
-          <div>{confirmOverwriteMessage}</div>
+          <div className="confirm-overwrite">{confirmOverwriteMessage}</div>
           <div className="control-label">
-            <label htmlFor="overwrite">
-              {t('Type "%s" to confirm', t('OVERWRITE'))}
-            </label>
+            {t('Type "%s" to confirm', t('OVERWRITE'))}
           </div>
           <input
             data-test="overwrite-modal-input"
@@ -256,7 +255,7 @@ const ImportModelsModal: FunctionComponent<ImportModelsModalProps> = ({
   }
 
   return (
-    <Modal
+    <StyledModal
       name="model"
       className="import-model-modal"
       disablePrimaryButton={
@@ -268,12 +267,7 @@ const ImportModelsModal: FunctionComponent<ImportModelsModalProps> = ({
       primaryButtonType={needsOverwriteConfirm ? 'danger' : 'primary'}
       width="750px"
       show={show}
-      title={
-        <h4>
-          {icon}
-          {t('Import %s', resourceLabel)}
-        </h4>
-      }
+      title={<h4>{t('Import %s', resourceLabel)}</h4>}
     >
       <StyledInputContainer>
         <div className="control-label">
@@ -294,7 +288,7 @@ const ImportModelsModal: FunctionComponent<ImportModelsModalProps> = ({
       </StyledInputContainer>
       {renderPasswordFields()}
       {renderOverwriteConfirmation()}
-    </Modal>
+    </StyledModal>
   );
 };
 

diff --git a/superset-frontend/src/components/Menu/SubMenu.tsx b/superset-frontend/src/components/Menu/SubMenu.tsx
@@ -76,6 +76,10 @@ const StyledHeader = styled.header`
       }
     }
   }
+
+  .btn-link {
+    padding: 10px 0;
+  }
 `;
 
 type MenuChild = {

diff --git a/superset-frontend/src/views/CRUD/chart/ChartList.tsx b/superset-frontend/src/views/CRUD/chart/ChartList.tsx
@@ -43,9 +43,7 @@ import ListView, {
 } from 'src/components/ListView';
 import withToasts from 'src/messageToasts/enhancers/withToasts';
 import PropertiesModal from 'src/explore/components/PropertiesModal';
-import ImportModelsModal, {
-  StyledIcon,
-} from 'src/components/ImportModal/index';
+import ImportModelsModal from 'src/components/ImportModal/index';
 import Chart from 'src/types/Chart';
 import TooltipWrapper from 'src/components/TooltipWrapper';
 import ChartCard from './ChartCard';
@@ -58,6 +56,11 @@ const PASSWORDS_NEEDED_MESSAGE = t(
     'the database configuration are not present in export files, and ' +
     'should be added manually after the import if they are needed.',
 );
+const CONFIRM_OVERWRITE_MESSAGE = t(
+  'You are importing one or more charts that already exist. ' +
+    'Overwriting might cause you to lose some of your work. Are you ' +
+    'sure you want to overwrite?',
+);
 
 const createFetchDatasets = (handleError: (err: Response) => void) => async (
   filterValue = '',
@@ -580,11 +583,8 @@ function ChartList(props: ChartListProps) {
       <ImportModelsModal
         resourceName="chart"
         resourceLabel={t('chart')}
-        icon={<StyledIcon name="nav-charts" />}
         passwordsNeededMessage={PASSWORDS_NEEDED_MESSAGE}
-        confirmOverwriteMessage={t(
-          'One or more charts to be imported already exist.',
-        )}
+        confirmOverwriteMessage={CONFIRM_OVERWRITE_MESSAGE}
         addDangerToast={addDangerToast}
         addSuccessToast={addSuccessToast}
         onModelImport={handleChartImport}

diff --git a/superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx b/superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx
@@ -37,9 +37,7 @@ import Icon from 'src/components/Icon';
 import FaveStar from 'src/components/FaveStar';
 import PropertiesModal from 'src/dashboard/components/PropertiesModal';
 import TooltipWrapper from 'src/components/TooltipWrapper';
-import ImportModelsModal, {
-  StyledIcon,
-} from 'src/components/ImportModal/index';
+import ImportModelsModal from 'src/components/ImportModal/index';
 
 import Dashboard from 'src/dashboard/containers/Dashboard';
 import DashboardCard from './DashboardCard';
@@ -52,6 +50,11 @@ const PASSWORDS_NEEDED_MESSAGE = t(
     'the database configuration are not present in export files, and ' +
     'should be added manually after the import if they are needed.',
 );
+const CONFIRM_OVERWRITE_MESSAGE = t(
+  'You are importing one or more dashboards that already exist. ' +
+    'Overwriting might cause you to lose some of your work. Are you ' +
+    'sure you want to overwrite?',
+);
 
 interface DashboardListProps {
   addDangerToast: (msg: string) => void;
@@ -541,11 +544,8 @@ function DashboardList(props: DashboardListProps) {
       <ImportModelsModal
         resourceName="dashboard"
         resourceLabel={t('dashboard')}
-        icon={<StyledIcon name="nav-dashboard" />}
         passwordsNeededMessage={PASSWORDS_NEEDED_MESSAGE}
-        confirmOverwriteMessage={t(
-          'One or more dashboards to be imported already exist.',
-        )}
+        confirmOverwriteMessage={CONFIRM_OVERWRITE_MESSAGE}
         addDangerToast={addDangerToast}
         addSuccessToast={addSuccessToast}
         onModelImport={handleDashboardImport}

diff --git a/superset-frontend/src/views/CRUD/data/database/DatabaseList.tsx b/superset-frontend/src/views/CRUD/data/database/DatabaseList.tsx
@@ -29,9 +29,7 @@ import TooltipWrapper from 'src/components/TooltipWrapper';
 import Icon from 'src/components/Icon';
 import ListView, { Filters } from 'src/components/ListView';
 import { commonMenuData } from 'src/views/CRUD/data/common';
-import ImportModelsModal, {
-  StyledIcon,
-} from 'src/components/ImportModal/index';
+import ImportModelsModal from 'src/components/ImportModal/index';
 import DatabaseModal from './DatabaseModal';
 import { DatabaseObject } from './types';
 
@@ -42,6 +40,11 @@ const PASSWORDS_NEEDED_MESSAGE = t(
     'sections of the database configuration are not present in export ' +
     'files, and should be added manually after the import if they are needed.',
 );
+const CONFIRM_OVERWRITE_MESSAGE = t(
+  'You are importing one or more databases that already exist. ' +
+    'Overwriting might cause you to lose some of your work. Are you ' +
+    'sure you want to overwrite?',
+);
 
 interface DatabaseDeleteObject extends DatabaseObject {
   chart_count: number;
@@ -436,11 +439,8 @@ function DatabaseList({ addDangerToast, addSuccessToast }: DatabaseListProps) {
       <ImportModelsModal
         resourceName="database"
         resourceLabel={t('database')}
-        icon={<StyledIcon name="database" />}
         passwordsNeededMessage={PASSWORDS_NEEDED_MESSAGE}
-        confirmOverwriteMessage={t(
-          'One or more databases to be imported already exist.',
-        )}
+        confirmOverwriteMessage={CONFIRM_OVERWRITE_MESSAGE}
         addDangerToast={addDangerToast}
         addSuccessToast={addSuccessToast}
         onModelImport={handleDatabaseImport}

diff --git a/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx b/superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx
@@ -45,9 +45,7 @@ import TooltipWrapper from 'src/components/TooltipWrapper';
 import Icon from 'src/components/Icon';
 import FacePile from 'src/components/FacePile';
 import CertifiedIconWithTooltip from 'src/components/CertifiedIconWithTooltip';
-import ImportModelsModal, {
-  StyledIcon,
-} from 'src/components/ImportModal/index';
+import ImportModelsModal from 'src/components/ImportModal/index';
 import { isFeatureEnabled, FeatureFlag } from 'src/featureFlags';
 import AddDatasetModal from './AddDatasetModal';
 
@@ -59,6 +57,11 @@ const PASSWORDS_NEEDED_MESSAGE = t(
     'the database configuration are not present in export files, and ' +
     'should be added manually after the import if they are needed.',
 );
+const CONFIRM_OVERWRITE_MESSAGE = t(
+  'You are importing one or more datasets that already exist. ' +
+    'Overwriting might cause you to lose some of your work. Are you ' +
+    'sure you want to overwrite?',
+);
 
 const FlexRowContainer = styled.div`
   align-items: center;
@@ -659,11 +662,8 @@ const DatasetList: FunctionComponent<DatasetListProps> = ({
       <ImportModelsModal
         resourceName="dataset"
         resourceLabel={t('dataset')}
-        icon={<StyledIcon name="table" />}
         passwordsNeededMessage={PASSWORDS_NEEDED_MESSAGE}
-        confirmOverwriteMessage={t(
-          'One or more datasets to be imported already exist.',
-        )}
+        confirmOverwriteMessage={CONFIRM_OVERWRITE_MESSAGE}
         addDangerToast={addDangerToast}
         addSuccessToast={addSuccessToast}
         onModelImport={handleDatasetImport}