From ba8eeb392b284d8ff71075ad47a9b126bc96277a Mon Sep 17 00:00:00 2001 From: "a.pourarian" Date: Sun, 2 Feb 2025 10:35:24 +0330 Subject: [PATCH 1/6] feat(image): add loading src to display loading state --- .../components/image/customLoading.raw.jsx | 13 +++++++ .../content/components/image/customLoading.ts | 9 +++++ apps/docs/content/docs/components/image.mdx | 19 ++++++++-- packages/components/image/src/image.tsx | 3 +- packages/components/image/src/use-image.ts | 26 ++++++++++---- .../image/stories/image.stories.tsx | 36 ++++++++++++++++--- 6 files changed, 92 insertions(+), 14 deletions(-) create mode 100644 apps/docs/content/components/image/customLoading.raw.jsx create mode 100644 apps/docs/content/components/image/customLoading.ts diff --git a/apps/docs/content/components/image/customLoading.raw.jsx b/apps/docs/content/components/image/customLoading.raw.jsx new file mode 100644 index 0000000000..de4dd0e302 --- /dev/null +++ b/apps/docs/content/components/image/customLoading.raw.jsx @@ -0,0 +1,13 @@ +import {Image} from "@nextui-org/react"; + +export default function App() { + return ( + NextUI Image with custom loading + ); +} diff --git a/apps/docs/content/components/image/customLoading.ts b/apps/docs/content/components/image/customLoading.ts new file mode 100644 index 0000000000..a04ee43971 --- /dev/null +++ b/apps/docs/content/components/image/customLoading.ts @@ -0,0 +1,9 @@ +import App from "./customLoading.raw.jsx?raw"; + +const react = { + "/App.jsx": App, +}; + +export default { + ...react, +}; diff --git a/apps/docs/content/docs/components/image.mdx b/apps/docs/content/docs/components/image.mdx index 97a90a6d91..4354e552e3 100644 --- a/apps/docs/content/docs/components/image.mdx +++ b/apps/docs/content/docs/components/image.mdx @@ -63,17 +63,26 @@ Image component has a built-in `skeleton` animation to indicate the image is loa > **Note**: The `URL` uses `https://app.requestly.io/delay` to simulate a slow network. +### Custom Loading Image + +You can use the `loadingSrc` prop to display a loading image when the image provided in `src` is still loading. + + + +> **Note**: The `URL` uses `https://app.requestly.io/delay` to simulate a slow network. + ### Image with fallback You can use the `fallbackSrc` prop to display a fallback image when: - The `fallbackSrc` prop is provided. -- The image provided in `src` is still loading. - The image provided in `src` fails to load. - The image provided in `src` is not found. +> **Note**: You can have both `loadingSrc` and `fallbackSrc` props to cover multiple possibilities while loading and handling image errors. + ### With Next.js Image Next.js provides an optimized [Image](https://nextjs.org/docs/app/api-reference/components/image) component, @@ -152,11 +161,17 @@ you can use it with HeroUI `Image` component as well. type: "eager | lazy", description: "A loading strategy to use for the image.", default: "-" + }, + { + attribute: "loadingSrc", + type: "string", + description: "The image source to display while the main image is loading. This helps provide visual feedback during the loading process.", + default: "-" }, { attribute: "fallbackSrc", type: "string", - description: "The fallback image source.", + description: "The image source to display when the main image fails to load or encounters an error.", default: "-" }, { diff --git a/packages/components/image/src/image.tsx b/packages/components/image/src/image.tsx index fab736a152..985f1137e2 100644 --- a/packages/components/image/src/image.tsx +++ b/packages/components/image/src/image.tsx @@ -13,6 +13,7 @@ const Image = forwardRef<"img", ImageProps>((props, ref) => { classNames, isBlurred, isZoomed, + loadingSrc, fallbackSrc, removeWrapper, disableSkeleton, @@ -45,7 +46,7 @@ const Image = forwardRef<"img", ImageProps>((props, ref) => { } // when zoomed or showSkeleton, we need to wrap the image - if (isZoomed || !disableSkeleton || fallbackSrc) { + if (isZoomed || !disableSkeleton || loadingSrc || fallbackSrc) { return
{isZoomed ? zoomed : img}
; } diff --git a/packages/components/image/src/use-image.ts b/packages/components/image/src/use-image.ts index 9f761bb6c0..5f0a1421c4 100644 --- a/packages/components/image/src/use-image.ts +++ b/packages/components/image/src/use-image.ts @@ -21,9 +21,13 @@ interface Props extends HTMLHeroUIProps<"img"> { */ isBlurred?: boolean; /** - * A fallback image. + * A fallback image when error encountered. */ fallbackSrc?: React.ReactNode; + /** + * A loading image. + */ + loadingSrc?: React.ReactNode; /** * Whether to disable the loading skeleton. * @default false @@ -82,9 +86,10 @@ export function useImage(originalProps: UseImageProps) { classNames, loading, isBlurred, + loadingSrc, fallbackSrc, isLoading: isLoadingProp, - disableSkeleton = !!fallbackSrc, + disableSkeleton = !!loadingSrc, removeWrapper = false, onError, onLoad, @@ -110,6 +115,7 @@ export function useImage(originalProps: UseImageProps) { const isImgLoaded = imageStatus === "loaded" && !isLoadingProp; const isLoading = imageStatus === "loading" || isLoadingProp; + const isFailed = imageStatus === "failed"; const isZoomed = originalProps.isZoomed; const Component = as || "img"; @@ -131,8 +137,9 @@ export function useImage(originalProps: UseImageProps) { }; }, [props?.width, props?.height]); - const showFallback = (!src || !isImgLoaded) && !!fallbackSrc; - const showSkeleton = isLoading && !disableSkeleton; + const showLoading = isLoading && !!loadingSrc; + const showFallback = (isFailed || !src || !isImgLoaded) && !!fallbackSrc; + const showSkeleton = isLoading && !disableSkeleton && !loadingSrc; const slots = useMemo( () => @@ -170,7 +177,11 @@ export function useImage(originalProps: UseImageProps) { }; const getWrapperProps = useCallback(() => { - const fallbackStyle = showFallback + const wrapperStyle = showLoading + ? { + backgroundImage: `url(${loadingSrc})`, + } + : showFallback && !showSkeleton ? { backgroundImage: `url(${fallbackSrc})`, } @@ -179,11 +190,11 @@ export function useImage(originalProps: UseImageProps) { return { className: slots.wrapper({class: classNames?.wrapper}), style: { - ...fallbackStyle, + ...wrapperStyle, maxWidth: w, }, }; - }, [slots, showFallback, fallbackSrc, classNames?.wrapper, w]); + }, [slots, showLoading, showFallback, showSkeleton, fallbackSrc, classNames?.wrapper, w]); const getBlurredImgProps = useCallback(() => { return { @@ -200,6 +211,7 @@ export function useImage(originalProps: UseImageProps) { classNames, isBlurred, disableSkeleton, + loadingSrc, fallbackSrc, removeWrapper, isZoomed, diff --git a/packages/components/image/stories/image.stories.tsx b/packages/components/image/stories/image.stories.tsx index 53ea412568..4a199c5886 100644 --- a/packages/components/image/stories/image.stories.tsx +++ b/packages/components/image/stories/image.stories.tsx @@ -114,6 +114,19 @@ export const Shadow = { }, }; +export const Skeleton = { + render: LoadingTemplate, + + args: { + ...defaultProps, + width: 300, + height: 450, + radius: "lg", + src: "https://app.requestly.io/delay/3000/https://images.unsplash.com/photo-1494790108377-be9c29b29330", + disableSkeleton: false, + }, +}; + export const AnimatedLoad = { args: { ...defaultProps, @@ -123,7 +136,7 @@ export const AnimatedLoad = { }, }; -export const Fallback = { +export const CustomLoading = { render: LoadingTemplate, args: { @@ -131,11 +144,11 @@ export const Fallback = { width: 300, radius: "lg", src: "https://app.requestly.io/delay/3000/https://images.unsplash.com/photo-1539571696357-5a69c17a67c6", - fallbackSrc: "/images/placeholder_300x450.png", + loadingSrc: "/images/placeholder_300x450.png", }, }; -export const Skeleton = { +export const Fallback = { render: LoadingTemplate, args: { @@ -143,7 +156,22 @@ export const Skeleton = { width: 300, height: 450, radius: "lg", - src: "https://app.requestly.io/delay/3000/https://images.unsplash.com/photo-1494790108377-be9c29b29330", + src: "wrong-src-address", + fallbackSrc: "/images/placeholder_300x450.png", disableSkeleton: false, }, }; + +export const CustomLoadingAndFallback = { + render: LoadingTemplate, + + args: { + ...defaultProps, + width: 300, + height: 450, + radius: "lg", + src: "wrong-src-address", + loadingSrc: "/images/placeholder_300x450.png", + fallbackSrc: "/images/local-image-small.jpg", + }, +}; From 33053b4dcc5e03706a24fea04c968521ffa40e0e Mon Sep 17 00:00:00 2001 From: "a.pourarian" Date: Sun, 2 Feb 2025 16:49:58 +0330 Subject: [PATCH 2/6] fix(image): fix sb issues --- apps/docs/content/components/image/customLoading.raw.jsx | 4 ++-- apps/docs/content/components/image/fallback.raw.jsx | 2 +- apps/docs/content/components/image/index.ts | 2 ++ apps/docs/content/docs/components/image.mdx | 5 ++--- 4 files changed, 7 insertions(+), 6 deletions(-) diff --git a/apps/docs/content/components/image/customLoading.raw.jsx b/apps/docs/content/components/image/customLoading.raw.jsx index de4dd0e302..f4056c695b 100644 --- a/apps/docs/content/components/image/customLoading.raw.jsx +++ b/apps/docs/content/components/image/customLoading.raw.jsx @@ -1,9 +1,9 @@ -import {Image} from "@nextui-org/react"; +import {Image} from "@heroui/react"; export default function App() { return ( NextUI Image with custom loading ); diff --git a/apps/docs/content/components/image/index.ts b/apps/docs/content/components/image/index.ts index a7741c384c..de64dcada3 100644 --- a/apps/docs/content/components/image/index.ts +++ b/apps/docs/content/components/image/index.ts @@ -2,6 +2,7 @@ import usage from "./usage"; import blurred from "./blurred"; import zoomed from "./zoomed"; import loading from "./loading"; +import customLoading from "./customLoading"; import fallback from "./fallback"; import nextjs from "./nextjs"; @@ -10,6 +11,7 @@ export const imageContent = { blurred, zoomed, loading, + customLoading, fallback, nextjs, }; diff --git a/apps/docs/content/docs/components/image.mdx b/apps/docs/content/docs/components/image.mdx index 4354e552e3..e864ed2269 100644 --- a/apps/docs/content/docs/components/image.mdx +++ b/apps/docs/content/docs/components/image.mdx @@ -54,10 +54,9 @@ You can use the `isZoomed` prop make the image zoomed when hovered. -### Animated Loading +### Animated (Skeleton) Loading -Image component has a built-in `skeleton` animation to indicate the image is loading and an -`opacity` animation when the image loads. +Image component has a built-in `skeleton` animation to indicate the image is loading in case the `loadingSrc` is not defined. From d763e24e57e349005042fa448f457a6d9d6d91fa Mon Sep 17 00:00:00 2001 From: "a.pourarian" Date: Sun, 2 Feb 2025 16:52:18 +0330 Subject: [PATCH 3/6] test(image): add two cases (second test is not complete) --- .../components/image/__tests__/image.test.tsx | 32 +++++++++++++++++-- packages/components/image/src/image.tsx | 6 +++- 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/packages/components/image/__tests__/image.test.tsx b/packages/components/image/__tests__/image.test.tsx index 69795d3866..7d33943dac 100644 --- a/packages/components/image/__tests__/image.test.tsx +++ b/packages/components/image/__tests__/image.test.tsx @@ -1,10 +1,11 @@ import * as React from "react"; -import {render, act} from "@testing-library/react"; +import {render, act, fireEvent, waitFor} from "@testing-library/react"; import {Image} from "../src"; const src = "https://via.placeholder.com/300x450"; const fallbackSrc = "https://via.placeholder.com/300x450"; +const loadingSrc = "/images/local-image-small.jpg"; describe("Image", () => { it("should render correctly", () => { @@ -26,7 +27,34 @@ describe("Image", () => { expect(wrapper.getByRole("img")).toBeInstanceOf(HTMLImageElement); }); - test("renders image if there is no fallback behavior defined", async () => { + test("renders loading source while loading the image.", async () => { + const wrapper = render(); + const imageParent = wrapper.getByRole("img").parentElement; + + expect(imageParent?.getAttribute("data-testid")).toEqual("heroUI/image_parent"); + + const computedStyle = window.getComputedStyle(imageParent!); + + expect(computedStyle.backgroundImage).toBe(`url(${loadingSrc})`); + }); + + test("renders fallback source if src is wrong or not found.", async () => { + const onError = jest.fn(); + const wrapper = render( + test, + ); + const imageParent = wrapper.getByRole("img").parentElement; + + fireEvent.error(wrapper.getByRole("img")); + await waitFor(() => expect(onError).toHaveBeenCalled(), {timeout: 5_000}); + expect(imageParent?.getAttribute("data-testid")).toEqual("heroUI/image_parent"); + + const computedStyle = window.getComputedStyle(imageParent!); + + expect(computedStyle.backgroundImage).toBe(`url(${fallbackSrc})`); + }, 6_000); + + test("renders image if there is no loading or fallback behavior defined", async () => { const wrapper = render(); expect(wrapper.getByRole("img")).toHaveAttribute("src", src); diff --git a/packages/components/image/src/image.tsx b/packages/components/image/src/image.tsx index 985f1137e2..4651a1dca0 100644 --- a/packages/components/image/src/image.tsx +++ b/packages/components/image/src/image.tsx @@ -47,7 +47,11 @@ const Image = forwardRef<"img", ImageProps>((props, ref) => { // when zoomed or showSkeleton, we need to wrap the image if (isZoomed || !disableSkeleton || loadingSrc || fallbackSrc) { - return
{isZoomed ? zoomed : img}
; + return ( +
+ {isZoomed ? zoomed : img} +
+ ); } return img; From 654dcf36f1497304ae6afb82b53e0dd38c85804f Mon Sep 17 00:00:00 2001 From: Arian Pourarian Date: Sat, 8 Feb 2025 19:29:21 +0330 Subject: [PATCH 4/6] test(image): rectify the 2 tests regarding loading and fallback states --- .../components/image/__tests__/image.test.tsx | 89 +++++++++++++------ 1 file changed, 62 insertions(+), 27 deletions(-) diff --git a/packages/components/image/__tests__/image.test.tsx b/packages/components/image/__tests__/image.test.tsx index 7d33943dac..4e645ff9ed 100644 --- a/packages/components/image/__tests__/image.test.tsx +++ b/packages/components/image/__tests__/image.test.tsx @@ -1,13 +1,31 @@ import * as React from "react"; -import {render, act, fireEvent, waitFor} from "@testing-library/react"; +import {render, act} from "@testing-library/react"; import {Image} from "../src"; const src = "https://via.placeholder.com/300x450"; -const fallbackSrc = "https://via.placeholder.com/300x450"; +const fallbackSrc = "https://via.placeholder.com/300x200"; const loadingSrc = "/images/local-image-small.jpg"; describe("Image", () => { + let imageOnload: any = null; + + beforeAll(() => { + function trackImageOnload() { + Object.defineProperty(window.Image.prototype, "onload", { + get() { + return this._onload; + }, + set(fn) { + imageOnload = fn; + this._onload = fn; + }, + }); + } + + trackImageOnload(); + }); + it("should render correctly", () => { const wrapper = render(); @@ -27,32 +45,65 @@ describe("Image", () => { expect(wrapper.getByRole("img")).toBeInstanceOf(HTMLImageElement); }); - test("renders loading source while loading the image.", async () => { - const wrapper = render(); + test("renders an image while loading the src image. When loading finished, renders the src image.", async () => { + const onLoad = jest.fn(); + const wrapper = render(); const imageParent = wrapper.getByRole("img").parentElement; - expect(imageParent?.getAttribute("data-testid")).toEqual("heroUI/image_parent"); + expect(imageParent).not.toBeNull(); + expect(imageParent!.getAttribute("data-testid")).toEqual("heroUI/image_parent"); - const computedStyle = window.getComputedStyle(imageParent!); + const computedLoadingStyle = window.getComputedStyle(imageParent!); - expect(computedStyle.backgroundImage).toBe(`url(${loadingSrc})`); + expect(computedLoadingStyle.backgroundImage).toBe(`url(${loadingSrc})`); + + act(() => { + imageOnload(); + }); + + const computedLoadedStyle = window.getComputedStyle(imageParent!); + + expect(onLoad).toHaveBeenCalled(); + expect(computedLoadedStyle.backgroundImage).toBe(""); + wrapper.unmount(); }); test("renders fallback source if src is wrong or not found.", async () => { + let imageOnerror: any = null; + + function trackImageOnerror() { + Object.defineProperty(window.Image.prototype, "onerror", { + get() { + return this._onload; + }, + set(fn) { + imageOnerror = fn; + this._onerror = fn; + }, + }); + } + + trackImageOnerror(); + const onError = jest.fn(); const wrapper = render( test, ); const imageParent = wrapper.getByRole("img").parentElement; - fireEvent.error(wrapper.getByRole("img")); - await waitFor(() => expect(onError).toHaveBeenCalled(), {timeout: 5_000}); - expect(imageParent?.getAttribute("data-testid")).toEqual("heroUI/image_parent"); + expect(imageParent).not.toBeNull(); + expect(imageParent!.getAttribute("data-testid")).toEqual("heroUI/image_parent"); + + act(() => { + imageOnerror(); + }); + expect(onError).toHaveBeenCalled(); const computedStyle = window.getComputedStyle(imageParent!); expect(computedStyle.backgroundImage).toBe(`url(${fallbackSrc})`); - }, 6_000); + wrapper.unmount(); + }); test("renders image if there is no loading or fallback behavior defined", async () => { const wrapper = render(); @@ -74,22 +125,6 @@ describe("Image", () => { }); test("should fire onload", () => { - let imageOnload: any = null; - - function trackImageOnload() { - Object.defineProperty(window.Image.prototype, "onload", { - get() { - return this._onload; - }, - set(fn) { - imageOnload = fn; - this._onload = fn; - }, - }); - } - - trackImageOnload(); - const onLoad = jest.fn(); const wrapper = render(); From de08d982c497fa0cc96a7c597bcdeb87dfa03ac5 Mon Sep 17 00:00:00 2001 From: Arian Pourarian Date: Sat, 8 Feb 2025 19:33:38 +0330 Subject: [PATCH 5/6] test(image): improve tests (cleanup renders) --- packages/components/image/__tests__/image.test.tsx | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/packages/components/image/__tests__/image.test.tsx b/packages/components/image/__tests__/image.test.tsx index 4e645ff9ed..387cad2d6b 100644 --- a/packages/components/image/__tests__/image.test.tsx +++ b/packages/components/image/__tests__/image.test.tsx @@ -30,6 +30,7 @@ describe("Image", () => { const wrapper = render(); expect(() => wrapper.unmount()).not.toThrow(); + wrapper.unmount(); }); it("ref should be forwarded", () => { @@ -43,6 +44,7 @@ describe("Image", () => { const wrapper = render(); expect(wrapper.getByRole("img")).toBeInstanceOf(HTMLImageElement); + wrapper.unmount(); }); test("renders an image while loading the src image. When loading finished, renders the src image.", async () => { @@ -109,12 +111,14 @@ describe("Image", () => { const wrapper = render(); expect(wrapper.getByRole("img")).toHaveAttribute("src", src); + wrapper.unmount(); }); test("should render a wrapper when isZoomed or isBlurred is true", () => { const wrapper = render(); expect(wrapper.getByRole("img").parentElement).toBeInstanceOf(HTMLDivElement); + wrapper.unmount(); }); test("should render a blurred image when isBlurred is true", () => { @@ -122,6 +126,7 @@ describe("Image", () => { const blurredImage = wrapper.getByRole("img").nextElementSibling; expect(blurredImage).toBeInstanceOf(HTMLImageElement); + wrapper.unmount(); }); test("should fire onload", () => { @@ -135,6 +140,7 @@ describe("Image", () => { expect(wrapper.getByRole("img")).toHaveAttribute("src", src); expect(onLoad).toHaveBeenCalled(); + wrapper.unmount(); }); test("should disable aspect ratio if height is set", () => { @@ -155,5 +161,6 @@ describe("Image", () => { expect(getComputedStyle(images[1]).height).toBe("40px"); expect(getComputedStyle(images[2]).height).toBe("50px"); expect(getComputedStyle(images[3]).height).toBe("60px"); + wrapper.unmount(); }); }); From ef99401ac7386f65be9c7ded3ae566fd6c6a8395 Mon Sep 17 00:00:00 2001 From: Arian Pourarian Date: Sat, 8 Feb 2025 21:18:35 +0330 Subject: [PATCH 6/6] test(image): add coderabbitai suggestions --- .../components/image/__tests__/image.test.tsx | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/packages/components/image/__tests__/image.test.tsx b/packages/components/image/__tests__/image.test.tsx index 387cad2d6b..e575b03d83 100644 --- a/packages/components/image/__tests__/image.test.tsx +++ b/packages/components/image/__tests__/image.test.tsx @@ -20,12 +20,23 @@ describe("Image", () => { imageOnload = fn; this._onload = fn; }, + configurable: true, }); } trackImageOnload(); }); + afterAll(() => { + // Restore original Image prototype + delete window.Image.prototype._onload; + Object.defineProperty(window.Image.prototype, "onload", { + value: null, + writable: true, + configurable: true, + }); + }); + it("should render correctly", () => { const wrapper = render(); @@ -82,11 +93,21 @@ describe("Image", () => { imageOnerror = fn; this._onerror = fn; }, + configurable: true, }); } trackImageOnerror(); + const cleanup = () => { + delete window.Image.prototype._onerror; + Object.defineProperty(window.Image.prototype, "onerror", { + value: null, + writable: true, + configurable: true, + }); + }; + const onError = jest.fn(); const wrapper = render( test, @@ -105,6 +126,7 @@ describe("Image", () => { expect(computedStyle.backgroundImage).toBe(`url(${fallbackSrc})`); wrapper.unmount(); + cleanup(); }); test("renders image if there is no loading or fallback behavior defined", async () => {