From 8d5a5d0860c7747802720030f91b7658196d7e88 Mon Sep 17 00:00:00 2001 From: Ryland Herrick Date: Tue, 21 Jul 2020 15:26:51 -0500 Subject: [PATCH] [Security Solution][Detections] Adds loading states to export/delete on modal (#72562) * Add loading spinners to Value Lists modal While export or a delete is pending, we display a loading spinner instead of the button that was clicked. Since state is controlled in the parent, we must pass this additional state in the same way; the table component simply reacts to this state. * Fix bug with useAsync and multiple calls Multiple calls to start() would not previously reset the hook's state, where useEffect on the hook's state would fire improperly as subsequent calls would not travel the same undefined -> result path. * Fix style of loading spinner This fits the size of the button it's replacing, so no shifting occurs when replacing elements. * Better styling of spinner Keep it roughly the same size as the icons themselves, and fill the space with margin. * Fix circular dependency in value lists modal Moves our shared types into a separate module to prevent a circular dependency. --- .../public/common/hooks/use_async.test.ts | 32 ++++++ .../lists/public/common/hooks/use_async.ts | 2 + .../value_lists_management_modal/modal.tsx | 17 +++- .../table.test.tsx | 20 +++- .../value_lists_management_modal/table.tsx | 67 ++----------- .../table_helpers.tsx | 98 +++++++++++++++++++ .../value_lists_management_modal/types.ts | 16 +++ 7 files changed, 185 insertions(+), 67 deletions(-) create mode 100644 x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/table_helpers.tsx create mode 100644 x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/types.ts diff --git a/x-pack/plugins/lists/public/common/hooks/use_async.test.ts b/x-pack/plugins/lists/public/common/hooks/use_async.test.ts index 6f115929c3f67..33f28cfc97291 100644 --- a/x-pack/plugins/lists/public/common/hooks/use_async.test.ts +++ b/x-pack/plugins/lists/public/common/hooks/use_async.test.ts @@ -95,4 +95,36 @@ describe('useAsync', () => { expect(result.current.loading).toBe(false); }); + + it('multiple start calls reset state', async () => { + let resolve: (result: string) => void; + fn.mockImplementation(() => new Promise((_resolve) => (resolve = _resolve))); + + const { result, waitForNextUpdate } = renderHook(() => useAsync(fn)); + + act(() => { + result.current.start(args); + }); + + expect(result.current.loading).toBe(true); + + act(() => resolve('result')); + await waitForNextUpdate(); + + expect(result.current.loading).toBe(false); + expect(result.current.result).toBe('result'); + + act(() => { + result.current.start(args); + }); + + expect(result.current.loading).toBe(true); + expect(result.current.result).toBe(undefined); + + act(() => resolve('result')); + await waitForNextUpdate(); + + expect(result.current.loading).toBe(false); + expect(result.current.result).toBe('result'); + }); }); diff --git a/x-pack/plugins/lists/public/common/hooks/use_async.ts b/x-pack/plugins/lists/public/common/hooks/use_async.ts index 362cad069b7ea..2f4d611a4a73a 100644 --- a/x-pack/plugins/lists/public/common/hooks/use_async.ts +++ b/x-pack/plugins/lists/public/common/hooks/use_async.ts @@ -32,6 +32,8 @@ export const useAsync = ( const start = useCallback( (...args: Args) => { setLoading(true); + setResult(undefined); + setError(undefined); fn(...args) .then((r) => isMounted() && setResult(r)) .catch((e) => isMounted() && setError(e)) diff --git a/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/modal.tsx b/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/modal.tsx index d7d4be6d951b8..dc72260439090 100644 --- a/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/modal.tsx +++ b/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/modal.tsx @@ -46,6 +46,7 @@ export const ValueListsModalComponent: React.FC = ({ const { start: findLists, ...lists } = useFindLists(); const { start: deleteList, result: deleteResult } = useDeleteList(); const [exportListId, setExportListId] = useState(); + const [deletingListIds, setDeletingListIds] = useState([]); const { addError, addSuccess } = useAppToasts(); const fetchLists = useCallback(() => { @@ -54,16 +55,18 @@ export const ValueListsModalComponent: React.FC = ({ const handleDelete = useCallback( ({ id }: { id: string }) => { + setDeletingListIds([...deletingListIds, id]); deleteList({ http, id }); }, - [deleteList, http] + [deleteList, deletingListIds, http] ); useEffect(() => { - if (deleteResult != null) { + if (deleteResult != null && deletingListIds.length > 0) { + setDeletingListIds([...deletingListIds.filter((id) => id !== deleteResult.id)]); fetchLists(); } - }, [deleteResult, fetchLists]); + }, [deleteResult, deletingListIds, fetchLists]); const handleExport = useCallback( async ({ ids }: { ids: string[] }) => @@ -116,6 +119,12 @@ export const ValueListsModalComponent: React.FC = ({ return null; } + const tableItems = (lists.result?.data ?? []).map((item) => ({ + ...item, + isExporting: item.id === exportListId, + isDeleting: deletingListIds.includes(item.id), + })); + const pagination = { pageIndex, pageSize, @@ -133,7 +142,7 @@ export const ValueListsModalComponent: React.FC = ({ + lists.map((list) => ({ + ...list, + isDeleting: false, + isExporting: false, + })); describe('ValueListsTable', () => { it('renders a row for each list', () => { const lists = Array(3).fill(getListResponseMock()); + const items = buildItems(lists); const container = mount( { it('calls onChange when pagination is modified', () => { const lists = Array(6).fill(getListResponseMock()); + const items = buildItems(lists); const onChange = jest.fn(); const container = mount( { it('calls onExport when export is clicked', () => { const lists = Array(3).fill(getListResponseMock()); + const items = buildItems(lists); const onExport = jest.fn(); const container = mount( { it('calls onDelete when delete is clicked', () => { const lists = Array(3).fill(getListResponseMock()); + const items = buildItems(lists); const onDelete = jest.fn(); const container = mount( ; -type ActionCallback = (item: ListSchema) => void; +import { buildColumns } from './table_helpers'; +import { TableProps, TableItemCallback } from './types'; export interface ValueListsTableProps { - lists: TableProps['items']; + items: TableProps['items']; loading: boolean; onChange: TableProps['onChange']; - onExport: ActionCallback; - onDelete: ActionCallback; + onExport: TableItemCallback; + onDelete: TableItemCallback; pagination: Exclude; } -const buildColumns = ( - onExport: ActionCallback, - onDelete: ActionCallback -): TableProps['columns'] => [ - { - field: 'name', - name: i18n.COLUMN_FILE_NAME, - truncateText: true, - }, - { - field: 'created_at', - name: i18n.COLUMN_UPLOAD_DATE, - /* eslint-disable-next-line react/display-name */ - render: (value: ListSchema['created_at']) => ( - - ), - width: '30%', - }, - { - field: 'created_by', - name: i18n.COLUMN_CREATED_BY, - truncateText: true, - width: '20%', - }, - { - name: i18n.COLUMN_ACTIONS, - actions: [ - { - name: i18n.ACTION_EXPORT_NAME, - description: i18n.ACTION_EXPORT_DESCRIPTION, - icon: 'exportAction', - type: 'icon', - onClick: onExport, - 'data-test-subj': 'action-export-value-list', - }, - { - name: i18n.ACTION_DELETE_NAME, - description: i18n.ACTION_DELETE_DESCRIPTION, - icon: 'trash', - type: 'icon', - onClick: onDelete, - 'data-test-subj': 'action-delete-value-list', - }, - ], - width: '15%', - }, -]; - export const ValueListsTableComponent: React.FC = ({ - lists, + items, loading, onChange, onExport, @@ -87,7 +36,7 @@ export const ValueListsTableComponent: React.FC = ({ theme.eui.euiSizeXS}; + vertical-align: middle; +`; + +const ActionButton: React.FC<{ + content: string; + dataTestSubj: string; + icon: IconType; + isLoading: boolean; + item: TableItem; + onClick: TableItemCallback; +}> = ({ content, dataTestSubj, icon, item, onClick, isLoading }) => ( + + {isLoading ? ( + + ) : ( + onClick(item)} + /> + )} + +); + +export const buildColumns = ( + onExport: TableItemCallback, + onDelete: TableItemCallback +): TableProps['columns'] => [ + { + field: 'name', + name: i18n.COLUMN_FILE_NAME, + truncateText: true, + }, + { + field: 'created_at', + name: i18n.COLUMN_UPLOAD_DATE, + render: (value: ListSchema['created_at']) => ( + + ), + width: '30%', + }, + { + field: 'created_by', + name: i18n.COLUMN_CREATED_BY, + truncateText: true, + width: '20%', + }, + { + name: i18n.COLUMN_ACTIONS, + actions: [ + { + render: (item) => ( + + ), + }, + { + render: (item) => ( + + ), + }, + ], + width: '15%', + }, +]; diff --git a/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/types.ts b/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/types.ts new file mode 100644 index 0000000000000..f85e275247728 --- /dev/null +++ b/x-pack/plugins/security_solution/public/detections/components/value_lists_management_modal/types.ts @@ -0,0 +1,16 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { EuiBasicTableProps } from '@elastic/eui'; + +import { ListSchema } from '../../../../../lists/common/schemas/response'; + +export interface TableItem extends ListSchema { + isDeleting: boolean; + isExporting: boolean; +} +export type TableProps = EuiBasicTableProps; +export type TableItemCallback = (item: TableItem) => void;