-
Notifications
You must be signed in to change notification settings - Fork 3.7k
Fix anchor renderer for links in report messages & clean up #10144
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
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
0525413
Fix anchor renderer
parasharrajat 9a92a2b
Add the link press handler to the AnchorRenderer
parasharrajat 8c89340
Cleanup and refactor
parasharrajat 7b2d0d9
more refactor and cleanup of props
parasharrajat 8e9e020
Fix the imports
parasharrajat b434079
Add story for Banner with link
parasharrajat a04ab01
remove extra prop
parasharrajat 601dd5a
Fix formatting issues
parasharrajat 1787a78
Merge branch 'main' of github.com:Expensify/Expensify.cash into fix-l…
parasharrajat 1f7845a
Fix comments
parasharrajat File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
56 changes: 56 additions & 0 deletions
56
src/components/AnchorForAttachmentsOnly/BaseAnchorForAttachmentsOnly.js
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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; | ||
21 changes: 21 additions & 0 deletions
21
src/components/AnchorForAttachmentsOnly/anchorForAttachmentsOnlyPropTypes.js
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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 for attachments. */ | ||
| displayName: PropTypes.string, | ||
|
|
||
| /** Any additional styles to apply */ | ||
| style: stylePropTypes, | ||
| }; | ||
|
|
||
| const defaultProps = { | ||
| source: '', | ||
| style: {}, | ||
| displayName: '', | ||
| }; | ||
|
|
||
| export {propTypes, defaultProps}; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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, | ||
|
|
||
| /** Anchor text of URLs or emails. */ | ||
| 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); |
104 changes: 0 additions & 104 deletions
104
src/components/AnchorForCommentsOnly/BaseAnchorForCommentsOnly/index.js
This file was deleted.
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
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?
Uh oh!
There was an error while loading. Please reload this page.
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.