Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions .changeset/moody-monkeys-swim.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
"@radix-ui/react-avatar": minor
"radix-ui": minor
---

Fixed several edge cases with Avatar's loading state:
- An avatar's fallback would not be displayed again if its image component unmounted. This is now fixed.
- Rendering multiple `Avatar.Image` components per `Avatar.Root` was never supported and results in buggy, unpredictable behavior. We now warn about this in development.
- Zero-sized images were treated as `loading`, meaning that `onLoadingStatusChange` is never called once loaded. A zero-sized image now triggers an `error` status on load.
123 changes: 122 additions & 1 deletion packages/react/avatar/src/avatar.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,26 @@ describe('given an Avatar with fallback and an image', () => {
expect(image).not.toBeInTheDocument();
});

it('should render the fallback again after a loaded image unmounts', async () => {
const conditionalUi = (showImage: boolean) => (
<Avatar.Root data-testid={ROOT_TEST_ID}>
<Avatar.Fallback>{FALLBACK_TEXT}</Avatar.Fallback>
{showImage ? <Avatar.Image src="/test.png" alt={IMAGE_ALT_TEXT} /> : null}
</Avatar.Root>
);
rendered.unmount();
rendered = render(conditionalUi(true));

image = await rendered.findByRole('img');
expect(image).toBeInTheDocument();
expect(rendered.queryByText(FALLBACK_TEXT)).not.toBeInTheDocument();

rendered.rerender(conditionalUi(false));
image = rendered.queryByRole('img');
expect(image).not.toBeInTheDocument();
expect(rendered.queryByText(FALLBACK_TEXT)).toBeInTheDocument();
});

