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.

Sign up for GitHub

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

Jump to bottom

fix gray placeholder shown for markdown images with invalid URLs #50918

Merged
merged 4 commits into from
Oct 21, 2024
Merged

fix gray placeholder shown for markdown images with invalid URLs #50918

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
42121a9
fix gray placeholder shown for markdown images with invalid URLs
huult Oct 16, 2024
cced27f
update fallback background color and icon color
huult Oct 17, 2024
2ee73ab
Merge remote-tracking branch 'upstream/main' into 49992-gray-placehol…
huult Oct 19, 2024
1b0ebc6
remove check for disable on offline to make consistent with online mode
huult Oct 19, 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
18 changes: 18 additions & 0 deletions assets/images/gallery-not-found.svg
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion src/components/AttachmentOfflineIndicator.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ function AttachmentOfflineIndicator({isPreview = false}: AttachmentOfflineIndica
return (
<View style={[styles.flexColumn, styles.alignItemsCenter, styles.justifyContentCenter, styles.pAbsolute, styles.h100, styles.w100, isPreview && styles.hoveredComponentBG]}>
<Icon
fill={theme.border}
fill={theme.icon}
src={Expensicons.OfflineCloud}
width={variables.iconSizeSuperLarge}
height={variables.iconSizeSuperLarge}
Expand Down
13 changes: 11 additions & 2 deletions src/components/HTMLEngineProvider/HTMLRenderers/ImageRenderer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, {memo} from 'react';
import React, {memo, useState} from 'react';
import {useOnyx} from 'react-native-onyx';
import type {OnyxEntry} from 'react-native-onyx';
import type {CustomRendererProps, TBlock} from 'react-native-render-html';
Expand All @@ -8,6 +8,7 @@ import PressableWithoutFocus from '@components/Pressable/PressableWithoutFocus';
import {ShowContextMenuContext, showContextMenuForReport} from '@components/ShowContextMenuContext';
import ThumbnailImage from '@components/ThumbnailImage';
import useLocalize from '@hooks/useLocalize';
import useTheme from '@hooks/useTheme';
import useThemeStyles from '@hooks/useThemeStyles';
import * as FileUtils from '@libs/fileDownload/FileUtils';
import Navigation from '@libs/Navigation/Navigation';
Expand Down Expand Up @@ -63,7 +64,10 @@ function ImageRenderer({tnode}: ImageRendererProps) {
const imagePreviewModalDisabled = htmlAttribs['data-expensify-preview-modal-disabled'] === 'true';

const fileType = FileUtils.getFileType(attachmentSourceAttribute);
const fallbackIcon = fileType === CONST.ATTACHMENT_FILE_TYPE.FILE ? Expensicons.Document : Expensicons.Gallery;
const fallbackIcon = fileType === CONST.ATTACHMENT_FILE_TYPE.FILE ? Expensicons.Document : Expensicons.GalleryNotFound;
const [hasLoadFailed, setHasLoadFailed] = useState(true);
const theme = useTheme();
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any specific reason why we are setting failed as the default?

Suggested change
const [hasLoadFailed, setHasLoadFailed] = useState(true);
const [hasLoadFailed, setHasLoadFailed] = useState(false);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@suneox I set the default value of hasLoadFailed to true to handle the bug that occurs if the user clicks on the image too quickly.


const thumbnailImageComponent = (
<ThumbnailImage
previewSourceURL={previewSource}
Expand All @@ -73,6 +77,10 @@ function ImageRenderer({tnode}: ImageRendererProps) {
imageWidth={imageWidth}
imageHeight={imageHeight}
altText={alt}
onLoadFailure={() => setHasLoadFailed(true)}
onMeasure={() => setHasLoadFailed(false)}
fallbackIconBackground={theme.highlightBG}
fallbackIconColor={theme.border}
/>
);

Expand Down Expand Up @@ -102,6 +110,7 @@ function ImageRenderer({tnode}: ImageRendererProps) {
shouldUseHapticsOnLongPress
accessibilityRole={CONST.ROLE.BUTTON}
accessibilityLabel={translate('accessibilityHints.viewAttachment')}
disabled={hasLoadFailed}
>
{thumbnailImageComponent}
</PressableWithoutFocus>
Expand Down
2 changes: 2 additions & 0 deletions src/components/Icon/Expensicons.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ import FlagLevelTwo from '@assets/images/flag_level_02.svg';
import FlagLevelThree from '@assets/images/flag_level_03.svg';
import Folder from '@assets/images/folder.svg';
import Fullscreen from '@assets/images/fullscreen.svg';
import GalleryNotFound from '@assets/images/gallery-not-found.svg';
import Gallery from '@assets/images/gallery.svg';
import Gear from '@assets/images/gear.svg';
import Globe from '@assets/images/globe.svg';
Expand Down Expand Up @@ -404,4 +405,5 @@ export {
Bookmark,
Star,
QBDSquare,
GalleryNotFound,
};
18 changes: 16 additions & 2 deletions src/components/ThumbnailImage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,12 @@ type ThumbnailImageProps = {

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

/** Callback fired when the image fails to load */
onLoadFailure?: () => void;

/** Callback fired when the image has been measured */
onMeasure?: () => void;
};

type UpdateImageSizeParams = {
Expand All @@ -75,6 +81,8 @@ function ThumbnailImage({
fallbackIconColor,
fallbackIconBackground,
objectPosition = CONST.IMAGE_OBJECT_POSITION.INITIAL,
onLoadFailure,
onMeasure,
}: ThumbnailImageProps) {
const styles = useThemeStyles();
const theme = useTheme();
Expand Down Expand Up @@ -137,8 +145,14 @@ function ThumbnailImage({
<ImageWithSizeCalculation
url={previewSourceURL}
altText={altText}
onMeasure={updateImageSize}
onLoadFailure={() => setFailedToLoad(true)}
onMeasure={(args) => {
updateImageSize(args);
onMeasure?.();
}}
onLoadFailure={() => {
setFailedToLoad(true);
onLoadFailure?.();
}}
isAuthTokenRequired={isAuthTokenRequired}
objectPosition={objectPosition}
/>
Expand Down
Loading