From 77549568cf0e596ab0113c42e3a60b5cf9b4b4f0 Mon Sep 17 00:00:00 2001 From: dukenv0307 Date: Thu, 7 Dec 2023 17:29:44 +0700 Subject: [PATCH 1/5] fix the current issue and regression --- src/components/AttachmentModal.js | 27 +++++++++------- .../extractAttachmentsFromReport.js | 29 ++--------------- .../Attachments/AttachmentCarousel/index.js | 27 ++-------------- .../AttachmentCarousel/index.native.js | 26 ++-------------- .../ReportActionItem/ReportActionItemImage.js | 31 ++++++++++++------- 5 files changed, 42 insertions(+), 98 deletions(-) diff --git a/src/components/AttachmentModal.js b/src/components/AttachmentModal.js index 57b0c6466a7f..5bfc607c0480 100755 --- a/src/components/AttachmentModal.js +++ b/src/components/AttachmentModal.js @@ -90,6 +90,9 @@ const propTypes = { /** Denotes whether it is a workspace avatar or not */ isWorkspaceAvatar: PropTypes.bool, + + /** Whether it is a receipt attachment or not */ + isReceiptAttachment: PropTypes.bool, }; const defaultProps = { @@ -107,6 +110,7 @@ const defaultProps = { onModalHide: () => {}, onCarouselAttachmentChange: () => {}, isWorkspaceAvatar: false, + isReceiptAttachment: false, }; function AttachmentModal(props) { @@ -118,7 +122,6 @@ function AttachmentModal(props) { const [isAttachmentInvalid, setIsAttachmentInvalid] = useState(false); const [isDeleteReceiptConfirmModalVisible, setIsDeleteReceiptConfirmModalVisible] = useState(false); const [isAuthTokenRequired, setIsAuthTokenRequired] = useState(props.isAuthTokenRequired); - const [isAttachmentReceipt, setIsAttachmentReceipt] = useState(null); const [attachmentInvalidReasonTitle, setAttachmentInvalidReasonTitle] = useState(''); const [attachmentInvalidReason, setAttachmentInvalidReason] = useState(null); const [source, setSource] = useState(props.source); @@ -154,7 +157,6 @@ function AttachmentModal(props) { (attachment) => { setSource(attachment.source); setFile(attachment.file); - setIsAttachmentReceipt(attachment.isReceipt); setIsAuthTokenRequired(attachment.isAuthTokenRequired); onCarouselAttachmentChange(attachment); }, @@ -357,7 +359,7 @@ function AttachmentModal(props) { const sourceForAttachmentView = props.source || source; const threeDotsMenuItems = useMemo(() => { - if (!isAttachmentReceipt || !props.parentReport || !props.parentReportActions) { + if (!props.isReceiptAttachment || !props.parentReport || !props.parentReportActions) { return []; } const menuItems = []; @@ -392,17 +394,17 @@ function AttachmentModal(props) { } return menuItems; // eslint-disable-next-line react-hooks/exhaustive-deps - }, [isAttachmentReceipt, props.parentReport, props.parentReportActions, props.policy, props.transaction, file]); + }, [props.isReceiptAttachment, props.parentReport, props.parentReportActions, props.policy, props.transaction, file]); // There are a few things that shouldn't be set until we absolutely know if the file is a receipt or an attachment. - // isAttachmentReceipt will be null until its certain what the file is, in which case it will then be true|false. + // props.isReceiptAttachment will be null until its certain what the file is, in which case it will then be true|false. let headerTitle = props.headerTitle; let shouldShowDownloadButton = false; let shouldShowThreeDotsButton = false; - if (!_.isNull(isAttachmentReceipt)) { - headerTitle = translate(isAttachmentReceipt ? 'common.receipt' : 'common.attachment'); - shouldShowDownloadButton = props.allowDownload && isDownloadButtonReadyToBeShown && !isAttachmentReceipt && !isOffline; - shouldShowThreeDotsButton = isAttachmentReceipt && isModalOpen; + if (!_.isEmpty(props.report)) { + headerTitle = translate(props.isReceiptAttachment ? 'common.receipt' : 'common.attachment'); + shouldShowDownloadButton = props.allowDownload && isDownloadButtonReadyToBeShown && !props.isReceiptAttachment && !isOffline; + shouldShowThreeDotsButton = props.isReceiptAttachment && isModalOpen; } return ( @@ -443,7 +445,7 @@ function AttachmentModal(props) { shouldOverlay /> - {!_.isEmpty(props.report) ? ( + {!_.isEmpty(props.report) && !props.isReceiptAttachment ? ( ) )} @@ -486,7 +489,7 @@ function AttachmentModal(props) { )} )} - {isAttachmentReceipt && ( + {props.isReceiptAttachment && ( )} - {!isAttachmentReceipt && ( + {!props.isReceiptAttachment && ( { - if (!ReportActionsUtils.shouldReportActionBeVisible(action, key)) { + if (!ReportActionsUtils.shouldReportActionBeVisible(action, key) || ReportActionsUtils.isMoneyRequestAction(action)) { return; } - // We're handling receipts differently here because receipt images are not - // part of the report action message, the images are constructed client-side - if (ReportActionsUtils.isMoneyRequestAction(action)) { - const transactionID = lodashGet(action, ['originalMessage', 'IOUTransactionID']); - if (!transactionID) { - return; - } - - if (TransactionUtils.hasReceipt(transaction)) { - const {image} = ReceiptUtils.getThumbnailAndImageURIs(transaction); - const isLocalFile = typeof image === 'string' && _.some(CONST.ATTACHMENT_LOCAL_URL_PREFIX, (prefix) => image.startsWith(prefix)); - attachments.unshift({ - source: tryResolveUrlFromApiRoot(image), - isAuthTokenRequired: !isLocalFile, - file: {name: transaction.filename}, - isReceipt: true, - transactionID, - }); - return; - } - } - const decision = _.get(action, ['message', 0, 'moderationDecision', 'decision'], ''); const hasBeenFlagged = decision === CONST.MODERATION.MODERATOR_DECISION_PENDING_HIDE || decision === CONST.MODERATION.MODERATOR_DECISION_HIDDEN; const html = _.get(action, ['message', 0, 'html'], '').replace('/>', `data-flagged="${hasBeenFlagged}" data-id="${action.reportActionID}"/>`); diff --git a/src/components/Attachments/AttachmentCarousel/index.js b/src/components/Attachments/AttachmentCarousel/index.js index 141e619e489e..1696f4adf0b4 100644 --- a/src/components/Attachments/AttachmentCarousel/index.js +++ b/src/components/Attachments/AttachmentCarousel/index.js @@ -1,4 +1,3 @@ -import lodashGet from 'lodash/get'; import React, {useCallback, useEffect, useRef, useState} from 'react'; import {FlatList, Keyboard, PixelRatio, View} from 'react-native'; import {withOnyx} from 'react-native-onyx'; @@ -28,7 +27,7 @@ const viewabilityConfig = { itemVisiblePercentThreshold: 95, }; -function AttachmentCarousel({report, reportActions, parentReportActions, source, onNavigate, setDownloadButtonVisibility, translate, transaction}) { +function AttachmentCarousel({report, reportActions, parentReportActions, source, onNavigate, setDownloadButtonVisibility, translate}) { const styles = useThemeStyles(); const scrollRef = useRef(null); @@ -39,21 +38,12 @@ function AttachmentCarousel({report, reportActions, parentReportActions, source, const [attachments, setAttachments] = useState([]); const [activeSource, setActiveSource] = useState(source); const [shouldShowArrows, setShouldShowArrows, autoHideArrows, cancelAutoHideArrows] = useCarouselArrows(); - const [isReceipt, setIsReceipt] = useState(false); - const compareImage = useCallback( - (attachment) => { - if (attachment.isReceipt && isReceipt) { - return attachment.transactionID === transaction.transactionID; - } - return attachment.source === source; - }, - [source, isReceipt, transaction], - ); + const compareImage = useCallback((attachment) => attachment.source === source, [source]); useEffect(() => { const parentReportAction = parentReportActions[report.parentReportActionID]; - const attachmentsFromReport = extractAttachmentsFromReport(parentReportAction, reportActions, transaction); + const attachmentsFromReport = extractAttachmentsFromReport(parentReportAction, reportActions); const initialPage = _.findIndex(attachmentsFromReport, compareImage); @@ -88,12 +78,10 @@ function AttachmentCarousel({report, reportActions, parentReportActions, source, // to get the index of the current page const entry = _.first(viewableItems); if (!entry) { - setIsReceipt(false); setActiveSource(null); return; } - setIsReceipt(entry.item.isReceipt); setPage(entry.index); setActiveSource(entry.item.source); @@ -241,15 +229,6 @@ export default compose( canEvict: false, }, }), - // eslint-disable-next-line rulesdir/no-multiple-onyx-in-file - withOnyx({ - transaction: { - key: ({report, parentReportActions}) => { - const parentReportAction = lodashGet(parentReportActions, [report.parentReportActionID]); - return `${ONYXKEYS.COLLECTION.TRANSACTION}${lodashGet(parentReportAction, 'originalMessage.IOUTransactionID', 0)}`; - }, - }, - }), withLocalize, withWindowDimensions, )(AttachmentCarousel); diff --git a/src/components/Attachments/AttachmentCarousel/index.native.js b/src/components/Attachments/AttachmentCarousel/index.native.js index 6bf4e63c01e7..d3f70531cf28 100644 --- a/src/components/Attachments/AttachmentCarousel/index.native.js +++ b/src/components/Attachments/AttachmentCarousel/index.native.js @@ -1,4 +1,3 @@ -import lodashGet from 'lodash/get'; import React, {useCallback, useEffect, useRef, useState} from 'react'; import {Keyboard, PixelRatio, View} from 'react-native'; import {withOnyx} from 'react-native-onyx'; @@ -18,7 +17,7 @@ import extractAttachmentsFromReport from './extractAttachmentsFromReport'; import AttachmentCarouselPager from './Pager'; import useCarouselArrows from './useCarouselArrows'; -function AttachmentCarousel({report, reportActions, parentReportActions, source, onNavigate, setDownloadButtonVisibility, translate, transaction, onClose}) { +function AttachmentCarousel({report, reportActions, parentReportActions, source, onNavigate, setDownloadButtonVisibility, translate, onClose}) { const styles = useThemeStyles(); const pagerRef = useRef(null); @@ -28,21 +27,12 @@ function AttachmentCarousel({report, reportActions, parentReportActions, source, const [activeSource, setActiveSource] = useState(source); const [isPinchGestureRunning, setIsPinchGestureRunning] = useState(true); const [shouldShowArrows, setShouldShowArrows, autoHideArrows, cancelAutoHideArrows] = useCarouselArrows(); - const [isReceipt, setIsReceipt] = useState(false); - const compareImage = useCallback( - (attachment) => { - if (attachment.isReceipt && isReceipt) { - return attachment.transactionID === transaction.transactionID; - } - return attachment.source === source; - }, - [source, isReceipt, transaction], - ); + const compareImage = useCallback((attachment) => attachment.source === source, [source]); useEffect(() => { const parentReportAction = parentReportActions[report.parentReportActionID]; - const attachmentsFromReport = extractAttachmentsFromReport(parentReportAction, reportActions, transaction); + const attachmentsFromReport = extractAttachmentsFromReport(parentReportAction, reportActions); const initialPage = _.findIndex(attachmentsFromReport, compareImage); @@ -77,7 +67,6 @@ function AttachmentCarousel({report, reportActions, parentReportActions, source, const item = attachments[newPageIndex]; setPage(newPageIndex); - setIsReceipt(item.isReceipt); setActiveSource(item.source); onNavigate(item); @@ -186,14 +175,5 @@ export default compose( canEvict: false, }, }), - // eslint-disable-next-line rulesdir/no-multiple-onyx-in-file - withOnyx({ - transaction: { - key: ({report, parentReportActions}) => { - const parentReportAction = lodashGet(parentReportActions, [report.parentReportActionID]); - return `${ONYXKEYS.COLLECTION.TRANSACTION}${lodashGet(parentReportAction, 'originalMessage.IOUTransactionID', 0)}`; - }, - }, - }), withLocalize, )(AttachmentCarousel); diff --git a/src/components/ReportActionItem/ReportActionItemImage.js b/src/components/ReportActionItem/ReportActionItemImage.js index f0eed3ac2f02..6fd322a24d3c 100644 --- a/src/components/ReportActionItem/ReportActionItemImage.js +++ b/src/components/ReportActionItem/ReportActionItemImage.js @@ -2,6 +2,7 @@ import PropTypes from 'prop-types'; import React from 'react'; import {View} from 'react-native'; import _ from 'underscore'; +import AttachmentModal from '@components/AttachmentModal'; import EReceiptThumbnail from '@components/EReceiptThumbnail'; import Image from '@components/Image'; import PressableWithoutFocus from '@components/Pressable/PressableWithoutFocus'; @@ -9,12 +10,10 @@ import {ShowContextMenuContext} from '@components/ShowContextMenuContext'; import ThumbnailImage from '@components/ThumbnailImage'; import transactionPropTypes from '@components/transactionPropTypes'; import useLocalize from '@hooks/useLocalize'; -import Navigation from '@libs/Navigation/Navigation'; import * as TransactionUtils from '@libs/TransactionUtils'; import tryResolveUrlFromApiRoot from '@libs/tryResolveUrlFromApiRoot'; import useThemeStyles from '@styles/useThemeStyles'; import CONST from '@src/CONST'; -import ROUTES from '@src/ROUTES'; const propTypes = { /** thumbnail URI for the image */ @@ -83,17 +82,25 @@ function ReportActionItemImage({thumbnail, image, enablePreviewModal, transactio return ( {({report}) => ( - { - const route = ROUTES.REPORT_ATTACHMENTS.getRoute(report.reportID, imageSource); - Navigation.navigate(route); - }} - role={CONST.ACCESSIBILITY_ROLE.IMAGEBUTTON} - accessibilityLabel={translate('accessibilityHints.viewAttachment')} + - {receiptImageComponent} - + {({show}) => ( + + {receiptImageComponent} + + )} + )} ); From 0e7b5cfa375f2be3f95329909d9c6d396e53c62d Mon Sep 17 00:00:00 2001 From: dukenv0307 Date: Thu, 7 Dec 2023 18:01:43 +0700 Subject: [PATCH 2/5] remove call back --- src/components/AttachmentModal.js | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/components/AttachmentModal.js b/src/components/AttachmentModal.js index 5bfc607c0480..6e47657eb593 100755 --- a/src/components/AttachmentModal.js +++ b/src/components/AttachmentModal.js @@ -116,7 +116,6 @@ const defaultProps = { function AttachmentModal(props) { const theme = useTheme(); const styles = useThemeStyles(); - const onModalHideCallbackRef = useRef(null); const [isModalOpen, setIsModalOpen] = useState(props.defaultOpen); const [shouldLoadAttachment, setShouldLoadAttachment] = useState(false); const [isAttachmentInvalid, setIsAttachmentInvalid] = useState(false); @@ -373,7 +372,7 @@ function AttachmentModal(props) { icon: Expensicons.Camera, text: props.translate('common.replace'), onSelected: () => { - onModalHideCallbackRef.current = () => Navigation.navigate(ROUTES.EDIT_REQUEST.getRoute(props.report.reportID, CONST.EDIT_REQUEST_FIELD.RECEIPT)); + Navigation.navigate(ROUTES.EDIT_REQUEST.getRoute(props.report.reportID, CONST.EDIT_REQUEST_FIELD.RECEIPT)); closeModal(); }, }); @@ -421,10 +420,6 @@ function AttachmentModal(props) { }} onModalHide={(e) => { props.onModalHide(e); - if (onModalHideCallbackRef.current) { - onModalHideCallbackRef.current(); - } - setShouldLoadAttachment(false); }} propagateSwipe From 8ec3cb58847c6d6bc01a03c9598caab34ec9423b Mon Sep 17 00:00:00 2001 From: dukenv0307 Date: Thu, 7 Dec 2023 18:37:39 +0700 Subject: [PATCH 3/5] fix lint --- src/components/AttachmentModal.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/AttachmentModal.js b/src/components/AttachmentModal.js index 6e47657eb593..f8396a1b47ff 100755 --- a/src/components/AttachmentModal.js +++ b/src/components/AttachmentModal.js @@ -2,7 +2,7 @@ import Str from 'expensify-common/lib/str'; import lodashExtend from 'lodash/extend'; import lodashGet from 'lodash/get'; import PropTypes from 'prop-types'; -import React, {useCallback, useEffect, useMemo, useRef, useState} from 'react'; +import React, {useCallback, useEffect, useMemo, useState} from 'react'; import {Animated, Keyboard, View} from 'react-native'; import {withOnyx} from 'react-native-onyx'; import _ from 'underscore'; From bed83190c2015b02e2d76350cf1fd053351a8ce8 Mon Sep 17 00:00:00 2001 From: dukenv0307 Date: Mon, 11 Dec 2023 20:33:19 +0700 Subject: [PATCH 4/5] remove comment --- src/components/Attachments/AttachmentCarousel/index.js | 1 - src/components/Attachments/AttachmentCarousel/index.native.js | 1 - 2 files changed, 2 deletions(-) diff --git a/src/components/Attachments/AttachmentCarousel/index.js b/src/components/Attachments/AttachmentCarousel/index.js index 1696f4adf0b4..fb31e32de91c 100644 --- a/src/components/Attachments/AttachmentCarousel/index.js +++ b/src/components/Attachments/AttachmentCarousel/index.js @@ -215,7 +215,6 @@ AttachmentCarousel.defaultProps = defaultProps; AttachmentCarousel.displayName = 'AttachmentCarousel'; export default compose( - // eslint-disable-next-line rulesdir/no-multiple-onyx-in-file withOnyx({ reportActions: { key: ({report}) => `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report.reportID}`, diff --git a/src/components/Attachments/AttachmentCarousel/index.native.js b/src/components/Attachments/AttachmentCarousel/index.native.js index d3f70531cf28..ea45509d6ce3 100644 --- a/src/components/Attachments/AttachmentCarousel/index.native.js +++ b/src/components/Attachments/AttachmentCarousel/index.native.js @@ -161,7 +161,6 @@ AttachmentCarousel.defaultProps = defaultProps; AttachmentCarousel.displayName = 'AttachmentCarousel'; export default compose( - // eslint-disable-next-line rulesdir/no-multiple-onyx-in-file withOnyx({ reportActions: { key: ({report}) => `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report.reportID}`, From e963f05b37b681454c547e5a4c538e5e9b76e708 Mon Sep 17 00:00:00 2001 From: dukenv0307 Date: Mon, 11 Dec 2023 20:39:29 +0700 Subject: [PATCH 5/5] pass transactionID to AttachmentView --- src/components/AttachmentModal.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/AttachmentModal.js b/src/components/AttachmentModal.js index e0646f39a148..79be536945ac 100755 --- a/src/components/AttachmentModal.js +++ b/src/components/AttachmentModal.js @@ -462,7 +462,7 @@ function AttachmentModal(props) { isWorkspaceAvatar={props.isWorkspaceAvatar} fallbackSource={props.fallbackSource} isUsedInAttachmentModal - transaction={props.transaction} + transactionID={props.transaction.transactionID} /> ) )}