Fix - Attachment - When opening the last one, the first added Public Image is opened

src/CONST.ts

@@ -1784,6 +1784,7 @@ const CONST = {
 
     ATTACHMENT_MESSAGE_TEXT: '[Attachment]',
     ATTACHMENT_SOURCE_ATTRIBUTE: 'data-expensify-source',
+    ATTACHMENT_ID_ATTRIBUTE: 'data-attachment-id',
     ATTACHMENT_OPTIMISTIC_SOURCE_ATTRIBUTE: 'data-optimistic-src',
     ATTACHMENT_PREVIEW_ATTRIBUTE: 'src',
     ATTACHMENT_ORIGINAL_FILENAME_ATTRIBUTE: 'data-name',

src/ROUTES.ts

@@ -403,6 +403,7 @@ const ROUTES = {
         route: 'attachment',
         getRoute: (
             reportID: string | undefined,
+            attachmentID: string | undefined,
             type: ValueOf<typeof CONST.ATTACHMENT_TYPE>,
             url: string,
             accountID?: number,
@@ -415,8 +416,11 @@ const ROUTES = {
             const authTokenParam = isAuthTokenRequired ? '&isAuthTokenRequired=true' : '';
             const fileNameParam = fileName ? `&fileName=${fileName}` : '';
             const attachmentLinkParam = attachmentLink ? `&attachmentLink=${attachmentLink}` : '';
+            const attachmentIDParam = attachmentID ? `&attachmentID=${attachmentID}` : '';
 
-            return `attachment?source=${encodeURIComponent(url)}&type=${type as string}${reportParam}${accountParam}${authTokenParam}${fileNameParam}${attachmentLinkParam}` as const;
+            return `attachment?source=${encodeURIComponent(url)}&type=${
+                type as string
+            }${reportParam}${attachmentIDParam}${accountParam}${authTokenParam}${fileNameParam}${attachmentLinkParam}` as const;
         },
     },
     REPORT_PARTICIPANTS: {

src/components/AttachmentModal.tsx

@@ -69,6 +69,9 @@ type AttachmentModalProps = {
     /** Optional source (URL, SVG function) for the image shown. If not passed in via props must be specified when modal is opened. */
     source?: AvatarSource;
 
+    /** The id of the attachment. */
+    attachmentID?: string;
+
     /** Optional callback to fire when we want to preview an image and approve it for use. */
     onConfirm?: ((file: FileObject) => void) | null;
 
@@ -164,6 +167,7 @@ function AttachmentModal({
     isLoading = false,
     shouldShowNotFoundPage = false,
     type = undefined,
+    attachmentID,
     accountID = undefined,
     shouldDisableSendButton = false,
     attachmentLink = '',
@@ -558,6 +562,7 @@ function AttachmentModal({
                                 <AttachmentCarousel
                                     accountID={accountID}
                                     type={type}
+                                    attachmentID={attachmentID}
                                     report={report}
                                     onNavigate={onNavigate}
                                     onClose={closeModal}

src/components/Attachments/AttachmentCarousel/Pager/index.tsx

@@ -32,8 +32,8 @@ type AttachmentCarouselPagerProps = {
     /** The attachments to be rendered in the pager. */
     items: Attachment[];
 
-    /** The source (URL) of the currently active attachment. */
-    activeSource: AttachmentSource;
+    /** The id or source (URL) of the currently active attachment. */
+    activeAttachmentID: AttachmentSource;
 
     /** The index of the initial page to be rendered. */
     initialPage: number;
@@ -58,7 +58,7 @@ type AttachmentCarouselPagerProps = {
 };
 
 function AttachmentCarouselPager(
-    {items, activeSource, initialPage, setShouldShowArrows, onPageSelected, onClose, reportID}: AttachmentCarouselPagerProps,
+    {items, activeAttachmentID, initialPage, setShouldShowArrows, onPageSelected, onClose, reportID}: AttachmentCarouselPagerProps,
     ref: ForwardedRef<AttachmentCarouselPagerHandle>,
 ) {
     const {handleTap, handleScaleChange, isScrollEnabled} = useCarouselContextEvents(setShouldShowArrows);
@@ -88,7 +88,7 @@ function AttachmentCarouselPager(
         [activePageIndex, items],
     );
 
-    const extractItemKey = useCallback((item: Attachment, index: number) => `reportActionID-${item.reportActionID}-${index}`, []);
+    const extractItemKey = useCallback((item: Attachment, index: number) => `attachmentID-${item.attachmentID}-${index}`, []);
 
     const contextValue = useMemo(
         () => ({
@@ -129,7 +129,7 @@ function AttachmentCarouselPager(
         >
             <CarouselItem
                 item={item}
-                isFocused={index === activePageIndex && activeSource === item.source}
+                isFocused={index === activePageIndex && activeAttachmentID === (item.attachmentID ?? item.source)}
                 reportID={reportID}
             />
         </View>

src/components/Attachments/AttachmentCarousel/extractAttachments.ts

@@ -3,7 +3,7 @@ import type {OnyxEntry} from 'react-native-onyx';
 import type {ValueOf} from 'type-fest';
 import type {Attachment} from '@components/Attachments/types';
 import {getFileName, splitExtensionFromFileName} from '@libs/fileDownload/FileUtils';
-import {getReportActionHtml, getReportActionMessage, getSortedReportActions, isMoneyRequestAction, shouldReportActionBeVisible} from '@libs/ReportActionsUtils';
+import {getHtmlWithAttachmentID, getReportActionHtml, getReportActionMessage, getSortedReportActions, isMoneyRequestAction, shouldReportActionBeVisible} from '@libs/ReportActionsUtils';
 import {canUserPerformWriteAction} from '@libs/ReportUtils';
 import tryResolveUrlFromApiRoot from '@libs/tryResolveUrlFromApiRoot';
 import CONST from '@src/CONST';
@@ -27,11 +27,6 @@ function extractAttachments(
     const description = report?.description ?? '';
     const attachments: Attachment[] = [];
     const canUserPerformAction = canUserPerformWriteAction(report);
-
-    // We handle duplicate image sources by considering the first instance as original. Selecting any duplicate
-    // and navigating back (<) shows the image preceding the first instance, not the selected duplicate's position.
-    const uniqueSourcesAndLinks = new Set();
-
     let currentLink = '';
 
     const htmlParser = new HtmlParser({
@@ -41,13 +36,10 @@ function extractAttachments(
             }
             if (name === 'video') {
                 const source = tryResolveUrlFromApiRoot(attribs[CONST.ATTACHMENT_SOURCE_ATTRIBUTE]);
-                if (uniqueSourcesAndLinks.has(source)) {
-                    return;
-                }
 
-                uniqueSourcesAndLinks.add(source);
                 const fileName = attribs[CONST.ATTACHMENT_ORIGINAL_FILENAME_ATTRIBUTE] || getFileName(`${source}`);
                 attachments.unshift({
+                    attachmentID: attribs[CONST.ATTACHMENT_ID_ATTRIBUTE],
                     source: tryResolveUrlFromApiRoot(attribs[CONST.ATTACHMENT_SOURCE_ATTRIBUTE]),
                     isAuthTokenRequired: !!attribs[CONST.ATTACHMENT_SOURCE_ATTRIBUTE],
                     file: {name: fileName},
@@ -62,13 +54,6 @@ function extractAttachments(
                 const expensifySource = attribs[CONST.ATTACHMENT_SOURCE_ATTRIBUTE] ?? (new RegExp(CONST.ATTACHMENT_OR_RECEIPT_LOCAL_URL, 'i').test(attribs.src) ? attribs.src : null);
                 const source = tryResolveUrlFromApiRoot(expensifySource || attribs.src);
                 const previewSource = tryResolveUrlFromApiRoot(attribs.src);
-                const sourceLinkKey = `${source}|${currentLink}`;
-
-                if (uniqueSourcesAndLinks.has(sourceLinkKey)) {
-                    return;
-                }
-
-                uniqueSourcesAndLinks.add(sourceLinkKey);
 
                 let fileName = attribs[CONST.ATTACHMENT_ORIGINAL_FILENAME_ATTRIBUTE] || getFileName(`${source}`);
 
@@ -87,6 +72,7 @@ function extractAttachments(
                 // we ensure correct order of attachments even across actions with multiple attachments.
                 attachments.unshift({
                     reportActionID: attribs['data-id'],
+                    attachmentID: attribs[CONST.ATTACHMENT_ID_ATTRIBUTE],
                     source,
                     previewSource,
                     isAuthTokenRequired: !!expensifySource,
@@ -128,8 +114,8 @@ function extractAttachments(
 
         const decision = getReportActionMessage(action)?.moderationDecision?.decision;
         const hasBeenFlagged = decision === CONST.MODERATION.MODERATOR_DECISION_PENDING_HIDE || decision === CONST.MODERATION.MODERATOR_DECISION_HIDDEN;
-        const html = getReportActionHtml(action).replace('/>', `data-flagged="${hasBeenFlagged}" data-id="${action.reportActionID}"/>`);
-        htmlParser.write(html);
+        const html = getReportActionHtml(action).replaceAll('/>', `data-flagged="${hasBeenFlagged}" data-id="${action.reportActionID}"/>`);
+        htmlParser.write(getHtmlWithAttachmentID(html, action.reportActionID));
     });
     htmlParser.end();
 

src/components/Attachments/AttachmentCarousel/index.native.tsx

@@ -18,7 +18,7 @@ import AttachmentCarouselPager from './Pager';
 import type {AttachmentCarouselProps} from './types';
 import useCarouselArrows from './useCarouselArrows';
 
-function AttachmentCarousel({report, source, onNavigate, setDownloadButtonVisibility, onClose, type, accountID}: AttachmentCarouselProps) {
+function AttachmentCarousel({report, source, attachmentID, onNavigate, setDownloadButtonVisibility, onClose, type, accountID}: AttachmentCarouselProps) {
     const styles = useThemeStyles();
     const {translate} = useLocalize();
     const pagerRef = useRef<AttachmentCarouselPagerHandle>(null);
@@ -27,8 +27,8 @@ function AttachmentCarousel({report, source, onNavigate, setDownloadButtonVisibi
     const [page, setPage] = useState<number>();
     const [attachments, setAttachments] = useState<Attachment[]>([]);
     const {shouldShowArrows, setShouldShowArrows, autoHideArrows, cancelAutoHideArrows} = useCarouselArrows();
-    const [activeSource, setActiveSource] = useState<AttachmentSource>(source);
-    const compareImage = useCallback((attachment: Attachment) => attachment.source === source, [source]);
+    const [activeAttachmentID, setActiveAttachmentID] = useState<AttachmentSource>(attachmentID ?? source);
+    const compareImage = useCallback((attachment: Attachment) => (attachmentID ? attachment.attachmentID === attachmentID : attachment.source === source), [attachmentID, source]);
 
     useEffect(() => {
         const parentReportAction = report.parentReportActionID && parentReportActions ? parentReportActions[report.parentReportActionID] : undefined;
@@ -82,7 +82,7 @@ function AttachmentCarousel({report, source, onNavigate, setDownloadButtonVisibi
 
             setPage(newPageIndex);
             if (newPageIndex >= 0 && item) {
-                setActiveSource(item.source);
+                setActiveAttachmentID(item.attachmentID ?? item.source);
                 if (onNavigate) {
                     onNavigate(item);
                 }
@@ -144,7 +144,7 @@ function AttachmentCarousel({report, source, onNavigate, setDownloadButtonVisibi
                     <AttachmentCarouselPager
                         items={attachments}
                         initialPage={page}
-                        activeSource={activeSource}
+                        activeAttachmentID={activeAttachmentID}
                         setShouldShowArrows={setShouldShowArrows}
                         onPageSelected={({nativeEvent: {position: newPage}}) => updatePage(newPage)}
                         onClose={onClose}

src/components/Attachments/AttachmentCarousel/index.tsx

@@ -9,7 +9,7 @@ import {useOnyx} from 'react-native-onyx';
 import Animated, {scrollTo, useAnimatedRef, useSharedValue} from 'react-native-reanimated';
 import type {Attachment, AttachmentSource} from '@components/Attachments/types';
 import BlockingView from '@components/BlockingViews/BlockingView';
-import * as Illustrations from '@components/Icon/Illustrations';
+import {ToddBehindCloud} from '@components/Icon/Illustrations';
 import {useFullScreenContext} from '@components/VideoPlayerContexts/FullScreenContext';
 import useLocalize from '@hooks/useLocalize';
 import useResponsiveLayout from '@hooks/useResponsiveLayout';
@@ -51,7 +51,7 @@ function DeviceAwareGestureDetector({canUseTouchScreen, gesture, children}: Devi
     return canUseTouchScreen ? <GestureDetector gesture={gesture}>{children}</GestureDetector> : children;
 }
 
-function AttachmentCarousel({report, source, onNavigate, setDownloadButtonVisibility, type, accountID, onClose, attachmentLink}: AttachmentCarouselProps) {
+function AttachmentCarousel({report, attachmentID, source, onNavigate, setDownloadButtonVisibility, type, accountID, onClose, attachmentLink}: AttachmentCarouselProps) {
     const theme = useTheme();
     const {translate} = useLocalize();
     const {windowWidth} = useWindowDimensions();
@@ -72,7 +72,7 @@ function AttachmentCarousel({report, source, onNavigate, setDownloadButtonVisibi
     );
     const [page, setPage] = useState(0);
     const [attachments, setAttachments] = useState<Attachment[]>([]);
-    const [activeSource, setActiveSource] = useState<AttachmentSource | null>(source);
+    const [activeAttachmentID, setActiveAttachmentID] = useState<AttachmentSource | null>(attachmentID ?? source);
     const {shouldShowArrows, setShouldShowArrows, autoHideArrows, cancelAutoHideArrows} = useCarouselArrows();
     const {handleTap, handleScaleChange, isScrollEnabled} = useCarouselContextEvents(setShouldShowArrows);
 
@@ -83,7 +83,11 @@ function AttachmentCarousel({report, source, onNavigate, setDownloadButtonVisibi
         setShouldShowArrows(true);
     }, [canUseTouchScreen, page, setShouldShowArrows]);
 
-    const compareImage = useCallback((attachment: Attachment) => attachment.source === source && (!attachmentLink || attachment.attachmentLink === attachmentLink), [attachmentLink, source]);
+    const compareImage = useCallback(
+        (attachment: Attachment) =>
+            (attachmentID ? attachment.attachmentID === attachmentID : attachment.source === source) && (!attachmentLink || attachment.attachmentLink === attachmentLink),
+        [attachmentLink, attachmentID, source],
+    );
 
     useEffect(() => {
         const parentReportAction = report.parentReportActionID && parentReportActions ? parentReportActions[report.parentReportActionID] : undefined;
@@ -158,14 +162,14 @@ function AttachmentCarousel({report, source, onNavigate, setDownloadButtonVisibi
             // to get the index of the current page
             const entry = viewableItems.at(0);
             if (!entry) {
-                setActiveSource(null);
+                setActiveAttachmentID(null);
                 return;
             }
 
             const item = entry.item as Attachment;
             if (entry.index !== null) {
                 setPage(entry.index);
-                setActiveSource(item.source);
+                setActiveAttachmentID(item.attachmentID ?? item.source);
             }
 
             if (onNavigate) {
@@ -195,7 +199,10 @@ function AttachmentCarousel({report, source, onNavigate, setDownloadButtonVisibi
     );
 
     const extractItemKey = useCallback(
-        (item: Attachment) => (typeof item.source === 'string' || typeof item.source === 'number' ? `source-${item.source}|${item.attachmentLink}` : `reportActionID-${item.reportActionID}`),
+        (item: Attachment) =>
+            !!item.attachmentID || (typeof item.source !== 'string' && typeof item.source !== 'number')
+                ? `attachmentID-${item.attachmentID}`
+                : `source-${item.source}|${item.attachmentLink}`,
         [],
     );
 
@@ -229,14 +236,14 @@ function AttachmentCarousel({report, source, onNavigate, setDownloadButtonVisibi
             <View style={[styles.h100, {width: cellWidth}]}>
                 <CarouselItem
                     item={item}
-                    isFocused={activeSource === item.source}
+                    isFocused={activeAttachmentID === (item.attachmentID ?? item.source)}
                     onPress={canUseTouchScreen ? handleTap : undefined}
                     isModalHovered={shouldShowArrows}
                     reportID={report.reportID}
                 />
             </View>
         ),
-        [activeSource, canUseTouchScreen, cellWidth, handleTap, report.reportID, shouldShowArrows, styles.h100],
+        [activeAttachmentID, canUseTouchScreen, cellWidth, handleTap, report.reportID, shouldShowArrows, styles.h100],
     );
     /** Pan gesture handing swiping through attachments on touch screen devices */
     const pan = useMemo(
@@ -288,7 +295,7 @@ function AttachmentCarousel({report, source, onNavigate, setDownloadButtonVisibi
         >
             {page === -1 ? (
                 <BlockingView
-                    icon={Illustrations.ToddBehindCloud}
+                    icon={ToddBehindCloud}
                     iconColor={theme.offline}
                     iconWidth={variables.modalTopIconWidth}
                     iconHeight={variables.modalTopIconHeight}

src/components/Attachments/AttachmentCarousel/types.ts

@@ -12,6 +12,9 @@ type AttachmentCarouselProps = {
     /** Source is used to determine the starting index in the array of attachments */
     source: AttachmentSource;
 
+    /** The id of the current active attachment */
+    attachmentID?: string;
+
     /** Callback to update the parent modal's state with a source and name from the attachments array */
     onNavigate?: (attachment: Attachment) => void;
 

src/components/Attachments/types.ts

@@ -7,6 +7,9 @@ type Attachment = {
     /** Report action ID of the attachment */
     reportActionID?: string;
 
+    /** The attachment id, which is the concatenation of the report action id it is in and its order index within that report action. */
+    attachmentID?: string;
+
     /** Whether source url requires authentication */
     isAuthTokenRequired?: boolean;
 

src/components/HTMLEngineProvider/HTMLRenderers/ImageRenderer.tsx

@@ -5,7 +5,7 @@ import type {CustomRendererProps, TBlock} from 'react-native-render-html';
 import {AttachmentContext} from '@components/AttachmentContext';
 import {getButtonRole} from '@components/Button/utils';
 import {isDeletedNode} from '@components/HTMLEngineProvider/htmlEngineUtils';
-import * as Expensicons from '@components/Icon/Expensicons';
+import {Document, GalleryNotFound} from '@components/Icon/Expensicons';
 import PressableWithoutFocus from '@components/Pressable/PressableWithoutFocus';
 import {ShowContextMenuContext, showContextMenuForReport} from '@components/ShowContextMenuContext';
 import ThumbnailImage from '@components/ThumbnailImage';
@@ -57,6 +57,7 @@ function ImageRenderer({tnode}: ImageRendererProps) {
     const attachmentSourceAttribute =
         htmlAttribs[CONST.ATTACHMENT_SOURCE_ATTRIBUTE] ?? (new RegExp(CONST.ATTACHMENT_OR_RECEIPT_LOCAL_URL, 'i').test(htmlAttribs.src) ? htmlAttribs.src : null);
     const isAttachmentOrReceipt = !!attachmentSourceAttribute;
+    const attachmentID = htmlAttribs[CONST.ATTACHMENT_ID_ATTRIBUTE];
 
     // Files created/uploaded/hosted by App should resolve from API ROOT. Other URLs aren't modified
     const previewSource = tryResolveUrlFromApiRoot(htmlAttribs.src);
@@ -72,7 +73,7 @@ function ImageRenderer({tnode}: ImageRendererProps) {
     const imagePreviewModalDisabled = htmlAttribs['data-expensify-preview-modal-disabled'] === 'true';
 
     const fileType = getFileType(attachmentSourceAttribute);
-    const fallbackIcon = fileType === CONST.ATTACHMENT_FILE_TYPE.FILE ? Expensicons.Document : Expensicons.GalleryNotFound;
+    const fallbackIcon = fileType === CONST.ATTACHMENT_FILE_TYPE.FILE ? Document : GalleryNotFound;
     const theme = useTheme();
 
     let fileName = htmlAttribs[CONST.ATTACHMENT_ORIGINAL_FILENAME_ATTRIBUTE] || getFileName(`${isAttachmentOrReceipt ? attachmentSourceAttribute : htmlAttribs.src}`);
@@ -111,7 +112,7 @@ function ImageRenderer({tnode}: ImageRendererProps) {
                                 }
 
                                 const attachmentLink = tnode.parent?.attributes?.href;
-                                const route = ROUTES.ATTACHMENTS?.getRoute(reportID, type, source, accountID, isAttachmentOrReceipt, fileName, attachmentLink);
+                                const route = ROUTES.ATTACHMENTS?.getRoute(reportID, attachmentID, type, source, accountID, isAttachmentOrReceipt, fileName, attachmentLink);
                                 Navigation.navigate(route);
                             }}
                             onLongPress={(event) => {