Skip to content
Snippets Groups Projects
Unverified Commit 8f3f8a41 authored by Emmad Usmani's avatar Emmad Usmani Committed by GitHub
Browse files

allow users to duplicate dashboard tabs (#38190)

Closes https://github.com/metabase/metabase/issues/38208

### Description

Adds a item to the dashboard tab menu to duplicate the tab.

### How to verify

1. Open a dashboard you can edit
2. Go to edit more, click the tab menu for a tab
3. Click duplicate, tab should be duplicated

### Demo

https://www.loom.com/share/d73aad469074440992618ee179518d39

### Checklist

~~- [ ] Tests have been added/updated to cover changes in this PR~~

e2e tests are added in the next PR in the stack
parent 784d3c69
No related branches found
No related tags found
No related merge requests found
......@@ -246,17 +246,20 @@ export function RenameableTabButton<T>({
setIsRenaming(false);
};
const renameItem = {
label: renameMenuLabel,
action: () => {
setIsRenaming(true);
},
};
const menuItems = [
...originalMenuItems.slice(0, renameMenuIndex),
renameItem,
...originalMenuItems.slice(renameMenuIndex),
];
let menuItems = [...originalMenuItems];
if (canRename) {
const renameItem = {
label: renameMenuLabel,
action: () => {
setIsRenaming(true);
},
};
menuItems = [
...menuItems.slice(0, renameMenuIndex),
renameItem,
...menuItems.slice(renameMenuIndex),
];
}
const dragLabel = (s: string) => {
if (s.length < 20) {
......
......@@ -13,6 +13,7 @@ import type {
Dispatch,
GetState,
SelectedTabId,
StoreDashboard,
TabDeletionId,
} from "metabase-types/store";
import { INITIALIZE } from "metabase/dashboard/actions/core";
......@@ -23,10 +24,15 @@ import { addUndo } from "metabase/redux/undo";
import { INITIAL_DASHBOARD_STATE } from "../constants";
import { getDashCardById } from "../selectors";
import { trackCardMoved } from "../analytics";
import { calculateDashCardRowAfterUndo } from "../utils";
import { calculateDashCardRowAfterUndo, isVirtualDashCard } from "../utils";
import { getDashCardMoveToTabUndoMessage, getExistingDashCards } from "./utils";
import { generateTemporaryDashcardId } from "./cards";
type CreateNewTabPayload = { tabId: DashboardTabId };
type DuplicateTabPayload = {
sourceTabId: DashboardTabId | null;
newTabId: DashboardTabId;
};
type DeleteTabPayload = {
tabId: DashboardTabId | null;
tabDeletionId: TabDeletionId;
......@@ -53,6 +59,7 @@ type UndoMoveDashCardToTabPayload = {
type InitTabsPayload = { slug: string | undefined };
const CREATE_NEW_TAB = "metabase/dashboard/CREATE_NEW_TAB";
const DUPLICATE_TAB = "metabase/dashboard/DUPLICATE_TAB";
const DELETE_TAB = "metabase/dashboard/DELETE_TAB";
const UNDO_DELETE_TAB = "metabase/dashboard/UNDO_DELETE_TAB";
const RENAME_TAB = "metabase/dashboard/RENAME_TAB";
......@@ -72,6 +79,42 @@ export function resetTempTabId() {
tempTabId = -2;
}
function _createInitialTabs({
dashId,
newTabId,
state,
prevDash,
firstTabName = t`Tab 1`,
secondTabName = t`Tab 2`,
}: {
dashId: DashboardId;
newTabId: DashboardTabId;
state: Draft<DashboardState> | DashboardState; // union type needed to fix `possibly infinite` type error https://metaboat.slack.com/archives/C505ZNNH4/p1699541570878059?thread_ts=1699520485.702539&cid=C505ZNNH4
prevDash: StoreDashboard;
firstTabName?: string;
secondTabName?: string;
}) {
// 1. Create two new tabs, add to dashboard
const firstTabId = newTabId + 1;
const secondTabId = newTabId;
const newTabs = [
getDefaultTab({ tabId: firstTabId, dashId, name: firstTabName }),
getDefaultTab({ tabId: secondTabId, dashId, name: secondTabName }),
];
prevDash.tabs = newTabs;
// 2. Assign existing dashcards to first tab
prevDash.dashcards.forEach(id => {
state.dashcards[id] = {
...state.dashcards[id],
isDirty: true,
dashboard_tab_id: firstTabId,
};
});
return { firstTabId, secondTabId };
}
export function createNewTab() {
// Decrement by 2 to leave space for two new tabs if dash doesn't have tabs already
const tabId = tempTabId;
......@@ -80,6 +123,16 @@ export function createNewTab() {
return createNewTabAction({ tabId });
}
const duplicateTabAction = createAction<DuplicateTabPayload>(DUPLICATE_TAB);
export function duplicateTab(sourceTabId: DashboardTabId | null) {
// Decrement by 2 to leave space for two new tabs if dash doesn't have tabs already
const newTabId = tempTabId;
tempTabId -= 2;
return duplicateTabAction({ sourceTabId, newTabId });
}
export const selectTab = createAction<SelectTabPayload>(SELECT_TAB);
function _selectTab({
......@@ -214,26 +267,94 @@ export const tabsReducer = createReducer<DashboardState>(
// Case 2: Dashboard doesn't have tabs
// 1. Create two new tabs, add to dashboard
const firstTabId = tabId + 1;
const secondTabId = tabId;
const newTabs = [
getDefaultTab({ tabId: firstTabId, dashId, name: t`Tab 1` }),
getDefaultTab({ tabId: secondTabId, dashId, name: t`Tab 2` }),
];
prevDash.tabs = [...prevTabs, ...newTabs];
// 1. Create two new tabs, add to dashboard, assign existing dashcards to first tab
const { secondTabId } = _createInitialTabs({
dashId,
newTabId: tabId,
state,
prevDash,
});
// 2. Select second tab
state.selectedTabId = secondTabId;
},
);
// 3. Assign existing dashcards to first tab
prevDash.dashcards.forEach(id => {
state.dashcards[id] = {
...state.dashcards[id],
builder.addCase<typeof duplicateTabAction>(
duplicateTabAction,
(state, { payload: { sourceTabId, newTabId } }) => {
const { dashId, prevDash, prevTabs } = getPrevDashAndTabs({ state });
if (!dashId || !prevDash) {
throw new Error(
`DUPLICATE_TAB was dispatched but either dashId (${dashId}) or prevDash (${prevDash}) are null`,
);
}
const sourceTab = prevTabs.find(tab => tab.id === sourceTabId);
if (sourceTabId !== null && !sourceTab) {
throw new Error(
`DUPLICATED_TAB was dispatched but no tab with sourceTabId ${sourceTabId} was found`,
);
}
// 1. Create empty tab(s)
// Case 1: Dashboard already has tabs
if (sourceTab !== undefined) {
const newTab = getDefaultTab({
tabId: newTabId,
dashId,
name: t`Copy of ${sourceTab.name}`,
});
prevDash.tabs = [...prevTabs, newTab];
// Case 2: Dashboard doesn't have tabs
} else {
const { firstTabId, secondTabId } = _createInitialTabs({
dashId,
prevDash,
state,
newTabId,
firstTabName: t`Tab 1`,
secondTabName: t`Copy of Tab 1`,
});
sourceTabId = firstTabId;
newTabId = secondTabId;
}
// 2. Duplicate dashcards
const sourceTabDashCards = prevDash.dashcards
.map(id => state.dashcards[id])
.filter(dashCard => dashCard.dashboard_tab_id === sourceTabId);
sourceTabDashCards.forEach(sourceDashCard => {
const newDashCardId = generateTemporaryDashcardId();
prevDash.dashcards.push(newDashCardId);
state.dashcards[newDashCardId] = {
...sourceDashCard,
id: newDashCardId,
dashboard_tab_id: newTabId,
isDirty: true,
dashboard_tab_id: firstTabId,
};
// We don't have card (question) data for virtual dashcards (text, heading, link, action)
// @ts-expect-error - possibly infinite type error https://metaboat.slack.com/archives/C505ZNNH4/p1699541570878059?thread_ts=1699520485.702539&cid=C505ZNNH4
if (isVirtualDashCard(sourceDashCard)) {
return;
}
if (sourceDashCard.card_id === null) {
throw Error("sourceDashCard is non-virtual yet has null card_id");
}
state.dashcardData[newDashCardId] = {
[sourceDashCard.card_id]:
state.dashcardData[sourceDashCard.id][sourceDashCard.card_id],
};
});
// 3. Select new tab
state.selectedTabId = newTabId;
return;
},
);
......
import styled from "@emotion/styled";
import { TabButton as BaseTabButton } from "metabase/core/components/TabButton";
import BaseButton from "metabase/core/components/Button";
export const Container = styled.div`
......@@ -10,10 +9,6 @@ export const Container = styled.div`
width: 100%;
`;
export const PlaceholderTab = ({ label }: { label: string }) => (
<BaseTabButton label={label} value={null} disabled />
);
export const CreateTabButton = styled(BaseButton)`
border: none;
padding: 0.25rem;
......
......@@ -2,15 +2,12 @@ import { t } from "ttag";
import type { Location } from "history";
import { TabRow } from "metabase/core/components/TabRow";
import type { TabButtonMenuItem } from "metabase/core/components/TabButton";
import { TabButton } from "metabase/core/components/TabButton";
import type { SelectedTabId } from "metabase-types/store";
import { Sortable } from "metabase/core/components/Sortable";
import {
Container,
CreateTabButton,
PlaceholderTab,
} from "./DashboardTabs.styled";
import { Container, CreateTabButton } from "./DashboardTabs.styled";
import { useDashboardTabs } from "./use-dashboard-tabs";
interface DashboardTabsProps {
......@@ -25,19 +22,34 @@ export function DashboardTabs({
const {
tabs,
createNewTab,
duplicateTab,
deleteTab,
renameTab,
selectTab,
selectedTabId,
moveTab,
} = useDashboardTabs({ location });
const showTabs = tabs.length > 1 || isEditing;
const showPlaceholder = tabs.length <= 1 && isEditing;
const hasMultipleTabs = tabs.length > 1;
const showTabs = hasMultipleTabs || isEditing;
const showPlaceholder = tabs.length === 0 && isEditing;
if (!showTabs) {
return null;
}
const menuItems: TabButtonMenuItem<SelectedTabId>[] = [
{
label: t`Duplicate`,
action: (_, value) => duplicateTab(value),
},
];
if (hasMultipleTabs) {
menuItems.push({
label: t`Delete`,
action: (_, value) => deleteTab(value),
});
}
return (
<Container>
<TabRow<SelectedTabId>
......@@ -47,7 +59,12 @@ export function DashboardTabs({
handleDragEnd={moveTab}
>
{showPlaceholder ? (
<PlaceholderTab label={tabs.length === 1 ? tabs[0].name : t`Tab 1`} />
<TabButton
label={t`Tab 1`}
value={null}
showMenu
menuItems={menuItems}
/>
) : (
tabs.map(tab => (
<Sortable key={tab.id} id={tab.id} disabled={!isEditing}>
......@@ -55,14 +72,9 @@ export function DashboardTabs({
value={tab.id}
label={tab.name}
onRename={name => renameTab(tab.id, name)}
canRename={isEditing}
canRename={isEditing && hasMultipleTabs}
showMenu={isEditing}
menuItems={[
{
label: t`Delete`,
action: (_, value) => deleteTab(value),
},
]}
menuItems={menuItems}
/>
</Sortable>
))
......
......@@ -92,7 +92,19 @@ function createNewTab() {
userEvent.click(screen.getByLabelText("Create new tab"));
}
async function selectTabMenuItem(num: number, name: "Delete" | "Rename") {
async function openTabMenu(num: number) {
const dropdownIcons = screen.getAllByRole("img", {
name: "chevrondown icon",
hidden: true,
});
userEvent.click(dropdownIcons[num - 1]);
await screen.findByRole("option");
}
async function selectTabMenuItem(
num: number,
name: "Delete" | "Rename" | "Duplicate",
) {
const dropdownIcons = screen.getAllByRole("img", {
name: "chevrondown icon",
hidden: true,
......@@ -115,6 +127,10 @@ async function renameTab(num: number, name: string) {
userEvent.type(inputEl, `${name}{enter}`);
}
async function duplicateTab(num: number) {
return selectTabMenuItem(num, "Duplicate");
}
async function findSlug({ tabId, name }: { tabId: number; name: string }) {
return screen.findByText(new RegExp(getSlug({ tabId, name })));
}
......@@ -202,21 +218,27 @@ describe("DashboardTabs", () => {
});
describe("when editing", () => {
it("should display a placeholder tab when there are none", () => {
it("should display a placeholder tab when there are none", async () => {
setup({ tabs: [] });
const placeholderTab = queryTab("Tab 1");
expect(placeholderTab).toHaveAttribute("aria-disabled", "true");
expect(queryTab("Tab 1")).toBeInTheDocument();
await openTabMenu(1);
expect(screen.getByText("Duplicate")).toBeInTheDocument();
expect(screen.getByText("Path is /dashboard/1")).toBeInTheDocument();
});
it("should display a placeholder tab when there is only one", () => {
it("should display a placeholder tab when there is only one", async () => {
setup({
tabs: [getDefaultTab({ tabId: 1, dashId: 1, name: "Lonely tab" })],
});
const placeholderTab = queryTab("Lonely tab");
expect(placeholderTab).toHaveAttribute("aria-disabled", "true");
expect(queryTab("Lonely tab")).toBeInTheDocument();
await openTabMenu(1);
expect(screen.getByText("Duplicate")).toBeInTheDocument();
expect(screen.getByText("Path is /dashboard/1")).toBeInTheDocument();
});
......@@ -296,7 +318,7 @@ describe("DashboardTabs", () => {
expect(await findSlug({ tabId: 2, name: "Tab 2" })).toBeInTheDocument();
});
it("should disable the last tab and remove slug if the penultimate tab was deleted", async () => {
it("should keep the last tab and remove slug if the penultimate tab was deleted", async () => {
setup();
await deleteTab(3);
......@@ -304,7 +326,10 @@ describe("DashboardTabs", () => {
await deleteTab(2);
expect(queryTab(1)).toHaveAttribute("aria-disabled", "true");
expect(queryTab(1)).toBeInTheDocument();
await openTabMenu(1);
expect(screen.getByText("Duplicate")).toBeInTheDocument();
expect(screen.getByText("Path is /dashboard/1")).toBeInTheDocument();
});
......@@ -348,6 +373,31 @@ describe("DashboardTabs", () => {
});
});
describe("when duplicating tabs", () => {
it("should allow the user to duplicate the placeholder tab if there are none", async () => {
setup({ tabs: [] });
await duplicateTab(1);
expect(queryTab("Tab 1")).toBeInTheDocument();
expect(queryTab("Copy of Tab 1")).toBeInTheDocument();
expect(screen.getByText("Selected tab id is -2")).toBeInTheDocument();
expect(screen.getByText("Path is /dashboard/1")).toBeInTheDocument();
});
it("should allow the user to duplicate a tab", async () => {
setup();
await duplicateTab(1);
expect(queryTab("Copy of Tab 1")).toBeInTheDocument();
expect(screen.getByText("Selected tab id is -2")).toBeInTheDocument();
expect(screen.getByText("Path is /dashboard/1")).toBeInTheDocument();
});
});
describe("when navigating away from dashboard", () => {
it("should preserve selected tab id", () => {
setup();
......
......@@ -72,4 +72,8 @@ export const TEST_DASHBOARD_STATE: DashboardState = {
1: createMockDashCard({ dashCardId: 1, tabId: 1 }),
2: createMockDashCard({ dashCardId: 2, tabId: 2 }),
},
dashcardData: {
1: { 1: null },
2: { 1: null },
},
};
......@@ -12,6 +12,7 @@ import {
selectTab,
undoDeleteTab,
moveTab as moveTabAction,
duplicateTab,
} from "metabase/dashboard/actions";
import type { SelectedTabId } from "metabase-types/store";
import { getSelectedTabId, getTabs } from "metabase/dashboard/selectors";
......@@ -60,6 +61,7 @@ export function useDashboardTabs({ location }: { location: Location }) {
tabs,
selectedTabId,
createNewTab: () => dispatch(createNewTab()),
duplicateTab: (tabId: SelectedTabId) => dispatch(duplicateTab(tabId)),
deleteTab,
renameTab: (tabId: SelectedTabId, name: string) =>
dispatch(renameTab({ tabId, name })),
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment