-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
feat: use pressable with press feedback #19391
Changes from 8 commits
ad3cec2
0fd1221
b0548a8
43c1e4d
15572ba
152daf1
c07527c
4678f93
e68fc31
db1dde0
5bda66e
6488188
5e98507
ef3cb4f
ba6db16
67758d7
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 |
---|---|---|
@@ -1,18 +1,9 @@ | ||
import React from 'react'; | ||
import {Pressable} from 'react-native'; | ||
import PropTypes from 'prop-types'; | ||
import Text from './Text'; | ||
import * as Expensicons from './Icon/Expensicons'; | ||
import compose from '../libs/compose'; | ||
import PressableWithDelayToggle from './PressableWithDelayToggle'; | ||
import Clipboard from '../libs/Clipboard'; | ||
import getButtonState from '../libs/getButtonState'; | ||
import Icon from './Icon'; | ||
import Tooltip from './Tooltip'; | ||
import styles from '../styles/styles'; | ||
import * as StyleUtils from '../styles/StyleUtils'; | ||
import variables from '../styles/variables'; | ||
import withLocalize, {withLocalizePropTypes} from './withLocalize'; | ||
import withDelayToggleButtonState, {withDelayToggleButtonStatePropTypes} from './withDelayToggleButtonState'; | ||
|
||
const propTypes = { | ||
/** The text to display and copy to the clipboard */ | ||
|
@@ -23,8 +14,6 @@ const propTypes = { | |
textStyles: PropTypes.arrayOf(PropTypes.object), | ||
|
||
...withLocalizePropTypes, | ||
|
||
...withDelayToggleButtonStatePropTypes, | ||
}; | ||
|
||
const defaultProps = { | ||
|
@@ -39,40 +28,24 @@ class CopyTextToClipboard extends React.Component { | |
} | ||
|
||
copyToClipboard() { | ||
if (this.props.isDelayButtonStateComplete) { | ||
return; | ||
} | ||
Clipboard.setString(this.props.text); | ||
this.props.toggleDelayButtonState(true); | ||
} | ||
|
||
render() { | ||
return ( | ||
<Text | ||
<PressableWithDelayToggle | ||
text={this.props.text} | ||
tooltipText={this.props.translate('reportActionContextMenu.copyToClipboard')} | ||
tooltipTextChecked={this.props.translate('reportActionContextMenu.copied')} | ||
icon={Expensicons.Copy} | ||
textStyles={this.props.textStyles} | ||
onPress={this.copyToClipboard} | ||
style={[styles.flexRow, styles.cursorPointer]} | ||
suppressHighlighting | ||
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. Coming from #24591 We are missing the prop |
||
> | ||
<Text style={this.props.textStyles}>{`${this.props.text} `}</Text> | ||
<Tooltip text={this.props.translate(`reportActionContextMenu.${this.props.isDelayButtonStateComplete ? 'copied' : 'copyToClipboard'}`)}> | ||
<Pressable onPress={this.copyToClipboard}> | ||
{({hovered, pressed}) => ( | ||
<Icon | ||
src={this.props.isDelayButtonStateComplete ? Expensicons.Checkmark : Expensicons.Copy} | ||
fill={StyleUtils.getIconFillColor(getButtonState(hovered, pressed, this.props.isDelayButtonStateComplete))} | ||
width={variables.iconSizeSmall} | ||
height={variables.iconSizeSmall} | ||
inline | ||
/> | ||
)} | ||
</Pressable> | ||
</Tooltip> | ||
</Text> | ||
/> | ||
); | ||
} | ||
} | ||
|
||
CopyTextToClipboard.propTypes = propTypes; | ||
CopyTextToClipboard.defaultProps = defaultProps; | ||
|
||
export default compose(withLocalize, withDelayToggleButtonState)(CopyTextToClipboard); | ||
export default withLocalize(CopyTextToClipboard); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,139 @@ | ||
import React from 'react'; | ||
import PropTypes from 'prop-types'; | ||
import {Pressable} from 'react-native'; | ||
import * as Expensicons from './Icon/Expensicons'; | ||
import compose from '../libs/compose'; | ||
import Icon from './Icon'; | ||
import Tooltip from './Tooltip'; | ||
import Text from './Text'; | ||
import styles from '../styles/styles'; | ||
import variables from '../styles/variables'; | ||
import getButtonState from '../libs/getButtonState'; | ||
import * as StyleUtils from '../styles/StyleUtils'; | ||
import withLocalize, {withLocalizePropTypes} from './withLocalize'; | ||
import withDelayToggleButtonState, {withDelayToggleButtonStatePropTypes} from './withDelayToggleButtonState'; | ||
|
||
const propTypes = { | ||
/** The text to display */ | ||
text: PropTypes.string, | ||
|
||
/** The text to display once the pressable is pressed */ | ||
textChecked: PropTypes.string, | ||
|
||
/** The tooltip text to display */ | ||
tooltipText: PropTypes.string, | ||
|
||
/** The tooltip text to display once the pressable is pressed */ | ||
tooltipTextChecked: PropTypes.string, | ||
|
||
/** Styles to apply to the container */ | ||
// eslint-disable-next-line react/forbid-prop-types | ||
styles: PropTypes.arrayOf(PropTypes.object), | ||
|
||
/** Styles to apply to the text */ | ||
// eslint-disable-next-line react/forbid-prop-types | ||
textStyles: PropTypes.arrayOf(PropTypes.object), | ||
|
||
/** Styles to apply to the icon */ | ||
// eslint-disable-next-line react/forbid-prop-types | ||
iconStyles: PropTypes.arrayOf(PropTypes.object), | ||
|
||
/** Callback to be called on onPress */ | ||
onPress: PropTypes.func.isRequired, | ||
|
||
/** The icon to display */ | ||
icon: PropTypes.func, | ||
|
||
/** The icon to display once the pressable is pressed */ | ||
iconChecked: PropTypes.func, | ||
|
||
/** If the component should be inline with text or not */ | ||
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 comment should describe when / why would we use this prop |
||
inline: PropTypes.bool, | ||
|
||
...withLocalizePropTypes, | ||
|
||
...withDelayToggleButtonStatePropTypes, | ||
BeeMargarida marked this conversation as resolved.
Show resolved
Hide resolved
|
||
}; | ||
|
||
const defaultProps = { | ||
text: '', | ||
textChecked: '', | ||
tooltipText: '', | ||
tooltipTextChecked: '', | ||
styles: [], | ||
textStyles: [], | ||
iconStyles: [], | ||
icon: null, | ||
inline: true, | ||
iconChecked: Expensicons.Checkmark, | ||
}; | ||
|
||
function PressableWithDelayToggle(props) { | ||
const updatePressState = () => { | ||
if (props.isDelayButtonStateComplete) { | ||
return; | ||
} | ||
props.toggleDelayButtonState(true); | ||
BeeMargarida marked this conversation as resolved.
Show resolved
Hide resolved
|
||
props.onPress(); | ||
}; | ||
|
||
// Due to limitations in RN regarding the vertical text alignment of non-Text elements, | ||
// for elements that are supposed to be inline, we need to use a Text element instead | ||
// of a Pressable | ||
const PressableView = props.inline ? Text : Pressable; | ||
|
||
return ( | ||
<PressableView | ||
style={[...props.styles, styles.flexRow]} | ||
BeeMargarida marked this conversation as resolved.
Show resolved
Hide resolved
|
||
onPress={updatePressState} | ||
> | ||
<Tooltip | ||
containerStyles={styles.flexRow} | ||
text={props.isDelayButtonStateComplete ? props.tooltipTextChecked : props.tooltipText} | ||
> | ||
<Text | ||
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. There is a console error caused by this line - ![]()
|
||
suppressHighlighting | ||
style={[...props.textStyles, styles.mr1]} | ||
BeeMargarida marked this conversation as resolved.
Show resolved
Hide resolved
|
||
> | ||
{props.isDelayButtonStateComplete && props.textChecked ? props.textChecked : props.text} | ||
</Text> | ||
<Pressable | ||
ref={props.innerRef} | ||
focusable | ||
accessibilityLabel={props.isDelayButtonStateComplete ? props.tooltipTextChecked : props.tooltipText} | ||
onPress={updatePressState} | ||
> | ||
{({hovered, pressed}) => ( | ||
<> | ||
{props.icon && ( | ||
<Icon | ||
src={props.isDelayButtonStateComplete ? props.iconChecked : props.icon} | ||
fill={StyleUtils.getIconFillColor(getButtonState(hovered, pressed, props.isDelayButtonStateComplete))} | ||
style={props.iconStyles} | ||
width={variables.iconSizeSmall} | ||
height={variables.iconSizeSmall} | ||
/> | ||
)} | ||
</> | ||
)} | ||
</Pressable> | ||
</Tooltip> | ||
</PressableView> | ||
); | ||
} | ||
|
||
PressableWithDelayToggle.propTypes = propTypes; | ||
PressableWithDelayToggle.defaultProps = defaultProps; | ||
|
||
export default compose( | ||
withLocalize, | ||
withDelayToggleButtonState, | ||
)( | ||
React.forwardRef((props, ref) => ( | ||
<PressableWithDelayToggle | ||
// eslint-disable-next-line react/jsx-props-no-spreading | ||
{...props} | ||
innerRef={ref} | ||
/> | ||
)), | ||
); |
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.
Looks like this has caused issues in such button components
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.
@MonilBhavsar Seems like the left icon styles were also being applied to the right icon. Fixed ✅