diff --git a/static/app/utils/useDispatchingReducer.tsx b/static/app/utils/useDispatchingReducer.tsx index 1a1665916414f..aa4a4f1211ed2 100644 --- a/static/app/utils/useDispatchingReducer.tsx +++ b/static/app/utils/useDispatchingReducer.tsx @@ -126,11 +126,12 @@ export function useDispatchingReducer>( // we are likely going to have to rethrow async and lose stack traces... actionQueue.current.push(a); - if (updatesRef.current !== null) { + if (updatesRef.current) { window.cancelAnimationFrame(updatesRef.current); + updatesRef.current = null; } - window.requestAnimationFrame(() => { + updatesRef.current = window.requestAnimationFrame(() => { setState(s => { const next = update(s, actionQueue.current, reducerRef.current, emitter); stateRef.current = next; diff --git a/static/app/views/performance/newTraceDetails/index.tsx b/static/app/views/performance/newTraceDetails/index.tsx index 6e560999963d0..628cfec5d3fbf 100644 --- a/static/app/views/performance/newTraceDetails/index.tsx +++ b/static/app/views/performance/newTraceDetails/index.tsx @@ -441,6 +441,7 @@ export function TraceViewWaterfall(props: TraceViewWaterfallProps) { ) => { if (searchingRaf.current?.id) { window.cancelAnimationFrame(searchingRaf.current.id); + searchingRaf.current = null; } function done([matches, lookup, activeNodeSearchResult]) { diff --git a/static/app/views/performance/newTraceDetails/trace.spec.tsx b/static/app/views/performance/newTraceDetails/trace.spec.tsx index 09bbf5f7e54ee..b69d838beae3f 100644 --- a/static/app/views/performance/newTraceDetails/trace.spec.tsx +++ b/static/app/views/performance/newTraceDetails/trace.spec.tsx @@ -6,7 +6,6 @@ import {initializeOrg} from 'sentry-test/initializeOrg'; import { findAllByText, findByText, - fireEvent, render, screen, userEvent, @@ -37,17 +36,18 @@ class MockResizeObserver { } observe(element: HTMLElement) { - // Executes in sync so we dont have to - this.callback( - [ - { - target: element, - // @ts-expect-error partial mock - contentRect: {width: 1000, height: 24 * 20 - 1}, - }, - ], - this - ); + setTimeout(() => { + this.callback( + [ + { + target: element, + // @ts-expect-error partial mock + contentRect: {width: 1000, height: 24 * 20 - 1}, + }, + ], + this + ); + }, 0); } disconnect() {} } @@ -229,9 +229,13 @@ function getVirtualizedScrollContainer(): HTMLElement { return virtualizedScrollContainer; } +function getVirtualizedRows(container: HTMLElement) { + return Array.from(container.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR)); +} + async function keyboardNavigationTestSetup() { const keyboard_navigation_transactions: TraceFullDetailed[] = []; - for (let i = 0; i < 1e4; i++) { + for (let i = 0; i < 1e2; i++) { keyboard_navigation_transactions.push( makeTransaction({ span_id: i + '', @@ -241,7 +245,7 @@ async function keyboardNavigationTestSetup() { project_slug: 'project_slug', }) ); - mockTransactionDetailsResponse(i.toString()); + mockTransactionDetailsResponse(`${i}`); } mockTraceResponse({ body: { @@ -271,7 +275,12 @@ async function keyboardNavigationTestSetup() { const virtualizedScrollContainer = getVirtualizedScrollContainer(); // Awaits for the placeholder rendering rows to be removed - expect(await findByText(value.container, /transaction-op-0/i)).toBeInTheDocument(); + await findAllByText(virtualizedContainer, /transaction-op-/i, undefined, { + timeout: 5000, + }).catch(e => { + printVirtualizedList(virtualizedContainer); + throw e; + }); return {...value, virtualizedContainer, virtualizedScrollContainer}; } @@ -287,14 +296,17 @@ async function pageloadTestSetup() { project_slug: 'project_slug', }) ); - mockTransactionDetailsResponse(i.toString()); + + mockTransactionDetailsResponse(`${i}`); } + mockTraceResponse({ body: { transactions: pageloadTransactions, orphan_errors: [], }, }); + mockTraceMetaResponse({ body: { errors: 0, @@ -317,7 +329,12 @@ async function pageloadTestSetup() { const virtualizedScrollContainer = getVirtualizedScrollContainer(); // Awaits for the placeholder rendering rows to be removed - expect((await screen.findAllByText(/transaction-op-/i)).length).toBeGreaterThan(0); + await findAllByText(virtualizedContainer, /transaction-op-/i, undefined, { + timeout: 5000, + }).catch(e => { + printVirtualizedList(virtualizedContainer); + throw e; + }); return {...value, virtualizedContainer, virtualizedScrollContainer}; } @@ -347,7 +364,7 @@ async function nestedTransactionsTestSetup() { txn = next; transactions.push(next); - mockTransactionDetailsResponse(i.toString()); + mockTransactionDetailsResponse(`${i}`); } mockTraceResponse({ @@ -367,7 +384,13 @@ async function nestedTransactionsTestSetup() { const virtualizedScrollContainer = getVirtualizedScrollContainer(); // Awaits for the placeholder rendering rows to be removed - expect((await screen.findAllByText(/transaction-op-/i)).length).toBeGreaterThan(0); + await findAllByText(virtualizedContainer, /transaction-op-/i, undefined, { + timeout: 5000, + }).catch(e => { + printVirtualizedList(virtualizedContainer); + throw e; + }); + return {...value, virtualizedContainer, virtualizedScrollContainer}; } @@ -383,7 +406,7 @@ async function searchTestSetup() { project_slug: 'project_slug', }) ); - mockTransactionDetailsResponse(i.toString()); + mockTransactionDetailsResponse(`${i}`); } mockTraceResponse({ body: { @@ -415,7 +438,12 @@ async function searchTestSetup() { const virtualizedScrollContainer = getVirtualizedScrollContainer(); // Awaits for the placeholder rendering rows to be removed - expect(await findByText(value.container, /transaction-op-0/i)).toBeInTheDocument(); + await findAllByText(virtualizedContainer, /transaction-op-/i, undefined, { + timeout: 5000, + }).catch(e => { + printVirtualizedList(virtualizedContainer); + throw e; + }); return {...value, virtualizedContainer, virtualizedScrollContainer}; } @@ -437,7 +465,7 @@ async function simpleTestSetup() { transactions.push(next); } parent = next; - mockTransactionDetailsResponse(i.toString()); + mockTransactionDetailsResponse(`${i}`); } mockTraceResponse({ body: { @@ -467,7 +495,12 @@ async function simpleTestSetup() { const virtualizedScrollContainer = getVirtualizedScrollContainer(); // Awaits for the placeholder rendering rows to be removed - expect(await findByText(value.container, /transaction-op-0/i)).toBeInTheDocument(); + await findAllByText(virtualizedContainer, /transaction-op-/i, undefined, { + timeout: 5000, + }).catch(e => { + printVirtualizedList(virtualizedContainer); + throw e; + }); return {...value, virtualizedContainer, virtualizedScrollContainer}; } @@ -671,7 +704,12 @@ async function completeTestSetup() { const virtualizedScrollContainer = getVirtualizedScrollContainer(); // Awaits for the placeholder rendering rows to be removed - expect(await findByText(value.container, /transaction-op-0/i)).toBeInTheDocument(); + await findAllByText(virtualizedContainer, /transaction-op-/i, undefined, { + timeout: 5000, + }).catch(e => { + printVirtualizedList(virtualizedContainer); + throw e; + }); return {...value, virtualizedContainer, virtualizedScrollContainer}; } @@ -680,16 +718,10 @@ const DRAWER_TABS_PIN_BUTTON_TEST_ID = 'trace-drawer-tab-pin-button'; const VISIBLE_TRACE_ROW_SELECTOR = '.TraceRow:not(.Hidden)'; const ACTIVE_SEARCH_HIGHLIGHT_ROW = '.TraceRow.SearchResult.Highlight:not(.Hidden)'; -const wait = (ms: number) => new Promise(resolve => setTimeout(resolve, ms)); -const searchToUpdate = async (): Promise => { - await wait(500); -}; -const scrollToEnd = async (): Promise => { - await wait(500); +const searchToResolve = async (): Promise => { + await screen.findByTestId('trace-search-success'); }; -// @ts-expect-error ignore this line -// eslint-disable-next-line function printVirtualizedList(container: HTMLElement) { const stdout: string[] = []; const scrollContainer = screen.queryByTestId( @@ -698,18 +730,32 @@ function printVirtualizedList(container: HTMLElement) { const rows = Array.from(container.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR)); const searchResultIterator = screen.queryByTestId('trace-search-result-iterator'); + const searchInput = screen.queryByPlaceholderText( + 'Search in trace' + ) as HTMLInputElement; + const loading = screen.queryByTestId('trace-search-loading'); + const success = screen.queryByTestId('trace-search-success'); + stdout.push( - 'Scroll Container: ' + - 'top=' + - scrollContainer.scrollTop + + 'Debug Information: ' + + 'Rows=' + + rows.length + ' ' + - 'left=' + - scrollContainer.scrollLeft + + 'Search Query:' + + (searchInput?.value || '') + ' ' + - rows.length + + (searchResultIterator?.textContent || '') + + ' ' + + 'Search Status:' + + (loading ? 'loading' : success ? 'success' : '') + ' ' + - 'Search:' + - (searchResultIterator?.textContent ?? '') + 'Scroll=' + + 'top:' + + scrollContainer.scrollTop + + ' ' + + 'left:' + + scrollContainer.scrollLeft + + ' ' ); for (const r of [...rows]) { @@ -759,17 +805,24 @@ function printTabs() { console.log(stdout.join(' | ')); } -function assertHighlightedRowAtIndex(virtualizedContainer: HTMLElement, index: number) { - expect(virtualizedContainer.querySelectorAll('.TraceRow.Highlight')).toHaveLength(1); - const highlighted_row = virtualizedContainer.querySelector(ACTIVE_SEARCH_HIGHLIGHT_ROW); - const r = Array.from(virtualizedContainer.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR)); - expect(r.indexOf(highlighted_row!)).toBe(index); +async function assertHighlightedRowAtIndex( + virtualizedContainer: HTMLElement, + index: number +) { + await waitFor(() => { + expect(virtualizedContainer.querySelectorAll('.TraceRow.Highlight')).toHaveLength(1); + const highlighted_row = virtualizedContainer.querySelector( + ACTIVE_SEARCH_HIGHLIGHT_ROW + ); + const r = Array.from( + virtualizedContainer.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR) + ); + expect(r.indexOf(highlighted_row!)).toBe(index); + }); } -// biome-ignore lint/suspicious/noSkippedTests: -describe.skip('trace view', () => { +describe('trace view', () => { beforeEach(() => { - jest.spyOn(console, 'error').mockImplementation(() => {}); globalThis.ResizeObserver = MockResizeObserver as any; mockQueryString(''); MockDate.reset(); @@ -834,7 +887,7 @@ describe.skip('trace view', () => { mockQueryString('?node=trace-root'); const {virtualizedContainer} = await completeTestSetup(); await waitFor(() => { - const rows = virtualizedContainer.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR); + const rows = getVirtualizedRows(virtualizedContainer); expect(rows[0]).toHaveFocus(); }); }); @@ -843,7 +896,7 @@ describe.skip('trace view', () => { mockQueryString('?node=txn-1'); const {virtualizedContainer} = await completeTestSetup(); await waitFor(() => { - const rows = virtualizedContainer.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR); + const rows = getVirtualizedRows(virtualizedContainer); expect(rows[2]).toHaveFocus(); }); }); @@ -856,7 +909,7 @@ describe.skip('trace view', () => { await waitFor(() => { // We need to await a tick because the row is not focused until the next tick - const rows = virtualizedContainer.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR); + const rows = getVirtualizedRows(virtualizedContainer); expect(rows[3]).toHaveFocus(); expect(rows[3].textContent?.includes('http — request')).toBe(true); }); @@ -870,7 +923,7 @@ describe.skip('trace view', () => { await waitFor(() => { // We need to await a tick because the row is not focused until the next tick - const rows = virtualizedContainer.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR); + const rows = getVirtualizedRows(virtualizedContainer); expect(rows[4]).toHaveFocus(); expect(rows[4].textContent?.includes('Autogrouped')).toBe(true); }); @@ -883,7 +936,7 @@ describe.skip('trace view', () => { await waitFor(() => { // We need to await a tick because the row is not focused until the next tick - const rows = virtualizedContainer.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR); + const rows = getVirtualizedRows(virtualizedContainer); expect(rows[5]).toHaveFocus(); expect(rows[5].textContent?.includes('db — redis')).toBe(true); }); @@ -896,7 +949,7 @@ describe.skip('trace view', () => { await findAllByText(virtualizedContainer, /Autogrouped/i); await waitFor(() => { // We need to await a tick because the row is not focused until the next tick - const rows = virtualizedContainer.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR); + const rows = getVirtualizedRows(virtualizedContainer); expect(rows[5]).toHaveFocus(); expect(rows[5].textContent?.includes('5Autogrouped')).toBe(true); }); @@ -910,7 +963,7 @@ describe.skip('trace view', () => { await waitFor(() => { // We need to await a tick because the row is not focused until the next tick - const rows = virtualizedContainer.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR); + const rows = getVirtualizedRows(virtualizedContainer); expect(rows[6]).toHaveFocus(); expect(rows[6].textContent?.includes('http — request')).toBe(true); }); @@ -923,7 +976,7 @@ describe.skip('trace view', () => { await findAllByText(virtualizedContainer, /Autogrouped/i); // We need to await a tick because the row is not focused until the next ticks - const rows = virtualizedContainer.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR); + const rows = getVirtualizedRows(virtualizedContainer); await waitFor(() => { expect(rows[7]).toHaveFocus(); @@ -939,7 +992,7 @@ describe.skip('trace view', () => { await waitFor(() => { // We need to await a tick because the row is not focused until the next ticks - const rows = virtualizedContainer.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR); + const rows = getVirtualizedRows(virtualizedContainer); expect(rows[11]).toHaveFocus(); expect(rows[11].textContent?.includes('error-title')).toBe(true); }); @@ -950,7 +1003,7 @@ describe.skip('trace view', () => { const {virtualizedContainer} = await completeTestSetup(); await waitFor(() => { - const rows = virtualizedContainer.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR); + const rows = getVirtualizedRows(virtualizedContainer); expect(rows[2]).toHaveFocus(); }); }); @@ -961,7 +1014,7 @@ describe.skip('trace view', () => { await findAllByText(virtualizedContainer, /Autogrouped/i); await waitFor(() => { - const rows = virtualizedContainer.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR); + const rows = getVirtualizedRows(virtualizedContainer); expect(rows[3]).toHaveFocus(); expect(rows[3].textContent?.includes('http — request')).toBe(true); }); @@ -1010,8 +1063,7 @@ describe.skip('trace view', () => { expect(screen.queryByText(/Missing instrumentation/i)).not.toBeInTheDocument(); }); - // biome-ignore lint/suspicious/noSkippedTests: @JonasBa will fix these flakey tests soon - describe.skip('preferences', () => { + describe('preferences', () => { it('toggles autogrouping', async () => { mockTracePreferences({autogroup: {parent: true, sibling: true}}); mockQueryString('?node=span-span0&node=txn-1'); @@ -1022,17 +1074,19 @@ describe.skip('trace view', () => { const preferencesDropdownTrigger = screen.getByLabelText('Trace Preferences'); await userEvent.click(preferencesDropdownTrigger); + expect(await screen.findByText('Autogrouping')).toBeInTheDocument(); + // Toggle autogrouping off - await userEvent.click(await screen.findByText('Autogrouping')); + const autogroupingOption = await screen.findByText('Autogrouping'); + await userEvent.click(autogroupingOption); + await waitFor(() => { expect(screen.queryByText('Autogrouped')).not.toBeInTheDocument(); }); - // Toggle autogrouping on + // Toggle autogrouping back on await userEvent.click(await screen.findByText('Autogrouping')); - await waitFor(() => { - expect(screen.queryAllByText('Autogrouped')).toHaveLength(2); - }); + expect(await screen.findAllByText('Autogrouped')).toHaveLength(2); }); it('toggles missing instrumentation', async () => { @@ -1046,62 +1100,27 @@ describe.skip('trace view', () => { // Toggle missing instrumentation off await userEvent.click(preferencesDropdownTrigger); - const missingInstrumentationOption = await screen.findByText( - 'Missing Instrumentation' - ); + + expect(await screen.findByText('Missing Instrumentation')).toBeInTheDocument(); // Toggle missing instrumentation off - await userEvent.click(missingInstrumentationOption); + await userEvent.click(await screen.findByText('Missing Instrumentation')); + await waitFor(() => { expect(screen.queryByText('Missing instrumentation')).not.toBeInTheDocument(); }); // Toggle missing instrumentation on - await userEvent.click(missingInstrumentationOption); - await waitFor(() => { - expect(screen.queryByText('Missing instrumentation')).toBeInTheDocument(); - }); + await userEvent.click(await screen.findByText('Missing Instrumentation')); + expect(await screen.findAllByText('Missing instrumentation')).toHaveLength(1); }); }); - - it('triggers search on load but does not steal focus from node param', async () => { - mockQueryString('?search=transaction-op-999&node=txn-0'); - - const {virtualizedContainer} = await pageloadTestSetup(); - const searchInput = await screen.findByPlaceholderText('Search in trace'); - expect(searchInput).toHaveValue('transaction-op-999'); - - await waitFor(() => { - expect(screen.queryByTestId('trace-search-result-iterator')).toHaveTextContent( - '-/1' - ); - }); - - const rows = virtualizedContainer.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR); - expect(rows[1]).toHaveFocus(); - }); - - it('if search on load does not match anything, it does not steal focus or highlight first result', async () => { - mockQueryString('?search=dead&node=txn-5'); - const {container} = await pageloadTestSetup(); - const searchInput = await screen.findByPlaceholderText('Search in trace'); - expect(searchInput).toHaveValue('dead'); - - await waitFor(() => { - expect(screen.getByTestId('trace-search-result-iterator')).toHaveTextContent( - 'no results' - ); - }); - - const rows = container.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR); - expect(rows[6]).toHaveFocus(); - }); }); describe('keyboard navigation', () => { it('arrow down', async () => { const {virtualizedContainer} = await keyboardNavigationTestSetup(); - const rows = virtualizedContainer.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR); + const rows = getVirtualizedRows(virtualizedContainer); await userEvent.click(rows[0]); await waitFor(() => expect(rows[0]).toHaveFocus()); @@ -1109,9 +1128,10 @@ describe.skip('trace view', () => { await userEvent.keyboard('{arrowdown}'); await waitFor(() => expect(rows[1]).toHaveFocus()); }); + it('arrow up', async () => { const {virtualizedContainer} = await keyboardNavigationTestSetup(); - const rows = virtualizedContainer.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR); + const rows = getVirtualizedRows(virtualizedContainer); await userEvent.click(rows[1]); await waitFor(() => expect(rows[1]).toHaveFocus()); @@ -1119,9 +1139,10 @@ describe.skip('trace view', () => { await userEvent.keyboard('{arrowup}'); await waitFor(() => expect(rows[0]).toHaveFocus()); }); + it('arrow right expands row and fetches data', async () => { const {virtualizedContainer} = await keyboardNavigationTestSetup(); - const rows = virtualizedContainer.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR); + const rows = getVirtualizedRows(virtualizedContainer); mockSpansResponse( '0', @@ -1143,9 +1164,9 @@ describe.skip('trace view', () => { it('arrow left collapses row', async () => { const {virtualizedContainer} = await keyboardNavigationTestSetup(); - const rows = virtualizedContainer.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR); + const rows = getVirtualizedRows(virtualizedContainer); - const request = mockSpansResponse( + mockSpansResponse( '0', {}, { @@ -1158,16 +1179,18 @@ describe.skip('trace view', () => { await waitFor(() => expect(rows[1]).toHaveFocus()); await userEvent.keyboard('{arrowright}'); - expect(request).toHaveBeenCalledTimes(1); expect(await screen.findByText('special-span')).toBeInTheDocument(); await userEvent.keyboard('{arrowleft}'); - expect(screen.queryByText('special-span')).not.toBeInTheDocument(); + + await waitFor(() => { + expect(screen.queryByText('special-span')).not.toBeInTheDocument(); + }); }); it('arrow left does not collapse trace root row', async () => { const {virtualizedContainer} = await keyboardNavigationTestSetup(); - const rows = virtualizedContainer.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR); + const rows = getVirtualizedRows(virtualizedContainer); await userEvent.click(rows[0]); await waitFor(() => expect(rows[0]).toHaveFocus()); @@ -1178,7 +1201,7 @@ describe.skip('trace view', () => { it('arrow left on transaction row still renders transaction children', async () => { const {virtualizedContainer} = await nestedTransactionsTestSetup(); - const rows = virtualizedContainer.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR); + const rows = getVirtualizedRows(virtualizedContainer); await userEvent.click(rows[1]); await waitFor(() => expect(rows[1]).toHaveFocus()); @@ -1189,7 +1212,7 @@ describe.skip('trace view', () => { it('roving updates the element in the drawer', async () => { const {virtualizedContainer} = await keyboardNavigationTestSetup(); - const rows = virtualizedContainer.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR); + const rows = getVirtualizedRows(virtualizedContainer); mockSpansResponse( '0', @@ -1226,18 +1249,18 @@ describe.skip('trace view', () => { it('arrowup on first node jumps to end', async () => { const {virtualizedContainer} = await keyboardNavigationTestSetup(); - let rows = virtualizedContainer.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR); + let rows = getVirtualizedRows(virtualizedContainer); await userEvent.click(rows[0]); await waitFor(() => expect(rows[0]).toHaveFocus()); await userEvent.keyboard('{arrowup}'); expect( - await findByText(virtualizedContainer, /transaction-op-9999/i) + await findByText(virtualizedContainer, /transaction-op-99/i) ).toBeInTheDocument(); await waitFor(() => { - rows = virtualizedContainer.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR); + rows = getVirtualizedRows(virtualizedContainer); expect(rows[rows.length - 1]).toHaveFocus(); }); }); @@ -1245,67 +1268,76 @@ describe.skip('trace view', () => { it('arrowdown on last node jumps to start', async () => { const {virtualizedContainer} = await keyboardNavigationTestSetup(); - let rows = virtualizedContainer.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR); + let rows = getVirtualizedRows(virtualizedContainer); await userEvent.click(rows[0]); await waitFor(() => expect(rows[0]).toHaveFocus()); - await userEvent.keyboard('{arrowup}', {delay: null}); + await userEvent.keyboard('{arrowup}'); await waitFor(() => { - rows = virtualizedContainer.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR); + rows = getVirtualizedRows(virtualizedContainer); expect(rows[rows.length - 1]).toHaveFocus(); }); expect( - await within(virtualizedContainer).findByText(/transaction-op-9999/i) + await within(virtualizedContainer).findByText(/transaction-op-99/i) ).toBeInTheDocument(); - await userEvent.keyboard('{arrowdown}', {delay: null}); + await userEvent.keyboard('{arrowdown}'); await waitFor(() => { - rows = virtualizedContainer.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR); + rows = getVirtualizedRows(virtualizedContainer); expect(rows[0]).toHaveFocus(); }); expect( await within(virtualizedContainer).findByText(/transaction-op-0/i) ).toBeInTheDocument(); }); + it('tab scrolls to next node', async () => { const {virtualizedContainer} = await keyboardNavigationTestSetup(); - let rows = virtualizedContainer.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR); + let rows = getVirtualizedRows(virtualizedContainer); await userEvent.click(rows[0]); await waitFor(() => expect(rows[0]).toHaveFocus()); await userEvent.keyboard('{tab}'); await waitFor(() => { - rows = virtualizedContainer.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR); + rows = getVirtualizedRows(virtualizedContainer); expect(rows[1]).toHaveFocus(); }); }); + it('shift+tab scrolls to previous node', async () => { const {virtualizedContainer} = await keyboardNavigationTestSetup(); - let rows = virtualizedContainer.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR); + let rows = getVirtualizedRows(virtualizedContainer); await userEvent.click(rows[1]); - await waitFor(() => expect(rows[1]).toHaveFocus()); + await waitFor(() => { + rows = getVirtualizedRows(virtualizedContainer); + expect(rows[1]).toHaveFocus(); + }); await userEvent.keyboard('{Shift>}{tab}{/Shift}'); await waitFor(() => { - rows = virtualizedContainer.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR); + rows = getVirtualizedRows(virtualizedContainer); expect(rows[0]).toHaveFocus(); }); }); + it('arrowdown+shift scrolls to the end of the list', async () => { const {container, virtualizedContainer} = await keyboardNavigationTestSetup(); let rows = container.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR); await userEvent.click(rows[0]); - await waitFor(() => expect(rows[0]).toHaveFocus()); + await waitFor(() => { + rows = container.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR); + expect(rows[0]).toHaveFocus(); + }); await userEvent.keyboard('{Shift>}{arrowdown}{/Shift}'); expect( - await findByText(virtualizedContainer, /transaction-op-9999/i) + await findByText(virtualizedContainer, /transaction-op-99/i) ).toBeInTheDocument(); await waitFor(() => { rows = container.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR); @@ -1314,84 +1346,109 @@ describe.skip('trace view', () => { }); it('arrowup+shift scrolls to the start of the list', async () => { - const {container, virtualizedContainer} = await keyboardNavigationTestSetup(); + const {virtualizedContainer} = await keyboardNavigationTestSetup(); - let rows = container.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR); - await userEvent.click(rows[0]); + let rows = getVirtualizedRows(virtualizedContainer); - await waitFor(() => expect(rows[0]).toHaveFocus()); - await userEvent.keyboard('{Shift>}{arrowdown}{/Shift}'); + await userEvent.click(rows[1]); + await waitFor(() => { + rows = getVirtualizedRows(virtualizedContainer); + expect(rows[1]).toHaveFocus(); + }); + await userEvent.keyboard('{Shift>}{arrowdown}{/Shift}'); expect( - await findByText(virtualizedContainer, /transaction-op-9999/i) + await findByText(virtualizedContainer, /transaction-op-99/i) ).toBeInTheDocument(); await waitFor(() => { - rows = container.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR); + rows = getVirtualizedRows(virtualizedContainer); expect(rows[rows.length - 1]).toHaveFocus(); }); await userEvent.keyboard('{Shift>}{arrowup}{/Shift}'); + expect( await findByText(virtualizedContainer, /transaction-op-0/i) ).toBeInTheDocument(); - await scrollToEnd(); + await waitFor(() => { - rows = container.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR); + rows = getVirtualizedRows(virtualizedContainer); expect(rows[0]).toHaveFocus(); }); }); }); describe('search', () => { - it('searches in transaction', async () => { - const {container} = await searchTestSetup(); - let rows = Array.from(container.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR)); + it('triggers search on load but does not steal focus from node param', async () => { + mockQueryString('?search=transaction-op-99&node=txn-0'); + const {virtualizedContainer} = await pageloadTestSetup(); const searchInput = await screen.findByPlaceholderText('Search in trace'); + expect(searchInput).toHaveValue('transaction-op-99'); - await userEvent.click(searchInput); - fireEvent.change(searchInput, {target: {value: 'transaction-op'}}); + await waitFor(() => { + expect(screen.queryByTestId('trace-search-result-iterator')).toHaveTextContent( + '-/1' + ); + }); + + const rows = getVirtualizedRows(virtualizedContainer); + expect(rows[1]).toHaveFocus(); + }); + + it('if search on load does not match anything, it does not steal focus or highlight first result', async () => { + mockQueryString('?search=dead&node=txn-5'); + + const {container} = await pageloadTestSetup(); + const searchInput = await screen.findByPlaceholderText('Search in trace'); + expect(searchInput).toHaveValue('dead'); await waitFor(() => { - const highlighted_row = container.querySelector( - '.TraceRow:not(.Hidden).SearchResult.Highlight' + expect(screen.getByTestId('trace-search-result-iterator')).toHaveTextContent( + 'no results' ); + }); - rows = Array.from(container.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR)); - expect(rows.indexOf(highlighted_row!)).toBe(1); + await waitFor(() => { + const rows = container.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR); + expect(rows[6]).toHaveFocus(); }); }); - it('supports roving with arrowup and arrowdown', async () => { + it('searches in transaction', async () => { const {container} = await searchTestSetup(); const searchInput = await screen.findByPlaceholderText('Search in trace'); - await userEvent.click(searchInput); - // Fire change because userEvent triggers this letter by letter - fireEvent.change(searchInput, {target: {value: 'transaction-op'}}); + await userEvent.type(searchInput, 'transaction-op'); - // Wait for the search results to resolve - await searchToUpdate(); + expect(searchInput).toHaveValue('transaction-op'); + await searchToResolve(); + + await assertHighlightedRowAtIndex(container, 1); + }); + + it('supports roving with arrowup and arrowdown', async () => { + const {container} = await searchTestSetup(); + + const searchInput = await screen.findByPlaceholderText('Search in trace'); + await userEvent.type(searchInput, 'transaction-op'); + expect(searchInput).toHaveValue('transaction-op'); + await searchToResolve(); for (const action of [ - // starting at the top, jumpt bottom with shift+arrowdown + // starting at the top, jump bottom with shift+arrowdown ['{Shift>}{arrowdown}{/Shift}', 11], - // // move to row above with arrowup + // move to row above with arrowup ['{arrowup}', 10], - // // and jump back to top with shift+arrowup + // and jump back to top with shift+arrowup ['{Shift>}{arrowup}{/Shift}', 1], - // // // and jump to next row with arrowdown + // and jump to next row with arrowdown ['{arrowdown}', 2], ] as const) { - await userEvent.keyboard(action[0] as string); - // assert that focus on search input is never lost - expect(searchInput).toHaveFocus(); + await userEvent.keyboard(action[0]); - await waitFor(() => { - // Only a single row is highlighted, the rest are search results - assertHighlightedRowAtIndex(container, action[1]); - }); + await assertHighlightedRowAtIndex(container, action[1]); } }); @@ -1399,12 +1456,11 @@ describe.skip('trace view', () => { await searchTestSetup(); const searchInput = await screen.findByPlaceholderText('Search in trace'); - await userEvent.click(searchInput); - // Fire change because userEvent triggers this letter by letter - fireEvent.change(searchInput, {target: {value: 'transaction-op'}}); + await userEvent.type(searchInput, 'transaction-op'); + expect(searchInput).toHaveValue('transaction-op'); // Wait for the search results to resolve - await searchToUpdate(); + await searchToResolve(); expect(await screen.findByTestId('trace-drawer-title')).toHaveTextContent( 'transaction-op-0' @@ -1420,48 +1476,48 @@ describe.skip('trace view', () => { ); }); }); + it('highlighted node narrows down on the first result', async () => { const {container} = await searchTestSetup(); const searchInput = await screen.findByPlaceholderText('Search in trace'); - await userEvent.click(searchInput); - // Fire change because userEvent triggers this letter by letter - fireEvent.change(searchInput, {target: {value: 'transaction-op-1'}}); - // Wait for the search results to resolve - await searchToUpdate(); + await userEvent.type(searchInput, 'transaction-op-1'); + expect(searchInput).toHaveValue('transaction-op-1'); + await searchToResolve(); - assertHighlightedRowAtIndex(container, 2); + await assertHighlightedRowAtIndex(container, 2); - fireEvent.change(searchInput, {target: {value: 'transaction-op-10'}}); - await searchToUpdate(); + await userEvent.clear(searchInput); + await userEvent.type(searchInput, 'transaction-op-5'); + await searchToResolve(); - await waitFor(() => { - assertHighlightedRowAtIndex(container, 11); - }); + await assertHighlightedRowAtIndex(container, 6); }); + it('highlighted is persisted on node while it is part of the search results', async () => { const {container} = await searchTestSetup(); const searchInput = await screen.findByPlaceholderText('Search in trace'); - await userEvent.click(searchInput); - // Fire change because userEvent triggers this letter by letter - fireEvent.change(searchInput, {target: {value: 'trans'}}); - + await userEvent.type(searchInput, 'trans'); + expect(searchInput).toHaveValue('trans'); // Wait for the search results to resolve - await searchToUpdate(); + await searchToResolve(); await userEvent.keyboard('{arrowdown}'); - await searchToUpdate(); + await searchToResolve(); - assertHighlightedRowAtIndex(container, 2); + await assertHighlightedRowAtIndex(container, 2); - fireEvent.change(searchInput, {target: {value: 'transa'}}); - await searchToUpdate(); + await userEvent.type(searchInput, 'act'); + expect(searchInput).toHaveValue('transact'); + await searchToResolve(); // Highlighting is persisted on the row - assertHighlightedRowAtIndex(container, 2); + await assertHighlightedRowAtIndex(container, 2); - fireEvent.change(searchInput, {target: {value: 'this wont match anything'}}); - await searchToUpdate(); + await userEvent.clear(searchInput); + await userEvent.type(searchInput, 'this wont match anything'); + expect(searchInput).toHaveValue('this wont match anything'); + await searchToResolve(); // When there is no match, the highlighting is removed expect(container.querySelectorAll('.TraceRow.Highlight')).toHaveLength(0); @@ -1470,35 +1526,34 @@ describe.skip('trace view', () => { it('auto highlights the first result when search begins', async () => { const {container} = await searchTestSetup(); const searchInput = await screen.findByPlaceholderText('Search in trace'); - await userEvent.click(searchInput); // Nothing is highlighted expect(container.querySelectorAll('.TraceRow.Highlight')).toHaveLength(0); - // Fire change because userEvent triggers this letter by letter - fireEvent.change(searchInput, {target: {value: 't'}}); + await userEvent.type(searchInput, 't'); + expect(searchInput).toHaveValue('t'); // Wait for the search results to resolve - await searchToUpdate(); + await searchToResolve(); - assertHighlightedRowAtIndex(container, 1); + await assertHighlightedRowAtIndex(container, 1); }); + it('clicking a row that is also a search result updates the result index', async () => { - const {container} = await searchTestSetup(); - const searchInput = await screen.findByPlaceholderText('Search in trace'); - await userEvent.click(searchInput); - // Fire change because userEvent triggers this letter by letter - fireEvent.change(searchInput, {target: {value: 'transaction-op-1'}}); + const {container, virtualizedContainer} = await searchTestSetup(); - await searchToUpdate(); + const searchInput = await screen.findByPlaceholderText('Search in trace'); + await userEvent.type(searchInput, 'transaction-op-1'); + expect(searchInput).toHaveValue('transaction-op-1'); - assertHighlightedRowAtIndex(container, 2); - const rows = container.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR); + await searchToResolve(); + await assertHighlightedRowAtIndex(container, 2); + const rows = getVirtualizedRows(virtualizedContainer); // By default, we highlight the first result expect(await screen.findByTestId('trace-search-result-iterator')).toHaveTextContent( '1/2' ); - await scrollToEnd(); + // Click on a random row in the list that is not a search result await userEvent.click(rows[5]); await waitFor(() => { @@ -1506,7 +1561,7 @@ describe.skip('trace view', () => { '-/2' ); }); - await scrollToEnd(); + // Click on a the row in the list that is a search result await userEvent.click(rows[2]); await waitFor(() => { @@ -1515,6 +1570,7 @@ describe.skip('trace view', () => { ); }); }); + it('during search, expanding a row retriggers search', async () => { mockTraceRootFacets(); mockTraceRootEvent('0'); @@ -1602,13 +1658,13 @@ describe.skip('trace view', () => { const {container} = render(, {router}); // Awaits for the placeholder rendering rows to be removed - expect(await findByText(container, /transaction-op-0/i)).toBeInTheDocument(); + await findByText(container, /transaction-op-0/i); const searchInput = await screen.findByPlaceholderText('Search in trace'); - await userEvent.click(searchInput); - // Fire change because userEvent triggers this letter by letter - fireEvent.change(searchInput, {target: {value: 'op-0'}}); - await searchToUpdate(); + await userEvent.type(searchInput, 'op-0'); + expect(searchInput).toHaveValue('op-0'); + + await searchToResolve(); await waitFor(() => { expect(screen.queryByTestId('trace-search-result-iterator')).toHaveTextContent( @@ -1638,21 +1694,16 @@ describe.skip('trace view', () => { it('during search, highlighting is persisted on the row', async () => { const {container} = await searchTestSetup(); const searchInput = await screen.findByPlaceholderText('Search in trace'); - await userEvent.click(searchInput); - // Fire change because userEvent triggers this letter by letter - fireEvent.change(searchInput, {target: {value: 'transaction-op'}}); - await searchToUpdate(); + await userEvent.type(searchInput, 'transaction-op'); + expect(searchInput).toHaveValue('transaction-op'); + await searchToResolve(); - assertHighlightedRowAtIndex(container, 1); - await searchToUpdate(); + await assertHighlightedRowAtIndex(container, 1); // User moves down the list using keyboard navigation - for (const _ of [1, 2, 3, 4, 5]) { - const initial = screen.getByTestId('trace-search-result-iterator').textContent; + for (let i = 1; i < 6; i++) { await userEvent.keyboard('{arrowDown}'); - await waitFor(() => { - expect(screen.getByTestId('trace-search-result-iterator')).not.toBe(initial); - }); + await assertHighlightedRowAtIndex(container, 1 + i); } // User clicks on an entry in the list, then proceeds to search @@ -1662,28 +1713,26 @@ describe.skip('trace view', () => { ); }); // And then continues the query - the highlighting is preserved as long as the - // rwo is part of the search results - assertHighlightedRowAtIndex(container, 6); - fireEvent.change(searchInput, {target: {value: 'transaction-op-'}}); - await searchToUpdate(); - assertHighlightedRowAtIndex(container, 6); - fireEvent.change(searchInput, {target: {value: 'transaction-op-5'}}); - await searchToUpdate(); - assertHighlightedRowAtIndex(container, 6); - fireEvent.change(searchInput, {target: {value: 'transaction-op-none'}}); - await searchToUpdate(); + // row is part of the search results + await assertHighlightedRowAtIndex(container, 6); + + await userEvent.type(searchInput, '-5'); + expect(searchInput).toHaveValue('transaction-op-5'); + + await searchToResolve(); + await assertHighlightedRowAtIndex(container, 6); + + await userEvent.clear(searchInput); + await userEvent.type(searchInput, 'transaction-op-none'); + await searchToResolve(); expect(container.querySelectorAll('.TraceRow.Highlight')).toHaveLength(0); }); }); describe('tabbing', () => { - beforeEach(() => {}); - afterEach(() => { - jest.restoreAllMocks(); - }); it('clicking on a node spawns a new tab when none is selected', async () => { const {virtualizedContainer} = await simpleTestSetup(); - const rows = virtualizedContainer.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR); + const rows = getVirtualizedRows(virtualizedContainer); expect(screen.queryAllByTestId(DRAWER_TABS_TEST_ID)).toHaveLength(1); await userEvent.click(rows[5]); @@ -1694,7 +1743,7 @@ describe.skip('trace view', () => { it('clicking on a node replaces the previously selected tab', async () => { const {virtualizedContainer} = await simpleTestSetup(); - const rows = virtualizedContainer.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR); + const rows = getVirtualizedRows(virtualizedContainer); expect(screen.queryAllByTestId(DRAWER_TABS_TEST_ID)).toHaveLength(1); await userEvent.click(rows[5]); @@ -1718,9 +1767,10 @@ describe.skip('trace view', () => { ).toBeTruthy(); }); }); + it('pinning a tab and clicking on a new node spawns a new tab', async () => { const {virtualizedContainer} = await simpleTestSetup(); - const rows = virtualizedContainer.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR); + const rows = getVirtualizedRows(virtualizedContainer); expect(screen.queryAllByTestId(DRAWER_TABS_TEST_ID)).toHaveLength(1); await userEvent.click(rows[5]); @@ -1745,9 +1795,10 @@ describe.skip('trace view', () => { ).toBeTruthy(); }); }); + it('unpinning a tab removes it', async () => { const {virtualizedContainer} = await simpleTestSetup(); - const rows = virtualizedContainer.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR); + const rows = getVirtualizedRows(virtualizedContainer); expect(screen.queryAllByTestId(DRAWER_TABS_TEST_ID)).toHaveLength(1); await userEvent.click(rows[5]); @@ -1770,9 +1821,10 @@ describe.skip('trace view', () => { expect(screen.queryAllByTestId(DRAWER_TABS_TEST_ID)).toHaveLength(2); }); }); + it('clicking a node that is already open in a tab switches to that tab and persists the previous node', async () => { const {virtualizedContainer} = await simpleTestSetup(); - const rows = virtualizedContainer.querySelectorAll(VISIBLE_TRACE_ROW_SELECTOR); + const rows = getVirtualizedRows(virtualizedContainer); expect(screen.queryAllByTestId(DRAWER_TABS_TEST_ID)).toHaveLength(1); await userEvent.click(rows[5]); diff --git a/static/app/views/performance/newTraceDetails/traceApi/useTraceRootEvent.tsx b/static/app/views/performance/newTraceDetails/traceApi/useTraceRootEvent.tsx index b055a998de20c..75eb6da78ea58 100644 --- a/static/app/views/performance/newTraceDetails/traceApi/useTraceRootEvent.tsx +++ b/static/app/views/performance/newTraceDetails/traceApi/useTraceRootEvent.tsx @@ -21,7 +21,7 @@ export function useTraceRootEvent(trace: TraceSplitResults | ], { staleTime: 0, - enabled: !!trace && !!root, + enabled: !!trace && !!root?.project_slug && !!root?.event_id, } ); } diff --git a/static/app/views/performance/newTraceDetails/traceApi/useTransaction.tsx b/static/app/views/performance/newTraceDetails/traceApi/useTransaction.tsx index bb4f35b542aef..723c5c165766f 100644 --- a/static/app/views/performance/newTraceDetails/traceApi/useTransaction.tsx +++ b/static/app/views/performance/newTraceDetails/traceApi/useTransaction.tsx @@ -22,7 +22,7 @@ export function useTransaction(props: UseTransactionProps) { ], { staleTime: 0, - enabled: !!props.node, + enabled: !!props.node?.value?.project_slug && !!props.node?.value.event_id, } ); } diff --git a/static/app/views/performance/newTraceDetails/traceHeader/index.tsx b/static/app/views/performance/newTraceDetails/traceHeader/index.tsx index b7897b81d93ed..059061561670e 100644 --- a/static/app/views/performance/newTraceDetails/traceHeader/index.tsx +++ b/static/app/views/performance/newTraceDetails/traceHeader/index.tsx @@ -159,7 +159,7 @@ export function TraceMetaDataHeader(props: TraceMetadataHeaderProps) { const onProjectClick = useCallback( (projectSlug: string) => { - dispatch({type: 'set query', query: `project:${projectSlug}`}); + dispatch({type: 'set query', query: `project:${projectSlug}`, source: 'external'}); }, [dispatch] ); diff --git a/static/app/views/performance/newTraceDetails/tracePreferencesDropdown.tsx b/static/app/views/performance/newTraceDetails/tracePreferencesDropdown.tsx index 8898beae68a54..287f3272ede47 100644 --- a/static/app/views/performance/newTraceDetails/tracePreferencesDropdown.tsx +++ b/static/app/views/performance/newTraceDetails/tracePreferencesDropdown.tsx @@ -19,27 +19,24 @@ interface TracePreferencesDropdownProps { onMissingInstrumentationChange: () => void; } -export function TracePreferencesDropdown(props: TracePreferencesDropdownProps) { - const options: SelectOption[] = useMemo( - () => [ - { - label: t('Autogrouping'), - value: 'autogroup', - details: t( - 'Collapses 5 or more sibling spans with the same description or any spans with 2 or more descendants with the same operation.' - ), - }, - { - label: t('Missing Instrumentation'), - value: 'missing-instrumentation', - details: t( - 'Shows when there is more than 100ms of unaccounted elapsed time between two spans.' - ), - }, - ], - [] - ); +const TRACE_PREFERENCES_DROPDOWN_OPTIONS: SelectOption[] = [ + { + label: t('Autogrouping'), + value: 'autogroup', + details: t( + 'Collapses 5 or more sibling spans with the same description or any spans with 2 or more descendants with the same operation.' + ), + }, + { + label: t('Missing Instrumentation'), + value: 'missing-instrumentation', + details: t( + 'Shows when there is more than 100ms of unaccounted elapsed time between two spans.' + ), + }, +]; +export function TracePreferencesDropdown(props: TracePreferencesDropdownProps) { const values = useMemo(() => { const value: string[] = []; if (props.autogroup) { @@ -88,7 +85,7 @@ export function TracePreferencesDropdown(props: TracePreferencesDropdownProps) { // Force the trigger to be so that we only render the icon triggerLabel="" triggerProps={CompactSelectTriggerProps} - options={options} + options={TRACE_PREFERENCES_DROPDOWN_OPTIONS} onChange={onChange} /> ); diff --git a/static/app/views/performance/newTraceDetails/traceSearch/traceSearchInput.tsx b/static/app/views/performance/newTraceDetails/traceSearch/traceSearchInput.tsx index 2b9b5f3b2c03b..c12361956eecf 100644 --- a/static/app/views/performance/newTraceDetails/traceSearch/traceSearchInput.tsx +++ b/static/app/views/performance/newTraceDetails/traceSearch/traceSearchInput.tsx @@ -10,13 +10,19 @@ import {t} from 'sentry/locale'; import {space} from 'sentry/styles/space'; import type {Organization} from 'sentry/types/organization'; import {trackAnalytics} from 'sentry/utils/analytics'; +import type {DispatchingReducerMiddleware} from 'sentry/utils/useDispatchingReducer'; import useOrganization from 'sentry/utils/useOrganization'; import {traceAnalytics} from '../traceAnalytics'; import type {TraceTree} from '../traceModels/traceTree'; import type {TraceTreeNode} from '../traceModels/traceTreeNode'; +import type {TraceReducer} from '../traceState'; import type {TraceSearchState} from '../traceState/traceSearch'; -import {useTraceState, useTraceStateDispatch} from '../traceState/traceStateProvider'; +import { + useTraceState, + useTraceStateDispatch, + useTraceStateEmitter, +} from '../traceState/traceStateProvider'; interface TraceSearchInputProps { onTraceSearch: ( @@ -33,7 +39,8 @@ export function TraceSearchInput(props: TraceSearchInputProps) { const organization = useOrganization(); const traceState = useTraceState(); const traceDispatch = useTraceStateDispatch(); - const [status, setStatus] = useState(); + const traceStateEmitter = useTraceStateEmitter(); + const [status, setStatus] = useState([0, 'success']); const timeoutRef = useRef(undefined); const statusRef = useRef(status); @@ -46,30 +53,39 @@ export function TraceSearchInput(props: TraceSearchInputProps) { useLayoutEffect(() => { if (typeof timeoutRef.current === 'number') { window.clearTimeout(timeoutRef.current); + timeoutRef.current = undefined; } // if status is loading, show loading icon immediately // if previous status was loading, show loading icon for at least 500ms if (!statusRef.current && traceState.search.status) { setStatus([performance.now(), traceState.search.status[1]]); - return; + return undefined; } + let cancel = false; + const nextStatus = traceState.search.status; if (nextStatus) { const elapsed = performance.now() - nextStatus[0]; if (elapsed > MIN_LOADING_TIME || nextStatus[1] === 'loading') { setStatus(nextStatus); - return; + return undefined; } const schedule = nextStatus[0] + MIN_LOADING_TIME - performance.now(); timeoutRef.current = window.setTimeout(() => { - setStatus(nextStatus); + if (!cancel) { + setStatus(nextStatus); + } }, schedule); } else { setStatus(nextStatus); } + + return () => { + cancel = true; + }; }, [traceState.search.status]); const onSearchFocus = useCallback(() => { @@ -169,18 +185,27 @@ export function TraceSearchInput(props: TraceSearchInputProps) { const inputRef = useRef(null); useLayoutEffect(() => { - // Search value can be changed externally, e.g. by actions that trigger a search. - // When this happens, sync the input value to the search value and trigger a search. - if ( - inputRef.current && - traceState.search.query && - inputRef.current.value !== traceState.search.query - ) { - inputRef.current.focus(); - inputRef.current.value = traceState.search.query ?? ''; - onChange({target: inputRef.current} as React.ChangeEvent); - } - }, [traceState.search.query, onChange]); + const beforeTraceNextStateDispatch: DispatchingReducerMiddleware< + typeof TraceReducer + >['before next state'] = (_prevState, _nextState, action) => { + if ( + action.type === 'set query' && + action.source === 'external' && + action.query && + inputRef.current + ) { + inputRef.current.value = action.query; + traceDispatch({type: 'clear roving index'}); + onTraceSearch(action.query, traceStateRef.current.search.node, 'track result'); + } + }; + + traceStateEmitter.on('before next state', beforeTraceNextStateDispatch); + + return () => { + traceStateEmitter.off('before next state', beforeTraceNextStateDispatch); + }; + }, [traceStateEmitter, onTraceSearch, traceDispatch]); return ( diff --git a/static/app/views/performance/newTraceDetails/traceState/traceRovingTabIndex.tsx b/static/app/views/performance/newTraceDetails/traceState/traceRovingTabIndex.tsx index 9f27fc5dd2832..c47c931c56fc4 100644 --- a/static/app/views/performance/newTraceDetails/traceState/traceRovingTabIndex.tsx +++ b/static/app/views/performance/newTraceDetails/traceState/traceRovingTabIndex.tsx @@ -21,7 +21,6 @@ export type TraceRovingTabIndexAction = node: TraceTreeNode; type: 'set roving index'; } - | {type: 'clear'} | {type: 'clear roving index'} | {items: number; type: 'set roving count'}; @@ -41,7 +40,6 @@ export function traceRovingTabIndexReducer( case 'set roving index': return {...state, node: action.node, index: action.index}; case 'clear roving index': - case 'clear': return {...state, index: null, node: null}; default: traceReducerExhaustiveActionCheck(action); diff --git a/static/app/views/performance/newTraceDetails/traceState/traceSearch.tsx b/static/app/views/performance/newTraceDetails/traceState/traceSearch.tsx index ce487214e448d..e1db552c50436 100644 --- a/static/app/views/performance/newTraceDetails/traceState/traceSearch.tsx +++ b/static/app/views/performance/newTraceDetails/traceState/traceSearch.tsx @@ -4,7 +4,7 @@ import type {TraceSearchResult} from '../traceSearch/traceSearchEvaluator'; import {traceReducerExhaustiveActionCheck} from '../traceState'; export type TraceSearchAction = - | {query: string | undefined; type: 'set query'} + | {query: string; type: 'set query'; source?: 'external'} | {type: 'go to first match'} | {type: 'go to last match'} | {type: 'go to next match'} @@ -14,7 +14,6 @@ export type TraceSearchAction = resultIteratorIndex: number; type: 'set search iterator index'; } - | {type: 'clear'} | {type: 'clear search iterator index'} | {type: 'clear query'} | { @@ -180,10 +179,6 @@ export function traceSearchReducer( node: null, }; - case 'clear': { - return {...state, node: null, resultIteratorIndex: null, resultIndex: null}; - } - default: { traceReducerExhaustiveActionCheck(action); return state; diff --git a/static/app/views/performance/newTraceDetails/traceState/traceTabs.tsx b/static/app/views/performance/newTraceDetails/traceState/traceTabs.tsx index 96bfcfd5ab24c..19fc0c67b7656 100644 --- a/static/app/views/performance/newTraceDetails/traceState/traceTabs.tsx +++ b/static/app/views/performance/newTraceDetails/traceState/traceTabs.tsx @@ -66,7 +66,6 @@ export type TraceTabsReducerAction = } | {type: 'pin tab'} | {payload: number; type: 'unpin tab'} - | {type: 'clear'} | {type: 'clear clicked tab'}; export function traceTabsReducer( @@ -179,8 +178,7 @@ export function traceTabsReducer( }; } - case 'clear clicked tab': - case 'clear': { + case 'clear clicked tab': { const next = state.last_clicked_tab === state.current_tab ? state.tabs[state.tabs.length - 1]