From 14bda8a5cfdd65bc6d188fcdcb718f9283d1f4b9 Mon Sep 17 00:00:00 2001 From: Joshen Lim Date: Tue, 22 Sep 2026 16:21:55 +0800 Subject: [PATCH] Joshenlim/fe 4438 explorer allow deleting notebookchat without first having to (#50701) ### Context Adds context menu to the explorer nav to allow users to delete notebooks or chats from there (instead of having to open the notebook / chat first, then select delete from the header) image ### Changes involved: - Consolidates the rendering of explorer nav items into a single component that both `ExplorerNavNotebook`, `ExplorerNavHome` and `ExplorerNavChats` use. - We were previously rendering a link for notebook, and a button for chats. But chats' button was just calling `router.push` under the hood. - Opting to use a Context to handle the delete functionality such that we can render the confirmation modal just once at the layout level since there's multiple places that have this delete functionality - notebook tab, chat tab, context menu in each of the explorer navs ### To test - [ ] Verify that navigating around the explorer is status quo - [ ] Verify that you can delete a notebook/chat from within the notebook/chat tab - [ ] Verify that you can delete a notebook/chat from the explorer nav - [ ] Verify that after deleting a notebook/chat from either locations, the tabs should clear ## Summary by CodeRabbit - **New Features** - Unified chat and notebook navigation with direct links and active-item highlighting. - Added context-menu options to delete chats and notebooks with confirmation and status notifications. - Added double-click support for keeping recently viewed chats and notebooks open as tabs. - Deleted items and their related tabs are removed automatically. - **Tests** - Updated Explorer navigation tests for the unified link-based item behavior. --- .../Explorer/ExplorerChatToolbar.tsx | 29 ++---- .../Explorer/ExplorerNotebookTab.tsx | 45 ++------- ...kTab.assistant-cache-invalidation.test.tsx | 5 +- .../__tests__/ExplorerNotebookTab.test.tsx | 5 +- .../layouts/ExplorerLayout/ExplorerLayout.tsx | 71 +++++++------- .../ExplorerLayout/ExplorerNavChats.test.tsx | 12 ++- .../ExplorerLayout/ExplorerNavChats.tsx | 32 +++---- .../ExplorerLayout/ExplorerNavHome.tsx | 63 ++++++------ .../ExplorerLayout/ExplorerNavItem.tsx | 57 +++++++++++ .../ExplorerLayout/ExplorerNavNotebooks.tsx | 31 +++--- .../ExplorerLayout/ExplorerProvider.tsx | 96 +++++++++++++++++++ 11 files changed, 281 insertions(+), 165 deletions(-) create mode 100644 apps/studio/components/layouts/ExplorerLayout/ExplorerNavItem.tsx create mode 100644 apps/studio/components/layouts/ExplorerLayout/ExplorerProvider.tsx diff --git a/apps/studio/components/interfaces/Explorer/ExplorerChatToolbar.tsx b/apps/studio/components/interfaces/Explorer/ExplorerChatToolbar.tsx index 07748bfc964..3c5d5689d14 100644 --- a/apps/studio/components/interfaces/Explorer/ExplorerChatToolbar.tsx +++ b/apps/studio/components/interfaces/Explorer/ExplorerChatToolbar.tsx @@ -9,7 +9,6 @@ import { DropdownMenuSeparator, DropdownMenuTrigger, } from 'ui' -import ConfirmationModal from 'ui-patterns/Dialogs/ConfirmationModal' import { ExplorerToolbar, @@ -18,6 +17,7 @@ import { ExplorerToolbarIcon, ExplorerToolbarTitle, } from './ExplorerToolbar' +import { useExplorerDeleteItem } from '@/components/layouts/ExplorerLayout/ExplorerProvider' import { AIAssistantMetadataWarning } from '@/components/ui/AIAssistantPanel/AIAssistantMetadataWarning' import type { AssistantChatHeaderProps } from '@/components/ui/AIAssistantPanel/AssistantChat' import { ShortcutPills } from '@/components/ui/ShortcutTooltip' @@ -41,7 +41,7 @@ export const ExplorerChatToolbar = ({ const snap = useAiAssistantStateSnapshot() const chat = snap.chats[chatId] const [isOptInModalOpen, setIsOptInModalOpen] = useState(false) - const [isDeleteModalOpen, setIsDeleteModalOpen] = useState(false) + const { onSelectDelete } = useExplorerDeleteItem() const handleCopyChatId = () => { copyToClipboard(chatId, () => toast.success(`Copied chat ID for ${chat?.name}`)) @@ -51,12 +51,6 @@ export const ExplorerChatToolbar = ({ if (name.trim()) snap.renameChat(chatId, name.trim()) } - const handleDeleteChat = () => { - snap.deleteChat(chatId) - setIsDeleteModalOpen(false) - toast.success(`Deleted "${chat?.name}"`) - } - useShortcut(SHORTCUT_IDS.AI_ASSISTANT_COPY_CHAT_ID, handleCopyChatId, { enabled: shortcutsEnabled && !isChatLoading, }) @@ -105,7 +99,10 @@ export const ExplorerChatToolbar = ({ /> - setIsDeleteModalOpen(true)}> + onSelectDelete({ id: chatId, type: 'chat', name: chat?.name ?? '' })} + > Delete chat @@ -121,20 +118,6 @@ export const ExplorerChatToolbar = ({ updatedOptInSinceMCP={updatedOptInSinceMCP} aiOptInLevel={aiOptInLevel} /> - - setIsDeleteModalOpen(false)} - onConfirm={handleDeleteChat} - > -

- This will permanently delete this chat and its message history. This action cannot be - undone. -

- ) } diff --git a/apps/studio/components/interfaces/Explorer/ExplorerNotebookTab.tsx b/apps/studio/components/interfaces/Explorer/ExplorerNotebookTab.tsx index 2b1c469df03..35c710418bc 100644 --- a/apps/studio/components/interfaces/Explorer/ExplorerNotebookTab.tsx +++ b/apps/studio/components/interfaces/Explorer/ExplorerNotebookTab.tsx @@ -29,7 +29,6 @@ import { SquareCode, Trash, } from 'lucide-react' -import { useRouter } from 'next/router' import { useEffect, useEffectEvent, useRef, useState } from 'react' import { toast } from 'sonner' import { @@ -67,8 +66,8 @@ import { QueryCell } from './QueryCell' import { type QueryEditorHandle } from './QueryEditor' import { createMarkdownCellSkeleton, createQueryCellSkeleton } from './utils' import { checkDestructiveQuery } from '@/components/interfaces/SQLEditor/SQLEditor.utils' +import { useExplorerDeleteItem } from '@/components/layouts/ExplorerLayout/ExplorerProvider' import { ButtonTooltip } from '@/components/ui/ButtonTooltip' -import { useContentDeleteMutation } from '@/data/content/content-delete-mutation' import { evictNotebookFromCaches, hasDiscardableChanges, @@ -89,12 +88,12 @@ import { import { createTabId, useTabsStateSnapshot } from '@/state/tabs' export const ExplorerNotebookTab = () => { - const router = useRouter() const { id, ref } = useParams() const tabs = useTabsStateSnapshot() const snap = useNotebooksStateSnapshot() const queryClient = useQueryClient() const { createChat, isCreating } = useCreateChat() + const { onSelectDelete } = useExplorerDeleteItem() const [isIntellisenseEnabled, setIsIntellisenseEnabled] = useLocalStorageQuery( LOCAL_STORAGE_KEYS.SQL_EDITOR_INTELLISENSE, @@ -108,7 +107,6 @@ export const ExplorerNotebookTab = () => { const queryCellIds = cells.filter(isQueryCell).map((cell) => cell._id) const [isRunningNotebook, setIsRunningNotebook] = useState(false) - const [isDeleteModalOpen, setIsDeleteModalOpen] = useState(false) const [isSaveBeforeAnalyzeOpen, setIsSaveBeforeAnalyzeOpen] = useState(false) const [isSaveConflictOpen, setIsSaveConflictOpen] = useState(false) const [pendingQueryMatches, setPendingQueryMatches] = useState<{ @@ -133,19 +131,6 @@ export const ExplorerNotebookTab = () => { }, }) - const { mutate: deleteNotebook, isPending: isDeleting } = useContentDeleteMutation({ - onSuccess: () => { - toast.success('Successfully deleted notebook') - if (id) { - tabs.removeTab(createTabId('notebook', { id })) - snap.removeNotebook({ id }) - } - setIsDeleteModalOpen(false) - router.push(`/project/${ref}/explorer`) - }, - onError: (error) => toast.error(`Failed to delete notebook: ${error.message}`), - }) - const sensors = useSensors( useSensor(PointerSensor, { activationConstraint: { distance: 8 } }), useSensor(KeyboardSensor, { coordinateGetter: sortableKeyboardCoordinates }) @@ -331,11 +316,6 @@ export const ExplorerNotebookTab = () => { } } - const handleConfirmDeleteNotebook = () => { - if (!ref || !id) return - deleteNotebook({ projectRef: ref, ids: [id] }) - } - const handleDragEnd = (event: DragEndEvent) => { persistNotebookTab() @@ -443,7 +423,10 @@ export const ExplorerNotebookTab = () => { Copy as Markdown - setIsDeleteModalOpen(true)}> + id && onSelectDelete({ id, type: 'notebook', name: name ?? '' })} + > Delete notebook @@ -533,22 +516,6 @@ export const ExplorerNotebookTab = () => { - setIsDeleteModalOpen(false)} - onConfirm={handleConfirmDeleteNotebook} - > -

- This action cannot be undone. Are you sure you want to delete '{name}'? -

- - { const renderNotebookTab = (queryClient: QueryClient, tabsState = createTabsState(PROJECT_REF)) => customRender( - + + + , { queryClient } ) diff --git a/apps/studio/components/interfaces/Explorer/__tests__/ExplorerNotebookTab.test.tsx b/apps/studio/components/interfaces/Explorer/__tests__/ExplorerNotebookTab.test.tsx index 003458f2df5..ded7a3582df 100644 --- a/apps/studio/components/interfaces/Explorer/__tests__/ExplorerNotebookTab.test.tsx +++ b/apps/studio/components/interfaces/Explorer/__tests__/ExplorerNotebookTab.test.tsx @@ -7,6 +7,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { ExplorerNotebookTab } from '../ExplorerNotebookTab' import { setCellSql } from '../QueryCell/QueryCell.utils' import { createMarkdownCellSkeleton, createQueryCellSkeleton } from '../utils' +import { ExplorerProvider } from '@/components/layouts/ExplorerLayout/ExplorerProvider' import { isQueryCell } from '@/data/content/notebooks/notebook-schema' import { untrustedLogSql } from '@/data/logs/safe-analytics-sql' import { notebooksState } from '@/state/notebooks/notebooks-state' @@ -84,7 +85,9 @@ const seedNotebook = (cells: Notebooks.Cell[], status: 'new' | 'saved' = 'saved' const renderNotebookTab = (tabsState = createTabsState('default')) => customRender( - + + + ) diff --git a/apps/studio/components/layouts/ExplorerLayout/ExplorerLayout.tsx b/apps/studio/components/layouts/ExplorerLayout/ExplorerLayout.tsx index f73170c15a1..f9e218cc8df 100644 --- a/apps/studio/components/layouts/ExplorerLayout/ExplorerLayout.tsx +++ b/apps/studio/components/layouts/ExplorerLayout/ExplorerLayout.tsx @@ -20,6 +20,7 @@ import { ExplorerNavChats } from './ExplorerNavChats' import { ExplorerNavHeader } from './ExplorerNavHeader' import { ExplorerNavHome } from './ExplorerNavHome' import { ExplorerNavNotebooks } from './ExplorerNavNotebooks' +import { ExplorerProvider } from './ExplorerProvider' import { useExplorerPreferences } from '@/components/interfaces/Account/Preferences/useExplorerPreferences' import { ExplorerNotebookTabCoordinator } from '@/components/interfaces/Explorer/ExplorerNotebookTabCoordinator' import { ExplorerQueryTabCoordinator } from '@/components/interfaces/Explorer/ExplorerQueryTabCoordinator' @@ -75,42 +76,44 @@ export const ExplorerLayout = ({ browserTitle, children, title }: ExplorerLayout } return ( - setSection(undefined)} - rootAction={} - /> - } - productMenu={ -
- - {section === undefined && } - {section === 'notebook' && } - {section === 'chat' && } - -
- } - > - - - - -
-
- : undefined} - newTabButton={} - onTabChange={handleTabChange} + + setSection(undefined)} + rootAction={} /> + } + productMenu={ +
+ + {section === undefined && } + {section === 'notebook' && } + {section === 'chat' && } + +
+ } + > + + + + +
+
+ : undefined} + newTabButton={} + onTabChange={handleTabChange} + /> +
+
{children}
-
{children}
-
- + + ) } diff --git a/apps/studio/components/layouts/ExplorerLayout/ExplorerNavChats.test.tsx b/apps/studio/components/layouts/ExplorerLayout/ExplorerNavChats.test.tsx index a8777fa202e..4f9d7439a95 100644 --- a/apps/studio/components/layouts/ExplorerLayout/ExplorerNavChats.test.tsx +++ b/apps/studio/components/layouts/ExplorerLayout/ExplorerNavChats.test.tsx @@ -36,6 +36,10 @@ vi.mock('@/components/interfaces/Explorer/hooks', () => ({ useCreateChat: () => ({ openChat: vi.fn() }), })) +vi.mock('./ExplorerProvider', () => ({ + useExplorerDeleteItem: () => ({ confirmDelete: vi.fn() }), +})) + vi.mock('@/state/ai-assistant-state', () => ({ useAiAssistantChatList: () => [ { id: 'old-chat', name: 'Older investigation', updatedAt: new Date('2026-01-01') }, @@ -54,20 +58,20 @@ describe('ExplorerNavChats', () => { it('filters support chats, safely sorts rehydrated chats, and marks the route active', () => { customRender() - const chatButtons = screen.getAllByRole('button') - expect(chatButtons.map((button) => button.textContent)).toEqual([ + const chatLinks = screen.getAllByRole('link') + expect(chatLinks.map((link) => link.textContent)).toEqual([ 'Recent investigation', 'Older investigation', 'Rehydrated chat', ]) - expect(chatButtons[0]).toHaveClass('active') + expect(chatLinks[0]).toHaveClass('active') expect(screen.queryByText('Support conversation')).not.toBeInTheDocument() fireEvent.change(screen.getByRole('textbox', { name: 'Search chats' }), { target: { value: 'rehydrated' }, }) - expect(screen.getByRole('button')).toHaveTextContent('Rehydrated chat') + expect(screen.getByRole('link')).toHaveTextContent('Rehydrated chat') expect(screen.queryByText('Recent investigation')).not.toBeInTheDocument() }) }) diff --git a/apps/studio/components/layouts/ExplorerLayout/ExplorerNavChats.tsx b/apps/studio/components/layouts/ExplorerLayout/ExplorerNavChats.tsx index 5635f23d706..c2211b926d8 100644 --- a/apps/studio/components/layouts/ExplorerLayout/ExplorerNavChats.tsx +++ b/apps/studio/components/layouts/ExplorerLayout/ExplorerNavChats.tsx @@ -1,11 +1,10 @@ import { useParams } from 'common' -import { MessageSquare } from 'lucide-react' import { useRouter } from 'next/router' import { useState } from 'react' -import { cn } from 'ui' -import { ExplorerNavResourceWrapper, rowClassName } from './ExplorerLayout.constants' -import { useCreateChat } from '@/components/interfaces/Explorer/hooks' +import { ExplorerNavResourceWrapper } from './ExplorerLayout.constants' +import { ExplorerNavItem } from './ExplorerNavItem' +import { useExplorerDeleteItem } from './ExplorerProvider' import type { ChatSession } from '@/state/ai-assistant-state' import { useAiAssistantChatList } from '@/state/ai-assistant-state' import { createTabId, useTabsStateSnapshot } from '@/state/tabs' @@ -22,10 +21,10 @@ const getVisibleChats = (chats: ChatSession[], search: string): ChatSession[] => export const ExplorerNavChats = () => { const [search, setSearch] = useState('') const router = useRouter() - const { id } = useParams() - const { openChat } = useCreateChat() + const { id, ref } = useParams() const chatList = useAiAssistantChatList() const tabs = useTabsStateSnapshot() + const { onSelectDelete } = useExplorerDeleteItem() const chats = getVisibleChats(chatList, search) @@ -41,20 +40,17 @@ export const ExplorerNavChats = () => { const isActive = router.pathname.includes('/explorer/chat/') && id === chat.id return ( - + onSelectDelete={() => + onSelectDelete({ id: chat.id, type: 'chat', name: chat.name }) + } + /> ) }) )} diff --git a/apps/studio/components/layouts/ExplorerLayout/ExplorerNavHome.tsx b/apps/studio/components/layouts/ExplorerLayout/ExplorerNavHome.tsx index c5c44e7cdb0..7937a35f1de 100644 --- a/apps/studio/components/layouts/ExplorerLayout/ExplorerNavHome.tsx +++ b/apps/studio/components/layouts/ExplorerLayout/ExplorerNavHome.tsx @@ -13,7 +13,9 @@ import { rowClassName, } from './ExplorerLayout.constants' import { formatRelativeTimeShort, getRecentlyUpdatedItems } from './ExplorerNavHome.utils' -import { useCreateChat, useCreateQuery } from '@/components/interfaces/Explorer/hooks' +import { ExplorerNavItem } from './ExplorerNavItem' +import { useExplorerDeleteItem } from './ExplorerProvider' +import { useCreateQuery } from '@/components/interfaces/Explorer/hooks' import { useContentCountQuery } from '@/data/content/content-count-query' import { useNotebooksInfiniteQuery } from '@/data/content/notebooks/notebooks-infinite-query' import { useAiAssistantChatList } from '@/state/ai-assistant-state' @@ -26,12 +28,12 @@ export const ExplorerNavHome = ({ onSelectSection: (section: ExplorerResourceType) => void }) => { const router = useRouter() - const { ref } = useParams() + const { id, ref } = useParams() const tabs = useTabsStateSnapshot() const appStateSnapshot = useAppStateSnapshot() - const { openChat } = useCreateChat() const { createQuery } = useCreateQuery() + const { onSelectDelete } = useExplorerDeleteItem() const { data: notebooksData } = useNotebooksInfiniteQuery({ projectRef: ref, limit: 100 }) const notebooks = notebooksData?.pages.flatMap((page) => page.content) ?? [] @@ -103,39 +105,34 @@ export const ExplorerNavHome = ({

Nothing edited yet

) : ( recentItems.map((item) => { - const Icon = EXPLORER_SECTIONS.find((section) => section.type === item.type)?.icon - const content = ( - <> - {Icon &&