-
Notifications
You must be signed in to change notification settings - Fork 31
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: DH-14630 - ACL Editor Hooks #1257
Merged
Merged
Changes from 53 commits
Commits
Show all changes
54 commits
Select commit
Hold shift + click to select a range
1f4e4ae
useRollupTable hook
bmingles 1145b33
useInitializeViewportData: handle size change
bmingles 65a792c
createOnTableUpdatedHandler: Remove table arg
bmingles a145a1d
useInitializeViewportData: handle size change
bmingles b19bc58
TableUtils.applyCustomColumns
bmingles dbda2ef
ViewportDataUtils.getItems
bmingles e535b3c
useViewportData: applyFiltersAndRefresh function
bmingles a8f445e
useTableSize hook
bmingles 05b973d
Selection utils
bmingles b7ae694
useTableSize tests
bmingles db37d61
useViewportData: switched to useTableSize
bmingles 5414821
Renamed getItems to getItemsFromListData
bmingles 7f0f9e8
useViewportData: added table to return data
bmingles 58b638b
useTableCloseOnUnmount hook
bmingles 660b08f
useViewportData: removed console log
bmingles 234941a
useDebouncedViewportSearch
bmingles 825a69e
useFormWithDetachedSubmitButton hook
bmingles af403f1
TableUtils: wrappers for table apply methods
bmingles 3d44c49
Comment cleanup
bmingles 1f01a00
useDebouncedViewportSearch: unit tests
bmingles b37ee85
useInitializeView: bug fix + unit tests
bmingles 2e0a9ca
useRollupTable: deleting unused hook
bmingles 9b12198
useInitializeViewportData: re-enabled tests
bmingles da16cad
useTableCloseOnUnmount: unit tests
bmingles ee6e407
useTableSize: unit test improvement
bmingles 131d24d
useViewportData: fixed tests
bmingles 8e0b2af
useViewportData: consolidated size check
bmingles e16e959
TestUtils.asMock
bmingles fae21b0
Simplified mock
bmingles 8dc80cc
useViewportData:
bmingles adf735b
useViewportData: unit test
bmingles 751ece2
TableUtils unit tests
bmingles 6a64ca1
TableUtils: split out executeAndWaitForEvent
bmingles 4dd3d3c
executeAndWaitForEvent tests
bmingles b4463d7
TableUtils: unit tests
bmingles 9b045e6
Renamed arg + added comment
bmingles ba467bb
ViewportDataUtils: fixed tests
bmingles 5039de8
getItemsFromListData test
bmingles 2d48455
useFormWithDetachedSubmitButton: Unit tests
bmingles 3659886
useFormWithDetachedSubmitButton: Cleanup counter
bmingles 34050f4
useFormWithDetachedSubmitButton test
bmingles 36d9816
findLastCall test
bmingles 7ce7ea0
createMockProxy test
bmingles 396ca14
Added missing import after rebase
bmingles 21d7b82
Changed some imports to @deephaven/jsapi-types
bmingles 0187f2a
Change to a type only import
bmingles 124b423
Code coverage
bmingles 7f6eaa5
Code coverage
bmingles b437f6e
Code coverage
bmingles 0c3e0f4
Code coverage
bmingles ebdb99e
Addressed some code review comments
bmingles 55aee4f
Addressed code review comments
bmingles 73f57d7
Moved @testing-library/react-hooks dev dependency
bmingles 3a6c828
Address code review comment
bmingles File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
132 changes: 132 additions & 0 deletions
132
packages/jsapi-components/src/useDebouncedViewportSearch.test.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,132 @@ | ||
import { renderHook } from '@testing-library/react-hooks'; | ||
import type { | ||
Column, | ||
FilterCondition, | ||
FilterValue, | ||
Table, | ||
} from '@deephaven/jsapi-types'; | ||
import { TableUtils } from '@deephaven/jsapi-utils'; | ||
import { TestUtils } from '@deephaven/utils'; | ||
import useDebouncedViewportSearch, { | ||
DEBOUNCE_VIEWPORT_SEARCH_MS, | ||
} from './useDebouncedViewportSearch'; | ||
import { UseViewportDataResult } from './useViewportData'; | ||
|
||
// Mock js api objects | ||
const column = TestUtils.createMockProxy<Column>({ | ||
type: 'java.lang.String', | ||
}); | ||
const filterCondition = TestUtils.createMockProxy<FilterCondition>({}); | ||
const columnFilterValue = TestUtils.createMockProxy<FilterValue>({}); | ||
const matchFilterValue = TestUtils.createMockProxy<FilterValue>({}); | ||
const table = TestUtils.createMockProxy<Table>({}); | ||
const viewportData = TestUtils.createMockProxy< | ||
UseViewportDataResult<unknown, Table> | ||
>({ table }); | ||
|
||
beforeEach(() => { | ||
jest.clearAllMocks(); | ||
jest.useFakeTimers(); | ||
|
||
TestUtils.asMock(column.filter).mockReturnValue(columnFilterValue); | ||
TestUtils.asMock(columnFilterValue.contains).mockReturnValue(filterCondition); | ||
TestUtils.asMock(table.findColumn).mockReturnValue(column); | ||
|
||
jest.spyOn(TableUtils, 'makeFilterValue').mockReturnValue(matchFilterValue); | ||
}); | ||
|
||
it.each([undefined, 400])( | ||
'should return a function that debounces search filtering', | ||
debounceMs => { | ||
const searchText = 'mock.searchText'; | ||
|
||
const { result } = renderHook(() => | ||
useDebouncedViewportSearch(viewportData, 'mock.column', debounceMs) | ||
); | ||
|
||
result.current(searchText); | ||
|
||
expect(viewportData.applyFiltersAndRefresh).not.toHaveBeenCalled(); | ||
|
||
jest.advanceTimersByTime(debounceMs ?? DEBOUNCE_VIEWPORT_SEARCH_MS); | ||
|
||
expect(TableUtils.makeFilterValue).toHaveBeenCalledWith( | ||
column.type, | ||
searchText | ||
); | ||
expect(viewportData.applyFiltersAndRefresh).toHaveBeenCalledWith([ | ||
filterCondition, | ||
]); | ||
} | ||
); | ||
|
||
it('should return a function that safely ignores no table', () => { | ||
const viewportNoTable = TestUtils.createMockProxy< | ||
UseViewportDataResult<unknown, Table> | ||
>({ table: null }); | ||
|
||
const searchText = 'mock.searchText'; | ||
const debounceMs = 400; | ||
|
||
const { result } = renderHook(() => | ||
useDebouncedViewportSearch(viewportNoTable, 'mock.column', debounceMs) | ||
); | ||
|
||
result.current(searchText); | ||
jest.advanceTimersByTime(debounceMs); | ||
|
||
expect(TableUtils.makeFilterValue).not.toHaveBeenCalled(); | ||
expect(viewportNoTable.applyFiltersAndRefresh).not.toHaveBeenCalled(); | ||
}); | ||
|
||
it('should trim search text', () => { | ||
const searchText = ' mock.searchText '; | ||
const trimmedSearchText = 'mock.searchText'; | ||
const debounceMs = 400; | ||
|
||
const { result } = renderHook(() => | ||
useDebouncedViewportSearch(viewportData, 'mock.column', debounceMs) | ||
); | ||
|
||
result.current(searchText); | ||
jest.advanceTimersByTime(debounceMs); | ||
|
||
expect(TableUtils.makeFilterValue).toHaveBeenCalledWith( | ||
column.type, | ||
trimmedSearchText | ||
); | ||
}); | ||
|
||
it.each(['', ' '])( | ||
'should pass apply empty filter if search text is empty', | ||
searchText => { | ||
const debounceMs = 400; | ||
|
||
const { result } = renderHook(() => | ||
useDebouncedViewportSearch(viewportData, 'mock.column', debounceMs) | ||
); | ||
|
||
result.current(searchText); | ||
jest.advanceTimersByTime(debounceMs); | ||
|
||
expect(TableUtils.makeFilterValue).not.toHaveBeenCalled(); | ||
expect(viewportData.applyFiltersAndRefresh).toHaveBeenCalledWith([]); | ||
} | ||
); | ||
|
||
it('should cancel debounce on unmount', () => { | ||
const searchText = 'mock.searchText'; | ||
const debounceMs = 400; | ||
|
||
const { result, unmount } = renderHook(() => | ||
useDebouncedViewportSearch(viewportData, 'mock.column', debounceMs) | ||
); | ||
|
||
result.current(searchText); | ||
jest.advanceTimersByTime(5); | ||
unmount(); | ||
jest.advanceTimersByTime(debounceMs); | ||
|
||
expect(TableUtils.makeFilterValue).not.toHaveBeenCalled(); | ||
expect(viewportData.applyFiltersAndRefresh).not.toHaveBeenCalled(); | ||
}); |
58 changes: 58 additions & 0 deletions
58
packages/jsapi-components/src/useDebouncedViewportSearch.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,58 @@ | ||
import debounce from 'lodash.debounce'; | ||
import type { Table, TreeTable } from '@deephaven/jsapi-types'; | ||
import { TableUtils } from '@deephaven/jsapi-utils'; | ||
import { useEffect, useMemo } from 'react'; | ||
import { UseViewportDataResult } from './useViewportData'; | ||
|
||
export const DEBOUNCE_VIEWPORT_SEARCH_MS = 200; | ||
|
||
/** | ||
* React hook that returns a debounced search callback for filtering a table | ||
* viewport. | ||
* @param viewportData Table viewport to filter | ||
* @param columnName Column name to filter by | ||
* @param debounceMs Millisecond value to debounce | ||
*/ | ||
export default function useDebouncedViewportSearch< | ||
I, | ||
T extends Table | TreeTable | ||
>( | ||
viewportData: UseViewportDataResult<I, T>, | ||
columnName: string, | ||
debounceMs = DEBOUNCE_VIEWPORT_SEARCH_MS | ||
): (searchText: string) => void { | ||
const debouncedSearch = useMemo( | ||
() => | ||
debounce((searchText: string) => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should be cancelling the debounce on unmount. |
||
if (viewportData.table == null) { | ||
return; | ||
} | ||
|
||
const searchTextTrimmed = searchText.trim(); | ||
|
||
if (searchTextTrimmed === '') { | ||
viewportData.applyFiltersAndRefresh([]); | ||
return; | ||
} | ||
|
||
const column = viewportData.table.findColumn(columnName); | ||
const value = TableUtils.makeFilterValue( | ||
column.type, | ||
searchTextTrimmed | ||
); | ||
const filter = [column.filter().contains(value)]; | ||
|
||
viewportData.applyFiltersAndRefresh(filter); | ||
}, debounceMs), | ||
[columnName, debounceMs, viewportData] | ||
); | ||
|
||
useEffect( | ||
() => () => { | ||
debouncedSearch.cancel(); | ||
}, | ||
[debouncedSearch] | ||
); | ||
|
||
return debouncedSearch; | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd add another test: