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

Added tapable links to user's login information on DetailsPage #3870

Merged
merged 11 commits into from
Jul 12, 2021
Merged

Added tapable links to user's login information on DetailsPage #3870

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
cfc8420
added tapable links to user's login info
parasharrajat Jul 2, 2021
5008009
remove Console logs
parasharrajat Jul 4, 2021
fae93e5
moved tapping behavipour to a component
parasharrajat Jul 7, 2021
9481a68
Merge branch 'main' of github.com:Expensify/Expensify.cash into tap-d…
parasharrajat Jul 7, 2021
26123f9
used constants
parasharrajat Jul 7, 2021
4e1aae3
rename and refactor
parasharrajat Jul 8, 2021
42e8a12
comment updated
parasharrajat Jul 9, 2021
7a34f08
refactor
parasharrajat Jul 9, 2021
4f3f1db
rename style prop
parasharrajat Jul 9, 2021
a681442
Pulled latest changes
parasharrajat Jul 9, 2021
ddff57b
simplified code
parasharrajat Jul 12, 2021
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
79 changes: 79 additions & 0 deletions src/components/TappableCopy.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
import React from 'react';
import {View, Pressable, Linking} from 'react-native';
import PropTypes from 'prop-types';
import styles from '../styles/styles';
import compose from '../libs/compose';
import {Checkmark, Clipboard as ClipboardIcon} from './Icon/Expensicons';
import Clipboard from '../libs/Clipboard';
import ReportActionContextMenuItem from '../pages/home/report/ReportActionContextMenuItem';
import withLocalize, {withLocalizePropTypes} from './withLocalize';
import withWindowDimensions, {windowDimensionsPropTypes} from './withWindowDimensions';

const propTypes = {
/** Children to wrap in TappableCopy. */
children: PropTypes.node.isRequired,

/** Styles to be assigned to Container */
style: PropTypes.arrayOf(PropTypes.object),
parasharrajat marked this conversation as resolved.
Show resolved Hide resolved

/** Decides Tap behaviour. */
type: PropTypes.oneOf(['phone', 'email']),
parasharrajat marked this conversation as resolved.
Show resolved Hide resolved

/** Value to be copied or passed via tap. */
value: PropTypes.string.isRequired,

...windowDimensionsPropTypes,
...withLocalizePropTypes,
};

