Skip to content

Commit

Permalink
[Files] Files components to return whole fileJSON instead of just `…
Browse files Browse the repository at this point in the history
…id` (#145126)

Changes files components (`FileUpload`, `FilePicker`) so that
their API returns the whole `fileJSON` instead of just and `id` (and
`kind`)

This is needed, for example, in image emebeddable to also get an
blurHash after uploading or picking an image without fetching the whole
file again.
  • Loading branch information
Dosant committed Nov 15, 2022
1 parent cac8843 commit 768a56a
Show file tree
Hide file tree
Showing 13 changed files with 73 additions and 45 deletions.
2 changes: 1 addition & 1 deletion examples/files_example/public/components/file_picker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export const MyFilePicker: FunctionComponent<Props> = ({ onClose, onDone, onUplo
<FilePicker
kind={exampleFileKind.id}
onClose={onClose}
onDone={onDone}
onDone={(files) => onDone(files.map((f) => f.id))}
onUpload={(n) => onUpload(n.map(({ id }) => id))}
pageSize={50}
multiple
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export const EmptyPrompt: FunctionComponent<Props> = ({ kind, multiple }) => {
immediate
multiple={multiple}
onDone={(file) => {
state.selectFile(file.map(({ id }) => id));
state.selectFile(file.map(({ fileJSON }) => fileJSON));
state.retry();
}}
/>,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ export const FileCard: FunctionComponent<Props> = ({ file }) => {
paddingSize="s"
selectable={{
isSelected,
onClick: () => (isSelected ? state.unselectFile(file.id) : state.selectFile(file.id)),
onClick: () => (isSelected ? state.unselectFile(file.id) : state.selectFile(file)),
}}
image={
<div
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ export const ModalFooter: FunctionComponent<Props> = ({ kind, onDone, onUpload,
>
<UploadFile
onDone={(n) => {
state.selectFile(n.map(({ id }) => id));
state.selectFile(n.map(({ fileJSON }) => fileJSON));
state.resetFilters();
onUpload?.(n);
}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,16 @@ import React from 'react';
import { useBehaviorSubject } from '../../use_behavior_subject';
import { useFilePickerContext } from '../context';
import { i18nTexts } from '../i18n_texts';
import type { FileJSON } from '../../../../common';

export interface Props {
onClick: (selectedFiles: string[]) => void;
onClick: (selectedFiles: FileJSON[]) => void;
}

export const SelectButton: FunctionComponent<Props> = ({ onClick }) => {
const { state } = useFilePickerContext();
const isUploading = useBehaviorSubject(state.isUploading$);
const selectedFiles = useBehaviorSubject(state.selectedFileIds$);
const selectedFiles = useBehaviorSubject(state.selectedFiles$);
return (
<EuiButton
data-test-subj="selectButton"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ describe('FilePicker', () => {
expect(find(testSubjects.selectButton).props().disabled).toBe(false);
actions.done();
expect(onDone).toHaveBeenCalledTimes(1);
expect(onDone).toHaveBeenNthCalledWith(1, ['a', 'b']);
expect(onDone).toHaveBeenNthCalledWith(1, [{ id: 'a' }, { id: 'b' }]);
});
it('hides pagination if there are no files', async () => {
client.list.mockImplementation(() => Promise.resolve({ files: [] as FileJSON[], total: 2 }));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import { ModalFooter } from './components/modal_footer';

import './file_picker.scss';
import { ClearFilterButton } from './components/clear_filter_button';
import type { FileJSON } from '../../../common';

export interface Props<Kind extends string = string> {
/**
Expand All @@ -44,9 +45,9 @@ export interface Props<Kind extends string = string> {
/**
* Will be called after a user has a selected a set of files
*/
onDone: (fileIds: string[]) => void;
onDone: (files: FileJSON[]) => void;
/**
* When a user has succesfully uploaded some files this callback will be called
* When a user has successfully uploaded some files this callback will be called
*/
onUpload?: (done: DoneNotification[]) => void;
/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,9 @@ describe('FilePickerState', () => {
});
it('updates when files are added', () => {
getTestScheduler().run(({ expectObservable, cold, flush }) => {
const addFiles$ = cold('--a-b|').pipe(tap((id) => filePickerState.selectFile(id)));
const addFiles$ = cold('--a-b|').pipe(
tap((id) => filePickerState.selectFile({ id } as FileJSON))
);
expectObservable(addFiles$).toBe('--a-b|');
expectObservable(filePickerState.selectedFileIds$).toBe('a-b-c-', {
a: [],
Expand All @@ -54,7 +56,9 @@ describe('FilePickerState', () => {
});
it('adds files simultaneously as one update', () => {
getTestScheduler().run(({ expectObservable, cold, flush }) => {
const addFiles$ = cold('--a|').pipe(tap(() => filePickerState.selectFile(['1', '2', '3'])));
const addFiles$ = cold('--a|').pipe(
tap(() => filePickerState.selectFile([{ id: '1' }, { id: '2' }, { id: '3' }] as FileJSON[]))
);
expectObservable(addFiles$).toBe('--a|');
expectObservable(filePickerState.selectedFileIds$).toBe('a-b-', {
a: [],
Expand All @@ -67,7 +71,9 @@ describe('FilePickerState', () => {
});
it('updates when files are removed', () => {
getTestScheduler().run(({ expectObservable, cold, flush }) => {
const addFiles$ = cold(' --a-b---c|').pipe(tap((id) => filePickerState.selectFile(id)));
const addFiles$ = cold(' --a-b---c|').pipe(
tap((id) => filePickerState.selectFile({ id } as FileJSON))
);
const removeFiles$ = cold('------a|').pipe(tap((id) => filePickerState.unselectFile(id)));
expectObservable(merge(addFiles$, removeFiles$)).toBe('--a-b-a-c|');
expectObservable(filePickerState.selectedFileIds$).toBe('a-b-c-d-e-', {
Expand All @@ -84,7 +90,9 @@ describe('FilePickerState', () => {
});
it('does not add duplicates', () => {
getTestScheduler().run(({ expectObservable, cold, flush }) => {
const addFiles$ = cold('--a-b-a-a-a|').pipe(tap((id) => filePickerState.selectFile(id)));
const addFiles$ = cold('--a-b-a-a-a|').pipe(
tap((id) => filePickerState.selectFile({ id } as FileJSON))
);
expectObservable(addFiles$).toBe('--a-b-a-a-a|');
expectObservable(filePickerState.selectedFileIds$).toBe('a-b-c-d-e-f-', {
a: [],
Expand Down Expand Up @@ -192,9 +200,9 @@ describe('FilePickerState', () => {
});
});
it('allows only one file to be selected', () => {
filePickerState.selectFile('a');
filePickerState.selectFile({ id: 'a' } as FileJSON);
expect(filePickerState.getSelectedFileIds()).toEqual(['a']);
filePickerState.selectFile(['b', 'a', 'c']);
filePickerState.selectFile([{ id: 'b' }, { id: 'a' }, { id: 'c' }] as FileJSON[]);
expect(filePickerState.getSelectedFileIds()).toEqual(['b']);
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,10 @@ export class FilePickerState {
/**
* Files the user has selected
*/
public readonly selectedFileIds$ = new BehaviorSubject<string[]>([]);
public readonly selectedFiles$ = new BehaviorSubject<FileJSON[]>([]);
public readonly selectedFileIds$ = this.selectedFiles$.pipe(
map((files) => files.map((file) => file.id))
);

public readonly isLoading$ = new BehaviorSubject<boolean>(true);
public readonly loadingError$ = new BehaviorSubject<undefined | Error>(undefined);
Expand All @@ -44,11 +47,7 @@ export class FilePickerState {
public readonly totalPages$ = new BehaviorSubject<undefined | number>(undefined);
public readonly isUploading$ = new BehaviorSubject<boolean>(false);

/**
* This is how we keep a deduplicated list of file ids representing files a user
* has selected
*/
private readonly fileSet = new Set<string>();
private readonly selectedFiles = new Map<string, FileJSON>();
private readonly retry$ = new BehaviorSubject<void>(undefined);
private readonly subscriptions: Subscription[] = [];
private readonly internalIsLoading$ = new BehaviorSubject<boolean>(true);
Expand Down Expand Up @@ -84,7 +83,6 @@ export class FilePickerState {
* easily be passed to all relevant UI.
*
* @note This is not explicitly kept in sync with the selected files!
* @note This is not explicitly kept in sync with the selected files!
*/
public readonly files$ = this.requests$.pipe(
switchMap(([page, query]) => this.sendRequest(page, query)),
Expand All @@ -99,7 +97,7 @@ export class FilePickerState {
};

private sendNextSelectedFiles() {
this.selectedFileIds$.next(this.getSelectedFileIds());
this.selectedFiles$.next(Array.from(this.selectedFiles.values()));
}

private setIsLoading(value: boolean) {
Expand All @@ -110,13 +108,13 @@ export class FilePickerState {
* If multiple selection is not configured, this will take the first file id
* if an array of file ids was provided.
*/
public selectFile = (fileId: string | string[]): void => {
const fileIds = Array.isArray(fileId) ? fileId : [fileId];
public selectFile = (file: FileJSON | FileJSON[]): void => {
const files = Array.isArray(file) ? file : [file];
if (!this.selectMultiple) {
this.fileSet.clear();
this.fileSet.add(fileIds[0]);
this.selectedFiles.clear();
this.selectedFiles.set(files[0].id, files[0]);
} else {
for (const id of fileIds) this.fileSet.add(id);
for (const f of files) this.selectedFiles.set(f.id, f);
}
this.sendNextSelectedFiles();
};
Expand Down Expand Up @@ -182,19 +180,19 @@ export class FilePickerState {
};

public hasFilesSelected = (): boolean => {
return this.fileSet.size > 0;
return this.selectedFiles.size > 0;
};

public unselectFile = (fileId: string): void => {
if (this.fileSet.delete(fileId)) this.sendNextSelectedFiles();
if (this.selectedFiles.delete(fileId)) this.sendNextSelectedFiles();
};

public isFileIdSelected = (fileId: string): boolean => {
return this.fileSet.has(fileId);
return this.selectedFiles.has(fileId);
};

public getSelectedFileIds = (): string[] => {
return Array.from(this.fileSet);
return Array.from(this.selectedFiles.keys());
};

public setQuery = (query: undefined | string): void => {
Expand All @@ -216,8 +214,8 @@ export class FilePickerState {
};

watchFileSelected$ = (id: string): Observable<boolean> => {
return this.selectedFileIds$.pipe(
map(() => this.fileSet.has(id)),
return this.selectedFiles$.pipe(
map(() => this.selectedFiles.has(id)),
distinctUntilChanged()
);
};
Expand Down
15 changes: 11 additions & 4 deletions src/plugins/files/public/components/upload_file/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,24 @@
* Side Public License, v 1.
*/

import React, { lazy, Suspense } from 'react';
import React, { lazy, Suspense, ReactNode } from 'react';
import { EuiLoadingSpinner } from '@elastic/eui';
import type { Props } from './upload_file';

export type { DoneNotification } from './upload_state';
export type { Props as UploadFileProps };

export type UploadFileProps = Props & {
/**
* A custom fallback for when component is lazy loading,
* If not provided, <EuiLoadingSpinner /> is used
*/
lazyLoadFallback?: ReactNode;
};

const UploadFileContainer = lazy(() => import('./upload_file'));

export const UploadFile = (props: Props) => (
<Suspense fallback={<EuiLoadingSpinner size="xl" />}>
export const UploadFile = (props: UploadFileProps) => (
<Suspense fallback={props.lazyLoadFallback ?? <EuiLoadingSpinner size="xl" />}>
<UploadFileContainer {...props} />
</Suspense>
);
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,12 @@ import { useFilesContext } from '../context';
import { UploadFile as Component } from './upload_file.component';
import { createUploadState } from './upload_state';
import { context } from './context';
import type { FileJSON } from '../../../common';

/**
* An object representing an uploadded file
* An object representing an uploaded file
*/
interface UploadedFile {
interface UploadedFile<Meta = unknown> {
/**
* The ID that was generated for the uploaded file
*/
Expand All @@ -27,6 +28,10 @@ interface UploadedFile {
* The kind of the file that was passed in to this component
*/
kind: string;
/**
* Attributes of a file that represent a serialised version of the file.
*/
fileJSON: FileJSON<Meta>;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,8 @@ describe('UploadState', () => {
{ file: file2, status: 'uploading' },
],
c: [
{ file: file1, status: 'uploaded', id: 'test' },
{ file: file2, status: 'uploaded', id: 'test' },
{ file: file1, status: 'uploaded', id: 'test', fileJSON: { id: 'test' } },
{ file: file2, status: 'uploaded', id: 'test', fileJSON: { id: 'test' } },
],
});

Expand Down
14 changes: 11 additions & 3 deletions src/plugins/files/public/components/upload_file/upload_state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,16 @@ interface FileState {
file: File;
status: 'idle' | 'uploading' | 'uploaded' | 'upload_failed';
id?: string;
fileJSON?: FileJSON;
error?: Error;
}

type Upload = SimpleStateSubject<FileState>;

export interface DoneNotification {
export interface DoneNotification<Meta = unknown> {
id: string;
kind: string;
fileJSON: FileJSON<Meta>;
}

interface UploadOptions {
Expand Down Expand Up @@ -97,7 +99,13 @@ export class UploadState {
filter(
(files) => Boolean(files.length) && files.every((file) => file.status === 'uploaded')
),
map((files) => files.map((file) => ({ id: file.id!, kind: this.fileKind.id })))
map((files) =>
files.map((file) => ({
id: file.id!,
kind: this.fileKind.id,
fileJSON: file.fileJSON!,
}))
)
)
.subscribe(this.done$),
];
Expand Down Expand Up @@ -205,7 +213,7 @@ export class UploadState {
);
}),
map(() => {
file$.setState({ status: 'uploaded', id: uploadTarget?.id });
file$.setState({ status: 'uploaded', id: uploadTarget?.id, fileJSON: uploadTarget });
}),
catchError((e) => {
const isAbortError = e.message === 'Abort!';
Expand Down

0 comments on commit 768a56a

Please sign in to comment.