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.

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

Added Tooltips to the report users' titles. #1632

Merged
merged 22 commits into from
Mar 19, 2021
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
cf86c6f
started Tooltips
parasharrajat Mar 4, 2021
815daf4
Merge remote-tracking branch 'up/master' into parasharrajat/tooltip
parasharrajat Mar 4, 2021
21fe2e0
New: added tooltip to various parts of the app
parasharrajat Mar 5, 2021
8dece9d
added ellipsis detection
parasharrajat Mar 5, 2021
f8f6e98
new: Tooltip position for Movable elments
parasharrajat Mar 11, 2021
9c60e2b
added tooltip for various parts of the app
parasharrajat Mar 11, 2021
f798513
new: tooltip comments & linting
parasharrajat Mar 11, 2021
de5bbbc
Merge branch 'master' into parasharrajat/tooltip
parasharrajat Mar 11, 2021
b34457c
fix: Report is option in headerView
parasharrajat Mar 11, 2021
134367b
chg: refactor
parasharrajat Mar 11, 2021
9b85137
fix: styling of tooltip
parasharrajat Mar 11, 2021
e204a20
fix: Added tooltip prop & ecllipse only on multiple particpants
parasharrajat Mar 11, 2021
9900d03
new: added tooltip support for report Header
parasharrajat Mar 12, 2021
6ca2c49
fix: tooltip pointer fix
parasharrajat Mar 12, 2021
6d1d6af
Merge remote-tracking branch 'up/master' into parasharrajat/tooltip
parasharrajat Mar 12, 2021
951f971
Merge remote-tracking branch 'up/master' into parasharrajat/tooltip
parasharrajat Mar 12, 2021
a63c24c
Merge branch 'master' into parasharrajat/tooltip
parasharrajat Mar 15, 2021
3a9df36
fix: linting issues and refactor code
parasharrajat Mar 16, 2021
21b51ee
refactor Code && showtooltip glitch
parasharrajat Mar 16, 2021
f4ef0cd
Merge remote-tracking branch 'up/master' into parasharrajat/tooltip
parasharrajat Mar 18, 2021
237968c
removed unnecessary line
parasharrajat Mar 19, 2021
996ae0e
refactor
parasharrajat Mar 19, 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
3 changes: 3 additions & 0 deletions src/components/Hoverable/HoverablePropTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ const propTypes = {
PropTypes.func,
]).isRequired,

// Styles to be assigned to the Hoverable Container
containerStyle: PropTypes.object,
parasharrajat marked this conversation as resolved.
Show resolved Hide resolved
parasharrajat marked this conversation as resolved.
Show resolved Hide resolved

// Function that executes when the mouse moves over the children.
onHoverIn: PropTypes.func,