it('should show fallback if image has no data', async () => {
rendered.unmount();
const spy = vi.spyOn(window.Image.prototype, 'naturalWidth', 'get');
Expand All @@ -142,6 +162,63 @@ describe('given an Avatar with fallback and an image', () => {
spy.mockRestore();
});

it('should show the fallback if a loaded image reports no natural size', async () => {
rendered.unmount();
const spy = vi.spyOn(window.Image.prototype, 'naturalWidth', 'get').mockReturnValue(0);
const onLoadingStatusChange = vi.fn();
rendered = render(
<Avatar.Root data-testid={ROOT_TEST_ID}>
<Avatar.Fallback>{FALLBACK_TEXT}</Avatar.Fallback>
<Avatar.Image
src="/no-size.png"
alt={IMAGE_ALT_TEXT}
onLoadingStatusChange={onLoadingStatusChange}
/>
</Avatar.Root>,
);

// The image dispatches `load`, but because its natural size is 0 we treat it as an error.
await waitFor(() => expect(onLoadingStatusChange).toHaveBeenCalledWith('error'));
expect(rendered.queryByRole('img')).not.toBeInTheDocument();
expect(rendered.queryByText(FALLBACK_TEXT)).toBeInTheDocument();
spy.mockRestore();
});

describe('onLoadingStatusChange', () => {
const renderWithCallback = (src?: string) => {
const onLoadingStatusChange = vi.fn();
rendered.unmount();
rendered = render(
<Avatar.Root data-testid={ROOT_TEST_ID}>
<Avatar.Fallback>{FALLBACK_TEXT}</Avatar.Fallback>
<Avatar.Image
src={src}
alt={IMAGE_ALT_TEXT}
onLoadingStatusChange={onLoadingStatusChange}
/>
</Avatar.Root>,
);
return onLoadingStatusChange;
};

it('is called with "loaded" once the image loads', async () => {
const onLoadingStatusChange = renderWithCallback('/test.png');
await rendered.findByRole('img');
expect(onLoadingStatusChange).toHaveBeenCalledWith('loaded');
});

it('is called with "error" when there is no src', async () => {
const onLoadingStatusChange = renderWithCallback();
await waitFor(() => expect(onLoadingStatusChange).toHaveBeenCalledWith('error'));
});

it('is not called with "idle" while the image is mounted', async () => {
const onLoadingStatusChange = renderWithCallback('/test.png');
await rendered.findByRole('img');
expect(onLoadingStatusChange).not.toHaveBeenCalledWith('idle');
});
});

describe('SSR', () => {
afterEach(cleanup);

Expand Down Expand Up @@ -215,6 +292,8 @@ describe('given an Avatar with an image that only works when referrerPolicy=no-r
onSrcChange() {
setTimeout(() => {
if (this.referrerPolicy === 'no-referrer') {
// Mirror real browsers: mark the image complete before `load` fires.
cache.add(this.src);
this.dispatchEvent(new Event('load'));
} else {
this.dispatchEvent(new Event('error'));
Expand Down Expand Up @@ -283,6 +362,46 @@ describe('given an Avatar with an image that only works when referrerPolicy=no-r
});
});

describe('given an Avatar with multiple images (development)', () => {
let warnSpy: ReturnType<typeof vi.spyOn>;

beforeEach(() => {
vi.stubEnv('NODE_ENV', 'development');
warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
});

afterEach(() => {
cleanup();
vi.unstubAllEnvs();
vi.restoreAllMocks();
});

it('warns that multiple images are not supported', async () => {
render(
<Avatar.Root data-testid={ROOT_TEST_ID}>
<Avatar.Fallback>{FALLBACK_TEXT}</Avatar.Fallback>
<Avatar.Image src="/a.png" alt="a" />
<Avatar.Image src="/b.png" alt="b" />
</Avatar.Root>,
);

await waitFor(() =>
expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('multiple were detected')),
);
});

it('does not warn for a single image', () => {
render(
<Avatar.Root data-testid={ROOT_TEST_ID}>
<Avatar.Fallback>{FALLBACK_TEXT}</Avatar.Fallback>
<Avatar.Image src="/a.png" alt="a" />
</Avatar.Root>,
);

expect(warnSpy).not.toHaveBeenCalled();
});
});

class MockImage extends EventTarget {
_src: string = '';

Expand Down Expand Up @@ -313,8 +432,10 @@ class MockImage extends EventTarget {

onSrcChange() {
setTimeout(() => {
this.dispatchEvent(new Event('load'));
// Mirror real browsers: the image is `complete` (and `naturalWidth` is
// populated) before the `load` event fires.
cache.add(this.src);
this.dispatchEvent(new Event('load'));
}, DELAY);
}
}
152 changes: 96 additions & 56 deletions packages/react/avatar/src/avatar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import { createContextScope } from '@radix-ui/react-context';
import { useCallbackRef } from '@radix-ui/react-use-callback-ref';
import { useLayoutEffect } from '@radix-ui/react-use-layout-effect';
import { Primitive } from '@radix-ui/react-primitive';
import { useIsHydrated } from '@radix-ui/react-use-is-hydrated';

import type { Scope } from '@radix-ui/react-context';

Expand All @@ -20,9 +19,16 @@ type ImageLoadingStatus = 'idle' | 'loading' | 'loaded' | 'error';

type AvatarContextValue = {
imageLoadingStatus: ImageLoadingStatus;
onImageLoadingStatusChange(status: ImageLoadingStatus): void;
setImageLoadingStatus: React.Dispatch<React.SetStateAction<ImageLoadingStatus>>;
imageCount: number;
setImageCount: React.Dispatch<React.SetStateAction<number>>;
};

const STATIC_IMAGE_COUNT_STATE: [number, React.Dispatch<React.SetStateAction<number>>] = [
0,
() => void 0,
];

const [AvatarProvider, useAvatarContext] = createAvatarContext<AvatarContextValue>(AVATAR_NAME);

type AvatarElement = React.ComponentRef<typeof Primitive.span>;
Expand All @@ -33,11 +39,15 @@ const Avatar = React.forwardRef<AvatarElement, AvatarProps>(
(props: ScopedProps<AvatarProps>, forwardedRef) => {
const { __scopeAvatar, ...avatarProps } = props;
const [imageLoadingStatus, setImageLoadingStatus] = React.useState<ImageLoadingStatus>('idle');
const [imageCount, setImageCount] = useImageCount();

return (
<AvatarProvider
scope={__scopeAvatar}
imageLoadingStatus={imageLoadingStatus}
onImageLoadingStatusChange={setImageLoadingStatus}
setImageLoadingStatus={setImageLoadingStatus}
imageCount={imageCount}
setImageCount={setImageCount}
>
<Primitive.span {...avatarProps} ref={forwardedRef} />
</AvatarProvider>
Expand All @@ -61,16 +71,27 @@ interface AvatarImageProps extends PrimitiveImageProps {

const AvatarImage = React.forwardRef<AvatarImageElement, AvatarImageProps>(
(props: ScopedProps<AvatarImageProps>, forwardedRef) => {
const { __scopeAvatar, src, onLoadingStatusChange = () => {}, ...imageProps } = props;
const { __scopeAvatar, src, onLoadingStatusChange, ...imageProps } = props;
const context = useAvatarContext(IMAGE_NAME, __scopeAvatar);
const imageLoadingStatus = useImageLoadingStatus(src, imageProps);
useUpdateImageCount(context.setImageCount);

const imageLoadingStatus = useImageLoadingStatus(src, {
referrerPolicy: imageProps.referrerPolicy,
crossOrigin: imageProps.crossOrigin,
loadingStatus: context.imageLoadingStatus,
setLoadingStatus: context.setImageLoadingStatus,
});
const handleLoadingStatusChange = useCallbackRef((status: ImageLoadingStatus) => {
onLoadingStatusChange(status);
context.onImageLoadingStatusChange(status);
onLoadingStatusChange?.(status);
});

const loadingStatusRef = React.useRef<ImageLoadingStatus>(imageLoadingStatus);

useLayoutEffect(() => {
if (imageLoadingStatus !== 'idle') {
const previousLoadingStatus = loadingStatusRef.current;
loadingStatusRef.current = imageLoadingStatus;

if (imageLoadingStatus !== previousLoadingStatus) {
handleLoadingStatusChange(imageLoadingStatus);
}
}, [imageLoadingStatus, handleLoadingStatusChange]);
Expand Down Expand Up @@ -117,71 +138,90 @@ AvatarFallback.displayName = FALLBACK_NAME;

/* -----------------------------------------------------------------------------------------------*/

function resolveLoadingStatus(image: HTMLImageElement | null, src?: string): ImageLoadingStatus {
if (!image) {
return 'idle';
}
if (!src) {
return 'error';
}
if (image.src !== src) {
image.src = src;
}
return image.complete && image.naturalWidth > 0 ? 'loaded' : 'loading';
}

function useImageLoadingStatus(
src: string | undefined,
{ referrerPolicy, crossOrigin }: AvatarImageProps,
{
loadingStatus,
setLoadingStatus,
referrerPolicy,
crossOrigin,
}: {
referrerPolicy: React.HTMLAttributeReferrerPolicy | undefined;
crossOrigin: string | undefined;
loadingStatus: ImageLoadingStatus;
setLoadingStatus: React.Dispatch<React.SetStateAction<ImageLoadingStatus>>;
},
) {
const isHydrated = useIsHydrated();
const imageRef = React.useRef<HTMLImageElement | null>(null);
const image = (() => {
if (!isHydrated) return null;
if (!imageRef.current) {
imageRef.current = new window.Image();
}
return imageRef.current;
})();

const [loadingStatus, setLoadingStatus] = React.useState<ImageLoadingStatus>(() =>
resolveLoadingStatus(image, src),
);

useLayoutEffect(() => {
setLoadingStatus(resolveLoadingStatus(image, src));
}, [image, src]);
if (!src) {
setLoadingStatus('error');
return;
}

useLayoutEffect(() => {
const updateStatus = (status: ImageLoadingStatus) => () => {
setLoadingStatus(status);
const image = new window.Image();
const handleLoad = (event: Event) => {
const image = event.currentTarget as HTMLImageElement;
setLoadingStatus(getImageLoadingStatus(image));
};

if (!image) return;

const handleLoad = updateStatus('loaded');
const handleError = updateStatus('error');
const handleError = () => setLoadingStatus('error');
image.addEventListener('load', handleLoad);
image.addEventListener('error', handleError);
if (referrerPolicy) {
image.referrerPolicy = referrerPolicy;
}
if (typeof crossOrigin === 'string') {
image.crossOrigin = crossOrigin;
}
image.crossOrigin = crossOrigin ?? null;
image.src = src;

setLoadingStatus(getImageLoadingStatus(image));
return () => {
image.removeEventListener('load', handleLoad);
image.removeEventListener('error', handleError);
setLoadingStatus('idle');
};
}, [image, crossOrigin, referrerPolicy]);
}, [src, crossOrigin, referrerPolicy, setLoadingStatus]);

return loadingStatus;
}

const Root = Avatar;
const Image = AvatarImage;
const Fallback = AvatarFallback;
function getImageLoadingStatus(image: HTMLImageElement) {
return image.complete ? (image.naturalWidth > 0 ? 'loaded' : 'error') : 'loading';
}

// Image count is only used in development to warn about multiple images, which
// is unsupported. Gating behind a development flag to avoid performance
// overhead in production is safe because useState is guaranteed to run
// consistently in the same environment.
// oxlint-disable react-hooks/rules-of-hooks
function useImageCount() {
let state = STATIC_IMAGE_COUNT_STATE;
if (process.env.NODE_ENV === 'development') {
state = React.useState(0);
const [imageCount] = state;
const hasWarnedRef = React.useRef(false);
React.useEffect(() => {
if (imageCount > 1 && !hasWarnedRef.current) {
hasWarnedRef.current = true;
console.warn(
'Avatar: Only one `Avatar.Image` component should be rendered per `Avatar.Root`, but multiple were detected. This will lead to unexpected behavior.',
);
}
}, [imageCount]);
}

return state;
}

function useUpdateImageCount(setImageCount: React.Dispatch<React.SetStateAction<number>>) {
if (process.env.NODE_ENV === 'development') {
React.useEffect(() => {
setImageCount((imageCount) => imageCount + 1);
return () => {
setImageCount((imageCount) => imageCount - 1);
};
}, [setImageCount]);
}
}
// oxlint-enable react-hooks/rules-of-hooks

export {
createAvatarScope,
Expand All @@ -190,8 +230,8 @@ export {
AvatarImage,
AvatarFallback,
//
Root,
Image,
Fallback,
Avatar as Root,
AvatarImage as Image,
AvatarFallback as Fallback,
};
export type { AvatarProps, AvatarImageProps, AvatarFallbackProps };
Loading