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 image from opening in both a new tab and app modal #52013

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
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
6 changes: 4 additions & 2 deletions src/ROUTES.ts
Original file line number Diff line number Diff line change
Expand Up @@ -310,11 +310,13 @@ const ROUTES = {
},
ATTACHMENTS: {
route: 'attachment',
getRoute: (reportID: string, type: ValueOf<typeof CONST.ATTACHMENT_TYPE>, url: string, accountID?: number, isAuthTokenRequired?: boolean) => {
getRoute: (reportID: string, type: ValueOf<typeof CONST.ATTACHMENT_TYPE>, url: string, accountID?: number, isAuthTokenRequired?: boolean, attachmentLink?: string) => {
const reportParam = reportID ? `&reportID=${reportID}` : '';
const accountParam = accountID ? `&accountID=${accountID}` : '';
const authTokenParam = isAuthTokenRequired ? '&isAuthTokenRequired=true' : '';
return `attachment?source=${encodeURIComponent(url)}&type=${type}${reportParam}${accountParam}${authTokenParam}` as const;
const attachmentLinkParam = attachmentLink ? `&attachmentLink=${attachmentLink}` : '';

return `attachment?source=${encodeURIComponent(url)}&type=${type}${reportParam}${accountParam}${authTokenParam}${attachmentLinkParam}` as const;
},
},
REPORT_PARTICIPANTS: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import type {BaseAnchorForCommentsOnlyProps, LinkProps} from './types';
/*
* This is a default anchor component for regular links.
*/
function BaseAnchorForCommentsOnly({onPressIn, onPressOut, href = '', rel = '', target = '', children = null, style, onPress, ...rest}: BaseAnchorForCommentsOnlyProps) {
function BaseAnchorForCommentsOnly({onPressIn, onPressOut, href = '', rel = '', target = '', children = null, style, onPress, isLinkHasImage, ...rest}: BaseAnchorForCommentsOnlyProps) {
const styles = useThemeStyles();
const StyleUtils = useStyleUtils();
const linkRef = useRef<RNText>(null);
Expand Down Expand Up @@ -62,7 +62,7 @@ function BaseAnchorForCommentsOnly({onPressIn, onPressOut, href = '', rel = '',
role={CONST.ROLE.LINK}
accessibilityLabel={href}
>
<Tooltip text={href}>
<Tooltip text={!isLinkHasImage ? href : undefined}>
<Text
ref={linkRef}
style={StyleSheet.flatten([style, defaultTextStyle])}
Expand All @@ -71,7 +71,7 @@ function BaseAnchorForCommentsOnly({onPressIn, onPressOut, href = '', rel = '',
rel,
target: isEmail || !linkProps.href ? '_self' : target,
}}
href={href}
href={!isLinkHasImage ? href : undefined}
suppressHighlighting
// Add testID so it gets selected as an anchor tag by SelectionScraper
testID="a"
Expand Down
3 changes: 3 additions & 0 deletions src/components/AnchorForCommentsOnly/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ type AnchorForCommentsOnlyProps = ChildrenProps & {

/** Press handler for the link, when not passed, default href is used to create a link like behaviour */
onPress?: () => void;

/** Indicates whether an image is wrapped in an anchor (`<a>`) tag with an `href` link */
isLinkHasImage?: boolean;
};

type BaseAnchorForCommentsOnlyProps = AnchorForCommentsOnlyProps & {
Expand Down
22 changes: 22 additions & 0 deletions src/components/AttachmentModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,8 @@ type AttachmentModalProps = {
canEditReceipt?: boolean;

shouldDisableSendButton?: boolean;

attachmentLink?: string;
};

function AttachmentModal({
Expand Down Expand Up @@ -161,6 +163,7 @@ function AttachmentModal({
type = undefined,
accountID = undefined,
shouldDisableSendButton = false,
attachmentLink = '',
}: AttachmentModalProps) {
const styles = useThemeStyles();
const StyleUtils = useStyleUtils();
Expand All @@ -185,6 +188,7 @@ function AttachmentModal({
const parentReportAction = ReportActionsUtils.getReportAction(report?.parentReportID ?? '-1', report?.parentReportActionID ?? '-1');
const transactionID = ReportActionsUtils.isMoneyRequestAction(parentReportAction) ? ReportActionsUtils.getOriginalMessage(parentReportAction)?.IOUTransactionID ?? '-1' : '-1';
const [transaction] = useOnyx(`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`);
const [currentAttachmentLink, setCurrentAttachmentLink] = useState('');

const [file, setFile] = useState<FileObject | undefined>(
originalFileName
Expand All @@ -211,6 +215,7 @@ function AttachmentModal({
setFile(attachment.file);
setIsAuthTokenRequiredState(attachment.isAuthTokenRequired ?? false);
onCarouselAttachmentChange(attachment);
setCurrentAttachmentLink(attachment?.attachmentLink ?? '');
},
[onCarouselAttachmentChange],
);
Expand Down Expand Up @@ -482,6 +487,22 @@ function AttachmentModal({

const submitRef = useRef<View | HTMLElement>(null);

const getSubTitleLink = useMemo(() => {
if (shouldShowNotFoundPage) {
return '';
}

if (!isEmptyObject(report) && !isReceiptAttachment) {
return currentAttachmentLink;
}

if (!isAuthTokenRequired && attachmentLink) {
return attachmentLink;
}

return '';
}, [shouldShowNotFoundPage, report, isReceiptAttachment, currentAttachmentLink, isAuthTokenRequired, attachmentLink]);

return (
<>
<Modal
Expand Down Expand Up @@ -527,6 +548,7 @@ function AttachmentModal({
threeDotsAnchorPosition={styles.threeDotsPopoverOffsetAttachmentModal(windowWidth)}
threeDotsMenuItems={threeDotsMenuItems}
shouldOverlayDots
subTitleLink={getSubTitleLink}
/>
<View style={styles.imageModalImageCenterContainer}>
{isLoading && <FullScreenLoadingIndicator />}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,13 @@ function extractAttachments(
// and navigating back (<) shows the image preceding the first instance, not the selected duplicate's position.
const uniqueSources = new Set();

let currentLink = '';

const htmlParser = new HtmlParser({
onopentag: (name, attribs) => {
if (name === 'a' && attribs.href) {
currentLink = attribs.href;
}
if (name === 'video') {
const source = tryResolveUrlFromApiRoot(attribs[CONST.ATTACHMENT_SOURCE_ATTRIBUTE]);
if (uniqueSources.has(source)) {
Expand Down Expand Up @@ -81,9 +86,17 @@ function extractAttachments(
file: {name: fileName, width, height},
isReceipt: false,
hasBeenFlagged: attribs['data-flagged'] === 'true',
attachmentLink: currentLink,
});
}
},
onclosetag: (name) => {
if (name !== 'a' || !currentLink) {
return;
}

currentLink = '';
},
});

if (type === CONST.ATTACHMENT_TYPE.NOTE) {
Expand Down
2 changes: 2 additions & 0 deletions src/components/Attachments/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ type Attachment = {
isReceipt?: boolean;

duration?: number;

attachmentLink?: string;
};

export type {AttachmentSource, Attachment};
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ function AnchorRenderer({tnode, style, key}: AnchorRendererProps) {
const internalNewExpensifyPath = Link.getInternalNewExpensifyPath(attrHref);
const internalExpensifyPath = Link.getInternalExpensifyPath(attrHref);
const isVideo = attrHref && Str.isVideo(attrHref);
const isLinkHasImage = tnode.tagName === 'a' && tnode.children.some((child) => child.tagName === 'img');

const isDeleted = HTMLEngineUtils.isDeletedNode(tnode);
const textDecorationLineStyle = isDeleted ? styles.underlineLineThrough : {};
Expand Down Expand Up @@ -73,6 +74,7 @@ function AnchorRenderer({tnode, style, key}: AnchorRendererProps) {
key={key}
// Only pass the press handler for internal links. For public links or whitelisted internal links fallback to default link handling
onPress={internalNewExpensifyPath || internalExpensifyPath ? () => Link.openLink(attrHref, environmentURL, isAttachment) : undefined}
isLinkHasImage={isLinkHasImage}
>
<TNodeChildrenRenderer
tnode={tnode}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,8 @@ function ImageRenderer({tnode}: ImageRendererProps) {
return;
}

const route = ROUTES.ATTACHMENTS?.getRoute(reportID ?? '-1', type, source, accountID, isAttachmentOrReceipt);
const attachmentLink = tnode.parent?.attributes?.href;
const route = ROUTES.ATTACHMENTS?.getRoute(reportID ?? '-1', type, source, accountID, isAttachmentOrReceipt, attachmentLink);
Navigation.navigate(route);
}}
onLongPress={(event) => {
Expand Down
25 changes: 23 additions & 2 deletions src/components/Header.tsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import type {ReactNode} from 'react';
import React, {useMemo} from 'react';
import type {StyleProp, TextStyle, ViewStyle} from 'react-native';
import {View} from 'react-native';
import {Linking, View} from 'react-native';
import useThemeStyles from '@hooks/useThemeStyles';
import EnvironmentBadge from './EnvironmentBadge';
import Text from './Text';
import TextLink from './TextLink';

type HeaderProps = {
/** Title of the Header */
Expand All @@ -21,9 +22,12 @@ type HeaderProps = {

/** Additional header container styles */
containerStyles?: StyleProp<ViewStyle>;

/** The URL link associated with the attachment's subtitle, if available */
subTitleLink?: string;
};

function Header({title = '', subtitle = '', textStyles = [], containerStyles = [], shouldShowEnvironmentBadge = false}: HeaderProps) {
function Header({title = '', subtitle = '', textStyles = [], containerStyles = [], shouldShowEnvironmentBadge = false, subTitleLink = ''}: HeaderProps) {
const styles = useThemeStyles();
const renderedSubtitle = useMemo(
() => (
Expand All @@ -43,6 +47,22 @@ function Header({title = '', subtitle = '', textStyles = [], containerStyles = [
),
[subtitle, styles],
);

const renderedSubTitleLink = useMemo(
() => (
<TextLink
onPress={() => {
Linking.openURL(subTitleLink);
}}
numberOfLines={1}
style={styles.label}
>
{subTitleLink}
</TextLink>
),
[styles.label, subTitleLink],
);

return (
<View style={[styles.flex1, styles.flexRow, containerStyles]}>
<View style={styles.mw100}>
Expand All @@ -57,6 +77,7 @@ function Header({title = '', subtitle = '', textStyles = [], containerStyles = [
)
: title}
{renderedSubtitle}
{!!subTitleLink && renderedSubTitleLink}
</View>
{shouldShowEnvironmentBadge && <EnvironmentBadge />}
</View>
Expand Down
3 changes: 3 additions & 0 deletions src/components/HeaderWithBackButton/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ function HeaderWithBackButton({
shouldDisplaySearchRouter = false,
progressBarPercentage,
style,
subTitleLink = '',
}: HeaderWithBackButtonProps) {
const theme = useTheme();
const styles = useThemeStyles();
Expand Down Expand Up @@ -108,10 +109,12 @@ function HeaderWithBackButton({
title={title}
subtitle={stepCounter ? translate('stepCounter', stepCounter) : subtitle}
textStyles={[titleColor ? StyleUtils.getTextColorStyle(titleColor) : {}, isCentralPaneSettings && styles.textHeadlineH2]}
subTitleLink={subTitleLink}
/>
);
}, [
StyleUtils,
subTitleLink,
isCentralPaneSettings,
policy,
progressBarPercentage,
Expand Down
3 changes: 3 additions & 0 deletions src/components/HeaderWithBackButton/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,9 @@ type HeaderWithBackButtonProps = Partial<ChildrenProps> & {

/** Additional styles to add to the component */
style?: StyleProp<ViewStyle>;

/** The URL link associated with the attachment's subtitle, if available */
subTitleLink?: string;
};

export type {ThreeDotsMenuItem};
Expand Down
1 change: 1 addition & 0 deletions src/libs/Navigation/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1549,6 +1549,7 @@ type AuthScreensParamList = CentralPaneScreensParamList &
type: ValueOf<typeof CONST.ATTACHMENT_TYPE>;
accountID: string;
isAuthTokenRequired?: string;
attachmentLink?: string;
};
[SCREENS.PROFILE_AVATAR]: {
accountID: string;
Expand Down
6 changes: 4 additions & 2 deletions src/pages/home/report/ReportAttachments.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ function ReportAttachments({route}: ReportAttachmentsProps) {
const type = route.params.type;
const accountID = route.params.accountID;
const isAuthTokenRequired = route.params.isAuthTokenRequired;
const attachmentLink = route.params.attachmentLink;
const [report] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${reportID || -1}`);
const [isLoadingApp] = useOnyx(ONYXKEYS.IS_LOADING_APP);

Expand All @@ -26,10 +27,10 @@ function ReportAttachments({route}: ReportAttachmentsProps) {

const onCarouselAttachmentChange = useCallback(
(attachment: Attachment) => {
const routeToNavigate = ROUTES.ATTACHMENTS.getRoute(reportID, type, String(attachment.source), Number(accountID));
const routeToNavigate = ROUTES.ATTACHMENTS.getRoute(reportID, type, String(attachment.source), Number(accountID), attachment.isAuthTokenRequired, attachment.attachmentLink);
Navigation.navigate(routeToNavigate);
},
[reportID, accountID, type],
[reportID, type, accountID],
);

return (
Expand All @@ -48,6 +49,7 @@ function ReportAttachments({route}: ReportAttachmentsProps) {
onCarouselAttachmentChange={onCarouselAttachmentChange}
shouldShowNotFoundPage={!isLoadingApp && type !== CONST.ATTACHMENT_TYPE.SEARCH && !report?.reportID}
isAuthTokenRequired={!!isAuthTokenRequired}
attachmentLink={attachmentLink ?? ''}
/>
);
}
Expand Down
Loading