From 7b00c12f563da5c5e7bc350589e8b3506f5191a3 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Mon, 5 Jun 2023 14:57:04 +0300 Subject: [PATCH 01/10] Add `REPORT_ATTACHMENT` route --- src/ROUTES.js | 2 + .../HTMLRenderers/ImageRenderer.js | 41 ++++++++----------- .../Navigation/AppNavigator/AuthScreens.js | 10 +++++ src/libs/Navigation/linkingConfig.js | 2 + 4 files changed, 31 insertions(+), 24 deletions(-) diff --git a/src/ROUTES.js b/src/ROUTES.js index bd0ad304c2f5..ad8608593614 100644 --- a/src/ROUTES.js +++ b/src/ROUTES.js @@ -68,6 +68,8 @@ export default { getReportRoute: (reportID) => `r/${reportID}`, REPORT_WITH_ID_DETAILS_SHARE_CODE: 'r/:reportID/details/shareCode', getReportShareCodeRoute: (reportID) => `r/${reportID}/details/shareCode`, + REPORT_ATTACHMENT: 'r/:reportID/attachment', + getReportAttachmentRoute: (reportID, source) => `r/${reportID}/attachment?source=${encodeURI(source)}`, SELECT_YEAR: 'select-year', getYearSelectionRoute: (minYear, maxYear, currYear, backTo) => `select-year?min=${minYear}&max=${maxYear}&year=${currYear}&backTo=${backTo}`, diff --git a/src/components/HTMLEngineProvider/HTMLRenderers/ImageRenderer.js b/src/components/HTMLEngineProvider/HTMLRenderers/ImageRenderer.js index 4342b86ec6f2..15ca2bee58f8 100644 --- a/src/components/HTMLEngineProvider/HTMLRenderers/ImageRenderer.js +++ b/src/components/HTMLEngineProvider/HTMLRenderers/ImageRenderer.js @@ -1,6 +1,6 @@ import React from 'react'; +import Navigation from '../../../libs/Navigation/Navigation'; import htmlRendererPropTypes from './htmlRendererPropTypes'; -import AttachmentModal from '../../AttachmentModal'; import styles from '../../../styles/styles'; import ThumbnailImage from '../../ThumbnailImage'; import PressableWithoutFocus from '../../PressableWithoutFocus'; @@ -8,6 +8,7 @@ import CONST from '../../../CONST'; import {ShowContextMenuContext, showContextMenuForReport} from '../../ShowContextMenuContext'; import tryResolveUrlFromApiRoot from '../../../libs/tryResolveUrlFromApiRoot'; import * as ReportUtils from '../../../libs/ReportUtils'; +import ROUTES from '../../../ROUTES'; const ImageRenderer = (props) => { const htmlAttribs = props.tnode.attributes; @@ -30,7 +31,6 @@ const ImageRenderer = (props) => { // control and thus require no authToken to verify access. // const isAttachment = Boolean(htmlAttribs[CONST.ATTACHMENT_SOURCE_ATTRIBUTE]); - const originalFileName = htmlAttribs['data-name']; // Files created/uploaded/hosted by App should resolve from API ROOT. Other URLs aren't modified const previewSource = tryResolveUrlFromApiRoot(htmlAttribs.src); @@ -51,29 +51,22 @@ const ImageRenderer = (props) => { ) : ( {({anchor, report, action, checkIfContextMenuActive}) => ( - { + const route = ROUTES.getReportAttachmentRoute(report.reportID, source); + Navigation.navigate(route); + }} + onLongPress={(event) => showContextMenuForReport(event, anchor, report.reportID, action, checkIfContextMenuActive, ReportUtils.isArchivedRoom(report))} > - {({show}) => ( - showContextMenuForReport(event, anchor, report.reportID, action, checkIfContextMenuActive, ReportUtils.isArchivedRoom(report))} - > - - - )} - + + )} ); diff --git a/src/libs/Navigation/AppNavigator/AuthScreens.js b/src/libs/Navigation/AppNavigator/AuthScreens.js index 8539f384504c..385290943299 100644 --- a/src/libs/Navigation/AppNavigator/AuthScreens.js +++ b/src/libs/Navigation/AppNavigator/AuthScreens.js @@ -261,6 +261,16 @@ class AuthScreens extends React.Component { component={RightModalNavigator} listeners={modalScreenListeners} /> + + { + const ReportAttachments = require('../../../pages/home/report/ReportAttachments').default; + return ReportAttachments; + }} + listeners={modalScreenListeners} + /> ); } diff --git a/src/libs/Navigation/linkingConfig.js b/src/libs/Navigation/linkingConfig.js index 666506ca1ab8..a8c1e8abdeff 100644 --- a/src/libs/Navigation/linkingConfig.js +++ b/src/libs/Navigation/linkingConfig.js @@ -23,6 +23,7 @@ export default { [NAVIGATORS.CENTRAL_PANE_NAVIGATOR]: { screens: { [SCREENS.REPORT]: ROUTES.REPORT_WITH_ID, + Report_Attachments: ROUTES.REPORT_ATTACHMENT, }, }, [NAVIGATORS.FULL_SCREEN_NAVIGATOR]: { @@ -340,6 +341,7 @@ export default { }, }, }, + [SCREENS.NOT_FOUND]: '*', }, }, }; From 1aec799a831efdd4ffca27d2900d52f6c90f8095 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Mon, 5 Jun 2023 15:44:32 +0300 Subject: [PATCH 02/10] Update parent modal attachment state The Carousel already updates the parent component this way, when attachments change Updating it here allows the parent component to not need `isAuthenticated` and `originalFileName` for the initial image source provided, since they are obtained here as well --- src/components/AttachmentCarousel/index.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/components/AttachmentCarousel/index.js b/src/components/AttachmentCarousel/index.js index 778044586172..ed80bce87697 100644 --- a/src/components/AttachmentCarousel/index.js +++ b/src/components/AttachmentCarousel/index.js @@ -177,6 +177,9 @@ class AttachmentCarousel extends React.Component { throw new Error('Attachment not found'); } + // Update the parent modal's state with the source and name from the mapped attachments + this.props.onNavigate(attachments[page]); + return { page, attachments, From eb3e0924db1b8bb7d5a0a1f9959fee548d54b6e8 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Mon, 5 Jun 2023 16:12:39 +0300 Subject: [PATCH 03/10] AttachmentModal: add `defaultOpen` prop --- src/components/AttachmentModal.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/components/AttachmentModal.js b/src/components/AttachmentModal.js index b29c910e83e4..e56fa19590e6 100755 --- a/src/components/AttachmentModal.js +++ b/src/components/AttachmentModal.js @@ -36,6 +36,9 @@ const propTypes = { /** Optional callback to fire when we want to preview an image and approve it for use. */ onConfirm: PropTypes.func, + /** Whether the modal should be open by default */ + defaultOpen: PropTypes.bool, + /** Optional callback to fire when we want to do something after modal show. */ onModalShow: PropTypes.func, @@ -68,6 +71,7 @@ const propTypes = { const defaultProps = { source: '', onConfirm: null, + defaultOpen: false, originalFileName: '', isAuthTokenRequired: false, allowDownload: false, @@ -82,7 +86,7 @@ class AttachmentModal extends PureComponent { super(props); this.state = { - isModalOpen: false, + isModalOpen: this.props.defaultOpen, shouldLoadAttachment: false, isAttachmentInvalid: false, isAuthTokenRequired: props.isAuthTokenRequired, From 2c758a4b7511f095760f6b49ffc387b6a8187b2d Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Mon, 5 Jun 2023 16:16:34 +0300 Subject: [PATCH 04/10] AttachmentModal: make passing `children` optional --- src/components/AttachmentModal.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/components/AttachmentModal.js b/src/components/AttachmentModal.js index e56fa19590e6..71e479b71dc5 100755 --- a/src/components/AttachmentModal.js +++ b/src/components/AttachmentModal.js @@ -49,7 +49,7 @@ const propTypes = { originalFileName: PropTypes.string, /** A function as a child to pass modal launching methods to */ - children: PropTypes.func.isRequired, + children: PropTypes.func, /** Whether source url requires authentication */ isAuthTokenRequired: PropTypes.bool, @@ -73,6 +73,7 @@ const defaultProps = { onConfirm: null, defaultOpen: false, originalFileName: '', + children: null, isAuthTokenRequired: false, allowDownload: false, headerTitle: null, @@ -341,7 +342,7 @@ class AttachmentModal extends PureComponent { shouldShowCancelButton={false} /> - {this.props.children({ + {this.props.children && this.props.children({ displayFileInModal: this.validateAndDisplayFileToUpload, show: () => { this.setState({isModalOpen: true}); From 236df33afb4d3ef60ad8a503f26bcecab133c3b8 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Mon, 5 Jun 2023 16:18:46 +0300 Subject: [PATCH 05/10] Add `src/pages/home/report/ReportAttachments.js` modal screen --- src/pages/home/report/ReportAttachments.js | 37 ++++++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 src/pages/home/report/ReportAttachments.js diff --git a/src/pages/home/report/ReportAttachments.js b/src/pages/home/report/ReportAttachments.js new file mode 100644 index 000000000000..217028841c83 --- /dev/null +++ b/src/pages/home/report/ReportAttachments.js @@ -0,0 +1,37 @@ +import React from 'react'; +import _ from 'underscore'; +import PropTypes from 'prop-types'; +import AttachmentModal from '../../../components/AttachmentModal'; +import Navigation from '../../../libs/Navigation/Navigation'; + +const propTypes = { + /** Navigation route context info provided by react navigation */ + route: PropTypes.shape({ + /** Route specific parameters used on this screen */ + params: PropTypes.shape({ + /** The report ID which the attachment is associated with */ + reportID: PropTypes.string.isRequired, + /** The uri encoded source of the attachment */ + source: PropTypes.string.isRequired, + }).isRequired, + }).isRequired, +}; + +function ReportAttachments(props) { + const reportID = _.get(props, ['route', 'params', 'reportID']); + const source = decodeURI(_.get(props, ['route', 'params', 'source'])); + + return ( + + ); +} + +ReportAttachments.propTypes = propTypes; +ReportAttachments.displayName = 'ReportAttachments'; + +export default ReportAttachments; From 2b164e98521d41c83e38e40d4f124d96e63530e1 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Tue, 6 Jun 2023 22:05:47 +0300 Subject: [PATCH 06/10] Apply prettier on the changes --- src/components/AttachmentModal.js | 13 +++++++------ src/pages/home/report/ReportAttachments.js | 3 ++- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/components/AttachmentModal.js b/src/components/AttachmentModal.js index 71e479b71dc5..a6022f4a8fce 100755 --- a/src/components/AttachmentModal.js +++ b/src/components/AttachmentModal.js @@ -342,12 +342,13 @@ class AttachmentModal extends PureComponent { shouldShowCancelButton={false} /> - {this.props.children && this.props.children({ - displayFileInModal: this.validateAndDisplayFileToUpload, - show: () => { - this.setState({isModalOpen: true}); - }, - })} + {this.props.children && + this.props.children({ + displayFileInModal: this.validateAndDisplayFileToUpload, + show: () => { + this.setState({isModalOpen: true}); + }, + })} ); } diff --git a/src/pages/home/report/ReportAttachments.js b/src/pages/home/report/ReportAttachments.js index 217028841c83..b591a8740a4e 100644 --- a/src/pages/home/report/ReportAttachments.js +++ b/src/pages/home/report/ReportAttachments.js @@ -27,7 +27,8 @@ function ReportAttachments(props) { defaultOpen reportID={reportID} source={source} - onModalHide={Navigation.dismissModal} /> + onModalHide={Navigation.dismissModal} + /> ); } From ca27f0d073b6815b540f8917df942f326ac49f41 Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Wed, 14 Jun 2023 13:55:27 +0300 Subject: [PATCH 07/10] Refactor after rebasing - move `Report_Attachments` to `CentralPaneNavigator` --- src/libs/Navigation/AppNavigator/AuthScreens.js | 10 ---------- .../AppNavigator/Navigators/CentralPaneNavigator.js | 10 ++++++++++ src/libs/Navigation/linkingConfig.js | 1 - 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/src/libs/Navigation/AppNavigator/AuthScreens.js b/src/libs/Navigation/AppNavigator/AuthScreens.js index 385290943299..8539f384504c 100644 --- a/src/libs/Navigation/AppNavigator/AuthScreens.js +++ b/src/libs/Navigation/AppNavigator/AuthScreens.js @@ -261,16 +261,6 @@ class AuthScreens extends React.Component { component={RightModalNavigator} listeners={modalScreenListeners} /> - - { - const ReportAttachments = require('../../../pages/home/report/ReportAttachments').default; - return ReportAttachments; - }} - listeners={modalScreenListeners} - /> ); } diff --git a/src/libs/Navigation/AppNavigator/Navigators/CentralPaneNavigator.js b/src/libs/Navigation/AppNavigator/Navigators/CentralPaneNavigator.js index 471be5c7209c..188f152bc089 100644 --- a/src/libs/Navigation/AppNavigator/Navigators/CentralPaneNavigator.js +++ b/src/libs/Navigation/AppNavigator/Navigators/CentralPaneNavigator.js @@ -28,6 +28,16 @@ function CentralPaneNavigator() { }} component={ReportScreenWrapper} /> + { + const ReportAttachments = require('../../../../pages/home/report/ReportAttachments').default; + return ReportAttachments; + }} + /> ); diff --git a/src/libs/Navigation/linkingConfig.js b/src/libs/Navigation/linkingConfig.js index a8c1e8abdeff..3f63deedde7c 100644 --- a/src/libs/Navigation/linkingConfig.js +++ b/src/libs/Navigation/linkingConfig.js @@ -341,7 +341,6 @@ export default { }, }, }, - [SCREENS.NOT_FOUND]: '*', }, }, }; From d5c6e4ccabcaaa1497c7e1330458d509620af2eb Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Wed, 14 Jun 2023 15:40:48 +0300 Subject: [PATCH 08/10] Refactor after rebasing - move `Report_Attachments` to root navigation Otherwise, when closing the modal there's a blank screen displayed briefly It seems the previous screen does not stay underneath the modal unless it's on the root --- .../Navigation/AppNavigator/AuthScreens.js | 12 ++++++++ .../Navigators/CentralPaneNavigator.js | 10 ------- src/libs/Navigation/Navigation.js | 29 +++++++++++-------- src/libs/Navigation/linkingConfig.js | 2 +- src/pages/home/report/ReportAttachments.js | 2 +- 5 files changed, 31 insertions(+), 24 deletions(-) diff --git a/src/libs/Navigation/AppNavigator/AuthScreens.js b/src/libs/Navigation/AppNavigator/AuthScreens.js index 8539f384504c..735f531bcaef 100644 --- a/src/libs/Navigation/AppNavigator/AuthScreens.js +++ b/src/libs/Navigation/AppNavigator/AuthScreens.js @@ -250,6 +250,18 @@ class AuthScreens extends React.Component { return ConciergePage; }} /> + { + const ReportAttachments = require('../../../pages/home/report/ReportAttachments').default; + return ReportAttachments; + }} + listeners={modalScreenListeners} + /> - { - const ReportAttachments = require('../../../../pages/home/report/ReportAttachments').default; - return ReportAttachments; - }} - /> ); diff --git a/src/libs/Navigation/Navigation.js b/src/libs/Navigation/Navigation.js index d61eb7143de0..cf9b7b979daf 100644 --- a/src/libs/Navigation/Navigation.js +++ b/src/libs/Navigation/Navigation.js @@ -132,19 +132,24 @@ function dismissModal(targetReportID) { } const rootState = navigationRef.getRootState(); const lastRoute = _.last(rootState.routes); - if (lastRoute.name === NAVIGATORS.RIGHT_MODAL_NAVIGATOR || lastRoute.name === NAVIGATORS.FULL_SCREEN_NAVIGATOR) { - // if we are not in the target report, we need to navigate to it after dismissing the modal - if (targetReportID && targetReportID !== getTopmostReportId(rootState)) { - const state = getStateFromPath(ROUTES.getReportRoute(targetReportID)); - - const action = getActionFromState(state, linkingConfig.config); - action.type = 'REPLACE'; - navigationRef.current.dispatch(action); - } else { - navigationRef.current.dispatch(StackActions.pop()); + switch (lastRoute.name) { + case NAVIGATORS.RIGHT_MODAL_NAVIGATOR: + case NAVIGATORS.FULL_SCREEN_NAVIGATOR: + case 'Report_Attachments': + // if we are not in the target report, we need to navigate to it after dismissing the modal + if (targetReportID && targetReportID !== getTopmostReportId(rootState)) { + const state = getStateFromPath(ROUTES.getReportRoute(targetReportID)); + + const action = getActionFromState(state, linkingConfig.config); + action.type = 'REPLACE'; + navigationRef.current.dispatch(action); + } else { + navigationRef.current.dispatch(StackActions.pop()); + } + break; + default: { + Log.hmmm('[Navigation] dismissModal failed because there is no modal stack to dismiss'); } - } else { - Log.hmmm('[Navigation] dismissModal failed because there is no modal stack to dismiss'); } } diff --git a/src/libs/Navigation/linkingConfig.js b/src/libs/Navigation/linkingConfig.js index 3f63deedde7c..a4a142658236 100644 --- a/src/libs/Navigation/linkingConfig.js +++ b/src/libs/Navigation/linkingConfig.js @@ -14,6 +14,7 @@ export default { UnlinkLogin: ROUTES.UNLINK_LOGIN, [SCREENS.TRANSITION_FROM_OLD_DOT]: ROUTES.TRANSITION_FROM_OLD_DOT, Concierge: ROUTES.CONCIERGE, + Report_Attachments: ROUTES.REPORT_ATTACHMENT, // Sidebar [SCREENS.HOME]: { @@ -23,7 +24,6 @@ export default { [NAVIGATORS.CENTRAL_PANE_NAVIGATOR]: { screens: { [SCREENS.REPORT]: ROUTES.REPORT_WITH_ID, - Report_Attachments: ROUTES.REPORT_ATTACHMENT, }, }, [NAVIGATORS.FULL_SCREEN_NAVIGATOR]: { diff --git a/src/pages/home/report/ReportAttachments.js b/src/pages/home/report/ReportAttachments.js index b591a8740a4e..203ba3d8c93e 100644 --- a/src/pages/home/report/ReportAttachments.js +++ b/src/pages/home/report/ReportAttachments.js @@ -27,7 +27,7 @@ function ReportAttachments(props) { defaultOpen reportID={reportID} source={source} - onModalHide={Navigation.dismissModal} + onModalHide={() => Navigation.dismissModal(reportID)} /> ); } From 3d5bc852356f98109d91b64bcb4b42c0548859de Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Wed, 14 Jun 2023 15:53:47 +0300 Subject: [PATCH 09/10] Refactor: extract "Report_Attachments" string to SCREENS --- src/ROUTES.js | 2 +- src/SCREENS.js | 1 + src/libs/Navigation/AppNavigator/AuthScreens.js | 2 +- src/libs/Navigation/Navigation.js | 3 ++- src/libs/Navigation/linkingConfig.js | 2 +- 5 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/ROUTES.js b/src/ROUTES.js index ad8608593614..4c3f24b08bd4 100644 --- a/src/ROUTES.js +++ b/src/ROUTES.js @@ -68,7 +68,7 @@ export default { getReportRoute: (reportID) => `r/${reportID}`, REPORT_WITH_ID_DETAILS_SHARE_CODE: 'r/:reportID/details/shareCode', getReportShareCodeRoute: (reportID) => `r/${reportID}/details/shareCode`, - REPORT_ATTACHMENT: 'r/:reportID/attachment', + REPORT_ATTACHMENTS: 'r/:reportID/attachment', getReportAttachmentRoute: (reportID, source) => `r/${reportID}/attachment?source=${encodeURI(source)}`, SELECT_YEAR: 'select-year', getYearSelectionRoute: (minYear, maxYear, currYear, backTo) => `select-year?min=${minYear}&max=${maxYear}&year=${currYear}&backTo=${backTo}`, diff --git a/src/SCREENS.js b/src/SCREENS.js index 24ea27fe9689..aec13a490376 100644 --- a/src/SCREENS.js +++ b/src/SCREENS.js @@ -6,6 +6,7 @@ export default { HOME: 'Home', LOADING: 'Loading', REPORT: 'Report', + REPORT_ATTACHMENTS: 'ReportAttachments', NOT_FOUND: 'not-found', TRANSITION_FROM_OLD_DOT: 'TransitionFromOldDot', }; diff --git a/src/libs/Navigation/AppNavigator/AuthScreens.js b/src/libs/Navigation/AppNavigator/AuthScreens.js index 735f531bcaef..edac1d1638bf 100644 --- a/src/libs/Navigation/AppNavigator/AuthScreens.js +++ b/src/libs/Navigation/AppNavigator/AuthScreens.js @@ -251,7 +251,7 @@ class AuthScreens extends React.Component { }} /> { @@ -135,7 +136,7 @@ function dismissModal(targetReportID) { switch (lastRoute.name) { case NAVIGATORS.RIGHT_MODAL_NAVIGATOR: case NAVIGATORS.FULL_SCREEN_NAVIGATOR: - case 'Report_Attachments': + case SCREENS.REPORT_ATTACHMENTS: // if we are not in the target report, we need to navigate to it after dismissing the modal if (targetReportID && targetReportID !== getTopmostReportId(rootState)) { const state = getStateFromPath(ROUTES.getReportRoute(targetReportID)); diff --git a/src/libs/Navigation/linkingConfig.js b/src/libs/Navigation/linkingConfig.js index a4a142658236..33a35fc25e69 100644 --- a/src/libs/Navigation/linkingConfig.js +++ b/src/libs/Navigation/linkingConfig.js @@ -14,7 +14,7 @@ export default { UnlinkLogin: ROUTES.UNLINK_LOGIN, [SCREENS.TRANSITION_FROM_OLD_DOT]: ROUTES.TRANSITION_FROM_OLD_DOT, Concierge: ROUTES.CONCIERGE, - Report_Attachments: ROUTES.REPORT_ATTACHMENT, + [SCREENS.REPORT_ATTACHMENTS]: ROUTES.REPORT_ATTACHMENTS, // Sidebar [SCREENS.HOME]: { From 305fcc4bdb68212130998eaf4bdfe63be7b8261f Mon Sep 17 00:00:00 2001 From: Peter Velkov Date: Mon, 19 Jun 2023 17:43:45 +0300 Subject: [PATCH 10/10] Refactor: ReportAttachments pass `report` prop to `AttachmentModal` --- src/pages/home/report/ReportAttachments.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/pages/home/report/ReportAttachments.js b/src/pages/home/report/ReportAttachments.js index 203ba3d8c93e..38e7781aedf5 100644 --- a/src/pages/home/report/ReportAttachments.js +++ b/src/pages/home/report/ReportAttachments.js @@ -3,6 +3,7 @@ import _ from 'underscore'; import PropTypes from 'prop-types'; import AttachmentModal from '../../../components/AttachmentModal'; import Navigation from '../../../libs/Navigation/Navigation'; +import * as ReportUtils from '../../../libs/ReportUtils'; const propTypes = { /** Navigation route context info provided by react navigation */ @@ -19,13 +20,14 @@ const propTypes = { function ReportAttachments(props) { const reportID = _.get(props, ['route', 'params', 'reportID']); + const report = ReportUtils.getReport(reportID); const source = decodeURI(_.get(props, ['route', 'params', 'source'])); return ( Navigation.dismissModal(reportID)} />