Skip to content

Commit

Permalink
[Security Solution][Detections] Adds loading states to export/delete …
Browse files Browse the repository at this point in the history
…on modal (elastic#72562) (elastic#72734)

* 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.
  • Loading branch information
rylnd authored Jul 21, 2020
1 parent b563297 commit e0f017b
Show file tree
Hide file tree
Showing 7 changed files with 185 additions and 67 deletions.
32 changes: 32 additions & 0 deletions x-pack/plugins/lists/public/common/hooks/use_async.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});
});
2 changes: 2 additions & 0 deletions x-pack/plugins/lists/public/common/hooks/use_async.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ export const useAsync = <Args extends unknown[], Result>(
const start = useCallback(
(...args: Args) => {
setLoading(true);
setResult(undefined);
setError(undefined);
fn(...args)
.then((r) => isMounted() && setResult(r))
.catch((e) => isMounted() && setError(e))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ export const ValueListsModalComponent: React.FC<ValueListsModalProps> = ({
const { start: findLists, ...lists } = useFindLists();
const { start: deleteList, result: deleteResult } = useDeleteList();
const [exportListId, setExportListId] = useState<string>();
const [deletingListIds, setDeletingListIds] = useState<string[]>([]);
const { addError, addSuccess } = useAppToasts();

const fetchLists = useCallback(() => {
Expand All @@ -54,16 +55,18 @@ export const ValueListsModalComponent: React.FC<ValueListsModalProps> = ({

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[] }) =>
Expand Down Expand Up @@ -116,6 +119,12 @@ export const ValueListsModalComponent: React.FC<ValueListsModalProps> = ({
return null;
}

const tableItems = (lists.result?.data ?? []).map((item) => ({
...item,
isExporting: item.id === exportListId,
isDeleting: deletingListIds.includes(item.id),
}));

const pagination = {
pageIndex,
pageSize,
Expand All @@ -133,7 +142,7 @@ export const ValueListsModalComponent: React.FC<ValueListsModalProps> = ({
<ValueListsForm onSuccess={handleUploadSuccess} onError={handleUploadError} />
<EuiSpacer />
<ValueListsTable
lists={lists.result?.data ?? []}
items={tableItems}
loading={lists.loading}
onDelete={handleDelete}
onExport={handleExportClick}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,23 @@ import { getListResponseMock } from '../../../../../lists/common/schemas/respons
import { ListSchema } from '../../../../../lists/common/schemas/response';
import { TestProviders } from '../../../common/mock';
import { ValueListsTable } from './table';
import { TableItem } from './types';

const buildItems = (lists: ListSchema[]): TableItem[] =>
lists.map((list) => ({
...list,
isDeleting: false,
isExporting: false,
}));

describe('ValueListsTable', () => {
it('renders a row for each list', () => {
const lists = Array<ListSchema>(3).fill(getListResponseMock());
const items = buildItems(lists);
const container = mount(
<TestProviders>
<ValueListsTable
lists={lists}
items={items}
onChange={jest.fn()}
loading={false}
onExport={jest.fn()}
Expand All @@ -34,11 +43,12 @@ describe('ValueListsTable', () => {

it('calls onChange when pagination is modified', () => {
const lists = Array<ListSchema>(6).fill(getListResponseMock());
const items = buildItems(lists);
const onChange = jest.fn();
const container = mount(
<TestProviders>
<ValueListsTable
lists={lists}
items={items}
onChange={onChange}
loading={false}
onExport={jest.fn()}
Expand All @@ -59,11 +69,12 @@ describe('ValueListsTable', () => {

it('calls onExport when export is clicked', () => {
const lists = Array<ListSchema>(3).fill(getListResponseMock());
const items = buildItems(lists);
const onExport = jest.fn();
const container = mount(
<TestProviders>
<ValueListsTable
lists={lists}
items={items}
onChange={jest.fn()}
loading={false}
onExport={onExport}
Expand All @@ -86,11 +97,12 @@ describe('ValueListsTable', () => {

it('calls onDelete when delete is clicked', () => {
const lists = Array<ListSchema>(3).fill(getListResponseMock());
const items = buildItems(lists);
const onDelete = jest.fn();
const container = mount(
<TestProviders>
<ValueListsTable
lists={lists}
items={items}
onChange={jest.fn()}
loading={false}
onExport={jest.fn()}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,74 +5,23 @@
*/

import React from 'react';
import { EuiBasicTable, EuiBasicTableProps, EuiText, EuiPanel } from '@elastic/eui';
import { EuiBasicTable, EuiText, EuiPanel } from '@elastic/eui';

import { ListSchema } from '../../../../../lists/common/schemas/response';
import { FormattedDate } from '../../../common/components/formatted_date';
import * as i18n from './translations';

type TableProps = EuiBasicTableProps<ListSchema>;
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<TableProps['pagination'], undefined>;
}

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']) => (
<FormattedDate value={value} fieldName="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<ValueListsTableProps> = ({
lists,
items,
loading,
onChange,
onExport,
Expand All @@ -87,7 +36,7 @@ export const ValueListsTableComponent: React.FC<ValueListsTableProps> = ({
</EuiText>
<EuiBasicTable
columns={columns}
items={lists}
items={items}
loading={loading}
onChange={onChange}
pagination={pagination}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
/*
* 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.
*/

/* eslint-disable react/display-name */

import React from 'react';
import styled from 'styled-components';
import { EuiButtonIcon, IconType, EuiLoadingSpinner, EuiToolTip } from '@elastic/eui';

import { ListSchema } from '../../../../../lists/common/schemas/response';
import { FormattedDate } from '../../../common/components/formatted_date';
import * as i18n from './translations';
import { TableItem, TableItemCallback, TableProps } from './types';

const AlignedSpinner = styled(EuiLoadingSpinner)`
margin: ${({ theme }) => 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 }) => (
<EuiToolTip content={content}>
{isLoading ? (
<AlignedSpinner size="m" />
) : (
<EuiButtonIcon
aria-label={content}
data-test-subj={dataTestSubj}
iconType={icon}
onClick={() => onClick(item)}
/>
)}
</EuiToolTip>
);

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']) => (
<FormattedDate value={value} fieldName="created_at" />
),
width: '30%',
},
{
field: 'created_by',
name: i18n.COLUMN_CREATED_BY,
truncateText: true,
width: '20%',
},
{
name: i18n.COLUMN_ACTIONS,
actions: [
{
render: (item) => (
<ActionButton
content={i18n.ACTION_EXPORT_DESCRIPTION}
dataTestSubj="action-export-value-list"
icon="exportAction"
item={item}
onClick={onExport}
isLoading={item.isExporting}
/>
),
},
{
render: (item) => (
<ActionButton
content={i18n.ACTION_DELETE_DESCRIPTION}
dataTestSubj="action-delete-value-list"
icon="trash"
item={item}
onClick={onDelete}
isLoading={item.isDeleting}
/>
),
},
],
width: '15%',
},
];
Original file line number Diff line number Diff line change
@@ -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<TableItem>;
export type TableItemCallback = (item: TableItem) => void;

0 comments on commit e0f017b

Please sign in to comment.