From b0ea0a0738d813ee3eb3bde7ddf20832b6aa236f Mon Sep 17 00:00:00 2001 From: Devon Govett Date: Wed, 6 May 2020 20:02:49 -0700 Subject: [PATCH 1/4] useAsyncList refactor --- packages/@react-stately/data/src/index.ts | 3 +- .../@react-stately/data/src/useAsyncList.ts | 265 ++++++- .../@react-stately/data/src/useListData.ts | 142 ++-- .../data/stories/AsyncHook.stories.tsx | 109 --- .../data/test/useAsyncList.test.js | 655 ++++++++++++++++-- 5 files changed, 930 insertions(+), 244 deletions(-) delete mode 100644 packages/@react-stately/data/stories/AsyncHook.stories.tsx diff --git a/packages/@react-stately/data/src/index.ts b/packages/@react-stately/data/src/index.ts index 7811bd8108a..c594b1fbbce 100644 --- a/packages/@react-stately/data/src/index.ts +++ b/packages/@react-stately/data/src/index.ts @@ -12,5 +12,4 @@ export * from './useAsyncList'; export * from './useTreeData'; -export * from './useListData'; - +export {useListData} from './useListData'; diff --git a/packages/@react-stately/data/src/useAsyncList.ts b/packages/@react-stately/data/src/useAsyncList.ts index ed7590f53a4..ae150646bc5 100644 --- a/packages/@react-stately/data/src/useAsyncList.ts +++ b/packages/@react-stately/data/src/useAsyncList.ts @@ -10,60 +10,249 @@ * governing permissions and limitations under the License. */ -import {AsyncListOptions, AsyncListProps, SortDescriptor} from '@react-types/shared'; -import {useEffect, useReducer} from 'react'; +import {SortDescriptor} from '@react-types/shared'; +import {useEffect, useReducer, Key, Reducer} from 'react'; +import { ListData, useListData, createListActions, ListState } from './useListData'; -function reducer(state, {type, ...rest}) { - switch (type) { - case 'fetching': - case 'sorting': - return {...state, ...rest, isLoading: true}; +interface AsyncListOptions { + initialSelectedKeys?: Iterable, + initialSortDescriptor?: SortDescriptor, + getKey?: (item: T) => Key, + load: AsyncListLoadFunction, + sort?: AsyncListLoadFunction +} + +type AsyncListLoadFunction = (state: AsyncListLoadOptions) => Promise>; +interface AsyncListLoadOptions { + items: T[], + selectedKeys: Set, + sortDescriptor: SortDescriptor, + signal: AbortSignal, + cursor?: C +} + +interface AsyncListStateUpdate { + items: Iterable, + selectedKeys?: Iterable, + sortDescriptor?: SortDescriptor, + cursor?: C +} + +interface AsyncListState extends ListState { + state: 'loading' | 'sorting' | 'loadingMore' | 'error' | 'idle' + items: T[], + // disabledKeys?: Iterable, + selectedKeys: Set, + // selectedKey?: Key, + // expandedKeys?: Iterable, + sortDescriptor?: SortDescriptor, + error?: Error, + abortController?: AbortController, + cursor?: C +} + +type ActionType = 'success' | 'error' | 'loading' | 'loadingMore' | 'sorting' | 'update'; +interface Action { + type: ActionType, + items?: Iterable, + selectedKeys?: Iterable, + sortDescriptor?: SortDescriptor, + error?: Error, + abortController?: AbortController, + updater?: (state: ListState) => ListState, + cursor?: C +} + +interface AsyncListData extends ListData { + isLoading: boolean, + error?: Error, + // disabledKeys?: Set, + // selectedKey?: Key, + // expandedKeys?: Set, + sortDescriptor?: SortDescriptor, + + reload(): void, + loadMore(): void, + sort(desc: SortDescriptor): void +} + +function reducer(data: AsyncListState, action: Action): AsyncListState { + switch (data.state) { + case 'idle': case 'error': - return {...state, ...rest, isLoading: false}; - case 'success': - return {...state, ...rest, isLoading: false}; - default: throw new Error(); + switch (action.type) { + case 'loading': + case 'loadingMore': + case 'sorting': + return { + ...data, + state: action.type, + // Reset items to an empty list if loading, but not when sorting. + items: action.type === 'loading' ? [] : data.items, + sortDescriptor: action.sortDescriptor ?? data.sortDescriptor, + abortController: action.abortController + }; + case 'update': + return { + ...data, + ...action.updater(data) + }; + case 'success': + case 'error': + return data; + default: + throw new Error(`Invalid action "${action.type}" in state "${data.state}"`); + } + case 'loading': + case 'sorting': + switch (action.type) { + case 'success': + // Ignore if there is a newer abortcontroller in state. + // This means that multiple requests were going at once. + // We want to take only the latest result. + if (action.abortController !== data.abortController) { + return data; + } + + return { + ...data, + state: 'idle', + items: [...action.items], + selectedKeys: new Set(action.selectedKeys ?? data.selectedKeys), + sortDescriptor: action.sortDescriptor ?? data.sortDescriptor, + abortController: null, + cursor: action.cursor + }; + case 'error': + if (action.abortController !== data.abortController) { + return data; + } + + return { + ...data, + state: 'error', + error: action.error, + abortController: null + }; + case 'loading': + case 'loadingMore': + case 'sorting': + // We're already loading, and another load was triggered at the same time. + // We need to abort the previous load and start a new one. + data.abortController.abort(); + return { + ...data, + state: action.type, + // Reset items to an empty list if loading, but not when sorting. + items: action.type === 'loading' ? [] : data.items, + abortController: action.abortController + }; + default: + throw new Error(`Invalid action "${action.type}" in state "${data.state}"`); + } + case 'loadingMore': + switch (action.type) { + case 'success': + // Append the new items + return { + ...data, + state: 'idle', + items: [...data.items, ...action.items], + selectedKeys: new Set(action.selectedKeys ?? data.selectedKeys), + sortDescriptor: action.sortDescriptor ?? data.sortDescriptor, + abortController: null, + cursor: action.cursor + }; + case 'error': + return { + ...data, + state: 'error', + error: action.error + }; + case 'loading': + case 'sorting': + // We're already loading more, and another load was triggered at the same time. + // We need to abort the previous load more and start a new one. + data.abortController.abort(); + return { + ...data, + state: 'loading', + // Reset items to an empty list if loading, but not when sorting. + items: action.type === 'loading' ? [] : data.items, + abortController: action.abortController + }; + default: + throw new Error(`Invalid action "${action.type}" in state "${data.state}"`); + } + default: + throw new Error(`Invalid state "${data.state}"`); } } -export function useAsyncList(options: AsyncListOptions): AsyncListProps { - const {load, loadMore, sort, defaultSortDescriptor} = options; +export function useAsyncList(options: AsyncListOptions): AsyncListData { + const { + load, + sort, + initialSelectedKeys, + initialSortDescriptor, + getKey = (item: any) => item.id || item.key + } = options; - let [state, dispatch] = useReducer(reducer, { + let [data, dispatch] = useReducer, Action>>(reducer, { + state: 'idle', error: null, - isLoading: true, - items: [] as Iterable, - sortDescriptor: defaultSortDescriptor || null, - fetchFunction: load + items: [], + selectedKeys: new Set(initialSelectedKeys), + sortDescriptor: initialSortDescriptor }); - const fetchData = async fn => { + const dispatchFetch = async (action: Action, fn: AsyncListLoadFunction) => { + let abortController = new AbortController(); try { - let response = await fn({...state}); - dispatch({type: 'success', ...response}); + dispatch({...action, abortController}); + + let response = await fn({ + items: data.items, + selectedKeys: data.selectedKeys, + sortDescriptor: action.sortDescriptor ?? data.sortDescriptor, + signal: abortController.signal, + cursor: action.type === 'loadingMore' ? data.cursor : null + }); + dispatch({type: 'success', ...response, abortController}); } catch (e) { - dispatch({type: 'error', error: e}); + dispatch({type: 'error', error: e, abortController}); } }; - const onLoadMore = !loadMore ? null : (() => { - dispatch({type: 'fetching', fetchFunction: loadMore}); - }); - - const onSortChange = (desc: SortDescriptor) => { - dispatch({type: 'sorting', fetchFunction: sort || load, sortDescriptor: desc}); - }; - - let {fetchFunction, ...otherState} = state; useEffect(() => { - if (state.isLoading) { - fetchData(fetchFunction); - } - }); + dispatchFetch({type: 'loading'}, load); + }, []); return { - ...otherState, - onLoadMore, - onSortChange + items: data.items, + selectedKeys: data.selectedKeys, + sortDescriptor: data.sortDescriptor, + isLoading: data.state === 'loading' || data.state === 'loadingMore' || data.state === 'sorting', + error: data.error, + getItem(key: Key) { + return data.items.find(item => getKey(item) === key); + }, + reload() { + dispatchFetch({type: 'loading'}, load); + }, + loadMore() { + // Ignore if already loading more. + if (data.state === 'loadingMore' || data.cursor == null) { + return; + } + + dispatchFetch({type: 'loadingMore'}, load); + }, + sort(sortDescriptor: SortDescriptor) { + dispatchFetch({type: 'sorting', sortDescriptor}, sort || load); + }, + ...createListActions(options, fn => { + dispatch({type: 'update', updater: fn}) + }) }; } diff --git a/packages/@react-stately/data/src/useListData.ts b/packages/@react-stately/data/src/useListData.ts index 8bd2dab4a26..d983846eed3 100644 --- a/packages/@react-stately/data/src/useListData.ts +++ b/packages/@react-stately/data/src/useListData.ts @@ -12,13 +12,13 @@ import {Key, useState} from 'react'; -interface ListOptions { +interface ListOptions { initialItems?: T[], initialSelectedKeys?: Iterable, getKey?: (item: T) => Key } -interface ListData { +export interface ListData { /** The items in the list. */ items: T[], @@ -94,94 +94,142 @@ interface ListData { update(key: Key, newValue: T): void } +export interface ListState { + items: T[], + selectedKeys: Set +} + /** * Manages state for an immutable list data structure, and provides convenience methods to * update the data over time. */ -export function useListData(opts: ListOptions): ListData { +export function useListData(opts: ListOptions): ListData { let { initialItems = [], initialSelectedKeys, getKey = (item: any) => item.id || item.key } = opts; - let [items, setItems] = useState(initialItems); - let [selectedKeys, setSelectedKeys] = useState(new Set(initialSelectedKeys || [])); + let [state, setState] = useState>({ + items: initialItems, + selectedKeys: new Set(initialSelectedKeys || []) + }); return { - items, - selectedKeys, - setSelectedKeys, + ...state, + ...createListActions({getKey}, setState), getItem(key: Key) { - return items.find(item => getKey(item) === key); + return state.items.find(item => getKey(item) === key); + } + }; +} + +function insert(state: ListState, index: number, ...values: T[]): ListState { + return { + ...state, + items: [ + ...state.items.slice(0, index), + ...values, + ...state.items.slice(index) + ] + }; +} + +export function createListActions(opts: ListOptions, dispatch: (updater: (state: ListState) => ListState) => void): Omit, 'items' | 'selectedKeys' | 'getItem'> { + let {getKey} = opts; + return { + setSelectedKeys(selectedKeys: Set) { + dispatch(state => ({ + ...state, + selectedKeys + })); }, insert(index: number, ...values: T[]) { - setItems(items => [ - ...items.slice(0, index), - ...values, - ...items.slice(index) - ]); + dispatch(state => insert(state, index, ...values)); }, insertBefore(key: Key, ...values: T[]) { - let index = items.findIndex(item => getKey(item) === key); - if (index === -1) { - return; - } + dispatch(state => { + let index = state.items.findIndex(item => getKey(item) === key); + if (index === -1) { + return; + } - this.insert(index, ...values); + return insert(state, index, ...values); + }); }, insertAfter(key: Key, ...values: T[]) { - let index = items.findIndex(item => getKey(item) === key); - if (index === -1) { - return; - } + dispatch(state => { + let index = state.items.findIndex(item => getKey(item) === key); + if (index === -1) { + return; + } - this.insert(index + 1, ...values); + return insert(state, index + 1, ...values); + }); }, prepend(...values: T[]) { - this.insert(0, ...values); + dispatch(state => insert(state, 0, ...values)); }, append(...values: T[]) { - this.insert(items.length, ...values); + dispatch(state => insert(state, state.items.length, ...values)); }, remove(...keys: Key[]) { - let keySet = new Set(keys); - setItems(items => items.filter(item => !keySet.has(getKey(item)))); + dispatch(state => { + let keySet = new Set(keys); + let items = state.items.filter(item => !keySet.has(getKey(item))); - let selection = new Set(selectedKeys); - for (let key of keys) { - selection.delete(key); - } + let selection = new Set(state.selectedKeys); + for (let key of keys) { + selection.delete(key); + } - setSelectedKeys(selection); + return { + ...state, + items, + selectedKeys: selection + }; + }); }, removeSelectedItems() { - this.remove(...selectedKeys); + dispatch(state => { + let items = state.items.filter(item => !state.selectedKeys.has(getKey(item))); + return { + ...state, + items, + selectedKeys: new Set() + } + }); }, move(key: Key, toIndex: number) { - setItems(items => { - let index = items.findIndex(item => getKey(item) === key); + dispatch(state => { + let index = state.items.findIndex(item => getKey(item) === key); if (index === -1) { - return items; + return state; } - let copy = items.slice(); + let copy = state.items.slice(); let [item] = copy.splice(index, 1); copy.splice(toIndex, 0, item); - return copy; + return { + ...state, + items: copy + }; }); }, update(key: Key, newValue: T) { - setItems(items => { - let index = items.findIndex(item => getKey(item) === key); + dispatch(state => { + let index = state.items.findIndex(item => getKey(item) === key); if (index === -1) { - return items; + return state; } - return [ - ...items.slice(0, index), - newValue, - ...items.slice(index + 1) - ]; + return { + ...state, + items: [ + ...state.items.slice(0, index), + newValue, + ...state.items.slice(index + 1) + ] + }; }); } }; diff --git a/packages/@react-stately/data/stories/AsyncHook.stories.tsx b/packages/@react-stately/data/stories/AsyncHook.stories.tsx deleted file mode 100644 index 72ddffdff5b..00000000000 --- a/packages/@react-stately/data/stories/AsyncHook.stories.tsx +++ /dev/null @@ -1,109 +0,0 @@ -/* - * Copyright 2020 Adobe. All rights reserved. - * This file is licensed to you under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. You may obtain a copy - * of the License at http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software distributed under - * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS - * OF ANY KIND, either express or implied. See the License for the specific language - * governing permissions and limitations under the License. - */ - -import {ActionButton} from '@react-spectrum/button'; -import {Item} from '@react-stately/collections'; -import {ProgressCircle} from '@react-spectrum/progress'; -import React, {useEffect} from 'react'; -// import {storiesOf} from '@storybook/react'; -import {Tree} from '@react-spectrum/tree'; -import {useAsyncList} from '../src'; - -/* - these are broken, commenting out for now -storiesOf('useAsyncList', module) - .add( - 'loadMore support', - () => ( { - let res = await retrieveMore(); - return {items: [...items, ...res]}; - }} />) - ) - .add( - 'sort support', - () => ( - new Promise(resolve => { - setTimeout(() => { - let i = items.sort(); - resolve({items: i.reverse()}); - }, 500); - })} /> - ) - ); - */ - -interface IItem { - name: string, - key: string -} -let counter = 1; - -// eslint-disable-next-line @typescript-eslint/no-unused-vars -function Component(props) { - let {isLoading, items, onLoadMore, onSortChange, sortDescriptor} = useAsyncList({ - load: async () => { - let res = await retrieve(); - return {items: res}; - }, - ...props - }); - - useEffect(() => {counter = 1;}, []); - return ( -
- - {({name}) => {name}} - - {isLoading && } - {!isLoading && props.loadMore && - - Load More - - } - {!isLoading && props.sort && - onSortChange({direction: sortDescriptor.direction === 1 ? 0 : 1})}> - {sortDescriptor.direction === 1 ? 'DESC' : 'ASC'} - - } -
- ); -} - -async function retrieve() { - return new Promise((resolve) => { - setTimeout(() => { - let items = []; - for (let i = 0; i < 30; i++) { - items.push({name: `Item ${counter++}`, key: counter}); - } - resolve(items); - }, 1000); - }); -} - -// eslint-disable-next-line @typescript-eslint/no-unused-vars -async function retrieveMore() { - return new Promise((resolve) => { - setTimeout(() => { - let items = []; - for (let i = 0; i < 30; i++) { - items.push({name: `Item ${counter++}`, key: counter}); - } - resolve(items); - }, 1500); - }); -} diff --git a/packages/@react-stately/data/test/useAsyncList.test.js b/packages/@react-stately/data/test/useAsyncList.test.js index 56b5e1a31a9..008c20f045e 100644 --- a/packages/@react-stately/data/test/useAsyncList.test.js +++ b/packages/@react-stately/data/test/useAsyncList.test.js @@ -15,98 +15,179 @@ import React from 'react'; import {useAsyncList} from '../src'; const ITEMS = [{id: 1, name: '1'}, {id: 2, name: '2'}]; -describe('useAsyncList', () => { - let loadSpy = jest.fn().mockImplementation(() => ({items: ITEMS})); - let loadMoreSpy = jest.fn().mockImplementation(({items}) => ({items: [...items, ...ITEMS]})); - let sortSpy = jest.fn(); +const ITEMS2 = [{id: 2, name: '1'}, {id: 1, name: '2'}]; + +function getItems() { + return new Promise(resolve => { + setTimeout(() => resolve({items: ITEMS, cursor: 3}), 100); + }); +} - afterEach(() => { - loadMoreSpy.mockClear(); - loadSpy.mockClear(); - sortSpy.mockClear(); +function getItems2() { + return new Promise(resolve => { + setTimeout(() => resolve({items: ITEMS2, cursor: 0}), 100); + }); +} + +describe('useAsyncList', () => { + beforeAll(() => { + jest.useFakeTimers(); }); - it('will call load function on init', async () => { - let {result, waitForNextUpdate} = renderHook(() => useAsyncList({load: loadSpy})); + function flushPromises() { + return new Promise(function(resolve) { + setImmediate(resolve); + }); + } + + it('should call load function on init', async () => { + let load = jest.fn().mockImplementation(getItems); + let {result, waitForNextUpdate} = renderHook(() => useAsyncList({load})); + + expect(load).toHaveBeenCalledTimes(1); + let args = load.mock.calls[0][0]; + expect(args.items).toEqual([]); + expect(args.selectedKeys).toEqual(new Set()); + + expect(result.current.isLoading).toBe(true); + expect(result.current.items).toEqual([]); + await act(async () => { + jest.runAllTimers(); await waitForNextUpdate(); }); - let args = loadSpy.mock.calls[0][0]; - expect(args.isLoading).toBe(true); - expect(args.items).toStrictEqual([]); expect(result.current.isLoading).toBe(false); - expect(result.current.items.length).toBe(2); - expect(result.current.onSortChange).toBeDefined(); - expect(result.current.onLoadMore).toBeFalsy(); + expect(result.current.items).toEqual(ITEMS); }); - it('will call loadMore function when onLoadMore is called', async () => { + it('should call load function when loadMore is called', async () => { + let load = jest.fn().mockImplementation(getItems); let {result, waitForNextUpdate} = renderHook( - () => useAsyncList({ - load: loadSpy, - loadMore: loadMoreSpy - }) + () => useAsyncList({load}) ); + + expect(load).toHaveBeenCalledTimes(1); + expect(result.current.isLoading).toBe(true); + expect(result.current.items).toEqual([]); + await act(async () => { + jest.runAllTimers(); await waitForNextUpdate(); }); - expect(loadSpy).toHaveBeenCalled(); expect(result.current.isLoading).toBe(false); - expect(result.current.items.length).toBe(2); - expect(result.current.onLoadMore).toBeDefined(); + expect(result.current.items).toEqual(ITEMS); await act(async () => { - result.current.onLoadMore(); + result.current.loadMore(); await waitForNextUpdate(); }); - let args = loadMoreSpy.mock.calls[0][0]; - expect(args.isLoading).toBe(true); + + expect(load).toHaveBeenCalledTimes(2); + let args = load.mock.calls[1][0]; expect(args.items).toStrictEqual(ITEMS); + expect(args.cursor).toStrictEqual(3); + + expect(result.current.isLoading).toBe(true); + expect(result.current.items).toEqual(ITEMS); + + await act(async () => { + jest.runAllTimers(); + await waitForNextUpdate(); + }); - expect(result.current.items.length).toBe(4); expect(result.current.isLoading).toBe(false); + expect(result.current.items).toEqual([...ITEMS, ...ITEMS]); }); - it('will call load if sort callback function is not provided', async () => { - let {result, waitForNextUpdate} = renderHook(() => useAsyncList({ - load: loadSpy - })); + it('should call load if sort callback function is not provided', async () => { + let load = jest.fn().mockImplementation(getItems); + let {result, waitForNextUpdate} = renderHook( + () => useAsyncList({load}) + ); + + expect(load).toHaveBeenCalledTimes(1); + expect(result.current.isLoading).toBe(true); + expect(result.current.items).toEqual([]); + + await act(async () => { + jest.runAllTimers(); + await waitForNextUpdate(); + }); + + expect(result.current.isLoading).toBe(false); + expect(result.current.items).toEqual(ITEMS); + + load.mockImplementation(getItems2); + await act(async () => { + result.current.sort({column: 'name'}); await waitForNextUpdate(); }); - expect(loadSpy).toHaveBeenCalled(); - loadSpy.mockReset(); + + expect(result.current.isLoading).toBe(true); + expect(result.current.items).toEqual(ITEMS); + + expect(load).toHaveBeenCalledTimes(2); + let args = load.mock.calls[1][0]; + expect(args.items).toStrictEqual(ITEMS); + await act(async () => { - result.current.onSortChange({}); + jest.runAllTimers(); await waitForNextUpdate(); }); - expect(loadSpy).toHaveBeenCalled(); - expect(sortSpy).not.toHaveBeenCalled(); + + expect(result.current.isLoading).toBe(false); + expect(result.current.items).toEqual(ITEMS2); }); - it('will call sort callback function when onSortChange is called', async () => { + it('should call sort callback function', async () => { + let load = jest.fn().mockImplementation(getItems); + let sort = jest.fn().mockImplementation(getItems2); let {result, waitForNextUpdate} = renderHook(() => useAsyncList({ - load: loadSpy, - sort: sortSpy, - defaultSortDescriptor: {direction: 'ASC'} + load, + sort, + initialSortDescriptor: {direction: 'ASC'} })); + + expect(load).toHaveBeenCalledTimes(1); + let args = load.mock.calls[0][0]; + expect(args.sortDescriptor).toEqual({direction: 'ASC'}); + + expect(result.current.isLoading).toBe(true); + expect(result.current.items).toEqual([]); + await act(async () => { + jest.runAllTimers(); await waitForNextUpdate(); }); - expect(loadSpy).toHaveBeenCalled(); - loadSpy.mockReset(); + + expect(result.current.isLoading).toBe(false); + expect(result.current.items).toEqual(ITEMS); + + await act(async () => { + result.current.sort({column: 'name'}); + await waitForNextUpdate(); + }); + + expect(result.current.isLoading).toBe(true); + expect(result.current.items).toEqual(ITEMS); + + expect(sort).toHaveBeenCalledTimes(1); + args = sort.mock.calls[0][0]; + expect(args.items).toStrictEqual(ITEMS); + await act(async () => { - result.current.onSortChange({}); + jest.runAllTimers(); await waitForNextUpdate(); }); - expect(sortSpy).toHaveBeenCalled(); - expect(loadSpy).not.toHaveBeenCalled(); - expect(result.current.sortDescriptor).toStrictEqual({}); + + expect(result.current.isLoading).toBe(false); + expect(result.current.items).toEqual(ITEMS2); }); - it('will return error in case fetch throws an error', async () => { + it('should return error in case fetch throws an error', async () => { let loadSpyThatThrows = jest.fn().mockRejectedValue(new Error('error')); let {result, waitForNextUpdate} = renderHook(() => useAsyncList({ load: loadSpyThatThrows @@ -122,4 +203,482 @@ describe('useAsyncList', () => { expect(result.current.isLoading).toBe(false); expect(result.current.items.length).toBe(0); }); + + it('should return error in case fetch throws an error during loadMore', async () => { + let load = jest.fn().mockImplementation(getItems); + let {result, waitForNextUpdate} = renderHook(() => useAsyncList({ + load + })); + + expect(load).toHaveBeenCalledTimes(1); + expect(result.current.isLoading).toBe(true); + expect(result.current.items).toEqual([]); + + await act(async () => { + jest.runAllTimers(); + await waitForNextUpdate(); + }); + + expect(result.current.isLoading).toBe(false); + expect(result.current.items).toEqual(ITEMS); + + load.mockRejectedValue(new Error('error')); + + await act(async () => { + result.current.loadMore(); + await waitForNextUpdate(); + }); + + expect(load).toHaveBeenCalledTimes(2); + expect(result.current.error).toBeDefined(); + expect(result.current.error.message).toBe('error'); + expect(result.current.isLoading).toBe(false); + expect(result.current.items).toEqual(ITEMS); + }); + + it('should support reloading data', async () => { + let load = jest.fn().mockImplementation(getItems); + let {result, waitForNextUpdate} = renderHook( + () => useAsyncList({load}) + ); + + expect(load).toHaveBeenCalledTimes(1); + expect(result.current.isLoading).toBe(true); + expect(result.current.items).toEqual([]); + + await act(async () => { + jest.runAllTimers(); + await waitForNextUpdate(); + }); + + expect(result.current.isLoading).toBe(false); + expect(result.current.items).toEqual(ITEMS); + + await act(async () => { + result.current.reload(); + await waitForNextUpdate(); + }); + + expect(result.current.isLoading).toBe(true); + expect(result.current.items).toEqual([]); + + expect(load).toHaveBeenCalledTimes(2); + let args = load.mock.calls[1][0]; + expect(args.items).toStrictEqual(ITEMS); + + await act(async () => { + jest.runAllTimers(); + await waitForNextUpdate(); + }); + + expect(result.current.isLoading).toBe(false); + expect(result.current.items).toEqual(ITEMS); + }); + + it('should abort duplicate concurrent loads', async () => { + let load = jest.fn().mockImplementation(getItems2); + let isAborted; + let sort = jest.fn().mockImplementation(({signal}) => { + return new Promise((resolve, reject) => { + isAborted = false; + signal.addEventListener('abort', () => { + isAborted = true; + reject(); + }); + + setTimeout(() => resolve({items: ITEMS}), 100); + }); + }); + + let {result, waitForNextUpdate} = renderHook( + () => useAsyncList({load, sort}) + ); + + expect(load).toHaveBeenCalledTimes(1); + expect(result.current.isLoading).toBe(true); + expect(result.current.items).toEqual([]); + + await act(async () => { + jest.runAllTimers(); + await waitForNextUpdate(); + }); + + expect(result.current.isLoading).toBe(false); + expect(result.current.items).toEqual(ITEMS2); + + await act(async () => { + result.current.sort({column: 'name'}); + await waitForNextUpdate(); + }); + + expect(result.current.isLoading).toBe(true); + expect(result.current.items).toEqual(ITEMS2); + expect(isAborted).toBe(false); + + sort.mockImplementation(getItems); + + await act(async () => { + result.current.sort({column: 'id'}); + await waitForNextUpdate(); + }); + + expect(isAborted).toBe(true); + expect(result.current.isLoading).toBe(true); + expect(result.current.items).toEqual(ITEMS2); + + await act(async () => { + jest.runAllTimers(); + await waitForNextUpdate(); + }); + + expect(result.current.isLoading).toBe(false); + expect(result.current.items).toEqual(ITEMS); + }); + + it('should ignore duplicate loads where first resolves first', async () => { + let load = jest.fn().mockImplementation(getItems2); + let sort = jest.fn().mockImplementation(() => new Promise(resolve => setTimeout(() => resolve({items: ITEMS2}), 100))); + let {result, waitForNextUpdate} = renderHook( + () => useAsyncList({load, sort}) + ); + + expect(load).toHaveBeenCalledTimes(1); + expect(result.current.isLoading).toBe(true); + expect(result.current.items).toEqual([]); + + await act(async () => { + jest.runAllTimers(); + await waitForNextUpdate(); + }); + + expect(result.current.isLoading).toBe(false); + expect(result.current.items).toEqual(ITEMS2); + + await act(async () => { + result.current.sort({column: 'name'}); + await waitForNextUpdate(); + }); + + expect(result.current.isLoading).toBe(true); + expect(result.current.items).toEqual(ITEMS2); + + sort.mockImplementation(() => new Promise(resolve => setTimeout(() => resolve({items: ITEMS}), 500))); + + await act(async () => { + result.current.sort({column: 'id'}); + await waitForNextUpdate(); + }); + + expect(result.current.isLoading).toBe(true); + expect(result.current.items).toEqual(ITEMS2); + + await act(async () => { + jest.advanceTimersByTime(200); + await waitForNextUpdate(); + }); + + // Should ignore the first load since there is a newer one + expect(result.current.isLoading).toBe(true); + expect(result.current.items).toEqual(ITEMS2); + + await act(async () => { + jest.advanceTimersByTime(500); + await waitForNextUpdate(); + }); + + expect(result.current.isLoading).toBe(false); + expect(result.current.items).toEqual(ITEMS); + }); + + it('should ignore duplicate loads where second resolves first', async () => { + let load = jest.fn().mockImplementation(getItems2); + let sort = jest.fn().mockImplementation(() => new Promise(resolve => setTimeout(() => resolve({items: ITEMS2}), 500))); + let {result, waitForNextUpdate} = renderHook( + () => useAsyncList({load, sort}) + ); + + expect(load).toHaveBeenCalledTimes(1); + expect(result.current.isLoading).toBe(true); + expect(result.current.items).toEqual([]); + + await act(async () => { + jest.runAllTimers(); + await waitForNextUpdate(); + }); + + expect(result.current.isLoading).toBe(false); + expect(result.current.items).toEqual(ITEMS2); + + await act(async () => { + result.current.sort({column: 'name'}); + await waitForNextUpdate(); + }); + + expect(result.current.isLoading).toBe(true); + expect(result.current.items).toEqual(ITEMS2); + + sort.mockImplementation(() => new Promise(resolve => setTimeout(() => resolve({items: ITEMS}), 100))); + + await act(async () => { + result.current.sort({column: 'id'}); + await waitForNextUpdate(); + }); + + expect(result.current.isLoading).toBe(true); + expect(result.current.items).toEqual(ITEMS2); + + await act(async () => { + jest.advanceTimersByTime(200); + await waitForNextUpdate(); + }); + + expect(result.current.isLoading).toBe(false); + expect(result.current.items).toEqual(ITEMS); + + await act(async () => { + jest.advanceTimersByTime(500); + await waitForNextUpdate(); + }); + + // Should ignore this load, since the second one loaded first + expect(result.current.isLoading).toBe(false); + expect(result.current.items).toEqual(ITEMS); + }); + + it('should abort loadMore when a sort happens', async () => { + let load = jest.fn().mockImplementation(getItems2); + let sort = jest.fn().mockImplementation(getItems); + let {result, waitForNextUpdate} = renderHook( + () => useAsyncList({load, sort}) + ); + + expect(load).toHaveBeenCalledTimes(1); + expect(result.current.isLoading).toBe(true); + expect(result.current.items).toEqual([]); + + await act(async () => { + jest.runAllTimers(); + await waitForNextUpdate(); + }); + + expect(result.current.isLoading).toBe(false); + expect(result.current.items).toEqual(ITEMS2); + + let isAborted = false; + load.mockImplementation(({signal}) => { + return new Promise((resolve, reject) => { + signal.addEventListener('abort', () => { + isAborted = true; + reject(); + }); + + setTimeout(() => resolve({items: ITEMS}), 100); + }); + }); + + await act(async () => { + result.current.loadMore(); + await waitForNextUpdate(); + }); + + expect(result.current.isLoading).toBe(true); + expect(result.current.items).toEqual(ITEMS2); + expect(isAborted).toBe(false); + + sort.mockImplementation(getItems); + + await act(async () => { + result.current.sort({column: 'id'}); + await waitForNextUpdate(); + }); + + expect(isAborted).toBe(true); + expect(result.current.isLoading).toBe(true); + expect(result.current.items).toEqual(ITEMS2); + + await act(async () => { + jest.runAllTimers(); + await waitForNextUpdate(); + }); + + expect(result.current.isLoading).toBe(false); + expect(result.current.items).toEqual(ITEMS); + }); + + it('should ignore loadMore when a sort happens and the sort loads first', async () => { + let load = jest.fn().mockImplementation(getItems2); + let sort = jest.fn().mockImplementation(() => new Promise(resolve => setTimeout(() => resolve({items: ITEMS}), 100))); + let {result, waitForNextUpdate} = renderHook( + () => useAsyncList({load, sort}) + ); + + expect(load).toHaveBeenCalledTimes(1); + expect(result.current.isLoading).toBe(true); + expect(result.current.items).toEqual([]); + + await act(async () => { + jest.runAllTimers(); + await waitForNextUpdate(); + }); + + expect(result.current.isLoading).toBe(false); + expect(result.current.items).toEqual(ITEMS2); + + load.mockImplementation(() => new Promise(resolve => setTimeout(() => resolve({items: ITEMS}), 500))); + + await act(async () => { + result.current.loadMore(); + await waitForNextUpdate(); + }); + + expect(result.current.isLoading).toBe(true); + expect(result.current.items).toEqual(ITEMS2); + + await act(async () => { + result.current.sort({column: 'id'}); + await waitForNextUpdate(); + }); + + expect(result.current.isLoading).toBe(true); + expect(result.current.items).toEqual(ITEMS2); + + await act(async () => { + jest.advanceTimersByTime(200); + await waitForNextUpdate(); + }); + + expect(result.current.isLoading).toBe(false); + expect(result.current.items).toEqual(ITEMS); + + await act(async () => { + jest.advanceTimersByTime(500); + await waitForNextUpdate(); + }); + + // Should ignore the loadMore because the sort resolved first + expect(result.current.isLoading).toBe(false); + expect(result.current.items).toEqual(ITEMS); + }); + + it('should ignore loadMore when a sort happens and the loadMore loads first', async () => { + let load = jest.fn().mockImplementation(getItems2); + let sort = jest.fn().mockImplementation(() => new Promise(resolve => setTimeout(() => resolve({items: ITEMS}), 500))); + let {result, waitForNextUpdate} = renderHook( + () => useAsyncList({load, sort}) + ); + + expect(load).toHaveBeenCalledTimes(1); + expect(result.current.isLoading).toBe(true); + expect(result.current.items).toEqual([]); + + await act(async () => { + jest.runAllTimers(); + await waitForNextUpdate(); + }); + + expect(result.current.isLoading).toBe(false); + expect(result.current.items).toEqual(ITEMS2); + + load.mockImplementation(() => new Promise(resolve => setTimeout(() => resolve({items: ITEMS}), 100))); + + await act(async () => { + result.current.loadMore(); + await waitForNextUpdate(); + }); + + expect(result.current.isLoading).toBe(true); + expect(result.current.items).toEqual(ITEMS2); + + await act(async () => { + result.current.sort({column: 'id'}); + await waitForNextUpdate(); + }); + + expect(result.current.isLoading).toBe(true); + expect(result.current.items).toEqual(ITEMS2); + + await act(async () => { + jest.advanceTimersByTime(200); + await waitForNextUpdate(); + }); + + // Should ignore the loadMore since the sort is still loading + expect(result.current.isLoading).toBe(true); + expect(result.current.items).toEqual(ITEMS2); + + await act(async () => { + jest.advanceTimersByTime(500); + await waitForNextUpdate(); + }); + + expect(result.current.isLoading).toBe(false); + expect(result.current.items).toEqual(ITEMS); + }); + + it('should support updating the list', async () => { + let load = jest.fn().mockImplementation(getItems); + let {result, waitForNextUpdate} = renderHook( + () => useAsyncList({load}) + ); + + expect(load).toHaveBeenCalledTimes(1); + expect(result.current.isLoading).toBe(true); + expect(result.current.items).toEqual([]); + + await act(async () => { + jest.runAllTimers(); + await waitForNextUpdate(); + }); + + expect(result.current.isLoading).toBe(false); + expect(result.current.items).toEqual(ITEMS); + + await act(async () => { + result.current.insert(1, {name: 'Test', id: 5}); + await waitForNextUpdate(); + }); + + expect(result.current.isLoading).toBe(false); + expect(result.current.items).toEqual([ITEMS[0], {name: 'Test', id: 5}, ...ITEMS.slice(1)]); + }); + + it('should throw if updating the list while loading', async () => { + let load = jest.fn().mockImplementation(getItems); + let {result, waitForNextUpdate} = renderHook( + () => useAsyncList({load}) + ); + + expect(load).toHaveBeenCalledTimes(1); + expect(result.current.isLoading).toBe(true); + expect(result.current.items).toEqual([]); + + // Ignore fewer hooks than expected error from react + try { + act(() => { + result.current.insert(1, {name: 'Test', id: 5}); + }); + } catch (err) {} + + expect(result.error).toEqual(new Error('Invalid action "update" in state "loading"')); + }); + + it('should get an item by key', async function () { + let load = jest.fn().mockImplementation(getItems); + let {result, waitForNextUpdate} = renderHook( + () => useAsyncList({load}) + ); + + expect(load).toHaveBeenCalledTimes(1); + expect(result.current.isLoading).toBe(true); + expect(result.current.items).toEqual([]); + + await act(async () => { + jest.runAllTimers(); + await waitForNextUpdate(); + }); + + expect(result.current.isLoading).toBe(false); + expect(result.current.items).toEqual(ITEMS); + expect(result.current.getItem(1)).toBe(ITEMS[0]); + }); }); From 223e1eac555cfa9f35310c5d90919c7c1c0f9376 Mon Sep 17 00:00:00 2001 From: Devon Govett Date: Wed, 6 May 2020 20:07:52 -0700 Subject: [PATCH 2/4] Add async loading, infinite scrolling, sorting, and empty state to Table --- .../@react-aria/grid/src/useColumnHeader.ts | 20 ++- packages/@react-spectrum/table/src/Table.tsx | 62 +++++++- .../@react-spectrum/table/src/TableLayout.ts | 26 +++- .../table/stories/Table.stories.tsx | 133 +++++++++++++++++- .../collections/src/CollectionManager.ts | 71 +++++----- .../collections/src/ListLayout.ts | 6 +- .../@react-stately/collections/src/types.ts | 5 +- .../@react-stately/grid/src/GridCollection.ts | 44 ++---- packages/@react-stately/grid/src/TableBody.ts | 49 ++++--- .../@react-stately/grid/src/useGridState.ts | 26 +++- .../illustratedmessage/src/index.d.ts | 2 +- .../@react-types/shared/src/collections.d.ts | 45 +----- packages/@react-types/table/src/index.d.ts | 7 +- 13 files changed, 343 insertions(+), 153 deletions(-) diff --git a/packages/@react-aria/grid/src/useColumnHeader.ts b/packages/@react-aria/grid/src/useColumnHeader.ts index c1d6a57c0fd..979e5f5ae9b 100644 --- a/packages/@react-aria/grid/src/useColumnHeader.ts +++ b/packages/@react-aria/grid/src/useColumnHeader.ts @@ -14,6 +14,8 @@ import {getColumnHeaderId} from './utils'; import {GridNode, GridState} from '@react-stately/grid'; import {HTMLAttributes, RefObject} from 'react'; import {useGridCell} from './useGridCell'; +import { usePress } from '@react-aria/interactions'; +import { mergeProps } from '@react-aria/utils'; interface ColumnHeaderProps { node: GridNode, @@ -30,13 +32,25 @@ export function useColumnHeader(props: ColumnHeaderProps, state: GridState let {node, colspan} = props; let {gridCellProps} = useGridCell(props, state); + let {pressProps} = usePress({ + isDisabled: !node.props.allowsSorting, + onPress() { + state.sort(node.key); + } + }); + + let ariaSort: HTMLAttributes['aria-sort'] = null; + if (node.props.allowsSorting) { + ariaSort = state.sortDescriptor?.column === node.key ? state.sortDescriptor.direction : 'none'; + } + return { columnHeaderProps: { - ...gridCellProps, + ...mergeProps(gridCellProps, pressProps), role: 'columnheader', id: getColumnHeaderId(state, node.key), - 'aria-colspan': colspan && colspan > 1 ? colspan : null - // 'aria-sort' + 'aria-colspan': colspan && colspan > 1 ? colspan : null, + 'aria-sort': ariaSort } }; } diff --git a/packages/@react-spectrum/table/src/Table.tsx b/packages/@react-spectrum/table/src/Table.tsx index 92f3af1bf18..0dba7775771 100644 --- a/packages/@react-spectrum/table/src/Table.tsx +++ b/packages/@react-spectrum/table/src/Table.tsx @@ -17,7 +17,7 @@ import {DOMRef} from '@react-types/shared'; import {FocusRing} from '@react-aria/focus'; import {GridState, useGridState} from '@react-stately/grid'; import {mergeProps} from '@react-aria/utils'; -import {Node, ReusableView, useCollectionState} from '@react-stately/collections'; +import {Node, ReusableView, useCollectionState, Rect} from '@react-stately/collections'; import React, {ReactElement, useCallback, useContext, useMemo, useRef} from 'react'; import {SpectrumColumnProps, SpectrumTableProps} from '@react-types/table'; import styles from '@adobe/spectrum-css-temp/components/table/vars.css'; @@ -26,6 +26,9 @@ import {TableLayout} from './TableLayout'; import {useColumnHeader, useGrid, useGridCell, useRow, useRowGroup, useRowHeader, useSelectAllCheckbox, useSelectionCheckbox} from '@react-aria/grid'; import {useLocale} from '@react-aria/i18n'; import {useProviderProps} from '@react-spectrum/provider'; +import { ProgressCircle } from '@react-spectrum/progress'; +import { Flex } from '@react-spectrum/layout'; +import ArrowDownSmall from '@spectrum-icons/ui/ArrowDownSmall'; const TableContext = React.createContext>(null); function useTableContext() { @@ -137,6 +140,25 @@ function Table(props: SpectrumTableProps, ref: DOMRef) { ); case 'column': return ; + case 'loader': + return ( + + 0 ? 'Loading more...' : 'Loading...'} /> + + ); + case 'empty': + let emptyState = props.renderEmptyState ? props.renderEmptyState() : null; + if (emptyState == null) { + return null; + } + + return ( + + {emptyState} + + ); } }; @@ -182,6 +204,17 @@ function TableCollectionView({layout, collection, focusedKey, renderView, render headerRef.current.scrollLeft = domRef.current.scrollLeft; }, [domRef]); + let onVisibleRectChange = useCallback((rect: Rect) => { + collectionState.setVisibleRect(rect); + + if (!collection.body.props.isLoading && collection.body.props.onLoadMore && collectionState.collectionManager.contentSize.height > rect.height * 2) { + let scrollOffset = collectionState.collectionManager.contentSize.height - rect.height * 2; + if (rect.y > scrollOffset) { + collection.body.props.onLoadMore(); + } + } + }, [collection]); + return (
@@ -264,7 +297,10 @@ function TableColumnHeader({column}) { { 'spectrum-Table-checkboxCell': isCheckboxCell, 'spectrum-Table-cell--alignCenter': columnProps.align === 'center' || column.colspan > 1, - 'spectrum-Table-cell--alignEnd': columnProps.align === 'end' + 'spectrum-Table-cell--alignEnd': columnProps.align === 'end', + 'is-sortable': columnProps.allowsSorting, + 'is-sorted-desc': state.sortDescriptor?.column === column.key && state.sortDescriptor?.direction === 'descending', + 'is-sorted-asc': state.sortDescriptor?.column === column.key && state.sortDescriptor?.direction === 'ascending' } ) }> @@ -274,6 +310,9 @@ function TableColumnHeader({column}) { {...checkboxProps} UNSAFE_className={classNames(styles, 'spectrum-Table-checkbox')} /> } + {columnProps.allowsSorting && + + }
); @@ -435,5 +474,22 @@ function TableRowHeader({cell}) { ); } +function CenteredWrapper({children}) { + let state = useTableContext(); + return ( + +
+ {children} +
+ + ); +} + const _Table = React.forwardRef(Table); export {_Table as Table}; diff --git a/packages/@react-spectrum/table/src/TableLayout.ts b/packages/@react-spectrum/table/src/TableLayout.ts index f018853f8ad..543a219b020 100644 --- a/packages/@react-spectrum/table/src/TableLayout.ts +++ b/packages/@react-spectrum/table/src/TableLayout.ts @@ -24,6 +24,7 @@ export class TableLayout extends ListLayout { this.buildColumnWidths(); let header = this.buildHeader(); let body = this.buildBody(0); + body.layoutInfo.rect.width = Math.max(header.layoutInfo.rect.width, body.layoutInfo.rect.width); this.contentSize = new Size(body.layoutInfo.rect.width, body.layoutInfo.rect.maxY); return [ header, @@ -144,7 +145,7 @@ export class TableLayout extends ListLayout { let startY = y; let width = 0; let children: LayoutNode[] = []; - for (let node of this.collection) { + for (let node of this.collection.body.childNodes) { let layoutNode = this.buildChild(node, 0, y); layoutNode.layoutInfo.parentKey = 'body'; y = layoutNode.layoutInfo.rect.maxY; @@ -152,6 +153,27 @@ export class TableLayout extends ListLayout { children.push(layoutNode); } + // TODO: not show the spinner at the bottom when sorting? + if (this.collection.body.props.isLoading) { + let rect = new Rect(0, y, width || this.collectionManager.visibleRect.width, children.length === 0 ? this.collectionManager.visibleRect.height : 60); + let loader = new LayoutInfo('loader', 'loader', rect); + loader.parentKey = 'body'; + loader.isSticky = children.length === 0; + this.layoutInfos.set('loader', loader); + children.push({layoutInfo: loader}); + y = loader.rect.maxY; + width = Math.max(width, rect.width); + } else if (children.length === 0) { + let rect = new Rect(0, y, this.collectionManager.visibleRect.width, this.collectionManager.visibleRect.height); + let empty = new LayoutInfo('empty', 'empty', rect); + empty.parentKey = 'body'; + empty.isSticky = true; + this.layoutInfos.set('empty', empty); + children.push({layoutInfo: empty}); + y = empty.rect.maxY; + width = Math.max(width, rect.width); + } + rect.width = width; rect.height = y - startY; @@ -230,7 +252,7 @@ export class TableLayout extends ListLayout { } addVisibleLayoutInfos(res: LayoutInfo[], node: LayoutNode, rect: Rect) { - if (node.children.length === 0) { + if (!node.children || node.children.length === 0) { return; } diff --git a/packages/@react-spectrum/table/stories/Table.stories.tsx b/packages/@react-spectrum/table/stories/Table.stories.tsx index 604c8beb84b..48db6d72d71 100644 --- a/packages/@react-spectrum/table/stories/Table.stories.tsx +++ b/packages/@react-spectrum/table/stories/Table.stories.tsx @@ -17,6 +17,10 @@ import {Link} from '@react-spectrum/link'; import React from 'react'; import {storiesOf} from '@storybook/react'; import {Switch} from '@react-spectrum/switch'; +import { useAsyncList } from '@react-stately/data'; +import { IllustratedMessage } from '../../illustratedmessage'; +import { Heading } from '@react-spectrum/typography'; +import { Content } from '@react-spectrum/view'; let columns = [ {name: 'Foo', key: 'foo'}, @@ -64,6 +68,18 @@ for (let i = 0; i < 1000; i++) { manyRows.push(row); } +function renderEmptyState() { + return ( + + + + + No results + No results found + + ); +} + let onSelectionChange = action('onSelectionChange'); storiesOf('Table', module) .add( @@ -184,7 +200,7 @@ storiesOf('Table', module) .add( 'many columns and rows', () => ( - onSelectionChange([...s])}> +
onSelectionChange([...s])}> {column => {column.name} @@ -203,7 +219,7 @@ storiesOf('Table', module) .add( 'isQuiet, many columns and rows', () => ( -
onSelectionChange([...s])}> +
onSelectionChange([...s])}> {column => {column.name} @@ -296,4 +312,117 @@ storiesOf('Table', module) () => ( ) + ) + .add( + 'isLoading', + () => ( +
onSelectionChange([...s])}> + + {column => + {column.name} + } + + + {item => + ( + {key => {item[key]}} + ) + } + +
+ ) + ) + .add( + 'isLoading more', + () => ( + onSelectionChange([...s])}> + + {column => + {column.name} + } + + + {item => + ( + {key => {item[key]}} + ) + } + +
+ ) + ) + .add( + 'renderEmptyState', + () => ( + onSelectionChange([...s])}> + + {column => + {column.name} + } + + + {[]} + +
+ ) + ) + .add( + 'async loading', + () => ); + +function AsyncLoadingExample() { + interface Item { + data: { + id: string, + url: string, + title: string + } + } + + let list = useAsyncList({ + async load({signal, cursor}) { + let url = new URL(`https://www.reddit.com/r/news.json`); + if (cursor) { + url.searchParams.append('after', cursor); + } + + let res = await fetch(url.toString(), {signal}); + let json = await res.json(); + return {items: json.data.children, cursor: json.data.after}; + }, + async sort({items, sortDescriptor}) { + return { + items: items.slice().sort((a, b) => { + let cmp = a.data[sortDescriptor.column] < b.data[sortDescriptor.column] ? -1 : 1; + if (sortDescriptor.direction === 'descending') { + cmp *= -1; + } + return cmp; + }) + } + } + }); + + return ( + + + Score + Title + Author + Comments + + + {item => + ( + {key => + key === 'title' + ? {item.data.title} + : {item.data[key]} + } + ) + } + +
+ ); +} diff --git a/packages/@react-stately/collections/src/CollectionManager.ts b/packages/@react-stately/collections/src/CollectionManager.ts index 15af0300131..8dcb0329b5c 100644 --- a/packages/@react-stately/collections/src/CollectionManager.ts +++ b/packages/@react-stately/collections/src/CollectionManager.ts @@ -218,29 +218,13 @@ export class CollectionManager { if (this._collection) { this._runTransaction(() => { this._collection = data; - }, this._shouldAnimate(data)); + }, true); } else { this._collection = data; this.reloadData(); } } - private _shouldAnimate(collection: Collection) { - for (let key of collection.getKeys()) { - if (!this._collection.getItem(key)) { - return true; - } - } - - for (let key of this._collection.getKeys()) { - if (!collection.getItem(key)) { - return true; - } - } - - return false; - } - /** * Reloads the data from the data source and relayouts the collection view. * Does not animate any changes. Equivalent to re-assigning the same data source @@ -463,11 +447,6 @@ export class CollectionManager { let scrollAnchor = this._getScrollAnchor(); - // Enable transitions if this is an animated relayout - if (context.animated) { - this._enableTransitions(); - } - // Trigger the beforeLayout hook, if provided if (typeof context.beforeLayout === 'function') { context.beforeLayout(); @@ -491,6 +470,7 @@ export class CollectionManager { contentOffsetX = Math.max(0, Math.min(this.contentSize.width - visibleRect.width, contentOffsetX)); contentOffsetY = Math.max(0, Math.min(this.contentSize.height - visibleRect.height, contentOffsetY)); + let hasLayoutUpdates = false; if (contentOffsetX !== visibleRect.x || contentOffsetY !== visibleRect.y) { // If this is an animated relayout, we do not immediately scroll because it would be jittery. // Save the difference between the current and new content offsets, and apply it to the @@ -500,12 +480,12 @@ export class CollectionManager { if (context.animated || !this._animatedContentOffset.isOrigin()) { this._animatedContentOffset.x += visibleRect.x - contentOffsetX; this._animatedContentOffset.y += visibleRect.y - contentOffsetY; - this.updateSubviews(context.contentChanged); + hasLayoutUpdates = this.updateSubviews(context.contentChanged); } else { this._setContentOffset(new Point(contentOffsetX, contentOffsetY)); } } else { - this.updateSubviews(context.contentChanged); + hasLayoutUpdates = this.updateSubviews(context.contentChanged); } // Apply layout infos, unless this is coming from an animated transaction @@ -514,7 +494,9 @@ export class CollectionManager { } // Wait for animations, and apply the afterAnimation hook, if provided - if (context.animated) { + if (context.animated && hasLayoutUpdates) { + this._enableTransitions(); + let done = () => { this._disableTransitions(); @@ -531,7 +513,8 @@ export class CollectionManager { } }; - setTimeout(done, this.transitionDuration); + // Sometimes the animation takes slightly longer than expected. + setTimeout(done, this.transitionDuration + 100); return; } else if (typeof context.afterAnimation === 'function') { context.afterAnimation(); @@ -771,7 +754,9 @@ export class CollectionManager { this._correctItemOrder(); this._flushVisibleViews(); - if (this._transaction) { + + let hasLayoutUpdates = this._transaction && (toAdd.size > 0 || toRemove.size > 0 || this._hasLayoutUpdates()); + if (hasLayoutUpdates) { requestAnimationFrame(() => { // If we're in a transaction, apply animations to visible views // and "to be removed" views, which animate off screen. @@ -780,6 +765,8 @@ export class CollectionManager { } }); } + + return hasLayoutUpdates; } afterRender() { @@ -832,11 +819,7 @@ export class CollectionManager { let updated = false; // Apply layout infos to visible views - for (let [key, view] of this._visibleViews) { - if (this._transaction && this._transaction.initialLayoutInfo.get(key) === view.layoutInfo) { - // view.forceStyleUpdate(); - } - + for (let view of this._visibleViews.values()) { let cur = view.layoutInfo; if (cur) { let layoutInfo = this.layout.getLayoutInfo(cur.key); @@ -871,6 +854,30 @@ export class CollectionManager { } } + private _hasLayoutUpdates() { + if (!this._transaction) { + return false; + } + + for (let view of this._visibleViews.values()) { + let cur = view.layoutInfo; + if (!cur) { + return true; + } + + let layoutInfo = this.layout.getLayoutInfo(cur.key); + if ( + !cur.rect.pointEquals(layoutInfo.rect) || + cur.opacity !== layoutInfo.opacity || + cur.transform !== layoutInfo.transform + ) { + return true; + } + } + + return false; + } + reuseView(view: ReusableView) { view.prepareForReuse(); this._reusableViews[view.viewType].push(view); diff --git a/packages/@react-stately/collections/src/ListLayout.ts b/packages/@react-stately/collections/src/ListLayout.ts index 369d3dfd021..6197c7df62e 100644 --- a/packages/@react-stately/collections/src/ListLayout.ts +++ b/packages/@react-stately/collections/src/ListLayout.ts @@ -118,11 +118,6 @@ export class ListLayout extends Layout> implements KeyboardDelegate { return node.layoutInfo.rect.intersects(rect) || node.layoutInfo.isSticky; } - shouldInvalidate(newRect: Rect, oldRect: Rect): boolean { - // We only care if the width changes. - return newRect.width !== oldRect.width; - } - validate(invalidationContext: InvalidationContext, unknown>) { // Invalidate cache if the size of the collection changed. // In this case, we need to recalculate the entire layout. @@ -130,6 +125,7 @@ export class ListLayout extends Layout> implements KeyboardDelegate { this.cache = new WeakMap(); } + this.collection = this.collectionManager.collection; this.rootNodes = this.buildCollection(); this.lastWidth = this.collectionManager.visibleRect.width; diff --git a/packages/@react-stately/collections/src/types.ts b/packages/@react-stately/collections/src/types.ts index 53370e02fdf..e8c7dc62490 100644 --- a/packages/@react-stately/collections/src/types.ts +++ b/packages/@react-stately/collections/src/types.ts @@ -18,9 +18,8 @@ import {Size} from './Size'; import {Transaction} from './Transaction'; export interface Node { - // TODO: determine how to keep this limited to shared node types /** The type of item this node represents. */ - type: 'section' | 'item' | 'column' | 'cell' | 'rowheader' | 'placeholder' | 'headerrow', + type: string, /** A unique key for the node. */ key: Key, /** The object value the node was created from. */ @@ -52,7 +51,7 @@ export interface Node { } export interface PartialNode { - type?: 'section' | 'item' | 'column' | 'cell' | 'rowheader', + type?: string, key?: Key, value?: T, element?: ReactElement, diff --git a/packages/@react-stately/grid/src/GridCollection.ts b/packages/@react-stately/grid/src/GridCollection.ts index e7a28bdd2bd..beb18217ce6 100644 --- a/packages/@react-stately/grid/src/GridCollection.ts +++ b/packages/@react-stately/grid/src/GridCollection.ts @@ -28,15 +28,12 @@ export class GridCollection implements Collection> { headerRows: GridNode[]; columns: GridNode[]; rowHeaderColumnKeys: Set; - body: GridNode[]; - private firstKey: Key; - private lastKey: Key; + body: GridNode; constructor(nodes: Iterable>, prev?: GridCollection, opts?: GridCollectionOptions) { this.keyMap = new Map(prev?.keyMap) || new Map(); this.columns = []; this.rowHeaderColumnKeys = new Set(); - this.body = prev?.body || []; let visit = (node: GridNode) => { // If the node is the same object as the previous node for the same key, @@ -70,14 +67,8 @@ export class GridCollection implements Collection> { child.prevKey = null; } - if (node.type === 'item') { - child.index = index++; - } - - if (child.type !== 'item') { - visit(child); - } - + child.index = index++; + visit(child); last = child; } @@ -107,7 +98,6 @@ export class GridCollection implements Collection> { let bodyKeys = new Set(); let last: GridNode; let index = 0; - let body = []; for (let node of nodes) { if (last) { last.nextKey = node.key; @@ -122,30 +112,21 @@ export class GridCollection implements Collection> { visit(node); if (node.type !== 'column') { - if (this.firstKey == null) { - this.firstKey = node.key; - } - bodyKeys.add(node.key); - body.push(node); + } + + if (node.type === 'body') { + this.body = node; } last = node; } + if (last) { last.nextKey = null; - this.lastKey = last.key; - } - - for (let node of this.body) { - if (!bodyKeys.has(node.key)) { - remove(node); - } } - this.body = body; - // Default row header column to the first one. if (this.rowHeaderColumnKeys.size === 0) { this.rowHeaderColumnKeys.add(this.columns[0].key); @@ -306,11 +287,11 @@ export class GridCollection implements Collection> { } *[Symbol.iterator]() { - yield* this.body; + yield* this.body.childNodes; } get size() { - return this.body.length; + return [...this.body.childNodes].length; } getKeys() { @@ -328,11 +309,12 @@ export class GridCollection implements Collection> { } getFirstKey() { - return this.firstKey; + return [...this.body.childNodes][0]?.key; } getLastKey() { - return this.lastKey; + let rows = [...this.body.childNodes]; + return rows[rows.length - 1]?.key; } getItem(key: Key) { diff --git a/packages/@react-stately/grid/src/TableBody.ts b/packages/@react-stately/grid/src/TableBody.ts index 4b528da32ac..e2cf58c4b76 100644 --- a/packages/@react-stately/grid/src/TableBody.ts +++ b/packages/@react-stately/grid/src/TableBody.ts @@ -20,28 +20,35 @@ function TableBody(props: TableBodyProps): ReactElement { // eslint-disabl TableBody.getCollectionNode = function* getCollectionNode(props: TableBodyProps): Generator> { let {children, items, itemKey} = props; - if (typeof children === 'function') { - if (!items) { - throw new Error('props.children was a function but props.items is missing'); + yield { + type: 'body', + hasChildNodes: true, + props, + *childNodes() { + if (typeof children === 'function') { + if (!items) { + throw new Error('props.children was a function but props.items is missing'); + } + + for (let item of items) { + yield { + type: 'item', + value: item, + childKey: itemKey, + renderer: children + }; + } + } else { + let items = React.Children.toArray(children); + for (let item of items) { + yield { + type: 'item', + element: item + }; + } + } } - - for (let item of items) { - yield { - type: 'item', - value: item, - childKey: itemKey, - renderer: children - }; - } - } else { - let items = React.Children.toArray(children); - for (let item of items) { - yield { - type: 'item', - element: item - }; - } - } + }; }; // We don't want getCollectionNode to show up in the type definition diff --git a/packages/@react-stately/grid/src/useGridState.ts b/packages/@react-stately/grid/src/useGridState.ts index 3f8e726f316..1c9d9c93e68 100644 --- a/packages/@react-stately/grid/src/useGridState.ts +++ b/packages/@react-stately/grid/src/useGridState.ts @@ -10,7 +10,7 @@ * governing permissions and limitations under the License. */ -import {CollectionBase, MultipleSelection, SelectionMode} from '@react-types/shared'; +import {CollectionBase, MultipleSelection, SelectionMode, SortDescriptor, Sortable, SortDirection} from '@react-types/shared'; import {CollectionBuilder, Node} from '@react-stately/collections'; import {GridCollection} from './GridCollection'; import {Key, useMemo, useRef} from 'react'; @@ -20,7 +20,9 @@ export interface GridState { collection: GridCollection, disabledKeys: Set, selectionManager: SelectionManager, - showSelectionCheckboxes: boolean + showSelectionCheckboxes: boolean, + sortDescriptor: SortDescriptor, + sort(columnKey: Key): void } export interface CollectionBuilderContext { @@ -29,10 +31,15 @@ export interface CollectionBuilderContext { columns: Node[] } -export interface GridStateProps extends CollectionBase, MultipleSelection { +export interface GridStateProps extends CollectionBase, MultipleSelection, Sortable { showSelectionCheckboxes?: boolean } +const OPPOSITE_SORT_DIRECTION = { + ascending: 'descending' as SortDirection, + descending: 'ascending' as SortDirection +}; + export function useGridState(props: GridStateProps): GridState { let selectionState = useMultipleSelectionState(props); let disabledKeys = useMemo(() => @@ -52,12 +59,21 @@ export function useGridState(props: GridStateProps): GridSt collectionRef.current = new GridCollection(nodes, collectionRef.current, context); return collectionRef.current; - }, [props, selectionState.selectionMode, builder]); + }, [props.children, selectionState.selectionMode, builder]); return { collection, disabledKeys, selectionManager: new SelectionManager(collection, selectionState), - showSelectionCheckboxes: props.showSelectionCheckboxes || false + showSelectionCheckboxes: props.showSelectionCheckboxes || false, + sortDescriptor: props.sortDescriptor, + sort(columnKey: Key) { + props.onSortChange({ + column: columnKey, + direction: props.sortDescriptor?.column === columnKey + ? OPPOSITE_SORT_DIRECTION[props.sortDescriptor.direction] + : 'ascending' + }); + } }; } diff --git a/packages/@react-types/illustratedmessage/src/index.d.ts b/packages/@react-types/illustratedmessage/src/index.d.ts index 3d6e01f49fb..91774ed6060 100644 --- a/packages/@react-types/illustratedmessage/src/index.d.ts +++ b/packages/@react-types/illustratedmessage/src/index.d.ts @@ -15,5 +15,5 @@ import {ReactElement} from 'react'; export interface SpectrumIllustratedMessageProps extends DOMProps, StyleProps { /** The contents of the IllustratedMessage. */ - children: [ReactElement] + children: ReactNode } diff --git a/packages/@react-types/shared/src/collections.d.ts b/packages/@react-types/shared/src/collections.d.ts index adcfb26510f..6bc5036d480 100644 --- a/packages/@react-types/shared/src/collections.d.ts +++ b/packages/@react-types/shared/src/collections.d.ts @@ -21,10 +21,7 @@ export interface ItemProps { textValue?: string, /** An accessibility label for this item. */ 'aria-label'?: string, - /** - * Child items of this item. The function that rendered - * this items will be called recursively to render each one. - * */ + /** A list of child item objects. Used for dynamic collections. */ childItems?: Iterable, /** Whether this item has children, even if not loaded yet. */ hasChildItems?: boolean, @@ -99,10 +96,8 @@ export interface Expandable { } export interface Sortable { - /** The current sorted column and direction (controlled). */ + /** The current sorted column and direction. */ sortDescriptor?: SortDescriptor, - /** The initial sorted column and direction (uncontrolled). */ - defaultSortDescriptor?: SortDescriptor, /** Handler that is called when the sorted column or direction changes. */ onSortChange?: (descriptor: SortDescriptor) => any } @@ -114,10 +109,7 @@ export interface SortDescriptor { direction?: SortDirection } -export enum SortDirection { - ASC, - DESC -} +export type SortDirection = 'ascending' | 'descending'; export interface KeyboardDelegate { /** Returns the key visually below the given one, or `null` for none. */ @@ -147,34 +139,3 @@ export interface KeyboardDelegate { /** Returns the next key after `fromKey` that matches the given search string, or `null` for none. */ getKeyForSearch?(search: string, fromKey?: Key): Key } - -interface AsyncListOptions { - load: (state: ListState) => Promise>, - loadMore?: (state: ListState) => Promise>, - defaultSortDescriptor?: SortDescriptor, - sort?: (state: ListState) => Promise> -} - -interface ListState { - items: Iterable, - disabledKeys?: Iterable, - selectedKeys?: Iterable, - selectedKey?: Key, - expandedKeys?: Iterable, - sortDescriptor?: SortDescriptor -} - -interface AsyncListProps { - items: Iterable, - isLoading: boolean, - error?: Error, - onLoadMore?: () => void, - sortDescriptor?: SortDescriptor, - onSortChange?: (desc: SortDescriptor) => void, - disabledKeys?: Iterable, - selectedKeys?: Iterable, - selectedKey?: Key, - expandedKeys?: Iterable -} - -declare function useAsyncList(opts: AsyncListOptions): AsyncListProps; diff --git a/packages/@react-types/table/src/index.d.ts b/packages/@react-types/table/src/index.d.ts index 1f66468f5ed..d75630740ee 100644 --- a/packages/@react-types/table/src/index.d.ts +++ b/packages/@react-types/table/src/index.d.ts @@ -10,17 +10,18 @@ * governing permissions and limitations under the License. */ -import {AsyncLoadable, CollectionChildren, DOMProps, MultipleSelection, SectionProps, StyleProps} from '@react-types/shared'; +import {AsyncLoadable, CollectionChildren, DOMProps, MultipleSelection, SectionProps, StyleProps, Sortable} from '@react-types/shared'; import {Key, ReactElement} from 'react'; -export interface TableProps extends MultipleSelection { +export interface TableProps extends MultipleSelection, Sortable { children: ReactElement[], disabledKeys?: Iterable } export interface SpectrumTableProps extends TableProps, DOMProps, StyleProps { rowHeight?: number | 'auto', - isQuiet?: boolean + isQuiet?: boolean, + renderEmptyState?: () => JSX.Element } export interface TableHeaderProps { From 8ebf7cb084815f39df0a9a5ec45f65fe6aef493e Mon Sep 17 00:00:00 2001 From: Devon Govett Date: Thu, 7 May 2020 09:26:48 -0700 Subject: [PATCH 3/4] Add tests for async loading and sorting --- packages/@react-spectrum/table/src/Table.tsx | 15 +- packages/@react-spectrum/table/src/table.css | 8 + .../@react-spectrum/table/test/Table.test.js | 386 ++++++++++++++++++ .../@react-stately/grid/src/GridCollection.ts | 5 +- 4 files changed, 401 insertions(+), 13 deletions(-) diff --git a/packages/@react-spectrum/table/src/Table.tsx b/packages/@react-spectrum/table/src/Table.tsx index 0dba7775771..c0286d5253f 100644 --- a/packages/@react-spectrum/table/src/Table.tsx +++ b/packages/@react-spectrum/table/src/Table.tsx @@ -477,17 +477,14 @@ function TableRowHeader({cell}) { function CenteredWrapper({children}) { let state = useTableContext(); return ( - -
+
+
{children}
- +
); } diff --git a/packages/@react-spectrum/table/src/table.css b/packages/@react-spectrum/table/src/table.css index ed3975166f9..be840accf6b 100644 --- a/packages/@react-spectrum/table/src/table.css +++ b/packages/@react-spectrum/table/src/table.css @@ -28,3 +28,11 @@ .react-spectrum-Table-cell--alignEnd { justify-content: flex-end; } + +.react-spectrum-Table-centeredWrapper { + display: flex; + align-items: center; + justify-content: center; + width: 100%; + height: 100%; +} diff --git a/packages/@react-spectrum/table/test/Table.test.js b/packages/@react-spectrum/table/test/Table.test.js index 7546ad5f08d..3cb87c8e951 100644 --- a/packages/@react-spectrum/table/test/Table.test.js +++ b/packages/@react-spectrum/table/test/Table.test.js @@ -107,6 +107,10 @@ describe('Table', function () { expect(headers[2]).toHaveAttribute('aria-colindex', '3'); expect(headers[3]).toHaveAttribute('aria-colindex', '4'); + for (let header of headers) { + expect(header).not.toHaveAttribute('aria-sort'); + } + let checkbox = within(headers[0]).getByRole('checkbox'); expect(checkbox).toHaveAttribute('aria-label', 'Select All'); @@ -1171,4 +1175,386 @@ describe('Table', function () { expect(rowHeaders[1]).toHaveTextContent('Jones'); }); }); + + describe('async loading', function () { + let defaultTable = ( + + + Foo + Bar + + + + Foo 1 + Bar 1 + + + Foo 2 + Bar 2 + + +
+ ); + + it('should display a spinner when loading', function () { + let tree = render( + + + Foo + Bar + + + {[]} + +
+ ); + + let table = tree.getByRole('grid'); + let rows = within(table).getAllByRole('row'); + expect(rows).toHaveLength(2); + expect(rows[1]).toHaveAttribute('aria-rowindex', '2'); + + let cell = within(rows[1]).getByRole('rowheader'); + expect(cell).toHaveAttribute('aria-colspan', '3'); + + let spinner = within(cell).getByRole('progressbar'); + expect(spinner).toBeVisible(); + expect(spinner).toHaveAttribute('aria-label', 'Loading...'); + expect(spinner).not.toHaveAttribute('aria-valuenow'); + + tree.rerender(defaultTable); + act(() => jest.runAllTimers()); + + rows = within(table).getAllByRole('row'); + expect(rows).toHaveLength(3); + expect(spinner).not.toBeInTheDocument(); + }); + + it('should display a spinner at the bottom when loading more', function () { + let tree = render( + + + Foo + Bar + + + + Foo 1 + Bar 1 + + + Foo 2 + Bar 2 + + +
+ ); + + let table = tree.getByRole('grid'); + let rows = within(table).getAllByRole('row'); + expect(rows).toHaveLength(4); + expect(rows[3]).toHaveAttribute('aria-rowindex', '4'); + + let cell = within(rows[3]).getByRole('rowheader'); + expect(cell).toHaveAttribute('aria-colspan', '3'); + + let spinner = within(cell).getByRole('progressbar'); + expect(spinner).toBeVisible(); + expect(spinner).toHaveAttribute('aria-label', 'Loading more...'); + expect(spinner).not.toHaveAttribute('aria-valuenow'); + + tree.rerender(defaultTable); + act(() => jest.runAllTimers()); + + rows = within(table).getAllByRole('row'); + expect(rows).toHaveLength(3); + expect(spinner).not.toBeInTheDocument(); + }); + + it('should fire onLoadMore when scrolling near the bottom', function () { + let items = []; + for (let i = 1; i <= 100; i++) { + items.push({id: i, foo: 'Foo ' + i, bar: 'Bar ' + i}); + } + + let onLoadMore = jest.fn(); + let tree = render( + + + Foo + Bar + + + {row => + + {key => row[key]} + + } + +
+ ); + + let table = tree.getByRole('grid'); + let body = tree.getAllByRole('rowgroup')[1]; + let scrollView = body.parentNode.parentNode; + + let rows = within(body).getAllByRole('row'); + expect(rows).toHaveLength(20); // each row is 50px tall. table is 1000px tall. 20 rows fit. + + scrollView.scrollTop = 250; + fireEvent.scroll(scrollView); + + scrollView.scrollTop = 1500; + fireEvent.scroll(scrollView); + + scrollView.scrollTop = 3000; + fireEvent.scroll(scrollView); + + scrollView.scrollTop = 3500; + fireEvent.scroll(scrollView); + + expect(onLoadMore).toHaveBeenCalledTimes(1); + }); + + it('should display an empty state when there are no items', function () { + let tree = render( +

No results

}> + + Foo + Bar + + + {[]} + +
+ ); + + let table = tree.getByRole('grid'); + let rows = within(table).getAllByRole('row'); + expect(rows).toHaveLength(2); + expect(rows[1]).toHaveAttribute('aria-rowindex', '2'); + + let cell = within(rows[1]).getByRole('rowheader'); + expect(cell).toHaveAttribute('aria-colspan', '3'); + + let heading = within(cell).getByRole('heading'); + expect(heading).toBeVisible(); + expect(heading).toHaveTextContent('No results'); + + tree.rerender(defaultTable); + act(() => jest.runAllTimers()); + + rows = within(table).getAllByRole('row'); + expect(rows).toHaveLength(3); + expect(heading).not.toBeInTheDocument(); + }); + }); + + describe('sorting', function() { + it('should set aria-sort="none" on sortable column headers', function () { + let tree = render( + + + Foo + Bar + Baz + + + + Foo 1 + Bar 1 + Baz 1 + + +
+ ); + + let table = tree.getByRole('grid'); + let columnheaders = within(table).getAllByRole('columnheader'); + expect(columnheaders).toHaveLength(3); + expect(columnheaders[0]).toHaveAttribute('aria-sort', 'none'); + expect(columnheaders[1]).toHaveAttribute('aria-sort', 'none'); + expect(columnheaders[2]).not.toHaveAttribute('aria-sort'); + }); + + it('should set aria-sort="ascending" on sorted column header', function () { + let tree = render( + + + Foo + Bar + Baz + + + + Foo 1 + Bar 1 + Baz 1 + + +
+ ); + + let table = tree.getByRole('grid'); + let columnheaders = within(table).getAllByRole('columnheader'); + expect(columnheaders).toHaveLength(3); + expect(columnheaders[0]).toHaveAttribute('aria-sort', 'none'); + expect(columnheaders[1]).toHaveAttribute('aria-sort', 'ascending'); + expect(columnheaders[2]).not.toHaveAttribute('aria-sort'); + }); + + it('should set aria-sort="descending" on sorted column header', function () { + let tree = render( + + + Foo + Bar + Baz + + + + Foo 1 + Bar 1 + Baz 1 + + +
+ ); + + let table = tree.getByRole('grid'); + let columnheaders = within(table).getAllByRole('columnheader'); + expect(columnheaders).toHaveLength(3); + expect(columnheaders[0]).toHaveAttribute('aria-sort', 'none'); + expect(columnheaders[1]).toHaveAttribute('aria-sort', 'descending'); + expect(columnheaders[2]).not.toHaveAttribute('aria-sort'); + }); + + it('should fire onSortChange when there is no existing sortDescriptor', function () { + let onSortChange = jest.fn(); + let tree = render( + + + Foo + Bar + Baz + + + + Foo 1 + Bar 1 + Baz 1 + + +
+ ); + + let table = tree.getByRole('grid'); + let columnheaders = within(table).getAllByRole('columnheader'); + expect(columnheaders).toHaveLength(3); + expect(columnheaders[0]).toHaveAttribute('aria-sort', 'none'); + expect(columnheaders[1]).toHaveAttribute('aria-sort', 'none'); + expect(columnheaders[2]).not.toHaveAttribute('aria-sort'); + + act(() => triggerPress(columnheaders[0])); + + expect(onSortChange).toHaveBeenCalledTimes(1); + expect(onSortChange).toHaveBeenCalledWith({column: 'foo', direction: 'ascending'}); + }); + + it('should toggle the sort direction from ascending to descending', function () { + let onSortChange = jest.fn(); + let tree = render( + + + Foo + Bar + Baz + + + + Foo 1 + Bar 1 + Baz 1 + + +
+ ); + + let table = tree.getByRole('grid'); + let columnheaders = within(table).getAllByRole('columnheader'); + expect(columnheaders).toHaveLength(3); + expect(columnheaders[0]).toHaveAttribute('aria-sort', 'ascending'); + expect(columnheaders[1]).toHaveAttribute('aria-sort', 'none'); + expect(columnheaders[2]).not.toHaveAttribute('aria-sort'); + + act(() => triggerPress(columnheaders[0])); + + expect(onSortChange).toHaveBeenCalledTimes(1); + expect(onSortChange).toHaveBeenCalledWith({column: 'foo', direction: 'descending'}); + }); + + it('should toggle the sort direction from descending to ascending', function () { + let onSortChange = jest.fn(); + let tree = render( + + + Foo + Bar + Baz + + + + Foo 1 + Bar 1 + Baz 1 + + +
+ ); + + let table = tree.getByRole('grid'); + let columnheaders = within(table).getAllByRole('columnheader'); + expect(columnheaders).toHaveLength(3); + expect(columnheaders[0]).toHaveAttribute('aria-sort', 'descending'); + expect(columnheaders[1]).toHaveAttribute('aria-sort', 'none'); + expect(columnheaders[2]).not.toHaveAttribute('aria-sort'); + + act(() => triggerPress(columnheaders[0])); + + expect(onSortChange).toHaveBeenCalledTimes(1); + expect(onSortChange).toHaveBeenCalledWith({column: 'foo', direction: 'ascending'}); + }); + + it('should trigger sorting on a different column', function () { + let onSortChange = jest.fn(); + let tree = render( + + + Foo + Bar + Baz + + + + Foo 1 + Bar 1 + Baz 1 + + +
+ ); + + let table = tree.getByRole('grid'); + let columnheaders = within(table).getAllByRole('columnheader'); + expect(columnheaders).toHaveLength(3); + expect(columnheaders[0]).toHaveAttribute('aria-sort', 'ascending'); + expect(columnheaders[1]).toHaveAttribute('aria-sort', 'none'); + expect(columnheaders[2]).not.toHaveAttribute('aria-sort'); + + act(() => triggerPress(columnheaders[1])); + + expect(onSortChange).toHaveBeenCalledTimes(1); + expect(onSortChange).toHaveBeenCalledWith({column: 'bar', direction: 'ascending'}); + }); + }); }); diff --git a/packages/@react-stately/grid/src/GridCollection.ts b/packages/@react-stately/grid/src/GridCollection.ts index beb18217ce6..78b273279d5 100644 --- a/packages/@react-stately/grid/src/GridCollection.ts +++ b/packages/@react-stately/grid/src/GridCollection.ts @@ -106,10 +106,7 @@ export class GridCollection implements Collection> { node.prevKey = null; } - if (node.type === 'item') { - node.index = index++; - } - + node.index = index++; visit(node); if (node.type !== 'column') { bodyKeys.add(node.key); From 1060d38b1493eb51678502a67c29b7357d6c0ffd Mon Sep 17 00:00:00 2001 From: Devon Govett Date: Thu, 7 May 2020 10:03:31 -0700 Subject: [PATCH 4/4] lints --- .../@react-aria/grid/src/useColumnHeader.ts | 4 +- packages/@react-spectrum/table/package.json | 4 +- packages/@react-spectrum/table/src/Table.tsx | 12 ++--- .../table/stories/Table.stories.tsx | 12 ++--- .../@react-spectrum/table/test/Table.test.js | 7 ++- .../@react-stately/data/src/useAsyncList.ts | 7 +-- .../@react-stately/data/src/useListData.ts | 2 +- .../data/test/useAsyncList.test.js | 48 ++++++++----------- .../@react-stately/grid/src/useGridState.ts | 5 +- .../illustratedmessage/src/index.d.ts | 2 +- packages/@react-types/table/src/index.d.ts | 2 +- 11 files changed, 50 insertions(+), 55 deletions(-) diff --git a/packages/@react-aria/grid/src/useColumnHeader.ts b/packages/@react-aria/grid/src/useColumnHeader.ts index 979e5f5ae9b..28e4c5306f4 100644 --- a/packages/@react-aria/grid/src/useColumnHeader.ts +++ b/packages/@react-aria/grid/src/useColumnHeader.ts @@ -13,9 +13,9 @@ import {getColumnHeaderId} from './utils'; import {GridNode, GridState} from '@react-stately/grid'; import {HTMLAttributes, RefObject} from 'react'; +import {mergeProps} from '@react-aria/utils'; import {useGridCell} from './useGridCell'; -import { usePress } from '@react-aria/interactions'; -import { mergeProps } from '@react-aria/utils'; +import {usePress} from '@react-aria/interactions'; interface ColumnHeaderProps { node: GridNode, diff --git a/packages/@react-spectrum/table/package.json b/packages/@react-spectrum/table/package.json index af89d5de21c..0175d992fdf 100644 --- a/packages/@react-spectrum/table/package.json +++ b/packages/@react-spectrum/table/package.json @@ -33,7 +33,9 @@ "@react-spectrum/utils": "^3.0.0-alpha.1", "@react-stately/collections": "^3.0.0-alpha.1", "@react-stately/grid": "^3.0.0-alpha.1", - "@react-types/table": "^3.0.0-alpha.1" + "@react-spectrum/progress": "^3.0.0-rc.2", + "@react-types/table": "^3.0.0-alpha.1", + "@spectrum-icons/ui": "^3.0.0-rc.2" }, "devDependencies": { "@adobe/spectrum-css-temp": "^3.0.0-alpha.1" diff --git a/packages/@react-spectrum/table/src/Table.tsx b/packages/@react-spectrum/table/src/Table.tsx index c0286d5253f..5999a18299f 100644 --- a/packages/@react-spectrum/table/src/Table.tsx +++ b/packages/@react-spectrum/table/src/Table.tsx @@ -10,6 +10,7 @@ * governing permissions and limitations under the License. */ +import ArrowDownSmall from '@spectrum-icons/ui/ArrowDownSmall'; import {Checkbox} from '@react-spectrum/checkbox'; import {classNames, filterDOMProps, useDOMRef, useStyleProps} from '@react-spectrum/utils'; import {CollectionItem, layoutInfoToStyle, ScrollView, setScrollLeft, useCollectionView} from '@react-aria/collections'; @@ -17,7 +18,8 @@ import {DOMRef} from '@react-types/shared'; import {FocusRing} from '@react-aria/focus'; import {GridState, useGridState} from '@react-stately/grid'; import {mergeProps} from '@react-aria/utils'; -import {Node, ReusableView, useCollectionState, Rect} from '@react-stately/collections'; +import {Node, Rect, ReusableView, useCollectionState} from '@react-stately/collections'; +import {ProgressCircle} from '@react-spectrum/progress'; import React, {ReactElement, useCallback, useContext, useMemo, useRef} from 'react'; import {SpectrumColumnProps, SpectrumTableProps} from '@react-types/table'; import styles from '@adobe/spectrum-css-temp/components/table/vars.css'; @@ -26,9 +28,6 @@ import {TableLayout} from './TableLayout'; import {useColumnHeader, useGrid, useGridCell, useRow, useRowGroup, useRowHeader, useSelectAllCheckbox, useSelectionCheckbox} from '@react-aria/grid'; import {useLocale} from '@react-aria/i18n'; import {useProviderProps} from '@react-spectrum/provider'; -import { ProgressCircle } from '@react-spectrum/progress'; -import { Flex } from '@react-spectrum/layout'; -import ArrowDownSmall from '@spectrum-icons/ui/ArrowDownSmall'; const TableContext = React.createContext>(null); function useTableContext() { @@ -148,7 +147,7 @@ function Table(props: SpectrumTableProps, ref: DOMRef) { aria-label={state.collection.size > 0 ? 'Loading more...' : 'Loading...'} /> ); - case 'empty': + case 'empty': { let emptyState = props.renderEmptyState ? props.renderEmptyState() : null; if (emptyState == null) { return null; @@ -159,6 +158,7 @@ function Table(props: SpectrumTableProps, ref: DOMRef) { {emptyState} ); + } } }; @@ -213,7 +213,7 @@ function TableCollectionView({layout, collection, focusedKey, renderView, render collection.body.props.onLoadMore(); } } - }, [collection]); + }, [collection.body.props, collectionState]); return (
({ async load({signal, cursor}) { - let url = new URL(`https://www.reddit.com/r/news.json`); + let url = new URL('https://www.reddit.com/r/news.json'); if (cursor) { url.searchParams.append('after', cursor); } @@ -400,7 +400,7 @@ function AsyncLoadingExample() { } return cmp; }) - } + }; } }); diff --git a/packages/@react-spectrum/table/test/Table.test.js b/packages/@react-spectrum/table/test/Table.test.js index 3cb87c8e951..f717c31e55a 100644 --- a/packages/@react-spectrum/table/test/Table.test.js +++ b/packages/@react-spectrum/table/test/Table.test.js @@ -1285,16 +1285,15 @@ describe('Table', function () { Bar - {row => + {row => ( {key => row[key]} - } + )} ); - let table = tree.getByRole('grid'); let body = tree.getAllByRole('rowgroup')[1]; let scrollView = body.parentNode.parentNode; @@ -1350,7 +1349,7 @@ describe('Table', function () { }); }); - describe('sorting', function() { + describe('sorting', function () { it('should set aria-sort="none" on sortable column headers', function () { let tree = render( diff --git a/packages/@react-stately/data/src/useAsyncList.ts b/packages/@react-stately/data/src/useAsyncList.ts index ae150646bc5..37db662c530 100644 --- a/packages/@react-stately/data/src/useAsyncList.ts +++ b/packages/@react-stately/data/src/useAsyncList.ts @@ -10,9 +10,9 @@ * governing permissions and limitations under the License. */ +import {createListActions, ListData, ListState} from './useListData'; +import {Key, Reducer, useEffect, useReducer} from 'react'; import {SortDescriptor} from '@react-types/shared'; -import {useEffect, useReducer, Key, Reducer} from 'react'; -import { ListData, useListData, createListActions, ListState } from './useListData'; interface AsyncListOptions { initialSelectedKeys?: Iterable, @@ -226,6 +226,7 @@ export function useAsyncList(options: AsyncListOptions): As useEffect(() => { dispatchFetch({type: 'loading'}, load); + // eslint-disable-next-line react-hooks/exhaustive-deps }, []); return { @@ -252,7 +253,7 @@ export function useAsyncList(options: AsyncListOptions): As dispatchFetch({type: 'sorting', sortDescriptor}, sort || load); }, ...createListActions(options, fn => { - dispatch({type: 'update', updater: fn}) + dispatch({type: 'update', updater: fn}); }) }; } diff --git a/packages/@react-stately/data/src/useListData.ts b/packages/@react-stately/data/src/useListData.ts index d983846eed3..3b8c0d77f7a 100644 --- a/packages/@react-stately/data/src/useListData.ts +++ b/packages/@react-stately/data/src/useListData.ts @@ -196,7 +196,7 @@ export function createListActions(opts: ListOptions, dispatch: (updater: ( ...state, items, selectedKeys: new Set() - } + }; }); }, move(key: Key, toIndex: number) { diff --git a/packages/@react-stately/data/test/useAsyncList.test.js b/packages/@react-stately/data/test/useAsyncList.test.js index 008c20f045e..dd879cf7f7e 100644 --- a/packages/@react-stately/data/test/useAsyncList.test.js +++ b/packages/@react-stately/data/test/useAsyncList.test.js @@ -34,12 +34,6 @@ describe('useAsyncList', () => { jest.useFakeTimers(); }); - function flushPromises() { - return new Promise(function(resolve) { - setImmediate(resolve); - }); - } - it('should call load function on init', async () => { let load = jest.fn().mockImplementation(getItems); let {result, waitForNextUpdate} = renderHook(() => useAsyncList({load})); @@ -278,17 +272,15 @@ describe('useAsyncList', () => { it('should abort duplicate concurrent loads', async () => { let load = jest.fn().mockImplementation(getItems2); let isAborted; - let sort = jest.fn().mockImplementation(({signal}) => { - return new Promise((resolve, reject) => { - isAborted = false; - signal.addEventListener('abort', () => { - isAborted = true; - reject(); - }); - - setTimeout(() => resolve({items: ITEMS}), 100); + let sort = jest.fn().mockImplementation(({signal}) => new Promise((resolve, reject) => { + isAborted = false; + signal.addEventListener('abort', () => { + isAborted = true; + reject(); }); - }); + + setTimeout(() => resolve({items: ITEMS}), 100); + })); let {result, waitForNextUpdate} = renderHook( () => useAsyncList({load, sort}) @@ -465,16 +457,14 @@ describe('useAsyncList', () => { expect(result.current.items).toEqual(ITEMS2); let isAborted = false; - load.mockImplementation(({signal}) => { - return new Promise((resolve, reject) => { - signal.addEventListener('abort', () => { - isAborted = true; - reject(); - }); - - setTimeout(() => resolve({items: ITEMS}), 100); + load.mockImplementation(({signal}) => new Promise((resolve, reject) => { + signal.addEventListener('abort', () => { + isAborted = true; + reject(); }); - }); + + setTimeout(() => resolve({items: ITEMS}), 100); + })); await act(async () => { result.current.loadMore(); @@ -609,7 +599,7 @@ describe('useAsyncList', () => { await act(async () => { jest.advanceTimersByTime(500); await waitForNextUpdate(); - }); + }); expect(result.current.isLoading).toBe(false); expect(result.current.items).toEqual(ITEMS); @@ -644,7 +634,7 @@ describe('useAsyncList', () => { it('should throw if updating the list while loading', async () => { let load = jest.fn().mockImplementation(getItems); - let {result, waitForNextUpdate} = renderHook( + let {result} = renderHook( () => useAsyncList({load}) ); @@ -657,7 +647,9 @@ describe('useAsyncList', () => { act(() => { result.current.insert(1, {name: 'Test', id: 5}); }); - } catch (err) {} + } catch (err) { + // ignore + } expect(result.error).toEqual(new Error('Invalid action "update" in state "loading"')); }); diff --git a/packages/@react-stately/grid/src/useGridState.ts b/packages/@react-stately/grid/src/useGridState.ts index 1c9d9c93e68..2933876a940 100644 --- a/packages/@react-stately/grid/src/useGridState.ts +++ b/packages/@react-stately/grid/src/useGridState.ts @@ -10,7 +10,7 @@ * governing permissions and limitations under the License. */ -import {CollectionBase, MultipleSelection, SelectionMode, SortDescriptor, Sortable, SortDirection} from '@react-types/shared'; +import {CollectionBase, MultipleSelection, SelectionMode, Sortable, SortDescriptor, SortDirection} from '@react-types/shared'; import {CollectionBuilder, Node} from '@react-stately/collections'; import {GridCollection} from './GridCollection'; import {Key, useMemo, useRef} from 'react'; @@ -59,7 +59,8 @@ export function useGridState(props: GridStateProps): GridSt collectionRef.current = new GridCollection(nodes, collectionRef.current, context); return collectionRef.current; - }, [props.children, selectionState.selectionMode, builder]); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [props.children, props.showSelectionCheckboxes, selectionState.selectionMode, builder]); return { collection, diff --git a/packages/@react-types/illustratedmessage/src/index.d.ts b/packages/@react-types/illustratedmessage/src/index.d.ts index 91774ed6060..f2f3c68e837 100644 --- a/packages/@react-types/illustratedmessage/src/index.d.ts +++ b/packages/@react-types/illustratedmessage/src/index.d.ts @@ -11,7 +11,7 @@ */ import {DOMProps, StyleProps} from '@react-types/shared'; -import {ReactElement} from 'react'; +import {ReactNode} from 'react'; export interface SpectrumIllustratedMessageProps extends DOMProps, StyleProps { /** The contents of the IllustratedMessage. */ diff --git a/packages/@react-types/table/src/index.d.ts b/packages/@react-types/table/src/index.d.ts index d75630740ee..9f99f56ee8e 100644 --- a/packages/@react-types/table/src/index.d.ts +++ b/packages/@react-types/table/src/index.d.ts @@ -10,7 +10,7 @@ * governing permissions and limitations under the License. */ -import {AsyncLoadable, CollectionChildren, DOMProps, MultipleSelection, SectionProps, StyleProps, Sortable} from '@react-types/shared'; +import {AsyncLoadable, CollectionChildren, DOMProps, MultipleSelection, SectionProps, Sortable, StyleProps} from '@react-types/shared'; import {Key, ReactElement} from 'react'; export interface TableProps extends MultipleSelection, Sortable {