Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -563,11 +563,9 @@ describe('TraceTimelineViewer/duck', () => {
expect(retained.isResourceOpen).toBe(true);
});

it('persists mode to localStorage', () => {
it('does not persist mode to localStorage (persistence moved to useTraceTimelineStore)', () => {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All changes in this file are similar, but the alternative approach would be to check that the settings DID persist, just in a different storage.

Having said that, detailPanelMode and timelineVisible are absolutely meant to be persisted to local storage as sticky configuration options (user profile style), they are not supposed to be lost when UI reloads.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

btw a future enhancement: #3720

Copy link
Copy Markdown
Member Author

@Parship12 Parship12 Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the tests are edited, just to stop them from failing.

The localStorage persistence is still there, it's now handled by useTraceTimelineStore, which is tested in src/stores/trace-timeline-store.test.ts.

The duck tests for these 4 actions are temporary, the duck itself will be deleted in next sub-PRs along with the remaining redux state. At that point, the only persistence tests will be the ones in trace-timeline-store.test.ts

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

btw a future enhancement: #3720

our Zustand store is already a good foundation for it.

store.dispatch(actions.setDetailPanelMode('sidepanel'));
expect(localStorage.getItem('detailPanelMode')).toBe('sidepanel');
store.dispatch(actions.setDetailPanelMode('inline'));
expect(localStorage.getItem('detailPanelMode')).toBe('inline');
expect(localStorage.getItem('detailPanelMode')).toBeNull();
});
});

Expand All @@ -583,11 +581,9 @@ describe('TraceTimelineViewer/duck', () => {
expect(store.getState().timelineBarsVisible).toBe(true);
});

it('persists visibility to localStorage', () => {
it('does not persist visibility to localStorage (persistence moved to useTraceTimelineStore)', () => {
store.dispatch(actions.setTimelineBarsVisible(false));
expect(localStorage.getItem('timelineVisible')).toBe('false');
store.dispatch(actions.setTimelineBarsVisible(true));
expect(localStorage.getItem('timelineVisible')).toBe('true');
expect(localStorage.getItem('timelineVisible')).toBeNull();
});
});

Expand All @@ -597,21 +593,19 @@ describe('TraceTimelineViewer/duck', () => {
expect(store.getState().sidePanelWidth).toBe(0.3);
});

