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

Chat - File does not appear with strikethrough style when uploaded offline and deleted #47971

Merged
merged 22 commits into from
Oct 23, 2024
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
24221c7
fix: Chat - File does not appear with strikethrough style when upload…
Krishna2323 Aug 1, 2024
8bcdcbc
Merge branch 'Expensify:main' into krishna2323/issue/46616
Krishna2323 Aug 19, 2024
05b2f2d
updated design.
Krishna2323 Aug 19, 2024
ed9a174
Merge branch 'Expensify:main' into krishna2323/issue/46616
Krishna2323 Aug 24, 2024
365fd73
add deleted indicator for video renderer.
Krishna2323 Aug 24, 2024
98aa154
add styles object for deleted indicator.
Krishna2323 Aug 24, 2024
0100314
add AttachmentDeletedIndicator.tsx
Krishna2323 Aug 24, 2024
dc8d1ce
Merge branch 'main' into krishna2323/issue/46616
Krishna2323 Aug 27, 2024
1eb1b71
Merge branch 'Expensify:main' into krishna2323/issue/46616
Krishna2323 Aug 28, 2024
609dd79
Merge branch 'Expensify:main' into krishna2323/issue/46616
Krishna2323 Aug 31, 2024
15353c7
fix icon colot.
Krishna2323 Sep 1, 2024
9d305c0
Merge branch 'main' into krishna2323/issue/46616
Krishna2323 Sep 23, 2024
0cb93cd
fix lint issues.
Krishna2323 Sep 23, 2024
434b6a6
Merge branch 'Expensify:main' into krishna2323/issue/46616
Krishna2323 Sep 27, 2024
6ca5df9
add suggested changes.
Krishna2323 Sep 27, 2024
2f60806
Merge branch 'Expensify:main' into krishna2323/issue/46616
Krishna2323 Oct 2, 2024
d4a068f
fix: lint issues.
Krishna2323 Oct 2, 2024
ff3654b
Merge branch 'Expensify:main' into krishna2323/issue/46616
Krishna2323 Oct 13, 2024
2c26757
Merge branch 'main' into krishna2323/issue/46616
Krishna2323 Oct 14, 2024
98c927b
Merge branch 'main' into krishna2323/issue/46616
Krishna2323 Oct 18, 2024
a7821d1
fix: lint issues.
Krishna2323 Oct 20, 2024
ec696aa
Merge branch 'main' into krishna2323/issue/46616
Krishna2323 Oct 22, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ type BaseAnchorForAttachmentsOnlyProps = AnchorForAttachmentsOnlyProps &
onPressOut?: () => void;
};

