From 31c8411a0b4d6274a14b367567c9c341ad62bc8b Mon Sep 17 00:00:00 2001 From: Kyrylo Hudym-Levkovych Date: Wed, 22 Nov 2023 17:44:43 +0200 Subject: [PATCH 01/13] fix: option1 - handle src loading state and show skeleton while loading --- src/Card/CardImageCap.jsx | 42 +++++++++++++++++++++++++++++++++++---- src/Card/index.scss | 10 ++++++++++ 2 files changed, 48 insertions(+), 4 deletions(-) diff --git a/src/Card/CardImageCap.jsx b/src/Card/CardImageCap.jsx index e271246e43..0eb6049d11 100644 --- a/src/Card/CardImageCap.jsx +++ b/src/Card/CardImageCap.jsx @@ -1,4 +1,4 @@ -import React, { useContext, useState } from 'react'; +import React, { useContext, useState, useMemo } from 'react'; import PropTypes from 'prop-types'; import classNames from 'classnames'; import Skeleton from 'react-loading-skeleton'; @@ -27,6 +27,22 @@ const CardImageCap = React.forwardRef(({ const [showImageCap, setShowImageCap] = useState(false); const [showLogoCap, setShowLogoCap] = useState(false); + const showSkeleton = useMemo(() => { + let show; + if (src && logoSrc) { + show = !(showImageCap && showLogoCap); + } else if (src) { + show = !showImageCap; + } else if (logoSrc) { + show = !showLogoCap; + } + return show; + }, [src, logoSrc, showImageCap, showLogoCap]); + + const imageSkeletonHeight = useMemo(() => ( + orientation === 'horizontal' ? '100%' : skeletonHeight + ), [orientation, skeletonHeight]); + const wrapperClassName = `pgn__card-wrapper-image-cap ${orientation}`; if (isLoading) { @@ -37,7 +53,7 @@ const CardImageCap = React.forwardRef(({ > {logoSkeleton && ( @@ -57,6 +73,7 @@ const CardImageCap = React.forwardRef(({ if (!altSrc || currentTarget.src.endsWith(altSrc)) { if (imageKey === 'imageCap') { currentTarget.src = cardSrcFallbackImg; + setShowImageCap(false); } else { setShowLogoCap(false); } @@ -69,9 +86,26 @@ const CardImageCap = React.forwardRef(({ return (
+
+ + {logoSkeleton && ( + + )} +
{!!src && ( handleSrcFallback(event, fallbackSrc, 'imageCap')} onLoad={() => setShowImageCap(true)} @@ -81,7 +115,7 @@ const CardImageCap = React.forwardRef(({ )} {!!logoSrc && ( handleSrcFallback(event, fallbackLogoSrc, 'logoCap')} onLoad={() => setShowLogoCap(true)} diff --git a/src/Card/index.scss b/src/Card/index.scss index 56d0269aad..d8723b2e06 100644 --- a/src/Card/index.scss +++ b/src/Card/index.scss @@ -306,6 +306,16 @@ a.pgn__card { .pgn__card-wrapper-image-cap { position: relative; + .image-loader { + display: none; + + &.show { + display: block; + height: 100%; + } + } + + &.horizontal { max-width: $card-image-horizontal-max-width; min-width: $card-image-horizontal-min-width; From f160afc16bb1e258e4cf7bccf61c73cc2b0dfbde Mon Sep 17 00:00:00 2001 From: Kyrylo Hudym-Levkovych Date: Thu, 23 Nov 2023 12:27:49 +0200 Subject: [PATCH 02/13] fix: option2 - handle loading src in hook --- src/Card/CardImageCap.jsx | 51 ++++++----------------- src/Card/hooks/useImagesLoader.jsx | 65 ++++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+), 38 deletions(-) create mode 100644 src/Card/hooks/useImagesLoader.jsx diff --git a/src/Card/CardImageCap.jsx b/src/Card/CardImageCap.jsx index 0eb6049d11..dce1f7599b 100644 --- a/src/Card/CardImageCap.jsx +++ b/src/Card/CardImageCap.jsx @@ -1,9 +1,10 @@ -import React, { useContext, useState, useMemo } from 'react'; +import React, { useContext, useMemo } from 'react'; import PropTypes from 'prop-types'; import classNames from 'classnames'; import Skeleton from 'react-loading-skeleton'; import CardContext from './CardContext'; import cardSrcFallbackImg from './fallback-default.png'; +import useImagesLoader from './hooks/useImagesLoader'; const SKELETON_HEIGHT_VALUE = 140; const LOGO_SKELETON_HEIGHT_VALUE = 41; @@ -24,20 +25,15 @@ const CardImageCap = React.forwardRef(({ imageLoadingType, }, ref) => { const { orientation, isLoading } = useContext(CardContext); - const [showImageCap, setShowImageCap] = useState(false); - const [showLogoCap, setShowLogoCap] = useState(false); + const urls = useMemo(() => ( + [ + { mainSrc: src, fallback: fallbackSrc, type: 'image' }, + { mainSrc: logoSrc, fallback: fallbackLogoSrc, type: 'logo' }, + ] + .filter(obj => obj.mainSrc || obj.fallback) + ), [src, logoSrc, fallbackSrc, fallbackLogoSrc]); - const showSkeleton = useMemo(() => { - let show; - if (src && logoSrc) { - show = !(showImageCap && showLogoCap); - } else if (src) { - show = !showImageCap; - } else if (logoSrc) { - show = !showLogoCap; - } - return show; - }, [src, logoSrc, showImageCap, showLogoCap]); + const { loadedImages, allLoaded } = useImagesLoader(urls); const imageSkeletonHeight = useMemo(() => ( orientation === 'horizontal' ? '100%' : skeletonHeight @@ -67,27 +63,10 @@ const CardImageCap = React.forwardRef(({ ); } - const handleSrcFallback = (event, altSrc, imageKey) => { - const { currentTarget } = event; - - if (!altSrc || currentTarget.src.endsWith(altSrc)) { - if (imageKey === 'imageCap') { - currentTarget.src = cardSrcFallbackImg; - setShowImageCap(false); - } else { - setShowLogoCap(false); - } - - return; - } - - currentTarget.src = altSrc; - }; - return (
{!!src && ( handleSrcFallback(event, fallbackSrc, 'imageCap')} - onLoad={() => setShowImageCap(true)} alt={srcAlt} loading={imageLoadingType} /> )} {!!logoSrc && ( handleSrcFallback(event, fallbackLogoSrc, 'logoCap')} - onLoad={() => setShowLogoCap(true)} alt={logoAlt} loading={imageLoadingType} /> diff --git a/src/Card/hooks/useImagesLoader.jsx b/src/Card/hooks/useImagesLoader.jsx new file mode 100644 index 0000000000..4896e4236a --- /dev/null +++ b/src/Card/hooks/useImagesLoader.jsx @@ -0,0 +1,65 @@ +import { useState, useEffect, useCallback } from 'react'; +import cardSrcFallbackImg from '../fallback-default.png'; + +/** + * Custom hook to load images and track their loading status. + * + * @param {Object[]} urls - An array of image objects to load and track. + * @param {string} urls[].src - The main source URL of the image. + * @param {string} [urls[].fallback] - The fallback source URL to use if the main source fails to load. + * // TODO + * @param {string} urls[].type ? do i need this + * + * @returns {Object} An object containing the loaded img statuses and a boolean indicating whether all imgs are loaded. + * @property {Object} loadedImages - An object with image URLs as keys and their loading status as values. + * @property {boolean} allLoaded - A boolean indicating whether all images are loaded. + */ +const useImagesLoader = (urls) => { + const [loadedImages, setLoadedImages] = useState({}); + const [allLoaded, setAllLoaded] = useState(false); + + const isValidUrl = useCallback((url) => { + try { + // eslint-disable-next-line no-new + new URL(url); + return true; + } catch (error) { + return false; + } + }, []); + + useEffect(() => { + const loadImage = async ({ mainSrc, fallback, type }) => new Promise((resolve) => { + const img = new Image(); + img.onload = () => resolve({ [mainSrc]: true }); + img.onerror = () => { + if (fallback && type === 'image') { + resolve({ [mainSrc]: cardSrcFallbackImg }); + } else if (fallback) { + resolve({ [fallback]: fallback }); + } else { + resolve({ [mainSrc]: false }); + } + }; + img.src = isValidUrl(mainSrc) && mainSrc; + }); + + const loadImages = async () => { + const results = await Promise.all(urls.map(loadImage)); + const loadedImagesResult = results.reduce((acc, result) => { + Object.assign(acc, result); + return acc; + }, {}); + setLoadedImages(loadedImagesResult); + + const isAllLoaded = Object.values(loadedImagesResult).every((loaded) => loaded); + setAllLoaded(isAllLoaded); + }; + + loadImages(); + }, [isValidUrl, urls]); + + return { loadedImages, allLoaded }; +}; + +export default useImagesLoader; From 050b8fa6b693bd3cbb3e058adf901500190b09ec Mon Sep 17 00:00:00 2001 From: Kyrylo Hudym-Levkovych Date: Fri, 1 Dec 2023 10:25:22 +0200 Subject: [PATCH 03/13] feat: update useImageLoader hook, add img with skeleton component --- src/Card/CardImageCap.jsx | 56 +++++++------------ src/Card/CardImageWithSkeleton.jsx | 88 ++++++++++++++++++++++++++++++ src/Card/hooks/useImageLoader.jsx | 59 ++++++++++++++++++++ src/Card/hooks/useImagesLoader.jsx | 65 ---------------------- src/Card/index.scss | 23 ++++---- 5 files changed, 177 insertions(+), 114 deletions(-) create mode 100644 src/Card/CardImageWithSkeleton.jsx create mode 100644 src/Card/hooks/useImageLoader.jsx delete mode 100644 src/Card/hooks/useImagesLoader.jsx diff --git a/src/Card/CardImageCap.jsx b/src/Card/CardImageCap.jsx index dce1f7599b..0d0bf63490 100644 --- a/src/Card/CardImageCap.jsx +++ b/src/Card/CardImageCap.jsx @@ -1,15 +1,15 @@ -import React, { useContext, useMemo } from 'react'; +import React, { useContext, useMemo, memo } from 'react'; import PropTypes from 'prop-types'; import classNames from 'classnames'; import Skeleton from 'react-loading-skeleton'; import CardContext from './CardContext'; import cardSrcFallbackImg from './fallback-default.png'; -import useImagesLoader from './hooks/useImagesLoader'; +import CardImageWithSkeleton from './CardImageWithSkeleton'; const SKELETON_HEIGHT_VALUE = 140; const LOGO_SKELETON_HEIGHT_VALUE = 41; -const CardImageCap = React.forwardRef(({ +const CardImageCap = memo(React.forwardRef(({ src, fallbackSrc, srcAlt, @@ -25,15 +25,6 @@ const CardImageCap = React.forwardRef(({ imageLoadingType, }, ref) => { const { orientation, isLoading } = useContext(CardContext); - const urls = useMemo(() => ( - [ - { mainSrc: src, fallback: fallbackSrc, type: 'image' }, - { mainSrc: logoSrc, fallback: fallbackLogoSrc, type: 'logo' }, - ] - .filter(obj => obj.mainSrc || obj.fallback) - ), [src, logoSrc, fallbackSrc, fallbackLogoSrc]); - - const { loadedImages, allLoaded } = useImagesLoader(urls); const imageSkeletonHeight = useMemo(() => ( orientation === 'horizontal' ? '100%' : skeletonHeight @@ -65,42 +56,35 @@ const CardImageCap = React.forwardRef(({ return (
-
- - {logoSkeleton && ( - - )} -
{!!src && ( - {srcAlt} )} {!!logoSrc && ( - {logoAlt} )}
); -}); +})); CardImageCap.propTypes = { /** Specifies class name to append to the base element. */ diff --git a/src/Card/CardImageWithSkeleton.jsx b/src/Card/CardImageWithSkeleton.jsx new file mode 100644 index 0000000000..f76b7fba6d --- /dev/null +++ b/src/Card/CardImageWithSkeleton.jsx @@ -0,0 +1,88 @@ +import React, { + useMemo, memo, useState, useEffect, +} from 'react'; +import PropTypes from 'prop-types'; +import classNames from 'classnames'; +import Skeleton from 'react-loading-skeleton'; + +import useImageLoader from './hooks/useImageLoader'; + +const CardImageWithSkeleton = memo(({ + src, + alt, + fallback, + className, + useDefaultSrc, + withSkeleton, + skeletonWidth, + skeletonHeight, + imageLoadingType, + skeletonClassName, +}) => { + const config = useMemo( + () => ({ mainSrc: src, fallback, useDefaultSrc }), + [src, fallback, useDefaultSrc], + ); + const [showSkeleton, setShowSkeleton] = useState(true); + + const { loadedImage, isSrcLoading } = useImageLoader(config); + + useEffect(() => { + if (!isSrcLoading && !withSkeleton) { + setShowSkeleton(false); + } else { + setShowSkeleton(true); + } + }, [isSrcLoading, withSkeleton]); + + return ( + <> + + setShowSkeleton(false)} + alt={alt} + loading={imageLoadingType} + /> + + + ); +}); + +CardImageWithSkeleton.propTypes = { + /** Specifies class name to append to the base element. */ + className: PropTypes.string, + /** Specifies image src. */ + src: PropTypes.string.isRequired, + /** Specifies fallback image src. */ + fallback: PropTypes.string.isRequired, + /** Specifies whether to use the default fallback source if both the main source and fallback fail to load. */ + useDefaultSrc: PropTypes.bool, + /** Specifies whether to render skeleton. */ + withSkeleton: PropTypes.bool, + /** Specifies image alt text. */ + alt: PropTypes.string.isRequired, + /** Specifies height of Image skeleton in loading state. */ + skeletonHeight: PropTypes.number.isRequired, + /** Specifies width of Image skeleton in loading state. */ + skeletonWidth: PropTypes.number.isRequired, + /** Specifies class name to append to the skeleton element. */ + skeletonClassName: PropTypes.string, + /** Specifies loading type for images */ + imageLoadingType: PropTypes.oneOf(['eager', 'lazy']), +}; + +CardImageWithSkeleton.defaultProps = { + className: undefined, + skeletonClassName: undefined, + useDefaultSrc: false, + withSkeleton: true, + imageLoadingType: 'eager', +}; + +export default CardImageWithSkeleton; diff --git a/src/Card/hooks/useImageLoader.jsx b/src/Card/hooks/useImageLoader.jsx new file mode 100644 index 0000000000..c3ceeb9272 --- /dev/null +++ b/src/Card/hooks/useImageLoader.jsx @@ -0,0 +1,59 @@ +/* eslint-disable no-shadow */ +import { useState, useEffect } from 'react'; +import cardSrcFallbackImg from '../fallback-default.png'; + +const useImageLoader = ({ mainSrc, fallback, useDefaultSrc = false }) => { + const [loadedImage, setLoadedImage] = useState({}); + const [isSrcLoading, setIsLoading] = useState(true); + + useEffect(() => { + if (!mainSrc || !fallback) { + return; + } + + const loadImageWithRetry = async ({ mainSrc, src }) => { + const img = new Image(); + await new Promise((resolve, reject) => { + img.onload = resolve; + img.onerror = reject; + img.src = src; + }); + return { [mainSrc]: img }; + }; + + const loadImage = async ({ mainSrc, fallback, useDefaultSrc }) => { + let loadedImage = {}; + + try { + loadedImage = await loadImageWithRetry({ mainSrc, src: mainSrc }); + } catch (error) { + if (fallback) { + try { + loadedImage = await loadImageWithRetry({ mainSrc, src: fallback }); + } catch (error) { + if (useDefaultSrc) { + loadedImage = { [mainSrc]: cardSrcFallbackImg }; + } + } + } + } + + return loadedImage; + }; + + const loadAllImages = async () => { + try { + const loadedImageObject = await loadImage({ mainSrc, fallback, useDefaultSrc }); + setLoadedImage(loadedImageObject); + } finally { + setIsLoading(false); + } + }; + + loadAllImages(); + }, [mainSrc, fallback, useDefaultSrc]); + + return { loadedImage, isSrcLoading }; +}; + +export default useImageLoader; diff --git a/src/Card/hooks/useImagesLoader.jsx b/src/Card/hooks/useImagesLoader.jsx deleted file mode 100644 index 4896e4236a..0000000000 --- a/src/Card/hooks/useImagesLoader.jsx +++ /dev/null @@ -1,65 +0,0 @@ -import { useState, useEffect, useCallback } from 'react'; -import cardSrcFallbackImg from '../fallback-default.png'; - -/** - * Custom hook to load images and track their loading status. - * - * @param {Object[]} urls - An array of image objects to load and track. - * @param {string} urls[].src - The main source URL of the image. - * @param {string} [urls[].fallback] - The fallback source URL to use if the main source fails to load. - * // TODO - * @param {string} urls[].type ? do i need this - * - * @returns {Object} An object containing the loaded img statuses and a boolean indicating whether all imgs are loaded. - * @property {Object} loadedImages - An object with image URLs as keys and their loading status as values. - * @property {boolean} allLoaded - A boolean indicating whether all images are loaded. - */ -const useImagesLoader = (urls) => { - const [loadedImages, setLoadedImages] = useState({}); - const [allLoaded, setAllLoaded] = useState(false); - - const isValidUrl = useCallback((url) => { - try { - // eslint-disable-next-line no-new - new URL(url); - return true; - } catch (error) { - return false; - } - }, []); - - useEffect(() => { - const loadImage = async ({ mainSrc, fallback, type }) => new Promise((resolve) => { - const img = new Image(); - img.onload = () => resolve({ [mainSrc]: true }); - img.onerror = () => { - if (fallback && type === 'image') { - resolve({ [mainSrc]: cardSrcFallbackImg }); - } else if (fallback) { - resolve({ [fallback]: fallback }); - } else { - resolve({ [mainSrc]: false }); - } - }; - img.src = isValidUrl(mainSrc) && mainSrc; - }); - - const loadImages = async () => { - const results = await Promise.all(urls.map(loadImage)); - const loadedImagesResult = results.reduce((acc, result) => { - Object.assign(acc, result); - return acc; - }, {}); - setLoadedImages(loadedImagesResult); - - const isAllLoaded = Object.values(loadedImagesResult).every((loaded) => loaded); - setAllLoaded(isAllLoaded); - }; - - loadImages(); - }, [isValidUrl, urls]); - - return { loadedImages, allLoaded }; -}; - -export default useImagesLoader; diff --git a/src/Card/index.scss b/src/Card/index.scss index d8723b2e06..46f55e89ea 100644 --- a/src/Card/index.scss +++ b/src/Card/index.scss @@ -306,16 +306,6 @@ a.pgn__card { .pgn__card-wrapper-image-cap { position: relative; - .image-loader { - display: none; - - &.show { - display: block; - height: 100%; - } - } - - &.horizontal { max-width: $card-image-horizontal-max-width; min-width: $card-image-horizontal-min-width; @@ -347,6 +337,13 @@ a.pgn__card { } .pgn__card-image-cap-loader { + display: none; + + &.show { + display: block; + height: 100%; + } + .react-loading-skeleton { margin-bottom: -$loading-skeleton-spacer; position: relative; @@ -369,9 +366,9 @@ a.pgn__card { background-color: $white; display: none; - &.show { - display: block; - } + &.show { + display: block; + } } } From ee2723df499841f33eaddbf7f38a3eb4fd4d98ed Mon Sep 17 00:00:00 2001 From: Kyrylo Hudym-Levkovych Date: Fri, 1 Dec 2023 11:37:07 +0200 Subject: [PATCH 04/13] feat: add skeleton --- www/src/scss/base.scss | 1 + 1 file changed, 1 insertion(+) diff --git a/www/src/scss/base.scss b/www/src/scss/base.scss index 913b13f76d..1b78d9a3ee 100644 --- a/www/src/scss/base.scss +++ b/www/src/scss/base.scss @@ -8,6 +8,7 @@ @import "../components/Search/Search"; @import "../components/IconsTable"; @import "../components/exampleComponents/ExamplePropsForm"; +@import "~react-loading-skeleton/dist/skeleton"; body { -webkit-font-smoothing: antialiased; From 65b0d797fda774ed12ce71c222e5a94be7ebeba3 Mon Sep 17 00:00:00 2001 From: Kyrylo Hudym-Levkovych Date: Fri, 1 Dec 2023 12:11:22 +0200 Subject: [PATCH 05/13] fix: src issue --- src/Card/CardImageWithSkeleton.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Card/CardImageWithSkeleton.jsx b/src/Card/CardImageWithSkeleton.jsx index f76b7fba6d..ccf1ea6bb0 100644 --- a/src/Card/CardImageWithSkeleton.jsx +++ b/src/Card/CardImageWithSkeleton.jsx @@ -44,7 +44,7 @@ const CardImageWithSkeleton = memo(({ /> setShowSkeleton(false)} alt={alt} loading={imageLoadingType} From ed359e6191fe44c54294e3bc4e452368dd1e6f4c Mon Sep 17 00:00:00 2001 From: Kyrylo Hudym-Levkovych Date: Fri, 1 Dec 2023 13:34:37 +0200 Subject: [PATCH 06/13] feat: add TS for the hook --- src/Card/hooks/useImageLoader.jsx | 59 ------------------------ src/Card/hooks/useImageLoader.tsx | 76 +++++++++++++++++++++++++++++++ src/index.d.ts | 1 + 3 files changed, 77 insertions(+), 59 deletions(-) delete mode 100644 src/Card/hooks/useImageLoader.jsx create mode 100644 src/Card/hooks/useImageLoader.tsx create mode 100644 src/index.d.ts diff --git a/src/Card/hooks/useImageLoader.jsx b/src/Card/hooks/useImageLoader.jsx deleted file mode 100644 index c3ceeb9272..0000000000 --- a/src/Card/hooks/useImageLoader.jsx +++ /dev/null @@ -1,59 +0,0 @@ -/* eslint-disable no-shadow */ -import { useState, useEffect } from 'react'; -import cardSrcFallbackImg from '../fallback-default.png'; - -const useImageLoader = ({ mainSrc, fallback, useDefaultSrc = false }) => { - const [loadedImage, setLoadedImage] = useState({}); - const [isSrcLoading, setIsLoading] = useState(true); - - useEffect(() => { - if (!mainSrc || !fallback) { - return; - } - - const loadImageWithRetry = async ({ mainSrc, src }) => { - const img = new Image(); - await new Promise((resolve, reject) => { - img.onload = resolve; - img.onerror = reject; - img.src = src; - }); - return { [mainSrc]: img }; - }; - - const loadImage = async ({ mainSrc, fallback, useDefaultSrc }) => { - let loadedImage = {}; - - try { - loadedImage = await loadImageWithRetry({ mainSrc, src: mainSrc }); - } catch (error) { - if (fallback) { - try { - loadedImage = await loadImageWithRetry({ mainSrc, src: fallback }); - } catch (error) { - if (useDefaultSrc) { - loadedImage = { [mainSrc]: cardSrcFallbackImg }; - } - } - } - } - - return loadedImage; - }; - - const loadAllImages = async () => { - try { - const loadedImageObject = await loadImage({ mainSrc, fallback, useDefaultSrc }); - setLoadedImage(loadedImageObject); - } finally { - setIsLoading(false); - } - }; - - loadAllImages(); - }, [mainSrc, fallback, useDefaultSrc]); - - return { loadedImage, isSrcLoading }; -}; - -export default useImageLoader; diff --git a/src/Card/hooks/useImageLoader.tsx b/src/Card/hooks/useImageLoader.tsx new file mode 100644 index 0000000000..2894c97e9c --- /dev/null +++ b/src/Card/hooks/useImageLoader.tsx @@ -0,0 +1,76 @@ +/* eslint-disable @typescript-eslint/no-shadow */ +import { useState, useEffect } from 'react'; +import cardSrcFallbackImg from '../fallback-default.png'; + +interface ImageLoaderProps { + mainSrc: string; + fallback: string; + useDefaultSrc?: boolean; +} + +interface LoadedImage { + [key: string]: HTMLImageElement; +} + +interface UseImageLoaderResult { + loadedImage: LoadedImage; + isSrcLoading: boolean; +} + +const useImageLoader = ({ mainSrc, fallback, useDefaultSrc = false }: ImageLoaderProps): UseImageLoaderResult => { + const [loadedImage, setLoadedImage] = useState({}); + const [isSrcLoading, setIsLoading] = useState(true); + + useEffect(() => { + if (!mainSrc || !fallback) { + return; + } + + const loadImageWithRetry = async (src: string): Promise => { + const img = new Image(); + await new Promise((resolve, reject) => { + img.onload = resolve; + img.onerror = reject; + img.src = src; + }); + return { [mainSrc]: img }; + }; + + const loadImage = async (): Promise => { + let objWithImage: LoadedImage = {}; + + try { + objWithImage = await loadImageWithRetry(mainSrc); + } catch (error) { + if (fallback) { + try { + objWithImage = await loadImageWithRetry(fallback); + } catch (error) { + if (useDefaultSrc) { + const defaultImage = new Image(); + defaultImage.src = cardSrcFallbackImg; + objWithImage = { [mainSrc]: defaultImage }; + } + } + } + } + + return objWithImage; + }; + + const loadAllImages = async (): Promise => { + try { + const loadedImageObject = await loadImage(); + setLoadedImage(loadedImageObject); + } finally { + setIsLoading(false); + } + }; + + loadAllImages(); + }, [mainSrc, fallback, useDefaultSrc]); + + return { loadedImage, isSrcLoading }; +}; + +export default useImageLoader; diff --git a/src/index.d.ts b/src/index.d.ts new file mode 100644 index 0000000000..e2937d470e --- /dev/null +++ b/src/index.d.ts @@ -0,0 +1 @@ +declare module '*.png'; From 773d02f940102407a2d466b4a340d665a273c47c Mon Sep 17 00:00:00 2001 From: monteri Date: Fri, 1 Dec 2023 18:52:09 +0200 Subject: [PATCH 07/13] fix: use ref for image --- src/Card/CardImageCap.jsx | 40 +++---------- src/Card/CardImageWithSkeleton.jsx | 60 +++++++------------ src/Card/_variables.scss | 2 +- src/Card/constants.js | 2 + src/Card/hooks/useImageLoader.tsx | 92 +++++++++++++++++------------- src/Card/index.scss | 4 +- 6 files changed, 85 insertions(+), 115 deletions(-) create mode 100644 src/Card/constants.js diff --git a/src/Card/CardImageCap.jsx b/src/Card/CardImageCap.jsx index 0d0bf63490..a9cee156a8 100644 --- a/src/Card/CardImageCap.jsx +++ b/src/Card/CardImageCap.jsx @@ -1,15 +1,12 @@ -import React, { useContext, useMemo, memo } from 'react'; +import React, { useContext, useMemo } from 'react'; import PropTypes from 'prop-types'; import classNames from 'classnames'; -import Skeleton from 'react-loading-skeleton'; import CardContext from './CardContext'; import cardSrcFallbackImg from './fallback-default.png'; import CardImageWithSkeleton from './CardImageWithSkeleton'; +import { LOGO_SKELETON_HEIGHT_VALUE, SKELETON_HEIGHT_VALUE } from './constants'; -const SKELETON_HEIGHT_VALUE = 140; -const LOGO_SKELETON_HEIGHT_VALUE = 41; - -const CardImageCap = memo(React.forwardRef(({ +const CardImageCap = React.forwardRef(({ src, fallbackSrc, srcAlt, @@ -32,28 +29,6 @@ const CardImageCap = memo(React.forwardRef(({ const wrapperClassName = `pgn__card-wrapper-image-cap ${orientation}`; - if (isLoading) { - return ( -
- - {logoSkeleton && ( - - )} -
- ); - } - return (
{!!src && ( @@ -67,9 +42,10 @@ const CardImageCap = memo(React.forwardRef(({ skeletonHeight={imageSkeletonHeight} imageLoadingType={imageLoadingType} skeletonClassName="pgn__card-image-cap-loader" + isLoading={isLoading} /> )} - {!!logoSrc && ( + {!!logoSrc && !isLoading && ( ); -})); +}); CardImageCap.propTypes = { /** Specifies class name to append to the base element. */ @@ -102,9 +78,9 @@ CardImageCap.propTypes = { /** Specifies logo image alt text. */ logoAlt: PropTypes.string, /** Specifies height of Image skeleton in loading state. */ - skeletonHeight: PropTypes.number, + skeletonHeight: PropTypes.oneOfType([PropTypes.number, PropTypes.string]), /** Specifies width of Image skeleton in loading state. */ - skeletonWidth: PropTypes.number, + skeletonWidth: PropTypes.oneOfType([PropTypes.number, PropTypes.string]), /** Specifies whether the cap should be displayed during loading. */ logoSkeleton: PropTypes.bool, /** Specifies height of Logo skeleton in loading state. */ diff --git a/src/Card/CardImageWithSkeleton.jsx b/src/Card/CardImageWithSkeleton.jsx index ccf1ea6bb0..92a322e462 100644 --- a/src/Card/CardImageWithSkeleton.jsx +++ b/src/Card/CardImageWithSkeleton.jsx @@ -1,88 +1,72 @@ -import React, { - useMemo, memo, useState, useEffect, -} from 'react'; +import React, { useMemo } from 'react'; import PropTypes from 'prop-types'; import classNames from 'classnames'; import Skeleton from 'react-loading-skeleton'; import useImageLoader from './hooks/useImageLoader'; -const CardImageWithSkeleton = memo(({ +function CardImageWithSkeleton({ src, alt, fallback, className, useDefaultSrc, - withSkeleton, skeletonWidth, skeletonHeight, imageLoadingType, skeletonClassName, -}) => { + isLoading = false, +}) { const config = useMemo( - () => ({ mainSrc: src, fallback, useDefaultSrc }), + () => ({ + mainSrc: src, fallback, useDefaultSrc, + }), [src, fallback, useDefaultSrc], ); - const [showSkeleton, setShowSkeleton] = useState(true); - const { loadedImage, isSrcLoading } = useImageLoader(config); - - useEffect(() => { - if (!isSrcLoading && !withSkeleton) { - setShowSkeleton(false); - } else { - setShowSkeleton(true); - } - }, [isSrcLoading, withSkeleton]); + const { ref, isSrcLoading } = useImageLoader(config); return ( <> setShowSkeleton(false)} + ref={ref} + className={classNames(className, { show: !isSrcLoading && !isLoading })} alt={alt} loading={imageLoadingType} /> - ); -}); +} CardImageWithSkeleton.propTypes = { - /** Specifies class name to append to the base element. */ className: PropTypes.string, - /** Specifies image src. */ src: PropTypes.string.isRequired, - /** Specifies fallback image src. */ - fallback: PropTypes.string.isRequired, - /** Specifies whether to use the default fallback source if both the main source and fallback fail to load. */ + fallback: PropTypes.string, useDefaultSrc: PropTypes.bool, - /** Specifies whether to render skeleton. */ - withSkeleton: PropTypes.bool, - /** Specifies image alt text. */ alt: PropTypes.string.isRequired, - /** Specifies height of Image skeleton in loading state. */ - skeletonHeight: PropTypes.number.isRequired, - /** Specifies width of Image skeleton in loading state. */ - skeletonWidth: PropTypes.number.isRequired, - /** Specifies class name to append to the skeleton element. */ + skeletonHeight: PropTypes.oneOfType([PropTypes.number, PropTypes.string]), + skeletonWidth: PropTypes.oneOfType([PropTypes.number, PropTypes.string]), skeletonClassName: PropTypes.string, - /** Specifies loading type for images */ imageLoadingType: PropTypes.oneOf(['eager', 'lazy']), + isLoading: PropTypes.bool, + parentRef: PropTypes.shape({ current: PropTypes.instanceOf(Element) }), }; CardImageWithSkeleton.defaultProps = { className: undefined, skeletonClassName: undefined, + fallback: undefined, useDefaultSrc: false, - withSkeleton: true, imageLoadingType: 'eager', + skeletonHeight: undefined, + skeletonWidth: undefined, + isLoading: false, + parentRef: { current: null }, }; export default CardImageWithSkeleton; diff --git a/src/Card/_variables.scss b/src/Card/_variables.scss index 703e99dc26..5e1c581bd8 100644 --- a/src/Card/_variables.scss +++ b/src/Card/_variables.scss @@ -48,7 +48,7 @@ $card-footer-text-font-size: $x-small-font-size; $card-image-horizontal-max-width: 240px !default; $card-image-horizontal-min-width: $card-image-horizontal-max-width !default; $card-image-vertical-max-height: 140px !default; -$loading-skeleton-spacer: .313rem !default; +$loading-skeleton-spacer: -4px !default; $card-focus-border-offset: 5px !default; $card-focus-border-width: 2px !default; diff --git a/src/Card/constants.js b/src/Card/constants.js new file mode 100644 index 0000000000..09a1677f28 --- /dev/null +++ b/src/Card/constants.js @@ -0,0 +1,2 @@ +export const SKELETON_HEIGHT_VALUE = 140; +export const LOGO_SKELETON_HEIGHT_VALUE = 41; diff --git a/src/Card/hooks/useImageLoader.tsx b/src/Card/hooks/useImageLoader.tsx index 2894c97e9c..4fb7bfb44d 100644 --- a/src/Card/hooks/useImageLoader.tsx +++ b/src/Card/hooks/useImageLoader.tsx @@ -1,5 +1,6 @@ -/* eslint-disable @typescript-eslint/no-shadow */ -import { useState, useEffect } from 'react'; +import { + useState, useEffect, useRef, RefObject, +} from 'react'; import cardSrcFallbackImg from '../fallback-default.png'; interface ImageLoaderProps { @@ -8,69 +9,78 @@ interface ImageLoaderProps { useDefaultSrc?: boolean; } -interface LoadedImage { - [key: string]: HTMLImageElement; -} - interface UseImageLoaderResult { - loadedImage: LoadedImage; + ref: RefObject; + loadedImage: string | null; isSrcLoading: boolean; } -const useImageLoader = ({ mainSrc, fallback, useDefaultSrc = false }: ImageLoaderProps): UseImageLoaderResult => { - const [loadedImage, setLoadedImage] = useState({}); +const useImageLoader = ({ + mainSrc, + fallback, + useDefaultSrc = false, +}: ImageLoaderProps): UseImageLoaderResult => { + const ref = useRef(null); + const [loadedImage, setLoadedImage] = useState(null); const [isSrcLoading, setIsLoading] = useState(true); useEffect(() => { - if (!mainSrc || !fallback) { + if (!mainSrc || !fallback || !ref.current) { return; } + const img = ref.current; - const loadImageWithRetry = async (src: string): Promise => { - const img = new Image(); - await new Promise((resolve, reject) => { - img.onload = resolve; - img.onerror = reject; + const loadImageWithRetry = async (src: string): Promise => { + await new Promise((resolve, reject) => { + img.onload = () => resolve(); + img.onerror = () => reject(new Error(`Failed to load image: ${src}`)); img.src = src; }); - return { [mainSrc]: img }; }; - const loadImage = async (): Promise => { - let objWithImage: LoadedImage = {}; + const loadImage = async (): Promise => { + let imageSrc: string | null = null; + const sources = [mainSrc]; - try { - objWithImage = await loadImageWithRetry(mainSrc); - } catch (error) { - if (fallback) { - try { - objWithImage = await loadImageWithRetry(fallback); - } catch (error) { - if (useDefaultSrc) { - const defaultImage = new Image(); - defaultImage.src = cardSrcFallbackImg; - objWithImage = { [mainSrc]: defaultImage }; - } - } + if (fallback) { + sources.push(fallback); + } + + // Add default image source if useDefaultSrc is true + if (useDefaultSrc) { + sources.push(cardSrcFallbackImg); + } + + // eslint-disable-next-line no-restricted-syntax + for (const src of sources) { + if (!src) { + // eslint-disable-next-line no-continue + continue; + } + + try { + // eslint-disable-next-line no-await-in-loop + await loadImageWithRetry(src); + imageSrc = src; + break; + } catch (error) { + // Continue to the next source if loading fails } } - return objWithImage; + return imageSrc; }; - const loadAllImages = async (): Promise => { - try { - const loadedImageObject = await loadImage(); - setLoadedImage(loadedImageObject); - } finally { - setIsLoading(false); - } + const loadImages = async (): Promise => { + const imageSrc = await loadImage(); + setLoadedImage(imageSrc); + setIsLoading(false); }; - loadAllImages(); + loadImages(); }, [mainSrc, fallback, useDefaultSrc]); - return { loadedImage, isSrcLoading }; + return { ref, loadedImage, isSrcLoading }; }; export default useImageLoader; diff --git a/src/Card/index.scss b/src/Card/index.scss index 46f55e89ea..93c47e1b2e 100644 --- a/src/Card/index.scss +++ b/src/Card/index.scss @@ -337,6 +337,7 @@ a.pgn__card { } .pgn__card-image-cap-loader { + margin-top: $loading-skeleton-spacer; display: none; &.show { @@ -345,9 +346,6 @@ a.pgn__card { } .react-loading-skeleton { - margin-bottom: -$loading-skeleton-spacer; - position: relative; - top: -$loading-skeleton-spacer; height: 100%; border-bottom-right-radius: 0; border-bottom-left-radius: 0; From ffbf061fe7df987e7102e0182fafc126bed47d32 Mon Sep 17 00:00:00 2001 From: Kyrylo Hudym-Levkovych Date: Mon, 4 Dec 2023 11:44:02 +0200 Subject: [PATCH 08/13] test: update tests --- .../__snapshots__/CardCarousel.test.jsx.snap | 300 +++++++++++-- src/Card/CardImageWithSkeleton.jsx | 2 +- src/Card/tests/CardImageCap.test.jsx | 70 +-- .../__snapshots__/CardDeck.test.jsx.snap | 402 +++++++++++++++--- .../__snapshots__/CardGrid.test.jsx.snap | 62 ++- .../__snapshots__/CardImageCap.test.jsx.snap | 192 +++++++-- src/hooks/tests/useImageLoader.test.jsx | 54 +++ src/hooks/useImageLoader.mdx | 47 ++ src/{Card => }/hooks/useImageLoader.tsx | 9 +- 9 files changed, 909 insertions(+), 229 deletions(-) create mode 100644 src/hooks/tests/useImageLoader.test.jsx create mode 100644 src/hooks/useImageLoader.mdx rename src/{Card => }/hooks/useImageLoader.tsx (89%) diff --git a/src/Card/CardCarousel/tests/__snapshots__/CardCarousel.test.jsx.snap b/src/Card/CardCarousel/tests/__snapshots__/CardCarousel.test.jsx.snap index 2ad5754ef4..d668a3679d 100644 --- a/src/Card/CardCarousel/tests/__snapshots__/CardCarousel.test.jsx.snap +++ b/src/Card/CardCarousel/tests/__snapshots__/CardCarousel.test.jsx.snap @@ -108,12 +108,26 @@ exports[` renders card carousel with custom title and subtitles
+ + + ‌ + +
+
renders card carousel with custom title and subtitles
+ + + ‌ + +
+
renders card carousel with custom title and subtitles
+ + + ‌ + +
+
renders card carousel with custom title and subtitles
+ + + ‌ + +
+
renders card carousel with custom title and subtitles
+ + + ‌ + +
+
renders card carousel with title and subtitles 1`] = `
+ + + ‌ + +
+
renders card carousel with title and subtitles 1`] = `
+ + + ‌ + +
+
renders card carousel with title and subtitles 1`] = `
+ + + ‌ + +
+
renders card carousel with title and subtitles 1`] = `
+ + + ‌ + +
+
renders card carousel with title and subtitles 1`] = `
+ + + ‌ + +
+
renders default card carousel 1`] = `
+ + + ‌ + +
+
renders default card carousel 1`] = `
+ + + ‌ + +
+
renders default card carousel 1`] = `
+ + + ‌ + +
+
renders default card carousel 1`] = `
+ + + ‌ + +
+
renders default card carousel 1`] = `
+ + + ‌ + +
+
', () => { />, ); - expect(screen.getByAltText('Src alt text').src).toEqual('http://src.image/'); - expect(screen.getByAltText('Logo alt text').src).toEqual('http://logo.image/'); - }); - - it('render with loading state', () => { - render( - , - ); - - expect(screen.queryByAltText('Src alt text')).toBeNull(); - expect(screen.queryByAltText('Logo alt text')).toBeNull(); - - expect(screen.getByTestId('image-loader-wrapper')).toBeInTheDocument(); - expect(screen.getByTestId('image-loader-wrapper').firstChild).toHaveClass('pgn__card-image-cap-loader'); - expect(screen.getByTestId('image-loader-wrapper').lastChild).toHaveClass('pgn__card-logo-cap'); - }); - - it('replaces image with fallback one in case of error', () => { - render( - , - ); - const srcImg = screen.getByAltText('Src alt text'); - expect(srcImg.src).toEqual('http://src.image/'); - fireEvent.load(srcImg); - fireEvent.error(srcImg); - expect(srcImg.src).toEqual('http://src.image.fallback/'); - - const logoImg = screen.getByAltText('Logo alt text'); - expect(logoImg.src).toEqual('http://logo.image/'); - fireEvent.load(srcImg); - fireEvent.error(logoImg); - expect(logoImg.src).toEqual('http://logo.image.fallback/'); - }); - - it('hiding component if it isn`t fallbackLogoSrc and logoSrc don`t work', () => { - render(); - - const logoImg = screen.getByAltText('Logo alt text'); - fireEvent.load(logoImg); - expect(logoImg.className).toEqual('pgn__card-logo-cap show'); - fireEvent.error(logoImg); - expect(logoImg.className).toEqual('pgn__card-logo-cap'); - }); - - it('hiding component if it isn`t fallbackSrc and src don`t work', () => { - render(); - - const srcImg = screen.getByAltText('Src alt text'); - fireEvent.load(srcImg); - fireEvent.error(srcImg); - // test-file-stub is what our fileMock.js returns for all images - expect(srcImg.src.endsWith('test-file-stub')).toEqual(true); + expect(screen.getByAltText('Src alt text')).toBeInTheDocument(); + expect(screen.getByAltText('Logo alt text')).toBeInTheDocument(); }); }); diff --git a/src/Card/tests/__snapshots__/CardDeck.test.jsx.snap b/src/Card/tests/__snapshots__/CardDeck.test.jsx.snap index b5bc5b8e96..58b0c978d5 100644 --- a/src/Card/tests/__snapshots__/CardDeck.test.jsx.snap +++ b/src/Card/tests/__snapshots__/CardDeck.test.jsx.snap @@ -18,12 +18,26 @@ exports[` has tabIndex="-1" when \`hasInteractiveChildren\` is true
+ + + ‌ + +
+
has tabIndex="-1" when \`hasInteractiveChildren\` is true
+ + + ‌ + +
+
has tabIndex="-1" when \`hasInteractiveChildren\` is true
+ + + ‌ + +
+
has tabIndex="-1" when \`hasInteractiveChildren\` is true
+ + + ‌ + +
+
has tabIndex="-1" when \`hasInteractiveChildren\` is true
+ + + ‌ + +
+
renders default columnSizes 1`] = `
+ + + ‌ + +
+
renders default columnSizes 1`] = `
+ + + ‌ + +
+
renders default columnSizes 1`] = `
+ + + ‌ + +
+
renders default columnSizes 1`] = `
+ + + ‌ + +
+
renders default columnSizes 1`] = `
+ + + ‌ + +
+
renders with controlled columnSizes 1`] = `
+ + + ‌ + +
+
renders with controlled columnSizes 1`] = `
+ + + ‌ + +
+
renders with controlled columnSizes 1`] = `
+ + + ‌ + +
+
renders with controlled columnSizes 1`] = `
+ + + ‌ + +
+
renders with controlled columnSizes 1`] = `
+ + + ‌ + +
+
renders with disabled equal height 1`] = `
+ + + ‌ + +
+
renders with disabled equal height 1`] = `
+ + + ‌ + +
+
renders with disabled equal height 1`] = `
+ + + ‌ + +
+
renders with disabled equal height 1`] = `
+ + + ‌ + +
+
renders with disabled equal height 1`] = `
+ + + ‌ + +
+
renders with disabled equal height 1`] = `
-`; \ No newline at end of file +`; diff --git a/src/Card/tests/__snapshots__/CardGrid.test.jsx.snap b/src/Card/tests/__snapshots__/CardGrid.test.jsx.snap index 65425dd9d7..0369e7579c 100644 --- a/src/Card/tests/__snapshots__/CardGrid.test.jsx.snap +++ b/src/Card/tests/__snapshots__/CardGrid.test.jsx.snap @@ -17,12 +17,26 @@ exports[` Controlled Rendering renders with controlled columnSizes 1
+ + + ‌ + +
+
Controlled Rendering renders with disabled equal height 1`
+ + + ‌ + +
+
Uncontrolled Rendering renders default columnSizes 1`] = `
+ + + ‌ + +
+
Uncontrolled Rendering renders default columnSizes 1`] = `
-`; \ No newline at end of file +`; diff --git a/src/Card/tests/__snapshots__/CardImageCap.test.jsx.snap b/src/Card/tests/__snapshots__/CardImageCap.test.jsx.snap index ad20491c2a..395b4c8a55 100644 --- a/src/Card/tests/__snapshots__/CardImageCap.test.jsx.snap +++ b/src/Card/tests/__snapshots__/CardImageCap.test.jsx.snap @@ -1,16 +1,80 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[` renders with loading equals lazy 1`] = ` +
+ + + ‌ + +
+ + + + + ‌ + +
+ + Logo alt +
+`; + exports[` renders with scr prop and srcAlt 1`] = `
+ + + ‌ + +
+ Alt text
`; @@ -19,12 +83,26 @@ exports[` renders with scr prop in horizontal orientation 1`] =
+ + + ‌ + +
+
`; @@ -33,19 +111,47 @@ exports[` renders with src and logoSrc prop in horizontal orient
+ + + ‌ + +
+ + + + ‌ + +
+
`; @@ -54,42 +160,48 @@ exports[` renders with src, logoSrc and logoAlt props 1`] = `
+ + + ‌ + +
+ + + + ‌ + +
+ Logo alt
`; - -exports[` renders with loading equals lazy 1`] = ` -
- - Logo alt -
-`; \ No newline at end of file diff --git a/src/hooks/tests/useImageLoader.test.jsx b/src/hooks/tests/useImageLoader.test.jsx new file mode 100644 index 0000000000..ea1984814a --- /dev/null +++ b/src/hooks/tests/useImageLoader.test.jsx @@ -0,0 +1,54 @@ +import React from 'react'; +import { + render, screen, act, waitFor, fireEvent, +} from '@testing-library/react'; + +import useImageLoader from '../useImageLoader'; + +const MAIN_SRC = 'main-source.jpg'; +const FALLBACK = 'fallback-source.jpg'; +const ALT = 'test'; +const TEST_ID = 'loading-indicator'; + +function TestComponent({ +// eslint-disable-next-line react/prop-types + mainSrc, fallback, alt, ...rest +}) { + const { ref, isSrcLoading } = useImageLoader({ mainSrc, fallback, ...rest }); + return ( + <> + {isSrcLoading &&
Loading...
} + {alt} + + ); +} + +describe('useImageLoader', () => { + it('should set loading state to false when the main source loads successfully and update img src', async () => { + render(); + const imgElement = screen.getByAltText(ALT); + + expect(screen.getByTestId(TEST_ID)).toBeInTheDocument(); + + await act(async () => { + fireEvent.load(imgElement); + }); + + await waitFor(() => expect(screen.queryByTestId(TEST_ID)).not.toBeInTheDocument()); + + 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(); + const imgElement = screen.getByAltText(ALT); + + expect(screen.getByTestId(TEST_ID)).toBeInTheDocument(); + + await act(async () => { + fireEvent.error(imgElement); + }); + + expect(imgElement.src).toContain(FALLBACK); + }); +}); diff --git a/src/hooks/useImageLoader.mdx b/src/hooks/useImageLoader.mdx new file mode 100644 index 0000000000..3bae5a7aa8 --- /dev/null +++ b/src/hooks/useImageLoader.mdx @@ -0,0 +1,47 @@ +--- +title: 'useImageLoader' +type: 'hook' +categories: +- Hooks +components: +- useImageLoader +status: 'New' +designStatus: 'Done' +devStatus: 'Done' +notes: '' +--- + +## Sample Usage +This hook designed to simplify the loading and handling of images in a React application. It provides a convenient way to asynchronously load images with fallbacks and retry mechanisms, making it robust for scenarios where primary image sources may fail. + + +```jsx live + +export const MyImageComponent = ({ mainSrc, fallback, alt, useDefaultSrc }) => { + const { ref, isSrcLoading } = useImageLoader({ + mainSrc, + fallback, + useDefaultSrc, + }); + + return ( +
+ {isSrcLoading &&
Loading...
} + {alt} +
+ ); +}; + +// Now you can use your custom component within the MDX content +# My MDX Document + +Here's an example of using the `MyImageComponent` with the `useImageLoader` hook: + + + +``` diff --git a/src/Card/hooks/useImageLoader.tsx b/src/hooks/useImageLoader.tsx similarity index 89% rename from src/Card/hooks/useImageLoader.tsx rename to src/hooks/useImageLoader.tsx index 4fb7bfb44d..2d0c2d090e 100644 --- a/src/Card/hooks/useImageLoader.tsx +++ b/src/hooks/useImageLoader.tsx @@ -11,7 +11,6 @@ interface ImageLoaderProps { interface UseImageLoaderResult { ref: RefObject; - loadedImage: string | null; isSrcLoading: boolean; } @@ -21,7 +20,6 @@ const useImageLoader = ({ useDefaultSrc = false, }: ImageLoaderProps): UseImageLoaderResult => { const ref = useRef(null); - const [loadedImage, setLoadedImage] = useState(null); const [isSrcLoading, setIsLoading] = useState(true); useEffect(() => { @@ -31,6 +29,8 @@ const useImageLoader = ({ const img = ref.current; const loadImageWithRetry = async (src: string): Promise => { + setIsLoading(true); + await new Promise((resolve, reject) => { img.onload = () => resolve(); img.onerror = () => reject(new Error(`Failed to load image: ${src}`)); @@ -72,15 +72,14 @@ const useImageLoader = ({ }; const loadImages = async (): Promise => { - const imageSrc = await loadImage(); - setLoadedImage(imageSrc); + await loadImage(); setIsLoading(false); }; loadImages(); }, [mainSrc, fallback, useDefaultSrc]); - return { ref, loadedImage, isSrcLoading }; + return { ref, isSrcLoading }; }; export default useImageLoader; From 3fb82f3f3d4883535eb57d8fcf4c8ffd5fc1aec8 Mon Sep 17 00:00:00 2001 From: Kyrylo Hudym-Levkovych Date: Mon, 4 Dec 2023 13:09:42 +0200 Subject: [PATCH 09/13] refactor: rename index.d.ts file --- src/{index.d.ts => declaration.d.ts} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename src/{index.d.ts => declaration.d.ts} (100%) diff --git a/src/index.d.ts b/src/declaration.d.ts similarity index 100% rename from src/index.d.ts rename to src/declaration.d.ts From b9ef7fb1fdd8550f4e7f23d2fdaeec104a261e1f Mon Sep 17 00:00:00 2001 From: Kyrylo Hudym-Levkovych Date: Mon, 4 Dec 2023 13:18:39 +0200 Subject: [PATCH 10/13] fix: add linter fixes --- src/Card/index.scss | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/Card/index.scss b/src/Card/index.scss index 93c47e1b2e..33860726ce 100644 --- a/src/Card/index.scss +++ b/src/Card/index.scss @@ -340,10 +340,10 @@ a.pgn__card { margin-top: $loading-skeleton-spacer; display: none; - &.show { - display: block; - height: 100%; - } + &.show { + display: block; + height: 100%; + } .react-loading-skeleton { height: 100%; @@ -364,9 +364,9 @@ a.pgn__card { background-color: $white; display: none; - &.show { - display: block; - } + &.show { + display: block; + } } } From 4c2bea3d57cabc37481746f7a9e70a1389b09e36 Mon Sep 17 00:00:00 2001 From: Kyrylo Hudym-Levkovych Date: Mon, 4 Dec 2023 16:00:53 +0200 Subject: [PATCH 11/13] fix: add fixes to build docs --- src/Card/CardImageWithSkeleton.jsx | 2 -- src/hooks/useImageLoader.mdx | 47 ------------------------------ src/hooks/useImageLoader.tsx | 2 +- 3 files changed, 1 insertion(+), 50 deletions(-) delete mode 100644 src/hooks/useImageLoader.mdx diff --git a/src/Card/CardImageWithSkeleton.jsx b/src/Card/CardImageWithSkeleton.jsx index 09b4181cfe..d93773488a 100644 --- a/src/Card/CardImageWithSkeleton.jsx +++ b/src/Card/CardImageWithSkeleton.jsx @@ -54,7 +54,6 @@ CardImageWithSkeleton.propTypes = { skeletonClassName: PropTypes.string, imageLoadingType: PropTypes.oneOf(['eager', 'lazy']), isLoading: PropTypes.bool, - parentRef: PropTypes.shape({ current: PropTypes.instanceOf(Element) }), }; CardImageWithSkeleton.defaultProps = { @@ -66,7 +65,6 @@ CardImageWithSkeleton.defaultProps = { skeletonHeight: undefined, skeletonWidth: undefined, isLoading: false, - parentRef: { current: null }, }; export default CardImageWithSkeleton; diff --git a/src/hooks/useImageLoader.mdx b/src/hooks/useImageLoader.mdx deleted file mode 100644 index 3bae5a7aa8..0000000000 --- a/src/hooks/useImageLoader.mdx +++ /dev/null @@ -1,47 +0,0 @@ ---- -title: 'useImageLoader' -type: 'hook' -categories: -- Hooks -components: -- useImageLoader -status: 'New' -designStatus: 'Done' -devStatus: 'Done' -notes: '' ---- - -## Sample Usage -This hook designed to simplify the loading and handling of images in a React application. It provides a convenient way to asynchronously load images with fallbacks and retry mechanisms, making it robust for scenarios where primary image sources may fail. - - -```jsx live - -export const MyImageComponent = ({ mainSrc, fallback, alt, useDefaultSrc }) => { - const { ref, isSrcLoading } = useImageLoader({ - mainSrc, - fallback, - useDefaultSrc, - }); - - return ( -
- {isSrcLoading &&
Loading...
} - {alt} -
- ); -}; - -// Now you can use your custom component within the MDX content -# My MDX Document - -Here's an example of using the `MyImageComponent` with the `useImageLoader` hook: - - - -``` diff --git a/src/hooks/useImageLoader.tsx b/src/hooks/useImageLoader.tsx index 2d0c2d090e..08c843a9c2 100644 --- a/src/hooks/useImageLoader.tsx +++ b/src/hooks/useImageLoader.tsx @@ -1,7 +1,7 @@ import { useState, useEffect, useRef, RefObject, } from 'react'; -import cardSrcFallbackImg from '../fallback-default.png'; +import cardSrcFallbackImg from '../Card/fallback-default.png'; interface ImageLoaderProps { mainSrc: string; From 92d5b38bd1cc17105641ad0a16f9e1ae25a3ab36 Mon Sep 17 00:00:00 2001 From: Kyrylo Hudym-Levkovych Date: Tue, 5 Dec 2023 14:41:51 +0200 Subject: [PATCH 12/13] fix: fix issue with logo, update doc --- src/Card/CardImageCap.jsx | 1 + src/Card/CardImageWithSkeleton.jsx | 5 +- .../__snapshots__/CardImageCap.test.jsx.snap | 6 +-- ...{useImageLoader.tsx => useImageLoader.jsx} | 33 ++++--------- src/hooks/useImageLoader.mdx | 46 +++++++++++++++++++ src/index.js | 1 + 6 files changed, 65 insertions(+), 27 deletions(-) rename src/hooks/{useImageLoader.tsx => useImageLoader.jsx} (66%) create mode 100644 src/hooks/useImageLoader.mdx diff --git a/src/Card/CardImageCap.jsx b/src/Card/CardImageCap.jsx index a9cee156a8..88b2406bac 100644 --- a/src/Card/CardImageCap.jsx +++ b/src/Card/CardImageCap.jsx @@ -38,6 +38,7 @@ const CardImageCap = React.forwardRef(({ fallback={fallbackSrc} className="pgn__card-image-cap" useDefaultSrc + withSkeleton skeletonWidth={skeletonWidth} skeletonHeight={imageSkeletonHeight} imageLoadingType={imageLoadingType} diff --git a/src/Card/CardImageWithSkeleton.jsx b/src/Card/CardImageWithSkeleton.jsx index d93773488a..86b6aa76be 100644 --- a/src/Card/CardImageWithSkeleton.jsx +++ b/src/Card/CardImageWithSkeleton.jsx @@ -10,6 +10,7 @@ function CardImageWithSkeleton({ alt, fallback, className, + withSkeleton, useDefaultSrc, skeletonWidth, skeletonHeight, @@ -29,7 +30,7 @@ function CardImageWithSkeleton({ return ( <> @@ -54,6 +55,7 @@ CardImageWithSkeleton.propTypes = { skeletonClassName: PropTypes.string, imageLoadingType: PropTypes.oneOf(['eager', 'lazy']), isLoading: PropTypes.bool, + withSkeleton: PropTypes.bool, }; CardImageWithSkeleton.defaultProps = { @@ -65,6 +67,7 @@ CardImageWithSkeleton.defaultProps = { skeletonHeight: undefined, skeletonWidth: undefined, isLoading: false, + withSkeleton: false, }; export default CardImageWithSkeleton; diff --git a/src/Card/tests/__snapshots__/CardImageCap.test.jsx.snap b/src/Card/tests/__snapshots__/CardImageCap.test.jsx.snap index 395b4c8a55..736696b4fb 100644 --- a/src/Card/tests/__snapshots__/CardImageCap.test.jsx.snap +++ b/src/Card/tests/__snapshots__/CardImageCap.test.jsx.snap @@ -28,7 +28,7 @@ exports[` renders with loading equals lazy 1`] = ` renders with src and logoSrc prop in horizontal orient renders with src, logoSrc and logoAlt props 1`] = ` ; - isSrcLoading: boolean; -} - const useImageLoader = ({ mainSrc, fallback, useDefaultSrc = false, -}: ImageLoaderProps): UseImageLoaderResult => { - const ref = useRef(null); +}) => { + const ref = useRef(null); const [isSrcLoading, setIsLoading] = useState(true); useEffect(() => { - if (!mainSrc || !fallback || !ref.current) { + if ((!mainSrc && !fallback) || !ref.current) { return; } const img = ref.current; - const loadImageWithRetry = async (src: string): Promise => { + const loadImageWithRetry = async (src) => { setIsLoading(true); - await new Promise((resolve, reject) => { + await new Promise((resolve, reject) => { img.onload = () => resolve(); img.onerror = () => reject(new Error(`Failed to load image: ${src}`)); img.src = src; }); }; - const loadImage = async (): Promise => { - let imageSrc: string | null = null; + const loadImage = async () => { + let imageSrc = null; const sources = [mainSrc]; if (fallback) { @@ -64,14 +51,14 @@ const useImageLoader = ({ imageSrc = src; break; } catch (error) { + console.error(error); // Continue to the next source if loading fails } } - return imageSrc; }; - const loadImages = async (): Promise => { + const loadImages = async () => { await loadImage(); setIsLoading(false); }; diff --git a/src/hooks/useImageLoader.mdx b/src/hooks/useImageLoader.mdx new file mode 100644 index 0000000000..6bd25dc66a --- /dev/null +++ b/src/hooks/useImageLoader.mdx @@ -0,0 +1,46 @@ +--- +title: 'useImageLoader' +type: 'hook' +categories: +- Hooks +components: +- useImageLoader +status: 'New' +designStatus: 'Done' +devStatus: 'Done' +notes: '' +--- + +## Base Usage +This hook designed to simplify the loading and handling of images. It provides a convenient +way to asynchronously load images with fallbacks and retry mechanisms, making it robust for scenarios where primary +image sources may fail. + +```jsx live + +() => { + const MyImageComponent = ({ mainSrc, fallback, alt, useDefaultSrc }) => { + const { ref, isSrcLoading } = useImageLoader({ + mainSrc, + fallback, + useDefaultSrc, + }); + + return ( +
+ {isSrcLoading &&
Loading...
} + {alt} +
+ ); + } + + return ( + + ); +} +``` \ No newline at end of file diff --git a/src/index.js b/src/index.js index a25410d8f9..cfde7bd75d 100644 --- a/src/index.js +++ b/src/index.js @@ -166,6 +166,7 @@ export { default as useToggle } from './hooks/useToggle'; export { default as useArrowKeyNavigation } from './hooks/useArrowKeyNavigation'; export { default as useIndexOfLastVisibleChild } from './hooks/useIndexOfLastVisibleChild'; export { default as useIsVisible } from './hooks/useIsVisible'; +export { default as useImageLoader } from './hooks/useImageLoader'; export { OverflowScrollContext, OverflowScroll, From 0f9c94c297552d9a09bdc923d829e169b1f73e01 Mon Sep 17 00:00:00 2001 From: Kyrylo Hudym-Levkovych Date: Thu, 7 Dec 2023 09:57:53 +0200 Subject: [PATCH 13/13] refactor: add changes after review --- src/Card/CardImageWithSkeleton.jsx | 5 ++- src/hooks/tests/useImageLoader.test.jsx | 49 +++++++++++++------------ src/hooks/useImageLoader.jsx | 10 ++--- src/hooks/useImageLoader.mdx | 6 +-- www/src/scss/base.scss | 1 - 5 files changed, 37 insertions(+), 34 deletions(-) diff --git a/src/Card/CardImageWithSkeleton.jsx b/src/Card/CardImageWithSkeleton.jsx index 86b6aa76be..8d326217e7 100644 --- 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} /> - {alt} {isSrcLoading &&
Loading...
} - {alt} + {alt} ); } describe('useImageLoader', () => { - it('should set loading state to false when the main source loads successfully and update img src', async () => { - render(); - 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(); + 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(); - 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(); + 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 index a717246df9..76c75e9c6d 100644 --- 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 index 6bd25dc66a..45b3e52c31 100644 --- 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 ( diff --git a/www/src/scss/base.scss b/www/src/scss/base.scss index 1b78d9a3ee..913b13f76d 100644 --- 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;