const defaultProps = {
style: [],
type: undefined,
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't quite understand this. When are we using this component and why would it not have a type? It looks like if there's no type we just return an empty View. 🤔

Can we maybe give this a better name than TappableCopy? It looks like it has a pretty specific purpose i.e. wrap anything (I don't see anything that limits this to "copy") in a Pressable and then do an email or phone link or generically copy the text to clipboard.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe something like CommunicationsLink

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure.

Copy link
Contributor

Choose a reason for hiding this comment

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

There were two points to address so I'm reopening. Please allow reviewers to resolve comments.

};

const TappableCopy = props => (
<View style={[styles.flexRow, styles.pRelative, ...props.style]}>
{props.type && props.isSmallScreenWidth
chiragsalian marked this conversation as resolved.
Show resolved Hide resolved
? (
<Pressable
onPress={() => Linking.openURL(
props.type === 'phone'
parasharrajat marked this conversation as resolved.
Show resolved Hide resolved
? `tel:${props.value}`
: `mailto:${props.value}`,
)}
>
{props.children}
</Pressable>
)
: props.children}
{props.type && !props.isSmallScreenWidth
&& (
<View style={[
styles.pAbsolute,
styles.alignItemsCenter,
styles.justifyContentCenter,
{right: -36, top: 0, bottom: 0}]}
parasharrajat marked this conversation as resolved.
Show resolved Hide resolved
>
<ReportActionContextMenuItem
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems incorrect to re-use a ReportActionContextMenuItem here, no?

This is not inside the ReportActionContextMenu 🤔

Copy link
Member Author

@parasharrajat parasharrajat Jul 7, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah. I think we should rename it as we need exactly the same behaviour. Any name suggestions?

Copy link
Contributor

Choose a reason for hiding this comment

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

i would vote to rename it to ContextMenuItem

icon={ClipboardIcon}
text={props.translate('reportActionContextMenu.copyToClipboard')}
successIcon={Checkmark}
successText={props.translate('reportActionContextMenu.copied')}
parasharrajat marked this conversation as resolved.
Show resolved Hide resolved
isMini
autoReset
onPress={() => Clipboard.setString(props.value)}
/>
</View>
)}
</View>
);


parasharrajat marked this conversation as resolved.
Show resolved Hide resolved
TappableCopy.propTypes = propTypes;
TappableCopy.defaultProps = defaultProps;
TappableCopy.displayName = 'TappableCopy';

export default compose(
withWindowDimensions,
withLocalize,
)(TappableCopy);
37 changes: 24 additions & 13 deletions src/pages/DetailsPage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
import React from 'react';
import {
View,
} from 'react-native';
import {View} from 'react-native';
import PropTypes from 'prop-types';
import {withOnyx} from 'react-native-onyx';
import Str from 'expensify-common/lib/str';
Expand All @@ -16,6 +14,7 @@ import ScreenWrapper from '../components/ScreenWrapper';
import personalDetailsPropType from './personalDetailsPropType';
import withLocalize, {withLocalizePropTypes} from '../components/withLocalize';
import compose from '../libs/compose';
import TappableCopy from '../components/TappableCopy';

const matchType = PropTypes.shape({
params: PropTypes.shape({
Expand Down Expand Up @@ -69,6 +68,7 @@ const DetailsPage = ({
const timezone = moment().tz(details.timezone.selected);
const GMTTime = `${timezone.toString().split(/[+-]/)[0].slice(-3)} ${timezone.zoneAbbr()}`;
const currentTime = Number.isNaN(Number(timezone.zoneAbbr())) ? timezone.zoneAbbr() : GMTTime;

return (
<ScreenWrapper>
<HeaderWithCloseButton
Expand All @@ -91,23 +91,34 @@ const DetailsPage = ({
imageStyles={[styles.avatarLarge]}
source={details.avatar}
/>
<Text style={[styles.displayName, styles.mt1, styles.mb6]} numberOfLines={1}>
{details.displayName && isSMSLogin
? toLocalPhone(details.displayName)
: (details.displayName || null)}
</Text>
<TappableCopy
style={[styles.mt1, styles.mb6]}
type={details.displayName && isSMSLogin ? 'phone' : undefined}
parasharrajat marked this conversation as resolved.
Show resolved Hide resolved
value={getPhoneNumber(details)}
>
<Text style={[styles.displayName]} numberOfLines={1}>
{details.displayName && isSMSLogin
? toLocalPhone(details.displayName)
: (details.displayName || null)}
</Text>
</TappableCopy>
{details.login ? (
<View style={[styles.mb6, styles.detailsPageSectionContainer]}>
<Text style={[styles.formLabel, styles.mb2]} numberOfLines={1}>
{translate(isSMSLogin
? 'common.phoneNumber'
: 'common.email')}
</Text>
<Text style={[styles.textP]} numberOfLines={1}>
{isSMSLogin
? toLocalPhone(getPhoneNumber(details))
: details.login}
</Text>
<TappableCopy
type={isSMSLogin ? 'phone' : 'email'}
parasharrajat marked this conversation as resolved.
Show resolved Hide resolved
value={isSMSLogin ? getPhoneNumber(details) : details.login}
>
<Text style={[styles.textP]} numberOfLines={1}>
{isSMSLogin
? toLocalPhone(getPhoneNumber(details))
: details.login}
</Text>
</TappableCopy>
</View>
) : null}
{details.pronouns ? (
Expand Down
13 changes: 13 additions & 0 deletions src/pages/home/report/ReportActionContextMenuItem.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,16 @@ const propTypes = {

/** Callback to fire when the item is pressed */
onPress: PropTypes.func.isRequired,

/** Automatically reset the success status */
autoReset: PropTypes.bool,
};

const defaultProps = {
isMini: false,
successIcon: null,
successText: '',
autoReset: false,
};

class ReportActionContextMenuItem extends Component {
Expand All @@ -42,6 +46,12 @@ class ReportActionContextMenuItem extends Component {
this.triggerPressAndUpdateSuccess = this.triggerPressAndUpdateSuccess.bind(this);
}

componentWillUnmount() {
if (this.successResetTimer) {
clearTimeout(this.successResetTimer);
}
}

/**
* Called on button press and mark the run
*/
Expand All @@ -57,6 +67,9 @@ class ReportActionContextMenuItem extends Component {
this.setState({
success: true,
});
if (this.props.autoReset) {
this.successResetTimer = setTimeout(() => this.setState({success: false}), 1800);
}
}
}

Expand Down