Expand Down
1 change: 1 addition & 0 deletions src/components/Hoverable/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ class Hoverable extends Component {
render() {
return (
<View
style={this.props.containerStyle}
onMouseEnter={this.toggleHoverState}
onMouseLeave={this.toggleHoverState}
>
Expand Down
26 changes: 26 additions & 0 deletions src/components/Tooltip/TooltipPropTypes.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import PropTypes from 'prop-types';
import {windowDimensionsPropTypes} from '../withWindowDimensions';

const propTypes = {
// The text to display in the tooltip.
text: PropTypes.string.isRequired,

// Styles to be assigned to the Tooltip wrapper views
containerStyle: PropTypes.object,

// Children to wrap with Tooltip.
children: PropTypes.node.isRequired,

// Props inherited from withWindowDimensions
...windowDimensionsPropTypes,

// Any additional amount to manually adjust the horizontal position of the tooltip.
// A positive value shifts the tooltip to the right, and a negative value shifts it to the left.
shiftHorizontal: PropTypes.oneOfType([PropTypes.number, PropTypes.func]),

// Any additional amount to manually adjust the vertical position of the tooltip.
// A positive value shifts the tooltip down, and a negative value shifts it up.
shiftVertical: PropTypes.oneOfType([PropTypes.number, PropTypes.func]),
};

export default propTypes;
parasharrajat marked this conversation as resolved.
Show resolved Hide resolved
109 changes: 59 additions & 50 deletions src/components/Tooltip.js → src/components/Tooltip/index.js
Original file line number Diff line number Diff line change
@@ -1,28 +1,10 @@
import React, {PureComponent} from 'react';
import PropTypes from 'prop-types';
import {Animated, Text, View} from 'react-native';
import Hoverable from './Hoverable';
import withWindowDimensions, {windowDimensionsPropTypes} from './withWindowDimensions';
import getTooltipStyles from '../styles/getTooltipStyles';

const propTypes = {
// The text to display in the tooltip.
text: PropTypes.string.isRequired,

// Children to wrap with Tooltip.
children: PropTypes.node.isRequired,

// Props inherited from withWindowDimensions
...windowDimensionsPropTypes,

// Any additional amount to manually adjust the horizontal position of the tooltip.
// A positive value shifts the tooltip to the right, and a negative value shifts it to the left.
shiftHorizontal: PropTypes.number,

// Any additional amount to manually adjust the vertical position of the tooltip.
// A positive value shifts the tooltip down, and a negative value shifts it up.
shiftVertical: PropTypes.number,
};
import ReactDOM from 'react-dom';
import Hoverable from '../Hoverable';
import withWindowDimensions from '../withWindowDimensions';
import getTooltipStyles from '../../styles/getTooltipStyles';
import tooltipPropTypes from './TooltipPropTypes';

const defaultProps = {
shiftHorizontal: 0,
Expand Down Expand Up @@ -62,6 +44,7 @@ class Tooltip extends PureComponent {
this.measureTooltip = this.measureTooltip.bind(this);
this.showTooltip = this.showTooltip.bind(this);
this.hideTooltip = this.hideTooltip.bind(this);
this.createPortal = this.createPortal.bind(this);
}

componentDidUpdate(prevProps) {
Expand All @@ -79,7 +62,11 @@ class Tooltip extends PureComponent {
* @returns {Promise}
*/
getWrapperPosition() {
return new Promise((resolve => this.wrapperView.measureInWindow((x, y) => resolve({x, y}))));
return new Promise((resolve => this.wrapperView.measureInWindow(
(x, y, width, height) => resolve({
x, y, width, height,
}),
)));
}

/**
Expand Down Expand Up @@ -117,6 +104,17 @@ class Tooltip extends PureComponent {
* Display the tooltip in an animation.
*/
showTooltip() {
// We have to dynamically calculate the position here as tooltip could have been rendered on some elments
// that has changed its position
this.getWrapperPosition()
.then(({
x, y, width, height,
}) => this.setState({
wrapperWidth: width,
wrapperHeight: height,
xOffset: x,
yOffset: y,
}));
Animated.timing(this.animation, {
parasharrajat marked this conversation as resolved.
Show resolved Hide resolved
toValue: 1,
duration: 140,
Expand All @@ -133,7 +131,7 @@ class Tooltip extends PureComponent {
}).start();
}

render() {
createPortal() {
parasharrajat marked this conversation as resolved.
Show resolved Hide resolved
const {
animationStyle,
tooltipWrapperStyle,
Expand All @@ -149,38 +147,49 @@ class Tooltip extends PureComponent {
this.state.wrapperHeight,
this.state.tooltipWidth,
this.state.tooltipHeight,
this.props.shiftHorizontal,
this.props.shiftVertical,
typeof this.props.shiftHorizontal === 'function'
parasharrajat marked this conversation as resolved.
Show resolved Hide resolved
? this.props.shiftHorizontal()
: this.props.shiftHorizontal,
typeof this.props.shiftVertical === 'function'
parasharrajat marked this conversation as resolved.
Show resolved Hide resolved
? this.props.shiftVertical()
: this.props.shiftVertical,
);
return ReactDOM.createPortal(
parasharrajat marked this conversation as resolved.
Show resolved Hide resolved
<Animated.View
ref={el => this.tooltip = el}
onLayout={this.measureTooltip}
style={{...tooltipWrapperStyle, ...animationStyle}}
parasharrajat marked this conversation as resolved.
Show resolved Hide resolved
>
<Text style={tooltipTextStyle} numberOfLines={1}>{this.props.text}</Text>
<View style={pointerWrapperStyle}>
<View style={pointerStyle} />
</View>
</Animated.View>, document.querySelector('body'),
parasharrajat marked this conversation as resolved.
Show resolved Hide resolved
);
}

render() {
return (
<Hoverable
onHoverIn={this.showTooltip}
onHoverOut={this.hideTooltip}
>
<View
ref={el => this.wrapperView = el}
onLayout={this.measureWrapperAndGetPosition}
<>
{this.createPortal()}
parasharrajat marked this conversation as resolved.
Show resolved Hide resolved
<Hoverable
containerStyle={this.props.containerStyle}
onHoverIn={this.showTooltip}
onHoverOut={this.hideTooltip}
>
<Animated.View style={animationStyle}>
<View
ref={el => this.tooltip = el}
onLayout={this.measureTooltip}
style={tooltipWrapperStyle}
>
<Text style={tooltipTextStyle} numberOfLines={1}>{this.props.text}</Text>
</View>
<View style={pointerWrapperStyle}>
<View style={pointerStyle} />
</View>
</Animated.View>
{this.props.children}
</View>
</Hoverable>
<View
ref={el => this.wrapperView = el}
onLayout={this.measureWrapperAndGetPosition}
style={this.props.containerStyle}
>
{this.props.children}
</View>
</Hoverable>
</>
);
}
}

Tooltip.propTypes = propTypes;
Tooltip.propTypes = tooltipPropTypes;
Tooltip.defaultProps = defaultProps;
export default withWindowDimensions(Tooltip);
12 changes: 12 additions & 0 deletions src/components/Tooltip/index.native.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import tooltipPropTypes from './TooltipPropTypes';

const defaultProps = {
shiftHorizontal: 0,
shiftVertical: 0,
};

parasharrajat marked this conversation as resolved.
Show resolved Hide resolved
const Tooltip = props => props.children;

Tooltip.propTypes = tooltipPropTypes;
Tooltip.defaultProps = defaultProps;
parasharrajat marked this conversation as resolved.
Show resolved Hide resolved
export default Tooltip;
4 changes: 4 additions & 0 deletions src/libs/OptionsListUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import Str from 'expensify-common/lib/str';
import {getDefaultAvatar} from './actions/PersonalDetails';
import ONYXKEYS from '../ONYXKEYS';
import CONST from '../CONST';
import {getReportParticipantsTitle} from './reportUtils';

/**
* OptionsListUtils is used to build a list options passed to the OptionsList component. Several different UI views can
Expand Down Expand Up @@ -91,11 +92,14 @@ function createOption(personalDetailList, report, draftComments, activeReportID,
: '')
+ _.unescape(report.lastMessageText)
: '';
const tooltipText = getReportParticipantsTitle(lodashGet(report, ['participants'], []));
parasharrajat marked this conversation as resolved.
Show resolved Hide resolved

return {
text: report ? report.reportName : personalDetail.displayName,
alternateText: (showChatPreviewLine && lastMessageText) ? lastMessageText : personalDetail.login,
icons: report ? report.icons : [personalDetail.avatar],
tooltipText,
participantsList: personalDetailList,
parasharrajat marked this conversation as resolved.
Show resolved Hide resolved

// It doesn't make sense to provide a login in the case of a report with multiple participants since
// there isn't any one single login to refer to for a report.
Expand Down
12 changes: 12 additions & 0 deletions src/libs/hasEllipsis/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// eslint-disable-next-line valid-jsdoc
parasharrajat marked this conversation as resolved.
Show resolved Hide resolved
/**
* Does an elment has ellipsis
parasharrajat marked this conversation as resolved.
Show resolved Hide resolved
*
* @param {*} el
* @returns
parasharrajat marked this conversation as resolved.
Show resolved Hide resolved
*/
function hasEllipsis(el) {
return el.offsetWidth < el.scrollWidth;
}

export default hasEllipsis;
1 change: 1 addition & 0 deletions src/libs/hasEllipsis/index.native.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export default () => {};
17 changes: 17 additions & 0 deletions src/libs/reportUtils.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import _ from 'underscore';
import Str from 'expensify-common/lib/str';

/**
* Returns the concatenated title for the PrimaryLogins of a report
*
* @param {Array} logins
* @returns {string}
*/
function getReportParticipantsTitle(logins) {
return _.map(logins, login => Str.removeSMSDomain(login)).join(',');
parasharrajat marked this conversation as resolved.
Show resolved Hide resolved
}

export {
// eslint-disable-next-line import/prefer-default-export
getReportParticipantsTitle,
};
88 changes: 51 additions & 37 deletions src/pages/home/HeaderView.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import withWindowDimensions, {windowDimensionsPropTypes} from '../../components/
import MultipleAvatars from '../../components/MultipleAvatars';
import {redirect} from '../../libs/actions/App';
import ROUTES from '../../ROUTES';
import Tooltip from '../../components/Tooltip';
import {getReportParticipantsTitle} from '../../libs/reportUtils';

const propTypes = {
// Toggles the navigationMenu open and closed
Expand All @@ -25,6 +27,9 @@ const propTypes = {
// Name of the report
reportName: PropTypes.string,

// list of primarylogins of participants of the report
participants: PropTypes.arrayOf(PropTypes.string),

// ID of the report
reportID: PropTypes.number,

Expand All @@ -39,51 +44,60 @@ const defaultProps = {
report: null,
};

const HeaderView = props => (
<View style={[styles.appContentHeader]} nativeID="drag-area">
<View style={[styles.appContentHeaderTitle, !props.isSmallScreenWidth && styles.pl5]}>
{props.isSmallScreenWidth && (
<Pressable
onPress={props.onNavigationMenuButtonClicked}
style={[styles.LHNToggle]}
>
<Icon src={BackArrow} />
</Pressable>
)}
{props.report && props.report.reportName ? (
<View
style={[
styles.flex1,
styles.flexRow,
styles.alignItemsCenter,
styles.justifyContentBetween,
]}
>
const HeaderView = (props) => {
const participantsTitle = props.report && props.report.participants
? getReportParticipantsTitle(props.report.participants)
: '';

return (
<View style={[styles.appContentHeader]} nativeID="drag-area">
<View style={[styles.appContentHeaderTitle, !props.isSmallScreenWidth && styles.pl5]}>
{props.isSmallScreenWidth && (
<Pressable
onPress={() => {
const {participants} = props.report;
if (participants.length === 1) {
redirect(ROUTES.getProfileRoute(participants[0]));
}
}}
onPress={props.onNavigationMenuButtonClicked}
style={[styles.LHNToggle]}
>
<MultipleAvatars avatarImageURLs={props.report.icons} />
<Icon src={BackArrow} />
</Pressable>
<Header title={props.report.reportName} />
<View style={[styles.reportOptions, styles.flexRow]}>
)}
{props.report && props.report.reportName ? (
<View
style={[
styles.flex1,
Copy link
Contributor

Choose a reason for hiding this comment

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

NAB maybe rather than composing 4 styles into a new one here you could add a new style that composes these four, like flexRowCenterChildren. Up to you though. I personally think it's better not to compose too many styles inline, because it's equivalent to creating a new style, and we want our JSX to have as little style logic as possible in general.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm. I actually prefer is granular. All four of the classes are generic and it makes sense to create a class if we use all of them together very often. But I prefer tailwind-styled code. Let's see what @tgolen has to say here.

styles.flexRow,
styles.alignItemsCenter,
styles.justifyContentBetween,
]}
>
<Pressable
onPress={() => togglePinnedState(props.report)}
style={[styles.touchableButtonImage, styles.mr0]}
onPress={() => {
const {participants} = props.report;
if (participants.length === 1) {
redirect(ROUTES.getProfileRoute(participants[0]));
}
}}
>
<Icon src={Pin} fill={props.report.isPinned ? themeColors.heading : themeColors.icon} />
<MultipleAvatars avatarImageURLs={props.report.icons} />
</Pressable>
<View style={[styles.flex1, styles.flexRow]}>
<Tooltip text={participantsTitle}>
<Header title={props.report.reportName} />
</Tooltip>
</View>
<View style={[styles.reportOptions, styles.flexRow]}>
<Pressable
onPress={() => togglePinnedState(props.report)}
style={[styles.touchableButtonImage, styles.mr0]}
>
<Icon src={Pin} fill={props.report.isPinned ? themeColors.heading : themeColors.icon} />
</Pressable>
</View>
</View>
</View>
) : null}
) : null}
parasharrajat marked this conversation as resolved.
Show resolved Hide resolved
</View>
</View>
</View>
);

);
};
HeaderView.propTypes = propTypes;
HeaderView.displayName = 'HeaderView';
HeaderView.defaultProps = defaultProps;
Expand Down
Loading