diff --git a/public/app/features/explore/TraceView/TraceView.tsx b/public/app/features/explore/TraceView/TraceView.tsx index 2c2049d0974..3de2d2da2ca 100644 --- a/public/app/features/explore/TraceView/TraceView.tsx +++ b/public/app/features/explore/TraceView/TraceView.tsx @@ -166,7 +166,6 @@ export function TraceView(props: Props) { setShowSpanFilters={setShowSpanFilters} showSpanFilterMatchesOnly={showSpanFilterMatchesOnly} setShowSpanFilterMatchesOnly={setShowSpanFilterMatchesOnly} - focusedSpanIdForSearch={newTraceViewHeaderFocusedSpanIdForSearch} setFocusedSpanIdForSearch={setNewTraceViewHeaderFocusedSpanIdForSearch} spanFilterMatches={spanFilterMatches} datasourceType={datasourceType} diff --git a/public/app/features/explore/TraceView/TraceViewContainer.test.tsx b/public/app/features/explore/TraceView/TraceViewContainer.test.tsx index 86677a298e6..367d1176956 100644 --- a/public/app/features/explore/TraceView/TraceViewContainer.test.tsx +++ b/public/app/features/explore/TraceView/TraceViewContainer.test.tsx @@ -134,6 +134,56 @@ describe('TraceViewContainer', () => { ).toContain('rowFocused'); }); + it('can select next/prev results', async () => { + config.featureToggles.newTraceViewHeader = true; + renderTraceViewContainer(); + const spanFiltersButton = screen.getByRole('button', { name: 'Span Filters' }); + await user.click(spanFiltersButton); + + const nextResultButton = screen.getByRole('button', { name: 'Next result button' }); + const prevResultButton = screen.getByRole('button', { name: 'Prev result button' }); + expect((nextResultButton as HTMLButtonElement)['disabled']).toBe(true); + expect((prevResultButton as HTMLButtonElement)['disabled']).toBe(true); + + await user.click(screen.getByLabelText('Select tag key')); + const tagOption = screen.getByText('component'); + await waitFor(() => expect(tagOption).toBeInTheDocument()); + await user.click(tagOption); + + await waitFor(() => { + expect( + screen.queryAllByText('', { selector: 'div[data-testid="span-view"]' })[0].parentElement!.className + ).toContain('rowMatchingFilter'); + expect( + screen.queryAllByText('', { selector: 'div[data-testid="span-view"]' })[1].parentElement!.className + ).toContain('rowMatchingFilter'); + expect( + screen.queryAllByText('', { selector: 'div[data-testid="span-view"]' })[2].parentElement!.className + ).toContain('rowMatchingFilter'); + }); + + expect((nextResultButton as HTMLButtonElement)['disabled']).toBe(false); + expect((prevResultButton as HTMLButtonElement)['disabled']).toBe(false); + await user.click(nextResultButton); + await waitFor(() => { + expect( + screen.queryAllByText('', { selector: 'div[data-testid="span-view"]' })[0].parentElement!.className + ).toContain('rowFocused'); + }); + await user.click(nextResultButton); + await waitFor(() => { + expect( + screen.queryAllByText('', { selector: 'div[data-testid="span-view"]' })[1].parentElement!.className + ).toContain('rowFocused'); + }); + await user.click(prevResultButton); + await waitFor(() => { + expect( + screen.queryAllByText('', { selector: 'div[data-testid="span-view"]' })[0].parentElement!.className + ).toContain('rowFocused'); + }); + }); + it('show matches only works as expected', async () => { config.featureToggles.newTraceViewHeader = true; renderTraceViewContainer(); diff --git a/public/app/features/explore/TraceView/components/TracePageHeader/NewTracePageHeader.test.tsx b/public/app/features/explore/TraceView/components/TracePageHeader/NewTracePageHeader.test.tsx index eec1a45a7b2..05b7ad0bee8 100644 --- a/public/app/features/explore/TraceView/components/TracePageHeader/NewTracePageHeader.test.tsx +++ b/public/app/features/explore/TraceView/components/TracePageHeader/NewTracePageHeader.test.tsx @@ -33,7 +33,6 @@ const setup = () => { showSpanFilterMatchesOnly: false, setShowSpanFilterMatchesOnly: jest.fn(), spanFilterMatches: undefined, - focusedSpanIdForSearch: '', setFocusedSpanIdForSearch: jest.fn(), datasourceType: 'tempo', setHeaderHeight: jest.fn(), diff --git a/public/app/features/explore/TraceView/components/TracePageHeader/NewTracePageHeader.tsx b/public/app/features/explore/TraceView/components/TracePageHeader/NewTracePageHeader.tsx index 1e0ffdcbb9f..60c0e40a72d 100644 --- a/public/app/features/explore/TraceView/components/TracePageHeader/NewTracePageHeader.tsx +++ b/public/app/features/explore/TraceView/components/TracePageHeader/NewTracePageHeader.tsx @@ -41,7 +41,6 @@ export type TracePageHeaderProps = { setShowSpanFilters: (isOpen: boolean) => void; showSpanFilterMatchesOnly: boolean; setShowSpanFilterMatchesOnly: (showMatchesOnly: boolean) => void; - focusedSpanIdForSearch: string; setFocusedSpanIdForSearch: React.Dispatch>; spanFilterMatches: Set | undefined; datasourceType: string; @@ -58,7 +57,6 @@ export const NewTracePageHeader = memo((props: TracePageHeaderProps) => { setShowSpanFilters, showSpanFilterMatchesOnly, setShowSpanFilterMatchesOnly, - focusedSpanIdForSearch, setFocusedSpanIdForSearch, spanFilterMatches, datasourceType, @@ -140,7 +138,6 @@ export const NewTracePageHeader = memo((props: TracePageHeaderProps) => { search={search} setSearch={setSearch} spanFilterMatches={spanFilterMatches} - focusedSpanIdForSearch={focusedSpanIdForSearch} setFocusedSpanIdForSearch={setFocusedSpanIdForSearch} datasourceType={datasourceType} /> diff --git a/public/app/features/explore/TraceView/components/TracePageHeader/NewTracePageSearchBar.test.tsx b/public/app/features/explore/TraceView/components/TracePageHeader/NewTracePageSearchBar.test.tsx index 4515b0bbbeb..ea43b80c2f1 100644 --- a/public/app/features/explore/TraceView/components/TracePageHeader/NewTracePageSearchBar.test.tsx +++ b/public/app/features/explore/TraceView/components/TracePageHeader/NewTracePageSearchBar.test.tsx @@ -12,25 +12,49 @@ // See the License for the specific language governing permissions and // limitations under the License. -import { render, screen } from '@testing-library/react'; +import { render, screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; import React from 'react'; import { defaultFilters } from '../../useSearch'; -import NewTracePageSearchBar, { TracePageSearchBarProps } from './NewTracePageSearchBar'; - -const defaultProps = { - search: defaultFilters, - setFocusedSpanIdForSearch: jest.fn(), - showSpanFilterMatchesOnly: false, - setShowSpanFilterMatchesOnly: jest.fn(), -}; +import NewTracePageSearchBar, { getStyles } from './NewTracePageSearchBar'; describe('', () => { + let user: ReturnType; + beforeEach(() => { + jest.useFakeTimers(); + // Need to use delay: null here to work with fakeTimers + // see https://github.com/testing-library/user-event/issues/833 + user = userEvent.setup({ delay: null }); + }); + afterEach(() => { + jest.useRealTimers(); + }); + + const NewTracePageSearchBarWithProps = (props: { matches: string[] | undefined }) => { + const searchBarProps = { + search: defaultFilters, + spanFilterMatches: props.matches ? new Set(props.matches) : undefined, + showSpanFilterMatchesOnly: false, + setShowSpanFilterMatchesOnly: jest.fn(), + setFocusedSpanIdForSearch: jest.fn(), + datasourceType: '', + reset: jest.fn(), + totalSpans: 100, + }; + + return ; + }; + + it('should render', () => { + expect(() => render()).not.toThrow(); + }); + it('renders buttons', () => { - render(); - const nextResButton = screen.getByRole('button', { name: 'Next result button' }); - const prevResButton = screen.getByRole('button', { name: 'Prev result button' }); + render(); + const nextResButton = screen.queryByRole('button', { name: 'Next result button' }); + const prevResButton = screen.queryByRole('button', { name: 'Prev result button' }); const resetFiltersButton = screen.getByRole('button', { name: 'Reset filters button' }); expect(nextResButton).toBeInTheDocument(); expect(prevResButton).toBeInTheDocument(); @@ -40,22 +64,55 @@ describe('', () => { expect((resetFiltersButton as HTMLButtonElement)['disabled']).toBe(true); }); - it('renders buttons that can be used to search if results found', () => { - const props = { - ...defaultProps, - spanFilterMatches: new Set(['2ed38015486087ca']), - }; - render(); - const nextResButton = screen.getByRole('button', { name: 'Next result button' }); - const prevResButton = screen.getByRole('button', { name: 'Prev result button' }); + it('renders total spans', async () => { + render(); + expect(screen.getByText('100 spans')).toBeDefined(); + }); + + it('renders buttons that can be used to search if filters added', () => { + render(); + const nextResButton = screen.queryByRole('button', { name: 'Next result button' }); + const prevResButton = screen.queryByRole('button', { name: 'Prev result button' }); expect(nextResButton).toBeInTheDocument(); expect(prevResButton).toBeInTheDocument(); expect((nextResButton as HTMLButtonElement)['disabled']).toBe(false); expect((prevResButton as HTMLButtonElement)['disabled']).toBe(false); + expect(screen.getByText('1 match')).toBeDefined(); + }); + + it('renders correctly when moving through matches', async () => { + render(); + const nextResButton = screen.queryByRole('button', { name: 'Next result button' }); + const prevResButton = screen.queryByRole('button', { name: 'Prev result button' }); + expect(screen.getByText('3 matches')).toBeDefined(); + await user.click(nextResButton!); + expect(screen.getByText('1/3 matches')).toBeDefined(); + await user.click(nextResButton!); + expect(screen.getByText('2/3 matches')).toBeDefined(); + await user.click(nextResButton!); + expect(screen.getByText('3/3 matches')).toBeDefined(); + await user.click(nextResButton!); + expect(screen.getByText('1/3 matches')).toBeDefined(); + await user.click(prevResButton!); + expect(screen.getByText('3/3 matches')).toBeDefined(); + await user.click(prevResButton!); + expect(screen.getByText('2/3 matches')).toBeDefined(); + }); + + it('renders correctly when there are no matches i.e. too many filters added', async () => { + const { container } = render(); + const styles = getStyles(); + const tooltip = container.querySelector('.' + styles.matchesTooltip); + expect(screen.getByText('0 matches')).toBeDefined(); + userEvent.hover(tooltip!); + jest.advanceTimersByTime(1000); + await waitFor(() => { + expect(screen.getByText(/0 span matches for the filters selected/)).toBeDefined(); + }); }); it('renders show span filter matches only switch', async () => { - render(); + render(); const matchesSwitch = screen.getByRole('checkbox', { name: 'Show matches only switch' }); expect(matchesSwitch).toBeInTheDocument(); }); diff --git a/public/app/features/explore/TraceView/components/TracePageHeader/NewTracePageSearchBar.tsx b/public/app/features/explore/TraceView/components/TracePageHeader/NewTracePageSearchBar.tsx index bf0ddd18a8b..ce3d44da620 100644 --- a/public/app/features/explore/TraceView/components/TracePageHeader/NewTracePageSearchBar.tsx +++ b/public/app/features/explore/TraceView/components/TracePageHeader/NewTracePageSearchBar.tsx @@ -13,42 +13,50 @@ // limitations under the License. import { css } from '@emotion/css'; -import React, { memo, Dispatch, SetStateAction, useEffect, useMemo } from 'react'; +import React, { memo, Dispatch, SetStateAction, useEffect, useMemo, useState } from 'react'; import { config, reportInteraction } from '@grafana/runtime'; -import { Button, Switch, useStyles2 } from '@grafana/ui'; +import { Button, Icon, Switch, Tooltip, useStyles2 } from '@grafana/ui'; import { SearchProps } from '../../useSearch'; import { convertTimeFilter } from '../utils/filter-spans'; export type TracePageSearchBarProps = { search: SearchProps; - setSearch: React.Dispatch>; spanFilterMatches: Set | undefined; showSpanFilterMatchesOnly: boolean; setShowSpanFilterMatchesOnly: (showMatchesOnly: boolean) => void; - focusedSpanIdForSearch: string; setFocusedSpanIdForSearch: Dispatch>; datasourceType: string; reset: () => void; + totalSpans: number; }; export default memo(function NewTracePageSearchBar(props: TracePageSearchBarProps) { const { search, spanFilterMatches, - focusedSpanIdForSearch, + showSpanFilterMatchesOnly, + setShowSpanFilterMatchesOnly, setFocusedSpanIdForSearch, datasourceType, reset, - showSpanFilterMatchesOnly, - setShowSpanFilterMatchesOnly, + totalSpans, } = props; + const [currentSpanIndex, setCurrentSpanIndex] = useState(-1); const styles = useStyles2(getStyles); useEffect(() => { + setCurrentSpanIndex(-1); setFocusedSpanIdForSearch(''); - }, [search, setFocusedSpanIdForSearch]); + }, [setFocusedSpanIdForSearch, spanFilterMatches]); + + useEffect(() => { + if (spanFilterMatches) { + const spanMatches = Array.from(spanFilterMatches!); + setFocusedSpanIdForSearch(spanMatches[currentSpanIndex]); + } + }, [currentSpanIndex, setFocusedSpanIdForSearch, spanFilterMatches]); const nextResult = () => { reportInteraction('grafana_traces_trace_view_find_next_prev_clicked', { @@ -57,17 +65,14 @@ export default memo(function NewTracePageSearchBar(props: TracePageSearchBarProp direction: 'next', }); - const spanMatches = Array.from(spanFilterMatches!); - const prevMatchedIndex = spanMatches.indexOf(focusedSpanIdForSearch); - // new query || at end, go to start - if (prevMatchedIndex === -1 || prevMatchedIndex === spanMatches.length - 1) { - setFocusedSpanIdForSearch(spanMatches[0]); + if (currentSpanIndex === -1 || (spanFilterMatches && currentSpanIndex === spanFilterMatches.size - 1)) { + setCurrentSpanIndex(0); return; } // get next - setFocusedSpanIdForSearch(spanMatches[prevMatchedIndex + 1]); + setCurrentSpanIndex(currentSpanIndex + 1); }; const prevResult = () => { @@ -77,19 +82,17 @@ export default memo(function NewTracePageSearchBar(props: TracePageSearchBarProp direction: 'prev', }); - const spanMatches = Array.from(spanFilterMatches!); - const prevMatchedIndex = spanMatches.indexOf(focusedSpanIdForSearch); - // new query || at start, go to end - if (prevMatchedIndex === -1 || prevMatchedIndex === 0) { - setFocusedSpanIdForSearch(spanMatches[spanMatches.length - 1]); + if (spanFilterMatches && (currentSpanIndex === -1 || currentSpanIndex === 0)) { + setCurrentSpanIndex(spanFilterMatches.size - 1); return; } // get prev - setFocusedSpanIdForSearch(spanMatches[prevMatchedIndex - 1]); + setCurrentSpanIndex(currentSpanIndex - 1); }; + const buttonEnabled = spanFilterMatches && spanFilterMatches?.size > 0; const resetEnabled = useMemo(() => { return ( (search.serviceName && search.serviceName !== '') || @@ -102,7 +105,26 @@ export default memo(function NewTracePageSearchBar(props: TracePageSearchBarProp }) ); }, [search.serviceName, search.spanName, search.from, search.to, search.tags]); - const buttonEnabled = spanFilterMatches && spanFilterMatches?.size > 0; + + const amountText = spanFilterMatches?.size === 1 ? 'match' : 'matches'; + const matches = + spanFilterMatches?.size === 0 ? ( + <> + 0 matches + + + + + + + ) : currentSpanIndex !== -1 ? ( + `${currentSpanIndex + 1}/${spanFilterMatches?.size} ${amountText}` + ) : ( + `${spanFilterMatches?.size} ${amountText}` + ); return (
@@ -129,6 +151,7 @@ export default memo(function NewTracePageSearchBar(props: TracePageSearchBarProp
+ {spanFilterMatches ? matches : `${totalSpans} spans`}