From d4f0add14a88138d60da99386971eb027bd5fd8b Mon Sep 17 00:00:00 2001 From: Karl Power Date: Mon, 29 Jun 2026 16:49:12 +0200 Subject: [PATCH 1/8] feat: single drawer navigation --- .changeset/single-drawer-navigation.md | 5 + .../app/src/NamespaceDetailsSidePanel.tsx | 2 - packages/app/src/NodeDetailsSidePanel.tsx | 6 +- packages/app/src/PodDetailsSidePanel.tsx | 3 - packages/app/src/SessionSidePanel.tsx | 276 +++++-- packages/app/src/SessionSubpanel.tsx | 60 +- .../src/__tests__/SessionSidePanel.test.tsx | 11 + .../app/src/components/ContextSidePanel.tsx | 113 +-- .../app/src/components/DBRowDataPanel.tsx | 24 +- .../app/src/components/DBRowOverviewPanel.tsx | 1 - .../app/src/components/DBRowSidePanel.tsx | 770 +++++++++++++++--- .../src/components/DBRowSidePanelHeader.tsx | 211 +---- packages/app/src/components/DBRowTable.tsx | 7 +- .../app/src/components/DBSessionPanel.tsx | 7 +- .../components/DBSqlRowTableWithSidebar.tsx | 23 +- .../app/src/components/PatternSidePanel.tsx | 2 - .../ServiceDashboardSlowestEventsTile.tsx | 2 - .../src/components/SidePanelBreadcrumbs.tsx | 138 ++++ .../__tests__/DBRowSidePanelHeader.test.tsx | 10 +- packages/app/styles/LogSidePanel.module.scss | 1 - .../tests/e2e/features/search/search.spec.ts | 2 +- 21 files changed, 1089 insertions(+), 585 deletions(-) create mode 100644 .changeset/single-drawer-navigation.md create mode 100644 packages/app/src/components/SidePanelBreadcrumbs.tsx diff --git a/.changeset/single-drawer-navigation.md b/.changeset/single-drawer-navigation.md new file mode 100644 index 0000000000..f6f6995e14 --- /dev/null +++ b/.changeset/single-drawer-navigation.md @@ -0,0 +1,5 @@ +--- +"@hyperdx/app": minor +--- + +Redesign the event side panel into a single right-hand drawer with breadcrumb-stack navigation. Logs, traces, and sessions now navigate in-place (surrounding-context drilldowns, log → trace via a new "View Trace" action, and session → event) instead of stacking layered drawers, with a unified `SidePanelBreadcrumbs` trail that is restorable from the URL. The header gains a metadata row (timestamp, service, duration, status, Copy Trace ID) and the session drawer's close button now closes the entire panel. diff --git a/packages/app/src/NamespaceDetailsSidePanel.tsx b/packages/app/src/NamespaceDetailsSidePanel.tsx index d174eac198..61c0734371 100644 --- a/packages/app/src/NamespaceDetailsSidePanel.tsx +++ b/packages/app/src/NamespaceDetailsSidePanel.tsx @@ -181,8 +181,6 @@ function NamespaceLogs({ - {/* + {/* Search - + */} @@ -194,8 +194,6 @@ function NodeLogs({ @@ -460,7 +458,6 @@ export default function PodDetailsSidePanel({ rowId={rowId} aliasWith={aliasWith} onClose={handleCloseRowSidePanel} - isNestedPanel={true} /> )} diff --git a/packages/app/src/SessionSidePanel.tsx b/packages/app/src/SessionSidePanel.tsx index 2cee84a423..a94643290d 100644 --- a/packages/app/src/SessionSidePanel.tsx +++ b/packages/app/src/SessionSidePanel.tsx @@ -1,33 +1,60 @@ -import { useCallback, useMemo, useState } from 'react'; +import { useCallback, useMemo } from 'react'; +import { useQueryState } from 'nuqs'; +import { ErrorBoundary } from 'react-error-boundary'; import { useHotkeys } from 'react-hotkeys-hook'; import { DateRange, SearchCondition, SearchConditionLanguage, + SourceKind, TSessionSource, TTraceSource, } from '@hyperdx/common-utils/dist/types'; -import { ActionIcon, Box, Button, Drawer, Group } from '@mantine/core'; +import { + ActionIcon, + Box, + Button, + Drawer, + Flex, + Group, + Stack, + Text, + Tooltip, +} from '@mantine/core'; import { notifications } from '@mantine/notifications'; import { IconLink, IconX } from '@tabler/icons-react'; +import { + DBRowSidePanelInner, + RowSidePanelContext, +} from '@/components/DBRowSidePanel'; import { DrawerFullWidthToggle, INITIAL_DRAWER_WIDTH_PERCENT, } from '@/components/DrawerUtils'; +import SidePanelBreadcrumbs, { + BreadcrumbItem, +} from '@/components/SidePanelBreadcrumbs'; import useResizable from '@/hooks/useResizable'; +import { WithClause } from '@/hooks/useRowWhere'; import { CLIPBOARD_ERROR_MESSAGE, copyTextToClipboard, } from '@/utils/clipboard'; +import { parseAsJsonEncoded } from '@/utils/queryParsers'; +import { ZIndexContext } from '@/zIndex'; import { Session } from './sessions'; import SessionSubpanel from './SessionSubpanel'; import { formatDistanceToNowStrictShort } from './utils'; -import { ZIndexContext } from './zIndex'; import styles from '@/../styles/LogSidePanel.module.scss'; +type SelectedEvent = { + rowId: string; + aliasWith: WithClause[]; +}; + export default function SessionSidePanel({ traceSource, sessionSource, @@ -50,8 +77,21 @@ export default function SessionSidePanel({ onClose: () => void; zIndex?: number; }) { - // Keep track of sub-drawers so we can disable closing this root drawer - const [subDrawerOpen, setSubDrawerOpen] = useState(false); + // A single in-place event view (session → event), persisted to the URL so it + // survives reload and shared links. Deeper navigation (View Trace, + // surrounding context) is handled by `DBRowSidePanelInner`, which persists + // its own state to the shared `sidePanel*` params below. + const [selectedEvent, setSelectedEvent] = useQueryState( + 'sessionPanelEvent', + parseAsJsonEncoded(), + ); + + // The embedded `DBRowSidePanelInner` owns these shared params. We clear them + // whenever the session-level selection changes so a stale inner stack can't + // leak across events or sessions. + const [, setSourceStackParam] = useQueryState('sidePanelSourceStack'); + const [, setNavStackParam] = useQueryState('sidePanelNavStack'); + const [, setSidePanelTab] = useQueryState('sidePanelTab'); const { size, setSize, startResize } = useResizable( INITIAL_DRAWER_WIDTH_PERCENT, @@ -61,14 +101,59 @@ export default function SessionSidePanel({ setSize(isFullWidth ? INITIAL_DRAWER_WIDTH_PERCENT : 100); }, [isFullWidth, setSize]); - useHotkeys( - ['esc'], - () => { - onClose(); - }, - { - enabled: subDrawerOpen === false, + const sessionLabel = session?.userEmail || `Anonymous Session ${sessionId}`; + + const clearInnerNavigation = useCallback(() => { + setSourceStackParam(null); + setNavStackParam(null); + setSidePanelTab(null); + }, [setSourceStackParam, setNavStackParam, setSidePanelTab]); + + const handleBackToSession = useCallback(() => { + setSelectedEvent(null); + clearInnerNavigation(); + }, [setSelectedEvent, clearInnerNavigation]); + + const handleEventNavigate = useCallback( + (rowId: string, aliasWith: WithClause[]) => { + clearInnerNavigation(); + setSelectedEvent({ rowId, aliasWith }); }, + [setSelectedEvent, clearInnerNavigation], + ); + + // X / Esc-at-root closes the whole panel and clears the session-panel params. + const handleClose = useCallback(() => { + setSelectedEvent(null); + clearInnerNavigation(); + onClose(); + }, [setSelectedEvent, clearInnerNavigation, onClose]); + + // Back pops to the session root; X always closes the whole panel. + const handleNavigateBack = useCallback(() => { + if (selectedEvent) { + handleBackToSession(); + } else { + handleClose(); + } + }, [selectedEvent, handleBackToSession, handleClose]); + + useHotkeys(['esc'], handleNavigateBack); + + const shareSession = useCallback(async () => { + const ok = await copyTextToClipboard(window.location.href); + notifications.show( + ok + ? { color: 'green', message: 'Copied link to clipboard' } + : { color: 'red', message: CLIPBOARD_ERROR_MESSAGE }, + ); + }, []); + + const breadcrumbs = useMemo( + (): BreadcrumbItem[] => [ + { label: sessionLabel, sourceKind: SourceKind.Session }, + ], + [sessionLabel], ); const timeAgo = useMemo(() => { @@ -81,91 +166,128 @@ export default function SessionSidePanel({ return ( { - if (!subDrawerOpen) { - onClose(); - } - }} + onClose={handleClose} position="right" size={`${size}vw`} withCloseButton={false} + closeOnEscape={false} zIndex={zIndex} styles={{ + content: { + border: 'none', + boxShadow: 'var(--shadow-drawer)', + }, body: { padding: 0, - height: '100vh', + height: '100%', }, }} - className="border-start" >
-
-
-
- {session?.userEmail || `Anonymous Session ${sessionId}`} -
- Last active {timeAgo} ago - · - {Number.parseInt(session?.errorCount ?? '0') > 0 ? ( + {selectedEvent ? ( + + ( + +
+ An error occurred while rendering this event. +
+
+ {error?.error?.message} +
+ + )} + > + + + + ) : ( + <> + + + + + + + + + + + + + + + Last active {timeAgo} ago + {Number.parseInt(session?.errorCount ?? '0') > 0 && ( <> - + {' · '} + {session?.errorCount} Errors - - · + - ) : null} - {session?.sessionCount} Events -
-
- - + +
+ - - - - - -
-
- {sessionId != null ? ( - - ) : null} +
+ + )}
diff --git a/packages/app/src/SessionSubpanel.tsx b/packages/app/src/SessionSubpanel.tsx index abbca03ab5..c5ef31a75d 100644 --- a/packages/app/src/SessionSubpanel.tsx +++ b/packages/app/src/SessionSubpanel.tsx @@ -17,7 +17,6 @@ import { Button, Divider, Group, - Portal, SegmentedControl, Tooltip, } from '@mantine/core'; @@ -31,9 +30,7 @@ import { IconToggleRight, } from '@tabler/icons-react'; -import DBRowSidePanel from '@/components/DBRowSidePanel'; import { RowWhereResult, WithClause } from '@/hooks/useRowWhere'; -import { useZIndex, ZIndexContext } from '@/zIndex'; import SearchWhereInput from './components/SearchInput/SearchWhereInput'; import useFieldExpressionGenerator from './hooks/useFieldExpressionGenerator'; @@ -149,12 +146,12 @@ function useSessionChartConfigs({ type: 'lucene' as const, condition: `${traceSource.resourceAttributesExpression}.rum.sessionId:"${rumSessionId}" AND ( - ${traceSource.eventAttributesExpression}.http.status_code:>299 - OR ${traceSource.eventAttributesExpression}.component:"error" - OR ${traceSource.spanNameExpression}:"routeChange" - OR ${traceSource.spanNameExpression}:"documentLoad" - OR ${traceSource.spanNameExpression}:"intercom.onShow" - OR ScopeName:"custom-action" + ${traceSource.eventAttributesExpression}.http.status_code:>299 + OR ${traceSource.eventAttributesExpression}.component:"error" + OR ${traceSource.spanNameExpression}:"routeChange" + OR ${traceSource.spanNameExpression}:"documentLoad" + OR ${traceSource.spanNameExpression}:"intercom.onShow" + OR ScopeName:"custom-action" )`, }), [traceSource, rumSessionId], @@ -166,13 +163,13 @@ function useSessionChartConfigs({ type: 'lucene' as const, condition: `${traceSource.resourceAttributesExpression}.rum.sessionId:"${rumSessionId}" AND ( - ${traceSource.eventAttributesExpression}.http.status_code:* - OR ${traceSource.eventAttributesExpression}.component:"console" - OR ${traceSource.eventAttributesExpression}.component:"error" - OR ${traceSource.spanNameExpression}:"routeChange" - OR ${traceSource.spanNameExpression}:"documentLoad" - OR ${traceSource.spanNameExpression}:"intercom.onShow" - OR ScopeName:"custom-action" + ${traceSource.eventAttributesExpression}.http.status_code:* + OR ${traceSource.eventAttributesExpression}.component:"console" + OR ${traceSource.eventAttributesExpression}.component:"error" + OR ${traceSource.spanNameExpression}:"routeChange" + OR ${traceSource.spanNameExpression}:"documentLoad" + OR ${traceSource.spanNameExpression}:"intercom.onShow" + OR ScopeName:"custom-action" )`, }; }, [traceSource, rumSessionId, getTraceSourceFieldExpression]); @@ -235,30 +232,25 @@ export default function SessionSubpanel({ traceSource, sessionSource, session, - setDrawerOpen, rumSessionId, start, end, initialTs, whereLanguage = 'lucene', onLanguageChange, + onEventNavigate, }: { traceSource: TTraceSource; sessionSource: TSessionSource; session: { serviceName: string }; - setDrawerOpen: (open: boolean) => void; rumSessionId: string; start: Date; end: Date; initialTs?: number; whereLanguage?: SearchConditionLanguage; onLanguageChange?: (lang: 'sql' | 'lucene') => void; + onEventNavigate?: (rowId: string, aliasWith: WithClause[]) => void; }) { - const contextZIndex = useZIndex(); - - const [rowId, setRowId] = useState(undefined); - const [aliasWith, setAliasWith] = useState([]); - const [tsQuery, setTsQuery] = useQueryState( 'ts', parseAsInteger.withOptions({ history: 'replace' }), @@ -430,11 +422,9 @@ export default function SessionSubpanel({ ); const onSessionEventClick = useCallback( (rowWhere: RowWhereResult) => { - setDrawerOpen(true); - setRowId(rowWhere.where); - setAliasWith(rowWhere.aliasWith); + onEventNavigate?.(rowWhere.where, rowWhere.aliasWith); }, - [setDrawerOpen, setRowId, setAliasWith], + [onEventNavigate], ); const onSessionEventTimeClick = useCallback( (ts: number) => { @@ -453,22 +443,6 @@ export default function SessionSubpanel({ return (
- {rowId != null && traceSource && ( - - - { - setDrawerOpen(false); - setRowId(undefined); - }} - /> - - - )}
({ default: () =>
, })); +jest.mock('nuqs', () => { + const actual = jest.requireActual('nuqs'); + // eslint-disable-next-line @typescript-eslint/no-empty-function + const noop = () => {}; + return { + ...actual, + // eslint-disable-next-line @eslint-react/no-unnecessary-use-prefix + useQueryState: () => [null, noop], + }; +}); + jest.mock('../utils/clipboard', () => ({ __esModule: true, CLIPBOARD_ERROR_MESSAGE: diff --git a/packages/app/src/components/ContextSidePanel.tsx b/packages/app/src/components/ContextSidePanel.tsx index e73f5a9532..a50a326515 100644 --- a/packages/app/src/components/ContextSidePanel.tsx +++ b/packages/app/src/components/ContextSidePanel.tsx @@ -1,12 +1,12 @@ import { useCallback, useContext, useMemo, useState } from 'react'; import ms from 'ms'; -import { useQueryState } from 'nuqs'; import { useForm, useWatch } from 'react-hook-form'; import { tcFromSource } from '@hyperdx/common-utils/dist/core/metadata'; import { BuilderChartConfigWithDateRange, isLogSource, isTraceSource, + SourceKind, TSource, } from '@hyperdx/common-utils/dist/types'; import { Badge, Flex, Group, SegmentedControl } from '@mantine/core'; @@ -16,16 +16,10 @@ import SearchWhereInput, { getStoredLanguage, } from '@/components/SearchInput/SearchWhereInput'; import { RowWhereResult, WithClause } from '@/hooks/useRowWhere'; -import { useSource } from '@/source'; import { formatAttributeClause } from '@/utils'; -import { parseAsStringEncoded } from '@/utils/queryParsers'; import { ROW_DATA_ALIASES } from './DBRowDataPanel'; -import DBRowSidePanel, { RowSidePanelContext } from './DBRowSidePanel'; -import { - BreadcrumbNavigationCallback, - BreadcrumbPath, -} from './DBRowSidePanelHeader'; +import { RowSidePanelContext } from './DBRowSidePanel'; import { DBSqlRowTable } from './DBRowTable'; enum ContextBy { @@ -42,35 +36,13 @@ interface ContextSubpanelProps { dbSqlRowTableConfig: BuilderChartConfigWithDateRange | undefined; rowData: Record; rowId: string | undefined; - breadcrumbPath?: BreadcrumbPath; - onBreadcrumbClick?: BreadcrumbNavigationCallback; -} - -// Custom hook to manage nested panel state -export function useNestedPanelState(isNested?: boolean) { - // Query state (URL-based) for root level - const queryState = { - contextRowId: useQueryState('contextRowId', parseAsStringEncoded), - // Source IDs are MongoDB ObjectIDs (hex strings) and contain no special - // characters, so no encoding is needed here. - contextRowSource: useQueryState('contextRowSource'), - }; - - // Local state for nested levels - const localState = { - contextRowId: useState(null), - contextRowSource: useState(null), - }; - - // Choose which state to use based on nesting level - const activeState = isNested ? localState : queryState; - - return { - contextRowId: activeState.contextRowId[0], - contextRowSource: activeState.contextRowSource[0], - setContextRowId: activeState.contextRowId[1], - setContextRowSource: activeState.contextRowSource[1], - }; + onNavigateToRow?: ( + rowId: string, + aliasWith: WithClause[], + label: string, + sourceKind?: SourceKind, + ) => void; + 'data-testid'?: string; } export default function ContextSubpanel({ @@ -78,8 +50,8 @@ export default function ContextSubpanel({ dbSqlRowTableConfig, rowData, rowId, - breadcrumbPath, - onBreadcrumbClick, + onNavigateToRow, + 'data-testid': dataTestId, }: ContextSubpanelProps) { const QUERY_KEY_PREFIX = 'context'; const origTimestamp = rowData[ROW_DATA_ALIASES.TIMESTAMP]; @@ -100,36 +72,21 @@ export default function ContextSubpanel({ const formWhere = useWatch({ control, name: 'where' }); const [debouncedWhere] = useDebouncedValue(formWhere, 1000); - // State management for nested panels - const isNested = !!breadcrumbPath?.length; - - const { - contextRowId, - contextRowSource, - setContextRowId, - setContextRowSource, - } = useNestedPanelState(isNested); - - const { data: contextRowSidePanelSource } = useSource({ - id: contextRowSource || '', - }); - - const [contextAliasWith, setContextAliasWith] = useState([]); - - const handleContextSidePanelClose = useCallback(() => { - setContextRowId(null); - setContextRowSource(null); - }, [setContextRowId, setContextRowSource]); - const { setChildModalOpen } = useContext(RowSidePanelContext); const handleRowExpandClick = useCallback( - (rowWhere: RowWhereResult) => { - setContextRowId(rowWhere.where); - setContextAliasWith(rowWhere.aliasWith); - setContextRowSource(source.id); + (rowWhere: RowWhereResult, row: Record) => { + const body = row?.[ROW_DATA_ALIASES.BODY]; + const fallback = isTraceSource(source) ? 'Span' : 'Log'; + const label = + typeof body === 'string' && body.length > 0 + ? body + : body != null + ? JSON.stringify(body) + : fallback; + onNavigateToRow?.(rowWhere.where, rowWhere.aliasWith, label, source.kind); }, - [source.id, setContextRowId, setContextRowSource], + [onNavigateToRow, source], ); const date = useMemo(() => new Date(origTimestamp), [origTimestamp]); @@ -144,7 +101,7 @@ export default function ContextSubpanel({ /* Functions to help generate WHERE clause based on which Context the user chooses (All, Host, Node, etc...). - Since we support lucene and sql, we need to format the condition + Since we support lucene and sql, we need to format the condition based on the language */ const { @@ -259,7 +216,12 @@ export default function ContextSubpanel({ return ( <> {config && ( - + )} - {contextRowId && contextRowSidePanelSource && ( - - )} ); } diff --git a/packages/app/src/components/DBRowDataPanel.tsx b/packages/app/src/components/DBRowDataPanel.tsx index e91abd5e31..1f631148ae 100644 --- a/packages/app/src/components/DBRowDataPanel.tsx +++ b/packages/app/src/components/DBRowDataPanel.tsx @@ -11,7 +11,11 @@ import { Box } from '@mantine/core'; import { useQueriedChartConfig } from '@/hooks/useChartConfig'; import { WithClause } from '@/hooks/useRowWhere'; -import { getDisplayedTimestampValueExpression, getEventBody } from '@/source'; +import { + getDisplayedTimestampValueExpression, + getDurationMsExpression, + getEventBody, +} from '@/source'; import { getSelectExpressionsForHighlightedAttributes } from '@/utils/highlightedAttributes'; import { DBRowJsonViewer } from './DBRowJsonViewer'; @@ -28,6 +32,8 @@ export enum ROW_DATA_ALIASES { EVENT_ATTRIBUTES = '__hdx_event_attributes', EVENTS_EXCEPTION_ATTRIBUTES = '__hdx_events_exception_attributes', SPAN_EVENTS = '__hdx_span_events', + DURATION_MS = '__hdx_duration_ms', + SPAN_KIND = '__hdx_span_kind', } export function useRowData({ @@ -154,6 +160,22 @@ export function useRowData({ }, ] : []), + ...(source.kind === SourceKind.Trace && source.durationExpression + ? [ + { + valueExpression: getDurationMsExpression(source), + alias: ROW_DATA_ALIASES.DURATION_MS, + }, + ] + : []), + ...(source.kind === SourceKind.Trace && source.spanKindExpression + ? [ + { + valueExpression: source.spanKindExpression, + alias: ROW_DATA_ALIASES.SPAN_KIND, + }, + ] + : []), ...selectHighlightedRowAttributes, ], where: rowId ?? '0=1', diff --git a/packages/app/src/components/DBRowOverviewPanel.tsx b/packages/app/src/components/DBRowOverviewPanel.tsx index 2d8560fff9..1ea1e945dd 100644 --- a/packages/app/src/components/DBRowOverviewPanel.tsx +++ b/packages/app/src/components/DBRowOverviewPanel.tsx @@ -196,7 +196,6 @@ export function RowOverviewPanel({ void; + isFullWidth?: boolean; + onToggleFullWidth?: () => void; +}) { + const [shortcutsOpen, setShortcutsOpen] = useState(false); + + return ( + <> + + {onToggleFullWidth && ( + + )} + + setShortcutsOpen(true)} + aria-label="Keyboard shortcuts" + > + + + + + {({ copied, copy }) => ( + + + + + + )} + + + + + + + + setShortcutsOpen(false)} + /> + + ); +} + +/** A same-source row navigation (e.g. surrounding-context drilldown). */ +type NavEntry = { + rowId: string; + aliasWith?: WithClause[]; + label: string; + sourceKind?: SourceKind; + originTab?: Tab; +}; + +/** A cross-source navigation frame (e.g. log → trace via "View Trace") */ +type SourceFrame = { + sourceId: string; + rowId: string; + aliasWith?: WithClause[]; + label: string; + sourceKind?: SourceKind; + originTab?: Tab; +}; + +const SPAN_KIND_LABELS: Record = { + '1': 'Internal', + '2': 'Server', + '3': 'Client', + '4': 'Producer', + '5': 'Consumer', + Internal: 'Internal', + Server: 'Server', + Client: 'Client', + Producer: 'Producer', + Consumer: 'Consumer', + SPAN_KIND_INTERNAL: 'Internal', + SPAN_KIND_SERVER: 'Server', + SPAN_KIND_CLIENT: 'Client', + SPAN_KIND_PRODUCER: 'Producer', + SPAN_KIND_CONSUMER: 'Consumer', +}; + type DBRowSidePanelProps = { source: TSource; rowId: string | undefined; aliasWith?: WithClause[]; onClose: () => void; - isNestedPanel?: boolean; - breadcrumbPath?: BreadcrumbPath; - onBreadcrumbClick?: BreadcrumbNavigationCallback; }; -const DBRowSidePanel = ({ - rowId: rowId, - aliasWith, - source, - isNestedPanel = false, - setSubDrawerOpen, +type DBRowSidePanelInnerProps = DBRowSidePanelProps & { + isFullWidth?: boolean; + onToggleFullWidth?: () => void; + persistStacksInUrl?: boolean; + parentBreadcrumbs?: BreadcrumbItem[]; + onNavigateToParent?: () => void; +}; + +const EMPTY_SOURCE_STACK: SourceFrame[] = []; +const EMPTY_NAV_STACK: NavEntry[] = []; + +export const DBRowSidePanelInner = ({ + rowId: initialRowId, + aliasWith: initialAliasWith, + source: rootSource, onClose, - breadcrumbPath, - onBreadcrumbClick, isFullWidth, onToggleFullWidth, -}: DBRowSidePanelProps & { - setSubDrawerOpen: Dispatch>; - isFullWidth?: boolean; - onToggleFullWidth?: () => void; -}) => { + parentBreadcrumbs, + onNavigateToParent, +}: DBRowSidePanelInnerProps) => { + const [sourceStack, setSourceStack] = useQueryState( + 'sidePanelSourceStack', + parseAsJsonEncoded().withDefault(EMPTY_SOURCE_STACK), + ); + + const [navStack, setNavStack] = useQueryState( + 'sidePanelNavStack', + parseAsJsonEncoded().withDefault(EMPTY_NAV_STACK), + ); + + const activeSourceFrame = + sourceStack.length > 0 ? sourceStack[sourceStack.length - 1] : null; + + // Resolve the leaf source (cross-source navigation). Intermediate frames only + // need their stored label/kind for breadcrumbs. + const { data: activeStackSource } = useSource({ + id: activeSourceFrame?.sourceId ?? null, + }); + const isResolvingSource = + activeSourceFrame != null && activeStackSource == null; + const source = activeStackSource ?? rootSource; + + const baseRowId = activeSourceFrame?.rowId ?? initialRowId; + const baseAliasWith = activeSourceFrame?.aliasWith ?? initialAliasWith; + + const leafNav = navStack.length > 0 ? navStack[navStack.length - 1] : null; + const resolvedRowId = leafNav?.rowId ?? baseRowId; + const resolvedAliasWith = leafNav?.aliasWith ?? baseAliasWith; + + // Avoid querying the (transiently wrong) root source with a leaf row id. + const activeRowId = isResolvingSource ? undefined : resolvedRowId; + const activeAliasWith = isResolvingSource ? undefined : resolvedAliasWith; + const { data: rowData, isLoading: isRowLoading, @@ -125,39 +293,16 @@ const DBRowSidePanel = ({ error: rowError, } = useRowData({ source, - rowId, - aliasWith, + rowId: activeRowId, + aliasWith: activeAliasWith, }); - const { dbSqlRowTableConfig } = useContext(RowSidePanelContext); - - const handleBreadcrumbClick = useCallback( - (targetLevel: number) => { - // Current panel's level in the hierarchy - const currentLevel = breadcrumbPath?.length ?? 0; - - // The target panel level corresponds to the breadcrumb index: - // - targetLevel 0 = root panel (breadcrumbPath.length = 0) - // - targetLevel 1 = first nested panel (breadcrumbPath.length = 1) - // - etc. - - // If our current level is greater than the target panel level, close this panel - if (currentLevel > targetLevel) { - onClose(); - onBreadcrumbClick?.(targetLevel); - } - // If our current level equals the target panel level, we're the target - don't close - else if (currentLevel === targetLevel) { - // This is the panel the user wants to navigate to - do nothing (stay open) - return; - } - // If our current level is less than target, propagate up (this panel should stay open) - else { - onBreadcrumbClick?.(targetLevel); - } - }, - [breadcrumbPath?.length, onBreadcrumbClick, onClose], - ); + const parentContext = useContext(RowSidePanelContext); + // Nested rows shouldn't inherit the parent table's row config. + const dbSqlRowTableConfig = + sourceStack.length > 0 || navStack.length > 0 + ? undefined + : parentContext.dbSqlRowTableConfig; const hasOverviewPanel = useMemo(() => { if (isLogSource(source) || isTraceSource(source)) { @@ -178,30 +323,151 @@ const DBRowSidePanel = ({ return false; }, [source]); - const defaultTab = - source.kind === 'trace' - ? Tab.Trace - : hasOverviewPanel - ? Tab.Overview - : Tab.Parsed; + const sourceIsTrace = source.kind === SourceKind.Trace; + + const defaultTab = sourceIsTrace + ? Tab.Trace + : hasOverviewPanel + ? Tab.Overview + : Tab.Parsed; const [queryTab, setQueryTab] = useQueryState( 'sidePanelTab', parseAsStringEnum(Object.values(Tab)).withDefault(defaultTab), ); - const [stateTab, setStateTab] = useState(defaultTab); - // Nested panels can't share the query param or else they'll conflict, so we'll use local state for nested panels - // We'll need to handle this properly eventually... - const tab = isNestedPanel ? stateTab : queryTab; - const setTab = isNestedPanel ? setStateTab : setQueryTab; + const handleNavigateToRow = useCallback( + ( + rowId: string, + aliasWith: WithClause[], + label: string, + sourceKind?: SourceKind, + ) => { + setNavStack(prev => [ + ...prev, + { rowId, aliasWith, label, sourceKind, originTab: queryTab }, + ]); + }, + [setNavStack, queryTab], + ); - const displayedTab = tab; + const handleSourceStackPush = useCallback( + (frame: SourceFrame) => { + setSourceStack(prev => [...prev, { ...frame, originTab: queryTab }]); + setNavStack([]); + }, + [setSourceStack, setNavStack, queryTab], + ); + + const handlePanelBack = useCallback(() => { + if (navStack.length > 0) { + // Returning from a same-source drilldown — restore the tab the user was + // on before drilling into the row we're leaving (e.g. back to + // Surrounding Context after viewing a related row). + const restoreTab = navStack[navStack.length - 1]?.originTab; + setNavStack(prev => prev.slice(0, -1)); + if (restoreTab) { + setQueryTab(restoreTab); + } + } else if (sourceStack.length > 0) { + // Returning from a cross-source drilldown (e.g. "View Trace") — restore + // the tab the user was on before pushing that source. + const restoreTab = sourceStack[sourceStack.length - 1]?.originTab; + setSourceStack(prev => prev.slice(0, -1)); + setNavStack([]); + if (restoreTab) { + setQueryTab(restoreTab); + } + } else if (onNavigateToParent) { + onNavigateToParent(); + } else { + onClose(); + } + }, [ + navStack, + sourceStack, + onNavigateToParent, + onClose, + setNavStack, + setSourceStack, + setQueryTab, + ]); + + // Esc pops one level (nav → source → parent → close), mirroring the Back + // button. Disabled when embedded (e.g. in SessionSidePanel), where the + // parent owns Esc — otherwise both handlers fire on a single keypress. + useHotkeys(['esc'], handlePanelBack, { enabled: !onNavigateToParent }); + + const handleBreadcrumbNavigation = useCallback( + (sourceLevel: number, navLevel: number) => { + // Restore the tab that was active at the level we're returning to: the + // first source frame being dropped, or — if we're staying within the same + // source — the first nav entry being dropped. + let restoreTab: Tab | undefined; + if (sourceLevel < sourceStack.length) { + restoreTab = sourceStack[sourceLevel]?.originTab; + } else if (navLevel < navStack.length) { + restoreTab = navStack[navLevel]?.originTab; + } + setSourceStack(prev => prev.slice(0, sourceLevel)); + setNavStack(prev => prev.slice(0, navLevel)); + if (restoreTab) { + setQueryTab(restoreTab); + } + }, + [setSourceStack, setNavStack, navStack, sourceStack, setQueryTab], + ); + + // Jump to the destination's default tab when a frame is *pushed* onto either + // stack (e.g. push a trace source → Trace). The refs are seeded to the + // *initial* (possibly URL-restored) stack lengths so this does not clobber a + // `sidePanelTab` value present in the URL on first mount. + const prevSourceStackLengthRef = useRef(sourceStack.length); + const prevNavStackLengthRef = useRef(navStack.length); + useEffect(() => { + const sourcePushed = sourceStack.length > prevSourceStackLengthRef.current; + const navPushed = navStack.length > prevNavStackLengthRef.current; + + // Only *pushes* jump to the destination's default tab. Pops and breadcrumb + // truncations are handled by the navigation handlers, which restore the + // tab the user was on before drilling in (see `originTab`). + if (sourcePushed) { + const leafKind = sourceStack[sourceStack.length - 1].sourceKind; + setQueryTab(leafKind === SourceKind.Trace ? Tab.Trace : Tab.Overview); + } else if (navPushed) { + const navDefault = sourceIsTrace + ? Tab.Trace + : hasOverviewPanel + ? Tab.Overview + : Tab.Parsed; + setQueryTab(navDefault); + } + + prevSourceStackLengthRef.current = sourceStack.length; + prevNavStackLengthRef.current = navStack.length; + }, [sourceStack, navStack, setQueryTab, sourceIsTrace, hasOverviewPanel]); + + // Reset to the default tab and clear drilldowns when a *different* root event + // is opened. Skipped on first mount so URL-restored state survives. + const prevInitialRowIdRef = useRef(undefined); + useEffect(() => { + if ( + prevInitialRowIdRef.current !== undefined && + initialRowId !== prevInitialRowIdRef.current + ) { + setSourceStack([]); + setNavStack([]); + setQueryTab(null); + } + prevInitialRowIdRef.current = initialRowId; + }, [initialRowId, setSourceStack, setNavStack, setQueryTab]); + + const displayedTab = queryTab; + const setTab = setQueryTab; const normalizedRow = rowData?.data?.[0]; const timestampValue = normalizedRow?.['__hdx_timestamp']; - // TODO: Improve parsing let timestampDate: Date; if (typeof timestampValue === 'number') { timestampDate = new Date(timestampValue * 1000); @@ -218,6 +484,23 @@ const DBRowSidePanel = ({ const severityText: string | undefined = normalizedRow?.['__hdx_severity_text']; + // Capture the root event body once for the root breadcrumb label. + const [initialMainContent, setInitialMainContent] = useState< + string | undefined + >(undefined); + + useEffect(() => { + if ( + mainContent != null && + initialMainContent == null && + sourceStack.length === 0 && + navStack.length === 0 + ) { + // eslint-disable-next-line react-hooks/set-state-in-effect + setInitialMainContent(mainContent); + } + }, [mainContent, initialMainContent, sourceStack.length, navStack.length]); + const highlightedAttributeValues = useMemo(() => { const attributeExpressions: NonNullable< (TLogSource | TTraceSource)['highlightedRowAttributeExpressions'] @@ -266,7 +549,10 @@ const DBRowSidePanel = ({ }, [timestampDate]); const focusDate = timestampDate; - const traceId: string | undefined = normalizedRow?.['__hdx_trace_id']; + // Coerce empty / falsy trace ids to undefined so "View Trace" / Trace ID + // is hidden for logs without trace context. + const traceId: string | undefined = + normalizedRow?.['__hdx_trace_id'] || undefined; const childSourceId = isLogSource(source) ? source.traceSourceId @@ -278,17 +564,46 @@ const DBRowSidePanel = ({ ? source.id : isLogSource(source) ? source.traceSourceId - : isSessionSource(source) + : source.kind === SourceKind.Session ? source.traceSourceId : undefined; const enableServiceMap = traceId && traceSourceId; + const { data: traceSourceData } = useSource({ id: traceSourceId }); + + const spanId = normalizedRow?.['__hdx_span_id']; + const spanIdExpression = + traceSourceData?.kind === SourceKind.Log || + traceSourceData?.kind === SourceKind.Trace + ? traceSourceData.spanIdExpression + : undefined; + + const traceSpanRowId = useMemo(() => { + if (!spanIdExpression || !spanId) return undefined; + return SqlString.format('?=?', [SqlString.raw(spanIdExpression), spanId]); + }, [spanIdExpression, spanId]); + + const handleSessionEventNavigate = useCallback( + (rowId: string, aliasWith: WithClause[]) => { + if (traceSourceData) { + handleSourceStackPush({ + sourceId: traceSourceData.id, + rowId, + aliasWith, + label: mainContent || 'Session Replay', + sourceKind: traceSourceData.kind as SourceKind, + }); + } + }, + [traceSourceData, handleSourceStackPush, mainContent], + ); + const { rumSessionId, rumServiceName } = useSessionId({ sourceId: traceSourceId, traceId, dateRange: oneHourRange, - enabled: rowId != null, + enabled: activeRowId != null, }); const hasK8sContext = useMemo( @@ -306,7 +621,85 @@ const DBRowSidePanel = ({ } }, [normalizedRow]); - if (isRowLoading) { + const durationMs = normalizedRow?.[ROW_DATA_ALIASES.DURATION_MS]; + const spanKind = normalizedRow?.[ROW_DATA_ALIASES.SPAN_KIND]; + const serviceName = normalizedRow?.[ROW_DATA_ALIASES.SERVICE_NAME]; + const statusCode = normalizedRow?.[ROW_DATA_ALIASES.SEVERITY_TEXT]; + + const formattedDuration = useMemo(() => { + if (durationMs == null || isNaN(Number(durationMs))) return undefined; + return renderMs(Number(durationMs)); + }, [durationMs]); + + const spanKindLabel = useMemo(() => { + if (spanKind == null) return undefined; + return SPAN_KIND_LABELS[String(spanKind)] ?? String(spanKind); + }, [spanKind]); + + const allBreadcrumbs = useMemo((): BreadcrumbItem[] => { + const items: BreadcrumbItem[] = []; + + if (parentBreadcrumbs) { + items.push(...parentBreadcrumbs); + } + + const hasStack = sourceStack.length > 0 || navStack.length > 0; + const rootLabel = + initialMainContent || + (rootSource.kind === SourceKind.Trace ? 'Trace' : 'Log'); + + if (hasStack) { + items.push({ + label: rootLabel, + sourceKind: rootSource.kind as SourceKind, + onClick: () => handleBreadcrumbNavigation(0, 0), + }); + } + + sourceStack.forEach((entry, i) => { + const isLeafSource = i === sourceStack.length - 1; + const isCurrent = isLeafSource && navStack.length === 0; + items.push({ + label: entry.label, + sourceKind: entry.sourceKind, + onClick: isCurrent + ? undefined + : () => handleBreadcrumbNavigation(i + 1, 0), + }); + }); + + navStack.forEach((entry, i) => { + const isCurrent = i === navStack.length - 1; + items.push({ + label: entry.label, + sourceKind: entry.sourceKind, + onClick: isCurrent + ? undefined + : () => handleBreadcrumbNavigation(sourceStack.length, i + 1), + }); + }); + + if (!hasStack) { + items.push({ + label: mainContent || (sourceIsTrace ? 'Trace' : 'Log'), + sourceKind: source.kind as SourceKind, + }); + } + + return items; + }, [ + sourceStack, + navStack, + rootSource.kind, + sourceIsTrace, + mainContent, + initialMainContent, + source.kind, + handleBreadcrumbNavigation, + parentBreadcrumbs, + ]); + + if (isRowLoading || isResolvingSource) { return
Loading...
; } @@ -321,29 +714,145 @@ const DBRowSidePanel = ({ return
Error loading row data
; } + const showLogTraceActions = !sourceIsTrace && traceId && traceSourceId; + return ( <> - + + + + + + + {!sourceIsTrace && severityText && } + {timestampDate && !isNaN(timestampDate.getTime()) && ( + + ·{' '} + {formatDistanceToNowStrictShort(timestampDate)} ago + + )} + {serviceName && ( + <> + + · + + + + Service + + + {serviceName} + + + + )} + {sourceIsTrace && formattedDuration && ( + <> + + · + + + + Duration + + + {formattedDuration} + + + + )} + {sourceIsTrace && statusCode && ( + <> + + · + + + + Status + + + {statusCode} + + + + )} + {sourceIsTrace && spanKindLabel && ( + + {spanKindLabel} + + )} + {(sourceIsTrace ? traceId : showLogTraceActions) && ( + <> + + · + + + {({ copied, copy }) => ( + + + + + Trace ID + + + + )} + + + )} + {showLogTraceActions && ( + + )} + - {/* */} )} - {displayedTab === Tab.Trace && ( + {displayedTab === Tab.Trace && sourceIsTrace && ( { console.error(err); @@ -474,8 +987,8 @@ const DBRowSidePanel = ({ )} @@ -495,9 +1008,8 @@ const DBRowSidePanel = ({ source={source} dbSqlRowTableConfig={dbSqlRowTableConfig} rowData={normalizedRow} - rowId={rowId} - breadcrumbPath={breadcrumbPath} - onBreadcrumbClick={handleBreadcrumbClick} + rowId={activeRowId} + onNavigateToRow={handleNavigateToRow} /> )} @@ -519,7 +1031,7 @@ const DBRowSidePanel = ({ (Object.values(Tab)), ); + const [, setSourceStackParam] = useQueryState( + 'sidePanelSourceStack', + parseAsJsonEncoded(), + ); + const [, setNavStackParam] = useQueryState( + 'sidePanelNavStack', + parseAsJsonEncoded(), + ); const { clear: clearTraceWaterfallSearchState } = useWaterfallSearchState({}); const _onClose = useCallback(() => { - // Reset tab to undefined when unmounting, so that when we open the drawer again, it doesn't open to the last tab - // (which might not be valid, ex session replay) - if (!isNestedPanel) { - setQueryTab(null); - } + // Reset the tab and navigation stacks so re-opening the drawer starts fresh. + setSidePanelTab(null); + setSourceStackParam(null); + setNavStackParam(null); // Clear waterfall search state on close, so that filters don't // persist when reopening another trace. clearTraceWaterfallSearchState(); onClose(); - }, [setQueryTab, isNestedPanel, onClose, clearTraceWaterfallSearchState]); - - useHotkeys(['esc'], _onClose, { enabled: subDrawerOpen === false }); + }, [ + setSidePanelTab, + setSourceStackParam, + setNavStackParam, + onClose, + clearTraceWaterfallSearchState, + ]); return ( { - if (!subDrawerOpen) { - _onClose(); - } - }} + closeOnEscape={false} + onClose={_onClose} position="right" size={`${size}vw`} styles={{ + content: { + border: 'none', + boxShadow: 'var(--shadow-drawer)', + }, body: { padding: '0', height: '100%', @@ -631,17 +1149,13 @@ export default function DBRowSidePanelErrorBoundary({ )} > -
diff --git a/packages/app/src/components/DBRowSidePanelHeader.tsx b/packages/app/src/components/DBRowSidePanelHeader.tsx index b37a1f383a..a253d71fe9 100644 --- a/packages/app/src/components/DBRowSidePanelHeader.tsx +++ b/packages/app/src/components/DBRowSidePanelHeader.tsx @@ -1,135 +1,20 @@ -import React, { - useCallback, - useContext, - useEffect, - useMemo, - useState, -} from 'react'; -import { - ActionIcon, - Box, - Breadcrumbs, - Button, - Flex, - Group, - Paper, - Text, - Tooltip, - UnstyledButton, -} from '@mantine/core'; +import React, { useCallback, useEffect, useMemo, useState } from 'react'; +import { Box, Button, Flex, Paper, Text } from '@mantine/core'; import { IconArrowsDiagonal, IconArrowsDiagonalMinimize2, - IconKeyboard, } from '@tabler/icons-react'; -import { KeyboardShortcutsModal } from '@/LogSidePanelElements'; -import { FormatTime } from '@/useFormatTime'; import { useUserPreferences } from '@/useUserPreferences'; -import { formatDistanceToNowStrictShort } from '@/utils'; import AISummarizeButton from './AISummarizeButton'; import { DBHighlightedAttributesList, HighlightedAttribute, } from './DBHighlightedAttributesList'; -import { RowSidePanelContext } from './DBRowSidePanel'; -import { DrawerFullWidthToggle } from './DrawerUtils'; -import LogLevel from './LogLevel'; - -const isValidDate = (date: Date) => 'getTime' in date && !isNaN(date.getTime()); const MAX_MAIN_CONTENT_LENGTH = 2000; -// Types for breadcrumb navigation -export type BreadcrumbEntry = { - label: string; - rowData?: Record; -}; - -export type BreadcrumbPath = BreadcrumbEntry[]; - -// Navigation callback type - called when user wants to navigate to a specific level -export type BreadcrumbNavigationCallback = (targetLevel: number) => void; - -function getBodyTextForBreadcrumb(rowData: Record): string { - const bodyText = (rowData.__hdx_body || '').trim(); - const BREADCRUMB_TOOLTIP_MAX_LENGTH = 200; - const BREADCRUMB_TOOLTIP_TRUNCATED_LENGTH = 197; - - return bodyText.length > BREADCRUMB_TOOLTIP_MAX_LENGTH - ? `${bodyText.substring(0, BREADCRUMB_TOOLTIP_TRUNCATED_LENGTH)}...` - : bodyText; -} - -function BreadcrumbNavigation({ - breadcrumbPath, - onNavigateToLevel, -}: { - breadcrumbPath: BreadcrumbPath; - onNavigateToLevel?: BreadcrumbNavigationCallback; -}) { - const handleBreadcrumbItemClick = useCallback( - (clickedIndex: number) => { - // Navigate to the clicked breadcrumb level - // This will close all panels above this level - onNavigateToLevel?.(clickedIndex); - }, - [onNavigateToLevel], - ); - - const breadcrumbItems = useMemo(() => { - if (breadcrumbPath.length === 0) return []; - - const items = []; - - // Add all previous levels from breadcrumbPath - breadcrumbPath.forEach((crumb, index) => { - const tooltipText = crumb.rowData - ? getBodyTextForBreadcrumb(crumb.rowData) - : ''; - - items.push( - - handleBreadcrumbItemClick(index)} - style={{ textDecoration: 'none' }} - > - - {index === 0 ? 'Original Event' : crumb.label} - - - , - ); - }); - - // Add current level - items.push( - - Selected Event - , - ); - - return items; - }, [breadcrumbPath, handleBreadcrumbItemClick]); - - if (breadcrumbPath.length === 0) return null; - - return ( - - - {breadcrumbItems} - - - ); -} - export default function DBRowSidePanelHeader({ attributes, mainContent = '', @@ -137,32 +22,19 @@ export default function DBRowSidePanelHeader({ // When `true`, the source has a body column configured. An empty value // for that column renders a soft empty-state paper. When `false` (the // source has neither body nor implicit column configured), the body - // paper is suppressed entirely; the header still shows timestamp, - // severity, and highlighted attributes. + // paper is suppressed entirely; the highlighted attributes still render. bodyConfigured = true, - date, severityText, rowData, - breadcrumbPath, - onBreadcrumbClick, - isFullWidth, - onToggleFullWidth, }: { - date: Date; mainContent?: string; mainContentHeader?: string; bodyConfigured?: boolean; attributes?: HighlightedAttribute[]; severityText?: string; rowData?: Record; - breadcrumbPath?: BreadcrumbPath; - onBreadcrumbClick?: BreadcrumbNavigationCallback; - isFullWidth?: boolean; - onToggleFullWidth?: () => void; }) { const [bodyExpanded, setBodyExpanded] = React.useState(false); - const [shortcutsOpen, setShortcutsOpen] = useState(false); - const { generateSearchUrl } = useContext(RowSidePanelContext); const isContentTruncated = mainContent.length > MAX_MAIN_CONTENT_LENGTH; const mainContentDisplayed = React.useMemo( @@ -200,74 +72,19 @@ export default function DBRowSidePanelHeader({ const { expandSidebarHeader } = userPreferences; const maxBoxHeight = 120; - const _generateSearchUrl = useCallback( - (query?: string, queryLanguage?: 'sql' | 'lucene') => { - return ( - generateSearchUrl?.({ - where: query, - whereLanguage: queryLanguage, - }) ?? '/' - ); - }, - [generateSearchUrl], - ); - - const breadCrumbPathWithDefault = useMemo(() => { - return breadcrumbPath ?? []; - }, [breadcrumbPath]); - const attributesWithDefault = useMemo(() => { return attributes ?? []; }, [attributes]); + const toggleExpandSidebarHeader = useCallback(() => { + setUserPreference({ + ...userPreferences, + expandSidebarHeader: !expandSidebarHeader, + }); + }, [expandSidebarHeader, setUserPreference, userPreferences]); + return ( <> - {/* Breadcrumb navigation */} - - - {/* Event timestamp and severity */} - - - {severityText && } - {severityText && isValidDate(date) && ( - - · - - )} - {isValidDate(date) && ( - - ·{' '} - {formatDistanceToNowStrictShort(date)} ago - - )} - - - - setShortcutsOpen(true)} - aria-label="Keyboard shortcuts" - > - - - - {onToggleFullWidth && ( - - )} - - - setShortcutsOpen(false)} - /> {!bodyConfigured ? null : mainContent ? ( - setUserPreference({ - ...userPreferences, - expandSidebarHeader: !expandSidebarHeader, - }) - } + onClick={toggleExpandSidebarHeader} > - {/* TODO: Only show expand button when maxHeight = 120? */} {expandSidebarHeader ? ( ) : ( diff --git a/packages/app/src/components/DBRowTable.tsx b/packages/app/src/components/DBRowTable.tsx index 89b1ae768b..4aa39c5b76 100644 --- a/packages/app/src/components/DBRowTable.tsx +++ b/packages/app/src/components/DBRowTable.tsx @@ -1524,7 +1524,10 @@ function DBSqlRowTableComponent({ }: { config: BuilderChartConfigWithDateRange; sourceId?: string; - onRowDetailsClick?: (rowWhere: RowWhereResult) => void; + onRowDetailsClick?: ( + rowWhere: RowWhereResult, + row: Record, + ) => void; highlightedLineId?: string; queryKeyPrefix?: string; enabled?: boolean; @@ -1707,7 +1710,7 @@ function DBSqlRowTableComponent({ const _onRowDetailsClick = useCallback( (row: Record) => { - return onRowDetailsClick?.(getRowWhere(row)); + return onRowDetailsClick?.(getRowWhere(row), row); }, [onRowDetailsClick, getRowWhere], ); diff --git a/packages/app/src/components/DBSessionPanel.tsx b/packages/app/src/components/DBSessionPanel.tsx index b248fd35f3..c9d84a20e2 100644 --- a/packages/app/src/components/DBSessionPanel.tsx +++ b/packages/app/src/components/DBSessionPanel.tsx @@ -4,6 +4,7 @@ import { isTraceSource, SourceKind } from '@hyperdx/common-utils/dist/types'; import { Loader } from '@mantine/core'; import useFieldExpressionGenerator from '@/hooks/useFieldExpressionGenerator'; +import { WithClause } from '@/hooks/useRowWhere'; import SessionSubpanel from '@/SessionSubpanel'; import { useSource } from '@/source'; @@ -93,14 +94,14 @@ export const DBSessionPanel = ({ dateRange, focusDate, serviceName, - setSubDrawerOpen, + onEventNavigate, }: { traceSourceId?: string; rumSessionId: string; dateRange: [Date, Date]; focusDate: Date; serviceName: string; - setSubDrawerOpen: (open: boolean) => void; + onEventNavigate?: (rowId: string, aliasWith: WithClause[]) => void; }) => { const { data: traceSource } = useSource({ id: traceSourceId, @@ -136,7 +137,7 @@ export const DBSessionPanel = ({ session={{ serviceName }} sessionSource={sessionSource} rumSessionId={rumSessionId} - setDrawerOpen={setSubDrawerOpen} + onEventNavigate={onEventNavigate} initialTs={focusDate.getTime()} /> ) : ( diff --git a/packages/app/src/components/DBSqlRowTableWithSidebar.tsx b/packages/app/src/components/DBSqlRowTableWithSidebar.tsx index b197486669..b1a4ab6983 100644 --- a/packages/app/src/components/DBSqlRowTableWithSidebar.tsx +++ b/packages/app/src/components/DBSqlRowTableWithSidebar.tsx @@ -17,14 +17,12 @@ import { useLocalStorage } from '@/utils'; import { parseAsStringEncoded } from '@/utils/queryParsers'; import { ChartErrorStateVariant } from './charts/ChartErrorState'; -import { useNestedPanelState } from './ContextSidePanel'; import { RowDataPanel } from './DBRowDataPanel'; import { RowOverviewPanel } from './DBRowOverviewPanel'; import DBRowSidePanel, { RowSidePanelContext, RowSidePanelContextProps, } from './DBRowSidePanel'; -import { BreadcrumbEntry } from './DBRowSidePanelHeader'; import { DBRowTableVariant, DBSqlRowTable } from './DBRowTable'; interface Props { @@ -41,8 +39,6 @@ interface Props { queryKeyPrefix?: string; denoiseResults?: boolean; collapseAllRows?: boolean; - isNestedPanel?: boolean; - breadcrumbPath?: BreadcrumbEntry[]; onSortingChange?: (v: SortingState | null) => void; initialSortBy?: SortingState; variant?: DBRowTableVariant; @@ -63,8 +59,6 @@ export default function DBSqlRowTableWithSideBar({ collapseAllRows, isLive, enabled, - isNestedPanel, - breadcrumbPath, onSidebarOpen, onSortingChange, initialSortBy, @@ -78,7 +72,6 @@ export default function DBSqlRowTableWithSideBar({ const [rowId, setRowId] = useQueryState('rowWhere', parseAsStringEncoded); const [rowSource, setRowSource] = useQueryState('rowSource'); const [aliasWith, setAliasWith] = useState([]); - const { setContextRowId, setContextRowSource } = useNestedPanelState(); const onOpenSidebar = useCallback( (rowWhere: RowWhereResult) => { @@ -93,19 +86,7 @@ export default function DBSqlRowTableWithSideBar({ const onCloseSidebar = useCallback(() => { setRowId(null); setRowSource(null); - // When closing the main drawer, clear the nested panel state - // this ensures that re-opening the main drawer will not open the nested panel - if (!isNestedPanel) { - setContextRowId(null); - setContextRowSource(null); - } - }, [ - setRowId, - setRowSource, - isNestedPanel, - setContextRowId, - setContextRowSource, - ]); + }, [setRowId, setRowSource]); const renderRowDetails = useCallback( (r: { id: string; aliasWith?: WithClause[]; [key: string]: unknown }) => { if (!sourceData) { @@ -129,8 +110,6 @@ export default function DBSqlRowTableWithSideBar({ source={sourceData} rowId={rowId ?? undefined} aliasWith={aliasWith} - isNestedPanel={isNestedPanel} - breadcrumbPath={breadcrumbPath} onClose={onCloseSidebar} /> )} diff --git a/packages/app/src/components/PatternSidePanel.tsx b/packages/app/src/components/PatternSidePanel.tsx index 994437c78b..2982ba2b35 100644 --- a/packages/app/src/components/PatternSidePanel.tsx +++ b/packages/app/src/components/PatternSidePanel.tsx @@ -176,8 +176,6 @@ export default function PatternSidePanel({ rowId={selectedRowWhere.where} aliasWith={selectedRowWhere.aliasWith} onClose={handleCloseRowSidePanel} - isNestedPanel={true} - breadcrumbPath={[{ label: 'Pattern Overview' }]} /> )}
diff --git a/packages/app/src/components/ServiceDashboardSlowestEventsTile.tsx b/packages/app/src/components/ServiceDashboardSlowestEventsTile.tsx index 32e8823079..934d8e5f4f 100644 --- a/packages/app/src/components/ServiceDashboardSlowestEventsTile.tsx +++ b/packages/app/src/components/ServiceDashboardSlowestEventsTile.tsx @@ -86,8 +86,6 @@ export default function SlowestEventsTile({ expressions && ( <> void; +}; + +function SourceIcon({ kind }: { kind?: SourceKind }) { + if (kind === SourceKind.Trace) { + return ; + } + if (kind === SourceKind.Log) { + return ; + } + if (kind === SourceKind.Session) { + return ; + } + return null; +} + +function truncate(text: string, max: number) { + if (text.length <= max) return text; + return `${text.slice(0, max)}…`; +} + +function SidePanelBreadcrumbs({ + items, + onBack, +}: { + items: BreadcrumbItem[]; + onBack: () => void; +}) { + const breadcrumbElements = useMemo(() => { + return items.map((item, i) => { + const isLast = i === items.length - 1; + const isSingle = items.length === 1; + const maxLen = isSingle + ? MAX_LABEL_LENGTH_SINGLE + : isLast + ? MAX_LABEL_LENGTH_CURRENT + : MAX_LABEL_LENGTH_PREVIOUS; + const truncatedLabel = truncate(item.label, maxLen); + const needsTooltip = item.label.length > maxLen; + + const content = ( + + {i === 0 && } + + {truncatedLabel} + + + ); + + const wrapped = needsTooltip ? ( + {item.label}} + position="bottom" + multiline + maw={400} + > + {content} + + ) : ( + content + ); + + if (isLast || !item.onClick) { + return ( + + {wrapped} + + ); + } + + return ( + + + {wrapped} + + + ); + }); + }, [items]); + + return ( + + + + + + + {items.length > 0 ? ( + + {breadcrumbElements} + + ) : null} + + ); +} + +export default memo(SidePanelBreadcrumbs); diff --git a/packages/app/src/components/__tests__/DBRowSidePanelHeader.test.tsx b/packages/app/src/components/__tests__/DBRowSidePanelHeader.test.tsx index 939a1f62d9..0bd32b7f3c 100644 --- a/packages/app/src/components/__tests__/DBRowSidePanelHeader.test.tsx +++ b/packages/app/src/components/__tests__/DBRowSidePanelHeader.test.tsx @@ -42,7 +42,6 @@ describe('DBRowSidePanelHeader: body section (HDX-4373)', () => { it('renders the configured body content when bodyConfigured + mainContent are truthy', () => { renderWithMantine( { it('renders the softened empty state when body is configured but value is empty', () => { renderWithMantine( { it('suppresses the body paper entirely when body is not configured on the source', () => { renderWithMantine( { }); it('defaults bodyConfigured to true (back-compat with callers that do not pass it)', () => { - renderWithMantine( - , - ); + renderWithMantine(); expect(screen.queryByText('No body for this event.')).toBeInTheDocument(); }); }); diff --git a/packages/app/styles/LogSidePanel.module.scss b/packages/app/styles/LogSidePanel.module.scss index e899b6b223..6a3e3d1c43 100644 --- a/packages/app/styles/LogSidePanel.module.scss +++ b/packages/app/styles/LogSidePanel.module.scss @@ -4,7 +4,6 @@ font-size: 12px; height: 100%; background: var(--color-bg-body); - box-shadow: 0 0 100px 40px rgb(0 0 0 / 70%); border-left: 1px solid var(--color-border); } diff --git a/packages/app/tests/e2e/features/search/search.spec.ts b/packages/app/tests/e2e/features/search/search.spec.ts index 5ca57692ff..64b31caf27 100644 --- a/packages/app/tests/e2e/features/search/search.spec.ts +++ b/packages/app/tests/e2e/features/search/search.spec.ts @@ -53,7 +53,7 @@ test.describe('Search', { tag: '@search' }, () => { }); await test.step('Navigate through all side panel tabs', async () => { - const tabs = ['parsed', 'trace', 'context', 'overview']; + const tabs = ['parsed', 'context', 'overview']; // Use side panel component to navigate tabs for (const tabName of tabs) { From b19a7a54edd0100efe8459a25bac2e565040fa11 Mon Sep 17 00:00:00 2001 From: Karl Power Date: Mon, 29 Jun 2026 17:39:40 +0200 Subject: [PATCH 2/8] greptile feedback, update e2e --- .../app/src/components/DBRowSidePanel.tsx | 33 ++++++++--- .../tests/e2e/features/search/search.spec.ts | 14 +++-- .../app/tests/e2e/features/sessions.spec.ts | 57 ++++++++++--------- .../tests/e2e/page-objects/SessionsPage.ts | 19 ------- 4 files changed, 64 insertions(+), 59 deletions(-) diff --git a/packages/app/src/components/DBRowSidePanel.tsx b/packages/app/src/components/DBRowSidePanel.tsx index 38ba1eaa8f..303104e059 100644 --- a/packages/app/src/components/DBRowSidePanel.tsx +++ b/packages/app/src/components/DBRowSidePanel.tsx @@ -448,13 +448,13 @@ export const DBRowSidePanelInner = ({ }, [sourceStack, navStack, setQueryTab, sourceIsTrace, hasOverviewPanel]); // Reset to the default tab and clear drilldowns when a *different* root event - // is opened. Skipped on first mount so URL-restored state survives. - const prevInitialRowIdRef = useRef(undefined); + // is opened. Seeded with the first-render rowId so a genuine deep-link (rowId + // + stacks both restored from the URL) is preserved, while a later change of + // the *root* rowId (e.g. undefined → clicked row, or switching to a different + // event) clears stale drilldown stacks. + const prevInitialRowIdRef = useRef(initialRowId); useEffect(() => { - if ( - prevInitialRowIdRef.current !== undefined && - initialRowId !== prevInitialRowIdRef.current - ) { + if (initialRowId !== prevInitialRowIdRef.current) { setSourceStack([]); setNavStack([]); setQueryTab(null); @@ -573,6 +573,11 @@ export const DBRowSidePanelInner = ({ const { data: traceSourceData } = useSource({ id: traceSourceId }); const spanId = normalizedRow?.['__hdx_span_id']; + const traceIdExpression = + traceSourceData?.kind === SourceKind.Log || + traceSourceData?.kind === SourceKind.Trace + ? traceSourceData.traceIdExpression + : undefined; const spanIdExpression = traceSourceData?.kind === SourceKind.Log || traceSourceData?.kind === SourceKind.Trace @@ -580,9 +585,19 @@ export const DBRowSidePanelInner = ({ : undefined; const traceSpanRowId = useMemo(() => { - if (!spanIdExpression || !spanId) return undefined; - return SqlString.format('?=?', [SqlString.raw(spanIdExpression), spanId]); - }, [spanIdExpression, spanId]); + const clauses: string[] = []; + if (traceIdExpression && traceId) { + clauses.push( + SqlString.format('?=?', [SqlString.raw(traceIdExpression), traceId]), + ); + } + if (spanIdExpression && spanId) { + clauses.push( + SqlString.format('?=?', [SqlString.raw(spanIdExpression), spanId]), + ); + } + return clauses.length > 0 ? clauses.join(' AND ') : undefined; + }, [traceIdExpression, traceId, spanIdExpression, spanId]); const handleSessionEventNavigate = useCallback( (rowId: string, aliasWith: WithClause[]) => { diff --git a/packages/app/tests/e2e/features/search/search.spec.ts b/packages/app/tests/e2e/features/search/search.spec.ts index 64b31caf27..3c282ca1a6 100644 --- a/packages/app/tests/e2e/features/search/search.spec.ts +++ b/packages/app/tests/e2e/features/search/search.spec.ts @@ -104,7 +104,11 @@ test.describe('Search', { tag: '@search' }, () => { }); await test.step('Navigate through all side panel tabs', async () => { - const tabs = ['trace', 'context', 'infrastructure', 'overview']; + // Logs sources no longer render a Trace tab (it's gated behind + // source.kind === Trace). For a Kubernetes log row the available tabs + // are Surrounding Context (always present), Infrastructure (k8s + // context) and Overview. + const tabs = ['context', 'infrastructure', 'overview']; // Use side panel component with proper waiting for (const tabName of tabs) { @@ -154,10 +158,10 @@ test.describe('Search', { tag: '@search' }, () => { }); await test.step('Infrastructure tab is not offered', async () => { - // The tab bar rendered (an always-present tab is visible), but the gate - // omits Infrastructure because the row carries no k8s correlation - // attributes. - await expect(searchPage.sidePanel.getTab('trace')).toBeVisible(); + // The tab bar rendered (the always-present Column Values tab is + // visible), but the gate omits Infrastructure because the row carries + // no k8s correlation attributes. + await expect(searchPage.sidePanel.getTab('parsed')).toBeVisible(); await expect(searchPage.sidePanel.getTab('infrastructure')).toHaveCount( 0, ); diff --git a/packages/app/tests/e2e/features/sessions.spec.ts b/packages/app/tests/e2e/features/sessions.spec.ts index aec7b09f9a..8386a5c2ef 100644 --- a/packages/app/tests/e2e/features/sessions.spec.ts +++ b/packages/app/tests/e2e/features/sessions.spec.ts @@ -46,8 +46,9 @@ test.describe('Client Sessions Functionality', { tag: ['@sessions'] }, () => { async ({ page }) => { await test.step('Navigate and open a session (with sidePanelTab=replay pre-set in URL to simulate search-page flow)', async () => { // Pre-set sidePanelTab=replay in the URL to simulate navigating from a search page - // row detail panel that had the Session Replay tab open. Without isNestedPanel=true, - // the inner DBRowSidePanel would inherit this URL param and open to the Replay tab again. + // row detail panel that had the Session Replay tab open. Selecting a session event + // must clear this param (clearInnerNavigation) so the in-place event detail opens to + // its default tab instead of re-rendering the Session Replay tab. await page.goto('/search'); await sessionsPage.goto(); await sessionsPage.selectDataSource(); @@ -75,43 +76,47 @@ test.describe('Client Sessions Functionality', { tag: ['@sessions'] }, () => { await sessionsPage.clickFirstSessionEvent(); }); - await test.step('Event detail panel opens alongside the session replay — not replacing it', async () => { - // The row-side-panel must be visible (event detail drawer opened on top of session replay) - await expect(sessionsPage.rowSidePanel).toBeVisible(); - - // The original session replay panel must still be open (not replaced/closed) + await test.step('Event detail opens in place inside the single session drawer — not as a second drawer', async () => { + // Single-drawer navigation: the event detail renders in place inside the + // existing session-side-panel (via DBRowSidePanelInner). No second + // row-side-panel Drawer is opened. await expect(sessionsPage.sessionSidePanel).toBeVisible(); - // Only one session-side-panel must exist (not a second replay opened inside the detail panel) + // Exactly one session drawer exists, and no separate row-side-panel + // drawer was stacked on top of it. await expect(page.getByTestId('session-side-panel')).toHaveCount(1); + await expect(page.getByTestId('row-side-panel')).toHaveCount(0); - // The row-side-panel must show the event detail TabBar (Overview, Trace, etc.) - // This guards against the regression where the inner panel re-opened session replay - // instead of showing event details (which has no TabBar, just the replay player) + // The session drawer now shows the event detail TabBar (Overview, Trace, + // etc.). This guards against the regression where selecting an event + // re-opened session replay instead of showing event details (which has + // no TabBar, just the replay player). await expect( - sessionsPage.rowSidePanel.getByTestId('side-panel-tabs'), + sessionsPage.sessionSidePanel.getByTestId('side-panel-tabs'), ).toBeVisible(); - // The inner panel must NOT be showing the Session Replay tab content. - // Without isNestedPanel=true (broken), the inner DBRowSidePanel reads sidePanelTab=replay - // from the URL (injected above) and renders the Session Replay tab content (side-panel-tab-replay). - // With isNestedPanel=true (fixed), the inner panel uses local state and ignores the URL, - // opening to its default tab (Trace/Overview) instead. + // The event detail must NOT land on the Session Replay tab. Selecting an + // event clears the sidePanelTab=replay param injected above + // (clearInnerNavigation), so the inner panel opens to its default tab + // (Trace/Overview) and the replay tab content is not rendered. await expect( - sessionsPage.rowSidePanel.getByTestId('side-panel-tab-replay'), + sessionsPage.sessionSidePanel.getByTestId('side-panel-tab-replay'), ).toHaveCount(0); }); - await test.step('Clicking the overlay closes the event detail panel but keeps the session replay open', async () => { - // Without the fix, withOverlay={!isNestedPanel} removed the overlay on nested panels, - // so there was nothing to click to close the panel (it had to be ESC only). - // With the fix (withOverlay always true), clicking the Mantine overlay dismisses the inner panel. - await sessionsPage.clickTopmostDrawerOverlay(); + await test.step('Pressing Escape returns to the session event list within the same drawer', async () => { + // SessionSidePanel owns the Esc hotkey: when an event is selected it + // pops back to the session root (handleNavigateBack) instead of closing + // the whole drawer. + await page.keyboard.press('Escape'); - // The event detail panel must close - await expect(sessionsPage.rowSidePanel).toBeHidden(); + // The event detail TabBar collapses back to the session event list. + await expect( + sessionsPage.sessionSidePanel.getByTestId('side-panel-tabs'), + ).toBeHidden(); + await expect(sessionsPage.getSessionEventRows().first()).toBeVisible(); - // The session replay drawer must still be open + // The session drawer itself stays open. await expect(sessionsPage.sessionSidePanel).toBeVisible(); }); }, diff --git a/packages/app/tests/e2e/page-objects/SessionsPage.ts b/packages/app/tests/e2e/page-objects/SessionsPage.ts index 274f563a10..9d13088a63 100644 --- a/packages/app/tests/e2e/page-objects/SessionsPage.ts +++ b/packages/app/tests/e2e/page-objects/SessionsPage.ts @@ -89,25 +89,6 @@ export class SessionsPage { await this.getSessionEventRows().first().click(); } - /** - * Get the row side panel (event detail drawer opened from within session replay) - */ - get rowSidePanel() { - return this.page.getByTestId('row-side-panel'); - } - - /** - * Click the Mantine overlay of the topmost open drawer to close it. - * Mantine renders one overlay per open Drawer. The last one belongs to - * the innermost (topmost) drawer. - */ - async clickTopmostDrawerOverlay() { - // Mantine overlays are siblings of the drawer content inside the portal root. - // Use the last one since the inner panel's overlay is rendered on top. - const overlay = this.page.locator('.mantine-Drawer-overlay').last(); - await overlay.click({ position: { x: 10, y: 10 } }); - } - // Getters for assertions get form() { From c11d94ababf8b8754064663ac371067b89226ec6 Mon Sep 17 00:00:00 2001 From: Karl Power Date: Thu, 2 Jul 2026 12:20:45 +0200 Subject: [PATCH 3/8] background scroll when side panel drawer is open --- packages/app/src/SessionSidePanel.tsx | 3 +++ packages/app/src/components/DBRowSidePanel.tsx | 3 +++ packages/app/src/components/Search/DirectTraceSidePanel.tsx | 3 +++ 3 files changed, 9 insertions(+) diff --git a/packages/app/src/SessionSidePanel.tsx b/packages/app/src/SessionSidePanel.tsx index a94643290d..89676f77cb 100644 --- a/packages/app/src/SessionSidePanel.tsx +++ b/packages/app/src/SessionSidePanel.tsx @@ -171,6 +171,9 @@ export default function SessionSidePanel({ size={`${size}vw`} withCloseButton={false} closeOnEscape={false} + lockScroll={false} + withOverlay={false} + trapFocus={false} zIndex={zIndex} styles={{ content: { diff --git a/packages/app/src/components/DBRowSidePanel.tsx b/packages/app/src/components/DBRowSidePanel.tsx index 303104e059..8520c8c6da 100644 --- a/packages/app/src/components/DBRowSidePanel.tsx +++ b/packages/app/src/components/DBRowSidePanel.tsx @@ -1133,6 +1133,9 @@ export default function DBRowSidePanelErrorBoundary({ opened={rowId != null} withCloseButton={false} closeOnEscape={false} + lockScroll={false} + withOverlay={false} + trapFocus={false} onClose={_onClose} position="right" size={`${size}vw`} diff --git a/packages/app/src/components/Search/DirectTraceSidePanel.tsx b/packages/app/src/components/Search/DirectTraceSidePanel.tsx index 6e38791916..7add520964 100644 --- a/packages/app/src/components/Search/DirectTraceSidePanel.tsx +++ b/packages/app/src/components/Search/DirectTraceSidePanel.tsx @@ -94,6 +94,9 @@ export default function DirectTraceSidePanel({ onClose={onClose} position="right" size="75vw" + lockScroll={false} + withOverlay={false} + trapFocus={false} title={ From 53fda56024953c1fa07ef25b828d0d71650cb700 Mon Sep 17 00:00:00 2001 From: Karl Power Date: Fri, 3 Jul 2026 12:28:40 +0200 Subject: [PATCH 4/8] don't render Overview or Column Values tabs for traces --- packages/app/src/components/DBRowSidePanel.tsx | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/packages/app/src/components/DBRowSidePanel.tsx b/packages/app/src/components/DBRowSidePanel.tsx index 8520c8c6da..25f54ead3d 100644 --- a/packages/app/src/components/DBRowSidePanel.tsx +++ b/packages/app/src/components/DBRowSidePanel.tsx @@ -872,7 +872,7 @@ export const DBRowSidePanelInner = ({ data-testid="side-panel-tabs" className="fs-8 mt-2" items={[ - ...(hasOverviewPanel + ...(hasOverviewPanel && !sourceIsTrace ? [ { text: 'Overview', @@ -880,10 +880,14 @@ export const DBRowSidePanelInner = ({ }, ] : []), - { - text: 'Column Values', - value: Tab.Parsed, - }, + ...(!sourceIsTrace + ? [ + { + text: 'Column Values', + value: Tab.Parsed, + }, + ] + : []), ...(sourceIsTrace ? [ { From 344d6e3d18da8e4ab8db6854b15ac1d71853aeef Mon Sep 17 00:00:00 2001 From: Karl Power Date: Fri, 3 Jul 2026 16:21:45 +0200 Subject: [PATCH 5/8] extract side panel URL logic to useSidePanelStack --- .changeset/single-drawer-navigation.md | 2 +- packages/app/src/SessionSidePanel.tsx | 65 +++- packages/app/src/SessionsPage.tsx | 12 +- .../src/__tests__/SessionSidePanel.test.tsx | 74 +++- .../app/src/components/DBRowSidePanel.tsx | 318 ++++++++---------- .../src/components/DBRowSidePanel.types.ts | 38 +++ packages/app/src/components/DBTracePanel.tsx | 68 +++- .../DBRowSidePanel.missingSource.test.tsx | 260 ++++++++++++++ .../DBRowSidePanel.staleStack.test.tsx | 245 ++++++++++++++ .../__tests__/SidePanelBreadcrumbs.test.tsx | 72 ++++ .../__tests__/useSidePanelStack.test.tsx | 277 +++++++++++++++ packages/app/src/hooks/useRowWhere.tsx | 5 +- packages/app/src/hooks/useSidePanelStack.ts | 213 ++++++++++++ .../src/utils/__tests__/queryParsers.test.ts | 39 +++ packages/app/src/utils/queryParsers.ts | 29 +- .../app/tests/e2e/features/sessions.spec.ts | 8 +- packages/common-utils/src/types.ts | 40 +-- 17 files changed, 1520 insertions(+), 245 deletions(-) create mode 100644 packages/app/src/components/DBRowSidePanel.types.ts create mode 100644 packages/app/src/components/__tests__/DBRowSidePanel.missingSource.test.tsx create mode 100644 packages/app/src/components/__tests__/DBRowSidePanel.staleStack.test.tsx create mode 100644 packages/app/src/components/__tests__/SidePanelBreadcrumbs.test.tsx create mode 100644 packages/app/src/hooks/__tests__/useSidePanelStack.test.tsx create mode 100644 packages/app/src/hooks/useSidePanelStack.ts diff --git a/.changeset/single-drawer-navigation.md b/.changeset/single-drawer-navigation.md index f6f6995e14..2a1c21cf62 100644 --- a/.changeset/single-drawer-navigation.md +++ b/.changeset/single-drawer-navigation.md @@ -2,4 +2,4 @@ "@hyperdx/app": minor --- -Redesign the event side panel into a single right-hand drawer with breadcrumb-stack navigation. Logs, traces, and sessions now navigate in-place (surrounding-context drilldowns, log → trace via a new "View Trace" action, and session → event) instead of stacking layered drawers, with a unified `SidePanelBreadcrumbs` trail that is restorable from the URL. The header gains a metadata row (timestamp, service, duration, status, Copy Trace ID) and the session drawer's close button now closes the entire panel. +Redesign the event side panel into a single right-hand drawer with breadcrumb-stack navigation. Logs, traces, and sessions now navigate in-place (surrounding-context drilldowns, log → trace via a new "View Trace" action, and session → event) instead of stacking layered drawers. diff --git a/packages/app/src/SessionSidePanel.tsx b/packages/app/src/SessionSidePanel.tsx index 89676f77cb..a0267c31b5 100644 --- a/packages/app/src/SessionSidePanel.tsx +++ b/packages/app/src/SessionSidePanel.tsx @@ -2,6 +2,7 @@ import { useCallback, useMemo } from 'react'; import { useQueryState } from 'nuqs'; import { ErrorBoundary } from 'react-error-boundary'; import { useHotkeys } from 'react-hotkeys-hook'; +import { z } from 'zod'; import { DateRange, SearchCondition, @@ -9,6 +10,7 @@ import { SourceKind, TSessionSource, TTraceSource, + WithClauseSchema, } from '@hyperdx/common-utils/dist/types'; import { ActionIcon, @@ -53,8 +55,19 @@ import styles from '@/../styles/LogSidePanel.module.scss'; type SelectedEvent = { rowId: string; aliasWith: WithClause[]; + sessionId: string; }; +const sessionEventSchema = z.object({ + rowId: z.string(), + aliasWith: z.array(WithClauseSchema), + sessionId: z.string(), +}); + +const sessionEventParser = parseAsJsonEncoded( + sessionEventSchema.parse, +); + export default function SessionSidePanel({ traceSource, sessionSource, @@ -81,17 +94,28 @@ export default function SessionSidePanel({ // survives reload and shared links. Deeper navigation (View Trace, // surrounding context) is handled by `DBRowSidePanelInner`, which persists // its own state to the shared `sidePanel*` params below. - const [selectedEvent, setSelectedEvent] = useQueryState( + const [persistedEvent, setSelectedEvent] = useQueryState( 'sessionPanelEvent', - parseAsJsonEncoded(), + sessionEventParser, ); + // Read-time ownership gate: a persisted event belongs to the session it was + // opened in. If the user clicks a different session card (which updates the + // session params without going through this drawer's handlers), the old event + // is simply not selected — it can never render inside the wrong session, + // regardless of remount timing. No evict effect required. + const selectedEvent = + persistedEvent != null && persistedEvent.sessionId === sessionId + ? persistedEvent + : null; + // The embedded `DBRowSidePanelInner` owns these shared params. We clear them // whenever the session-level selection changes so a stale inner stack can't // leak across events or sessions. const [, setSourceStackParam] = useQueryState('sidePanelSourceStack'); const [, setNavStackParam] = useQueryState('sidePanelNavStack'); const [, setSidePanelTab] = useQueryState('sidePanelTab'); + const [, setStackRootParam] = useQueryState('sidePanelStackRoot'); const { size, setSize, startResize } = useResizable( INITIAL_DRAWER_WIDTH_PERCENT, @@ -107,7 +131,13 @@ export default function SessionSidePanel({ setSourceStackParam(null); setNavStackParam(null); setSidePanelTab(null); - }, [setSourceStackParam, setNavStackParam, setSidePanelTab]); + setStackRootParam(null); + }, [ + setSourceStackParam, + setNavStackParam, + setSidePanelTab, + setStackRootParam, + ]); const handleBackToSession = useCallback(() => { setSelectedEvent(null); @@ -117,9 +147,9 @@ export default function SessionSidePanel({ const handleEventNavigate = useCallback( (rowId: string, aliasWith: WithClause[]) => { clearInnerNavigation(); - setSelectedEvent({ rowId, aliasWith }); + setSelectedEvent({ rowId, aliasWith, sessionId }); }, - [setSelectedEvent, clearInnerNavigation], + [setSelectedEvent, clearInnerNavigation, sessionId], ); // X / Esc-at-root closes the whole panel and clears the session-panel params. @@ -129,16 +159,7 @@ export default function SessionSidePanel({ onClose(); }, [setSelectedEvent, clearInnerNavigation, onClose]); - // Back pops to the session root; X always closes the whole panel. - const handleNavigateBack = useCallback(() => { - if (selectedEvent) { - handleBackToSession(); - } else { - handleClose(); - } - }, [selectedEvent, handleBackToSession, handleClose]); - - useHotkeys(['esc'], handleNavigateBack); + useHotkeys(['esc'], handleClose, { enabled: !selectedEvent }); const shareSession = useCallback(async () => { const ok = await copyTextToClipboard(window.location.href); @@ -200,6 +221,18 @@ export default function SessionSidePanel({ ( + + +
An error occurred while rendering this event.
@@ -233,7 +266,7 @@ export default function SessionSidePanel({ +
An error occurred while rendering this event.
diff --git a/packages/app/src/components/DBRowSidePanel.types.ts b/packages/app/src/components/DBRowSidePanel.types.ts new file mode 100644 index 0000000000..f36b22b5c1 --- /dev/null +++ b/packages/app/src/components/DBRowSidePanel.types.ts @@ -0,0 +1,38 @@ +import { z } from 'zod'; +import { SourceKind, WithClauseSchema } from '@hyperdx/common-utils/dist/types'; + +export enum Tab { + Overview = 'overview', + Parsed = 'parsed', + Debug = 'debug', + Trace = 'trace', + ServiceMap = 'serviceMap', + Context = 'context', + Replay = 'replay', + Infrastructure = 'infrastructure', +} + +const NavEntrySchema = z.object({ + rowId: z.string(), + aliasWith: z.array(WithClauseSchema), + label: z.string(), + sourceKind: z.nativeEnum(SourceKind).optional(), + originTab: z.nativeEnum(Tab).optional(), +}); + +export type NavEntry = z.infer; + +const SourceFrameSchema = z.object({ + sourceId: z.string(), + rowId: z.string(), + aliasWith: z.array(WithClauseSchema), + label: z.string(), + sourceKind: z.nativeEnum(SourceKind).optional(), + originTab: z.nativeEnum(Tab).optional(), +}); + +export type SourceFrame = z.infer; + +export const parseSourceStack = z.array(SourceFrameSchema).parse; + +export const parseNavStack = z.array(NavEntrySchema).parse; diff --git a/packages/app/src/components/DBTracePanel.tsx b/packages/app/src/components/DBTracePanel.tsx index 7f33c47f54..f5743f38a1 100644 --- a/packages/app/src/components/DBTracePanel.tsx +++ b/packages/app/src/components/DBTracePanel.tsx @@ -1,12 +1,14 @@ import { ReactNode, useCallback, useEffect, useMemo, useState } from 'react'; import { useQueryState } from 'nuqs'; import { useForm, useWatch } from 'react-hook-form'; +import { z } from 'zod'; import { tcFromSource } from '@hyperdx/common-utils/dist/core/metadata'; import { isLogSource, isTraceSource, SourceKind, TSource, + WithClauseSchema, } from '@hyperdx/common-utils/dist/types'; import { ActionIcon, @@ -43,9 +45,23 @@ type EventRowWhere = { id: string; type: string; aliasWith: WithClause[]; + // The trace this span selection was made in. Used to gate a selection left in + // the URL from a previous trace so it can't render against a different one. + traceId?: string; }; -const eventRowWhereParser = parseAsJsonEncoded(); +// Validate the persisted span selection so a stale / hand-edited `eventRowWhere` +// (valid JSON but wrong shape) resolves to null instead of feeding a malformed +// row id into the span detail query. +const eventRowWhereSchema = z.object({ + id: z.string(), + type: z.string(), + aliasWith: z.array(WithClauseSchema), + traceId: z.string().optional(), +}); +const eventRowWhereParser = parseAsJsonEncoded( + eventRowWhereSchema.parse, +); enum SpanDetailTab { Overview = 'overview', @@ -208,6 +224,25 @@ export default function DBTracePanel({ eventRowWhereParser, ); + // A persisted span selection belongs to the trace it was made in. Gate it by + // the current `traceId` at *read* time so a selection left in the URL from a + // previous trace (e.g. after "View Trace" opened a different trace, or an old + // shared link) can never render against this trace's waterfall — no matter how + // the panel was (re)mounted. + const selectedSpan = + eventRowWhere != null && eventRowWhere.traceId === traceId + ? eventRowWhere + : null; + + // Stamp the current trace onto every selection so the gate above can tell it + // apart from a stale one. + const selectSpan = useCallback( + (where: { id: string; type: string; aliasWith: WithClause[] }) => { + setEventRowWhere({ ...where, traceId }); + }, + [setEventRowWhere, traceId], + ); + const { control: traceIdControl, handleSubmit: traceIdHandleSubmit, @@ -233,13 +268,14 @@ export default function DBTracePanel({ const [showTraceIdInput, setShowTraceIdInput] = useState(false); - // Reset highlighted row when trace ID changes - // otherwise we'll show stale span details + // Tidy the URL: drop a selection that belongs to a different trace. Purely + // cosmetic — the read-time `selectedSpan` gate already prevents a foreign + // selection from rendering; this just keeps the param from lingering. useEffect(() => { - return () => { + if (eventRowWhere != null && eventRowWhere.traceId !== traceId) { setEventRowWhere(null); - }; - }, [traceId, setEventRowWhere]); + } + }, [eventRowWhere, traceId, setEventRowWhere]); const [isSourceSchemaPreviewOpen, setIsSourceSchemaPreviewOpen] = useState(false); @@ -254,12 +290,12 @@ export default function DBTracePanel({ }, [setEventRowWhere]); const selectedSpanSource = useMemo(() => { - if (!eventRowWhere) return null; - if (eventRowWhere.type === SourceKind.Log && logSourceData) { + if (!selectedSpan) return null; + if (selectedSpan.type === SourceKind.Log && logSourceData) { return logSourceData; } return traceSourceData; - }, [eventRowWhere, logSourceData, traceSourceData]); + }, [selectedSpan, logSourceData, traceSourceData]); return (
)}
- {eventRowWhere != null && ( + {selectedSpan != null && (
diff --git a/packages/app/src/components/__tests__/DBRowSidePanel.missingSource.test.tsx b/packages/app/src/components/__tests__/DBRowSidePanel.missingSource.test.tsx new file mode 100644 index 0000000000..a342bdf8e3 --- /dev/null +++ b/packages/app/src/components/__tests__/DBRowSidePanel.missingSource.test.tsx @@ -0,0 +1,260 @@ +import React from 'react'; +import { MantineProvider } from '@mantine/core'; +import { fireEvent, render, screen } from '@testing-library/react'; + +// Controlled, in-memory replacement for nuqs' useQueryState so each side-panel +// URL param can be seeded and its setter inspected independently. Values are +// the already-parsed shapes the component consumes (arrays / strings), not URL +// strings. Prefixed with `mock` so jest.mock's factory may reference them. +const mockQueryStore: Record = {}; +const mockSetters: Record = {}; + +function seedParam(key: string, value: unknown) { + mockQueryStore[key] = value; +} +function setterFor(key: string) { + if (!mockSetters[key]) mockSetters[key] = jest.fn(); + return mockSetters[key]; +} +function resetQueryState() { + Object.keys(mockQueryStore).forEach(k => delete mockQueryStore[k]); + Object.keys(mockSetters).forEach(k => delete mockSetters[k]); +} + +jest.mock('nuqs', () => { + const actual = jest.requireActual('nuqs'); + return { + ...actual, + // eslint-disable-next-line @eslint-react/no-unnecessary-use-prefix + useQueryState: (key: string, parser?: { defaultValue?: unknown }) => { + const hasValue = Object.prototype.hasOwnProperty.call( + mockQueryStore, + key, + ); + const fallback = + parser && 'defaultValue' in parser ? parser.defaultValue : null; + const value = hasValue ? mockQueryStore[key] : (fallback ?? null); + const setters = mockSetters; + if (!setters[key]) setters[key] = jest.fn(); + return [value, setters[key]]; + }, + }; +}); + +// Row data resolves off the requested rowId: when the panel skips the row query +// (leaf source loading or unresolvable) it passes `rowId: undefined`, which must +// NOT report `isLoading` — otherwise the loading branch would mask the +// missing-source branch we are asserting. +const mockUseRowData = jest.fn(); +jest.mock('../DBRowDataPanel', () => ({ + __esModule: true, + useRowData: (args: { rowId?: string }) => mockUseRowData(args), + ROW_DATA_ALIASES: { + DURATION_MS: '__hdx_duration', + SPAN_KIND: '__hdx_span_kind', + SERVICE_NAME: '__hdx_service_name', + SEVERITY_TEXT: '__hdx_severity_text', + }, + rowHasK8sContext: () => false, + RowDataPanel: () => null, +})); + +// Per-id source resolution state. An id absent from the store models the +// react-query `select` returning `undefined` for a deleted/renamed id even +// after the query has settled successfully. +const mockSourceStore: Record< + string, + { data: unknown; isLoading: boolean; isSuccess: boolean } +> = {}; +jest.mock('@/source', () => ({ + __esModule: true, + getEventBody: () => '__hdx_body', + useSource: ({ id }: { id: string | null }) => { + if (id == null) { + return { data: undefined, isLoading: false, isSuccess: false }; + } + if (Object.prototype.hasOwnProperty.call(mockSourceStore, id)) { + return mockSourceStore[id]; + } + // Settled, but the id no longer maps to a source. + return { data: undefined, isLoading: false, isSuccess: true }; + }, +})); + +jest.mock('../DBSessionPanel', () => ({ + __esModule: true, + useSessionId: () => ({ rumSessionId: undefined, rumServiceName: undefined }), + DBSessionPanel: () => null, +})); + +jest.mock('@/utils/highlightedAttributes', () => ({ + __esModule: true, + getHighlightedAttributesFromData: () => [], +})); + +// Heavy leaf components / chart deps that the panel imports but never renders +// under the loading / missing-source branches. Stub them so the module graph +// stays cheap to load. NOTE: SidePanelBreadcrumbs is intentionally *not* mocked +// so we can assert the Back control is present in the error state. +jest.mock('../DBTracePanel', () => ({ __esModule: true, default: () => null })); +jest.mock('../ContextSidePanel', () => ({ + __esModule: true, + default: () => null, +})); +jest.mock('../DBInfraPanel', () => ({ __esModule: true, default: () => null })); +jest.mock('../DBRowOverviewPanel', () => ({ + __esModule: true, + RowOverviewPanel: () => null, +})); +jest.mock('../DBRowSidePanelErrorState', () => ({ + __esModule: true, + DBRowSidePanelErrorState: () => null, +})); +jest.mock('../DBRowSidePanelHeader', () => ({ + __esModule: true, + default: () => null, +})); +jest.mock('../LogLevel', () => ({ __esModule: true, default: () => null })); +jest.mock('../ServiceMap/ServiceMapSidePanel', () => ({ + __esModule: true, + default: () => null, +})); +jest.mock('../TimelineChart/utils', () => ({ + __esModule: true, + renderMs: () => '', +})); +jest.mock('../DrawerUtils', () => ({ + __esModule: true, + DrawerFullWidthToggle: () => null, + INITIAL_DRAWER_WIDTH_PERCENT: 50, +})); +jest.mock('@/LogSidePanelElements', () => ({ + __esModule: true, + KeyboardShortcutsModal: () => null, +})); +jest.mock('@/TabBar', () => ({ __esModule: true, default: () => null })); +jest.mock('@/useFormatTime', () => ({ + __esModule: true, + FormatTime: () => null, +})); + +// NOTE: this import is intentionally placed after the mock factories above, +// which close over the `mock*` helpers declared at the top of this file. +import { DBRowSidePanelInner } from '@/components/DBRowSidePanel'; + +const ROOT_SOURCE = { id: 'log-src', kind: 'log', traceSourceId: 'trace-src' }; + +// A cross-source frame (e.g. "View Trace") whose source id no longer resolves. +const MISSING_FRAME = { + sourceId: 'deleted-src', + rowId: 'leaf-row', + aliasWith: [], + label: 'Deleted Trace', + sourceKind: 'trace', +}; + +function renderInner(rowId: string) { + return render( + + + , + ); +} + +describe('DBRowSidePanelInner — unresolvable stack source (HDX-3942 permanent-loading P0)', () => { + beforeEach(() => { + resetQueryState(); + Object.keys(mockSourceStore).forEach(k => delete mockSourceStore[k]); + mockUseRowData.mockReset(); + // Not loading when there is no row to query (the missing/loading source + // branches pass rowId: undefined); loading otherwise. + mockUseRowData.mockImplementation((args: { rowId?: string }) => + args?.rowId == null + ? { + data: undefined, + isLoading: false, + isSuccess: false, + isError: false, + error: null, + } + : { + data: undefined, + isLoading: true, + isSuccess: false, + isError: false, + error: null, + }, + ); + }); + + it('renders an error state with working Back/Close instead of hanging on Loading when the leaf source id no longer resolves', () => { + // A shared URL points at a cross-source frame whose source was deleted / + // renamed / lives in another workspace. Its stackRoot matches the mounted + // row so the trail is treated as valid (not stale). + seedParam('sidePanelSourceStack', [MISSING_FRAME]); + seedParam('sidePanelNavStack', []); + seedParam('sidePanelStackRoot', 'root-row'); + + renderInner('root-row'); + + // Not stuck on "Loading..." — an explicit, actionable message is shown. + expect(screen.queryByText('Loading...')).not.toBeInTheDocument(); + expect( + screen.getByText(/source is no longer available/i), + ).toBeInTheDocument(); + + // The trail collapse controls remain usable. + const back = screen.getByLabelText('Back'); + const close = screen.getByLabelText('Close'); + expect(back).toBeInTheDocument(); + expect(close).toBeInTheDocument(); + + // Back pops the broken frame off the source stack. + fireEvent.click(back); + expect(setterFor('sidePanelSourceStack')).toHaveBeenCalled(); + }); + + it('still shows Loading while the leaf source query is genuinely in flight', () => { + mockSourceStore['deleted-src'] = { + data: undefined, + isLoading: true, + isSuccess: false, + }; + seedParam('sidePanelSourceStack', [MISSING_FRAME]); + seedParam('sidePanelNavStack', []); + seedParam('sidePanelStackRoot', 'root-row'); + + renderInner('root-row'); + + expect(screen.getByText('Loading...')).toBeInTheDocument(); + expect( + screen.queryByText(/source is no longer available/i), + ).not.toBeInTheDocument(); + }); + + it('renders the row normally once the leaf source resolves', () => { + mockSourceStore['deleted-src'] = { + data: { id: 'deleted-src', kind: 'trace' }, + isLoading: false, + isSuccess: true, + }; + seedParam('sidePanelSourceStack', [MISSING_FRAME]); + seedParam('sidePanelNavStack', []); + seedParam('sidePanelStackRoot', 'root-row'); + + renderInner('root-row'); + + // Source resolved → the leaf row is queried (isLoading true → "Loading..."), + // and the missing-source error state is not shown. + expect( + screen.queryByText(/source is no longer available/i), + ).not.toBeInTheDocument(); + const resolvedRowId = mockUseRowData.mock.calls.at(-1)?.[0]?.rowId; + expect(resolvedRowId).toBe('leaf-row'); + }); +}); diff --git a/packages/app/src/components/__tests__/DBRowSidePanel.staleStack.test.tsx b/packages/app/src/components/__tests__/DBRowSidePanel.staleStack.test.tsx new file mode 100644 index 0000000000..fa709a87fe --- /dev/null +++ b/packages/app/src/components/__tests__/DBRowSidePanel.staleStack.test.tsx @@ -0,0 +1,245 @@ +import React from 'react'; +import { MantineProvider } from '@mantine/core'; +import { render } from '@testing-library/react'; + +// Controlled, in-memory replacement for nuqs' useQueryState so each side-panel +// URL param can be seeded and its setter inspected independently. Values are +// the already-parsed shapes the component consumes (arrays / strings), not URL +// strings. Prefixed with `mock` so jest.mock's factory may reference them. +const mockQueryStore: Record = {}; +const mockSetters: Record = {}; + +function seedParam(key: string, value: unknown) { + mockQueryStore[key] = value; +} +function setterFor(key: string) { + if (!mockSetters[key]) mockSetters[key] = jest.fn(); + return mockSetters[key]; +} +function resetQueryState() { + Object.keys(mockQueryStore).forEach(k => delete mockQueryStore[k]); + Object.keys(mockSetters).forEach(k => delete mockSetters[k]); +} + +jest.mock('nuqs', () => { + const actual = jest.requireActual('nuqs'); + return { + ...actual, + // eslint-disable-next-line @eslint-react/no-unnecessary-use-prefix + useQueryState: (key: string, parser?: { defaultValue?: unknown }) => { + const hasValue = Object.prototype.hasOwnProperty.call( + mockQueryStore, + key, + ); + const fallback = + parser && 'defaultValue' in parser ? parser.defaultValue : null; + const value = hasValue ? mockQueryStore[key] : (fallback ?? null); + const setters = mockSetters; + if (!setters[key]) setters[key] = jest.fn(); + return [value, setters[key]]; + }, + }; +}); + +// Capture what row the panel actually resolves to. Returning isLoading short- +// circuits the render before the heavy body/header, so those children never +// mount and the `rowId` arg is the single source of truth for "what is shown". +const mockUseRowData = jest.fn(); +jest.mock('../DBRowDataPanel', () => ({ + __esModule: true, + useRowData: (args: unknown) => mockUseRowData(args), + ROW_DATA_ALIASES: { + DURATION_MS: '__hdx_duration', + SPAN_KIND: '__hdx_span_kind', + SERVICE_NAME: '__hdx_service_name', + SEVERITY_TEXT: '__hdx_severity_text', + }, + rowHasK8sContext: () => false, + RowDataPanel: () => null, +})); + +jest.mock('@/source', () => ({ + __esModule: true, + getEventBody: () => '__hdx_body', + // A non-null id resolves to a trace source so `isResolvingSource` is false and + // the leaf frame's row id is used; a null id (no active frame) resolves to + // undefined so the panel falls back to the root source. + useSource: ({ id }: { id: string | null }) => + id ? { data: { id, kind: 'trace' } } : { data: undefined }, +})); + +jest.mock('../DBSessionPanel', () => ({ + __esModule: true, + useSessionId: () => ({ rumSessionId: undefined, rumServiceName: undefined }), + DBSessionPanel: () => null, +})); + +jest.mock('@/utils/highlightedAttributes', () => ({ + __esModule: true, + getHighlightedAttributesFromData: () => [], +})); + +// Heavy leaf components / chart deps that the panel imports but never renders +// under isLoading. Stub them so the module graph stays cheap to load. +jest.mock('../DBTracePanel', () => ({ __esModule: true, default: () => null })); +jest.mock('../ContextSidePanel', () => ({ + __esModule: true, + default: () => null, +})); +jest.mock('../DBInfraPanel', () => ({ __esModule: true, default: () => null })); +jest.mock('../DBRowOverviewPanel', () => ({ + __esModule: true, + RowOverviewPanel: () => null, +})); +jest.mock('../DBRowSidePanelErrorState', () => ({ + __esModule: true, + DBRowSidePanelErrorState: () => null, +})); +jest.mock('../DBRowSidePanelHeader', () => ({ + __esModule: true, + default: () => null, +})); +jest.mock('../SidePanelBreadcrumbs', () => ({ + __esModule: true, + default: () => null, +})); +jest.mock('../LogLevel', () => ({ __esModule: true, default: () => null })); +jest.mock('../ServiceMap/ServiceMapSidePanel', () => ({ + __esModule: true, + default: () => null, +})); +jest.mock('../TimelineChart/utils', () => ({ + __esModule: true, + renderMs: () => '', +})); +jest.mock('../DrawerUtils', () => ({ + __esModule: true, + DrawerFullWidthToggle: () => null, + INITIAL_DRAWER_WIDTH_PERCENT: 50, +})); +jest.mock('@/LogSidePanelElements', () => ({ + __esModule: true, + KeyboardShortcutsModal: () => null, +})); +jest.mock('@/TabBar', () => ({ __esModule: true, default: () => null })); +jest.mock('@/useFormatTime', () => ({ + __esModule: true, + FormatTime: () => null, +})); + +// NOTE: this import is intentionally placed after the mock factories above, +// which close over the `mock*` helpers declared at the top of this file. +import { DBRowSidePanelInner } from '@/components/DBRowSidePanel'; + +const ROOT_SOURCE = { id: 'log-src', kind: 'log', traceSourceId: 'trace-src' }; + +const TRACE_FRAME = { + sourceId: 'trace-src', + rowId: 'leaf-row', + aliasWith: [], + label: 'Trace', + sourceKind: 'trace', +}; + +function renderInner(rowId: string) { + return render( + + + , + ); +} + +function lastResolvedRowId() { + return mockUseRowData.mock.calls.at(-1)?.[0]?.rowId; +} + +describe('DBRowSidePanelInner — stale stack handling (HDX-3942 "Stale Stack Remains")', () => { + beforeEach(() => { + resetQueryState(); + mockUseRowData.mockReset(); + mockUseRowData.mockReturnValue({ + data: undefined, + isLoading: true, + isSuccess: false, + isError: false, + error: null, + }); + }); + + it('preserves a deep-linked trail when the stack root matches the mounted row', () => { + // A shared URL: stacks + a stackRoot that belongs to the mounted root row. + seedParam('sidePanelSourceStack', [TRACE_FRAME]); + seedParam('sidePanelNavStack', []); + seedParam('sidePanelStackRoot', 'root-row'); + + renderInner('root-row'); + + // Trail is honoured: the drawer resolves the leaf frame's row, not the root. + expect(lastResolvedRowId()).toBe('leaf-row'); + // ...and nothing is cleared. + expect(setterFor('sidePanelSourceStack')).not.toHaveBeenCalled(); + expect(setterFor('sidePanelNavStack')).not.toHaveBeenCalled(); + expect(setterFor('sidePanelStackRoot')).not.toHaveBeenCalled(); + }); + + it('ignores a stale trail left in the URL when a different root row mounts', () => { + // Stacks survived an unclosed drawer / cross-table switch: their stackRoot + // points at a *previous* root, but a different row is now mounted. + seedParam('sidePanelSourceStack', [TRACE_FRAME]); + seedParam('sidePanelNavStack', []); + seedParam('sidePanelStackRoot', 'old-root'); + + renderInner('new-root'); + + // The row the user actually opened wins over the stale leaf. Correctness is + // guaranteed at *read* time (the owner-gated trail is empty), so it does + // NOT depend on any clearing effect firing. + expect(lastResolvedRowId()).toBe('new-root'); + }); + + it('ignores the trail when the root row changes while the panel stays mounted', () => { + seedParam('sidePanelSourceStack', [TRACE_FRAME]); + seedParam('sidePanelNavStack', []); + seedParam('sidePanelStackRoot', 'root-1'); + + const { rerender } = renderInner('root-1'); + // Same-root render keeps the trail. + expect(lastResolvedRowId()).toBe('leaf-row'); + expect(setterFor('sidePanelSourceStack')).not.toHaveBeenCalled(); + + // The mounted panel now points at a different root row (e.g. the user + // clicked another row in the same table); the trail is stale and ignored. + rerender( + + + , + ); + + expect(lastResolvedRowId()).toBe('root-2'); + }); + + it('treats an ownerless trail (no recorded root) as stale and ignores it', () => { + // Malformed / truncated / pre-stackRoot URL: stacks present but no + // stackRoot token. Every real push writes stackRoot alongside the stacks + // (and shared links copy the full URL), so a stack we cannot prove owns the + // mounted row must not shadow the row the user actually clicked. + seedParam('sidePanelSourceStack', [TRACE_FRAME]); + seedParam('sidePanelNavStack', []); + // sidePanelStackRoot intentionally unseeded (null): ownerless. + + renderInner('some-row'); + + // The clicked row wins over the ownerless leaf at read time. + expect(lastResolvedRowId()).toBe('some-row'); + }); +}); diff --git a/packages/app/src/components/__tests__/SidePanelBreadcrumbs.test.tsx b/packages/app/src/components/__tests__/SidePanelBreadcrumbs.test.tsx new file mode 100644 index 0000000000..7957ef9f37 --- /dev/null +++ b/packages/app/src/components/__tests__/SidePanelBreadcrumbs.test.tsx @@ -0,0 +1,72 @@ +import { SourceKind } from '@hyperdx/common-utils/dist/types'; +import { MantineProvider } from '@mantine/core'; +import { fireEvent, render, screen } from '@testing-library/react'; + +import SidePanelBreadcrumbs, { + BreadcrumbItem, +} from '@/components/SidePanelBreadcrumbs'; + +function renderCrumbs(items: BreadcrumbItem[], onBack = jest.fn()) { + const utils = render( + + + , + ); + return { ...utils, onBack }; +} + +describe('SidePanelBreadcrumbs', () => { + it('renders each item label', () => { + renderCrumbs([ + { label: 'Root', sourceKind: SourceKind.Log }, + { label: 'Leaf', sourceKind: SourceKind.Trace }, + ]); + expect(screen.getByText('Root')).toBeInTheDocument(); + expect(screen.getByText('Leaf')).toBeInTheDocument(); + }); + + it('calls onBack when the back button is clicked', () => { + const { onBack } = renderCrumbs([{ label: 'Root' }]); + fireEvent.click(screen.getByLabelText('Back')); + expect(onBack).toHaveBeenCalledTimes(1); + }); + + it('invokes onClick for a non-last (ancestor) breadcrumb', () => { + const onClick = jest.fn(); + renderCrumbs([{ label: 'Ancestor', onClick }, { label: 'Current' }]); + fireEvent.click(screen.getByText('Ancestor')); + expect(onClick).toHaveBeenCalledTimes(1); + }); + + it('does not make the last (current) item clickable', () => { + const onClick = jest.fn(); + // Even though onClick is supplied, the leaf is the current view. + renderCrumbs([{ label: 'Ancestor' }, { label: 'Current', onClick }]); + fireEvent.click(screen.getByText('Current')); + expect(onClick).not.toHaveBeenCalled(); + }); + + it('truncates an over-length single label with an ellipsis', () => { + const long = 'x'.repeat(150); // > MAX_LABEL_LENGTH_SINGLE (120) + const { container } = renderCrumbs([{ label: long }]); + // The full label is not rendered verbatim; the visible text is truncated. + expect(screen.queryByText(long)).not.toBeInTheDocument(); + expect(container.textContent).toContain('…'); + }); + + it('truncates an ancestor label more aggressively than the current one', () => { + const label = 'y'.repeat(30); // > previous(20), < current(40) + // As an ancestor (not last) it should be truncated to 20 + ellipsis. + renderCrumbs([{ label }, { label: 'Current' }]); + expect(screen.getByText(`${'y'.repeat(20)}…`)).toBeInTheDocument(); + }); + + it.each([ + [SourceKind.Log, 'tabler-icon-logs'], + [SourceKind.Trace, 'tabler-icon-connection'], + [SourceKind.Session, 'tabler-icon-device-laptop'], + ])('renders the %s source icon on the first item', (kind, iconClass) => { + const { container } = renderCrumbs([{ label: 'First', sourceKind: kind }]); + expect(container.querySelector(`.${iconClass}`)).toBeInTheDocument(); + }); +}); diff --git a/packages/app/src/hooks/__tests__/useSidePanelStack.test.tsx b/packages/app/src/hooks/__tests__/useSidePanelStack.test.tsx new file mode 100644 index 0000000000..4ae50cb3ed --- /dev/null +++ b/packages/app/src/hooks/__tests__/useSidePanelStack.test.tsx @@ -0,0 +1,277 @@ +import { act, renderHook } from '@testing-library/react'; + +import { SourceFrame, Tab } from '@/components/DBRowSidePanel.types'; + +// In-memory nuqs so each side-panel param can be seeded and its setter +// inspected. Values are the already-parsed shapes the hook consumes (arrays / +// strings), not URL strings. Prefixed `mock` so jest.mock's factory can use it. +const mockQueryStore: Record = {}; +const mockSetters: Record = {}; + +function seedParam(key: string, value: unknown) { + mockQueryStore[key] = value; +} +function setterFor(key: string) { + if (!mockSetters[key]) mockSetters[key] = jest.fn(); + return mockSetters[key]; +} +function resetQueryState() { + Object.keys(mockQueryStore).forEach(k => delete mockQueryStore[k]); + Object.keys(mockSetters).forEach(k => delete mockSetters[k]); +} + +jest.mock('nuqs', () => { + const actual = jest.requireActual('nuqs'); + return { + ...actual, + // eslint-disable-next-line @eslint-react/no-unnecessary-use-prefix + useQueryState: (key: string, parser?: { defaultValue?: unknown }) => { + const hasValue = Object.prototype.hasOwnProperty.call( + mockQueryStore, + key, + ); + const fallback = + parser && 'defaultValue' in parser ? parser.defaultValue : null; + const value = hasValue ? mockQueryStore[key] : (fallback ?? null); + if (!mockSetters[key]) mockSetters[key] = jest.fn(); + return [value, mockSetters[key]]; + }, + }; +}); + +// NOTE: imported after the mock factories above, which close over the `mock*` +// helpers declared at the top of this file. +import useSidePanelStack, { + deriveEffectiveTrail, + reconcileTab, +} from '@/hooks/useSidePanelStack'; + +const FRAME: SourceFrame = { + sourceId: 'trace-src', + rowId: 'leaf-row', + aliasWith: [], + label: 'Trace', + sourceKind: undefined, +}; + +describe('deriveEffectiveTrail', () => { + it('keeps a trail whose owner matches the mounted row', () => { + const out = deriveEffectiveTrail({ + rawSourceStack: [FRAME], + rawNavStack: [], + stackRoot: 'root-1', + initialRowId: 'root-1', + }); + expect(out.isStale).toBe(false); + expect(out.sourceStack).toEqual([FRAME]); + }); + + it('discards a trail whose owner differs from the mounted row', () => { + const out = deriveEffectiveTrail({ + rawSourceStack: [FRAME], + rawNavStack: [], + stackRoot: 'old-root', + initialRowId: 'new-root', + }); + expect(out.isStale).toBe(true); + expect(out.sourceStack).toEqual([]); + expect(out.navStack).toEqual([]); + }); + + it('treats an ownerless trail (no stackRoot) as stale', () => { + const out = deriveEffectiveTrail({ + rawSourceStack: [FRAME], + rawNavStack: [], + stackRoot: null, + initialRowId: 'some-row', + }); + expect(out.isStale).toBe(true); + expect(out.sourceStack).toEqual([]); + }); + + it('is not stale when there are no stacks at all', () => { + const out = deriveEffectiveTrail({ + rawSourceStack: [], + rawNavStack: [], + stackRoot: null, + initialRowId: 'root-1', + }); + expect(out.isStale).toBe(false); + }); +}); + +describe('reconcileTab', () => { + const available = [Tab.Overview, Tab.Parsed, Tab.Context]; + + it('keeps a persisted tab that the source offers', () => { + expect(reconcileTab(Tab.Parsed, available, Tab.Overview)).toBe(Tab.Parsed); + }); + + it('falls back to the default when the persisted tab is unavailable', () => { + expect(reconcileTab(Tab.Trace, available, Tab.Overview)).toBe(Tab.Overview); + }); + + it('falls back to the default when no tab is persisted', () => { + expect(reconcileTab(null, available, Tab.Overview)).toBe(Tab.Overview); + }); +}); + +describe('useSidePanelStack', () => { + beforeEach(resetQueryState); + + it('exposes an owner-gated empty trail when the persisted trail is stale', () => { + seedParam('sidePanelSourceStack', [FRAME]); + seedParam('sidePanelStackRoot', 'old-root'); + const { result } = renderHook(() => + useSidePanelStack({ initialRowId: 'new-root' }), + ); + expect(result.current.isStale).toBe(true); + expect(result.current.sourceStack).toEqual([]); + }); + + it('pushSource records the frame + owner, resets nav, and jumps to the tab', () => { + seedParam('sidePanelTab', Tab.Overview); + const { result } = renderHook(() => + useSidePanelStack({ initialRowId: 'root-1' }), + ); + + act(() => result.current.pushSource(FRAME, Tab.Trace)); + + // Frame appended, stamped with the tab active before the push (originTab). + expect(setterFor('sidePanelSourceStack')).toHaveBeenCalledWith([ + { ...FRAME, originTab: Tab.Overview }, + ]); + // Cross-source push clears the same-source drilldown stack... + expect(setterFor('sidePanelNavStack')).toHaveBeenCalledWith([]); + // ...records the owning root... + expect(setterFor('sidePanelStackRoot')).toHaveBeenCalledWith('root-1'); + // ...and jumps to the destination tab. + expect(setterFor('sidePanelTab')).toHaveBeenCalledWith(Tab.Trace); + }); + + it('pushSource on a stale trail starts fresh instead of extending old frames', () => { + seedParam('sidePanelSourceStack', [FRAME]); + seedParam('sidePanelStackRoot', 'old-root'); + const { result } = renderHook(() => + useSidePanelStack({ initialRowId: 'new-root' }), + ); + + const next: SourceFrame = { ...FRAME, rowId: 'fresh' }; + act(() => result.current.pushSource(next, Tab.Trace)); + + // The new frame is the only one (the stale leaf is not carried over). + expect(setterFor('sidePanelSourceStack')).toHaveBeenCalledWith([ + { ...next, originTab: undefined }, + ]); + expect(setterFor('sidePanelStackRoot')).toHaveBeenCalledWith('new-root'); + }); + + it('pushNav appends a same-source entry and records the owner', () => { + seedParam('sidePanelTab', Tab.Overview); + const { result } = renderHook(() => + useSidePanelStack({ initialRowId: 'root-1' }), + ); + + act(() => + result.current.pushNav( + { rowId: 'ctx-row', aliasWith: [], label: 'Related' }, + Tab.Parsed, + ), + ); + + expect(setterFor('sidePanelNavStack')).toHaveBeenCalledWith([ + { + rowId: 'ctx-row', + aliasWith: [], + label: 'Related', + originTab: Tab.Overview, + }, + ]); + expect(setterFor('sidePanelStackRoot')).toHaveBeenCalledWith('root-1'); + expect(setterFor('sidePanelTab')).toHaveBeenCalledWith(Tab.Parsed); + }); + + it('popOne pops the nav leaf first, restoring its originTab', () => { + seedParam('sidePanelSourceStack', [{ ...FRAME, originTab: Tab.Overview }]); + seedParam('sidePanelNavStack', [ + { rowId: 'ctx', aliasWith: [], label: 'Ctx', originTab: Tab.Context }, + ]); + seedParam('sidePanelStackRoot', 'root-1'); + const { result } = renderHook(() => + useSidePanelStack({ initialRowId: 'root-1' }), + ); + + let popped: string | undefined; + act(() => { + popped = result.current.popOne(); + }); + + expect(popped).toBe('nav'); + expect(setterFor('sidePanelNavStack')).toHaveBeenCalledWith([]); + expect(setterFor('sidePanelTab')).toHaveBeenCalledWith(Tab.Context); + // The source frame is untouched while a nav entry remained. + expect(setterFor('sidePanelSourceStack')).not.toHaveBeenCalled(); + }); + + it('popOne pops the source frame when no nav entries remain', () => { + seedParam('sidePanelSourceStack', [{ ...FRAME, originTab: Tab.Parsed }]); + seedParam('sidePanelNavStack', []); + seedParam('sidePanelStackRoot', 'root-1'); + const { result } = renderHook(() => + useSidePanelStack({ initialRowId: 'root-1' }), + ); + + let popped: string | undefined; + act(() => { + popped = result.current.popOne(); + }); + + expect(popped).toBe('source'); + expect(setterFor('sidePanelSourceStack')).toHaveBeenCalledWith([]); + expect(setterFor('sidePanelTab')).toHaveBeenCalledWith(Tab.Parsed); + }); + + it('popOne returns "none" when the trail is empty (caller exits)', () => { + const { result } = renderHook(() => + useSidePanelStack({ initialRowId: 'root-1' }), + ); + let popped: string | undefined; + act(() => { + popped = result.current.popOne(); + }); + expect(popped).toBe('none'); + expect(setterFor('sidePanelSourceStack')).not.toHaveBeenCalled(); + }); + + it('truncateTo slices both stacks and restores the dropped level tab', () => { + seedParam('sidePanelSourceStack', [ + { ...FRAME, rowId: 'a', originTab: Tab.Overview }, + { ...FRAME, rowId: 'b', originTab: Tab.Trace }, + ]); + seedParam('sidePanelNavStack', []); + seedParam('sidePanelStackRoot', 'root-1'); + const { result } = renderHook(() => + useSidePanelStack({ initialRowId: 'root-1' }), + ); + + // Jump back to the first source frame (breadcrumb click). + act(() => result.current.truncateTo(1, 0)); + + expect(setterFor('sidePanelSourceStack')).toHaveBeenCalledWith([ + { ...FRAME, rowId: 'a', originTab: Tab.Overview }, + ]); + // Restores the tab active before the dropped frame (index 1) was entered. + expect(setterFor('sidePanelTab')).toHaveBeenCalledWith(Tab.Trace); + }); + + it('clearTrail nulls every nav param', () => { + const { result } = renderHook(() => + useSidePanelStack({ initialRowId: 'root-1' }), + ); + act(() => result.current.clearTrail()); + expect(setterFor('sidePanelSourceStack')).toHaveBeenCalledWith(null); + expect(setterFor('sidePanelNavStack')).toHaveBeenCalledWith(null); + expect(setterFor('sidePanelStackRoot')).toHaveBeenCalledWith(null); + expect(setterFor('sidePanelTab')).toHaveBeenCalledWith(null); + }); +}); diff --git a/packages/app/src/hooks/useRowWhere.tsx b/packages/app/src/hooks/useRowWhere.tsx index 25e664698f..a5ceffdefb 100644 --- a/packages/app/src/hooks/useRowWhere.tsx +++ b/packages/app/src/hooks/useRowWhere.tsx @@ -1,18 +1,19 @@ import { useCallback, useMemo } from 'react'; import MD5 from 'crypto-js/md5'; import SqlString from 'sqlstring'; +import z from 'zod'; import { ColumnMetaType, convertCHDataTypeToJSType, JSDataType, } from '@hyperdx/common-utils/dist/clickhouse'; import { aliasMapToWithClauses } from '@hyperdx/common-utils/dist/core/utils'; -import { BuilderChartConfig } from '@hyperdx/common-utils/dist/types'; +import { WithClauseSchema } from '@hyperdx/common-utils/dist/types'; const MAX_STRING_LENGTH = 512; // Type for WITH clause entries, derived from ChartConfig's with property -export type WithClause = NonNullable[number]; +export type WithClause = z.infer; // Internal row field names used by the table component for row tracking export const INTERNAL_ROW_FIELDS = { diff --git a/packages/app/src/hooks/useSidePanelStack.ts b/packages/app/src/hooks/useSidePanelStack.ts new file mode 100644 index 0000000000..8c6f5061c3 --- /dev/null +++ b/packages/app/src/hooks/useSidePanelStack.ts @@ -0,0 +1,213 @@ +import { useCallback, useMemo } from 'react'; +import { parseAsStringEnum, useQueryState } from 'nuqs'; + +import { + NavEntry, + parseNavStack, + parseSourceStack, + SourceFrame, + Tab, +} from '@/components/DBRowSidePanel.types'; +import { parseAsJsonEncoded, parseAsStringEncoded } from '@/utils/queryParsers'; + +const EMPTY_SOURCE_STACK: SourceFrame[] = []; +const EMPTY_NAV_STACK: NavEntry[] = []; + +export function deriveEffectiveTrail(params: { + rawSourceStack: SourceFrame[]; + rawNavStack: NavEntry[]; + stackRoot: string | null; + initialRowId: string | undefined; +}): { + sourceStack: SourceFrame[]; + navStack: NavEntry[]; + isStale: boolean; +} { + const { rawSourceStack, rawNavStack, stackRoot, initialRowId } = params; + const hasStacks = rawSourceStack.length > 0 || rawNavStack.length > 0; + const isStale = hasStacks && (stackRoot ?? null) !== (initialRowId ?? null); + return { + sourceStack: isStale ? EMPTY_SOURCE_STACK : rawSourceStack, + navStack: isStale ? EMPTY_NAV_STACK : rawNavStack, + isStale, + }; +} + +export function reconcileTab( + tab: Tab | null, + availableTabs: readonly Tab[], + defaultTab: Tab, +): Tab { + if (tab != null && availableTabs.includes(tab)) { + return tab; + } + return defaultTab; +} + +export type SidePanelStack = { + /** Owner-gated cross-source frames (empty if the persisted trail is stale). */ + sourceStack: SourceFrame[]; + /** Owner-gated same-source drilldown entries. */ + navStack: NavEntry[]; + /** True when a persisted trail was discarded because it belongs to another row. */ + isStale: boolean; + /** Persisted tab, or null when unset. Reconcile against the source's tabs. */ + tab: Tab | null; + + /** Push a cross-source frame (e.g. View Trace) and jump to its default tab. */ + pushSource: (frame: SourceFrame, destinationTab: Tab) => void; + /** Push a same-source drilldown (e.g. surrounding context) + destination tab. */ + pushNav: (entry: NavEntry, destinationTab: Tab) => void; + /** + * Pop one level (nav → source → nothing), restoring the tab that was active + * before the popped level was entered. Returns what was popped so the caller + * can route "nothing left" to parent/close. + */ + popOne: () => 'nav' | 'source' | 'none'; + /** Truncate both stacks to the given lengths (breadcrumb jump), restoring tab. */ + truncateTo: (sourceLevel: number, navLevel: number) => void; + /** Set the active tab. */ + setTab: (tab: Tab) => void; + /** Clear the whole trail + owner token + tab (panel close / new selection). */ + clearTrail: () => void; +}; + +/** + * Owns all side-panel navigation URL params behind a single read-time owner + * gate. Every consumer reads the *effective* trail from here rather than the raw + * params, so a stale value left in the URL by some un-cleared write path can + * never be rendered. + */ +export default function useSidePanelStack({ + initialRowId, +}: { + initialRowId: string | undefined; +}): SidePanelStack { + const [rawSourceStack, setSourceStack] = useQueryState( + 'sidePanelSourceStack', + parseAsJsonEncoded(parseSourceStack).withDefault( + EMPTY_SOURCE_STACK, + ), + ); + + const [rawNavStack, setNavStack] = useQueryState( + 'sidePanelNavStack', + parseAsJsonEncoded(parseNavStack).withDefault(EMPTY_NAV_STACK), + ); + + const [stackRoot, setStackRoot] = useQueryState( + 'sidePanelStackRoot', + parseAsStringEncoded, + ); + + const [tab, setQueryTab] = useQueryState( + 'sidePanelTab', + parseAsStringEnum(Object.values(Tab)), + ); + + const { sourceStack, navStack, isStale } = useMemo( + () => + deriveEffectiveTrail({ + rawSourceStack, + rawNavStack, + stackRoot, + initialRowId, + }), + [rawSourceStack, rawNavStack, stackRoot, initialRowId], + ); + + const pushSource = useCallback( + (frame: SourceFrame, destinationTab: Tab) => { + // Compute from the *effective* stack (empty when stale) so a push from a + // stale URL starts a fresh, owned trail instead of extending old frames. + setSourceStack([ + ...sourceStack, + { ...frame, originTab: tab ?? undefined }, + ]); + setNavStack([]); + setStackRoot(initialRowId ?? null); + setQueryTab(destinationTab); + }, + [ + sourceStack, + tab, + initialRowId, + setSourceStack, + setNavStack, + setStackRoot, + setQueryTab, + ], + ); + + const pushNav = useCallback( + (entry: NavEntry, destinationTab: Tab) => { + setNavStack([...navStack, { ...entry, originTab: tab ?? undefined }]); + setStackRoot(initialRowId ?? null); + setQueryTab(destinationTab); + }, + [navStack, tab, initialRowId, setNavStack, setStackRoot, setQueryTab], + ); + + const popOne = useCallback((): 'nav' | 'source' | 'none' => { + if (navStack.length > 0) { + const restoreTab = navStack[navStack.length - 1]?.originTab; + setNavStack(navStack.slice(0, -1)); + if (restoreTab) { + setQueryTab(restoreTab); + } + return 'nav'; + } + if (sourceStack.length > 0) { + const restoreTab = sourceStack[sourceStack.length - 1]?.originTab; + setSourceStack(sourceStack.slice(0, -1)); + setNavStack([]); + if (restoreTab) { + setQueryTab(restoreTab); + } + return 'source'; + } + return 'none'; + }, [navStack, sourceStack, setNavStack, setSourceStack, setQueryTab]); + + const truncateTo = useCallback( + (sourceLevel: number, navLevel: number) => { + // Restore the tab active at the level we're returning to: the first source + // frame being dropped, or (staying within the same source) the first nav + // entry being dropped. + let restoreTab: Tab | undefined; + if (sourceLevel < sourceStack.length) { + restoreTab = sourceStack[sourceLevel]?.originTab; + } else if (navLevel < navStack.length) { + restoreTab = navStack[navLevel]?.originTab; + } + setSourceStack(sourceStack.slice(0, sourceLevel)); + setNavStack(navStack.slice(0, navLevel)); + if (restoreTab) { + setQueryTab(restoreTab); + } + }, + [sourceStack, navStack, setSourceStack, setNavStack, setQueryTab], + ); + + const setTab = useCallback((next: Tab) => setQueryTab(next), [setQueryTab]); + + const clearTrail = useCallback(() => { + setSourceStack(null); + setNavStack(null); + setStackRoot(null); + setQueryTab(null); + }, [setSourceStack, setNavStack, setStackRoot, setQueryTab]); + + return { + sourceStack, + navStack, + isStale, + tab, + pushSource, + pushNav, + popOne, + truncateTo, + setTab, + clearTrail, + }; +} diff --git a/packages/app/src/utils/__tests__/queryParsers.test.ts b/packages/app/src/utils/__tests__/queryParsers.test.ts index 0508450771..ff1bf66873 100644 --- a/packages/app/src/utils/__tests__/queryParsers.test.ts +++ b/packages/app/src/utils/__tests__/queryParsers.test.ts @@ -102,4 +102,43 @@ describe('parseAsJsonEncoded', () => { ); }); }); + + describe('with a validator', () => { + // Accepts only an array of numbers; throws otherwise (mirrors zod.parse). + const numberArrayParser = parseAsJsonEncoded(value => { + if (!Array.isArray(value) || !value.every(n => typeof n === 'number')) { + throw new Error('expected number[]'); + } + return value; + }); + + it('returns a value that satisfies the validator', () => { + expect( + numberArrayParser.parse(numberArrayParser.serialize([1, 2, 3])), + ).toEqual([1, 2, 3]); + }); + + it.each([ + ['an object', {}], + ['a bare number', 5], + ['a bare string', 'x'], + ['a null literal', null], + ['an array of the wrong element type', ['a', 'b']], + ])('returns null for %s (valid JSON, wrong shape)', (_label, input) => { + const serialized = encodeURIComponent(JSON.stringify(input)); + expect(numberArrayParser.parse(serialized)).toBeNull(); + }); + + it('returns null when the validator returns null instead of throwing', () => { + const nullingParser = parseAsJsonEncoded(() => null as never); + expect(nullingParser.parse(encodeURIComponent('[1,2,3]'))).toBeNull(); + }); + + it('still returns null for non-JSON input before validation runs', () => { + const validate = jest.fn(); + const parser = parseAsJsonEncoded(validate); + expect(parser.parse('not-json')).toBeNull(); + expect(validate).not.toHaveBeenCalled(); + }); + }); }); diff --git a/packages/app/src/utils/queryParsers.ts b/packages/app/src/utils/queryParsers.ts index e6486465b3..5e165eda2c 100644 --- a/packages/app/src/utils/queryParsers.ts +++ b/packages/app/src/utils/queryParsers.ts @@ -40,8 +40,31 @@ export const parseAsStringEncoded = createParser({ * spaces, unencoded '[', ']', etc.) are handled via a fallback to plain * JSON.parse after the decodeURIComponent step naturally resolves '%22' → * '"', '+' → ' ', etc. via URLSearchParams.get(). + * + * Optional `validate`: a guard that either returns the (narrowed) value or + * throws / returns `null` for a shape mismatch. A stale, hand-edited, or + * cross-version URL whose param is valid JSON but the wrong shape (`{}`, `5`, + * `"x"`, an array of malformed frames) then resolves to `null` instead of a + * structurally-invalid object. Combined with `.withDefault(...)`, callers get + * the safe default rather than a value that throws downstream during render. + * Zod schemas satisfy this signature via `schema.parse`. */ -export function parseAsJsonEncoded() { +export function parseAsJsonEncoded(validate?: (value: unknown) => T) { + const finalize = (parsed: unknown): T | null => { + if (!validate) { + // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion + return parsed as T; + } + try { + const validated = validate(parsed); + // A validator may signal rejection by returning null/undefined instead + // of throwing; treat both the same as a thrown mismatch. + return validated == null ? null : validated; + } catch { + return null; + } + }; + return createParser({ parse: value => { let decoded: string; @@ -50,7 +73,7 @@ export function parseAsJsonEncoded() { } catch { // Malformed URI sequence — value is likely old-format plain JSON. try { - return JSON.parse(value); + return finalize(JSON.parse(value)); } catch { return null; } @@ -59,7 +82,7 @@ export function parseAsJsonEncoded() { // This handles both new-format (double-encoded) and old-format URLs, // since decodeURIComponent is a no-op on plain JSON strings. try { - return JSON.parse(decoded); + return finalize(JSON.parse(decoded)); } catch { return null; } diff --git a/packages/app/tests/e2e/features/sessions.spec.ts b/packages/app/tests/e2e/features/sessions.spec.ts index 8386a5c2ef..45cd1b6f7e 100644 --- a/packages/app/tests/e2e/features/sessions.spec.ts +++ b/packages/app/tests/e2e/features/sessions.spec.ts @@ -105,9 +105,11 @@ test.describe('Client Sessions Functionality', { tag: ['@sessions'] }, () => { }); await test.step('Pressing Escape returns to the session event list within the same drawer', async () => { - // SessionSidePanel owns the Esc hotkey: when an event is selected it - // pops back to the session root (handleNavigateBack) instead of closing - // the whole drawer. + // With an event open, the embedded DBRowSidePanelInner owns Esc and pops + // one level at a time; at the event root it hands back to the session + // (onNavigateToParent), so a single Esc returns to the session event + // list instead of closing the whole drawer. (The parent's own Esc is + // disabled while an event is selected to avoid a double-fire.) await page.keyboard.press('Escape'); // The event detail TabBar collapses back to the session event list. diff --git a/packages/common-utils/src/types.ts b/packages/common-utils/src/types.ts index e8c597fb03..067ab8f267 100644 --- a/packages/common-utils/src/types.ts +++ b/packages/common-utils/src/types.ts @@ -1221,34 +1221,30 @@ export const CteChartConfigSchema = z.intersection( export type CteChartConfig = z.infer; +export const WithClauseSchema = z.object({ + name: z.string(), + + // Need to specify either a sql or chartConfig instance. To avoid + // the schema falling into an any type, the fields are separate + // and listed as optional. + sql: ChSqlSchema.optional(), + chartConfig: CteChartConfigSchema.optional(), + + // If true, it'll render as WITH ident AS (subquery) + // If false, it'll be a "variable" ex. WITH (sql) AS ident + // where sql can be any expression, ex. a constant string + // see: https://clickhouse.com/docs/sql-reference/statements/select/with#syntax + // default assume true + isSubquery: z.boolean().optional(), +}); + // The `with` CTE property needs to be defined at this level, just above the // non-recursive chart config so that it can reference a complete chart config // schema. This structure does mean that we cannot nest `with` clauses but does // ensure the type system can catch more issues in the build pipeline. const BuilderChartConfigSchema = z.intersection( z.intersection(_ChartConfigSchema, SelectSQLStatementSchema), - z - .object({ - with: z.array( - z.object({ - name: z.string(), - - // Need to specify either a sql or chartConfig instance. To avoid - // the schema falling into an any type, the fields are separate - // and listed as optional. - sql: ChSqlSchema.optional(), - chartConfig: CteChartConfigSchema.optional(), - - // If true, it'll render as WITH ident AS (subquery) - // If false, it'll be a "variable" ex. WITH (sql) AS ident - // where sql can be any expression, ex. a constant string - // see: https://clickhouse.com/docs/sql-reference/statements/select/with#syntax - // default assume true - isSubquery: z.boolean().optional(), - }), - ), - }) - .partial(), + z.object({ with: z.array(WithClauseSchema) }).partial(), ); export type BuilderChartConfig = z.infer; From 76712eb9908592c8b8f7687974c61b9eab7e1192 Mon Sep 17 00:00:00 2001 From: Karl Power Date: Tue, 7 Jul 2026 11:22:00 +0200 Subject: [PATCH 6/8] reuse clearTrail, common SidePanelErrorFallback --- packages/app/src/SessionSidePanel.tsx | 72 ++++-------- .../app/src/components/DBRowSidePanel.tsx | 104 ++++++++---------- packages/app/src/hooks/useSidePanelStack.ts | 46 +++++--- 3 files changed, 96 insertions(+), 126 deletions(-) diff --git a/packages/app/src/SessionSidePanel.tsx b/packages/app/src/SessionSidePanel.tsx index a0267c31b5..f59d499575 100644 --- a/packages/app/src/SessionSidePanel.tsx +++ b/packages/app/src/SessionSidePanel.tsx @@ -19,7 +19,6 @@ import { Drawer, Flex, Group, - Stack, Text, Tooltip, } from '@mantine/core'; @@ -29,6 +28,7 @@ import { IconLink, IconX } from '@tabler/icons-react'; import { DBRowSidePanelInner, RowSidePanelContext, + SidePanelErrorFallback, } from '@/components/DBRowSidePanel'; import { DrawerFullWidthToggle, @@ -46,6 +46,7 @@ import { import { parseAsJsonEncoded } from '@/utils/queryParsers'; import { ZIndexContext } from '@/zIndex'; +import useSidePanelStack from './hooks/useSidePanelStack'; import { Session } from './sessions'; import SessionSubpanel from './SessionSubpanel'; import { formatDistanceToNowStrictShort } from './utils'; @@ -109,14 +110,6 @@ export default function SessionSidePanel({ ? persistedEvent : null; - // The embedded `DBRowSidePanelInner` owns these shared params. We clear them - // whenever the session-level selection changes so a stale inner stack can't - // leak across events or sessions. - const [, setSourceStackParam] = useQueryState('sidePanelSourceStack'); - const [, setNavStackParam] = useQueryState('sidePanelNavStack'); - const [, setSidePanelTab] = useQueryState('sidePanelTab'); - const [, setStackRootParam] = useQueryState('sidePanelStackRoot'); - const { size, setSize, startResize } = useResizable( INITIAL_DRAWER_WIDTH_PERCENT, ); @@ -127,37 +120,29 @@ export default function SessionSidePanel({ const sessionLabel = session?.userEmail || `Anonymous Session ${sessionId}`; - const clearInnerNavigation = useCallback(() => { - setSourceStackParam(null); - setNavStackParam(null); - setSidePanelTab(null); - setStackRootParam(null); - }, [ - setSourceStackParam, - setNavStackParam, - setSidePanelTab, - setStackRootParam, - ]); + const sidePanelStack = useSidePanelStack({ + initialRowId: selectedEvent?.rowId, + }); const handleBackToSession = useCallback(() => { setSelectedEvent(null); - clearInnerNavigation(); - }, [setSelectedEvent, clearInnerNavigation]); + sidePanelStack.clearTrail(); + }, [setSelectedEvent, sidePanelStack]); const handleEventNavigate = useCallback( (rowId: string, aliasWith: WithClause[]) => { - clearInnerNavigation(); + sidePanelStack.clearTrail(); setSelectedEvent({ rowId, aliasWith, sessionId }); }, - [setSelectedEvent, clearInnerNavigation, sessionId], + [setSelectedEvent, sidePanelStack, sessionId], ); // X / Esc-at-root closes the whole panel and clears the session-panel params. const handleClose = useCallback(() => { setSelectedEvent(null); - clearInnerNavigation(); + sidePanelStack.clearTrail(); onClose(); - }, [setSelectedEvent, clearInnerNavigation, onClose]); + }, [setSelectedEvent, sidePanelStack, onClose]); useHotkeys(['esc'], handleClose, { enabled: !selectedEvent }); @@ -207,7 +192,7 @@ export default function SessionSidePanel({ }, }} > - +
{selectedEvent ? ( - ( - - - - -
- An error occurred while rendering this event. -
-
- {error?.error?.message} -
- + fallbackRender={fallbackProps => ( + )} > - + ) : ( <> @@ -325,7 +295,7 @@ export default function SessionSidePanel({ )}
- + ); } diff --git a/packages/app/src/components/DBRowSidePanel.tsx b/packages/app/src/components/DBRowSidePanel.tsx index e528bbad7b..456d58469e 100644 --- a/packages/app/src/components/DBRowSidePanel.tsx +++ b/packages/app/src/components/DBRowSidePanel.tsx @@ -8,8 +8,7 @@ import { } from 'react'; import { add } from 'date-fns'; import { isString } from 'lodash'; -import { parseAsStringEnum, useQueryState } from 'nuqs'; -import { ErrorBoundary } from 'react-error-boundary'; +import { ErrorBoundary, FallbackProps } from 'react-error-boundary'; import { useHotkeys } from 'react-hotkeys-hook'; import SqlString from 'sqlstring'; import { @@ -38,7 +37,10 @@ import { IconCopy, IconKeyboard, IconShare, IconX } from '@tabler/icons-react'; import useResizable from '@/hooks/useResizable'; import { WithClause } from '@/hooks/useRowWhere'; -import useSidePanelStack, { reconcileTab } from '@/hooks/useSidePanelStack'; +import useSidePanelStack, { + reconcileTab, + SidePanelStack, +} from '@/hooks/useSidePanelStack'; import useWaterfallSearchState from '@/hooks/useWaterfallSearchState'; import { KeyboardShortcutsModal } from '@/LogSidePanelElements'; import { getEventBody, useSource } from '@/source'; @@ -47,7 +49,6 @@ import { SearchConfig } from '@/types'; import { FormatTime } from '@/useFormatTime'; import { formatDistanceToNowStrictShort } from '@/utils'; import { getHighlightedAttributesFromData } from '@/utils/highlightedAttributes'; -import { parseAsJsonEncoded, parseAsStringEncoded } from '@/utils/queryParsers'; import { useZIndex, ZIndexContext } from '@/zIndex'; import ServiceMapSidePanel from './ServiceMap/ServiceMapSidePanel'; @@ -61,7 +62,7 @@ import { useRowData, } from './DBRowDataPanel'; import { RowOverviewPanel } from './DBRowOverviewPanel'; -import { NavEntry, SourceFrame, Tab } from './DBRowSidePanel.types'; +import { SourceFrame, Tab } from './DBRowSidePanel.types'; import { DBRowSidePanelErrorState } from './DBRowSidePanelErrorState'; import DBRowSidePanelHeader from './DBRowSidePanelHeader'; import { DBSessionPanel, useSessionId } from './DBSessionPanel'; @@ -208,6 +209,7 @@ type DBRowSidePanelInnerProps = DBRowSidePanelProps & { persistStacksInUrl?: boolean; parentBreadcrumbs?: BreadcrumbItem[]; onNavigateToParent?: () => void; + sidePanelStack: SidePanelStack; }; export const DBRowSidePanelInner = ({ @@ -219,6 +221,7 @@ export const DBRowSidePanelInner = ({ onToggleFullWidth, parentBreadcrumbs, onNavigateToParent, + sidePanelStack, }: DBRowSidePanelInnerProps) => { const { sourceStack, @@ -229,7 +232,7 @@ export const DBRowSidePanelInner = ({ popOne, truncateTo, setTab, - } = useSidePanelStack({ initialRowId }); + } = sidePanelStack; const activeSourceFrame = sourceStack.length > 0 ? sourceStack[sourceStack.length - 1] : null; @@ -1024,6 +1027,33 @@ export const DBRowSidePanelInner = ({ ); }; +export const SidePanelErrorFallback = ({ + error, + onClose, +}: FallbackProps & { onClose: () => void }) => ( + + + + +
+ An error occurred while rendering this event. +
+ +
+ {error?.message} +
+ +); + export default function DBRowSidePanelErrorBoundary({ onClose, rowId, @@ -1042,43 +1072,18 @@ export default function DBRowSidePanelErrorBoundary({ setSize(isFullWidth ? INITIAL_DRAWER_WIDTH_PERCENT : 100); }, [isFullWidth, setSize]); - const [, setSidePanelTab] = useQueryState( - 'sidePanelTab', - parseAsStringEnum(Object.values(Tab)), - ); - const [, setSourceStackParam] = useQueryState( - 'sidePanelSourceStack', - parseAsJsonEncoded(), - ); - const [, setNavStackParam] = useQueryState( - 'sidePanelNavStack', - parseAsJsonEncoded(), - ); - const [, setStackRootParam] = useQueryState( - 'sidePanelStackRoot', - parseAsStringEncoded, - ); - const { clear: clearTraceWaterfallSearchState } = useWaterfallSearchState({}); + const sidePanelStack = useSidePanelStack({ initialRowId: rowId }); + const _onClose = useCallback(() => { // Reset the tab and navigation stacks so re-opening the drawer starts fresh. - setSidePanelTab(null); - setSourceStackParam(null); - setNavStackParam(null); - setStackRootParam(null); + sidePanelStack.clearTrail(); // Clear waterfall search state on close, so that filters don't // persist when reopening another trace. clearTraceWaterfallSearchState(); onClose(); - }, [ - setSidePanelTab, - setSourceStackParam, - setNavStackParam, - setStackRootParam, - onClose, - clearTraceWaterfallSearchState, - ]); + }, [sidePanelStack, onClose, clearTraceWaterfallSearchState]); return ( - +
( - - - - -
- An error occurred while rendering this event. -
- -
- {error?.error?.message} -
- + fallbackRender={fallbackProps => ( + )} >
- + ); } diff --git a/packages/app/src/hooks/useSidePanelStack.ts b/packages/app/src/hooks/useSidePanelStack.ts index 8c6f5061c3..2ac6e9eeb9 100644 --- a/packages/app/src/hooks/useSidePanelStack.ts +++ b/packages/app/src/hooks/useSidePanelStack.ts @@ -192,22 +192,36 @@ export default function useSidePanelStack({ const setTab = useCallback((next: Tab) => setQueryTab(next), [setQueryTab]); const clearTrail = useCallback(() => { - setSourceStack(null); - setNavStack(null); - setStackRoot(null); - setQueryTab(null); + setSourceStack(null); // sidePanelSourceStack + setNavStack(null); // sidePanelNavStack + setStackRoot(null); // sidePanelStackRoot + setQueryTab(null); // sidePanelTab }, [setSourceStack, setNavStack, setStackRoot, setQueryTab]); - return { - sourceStack, - navStack, - isStale, - tab, - pushSource, - pushNav, - popOne, - truncateTo, - setTab, - clearTrail, - }; + return useMemo( + () => ({ + sourceStack, + navStack, + isStale, + tab, + pushSource, + pushNav, + popOne, + truncateTo, + setTab, + clearTrail, + }), + [ + clearTrail, + isStale, + navStack, + popOne, + pushNav, + pushSource, + setTab, + sourceStack, + tab, + truncateTo, + ], + ); } From edf37d35b9f7d57ad1f11bbb3737e91e01b11376 Mon Sep 17 00:00:00 2001 From: Karl Power Date: Tue, 7 Jul 2026 12:05:10 +0200 Subject: [PATCH 7/8] test fixes --- .../src/__tests__/SessionSidePanel.test.tsx | 7 ++-- .../DBRowSidePanel.missingSource.test.tsx | 29 +++++++++++---- .../DBRowSidePanel.staleStack.test.tsx | 36 ++++++++++++------- 3 files changed, 50 insertions(+), 22 deletions(-) diff --git a/packages/app/src/__tests__/SessionSidePanel.test.tsx b/packages/app/src/__tests__/SessionSidePanel.test.tsx index 9432a1aca9..5bf4b5e467 100644 --- a/packages/app/src/__tests__/SessionSidePanel.test.tsx +++ b/packages/app/src/__tests__/SessionSidePanel.test.tsx @@ -41,10 +41,13 @@ jest.mock('nuqs', () => { return { ...actual, // eslint-disable-next-line @eslint-react/no-unnecessary-use-prefix - useQueryState: (key: string) => + useQueryState: (key: string, parser?: { defaultValue?: unknown }) => key === 'sessionPanelEvent' ? [mockNuqs.sessionPanelEvent, mockNuqs.setSessionPanelEvent] - : [null, noop], + : // Mirror real nuqs: a parser built with `.withDefault(...)` never + // yields null. Honor its defaultValue so hooks like + // useSidePanelStack read empty arrays, not null. + [parser?.defaultValue ?? null, noop], }; }); diff --git a/packages/app/src/components/__tests__/DBRowSidePanel.missingSource.test.tsx b/packages/app/src/components/__tests__/DBRowSidePanel.missingSource.test.tsx index a342bdf8e3..44b7f844cd 100644 --- a/packages/app/src/components/__tests__/DBRowSidePanel.missingSource.test.tsx +++ b/packages/app/src/components/__tests__/DBRowSidePanel.missingSource.test.tsx @@ -1,4 +1,5 @@ import React from 'react'; +import { TSource } from '@hyperdx/common-utils/dist/types'; import { MantineProvider } from '@mantine/core'; import { fireEvent, render, screen } from '@testing-library/react'; @@ -141,8 +142,14 @@ jest.mock('@/useFormatTime', () => ({ // NOTE: this import is intentionally placed after the mock factories above, // which close over the `mock*` helpers declared at the top of this file. import { DBRowSidePanelInner } from '@/components/DBRowSidePanel'; +import useSidePanelStack from '@/hooks/useSidePanelStack'; -const ROOT_SOURCE = { id: 'log-src', kind: 'log', traceSourceId: 'trace-src' }; +// eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion +const ROOT_SOURCE = { + id: 'log-src', + kind: 'log', + traceSourceId: 'trace-src', +} as TSource; // A cross-source frame (e.g. "View Trace") whose source id no longer resolves. const MISSING_FRAME = { @@ -153,15 +160,23 @@ const MISSING_FRAME = { sourceKind: 'trace', }; +function InnerHarness({ rowId }: { rowId: string }) { + const sidePanelStack = useSidePanelStack({ initialRowId: rowId }); + return ( + + ); +} + function renderInner(rowId: string) { return render( - + , ); } diff --git a/packages/app/src/components/__tests__/DBRowSidePanel.staleStack.test.tsx b/packages/app/src/components/__tests__/DBRowSidePanel.staleStack.test.tsx index fa709a87fe..471f270e14 100644 --- a/packages/app/src/components/__tests__/DBRowSidePanel.staleStack.test.tsx +++ b/packages/app/src/components/__tests__/DBRowSidePanel.staleStack.test.tsx @@ -1,4 +1,5 @@ import React from 'react'; +import { TSource } from '@hyperdx/common-utils/dist/types'; import { MantineProvider } from '@mantine/core'; import { render } from '@testing-library/react'; @@ -130,8 +131,14 @@ jest.mock('@/useFormatTime', () => ({ // NOTE: this import is intentionally placed after the mock factories above, // which close over the `mock*` helpers declared at the top of this file. import { DBRowSidePanelInner } from '@/components/DBRowSidePanel'; +import useSidePanelStack from '@/hooks/useSidePanelStack'; -const ROOT_SOURCE = { id: 'log-src', kind: 'log', traceSourceId: 'trace-src' }; +// eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion +const ROOT_SOURCE = { + id: 'log-src', + kind: 'log', + traceSourceId: 'trace-src', +} as TSource; const TRACE_FRAME = { sourceId: 'trace-src', @@ -141,15 +148,23 @@ const TRACE_FRAME = { sourceKind: 'trace', }; +function InnerHarness({ rowId }: { rowId: string }) { + const sidePanelStack = useSidePanelStack({ initialRowId: rowId }); + return ( + + ); +} + function renderInner(rowId: string) { return render( - + , ); } @@ -216,12 +231,7 @@ describe('DBRowSidePanelInner — stale stack handling (HDX-3942 "Stale Stack Re // clicked another row in the same table); the trail is stale and ignored. rerender( - + , ); From 721aa5aa0bd85a798fe2a9fd3e7337c6cdff2a7b Mon Sep 17 00:00:00 2001 From: Karl Power Date: Tue, 7 Jul 2026 13:06:06 +0200 Subject: [PATCH 8/8] pushNav fix --- .../__tests__/useSidePanelStack.test.tsx | 32 +++++++++++++++++++ packages/app/src/hooks/useSidePanelStack.ts | 12 ++++++- 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/packages/app/src/hooks/__tests__/useSidePanelStack.test.tsx b/packages/app/src/hooks/__tests__/useSidePanelStack.test.tsx index 4ae50cb3ed..25c55f1f4a 100644 --- a/packages/app/src/hooks/__tests__/useSidePanelStack.test.tsx +++ b/packages/app/src/hooks/__tests__/useSidePanelStack.test.tsx @@ -189,6 +189,38 @@ describe('useSidePanelStack', () => { ]); expect(setterFor('sidePanelStackRoot')).toHaveBeenCalledWith('root-1'); expect(setterFor('sidePanelTab')).toHaveBeenCalledWith(Tab.Parsed); + // Re-persists the effective source stack (empty here) so a nav push never + // leaves a source stack it did not own behind. + expect(setterFor('sidePanelSourceStack')).toHaveBeenCalledWith([]); + }); + + it('pushNav on a stale trail clears the stale source stack it revives', () => { + // A leftover source stack owned by a different row is still in the URL. + seedParam('sidePanelSourceStack', [FRAME]); + seedParam('sidePanelStackRoot', 'old-root'); + const { result } = renderHook(() => + useSidePanelStack({ initialRowId: 'new-root' }), + ); + + act(() => + result.current.pushNav( + { rowId: 'ctx-row', aliasWith: [], label: 'Related' }, + Tab.Parsed, + ), + ); + + // The stale source stack is overwritten with the effective (empty) stack, so + // stamping the new owner below can't revive the old source frame. + expect(setterFor('sidePanelSourceStack')).toHaveBeenCalledWith([]); + expect(setterFor('sidePanelNavStack')).toHaveBeenCalledWith([ + { + rowId: 'ctx-row', + aliasWith: [], + label: 'Related', + originTab: undefined, + }, + ]); + expect(setterFor('sidePanelStackRoot')).toHaveBeenCalledWith('new-root'); }); it('popOne pops the nav leaf first, restoring its originTab', () => { diff --git a/packages/app/src/hooks/useSidePanelStack.ts b/packages/app/src/hooks/useSidePanelStack.ts index 2ac6e9eeb9..9ffd68a496 100644 --- a/packages/app/src/hooks/useSidePanelStack.ts +++ b/packages/app/src/hooks/useSidePanelStack.ts @@ -141,11 +141,21 @@ export default function useSidePanelStack({ const pushNav = useCallback( (entry: NavEntry, destinationTab: Tab) => { + setSourceStack(sourceStack); setNavStack([...navStack, { ...entry, originTab: tab ?? undefined }]); setStackRoot(initialRowId ?? null); setQueryTab(destinationTab); }, - [navStack, tab, initialRowId, setNavStack, setStackRoot, setQueryTab], + [ + sourceStack, + navStack, + tab, + initialRowId, + setSourceStack, + setNavStack, + setStackRoot, + setQueryTab, + ], ); const popOne = useCallback((): 'nav' | 'source' | 'none' => {