From 0a35606376b6b579890dda6e794ac1e38ac7cb2b Mon Sep 17 00:00:00 2001 From: AllanZhengYP Date: Thu, 7 Nov 2024 16:51:43 -0800 Subject: [PATCH 1/2] feat(react-storage): enable upload crc32 by default (#6029) --- .../StorageBrowser/actions/handlers/__tests__/upload.spec.ts | 1 + .../src/components/StorageBrowser/actions/handlers/upload.ts | 1 + 2 files changed, 2 insertions(+) diff --git a/packages/react-storage/src/components/StorageBrowser/actions/handlers/__tests__/upload.spec.ts b/packages/react-storage/src/components/StorageBrowser/actions/handlers/__tests__/upload.spec.ts index 5a421c30141..1f724223ff7 100644 --- a/packages/react-storage/src/components/StorageBrowser/actions/handlers/__tests__/upload.spec.ts +++ b/packages/react-storage/src/components/StorageBrowser/actions/handlers/__tests__/upload.spec.ts @@ -71,6 +71,7 @@ describe('uploadHandler', () => { locationCredentialsProvider: credentials, onProgress: expect.any(Function), preventOverwrite: true, + checksumAlgorithm: 'crc-32', }, path: `${baseInput.destinationPrefix}${baseInput.data.key}`, }; diff --git a/packages/react-storage/src/components/StorageBrowser/actions/handlers/upload.ts b/packages/react-storage/src/components/StorageBrowser/actions/handlers/upload.ts index cc1671c7cd0..3063629b80c 100644 --- a/packages/react-storage/src/components/StorageBrowser/actions/handlers/upload.ts +++ b/packages/react-storage/src/components/StorageBrowser/actions/handlers/upload.ts @@ -67,6 +67,7 @@ export const uploadHandler: UploadHandler = ({ }, preventOverwrite, customEndpoint, + checksumAlgorithm: 'crc-32', }, }; From 7f22edddf77b4d5f90681dc9df034576ea4e251c Mon Sep 17 00:00:00 2001 From: Jordan Van Ness Date: Fri, 8 Nov 2024 08:30:32 -0800 Subject: [PATCH 2/2] chore: update usePaginate to use more generic props for paginate component (#5989) * chore: update usePaginate to use more generic props for paginate component * chore: fix typos, un-alphabetize props to reduce irrelevant changes * fix(tests): fix failures introduced to view testing * chore: change handlePaginate to onPaginate, make control props optional * chore: remove tests of pagination disabling props * chore: update usePaginate usage in useDestinationPicker * chore: update usage of usePaginate in useDestinationPicker * chore: consolodating pageItems, various other simplifications * chore: move result count determination into usePaginate, remove out of bounds pagination case --- .../views/Controls/Paginate.tsx | 32 +++--- .../Controls/__tests__/Paginate.spec.tsx | 9 +- .../useDestinationPicker.spec.ts.snap | 9 +- .../CopyView/useDestinationPicker.ts | 26 +++-- .../LocationActionView/DestinationPicker.tsx | 22 ++-- .../LocationDetailView/LocationDetailView.tsx | 14 ++- .../__tests__/useLocationDetailView.spec.tsx | 14 +-- .../useLocationDetailView.ts | 51 +++------ .../views/LocationsView/LocationsView.tsx | 18 ++-- .../__tests__/LocationsView.spec.tsx | 2 +- .../__tests__/useLocationsView.spec.tsx | 16 +-- .../views/LocationsView/useLocationsView.ts | 47 +++----- .../views/hooks/__tests__/usePaginate.spec.ts | 100 +++++++----------- .../StorageBrowser/views/hooks/usePaginate.ts | 71 +++++++------ 14 files changed, 179 insertions(+), 252 deletions(-) diff --git a/packages/react-storage/src/components/StorageBrowser/views/Controls/Paginate.tsx b/packages/react-storage/src/components/StorageBrowser/views/Controls/Paginate.tsx index a487afa21de..9013b83bc65 100644 --- a/packages/react-storage/src/components/StorageBrowser/views/Controls/Paginate.tsx +++ b/packages/react-storage/src/components/StorageBrowser/views/Controls/Paginate.tsx @@ -23,13 +23,7 @@ const getButtonVariantProps = ( variant: PaginateVariant, props: PaginateControlProps ): ButtonElementProps => { - const { - currentPage, - disableNext, - disablePrevious, - handleNext, - handlePrevious, - } = props; + const { currentPage, hasMorePages, onPaginate, highestPageVisited } = props; let ariaCurrent, ariaLabel, className, disabled, onClick, children; @@ -48,8 +42,14 @@ const getButtonVariantProps = ( case 'paginate-next': ariaLabel = 'Go to next page'; className = `${BLOCK_NAME}__button-next`; - disabled = disableNext; - onClick = handleNext; + disabled = + !!currentPage && + !!highestPageVisited && + currentPage >= highestPageVisited && + !hasMorePages; + onClick = () => { + if (currentPage && onPaginate) onPaginate(currentPage + 1); + }; children = ( ); @@ -57,8 +57,10 @@ const getButtonVariantProps = ( case 'paginate-previous': ariaLabel = 'Go to previous page'; className = `${BLOCK_NAME}__button-next`; - disabled = disablePrevious; - onClick = handlePrevious; + disabled = !!currentPage && currentPage <= 1; + onClick = () => { + if (currentPage && onPaginate) onPaginate(currentPage - 1); + }; children = ( ); @@ -83,13 +85,11 @@ export interface PaginateControlProps { // eslint-disable-next-line react/no-unused-prop-types currentPage?: number; // eslint-disable-next-line react/no-unused-prop-types - disableNext?: boolean; + onPaginate?: (page: number) => void; // eslint-disable-next-line react/no-unused-prop-types - disablePrevious?: boolean; + highestPageVisited?: number; // eslint-disable-next-line react/no-unused-prop-types - handleNext?: () => void; - // eslint-disable-next-line react/no-unused-prop-types - handlePrevious?: () => void; + hasMorePages?: boolean; } export const PaginateControl = ( diff --git a/packages/react-storage/src/components/StorageBrowser/views/Controls/__tests__/Paginate.spec.tsx b/packages/react-storage/src/components/StorageBrowser/views/Controls/__tests__/Paginate.spec.tsx index 5c01f56e800..f74c14c9929 100644 --- a/packages/react-storage/src/components/StorageBrowser/views/Controls/__tests__/Paginate.spec.tsx +++ b/packages/react-storage/src/components/StorageBrowser/views/Controls/__tests__/Paginate.spec.tsx @@ -4,7 +4,14 @@ import { PaginateControl } from '../Paginate'; describe('PaginationControl', () => { it('renders the PaginationControl', async () => { - render(); + render( + + ); const nav = screen.getByRole('navigation', { name: 'Pagination', diff --git a/packages/react-storage/src/components/StorageBrowser/views/LocationActionView/CopyView/__tests__/__snapshots__/useDestinationPicker.spec.ts.snap b/packages/react-storage/src/components/StorageBrowser/views/LocationActionView/CopyView/__tests__/__snapshots__/useDestinationPicker.spec.ts.snap index 1b8c6f4dfe2..7517ce0a724 100644 --- a/packages/react-storage/src/components/StorageBrowser/views/LocationActionView/CopyView/__tests__/__snapshots__/useDestinationPicker.spec.ts.snap +++ b/packages/react-storage/src/components/StorageBrowser/views/LocationActionView/CopyView/__tests__/__snapshots__/useDestinationPicker.spec.ts.snap @@ -4,17 +4,14 @@ exports[`useDestinationPicker should return the correct initial state 1`] = ` { "bucket": "bucket", "currentPage": 1, - "handleNext": [Function], - "handlePrevious": [Function], "hasError": false, "hasNextToken": false, + "highestPageVisited": 0, "isLoading": true, "items": [], "message": undefined, + "onPaginate": [Function], "onSearch": [Function], - "range": [ - 0, - 10, - ], + "pageItems": [], } `; diff --git a/packages/react-storage/src/components/StorageBrowser/views/LocationActionView/CopyView/useDestinationPicker.ts b/packages/react-storage/src/components/StorageBrowser/views/LocationActionView/CopyView/useDestinationPicker.ts index 47b32847792..b390491869d 100644 --- a/packages/react-storage/src/components/StorageBrowser/views/LocationActionView/CopyView/useDestinationPicker.ts +++ b/packages/react-storage/src/components/StorageBrowser/views/LocationActionView/CopyView/useDestinationPicker.ts @@ -3,6 +3,7 @@ import { usePaginate } from '../../hooks/usePaginate'; import { listLocationItemsHandler, ListLocationItemsHandlerOutput, + LocationItemData, } from '../../../actions'; import { useGetActionInput } from '../../../providers/configuration'; import { getDestinationListFullPrefix } from '../utils/getDestinationPickerDataTable'; @@ -34,12 +35,12 @@ export const useDestinationPicker = ({ hasNextToken: boolean; currentPage: number; isLoading: boolean; + highestPageVisited: number; hasError: boolean; message: string | undefined; - handleNext: () => void; - handlePrevious: () => void; + pageItems: LocationItemData[]; + onPaginate: (page: number) => void; onSearch: (query: string) => void; - range: [number, number]; } => { const prefix = getDestinationListFullPrefix(destinationList); @@ -55,10 +56,9 @@ export const useDestinationPicker = ({ const { items, nextToken } = data; - const resultCount = items.length; const hasNextToken = !!nextToken; - const onPaginateNext = () => { + const paginateCallback = () => { handleList({ config: getInput(), prefix, @@ -66,10 +66,12 @@ export const useDestinationPicker = ({ }); }; - const { currentPage, handlePaginateNext, handlePaginatePrevious, range } = + const { currentPage, onPaginate, highestPageVisited, pageItems } = usePaginate({ - onPaginateNext, + items, + paginateCallback, pageSize: 10, + hasNextToken, }); useEffect(() => { @@ -90,14 +92,11 @@ export const useDestinationPicker = ({ hasNextToken, currentPage, isLoading, + highestPageVisited, hasError, message, - handleNext: () => { - handlePaginateNext({ resultCount, hasNextToken }); - }, - handlePrevious: () => { - handlePaginatePrevious(); - }, + pageItems, + onPaginate, onSearch: (query: string) => { handleList({ config: getInput(), @@ -108,6 +107,5 @@ export const useDestinationPicker = ({ }, }); }, - range, }; }; diff --git a/packages/react-storage/src/components/StorageBrowser/views/LocationActionView/DestinationPicker.tsx b/packages/react-storage/src/components/StorageBrowser/views/LocationActionView/DestinationPicker.tsx index dadb0cdbfc4..8158fdc9063 100644 --- a/packages/react-storage/src/components/StorageBrowser/views/LocationActionView/DestinationPicker.tsx +++ b/packages/react-storage/src/components/StorageBrowser/views/LocationActionView/DestinationPicker.tsx @@ -42,14 +42,14 @@ export const DestinationPicker = ({ const { bucket, items, + highestPageVisited, hasNextToken, currentPage, isLoading, hasError, - handleNext, - handlePrevious, + onPaginate, onSearch, - range, + pageItems, } = useDestinationPicker({ destinationList }); const handleNavigateFolder = (key: string) => { @@ -62,15 +62,6 @@ export const DestinationPicker = ({ onDestinationChange(newPath); }; - const pageItems = React.useMemo(() => { - const [start, end] = range; - return items.slice(start, end); - }, [range, items]); - - const disableNext = - !hasNextToken && currentPage * DEFAULT_PAGE_SIZE > items.length; - const disablePrevious = currentPage === 1; - const tableData = getDestinationPickerTableData({ items: pageItems, handleNavigateFolder, @@ -122,10 +113,9 @@ export const DestinationPicker = ({ diff --git a/packages/react-storage/src/components/StorageBrowser/views/LocationDetailView/LocationDetailView.tsx b/packages/react-storage/src/components/StorageBrowser/views/LocationDetailView/LocationDetailView.tsx index d514daba53e..cb46a9fa41e 100644 --- a/packages/react-storage/src/components/StorageBrowser/views/LocationDetailView/LocationDetailView.tsx +++ b/packages/react-storage/src/components/StorageBrowser/views/LocationDetailView/LocationDetailView.tsx @@ -67,9 +67,9 @@ export function LocationDetailView({ const { page, pageItems, + hasNextPage, + highestPageVisited, isLoading, - isPaginatePreviousDisabled, - isPaginateNextDisabled, currentLocation, currentPath, areAllFilesSelected, @@ -82,8 +82,7 @@ export function LocationDetailView({ showIncludeSubfolders, onDropFiles, onRefresh, - onPaginateNext, - onPaginatePrevious, + onPaginate, onDownload, onNavigate, onNavigateHome, @@ -133,10 +132,9 @@ export function LocationDetailView({ /> { useLocationDetailView(initialValues) ); - expect(result.current.isPaginateNextDisabled).toBe(true); - expect(result.current.isPaginatePreviousDisabled).toBe(true); expect(result.current.pageItems).toEqual([]); // set up first page mock @@ -223,24 +221,20 @@ describe('useLocationDetailView', () => { // go next act(() => { - result.current.onPaginateNext(); + result.current.onPaginate(2); }); // check if data is correct expect(result.current.page).toEqual(2); - expect(result.current.isPaginateNextDisabled).toBe(true); - expect(result.current.isPaginatePreviousDisabled).toBe(false); expect(result.current.pageItems).toEqual(testData.slice(3)); // go previous act(() => { - result.current.onPaginatePrevious(); + result.current.onPaginate(1); }); // check data expect(result.current.page).toEqual(1); - expect(result.current.isPaginateNextDisabled).toBe(false); - expect(result.current.isPaginatePreviousDisabled).toBe(true); expect(result.current.pageItems).toEqual(testData.slice(0, 3)); }); @@ -248,7 +242,7 @@ describe('useLocationDetailView', () => { useStoreSpy.mockReturnValue([testStoreState, mockDispatchStoreAction]); const mockDataState = { - data: { items: [], nextToken: undefined }, + data: { result: [], nextToken: 'token123' }, message: '', hasError: false, isLoading: false, @@ -260,7 +254,7 @@ describe('useLocationDetailView', () => { // move to next page to check behavior act(() => { - result.current.onPaginateNext(); + result.current.onPaginate(2); }); expect(result.current.page).toEqual(2); diff --git a/packages/react-storage/src/components/StorageBrowser/views/LocationDetailView/useLocationDetailView.ts b/packages/react-storage/src/components/StorageBrowser/views/LocationDetailView/useLocationDetailView.ts index 23a56541af1..cb29e24b9e5 100644 --- a/packages/react-storage/src/components/StorageBrowser/views/LocationDetailView/useLocationDetailView.ts +++ b/packages/react-storage/src/components/StorageBrowser/views/LocationDetailView/useLocationDetailView.ts @@ -11,16 +11,16 @@ import { LocationItemData, listLocationItemsHandler, } from '../../actions'; -import { isFile, isLastPage } from '../utils'; +import { isFile } from '../utils'; import { createEnhancedListHandler } from '../../actions/createEnhancedListHandler'; import { useGetActionInput } from '../../providers/configuration'; import { displayText } from '../../displayText/en'; interface UseLocationDetailView { hasError: boolean; + hasNextPage: boolean; + highestPageVisited: number; isLoading: boolean; - isPaginateNextDisabled: boolean; - isPaginatePreviousDisabled: boolean; currentLocation: LocationData | undefined; currentPath: string; areAllFilesSelected: boolean; @@ -36,8 +36,7 @@ interface UseLocationDetailView { onRefresh: () => void; onNavigate: (location: LocationData, path?: string) => void; onNavigateHome: () => void; - onPaginateNext: () => void; - onPaginatePrevious: () => void; + onPaginate: (page: number) => void; onDownload: (fileItem: FileData) => void; onSearch: (query: string, includeSubfolders?: boolean) => void; onSelect: (isSelected: boolean, fileItem: FileData) => void; @@ -47,8 +46,6 @@ interface UseLocationDetailView { export type LocationDetailViewActionType = | { type: 'REFRESH_DATA' } // refresh data only | { type: 'RESET' } // reset view to initial state - | { type: 'PAGINATE_NEXT' } - | { type: 'PAGINATE_PREVIOUS' } | { type: 'PAGINATE'; page: number } | { type: 'ACCESS_ITEM'; key: string } | { type: 'NAVIGATE'; location: LocationData; path: string } @@ -95,7 +92,6 @@ export function useLocationDetailView( const { prefix } = currentLocation ?? {}; const { fileDataItems } = locationItems; const hasInvalidPrefix = isUndefined(prefix); - const { pageSize } = listOptions; const getConfig = useGetActionInput(); @@ -106,9 +102,8 @@ export function useLocationDetailView( // set up pagination const { items, nextToken } = data; - const resultCount = items.length; const hasNextToken = !!nextToken; - const onPaginateNext = () => { + const paginateCallback = () => { if (hasInvalidPrefix || !nextToken) return; dispatchStoreAction({ type: 'RESET_LOCATION_ITEMS' }); handleList({ @@ -118,20 +113,17 @@ export function useLocationDetailView( }); }; - const onPaginatePrevious = () => { - dispatchStoreAction({ type: 'RESET_LOCATION_ITEMS' }); - }; - const { currentPage, - handlePaginateNext, - handlePaginatePrevious, + onPaginate, handleReset, - range, + highestPageVisited, + pageItems, } = usePaginate({ - onPaginateNext, - onPaginatePrevious, - pageSize, + items, + paginateCallback, + pageSize: listOptions.pageSize, + hasNextToken, }); const onRefresh = () => { @@ -163,20 +155,12 @@ export function useLocationDetailView( key, ]); - const pageItems = React.useMemo(() => { - const [start, end] = range; - return items.slice(start, end); - }, [range, items]); - // Logic for Select All Files functionality const fileItems = React.useMemo( () => pageItems.filter((item): item is FileData => item.type === 'FILE'), [pageItems] ); const areAllFilesSelected = fileDataItems?.length === fileItems.length; - const isFinalPage = - !hasNextToken && isLastPage(currentPage, resultCount, pageSize); - const hasNoResults = pageItems.length === 0; const shouldShowEmptyMessage = pageItems.length === 0 && !isLoading && !hasError; @@ -188,20 +172,15 @@ export function useLocationDetailView( areAllFilesSelected, fileDataItems, hasFiles: fileItems.length > 0, - isPaginateNextDisabled: - isFinalPage || isLoading || hasError || hasNoResults, - isPaginatePreviousDisabled: - currentPage <= 1 || isLoading || hasError || hasNoResults, hasError, + hasNextPage: hasNextToken, + highestPageVisited, message, shouldShowEmptyMessage, isLoading, + onPaginate, showIncludeSubfolders: true, searchPlaceholder: displayText.searchDetailPlaceholder, - onPaginatePrevious: handlePaginatePrevious, - onPaginateNext: () => { - handlePaginateNext({ resultCount, hasNextToken }); - }, onRefresh, onNavigate: (location: LocationData, path?: string) => { onNavigate?.(location, path); diff --git a/packages/react-storage/src/components/StorageBrowser/views/LocationsView/LocationsView.tsx b/packages/react-storage/src/components/StorageBrowser/views/LocationsView/LocationsView.tsx index 784d9ce77da..c01f35f9676 100644 --- a/packages/react-storage/src/components/StorageBrowser/views/LocationsView/LocationsView.tsx +++ b/packages/react-storage/src/components/StorageBrowser/views/LocationsView/LocationsView.tsx @@ -47,18 +47,17 @@ export function LocationsView({ ...props }: LocationsViewProps): React.JSX.Element { const { - pageItems, hasError, - message, - isPaginatePreviousDisabled, - isPaginateNextDisabled, + hasNextPage, + highestPageVisited, page, isLoading, + pageItems, + message, searchPlaceholder, shouldShowEmptyMessage, onRefresh, - onPaginateNext, - onPaginatePrevious, + onPaginate, onNavigate, onSearch, } = useLocationsView(props); @@ -82,10 +81,9 @@ export function LocationsView({ { useLocationsDataSpy.mockReturnValue([ { - data: { result: results, nextToken: 'some-token' }, + data: { result: results, nextToken: undefined }, hasError: true, isLoading: false, message: errorMessage, diff --git a/packages/react-storage/src/components/StorageBrowser/views/LocationsView/__tests__/useLocationsView.spec.tsx b/packages/react-storage/src/components/StorageBrowser/views/LocationsView/__tests__/useLocationsView.spec.tsx index 19b9eabd858..3947aa2ac48 100644 --- a/packages/react-storage/src/components/StorageBrowser/views/LocationsView/__tests__/useLocationsView.spec.tsx +++ b/packages/react-storage/src/components/StorageBrowser/views/LocationsView/__tests__/useLocationsView.spec.tsx @@ -108,8 +108,6 @@ describe('useLocationsView', () => { useLocationsView(initialState) ); - expect(result.current.isPaginateNextDisabled).toBe(true); - expect(result.current.isPaginatePreviousDisabled).toBe(true); expect(result.current.pageItems).toEqual([]); // mock first page data @@ -127,8 +125,6 @@ describe('useLocationsView', () => { rerender(initialState); // check first page expect(result.current.page).toEqual(1); - expect(result.current.isPaginateNextDisabled).toBe(false); - expect(result.current.isPaginatePreviousDisabled).toBe(true); expect(result.current.pageItems).toEqual( mockData.slice(0, EXPECTED_PAGE_SIZE) ); @@ -143,26 +139,22 @@ describe('useLocationsView', () => { // go next act(() => { - result.current.onPaginateNext(); + result.current.onPaginate(2); }); // check next page expect(result.current.page).toEqual(2); - expect(result.current.isPaginateNextDisabled).toBe(true); - expect(result.current.isPaginatePreviousDisabled).toBe(false); expect(result.current.pageItems).toEqual( mockData.slice(EXPECTED_PAGE_SIZE) ); // go back act(() => { - result.current.onPaginatePrevious(); + result.current.onPaginate(1); }); // check first page expect(result.current.page).toEqual(1); - expect(result.current.isPaginateNextDisabled).toBe(false); - expect(result.current.isPaginatePreviousDisabled).toBe(true); expect(result.current.pageItems).toEqual( mockData.slice(0, EXPECTED_PAGE_SIZE) ); @@ -170,7 +162,7 @@ describe('useLocationsView', () => { it('should handle refreshing location data', () => { const mockDataState = { - data: { result: [], nextToken: undefined }, + data: { result: [], nextToken: 'token123' }, message: '', hasError: false, isLoading: false, @@ -181,7 +173,7 @@ describe('useLocationsView', () => { // go to second page to verify reset behavior act(() => { - result.current.onPaginateNext(); + result.current.onPaginate(2); }); expect(result.current.page).toEqual(2); diff --git a/packages/react-storage/src/components/StorageBrowser/views/LocationsView/useLocationsView.ts b/packages/react-storage/src/components/StorageBrowser/views/LocationsView/useLocationsView.ts index 09245c79d31..00444f2083b 100644 --- a/packages/react-storage/src/components/StorageBrowser/views/LocationsView/useLocationsView.ts +++ b/packages/react-storage/src/components/StorageBrowser/views/LocationsView/useLocationsView.ts @@ -5,14 +5,12 @@ import { usePaginate } from '../hooks/usePaginate'; import { LocationData } from '../../actions'; import { useStore } from '../../providers/store'; import { displayText } from '../../displayText/en'; -import { isLastPage } from '../utils'; interface UseLocationsView { hasNextPage: boolean; hasError: boolean; + highestPageVisited: number; isLoading: boolean; - isPaginateNextDisabled: boolean; - isPaginatePreviousDisabled: boolean; message: string | undefined; searchPlaceholder: string; shouldShowEmptyMessage: boolean; @@ -20,8 +18,7 @@ interface UseLocationsView { page: number; onNavigate: (location: LocationData) => void; onRefresh: () => void; - onPaginateNext: () => void; - onPaginatePrevious: () => void; + onPaginate: (page: number) => void; onSearch: (query: string) => void; } @@ -32,8 +29,6 @@ interface InitialValues { export type LocationsViewActionType = | { type: 'REFRESH_DATA' } | { type: 'RESET' } - | { type: 'PAGINATE_NEXT' } - | { type: 'PAGINATE_PREVIOUS' } | { type: 'PAGINATE'; page: number } | { type: 'NAVIGATE'; location: LocationData } | { type: 'SEARCH'; query: string }; @@ -58,7 +53,6 @@ export function useLocationsView( const [term, setTerm] = React.useState(''); const { data, message, hasError, isLoading } = state; const { result, nextToken } = data; - const resultCount = result.length; const hasNextToken = !!nextToken; const onNavigate = options?.onNavigate; @@ -69,7 +63,6 @@ export function useLocationsView( ...initialValues, }); const listOptions = listOptionsRef.current; - const { pageSize } = listOptions; // initial load React.useEffect(() => { @@ -79,23 +72,25 @@ export function useLocationsView( }, [handleList, listOptions]); // set up pagination - const onPaginateNext = () => + const paginateCallback = () => { + if (!nextToken) return; handleList({ options: { ...listOptions, nextToken }, }); + }; const { currentPage, - handlePaginateNext, - handlePaginatePrevious, + onPaginate, handleReset, - range, - } = usePaginate({ onPaginateNext, pageSize }); - - const pageItems = React.useMemo(() => { - const [start, end] = range; - return result.slice(start, end); - }, [range, result]); + highestPageVisited, + pageItems, + } = usePaginate({ + items: result, + paginateCallback, + pageSize: listOptions.pageSize, + hasNextToken, + }); const filteredItems = React.useMemo(() => { return pageItems.filter( @@ -103,10 +98,6 @@ export function useLocationsView( ); }, [pageItems, term]); - const isFinalPage = - !hasNextToken && isLastPage(currentPage, resultCount, pageSize); - const hasNoResults = pageItems.length === 0; - const shouldShowEmptyMessage = pageItems.length === 0 && !isLoading && !hasError; @@ -114,12 +105,9 @@ export function useLocationsView( isLoading, hasError, message, - isPaginateNextDisabled: - isFinalPage || isLoading || hasError || hasNoResults, - isPaginatePreviousDisabled: - currentPage <= 1 || isLoading || hasError || hasNoResults, page: currentPage, hasNextPage: hasNextToken, + highestPageVisited, pageItems: filteredItems, searchPlaceholder: displayText.filterLocationsPlaceholder, shouldShowEmptyMessage, @@ -133,10 +121,7 @@ export function useLocationsView( options: { ...listOptions, refresh: true }, }); }, - onPaginateNext: () => { - handlePaginateNext({ resultCount, hasNextToken }); - }, - onPaginatePrevious: handlePaginatePrevious, + onPaginate, onSearch: (query: string) => { setTerm(query); }, diff --git a/packages/react-storage/src/components/StorageBrowser/views/hooks/__tests__/usePaginate.spec.ts b/packages/react-storage/src/components/StorageBrowser/views/hooks/__tests__/usePaginate.spec.ts index 2f138fb0bac..8e843acdd35 100644 --- a/packages/react-storage/src/components/StorageBrowser/views/hooks/__tests__/usePaginate.spec.ts +++ b/packages/react-storage/src/components/StorageBrowser/views/hooks/__tests__/usePaginate.spec.ts @@ -1,89 +1,69 @@ +import { usePaginate } from '../usePaginate'; import { act, renderHook } from '@testing-library/react-hooks'; -import { usePaginate } from '../usePaginate'; +jest.mock('../../../controls/context'); describe('usePaginate', () => { - it('returns the expected values on initial call', () => { - const { result } = renderHook(() => usePaginate({ pageSize: 100 })); - - expect(result.current.currentPage).toBe(1); - expect(typeof result.current.handlePaginateNext).toBe('function'); - expect(typeof result.current.handlePaginatePrevious).toBe('function'); - expect(typeof result.current.handleReset).toBe('function'); - }); + const data: Parameters[0] = { + paginateCallback: jest.fn(), + pageSize: 1, + hasNextToken: false, + items: [ + { + key: 'key1', + id: 'id1', + type: 'FOLDER', + }, + { + key: 'key2', + id: 'id2', + type: 'FOLDER', + }, + ], + }; - it('returns the expected value of `currentPage` on paginate next', () => { - const { result } = renderHook(() => usePaginate({ pageSize: 100 })); + it('returns the expected values on initial call', () => { + const { result } = renderHook(() => usePaginate({ ...data })); - act(() => { - result.current.handlePaginateNext({ - hasNextToken: true, - resultCount: 100, - }); - }); + const { current } = result ?? {}; - expect(result.current.currentPage).toBe(2); + expect(current?.currentPage).toBe(1); + expect(typeof current?.onPaginate).toBe('function'); + expect(typeof current?.handleReset).toBe('function'); + expect(typeof current?.highestPageVisited).toBe('number'); }); - it('returns the expected value of `currentPage` on paginate previous', () => { - const { result } = renderHook(() => usePaginate({ pageSize: 100 })); + it('returns the expected value of `highestPageVisited` on paginate when not on the last page', () => { + const { result } = renderHook(() => usePaginate({ ...data })); - act(() => { - result.current.handlePaginateNext({ - hasNextToken: true, - resultCount: 100, - }); - }); - - expect(result.current.currentPage).toBe(2); + const expectedHighestPage = Math.ceil(data.items.length / data.pageSize); act(() => { - result.current.handlePaginatePrevious(); + result?.current?.onPaginate(expectedHighestPage); }); - expect(result.current.currentPage).toBe(1); + expect(result?.current?.highestPageVisited).toBe(2); }); - it('returns the expected value of `currentPage` on reset', () => { - const { result } = renderHook(() => usePaginate({ pageSize: 100 })); + it('returns the expected value of `currentPage` on paginate', () => { + const { result } = renderHook(() => usePaginate({ ...data })); - act(() => { - result.current.handlePaginateNext({ - hasNextToken: true, - resultCount: 100, - }); - }); - - expect(result.current.currentPage).toBe(2); + expect(result?.current?.currentPage).toBe(1); act(() => { - result.current.handleReset(); + result?.current?.onPaginate(2); }); - expect(result.current.currentPage).toBe(1); + expect(result?.current?.currentPage).toBe(2); }); - it('calls `onPaginateNext` and `onPaginatePrevious` as expected', () => { - const onPaginateNext = jest.fn(); - const onPaginatePrevious = jest.fn(); - - const { result } = renderHook(() => - usePaginate({ onPaginateNext, onPaginatePrevious, pageSize: 100 }) - ); - - act(() => { - result.current.handlePaginateNext({ - hasNextToken: true, - resultCount: 100, - }); - }); - - expect(onPaginateNext).toHaveBeenCalledTimes(1); + it('calls `onPaginate` as expected', () => { + const { result } = renderHook(() => usePaginate({ ...data })); act(() => { - result.current.handlePaginatePrevious(); + result?.current?.onPaginate(2); }); - expect(onPaginatePrevious).toHaveBeenCalledTimes(1); + expect(data.paginateCallback).toHaveBeenCalled(); }); }); diff --git a/packages/react-storage/src/components/StorageBrowser/views/hooks/usePaginate.ts b/packages/react-storage/src/components/StorageBrowser/views/hooks/usePaginate.ts index 608c7746a07..58b8d46662e 100644 --- a/packages/react-storage/src/components/StorageBrowser/views/hooks/usePaginate.ts +++ b/packages/react-storage/src/components/StorageBrowser/views/hooks/usePaginate.ts @@ -1,55 +1,64 @@ import React from 'react'; +import { LocationData, LocationItemData } from '../../actions'; +import { isFunction } from '@aws-amplify/ui'; -interface UsePaginate { +type ListItemType = LocationItemData | LocationData; + +interface UsePaginate { currentPage: number; - handlePaginateNext: (input: { - resultCount: number; - hasNextToken: boolean; - }) => void; - handlePaginatePrevious: (input?: {}) => void; + highestPageVisited: number; + onPaginate: (page: number) => void; handleReset: () => void; - range: [start: number, end: number]; + pageItems: T[]; } -export const usePaginate = ({ - onPaginateNext, - onPaginatePrevious, - pageSize, -}: { - onPaginateNext?: () => void; - onPaginatePrevious?: () => void; +interface UsePaginateProps { + hasNextToken: boolean; + items: T[]; + paginateCallback?: () => void; pageSize: number; -}): UsePaginate => { +} + +export const usePaginate = ({ + hasNextToken, + items, + paginateCallback, + pageSize, +}: UsePaginateProps): UsePaginate => { const [currentPage, setCurrentPage] = React.useState(1); const handleReset = React.useRef(() => { setCurrentPage(1); }).current; - return React.useMemo(() => { + return React.useMemo((): UsePaginate => { + const resultCount = Array.isArray(items) ? items.length : 0; + const highestPageVisited = Math.ceil(resultCount / pageSize); const isFirstPage = currentPage === 1; const start = isFirstPage ? 0 : (currentPage - 1) * pageSize; const end = isFirstPage ? pageSize : currentPage * pageSize; + const pageItems = Array.isArray(items) ? items.slice(start, end) : []; + return { currentPage, - handlePaginateNext: (input) => { - const { hasNextToken, resultCount } = input; - const highestPageVisited = Math.round(resultCount / pageSize); + onPaginate: (page) => { const shouldPaginate = - highestPageVisited === currentPage && hasNextToken; - - if (shouldPaginate && typeof onPaginateNext === 'function') { - onPaginateNext(); + page >= 1 && (page <= highestPageVisited || hasNextToken); + if (shouldPaginate) { + if (isFunction(paginateCallback)) paginateCallback(); + setCurrentPage(page); } - setCurrentPage((prev) => prev + 1); - }, - handlePaginatePrevious: () => { - if (typeof onPaginatePrevious === 'function') onPaginatePrevious(); - - setCurrentPage((prev) => prev - 1); }, handleReset, - range: [start, end], + highestPageVisited, + pageItems, }; - }, [currentPage, handleReset, onPaginateNext, onPaginatePrevious, pageSize]); + }, [ + currentPage, + handleReset, + hasNextToken, + items, + paginateCallback, + pageSize, + ]); };