From f56c95ac930cdcd6ed7a83071cfb8c83e0fa38f9 Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Mon, 24 Oct 2022 11:44:48 +0200 Subject: [PATCH 1/8] react with loading spinner to new page or retry requests --- .../components/file_picker/file_picker_state.ts | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/x-pack/plugins/files/public/components/file_picker/file_picker_state.ts b/x-pack/plugins/files/public/components/file_picker/file_picker_state.ts index 3ca6ee9ffca99..697f1fc58188d 100644 --- a/x-pack/plugins/files/public/components/file_picker/file_picker_state.ts +++ b/x-pack/plugins/files/public/components/file_picker/file_picker_state.ts @@ -61,12 +61,19 @@ export class FilePickerState { distinctUntilChanged() ) .subscribe(this.hasQuery$), + this.requests$.pipe(tap(() => this.setIsLoading(true))).subscribe(), this.internalIsLoading$ .pipe(debounceTime(100), distinctUntilChanged()) .subscribe(this.isLoading$), ]; } + private readonly requests$ = combineLatest([ + this.currentPage$.pipe(distinctUntilChanged()), + this.query$.pipe(distinctUntilChanged(), debounceTime(100)), + this.retry$, + ]); + /** * File objects we have loaded on the front end, stored here so that it can * easily be passed to all relevant UI. @@ -74,11 +81,7 @@ export class FilePickerState { * @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$ = combineLatest([ - this.currentPage$.pipe(distinctUntilChanged()), - this.query$.pipe(distinctUntilChanged(), debounceTime(100)), - this.retry$, - ]).pipe( + public readonly files$ = this.requests$.pipe( switchMap(([page, query]) => this.sendRequest(page, query)), tap(({ total }) => this.updateTotalPages({ total })), tap(({ total }) => this.hasFiles$.next(Boolean(total))), From 8305305199a978db270a68248046913989dfca34 Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Mon, 24 Oct 2022 12:08:14 +0200 Subject: [PATCH 2/8] do not fetch on every change? in files example app --- x-pack/examples/files_example/public/components/app.tsx | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/x-pack/examples/files_example/public/components/app.tsx b/x-pack/examples/files_example/public/components/app.tsx index d3dfbdeb71874..afdf8be1f4f6e 100644 --- a/x-pack/examples/files_example/public/components/app.tsx +++ b/x-pack/examples/files_example/public/components/app.tsx @@ -36,8 +36,10 @@ interface FilesExampleAppDeps { type ListResponse = FilesClientResponses['list']; export const FilesExampleApp = ({ files, notifications }: FilesExampleAppDeps) => { - const { data, isLoading, error, refetch } = useQuery(['files'], () => - files.example.list() + const { data, isLoading, error, refetch } = useQuery( + ['files'], + () => files.example.list(), + { refetchOnWindowFocus: false } ); const [showUploadModal, setShowUploadModal] = useState(false); const [showFilePickerModal, setShowFilePickerModal] = useState(false); From 7965f51c2b11899695eac55f0496fea8f73e4495 Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Mon, 24 Oct 2022 12:14:37 +0200 Subject: [PATCH 3/8] remove lazy from the file picker image --- .../public/components/file_picker/components/file_card.tsx | 5 ----- 1 file changed, 5 deletions(-) diff --git a/x-pack/plugins/files/public/components/file_picker/components/file_card.tsx b/x-pack/plugins/files/public/components/file_picker/components/file_card.tsx index 4c290b1b114e7..88c77f36a6c00 100644 --- a/x-pack/plugins/files/public/components/file_picker/components/file_card.tsx +++ b/x-pack/plugins/files/public/components/file_picker/components/file_card.tsx @@ -58,11 +58,6 @@ export const FileCard: FunctionComponent = ({ file }) => { `} meta={file.meta as FileImageMetadata} src={client.getDownloadHref({ id: file.id, fileKind: kind })} - // There is an issue where the intersection observer does not fire reliably. - // I'm not sure if this is becuause of the image being in a modal - // The result is that the image does not always get loaded. - // TODO: Investigate this behaviour further - lazy={false} /> ) : (
Date: Mon, 24 Oct 2022 12:15:17 +0200 Subject: [PATCH 4/8] absolute position the image tag to ensure that it is visible, this ensures the intersection observer will fire --- .../files/public/components/image/components/img.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/x-pack/plugins/files/public/components/image/components/img.tsx b/x-pack/plugins/files/public/components/image/components/img.tsx index 295b062ca1fd8..37cf745c8c687 100644 --- a/x-pack/plugins/files/public/components/image/components/img.tsx +++ b/x-pack/plugins/files/public/components/image/components/img.tsx @@ -12,20 +12,20 @@ import { css } from '@emotion/react'; import { sizes } from '../styles'; export interface Props extends ImgHTMLAttributes { - hidden: boolean; size?: EuiImageSize; observerRef: (el: null | HTMLImageElement) => void; } export const Img = React.forwardRef( - ({ observerRef, src, hidden, size, ...rest }, ref) => { + ({ observerRef, src, size, ...rest }, ref) => { const { euiTheme } = useEuiTheme(); const styles = [ css` transition: opacity ${euiTheme.animation.extraFast}; `, - hidden + !src ? css` + position: absolute; visibility: hidden; ` : undefined, From d4e365d976ed8255a42174724ee357a2a6e28564 Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Mon, 24 Oct 2022 12:15:33 +0200 Subject: [PATCH 5/8] remove the "lazy" prop from the image component --- .../files/public/components/image/image.tsx | 27 ++----------------- 1 file changed, 2 insertions(+), 25 deletions(-) diff --git a/x-pack/plugins/files/public/components/image/image.tsx b/x-pack/plugins/files/public/components/image/image.tsx index b83739d180c94..7ef70c17d6fd4 100644 --- a/x-pack/plugins/files/public/components/image/image.tsx +++ b/x-pack/plugins/files/public/components/image/image.tsx @@ -32,15 +32,6 @@ export interface Props extends ImgHTMLAttributes { * Emits when the image first becomes visible */ onFirstVisible?: () => void; - - /** - * As an optimisation images are only loaded when they are visible. - * This setting overrides this behavior and loads an image as soon as the - * component mounts. - * - * @default true - */ - lazy?: boolean; } /** @@ -55,26 +46,13 @@ export interface Props extends ImgHTMLAttributes { */ export const Image = React.forwardRef( ( - { - src, - alt, - onFirstVisible, - onLoad, - onError, - meta, - wrapperProps, - size = 'original', - lazy = true, - ...rest - }, + { src, alt, onFirstVisible, onLoad, onError, meta, wrapperProps, size = 'original', ...rest }, ref ) => { const [isLoaded, setIsLoaded] = useState(false); const [blurDelayExpired, setBlurDelayExpired] = useState(false); const { isVisible, ref: observerRef } = useViewportObserver({ onFirstVisible }); - const loadImage = lazy ? isVisible : true; - useEffect(() => { let unmounted = false; const id = window.setTimeout(() => { @@ -112,8 +90,7 @@ export const Image = React.forwardRef( observerRef={observerRef} ref={ref} size={size} - hidden={!loadImage} - src={loadImage ? src : undefined} + src={isVisible ? src : undefined} alt={alt} onLoad={(ev) => { setIsLoaded(true); From e4e48b0ecf158969c8bb51a0956b81645499f0e4 Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Mon, 24 Oct 2022 12:16:38 +0200 Subject: [PATCH 6/8] remove unnecessary if --- x-pack/plugins/files/public/components/image/image.tsx | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/x-pack/plugins/files/public/components/image/image.tsx b/x-pack/plugins/files/public/components/image/image.tsx index 7ef70c17d6fd4..e353fced3ec3e 100644 --- a/x-pack/plugins/files/public/components/image/image.tsx +++ b/x-pack/plugins/files/public/components/image/image.tsx @@ -54,12 +54,10 @@ export const Image = React.forwardRef( const { isVisible, ref: observerRef } = useViewportObserver({ onFirstVisible }); useEffect(() => { - let unmounted = false; const id = window.setTimeout(() => { - if (!unmounted) setBlurDelayExpired(true); + setBlurDelayExpired(true); }, 200); return () => { - unmounted = true; window.clearTimeout(id); }; }, []); From 54c53ceb17ff33f2d56b43dbb2bad989d7b7b98f Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Mon, 24 Oct 2022 12:20:58 +0200 Subject: [PATCH 7/8] added comment and increase page size of file picker in files example plugin --- .../examples/files_example/public/components/file_picker.tsx | 2 +- .../plugins/files/public/components/image/components/img.tsx | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/x-pack/examples/files_example/public/components/file_picker.tsx b/x-pack/examples/files_example/public/components/file_picker.tsx index 2bf5530655ba3..3c2178b299ea2 100644 --- a/x-pack/examples/files_example/public/components/file_picker.tsx +++ b/x-pack/examples/files_example/public/components/file_picker.tsx @@ -18,5 +18,5 @@ interface Props { } export const MyFilePicker: FunctionComponent = ({ onClose, onDone }) => { - return ; + return ; }; diff --git a/x-pack/plugins/files/public/components/image/components/img.tsx b/x-pack/plugins/files/public/components/image/components/img.tsx index 37cf745c8c687..9088cc86cac31 100644 --- a/x-pack/plugins/files/public/components/image/components/img.tsx +++ b/x-pack/plugins/files/public/components/image/components/img.tsx @@ -25,7 +25,8 @@ export const Img = React.forwardRef( `, !src ? css` - position: absolute; + position: absolute; // ensure that the image is visible at the top + top: 0; visibility: hidden; ` : undefined, From be1053338184c7a7a0c10b61c174b902f7c2cadf Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Mon, 24 Oct 2022 13:01:01 +0200 Subject: [PATCH 8/8] make the empty "hidden" image occupy the full container space --- .../files/public/components/image/components/img.tsx | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/files/public/components/image/components/img.tsx b/x-pack/plugins/files/public/components/image/components/img.tsx index 9088cc86cac31..953eb93a19917 100644 --- a/x-pack/plugins/files/public/components/image/components/img.tsx +++ b/x-pack/plugins/files/public/components/image/components/img.tsx @@ -25,9 +25,12 @@ export const Img = React.forwardRef( `, !src ? css` - position: absolute; // ensure that the image is visible at the top - top: 0; visibility: hidden; + position: absolute; // ensure that empty img tag occupies full container + top: 0; + right: 0; + bottom: 0; + left: 0; ` : undefined, size ? sizes[size] : undefined,