Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Files] Fix image intersection observer behavior #143843

Merged
merged 8 commits into from
Oct 25, 2022
6 changes: 4 additions & 2 deletions x-pack/examples/files_example/public/components/app.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,10 @@ interface FilesExampleAppDeps {
type ListResponse = FilesClientResponses<MyImageMetadata>['list'];

export const FilesExampleApp = ({ files, notifications }: FilesExampleAppDeps) => {
const { data, isLoading, error, refetch } = useQuery<ListResponse>(['files'], () =>
files.example.list()
const { data, isLoading, error, refetch } = useQuery<ListResponse>(
['files'],
() => files.example.list(),
{ refetchOnWindowFocus: false }
);
const [showUploadModal, setShowUploadModal] = useState(false);
const [showFilePickerModal, setShowFilePickerModal] = useState(false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,5 @@ interface Props {
}

export const MyFilePicker: FunctionComponent<Props> = ({ onClose, onDone }) => {
return <FilePicker kind={exampleFileKind.id} onClose={onClose} onDone={onDone} />;
return <FilePicker kind={exampleFileKind.id} onClose={onClose} onDone={onDone} pageSize={50} />;
};
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,6 @@ export const FileCard: FunctionComponent<Props> = ({ 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}
/>
) : (
<div
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,24 +61,27 @@ 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.
*
* @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))),
Expand Down
10 changes: 7 additions & 3 deletions x-pack/plugins/files/public/components/image/components/img.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,21 +12,25 @@ import { css } from '@emotion/react';
import { sizes } from '../styles';

export interface Props extends ImgHTMLAttributes<HTMLImageElement> {
hidden: boolean;
size?: EuiImageSize;
observerRef: (el: null | HTMLImageElement) => void;
}

export const Img = React.forwardRef<HTMLImageElement, Props>(
({ 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`
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,
Expand Down
31 changes: 3 additions & 28 deletions x-pack/plugins/files/public/components/image/image.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,6 @@ export interface Props extends ImgHTMLAttributes<HTMLImageElement> {
* 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;
}

/**
Expand All @@ -55,33 +46,18 @@ export interface Props extends ImgHTMLAttributes<HTMLImageElement> {
*/
export const Image = React.forwardRef<HTMLImageElement, Props>(
(
{
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<boolean>(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(() => {
if (!unmounted) setBlurDelayExpired(true);
setBlurDelayExpired(true);
}, 200);
return () => {
unmounted = true;
window.clearTimeout(id);
};
}, []);
Expand Down Expand Up @@ -112,8 +88,7 @@ export const Image = React.forwardRef<HTMLImageElement, Props>(
observerRef={observerRef}
ref={ref}
size={size}
hidden={!loadImage}
src={loadImage ? src : undefined}
src={isVisible ? src : undefined}
alt={alt}
onLoad={(ev) => {
setIsLoaded(true);
Expand Down