Skip to content

Commit

Permalink
feat: useSearchableViewportData - support non-text filters (#2092)
Browse files Browse the repository at this point in the history
* Support for search text filtering on non-text column types
* Better type assertions for Formatter.timeZone property
* Made the month capture group in our parsing regex more strict (fixes
#2102)

resolves #2091 

BREAKING CHANGE:
- @deephaven/jsapi-components - The contract of
`useSearchableViewportData` to be more consistent with
`useViewportData`. `usePickerWithSelectedValues` now requires
`timeZone`.
- @deephaven/jsapi-utils - `createSearchTextFilter` requires `timeZone`
  • Loading branch information
bmingles authored Jun 27, 2024
1 parent e13c9d7 commit 7009e21
Show file tree
Hide file tree
Showing 13 changed files with 716 additions and 89 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ const mock = {
searchTextFilter: jest.fn() as FilterConditionFactory,
selectedKey: 'mock.selectedKey',
excludeSelectedValuesFilter: jest.fn() as FilterConditionFactory,
timeZone: 'mock.timeZone',
value: 'mock.value',
viewportData: createMockProxy<WindowedListData<KeyedItem<MockItem>>>(),
};
Expand Down Expand Up @@ -101,6 +102,7 @@ async function renderOnceAndWait(
columnName: mock.columnName,
mapItemToValue: mock.mapItemToValue,
filterConditionFactories: mock.filterConditionFactories,
timeZone: 'mock.timeZone',
...overrides,
})
);
Expand Down Expand Up @@ -170,7 +172,8 @@ it.each([undefined, false, true])(
expect(createSearchTextFilter).toHaveBeenCalledWith(
tableUtils,
mock.columnName,
''
'',
mock.timeZone
);

expect(createSelectedValuesFilter).toHaveBeenCalledWith(
Expand Down Expand Up @@ -246,7 +249,8 @@ it.each([undefined, false, true])(
expect(createSearchTextFilter).toHaveBeenCalledWith(
tableUtils,
mock.columnName,
trimSearchText === true ? mock.searchTextTrimmed : mock.searchText
trimSearchText === true ? mock.searchTextTrimmed : mock.searchText,
mock.timeZone
);

expect(createSelectedValuesFilter).not.toHaveBeenCalled();
Expand Down
13 changes: 11 additions & 2 deletions packages/jsapi-components/src/usePickerWithSelectedValues.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,19 +48,22 @@ export interface UsePickerWithSelectedValuesResult<TItem, TValue> {
* @param mapItemToValue A function to map an item to a value
* @param filterConditionFactories Optional filter condition factories to apply to the list
* @param trimSearchText Whether to trim the search text before filtering. Defaults to false
* @param timeZone The timezone to use for date parsing
*/
export function usePickerWithSelectedValues<TItem, TValue>({
maybeTable,
columnName,
mapItemToValue,
filterConditionFactories = [],
trimSearchText = false,
timeZone,
}: {
maybeTable: dh.Table | null;
columnName: string;
mapItemToValue: (item: KeyedItem<TItem>) => TValue;
filterConditionFactories?: FilterConditionFactory[];
trimSearchText?: boolean;
timeZone: string;
}): UsePickerWithSelectedValuesResult<TItem, TValue> {
const { itemHeight } = usePickerItemScale();

Expand Down Expand Up @@ -117,8 +120,14 @@ export function usePickerWithSelectedValues<TItem, TValue>({
isApplyingFilter || valueExistsIsLoading ? null : valueExists;

const searchTextFilter = useMemo(
() => createSearchTextFilter(tableUtils, columnName, appliedSearchText),
[appliedSearchText, columnName, tableUtils]
() =>
createSearchTextFilter(
tableUtils,
columnName,
appliedSearchText,
timeZone
),
[appliedSearchText, columnName, tableUtils, timeZone]
);

// Filter out selected values from the picker
Expand Down
44 changes: 27 additions & 17 deletions packages/jsapi-components/src/useSearchableViewportData.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { act, renderHook } from '@testing-library/react-hooks';
import type { DebouncedFunc } from 'lodash';
import type { FilterCondition, Table } from '@deephaven/jsapi-types';
import type { dh as DhType } from '@deephaven/jsapi-types';
import {
createSearchTextFilter,
FilterConditionFactory,
Expand Down Expand Up @@ -43,19 +43,23 @@ beforeEach(() => {
describe('useSearchableViewportData: %s', () => {
type SearchTextChangeHandler = DebouncedFunc<(value: string) => void>;

const table = createMockProxy<Table>();
const columnNames = ['Aaa', 'Bbb', 'Ccc'];
const table = createMockProxy<DhType.Table>();
const searchColumnNames = ['Aaa', 'Bbb', 'Ccc'];
const additionalFilterConditionFactories = [
jest.fn(),
jest.fn(),
] as FilterConditionFactory[];

const mockTimeZone = 'mock.timeZone';

const mockResult = {
createSearchTextFilter: jest.fn() as FilterConditionFactory,
useDebouncedCallback: jest.fn() as unknown as SearchTextChangeHandler,
useFilterConditionFactories: [] as FilterCondition[],
useFilterConditionFactories: [] as DhType.FilterCondition[],
useTableUtils: createMockProxy<TableUtils>(),
useViewportData: createMockProxy<UseViewportDataResult<unknown, Table>>({
useViewportData: createMockProxy<
UseViewportDataResult<unknown, DhType.Table>
>({
table,
}),
};
Expand All @@ -82,7 +86,10 @@ describe('useSearchableViewportData: %s', () => {
asMock(useViewportData)
.mockName('useViewportData')
.mockReturnValue(
mockResult.useViewportData as UseViewportDataResult<unknown, Table>
mockResult.useViewportData as UseViewportDataResult<
unknown,
DhType.Table
>
);

asMock(createSearchTextFilter)
Expand All @@ -96,11 +103,12 @@ describe('useSearchableViewportData: %s', () => {

it('should create windowed viewport for list data', () => {
const { result } = renderHook(() =>
useSearchableViewportData(
useSearchableViewportData({
table,
columnNames,
...additionalFilterConditionFactories
)
searchColumnNames,
additionalFilterConditionFactories,
timeZone: mockTimeZone,
})
);

expect(useViewportData).toHaveBeenCalledWith({
Expand All @@ -116,25 +124,27 @@ describe('useSearchableViewportData: %s', () => {
);

expect(result.current).toEqual({
list: mockResult.useViewportData,
...mockResult.useViewportData,
onSearchTextChange: mockResult.useDebouncedCallback,
});
});

it('should filter data based on search text', () => {
const { result } = renderHook(() =>
useSearchableViewportData(
useSearchableViewportData({
table,
columnNames,
...additionalFilterConditionFactories
)
searchColumnNames,
additionalFilterConditionFactories,
timeZone: mockTimeZone,
})
);

const testCommon = (expectedSearchText: string) => {
expect(createSearchTextFilter).toHaveBeenCalledWith(
mockResult.useTableUtils,
columnNames,
expectedSearchText
searchColumnNames,
expectedSearchText,
mockTimeZone
);

expect(useFilterConditionFactories).toHaveBeenCalledWith(
Expand Down
49 changes: 36 additions & 13 deletions packages/jsapi-components/src/useSearchableViewportData.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,33 +12,56 @@ import {
} from '@deephaven/utils';
import { useDebouncedCallback } from '@deephaven/react-hooks';
import { useTableUtils } from './useTableUtils';
import useViewportData, { UseViewportDataResult } from './useViewportData';
import useViewportData, {
UseViewportDataProps,
UseViewportDataResult,
} from './useViewportData';
import useFilterConditionFactories from './useFilterConditionFactories';
import useViewportFilter from './useViewportFilter';

export interface SearchableViewportData<TData> {
list: UseViewportDataResult<TData, dh.Table>;
export interface UseSearchableViewportDataProps<TData>
extends UseViewportDataProps<TData, dh.Table> {
additionalFilterConditionFactories?: FilterConditionFactory[];
searchColumnNames: string | string[];
timeZone: string;
}

export interface SearchableViewportData<TData>
extends UseViewportDataResult<TData, dh.Table> {
onSearchTextChange: (searchText: string) => void;
}

/**
* Use a viewport data list with a search text filter. Supports additional filters.
* @param maybeTable The table to use
* @param table The table to use
* @param itemHeight The height of each item
* @param scrollDebounce The debounce time for scroll events
* @param viewportSize The size of the viewport
* @param viewportPadding The padding around the viewport
* @param deserializeRow The row deserializer
* @param searchColumnNames The column names to search
* @param timeZone Timezone to use for date parsing
* @param additionalFilterConditionFactories Additional filter condition factories
*/
export function useSearchableViewportData<TData>(
maybeTable: dh.Table | null,
searchColumnNames: string | string[],
...additionalFilterConditionFactories: FilterConditionFactory[]
): SearchableViewportData<TData> {
export function useSearchableViewportData<TData>({
additionalFilterConditionFactories = [],
searchColumnNames,
timeZone,
...props
}: UseSearchableViewportDataProps<TData>): SearchableViewportData<TData> {
const tableUtils = useTableUtils();

const [searchText, setSearchText] = useState<string>('');

const searchTextFilter = useMemo(
() => createSearchTextFilter(tableUtils, searchColumnNames, searchText),
[searchColumnNames, searchText, tableUtils]
() =>
createSearchTextFilter(
tableUtils,
searchColumnNames,
searchText,
timeZone
),
[searchColumnNames, searchText, tableUtils, timeZone]
);

const onSearchTextChange = useDebouncedCallback(
Expand All @@ -47,10 +70,10 @@ export function useSearchableViewportData<TData>(
);

const list = useViewportData<TData, dh.Table>({
table: maybeTable,
itemHeight: TABLE_ROW_HEIGHT,
viewportSize: VIEWPORT_SIZE,
viewportPadding: VIEWPORT_PADDING,
...props,
});

const filter = useFilterConditionFactories(
Expand All @@ -62,7 +85,7 @@ export function useSearchableViewportData<TData>(
useViewportFilter(list, filter);

return {
list,
...list,
onSearchTextChange,
};
}
Expand Down
Loading

0 comments on commit 7009e21

Please sign in to comment.