-
Notifications
You must be signed in to change notification settings - Fork 3k
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 anchor renderer for links in report messages & clean up #10144
Changes from 9 commits
0525413
9a92a2b
8c89340
7b2d0d9
8e9e020
b434079
a04ab01
601dd5a
1787a78
1f7845a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,56 @@ | ||
import React from 'react'; | ||
import {Pressable} from 'react-native'; | ||
import * as anchorForAttachmentsOnlyPropTypes from './anchorForAttachmentsOnlyPropTypes'; | ||
import AttachmentView from '../AttachmentView'; | ||
import fileDownload from '../../libs/fileDownload'; | ||
import addEncryptedAuthTokenToURL from '../../libs/addEncryptedAuthTokenToURL'; | ||
|
||
class BaseAnchorForAttachmentsOnly extends React.Component { | ||
constructor(props) { | ||
super(props); | ||
|
||
this.state = { | ||
isDownloading: false, | ||
}; | ||
this.processDownload = this.processDownload.bind(this); | ||
} | ||
|
||
/** | ||
* Initiate file downloading and update downloading flags | ||
* | ||
* @param {String} href | ||
* @param {String} fileName | ||
*/ | ||
processDownload(href, fileName) { | ||
this.setState({isDownloading: true}); | ||
fileDownload(href, fileName).then(() => this.setState({isDownloading: false})); | ||
} | ||
|
||
render() { | ||
const source = addEncryptedAuthTokenToURL(this.props.source); | ||
|
||
return ( | ||
<Pressable | ||
style={this.props.style} | ||
onPress={() => { | ||
if (this.state.isDownloading) { | ||
return; | ||
} | ||
this.processDownload(source, this.props.displayName); | ||
}} | ||
> | ||
<AttachmentView | ||
sourceURL={source} | ||
file={{name: this.props.displayName}} | ||
shouldShowDownloadIcon | ||
shouldShowLoadingSpinnerIcon={this.state.isDownloading} | ||
/> | ||
</Pressable> | ||
); | ||
} | ||
} | ||
|
||
BaseAnchorForAttachmentsOnly.propTypes = anchorForAttachmentsOnlyPropTypes.propTypes; | ||
BaseAnchorForAttachmentsOnly.defaultProps = anchorForAttachmentsOnlyPropTypes.defaultProps; | ||
|
||
export default BaseAnchorForAttachmentsOnly; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
import PropTypes from 'prop-types'; | ||
import stylePropTypes from '../../styles/stylePropTypes'; | ||
|
||
const propTypes = { | ||
/** The URL of the attachment */ | ||
source: PropTypes.string, | ||
|
||
/** Filename in case of attachments, anchor text in case of URLs or emails. */ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If this is for attachments only, why would there be URLs or emails? |
||
displayName: PropTypes.string, | ||
|
||
/** Any additional styles to apply */ | ||
style: stylePropTypes, | ||
}; | ||
|
||
const defaultProps = { | ||
source: '', | ||
style: {}, | ||
displayName: '', | ||
}; | ||
|
||
export {propTypes, defaultProps}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
import React from 'react'; | ||
import * as anchorForAttachmentsOnlyPropTypes from './anchorForAttachmentsOnlyPropTypes'; | ||
import BaseAnchorForAttachmentsOnly from './BaseAnchorForAttachmentsOnly'; | ||
|
||
// eslint-disable-next-line react/jsx-props-no-spreading | ||
const AnchorForAttachmentsOnly = props => <BaseAnchorForAttachmentsOnly {...props} />; | ||
|
||
AnchorForAttachmentsOnly.propTypes = anchorForAttachmentsOnlyPropTypes.propTypes; | ||
AnchorForAttachmentsOnly.defaultProps = anchorForAttachmentsOnlyPropTypes.defaultProps; | ||
AnchorForAttachmentsOnly.displayName = 'AnchorForAttachmentsOnly'; | ||
|
||
export default AnchorForAttachmentsOnly; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
import React from 'react'; | ||
import * as anchorForAttachmentsOnlyPropTypes from './anchorForAttachmentsOnlyPropTypes'; | ||
import BaseAnchorForAttachmentsOnly from './BaseAnchorForAttachmentsOnly'; | ||
import styles from '../../styles/styles'; | ||
|
||
// eslint-disable-next-line react/jsx-props-no-spreading | ||
const AnchorForAttachmentsOnly = props => <BaseAnchorForAttachmentsOnly {...props} style={styles.mw100} />; | ||
|
||
AnchorForAttachmentsOnly.propTypes = anchorForAttachmentsOnlyPropTypes.propTypes; | ||
AnchorForAttachmentsOnly.defaultProps = anchorForAttachmentsOnlyPropTypes.defaultProps; | ||
AnchorForAttachmentsOnly.displayName = 'AnchorForAttachmentsOnly'; | ||
|
||
export default AnchorForAttachmentsOnly; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,107 @@ | ||
import _ from 'underscore'; | ||
import React from 'react'; | ||
import {StyleSheet} from 'react-native'; | ||
import lodashGet from 'lodash/get'; | ||
import Str from 'expensify-common/lib/str'; | ||
import PropTypes from 'prop-types'; | ||
import Text from './Text'; | ||
import PressableWithSecondaryInteraction from './PressableWithSecondaryInteraction'; | ||
import * as ReportActionContextMenu from '../pages/home/report/ContextMenu/ReportActionContextMenu'; | ||
import * as ContextMenuActions from '../pages/home/report/ContextMenu/ContextMenuActions'; | ||
import Tooltip from './Tooltip'; | ||
import canUseTouchScreen from '../libs/canUseTouchscreen'; | ||
import styles from '../styles/styles'; | ||
import withWindowDimensions, {windowDimensionsPropTypes} from './withWindowDimensions'; | ||
|
||
const propTypes = { | ||
/** The URL to open */ | ||
href: PropTypes.string, | ||
|
||
/** What headers to send to the linked page (usually noopener and noreferrer) | ||
This is unused in native, but is here for parity with web */ | ||
rel: PropTypes.string, | ||
|
||
/** Used to determine where to open a link ("_blank" is passed for a new tab) | ||
This is unused in native, but is here for parity with web */ | ||
target: PropTypes.string, | ||
|
||
/** Any children to display */ | ||
children: PropTypes.node, | ||
|
||
/** Filename in case of attachments, anchor text in case of URLs or emails. */ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is for comments only, so I would think attachments don't apply. |
||
displayName: PropTypes.string, | ||
|
||
/** Any additional styles to apply */ | ||
// eslint-disable-next-line react/forbid-prop-types | ||
style: PropTypes.any, | ||
|
||
/** Press handler for the link, when not passed, default href is used to create a link like behaviour */ | ||
onPress: PropTypes.func, | ||
|
||
...windowDimensionsPropTypes, | ||
}; | ||
|
||
const defaultProps = { | ||
href: '', | ||
rel: '', | ||
target: '', | ||
children: null, | ||
style: {}, | ||
displayName: '', | ||
onPress: undefined, | ||
}; | ||
|
||
/* | ||
* This is a default anchor component for regular links. | ||
*/ | ||
const BaseAnchorForCommentsOnly = (props) => { | ||
let linkRef; | ||
const rest = _.omit(props, _.keys(propTypes)); | ||
const linkProps = {}; | ||
if (_.isFunction(props.onPress)) { | ||
linkProps.onPress = props.onPress; | ||
} else { | ||
linkProps.href = props.href; | ||
} | ||
const defaultTextStyle = canUseTouchScreen() || props.isSmallScreenWidth ? {} : styles.userSelectText; | ||
|
||
return ( | ||
<PressableWithSecondaryInteraction | ||
inline | ||
onSecondaryInteraction={ | ||
(event) => { | ||
ReportActionContextMenu.showContextMenu( | ||
Str.isValidEmail(props.displayName) ? ContextMenuActions.CONTEXT_MENU_TYPES.EMAIL : ContextMenuActions.CONTEXT_MENU_TYPES.LINK, | ||
event, | ||
props.href, | ||
lodashGet(linkRef, 'current'), | ||
); | ||
} | ||
} | ||
> | ||
<Tooltip text={Str.isValidEmail(props.displayName) ? '' : props.href}> | ||
<Text | ||
ref={el => linkRef = el} | ||
style={StyleSheet.flatten([props.style, defaultTextStyle])} | ||
accessibilityRole="link" | ||
hrefAttrs={{ | ||
rel: props.rel, | ||
target: props.target, | ||
}} | ||
// eslint-disable-next-line react/jsx-props-no-spreading | ||
{...linkProps} | ||
// eslint-disable-next-line react/jsx-props-no-spreading | ||
{...rest} | ||
> | ||
{props.children} | ||
</Text> | ||
</Tooltip> | ||
</PressableWithSecondaryInteraction> | ||
); | ||
}; | ||
|
||
BaseAnchorForCommentsOnly.propTypes = propTypes; | ||
BaseAnchorForCommentsOnly.defaultProps = defaultProps; | ||
BaseAnchorForCommentsOnly.displayName = 'BaseAnchorForCommentsOnly'; | ||
|
||
export default withWindowDimensions(BaseAnchorForCommentsOnly); |
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We shouldn't add encyptToken for local file, it will cause error console.More details: #54640There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@hungvu193 Seriously. This PR is 3 years old. At that time w me didn't have local file. Also, this PR only does refactoring.
Can you please find a proper PR which surfaced the issue?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, you're correct. My bad, just found the offending PR.