refactor: add changes after review

openedx · Dec 7, 2023 · 0f9c94c · 0f9c94c
1 parent 92d5b38
commit 0f9c94c
Show file tree

Hide file tree

Showing 5 changed files with 37 additions and 34 deletions.
diff --git a/src/Card/CardImageWithSkeleton.jsx b/src/Card/CardImageWithSkeleton.jsx
@@ -3,6 +3,7 @@ import PropTypes from 'prop-types';
 import classNames from 'classnames';
 import Skeleton from 'react-loading-skeleton';
 
+import Image from '../Image';
 import useImageLoader from '../hooks/useImageLoader';
 
 function CardImageWithSkeleton({
@@ -20,7 +21,7 @@ function CardImageWithSkeleton({
 }) {
   const config = useMemo(
     () => ({
-      mainSrc: src, fallback, useDefaultSrc,
+      mainSrc: src, fallbackSrc: fallback, useDefaultSrc,
     }),
     [src, fallback, useDefaultSrc],
   );
@@ -34,7 +35,7 @@ function CardImageWithSkeleton({
         height={skeletonHeight}
         width={skeletonWidth}
       />
-      <img
+      <Image
         ref={ref}
         className={classNames(className, { show: !isSrcLoading && !isLoading })}
         alt={alt}

diff --git a/src/hooks/tests/useImageLoader.test.jsx b/src/hooks/tests/useImageLoader.test.jsx
@@ -2,53 +2,56 @@ import React from 'react';
 import {
   render, screen, act, waitFor, fireEvent,
 } from '@testing-library/react';
+import { Image } from '../..';
 
 import useImageLoader from '../useImageLoader';
 
 const MAIN_SRC = 'main-source.jpg';
-const FALLBACK = 'fallback-source.jpg';
-const ALT = 'test';
+const FALLBACK_SRC = 'fallback-source.jpg';
+const ALT_TEXT = 'test';
 const TEST_ID = 'loading-indicator';
 
 function TestComponent({
 // eslint-disable-next-line react/prop-types
-  mainSrc, fallback, alt, ...rest
+  mainSrc, fallbackSrc, alt, ...rest
 }) {
-  const { ref, isSrcLoading } = useImageLoader({ mainSrc, fallback, ...rest });
+  const { ref, isSrcLoading } = useImageLoader({ mainSrc, fallbackSrc, ...rest });
   return (
     <>
       {isSrcLoading && <div data-testid={TEST_ID}>Loading...</div>}
-      <img ref={ref} alt={alt} />
+      <Image ref={ref} alt={alt} />
     </>
   );
 }
 
 describe('useImageLoader', () => {
-  it('should set loading state to false when the main source loads successfully and update img src', async () => {
-    render(<TestComponent mainSrc={MAIN_SRC} fallback={FALLBACK} alt={ALT} />);
-    const imgElement = screen.getByAltText(ALT);
+  describe('should set loading state to false', () => {
+    it('when the main source loads successfully and update img src', async () => {
+      render(<TestComponent mainSrc={MAIN_SRC} fallbackSrc={FALLBACK_SRC} alt={ALT_TEXT} />);
+      const imgElement = screen.getByAltText(ALT_TEXT);
 
-    expect(screen.getByTestId(TEST_ID)).toBeInTheDocument();
+      expect(screen.getByTestId(TEST_ID)).toBeInTheDocument();
 
-    await act(async () => {
-      fireEvent.load(imgElement);
-    });
+      await act(async () => {
+        fireEvent.load(imgElement);
+      });
 
-    await waitFor(() => expect(screen.queryByTestId(TEST_ID)).not.toBeInTheDocument());
+      await waitFor(() => expect(screen.queryByTestId(TEST_ID)).not.toBeInTheDocument());
 
-    expect(imgElement.src).toContain(MAIN_SRC);
-  });
+      expect(imgElement.src).toContain(MAIN_SRC);
+    });
 
-  it('should set loading state to false when the main source fails to load and falls back to the fallback source, and update img src', async () => {
-    render(<TestComponent mainSrc={MAIN_SRC} fallback={FALLBACK} alt={ALT} />);
-    const imgElement = screen.getByAltText(ALT);
+    it('when the main source fails to load and falls back to the fallback source, and update img src', async () => {
+      render(<TestComponent mainSrc={MAIN_SRC} fallbackSrc={FALLBACK_SRC} alt={ALT_TEXT} />);
+      const imgElement = screen.getByAltText(ALT_TEXT);
 
-    expect(screen.getByTestId(TEST_ID)).toBeInTheDocument();
+      expect(screen.getByTestId(TEST_ID)).toBeInTheDocument();
 
-    await act(async () => {
-      fireEvent.error(imgElement);
-    });
+      await act(async () => {
+        fireEvent.error(imgElement);
+      });
 
-    expect(imgElement.src).toContain(FALLBACK);
+      expect(imgElement.src).toContain(FALLBACK_SRC);
+    });
   });
 });
diff --git a/src/hooks/useImageLoader.jsx b/src/hooks/useImageLoader.jsx
@@ -3,14 +3,14 @@ import cardSrcFallbackImg from '../Card/fallback-default.png';
 
 const useImageLoader = ({
   mainSrc,
-  fallback,
+  fallbackSrc,
   useDefaultSrc = false,
 }) => {
   const ref = useRef(null);
   const [isSrcLoading, setIsLoading] = useState(true);
 
   useEffect(() => {
-    if ((!mainSrc && !fallback) || !ref.current) {
+    if ((!mainSrc && !fallbackSrc) || !ref.current) {
       return;
     }
     const img = ref.current;
@@ -29,8 +29,8 @@ const useImageLoader = ({
       let imageSrc = null;
       const sources = [mainSrc];
 
-      if (fallback) {
-        sources.push(fallback);
+      if (fallbackSrc) {
+        sources.push(fallbackSrc);
       }
 
       // Add default image source if useDefaultSrc is true
@@ -64,7 +64,7 @@ const useImageLoader = ({
     };
 
     loadImages();
-  }, [mainSrc, fallback, useDefaultSrc]);
+  }, [mainSrc, fallbackSrc, useDefaultSrc]);
 
   return { ref, isSrcLoading };
 };

diff --git a/src/hooks/useImageLoader.mdx b/src/hooks/useImageLoader.mdx
@@ -19,10 +19,10 @@ image sources may fail.
 ```jsx live
 
 () => {
-  const MyImageComponent = ({ mainSrc, fallback, alt, useDefaultSrc }) => {
+  const MyImageComponent = ({ mainSrc, fallbackSrc, alt, useDefaultSrc }) => {
     const { ref, isSrcLoading } = useImageLoader({
       mainSrc,
-      fallback,
+      fallbackSrc,
       useDefaultSrc,
     });
 
@@ -37,7 +37,7 @@ image sources may fail.
   return (
     <MyImageComponent
       mainSrc="https://picsum.photos/360/200/"
-      fallback="https://picsum.photos/360/200/"
+      fallbackSrc="https://picsum.photos/360/200/"
       alt="Image Alt Text"
       useDefaultSrc
     />

diff --git a/www/src/scss/base.scss b/www/src/scss/base.scss
@@ -8,7 +8,6 @@
 @import "../components/Search/Search";
 @import "../components/IconsTable";
 @import "../components/exampleComponents/ExamplePropsForm";
-@import "~react-loading-skeleton/dist/skeleton";
 
 body {
   -webkit-font-smoothing: antialiased;