Skip to content

Change UI view for Attachments in Chat #4736

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

Merged
merged 13 commits into from
Aug 23, 2021
Merged
Show file tree
Hide file tree
Changes from 6 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
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
import React from 'react';
import {StyleSheet} from 'react-native';
import {Pressable, StyleSheet} from 'react-native';
import lodashGet from 'lodash/get';
import Text from '../../Text';
import Icon from '../../Icon';
import {propTypes, defaultProps} from '../anchorForCommentsOnlyPropTypes';
import PressableWithSecondaryInteraction from '../../PressableWithSecondaryInteraction';
import {showContextMenu} from '../../../pages/home/report/ContextMenu/ReportActionContextMenu';
import {CONTEXT_MENU_TYPES} from '../../../pages/home/report/ContextMenu/ContextMenuActions';
import {Download} from '../../Icon/Expensicons';
import AttachmentView from '../../AttachmentView';
import fileDownload from '../../../libs/fileDownload';


/*
Expand All @@ -17,34 +21,54 @@ const BaseAnchorForCommentsOnly = ({
target,
children,
style,
text,
...props
}) => {
let linkRef;
return (
<PressableWithSecondaryInteraction
onSecondaryInteraction={
(event) => {
showContextMenu(
CONTEXT_MENU_TYPES.LINK,
event,
href,
lodashGet(linkRef, 'current'),
);
}
}
>
<Text
ref={el => linkRef = el}
style={StyleSheet.flatten(style)}
accessibilityRole="link"
href={href}
hrefAttrs={{rel, target}}

props.shouldDownloadFile
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's update this prop to isAttachment

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link
Contributor

Choose a reason for hiding this comment

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

update prop name to isAttachment

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

? (
<Pressable onPress={() => {
fileDownload(href);
}}
>
<AttachmentView
sourceURL={href}
file={{name: text}}
rightElement={(
<Icon src={Download} />
)}
/>
</Pressable>
)
: (
<PressableWithSecondaryInteraction
onSecondaryInteraction={
(event) => {
showContextMenu(
CONTEXT_MENU_TYPES.LINK,
event,
href,
lodashGet(linkRef, 'current'),
);
}
}
>
<Text
ref={el => linkRef = el}
style={StyleSheet.flatten(style)}
accessibilityRole="link"
href={href}
hrefAttrs={{rel, target}}
// eslint-disable-next-line react/jsx-props-no-spreading
{...props}
>
{children}
</Text>
</PressableWithSecondaryInteraction>
{...props}
>
{children}
</Text>
</PressableWithSecondaryInteraction>
)

);
};

Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
import React from 'react';
import lodashGet from 'lodash/get';
import {Linking, StyleSheet} from 'react-native';
import {Linking, StyleSheet, Pressable} from 'react-native';
import {propTypes, defaultProps} from '../anchorForCommentsOnlyPropTypes';
import fileDownload from '../../../libs/fileDownload';
import Text from '../../Text';
import PressableWithSecondaryInteraction from '../../PressableWithSecondaryInteraction';
import {showContextMenu} from '../../../pages/home/report/ContextMenu/ReportActionContextMenu';
import {CONTEXT_MENU_TYPES} from '../../../pages/home/report/ContextMenu/ContextMenuActions';
import Icon from '../../Icon';
import {Download} from '../../Icon/Expensicons';
import AttachmentView from '../../AttachmentView';

/*
* This is a default anchor component for regular links.
Expand All @@ -16,12 +19,29 @@ const BaseAnchorForCommentsOnly = ({
children,
style,
shouldDownloadFile,
text,
...props
}) => {
let linkRef;
return (
<PressableWithSecondaryInteraction
onSecondaryInteraction={
shouldDownloadFile
? (
<Pressable onPress={() => {
fileDownload(href);
}}
>
<AttachmentView
sourceURL={href}
file={{name: text}}
rightElement={(
<Icon src={Download} />
)}
/>
</Pressable>
)
: (
<PressableWithSecondaryInteraction
onSecondaryInteraction={
(event) => {
showContextMenu(
CONTEXT_MENU_TYPES.LINK,
Expand All @@ -31,17 +51,18 @@ const BaseAnchorForCommentsOnly = ({
);
}
}
onPress={() => (shouldDownloadFile ? fileDownload(href) : Linking.openURL(href))}
>
<Text
ref={el => linkRef = el}
style={StyleSheet.flatten(style)}
onPress={() => Linking.openURL(href)}
>
<Text
ref={el => linkRef = el}
style={StyleSheet.flatten(style)}
// eslint-disable-next-line react/jsx-props-no-spreading
{...props}
>
{children}
</Text>
</PressableWithSecondaryInteraction>
{...props}
>
{children}
</Text>
</PressableWithSecondaryInteraction>
)
);
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ const propTypes = {
/** Any additional styles to apply */
// eslint-disable-next-line react/forbid-prop-types
style: PropTypes.any,

text: PropTypes.string,
Copy link
Contributor

Choose a reason for hiding this comment

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

Move this prop above style, and add comment
Also rename from text to fileName

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

};

const defaultProps = {
Expand All @@ -33,6 +35,7 @@ const defaultProps = {
shouldDownloadFile: false,
children: null,
style: {},
text: '',
};

export {propTypes, defaultProps};
8 changes: 8 additions & 0 deletions src/components/AttachmentView.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,16 @@ const propTypes = {
name: PropTypes.string,
}),

rightElement: PropTypes.element,

...withLocalizePropTypes,
};

const defaultProps = {
file: {
name: '',
},
rightElement: undefined,
};

const AttachmentView = (props) => {
Expand Down Expand Up @@ -58,6 +61,11 @@ const AttachmentView = (props) => {
<Icon src={Paperclip} />
</View>
<Text style={[styles.textStrong]}>{props.file && props.file.name}</Text>
{props.rightElement && (
<View style={styles.ml2}>
{props.rightElement}
</View>
)}
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of passing down rightElement as props, pass down a prop called shouldShowDownloadIcon to conditionally render the download icon

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

</View>
);
};
Expand Down
4 changes: 4 additions & 0 deletions src/components/HTMLEngineProvider/BaseHTMLEngineProvider.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
splitBoxModelStyle,
} from 'react-native-render-html';
import PropTypes from 'prop-types';
import lodashGet from 'lodash/get';
import Config from '../../CONFIG';
import styles, {webViewStyles, getFontFamilyMonospace} from '../../styles/styles';
import fontFamily from '../../styles/fontFamily';
Expand Down Expand Up @@ -67,6 +68,8 @@ function AnchorRenderer({tnode, key, style}) {

// An auth token is needed to download Expensify chat attachments
const isAttachment = Boolean(htmlAttribs['data-expensify-source']);
const fileName = isAttachment ? lodashGet(tnode, 'domNode.children[0].data') : null;
Copy link
Contributor

Choose a reason for hiding this comment

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

Add a default param to lodashGet(tnode, 'domNode.children[0].data') of an empty string '' and remove the conditional and just use that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done


return (
<AnchorForCommentsOnly
href={htmlAttribs.href}
Expand All @@ -81,6 +84,7 @@ function AnchorRenderer({tnode, key, style}) {
rel={htmlAttribs.rel || 'noopener noreferrer'}
style={style}
key={key}
text={fileName}
Copy link
Contributor

Choose a reason for hiding this comment

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

like above, change text prop name to fileName

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

>
<TNodeChildrenRenderer tnode={tnode} />
</AnchorForCommentsOnly>
Expand Down