it('persists width to localStorage', () => {
it('does not persist width to localStorage (persistence moved to useTraceTimelineStore)', () => {
store.dispatch(actions.setSidePanelWidth(0.3));
expect(localStorage.getItem('sidePanelWidth')).toBe('0.3');
expect(localStorage.getItem('sidePanelWidth')).toBeNull();
});

it('clamps to SIDE_PANEL_WIDTH_MIN when width is below range', () => {
store.dispatch(actions.setSidePanelWidth(0));
expect(store.getState().sidePanelWidth).toBe(SIDE_PANEL_WIDTH_MIN);
expect(localStorage.getItem('sidePanelWidth')).toBe(String(SIDE_PANEL_WIDTH_MIN));
});

it('clamps to SIDE_PANEL_WIDTH_MAX when width is above range', () => {
store.dispatch(actions.setSidePanelWidth(1));
expect(store.getState().sidePanelWidth).toBe(SIDE_PANEL_WIDTH_MAX);
expect(localStorage.getItem('sidePanelWidth')).toBe(String(SIDE_PANEL_WIDTH_MAX));
});

it('clamps side panel width to leave room for the name column and timeline', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,22 @@ import getConfig from '../../../utils/config/get-config';
import guardReducer from '../../../utils/guardReducer';
import spanAncestorIds from '../../../utils/span-ancestor-ids';

export const SPAN_NAME_COLUMN_WIDTH_MIN = 0.15;
export const SPAN_NAME_COLUMN_WIDTH_MAX = 0.85;
export const SIDE_PANEL_WIDTH_MIN = 0.2;
export const SIDE_PANEL_WIDTH_MAX = 0.7;
// Minimum fraction of page width reserved for the timeline bars column when side panel is visible.
export const MIN_TIMELINE_COLUMN_WIDTH = 0.05;
// Constants are canonical in trace-timeline-store; imported for use in this file and re-exported
// for backward compatibility with other files that import them from duck.
export {
SPAN_NAME_COLUMN_WIDTH_MIN,
SPAN_NAME_COLUMN_WIDTH_MAX,
SIDE_PANEL_WIDTH_MIN,
SIDE_PANEL_WIDTH_MAX,
MIN_TIMELINE_COLUMN_WIDTH,
} from '../../../stores/trace-timeline-store';
import {
SPAN_NAME_COLUMN_WIDTH_MIN,
SPAN_NAME_COLUMN_WIDTH_MAX,
SIDE_PANEL_WIDTH_MIN,
SIDE_PANEL_WIDTH_MAX,
MIN_TIMELINE_COLUMN_WIDTH,
} from '../../../stores/trace-timeline-store';

// payloads
export type TSpanIdLogValue = { logItem: IEvent; spanID: string };
Expand Down Expand Up @@ -248,7 +258,6 @@ function setColumnWidth(state: TTraceTimeline, { width }: TWidthValue): TTraceTi
? Math.min(SPAN_NAME_COLUMN_WIDTH_MAX, 1 - state.sidePanelWidth - MIN_TIMELINE_COLUMN_WIDTH)
: SPAN_NAME_COLUMN_WIDTH_MAX;
const spanNameColumnWidth = Math.min(Math.max(width, SPAN_NAME_COLUMN_WIDTH_MIN), maxWidth);
localStorage.setItem('spanNameColumnWidth', spanNameColumnWidth.toString());
return { ...state, spanNameColumnWidth };
}

Expand Down Expand Up @@ -344,7 +353,6 @@ function detailToggle(state: TTraceTimeline, { spanID }: TSpanIdValue) {
}

function setDetailPanelMode(state: TTraceTimeline, { mode }: TDetailPanelModeValue): TTraceTimeline {
localStorage.setItem('detailPanelMode', mode);
let { detailStates } = state;
// When switching to sidepanel mode, keep at most one entry in detailStates and upgrade its
// DetailState to side-panel defaults so Attributes/Resource are expanded by default,
Expand All @@ -368,8 +376,6 @@ function setDetailPanelMode(state: TTraceTimeline, { mode }: TDetailPanelModeVal
}

function setTimelineBarsVisible(state: TTraceTimeline, { visible }: TTimelineVisibleValue): TTraceTimeline {
// localStorage key kept as 'timelineVisible' for backward compatibility with stored user preferences.
localStorage.setItem('timelineVisible', String(visible));
return { ...state, timelineBarsVisible: visible };
}

Expand All @@ -381,7 +387,6 @@ function setSidePanelWidth(state: TTraceTimeline, { width }: TWidthValue): TTrac
: 1 - state.spanNameColumnWidth;
const maxWidth = Math.max(SIDE_PANEL_WIDTH_MIN, Math.min(SIDE_PANEL_WIDTH_MAX, availableWidth));
const sidePanelWidth = Math.min(Math.max(width, SIDE_PANEL_WIDTH_MIN), maxWidth);
localStorage.setItem('sidePanelWidth', sidePanelWidth.toString());
return { ...state, sidePanelWidth };
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,28 @@ import * as KeyboardShortcuts from '../keyboard-shortcuts';
import traceGenerator from '../../../demo/trace-generators';
import transformTraceData from '../../../model/transform-trace-data';

const { mockTraceTimelineStore } = vi.hoisted(() => ({
mockTraceTimelineStore: {
spanNameColumnWidth: 0.25,
sidePanelWidth: 0.375,
detailPanelMode: 'inline',
timelineBarsVisible: true,
setSpanNameColumnWidth: vi.fn(),
setSidePanelWidth: vi.fn(),
setDetailPanelMode: vi.fn(),
setTimelineBarsVisible: vi.fn(),
},
}));

vi.mock('../../../stores/trace-timeline-store', () => ({
useTraceTimelineStore: vi.fn(selector => selector(mockTraceTimelineStore)),
SPAN_NAME_COLUMN_WIDTH_MIN: 0.15,
SPAN_NAME_COLUMN_WIDTH_MAX: 0.85,
SIDE_PANEL_WIDTH_MIN: 0.2,
SIDE_PANEL_WIDTH_MAX: 0.7,
MIN_TIMELINE_COLUMN_WIDTH: 0.05,
}));

vi.mock('./VirtualizedTraceView', () => mockDefault(() => <div data-testid="virtualized-trace-view-mock" />));
vi.mock('./SpanDetailSidePanel', () => mockDefault(() => <div data-testid="span-detail-side-panel-mock" />));
vi.mock('../../common/VerticalResizer', () => ({
Expand Down Expand Up @@ -50,9 +72,6 @@ describe('<TraceTimelineViewer>', () => {
current: [0, 1],
},
},
spanNameColumnWidth: 0.5,
sidePanelWidth: 0.3,
timelineBarsVisible: true,
expandAll: jest.fn(),
collapseAll: jest.fn(),
expandOne: jest.fn(),
Expand All @@ -62,7 +81,7 @@ describe('<TraceTimelineViewer>', () => {
};

const defaultState = {
traceTimeline: { spanNameColumnWidth: 0.25 },
traceTimeline: { detailStates: new Map() },
};

function renderWithRedux(ui, { initialState = defaultState } = {}) {
Expand All @@ -79,6 +98,14 @@ describe('<TraceTimelineViewer>', () => {
props.collapseAll.mockClear();
props.expandOne.mockClear();
props.collapseOne.mockClear();
props.setSpanNameColumnWidth.mockClear();
props.setSidePanelWidth.mockClear();
mockTraceTimelineStore.setSpanNameColumnWidth.mockClear();
mockTraceTimelineStore.setSidePanelWidth.mockClear();
mockTraceTimelineStore.spanNameColumnWidth = 0.25;
mockTraceTimelineStore.sidePanelWidth = 0.375;
mockTraceTimelineStore.detailPanelMode = 'inline';
mockTraceTimelineStore.timelineBarsVisible = true;
jest.spyOn(KeyboardShortcuts, 'merge').mockClear();
});

Expand All @@ -94,13 +121,11 @@ describe('<TraceTimelineViewer>', () => {
it('mapStateToProps derives selectedSpanID from non-empty detailStates', () => {
// The root span is selected → selectedSpanID === rootSpanID → label should be 'Trace Root'.
const spanID = trace.rootSpans[0].spanID;
mockTraceTimelineStore.detailPanelMode = 'sidepanel';
mockTraceTimelineStore.sidePanelWidth = 0.3;
renderWithRedux(<TraceTimelineViewer {...props} />, {
initialState: {
traceTimeline: {
spanNameColumnWidth: 0.25,
sidePanelWidth: 0.3,
timelineBarsVisible: true,
detailPanelMode: 'sidepanel',
detailStates: new Map([[spanID, {}]]),
},
},
Expand Down Expand Up @@ -165,14 +190,9 @@ describe('<TraceTimelineViewer>', () => {

combinations.forEach(({ detailPanelMode, timelineBarsVisible }) => {
it(`detailPanelMode=${detailPanelMode}, timelineBarsVisible=${timelineBarsVisible}`, () => {
render(
<TraceTimelineViewerImpl
{...props}
detailPanelMode={detailPanelMode}
timelineBarsVisible={timelineBarsVisible}
selectedSpanID={null}
/>
);
mockTraceTimelineStore.detailPanelMode = detailPanelMode;
mockTraceTimelineStore.timelineBarsVisible = timelineBarsVisible;
render(<TraceTimelineViewerImpl {...props} selectedSpanID={null} />);

const sidePanelActive = detailPanelMode === 'sidepanel';
const resizerExpected = sidePanelActive && timelineBarsVisible;
Expand All @@ -196,47 +216,46 @@ describe('<TraceTimelineViewer>', () => {
});

describe('side panel mode', () => {
const sidePanelProps = {
...props,
detailPanelMode: 'sidepanel',
selectedSpanID: null,
};
beforeEach(() => {
mockTraceTimelineStore.detailPanelMode = 'sidepanel';
mockTraceTimelineStore.timelineBarsVisible = true;
});

it('renders the side panel layout when detailPanelMode is sidepanel', () => {
render(<TraceTimelineViewerImpl {...sidePanelProps} />);
render(<TraceTimelineViewerImpl {...props} selectedSpanID={null} />);
expect(screen.getByTestId('span-detail-side-panel-mock')).toBeInTheDocument();
expect(screen.getByTestId('virtualized-trace-view-mock')).toBeInTheDocument();
});

it('renders a VerticalResizer between main and side panel when timeline bars are visible', () => {
render(<TraceTimelineViewerImpl {...sidePanelProps} />);
render(<TraceTimelineViewerImpl {...props} selectedSpanID={null} />);
expect(screen.getByTestId('vertical-resizer-mock')).toBeInTheDocument();
});

it('does not render a VerticalResizer when timeline bars are hidden', () => {
render(<TraceTimelineViewerImpl {...sidePanelProps} timelineBarsVisible={false} />);
mockTraceTimelineStore.timelineBarsVisible = false;
render(<TraceTimelineViewerImpl {...props} selectedSpanID={null} />);
expect(screen.queryByTestId('vertical-resizer-mock')).not.toBeInTheDocument();
});

it('calls setSidePanelWidth when the VerticalResizer onChange fires', () => {
render(<TraceTimelineViewerImpl {...sidePanelProps} />);
it('calls setSidePanelWidth (Zustand + Redux) when the VerticalResizer onChange fires', () => {
render(<TraceTimelineViewerImpl {...props} selectedSpanID={null} />);
fireEvent.click(screen.getByTestId('vertical-resizer-change'));
// onChange receives newPosition=0.7 → setSidePanelWidth(1 - 0.7 ≈ 0.3)
expect(mockTraceTimelineStore.setSidePanelWidth).toHaveBeenCalledTimes(1);
expect(mockTraceTimelineStore.setSidePanelWidth.mock.calls[0][0]).toBeCloseTo(0.3);
expect(props.setSidePanelWidth).toHaveBeenCalledTimes(1);
expect(props.setSidePanelWidth.mock.calls[0][0]).toBeCloseTo(0.3);
});

it('uses "Trace Root" label when selectedSpanID is null', () => {
// sidePanelLabel computation: selectedSpanID===null → 'Trace Root'
// The label is passed to TimelineHeaderRow which is mocked, so we verify it indirectly
// by confirming the component renders without throwing.
render(<TraceTimelineViewerImpl {...sidePanelProps} selectedSpanID={null} />);
render(<TraceTimelineViewerImpl {...props} selectedSpanID={null} />);
expect(screen.getByTestId('span-detail-side-panel-mock')).toBeInTheDocument();
});

it('uses "Span Details" label when selectedSpanID is a non-root span', () => {
const nonRootSpanID = trace.spans[1]?.spanID ?? 'some-span';
render(<TraceTimelineViewerImpl {...sidePanelProps} selectedSpanID={nonRootSpanID} />);
render(<TraceTimelineViewerImpl {...props} selectedSpanID={nonRootSpanID} />);
expect(screen.getByTestId('span-detail-side-panel-mock')).toBeInTheDocument();
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,14 @@ import React, { useCallback, useEffect, useLayoutEffect, useRef, useState } from
import { connect } from 'react-redux';
import { bindActionCreators, Dispatch } from 'redux';

import { actions, getSelectedSpanID } from './duck';
import { useTraceTimelineStore } from '../../../stores/trace-timeline-store';
import {
actions,
getSelectedSpanID,
MIN_TIMELINE_COLUMN_WIDTH,
SIDE_PANEL_WIDTH_MAX,
SIDE_PANEL_WIDTH_MIN,
SPAN_NAME_COLUMN_WIDTH_MAX,
} from './duck';
} from '../../../stores/trace-timeline-store';
import SpanDetailSidePanel from './SpanDetailSidePanel';
import TimelineHeaderRow from './TimelineHeaderRow';
import VirtualizedTraceView from './VirtualizedTraceView';
Expand All @@ -27,6 +27,7 @@ import { CriticalPathSection } from '../../../types/critical_path';
import './index.css';

type TDispatchProps = {
// These Redux dispatchers are kept for the tracking middleware only.
setSpanNameColumnWidth: (width: number) => void;
setSidePanelWidth: (width: number) => void;
collapseAll: (spans: ReadonlyArray<IOtelSpan>) => void;
Expand All @@ -39,10 +40,6 @@ type TProps = TDispatchProps & {
registerAccessors: (accessors: Accessors) => void;
findMatchesIDs: Set<string> | TNil;
scrollToFirstVisibleSpan: () => void;
detailPanelMode: 'inline' | 'sidepanel';
sidePanelWidth: number;
spanNameColumnWidth: number;
timelineBarsVisible: boolean;
selectedSpanID: string | null;
trace: IOtelTrace;
criticalPath: CriticalPathSection[];
Expand All @@ -67,21 +64,41 @@ export const TraceTimelineViewerImpl = (props: TProps) => {
collapseOne: collapseOneAction,
expandAll: expandAllAction,
expandOne: expandOneAction,
setSpanNameColumnWidth,
setSidePanelWidth,
setSpanNameColumnWidth: reduxSetSpanNameColumnWidth,
setSidePanelWidth: reduxSetSidePanelWidth,
updateNextViewRangeTime,
updateViewRangeTime,
viewRange,
trace,
detailPanelMode,
sidePanelWidth,
spanNameColumnWidth,
timelineBarsVisible,
selectedSpanID,
useOtelTerms,
...rest
} = props;

// Layout preferences are owned by Zustand; Redux setters are also called for the tracking middleware.
const detailPanelMode = useTraceTimelineStore(s => s.detailPanelMode);
const sidePanelWidth = useTraceTimelineStore(s => s.sidePanelWidth);
const spanNameColumnWidth = useTraceTimelineStore(s => s.spanNameColumnWidth);
const timelineBarsVisible = useTraceTimelineStore(s => s.timelineBarsVisible);
const zustandSetSpanNameColumnWidth = useTraceTimelineStore(s => s.setSpanNameColumnWidth);
const zustandSetSidePanelWidth = useTraceTimelineStore(s => s.setSidePanelWidth);

const setSpanNameColumnWidth = useCallback(
(width: number) => {
zustandSetSpanNameColumnWidth(width);
reduxSetSpanNameColumnWidth(width);
},
[zustandSetSpanNameColumnWidth, reduxSetSpanNameColumnWidth]
);

const setSidePanelWidth = useCallback(
(width: number) => {
zustandSetSidePanelWidth(width);
reduxSetSidePanelWidth(width);
},
[zustandSetSidePanelWidth, reduxSetSidePanelWidth]
);

// Side panel is permanently visible whenever side panel mode is active.
const sidePanelActive = detailPanelMode === 'sidepanel';

Expand Down Expand Up @@ -246,15 +263,9 @@ export const TraceTimelineViewerImpl = (props: TProps) => {
};

function mapStateToProps(state: ReduxState) {
const {
detailPanelMode,
sidePanelWidth,
spanNameColumnWidth,
timelineBarsVisible,
detailStates = new Map(),
} = state.traceTimeline;
const { detailStates = new Map() } = state.traceTimeline;
const selectedSpanID = getSelectedSpanID(detailStates);
return { detailPanelMode, sidePanelWidth, spanNameColumnWidth, timelineBarsVisible, selectedSpanID };
return { selectedSpanID };
}

function mapDispatchToProps(dispatch: Dispatch<ReduxState>): TDispatchProps {
Expand Down
Loading
Loading