From 83e15c051d1e472da3c7a28aefd2828dd7d30dc5 Mon Sep 17 00:00:00 2001 From: dominictb Date: Wed, 22 May 2024 18:39:36 +0700 Subject: [PATCH 1/7] feat: maintain aspect ratio of image container customize image behavior with context Signed-off-by: dominictb --- .../Image/ImageBehaviorContextProvider.tsx | 18 +++++++ src/components/Image/index.tsx | 12 +++-- .../ReportActionItemImage.tsx | 3 ++ .../ReportActionItemImages.tsx | 47 ++++++++++--------- 4 files changed, 54 insertions(+), 26 deletions(-) create mode 100644 src/components/Image/ImageBehaviorContextProvider.tsx diff --git a/src/components/Image/ImageBehaviorContextProvider.tsx b/src/components/Image/ImageBehaviorContextProvider.tsx new file mode 100644 index 000000000000..3d48c9ad955c --- /dev/null +++ b/src/components/Image/ImageBehaviorContextProvider.tsx @@ -0,0 +1,18 @@ +import React, {createContext} from 'react'; + +type ImageBehaviorContextValue = { + /** + * Disable the logic to set aspect ratio of the container div based on the image aspect ratio. + */ + doNotSetAspectRatio: boolean; +}; + +const ImageBehaviorContext = createContext({ + doNotSetAspectRatio: false, +}); + +const ImageBehaviorContextProvider = ({children, ...value}: {children: React.ReactNode} & ImageBehaviorContextValue) => { + return {children}; +}; + +export {ImageBehaviorContext, ImageBehaviorContextProvider}; diff --git a/src/components/Image/index.tsx b/src/components/Image/index.tsx index e6cecdc0d5ec..820bf199d356 100644 --- a/src/components/Image/index.tsx +++ b/src/components/Image/index.tsx @@ -1,14 +1,17 @@ -import React, {useCallback, useMemo, useState} from 'react'; +import React, {useCallback, useContext, useMemo, useState} from 'react'; import {withOnyx} from 'react-native-onyx'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; import BaseImage from './BaseImage'; +import {ImageBehaviorContext} from './ImageBehaviorContextProvider'; import type {ImageOnLoadEvent, ImageOnyxProps, ImageOwnProps, ImageProps} from './types'; function Image({source: propsSource, isAuthTokenRequired = false, session, onLoad, objectPosition = CONST.IMAGE_OBJECT_POSITION.INITIAL, style, ...forwardedProps}: ImageProps) { const [aspectRatio, setAspectRatio] = useState(null); const isObjectPositionTop = objectPosition === CONST.IMAGE_OBJECT_POSITION.TOP; + const {doNotSetAspectRatio} = useContext(ImageBehaviorContext); + const updateAspectRatio = useCallback( (width: number, height: number) => { if (!isObjectPositionTop) { @@ -30,10 +33,11 @@ function Image({source: propsSource, isAuthTokenRequired = false, session, onLoa const {width, height} = event.nativeEvent; onLoad?.(event); - - updateAspectRatio(width, height); + if (!doNotSetAspectRatio) { + updateAspectRatio(width, height); + } }, - [onLoad, updateAspectRatio], + [onLoad, updateAspectRatio, doNotSetAspectRatio], ); /** * Check if the image source is a URL - if so the `encryptedAuthToken` is appended diff --git a/src/components/ReportActionItem/ReportActionItemImage.tsx b/src/components/ReportActionItem/ReportActionItemImage.tsx index 4e7f123a2fb2..a2efc4ffb154 100644 --- a/src/components/ReportActionItem/ReportActionItemImage.tsx +++ b/src/components/ReportActionItem/ReportActionItemImage.tsx @@ -46,6 +46,9 @@ type ReportActionItemImageProps = { /** Whether there are other images displayed in the same parent container */ isSingleImage?: boolean; + + /** Whether there are other images displayed in the same parent container */ + doNotSetAspectRatio?: boolean; }; /** diff --git a/src/components/ReportActionItem/ReportActionItemImages.tsx b/src/components/ReportActionItem/ReportActionItemImages.tsx index e2bcce9b9f1b..6851f3a49810 100644 --- a/src/components/ReportActionItem/ReportActionItemImages.tsx +++ b/src/components/ReportActionItem/ReportActionItemImages.tsx @@ -2,6 +2,7 @@ import React from 'react'; import {View} from 'react-native'; import {Polygon, Svg} from 'react-native-svg'; +import {ImageBehaviorContextProvider} from '@components/Image/ImageBehaviorContextProvider'; import Text from '@components/Text'; import useStyleUtils from '@hooks/useStyleUtils'; import useTheme from '@hooks/useTheme'; @@ -65,28 +66,30 @@ function ReportActionItemImages({images, size, total, isHovered = false}: Report return ( - {shownImages.map(({thumbnail, isThumbnail, image, transaction, isLocalFile, fileExtension, filename}, index) => { - // Show a border to separate multiple images. Shown to the right for each except the last. - const shouldShowBorder = shownImages.length > 1 && index < shownImages.length - 1; - const borderStyle = shouldShowBorder ? styles.reportActionItemImageBorder : {}; - return ( - - - - ); - })} + + {shownImages.map(({thumbnail, isThumbnail, image, transaction, isLocalFile, fileExtension, filename}, index) => { + // Show a border to separate multiple images. Shown to the right for each except the last. + const shouldShowBorder = shownImages.length > 1 && index < shownImages.length - 1; + const borderStyle = shouldShowBorder ? styles.reportActionItemImageBorder : {}; + return ( + + + + ); + })} + {remaining > 0 && ( From baefafbebdf7e05a912769abeee3a60a23514a3f Mon Sep 17 00:00:00 2001 From: dominictb Date: Wed, 22 May 2024 19:01:28 +0700 Subject: [PATCH 2/7] fix: update logic for zero opacity style --- src/components/Image/index.tsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/components/Image/index.tsx b/src/components/Image/index.tsx index 820bf199d356..9827afebedd9 100644 --- a/src/components/Image/index.tsx +++ b/src/components/Image/index.tsx @@ -63,12 +63,14 @@ function Image({source: propsSource, isAuthTokenRequired = false, session, onLoa // eslint-disable-next-line react-hooks/exhaustive-deps }, [propsSource, isAuthTokenRequired]); + const shouldOpacityBeZero = isObjectPositionTop && !doNotSetAspectRatio && !aspectRatio; + return ( ); From ce082ab4adce3f8a9f72fd2ebaae07f523667538 Mon Sep 17 00:00:00 2001 From: dominictb Date: Wed, 22 May 2024 22:25:33 +0700 Subject: [PATCH 3/7] fix: lint --- src/components/Image/ImageBehaviorContextProvider.tsx | 4 ++-- src/components/ReportActionItem/ReportActionItemImages.tsx | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/components/Image/ImageBehaviorContextProvider.tsx b/src/components/Image/ImageBehaviorContextProvider.tsx index 3d48c9ad955c..16bac26f49a8 100644 --- a/src/components/Image/ImageBehaviorContextProvider.tsx +++ b/src/components/Image/ImageBehaviorContextProvider.tsx @@ -11,8 +11,8 @@ const ImageBehaviorContext = createContext({ doNotSetAspectRatio: false, }); -const ImageBehaviorContextProvider = ({children, ...value}: {children: React.ReactNode} & ImageBehaviorContextValue) => { +function ImageBehaviorContextProvider({children, ...value}: {children: React.ReactNode} & ImageBehaviorContextValue) { return {children}; -}; +} export {ImageBehaviorContext, ImageBehaviorContextProvider}; diff --git a/src/components/ReportActionItem/ReportActionItemImages.tsx b/src/components/ReportActionItem/ReportActionItemImages.tsx index 6851f3a49810..e5d0a54fba04 100644 --- a/src/components/ReportActionItem/ReportActionItemImages.tsx +++ b/src/components/ReportActionItem/ReportActionItemImages.tsx @@ -66,7 +66,7 @@ function ReportActionItemImages({images, size, total, isHovered = false}: Report return ( - + {shownImages.map(({thumbnail, isThumbnail, image, transaction, isLocalFile, fileExtension, filename}, index) => { // Show a border to separate multiple images. Shown to the right for each except the last. const shouldShowBorder = shownImages.length > 1 && index < shownImages.length - 1; From a144f2b6ad493219913c3db5aad136cde7a76a00 Mon Sep 17 00:00:00 2001 From: dominictb Date: Thu, 23 May 2024 00:44:40 +0700 Subject: [PATCH 4/7] fix: update prop name Signed-off-by: dominictb --- .../Image/ImageBehaviorContextProvider.tsx | 4 ++-- src/components/Image/index.tsx | 12 +++++------- .../ReportActionItem/ReportActionItemImages.tsx | 2 +- 3 files changed, 8 insertions(+), 10 deletions(-) diff --git a/src/components/Image/ImageBehaviorContextProvider.tsx b/src/components/Image/ImageBehaviorContextProvider.tsx index 16bac26f49a8..4489552eba89 100644 --- a/src/components/Image/ImageBehaviorContextProvider.tsx +++ b/src/components/Image/ImageBehaviorContextProvider.tsx @@ -4,11 +4,11 @@ type ImageBehaviorContextValue = { /** * Disable the logic to set aspect ratio of the container div based on the image aspect ratio. */ - doNotSetAspectRatio: boolean; + doNotSetAspectRatioInStyle: boolean; }; const ImageBehaviorContext = createContext({ - doNotSetAspectRatio: false, + doNotSetAspectRatioInStyle: false, }); function ImageBehaviorContextProvider({children, ...value}: {children: React.ReactNode} & ImageBehaviorContextValue) { diff --git a/src/components/Image/index.tsx b/src/components/Image/index.tsx index 9827afebedd9..96d943ceb643 100644 --- a/src/components/Image/index.tsx +++ b/src/components/Image/index.tsx @@ -10,7 +10,7 @@ function Image({source: propsSource, isAuthTokenRequired = false, session, onLoa const [aspectRatio, setAspectRatio] = useState(null); const isObjectPositionTop = objectPosition === CONST.IMAGE_OBJECT_POSITION.TOP; - const {doNotSetAspectRatio} = useContext(ImageBehaviorContext); + const {doNotSetAspectRatioInStyle} = useContext(ImageBehaviorContext); const updateAspectRatio = useCallback( (width: number, height: number) => { @@ -33,11 +33,9 @@ function Image({source: propsSource, isAuthTokenRequired = false, session, onLoa const {width, height} = event.nativeEvent; onLoad?.(event); - if (!doNotSetAspectRatio) { - updateAspectRatio(width, height); - } + updateAspectRatio(width, height); }, - [onLoad, updateAspectRatio, doNotSetAspectRatio], + [onLoad, updateAspectRatio], ); /** * Check if the image source is a URL - if so the `encryptedAuthToken` is appended @@ -63,14 +61,14 @@ function Image({source: propsSource, isAuthTokenRequired = false, session, onLoa // eslint-disable-next-line react-hooks/exhaustive-deps }, [propsSource, isAuthTokenRequired]); - const shouldOpacityBeZero = isObjectPositionTop && !doNotSetAspectRatio && !aspectRatio; + const shouldOpacityBeZero = isObjectPositionTop && !aspectRatio; return ( ); diff --git a/src/components/ReportActionItem/ReportActionItemImages.tsx b/src/components/ReportActionItem/ReportActionItemImages.tsx index e5d0a54fba04..d18ad4ea2509 100644 --- a/src/components/ReportActionItem/ReportActionItemImages.tsx +++ b/src/components/ReportActionItem/ReportActionItemImages.tsx @@ -66,7 +66,7 @@ function ReportActionItemImages({images, size, total, isHovered = false}: Report return ( - + {shownImages.map(({thumbnail, isThumbnail, image, transaction, isLocalFile, fileExtension, filename}, index) => { // Show a border to separate multiple images. Shown to the right for each except the last. const shouldShowBorder = shownImages.length > 1 && index < shownImages.length - 1; From ffddc8e0bb54dc784b3dd368a44988838490d896 Mon Sep 17 00:00:00 2001 From: dominictb Date: Thu, 23 May 2024 01:15:33 +0700 Subject: [PATCH 5/7] fix: remove unnecessary prop Signed-off-by: dominictb --- src/components/ReportActionItem/ReportActionItemImage.tsx | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/components/ReportActionItem/ReportActionItemImage.tsx b/src/components/ReportActionItem/ReportActionItemImage.tsx index a2efc4ffb154..4e7f123a2fb2 100644 --- a/src/components/ReportActionItem/ReportActionItemImage.tsx +++ b/src/components/ReportActionItem/ReportActionItemImage.tsx @@ -46,9 +46,6 @@ type ReportActionItemImageProps = { /** Whether there are other images displayed in the same parent container */ isSingleImage?: boolean; - - /** Whether there are other images displayed in the same parent container */ - doNotSetAspectRatio?: boolean; }; /** From 91d0852dc737d4ce4d6162303edcb0403b20a2fb Mon Sep 17 00:00:00 2001 From: dominictb Date: Mon, 27 May 2024 23:34:34 +0700 Subject: [PATCH 6/7] fix: update variable name --- src/components/Image/ImageBehaviorContextProvider.tsx | 4 ++-- src/components/Image/index.tsx | 7 +++++-- src/components/ReportActionItem/ReportActionItemImages.tsx | 2 +- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/components/Image/ImageBehaviorContextProvider.tsx b/src/components/Image/ImageBehaviorContextProvider.tsx index 4489552eba89..9029d9ccf763 100644 --- a/src/components/Image/ImageBehaviorContextProvider.tsx +++ b/src/components/Image/ImageBehaviorContextProvider.tsx @@ -4,11 +4,11 @@ type ImageBehaviorContextValue = { /** * Disable the logic to set aspect ratio of the container div based on the image aspect ratio. */ - doNotSetAspectRatioInStyle: boolean; + shouldSetAspectRatioInStyle: boolean; }; const ImageBehaviorContext = createContext({ - doNotSetAspectRatioInStyle: false, + shouldSetAspectRatioInStyle: true, }); function ImageBehaviorContextProvider({children, ...value}: {children: React.ReactNode} & ImageBehaviorContextValue) { diff --git a/src/components/Image/index.tsx b/src/components/Image/index.tsx index 96d943ceb643..f3cbc332c995 100644 --- a/src/components/Image/index.tsx +++ b/src/components/Image/index.tsx @@ -10,7 +10,7 @@ function Image({source: propsSource, isAuthTokenRequired = false, session, onLoa const [aspectRatio, setAspectRatio] = useState(null); const isObjectPositionTop = objectPosition === CONST.IMAGE_OBJECT_POSITION.TOP; - const {doNotSetAspectRatioInStyle} = useContext(ImageBehaviorContext); + const {shouldSetAspectRatioInStyle} = useContext(ImageBehaviorContext); const updateAspectRatio = useCallback( (width: number, height: number) => { @@ -61,6 +61,9 @@ function Image({source: propsSource, isAuthTokenRequired = false, session, onLoa // eslint-disable-next-line react-hooks/exhaustive-deps }, [propsSource, isAuthTokenRequired]); + /** + * If the image fails to load and the object position is top, we should hide the image by setting the opacity to 0. + */ const shouldOpacityBeZero = isObjectPositionTop && !aspectRatio; return ( @@ -68,7 +71,7 @@ function Image({source: propsSource, isAuthTokenRequired = false, session, onLoa // eslint-disable-next-line react/jsx-props-no-spreading {...forwardedProps} onLoad={handleLoad} - style={[style, !doNotSetAspectRatioInStyle && aspectRatio ? {aspectRatio, height: 'auto'} : {}, shouldOpacityBeZero && {opacity: 0}]} + style={[style, shouldSetAspectRatioInStyle && aspectRatio ? {aspectRatio, height: 'auto'} : {}, shouldOpacityBeZero && {opacity: 0}]} source={source} /> ); diff --git a/src/components/ReportActionItem/ReportActionItemImages.tsx b/src/components/ReportActionItem/ReportActionItemImages.tsx index d18ad4ea2509..5e3a26d3a870 100644 --- a/src/components/ReportActionItem/ReportActionItemImages.tsx +++ b/src/components/ReportActionItem/ReportActionItemImages.tsx @@ -66,7 +66,7 @@ function ReportActionItemImages({images, size, total, isHovered = false}: Report return ( - + {shownImages.map(({thumbnail, isThumbnail, image, transaction, isLocalFile, fileExtension, filename}, index) => { // Show a border to separate multiple images. Shown to the right for each except the last. const shouldShowBorder = shownImages.length > 1 && index < shownImages.length - 1; From aa836b77e85c53023d0762c2ed8952377f4c8ad4 Mon Sep 17 00:00:00 2001 From: Dominic <165644294+dominictb@users.noreply.github.com> Date: Wed, 29 May 2024 02:06:34 +0700 Subject: [PATCH 7/7] chore: update explanation comment Co-authored-by: Joel Davies --- src/components/Image/ImageBehaviorContextProvider.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/Image/ImageBehaviorContextProvider.tsx b/src/components/Image/ImageBehaviorContextProvider.tsx index 9029d9ccf763..cfff212c16f6 100644 --- a/src/components/Image/ImageBehaviorContextProvider.tsx +++ b/src/components/Image/ImageBehaviorContextProvider.tsx @@ -2,7 +2,7 @@ import React, {createContext} from 'react'; type ImageBehaviorContextValue = { /** - * Disable the logic to set aspect ratio of the container div based on the image aspect ratio. + * Determine whether or not to set the aspect ratio of the container div based on the image's aspect ratio. */ shouldSetAspectRatioInStyle: boolean; };