diff --git a/frontend/src/metabase/core/components/TabButton/TabButton.tsx b/frontend/src/metabase/core/components/TabButton/TabButton.tsx index b7fe80756bb136a9b516a57651495728bcd68475..6abc00ade51fd32ebec9ce96c287a9f9765b693a 100644 --- a/frontend/src/metabase/core/components/TabButton/TabButton.tsx +++ b/frontend/src/metabase/core/components/TabButton/TabButton.tsx @@ -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) { diff --git a/frontend/src/metabase/dashboard/actions/tabs.ts b/frontend/src/metabase/dashboard/actions/tabs.ts index 7c2a2da095037a3cf9933179f643af3c8b15dcba..fa6b6047bd3ce51bed58fcda5b318727ab674bf8 100644 --- a/frontend/src/metabase/dashboard/actions/tabs.ts +++ b/frontend/src/metabase/dashboard/actions/tabs.ts @@ -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; }, ); diff --git a/frontend/src/metabase/dashboard/components/DashboardTabs/DashboardTabs.styled.tsx b/frontend/src/metabase/dashboard/components/DashboardTabs/DashboardTabs.styled.tsx index b5ef332e5ee732a8b00391e93193718f55d7eee5..5439270b77a76635e6a5baefa43dc30783da06ac 100644 --- a/frontend/src/metabase/dashboard/components/DashboardTabs/DashboardTabs.styled.tsx +++ b/frontend/src/metabase/dashboard/components/DashboardTabs/DashboardTabs.styled.tsx @@ -1,6 +1,5 @@ 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; diff --git a/frontend/src/metabase/dashboard/components/DashboardTabs/DashboardTabs.tsx b/frontend/src/metabase/dashboard/components/DashboardTabs/DashboardTabs.tsx index 1b9aae7313dd79142ed0a09b1860ec0953834f42..813ebef42cb6de456c55eaddd4c44e1227b9b6e1 100644 --- a/frontend/src/metabase/dashboard/components/DashboardTabs/DashboardTabs.tsx +++ b/frontend/src/metabase/dashboard/components/DashboardTabs/DashboardTabs.tsx @@ -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> )) diff --git a/frontend/src/metabase/dashboard/components/DashboardTabs/DashboardTabs.unit.spec.tsx b/frontend/src/metabase/dashboard/components/DashboardTabs/DashboardTabs.unit.spec.tsx index c1c344bc78840aca9265d05454a26f5ce52a264f..5b5d2fb9320e974d4414ea772e590aa59d1a8fdd 100644 --- a/frontend/src/metabase/dashboard/components/DashboardTabs/DashboardTabs.unit.spec.tsx +++ b/frontend/src/metabase/dashboard/components/DashboardTabs/DashboardTabs.unit.spec.tsx @@ -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(); diff --git a/frontend/src/metabase/dashboard/components/DashboardTabs/test-utils.ts b/frontend/src/metabase/dashboard/components/DashboardTabs/test-utils.ts index 044ab09976b62bb37432e4f1e243f915d28208e3..a280606789b0cb9a2e3f0fd62d5b206b19a67ea9 100644 --- a/frontend/src/metabase/dashboard/components/DashboardTabs/test-utils.ts +++ b/frontend/src/metabase/dashboard/components/DashboardTabs/test-utils.ts @@ -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 }, + }, }; diff --git a/frontend/src/metabase/dashboard/components/DashboardTabs/use-dashboard-tabs.ts b/frontend/src/metabase/dashboard/components/DashboardTabs/use-dashboard-tabs.ts index 3dea983f776a06896e6d0740ae4634fa0265a1cd..404f701e7c7bbe39b939e754222e09ce060ad337 100644 --- a/frontend/src/metabase/dashboard/components/DashboardTabs/use-dashboard-tabs.ts +++ b/frontend/src/metabase/dashboard/components/DashboardTabs/use-dashboard-tabs.ts @@ -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 })),