From 5639f8d3f91ec547ff76a379d8faa233141456e9 Mon Sep 17 00:00:00 2001 From: Michael Sober <> Date: Wed, 15 Apr 2026 13:52:00 +0000 Subject: [PATCH 1/8] feat(storage-browser): implement cross-page search with pagination, caching, and progress - Fix hasNextPage to account for local pagination of search results - Add search result caching to avoid re-fetching from S3 on query changes - Add configurable search limit (default 10k, customizable via search.limit) - Add onProgress callback for search batch progress reporting - Expose searchProgress state in LocationDetailView for UI feedback - Add hasNextLocalPage to usePaginate hook - Add comprehensive unit tests for all new functionality - Add E2E feature file for cross-page search scenarios --- .../storage-browser/cross-page-search.feature | 53 ++++ .../createEnhancedListHandler.spec.ts | 264 ++++++++++++++++++ .../useAction/createEnhancedListHandler.ts | 70 +++-- .../__tests__/useLocationDetailView.spec.tsx | 123 +++++++- .../views/LocationDetailView/types.ts | 1 + .../useLocationDetailView.ts | 16 +- .../views/hooks/__tests__/usePaginate.spec.ts | 45 +++ .../StorageBrowser/views/hooks/usePaginate.ts | 2 + 8 files changed, 555 insertions(+), 19 deletions(-) create mode 100644 packages/e2e/features/ui/components/storage/storage-browser/cross-page-search.feature diff --git a/packages/e2e/features/ui/components/storage/storage-browser/cross-page-search.feature b/packages/e2e/features/ui/components/storage/storage-browser/cross-page-search.feature new file mode 100644 index 00000000000..e2d123064c6 --- /dev/null +++ b/packages/e2e/features/ui/components/storage/storage-browser/cross-page-search.feature @@ -0,0 +1,53 @@ +Feature: StorageBrowser cross-page search + + Tests that search works across multiple pages of results, + including pagination of search results and subfolder search. + + Background: + Given I'm running the example "ui/components/storage/storage-browser/default-auth" + And I type my "email" with status "CONFIRMED" + And I type my password + And I click the "Sign in" button + + @react + Scenario: Search returns results from across pages + When I click the first button containing "public" + When I see input with placeholder "Search current folder" and type "DO_NOT" + Then I click the "Search" button + Then I see the button containing "DO_NOT_DELETE" + + @react + Scenario: Clearing search returns to normal browsing + When I click the first button containing "public" + Then I see the button containing "DoNotDeleteThisFolder_CanDeleteAllChildren" + When I see input with placeholder "Search current folder" and type "DO_NOT" + Then I click the "Search" button + Then I see the button containing "DO_NOT_DELETE" + Then I do not see the button containing "DoNotDeleteThisFolder_CanDeleteAllChildren" + When I click the button containing "Clear search" + Then I see the button containing "DoNotDeleteThisFolder_CanDeleteAllChildren" + + @react + Scenario: Search with no results shows empty state + When I click the first button containing "public" + When I see input with placeholder "Search current folder" and type "zzz_nonexistent_item_xyz" + Then I click the "Search" button + Then I see "No files" + When I click the button containing "Clear search" + Then I do not see "No files" + + @react + Scenario: Search with subfolders enabled finds items in nested folders + When I click the first button containing "public" + When I click the "Include Subfolders" checkbox + When I see input with placeholder "Search current folder" and type "DELETE" + Then I click the "Search" button + Then I see "DO_NOT_DELETE/DONT_DELETE_SUB" + + @react + Scenario: Search without subfolders does not show nested items + When I click the first button containing "public" + When I see input with placeholder "Search current folder" and type "DO_NOT" + Then I click the "Search" button + Then I see the button containing "DO_NOT_DELETE" + Then I do not see the button containing "DO_NOT_DELETE/DONT_DELETE_SUB" diff --git a/packages/react-storage/src/components/StorageBrowser/useAction/__tests__/createEnhancedListHandler.spec.ts b/packages/react-storage/src/components/StorageBrowser/useAction/__tests__/createEnhancedListHandler.spec.ts index 8a014d89ab1..fb8144a3dc8 100644 --- a/packages/react-storage/src/components/StorageBrowser/useAction/__tests__/createEnhancedListHandler.spec.ts +++ b/packages/react-storage/src/components/StorageBrowser/useAction/__tests__/createEnhancedListHandler.spec.ts @@ -235,4 +235,268 @@ describe('createEnhancedListHandler', () => { expect(result.items).toEqual([{ id: 1 }, { id: 2 }, { id: 3 }]); expect(result.nextToken).toBe('next'); }); + + describe('search result caching', () => { + it('should reuse cached items on subsequent search with same prefix', async () => { + mockAction.mockResolvedValueOnce({ + items: [ + { name: 'a_prefix/apple' }, + { name: 'a_prefix/banana' }, + { name: 'a_prefix/avocado' }, + ], + nextToken: null, + }); + + const handler = createEnhancedListHandler(mockAction as Handler); + const prevState = { items: [], nextToken: undefined }; + + const result1 = await handler(prevState, { + prefix: 'a_prefix', + options: { search: { query: 'pl', filterBy: 'name' as const } }, + }); + + expect(mockAction).toHaveBeenCalledTimes(1); + expect(result1.items).toEqual([{ name: 'a_prefix/apple' }]); + + const result2 = await handler(prevState, { + prefix: 'a_prefix', + options: { search: { query: 'ban', filterBy: 'name' as const } }, + }); + + expect(mockAction).toHaveBeenCalledTimes(1); + expect(result2.items).toEqual([{ name: 'a_prefix/banana' }]); + }); + + it('should invalidate cache when refresh is true', async () => { + mockAction + .mockResolvedValueOnce({ + items: [{ name: 'a_prefix/apple' }], + nextToken: null, + }) + .mockResolvedValueOnce({ + items: [{ id: 1 }, { id: 2 }], + nextToken: 'next', + }) + .mockResolvedValueOnce({ + items: [{ name: 'a_prefix/apple' }, { name: 'a_prefix/banana' }], + nextToken: null, + }); + + const handler = createEnhancedListHandler(mockAction as Handler); + const prevState = { items: [], nextToken: undefined }; + + await handler(prevState, { + prefix: 'a_prefix', + options: { search: { query: 'a', filterBy: 'name' as const } }, + }); + expect(mockAction).toHaveBeenCalledTimes(1); + + await handler(prevState, { + prefix: 'a_prefix', + options: { refresh: true }, + }); + + const result = await handler(prevState, { + prefix: 'a_prefix', + options: { search: { query: 'a', filterBy: 'name' as const } }, + }); + + expect(mockAction).toHaveBeenCalledTimes(3); + expect(result.items).toEqual([ + { name: 'a_prefix/apple' }, + { name: 'a_prefix/banana' }, + ]); + }); + + it('should invalidate cache when reset is true', async () => { + mockAction + .mockResolvedValueOnce({ + items: [{ name: 'a_prefix/apple' }], + nextToken: null, + }) + .mockResolvedValueOnce({ + items: [{ name: 'a_prefix/apple' }, { name: 'a_prefix/avocado' }], + nextToken: null, + }); + + const handler = createEnhancedListHandler(mockAction as Handler); + const prevState = { items: [], nextToken: undefined }; + + await handler(prevState, { + prefix: 'a_prefix', + options: { search: { query: 'a', filterBy: 'name' as const } }, + }); + expect(mockAction).toHaveBeenCalledTimes(1); + + await handler(prevState, { + prefix: 'a_prefix', + options: { reset: true }, + }); + + const result = await handler(prevState, { + prefix: 'a_prefix', + options: { search: { query: 'a', filterBy: 'name' as const } }, + }); + + expect(mockAction).toHaveBeenCalledTimes(2); + expect(result.items).toEqual([ + { name: 'a_prefix/apple' }, + { name: 'a_prefix/avocado' }, + ]); + }); + + it('should invalidate cache when prefix changes', async () => { + mockAction + .mockResolvedValueOnce({ + items: [{ name: 'a/apple' }], + nextToken: null, + }) + .mockResolvedValueOnce({ + items: [{ name: 'b/banana' }], + nextToken: null, + }); + + const handler = createEnhancedListHandler(mockAction as Handler); + const prevState = { items: [], nextToken: undefined }; + + await handler(prevState, { + prefix: 'a/', + options: { search: { query: 'a', filterBy: 'name' as const } }, + }); + expect(mockAction).toHaveBeenCalledTimes(1); + + const result = await handler(prevState, { + prefix: 'b/', + options: { search: { query: 'ban', filterBy: 'name' as const } }, + }); + + expect(mockAction).toHaveBeenCalledTimes(2); + expect(result.items).toEqual([{ name: 'b/banana' }]); + }); + + it('should invalidate cache when refresh is passed in search options', async () => { + mockAction + .mockResolvedValueOnce({ + items: [{ name: 'a_prefix/apple' }], + nextToken: null, + }) + .mockResolvedValueOnce({ + items: [{ name: 'a_prefix/apple' }, { name: 'a_prefix/avocado' }], + nextToken: null, + }); + + const handler = createEnhancedListHandler(mockAction as Handler); + const prevState = { items: [], nextToken: undefined }; + + await handler(prevState, { + prefix: 'a_prefix', + options: { search: { query: 'a', filterBy: 'name' as const } }, + }); + expect(mockAction).toHaveBeenCalledTimes(1); + + const result = await handler(prevState, { + prefix: 'a_prefix', + options: { + refresh: true, + search: { query: 'a', filterBy: 'name' as const }, + }, + }); + + expect(mockAction).toHaveBeenCalledTimes(2); + expect(result.items).toEqual([ + { name: 'a_prefix/apple' }, + { name: 'a_prefix/avocado' }, + ]); + }); + }); + + describe('configurable search limit', () => { + it('should respect custom search limit', async () => { + const batchSize = 300; + const customLimit = 500; + const batch = Array.from({ length: batchSize }, (_, i) => ({ + name: `a_prefix/item${i}`, + })); + + mockAction + .mockResolvedValueOnce({ items: batch, nextToken: 'token1' }) + .mockResolvedValueOnce({ items: batch, nextToken: 'token2' }) + .mockResolvedValueOnce({ items: batch, nextToken: 'token3' }); + + const handler = createEnhancedListHandler(mockAction as Handler); + const prevState = { items: [], nextToken: undefined }; + + const result = await handler(prevState, { + prefix: 'a_prefix', + options: { + search: { + query: 'item', + filterBy: 'name' as const, + limit: customLimit, + }, + }, + }); + + expect(mockAction).toHaveBeenCalledTimes(2); + expect(result.items.length).toBeLessThanOrEqual(batchSize * 2); + expect(result.hasExhaustedSearch).toBe(true); + }); + }); + + describe('progress callback', () => { + it('should call onProgress after each batch', async () => { + mockAction + .mockResolvedValueOnce({ + items: [{ name: 'a_prefix/a1' }, { name: 'a_prefix/a2' }], + nextToken: 'token1', + }) + .mockResolvedValueOnce({ + items: [{ name: 'a_prefix/a3' }], + nextToken: 'token2', + }) + .mockResolvedValueOnce({ + items: [{ name: 'a_prefix/a4' }], + nextToken: null, + }); + + const onProgress = jest.fn(); + const handler = createEnhancedListHandler(mockAction as Handler); + const prevState = { items: [], nextToken: undefined }; + + await handler(prevState, { + prefix: 'a_prefix', + options: { + search: { + query: 'a', + filterBy: 'name' as const, + onProgress, + }, + }, + }); + + expect(onProgress).toHaveBeenCalledTimes(3); + expect(onProgress).toHaveBeenNthCalledWith(1, { fetchedCount: 2 }); + expect(onProgress).toHaveBeenNthCalledWith(2, { fetchedCount: 3 }); + expect(onProgress).toHaveBeenNthCalledWith(3, { fetchedCount: 4 }); + }); + + it('should not fail if onProgress is not provided', async () => { + mockAction.mockResolvedValueOnce({ + items: [{ name: 'a_prefix/apple' }], + nextToken: null, + }); + + const handler = createEnhancedListHandler(mockAction as Handler); + const prevState = { items: [], nextToken: undefined }; + + const result = await handler(prevState, { + prefix: 'a_prefix', + options: { + search: { query: 'a', filterBy: 'name' as const }, + }, + }); + + expect(result.items).toEqual([{ name: 'a_prefix/apple' }]); + }); + }); }); diff --git a/packages/react-storage/src/components/StorageBrowser/useAction/createEnhancedListHandler.ts b/packages/react-storage/src/components/StorageBrowser/useAction/createEnhancedListHandler.ts index 3517d183351..129ff79b0bf 100644 --- a/packages/react-storage/src/components/StorageBrowser/useAction/createEnhancedListHandler.ts +++ b/packages/react-storage/src/components/StorageBrowser/useAction/createEnhancedListHandler.ts @@ -16,7 +16,10 @@ export const SEARCH_PAGE_SIZE = 1000; export type EnhancedListHandlerOptions = TOptions & { refresh?: boolean; reset?: boolean; - search?: SearchOptions; + search?: SearchOptions & { + limit?: number; + onProgress?: (progress: { fetchedCount: number }) => void; + }; }; export type EnhancedListHandlerInput< @@ -45,6 +48,12 @@ export const createEnhancedListHandler = < >( handler: ListHandler> ): EnhancedListHandler => { + let searchCache: { + prefix: string; + items: TItem[]; + hasExhaustedSearch: boolean; + } | null = null; + return async function listActionHandler(prevState, { options, ...input }) { const { nextToken: _nextToken, @@ -55,32 +64,61 @@ export const createEnhancedListHandler = < } = options ?? {}; if (reset) { + searchCache = null; return { items: [], nextToken: undefined }; } // collect and filter results on `search` if (search) { - const items = []; - let nextToken = undefined; - do { - const output = await handler({ - ...input, - options: { ...rest, pageSize: SEARCH_PAGE_SIZE, nextToken }, - } as TInput); - - items.push(...output.items); - // eslint-disable-next-line prefer-destructuring - nextToken = output.nextToken; - } while (nextToken && items.length < SEARCH_LIMIT); + const { limit: searchLimit, onProgress, ...searchOptions } = search; + let allItems: TItem[]; + let hasExhaustedSearch: boolean; + + if (searchCache && searchCache.prefix === input.prefix && !refresh) { + allItems = searchCache.items; + ({ hasExhaustedSearch } = searchCache); + } else { + const items: TItem[] = []; + let nextToken = undefined; + const limit = searchLimit ?? SEARCH_LIMIT; + do { + const output = await handler({ + ...input, + options: { ...rest, pageSize: SEARCH_PAGE_SIZE, nextToken }, + } as TInput); + + items.push(...output.items); + // eslint-disable-next-line prefer-destructuring + nextToken = output.nextToken; + + onProgress?.({ fetchedCount: items.length }); + } while (nextToken && items.length < limit); + + allItems = items; + hasExhaustedSearch = !!nextToken; + + searchCache = { + prefix: input.prefix, + items: allItems, + hasExhaustedSearch, + }; + } return { - items: searchItems({ items, prefix: input.prefix, options: search }), - // search limit reached but we still have a next token - hasExhaustedSearch: !!nextToken, + items: searchItems({ + items: allItems, + prefix: input.prefix, + options: searchOptions, + }), + hasExhaustedSearch, nextToken: undefined, }; } + if (refresh) { + searchCache = null; + } + // ignore provided `nextToken` on `refresh` const nextToken = refresh ? undefined : _nextToken; const output = await handler({ diff --git a/packages/react-storage/src/components/StorageBrowser/views/LocationDetailView/__tests__/useLocationDetailView.spec.tsx b/packages/react-storage/src/components/StorageBrowser/views/LocationDetailView/__tests__/useLocationDetailView.spec.tsx index 5ba0e88fbb4..91ea94a8325 100644 --- a/packages/react-storage/src/components/StorageBrowser/views/LocationDetailView/__tests__/useLocationDetailView.spec.tsx +++ b/packages/react-storage/src/components/StorageBrowser/views/LocationDetailView/__tests__/useLocationDetailView.spec.tsx @@ -573,7 +573,12 @@ describe('useLocationDetailView', () => { options: { ...DEFAULT_LIST_OPTIONS, delimiter: '/', - search: { filterBy: 'key', query: 'moo' }, + search: { + filterBy: 'key', + query: 'moo', + groupBy: undefined, + onProgress: expect.any(Function), + }, }, prefix: 'item-b-key/', }); @@ -621,7 +626,12 @@ describe('useLocationDetailView', () => { options: { ...DEFAULT_LIST_OPTIONS, delimiter: undefined, - search: { filterBy: 'key', query: 'moo', groupBy: '/' }, + search: { + filterBy: 'key', + query: 'moo', + groupBy: '/', + onProgress: expect.any(Function), + }, }, prefix: 'item-b-key/', }); @@ -869,4 +879,113 @@ describe('useLocationDetailView', () => { }); }); }); + + describe('search pagination and progress', () => { + it('should set hasNextPage true when search results span multiple local pages', () => { + const manyItems: LocationItemData[] = Array.from( + { length: 200 }, + (_, i) => ({ + key: `some-prefix/file-${i}.txt`, + id: `id-${i}`, + type: 'FILE' as const, + lastModified: new Date(), + size: 1024, + }) + ); + + mockUseList.mockReturnValue([ + { + value: { items: manyItems, nextToken: undefined }, + message: '', + hasError: false, + isLoading: false, + }, + mockHandleList, + ]); + + const { result } = renderHook(() => + useLocationDetailView({ pageSize: EXPECTED_PAGE_SIZE }) + ); + + expect(result.current.hasNextPage).toBe(true); + }); + + it('should paginate through search results locally', () => { + const searchItems: LocationItemData[] = Array.from( + { length: 10 }, + (_, i) => ({ + key: `some-prefix/file-${i}.txt`, + id: `id-${i}`, + type: 'FILE' as const, + lastModified: new Date(), + size: 1024, + }) + ); + + mockUseList.mockReturnValue([ + { + value: { items: searchItems, nextToken: undefined }, + message: '', + hasError: false, + isLoading: false, + }, + mockHandleList, + ]); + + const { result } = renderHook(() => + useLocationDetailView({ pageSize: EXPECTED_PAGE_SIZE }) + ); + + expect(result.current.pageItems).toEqual( + searchItems.slice(0, EXPECTED_PAGE_SIZE) + ); + + act(() => { + result.current.onPaginate(2); + }); + + expect(result.current.page).toBe(2); + expect(result.current.pageItems).toEqual( + searchItems.slice(EXPECTED_PAGE_SIZE, EXPECTED_PAGE_SIZE * 2) + ); + }); + + it('should expose searchProgress as null initially', () => { + const { result } = renderHook(() => useLocationDetailView()); + + expect(result.current.searchProgress).toBeNull(); + }); + + it('should pass onProgress callback when searching', () => { + mockUseStore.mockReturnValue([mockStoreState, mockStoreDispatch]); + const mockDataState = { + value: { items: [], nextToken: undefined }, + message: '', + hasError: false, + isLoading: false, + }; + const localMockHandleList = jest.fn(); + mockUseList.mockReturnValue([mockDataState, localMockHandleList]); + + const { result } = renderHook(() => useLocationDetailView()); + + act(() => { + result.current.onSearchQueryChange('test'); + }); + + act(() => { + result.current.onSearch(); + }); + + expect(localMockHandleList).toHaveBeenCalledWith( + expect.objectContaining({ + options: expect.objectContaining({ + search: expect.objectContaining({ + onProgress: expect.any(Function), + }), + }), + }) + ); + }); + }); }); diff --git a/packages/react-storage/src/components/StorageBrowser/views/LocationDetailView/types.ts b/packages/react-storage/src/components/StorageBrowser/views/LocationDetailView/types.ts index af82865d0e0..a1910bdb81a 100644 --- a/packages/react-storage/src/components/StorageBrowser/views/LocationDetailView/types.ts +++ b/packages/react-storage/src/components/StorageBrowser/views/LocationDetailView/types.ts @@ -55,6 +55,7 @@ export interface LocationDetailViewState { page: number; pageItems: LocationItemData[]; searchQuery: string; + searchProgress: { fetchedCount: number } | null; filePreviewState: FilePreviewState; filePreviewEnabled: boolean; } 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 afe14acb61e..e5c960eecc3 100644 --- a/packages/react-storage/src/components/StorageBrowser/views/LocationDetailView/useLocationDetailView.ts +++ b/packages/react-storage/src/components/StorageBrowser/views/LocationDetailView/useLocationDetailView.ts @@ -74,6 +74,9 @@ export const useLocationDetailView = ( const [locationItems, locationItemsDispatch] = useLocationItems(); const fileItemsDispatch = useFileItems()[1]; const [activeFile, setActiveFile] = React.useState(); + const [searchProgress, setSearchProgress] = React.useState<{ + fetchedCount: number; + } | null>(null); const { current, key } = location; const { permissions, prefix } = current ?? {}; @@ -101,6 +104,7 @@ export const useLocationDetailView = ( currentPage, handlePaginate, handleReset, + hasNextLocalPage, highestPageVisited, pageItems, } = usePaginate({ @@ -118,11 +122,14 @@ export const useLocationDetailView = ( query, filterBy: 'key' as const, groupBy: includeSubfolders ? listOptions.delimiter : undefined, + onProgress: (progress: { fetchedCount: number }) => + setSearchProgress(progress), }, }; handleReset(); handleList({ prefix: key, options: searchOptions }); + setSearchProgress({ fetchedCount: 0 }); locationItemsDispatch({ type: 'RESET_LOCATION_ITEMS' }); }; @@ -225,6 +232,12 @@ export const useLocationDetailView = ( setActiveFile(undefined); }, [handleList, handleReset, listOptions, hasInvalidPrefix, key]); + React.useEffect(() => { + if (!isLoading && searchProgress) { + setSearchProgress(null); + } + }, [isLoading, searchProgress]); + const { actionConfigs } = useActionConfigs(); const actionItems = React.useMemo(() => { @@ -263,7 +276,7 @@ export const useLocationDetailView = ( fileDataItems, hasError, hasDownloadError: task?.status === 'FAILED', - hasNextPage: !!nextToken, + hasNextPage: !!nextToken || hasNextLocalPage, highestPageVisited, message, downloadErrorMessage: getDownloadErrorMessageFromFailedDownloadTask(task), @@ -274,6 +287,7 @@ export const useLocationDetailView = ( setActiveFile(undefined); }, searchQuery, + searchProgress, filePreviewState, filePreviewEnabled: !optout, hasExhaustedSearch, 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 c8df97cfb59..83015b38561 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 @@ -116,4 +116,49 @@ describe('usePaginate', () => { expect(result.current.currentPage).toBe(1); expect(result.current.pageItems).toEqual(['item1', 'item2']); }); + + describe('hasNextLocalPage', () => { + it('should return hasNextLocalPage=true when more pages exist', () => { + const items = Array.from({ length: 20 }, (_, i) => `item${i}`); + const { result } = renderHook(() => usePaginate({ items, pageSize: 10 })); + + expect(result.current.currentPage).toBe(1); + expect(result.current.hasNextLocalPage).toBe(true); + }); + + it('should return hasNextLocalPage=false on last page', () => { + const items = Array.from({ length: 20 }, (_, i) => `item${i}`); + const { result } = renderHook(() => usePaginate({ items, pageSize: 10 })); + + act(() => { + result.current.handlePaginate(2); + }); + + expect(result.current.currentPage).toBe(2); + expect(result.current.hasNextLocalPage).toBe(false); + }); + + it('should return hasNextLocalPage=false with empty items', () => { + const { result } = renderHook(() => + usePaginate({ items: [], pageSize: 10 }) + ); + + expect(result.current.hasNextLocalPage).toBe(false); + }); + + it('should return hasNextLocalPage=false when items fit in one page', () => { + const items = ['item1', 'item2', 'item3']; + const { result } = renderHook(() => usePaginate({ items, pageSize: 10 })); + + expect(result.current.hasNextLocalPage).toBe(false); + }); + + it('should return hasNextLocalPage=false when items is undefined', () => { + const { result } = renderHook(() => + usePaginate({ items: undefined, pageSize: 10 }) + ); + + expect(result.current.hasNextLocalPage).toBe(false); + }); + }); }); 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 08df9404a4f..7f197d5d5ca 100644 --- a/packages/react-storage/src/components/StorageBrowser/views/hooks/usePaginate.ts +++ b/packages/react-storage/src/components/StorageBrowser/views/hooks/usePaginate.ts @@ -7,6 +7,7 @@ interface UsePaginateState { highestPageVisited: number; handlePaginate: (page: number) => void; handleReset: () => void; + hasNextLocalPage: boolean; pageItems: T[]; } @@ -68,6 +69,7 @@ export const usePaginate = ({ setCurrentPage(page); }, handleReset, + hasNextLocalPage: adjustedCurrentPage < totalPages, highestPageVisited, pageItems, }; From cb2e886bc452a35bf64eceeac65b945c790c8265 Mon Sep 17 00:00:00 2001 From: Michael Sober <> Date: Thu, 16 Apr 2026 08:52:38 +0000 Subject: [PATCH 2/8] feat(storage-browser): add cross-page sorting with search integration --- .../controls/hooks/useDataTable.ts | 49 ++++- .../StorageBrowser/controls/types.ts | 9 + .../LocationDetailViewProvider.tsx | 6 + .../__tests__/useLocationDetailView.spec.tsx | 163 ++++++++++++++++ .../views/LocationDetailView/types.ts | 4 + .../useLocationDetailView.ts | 10 +- .../views/hooks/__tests__/sortItems.spec.ts | 165 ++++++++++++++++ .../views/hooks/__tests__/useSort.spec.ts | 176 ++++++++++++++++++ .../StorageBrowser/views/hooks/sortItems.ts | 85 +++++++++ .../StorageBrowser/views/hooks/useSort.ts | 77 ++++++++ 10 files changed, 735 insertions(+), 9 deletions(-) create mode 100644 packages/react-storage/src/components/StorageBrowser/views/hooks/__tests__/sortItems.spec.ts create mode 100644 packages/react-storage/src/components/StorageBrowser/views/hooks/__tests__/useSort.spec.ts create mode 100644 packages/react-storage/src/components/StorageBrowser/views/hooks/sortItems.ts create mode 100644 packages/react-storage/src/components/StorageBrowser/views/hooks/useSort.ts diff --git a/packages/react-storage/src/components/StorageBrowser/controls/hooks/useDataTable.ts b/packages/react-storage/src/components/StorageBrowser/controls/hooks/useDataTable.ts index f27cdc4206a..71ac0456e12 100644 --- a/packages/react-storage/src/components/StorageBrowser/controls/hooks/useDataTable.ts +++ b/packages/react-storage/src/components/StorageBrowser/controls/hooks/useDataTable.ts @@ -9,6 +9,7 @@ import type { DataTableNumberDataCell, } from '../../components'; import { useControlsContext } from '../context'; +import type { HeaderKeys } from '../../views/LocationDetailView/getLocationDetailViewTableData/types'; import { compareButtonData } from './compareFunctions/compareButtonData'; import { compareDateData } from './compareFunctions/compareDateData'; import { compareNumberData } from './compareFunctions/compareNumberData'; @@ -34,31 +35,53 @@ const GROUP_ORDER: DataTableDataCell['type'][] = [ const UNSORTABLE_GROUPS: DataTableDataCell['type'][] = ['checkbox']; export const useDataTable = (): DataTableProps => { - const { data } = useControlsContext(); - const { isLoading, tableData } = data; + const { data, onSort: onCrossPageSort } = useControlsContext(); + const { isLoading, tableData, sortState: crossPageSortState } = data; + + const hasCrossPageSort = !!onCrossPageSort; const defaultSortIndex = React.useMemo( () => tableData?.headers?.findIndex(({ type }) => type === 'sort') ?? -1, [tableData] ); - const [sortState, setSortState] = React.useState({ + const [localSortState, setLocalSortState] = React.useState({ index: defaultSortIndex, direction: 'ascending', }); + const sortState = localSortState; + const mappedHeaders = React.useMemo( () => tableData?.headers.map((header, index) => { const { type } = header; switch (type) { case 'sort': { + const headerKey = (header as { key?: HeaderKeys }).key; + + if (hasCrossPageSort && headerKey) { + const isActive = crossPageSortState?.field === headerKey; + return { + ...header, + content: { + ...header.content, + onSort: () => { + onCrossPageSort(headerKey); + }, + sortDirection: isActive + ? crossPageSortState.direction + : undefined, + }, + }; + } + return { ...header, content: { ...header.content, onSort: () => { - setSortState({ + setLocalSortState({ index, direction: sortState.index === index @@ -80,15 +103,25 @@ export const useDataTable = (): DataTableProps => { } } }), - [sortState, tableData] + [ + crossPageSortState, + hasCrossPageSort, + onCrossPageSort, + sortState, + tableData, + ] ); const sortedRows = React.useMemo(() => { - // Early return if there is no table data if (!tableData) { return; } - // Return rows as is if there are no sortable columns + + // When cross-page sort is active, rows are already sorted upstream + if (hasCrossPageSort) { + return tableData.rows; + } + if (sortState.index < 0) { return tableData.rows; } @@ -150,7 +183,7 @@ export const useDataTable = (): DataTableProps => { }); }) .flat(); - }, [sortState, tableData]); + }, [hasCrossPageSort, sortState, tableData]); return { headers: mappedHeaders ?? [], diff --git a/packages/react-storage/src/components/StorageBrowser/controls/types.ts b/packages/react-storage/src/components/StorageBrowser/controls/types.ts index 3bb41ad3c63..39af79319d1 100644 --- a/packages/react-storage/src/components/StorageBrowser/controls/types.ts +++ b/packages/react-storage/src/components/StorageBrowser/controls/types.ts @@ -5,11 +5,13 @@ import type { DataTableProps, DataTableSortHeader, MessageProps, + SortDirection, } from '../components'; import type { ActionConfirmationModalProps } from '../components/composables/ActionConfirmationModal'; import type { LocationState } from '../store'; import type { StatusCounts } from '../tasks'; import type { FilePreviewState } from '../views/hooks/useFilePreview'; +import type { HeaderKeys } from '../views/LocationDetailView/getLocationDetailViewTableData/types'; export interface Controls { props: React.ComponentProps; @@ -39,6 +41,11 @@ interface PaginationData { page: number; } +export interface SortState { + field: HeaderKeys; + direction: SortDirection; +} + export interface ControlsContext { data: { actions?: ActionListItem[]; @@ -90,6 +97,7 @@ export interface ControlsContext { statusDisplayFailedLabel?: string; statusDisplayQueuedLabel?: string; tableData?: TableData; + sortState?: SortState; title?: string; }; onActionCancel?: () => void; @@ -108,6 +116,7 @@ export interface ControlsContext { onSelectActiveFile?: (file?: FileData | 'prev' | 'next') => void; onSearchClear?: () => void; onSearchQueryChange?: (value: string) => void; + onSort?: (headerKey: HeaderKeys) => void; onOpenFilePreview?: (f: FileData) => void; onCloseFilePreview?: () => void; onRetryFilePreview?: () => void; diff --git a/packages/react-storage/src/components/StorageBrowser/views/LocationDetailView/LocationDetailViewProvider.tsx b/packages/react-storage/src/components/StorageBrowser/views/LocationDetailView/LocationDetailViewProvider.tsx index 4f95139a7dd..358152eed35 100644 --- a/packages/react-storage/src/components/StorageBrowser/views/LocationDetailView/LocationDetailViewProvider.tsx +++ b/packages/react-storage/src/components/StorageBrowser/views/LocationDetailView/LocationDetailViewProvider.tsx @@ -64,6 +64,8 @@ export function LocationDetailViewProvider({ filePreviewState, filePreviewEnabled, onRetryFilePreview, + onSort, + sortConfig, dataItems, } = props; @@ -137,6 +139,9 @@ export function LocationDetailViewProvider({ dataItems, }), title: getTitle(location), + sortState: sortConfig + ? { field: sortConfig.field, direction: sortConfig.direction } + : undefined, message: messageControlContent, }} onActionSelect={onActionSelect} @@ -149,6 +154,7 @@ export function LocationDetailViewProvider({ onSelectActiveFile={onSelectActiveFile} onSearchQueryChange={onSearchQueryChange} onSearchClear={onSearchClear} + onSort={onSort} onToggleSearchSubfolders={onToggleSearchSubfolders} onRetryFilePreview={onRetryFilePreview} > diff --git a/packages/react-storage/src/components/StorageBrowser/views/LocationDetailView/__tests__/useLocationDetailView.spec.tsx b/packages/react-storage/src/components/StorageBrowser/views/LocationDetailView/__tests__/useLocationDetailView.spec.tsx index 91ea94a8325..09b60048d64 100644 --- a/packages/react-storage/src/components/StorageBrowser/views/LocationDetailView/__tests__/useLocationDetailView.spec.tsx +++ b/packages/react-storage/src/components/StorageBrowser/views/LocationDetailView/__tests__/useLocationDetailView.spec.tsx @@ -988,4 +988,167 @@ describe('useLocationDetailView', () => { ); }); }); + + describe('cross-page sorting', () => { + const sortableItems: LocationItemData[] = [ + folderDataOne, + { + ...fileDataOne, + key: 'some-prefix/charlie.jpg', + size: 300, + lastModified: new Date('2024-03-15'), + }, + { + ...fileDataTwo, + key: 'some-prefix/alpha.png', + size: 1000, + lastModified: new Date('2024-01-01'), + }, + { + ...fileDataThree, + key: 'some-prefix/beta.doc', + size: 500, + lastModified: new Date('2024-06-20'), + }, + ]; + + beforeEach(() => { + mockUseList.mockReturnValue([ + { + value: { items: sortableItems, nextToken: undefined }, + message: '', + hasError: false, + isLoading: false, + }, + mockHandleList, + ]); + }); + + it('should return onSort and sortConfig', () => { + const { result } = renderHook(() => useLocationDetailView()); + + expect(result.current.onSort).toEqual(expect.any(Function)); + expect(result.current.sortConfig).toBeUndefined(); + }); + + it('should sort items by name across all pages', () => { + const { result } = renderHook(() => + useLocationDetailView({ pageSize: 2 }) + ); + + expect(result.current.pageItems).toHaveLength(2); + + act(() => { + result.current.onSort('name'); + }); + + expect(result.current.sortConfig).toEqual({ + field: 'name', + direction: 'ascending', + }); + + // folder first (ascending), then files alphabetically + const allItems: string[] = []; + allItems.push(...result.current.pageItems.map((i) => i.id)); + + act(() => { + result.current.onPaginate(2); + }); + allItems.push(...result.current.pageItems.map((i) => i.id)); + + // folder 1 -> file alpha -> file beta -> file charlie + expect(allItems[0]).toBe('1'); // folderDataOne + }); + + it('should toggle sort direction', () => { + const { result } = renderHook(() => useLocationDetailView()); + + act(() => { + result.current.onSort('name'); + }); + expect(result.current.sortConfig?.direction).toBe('ascending'); + + act(() => { + result.current.onSort('name'); + }); + expect(result.current.sortConfig?.direction).toBe('descending'); + }); + + it('should sort by size', () => { + const { result } = renderHook(() => useLocationDetailView()); + + act(() => { + result.current.onSort('size'); + }); + + expect(result.current.sortConfig).toEqual({ + field: 'size', + direction: 'ascending', + }); + + // folders first, then files sorted by size ascending + const fileItems = result.current.pageItems.filter( + (i) => i.type === 'FILE' + ); + const sizes = fileItems.map((i) => (i as FileData).size); + // 300, 500, 1000 ascending + expect(sizes).toEqual([300, 500, 1000]); + }); + + it('should reset sort on refresh', () => { + const { result } = renderHook(() => useLocationDetailView()); + + act(() => { + result.current.onSort('name'); + }); + expect(result.current.sortConfig).toBeDefined(); + + act(() => { + result.current.onRefresh(); + }); + expect(result.current.sortConfig).toBeUndefined(); + }); + + it('should reset sort on navigation', () => { + const { result } = renderHook(() => useLocationDetailView()); + + act(() => { + result.current.onSort('name'); + }); + expect(result.current.sortConfig).toBeDefined(); + + act(() => { + result.current.onNavigate(testLocation.current!); + }); + expect(result.current.sortConfig).toBeUndefined(); + }); + + it('should work with search results', () => { + mockUseList.mockReturnValue([ + { + value: { + items: sortableItems.filter((i) => i.type === 'FILE'), + nextToken: undefined, + }, + message: '', + hasError: false, + isLoading: false, + }, + mockHandleList, + ]); + + const { result } = renderHook(() => useLocationDetailView()); + + act(() => { + result.current.onSort('size'); + }); + + const fileItems = result.current.pageItems.filter( + (i) => i.type === 'FILE' + ); + const sizes = fileItems.map((i) => (i as FileData).size); + // 300, 500, 1000 ascending + expect(sizes).toEqual([300, 500, 1000]); + }); + }); }); diff --git a/packages/react-storage/src/components/StorageBrowser/views/LocationDetailView/types.ts b/packages/react-storage/src/components/StorageBrowser/views/LocationDetailView/types.ts index a1910bdb81a..57da53c7a46 100644 --- a/packages/react-storage/src/components/StorageBrowser/views/LocationDetailView/types.ts +++ b/packages/react-storage/src/components/StorageBrowser/views/LocationDetailView/types.ts @@ -9,6 +9,8 @@ import type { ActionListItem } from '../../components/composables/ActionsList'; import type { FilePreviewProps } from '../../components/composables/FilePreview'; import type { LocationState } from '../../store'; import type { FilePreviewState } from '../hooks/useFilePreview'; +import type { SortConfig } from '../hooks/sortItems'; +import type { HeaderKeys } from './getLocationDetailViewTableData/types'; import type { ListViewProps } from '../types'; @@ -50,11 +52,13 @@ export interface LocationDetailViewState { onRetryFilePreview: () => void; onSelect: (isSelected: boolean, item: LocationItemData) => void; onSelectActiveFile: (file?: FileData | 'prev' | 'next') => void; + onSort: (headerKey: HeaderKeys) => void; onToggleSearchSubfolders: () => void; onToggleSelectAll: () => void; page: number; pageItems: LocationItemData[]; searchQuery: string; + sortConfig: SortConfig | undefined; searchProgress: { fetchedCount: number } | null; filePreviewState: FilePreviewState; filePreviewEnabled: boolean; 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 e5c960eecc3..77fc78f603e 100644 --- a/packages/react-storage/src/components/StorageBrowser/views/LocationDetailView/useLocationDetailView.ts +++ b/packages/react-storage/src/components/StorageBrowser/views/LocationDetailView/useLocationDetailView.ts @@ -3,6 +3,7 @@ import React, { useCallback } from 'react'; import { isFunction, isUndefined } from '@aws-amplify/ui'; import { usePaginate } from '../hooks/usePaginate'; +import { useSort } from '../hooks/useSort'; import type { DownloadHandlerData, @@ -91,6 +92,9 @@ export const useLocationDetailView = ( // set up pagination const { items, nextToken, hasExhaustedSearch = false } = value; + // sort all items before pagination for cross-page sort + const { sortedItems, sortConfig, onSort, resetSort } = useSort({ items }); + const onPaginate = () => { if (hasInvalidPrefix || !nextToken) return; locationItemsDispatch({ type: 'RESET_LOCATION_ITEMS' }); @@ -108,7 +112,7 @@ export const useLocationDetailView = ( highestPageVisited, pageItems, } = usePaginate({ - items, + items: sortedItems, onPaginate, pageSize: listOptions.pageSize, }); @@ -213,6 +217,7 @@ export const useLocationDetailView = ( handleReset(); resetSearch(); + resetSort(); handleList({ prefix: key, options: { ...listOptions, refresh: true }, @@ -287,6 +292,7 @@ export const useLocationDetailView = ( setActiveFile(undefined); }, searchQuery, + sortConfig, searchProgress, filePreviewState, filePreviewEnabled: !optout, @@ -305,6 +311,7 @@ export const useLocationDetailView = ( onNavigate: (location: LocationData, path?: string) => { onNavigate?.(location, path); resetSearch(); + resetSort(); storeDispatch({ type: 'CHANGE_LOCATION', location, path }); locationItemsDispatch({ type: 'RESET_LOCATION_ITEMS' }); setActiveFile(undefined); @@ -356,6 +363,7 @@ export const useLocationDetailView = ( handleList({ prefix: key, options: { ...listOptions, refresh: true } }); handleReset(); }, + onSort, onSearchQueryChange, onRetryFilePreview: handleRetry, onToggleSearchSubfolders, diff --git a/packages/react-storage/src/components/StorageBrowser/views/hooks/__tests__/sortItems.spec.ts b/packages/react-storage/src/components/StorageBrowser/views/hooks/__tests__/sortItems.spec.ts new file mode 100644 index 00000000000..e31b38f51c2 --- /dev/null +++ b/packages/react-storage/src/components/StorageBrowser/views/hooks/__tests__/sortItems.spec.ts @@ -0,0 +1,165 @@ +import type { LocationItemData } from '../../../actions'; + +import { sortItems } from '../sortItems'; +import type { SortConfig } from '../sortItems'; + +const folder1: LocationItemData = { + key: 'beta-folder/', + id: 'folder-1', + type: 'FOLDER', +}; + +const folder2: LocationItemData = { + key: 'alpha-folder/', + id: 'folder-2', + type: 'FOLDER', +}; + +const file1: LocationItemData = { + key: 'prefix/charlie.txt', + id: 'file-1', + type: 'FILE', + lastModified: new Date('2024-03-15'), + size: 500, +}; + +const file2: LocationItemData = { + key: 'prefix/alpha.png', + id: 'file-2', + type: 'FILE', + lastModified: new Date('2024-01-01'), + size: 1000, +}; + +const file3: LocationItemData = { + key: 'prefix/beta.jpg', + id: 'file-3', + type: 'FILE', + lastModified: new Date('2024-06-20'), + size: 200, +}; + +const allItems: LocationItemData[] = [folder1, file1, folder2, file2, file3]; + +describe('sortItems', () => { + describe('sort by name', () => { + it('sorts ascending with folders first', () => { + const config: SortConfig = { field: 'name', direction: 'ascending' }; + const result = sortItems(allItems, config); + + expect(result.map((i) => i.id)).toEqual([ + 'folder-2', + 'folder-1', + 'file-2', + 'file-3', + 'file-1', + ]); + }); + + it('sorts descending with files first', () => { + const config: SortConfig = { field: 'name', direction: 'descending' }; + const result = sortItems(allItems, config); + + expect(result.map((i) => i.id)).toEqual([ + 'file-1', + 'file-3', + 'file-2', + 'folder-1', + 'folder-2', + ]); + }); + }); + + describe('sort by size', () => { + it('sorts ascending with folders first', () => { + const config: SortConfig = { field: 'size', direction: 'ascending' }; + const result = sortItems(allItems, config); + + const folderIds = result + .filter((i) => i.type === 'FOLDER') + .map((i) => i.id); + const fileIds = result.filter((i) => i.type === 'FILE').map((i) => i.id); + + expect(folderIds).toEqual(['folder-1', 'folder-2']); + expect(fileIds).toEqual(['file-3', 'file-1', 'file-2']); + }); + + it('sorts descending with files first', () => { + const config: SortConfig = { field: 'size', direction: 'descending' }; + const result = sortItems(allItems, config); + + const fileIds = result.filter((i) => i.type === 'FILE').map((i) => i.id); + const folderIds = result + .filter((i) => i.type === 'FOLDER') + .map((i) => i.id); + + expect(fileIds).toEqual(['file-2', 'file-1', 'file-3']); + expect(folderIds).toEqual(['folder-1', 'folder-2']); + }); + }); + + describe('sort by last-modified', () => { + it('sorts ascending with folders first', () => { + const config: SortConfig = { + field: 'last-modified', + direction: 'ascending', + }; + const result = sortItems(allItems, config); + + const fileIds = result.filter((i) => i.type === 'FILE').map((i) => i.id); + + expect(fileIds).toEqual(['file-2', 'file-1', 'file-3']); + }); + + it('sorts descending with files first', () => { + const config: SortConfig = { + field: 'last-modified', + direction: 'descending', + }; + const result = sortItems(allItems, config); + + expect(result[0].id).toBe('file-3'); + expect(result[1].id).toBe('file-1'); + expect(result[2].id).toBe('file-2'); + }); + }); + + describe('sort by type', () => { + it('sorts ascending by file extension', () => { + const config: SortConfig = { field: 'type', direction: 'ascending' }; + const result = sortItems(allItems, config); + + const fileIds = result.filter((i) => i.type === 'FILE').map((i) => i.id); + + // jpg < png < txt + expect(fileIds).toEqual(['file-3', 'file-2', 'file-1']); + }); + }); + + describe('edge cases', () => { + it('returns empty array for empty input', () => { + const config: SortConfig = { field: 'name', direction: 'ascending' }; + expect(sortItems([], config)).toEqual([]); + }); + + it('handles only folders', () => { + const config: SortConfig = { field: 'name', direction: 'ascending' }; + const result = sortItems([folder1, folder2], config); + expect(result.map((i) => i.id)).toEqual(['folder-2', 'folder-1']); + }); + + it('handles only files', () => { + const config: SortConfig = { field: 'name', direction: 'ascending' }; + const result = sortItems([file1, file2, file3], config); + expect(result.map((i) => i.id)).toEqual(['file-2', 'file-3', 'file-1']); + }); + + it('does not mutate the input array', () => { + const items = [file2, file1]; + const config: SortConfig = { field: 'name', direction: 'ascending' }; + sortItems(items, config); + expect(items[0].id).toBe('file-2'); + expect(items[1].id).toBe('file-1'); + }); + }); +}); diff --git a/packages/react-storage/src/components/StorageBrowser/views/hooks/__tests__/useSort.spec.ts b/packages/react-storage/src/components/StorageBrowser/views/hooks/__tests__/useSort.spec.ts new file mode 100644 index 00000000000..da57ec31774 --- /dev/null +++ b/packages/react-storage/src/components/StorageBrowser/views/hooks/__tests__/useSort.spec.ts @@ -0,0 +1,176 @@ +import { act, renderHook } from '@testing-library/react'; + +import type { LocationItemData } from '../../../actions'; + +import { useSort } from '../useSort'; + +const folder1: LocationItemData = { + key: 'beta-folder/', + id: 'folder-1', + type: 'FOLDER', +}; + +const file1: LocationItemData = { + key: 'prefix/charlie.txt', + id: 'file-1', + type: 'FILE', + lastModified: new Date('2024-03-15'), + size: 500, +}; + +const file2: LocationItemData = { + key: 'prefix/alpha.png', + id: 'file-2', + type: 'FILE', + lastModified: new Date('2024-01-01'), + size: 1000, +}; + +const file3: LocationItemData = { + key: 'prefix/beta.jpg', + id: 'file-3', + type: 'FILE', + lastModified: new Date('2024-06-20'), + size: 200, +}; + +const items: LocationItemData[] = [folder1, file1, file2, file3]; + +describe('useSort', () => { + it('returns items as-is when no sort is active', () => { + const { result } = renderHook(() => useSort({ items })); + + expect(result.current.sortedItems).toBe(items); + expect(result.current.sortConfig).toBeUndefined(); + }); + + it('returns empty array when items is undefined', () => { + const { result } = renderHook(() => useSort({ items: undefined })); + + expect(result.current.sortedItems).toEqual([]); + }); + + it('sorts by name ascending on first click', () => { + const { result } = renderHook(() => useSort({ items })); + + act(() => { + result.current.onSort('name'); + }); + + expect(result.current.sortConfig).toEqual({ + field: 'name', + direction: 'ascending', + }); + + const ids = result.current.sortedItems.map((i) => i.id); + expect(ids).toEqual(['folder-1', 'file-2', 'file-3', 'file-1']); + }); + + it('toggles to descending on second click of same column', () => { + const { result } = renderHook(() => useSort({ items })); + + act(() => { + result.current.onSort('name'); + }); + act(() => { + result.current.onSort('name'); + }); + + expect(result.current.sortConfig).toEqual({ + field: 'name', + direction: 'descending', + }); + }); + + it('resets to ascending when switching columns', () => { + const { result } = renderHook(() => useSort({ items })); + + act(() => { + result.current.onSort('name'); + }); + act(() => { + result.current.onSort('name'); + }); + + expect(result.current.sortConfig?.direction).toBe('descending'); + + act(() => { + result.current.onSort('size'); + }); + + expect(result.current.sortConfig).toEqual({ + field: 'size', + direction: 'ascending', + }); + }); + + it('ignores non-sortable header keys', () => { + const { result } = renderHook(() => useSort({ items })); + + act(() => { + result.current.onSort('checkbox'); + }); + + expect(result.current.sortConfig).toBeUndefined(); + }); + + it('ignores download header key', () => { + const { result } = renderHook(() => useSort({ items })); + + act(() => { + result.current.onSort('download'); + }); + + expect(result.current.sortConfig).toBeUndefined(); + }); + + it('resets sort state', () => { + const { result } = renderHook(() => useSort({ items })); + + act(() => { + result.current.onSort('name'); + }); + + expect(result.current.sortConfig).toBeDefined(); + + act(() => { + result.current.resetSort(); + }); + + expect(result.current.sortConfig).toBeUndefined(); + expect(result.current.sortedItems).toBe(items); + }); + + it('sorts by last-modified', () => { + const { result } = renderHook(() => useSort({ items })); + + act(() => { + result.current.onSort('last-modified'); + }); + + const fileIds = result.current.sortedItems + .filter((i) => i.type === 'FILE') + .map((i) => i.id); + + // ascending: oldest first + expect(fileIds).toEqual(['file-2', 'file-1', 'file-3']); + }); + + it('sorts by size descending', () => { + const { result } = renderHook(() => useSort({ items })); + + act(() => { + result.current.onSort('size'); + }); + act(() => { + result.current.onSort('size'); + }); + + const fileIds = result.current.sortedItems + .filter((i) => i.type === 'FILE') + .map((i) => i.id); + + // descending: largest first, files before folders + expect(fileIds).toEqual(['file-2', 'file-1', 'file-3']); + }); +}); diff --git a/packages/react-storage/src/components/StorageBrowser/views/hooks/sortItems.ts b/packages/react-storage/src/components/StorageBrowser/views/hooks/sortItems.ts new file mode 100644 index 00000000000..e03a1a71810 --- /dev/null +++ b/packages/react-storage/src/components/StorageBrowser/views/hooks/sortItems.ts @@ -0,0 +1,85 @@ +import type { LocationItemData } from '../../actions'; +import type { SortDirection } from '../../components'; + +/** + * Sort field identifiers matching the LocationDetailView table columns. + */ +export type SortField = 'name' | 'type' | 'last-modified' | 'size'; + +export interface SortConfig { + field: SortField; + direction: SortDirection; +} + +const getFileName = (item: LocationItemData): string => item.key; + +const getFileType = (item: LocationItemData): string => { + if (item.type === 'FOLDER') return ''; + const parts = item.key.split('.'); + return parts.length > 1 ? parts[parts.length - 1].toLowerCase() : ''; +}; + +const getLastModified = (item: LocationItemData): number | undefined => + item.type === 'FILE' ? item.lastModified.getTime() : undefined; + +const getSize = (item: LocationItemData): number | undefined => + item.type === 'FILE' ? item.size : undefined; + +const compareStrings = (a: string, b: string): number => a.localeCompare(b); + +const compareOptionalNumbers = ( + a: number | undefined, + b: number | undefined +): number => { + if (a === undefined) return b === undefined ? 0 : 1; + return b === undefined ? -1 : a - b; +}; + +/** + * Sorts an array of `LocationItemData` items by the given field and direction. + * + * Preserves the same grouping behavior as `useDataTable`: folders and files are + * sorted as separate groups and concatenated in order (folders first ascending, + * files first descending). Items within each group are sorted by the specified field. + * + * @param items - The full array of items to sort (across all pages) + * @param config - Sort configuration with field and direction + * @returns A new sorted array (does not mutate the input) + */ +export const sortItems = ( + items: LocationItemData[], + config: SortConfig +): LocationItemData[] => { + const { field, direction } = config; + + const folders = items.filter((item) => item.type === 'FOLDER'); + const files = items.filter((item) => item.type === 'FILE'); + + const comparator = (a: LocationItemData, b: LocationItemData): number => { + let result: number; + switch (field) { + case 'name': + result = compareStrings(getFileName(a), getFileName(b)); + break; + case 'type': + result = compareStrings(getFileType(a), getFileType(b)); + break; + case 'last-modified': + result = compareOptionalNumbers(getLastModified(a), getLastModified(b)); + break; + case 'size': + result = compareOptionalNumbers(getSize(a), getSize(b)); + break; + default: + result = 0; + } + return direction === 'ascending' ? result : -result; + }; + + const sortedFolders = [...folders].sort(comparator); + const sortedFiles = [...files].sort(comparator); + + return direction === 'ascending' + ? [...sortedFolders, ...sortedFiles] + : [...sortedFiles, ...sortedFolders]; +}; diff --git a/packages/react-storage/src/components/StorageBrowser/views/hooks/useSort.ts b/packages/react-storage/src/components/StorageBrowser/views/hooks/useSort.ts new file mode 100644 index 00000000000..62aa6ace0d2 --- /dev/null +++ b/packages/react-storage/src/components/StorageBrowser/views/hooks/useSort.ts @@ -0,0 +1,77 @@ +import React from 'react'; + +import type { LocationItemData } from '../../actions'; + +import type { SortField, SortConfig } from './sortItems'; +import { sortItems } from './sortItems'; + +import type { HeaderKeys } from '../LocationDetailView/getLocationDetailViewTableData/types'; + +export interface UseSortInput { + items: LocationItemData[] | undefined; +} + +export interface UseSortState { + /** Items sorted by the current sort configuration */ + sortedItems: LocationItemData[]; + /** Current sort column and direction, undefined when using default order */ + sortConfig: SortConfig | undefined; + /** + * Callback for sort header clicks. + * @param headerKey - The header key from the DataTable column + */ + onSort: (headerKey: HeaderKeys) => void; + /** Resets sort state back to default (no sorting) */ + resetSort: () => void; +} + +const SORTABLE_HEADERS: HeaderKeys[] = [ + 'name', + 'type', + 'last-modified', + 'size', +]; + +const isSortableHeader = (key: HeaderKeys): key is SortField => + SORTABLE_HEADERS.includes(key); + +/** + * Manages sort state for location detail items and returns sorted items. + * + * Sorting is applied to the full items array before pagination, enabling + * cross-page sorting. When no sort is active, items are returned as-is. + * + * @param input.items - The full array of items (across all pages) + * @returns Sorted items plus sort state and handlers + */ +export const useSort = ({ items }: UseSortInput): UseSortState => { + const [sortConfig, setSortConfig] = React.useState( + undefined + ); + + const resetSort = React.useRef(() => { + setSortConfig(undefined); + }).current; + + const onSort = React.useCallback((headerKey: HeaderKeys) => { + if (!isSortableHeader(headerKey)) return; + + setSortConfig((prev) => ({ + field: headerKey, + direction: + prev?.field === headerKey + ? prev.direction === 'ascending' + ? 'descending' + : 'ascending' + : 'ascending', + })); + }, []); + + const sortedItems = React.useMemo(() => { + if (!items) return []; + if (!sortConfig) return items; + return sortItems(items, sortConfig); + }, [items, sortConfig]); + + return { sortedItems, sortConfig, onSort, resetSort }; +}; From a1fa174af1271e64b513f604d7b3c6475b0983c8 Mon Sep 17 00:00:00 2001 From: Michael Sober <> Date: Thu, 16 Apr 2026 09:28:41 +0000 Subject: [PATCH 3/8] test(storage-browser): add E2E sorting scenarios and fix bundle size limits - Add cross-page-sort.feature with 7 E2E scenarios covering: - Sort by name ascending/descending - Sort by size and last modified columns - Changing sort column resets direction - Sort combined with search results - Sort direction toggle preservation - Add sorting step definitions to storagebrowser.ts: - Click sort header by column label - Verify first table row name - Verify table name column sort order - Assert minimum table row count - Bump size-limit thresholds for createStorageBrowser (130->131 kB) and StorageBrowser (154->155 kB) to accommodate sorting code --- .../integration/common/storagebrowser.ts | 32 ++++++++ .../storage-browser/cross-page-sort.feature | 74 +++++++++++++++++++ packages/react-storage/package.json | 4 +- 3 files changed, 108 insertions(+), 2 deletions(-) create mode 100644 packages/e2e/features/ui/components/storage/storage-browser/cross-page-sort.feature diff --git a/packages/e2e/cypress/integration/common/storagebrowser.ts b/packages/e2e/cypress/integration/common/storagebrowser.ts index 5f79840b04a..adb213a4bac 100644 --- a/packages/e2e/cypress/integration/common/storagebrowser.ts +++ b/packages/e2e/cypress/integration/common/storagebrowser.ts @@ -226,3 +226,35 @@ Then( }); } ); + +When('I click the {string} sort header', (columnLabel: string) => { + cy.get('.amplify-storage-browser__table-sort-header') + .contains(new RegExp(`^${columnLabel}$`, 'i')) + .click(); +}); + +Then('the first table row name should contain {string}', (expected: string) => { + cy.get('table tbody tr') + .first() + .find('td:nth-child(2)') + .should('contain.text', expected); +}); + +Then( + 'the table name column values should be in {string} order', + (direction: string) => { + cy.get('table tbody tr td:nth-child(2)').then(($cells) => { + const values = [...$cells].map( + (cell) => cell.textContent?.trim().toLowerCase() ?? '' + ); + const sorted = [...values].sort((a, b) => + direction === 'ascending' ? a.localeCompare(b) : b.localeCompare(a) + ); + expect(values).to.deep.equal(sorted); + }); + } +); + +Then('the table should have at least {string} rows', (count: string) => { + cy.get('table tbody tr').should('have.length.gte', parseInt(count)); +}); diff --git a/packages/e2e/features/ui/components/storage/storage-browser/cross-page-sort.feature b/packages/e2e/features/ui/components/storage/storage-browser/cross-page-sort.feature new file mode 100644 index 00000000000..e67d98f7172 --- /dev/null +++ b/packages/e2e/features/ui/components/storage/storage-browser/cross-page-sort.feature @@ -0,0 +1,74 @@ +Feature: StorageBrowser cross-page sorting + + Tests that sorting works across multiple pages of results, + including sorting by different columns and combining sort with search. + + Background: + Given I'm running the example "ui/components/storage/storage-browser/default-auth" + And I type my "email" with status "CONFIRMED" + And I type my password + And I click the "Sign in" button + + @react + Scenario: Sort by name ascending + When I click the first button containing "public" + Then the table should have at least "2" rows + When I click the "Name" sort header + Then the table name column values should be in "ascending" order + + @react + Scenario: Sort by name descending + When I click the first button containing "public" + Then the table should have at least "2" rows + When I click the "Name" sort header + Then the table name column values should be in "ascending" order + When I click the "Name" sort header + Then the table name column values should be in "descending" order + + @react + Scenario: Sort by size column + When I click the first button containing "public" + Then the table should have at least "2" rows + When I click the "Size" sort header + Then the table should have at least "2" rows + + @react + Scenario: Sort by last modified column + When I click the first button containing "public" + Then the table should have at least "2" rows + When I click the "Last modified" sort header + Then the table should have at least "2" rows + + @react + Scenario: Changing sort column resets to ascending + When I click the first button containing "public" + Then the table should have at least "2" rows + When I click the "Name" sort header + Then the table name column values should be in "ascending" order + When I click the "Name" sort header + Then the table name column values should be in "descending" order + # Switching to a different column should reset to ascending + When I click the "Size" sort header + Then the table should have at least "2" rows + + @react + Scenario: Sort combined with search results + When I click the first button containing "public" + When I see input with placeholder "Search current folder" and type "DO_NOT" + Then I click the "Search" button + Then I see the button containing "DO_NOT_DELETE" + # Sort the search results by name + When I click the "Name" sort header + Then the table should have at least "1" rows + + @react + Scenario: Sort direction preserved after toggling + When I click the first button containing "public" + Then the table should have at least "2" rows + When I click the "Name" sort header + Then the table name column values should be in "ascending" order + When I click the "Name" sort header + Then the table name column values should be in "descending" order + # Toggle back to ascending + When I click the "Name" sort header + Then the table name column values should be in "ascending" order diff --git a/packages/react-storage/package.json b/packages/react-storage/package.json index c544f1a9ffd..9cedc3220e8 100644 --- a/packages/react-storage/package.json +++ b/packages/react-storage/package.json @@ -69,7 +69,7 @@ "name": "createStorageBrowser", "path": "dist/esm/browser.mjs", "import": "{ createStorageBrowser }", - "limit": "130 kB", + "limit": "131 kB", "ignore": [ "@aws-amplify/storage" ] @@ -78,7 +78,7 @@ "name": "StorageBrowser", "path": "dist/esm/index.mjs", "import": "{ StorageBrowser }", - "limit": "154 kB" + "limit": "155 kB" }, { "name": "FileUploader", From d83011adb7befa7679290f4e59e8d415d16f40fb Mon Sep 17 00:00:00 2001 From: Michael Sober Date: Mon, 20 Apr 2026 14:44:15 +0000 Subject: [PATCH 4/8] fix(e2e): align sort order assertion with folders-first grouping The cross-page sort E2E step definition was asserting flat alphabetical order across all items, but the StorageBrowser sorts folders and files as separate groups (folders first ascending, files first descending). Update the assertion to partition by folder/file and verify sort within each group, matching the actual sorting behavior in sortItems.ts and useDataTable.ts. --- .../integration/common/storagebrowser.ts | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/packages/e2e/cypress/integration/common/storagebrowser.ts b/packages/e2e/cypress/integration/common/storagebrowser.ts index adb213a4bac..b9e475576ef 100644 --- a/packages/e2e/cypress/integration/common/storagebrowser.ts +++ b/packages/e2e/cypress/integration/common/storagebrowser.ts @@ -247,9 +247,21 @@ Then( const values = [...$cells].map( (cell) => cell.textContent?.trim().toLowerCase() ?? '' ); - const sorted = [...values].sort((a, b) => - direction === 'ascending' ? a.localeCompare(b) : b.localeCompare(a) - ); + + const folders = values.filter((v) => v.endsWith('/')); + const files = values.filter((v) => !v.endsWith('/')); + + const compare = (a: string, b: string) => + direction === 'ascending' ? a.localeCompare(b) : b.localeCompare(a); + + const sortedFolders = [...folders].sort(compare); + const sortedFiles = [...files].sort(compare); + + const sorted = + direction === 'ascending' + ? [...sortedFolders, ...sortedFiles] + : [...sortedFiles, ...sortedFolders]; + expect(values).to.deep.equal(sorted); }); } From 13427c62b534de832f7f9ee18b2639e2faf2e105 Mon Sep 17 00:00:00 2001 From: Michael Sober Date: Tue, 21 Apr 2026 12:06:19 +0000 Subject: [PATCH 5/8] feat(storage-browser): add sortScope option to toggle between page and all-items sorting --- .../StorageBrowser/configuration/index.ts | 2 + .../configuration/sortConfigContext.tsx | 45 ++++++ .../__tests__/createProvider.spec.tsx | 4 + .../createStorageBrowser/createProvider.tsx | 42 +++--- .../createStorageBrowser/types.ts | 18 ++- .../__tests__/useLocationDetailView.spec.tsx | 136 +++++++++++++++++- .../views/LocationDetailView/types.ts | 2 +- .../useLocationDetailView.ts | 11 +- 8 files changed, 228 insertions(+), 32 deletions(-) create mode 100644 packages/react-storage/src/components/StorageBrowser/configuration/sortConfigContext.tsx diff --git a/packages/react-storage/src/components/StorageBrowser/configuration/index.ts b/packages/react-storage/src/components/StorageBrowser/configuration/index.ts index 0eab98ad554..3cd63e22a4a 100644 --- a/packages/react-storage/src/components/StorageBrowser/configuration/index.ts +++ b/packages/react-storage/src/components/StorageBrowser/configuration/index.ts @@ -6,3 +6,5 @@ export { PaginationConfigProvider, } from './paginationContext'; export type { PaginationConfig } from './paginationContext'; +export { useSortConfig, SortConfigProvider } from './sortConfigContext'; +export type { SortScope, SortConfig } from './sortConfigContext'; diff --git a/packages/react-storage/src/components/StorageBrowser/configuration/sortConfigContext.tsx b/packages/react-storage/src/components/StorageBrowser/configuration/sortConfigContext.tsx new file mode 100644 index 00000000000..973bc51f4ff --- /dev/null +++ b/packages/react-storage/src/components/StorageBrowser/configuration/sortConfigContext.tsx @@ -0,0 +1,45 @@ +import React from 'react'; +import { createContextUtilities } from '@aws-amplify/ui-react-core'; + +/** + * Controls where sorting is applied in the Storage Browser. + * + * - `'all'` — Sort all loaded items before pagination (cross-page sort). + * Headers are handled by the view-level `useSort` hook. + * - `'page'` — Sort only the current display page. Headers are handled + * by the table-level local sort inside `useDataTable`. + * + * @default 'page' + */ +export type SortScope = 'all' | 'page'; + +export interface SortConfig { + sortScope: SortScope; +} + +const ERROR_MESSAGE = + '`useSortConfig` must be called from within a `SortConfigProvider`.'; + +export const { useSortConfig, SortConfigContext } = + createContextUtilities({ + contextName: 'SortConfig', + errorMessage: ERROR_MESSAGE, + }); + +export interface SortConfigProviderProps { + children?: React.ReactNode; + sortScope?: SortScope; +} + +export function SortConfigProvider({ + children, + sortScope = 'page', +}: SortConfigProviderProps): React.JSX.Element { + const value = React.useMemo(() => ({ sortScope }), [sortScope]); + + return ( + + {children} + + ); +} diff --git a/packages/react-storage/src/components/StorageBrowser/createStorageBrowser/__tests__/createProvider.spec.tsx b/packages/react-storage/src/components/StorageBrowser/createStorageBrowser/__tests__/createProvider.spec.tsx index 522d41787fb..5ce9c2d27d8 100644 --- a/packages/react-storage/src/components/StorageBrowser/createStorageBrowser/__tests__/createProvider.spec.tsx +++ b/packages/react-storage/src/components/StorageBrowser/createStorageBrowser/__tests__/createProvider.spec.tsx @@ -6,6 +6,7 @@ import { ComponentsProvider } from '../../components'; import { createConfigurationProvider, PaginationConfigProvider, + SortConfigProvider, } from '../../configuration'; import { DisplayTextProvider } from '../../displayText'; import { FileItemsProvider } from '../../fileItems'; @@ -31,6 +32,9 @@ jest.mock('../../views'); jest .mocked(PaginationConfigProvider) .mockImplementation(({ children }) => <>{children}); +jest + .mocked(SortConfigProvider) + .mockImplementation(({ children }) => <>{children}); const mockActionConfigsProvider = jest .mocked(ActionConfigsProvider) diff --git a/packages/react-storage/src/components/StorageBrowser/createStorageBrowser/createProvider.tsx b/packages/react-storage/src/components/StorageBrowser/createStorageBrowser/createProvider.tsx index 4f378d32b7f..2d4c5d6213b 100644 --- a/packages/react-storage/src/components/StorageBrowser/createStorageBrowser/createProvider.tsx +++ b/packages/react-storage/src/components/StorageBrowser/createStorageBrowser/createProvider.tsx @@ -11,6 +11,7 @@ import { import { createConfigurationProvider, PaginationConfigProvider, + SortConfigProvider, } from '../configuration'; import { DisplayTextProvider } from '../displayText'; import { defaultValidateFile, FileItemsProvider } from '../fileItems'; @@ -88,31 +89,34 @@ export default function createProvider< displayText, views, pageSize, + sortScope, ...props }: StorageBrowserProviderProps) { return ( - - - - - - - - - filePreview={filePreview} - > - {children} - - - - - - - - + + + + + + + + + + filePreview={filePreview} + > + {children} + + + + + + + + + diff --git a/packages/react-storage/src/components/StorageBrowser/createStorageBrowser/types.ts b/packages/react-storage/src/components/StorageBrowser/createStorageBrowser/types.ts index 010a8b0610e..9d09a3c9b72 100644 --- a/packages/react-storage/src/components/StorageBrowser/createStorageBrowser/types.ts +++ b/packages/react-storage/src/components/StorageBrowser/createStorageBrowser/types.ts @@ -9,6 +9,7 @@ import type { LocationData, } from '../actions'; import type { StorageBrowserComponents } from '../components'; +import type { SortScope } from '../configuration'; import type { GetLocationCredentials, RegisterAuthListener, @@ -327,6 +328,16 @@ export interface StorageBrowserProps { * @minimum 1 */ pageSize?: number; + + /** + * @description Controls where sorting is applied. + * + * - `'all'` — Sort all loaded items before pagination (cross-page sort). + * - `'page'` — Sort only the current display page (table-level local sort). + * + * @default 'page' + */ + sortScope?: SortScope; } /** @@ -337,7 +348,12 @@ export interface StorageBrowserProviderProps extends StoreProviderProps, Pick< StorageBrowserProps, - 'defaultValue' | 'displayText' | 'onValueChange' | 'value' | 'pageSize' + | 'defaultValue' + | 'displayText' + | 'onValueChange' + | 'value' + | 'pageSize' + | 'sortScope' > { // note: `views` intentionally scoped to custom slots to prevent conflicts with composability /** diff --git a/packages/react-storage/src/components/StorageBrowser/views/LocationDetailView/__tests__/useLocationDetailView.spec.tsx b/packages/react-storage/src/components/StorageBrowser/views/LocationDetailView/__tests__/useLocationDetailView.spec.tsx index 09b60048d64..d3a549f2776 100644 --- a/packages/react-storage/src/components/StorageBrowser/views/LocationDetailView/__tests__/useLocationDetailView.spec.tsx +++ b/packages/react-storage/src/components/StorageBrowser/views/LocationDetailView/__tests__/useLocationDetailView.spec.tsx @@ -10,6 +10,7 @@ import { import { useFileItems } from '../../../fileItems'; import { useLocationItems } from '../../../locationItems/context'; +import { useSortConfig } from '../../../configuration'; import { LocationState, useStore } from '../../../store'; import { useAction, useList } from '../../../useAction'; import { useFilePreview } from '../../hooks/useFilePreview'; @@ -32,6 +33,7 @@ jest.mock('../../../configuration', () => ({ usePaginationConfig: jest.fn((initialValues: InitialValues) => ({ pageSize: initialValues?.pageSize ?? 100, })), + useSortConfig: jest.fn(() => ({ sortScope: 'page' })), })); const folderDataOne: FolderData = { @@ -990,6 +992,8 @@ describe('useLocationDetailView', () => { }); describe('cross-page sorting', () => { + const mockUseSortConfig = jest.mocked(useSortConfig); + const sortableItems: LocationItemData[] = [ folderDataOne, { @@ -1013,6 +1017,7 @@ describe('useLocationDetailView', () => { ]; beforeEach(() => { + mockUseSortConfig.mockReturnValue({ sortScope: 'all' }); mockUseList.mockReturnValue([ { value: { items: sortableItems, nextToken: undefined }, @@ -1024,6 +1029,10 @@ describe('useLocationDetailView', () => { ]); }); + afterEach(() => { + mockUseSortConfig.mockReturnValue({ sortScope: 'page' }); + }); + it('should return onSort and sortConfig', () => { const { result } = renderHook(() => useLocationDetailView()); @@ -1039,7 +1048,7 @@ describe('useLocationDetailView', () => { expect(result.current.pageItems).toHaveLength(2); act(() => { - result.current.onSort('name'); + result.current.onSort!('name'); }); expect(result.current.sortConfig).toEqual({ @@ -1064,12 +1073,12 @@ describe('useLocationDetailView', () => { const { result } = renderHook(() => useLocationDetailView()); act(() => { - result.current.onSort('name'); + result.current.onSort!('name'); }); expect(result.current.sortConfig?.direction).toBe('ascending'); act(() => { - result.current.onSort('name'); + result.current.onSort!('name'); }); expect(result.current.sortConfig?.direction).toBe('descending'); }); @@ -1078,7 +1087,7 @@ describe('useLocationDetailView', () => { const { result } = renderHook(() => useLocationDetailView()); act(() => { - result.current.onSort('size'); + result.current.onSort!('size'); }); expect(result.current.sortConfig).toEqual({ @@ -1099,7 +1108,7 @@ describe('useLocationDetailView', () => { const { result } = renderHook(() => useLocationDetailView()); act(() => { - result.current.onSort('name'); + result.current.onSort!('name'); }); expect(result.current.sortConfig).toBeDefined(); @@ -1113,7 +1122,7 @@ describe('useLocationDetailView', () => { const { result } = renderHook(() => useLocationDetailView()); act(() => { - result.current.onSort('name'); + result.current.onSort!('name'); }); expect(result.current.sortConfig).toBeDefined(); @@ -1140,7 +1149,7 @@ describe('useLocationDetailView', () => { const { result } = renderHook(() => useLocationDetailView()); act(() => { - result.current.onSort('size'); + result.current.onSort!('size'); }); const fileItems = result.current.pageItems.filter( @@ -1151,4 +1160,117 @@ describe('useLocationDetailView', () => { expect(sizes).toEqual([300, 500, 1000]); }); }); + + describe('sortScope option', () => { + const mockUseSortConfig = jest.mocked(useSortConfig); + + const sortableItems: LocationItemData[] = [ + folderDataOne, + { + ...fileDataOne, + key: 'some-prefix/charlie.jpg', + size: 300, + lastModified: new Date('2024-03-15'), + }, + { + ...fileDataTwo, + key: 'some-prefix/alpha.png', + size: 1000, + lastModified: new Date('2024-01-01'), + }, + { + ...fileDataThree, + key: 'some-prefix/beta.doc', + size: 500, + lastModified: new Date('2024-06-20'), + }, + ]; + + beforeEach(() => { + mockUseList.mockReturnValue([ + { + value: { items: sortableItems, nextToken: undefined }, + message: '', + hasError: false, + isLoading: false, + }, + mockHandleList, + ]); + }); + + afterEach(() => { + mockUseSortConfig.mockReturnValue({ sortScope: 'page' }); + }); + + it('should default to page sort (sortScope "page")', () => { + const { result } = renderHook(() => useLocationDetailView()); + + expect(result.current.onSort).toBeUndefined(); + expect(result.current.sortConfig).toBeUndefined(); + }); + + it('should return undefined onSort and sortConfig when sortScope is "page"', () => { + mockUseSortConfig.mockReturnValue({ sortScope: 'page' }); + + const { result } = renderHook(() => useLocationDetailView()); + + expect(result.current.onSort).toBeUndefined(); + expect(result.current.sortConfig).toBeUndefined(); + }); + + it('should not sort items across pages when sortScope is "page"', () => { + mockUseSortConfig.mockReturnValue({ sortScope: 'page' }); + + const { result } = renderHook(() => + useLocationDetailView({ pageSize: 2 }) + ); + + // items should come in original order (no cross-page sorting applied) + const ids = result.current.pageItems.map((i) => i.id); + expect(ids).toEqual([sortableItems[0].id, sortableItems[1].id]); + }); + + it('should enable cross-page sort when sortScope is "all"', () => { + mockUseSortConfig.mockReturnValue({ sortScope: 'all' }); + + const { result } = renderHook(() => useLocationDetailView()); + + expect(result.current.onSort).toEqual(expect.any(Function)); + expect(result.current.sortConfig).toBeUndefined(); + + act(() => { + result.current.onSort!('name'); + }); + + expect(result.current.sortConfig).toEqual({ + field: 'name', + direction: 'ascending', + }); + }); + + it('should sort items across pages when sortScope is "all"', () => { + mockUseSortConfig.mockReturnValue({ sortScope: 'all' }); + + const { result } = renderHook(() => + useLocationDetailView({ pageSize: 2 }) + ); + + act(() => { + result.current.onSort!('size'); + }); + + expect(result.current.sortConfig).toEqual({ + field: 'size', + direction: 'ascending', + }); + + // folders first, then smallest file by size + const firstPageItems = result.current.pageItems; + expect(firstPageItems).toHaveLength(2); + + // folder comes first, then the smallest file (size 300) + expect(firstPageItems[0].type).toBe('FOLDER'); + expect((firstPageItems[1] as FileData).size).toBe(300); + }); + }); }); diff --git a/packages/react-storage/src/components/StorageBrowser/views/LocationDetailView/types.ts b/packages/react-storage/src/components/StorageBrowser/views/LocationDetailView/types.ts index 57da53c7a46..237097354a3 100644 --- a/packages/react-storage/src/components/StorageBrowser/views/LocationDetailView/types.ts +++ b/packages/react-storage/src/components/StorageBrowser/views/LocationDetailView/types.ts @@ -52,7 +52,7 @@ export interface LocationDetailViewState { onRetryFilePreview: () => void; onSelect: (isSelected: boolean, item: LocationItemData) => void; onSelectActiveFile: (file?: FileData | 'prev' | 'next') => void; - onSort: (headerKey: HeaderKeys) => void; + onSort: ((headerKey: HeaderKeys) => void) | undefined; onToggleSearchSubfolders: () => void; onToggleSelectAll: () => void; page: number; 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 77fc78f603e..965f1c2d068 100644 --- a/packages/react-storage/src/components/StorageBrowser/views/LocationDetailView/useLocationDetailView.ts +++ b/packages/react-storage/src/components/StorageBrowser/views/LocationDetailView/useLocationDetailView.ts @@ -14,7 +14,7 @@ import type { LocationItemData, } from '../../actions'; import { useActionConfigs } from '../../actions'; -import { usePaginationConfig } from '../../configuration'; +import { usePaginationConfig, useSortConfig } from '../../configuration'; import { useFileItems } from '../../fileItems'; import { useLocationItems } from '../../locationItems/context'; import { useStore } from '../../store'; @@ -52,6 +52,7 @@ export const useLocationDetailView = ( options?: UseLocationDetailViewOptions ): LocationDetailViewState => { const { pageSize: configPageSize } = usePaginationConfig(); + const { sortScope } = useSortConfig(); const { initialValues = {}, onExit, @@ -92,6 +93,8 @@ export const useLocationDetailView = ( // set up pagination const { items, nextToken, hasExhaustedSearch = false } = value; + const isCrossPageSort = sortScope === 'all'; + // sort all items before pagination for cross-page sort const { sortedItems, sortConfig, onSort, resetSort } = useSort({ items }); @@ -112,7 +115,7 @@ export const useLocationDetailView = ( highestPageVisited, pageItems, } = usePaginate({ - items: sortedItems, + items: isCrossPageSort ? sortedItems : items ?? [], onPaginate, pageSize: listOptions.pageSize, }); @@ -292,7 +295,7 @@ export const useLocationDetailView = ( setActiveFile(undefined); }, searchQuery, - sortConfig, + sortConfig: isCrossPageSort ? sortConfig : undefined, searchProgress, filePreviewState, filePreviewEnabled: !optout, @@ -363,7 +366,7 @@ export const useLocationDetailView = ( handleList({ prefix: key, options: { ...listOptions, refresh: true } }); handleReset(); }, - onSort, + onSort: isCrossPageSort ? onSort : undefined, onSearchQueryChange, onRetryFilePreview: handleRetry, onToggleSearchSubfolders, From 6dac7555d3d74b5c46d0774a9cfade31516e2632 Mon Sep 17 00:00:00 2001 From: Michael Sober Date: Tue, 21 Apr 2026 12:29:03 +0000 Subject: [PATCH 6/8] feat(storage-browser): add global sort scope that fetches all items before sorting --- .../configuration/sortConfigContext.tsx | 10 +- .../createEnhancedListHandler.spec.ts | 281 ++++++++++++++++++ .../useAction/createEnhancedListHandler.ts | 59 ++++ .../__tests__/useLocationDetailView.spec.tsx | 183 ++++++++++++ .../views/LocationDetailView/types.ts | 2 + .../useLocationDetailView.ts | 65 +++- 6 files changed, 591 insertions(+), 9 deletions(-) diff --git a/packages/react-storage/src/components/StorageBrowser/configuration/sortConfigContext.tsx b/packages/react-storage/src/components/StorageBrowser/configuration/sortConfigContext.tsx index 973bc51f4ff..c5e526702c2 100644 --- a/packages/react-storage/src/components/StorageBrowser/configuration/sortConfigContext.tsx +++ b/packages/react-storage/src/components/StorageBrowser/configuration/sortConfigContext.tsx @@ -4,14 +4,18 @@ import { createContextUtilities } from '@aws-amplify/ui-react-core'; /** * Controls where sorting is applied in the Storage Browser. * - * - `'all'` — Sort all loaded items before pagination (cross-page sort). - * Headers are handled by the view-level `useSort` hook. * - `'page'` — Sort only the current display page. Headers are handled * by the table-level local sort inside `useDataTable`. + * - `'all'` — Sort all loaded items before pagination (cross-page sort). + * Headers are handled by the view-level `useSort` hook. + * - `'global'` — Fetch ALL items from S3 (across all S3 pages, like + * search mode) before sorting. On first sort click, triggers a full + * fetch with progress reporting; subsequent sort changes reuse cached + * data. Cache invalidates on refresh or navigation. * * @default 'page' */ -export type SortScope = 'all' | 'page'; +export type SortScope = 'page' | 'all' | 'global'; export interface SortConfig { sortScope: SortScope; diff --git a/packages/react-storage/src/components/StorageBrowser/useAction/__tests__/createEnhancedListHandler.spec.ts b/packages/react-storage/src/components/StorageBrowser/useAction/__tests__/createEnhancedListHandler.spec.ts index fb8144a3dc8..95b253b33e5 100644 --- a/packages/react-storage/src/components/StorageBrowser/useAction/__tests__/createEnhancedListHandler.spec.ts +++ b/packages/react-storage/src/components/StorageBrowser/useAction/__tests__/createEnhancedListHandler.spec.ts @@ -499,4 +499,285 @@ describe('createEnhancedListHandler', () => { expect(result.items).toEqual([{ name: 'a_prefix/apple' }]); }); }); + + describe('fetchAll', () => { + it('should fetch all pages and return all items', async () => { + mockAction + .mockResolvedValueOnce({ + items: [{ name: 'a_prefix/item1' }, { name: 'a_prefix/item2' }], + nextToken: 'token1', + }) + .mockResolvedValueOnce({ + items: [{ name: 'a_prefix/item3' }], + nextToken: null, + }); + + const handler = createEnhancedListHandler(mockAction as Handler); + const prevState = { items: [], nextToken: undefined }; + + const result = await handler(prevState, { + prefix: 'a_prefix', + options: { fetchAll: {} }, + }); + + expect(mockAction).toHaveBeenCalledTimes(2); + expect(result.items).toEqual([ + { name: 'a_prefix/item1' }, + { name: 'a_prefix/item2' }, + { name: 'a_prefix/item3' }, + ]); + expect(result.nextToken).toBeUndefined(); + expect(result.hasExhaustedFetchAll).toBe(false); + }); + + it('should respect custom fetchAll limit', async () => { + const batchSize = 300; + const customLimit = 500; + const batch = Array.from({ length: batchSize }, (_, i) => ({ + name: `a_prefix/item${i}`, + })); + + mockAction + .mockResolvedValueOnce({ items: batch, nextToken: 'token1' }) + .mockResolvedValueOnce({ items: batch, nextToken: 'token2' }) + .mockResolvedValueOnce({ items: batch, nextToken: 'token3' }); + + const handler = createEnhancedListHandler(mockAction as Handler); + const prevState = { items: [], nextToken: undefined }; + + const result = await handler(prevState, { + prefix: 'a_prefix', + options: { fetchAll: { limit: customLimit } }, + }); + + expect(mockAction).toHaveBeenCalledTimes(2); + expect(result.items.length).toBe(batchSize * 2); + expect(result.hasExhaustedFetchAll).toBe(true); + }); + + it('should call onProgress after each batch', async () => { + mockAction + .mockResolvedValueOnce({ + items: [{ name: 'a_prefix/a1' }, { name: 'a_prefix/a2' }], + nextToken: 'token1', + }) + .mockResolvedValueOnce({ + items: [{ name: 'a_prefix/a3' }], + nextToken: null, + }); + + const onProgress = jest.fn(); + const handler = createEnhancedListHandler(mockAction as Handler); + const prevState = { items: [], nextToken: undefined }; + + await handler(prevState, { + prefix: 'a_prefix', + options: { fetchAll: { onProgress } }, + }); + + expect(onProgress).toHaveBeenCalledTimes(2); + expect(onProgress).toHaveBeenNthCalledWith(1, { fetchedCount: 2 }); + expect(onProgress).toHaveBeenNthCalledWith(2, { fetchedCount: 3 }); + }); + + it('should cache results and reuse on subsequent fetchAll with same prefix', async () => { + mockAction.mockResolvedValueOnce({ + items: [ + { name: 'a_prefix/item1' }, + { name: 'a_prefix/item2' }, + { name: 'a_prefix/item3' }, + ], + nextToken: null, + }); + + const handler = createEnhancedListHandler(mockAction as Handler); + const prevState = { items: [], nextToken: undefined }; + + const result1 = await handler(prevState, { + prefix: 'a_prefix', + options: { fetchAll: {} }, + }); + + expect(mockAction).toHaveBeenCalledTimes(1); + expect(result1.items).toHaveLength(3); + + const result2 = await handler(prevState, { + prefix: 'a_prefix', + options: { fetchAll: {} }, + }); + + expect(mockAction).toHaveBeenCalledTimes(1); + expect(result2.items).toEqual(result1.items); + }); + + it('should invalidate cache on refresh', async () => { + mockAction + .mockResolvedValueOnce({ + items: [{ name: 'a_prefix/item1' }], + nextToken: null, + }) + .mockResolvedValueOnce({ + items: [{ id: 1 }], + nextToken: null, + }) + .mockResolvedValueOnce({ + items: [{ name: 'a_prefix/item1' }, { name: 'a_prefix/item2' }], + nextToken: null, + }); + + const handler = createEnhancedListHandler(mockAction as Handler); + const prevState = { items: [], nextToken: undefined }; + + await handler(prevState, { + prefix: 'a_prefix', + options: { fetchAll: {} }, + }); + expect(mockAction).toHaveBeenCalledTimes(1); + + await handler(prevState, { + prefix: 'a_prefix', + options: { refresh: true }, + }); + + const result = await handler(prevState, { + prefix: 'a_prefix', + options: { fetchAll: {} }, + }); + + expect(mockAction).toHaveBeenCalledTimes(3); + expect(result.items).toEqual([ + { name: 'a_prefix/item1' }, + { name: 'a_prefix/item2' }, + ]); + }); + + it('should invalidate cache on reset', async () => { + mockAction + .mockResolvedValueOnce({ + items: [{ name: 'a_prefix/item1' }], + nextToken: null, + }) + .mockResolvedValueOnce({ + items: [{ name: 'a_prefix/item1' }, { name: 'a_prefix/item2' }], + nextToken: null, + }); + + const handler = createEnhancedListHandler(mockAction as Handler); + const prevState = { items: [], nextToken: undefined }; + + await handler(prevState, { + prefix: 'a_prefix', + options: { fetchAll: {} }, + }); + expect(mockAction).toHaveBeenCalledTimes(1); + + await handler(prevState, { + prefix: 'a_prefix', + options: { reset: true }, + }); + + const result = await handler(prevState, { + prefix: 'a_prefix', + options: { fetchAll: {} }, + }); + + expect(mockAction).toHaveBeenCalledTimes(2); + expect(result.items).toEqual([ + { name: 'a_prefix/item1' }, + { name: 'a_prefix/item2' }, + ]); + }); + + it('should invalidate cache when prefix changes', async () => { + mockAction + .mockResolvedValueOnce({ + items: [{ name: 'a/item1' }], + nextToken: null, + }) + .mockResolvedValueOnce({ + items: [{ name: 'b/item1' }, { name: 'b/item2' }], + nextToken: null, + }); + + const handler = createEnhancedListHandler(mockAction as Handler); + const prevState = { items: [], nextToken: undefined }; + + await handler(prevState, { + prefix: 'a/', + options: { fetchAll: {} }, + }); + expect(mockAction).toHaveBeenCalledTimes(1); + + const result = await handler(prevState, { + prefix: 'b/', + options: { fetchAll: {} }, + }); + + expect(mockAction).toHaveBeenCalledTimes(2); + expect(result.items).toEqual([{ name: 'b/item1' }, { name: 'b/item2' }]); + }); + + it('should invalidate cache when refresh is passed with fetchAll', async () => { + mockAction + .mockResolvedValueOnce({ + items: [{ name: 'a_prefix/item1' }], + nextToken: null, + }) + .mockResolvedValueOnce({ + items: [{ name: 'a_prefix/item1' }, { name: 'a_prefix/item2' }], + nextToken: null, + }); + + const handler = createEnhancedListHandler(mockAction as Handler); + const prevState = { items: [], nextToken: undefined }; + + await handler(prevState, { + prefix: 'a_prefix', + options: { fetchAll: {} }, + }); + expect(mockAction).toHaveBeenCalledTimes(1); + + const result = await handler(prevState, { + prefix: 'a_prefix', + options: { refresh: true, fetchAll: {} }, + }); + + expect(mockAction).toHaveBeenCalledTimes(2); + expect(result.items).toEqual([ + { name: 'a_prefix/item1' }, + { name: 'a_prefix/item2' }, + ]); + }); + + it('should stop when SEARCH_LIMIT is reached', async () => { + const mockItems = new Array(SEARCH_LIMIT).fill({ + name: 'a_prefix/item', + }); + mockAction + .mockResolvedValueOnce({ + items: mockItems.slice(0, SEARCH_LIMIT / 2), + nextToken: 'token', + }) + .mockResolvedValueOnce({ + items: mockItems.slice(SEARCH_LIMIT / 2), + nextToken: 'token2', + }) + .mockResolvedValueOnce({ + items: mockItems, + nextToken: 'token3', + }); + + const handler = createEnhancedListHandler(mockAction as Handler); + const prevState = { items: [], nextToken: undefined }; + + const result = await handler(prevState, { + prefix: 'a_prefix', + options: { fetchAll: {} }, + }); + + expect(mockAction).toHaveBeenCalledTimes(2); + expect(result.items.length).toBe(SEARCH_LIMIT); + expect(result.hasExhaustedFetchAll).toBe(true); + }); + }); }); diff --git a/packages/react-storage/src/components/StorageBrowser/useAction/createEnhancedListHandler.ts b/packages/react-storage/src/components/StorageBrowser/useAction/createEnhancedListHandler.ts index 129ff79b0bf..93772d676ca 100644 --- a/packages/react-storage/src/components/StorageBrowser/useAction/createEnhancedListHandler.ts +++ b/packages/react-storage/src/components/StorageBrowser/useAction/createEnhancedListHandler.ts @@ -13,9 +13,15 @@ import { searchItems } from './searchItems'; export const SEARCH_LIMIT = 10000; export const SEARCH_PAGE_SIZE = 1000; +export interface FetchAllOptions { + limit?: number; + onProgress?: (progress: { fetchedCount: number }) => void; +} + export type EnhancedListHandlerOptions = TOptions & { refresh?: boolean; reset?: boolean; + fetchAll?: FetchAllOptions; search?: SearchOptions & { limit?: number; onProgress?: (progress: { fetchedCount: number }) => void; @@ -32,6 +38,7 @@ export type EnhancedListHandlerInput< export interface EnhancedListHandlerOutput extends ListHandlerOutput { hasExhaustedSearch?: boolean; + hasExhaustedFetchAll?: boolean; } export interface EnhancedListHandler< @@ -54,20 +61,71 @@ export const createEnhancedListHandler = < hasExhaustedSearch: boolean; } | null = null; + let fetchAllCache: { + prefix: string; + items: TItem[]; + hasExhaustedFetchAll: boolean; + } | null = null; + return async function listActionHandler(prevState, { options, ...input }) { const { nextToken: _nextToken, refresh, reset, + fetchAll, search, ...rest } = options ?? {}; if (reset) { searchCache = null; + fetchAllCache = null; return { items: [], nextToken: undefined }; } + // fetch all items across all S3 pages (for global sort) + if (fetchAll) { + const { limit: fetchLimit, onProgress } = fetchAll; + + if (fetchAllCache && fetchAllCache.prefix === input.prefix && !refresh) { + return { + items: fetchAllCache.items, + hasExhaustedFetchAll: fetchAllCache.hasExhaustedFetchAll, + nextToken: undefined, + }; + } + + const items: TItem[] = []; + let nextToken = undefined; + const limit = fetchLimit ?? SEARCH_LIMIT; + do { + const output = await handler({ + ...input, + options: { ...rest, pageSize: SEARCH_PAGE_SIZE, nextToken }, + } as TInput); + + items.push(...output.items); + // eslint-disable-next-line prefer-destructuring + nextToken = output.nextToken; + + onProgress?.({ fetchedCount: items.length }); + } while (nextToken && items.length < limit); + + const hasExhaustedFetchAll = !!nextToken; + + fetchAllCache = { + prefix: input.prefix, + items, + hasExhaustedFetchAll, + }; + + return { + items, + hasExhaustedFetchAll, + nextToken: undefined, + }; + } + // collect and filter results on `search` if (search) { const { limit: searchLimit, onProgress, ...searchOptions } = search; @@ -117,6 +175,7 @@ export const createEnhancedListHandler = < if (refresh) { searchCache = null; + fetchAllCache = null; } // ignore provided `nextToken` on `refresh` diff --git a/packages/react-storage/src/components/StorageBrowser/views/LocationDetailView/__tests__/useLocationDetailView.spec.tsx b/packages/react-storage/src/components/StorageBrowser/views/LocationDetailView/__tests__/useLocationDetailView.spec.tsx index d3a549f2776..cc012c1531d 100644 --- a/packages/react-storage/src/components/StorageBrowser/views/LocationDetailView/__tests__/useLocationDetailView.spec.tsx +++ b/packages/react-storage/src/components/StorageBrowser/views/LocationDetailView/__tests__/useLocationDetailView.spec.tsx @@ -1273,4 +1273,187 @@ describe('useLocationDetailView', () => { expect((firstPageItems[1] as FileData).size).toBe(300); }); }); + + describe('sortScope "global"', () => { + const mockUseSortConfig = jest.mocked(useSortConfig); + + const sortableItems: LocationItemData[] = [ + folderDataOne, + { + ...fileDataOne, + key: 'some-prefix/charlie.jpg', + size: 300, + lastModified: new Date('2024-03-15'), + }, + { + ...fileDataTwo, + key: 'some-prefix/alpha.png', + size: 1000, + lastModified: new Date('2024-01-01'), + }, + { + ...fileDataThree, + key: 'some-prefix/beta.doc', + size: 500, + lastModified: new Date('2024-06-20'), + }, + ]; + + beforeEach(() => { + mockUseSortConfig.mockReturnValue({ sortScope: 'global' }); + mockUseList.mockReturnValue([ + { + value: { items: sortableItems, nextToken: undefined }, + message: '', + hasError: false, + isLoading: false, + }, + mockHandleList, + ]); + }); + + afterEach(() => { + mockUseSortConfig.mockReturnValue({ sortScope: 'page' }); + }); + + it('should return onSort and sortConfig when sortScope is "global"', () => { + const { result } = renderHook(() => useLocationDetailView()); + + expect(result.current.onSort).toEqual(expect.any(Function)); + expect(result.current.sortConfig).toBeUndefined(); + }); + + it('should trigger fetchAll on first sort click', () => { + const { result } = renderHook(() => useLocationDetailView()); + + act(() => { + result.current.onSort!('name'); + }); + + expect(result.current.sortConfig).toEqual({ + field: 'name', + direction: 'ascending', + }); + + // handleList should be called with fetchAll option (beyond initial mount call) + expect(mockHandleList).toHaveBeenCalledWith( + expect.objectContaining({ + options: expect.objectContaining({ + fetchAll: expect.objectContaining({ + onProgress: expect.any(Function), + }), + }), + }) + ); + }); + + it('should not trigger fetchAll on subsequent sort clicks', () => { + const { result } = renderHook(() => useLocationDetailView()); + + act(() => { + result.current.onSort!('name'); + }); + + const callCountAfterFirst = mockHandleList.mock.calls.length; + + act(() => { + result.current.onSort!('size'); + }); + + // no additional handleList calls for sort (only direction/field changed) + expect(mockHandleList.mock.calls.length).toBe(callCountAfterFirst); + }); + + it('should sort items across pages', () => { + const { result } = renderHook(() => + useLocationDetailView({ pageSize: 2 }) + ); + + act(() => { + result.current.onSort!('size'); + }); + + expect(result.current.sortConfig).toEqual({ + field: 'size', + direction: 'ascending', + }); + + // folders first, then smallest file + const firstPageItems = result.current.pageItems; + expect(firstPageItems).toHaveLength(2); + expect(firstPageItems[0].type).toBe('FOLDER'); + expect((firstPageItems[1] as FileData).size).toBe(300); + }); + + it('should reset global sort state on refresh', () => { + const { result } = renderHook(() => useLocationDetailView()); + + act(() => { + result.current.onSort!('name'); + }); + expect(result.current.sortConfig).toBeDefined(); + + act(() => { + result.current.onRefresh(); + }); + expect(result.current.sortConfig).toBeUndefined(); + + // after refresh, next sort click should trigger fetchAll again + mockHandleList.mockClear(); + + act(() => { + result.current.onSort!('name'); + }); + + expect(mockHandleList).toHaveBeenCalledWith( + expect.objectContaining({ + options: expect.objectContaining({ + fetchAll: expect.objectContaining({ + onProgress: expect.any(Function), + }), + }), + }) + ); + }); + + it('should reset global sort state on navigation', () => { + const { result } = renderHook(() => useLocationDetailView()); + + act(() => { + result.current.onSort!('name'); + }); + expect(result.current.sortConfig).toBeDefined(); + + act(() => { + result.current.onNavigate(testLocation.current!); + }); + expect(result.current.sortConfig).toBeUndefined(); + }); + + it('should expose sortFetchProgress', () => { + const { result } = renderHook(() => useLocationDetailView()); + + expect(result.current.sortFetchProgress).toBeNull(); + }); + + it('should expose hasExhaustedFetchAll', () => { + mockUseList.mockReturnValue([ + { + value: { + items: sortableItems, + nextToken: undefined, + hasExhaustedFetchAll: true, + }, + message: '', + hasError: false, + isLoading: false, + }, + mockHandleList, + ]); + + const { result } = renderHook(() => useLocationDetailView()); + + expect(result.current.hasExhaustedFetchAll).toBe(true); + }); + }); }); diff --git a/packages/react-storage/src/components/StorageBrowser/views/LocationDetailView/types.ts b/packages/react-storage/src/components/StorageBrowser/views/LocationDetailView/types.ts index 237097354a3..51da1f36a12 100644 --- a/packages/react-storage/src/components/StorageBrowser/views/LocationDetailView/types.ts +++ b/packages/react-storage/src/components/StorageBrowser/views/LocationDetailView/types.ts @@ -32,6 +32,7 @@ export interface LocationDetailViewState { hasDownloadError: boolean; hasError: boolean; hasExhaustedSearch: boolean; + hasExhaustedFetchAll: boolean; hasNextPage: boolean; highestPageVisited: number; isLoading: boolean; @@ -60,6 +61,7 @@ export interface LocationDetailViewState { searchQuery: string; sortConfig: SortConfig | undefined; searchProgress: { fetchedCount: number } | null; + sortFetchProgress: { fetchedCount: number } | null; filePreviewState: FilePreviewState; filePreviewEnabled: boolean; } 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 965f1c2d068..a62ebb40f65 100644 --- a/packages/react-storage/src/components/StorageBrowser/views/LocationDetailView/useLocationDetailView.ts +++ b/packages/react-storage/src/components/StorageBrowser/views/LocationDetailView/useLocationDetailView.ts @@ -28,6 +28,7 @@ import type { LocationDetailViewState, UseLocationDetailViewOptions, } from './types'; +import type { HeaderKeys } from './getLocationDetailViewTableData/types'; import { useFilePreview } from '../hooks/useFilePreview'; const DEFAULT_PAGE_SIZE = 100; @@ -91,12 +92,55 @@ export const useLocationDetailView = ( useList('locationItems'); // set up pagination - const { items, nextToken, hasExhaustedSearch = false } = value; - - const isCrossPageSort = sortScope === 'all'; + const { + items, + nextToken, + hasExhaustedSearch = false, + hasExhaustedFetchAll = false, + } = value; + + const isCrossPageSort = sortScope === 'all' || sortScope === 'global'; + const isGlobalSort = sortScope === 'global'; + + // track whether we've already fetched all items for global sort + const hasFetchedAllForSort = React.useRef(false); + const [sortFetchProgress, setSortFetchProgress] = React.useState<{ + fetchedCount: number; + } | null>(null); // sort all items before pagination for cross-page sort - const { sortedItems, sortConfig, onSort, resetSort } = useSort({ items }); + const { + sortedItems, + sortConfig, + onSort: onSortBase, + resetSort, + } = useSort({ + items, + }); + + const onSortWithGlobalFetch = React.useCallback( + (headerKey: HeaderKeys) => { + onSortBase(headerKey); + + if (!isGlobalSort || hasFetchedAllForSort.current || hasInvalidPrefix) { + return; + } + + hasFetchedAllForSort.current = true; + setSortFetchProgress({ fetchedCount: 0 }); + handleList({ + prefix: key, + options: { + ...listOptions, + fetchAll: { + onProgress: (progress: { fetchedCount: number }) => + setSortFetchProgress(progress), + }, + }, + }); + }, + [onSortBase, isGlobalSort, hasInvalidPrefix, handleList, key, listOptions] + ); const onPaginate = () => { if (hasInvalidPrefix || !nextToken) return; @@ -221,6 +265,8 @@ export const useLocationDetailView = ( handleReset(); resetSearch(); resetSort(); + hasFetchedAllForSort.current = false; + setSortFetchProgress(null); handleList({ prefix: key, options: { ...listOptions, refresh: true }, @@ -244,7 +290,10 @@ export const useLocationDetailView = ( if (!isLoading && searchProgress) { setSearchProgress(null); } - }, [isLoading, searchProgress]); + if (!isLoading && sortFetchProgress) { + setSortFetchProgress(null); + } + }, [isLoading, searchProgress, sortFetchProgress]); const { actionConfigs } = useActionConfigs(); @@ -297,9 +346,11 @@ export const useLocationDetailView = ( searchQuery, sortConfig: isCrossPageSort ? sortConfig : undefined, searchProgress, + sortFetchProgress, filePreviewState, filePreviewEnabled: !optout, hasExhaustedSearch, + hasExhaustedFetchAll, onRefresh, onActionExit: () => { storeDispatch({ type: 'RESET_ACTION_TYPE' }); @@ -315,6 +366,8 @@ export const useLocationDetailView = ( onNavigate?.(location, path); resetSearch(); resetSort(); + hasFetchedAllForSort.current = false; + setSortFetchProgress(null); storeDispatch({ type: 'CHANGE_LOCATION', location, path }); locationItemsDispatch({ type: 'RESET_LOCATION_ITEMS' }); setActiveFile(undefined); @@ -366,7 +419,7 @@ export const useLocationDetailView = ( handleList({ prefix: key, options: { ...listOptions, refresh: true } }); handleReset(); }, - onSort: isCrossPageSort ? onSort : undefined, + onSort: isCrossPageSort ? onSortWithGlobalFetch : undefined, onSearchQueryChange, onRetryFilePreview: handleRetry, onToggleSearchSubfolders, From 394a62182cbc1c761a4148f10a9839331836152c Mon Sep 17 00:00:00 2001 From: Michael Sober Date: Tue, 21 Apr 2026 13:13:04 +0000 Subject: [PATCH 7/8] fix(storage-browser): fix sortScope behavior for page, all, and global modes --- .../useLocationDetailView.ts | 31 +++++++++++++------ 1 file changed, 22 insertions(+), 9 deletions(-) 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 a62ebb40f65..7531b0fa94e 100644 --- a/packages/react-storage/src/components/StorageBrowser/views/LocationDetailView/useLocationDetailView.ts +++ b/packages/react-storage/src/components/StorageBrowser/views/LocationDetailView/useLocationDetailView.ts @@ -32,6 +32,7 @@ import type { HeaderKeys } from './getLocationDetailViewTableData/types'; import { useFilePreview } from '../hooks/useFilePreview'; const DEFAULT_PAGE_SIZE = 100; +const CROSS_PAGE_SORT_FETCH_SIZE = 1000; // Default options for tests export const DEFAULT_LIST_OPTIONS = { @@ -63,14 +64,24 @@ export const useLocationDetailView = ( const pageSize = propPageSize ?? configPageSize; + const isCrossPageSort = sortScope === 'all' || sortScope === 'global'; + const isGlobalSort = sortScope === 'global'; + + // For cross-page sort modes, fetch a larger batch from S3 so there are + // multiple display pages to sort across. The display page size stays at + // `pageSize` (the user-configured value). + const fetchPageSize = isCrossPageSort + ? Math.max(pageSize, CROSS_PAGE_SORT_FETCH_SIZE) + : pageSize; + const listOptions = React.useMemo( () => ({ ...initialValues, delimiter: '/', - pageSize, + pageSize: fetchPageSize, }), // eslint-disable-next-line react-hooks/exhaustive-deps - [pageSize, initialValues.delimiter, initialValues.pageSize] + [fetchPageSize, initialValues.delimiter, initialValues.pageSize] ); const [{ location, actionType }, storeDispatch] = useStore(); @@ -99,10 +110,6 @@ export const useLocationDetailView = ( hasExhaustedFetchAll = false, } = value; - const isCrossPageSort = sortScope === 'all' || sortScope === 'global'; - const isGlobalSort = sortScope === 'global'; - - // track whether we've already fetched all items for global sort const hasFetchedAllForSort = React.useRef(false); const [sortFetchProgress, setSortFetchProgress] = React.useState<{ fetchedCount: number; @@ -142,6 +149,10 @@ export const useLocationDetailView = ( [onSortBase, isGlobalSort, hasInvalidPrefix, handleList, key, listOptions] ); + // For 'page' mode, each S3 page IS the display page, so fetch the next + // S3 batch when the user paginates beyond what's loaded. + // For cross-page sort modes, all sorting + pagination is local; S3 + // fetches are not needed on page change. const onPaginate = () => { if (hasInvalidPrefix || !nextToken) return; locationItemsDispatch({ type: 'RESET_LOCATION_ITEMS' }); @@ -160,8 +171,8 @@ export const useLocationDetailView = ( pageItems, } = usePaginate({ items: isCrossPageSort ? sortedItems : items ?? [], - onPaginate, - pageSize: listOptions.pageSize, + onPaginate: isCrossPageSort ? undefined : onPaginate, + pageSize, }); const onSearch = (query: string, includeSubfolders?: boolean) => { @@ -333,7 +344,9 @@ export const useLocationDetailView = ( fileDataItems, hasError, hasDownloadError: task?.status === 'FAILED', - hasNextPage: !!nextToken || hasNextLocalPage, + hasNextPage: isCrossPageSort + ? hasNextLocalPage + : !!nextToken || hasNextLocalPage, highestPageVisited, message, downloadErrorMessage: getDownloadErrorMessageFromFailedDownloadTask(task), From a6d4611913ee263b8cfc17309febee463679900f Mon Sep 17 00:00:00 2001 From: Michael Sober Date: Tue, 21 Apr 2026 14:19:48 +0000 Subject: [PATCH 8/8] fix(storage-browser): correct sortScope - remove fetch size separation, scope controls sort location only --- .../useLocationDetailView.ts | 23 ++++++------------- 1 file changed, 7 insertions(+), 16 deletions(-) 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 7531b0fa94e..2e8240af8ce 100644 --- a/packages/react-storage/src/components/StorageBrowser/views/LocationDetailView/useLocationDetailView.ts +++ b/packages/react-storage/src/components/StorageBrowser/views/LocationDetailView/useLocationDetailView.ts @@ -32,7 +32,6 @@ import type { HeaderKeys } from './getLocationDetailViewTableData/types'; import { useFilePreview } from '../hooks/useFilePreview'; const DEFAULT_PAGE_SIZE = 100; -const CROSS_PAGE_SORT_FETCH_SIZE = 1000; // Default options for tests export const DEFAULT_LIST_OPTIONS = { @@ -67,21 +66,14 @@ export const useLocationDetailView = ( const isCrossPageSort = sortScope === 'all' || sortScope === 'global'; const isGlobalSort = sortScope === 'global'; - // For cross-page sort modes, fetch a larger batch from S3 so there are - // multiple display pages to sort across. The display page size stays at - // `pageSize` (the user-configured value). - const fetchPageSize = isCrossPageSort - ? Math.max(pageSize, CROSS_PAGE_SORT_FETCH_SIZE) - : pageSize; - const listOptions = React.useMemo( () => ({ ...initialValues, delimiter: '/', - pageSize: fetchPageSize, + pageSize, }), // eslint-disable-next-line react-hooks/exhaustive-deps - [fetchPageSize, initialValues.delimiter, initialValues.pageSize] + [pageSize, initialValues.delimiter, initialValues.pageSize] ); const [{ location, actionType }, storeDispatch] = useStore(); @@ -149,10 +141,9 @@ export const useLocationDetailView = ( [onSortBase, isGlobalSort, hasInvalidPrefix, handleList, key, listOptions] ); - // For 'page' mode, each S3 page IS the display page, so fetch the next - // S3 batch when the user paginates beyond what's loaded. - // For cross-page sort modes, all sorting + pagination is local; S3 - // fetches are not needed on page change. + // For 'page' and 'all' modes, fetch the next S3 batch when the user + // paginates beyond what's loaded. For 'global' mode, fetchAll retrieves + // everything upfront so S3 fetches on page change are not needed. const onPaginate = () => { if (hasInvalidPrefix || !nextToken) return; locationItemsDispatch({ type: 'RESET_LOCATION_ITEMS' }); @@ -171,7 +162,7 @@ export const useLocationDetailView = ( pageItems, } = usePaginate({ items: isCrossPageSort ? sortedItems : items ?? [], - onPaginate: isCrossPageSort ? undefined : onPaginate, + onPaginate: isGlobalSort ? undefined : onPaginate, pageSize, }); @@ -344,7 +335,7 @@ export const useLocationDetailView = ( fileDataItems, hasError, hasDownloadError: task?.status === 'FAILED', - hasNextPage: isCrossPageSort + hasNextPage: isGlobalSort ? hasNextLocalPage : !!nextToken || hasNextLocalPage, highestPageVisited,