Skip to content

Commit

Permalink
Merge pull request #40844 from nkdengineer/fix/38835-implement-attach…
Browse files Browse the repository at this point in the history
…ment-screen

fix error when clicking on attachment note
  • Loading branch information
chiragsalian authored May 30, 2024
2 parents 014aace + bbaf3bc commit 459173b
Show file tree
Hide file tree
Showing 20 changed files with 217 additions and 110 deletions.
4 changes: 4 additions & 0 deletions src/CONST.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1186,6 +1186,10 @@ const CONST = {
WEBP: 'image/webp',
JPEG: 'image/jpeg',
},
ATTACHMENT_TYPE: {
REPORT: 'r',
NOTE: 'n',
},

IMAGE_OBJECT_POSITION: {
TOP: 'top',
Expand Down
7 changes: 4 additions & 3 deletions src/ROUTES.ts
Original file line number Diff line number Diff line change
Expand Up @@ -251,9 +251,10 @@ const ROUTES = {
route: 'r/:reportID/details/shareCode',
getRoute: (reportID: string) => `r/${reportID}/details/shareCode` as const,
},
REPORT_ATTACHMENTS: {
route: 'r/:reportID/attachment',
getRoute: (reportID: string, source: string) => `r/${reportID}/attachment?source=${encodeURIComponent(source)}` as const,
ATTACHMENTS: {
route: 'attachment',
getRoute: (reportID: string, type: ValueOf<typeof CONST.ATTACHMENT_TYPE>, url: string, accountID?: number) =>
`attachment?source=${encodeURIComponent(url)}&type=${type}${reportID ? `&reportID=${reportID}` : ''}${accountID ? `&accountID=${accountID}` : ''}` as const,
},
REPORT_PARTICIPANTS: {
route: 'r/:reportID/participants',
Expand Down
2 changes: 1 addition & 1 deletion src/SCREENS.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import type DeepValueOf from './types/utils/DeepValueOf';
const PROTECTED_SCREENS = {
HOME: 'Home',
CONCIERGE: 'Concierge',
REPORT_ATTACHMENTS: 'ReportAttachments',
ATTACHMENTS: 'Attachments',
} as const;

const SCREENS = {
Expand Down
22 changes: 22 additions & 0 deletions src/components/AttachmentContext.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import {createContext} from 'react';
import type {ValueOf} from 'type-fest';
import type CONST from '@src/CONST';

type AttachmentContextProps = {
type?: ValueOf<typeof CONST.ATTACHMENT_TYPE>;
reportID?: string;
accountID?: number;
};

const AttachmentContext = createContext<AttachmentContextProps>({
type: undefined,
reportID: undefined,
accountID: undefined,
});

AttachmentContext.displayName = 'AttachmentContext';

export {
// eslint-disable-next-line import/prefer-default-export
AttachmentContext,
};
11 changes: 11 additions & 0 deletions src/components/AttachmentModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {GestureHandlerRootView} from 'react-native-gesture-handler';
import {withOnyx} from 'react-native-onyx';
import type {OnyxEntry} from 'react-native-onyx';
import {useSharedValue} from 'react-native-reanimated';
import type {ValueOf} from 'type-fest';
import useLocalize from '@hooks/useLocalize';
import useNetwork from '@hooks/useNetwork';
import useResponsiveLayout from '@hooks/useResponsiveLayout';
Expand Down Expand Up @@ -101,6 +102,12 @@ type AttachmentModalProps = AttachmentModalOnyxProps & {
/** The report that has this attachment */
report?: OnyxEntry<OnyxTypes.Report> | EmptyObject;

/** The type of the attachment */
type?: ValueOf<typeof CONST.ATTACHMENT_TYPE>;

/** If the attachment originates from a note, the accountID will represent the author of that note. */
accountID?: number;

/** Optional callback to fire when we want to do something after modal show. */
onModalShow?: () => void;

Expand Down Expand Up @@ -156,6 +163,8 @@ function AttachmentModal({
onModalClose = () => {},
isLoading = false,
shouldShowNotFoundPage = false,
type = undefined,
accountID = undefined,
}: AttachmentModalProps) {
const styles = useThemeStyles();
const StyleUtils = useStyleUtils();
Expand Down Expand Up @@ -519,6 +528,8 @@ function AttachmentModal({
)}
{!isEmptyObject(report) && !isReceiptAttachment ? (
<AttachmentCarousel
accountID={accountID}
type={type}
report={report}
onNavigate={onNavigate}
onClose={closeModal}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,17 +1,24 @@
import {Parser as HtmlParser} from 'htmlparser2';
import type {OnyxEntry} from 'react-native-onyx';
import type {ValueOf} from 'type-fest';
import type {Attachment} from '@components/Attachments/types';
import * as FileUtils from '@libs/fileDownload/FileUtils';
import * as ReportActionsUtils from '@libs/ReportActionsUtils';
import {getReport} from '@libs/ReportUtils';
import tryResolveUrlFromApiRoot from '@libs/tryResolveUrlFromApiRoot';
import CONST from '@src/CONST';
import type {ReportAction, ReportActions} from '@src/types/onyx';

/**
* Constructs the initial component state from report actions
*/
function extractAttachmentsFromReport(parentReportAction?: OnyxEntry<ReportAction>, reportActions?: OnyxEntry<ReportActions>) {
const actions = [...(parentReportAction ? [parentReportAction] : []), ...ReportActionsUtils.getSortedReportActions(Object.values(reportActions ?? {}))];
function extractAttachments(
type: ValueOf<typeof CONST.ATTACHMENT_TYPE>,
{reportID, accountID, parentReportAction, reportActions}: {reportID?: string; accountID?: number; parentReportAction?: OnyxEntry<ReportAction>; reportActions?: OnyxEntry<ReportActions>},
) {
const report = getReport(reportID);
const privateNotes = report?.privateNotes;
const targetNote = privateNotes?.[Number(accountID)]?.note ?? '';
const attachments: Attachment[] = [];

// We handle duplicate image sources by considering the first instance as original. Selecting any duplicate
Expand Down Expand Up @@ -71,6 +78,14 @@ function extractAttachmentsFromReport(parentReportAction?: OnyxEntry<ReportActio
},
});

if (type === CONST.ATTACHMENT_TYPE.NOTE) {
htmlParser.write(targetNote);
htmlParser.end();

return attachments.reverse();
}

const actions = [...(parentReportAction ? [parentReportAction] : []), ...ReportActionsUtils.getSortedReportActions(Object.values(reportActions ?? {}))];
actions.forEach((action, key) => {
if (!ReportActionsUtils.shouldReportActionBeVisible(action, key) || ReportActionsUtils.isMoneyRequestAction(action)) {
return;
Expand All @@ -86,4 +101,4 @@ function extractAttachmentsFromReport(parentReportAction?: OnyxEntry<ReportActio
return attachments.reverse();
}

export default extractAttachmentsFromReport;
export default extractAttachments;
20 changes: 13 additions & 7 deletions src/components/Attachments/AttachmentCarousel/index.native.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,16 @@ import useLocalize from '@hooks/useLocalize';
import useThemeStyles from '@hooks/useThemeStyles';
import Navigation from '@libs/Navigation/Navigation';
import variables from '@styles/variables';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import CarouselButtons from './CarouselButtons';
import extractAttachmentsFromReport from './extractAttachmentsFromReport';
import extractAttachments from './extractAttachments';
import type {AttachmentCarouselPagerHandle} from './Pager';
import AttachmentCarouselPager from './Pager';
import type {AttachmentCaraouselOnyxProps, AttachmentCarouselProps} from './types';
import useCarouselArrows from './useCarouselArrows';

function AttachmentCarousel({report, reportActions, parentReportActions, source, onNavigate, setDownloadButtonVisibility, onClose}: AttachmentCarouselProps) {
function AttachmentCarousel({report, reportActions, parentReportActions, source, onNavigate, setDownloadButtonVisibility, onClose, type, accountID}: AttachmentCarouselProps) {
const styles = useThemeStyles();
const {translate} = useLocalize();
const pagerRef = useRef<AttachmentCarouselPagerHandle>(null);
Expand All @@ -30,25 +31,30 @@ function AttachmentCarousel({report, reportActions, parentReportActions, source,

useEffect(() => {
const parentReportAction = report.parentReportActionID && parentReportActions ? parentReportActions[report.parentReportActionID] : undefined;
const attachmentsFromReport = extractAttachmentsFromReport(parentReportAction, reportActions);
let targetAttachments: Attachment[] = [];
if (type === CONST.ATTACHMENT_TYPE.NOTE && accountID) {
targetAttachments = extractAttachments(CONST.ATTACHMENT_TYPE.NOTE, {reportID: report.reportID, accountID});
} else {
targetAttachments = extractAttachments(CONST.ATTACHMENT_TYPE.REPORT, {parentReportAction, reportActions});
}

const initialPage = attachmentsFromReport.findIndex(compareImage);
const initialPage = targetAttachments.findIndex(compareImage);

// Dismiss the modal when deleting an attachment during its display in preview.
if (initialPage === -1 && attachments.find(compareImage)) {
Navigation.dismissModal();
} else {
setPage(initialPage);
setAttachments(attachmentsFromReport);
setAttachments(targetAttachments);

// Update the download button visibility in the parent modal
if (setDownloadButtonVisibility) {
setDownloadButtonVisibility(initialPage !== -1);
}

// Update the parent modal's state with the source and name from the mapped attachments
if (attachmentsFromReport[initialPage] !== undefined && onNavigate) {
onNavigate(attachmentsFromReport[initialPage]);
if (targetAttachments[initialPage] !== undefined && onNavigate) {
onNavigate(targetAttachments[initialPage]);
}
}
// eslint-disable-next-line react-hooks/exhaustive-deps
Expand Down
23 changes: 14 additions & 9 deletions src/components/Attachments/AttachmentCarousel/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import ONYXKEYS from '@src/ONYXKEYS';
import CarouselActions from './CarouselActions';
import CarouselButtons from './CarouselButtons';
import CarouselItem from './CarouselItem';
import extractAttachmentsFromReport from './extractAttachmentsFromReport';
import extractAttachments from './extractAttachments';
import type {AttachmentCaraouselOnyxProps, AttachmentCarouselProps, UpdatePageProps} from './types';
import useCarouselArrows from './useCarouselArrows';

Expand All @@ -33,7 +33,7 @@ const viewabilityConfig = {

const MIN_FLING_VELOCITY = 500;

function AttachmentCarousel({report, reportActions, parentReportActions, source, onNavigate, setDownloadButtonVisibility}: AttachmentCarouselProps) {
function AttachmentCarousel({report, reportActions, parentReportActions, source, onNavigate, setDownloadButtonVisibility, type, accountID}: AttachmentCarouselProps) {
const theme = useTheme();
const {translate} = useLocalize();
const {isSmallScreenWidth, windowWidth} = useWindowDimensions();
Expand All @@ -57,36 +57,41 @@ function AttachmentCarousel({report, reportActions, parentReportActions, source,

useEffect(() => {
const parentReportAction = report.parentReportActionID && parentReportActions ? parentReportActions[report.parentReportActionID] : undefined;
const attachmentsFromReport = extractAttachmentsFromReport(parentReportAction, reportActions ?? undefined);
let targetAttachments: Attachment[] = [];
if (type === CONST.ATTACHMENT_TYPE.NOTE && accountID) {
targetAttachments = extractAttachments(CONST.ATTACHMENT_TYPE.NOTE, {reportID: report.reportID, accountID});
} else {
targetAttachments = extractAttachments(CONST.ATTACHMENT_TYPE.REPORT, {parentReportAction, reportActions: reportActions ?? undefined});
}

if (isEqual(attachments, attachmentsFromReport)) {
if (isEqual(attachments, targetAttachments)) {
if (attachments.length === 0) {
setPage(-1);
setDownloadButtonVisibility?.(false);
}
return;
}

const initialPage = attachmentsFromReport.findIndex(compareImage);
const initialPage = targetAttachments.findIndex(compareImage);

// Dismiss the modal when deleting an attachment during its display in preview.
if (initialPage === -1 && attachments.find(compareImage)) {
Navigation.dismissModal();
} else {
setPage(initialPage);
setAttachments(attachmentsFromReport);
setAttachments(targetAttachments);

// Update the download button visibility in the parent modal
if (setDownloadButtonVisibility) {
setDownloadButtonVisibility(initialPage !== -1);
}

// Update the parent modal's state with the source and name from the mapped attachments
if (attachmentsFromReport[initialPage] !== undefined && onNavigate) {
onNavigate(attachmentsFromReport[initialPage]);
if (targetAttachments[initialPage] !== undefined && onNavigate) {
onNavigate(targetAttachments[initialPage]);
}
}
}, [reportActions, parentReportActions, compareImage, report.parentReportActionID, attachments, setDownloadButtonVisibility, onNavigate]);
}, [reportActions, parentReportActions, compareImage, report.parentReportActionID, attachments, setDownloadButtonVisibility, onNavigate, accountID, report.reportID, type]);

// Scroll position is affected when window width is resized, so we readjust it on width changes
useEffect(() => {
Expand Down
8 changes: 8 additions & 0 deletions src/components/Attachments/AttachmentCarousel/types.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import type {ViewToken} from 'react-native';
import type {OnyxEntry} from 'react-native-onyx';
import type {ValueOf} from 'type-fest';
import type {Attachment, AttachmentSource} from '@components/Attachments/types';
import type CONST from '@src/CONST';
import type {Report, ReportActions} from '@src/types/onyx';

type UpdatePageProps = {
Expand Down Expand Up @@ -28,6 +30,12 @@ type AttachmentCarouselProps = AttachmentCaraouselOnyxProps & {
/** The report currently being looked at */
report: Report;

/** The type of the attachment */
type?: ValueOf<typeof CONST.ATTACHMENT_TYPE>;

/** If the attachment originates from a note, the accountID will represent the author of that note. */
accountID?: number;

/** A callback that is called when swipe-down-to-close gesture happens */
onClose: () => void;
};
Expand Down
37 changes: 24 additions & 13 deletions src/components/HTMLEngineProvider/HTMLRenderers/ImageRenderer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import React, {memo} from 'react';
import {withOnyx} from 'react-native-onyx';
import type {OnyxEntry} from 'react-native-onyx';
import type {CustomRendererProps, TBlock} from 'react-native-render-html';
import {AttachmentContext} from '@components/AttachmentContext';
import * as Expensicons from '@components/Icon/Expensicons';
import PressableWithoutFocus from '@components/Pressable/PressableWithoutFocus';
import {ShowContextMenuContext, showContextMenuForReport} from '@components/ShowContextMenuContext';
Expand Down Expand Up @@ -78,19 +79,29 @@ function ImageRenderer({tnode}: ImageRendererProps) {
) : (
<ShowContextMenuContext.Consumer>
{({anchor, report, action, checkIfContextMenuActive}) => (
<PressableWithoutFocus
style={[styles.noOutline]}
onPress={() => {
const route = ROUTES.REPORT_ATTACHMENTS.getRoute(report?.reportID ?? '', source);
Navigation.navigate(route);
}}
onLongPress={(event) => showContextMenuForReport(event, anchor, report?.reportID ?? '', action, checkIfContextMenuActive, ReportUtils.isArchivedRoom(report))}
shouldUseHapticsOnLongPress
accessibilityRole={CONST.ACCESSIBILITY_ROLE.IMAGEBUTTON}
accessibilityLabel={translate('accessibilityHints.viewAttachment')}
>
{thumbnailImageComponent}
</PressableWithoutFocus>
<AttachmentContext.Consumer>
{({reportID, accountID, type}) => (
<PressableWithoutFocus
style={[styles.noOutline]}
onPress={() => {
if (!source || !type) {
return;
}

if (reportID) {
const route = ROUTES.ATTACHMENTS?.getRoute(reportID, type, source, accountID);
Navigation.navigate(route);
}
}}
onLongPress={(event) => showContextMenuForReport(event, anchor, report?.reportID ?? '', action, checkIfContextMenuActive, ReportUtils.isArchivedRoom(report))}
shouldUseHapticsOnLongPress
accessibilityRole={CONST.ACCESSIBILITY_ROLE.IMAGEBUTTON}
accessibilityLabel={translate('accessibilityHints.viewAttachment')}
>
{thumbnailImageComponent}
</PressableWithoutFocus>
)}
</AttachmentContext.Consumer>
)}
</ShowContextMenuContext.Consumer>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ function VideoRenderer({tnode, key}: VideoRendererProps) {
videoDimensions={{width, height}}
videoDuration={duration}
onShowModalPress={() => {
const route = ROUTES.REPORT_ATTACHMENTS.getRoute(report?.reportID ?? '', sourceURL);
const route = ROUTES.ATTACHMENTS.getRoute(report?.reportID ?? '', CONST.ATTACHMENT_TYPE.REPORT, sourceURL);
Navigation.navigate(route);
}}
/>
Expand Down
2 changes: 1 addition & 1 deletion src/libs/Navigation/AppNavigator/AuthScreens.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ function AuthScreens({session, lastOpenedPublicRoomID, initialLastUpdateIDApplie
getComponent={loadConciergePage}
/>
<RootStack.Screen
name={SCREENS.REPORT_ATTACHMENTS}
name={SCREENS.ATTACHMENTS}
options={{
headerShown: false,
presentation: 'transparentModal',
Expand Down
2 changes: 1 addition & 1 deletion src/libs/Navigation/dismissModal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ function dismissModal(navigationRef: NavigationContainerRef<RootStackParamList>)
case NAVIGATORS.ONBOARDING_MODAL_NAVIGATOR:
case NAVIGATORS.FEATURE_TRANING_MODAL_NAVIGATOR:
case SCREENS.NOT_FOUND:
case SCREENS.REPORT_ATTACHMENTS:
case SCREENS.ATTACHMENTS:
case SCREENS.TRANSACTION_RECEIPT:
case SCREENS.PROFILE_AVATAR:
case SCREENS.WORKSPACE_AVATAR:
Expand Down
2 changes: 1 addition & 1 deletion src/libs/Navigation/dismissModalWithReport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ function dismissModalWithReport(targetReport: Report | EmptyObject, navigationRe
case NAVIGATORS.LEFT_MODAL_NAVIGATOR:
case NAVIGATORS.RIGHT_MODAL_NAVIGATOR:
case SCREENS.NOT_FOUND:
case SCREENS.REPORT_ATTACHMENTS:
case SCREENS.ATTACHMENTS:
case SCREENS.TRANSACTION_RECEIPT:
case SCREENS.PROFILE_AVATAR:
case SCREENS.WORKSPACE_AVATAR:
Expand Down
2 changes: 1 addition & 1 deletion src/libs/Navigation/linkingConfig/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ const config: LinkingOptions<RootStackParamList>['config'] = {
[SCREENS.SIGN_IN_WITH_GOOGLE_DESKTOP]: ROUTES.GOOGLE_SIGN_IN,
[SCREENS.SAML_SIGN_IN]: ROUTES.SAML_SIGN_IN,
[SCREENS.DESKTOP_SIGN_IN_REDIRECT]: ROUTES.DESKTOP_SIGN_IN_REDIRECT,
[SCREENS.REPORT_ATTACHMENTS]: ROUTES.REPORT_ATTACHMENTS.route,
[SCREENS.ATTACHMENTS]: ROUTES.ATTACHMENTS.route,
[SCREENS.PROFILE_AVATAR]: ROUTES.PROFILE_AVATAR.route,
[SCREENS.WORKSPACE_AVATAR]: ROUTES.WORKSPACE_AVATAR.route,
[SCREENS.REPORT_AVATAR]: ROUTES.REPORT_AVATAR.route,
Expand Down
Loading

0 comments on commit 459173b

Please sign in to comment.