Skip to content

Commit

Permalink
fix: The tab delete modal should be more explicit about which tiles/c… (
Browse files Browse the repository at this point in the history
#9878)

* fix: The tab delete modal should be more explicit about which tiles/charts you will delete

* small fix

* fix save new chart issue

* remove redundant condition check
  • Loading branch information
Shifan-Gu committed Apr 26, 2024
1 parent 6bd30b1 commit 1187081
Show file tree
Hide file tree
Showing 8 changed files with 81 additions and 17 deletions.
50 changes: 41 additions & 9 deletions packages/frontend/src/components/DashboardTabs/DeleteTabModal.tsx
@@ -1,14 +1,22 @@
import { type DashboardTab, type DashboardTile } from '@lightdash/common';
import {
isChartTile,
type DashboardChartTile,
type DashboardTab,
type DashboardTile,
} from '@lightdash/common';
import {
Button,
Group,
List,
Modal,
Stack,
Text,
Title,
type ModalProps,
} from '@mantine/core';
import { IconTrash } from '@tabler/icons-react';
import { useMemo, type FC } from 'react';
import MantineIcon from '../common/MantineIcon';

type AddProps = ModalProps & {
tab: DashboardTab;
Expand All @@ -32,15 +40,25 @@ export const TabDeleteModal: FC<AddProps> = ({
handleDeleteTab(uuid);
};

const numberOfTiles = useMemo(() => {
return (dashboardTiles || []).filter((tile) => tile.tabUuid == tab.uuid)
?.length;
}, [tab.uuid, dashboardTiles]);
const isNewSavedChart = (tile: DashboardTile) => {
return isChartTile(tile) && tile.properties.belongsToDashboard;
};

const tilesToDelete = useMemo(
() => (dashboardTiles || []).filter((tile) => tile.tabUuid == tab.uuid),
[tab.uuid, dashboardTiles],
);

const newSavedCharts = useMemo(
() => tilesToDelete.filter(isNewSavedChart) as DashboardChartTile[],
[tilesToDelete],
);

return (
<Modal
title={
<Group spacing="xs">
<MantineIcon icon={IconTrash} color="red" size="lg" />
<Title order={4}>Remove tab</Title>
</Group>
}
Expand All @@ -50,11 +68,25 @@ export const TabDeleteModal: FC<AddProps> = ({
>
<Stack spacing="lg" pt="sm">
<Text>
Are you sure you want to delete tab {tab.name}?
Are you sure you want to remove tab <b>"{tab.name}"</b> and{' '}
<b>{tilesToDelete?.length}</b> tiles from this dashboard?
<br />
This action will permanently delete {numberOfTiles} tiles
once you save your dashboard changes.
{newSavedCharts.length > 0 && (
<Text>
<br />
Once you save changes to your dashboard, this action
will also permanently delete the following charts
that were created from within it:
</Text>
)}
</Text>
<List>
{newSavedCharts.map((tile) => (
<List.Item key={tile.uuid}>
<Text>{tile.properties.chartName}</Text>
</List.Item>
))}
</List>
<Group position="right" mt="sm">
<Button variant="outline" onClick={handleClose}>
Cancel
Expand All @@ -65,7 +97,7 @@ export const TabDeleteModal: FC<AddProps> = ({
color="red"
onClick={() => handleSubmit(tab.uuid)}
>
Delete
Remove
</Button>
</Group>
</Stack>
Expand Down
17 changes: 11 additions & 6 deletions packages/frontend/src/components/DashboardTabs/index.tsx
Expand Up @@ -320,16 +320,20 @@ const DashboardTabs: FC<DashboardTabsProps> = ({
>
Rename Tab
</Menu.Item>
{sortedTabs.length === 1 ? (
{sortedTabs.length === 1 ||
!currentTabHasTiles ? (
<Menu.Item
onClick={() =>
onClick={(e) => {
handleDeleteTab(
tab.uuid,
)
}
);
e.stopPropagation();
}}
>
Remove Tabs
Component
{sortedTabs.length ===
1
? 'Remove Tabs Component'
: 'Remove Tab'}
</Menu.Item>
) : (
<Menu.Item
Expand Down Expand Up @@ -417,6 +421,7 @@ const DashboardTabs: FC<DashboardTabsProps> = ({
}
isEditMode={isEditMode}
setAddingTab={setAddingTab}
activeTabUuid={activeTab?.uuid}
/>
)}
<TabAddModal
Expand Down
Expand Up @@ -31,9 +31,15 @@ import { TileAddModal } from './TileForms/TileAddModal';
type Props = {
onAddTiles: (tiles: Dashboard['tiles'][number][]) => void;
setAddingTab: (value: React.SetStateAction<boolean>) => void;
activeTabUuid?: string;
} & Pick<ButtonProps, 'disabled'>;

const AddTileButton: FC<Props> = ({ onAddTiles, setAddingTab, disabled }) => {
const AddTileButton: FC<Props> = ({
onAddTiles,
setAddingTab,
disabled,
activeTabUuid,
}) => {
const [addTileType, setAddTileType] = useState<DashboardTileTypes>();
const [isAddChartTilesModalOpen, setIsAddChartTilesModalOpen] =
useState<boolean>(false);
Expand Down Expand Up @@ -95,6 +101,7 @@ const AddTileButton: FC<Props> = ({ onAddTiles, setAddingTab, disabled }) => {
haveFiltersChanged,
dashboard?.uuid,
dashboard?.name,
activeTabUuid,
);
history.push(`/projects/${projectUuid}/tables`);
}}
Expand Down
Expand Up @@ -23,13 +23,15 @@ interface SavedChartsAvailableProps {
emptyContainerType?: 'dashboard' | 'tab';
isEditMode: boolean;
setAddingTab: (value: React.SetStateAction<boolean>) => void;
activeTabUuid?: string;
}

const EmptyStateNoTiles: FC<SavedChartsAvailableProps> = ({
onAddTiles,
emptyContainerType = 'dashboard',
isEditMode,
setAddingTab,
activeTabUuid,
}) => {
const { projectUuid } = useParams<{ projectUuid: string }>();
const { user } = useApp();
Expand Down Expand Up @@ -70,6 +72,7 @@ const EmptyStateNoTiles: FC<SavedChartsAvailableProps> = ({
<AddTileButton
onAddTiles={onAddTiles}
setAddingTab={setAddingTab}
activeTabUuid={activeTabUuid}
/>
) : undefined
}
Expand Down
Expand Up @@ -67,6 +67,7 @@ type DashboardHeaderProps = {
isFullscreen: boolean;
isPinned: boolean;
oldestCacheTime?: Date;
activeTabUuid?: string;
onAddTiles: (tiles: Dashboard['tiles'][number][]) => void;
onCancel: () => void;
onSaveDashboard: () => void;
Expand All @@ -89,6 +90,7 @@ const DashboardHeader = ({
isFullscreen,
isPinned,
oldestCacheTime,
activeTabUuid,
onAddTiles,
onCancel,
onSaveDashboard,
Expand Down Expand Up @@ -248,6 +250,7 @@ const DashboardHeader = ({
onAddTiles={onAddTiles}
disabled={isSaving}
setAddingTab={setAddingTab}
activeTabUuid={activeTabUuid}
/>
<Tooltip
fz="xs"
Expand Down
Expand Up @@ -76,6 +76,7 @@ export const SaveToDashboard: FC<Props> = ({
clearIsEditingDashboardChart,
getUnsavedDashboardTiles,
setUnsavedDashboardTiles,
getDashboardActiveTabUuid,
} = useDashboardStorage();
const unsavedDashboardTiles = getUnsavedDashboardTiles();
const form = useForm<FormValues>({
Expand All @@ -86,6 +87,8 @@ export const SaveToDashboard: FC<Props> = ({
validate: zodResolver(validationSchema),
});

const activeTabUuid = getDashboardActiveTabUuid();

const handleSaveChartInDashboard = useCallback(
async (values: SaveToDashboardFormValues) => {
if (!dashboardUuid) {
Expand All @@ -101,7 +104,7 @@ export const SaveToDashboard: FC<Props> = ({
const newTile: CreateDashboardChartTile = {
uuid: uuid4(),
type: DashboardTileTypes.SAVED_CHART,
tabUuid: undefined,
tabUuid: activeTabUuid ?? undefined,
properties: {
belongsToDashboard: true,
savedChartUuid: chart.uuid,
Expand Down Expand Up @@ -132,6 +135,7 @@ export const SaveToDashboard: FC<Props> = ({
projectUuid,
showToastSuccess,
dashboardName,
activeTabUuid,
],
);
return (
Expand Down
9 changes: 9 additions & 0 deletions packages/frontend/src/hooks/dashboard/useDashboardStorage.tsx
Expand Up @@ -59,6 +59,10 @@ const useDashboardStorage = () => {
);
}, []);

const getDashboardActiveTabUuid = useCallback(() => {
return sessionStorage.getItem('activeTabUuid');
}, []);

const clearDashboardStorage = useCallback(() => {
sessionStorage.removeItem('fromDashboard');
sessionStorage.removeItem('dashboardUuid');
Expand All @@ -77,6 +81,7 @@ const useDashboardStorage = () => {
haveFiltersChanged: boolean,
dashboardUuid?: string,
dashboardName?: string,
activeTabUuid?: string,
) => {
sessionStorage.setItem('fromDashboard', dashboardName ?? '');
sessionStorage.setItem('dashboardUuid', dashboardUuid ?? '');
Expand All @@ -97,6 +102,9 @@ const useDashboardStorage = () => {
'hasDashboardChanges',
JSON.stringify(haveTilesChanged || haveFiltersChanged),
);
if (activeTabUuid) {
sessionStorage.setItem('activeTabUuid', activeTabUuid);
}
// Trigger storage event to update NavBar
window.dispatchEvent(new Event('storage'));
},
Expand Down Expand Up @@ -132,6 +140,7 @@ const useDashboardStorage = () => {
getHasDashboardChanges,
getUnsavedDashboardTiles,
setUnsavedDashboardTiles,
getDashboardActiveTabUuid,
};
};

Expand Down
1 change: 1 addition & 0 deletions packages/frontend/src/pages/Dashboard.tsx
Expand Up @@ -605,6 +605,7 @@ const Dashboard: FC = () => {
oldestCacheTime={oldestCacheTime}
isFullscreen={isFullscreen}
isPinned={isPinned}
activeTabUuid={activeTab?.uuid}
onToggleFullscreen={handleToggleFullscreen}
hasDashboardChanged={
haveTilesChanged ||
Expand Down

0 comments on commit 1187081

Please sign in to comment.