function BaseAnchorForAttachmentsOnly({style, source = '', displayName = '', download, onPressIn, onPressOut}: BaseAnchorForAttachmentsOnlyProps) {
function BaseAnchorForAttachmentsOnly({style, source = '', displayName = '', download, onPressIn, onPressOut, isDeleted}: BaseAnchorForAttachmentsOnlyProps) {
const sourceURLWithAuth = addEncryptedAuthTokenToURL(source);
const sourceID = (source.match(CONST.REGEX.ATTACHMENT_ID) ?? [])[1];

Expand Down Expand Up @@ -69,6 +69,7 @@ function BaseAnchorForAttachmentsOnly({style, source = '', displayName = '', dow
shouldShowDownloadIcon={!!sourceID && !isOffline}
shouldShowLoadingSpinnerIcon={isDownloading}
isUsedAsChatAttachment
isDeleted={!!isDeleted}
/>
</PressableWithoutFeedback>
)}
Expand Down
3 changes: 3 additions & 0 deletions src/components/AnchorForAttachmentsOnly/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ type AnchorForAttachmentsOnlyProps = {

/** Any additional styles to apply */
style?: StyleProp<ViewStyle>;

/** Whether the attachment is deleted */
isDeleted?: boolean;
};

export default AnchorForAttachmentsOnlyProps;
44 changes: 44 additions & 0 deletions src/components/AttachmentDeletedIndicator.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
import React from 'react';
import {View} from 'react-native';
import type {StyleProp, ViewStyle} from 'react-native';
import useNetwork from '@hooks/useNetwork';
import useTheme from '@hooks/useTheme';
import useThemeStyles from '@hooks/useThemeStyles';
import variables from '@styles/variables';
import Icon from './Icon';
import * as Expensicons from './Icon/Expensicons';

type AttachmentOfflineIndicatorProps = {
Krishna2323 marked this conversation as resolved.
Show resolved Hide resolved
/** Any additional styles to apply */
Krishna2323 marked this conversation as resolved.
Show resolved Hide resolved
containerStyles?: StyleProp<ViewStyle>;
};

function AttachmentDeletedIndicator({containerStyles}: AttachmentOfflineIndicatorProps) {
Krishna2323 marked this conversation as resolved.
Show resolved Hide resolved
const theme = useTheme();
const styles = useThemeStyles();
const {isOffline} = useNetwork();

if (!isOffline) {
return null;
}

return (
<>
<View
style={[styles.pAbsolute, styles.alignItemsCenter, styles.justifyContentCenter, styles.highlightBG, styles.deletedIndicatorOverlay, styles.deletedIndicator, containerStyles]}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still going through these styles.

/>
<View style={[styles.pAbsolute, styles.deletedIndicator, styles.alignItemsCenter, styles.justifyContentCenter, containerStyles]}>
<Icon
fill={theme.icon}
src={Expensicons.Trashcan}
width={variables.iconSizeSuperLarge}
height={variables.iconSizeSuperLarge}
/>
</View>
</>
);
}

AttachmentDeletedIndicator.displayName = 'AttachmentDeletedIndicator';

export default AttachmentDeletedIndicator;
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,12 @@ type DefaultAttachmentViewProps = {
containerStyles?: StyleProp<ViewStyle>;

icon?: IconAsset;

/** Whether the attachment is deleted */
Krishna2323 marked this conversation as resolved.
Show resolved Hide resolved
isDeleted?: boolean;
};

function DefaultAttachmentView({fileName = '', shouldShowLoadingSpinnerIcon = false, shouldShowDownloadIcon, containerStyles, icon}: DefaultAttachmentViewProps) {
function DefaultAttachmentView({fileName = '', shouldShowLoadingSpinnerIcon = false, shouldShowDownloadIcon, containerStyles, icon, isDeleted}: DefaultAttachmentViewProps) {
const theme = useTheme();
const styles = useThemeStyles();
const {translate} = useLocalize();
Expand All @@ -40,7 +43,7 @@ function DefaultAttachmentView({fileName = '', shouldShowLoadingSpinnerIcon = fa
/>
</View>

<Text style={[styles.textStrong, styles.flexShrink1, styles.breakAll, styles.flexWrap, styles.mw100]}>{fileName}</Text>
<Text style={[styles.textStrong, styles.flexShrink1, styles.breakAll, styles.flexWrap, styles.mw100, isDeleted && styles.lineThrough]}>{fileName}</Text>
{!shouldShowLoadingSpinnerIcon && shouldShowDownloadIcon && (
<Tooltip text={translate('common.download')}>
<View style={styles.ml2}>
Expand Down
5 changes: 5 additions & 0 deletions src/components/Attachments/AttachmentView/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,9 @@ type AttachmentViewProps = AttachmentViewOnyxProps &

/* Flag indicating whether the attachment has been uploaded. */
isUploaded?: boolean;

/** Whether the attachment is deleted */
Krishna2323 marked this conversation as resolved.
Show resolved Hide resolved
isDeleted?: boolean;
};

function AttachmentView({
Expand All @@ -101,6 +104,7 @@ function AttachmentView({
duration,
isUsedAsChatAttachment,
isUploaded = true,
isDeleted,
}: AttachmentViewProps) {
const {translate} = useLocalize();
const {updateCurrentlyPlayingURL} = usePlaybackContext();
Expand Down Expand Up @@ -290,6 +294,7 @@ function AttachmentView({
shouldShowDownloadIcon={shouldShowDownloadIcon}
shouldShowLoadingSpinnerIcon={shouldShowLoadingSpinnerIcon}
containerStyles={containerStyles}
isDeleted={isDeleted}
/>
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,15 @@ function AnchorRenderer({tnode, style, key}: AnchorRendererProps) {
const isAttachment = !!htmlAttribs[CONST.ATTACHMENT_SOURCE_ATTRIBUTE];
const tNodeChild = tnode?.domNode?.children?.[0];
const displayName = tNodeChild && 'data' in tNodeChild && typeof tNodeChild.data === 'string' ? tNodeChild.data : '';
const parentStyle = tnode.parent?.styles?.nativeTextRet ?? {};
const attrHref = htmlAttribs.href || htmlAttribs[CONST.ATTACHMENT_SOURCE_ATTRIBUTE] || '';
const parentStyle = tnode.parent?.styles?.nativeTextRet ?? {};
const internalNewExpensifyPath = Link.getInternalNewExpensifyPath(attrHref);
const internalExpensifyPath = Link.getInternalExpensifyPath(attrHref);
const isVideo = attrHref && Str.isVideo(attrHref);

const hasStrikethroughStyle = 'textDecorationLine' in parentStyle && parentStyle.textDecorationLine === 'line-through';
const textDecorationLineStyle = hasStrikethroughStyle ? styles.underlineLineThrough : {};

if (!HTMLEngineUtils.isChildOfComment(tnode)) {
// This is not a comment from a chat, the AnchorForCommentsOnly uses a Pressable to create a context menu on right click.
// We don't have this behaviour in other links in NewDot
Expand All @@ -51,13 +54,11 @@ function AnchorRenderer({tnode, style, key}: AnchorRendererProps) {
<AnchorForAttachmentsOnly
source={tryResolveUrlFromApiRoot(attrHref)}
displayName={displayName}
isDeleted={hasStrikethroughStyle}
/>
);
}

const hasStrikethroughStyle = 'textDecorationLine' in parentStyle && parentStyle.textDecorationLine === 'line-through';
const textDecorationLineStyle = hasStrikethroughStyle ? styles.underlineLineThrough : {};

return (
<AnchorForCommentsOnly
href={attrHref}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ function ImageRenderer({tnode}: ImageRendererProps) {

const htmlAttribs = tnode.attributes;

const parentStyle = tnode.parent?.styles?.nativeTextRet ?? {};
const isDeleted = 'textDecorationLine' in parentStyle && parentStyle.textDecorationLine === 'line-through';

// There are two kinds of images that need to be displayed:
//
// - Chat Attachment images
Expand Down Expand Up @@ -72,6 +75,7 @@ function ImageRenderer({tnode}: ImageRendererProps) {
fallbackIcon={fallbackIcon}
imageWidth={imageWidth}
imageHeight={imageHeight}
isDeleted={isDeleted}
altText={alt}
/>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ function VideoRenderer({tnode, key}: VideoRendererProps) {
const height = Number(htmlAttribs[CONST.ATTACHMENT_THUMBNAIL_HEIGHT_ATTRIBUTE]);
const duration = Number(htmlAttribs[CONST.ATTACHMENT_DURATION_ATTRIBUTE]);
const currentReportIDValue = useCurrentReportID();
const parentStyle = tnode.parent?.styles?.nativeTextRet ?? {};
Krishna2323 marked this conversation as resolved.
Show resolved Hide resolved
const isDeleted = 'textDecorationLine' in parentStyle && parentStyle.textDecorationLine === 'line-through';

return (
<ShowContextMenuContext.Consumer>
Expand All @@ -39,6 +41,7 @@ function VideoRenderer({tnode, key}: VideoRendererProps) {
thumbnailUrl={thumbnailUrl}
videoDimensions={{width, height}}
videoDuration={duration}
isDeleted={isDeleted}
onShowModalPress={() => {
if (!sourceURL || !type) {
return;
Expand Down
6 changes: 6 additions & 0 deletions src/components/ThumbnailImage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import useThumbnailDimensions from '@hooks/useThumbnailDimensions';
import variables from '@styles/variables';
import CONST from '@src/CONST';
import type IconAsset from '@src/types/utils/IconAsset';
import AttachmentDeletedIndicator from './AttachmentDeletedIndicator';
import Icon from './Icon';
import * as Expensicons from './Icon/Expensicons';
import type {ImageObjectPosition} from './Image/types';
Expand Down Expand Up @@ -55,6 +56,9 @@ type ThumbnailImageProps = {

/** The object position of image */
objectPosition?: ImageObjectPosition;

/** Whether the image is deleted */
Krishna2323 marked this conversation as resolved.
Show resolved Hide resolved
isDeleted?: boolean;
};

type UpdateImageSizeParams = {
Expand All @@ -75,6 +79,7 @@ function ThumbnailImage({
fallbackIconColor,
fallbackIconBackground,
objectPosition = CONST.IMAGE_OBJECT_POSITION.INITIAL,
isDeleted,
}: ThumbnailImageProps) {
const styles = useThemeStyles();
const theme = useTheme();
Expand Down Expand Up @@ -133,6 +138,7 @@ function ThumbnailImage({

return (
<View style={[style, styles.overflowHidden]}>
{isDeleted && <AttachmentDeletedIndicator containerStyles={[...sizeStyles]} />}
<View style={[...sizeStyles, styles.alignItemsCenter, styles.justifyContentCenter]}>
<ImageWithSizeCalculation
url={previewSourceURL}
Expand Down
68 changes: 38 additions & 30 deletions src/components/VideoPlayerPreview/VideoPlayerThumbnail.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import React from 'react';
import {View} from 'react-native';
import type {GestureResponderEvent} from 'react-native';
import AttachmentDeletedIndicator from '@components/AttachmentDeletedIndicator';
import Icon from '@components/Icon';
import * as Expensicons from '@components/Icon/Expensicons';
import Image from '@components/Image';
Expand All @@ -22,9 +23,12 @@ type VideoPlayerThumbnailProps = {

/** Accessibility label for the thumbnail. */
accessibilityLabel: string;

/** Whether the video is deleted */
Krishna2323 marked this conversation as resolved.
Show resolved Hide resolved
isDeleted?: boolean;
};

function VideoPlayerThumbnail({thumbnailUrl, onPress, accessibilityLabel}: VideoPlayerThumbnailProps) {
function VideoPlayerThumbnail({thumbnailUrl, onPress, accessibilityLabel, isDeleted}: VideoPlayerThumbnailProps) {
const styles = useThemeStyles();

return (
Expand All @@ -39,35 +43,39 @@ function VideoPlayerThumbnail({thumbnailUrl, onPress, accessibilityLabel}: Video
/>
</View>
)}
<ShowContextMenuContext.Consumer>
{({anchor, report, reportNameValuePairs, action, checkIfContextMenuActive, isDisabled}) => (
<PressableWithoutFeedback
style={[styles.videoThumbnailContainer]}
accessibilityLabel={accessibilityLabel}
accessibilityRole={CONST.ROLE.BUTTON}
onPress={onPress}
onPressIn={() => DeviceCapabilities.canUseTouchScreen() && ControlSelection.block()}
onPressOut={() => ControlSelection.unblock()}
onLongPress={(event) => {
if (isDisabled) {
return;
}
showContextMenuForReport(event, anchor, report?.reportID ?? '-1', action, checkIfContextMenuActive, ReportUtils.isArchivedRoom(report, reportNameValuePairs));
}}
shouldUseHapticsOnLongPress
>
<View style={[styles.videoThumbnailPlayButton]}>
<Icon
src={Expensicons.Play}
fill="white"
width={variables.iconSizeXLarge}
height={variables.iconSizeXLarge}
additionalStyles={[styles.ml1]}
/>
</View>
</PressableWithoutFeedback>
)}
</ShowContextMenuContext.Consumer>
{!isDeleted ? (
<ShowContextMenuContext.Consumer>
{({anchor, report, reportNameValuePairs, action, checkIfContextMenuActive, isDisabled}) => (
<PressableWithoutFeedback
style={[styles.videoThumbnailContainer]}
accessibilityLabel={accessibilityLabel}
accessibilityRole={CONST.ROLE.BUTTON}
onPress={onPress}
onPressIn={() => DeviceCapabilities.canUseTouchScreen() && ControlSelection.block()}
onPressOut={() => ControlSelection.unblock()}
onLongPress={(event) => {
if (isDisabled) {
return;
}
showContextMenuForReport(event, anchor, report?.reportID ?? '-1', action, checkIfContextMenuActive, ReportUtils.isArchivedRoom(report, reportNameValuePairs));
}}
shouldUseHapticsOnLongPress
>
<View style={[styles.videoThumbnailPlayButton]}>
<Icon
src={Expensicons.Play}
fill="white"
width={variables.iconSizeXLarge}
height={variables.iconSizeXLarge}
additionalStyles={[styles.ml1]}
/>
</View>
</PressableWithoutFeedback>
)}
</ShowContextMenuContext.Consumer>
) : (
<AttachmentDeletedIndicator containerStyles={{borderRadius: variables.componentBorderRadiusNormal}} />
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's move this to styles?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think there's any benefit to that, but if you think otherwise, I can update it.

)}
</View>
);
}
Expand Down
8 changes: 6 additions & 2 deletions src/components/VideoPlayerPreview/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,12 @@ type VideoPlayerPreviewProps = {

/** Callback executed when modal is pressed. */
onShowModalPress: (event?: GestureResponderEvent | KeyboardEvent) => void | Promise<void>;

/** Whether the video is deleted */
Krishna2323 marked this conversation as resolved.
Show resolved Hide resolved
isDeleted?: boolean;
};

function VideoPlayerPreview({videoUrl, thumbnailUrl, reportID, fileName, videoDimensions, videoDuration, onShowModalPress}: VideoPlayerPreviewProps) {
function VideoPlayerPreview({videoUrl, thumbnailUrl, reportID, fileName, videoDimensions, videoDuration, onShowModalPress, isDeleted}: VideoPlayerPreviewProps) {
const styles = useThemeStyles();
const {translate} = useLocalize();
const {currentlyPlayingURL, currentlyPlayingURLReportID, updateCurrentlyPlayingURL} = usePlaybackContext();
Expand Down Expand Up @@ -71,11 +74,12 @@ function VideoPlayerPreview({videoUrl, thumbnailUrl, reportID, fileName, videoDi

return (
<View style={[styles.webViewStyles.tagStyles.video, thumbnailDimensionsStyles]}>
{shouldUseNarrowLayout || isThumbnail ? (
{shouldUseNarrowLayout || isThumbnail || isDeleted ? (
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need the isDeleted check in the || chain?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep, when attachment is deleted, we should be showing VideoPlayerThumbnail.

<VideoPlayerThumbnail
thumbnailUrl={thumbnailUrl}
onPress={handleOnPress}
accessibilityLabel={fileName}
isDeleted={isDeleted}
/>
) : (
<View style={styles.flex1}>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import React from 'react';
import {View} from 'react-native';
import useThemeStyles from '@hooks/useThemeStyles';
import CONST from '@src/CONST';
import type {OriginalMessageSource} from '@src/types/onyx/OriginalMessage';
import RenderCommentHTML from './RenderCommentHTML';

Expand All @@ -14,8 +13,7 @@ type AttachmentCommentFragmentProps = {

function AttachmentCommentFragment({addExtraMargin, html, source, styleAsDeleted}: AttachmentCommentFragmentProps) {
const styles = useThemeStyles();
const isUploading = html.includes(CONST.ATTACHMENT_OPTIMISTIC_SOURCE_ATTRIBUTE);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I missed this why is isUploading removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When isUploading was true, we were not applying the deleted styles. To show deleted icon even when the attachment is uploaded and deleted offline, we have to remove isUploading check.

const htmlContent = styleAsDeleted && isUploading ? `<del>${html}</del>` : html;
const htmlContent = styleAsDeleted ? `<del>${html}</del>` : html;

return (
<View style={addExtraMargin ? styles.mt2 : {}}>
Expand Down
11 changes: 11 additions & 0 deletions src/styles/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1111,6 +1111,17 @@ const styles = (theme: ThemeColors) =>
height: 25,
},

deletedIndicator: {
Krishna2323 marked this conversation as resolved.
Show resolved Hide resolved
zIndex: 20,
width: '100%',
height: '100%',
overflow: 'hidden',
},

deletedIndicatorOverlay: {
opacity: 0.8,
},

// Actions
actionAvatar: {
borderRadius: 20,
Expand Down
Loading