Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: improve import flow UI/UX #12070

Merged
merged 3 commits into from
Dec 16, 2020
Merged

fix: improve import flow UI/UX #12070

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
62eed92
fix: improve import flow UI/UX
betodealmeida Dec 15, 2020
8ca0471
Reset modal after import
betodealmeida Dec 16, 2020
53d8c03
Adjust margin before overwrite text
betodealmeida Dec 16, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions superset-frontend/src/common/components/Modal/Modal.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
3 changes: 1 addition & 2 deletions superset-frontend/src/components/ImportModal/ImportModal.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
Expand All @@ -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: () => {},
Expand Down
58 changes: 26 additions & 32 deletions superset-frontend/src/components/ImportModal/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

curious, is there something that is inherited that is making this div some other kind of display, since block is the default?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, you're right, it's not needed here. I copied the style from another helper class where the display is flex.

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 {
Expand Down Expand Up @@ -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;
Expand All @@ -115,7 +116,6 @@ export interface ImportModelsModalProps {
const ImportModelsModal: FunctionComponent<ImportModelsModalProps> = ({
resourceName,
resourceLabel,
icon,
passwordsNeededMessage,
confirmOverwriteMessage,
addDangerToast,
Expand All @@ -140,6 +140,8 @@ const ImportModelsModal: FunctionComponent<ImportModelsModalProps> = ({
setUploadFile(null);
setPasswordFields([]);
setPasswords({});
setNeedsOverwriteConfirm(false);
setConfirmedOverwrite(false);
if (fileInputRef && fileInputRef.current) {
fileInputRef.current.value = '';
}
Expand All @@ -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 = () => {
Expand Down Expand Up @@ -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">
Expand All @@ -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"
Expand All @@ -256,7 +255,7 @@ const ImportModelsModal: FunctionComponent<ImportModelsModalProps> = ({
}

return (
<Modal
<StyledModal
name="model"
className="import-model-modal"
disablePrimaryButton={
Expand All @@ -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">
Expand All @@ -294,7 +288,7 @@ const ImportModelsModal: FunctionComponent<ImportModelsModalProps> = ({
</StyledInputContainer>
{renderPasswordFields()}
{renderOverwriteConfirmation()}
</Modal>
</StyledModal>
);
};

Expand Down
4 changes: 4 additions & 0 deletions superset-frontend/src/components/Menu/SubMenu.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,10 @@ const StyledHeader = styled.header`
}
}
}

.btn-link {
padding: 10px 0;
}
`;

type MenuChild = {
Expand Down
14 changes: 7 additions & 7 deletions superset-frontend/src/views/CRUD/chart/ChartList.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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 = '',
Expand Down Expand Up @@ -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}
Expand Down
14 changes: 7 additions & 7 deletions superset-frontend/src/views/CRUD/dashboard/DashboardList.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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;
Expand Down Expand Up @@ -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}
Expand Down
14 changes: 7 additions & 7 deletions superset-frontend/src/views/CRUD/data/database/DatabaseList.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -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;
Expand Down Expand Up @@ -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}
Expand Down
14 changes: 7 additions & 7 deletions superset-frontend/src/views/CRUD/data/dataset/DatasetList.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -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;
Expand Down Expand Up @@ -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}
Expand Down