Skip to content
Open
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
@@ -1,7 +1,7 @@
import 'react-datepicker/dist/react-datepicker.css';

import { SerdeUsage, TopicMessageConsuming } from 'generated-sources';
import React, { ChangeEvent, useMemo, useState } from 'react';
import React, { ChangeEvent, useEffect, useMemo, useState } from 'react';
import MultiSelect from 'components/common/MultiSelect/MultiSelect.styled';
import Select from 'components/common/Select/Select';
import { Button } from 'components/common/Button/Button';
Expand Down Expand Up @@ -56,7 +56,6 @@ const Filters: React.FC<FiltersProps> = ({
offset,
setOffsetValue,
search,
setSearch,
partitions: p,
setPartition,
smartFilter,
Expand All @@ -66,6 +65,11 @@ const Filters: React.FC<FiltersProps> = ({

const { data: topic } = useTopicDetails({ clusterName, topicName });
const [createdEditedSmartId, setCreatedEditedSmartId] = useState<string>();
const [searchValue, setSearchValue] = useState(search);

useEffect(() => {
setSearchValue(search);
}, [search]);

const partitions = useMemo(() => {
return (topic?.partitions || []).reduce<{
Expand Down Expand Up @@ -101,7 +105,7 @@ const Filters: React.FC<FiltersProps> = ({
if (isLiveMode(mode) && isFetching) {
abortFetchData();
}
refreshData();
refreshData(searchValue);
};

return (
Expand Down Expand Up @@ -185,7 +189,12 @@ const Filters: React.FC<FiltersProps> = ({
</Button>
</FlexBox>

<Search placeholder="Search" value={search} onChange={setSearch} />
<Search
placeholder="Search"
value={searchValue}
onChange={setSearchValue}
debounceMs={0}
/>
</FlexBox>
<FlexBox
gap="10px"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,16 @@ import { PollingMode } from 'generated-sources';
import { ModeOptions } from 'lib/hooks/filterUtils';
import { MessagesFilterKeysTypes } from 'lib/types';
import { MessagesFilterKeys } from 'lib/constants';
import { useSearchParams } from 'react-router-dom';

const closeIconMock = 'closeIconMock';
const filtersSideBarMock = 'filtersSideBarMock';
const filterMetricsMock = 'filterMetricsMock';

jest.mock('use-debounce', () => ({
useDebouncedCallback: (fn: (value: string) => void) => fn,
}));

jest.mock('lib/hooks/api/topics', () => ({
useTopicDetails: jest.fn(),
}));
Expand All @@ -44,6 +49,11 @@ jest.mock(
const clusterName = 'cluster-name';
const topicName = 'topic-name';

const SearchParamsValue = () => {
const [searchParams] = useSearchParams();
return <div data-testid="search-params">{searchParams.toString()}</div>;
};

const renderComponent = (
props?: Partial<FiltersProps>,
queryParams?: Partial<Record<MessagesFilterKeysTypes, string>>
Expand All @@ -52,7 +62,10 @@ const renderComponent = (

return render(
<WithRoute path={clusterTopicPath()}>
<Filters isFetching={false} abortFetchData={jest.fn()} {...props} />
<>
<Filters isFetching={false} abortFetchData={jest.fn()} {...props} />
<SearchParamsValue />
</>
</WithRoute>,
{
initialEntries: [
Expand Down Expand Up @@ -128,6 +141,36 @@ describe('Filters component', () => {
});
});

describe('search refresh behavior', () => {
it('clears the field without applying the cleared search until refresh', async () => {
renderComponent(
{},
{ [MessagesFilterKeys.stringFilter]: 'searchFilter' }
);

const searchInput = screen.getByPlaceholderText('Search');
expect(searchInput).toHaveValue('searchFilter');

await userEvent.click(screen.getByTestId('search-clear-button'));
expect(searchInput).toHaveValue('');
expect(screen.getByTestId('search-params')).toHaveTextContent(
`${MessagesFilterKeys.stringFilter}=searchFilter`
);
expect(screen.getByTestId('search-params')).not.toHaveTextContent(
`${MessagesFilterKeys.r}=r`
);

await userEvent.click(screen.getByText('Refresh'));

expect(screen.getByTestId('search-params')).not.toHaveTextContent(
MessagesFilterKeys.stringFilter
);
expect(screen.getByTestId('search-params')).toHaveTextContent(
`${MessagesFilterKeys.r}=r`
);
});
});

describe('change from and to offset filter', () => {
const inputValue = 'Hello World!';

Expand Down
43 changes: 26 additions & 17 deletions frontend/src/components/common/Search/Search.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ interface SearchProps {
onChange?: (value: string) => void;
value?: string;
extraActions?: ReactNode;
debounceMs?: number;
}

const Search: React.FC<SearchProps> = ({
Expand All @@ -26,32 +27,43 @@ const Search: React.FC<SearchProps> = ({
value,
onChange,
extraActions,
debounceMs = 500,
}) => {
const [searchParams, setSearchParams] = useSearchParams();
const ref = useRef<ComponentRef<'input'>>(null);
const [showIcon, setShowIcon] = useState(!!value || !!searchParams.get('q'));
const [showIcon, setShowIcon] = useState(
typeof value !== 'undefined' ? !!value : !!searchParams.get('q')
);

useEffect(() => {
if (ref.current !== null && value) {
if (ref.current !== null && typeof value !== 'undefined') {
ref.current.value = value;
}
}, [value]);
setShowIcon(
typeof value !== 'undefined' ? !!value : !!searchParams.get('q')
);
}, [searchParams, value]);

const handleChange = useDebouncedCallback((e) => {
setShowIcon(!!e.target.value);
const handleChange = useDebouncedCallback((nextValue: string) => {
setShowIcon(!!nextValue);
if (ref.current != null) {
ref.current.value = e.target.value;
ref.current.value = nextValue;
}
if (onChange) {
onChange(e.target.value);
onChange(nextValue);
} else {
searchParams.set('q', e.target.value);
if (searchParams.get('page')) {
searchParams.set('page', '1');
}
setSearchParams(searchParams);
setSearchParams((params) => {
const nextParams = new URLSearchParams(params);

nextParams.set('q', nextValue);
if (nextParams.get('page')) {
nextParams.set('page', '1');
}

return nextParams;
});
}
}, 500);
}, debounceMs);
Comment thread
coderabbitai[bot] marked this conversation as resolved.

const clearSearchValue = () => {
if (onChange) {
Expand All @@ -64,17 +76,14 @@ const Search: React.FC<SearchProps> = ({
if (ref.current != null) {
ref.current.value = '';
}
if (onChange) {
onChange('');
}
setShowIcon(false);
};

return (
<Input
type="text"
placeholder={placeholder}
onChange={handleChange}
onChange={({ target: { value: nextValue } }) => handleChange(nextValue)}
defaultValue={value || searchParams.get('q') || ''}
inputSize="M"
disabled={disabled}
Expand Down
41 changes: 33 additions & 8 deletions frontend/src/components/common/Search/__tests__/Search.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import Search from 'components/common/Search/Search';
import React from 'react';
import { render } from 'lib/testHelpers';
import userEvent from '@testing-library/user-event';
import { screen, fireEvent } from '@testing-library/react';
import { screen, fireEvent, waitFor } from '@testing-library/react';
import { useSearchParams } from 'react-router-dom';

jest.mock('use-debounce', () => ({
Expand All @@ -16,11 +16,14 @@ jest.mock('react-router-dom', () => ({
}));

const placeholder = 'I am a search placeholder';
let searchParamsMock: URLSearchParams;

describe('Search', () => {
beforeEach(() => {
setSearchParamsMock.mockClear();
searchParamsMock = new URLSearchParams();
(useSearchParams as jest.Mock).mockImplementation(() => [
new URLSearchParams(),
searchParamsMock,
setSearchParamsMock,
]);
});
Expand All @@ -32,6 +35,25 @@ describe('Search', () => {
expect(setSearchParamsMock).toHaveBeenCalledTimes(5);
});

it('updates search params from the latest URL state', () => {
render(<Search placeholder={placeholder} />);

fireEvent.change(screen.getByPlaceholderText(placeholder), {
target: { value: 'topic' },
});

const updateSearchParams = setSearchParamsMock.mock.calls[0][0] as (
params: URLSearchParams
) => URLSearchParams;
const nextParams = updateSearchParams(
new URLSearchParams('page=3&cluster=local')
);

expect(nextParams.get('q')).toBe('topic');
expect(nextParams.get('page')).toBe('1');
expect(nextParams.get('cluster')).toBe('local');
});

it('when placeholder is provided', () => {
render(<Search placeholder={placeholder} />);
expect(screen.getByPlaceholderText(placeholder)).toBeInTheDocument();
Expand All @@ -57,19 +79,22 @@ describe('Search', () => {
});

it('Clear button should clear text from input', async () => {
render(<Search placeholder={placeholder} />);
render(<Search placeholder={placeholder} onChange={jest.fn()} />);

const searchField = screen.getAllByRole('textbox')[0];
fireEvent.change(searchField, { target: { value: 'hello' } });
await userEvent.type(searchField, 'hello');
expect(searchField).toHaveValue('hello');

let clearButton = screen.queryByTestId('search-clear-button');
const clearButton = await screen.findByTestId('search-clear-button');
expect(clearButton).toBeInTheDocument();
await userEvent.click(clearButton!);
await userEvent.click(clearButton);

expect(searchField).toHaveValue('');

clearButton = screen.queryByTestId('search-clear-button');
expect(clearButton).not.toBeInTheDocument();
await waitFor(() =>
expect(
screen.queryByTestId('search-clear-button')
).not.toBeInTheDocument()
);
});
});
29 changes: 23 additions & 6 deletions frontend/src/lib/hooks/useMessagesFilters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ export function usePaginateTopics(initSearchParams?: URLSearchParams) {

export function useMessagesFilters(topicName: string) {
const [searchParams, setSearchParams] = useSearchParams();
const refreshData = useRefreshData(searchParams);
const { clusterName } = useAppParams<{ clusterName: ClusterName }>();

const storageKey = `${topicName}:${clusterName}`;
Expand All @@ -73,6 +72,16 @@ export function useMessagesFilters(topicName: string) {
removeMessagesFiltersField,
} = useMessagesFiltersFields(storageKey);

const syncSearchValue = (params: URLSearchParams, value: string) => {
if (value) {
setMessagesFiltersField(MessagesFilterKeys.stringFilter, value);
params.set(MessagesFilterKeys.stringFilter, value);
} else {
removeMessagesFiltersField(MessagesFilterKeys.stringFilter);
params.delete(MessagesFilterKeys.stringFilter);
}
};

useEffect(() => {
setSearchParams((params) => {
initMessagesFiltersFields(params);
Expand Down Expand Up @@ -179,13 +188,21 @@ export function useMessagesFilters(topicName: string) {

const setSearch = (value: string) => {
setSearchParams((params) => {
if (value) {
setMessagesFiltersField(MessagesFilterKeys.stringFilter, value);
params.set(MessagesFilterKeys.stringFilter, value);
syncSearchValue(params, value);
return params;
});
};

const refreshData = (nextSearch = search) => {
setSearchParams((params) => {
syncSearchValue(params, nextSearch);

if (params.get(MessagesFilterKeys.r)) {
params.delete(MessagesFilterKeys.r);
} else {
removeMessagesFiltersField(MessagesFilterKeys.stringFilter);
params.delete(MessagesFilterKeys.stringFilter);
params.set(MessagesFilterKeys.r, 'r');
}

return params;
});
};
Expand Down
Loading