Skip to content

Commit 3eb2e55

Browse files
refactor(Thumbnail): queue for expensive file operations
BREAKING CHANGE: removed throttle and isShownOnLoad props as they are no longer required due to ability to cancel position in global queue Co-authored-by: Liam Ross <[email protected]>
1 parent ec869cb commit 3eb2e55

File tree

12 files changed

+232
-86
lines changed

12 files changed

+232
-86
lines changed

src/components/FileOrganizer/FileOrganizer.stories.tsx

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,10 @@ export default {
2020
interface TemplateProps {
2121
onRenderDragLayer?: boolean;
2222
numFiles?: number;
23-
lazy?: boolean;
2423
editable?: boolean;
2524
}
2625

27-
const Template: FC<TemplateProps> = ({ onRenderDragLayer, numFiles = 2, lazy, editable }) => {
26+
const Template: FC<TemplateProps> = ({ onRenderDragLayer, numFiles = 2, editable }) => {
2827
// This is the index organizing function.
2928
const [files, setFiles] = useState<FakeFile[]>([]);
3029
const handleOnMove = useCallback<NonNullable<FileOrganizerProps<FakeFile>['onMove']>>((fromIndex, toIndex) => {
@@ -47,13 +46,13 @@ const Template: FC<TemplateProps> = ({ onRenderDragLayer, numFiles = 2, lazy, ed
4746
if (prev.length < numFiles) {
4847
const newFiles = [];
4948
for (let index = prev.length; index < numFiles; index++) {
50-
newFiles.push(createFile(index, { lazy }));
49+
newFiles.push(createFile(index));
5150
}
5251
return [...prev, ...newFiles];
5352
}
5453
return prev;
5554
});
56-
}, [numFiles, lazy]);
55+
}, [numFiles]);
5756

5857
return (
5958
<FileOrganizer
@@ -100,11 +99,10 @@ export const WithCustomDragLayer = () => {
10099
const numFiles = number('number of files', 2, { min: 0, max: 16, step: 1, range: true });
101100
return <Template numFiles={numFiles} onRenderDragLayer />;
102101
};
103-
export const StressTestWithLazyThumbnails = () => <Template lazy numFiles={1000} />;
104102

105103
export const UseManagedFilesHook = () => {
106104
const { fileOrganizerProps, getThumbnailSelectionProps, draggingIds } = useManagedFiles({
107-
initialFiles: Array.from({ length: 100 }, (_, index) => createFile(index)),
105+
initialFiles: Array.from({ length: 1000 }, (_, index) => createFile(index)),
108106
preventMultiDrag: boolean('preventMultiDrag', false, 'useManagedFiles options'),
109107
preventMultiSelect: boolean('preventMultiSelect', false, 'useManagedFiles options'),
110108
preventDeselectOnDragOther: boolean('preventDeselectOnDragOther', false, 'useManagedFiles options'),

src/components/FileOrganizer/FileOrganizer.tsx

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -113,8 +113,6 @@ export interface OnRenderThumbnailProps<F> {
113113
otherDragging: boolean;
114114
/** Callback for setting whether the thumbnail is in editing mode. */
115115
onEditingChange: (isEditing: boolean) => void;
116-
/** Is this thumbnail shown on initial load. Use to turn off throttling. */
117-
isShownOnLoad: boolean;
118116
};
119117
/** ID of the file. */
120118
id: string;
@@ -172,24 +170,6 @@ export function FileOrganizer<F extends ObjectWithId>({
172170
[onDragChange],
173171
);
174172

175-
// Whenever files changes, detect which items are in view and prevent
176-
// throttling. Set as array of IDs to notify onRenderThumbnail.
177-
const [initialShownIds, setInitialShownIds] = useState<string[]>([]);
178-
useEffect(() => {
179-
if (files) {
180-
requestAnimationFrame(() => {
181-
if (!fileOrganizerRef.current) return;
182-
const itemsInView = fileOrganizerRef.current.querySelectorAll('div[draggable="true"]');
183-
const ids: string[] = [];
184-
itemsInView.forEach((draggableItem) => {
185-
const dataFileId = draggableItem.getAttribute('data-file-id');
186-
if (dataFileId) ids.push(dataFileId);
187-
});
188-
setInitialShownIds(ids);
189-
});
190-
}
191-
}, [files]);
192-
193173
const handleItemKeyDown = useCallback(
194174
(event: KeyboardEvent<HTMLDivElement>, index: number, _file: F, draggableRef: RefObject<HTMLDivElement>) => {
195175
let indexDiff = 0;
@@ -292,7 +272,6 @@ export function FileOrganizer<F extends ObjectWithId>({
292272
dragging: isDragging || isInDragGroup,
293273
otherDragging,
294274
onEditingChange: (editing) => setEditingId(editing ? file.id : undefined),
295-
isShownOnLoad: !initialShownIds || initialShownIds.includes(file.id),
296275
},
297276
id: file.id,
298277
index,
@@ -312,7 +291,6 @@ export function FileOrganizer<F extends ObjectWithId>({
312291
handleOnDragChange,
313292
handleItemKeyDown,
314293
onRenderThumbnail,
315-
initialShownIds,
316294
],
317295
);
318296

src/components/Thumbnail/Thumbnail.stories.tsx

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,20 +26,18 @@ const defaultProps = (options?: CreateFileOptions, index = 0, withToolButtons?:
2626

2727
export const Basic = () => <Thumbnail {...defaultProps()} />;
2828

29-
export const Throttled = () => <Thumbnail {...defaultProps({ lazy: true })} />;
30-
31-
export const Pending = () => <Thumbnail {...defaultProps({ pending: true })} />;
29+
export const Expensive = () => <Thumbnail {...defaultProps({ lazy: true })} />;
3230

3331
export const Rejected = () => <Thumbnail {...defaultProps({ error: true })} />;
3432

3533
export const WithToolButtons = () => <Thumbnail {...defaultProps(undefined, undefined, true)} />;
3634

3735
export const WithLabel = () => <Thumbnail label={text('label', 'some_label')} {...defaultProps()} />;
3836

39-
export const WithSelectedIcon = () => (
37+
export const SelectedIcon = () => (
4038
<Thumbnail
4139
{...defaultProps()}
42-
selectedIcon={<div style={{ color: 'red', fontSize: 20 }}>{text('selectedIcon', 'Selected Icon')}</div>}
40+
selectedIcon={<div style={{ color: 'red', fontSize: 20 }}>{text('selectedIcon', 'CUSTOM!')}</div>}
4341
/>
4442
);
4543

src/components/Thumbnail/Thumbnail.test.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { spy } from 'sinon';
44
import { createFile } from '../../storybook-helpers/data/files';
55
import { Thumbnail } from '../Thumbnail';
66

7-
const testFile = createFile(0, { pending: true });
7+
const testFile = createFile(0);
88

99
describe('Thumbnail component', () => {
1010
it('renders its contents', () => {

src/components/Thumbnail/Thumbnail.tsx

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -47,15 +47,6 @@ export interface ThumbnailProps<F> extends ClickableDivProps {
4747
* A node to render on the top-left of the thumbnail.
4848
*/
4949
selectedIcon?: ReactNode;
50-
/**
51-
* Default 500ms. Set to 0 to prevent all throttling. Set the delay for
52-
* fetching any lazy async items from file.
53-
*/
54-
throttle?: number;
55-
/**
56-
* If true, will prevent throttling.
57-
*/
58-
isShownOnLoad?: boolean;
5950
/**
6051
* Class for the thumbnail image.
6152
*/
@@ -84,8 +75,6 @@ export function Thumbnail<F extends FileLike>({
8475
selectedIcon,
8576
onRename,
8677
onEditingChange,
87-
throttle,
88-
isShownOnLoad,
8978
imageClassName,
9079
className,
9180
disabled,
@@ -99,8 +88,7 @@ export function Thumbnail<F extends FileLike>({
9988

10089
const { focused, handleOnFocus, handleOnBlur } = useFocus(onFocus, onBlur);
10190

102-
const toThrottle = isShownOnLoad ? 0 : throttle;
103-
const [thumbnail, thumbnailErr] = useFileSubscribe(file, (f) => f.thumbnail, 'onthumbnailchange', toThrottle);
91+
const [thumbnail, thumbnailErr] = useFileSubscribe(file, (f) => f.thumbnail, 'onthumbnailchange');
10492
const [name] = useFileSubscribe(file, (f) => f.name, 'onnamechange');
10593

10694
const handleOnSave = (newName: string) => {

src/data/file.ts

Lines changed: 48 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,16 @@ export interface FileDetails {
4949
* license will take priority over the global license.
5050
*/
5151
license?: string;
52+
/**
53+
* A reference to the document that was used to create this File class. Used as an optimization
54+
* where applicable. If passed, 'pageIndex' must also be passed
55+
*/
56+
fullDocumentObj?: CoreControls.Document;
57+
/**
58+
* Used in conjunction with 'fullDocumentObj'. Represents the pageIndex of 'fullDocumentObj' that this
59+
* file belongs too
60+
*/
61+
pageIndex?: number;
5262
}
5363

5464
export interface FileLike {
@@ -60,6 +70,8 @@ export interface FileLike {
6070
fileObj: MemoizedPromise<Blob>;
6171
documentObj: MemoizedPromise<CoreControls.Document>;
6272
subscribe: (...args: any) => () => void;
73+
fullDocumentObj?: CoreControls.Document;
74+
pageIndex?: number;
6375
}
6476

6577
export type FileEventType =
@@ -96,19 +108,40 @@ export class File implements FileLike {
96108
private _freezeThumbnail: boolean;
97109
private _subscribers: FileEventListenersObj;
98110
private _license?: string;
111+
private _fullDocumentObj?: CoreControls.Document;
112+
private _pageIndex?: number;
99113

100114
/**
101115
* Initialize the `File`.
102116
* @param fileDetails The file details object or file-like class to initialize
103117
* this `File` with.
104118
*/
105119
constructor(fileDetails: FileDetails) {
106-
const { name, id, originalName, extension, fileObj, documentObj, thumbnail, license } = fileDetails;
120+
const {
121+
name,
122+
id,
123+
originalName,
124+
extension,
125+
fileObj,
126+
documentObj,
127+
thumbnail,
128+
license,
129+
fullDocumentObj,
130+
pageIndex,
131+
} = fileDetails;
107132

108133
if (!fileObj && !documentObj) {
109134
throw new Error('One of `fileObj` or `documentObj` is required to initialize File.');
110135
}
111136

137+
if (fullDocumentObj) {
138+
if (typeof pageIndex !== 'number') {
139+
throw new Error('"pageIndex" must be passed if using "fullDocumentObj"');
140+
}
141+
this._fullDocumentObj = fullDocumentObj;
142+
this._pageIndex = pageIndex;
143+
}
144+
112145
this._id = id || getStringId('file');
113146
this._name = name;
114147
this._originalName = originalName || name;
@@ -164,6 +197,16 @@ export class File implements FileLike {
164197
return this._documentObj;
165198
}
166199

200+
/** Gets the full document object if provided during initialization. */
201+
get fullDocumentObj() {
202+
return this._fullDocumentObj;
203+
}
204+
205+
/** Gets the page index if provided during initialization. */
206+
get pageIndex() {
207+
return this._pageIndex;
208+
}
209+
167210
/** The name of the file. */
168211
get name() {
169212
return this._name;
@@ -299,7 +342,10 @@ export class File implements FileLike {
299342

300343
/** Generate a thumbnail from document object. */
301344
private _generateThumbnail = () => {
302-
return getThumbnail(this.documentObj.get(), this.extension);
345+
return getThumbnail(this.fullDocumentObj || this.documentObj.get(), {
346+
extension: this.extension,
347+
pageIndex: this.pageIndex,
348+
});
303349
};
304350

305351
/** Generate a file object from document object. */

src/hooks/useFile.ts

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,13 +32,12 @@ interface FileHook<F> {
3232
* This hook converts a file class with async values into a React-friendly hook
3333
* with async values set to undefined until they are fetched.
3434
* @param file The file to convert to react observable values.
35-
* @param throttle The timeout if unfetched memo promise.
3635
*/
37-
export function useFile<F extends FileLike>(file: F, throttle?: number): FileHook<F> {
38-
const [name, nameErr] = useFileSubscribe(file, (f) => f.name, 'onnamechange', throttle);
39-
const [thumbnail, thumbnailErr] = useFileSubscribe(file, (f) => f.thumbnail, 'onthumbnailchange', throttle);
40-
const [fileObj, fileObjErr] = useFileSubscribe(file, (f) => f.fileObj, 'onfileobjchange', throttle);
41-
const [documentObj, documentObjErr] = useFileSubscribe(file, (f) => f.documentObj, 'ondocumentobjchange', throttle);
36+
export function useFile<F extends FileLike>(file: F): FileHook<F> {
37+
const [name, nameErr] = useFileSubscribe(file, (f) => f.name, 'onnamechange');
38+
const [thumbnail, thumbnailErr] = useFileSubscribe(file, (f) => f.thumbnail, 'onthumbnailchange');
39+
const [fileObj, fileObjErr] = useFileSubscribe(file, (f) => f.fileObj, 'onfileobjchange');
40+
const [documentObj, documentObjErr] = useFileSubscribe(file, (f) => f.documentObj, 'ondocumentobjchange');
4241

4342
const fileValue = useMemo<FileHook<F>>(
4443
() => ({

src/hooks/useFileSubscribe.ts

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { useEffect, useMemo, useRef, useState } from 'react';
22
import { FileEventType, FileLike, MemoizedPromise } from '../data';
3-
import { DEFAULT_THROTTLE_TIMEOUT } from '../utils';
3+
import GlobalQueue from '../work/GlobalQueue';
44

55
/**
66
* Will subscribe to a value from a file and return the value, as well as any
@@ -14,7 +14,6 @@ export function useFileSubscribe<F extends FileLike, T>(
1414
file: F,
1515
getCurrentValue: (file: F) => T | MemoizedPromise<T>,
1616
eventType?: FileEventType,
17-
throttle?: number,
1817
) {
1918
const getValue = useRef(getCurrentValue);
2019

@@ -27,18 +26,17 @@ export function useFileSubscribe<F extends FileLike, T>(
2726

2827
useEffect(() => {
2928
let cancelled = false;
30-
let timeout: number;
29+
let cancel: () => void;
3130

32-
const setMemoValue = async (newValue: MemoizedPromise<T>) => {
31+
const setMemoValue = (val: T) => {
3332
try {
34-
const val = await newValue.get();
3533
if (!cancelled) setValue(val);
3634
} catch (error) {
3735
if (!cancelled) setError(error);
3836
}
3937
};
4038

41-
const subscribe = (delay?: boolean) => {
39+
const subscribe = () => {
4240
setError(undefined);
4341

4442
const val = getValue.current(file);
@@ -49,27 +47,34 @@ export function useFileSubscribe<F extends FileLike, T>(
4947
}
5048

5149
setValue(undefined);
52-
if (!delay || val.done || throttle === 0) {
53-
setMemoValue(val);
50+
51+
if (val.done) {
52+
val.get().then(setMemoValue);
5453
} else {
55-
timeout = window.setTimeout(() => {
56-
setMemoValue(val);
57-
}, throttle ?? DEFAULT_THROTTLE_TIMEOUT);
54+
const r = GlobalQueue.process<T>(() => val.get());
55+
cancel = r[1];
56+
r[0]
57+
.then((result: T) => {
58+
setMemoValue(result);
59+
})
60+
.catch((e) => {
61+
setError(e);
62+
});
5863
}
5964
};
6065

61-
subscribe(true);
66+
subscribe();
6267

6368
let unsubscribe: (() => void) | undefined;
6469

6570
if (eventType) unsubscribe = file.subscribe(eventType, subscribe);
6671

6772
return () => {
73+
cancel?.();
6874
unsubscribe?.();
6975
cancelled = true;
70-
clearTimeout(timeout);
7176
};
72-
}, [eventType, file, throttle]);
77+
}, [eventType, file]);
7378

7479
return useMemo(() => [value, error, setValue], [error, value]);
7580
}

src/storybook-helpers/data/files.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,7 @@ import testPdfThumbnailRotated from '../images/pdf-preview-2.png';
55
import testPdfThumbnail from '../images/pdf-preview.png';
66

77
export interface CreateFileOptions {
8-
pending?: boolean;
9-
lazy?: boolean;
8+
lazy?: boolean; // Expensive operation
109
error?: boolean;
1110
}
1211

@@ -22,6 +21,8 @@ export class FakeFile implements FileLike {
2221
thumbnail: MemoizedPromise<string>;
2322
fileObj: MemoizedPromise<Blob>;
2423
documentObj: MemoizedPromise<CoreControls.Document>;
24+
fullDocumentObj: CoreControls.Document | undefined;
25+
pageIndex: number | undefined;
2526

2627
constructor(index: number, options: CreateFileOptions = {}) {
2728
this.id = `file_${index + 1}`;
@@ -35,9 +36,8 @@ export class FakeFile implements FileLike {
3536

3637
private _getParameter<T>(parameter: T, options: CreateFileOptions) {
3738
const internals = (() => {
38-
if (options.pending) return async () => new Promise(() => {});
39-
if (options.lazy) return async () => Promise.resolve(parameter);
40-
if (options.error) return () => Promise.reject('Some error.');
39+
if (options.lazy) return () => new Promise((res) => setTimeout(() => res(parameter), 500));
40+
if (options.error) return () => new Promise((_res, rej) => setTimeout(() => rej('Some error.'), 500));
4141
return parameter;
4242
})();
4343
return new MemoizedPromise(internals as FuturableOrLazy<T>);

src/utils/debugUtils.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
export function logExecTime(tag: string): () => number {
2+
const now = performance.now();
3+
return () => {
4+
const now2 = performance.now();
5+
console.log(`${tag} took ${now2 - now}ms`);
6+
return now2 - now;
7+
};
8+
}

0 commit comments

Comments
 (0)