From cf86c6f6b2c00415857da8daf832bc94ad3e2d7b Mon Sep 17 00:00:00 2001 From: Rajat Parashar Date: Thu, 4 Mar 2021 19:58:42 +0530 Subject: [PATCH 01/16] started Tooltips --- src/components/Header.js | 1 + .../Hoverable/HoverablePropTypes.js | 3 + src/components/Hoverable/index.js | 1 + src/components/Tooltip/TooltipPropTypes.js | 26 +++++ .../{Tooltip.js => Tooltip/index.js} | 100 ++++++++++-------- src/components/Tooltip/index.native.js | 12 +++ src/libs/OptionsListUtils.js | 3 + src/libs/reportUtils.js | 17 +++ src/pages/home/HeaderView.js | 80 ++++++++------ src/pages/home/ReportScreen.js | 2 +- .../home/report/ReportActionItemFragment.js | 21 ++-- .../home/report/ReportActionItemSingle.js | 2 + src/pages/home/sidebar/OptionRow.js | 12 ++- src/styles/getTooltipStyles.js | 15 +-- 14 files changed, 198 insertions(+), 97 deletions(-) create mode 100644 src/components/Tooltip/TooltipPropTypes.js rename src/components/{Tooltip.js => Tooltip/index.js} (67%) create mode 100644 src/components/Tooltip/index.native.js create mode 100644 src/libs/reportUtils.js diff --git a/src/components/Header.js b/src/components/Header.js index 6795601c3367..ed3123458c35 100644 --- a/src/components/Header.js +++ b/src/components/Header.js @@ -3,6 +3,7 @@ import {View} from 'react-native'; import PropTypes from 'prop-types'; import styles from '../styles/styles'; import Text from './Text'; +import Tooltip from './Tooltip'; const propTypes = { /** Title of the Header */ diff --git a/src/components/Hoverable/HoverablePropTypes.js b/src/components/Hoverable/HoverablePropTypes.js index 2d2842be19c5..04d462b42637 100644 --- a/src/components/Hoverable/HoverablePropTypes.js +++ b/src/components/Hoverable/HoverablePropTypes.js @@ -7,6 +7,9 @@ const propTypes = { PropTypes.func, ]).isRequired, + // Styles to be assigned to the Hoverable Container + containerStyle: PropTypes.object, + // Function that executes when the mouse moves over the children. onHoverIn: PropTypes.func, diff --git a/src/components/Hoverable/index.js b/src/components/Hoverable/index.js index d6a96a45873e..e467dfc3619b 100644 --- a/src/components/Hoverable/index.js +++ b/src/components/Hoverable/index.js @@ -33,6 +33,7 @@ class Hoverable extends Component { render() { return ( diff --git a/src/components/Tooltip/TooltipPropTypes.js b/src/components/Tooltip/TooltipPropTypes.js new file mode 100644 index 000000000000..2d19f1b12355 --- /dev/null +++ b/src/components/Tooltip/TooltipPropTypes.js @@ -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.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, +}; + +export default propTypes; diff --git a/src/components/Tooltip.js b/src/components/Tooltip/index.js similarity index 67% rename from src/components/Tooltip.js rename to src/components/Tooltip/index.js index 48492bcd0a91..86c1f058432d 100644 --- a/src/components/Tooltip.js +++ b/src/components/Tooltip/index.js @@ -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, @@ -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) { @@ -88,17 +71,26 @@ class Tooltip extends PureComponent { * @param {Object} nativeEvent */ measureWrapperAndGetPosition({nativeEvent}) { - const {width, height} = nativeEvent.layout; + const { + left, top, width, height, + } = nativeEvent.layout; + console.debug(left, top, nativeEvent); + this.setState({ + wrapperWidth: width, + wrapperHeight: height, + xOffset: left, + yOffset: top, + }); // We need to use `measureInWindow` instead of the layout props provided by `onLayout` // because `measureInWindow` provides the x and y offset relative to the window, rather than the parent element. - this.getWrapperPosition() - .then(({x, y}) => this.setState({ - wrapperWidth: width, - wrapperHeight: height, - xOffset: x, - yOffset: y, - })); + // this.getWrapperPosition() + // .then(({x, y}) => this.setState({ + // wrapperWidth: width, + // wrapperHeight: height, + // xOffset: x, + // yOffset: y, + // })); } /** @@ -133,7 +125,7 @@ class Tooltip extends PureComponent { }).start(); } - render() { + createPortal() { const { animationStyle, tooltipWrapperStyle, @@ -152,17 +144,34 @@ class Tooltip extends PureComponent { this.props.shiftHorizontal, this.props.shiftVertical, ); + return ReactDOM.createPortal( + this.tooltip = el} + onLayout={this.measureTooltip} + style={{...tooltipWrapperStyle, ...animationStyle}} + > + {this.props.text} + + + + , document.querySelector('body'), + ); + } + render() { return ( - - this.wrapperView = el} - onLayout={this.measureWrapperAndGetPosition} + <> + {this.createPortal()} + - + this.wrapperView = el} + onLayout={this.measureWrapperAndGetPosition} + > + {/* this.tooltip = el} onLayout={this.measureTooltip} @@ -173,14 +182,15 @@ class Tooltip extends PureComponent { - - {this.props.children} - - + */} + {this.props.children} + + + ); } } -Tooltip.propTypes = propTypes; +Tooltip.propTypes = tooltipPropTypes; Tooltip.defaultProps = defaultProps; export default withWindowDimensions(Tooltip); diff --git a/src/components/Tooltip/index.native.js b/src/components/Tooltip/index.native.js new file mode 100644 index 000000000000..703b61d2ae25 --- /dev/null +++ b/src/components/Tooltip/index.native.js @@ -0,0 +1,12 @@ +import tooltipPropTypes from './TooltipPropTypes'; + +const defaultProps = { + shiftHorizontal: 0, + shiftVertical: 0, +}; + +const Tooltip = () => (this.props.children); + +Tooltip.propTypes = tooltipPropTypes; +Tooltip.defaultProps = defaultProps; +export default Tooltip; diff --git a/src/libs/OptionsListUtils.js b/src/libs/OptionsListUtils.js index 74c0c9b7a849..799306fe3eb9 100644 --- a/src/libs/OptionsListUtils.js +++ b/src/libs/OptionsListUtils.js @@ -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 @@ -91,11 +92,13 @@ function createOption(personalDetailList, report, draftComments, activeReportID, : '') + _.unescape(report.lastMessageText) : ''; + const tooltipText = getReportParticipantsTitle(lodashGet(report, ['participants'], [])); return { text: report ? report.reportName : personalDetail.displayName, alternateText: (showChatPreviewLine && lastMessageText) ? lastMessageText : personalDetail.login, icons: report ? report.icons : [personalDetail.avatarURL], + tooltipText, // 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. diff --git a/src/libs/reportUtils.js b/src/libs/reportUtils.js new file mode 100644 index 000000000000..bff7c0b3522d --- /dev/null +++ b/src/libs/reportUtils.js @@ -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(','); +} + +export { + // eslint-disable-next-line import/prefer-default-export + getReportParticipantsTitle, +}; diff --git a/src/pages/home/HeaderView.js b/src/pages/home/HeaderView.js index f437710c938a..675edf6b7e92 100644 --- a/src/pages/home/HeaderView.js +++ b/src/pages/home/HeaderView.js @@ -1,6 +1,6 @@ import React from 'react'; import {View, Pressable} from 'react-native'; -import PropTypes from 'prop-types'; +import PropTypes, {object} from 'prop-types'; import {withOnyx} from 'react-native-onyx'; import Header from '../../components/Header'; import styles from '../../styles/styles'; @@ -11,6 +11,8 @@ import {BackArrow, Pin} from '../../components/Icon/Expensicons'; import compose from '../../libs/compose'; import {togglePinnedState} from '../../libs/actions/Report'; import withWindowDimensions, {windowDimensionsPropTypes} from '../../components/withWindowDimensions'; +import Tooltip from '../../components/Tooltip'; +import {getReportParticipantsTitle} from '../../libs/reportUtils'; const propTypes = { // Toggles the navigationMenu open and closed @@ -22,6 +24,9 @@ const propTypes = { // Name of the report reportName: PropTypes.string, + // list of primarylogins of participants of the report + participants: PropTypes.arrayOf(PropTypes.object), + // ID of the report reportID: PropTypes.number, @@ -36,41 +41,48 @@ const defaultProps = { report: null, }; -const HeaderView = props => ( - - - {props.isSmallScreenWidth && ( - - - - )} - {props.report && props.report.reportName ? ( - -
- - togglePinnedState(props.report)} - style={[styles.touchableButtonImage, styles.mr0]} - > - - +const HeaderView = (props) => { + const participantsTitle = getReportParticipantsTitle(props.report.participants); + + return ( + + + {props.isSmallScreenWidth && ( + + + + )} + {props.report && props.report.reportName ? ( + + + +
+ + + + togglePinnedState(props.report)} + style={[styles.touchableButtonImage, styles.mr0]} + > + + + - - ) : null} + ) : null} + - -); - + ); +}; HeaderView.propTypes = propTypes; HeaderView.displayName = 'HeaderView'; HeaderView.defaultProps = defaultProps; diff --git a/src/pages/home/ReportScreen.js b/src/pages/home/ReportScreen.js index 61eff320cf0a..ea68d692942f 100644 --- a/src/pages/home/ReportScreen.js +++ b/src/pages/home/ReportScreen.js @@ -62,7 +62,7 @@ class ReportScreen extends Component { const isActiveReport = activeReportID === report.reportID; const finalData = {...memo}; let reportStyle; - + console.debug(report); if (isActiveReport) { activeReportID = report.reportID; reportStyle = [styles.dFlex, styles.flex1]; diff --git a/src/pages/home/report/ReportActionItemFragment.js b/src/pages/home/report/ReportActionItemFragment.js index 16abf1eaa504..a94cfebdb49c 100644 --- a/src/pages/home/report/ReportActionItemFragment.js +++ b/src/pages/home/report/ReportActionItemFragment.js @@ -7,11 +7,15 @@ import styles from '../../../styles/styles'; import themeColors from '../../../styles/themes/default'; import RenderHTML from '../../../components/RenderHTML'; import Text from '../../../components/Text'; +import Tooltip from '../../../components/Tooltip'; const propTypes = { // The message fragment needing to be displayed fragment: ReportActionFragmentPropTypes.isRequired, + // Text to be shown for tooltip When Fragment is report Actor + tooltipText: PropTypes.string, + // Is this fragment an attachment? isAttachment: PropTypes.bool, @@ -22,11 +26,12 @@ const propTypes = { const defaultProps = { isAttachment: false, loading: false, + tooltipText: '', }; class ReportActionItemFragment extends React.PureComponent { render() { - const {fragment} = this.props; + const {fragment, tooltipText} = this.props; switch (fragment.type) { case 'COMMENT': // If this is an attachment placeholder, return the placeholder component @@ -50,12 +55,14 @@ class ReportActionItemFragment extends React.PureComponent { ); case 'TEXT': return ( - - {Str.htmlDecode(fragment.text)} - + + + {Str.htmlDecode(fragment.text)} + + ); case 'LINK': return LINK; diff --git a/src/pages/home/report/ReportActionItemSingle.js b/src/pages/home/report/ReportActionItemSingle.js index ccd9ea1df280..9d330d6aadea 100644 --- a/src/pages/home/report/ReportActionItemSingle.js +++ b/src/pages/home/report/ReportActionItemSingle.js @@ -16,6 +16,7 @@ const propTypes = { }; const ReportActionItemSingle = ({action}) => { + console.debug(action); const avatarUrl = action.automatic ? `${CONST.CLOUDFRONT_URL}/images/icons/concierge_2019.svg` : action.avatar; @@ -31,6 +32,7 @@ const ReportActionItemSingle = ({action}) => { diff --git a/src/pages/home/sidebar/OptionRow.js b/src/pages/home/sidebar/OptionRow.js index cc870f2f65af..c213b806d559 100644 --- a/src/pages/home/sidebar/OptionRow.js +++ b/src/pages/home/sidebar/OptionRow.js @@ -14,6 +14,7 @@ import {Pencil, PinCircle, Checkmark} from '../../../components/Icon/Expensicons import MultipleAvatars from '../../../components/MultipleAvatars'; import themeColors from '../../../styles/themes/default'; import Hoverable from '../../../components/Hoverable'; +import Tooltip from '../../../components/Tooltip'; const propTypes = { // Style for hovered state @@ -104,9 +105,14 @@ const OptionRow = ({ ) } - - {option.text} - + + + + {option.text} + + + + {option.alternateText ? ( Date: Fri, 5 Mar 2021 22:39:53 +0530 Subject: [PATCH 02/16] New: added tooltip to various parts of the app --- src/components/Tooltip/index.js | 43 ++++++------------ src/components/Tooltip/index.native.js | 2 +- src/libs/OptionsListUtils.js | 3 +- src/pages/home/ReportScreen.js | 1 - .../home/report/ReportActionItemSingle.js | 1 - src/pages/home/sidebar/OptionRow.js | 12 ++--- .../home/sidebar/OptionRowTitle/index.js | 44 +++++++++++++++++++ .../sidebar/OptionRowTitle/index.native.js | 26 +++++++++++ src/pages/home/sidebar/optionPropTypes.js | 4 ++ src/styles/getTooltipStyles.js | 1 + src/styles/variables.js | 1 + 11 files changed, 99 insertions(+), 39 deletions(-) create mode 100644 src/pages/home/sidebar/OptionRowTitle/index.js create mode 100644 src/pages/home/sidebar/OptionRowTitle/index.native.js diff --git a/src/components/Tooltip/index.js b/src/components/Tooltip/index.js index 86c1f058432d..9e1a60e96932 100644 --- a/src/components/Tooltip/index.js +++ b/src/components/Tooltip/index.js @@ -71,26 +71,17 @@ class Tooltip extends PureComponent { * @param {Object} nativeEvent */ measureWrapperAndGetPosition({nativeEvent}) { - const { - left, top, width, height, - } = nativeEvent.layout; - console.debug(left, top, nativeEvent); - this.setState({ - wrapperWidth: width, - wrapperHeight: height, - xOffset: left, - yOffset: top, - }); + const {width, height} = nativeEvent.layout; // We need to use `measureInWindow` instead of the layout props provided by `onLayout` // because `measureInWindow` provides the x and y offset relative to the window, rather than the parent element. - // this.getWrapperPosition() - // .then(({x, y}) => this.setState({ - // wrapperWidth: width, - // wrapperHeight: height, - // xOffset: x, - // yOffset: y, - // })); + this.getWrapperPosition() + .then(({x, y}) => this.setState({ + wrapperWidth: width, + wrapperHeight: height, + xOffset: x, + yOffset: y, + })); } /** @@ -109,6 +100,11 @@ class Tooltip extends PureComponent { * Display the tooltip in an animation. */ showTooltip() { + this.getWrapperPosition() + .then(({x, y}) => this.setState({ + xOffset: x, + yOffset: y, + })); Animated.timing(this.animation, { toValue: 1, duration: 140, @@ -170,19 +166,8 @@ class Tooltip extends PureComponent { this.wrapperView = el} onLayout={this.measureWrapperAndGetPosition} + style={this.props.containerStyle} > - {/* - this.tooltip = el} - onLayout={this.measureTooltip} - style={tooltipWrapperStyle} - > - {this.props.text} - - - - - */} {this.props.children} diff --git a/src/components/Tooltip/index.native.js b/src/components/Tooltip/index.native.js index 703b61d2ae25..094c5b0feb67 100644 --- a/src/components/Tooltip/index.native.js +++ b/src/components/Tooltip/index.native.js @@ -5,7 +5,7 @@ const defaultProps = { shiftVertical: 0, }; -const Tooltip = () => (this.props.children); +const Tooltip = props => props.children; Tooltip.propTypes = tooltipPropTypes; Tooltip.defaultProps = defaultProps; diff --git a/src/libs/OptionsListUtils.js b/src/libs/OptionsListUtils.js index 6a52b6bd4038..2c33753f4745 100644 --- a/src/libs/OptionsListUtils.js +++ b/src/libs/OptionsListUtils.js @@ -7,7 +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'; +import {getReportParticipantsTitle} from './reportUtils'; /** * OptionsListUtils is used to build a list options passed to the OptionsList component. Several different UI views can @@ -99,6 +99,7 @@ function createOption(personalDetailList, report, draftComments, activeReportID, alternateText: (showChatPreviewLine && lastMessageText) ? lastMessageText : personalDetail.login, icons: report ? report.icons : [personalDetail.avatar], tooltipText, + participantsList: personalDetailList, // 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. diff --git a/src/pages/home/ReportScreen.js b/src/pages/home/ReportScreen.js index ea68d692942f..feca8b516206 100644 --- a/src/pages/home/ReportScreen.js +++ b/src/pages/home/ReportScreen.js @@ -62,7 +62,6 @@ class ReportScreen extends Component { const isActiveReport = activeReportID === report.reportID; const finalData = {...memo}; let reportStyle; - console.debug(report); if (isActiveReport) { activeReportID = report.reportID; reportStyle = [styles.dFlex, styles.flex1]; diff --git a/src/pages/home/report/ReportActionItemSingle.js b/src/pages/home/report/ReportActionItemSingle.js index 9d330d6aadea..bf40be5e8cda 100644 --- a/src/pages/home/report/ReportActionItemSingle.js +++ b/src/pages/home/report/ReportActionItemSingle.js @@ -16,7 +16,6 @@ const propTypes = { }; const ReportActionItemSingle = ({action}) => { - console.debug(action); const avatarUrl = action.automatic ? `${CONST.CLOUDFRONT_URL}/images/icons/concierge_2019.svg` : action.avatar; diff --git a/src/pages/home/sidebar/OptionRow.js b/src/pages/home/sidebar/OptionRow.js index c213b806d559..4bbc14b6c50f 100644 --- a/src/pages/home/sidebar/OptionRow.js +++ b/src/pages/home/sidebar/OptionRow.js @@ -14,7 +14,7 @@ import {Pencil, PinCircle, Checkmark} from '../../../components/Icon/Expensicons import MultipleAvatars from '../../../components/MultipleAvatars'; import themeColors from '../../../styles/themes/default'; import Hoverable from '../../../components/Hoverable'; -import Tooltip from '../../../components/Tooltip'; +import OptionRowTitle from './OptionRowTitle'; const propTypes = { // Style for hovered state @@ -106,11 +106,11 @@ const OptionRow = ({ } - - - {option.text} - - + {option.alternateText ? ( diff --git a/src/pages/home/sidebar/OptionRowTitle/index.js b/src/pages/home/sidebar/OptionRowTitle/index.js new file mode 100644 index 000000000000..3cdb52d63fe1 --- /dev/null +++ b/src/pages/home/sidebar/OptionRowTitle/index.js @@ -0,0 +1,44 @@ +/* eslint-disable react/forbid-prop-types */ +import _ from 'underscore'; +import React, {memo} from 'react'; +import { + Text, +} from 'react-native'; +import styles from '../../../../styles/styles'; +import Tooltip from '../../../../components/Tooltip'; +import propTypes from './OptionRowTitleProps'; + +const defaultProps = { + style: null, + tooltipEnabled: false, +}; + +const OptionRowTitle = ({ + style, + tooltipEnabled, + option, +}) => ( + + { + tooltipEnabled + ? _.map(option.participantsList, (participant, index) => ( + <> + + + {participant.displayName} + + + {index < option.participantsList.length - 1 ? {', '} : null} + + )) + : option.text + } + +); + +OptionRowTitle.propTypes = propTypes; +OptionRowTitle.defaultProps = defaultProps; +OptionRowTitle.displayName = 'OptionRowTitle'; + +// It it very important to use React.memo here so SectionList items will not unnecessarily re-render +export default memo(OptionRowTitle); diff --git a/src/pages/home/sidebar/OptionRowTitle/index.native.js b/src/pages/home/sidebar/OptionRowTitle/index.native.js new file mode 100644 index 000000000000..74963cd24e60 --- /dev/null +++ b/src/pages/home/sidebar/OptionRowTitle/index.native.js @@ -0,0 +1,26 @@ +/* eslint-disable react/forbid-prop-types */ +import React, {memo} from 'react'; +import { + Text, +} from 'react-native'; +import styles from '../../../../styles/styles'; +import propTypes from './OptionRowTitleProps'; + +const defaultProps = { + style: null, + tooltipEnabled: false, +}; + +const OptionRowTitle = ({ + style, +}) => ( + + option.text + +); + +OptionRowTitle.propTypes = propTypes; +OptionRowTitle.defaultProps = defaultProps; +OptionRowTitle.displayName = 'OptionRowTitle'; + +export default memo(OptionRowTitle); diff --git a/src/pages/home/sidebar/optionPropTypes.js b/src/pages/home/sidebar/optionPropTypes.js index 2d1d8d999dc1..e4923c97be41 100644 --- a/src/pages/home/sidebar/optionPropTypes.js +++ b/src/pages/home/sidebar/optionPropTypes.js @@ -1,3 +1,4 @@ +/* eslint-disable react/forbid-prop-types */ import PropTypes from 'prop-types'; export default PropTypes.shape({ @@ -7,6 +8,9 @@ export default PropTypes.shape({ // Subtitle to show under report displayName, mostly lastMessageText of the report alternateText: PropTypes.string.isRequired, + // list of particiapants of the report + participantsList: PropTypes.object.isRequired, + // The array URLs of the person's avatar icon: PropTypes.arrayOf(PropTypes.string), diff --git a/src/styles/getTooltipStyles.js b/src/styles/getTooltipStyles.js index 8d11ea229887..0ee6bf61504e 100644 --- a/src/styles/getTooltipStyles.js +++ b/src/styles/getTooltipStyles.js @@ -121,6 +121,7 @@ export default function getTooltipStyles( borderRadius: variables.componentBorderRadiusSmall, ...tooltipVerticalPadding, ...spacing.ph2, + zIndex: variables.tooltipzIndex, // Because it uses absolute positioning, the top-left corner of the tooltip is aligned // with the top-left corner of the wrapped component by default. diff --git a/src/styles/variables.js b/src/styles/variables.js index d6d649fca395..3675c03b733f 100644 --- a/src/styles/variables.js +++ b/src/styles/variables.js @@ -16,4 +16,5 @@ export default { safeInsertPercentage: 0.7, sideBarWidth: 375, pdfPageMaxWidth: 992, + tooltipzIndex: 10050, }; From 8dece9d98403e163e83fcbee18b6664a1dbfdc12 Mon Sep 17 00:00:00 2001 From: Rajat Parashar Date: Sat, 6 Mar 2021 00:56:21 +0530 Subject: [PATCH 03/16] added ellipsis detection --- src/components/Header.js | 1 - src/libs/hasEllipsis/index.js | 12 +++ src/libs/hasEllipsis/index.native.js | 1 + src/pages/home/HeaderView.js | 2 +- .../home/sidebar/OptionRowTitle/index.js | 88 +++++++++++++------ src/pages/home/sidebar/optionPropTypes.js | 2 +- src/styles/styles.js | 6 ++ src/styles/utilities/display.js | 4 + 8 files changed, 88 insertions(+), 28 deletions(-) create mode 100644 src/libs/hasEllipsis/index.js create mode 100644 src/libs/hasEllipsis/index.native.js diff --git a/src/components/Header.js b/src/components/Header.js index ed3123458c35..6795601c3367 100644 --- a/src/components/Header.js +++ b/src/components/Header.js @@ -3,7 +3,6 @@ import {View} from 'react-native'; import PropTypes from 'prop-types'; import styles from '../styles/styles'; import Text from './Text'; -import Tooltip from './Tooltip'; const propTypes = { /** Title of the Header */ diff --git a/src/libs/hasEllipsis/index.js b/src/libs/hasEllipsis/index.js new file mode 100644 index 000000000000..ce2c213b58be --- /dev/null +++ b/src/libs/hasEllipsis/index.js @@ -0,0 +1,12 @@ +// eslint-disable-next-line valid-jsdoc +/** + * Does an elment has ellipsis + * + * @param {*} el + * @returns + */ +function hasEllipsis(el) { + return el.offsetHeight < el.scrollHeight || el.offsetWidth < el.scrollWidth; +} + +export default hasEllipsis; diff --git a/src/libs/hasEllipsis/index.native.js b/src/libs/hasEllipsis/index.native.js new file mode 100644 index 000000000000..2d1ec238274a --- /dev/null +++ b/src/libs/hasEllipsis/index.native.js @@ -0,0 +1 @@ +export default () => {}; diff --git a/src/pages/home/HeaderView.js b/src/pages/home/HeaderView.js index 675edf6b7e92..137a266e7203 100644 --- a/src/pages/home/HeaderView.js +++ b/src/pages/home/HeaderView.js @@ -1,6 +1,6 @@ import React from 'react'; import {View, Pressable} from 'react-native'; -import PropTypes, {object} from 'prop-types'; +import PropTypes from 'prop-types'; import {withOnyx} from 'react-native-onyx'; import Header from '../../components/Header'; import styles from '../../styles/styles'; diff --git a/src/pages/home/sidebar/OptionRowTitle/index.js b/src/pages/home/sidebar/OptionRowTitle/index.js index 3cdb52d63fe1..bfa2f821fa55 100644 --- a/src/pages/home/sidebar/OptionRowTitle/index.js +++ b/src/pages/home/sidebar/OptionRowTitle/index.js @@ -1,44 +1,82 @@ /* eslint-disable react/forbid-prop-types */ import _ from 'underscore'; -import React, {memo} from 'react'; +import React, {Fragment, PureComponent} from 'react'; +import PropTypes from 'prop-types'; import { Text, + View, } from 'react-native'; +import optionPropTypes from '../optionPropTypes'; import styles from '../../../../styles/styles'; import Tooltip from '../../../../components/Tooltip'; -import propTypes from './OptionRowTitleProps'; +import hasEllipsis from '../../../../libs/hasEllipsis'; + +const propTypes = { + // styles of the title + style: PropTypes.object, + + tooltipEnabled: PropTypes.bool, + + // Option to allow the user to choose from can be type 'report' or 'user' + option: optionPropTypes.isRequired, + +}; const defaultProps = { style: null, tooltipEnabled: false, }; +class OptionRowTitle extends PureComponent { + constructor(props) { + super(props); + this.ref = React.createRef(); + this.state = { + isEllipsisActive: false, + }; + } + + componentDidMount() { + this.setState({ + isEllipsisActive: this.ref.current && hasEllipsis(this.ref.current), + }); + } + + render() { + const {option, style, tooltipEnabled} = this.props; + const {isEllipsisActive} = this.state; -const OptionRowTitle = ({ - style, - tooltipEnabled, - option, -}) => ( - - { - tooltipEnabled - ? _.map(option.participantsList, (participant, index) => ( - <> - - - {participant.displayName} - - - {index < option.participantsList.length - 1 ? {', '} : null} - - )) - : option.text + if (tooltipEnabled) { + return ( + + {_.map(option.participantsList, (participant, index) => ( + + + + {participant.displayName} + + + {index < option.participantsList.length - 1 ? {', '} : null} + + ))} + { + isEllipsisActive + ? ( + + + ... + + + ) : null + } + + ); } - -); + return {option.text}; + } +} OptionRowTitle.propTypes = propTypes; OptionRowTitle.defaultProps = defaultProps; OptionRowTitle.displayName = 'OptionRowTitle'; -// It it very important to use React.memo here so SectionList items will not unnecessarily re-render -export default memo(OptionRowTitle); +export default OptionRowTitle; diff --git a/src/pages/home/sidebar/optionPropTypes.js b/src/pages/home/sidebar/optionPropTypes.js index e4923c97be41..89ce9ce19107 100644 --- a/src/pages/home/sidebar/optionPropTypes.js +++ b/src/pages/home/sidebar/optionPropTypes.js @@ -9,7 +9,7 @@ export default PropTypes.shape({ alternateText: PropTypes.string.isRequired, // list of particiapants of the report - participantsList: PropTypes.object.isRequired, + participantsList: PropTypes.array.isRequired, // The array URLs of the person's avatar icon: PropTypes.arrayOf(PropTypes.string), diff --git a/src/styles/styles.js b/src/styles/styles.js index c0beb00707fe..8f54c9cbe7bb 100644 --- a/src/styles/styles.js +++ b/src/styles/styles.js @@ -542,6 +542,12 @@ const styles = { lineHeight: 18, ...whiteSpace.noWrap, }, + optionDisplayNameTooltipEllipsis: { + position: 'absolute', + opacity: 0, + right: 0, + bottom: 0, + }, optionAlternateText: { color: themeColors.textSupporting, diff --git a/src/styles/utilities/display.js b/src/styles/utilities/display.js index 275ac345883e..a16a62694af8 100644 --- a/src/styles/utilities/display.js +++ b/src/styles/utilities/display.js @@ -16,4 +16,8 @@ export default { dNone: { display: 'none', }, + + dInline: { + display: 'inline', + }, }; From f8f6e98b2b1544ee2d0bd01dbb7fbeadc17e6f95 Mon Sep 17 00:00:00 2001 From: Rajat Parashar Date: Thu, 11 Mar 2021 23:49:36 +0530 Subject: [PATCH 04/16] new: Tooltip position for Movable elments --- src/components/Tooltip/TooltipPropTypes.js | 4 ++-- src/components/Tooltip/index.js | 22 ++++++++++++++++++---- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/src/components/Tooltip/TooltipPropTypes.js b/src/components/Tooltip/TooltipPropTypes.js index 2d19f1b12355..c1c3dfa37aea 100644 --- a/src/components/Tooltip/TooltipPropTypes.js +++ b/src/components/Tooltip/TooltipPropTypes.js @@ -16,11 +16,11 @@ const propTypes = { // 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, + 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.number, + shiftVertical: PropTypes.oneOfType([PropTypes.number, PropTypes.func]), }; export default propTypes; diff --git a/src/components/Tooltip/index.js b/src/components/Tooltip/index.js index 9e1a60e96932..d7d4c41dd4cf 100644 --- a/src/components/Tooltip/index.js +++ b/src/components/Tooltip/index.js @@ -62,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, + }), + ))); } /** @@ -100,8 +104,14 @@ 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}) => this.setState({ + .then(({ + x, y, width, height, + }) => this.setState({ + wrapperWidth: width, + wrapperHeight: height, xOffset: x, yOffset: y, })); @@ -137,8 +147,12 @@ class Tooltip extends PureComponent { this.state.wrapperHeight, this.state.tooltipWidth, this.state.tooltipHeight, - this.props.shiftHorizontal, - this.props.shiftVertical, + typeof this.props.shiftHorizontal === 'function' + ? this.props.shiftHorizontal() + : this.props.shiftHorizontal, + typeof this.props.shiftVertical === 'function' + ? this.props.shiftVertical() + : this.props.shiftVertical, ); return ReactDOM.createPortal( Date: Thu, 11 Mar 2021 23:50:57 +0530 Subject: [PATCH 05/16] added tooltip for various parts of the app --- src/libs/hasEllipsis/index.js | 2 +- src/pages/home/HeaderView.js | 2 +- src/pages/home/sidebar/OptionRow.js | 12 ++- .../OptionRowTitle/OptionRowTitleProps.js | 17 ++++ .../home/sidebar/OptionRowTitle/index.js | 90 ++++++++++++++----- .../sidebar/OptionRowTitle/index.native.js | 3 +- src/styles/styles.js | 5 ++ 7 files changed, 97 insertions(+), 34 deletions(-) create mode 100644 src/pages/home/sidebar/OptionRowTitle/OptionRowTitleProps.js diff --git a/src/libs/hasEllipsis/index.js b/src/libs/hasEllipsis/index.js index ce2c213b58be..b50c0e2b1968 100644 --- a/src/libs/hasEllipsis/index.js +++ b/src/libs/hasEllipsis/index.js @@ -6,7 +6,7 @@ * @returns */ function hasEllipsis(el) { - return el.offsetHeight < el.scrollHeight || el.offsetWidth < el.scrollWidth; + return el.offsetWidth < el.scrollWidth; } export default hasEllipsis; diff --git a/src/pages/home/HeaderView.js b/src/pages/home/HeaderView.js index 137a266e7203..c2ee76fb6b9f 100644 --- a/src/pages/home/HeaderView.js +++ b/src/pages/home/HeaderView.js @@ -25,7 +25,7 @@ const propTypes = { reportName: PropTypes.string, // list of primarylogins of participants of the report - participants: PropTypes.arrayOf(PropTypes.object), + participants: PropTypes.arrayOf(PropTypes.string), // ID of the report reportID: PropTypes.number, diff --git a/src/pages/home/sidebar/OptionRow.js b/src/pages/home/sidebar/OptionRow.js index 4bbc14b6c50f..8700fa9fafaa 100644 --- a/src/pages/home/sidebar/OptionRow.js +++ b/src/pages/home/sidebar/OptionRow.js @@ -105,13 +105,11 @@ const OptionRow = ({ ) } - - - + {option.alternateText ? ( React.createRef()); + } + + componentDidUpdate(prevProps) { + if (prevProps.option !== this.props.option) { + this.cRefs = this.props.option.participantsList.map(() => React.createRef()); + } + } + + setContainerLayout({nativeEvent}) { + this.setState({ + containerLayout: nativeEvent.layout, + }); + } + + getTooltipShiftX(index) { + const {containerLayout} = this.state; + + // only shift when containerLayout or Refs to text node is available . + if (!containerLayout || !this.cRefs[index] || !this.cRefs[index].current) { + return; + } + const {width: cWidth, left: cLeft} = containerLayout; + + // we have to return the value as Number so we can't use `measureWindow` which takes a callback + const {width: tWidth, left: tLeft} = this.cRefs[index].current.getBoundingClientRect(); + const toolX = (tWidth / 2) + tLeft; + const cRight = cWidth + cLeft; + const tRight = tWidth + tLeft; + const newToolX = tLeft + ((cRight - tLeft) / 2); + + // when text right end is beyond the Container Right end + return tRight > cRight ? -(toolX - newToolX) : 0; } + render() { const {option, style, tooltipEnabled} = this.props; const {isEllipsisActive} = this.state; if (tooltipEnabled) { return ( - - {_.map(option.participantsList, (participant, index) => ( - - - - {participant.displayName} - - - {index < option.participantsList.length - 1 ? {', '} : null} - - ))} + + {_.map(option.participantsList, (participant, index) => { + const ref = this.cRefs[index] ? this.cRefs[index] : (this.cRefs[index] = React.createRef()); + return ( + + this.getTooltipShiftX(index)} + > + + {participant.displayName} + + + {index < option.participantsList.length - 1 ? ,  : null} + + ); + })} { isEllipsisActive ? ( diff --git a/src/pages/home/sidebar/OptionRowTitle/index.native.js b/src/pages/home/sidebar/OptionRowTitle/index.native.js index 74963cd24e60..05e0f5f918fe 100644 --- a/src/pages/home/sidebar/OptionRowTitle/index.native.js +++ b/src/pages/home/sidebar/OptionRowTitle/index.native.js @@ -13,9 +13,10 @@ const defaultProps = { const OptionRowTitle = ({ style, + option, }) => ( - option.text + {option.text} ); diff --git a/src/styles/styles.js b/src/styles/styles.js index 8f54c9cbe7bb..a1f9ae953d4e 100644 --- a/src/styles/styles.js +++ b/src/styles/styles.js @@ -542,6 +542,11 @@ const styles = { lineHeight: 18, ...whiteSpace.noWrap, }, + + optionDisplayNameTooltipWrapper: { + position: 'relative', + }, + optionDisplayNameTooltipEllipsis: { position: 'absolute', opacity: 0, From f798513195ba747dcc594ec2911f1b2fcdba2cc5 Mon Sep 17 00:00:00 2001 From: Rajat Parashar Date: Fri, 12 Mar 2021 00:11:43 +0530 Subject: [PATCH 06/16] new: tooltip comments & linting --- src/pages/home/ReportScreen.js | 1 + src/styles/getTooltipStyles.js | 19 ++++++++++++------- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/src/pages/home/ReportScreen.js b/src/pages/home/ReportScreen.js index feca8b516206..61eff320cf0a 100644 --- a/src/pages/home/ReportScreen.js +++ b/src/pages/home/ReportScreen.js @@ -62,6 +62,7 @@ class ReportScreen extends Component { const isActiveReport = activeReportID === report.reportID; const finalData = {...memo}; let reportStyle; + if (isActiveReport) { activeReportID = report.reportID; reportStyle = [styles.dFlex, styles.flex1]; diff --git a/src/styles/getTooltipStyles.js b/src/styles/getTooltipStyles.js index 0ee6bf61504e..f83caecf5f01 100644 --- a/src/styles/getTooltipStyles.js +++ b/src/styles/getTooltipStyles.js @@ -111,6 +111,9 @@ export default function getTooltipStyles( return { animationStyle: { + // remember Transform casuses a new Local cordinate system + // https://drafts.csswg.org/css-transforms-1/#transform-rendering + // so Position fixed children will be relative to this new Local cordinate system transform: [{ scale: currentSize, }], @@ -123,8 +126,9 @@ export default function getTooltipStyles( ...spacing.ph2, zIndex: variables.tooltipzIndex, - // Because it uses absolute positioning, the top-left corner of the tooltip is aligned - // with the top-left corner of the wrapped component by default. + // Because it uses fixed positioning, the top-left corner of the tooltip is aligned + // with the top-left corner of the window by default. + // we will use yOffset to position the tooltip relative to the Wrapped Component // So we need to shift the tooltip vertically and horizontally to position it correctly. // // First, we'll position it vertically. @@ -139,6 +143,7 @@ export default function getTooltipStyles( : yOffset - (tooltipHeight + POINTER_HEIGHT + manualShiftVertical), // Next, we'll position it horizontally. + // we will use xOffset to position the tooltip relative to the Wrapped Component // To shift the tooltip right, we'll give `left` a positive value. // To shift the tooltip left, we'll give `left` a negative value. // @@ -160,11 +165,11 @@ export default function getTooltipStyles( position: 'fixed', - // By default, the pointer's top-left will align with the top-left of the wrapped component. + // By default, the pointer's top-left will align with the top-left of the wrapped tooltip. // // To align it vertically, we'll: // - // Shift the pointer up (-) by its height, so that the bottom of the pointer lines up + // Shift the pointer up (-) by component's height, so that the bottom of the pointer lines up // with the top of the wrapped component. // // OR if it should show below: @@ -177,10 +182,10 @@ export default function getTooltipStyles( top: shouldShowBelow ? manualShiftVertical - POINTER_HEIGHT : tooltipHeight + manualShiftVertical, // To align it horizontally, we'll: - // 1) Shift the pointer to the right (+) by the half the component's width, - // so the left edge of the pointer lines up with the component's center. + // 1) Shift the pointer to the right (+) by the half the tooltipWidth's width, + // so the left edge of the pointer lines up with the tooltipWidth's center. // 2) To the left (-) by half the pointer's width, - // so the pointer's center lines up with the component's center. + // so the pointer's center lines up with the tooltipWidth's center. left: ((tooltipWidth / 2) - (POINTER_WIDTH / 2)), }, pointerStyle: { From b34457c849bd12ffff14a177bad577aaef04fb67 Mon Sep 17 00:00:00 2001 From: Rajat Parashar Date: Fri, 12 Mar 2021 01:04:01 +0530 Subject: [PATCH 07/16] fix: Report is option in headerView --- src/pages/home/HeaderView.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/pages/home/HeaderView.js b/src/pages/home/HeaderView.js index 40e7d9e89c50..feb640ba95e5 100644 --- a/src/pages/home/HeaderView.js +++ b/src/pages/home/HeaderView.js @@ -45,7 +45,9 @@ const defaultProps = { }; const HeaderView = (props) => { - const participantsTitle = getReportParticipantsTitle(props.report.participants); + const participantsTitle = props.report && props.report.participants + ? getReportParticipantsTitle(props.report.participants) + : ''; return ( From 134367b88f7db5f982b94650be7a17ac85784522 Mon Sep 17 00:00:00 2001 From: Rajat Parashar Date: Fri, 12 Mar 2021 03:44:39 +0530 Subject: [PATCH 08/16] chg: refactor --- .../Hoverable/HoverablePropTypes.js | 1 + src/components/Tooltip/TooltipPropTypes.js | 10 +++++- src/components/Tooltip/index.js | 19 +++++------ src/components/Tooltip/index.native.js | 10 ++---- src/libs/hasEllipsis/index.js | 5 ++- src/libs/reportUtils.js | 2 +- .../OptionRowTitle/OptionRowTitleProps.js | 11 ++++-- .../home/sidebar/OptionRowTitle/index.js | 34 ++++++++----------- .../sidebar/OptionRowTitle/index.native.js | 6 +--- src/pages/home/sidebar/optionPropTypes.js | 14 ++++++-- 10 files changed, 60 insertions(+), 52 deletions(-) diff --git a/src/components/Hoverable/HoverablePropTypes.js b/src/components/Hoverable/HoverablePropTypes.js index 04d462b42637..9acbbabafed3 100644 --- a/src/components/Hoverable/HoverablePropTypes.js +++ b/src/components/Hoverable/HoverablePropTypes.js @@ -18,6 +18,7 @@ const propTypes = { }; const defaultProps = { + containerStyle: {}, onHoverIn: () => {}, onHoverOut: () => {}, }; diff --git a/src/components/Tooltip/TooltipPropTypes.js b/src/components/Tooltip/TooltipPropTypes.js index c1c3dfa37aea..699c6c643fb3 100644 --- a/src/components/Tooltip/TooltipPropTypes.js +++ b/src/components/Tooltip/TooltipPropTypes.js @@ -23,4 +23,12 @@ const propTypes = { shiftVertical: PropTypes.oneOfType([PropTypes.number, PropTypes.func]), }; -export default propTypes; +const defaultProps = { + shiftHorizontal: 0, + shiftVertical: 0, +}; + +export { + propTypes, + defaultProps, +}; diff --git a/src/components/Tooltip/index.js b/src/components/Tooltip/index.js index d7d4c41dd4cf..3f161b32d2f4 100644 --- a/src/components/Tooltip/index.js +++ b/src/components/Tooltip/index.js @@ -1,15 +1,11 @@ +import _ from 'underscore'; import React, {PureComponent} from 'react'; import {Animated, Text, View} from 'react-native'; 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, - shiftVertical: 0, -}; +import {propTypes, defaultProps} from './TooltipPropTypes'; class Tooltip extends PureComponent { constructor(props) { @@ -147,10 +143,10 @@ class Tooltip extends PureComponent { this.state.wrapperHeight, this.state.tooltipWidth, this.state.tooltipHeight, - typeof this.props.shiftHorizontal === 'function' + _.isFunction(this.props.shiftHorizontal) ? this.props.shiftHorizontal() : this.props.shiftHorizontal, - typeof this.props.shiftVertical === 'function' + _.isFunction(this.props.shiftVertical) ? this.props.shiftVertical() : this.props.shiftVertical, ); @@ -158,13 +154,14 @@ class Tooltip extends PureComponent { this.tooltip = el} onLayout={this.measureTooltip} - style={{...tooltipWrapperStyle, ...animationStyle}} + style={[tooltipWrapperStyle, animationStyle]} > {this.props.text} - , document.querySelector('body'), + , + document.querySelector('body'), ); } @@ -190,6 +187,6 @@ class Tooltip extends PureComponent { } } -Tooltip.propTypes = tooltipPropTypes; +Tooltip.propTypes = propTypes; Tooltip.defaultProps = defaultProps; export default withWindowDimensions(Tooltip); diff --git a/src/components/Tooltip/index.native.js b/src/components/Tooltip/index.native.js index 094c5b0feb67..44feda551ac1 100644 --- a/src/components/Tooltip/index.native.js +++ b/src/components/Tooltip/index.native.js @@ -1,12 +1,8 @@ -import tooltipPropTypes from './TooltipPropTypes'; - -const defaultProps = { - shiftHorizontal: 0, - shiftVertical: 0, -}; +import {propTypes, defaultProps} from './TooltipPropTypes'; const Tooltip = props => props.children; -Tooltip.propTypes = tooltipPropTypes; +Tooltip.propTypes = propTypes; Tooltip.defaultProps = defaultProps; +Tooltip.displayName = 'Tooltip'; export default Tooltip; diff --git a/src/libs/hasEllipsis/index.js b/src/libs/hasEllipsis/index.js index b50c0e2b1968..dcadd632d702 100644 --- a/src/libs/hasEllipsis/index.js +++ b/src/libs/hasEllipsis/index.js @@ -1,9 +1,8 @@ -// eslint-disable-next-line valid-jsdoc /** * Does an elment has ellipsis * - * @param {*} el - * @returns + * @param {HTMLElement} el Element to check + * @returns {Boolean} */ function hasEllipsis(el) { return el.offsetWidth < el.scrollWidth; diff --git a/src/libs/reportUtils.js b/src/libs/reportUtils.js index bff7c0b3522d..538076d6c183 100644 --- a/src/libs/reportUtils.js +++ b/src/libs/reportUtils.js @@ -8,7 +8,7 @@ import Str from 'expensify-common/lib/str'; * @returns {string} */ function getReportParticipantsTitle(logins) { - return _.map(logins, login => Str.removeSMSDomain(login)).join(','); + return _.map(logins, login => Str.removeSMSDomain(login)).join(', '); } export { diff --git a/src/pages/home/sidebar/OptionRowTitle/OptionRowTitleProps.js b/src/pages/home/sidebar/OptionRowTitle/OptionRowTitleProps.js index edc038c3f26b..fd1e040f7c4d 100644 --- a/src/pages/home/sidebar/OptionRowTitle/OptionRowTitleProps.js +++ b/src/pages/home/sidebar/OptionRowTitle/OptionRowTitleProps.js @@ -1,4 +1,3 @@ -/* eslint-disable react/forbid-prop-types */ import PropTypes from 'prop-types'; import optionPropTypes from '../optionPropTypes'; @@ -14,4 +13,12 @@ const propTypes = { option: optionPropTypes.isRequired, }; -export default propTypes; + +const defaultProps = { + style: null, + tooltipEnabled: false, +}; +export { + propTypes, + defaultProps, +}; diff --git a/src/pages/home/sidebar/OptionRowTitle/index.js b/src/pages/home/sidebar/OptionRowTitle/index.js index 417bf151fa2e..fe560b80ada5 100644 --- a/src/pages/home/sidebar/OptionRowTitle/index.js +++ b/src/pages/home/sidebar/OptionRowTitle/index.js @@ -1,24 +1,25 @@ -/* eslint-disable react/forbid-prop-types */ import _ from 'underscore'; import React, {Fragment, PureComponent} from 'react'; import { Text, View, } from 'react-native'; -import propTypes from './OptionRowTitleProps'; +import {propTypes, defaultProps} from './OptionRowTitleProps'; import styles from '../../../../styles/styles'; import Tooltip from '../../../../components/Tooltip'; import hasEllipsis from '../../../../libs/hasEllipsis'; -const defaultProps = { - style: null, - tooltipEnabled: false, -}; class OptionRowTitle extends PureComponent { constructor(props) { super(props); - this.ref = React.createRef(); + this.ref = null; this.cRefs = []; + this.setContainerRef = (ref) => { + this.ref = ref; + }; + this.setOptionChildRef = index => (ref) => { + this.cRefs[index] = ref; + }; this.state = { isEllipsisActive: false, containerLayout: null, @@ -29,15 +30,8 @@ class OptionRowTitle extends PureComponent { componentDidMount() { this.setState({ - isEllipsisActive: this.ref.current && hasEllipsis(this.ref.current), + isEllipsisActive: this.ref && hasEllipsis(this.ref), }); - this.cRefs = this.props.option.participantsList.map(() => React.createRef()); - } - - componentDidUpdate(prevProps) { - if (prevProps.option !== this.props.option) { - this.cRefs = this.props.option.participantsList.map(() => React.createRef()); - } } setContainerLayout({nativeEvent}) { @@ -50,13 +44,13 @@ class OptionRowTitle extends PureComponent { const {containerLayout} = this.state; // only shift when containerLayout or Refs to text node is available . - if (!containerLayout || !this.cRefs[index] || !this.cRefs[index].current) { + if (!containerLayout || !this.cRefs[index]) { return; } const {width: cWidth, left: cLeft} = containerLayout; // we have to return the value as Number so we can't use `measureWindow` which takes a callback - const {width: tWidth, left: tLeft} = this.cRefs[index].current.getBoundingClientRect(); + const {width: tWidth, left: tLeft} = this.cRefs[index].getBoundingClientRect(); const toolX = (tWidth / 2) + tLeft; const cRight = cWidth + cLeft; const tRight = tWidth + tLeft; @@ -80,10 +74,10 @@ class OptionRowTitle extends PureComponent { } onLayout={this.setContainerLayout} numberOfLines={1} - ref={this.ref} + ref={this.setContainerRef} > {_.map(option.participantsList, (participant, index) => { - const ref = this.cRefs[index] ? this.cRefs[index] : (this.cRefs[index] = React.createRef()); + const setChildRef = this.setOptionChildRef(index); return ( this.getTooltipShiftX(index)} > - + {participant.displayName} diff --git a/src/pages/home/sidebar/OptionRowTitle/index.native.js b/src/pages/home/sidebar/OptionRowTitle/index.native.js index 05e0f5f918fe..0c6c69de591e 100644 --- a/src/pages/home/sidebar/OptionRowTitle/index.native.js +++ b/src/pages/home/sidebar/OptionRowTitle/index.native.js @@ -4,12 +4,8 @@ import { Text, } from 'react-native'; import styles from '../../../../styles/styles'; -import propTypes from './OptionRowTitleProps'; +import {propTypes, defaultProps} from './OptionRowTitleProps'; -const defaultProps = { - style: null, - tooltipEnabled: false, -}; const OptionRowTitle = ({ style, diff --git a/src/pages/home/sidebar/optionPropTypes.js b/src/pages/home/sidebar/optionPropTypes.js index 89ce9ce19107..d9d0a857442c 100644 --- a/src/pages/home/sidebar/optionPropTypes.js +++ b/src/pages/home/sidebar/optionPropTypes.js @@ -1,4 +1,3 @@ -/* eslint-disable react/forbid-prop-types */ import PropTypes from 'prop-types'; export default PropTypes.shape({ @@ -9,7 +8,18 @@ export default PropTypes.shape({ alternateText: PropTypes.string.isRequired, // list of particiapants of the report - participantsList: PropTypes.array.isRequired, + participantsList: PropTypes.arrayOf( + PropTypes.shape({ + // primary login of participant + login: PropTypes.string, + + // display Name of participant + displayName: PropTypes.string, + + // avatar url of participant + avatar: PropTypes.string, + }), + ).isRequired, // The array URLs of the person's avatar icon: PropTypes.arrayOf(PropTypes.string), From 9b85137121f0e32cbe78041bc02773deb4b620f6 Mon Sep 17 00:00:00 2001 From: Rajat Parashar Date: Fri, 12 Mar 2021 03:48:58 +0530 Subject: [PATCH 09/16] fix: styling of tooltip --- src/styles/getTooltipStyles.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/styles/getTooltipStyles.js b/src/styles/getTooltipStyles.js index f83caecf5f01..fde710065557 100644 --- a/src/styles/getTooltipStyles.js +++ b/src/styles/getTooltipStyles.js @@ -111,7 +111,7 @@ export default function getTooltipStyles( return { animationStyle: { - // remember Transform casuses a new Local cordinate system + // remember Transform causes a new Local cordinate system // https://drafts.csswg.org/css-transforms-1/#transform-rendering // so Position fixed children will be relative to this new Local cordinate system transform: [{ @@ -140,7 +140,7 @@ export default function getTooltipStyles( ? yOffset + componentHeight + POINTER_HEIGHT + manualShiftVertical // We need to shift the tooltip up above the component. So shift the tooltip up (-) by... - : yOffset - (tooltipHeight + POINTER_HEIGHT + manualShiftVertical), + : (yOffset - (tooltipHeight + POINTER_HEIGHT)) + manualShiftVertical, // Next, we'll position it horizontally. // we will use xOffset to position the tooltip relative to the Wrapped Component @@ -178,7 +178,6 @@ export default function getTooltipStyles( // so that the top of the pointer aligns with the bottom of the component. // // Always add the manual vertical shift passed in as a parameter. - // eslint-disable-next-line max-len top: shouldShowBelow ? manualShiftVertical - POINTER_HEIGHT : tooltipHeight + manualShiftVertical, // To align it horizontally, we'll: @@ -186,7 +185,7 @@ export default function getTooltipStyles( // so the left edge of the pointer lines up with the tooltipWidth's center. // 2) To the left (-) by half the pointer's width, // so the pointer's center lines up with the tooltipWidth's center. - left: ((tooltipWidth / 2) - (POINTER_WIDTH / 2)), + left: (tooltipWidth / 2) - (POINTER_WIDTH / 2), }, pointerStyle: { width: 0, From e204a20f0c08a3dc2d6e38d85dcfce7cec01d2e7 Mon Sep 17 00:00:00 2001 From: Rajat Parashar Date: Fri, 12 Mar 2021 04:39:04 +0530 Subject: [PATCH 10/16] fix: Added tooltip prop & ecllipse only on multiple particpants --- src/components/OptionsList.js | 5 +++++ src/components/OptionsSelector.js | 5 +++++ src/pages/SearchPage.js | 1 + src/pages/home/sidebar/OptionRow.js | 7 ++++++- src/pages/home/sidebar/OptionRowTitle/index.js | 5 +++-- src/pages/home/sidebar/SidebarLinks.js | 1 + 6 files changed, 21 insertions(+), 3 deletions(-) diff --git a/src/components/OptionsList.js b/src/components/OptionsList.js index a564f67afd41..d90c7985c7bd 100644 --- a/src/components/OptionsList.js +++ b/src/components/OptionsList.js @@ -61,6 +61,9 @@ const propTypes = { PropTypes.func, PropTypes.shape({current: PropTypes.instanceOf(SectionList)}), ]), + + // Whether to show the title tooltip + showTitleTooltip: PropTypes.bool, }; const defaultProps = { @@ -77,6 +80,7 @@ const defaultProps = { onSelectRow: () => {}, headerMessage: '', innerRef: null, + showTitleTooltip: false, }; class OptionsList extends Component { @@ -142,6 +146,7 @@ class OptionsList extends Component { return ( ); diff --git a/src/pages/SearchPage.js b/src/pages/SearchPage.js index ad32cbce5a0f..d32842664a67 100644 --- a/src/pages/SearchPage.js +++ b/src/pages/SearchPage.js @@ -167,6 +167,7 @@ class SearchPage extends Component { headerMessage={headerMessage} hideSectionHeaders hideAdditionalOptionStates + showTitleTooltip /> diff --git a/src/pages/home/sidebar/OptionRow.js b/src/pages/home/sidebar/OptionRow.js index ee7919781937..5cf5f684d2d7 100644 --- a/src/pages/home/sidebar/OptionRow.js +++ b/src/pages/home/sidebar/OptionRow.js @@ -41,6 +41,9 @@ const propTypes = { // Force the text style to be the unread style forceTextUnreadStyle: PropTypes.bool, + + // Whether to show the title tooltip + showTitleTooltip: PropTypes.bool, }; const defaultProps = { @@ -49,6 +52,7 @@ const defaultProps = { showSelectedState: false, isSelected: false, forceTextUnreadStyle: false, + showTitleTooltip: false, }; const OptionRow = ({ @@ -60,6 +64,7 @@ const OptionRow = ({ showSelectedState, isSelected, forceTextUnreadStyle, + showTitleTooltip, }) => { const textStyle = optionIsFocused ? styles.sidebarLinkActiveText @@ -108,7 +113,7 @@ const OptionRow = ({ diff --git a/src/pages/home/sidebar/OptionRowTitle/index.js b/src/pages/home/sidebar/OptionRowTitle/index.js index fe560b80ada5..3ee86d73e102 100644 --- a/src/pages/home/sidebar/OptionRowTitle/index.js +++ b/src/pages/home/sidebar/OptionRowTitle/index.js @@ -95,11 +95,12 @@ class OptionRowTitle extends PureComponent { ); })} { - isEllipsisActive + option.participantsList.length > 1 && isEllipsisActive ? ( - ... + {/* there is some Gap for real ellipsis so we are adding 4 `.` to cover */} + .... ) : null diff --git a/src/pages/home/sidebar/SidebarLinks.js b/src/pages/home/sidebar/SidebarLinks.js index f1759ab8b05e..90e45a7d8150 100644 --- a/src/pages/home/sidebar/SidebarLinks.js +++ b/src/pages/home/sidebar/SidebarLinks.js @@ -148,6 +148,7 @@ class SidebarLinks extends React.Component { this.props.onLinkClick(); }} hideSectionHeaders + showTitleTooltip /> From 9900d03b22f968899bfdf508042b284721a9fc6a Mon Sep 17 00:00:00 2001 From: Rajat Parashar Date: Fri, 12 Mar 2021 22:27:59 +0530 Subject: [PATCH 11/16] new: added tooltip support for report Header --- src/libs/OptionsListUtils.js | 1 + src/pages/home/HeaderView.js | 28 ++++++++++----- src/pages/home/sidebar/OptionRow.js | 3 +- .../OptionRowTitle/OptionRowTitleProps.js | 36 ++++++++++++++++--- .../home/sidebar/OptionRowTitle/index.js | 15 ++++---- .../sidebar/OptionRowTitle/index.native.js | 4 +-- src/pages/home/sidebar/optionPropTypes.js | 3 ++ 7 files changed, 67 insertions(+), 23 deletions(-) diff --git a/src/libs/OptionsListUtils.js b/src/libs/OptionsListUtils.js index 19dcdf1ff7de..c771dd381d13 100644 --- a/src/libs/OptionsListUtils.js +++ b/src/libs/OptionsListUtils.js @@ -432,4 +432,5 @@ export { getNewGroupOptions, getSidebarOptions, getHeaderMessage, + getPersonalDetailsForLogins, }; diff --git a/src/pages/home/HeaderView.js b/src/pages/home/HeaderView.js index feb640ba95e5..2377d22876d5 100644 --- a/src/pages/home/HeaderView.js +++ b/src/pages/home/HeaderView.js @@ -2,7 +2,7 @@ import React from 'react'; import {View, Pressable} from 'react-native'; import PropTypes from 'prop-types'; import {withOnyx} from 'react-native-onyx'; -import Header from '../../components/Header'; +import lodashGet from 'lodash.get'; import styles from '../../styles/styles'; import ONYXKEYS from '../../ONYXKEYS'; import themeColors from '../../styles/themes/default'; @@ -14,8 +14,9 @@ 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'; +import OptionRowTitle from './sidebar/OptionRowTitle'; +import {getPersonalDetailsForLogins} from '../../libs/OptionsListUtils'; const propTypes = { // Toggles the navigationMenu open and closed @@ -45,9 +46,12 @@ const defaultProps = { }; const HeaderView = (props) => { - const participantsTitle = props.report && props.report.participants - ? getReportParticipantsTitle(props.report.participants) - : ''; + const participants = lodashGet(props.report, 'participants', []); + const reportOption = { + text: lodashGet(props.report, 'reportName', ''), + tooltipText: getReportParticipantsTitle(participants), + participantsList: getPersonalDetailsForLogins(participants, props.personalDetails), + }; return ( @@ -71,7 +75,6 @@ const HeaderView = (props) => { > { - const {participants} = props.report; if (participants.length === 1) { redirect(ROUTES.getProfileRoute(participants[0])); } @@ -80,9 +83,13 @@ const HeaderView = (props) => { - -
- + `${ONYXKEYS.COLLECTION.REPORT}${currentlyViewedReportID}`, }, + personalDetails: { + key: ONYXKEYS.PERSONAL_DETAILS, + }, }), )(HeaderView); diff --git a/src/pages/home/sidebar/OptionRow.js b/src/pages/home/sidebar/OptionRow.js index 5cf5f684d2d7..fb4cfcd22408 100644 --- a/src/pages/home/sidebar/OptionRow.js +++ b/src/pages/home/sidebar/OptionRow.js @@ -114,7 +114,8 @@ const OptionRow = ({ {option.alternateText ? ( diff --git a/src/pages/home/sidebar/OptionRowTitle/OptionRowTitleProps.js b/src/pages/home/sidebar/OptionRowTitle/OptionRowTitleProps.js index fd1e040f7c4d..6bf2e0039be4 100644 --- a/src/pages/home/sidebar/OptionRowTitle/OptionRowTitleProps.js +++ b/src/pages/home/sidebar/OptionRowTitle/OptionRowTitleProps.js @@ -1,22 +1,50 @@ import PropTypes from 'prop-types'; -import optionPropTypes from '../optionPropTypes'; +import styles from '../../../../styles/styles'; const propTypes = { // styles of the title - style: PropTypes.arrayOf(PropTypes.object), + style: PropTypes.arrayOf(PropTypes.any), - // Does tooltip is needed + // lines before wrapping + numberOfLines: PropTypes.number, + + // Is tooltip needed? // When true, triggers complex title rendering tooltipEnabled: PropTypes.bool, + // Styles for the tooltip + tooltipContainerStyle: PropTypes.object, + // Option to allow the user to choose from can be type 'report' or 'user' - option: optionPropTypes.isRequired, + option: PropTypes.shape({ + // The full name of the user if available, otherwise the login (email/phone number) of the user + text: PropTypes.string.isRequired, + + // list of particiapants of the report + participantsList: PropTypes.arrayOf( + PropTypes.shape({ + // primary login of participant + login: PropTypes.string, + + // display Name of participant + displayName: PropTypes.string, + + // avatar url of participant + avatar: PropTypes.string, + }), + ).isRequired, + + // text to show for tooltip + tooltipText: PropTypes.string, + }).isRequired, }; const defaultProps = { style: null, tooltipEnabled: false, + tooltipContainerStyle: styles.dInline, + numberOfLines: 1, }; export { propTypes, diff --git a/src/pages/home/sidebar/OptionRowTitle/index.js b/src/pages/home/sidebar/OptionRowTitle/index.js index 3ee86d73e102..07dbe968facb 100644 --- a/src/pages/home/sidebar/OptionRowTitle/index.js +++ b/src/pages/home/sidebar/OptionRowTitle/index.js @@ -62,16 +62,17 @@ class OptionRowTitle extends PureComponent { render() { - const {option, style, tooltipEnabled} = this.props; + const { + option, style, tooltipEnabled, numberOfLines, tooltipContainerStyle, + } = this.props; const {isEllipsisActive} = this.state; if (tooltipEnabled) { return ( + + // Tokenization of string only support 1 numberofLines on Web this.getTooltipShiftX(index)} > @@ -109,7 +110,7 @@ class OptionRowTitle extends PureComponent { ); } - return {option.text}; + return {option.text}; } } OptionRowTitle.propTypes = propTypes; diff --git a/src/pages/home/sidebar/OptionRowTitle/index.native.js b/src/pages/home/sidebar/OptionRowTitle/index.native.js index 0c6c69de591e..a32cc716035e 100644 --- a/src/pages/home/sidebar/OptionRowTitle/index.native.js +++ b/src/pages/home/sidebar/OptionRowTitle/index.native.js @@ -3,15 +3,15 @@ import React, {memo} from 'react'; import { Text, } from 'react-native'; -import styles from '../../../../styles/styles'; import {propTypes, defaultProps} from './OptionRowTitleProps'; const OptionRowTitle = ({ style, option, + numberOfLines, }) => ( - + {option.text} ); diff --git a/src/pages/home/sidebar/optionPropTypes.js b/src/pages/home/sidebar/optionPropTypes.js index d9d0a857442c..65a9057d0808 100644 --- a/src/pages/home/sidebar/optionPropTypes.js +++ b/src/pages/home/sidebar/optionPropTypes.js @@ -29,4 +29,7 @@ export default PropTypes.shape({ // The option key provided to FlatList keyExtractor keyForList: PropTypes.string, + + // text to show for tooltip + tooltipText: PropTypes.string, }); From 6ca2c49887d44b8be2c759db0dbbefacd406e353 Mon Sep 17 00:00:00 2001 From: Rajat Parashar Date: Fri, 12 Mar 2021 22:57:36 +0530 Subject: [PATCH 12/16] fix: tooltip pointer fix --- src/styles/getTooltipStyles.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/styles/getTooltipStyles.js b/src/styles/getTooltipStyles.js index fde710065557..21699e5428d1 100644 --- a/src/styles/getTooltipStyles.js +++ b/src/styles/getTooltipStyles.js @@ -164,7 +164,6 @@ export default function getTooltipStyles( pointerWrapperStyle: { position: 'fixed', - // By default, the pointer's top-left will align with the top-left of the wrapped tooltip. // // To align it vertically, we'll: @@ -185,7 +184,9 @@ export default function getTooltipStyles( // so the left edge of the pointer lines up with the tooltipWidth's center. // 2) To the left (-) by half the pointer's width, // so the pointer's center lines up with the tooltipWidth's center. - left: (tooltipWidth / 2) - (POINTER_WIDTH / 2), + // 3) Due to the tip start from the left edge of wrapper Tooltip so we have to remove the + // horizontalShift which is added to adjust it into the Window + left: -horizontalShift + ((tooltipWidth / 2) - (POINTER_WIDTH / 2)), }, pointerStyle: { width: 0, From 3a9df36fa7ec20f915a00e22ddf2f234718368c0 Mon Sep 17 00:00:00 2001 From: Rajat Parashar Date: Tue, 16 Mar 2021 23:35:41 +0530 Subject: [PATCH 13/16] fix: linting issues and refactor code --- .../Hoverable/HoverablePropTypes.js | 1 + .../Tooltip/TooltipRenderedOnPageBody.js | 55 +++++++ src/components/Tooltip/index.js | 64 ++++---- src/components/Tooltip/index.native.js | 7 + src/libs/hasEllipsis/index.js | 2 +- src/pages/home/HeaderView.js | 7 +- src/pages/home/sidebar/OptionRow.js | 2 +- .../OptionRowTitle/OptionRowTitleProps.js | 27 +--- .../home/sidebar/OptionRowTitle/index.js | 138 ++++++++++-------- .../sidebar/OptionRowTitle/index.native.js | 11 +- src/pages/home/sidebar/SidebarLinks.js | 7 +- src/pages/home/sidebar/optionPropTypes.js | 36 +++-- 12 files changed, 207 insertions(+), 150 deletions(-) create mode 100644 src/components/Tooltip/TooltipRenderedOnPageBody.js diff --git a/src/components/Hoverable/HoverablePropTypes.js b/src/components/Hoverable/HoverablePropTypes.js index 9acbbabafed3..b92d4e0eb886 100644 --- a/src/components/Hoverable/HoverablePropTypes.js +++ b/src/components/Hoverable/HoverablePropTypes.js @@ -8,6 +8,7 @@ const propTypes = { ]).isRequired, // Styles to be assigned to the Hoverable Container + // eslint-disable-next-line. containerStyle: PropTypes.object, // Function that executes when the mouse moves over the children. diff --git a/src/components/Tooltip/TooltipRenderedOnPageBody.js b/src/components/Tooltip/TooltipRenderedOnPageBody.js new file mode 100644 index 000000000000..1f48f026cfb5 --- /dev/null +++ b/src/components/Tooltip/TooltipRenderedOnPageBody.js @@ -0,0 +1,55 @@ +import React, {memo} from 'react'; +import PropTypes from 'prop-types'; +import {Animated, Text, View} from 'react-native'; +import ReactDOM from 'react-dom'; + +const propTypes = { + // Style for Animation + // eslint-disable-next-line react/forbid-prop-types + animationStyle: PropTypes.object.isRequired, + + // Syle for Tooltip wrapper + // eslint-disable-next-line react/forbid-prop-types + tooltipWrapperStyle: PropTypes.object.isRequired, + + // Style for the text rendered inside tooltip + // eslint-disable-next-line react/forbid-prop-types + tooltipTextStyle: PropTypes.object.isRequired, + + // Style for the Tooltip pointer Wrapper + // eslint-disable-next-line react/forbid-prop-types + pointerWrapperStyle: PropTypes.object.isRequired, + + // Style for the Tooltip pointer + // eslint-disable-next-line react/forbid-prop-types + pointerStyle: PropTypes.object.isRequired, + + // Callback to set the Ref to the Tooltip + setTooltipRef: PropTypes.func.isRequired, + + // Text to be shown in the tooltip + text: PropTypes.string.isRequired, + + // Callback to be used to calulate the width and height of tooltip + measureTooltip: PropTypes.func.isRequired, +}; + +const defaultProps = {}; + +const TooltipRenderedOnPageBody = props => ReactDOM.createPortal( + + {props.text} + + + + , + document.querySelector('body'), +); + +TooltipRenderedOnPageBody.propTypes = propTypes; +TooltipRenderedOnPageBody.defaultProps = defaultProps; +export default memo(TooltipRenderedOnPageBody); diff --git a/src/components/Tooltip/index.js b/src/components/Tooltip/index.js index 79b646905f1d..7b62d8e6d9d0 100644 --- a/src/components/Tooltip/index.js +++ b/src/components/Tooltip/index.js @@ -1,7 +1,7 @@ import _ from 'underscore'; import React, {PureComponent} from 'react'; -import {Animated, Text, View} from 'react-native'; -import ReactDOM from 'react-dom'; +import {Animated, View} from 'react-native'; +import TooltipRenderedOnPageBody from './TooltipRenderedOnPageBody'; import Hoverable from '../Hoverable'; import withWindowDimensions from '../withWindowDimensions'; import getTooltipStyles from '../../styles/getTooltipStyles'; @@ -41,7 +41,6 @@ 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); } componentDidMount() { @@ -132,16 +131,18 @@ class Tooltip extends PureComponent { this.getWrapperPosition() .then(({ x, y, width, height, - }) => this.setState({ - wrapperWidth: width, - wrapperHeight: height, - xOffset: x, - yOffset: y, - })); - Animated.timing(this.animation, { - toValue: 1, - duration: 140, - }).start(); + }) => { + this.setState({ + wrapperWidth: width, + wrapperHeight: height, + xOffset: x, + yOffset: y, + }); + Animated.timing(this.animation, { + toValue: 1, + duration: 140, + }).start(); + }); } /** @@ -154,7 +155,7 @@ class Tooltip extends PureComponent { }).start(); } - createPortal() { + render() { const { animationStyle, tooltipWrapperStyle, @@ -170,32 +171,21 @@ class Tooltip extends PureComponent { this.state.wrapperHeight, this.state.tooltipWidth, this.state.tooltipHeight, - _.isFunction(this.props.shiftHorizontal) - ? this.props.shiftHorizontal() - : this.props.shiftHorizontal, - _.isFunction(this.props.shiftVertical) - ? this.props.shiftVertical() - : this.props.shiftVertical, - ); - return ReactDOM.createPortal( - this.tooltip = el} - onLayout={this.measureTooltip} - style={[tooltipWrapperStyle, animationStyle]} - > - {this.props.text} - - - - , - document.querySelector('body'), + _.result(this.props, 'shiftHorizontal'), + _.result(this.props, 'shiftVertical'), ); - } - - render() { return ( <> - {this.createPortal()} + this.tooltip = el} + measureTooltip={this.measureTooltip} + text={this.props.text} + /> props.children; Tooltip.propTypes = propTypes; diff --git a/src/libs/hasEllipsis/index.js b/src/libs/hasEllipsis/index.js index dcadd632d702..d18af237be35 100644 --- a/src/libs/hasEllipsis/index.js +++ b/src/libs/hasEllipsis/index.js @@ -1,5 +1,5 @@ /** - * Does an elment has ellipsis + * Does an elment have ellipsis * * @param {HTMLElement} el Element to check * @returns {Boolean} diff --git a/src/pages/home/HeaderView.js b/src/pages/home/HeaderView.js index ad51f31d0f12..605c68d09026 100644 --- a/src/pages/home/HeaderView.js +++ b/src/pages/home/HeaderView.js @@ -17,6 +17,7 @@ import ROUTES from '../../ROUTES'; import {getReportParticipantsTitle} from '../../libs/reportUtils'; import OptionRowTitle from './sidebar/OptionRowTitle'; import {getPersonalDetailsForLogins} from '../../libs/OptionsListUtils'; +import {participantPropTypes} from './sidebar/optionPropTypes'; const propTypes = { // Toggles the navigationMenu open and closed @@ -28,7 +29,7 @@ const propTypes = { // Name of the report reportName: PropTypes.string, - // list of primarylogins of participants of the report + // List of primarylogins of participants of the report participants: PropTypes.arrayOf(PropTypes.string), // ID of the report @@ -38,6 +39,9 @@ const propTypes = { isPinned: PropTypes.bool, }), + // Personal details of all the users + personalDetails: PropTypes.arrayOf(participantPropTypes).isRequired, + ...windowDimensionsPropTypes, }; @@ -87,7 +91,6 @@ const HeaderView = (props) => { option={reportOption} tooltipEnabled numberOfLines={2} - tooltipContainerStyle={styles.dInline} style={[styles.headerText]} /> diff --git a/src/pages/home/sidebar/OptionRow.js b/src/pages/home/sidebar/OptionRow.js index 589c54a3ce2f..ce55f3d76b2a 100644 --- a/src/pages/home/sidebar/OptionRow.js +++ b/src/pages/home/sidebar/OptionRow.js @@ -8,7 +8,7 @@ import { StyleSheet, } from 'react-native'; import styles from '../../../styles/styles'; -import optionPropTypes from './optionPropTypes'; +import {optionPropTypes} from './optionPropTypes'; import Icon from '../../../components/Icon'; import {Pencil, PinCircle, Checkmark} from '../../../components/Icon/Expensicons'; import MultipleAvatars from '../../../components/MultipleAvatars'; diff --git a/src/pages/home/sidebar/OptionRowTitle/OptionRowTitleProps.js b/src/pages/home/sidebar/OptionRowTitle/OptionRowTitleProps.js index 6bf2e0039be4..3bc7e93d15a2 100644 --- a/src/pages/home/sidebar/OptionRowTitle/OptionRowTitleProps.js +++ b/src/pages/home/sidebar/OptionRowTitle/OptionRowTitleProps.js @@ -1,40 +1,26 @@ import PropTypes from 'prop-types'; -import styles from '../../../../styles/styles'; +import {participantPropTypes} from '../optionPropTypes'; const propTypes = { - // styles of the title + // Styles of the title style: PropTypes.arrayOf(PropTypes.any), - // lines before wrapping + // Lines before wrapping numberOfLines: PropTypes.number, // Is tooltip needed? // When true, triggers complex title rendering tooltipEnabled: PropTypes.bool, - // Styles for the tooltip - tooltipContainerStyle: PropTypes.object, - // Option to allow the user to choose from can be type 'report' or 'user' option: PropTypes.shape({ // The full name of the user if available, otherwise the login (email/phone number) of the user text: PropTypes.string.isRequired, - // list of particiapants of the report - participantsList: PropTypes.arrayOf( - PropTypes.shape({ - // primary login of participant - login: PropTypes.string, - - // display Name of participant - displayName: PropTypes.string, - - // avatar url of participant - avatar: PropTypes.string, - }), - ).isRequired, + // List of particiapants of the report + participantsList: PropTypes.arrayOf(participantPropTypes).isRequired, - // text to show for tooltip + // Text to show for tooltip tooltipText: PropTypes.string, }).isRequired, @@ -43,7 +29,6 @@ const propTypes = { const defaultProps = { style: null, tooltipEnabled: false, - tooltipContainerStyle: styles.dInline, numberOfLines: 1, }; export { diff --git a/src/pages/home/sidebar/OptionRowTitle/index.js b/src/pages/home/sidebar/OptionRowTitle/index.js index 07dbe968facb..2c3c5fb91c3d 100644 --- a/src/pages/home/sidebar/OptionRowTitle/index.js +++ b/src/pages/home/sidebar/OptionRowTitle/index.js @@ -22,7 +22,6 @@ class OptionRowTitle extends PureComponent { }; this.state = { isEllipsisActive: false, - containerLayout: null, }; this.setContainerLayout = this.setContainerLayout.bind(this); this.getTooltipShiftX = this.getTooltipShiftX.bind(this); @@ -34,83 +33,100 @@ class OptionRowTitle extends PureComponent { }); } + /** + * Set the container layout for post calculations + * + * @param {*} {nativeEvent} + * @memberof OptionRowTitle + */ setContainerLayout({nativeEvent}) { - this.setState({ - containerLayout: nativeEvent.layout, - }); + this.containerLayout = nativeEvent.layout; } + /** + * We may need to shit the Tooltip horizontally as the some of the inline text wraps well with ellipsis + * .But their container node overflows the parent view which causes the tooltip to be misplaced. + * + * So we shift it by calculating it as follows: + * 1. We get the container layout and take the Child inline text node. + * 2. Now we get the tooltip original position. + * 3. If inline node's right edge is overflowing the containe's right edge, we set the tooltip to the center + * of the distance between the left edge of the inline node and right edge of the container. + * @param {Number} index Used to get the Ref to the node at the current index. + * @returns {Number} Distance to shift the tooltip horizontally + * @memberof OptionRowTitle + */ getTooltipShiftX(index) { - const {containerLayout} = this.state; - - // only shift when containerLayout or Refs to text node is available . - if (!containerLayout || !this.cRefs[index]) { + // Only shift when containerLayout or Refs to text node is available . + if (!this.containerLayout || !this.cRefs[index]) { return; } - const {width: cWidth, left: cLeft} = containerLayout; + const {width: containerWidth, left: containerLeft} = this.containerLayout; - // we have to return the value as Number so we can't use `measureWindow` which takes a callback - const {width: tWidth, left: tLeft} = this.cRefs[index].getBoundingClientRect(); - const toolX = (tWidth / 2) + tLeft; - const cRight = cWidth + cLeft; - const tRight = tWidth + tLeft; - const newToolX = tLeft + ((cRight - tLeft) / 2); + // We have to return the value as Number so we can't use `measureWindow` which takes a callback + const {width: textNodeWidth, left: textNodeLeft} = this.cRefs[index].getBoundingClientRect(); + const tooltipX = (textNodeWidth / 2) + textNodeLeft; + const containerRight = containerWidth + containerLeft; + const textNodeRight = textNodeWidth + textNodeLeft; + const newToolX = textNodeLeft + ((containerRight - textNodeLeft) / 2); - // when text right end is beyond the Container Right end - return tRight > cRight ? -(toolX - newToolX) : 0; + // When text right end is beyond the Container Right end + return textNodeRight > containerRight ? -(tooltipX - newToolX) : 0; } render() { const { - option, style, tooltipEnabled, numberOfLines, tooltipContainerStyle, + option, style, tooltipEnabled, numberOfLines, } = this.props; - const {isEllipsisActive} = this.state; - if (tooltipEnabled) { - return ( - - // Tokenization of string only support 1 numberofLines on Web - - {_.map(option.participantsList, (participant, index) => { - const setChildRef = this.setOptionChildRef(index); - return ( - - this.getTooltipShiftX(index)} - > - - {participant.displayName} - - - {index < option.participantsList.length - 1 ? ,  : null} - - ); - })} - { - option.participantsList.length > 1 && isEllipsisActive - ? ( - - - {/* there is some Gap for real ellipsis so we are adding 4 `.` to cover */} - .... - - - ) : null - } - - ); + if (!tooltipEnabled) { + return {option.text}; } + return ( - return {option.text}; + // Tokenization of string only support 1 numberofLines on Web + + {_.map(option.participantsList, (participant, index) => { + // We need to get the refs to all the names which will be used to correct + // the horizontal position of the tooltip + const setChildRef = this.setOptionChildRef(index); + return ( + + this.getTooltipShiftX(index)} + > + + {participant.displayName} + + + {index < option.participantsList.length - 1 + ? ,  + : null} + + ); + })} + { + option.participantsList.length > 1 && this.state.isEllipsisActive + ? ( + + + {/* There is some Gap for real ellipsis so we are adding 4 `.` to cover */} + .... + + + ) : null + } + + ); } } OptionRowTitle.propTypes = propTypes; diff --git a/src/pages/home/sidebar/OptionRowTitle/index.native.js b/src/pages/home/sidebar/OptionRowTitle/index.native.js index a32cc716035e..b863a115579a 100644 --- a/src/pages/home/sidebar/OptionRowTitle/index.native.js +++ b/src/pages/home/sidebar/OptionRowTitle/index.native.js @@ -1,11 +1,8 @@ -/* eslint-disable react/forbid-prop-types */ -import React, {memo} from 'react'; -import { - Text, -} from 'react-native'; +// As we don't have to show tooltips of the Native platform so we simply render the option title which wraps. +import React from 'react'; +import {Text} from 'react-native'; import {propTypes, defaultProps} from './OptionRowTitleProps'; - const OptionRowTitle = ({ style, option, @@ -20,4 +17,4 @@ OptionRowTitle.propTypes = propTypes; OptionRowTitle.defaultProps = defaultProps; OptionRowTitle.displayName = 'OptionRowTitle'; -export default memo(OptionRowTitle); +export default OptionRowTitle; diff --git a/src/pages/home/sidebar/SidebarLinks.js b/src/pages/home/sidebar/SidebarLinks.js index f578ab951100..2f64b227b13e 100644 --- a/src/pages/home/sidebar/SidebarLinks.js +++ b/src/pages/home/sidebar/SidebarLinks.js @@ -18,6 +18,7 @@ import {getSidebarOptions} from '../../../libs/OptionsListUtils'; import {getDefaultAvatar} from '../../../libs/actions/PersonalDetails'; import KeyboardSpacer from '../../../components/KeyboardSpacer'; import CONST from '../../../CONST'; +import {participantPropTypes} from './optionPropTypes'; const propTypes = { // Toggles the navigation menu open and closed @@ -41,11 +42,7 @@ const propTypes = { draftComments: PropTypes.objectOf(PropTypes.string), // List of users' personal details - personalDetails: PropTypes.objectOf(PropTypes.shape({ - login: PropTypes.string.isRequired, - avatar: PropTypes.string.isRequired, - displayName: PropTypes.string.isRequired, - })), + personalDetails: PropTypes.objectOf(participantPropTypes), // The personal details of the person who is logged in myPersonalDetails: PropTypes.shape({ diff --git a/src/pages/home/sidebar/optionPropTypes.js b/src/pages/home/sidebar/optionPropTypes.js index 65a9057d0808..17485920df20 100644 --- a/src/pages/home/sidebar/optionPropTypes.js +++ b/src/pages/home/sidebar/optionPropTypes.js @@ -1,25 +1,25 @@ import PropTypes from 'prop-types'; -export default PropTypes.shape({ +const participantPropTypes = PropTypes.shape({ + // Primary login of participant + login: PropTypes.string, + + // Display Name of participant + displayName: PropTypes.string, + + // Avatar url of participant + avatar: PropTypes.string, +}); + +const optionPropTypes = PropTypes.shape({ // The full name of the user if available, otherwise the login (email/phone number) of the user text: PropTypes.string.isRequired, // Subtitle to show under report displayName, mostly lastMessageText of the report alternateText: PropTypes.string.isRequired, - // list of particiapants of the report - participantsList: PropTypes.arrayOf( - PropTypes.shape({ - // primary login of participant - login: PropTypes.string, - - // display Name of participant - displayName: PropTypes.string, - - // avatar url of participant - avatar: PropTypes.string, - }), - ).isRequired, + // List of participants of the report + participantsList: PropTypes.arrayOf(participantPropTypes).isRequired, // The array URLs of the person's avatar icon: PropTypes.arrayOf(PropTypes.string), @@ -30,6 +30,12 @@ export default PropTypes.shape({ // The option key provided to FlatList keyExtractor keyForList: PropTypes.string, - // text to show for tooltip + // Text to show for tooltip tooltipText: PropTypes.string, }); + + +export { + participantPropTypes, + optionPropTypes, +}; From 21b51ee092c10caea27d5690ca7c00f355bae9fd Mon Sep 17 00:00:00 2001 From: Rajat Parashar Date: Wed, 17 Mar 2021 01:48:18 +0530 Subject: [PATCH 14/16] refactor Code && showtooltip glitch --- src/components/Tooltip/index.js | 17 ++++++++++++---- src/pages/home/HeaderView.js | 4 ++-- .../home/sidebar/OptionRowTitle/index.js | 20 +++++++++---------- 3 files changed, 24 insertions(+), 17 deletions(-) diff --git a/src/components/Tooltip/index.js b/src/components/Tooltip/index.js index 7b62d8e6d9d0..cd38fc3ce0fc 100644 --- a/src/components/Tooltip/index.js +++ b/src/components/Tooltip/index.js @@ -34,6 +34,7 @@ class Tooltip extends PureComponent { this.tooltip = null; this.isComponentMounted = false; + this.shouldStartShowAnimation = false; this.animation = new Animated.Value(0); this.getWrapperPosition = this.getWrapperPosition.bind(this); @@ -126,6 +127,8 @@ class Tooltip extends PureComponent { * Display the tooltip in an animation. */ showTooltip() { + this.shouldStartShowAnimation = true; + // We have to dynamically calculate the position here as tooltip could have been rendered on some elments // that has changed its position this.getWrapperPosition() @@ -138,10 +141,15 @@ class Tooltip extends PureComponent { xOffset: x, yOffset: y, }); - Animated.timing(this.animation, { - toValue: 1, - duration: 140, - }).start(); + + // We may need this check due to the reason that the animation start will fire async + // and hideTooltip could fire before it thus keeping the Tooltip visible + if (this.shouldStartShowAnimation) { + Animated.timing(this.animation, { + toValue: 1, + duration: 140, + }).start(); + } }); } @@ -149,6 +157,7 @@ class Tooltip extends PureComponent { * Hide the tooltip in an animation. */ hideTooltip() { + this.shouldStartShowAnimation = false; Animated.timing(this.animation, { toValue: 0, duration: 140, diff --git a/src/pages/home/HeaderView.js b/src/pages/home/HeaderView.js index 605c68d09026..8304509eb351 100644 --- a/src/pages/home/HeaderView.js +++ b/src/pages/home/HeaderView.js @@ -68,7 +68,7 @@ const HeaderView = (props) => { )} - {props.report && props.report.reportName ? ( + {props.report && props.report.reportName && ( { - ) : null} + )} ); diff --git a/src/pages/home/sidebar/OptionRowTitle/index.js b/src/pages/home/sidebar/OptionRowTitle/index.js index 2c3c5fb91c3d..9f74b0a632cb 100644 --- a/src/pages/home/sidebar/OptionRowTitle/index.js +++ b/src/pages/home/sidebar/OptionRowTitle/index.js @@ -114,17 +114,15 @@ class OptionRowTitle extends PureComponent { ); })} - { - option.participantsList.length > 1 && this.state.isEllipsisActive - ? ( - - - {/* There is some Gap for real ellipsis so we are adding 4 `.` to cover */} - .... - - - ) : null - } + {option.participantsList.length > 1 && this.state.isEllipsisActive + && ( + + + {/* There is some Gap for real ellipsis so we are adding 4 `.` to cover */} + .... + + + )} ); } From 237968cdaae6645af0b38a14a60bc9ceea62c5a9 Mon Sep 17 00:00:00 2001 From: Rajat Parashar Date: Fri, 19 Mar 2021 16:25:25 +0530 Subject: [PATCH 15/16] removed unnecessary line Co-authored-by: Rocio Perez --- src/pages/home/sidebar/optionPropTypes.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/pages/home/sidebar/optionPropTypes.js b/src/pages/home/sidebar/optionPropTypes.js index 17485920df20..ec2881f790b5 100644 --- a/src/pages/home/sidebar/optionPropTypes.js +++ b/src/pages/home/sidebar/optionPropTypes.js @@ -34,7 +34,6 @@ const optionPropTypes = PropTypes.shape({ tooltipText: PropTypes.string, }); - export { participantPropTypes, optionPropTypes, From 996ae0ebd76bf975e679594e2266ba0f1096be5b Mon Sep 17 00:00:00 2001 From: Rajat Parashar Date: Sat, 20 Mar 2021 03:31:46 +0530 Subject: [PATCH 16/16] refactor --- .../Hoverable/HoverablePropTypes.js | 2 +- .../Tooltip/TooltipRenderedOnPageBody.js | 7 +++ .../home/sidebar/OptionRowTitle/index.js | 57 ++++++++----------- 3 files changed, 31 insertions(+), 35 deletions(-) diff --git a/src/components/Hoverable/HoverablePropTypes.js b/src/components/Hoverable/HoverablePropTypes.js index b92d4e0eb886..9125e9c8669a 100644 --- a/src/components/Hoverable/HoverablePropTypes.js +++ b/src/components/Hoverable/HoverablePropTypes.js @@ -8,7 +8,7 @@ const propTypes = { ]).isRequired, // Styles to be assigned to the Hoverable Container - // eslint-disable-next-line. + // eslint-disable-next-line react/forbid-prop-types containerStyle: PropTypes.object, // Function that executes when the mouse moves over the children. diff --git a/src/components/Tooltip/TooltipRenderedOnPageBody.js b/src/components/Tooltip/TooltipRenderedOnPageBody.js index 1f48f026cfb5..1611147cc08b 100644 --- a/src/components/Tooltip/TooltipRenderedOnPageBody.js +++ b/src/components/Tooltip/TooltipRenderedOnPageBody.js @@ -52,4 +52,11 @@ const TooltipRenderedOnPageBody = props => ReactDOM.createPortal( TooltipRenderedOnPageBody.propTypes = propTypes; TooltipRenderedOnPageBody.defaultProps = defaultProps; +TooltipRenderedOnPageBody.displayName = 'TooltipRenderedOnPageBody'; + +// Props will change frequently. +// On every tooltip hover, we update the position in state which will result in re-rendering. +// We also update the state on layout changes which will be triggered often. +// There will be n number of tooltip components in the page. +// Its good to memorize this one. export default memo(TooltipRenderedOnPageBody); diff --git a/src/pages/home/sidebar/OptionRowTitle/index.js b/src/pages/home/sidebar/OptionRowTitle/index.js index 9f74b0a632cb..9f887f2a65d2 100644 --- a/src/pages/home/sidebar/OptionRowTitle/index.js +++ b/src/pages/home/sidebar/OptionRowTitle/index.js @@ -12,14 +12,8 @@ import hasEllipsis from '../../../../libs/hasEllipsis'; class OptionRowTitle extends PureComponent { constructor(props) { super(props); - this.ref = null; - this.cRefs = []; - this.setContainerRef = (ref) => { - this.ref = ref; - }; - this.setOptionChildRef = index => (ref) => { - this.cRefs[index] = ref; - }; + this.containerRef = null; + this.childRefs = []; this.state = { isEllipsisActive: false, }; @@ -29,7 +23,7 @@ class OptionRowTitle extends PureComponent { componentDidMount() { this.setState({ - isEllipsisActive: this.ref && hasEllipsis(this.ref), + isEllipsisActive: this.containerRef && hasEllipsis(this.containerRef), }); } @@ -58,13 +52,13 @@ class OptionRowTitle extends PureComponent { */ getTooltipShiftX(index) { // Only shift when containerLayout or Refs to text node is available . - if (!this.containerLayout || !this.cRefs[index]) { + if (!this.containerLayout || !this.childRefs[index]) { return; } const {width: containerWidth, left: containerLeft} = this.containerLayout; // We have to return the value as Number so we can't use `measureWindow` which takes a callback - const {width: textNodeWidth, left: textNodeLeft} = this.cRefs[index].getBoundingClientRect(); + const {width: textNodeWidth, left: textNodeLeft} = this.childRefs[index].getBoundingClientRect(); const tooltipX = (textNodeWidth / 2) + textNodeLeft; const containerRight = containerWidth + containerLeft; const textNodeRight = textNodeWidth + textNodeLeft; @@ -90,30 +84,25 @@ class OptionRowTitle extends PureComponent { style={[style, styles.optionDisplayNameTooltipWrapper]} onLayout={this.setContainerLayout} numberOfLines={1} - ref={this.setContainerRef} + ref={el => this.containerRef = el} > - {_.map(option.participantsList, (participant, index) => { - // We need to get the refs to all the names which will be used to correct - // the horizontal position of the tooltip - const setChildRef = this.setOptionChildRef(index); - return ( - - this.getTooltipShiftX(index)} - > - - {participant.displayName} - - - {index < option.participantsList.length - 1 - ? ,  - : null} - - ); - })} + {_.map(option.participantsList, (participant, index) => ( + + this.getTooltipShiftX(index)} + > + {/* // We need to get the refs to all the names which will be used to correct + the horizontal position of the tooltip */} + this.childRefs[index] = el}> + {participant.displayName} + + + {index < option.participantsList.length - 1 && , } + + ))} {option.participantsList.length > 1 && this.state.isEllipsisActive && (