Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix error when clicking on attachment note #40844

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading