From 6caff3ca9a49498cda995c14096f5202c1aa3bf3 Mon Sep 17 00:00:00 2001 From: LucioChavezFuentes Date: Mon, 21 Mar 2022 13:02:39 -0600 Subject: [PATCH 01/33] convert to class, get ref, add styles --- .../Tooltip/TooltipRenderedOnPageBody.js | 90 ++++++++++++------- src/styles/getTooltipStyles.js | 6 ++ 2 files changed, 63 insertions(+), 33 deletions(-) diff --git a/src/components/Tooltip/TooltipRenderedOnPageBody.js b/src/components/Tooltip/TooltipRenderedOnPageBody.js index 3ed7c472883b..1867af355478 100644 --- a/src/components/Tooltip/TooltipRenderedOnPageBody.js +++ b/src/components/Tooltip/TooltipRenderedOnPageBody.js @@ -51,39 +51,63 @@ const propTypes = { const defaultProps = {}; -const TooltipRenderedOnPageBody = (props) => { - const { - animationStyle, - tooltipWrapperStyle, - tooltipTextStyle, - pointerWrapperStyle, - pointerStyle, - } = getTooltipStyles( - props.animation, - props.windowWidth, - props.xOffset, - props.yOffset, - props.wrapperWidth, - props.wrapperHeight, - props.tooltipWidth, - props.tooltipHeight, - props.shiftHorizontal, - props.shiftVertical, - ); - return ReactDOM.createPortal( - - {props.text} - - - - , - document.querySelector('body'), - ); -}; +// ! 1 +// {wordsToShow.map(word => {word + " "})} +// ! 2 +// {wordsToShow.join(' ')} + +class TooltipRenderedOnPageBody extends React.Component { + + constructor(props) { + super(props); + } + state = { + tooltipTextWidth: 300, + } + + resizeTooltip(el) { + console.log(el); + } + + render() { + const { + animationStyle, + tooltipWrapperStyle, + tooltipTextStyle, + pointerWrapperStyle, + pointerStyle, + } = getTooltipStyles( + this.props.animation, + this.props.windowWidth, + this.props.xOffset, + this.props.yOffset, + this.props.wrapperWidth, + this.props.wrapperHeight, + this.props.tooltipWidth, + this.props.tooltipHeight, + this.props.shiftHorizontal, + this.props.shiftVertical, + ); + const maximumWords = 4; + const wordsToShow = this.props.text.split(' ').slice(0, maximumWords); + + return ReactDOM.createPortal( + + this.resizeTooltip(ref)}> + {wordsToShow.join(' ')} + + + + + , + document.querySelector('body'), + ); + } +} TooltipRenderedOnPageBody.propTypes = propTypes; TooltipRenderedOnPageBody.defaultProps = defaultProps; diff --git a/src/styles/getTooltipStyles.js b/src/styles/getTooltipStyles.js index f39fe2c42311..4a81abf72091 100644 --- a/src/styles/getTooltipStyles.js +++ b/src/styles/getTooltipStyles.js @@ -109,6 +109,7 @@ export default function getTooltipStyles( ...tooltipVerticalPadding, ...spacing.ph2, zIndex: variables.tooltipzIndex, + maxWidth: 300, // Because it uses fixed positioning, the top-left corner of the tooltip is aligned // with the top-left corner of the window by default. @@ -144,6 +145,11 @@ export default function getTooltipStyles( color: themeColors.textReversed, fontFamily: fontFamily.GTA, fontSize: tooltipFontSize, + overflowWrap: 'normal', + // whiteSpace: 'nowrap', + overflow: 'hidden', + textOverflow: 'ellipsis', + display: 'inline', }, pointerWrapperStyle: { position: 'fixed', From 2674c63bf15a22b402735e5bc3a8faff7a083127 Mon Sep 17 00:00:00 2001 From: LucioChavezFuentes Date: Tue, 22 Mar 2022 16:09:04 -0600 Subject: [PATCH 02/33] finish proposal --- src/components/MultipleAvatars.js | 2 +- .../Tooltip/TooltipRenderedOnPageBody.js | 132 +++++++++++------- src/components/Tooltip/index.js | 1 + src/components/Tooltip/tooltipPropTypes.js | 4 + src/styles/getTooltipStyles.js | 6 +- 5 files changed, 89 insertions(+), 56 deletions(-) diff --git a/src/components/MultipleAvatars.js b/src/components/MultipleAvatars.js index 993d36123c3a..f37bcc544dc1 100644 --- a/src/components/MultipleAvatars.js +++ b/src/components/MultipleAvatars.js @@ -80,7 +80,7 @@ const MultipleAvatars = (props) => { /> ) : ( - + diff --git a/src/components/Tooltip/TooltipRenderedOnPageBody.js b/src/components/Tooltip/TooltipRenderedOnPageBody.js index 1867af355478..e71fa082d275 100644 --- a/src/components/Tooltip/TooltipRenderedOnPageBody.js +++ b/src/components/Tooltip/TooltipRenderedOnPageBody.js @@ -4,6 +4,8 @@ import {Animated, View} from 'react-native'; import ReactDOM from 'react-dom'; import getTooltipStyles from '../../styles/getTooltipStyles'; import Text from '../Text'; +import variables from '../../styles/variables'; +import styles from '../../styles/styles'; const propTypes = { /** Window width */ @@ -47,66 +49,92 @@ const propTypes = { /** Callback to be used to calulate the width and height of tooltip */ measureTooltip: PropTypes.func.isRequired, + + /** Maximun amount of words the tooltip should show */ + maximumWords: PropTypes.number.isRequired, }; const defaultProps = {}; -// ! 1 -// {wordsToShow.map(word => {word + " "})} -// ! 2 -// {wordsToShow.join(' ')} - class TooltipRenderedOnPageBody extends React.Component { - - constructor(props) { - super(props); - } - state = { - tooltipTextWidth: 300, - } - - resizeTooltip(el) { - console.log(el); + constructor(props) { + super(props); + this.state = { + tooltipTextWidth: variables.sideBarWidth, + }; + + this.textRef = null; + this.getCorrectWidth = this.getCorrectWidth.bind(this); + } + + componentDidMount() { + this.setState({ + tooltipTextWidth: this.getCorrectWidth(this.textRef.offsetWidth), + }); + } + + getCorrectWidth(textWidth) { + const maxWidth = variables.sideBarWidth; + if (textWidth >= maxWidth) { + return maxWidth; } + const maxWidthDiffTextWidth = maxWidth - textWidth; - render() { - const { - animationStyle, - tooltipWrapperStyle, - tooltipTextStyle, - pointerWrapperStyle, - pointerStyle, - } = getTooltipStyles( - this.props.animation, - this.props.windowWidth, - this.props.xOffset, - this.props.yOffset, - this.props.wrapperWidth, - this.props.wrapperHeight, - this.props.tooltipWidth, - this.props.tooltipHeight, - this.props.shiftHorizontal, - this.props.shiftVertical, - ); - const maximumWords = 4; - const wordsToShow = this.props.text.split(' ').slice(0, maximumWords); - - return ReactDOM.createPortal( - - this.resizeTooltip(ref)}> - {wordsToShow.join(' ')} - - - - - , - document.querySelector('body'), - ); + // This operation will serve us to avoid adding more width than maxwidth + // Get padding of tooltipWrapper and sum the right and left + const leftRighPadding = styles.p2.padding * 2; + if (leftRighPadding > maxWidthDiffTextWidth) { + return textWidth + maxWidthDiffTextWidth; } + return textWidth + leftRighPadding; + } + + render() { + const { + animationStyle, + tooltipWrapperStyle, + tooltipTextStyle, + pointerWrapperStyle, + pointerStyle, + } = getTooltipStyles( + this.props.animation, + this.props.windowWidth, + this.props.xOffset, + this.props.yOffset, + this.props.wrapperWidth, + this.props.wrapperHeight, + this.props.tooltipWidth, + this.props.tooltipHeight, + this.props.shiftHorizontal, + this.props.shiftVertical, + this.state.tooltipTextWidth, + ); + const maximumWords = this.props.maximumWords; + const wordsProvided = this.props.text.split(' '); + + // Only show the amount words we want to see no matter the amount of lines needed to fit them + // This will give us an accurate width of visible text using ref.offsetWidth + // offsetWidth is accurate to visible text width if there no height overflow, + // otherwise will give us a longer width if there's longer line hidden in overflow + const wordsToShow = wordsProvided.slice(0, maximumWords); + + return ReactDOM.createPortal( + + + this.textRef = ref}>{wordsToShow.join(' ')} + {maximumWords < wordsProvided.length ? ... : '' } + + + + + , + document.querySelector('body'), + ); + } } TooltipRenderedOnPageBody.propTypes = propTypes; diff --git a/src/components/Tooltip/index.js b/src/components/Tooltip/index.js index 942029b4ab46..76ad28007f47 100644 --- a/src/components/Tooltip/index.js +++ b/src/components/Tooltip/index.js @@ -207,6 +207,7 @@ class Tooltip extends PureComponent { shiftVertical={_.result(this.props, 'shiftVertical')} measureTooltip={this.measureTooltip} text={this.props.text} + maximumWords={this.props.maximumWords} /> )} Date: Tue, 22 Mar 2022 19:17:07 -0600 Subject: [PATCH 03/33] move getCorrectWidth func --- .../Tooltip/TooltipRenderedOnPageBody.js | 21 ++-------------- src/styles/getTooltipStyles.js | 25 ++++++++++++++++++- 2 files changed, 26 insertions(+), 20 deletions(-) diff --git a/src/components/Tooltip/TooltipRenderedOnPageBody.js b/src/components/Tooltip/TooltipRenderedOnPageBody.js index e71fa082d275..3f5cde8c9b75 100644 --- a/src/components/Tooltip/TooltipRenderedOnPageBody.js +++ b/src/components/Tooltip/TooltipRenderedOnPageBody.js @@ -5,7 +5,6 @@ import ReactDOM from 'react-dom'; import getTooltipStyles from '../../styles/getTooltipStyles'; import Text from '../Text'; import variables from '../../styles/variables'; -import styles from '../../styles/styles'; const propTypes = { /** Window width */ @@ -60,35 +59,19 @@ class TooltipRenderedOnPageBody extends React.Component { constructor(props) { super(props); this.state = { + // Set maxWidth as initial so we can get width of word wrapped text tooltipTextWidth: variables.sideBarWidth, }; this.textRef = null; - this.getCorrectWidth = this.getCorrectWidth.bind(this); } componentDidMount() { this.setState({ - tooltipTextWidth: this.getCorrectWidth(this.textRef.offsetWidth), + tooltipTextWidth: this.textRef.offsetWidth, }); } - getCorrectWidth(textWidth) { - const maxWidth = variables.sideBarWidth; - if (textWidth >= maxWidth) { - return maxWidth; - } - const maxWidthDiffTextWidth = maxWidth - textWidth; - - // This operation will serve us to avoid adding more width than maxwidth - // Get padding of tooltipWrapper and sum the right and left - const leftRighPadding = styles.p2.padding * 2; - if (leftRighPadding > maxWidthDiffTextWidth) { - return textWidth + maxWidthDiffTextWidth; - } - return textWidth + leftRighPadding; - } - render() { const { animationStyle, diff --git a/src/styles/getTooltipStyles.js b/src/styles/getTooltipStyles.js index 3ff32b373fc4..44549bee2253 100644 --- a/src/styles/getTooltipStyles.js +++ b/src/styles/getTooltipStyles.js @@ -49,6 +49,29 @@ function computeHorizontalShift(windowWidth, xOffset, componentWidth, tooltipWid return 0; } +/** + * Correct tooltipWrapper width according to visible inner text offssetWidth + * + * @param {Number} textWidth - The inner text offsetWidth of Tooltip + * + * @returns {Number} + */ +function getCorrectWidth(textWidth) { + const maxWidth = variables.sideBarWidth; + if (textWidth >= maxWidth) { + return maxWidth; + } + const maxWidthDiffTextWidth = maxWidth - textWidth; + + // This operation will serve us to avoid adding more width than maxwidth + // Get padding of tooltipWrapper and sum the right and left + const leftRighPadding = styles.p2.padding * 2; + if (leftRighPadding > maxWidthDiffTextWidth) { + return textWidth + maxWidthDiffTextWidth; + } + return textWidth + leftRighPadding; +} + /** * Generate styles for the tooltip component. * @@ -111,7 +134,7 @@ export default function getTooltipStyles( ...tooltipVerticalPadding, ...spacing.ph2, zIndex: variables.tooltipzIndex, - maxWidth: tooltipTextWidth, + maxWidth: getCorrectWidth(tooltipTextWidth), // Because it uses fixed positioning, the top-left corner of the tooltip is aligned // with the top-left corner of the window by default. From 3c506fa38d6e14b16a0ea6e53fbf62b1002382d3 Mon Sep 17 00:00:00 2001 From: LucioChavezFuentes Date: Wed, 23 Mar 2022 11:30:23 -0600 Subject: [PATCH 04/33] create tooltipHorizontalPadding, add new param --- src/styles/getTooltipStyles.js | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/styles/getTooltipStyles.js b/src/styles/getTooltipStyles.js index 44549bee2253..50566ada685f 100644 --- a/src/styles/getTooltipStyles.js +++ b/src/styles/getTooltipStyles.js @@ -53,10 +53,11 @@ function computeHorizontalShift(windowWidth, xOffset, componentWidth, tooltipWid * Correct tooltipWrapper width according to visible inner text offssetWidth * * @param {Number} textWidth - The inner text offsetWidth of Tooltip + * @param {Number} tooltipHorizontalPadding - The horizontal padding set for wrapper tooltip * * @returns {Number} */ -function getCorrectWidth(textWidth) { +function getCorrectWidth(textWidth, tooltipHorizontalPadding) { const maxWidth = variables.sideBarWidth; if (textWidth >= maxWidth) { return maxWidth; @@ -64,8 +65,8 @@ function getCorrectWidth(textWidth) { const maxWidthDiffTextWidth = maxWidth - textWidth; // This operation will serve us to avoid adding more width than maxwidth - // Get padding of tooltipWrapper and sum the right and left - const leftRighPadding = styles.p2.padding * 2; + // Get horizontal padding of tooltipWrapper and sum the right and left + const leftRighPadding = tooltipHorizontalPadding * 2; if (leftRighPadding > maxWidthDiffTextWidth) { return textWidth + maxWidthDiffTextWidth; } @@ -117,6 +118,7 @@ export default function getTooltipStyles( const tooltipVerticalPadding = spacing.pv1; const tooltipFontSize = variables.fontSizeSmall; + const tooltipHorizontalPadding = spacing.ph2; return { animationStyle: { @@ -132,9 +134,9 @@ export default function getTooltipStyles( backgroundColor: themeColors.heading, borderRadius: variables.componentBorderRadiusSmall, ...tooltipVerticalPadding, - ...spacing.ph2, + ...tooltipHorizontalPadding, zIndex: variables.tooltipzIndex, - maxWidth: getCorrectWidth(tooltipTextWidth), + maxWidth: getCorrectWidth(tooltipTextWidth, tooltipHorizontalPadding.paddingHorizontal), // Because it uses fixed positioning, the top-left corner of the tooltip is aligned // with the top-left corner of the window by default. From c3aa57191d8372c6b8f0cc7ae6cd7629ab4dbd1f Mon Sep 17 00:00:00 2001 From: LucioChavezFuentes Date: Wed, 30 Mar 2022 14:21:34 -0600 Subject: [PATCH 05/33] remove unneeded maximumWords prop --- src/components/MultipleAvatars.js | 2 +- .../Tooltip/TooltipRenderedOnPageBody.js | 16 ++-------------- 2 files changed, 3 insertions(+), 15 deletions(-) diff --git a/src/components/MultipleAvatars.js b/src/components/MultipleAvatars.js index f37bcc544dc1..993d36123c3a 100644 --- a/src/components/MultipleAvatars.js +++ b/src/components/MultipleAvatars.js @@ -80,7 +80,7 @@ const MultipleAvatars = (props) => { /> ) : ( - + diff --git a/src/components/Tooltip/TooltipRenderedOnPageBody.js b/src/components/Tooltip/TooltipRenderedOnPageBody.js index 3f5cde8c9b75..3f0773d157a7 100644 --- a/src/components/Tooltip/TooltipRenderedOnPageBody.js +++ b/src/components/Tooltip/TooltipRenderedOnPageBody.js @@ -49,8 +49,6 @@ const propTypes = { /** Callback to be used to calulate the width and height of tooltip */ measureTooltip: PropTypes.func.isRequired, - /** Maximun amount of words the tooltip should show */ - maximumWords: PropTypes.number.isRequired, }; const defaultProps = {}; @@ -92,24 +90,14 @@ class TooltipRenderedOnPageBody extends React.Component { this.props.shiftVertical, this.state.tooltipTextWidth, ); - const maximumWords = this.props.maximumWords; - const wordsProvided = this.props.text.split(' '); - - // Only show the amount words we want to see no matter the amount of lines needed to fit them - // This will give us an accurate width of visible text using ref.offsetWidth - // offsetWidth is accurate to visible text width if there no height overflow, - // otherwise will give us a longer width if there's longer line hidden in overflow - const wordsToShow = wordsProvided.slice(0, maximumWords); - return ReactDOM.createPortal( - - this.textRef = ref}>{wordsToShow.join(' ')} - {maximumWords < wordsProvided.length ? ... : '' } + + this.textRef = ref}>{this.props.text} From 9e8271c3c4db8afe7b76b8f08892182fd4b45563 Mon Sep 17 00:00:00 2001 From: LucioChavezFuentes Date: Wed, 30 Mar 2022 19:07:43 -0600 Subject: [PATCH 06/33] simplify code --- .../Tooltip/TooltipRenderedOnPageBody.js | 2 +- src/styles/getTooltipStyles.js | 33 ++++--------------- 2 files changed, 7 insertions(+), 28 deletions(-) diff --git a/src/components/Tooltip/TooltipRenderedOnPageBody.js b/src/components/Tooltip/TooltipRenderedOnPageBody.js index 3f0773d157a7..bb400f1f1147 100644 --- a/src/components/Tooltip/TooltipRenderedOnPageBody.js +++ b/src/components/Tooltip/TooltipRenderedOnPageBody.js @@ -96,7 +96,7 @@ class TooltipRenderedOnPageBody extends React.Component { onLayout={this.props.measureTooltip} style={[tooltipWrapperStyle, animationStyle]} > - + this.textRef = ref}>{this.props.text} diff --git a/src/styles/getTooltipStyles.js b/src/styles/getTooltipStyles.js index 50566ada685f..44b47df0c5c7 100644 --- a/src/styles/getTooltipStyles.js +++ b/src/styles/getTooltipStyles.js @@ -49,30 +49,6 @@ function computeHorizontalShift(windowWidth, xOffset, componentWidth, tooltipWid return 0; } -/** - * Correct tooltipWrapper width according to visible inner text offssetWidth - * - * @param {Number} textWidth - The inner text offsetWidth of Tooltip - * @param {Number} tooltipHorizontalPadding - The horizontal padding set for wrapper tooltip - * - * @returns {Number} - */ -function getCorrectWidth(textWidth, tooltipHorizontalPadding) { - const maxWidth = variables.sideBarWidth; - if (textWidth >= maxWidth) { - return maxWidth; - } - const maxWidthDiffTextWidth = maxWidth - textWidth; - - // This operation will serve us to avoid adding more width than maxwidth - // Get horizontal padding of tooltipWrapper and sum the right and left - const leftRighPadding = tooltipHorizontalPadding * 2; - if (leftRighPadding > maxWidthDiffTextWidth) { - return textWidth + maxWidthDiffTextWidth; - } - return textWidth + leftRighPadding; -} - /** * Generate styles for the tooltip component. * @@ -118,7 +94,6 @@ export default function getTooltipStyles( const tooltipVerticalPadding = spacing.pv1; const tooltipFontSize = variables.fontSizeSmall; - const tooltipHorizontalPadding = spacing.ph2; return { animationStyle: { @@ -134,9 +109,13 @@ export default function getTooltipStyles( backgroundColor: themeColors.heading, borderRadius: variables.componentBorderRadiusSmall, ...tooltipVerticalPadding, - ...tooltipHorizontalPadding, + ...spacing.ph2, zIndex: variables.tooltipzIndex, - maxWidth: getCorrectWidth(tooltipTextWidth, tooltipHorizontalPadding.paddingHorizontal), + maxWidth: tooltipTextWidth >= variables.sideBarWidth + ? variables.sideBarWidth + + // Sum left and right padding + : tooltipTextWidth + (spacing.ph2.paddingHorizontal * 2), // Because it uses fixed positioning, the top-left corner of the tooltip is aligned // with the top-left corner of the window by default. From 5163c67656863f657d1e2beecd9582f03ea42177 Mon Sep 17 00:00:00 2001 From: LucioChavezFuentes Date: Thu, 31 Mar 2022 12:02:20 -0600 Subject: [PATCH 07/33] remove unneeded maximumWords prop --- src/components/Tooltip/index.js | 1 - src/components/Tooltip/tooltipPropTypes.js | 3 --- src/styles/getTooltipStyles.js | 6 +++--- 3 files changed, 3 insertions(+), 7 deletions(-) diff --git a/src/components/Tooltip/index.js b/src/components/Tooltip/index.js index 76ad28007f47..942029b4ab46 100644 --- a/src/components/Tooltip/index.js +++ b/src/components/Tooltip/index.js @@ -207,7 +207,6 @@ class Tooltip extends PureComponent { shiftVertical={_.result(this.props, 'shiftVertical')} measureTooltip={this.measureTooltip} text={this.props.text} - maximumWords={this.props.maximumWords} /> )} Date: Mon, 4 Apr 2022 14:11:14 -0500 Subject: [PATCH 08/33] insert maxWidth and numberLines props, move measureTooltip --- .../Tooltip/TooltipRenderedOnPageBody.js | 43 +++++++++++++------ src/components/Tooltip/index.js | 21 +-------- src/components/Tooltip/tooltipPropTypes.js | 9 ++++ src/styles/getTooltipStyles.js | 13 +++--- 4 files changed, 47 insertions(+), 39 deletions(-) diff --git a/src/components/Tooltip/TooltipRenderedOnPageBody.js b/src/components/Tooltip/TooltipRenderedOnPageBody.js index bb400f1f1147..fbaea286f035 100644 --- a/src/components/Tooltip/TooltipRenderedOnPageBody.js +++ b/src/components/Tooltip/TooltipRenderedOnPageBody.js @@ -4,7 +4,6 @@ import {Animated, View} from 'react-native'; import ReactDOM from 'react-dom'; import getTooltipStyles from '../../styles/getTooltipStyles'; import Text from '../Text'; -import variables from '../../styles/variables'; const propTypes = { /** Window width */ @@ -26,12 +25,6 @@ const propTypes = { /** The Height of the tooltip wrapper */ wrapperHeight: PropTypes.number.isRequired, - /** The width of the tooltip itself */ - tooltipWidth: PropTypes.number.isRequired, - - /** The Height of the tooltip itself */ - tooltipHeight: PropTypes.number.isRequired, - /** 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.isRequired, @@ -46,8 +39,11 @@ const propTypes = { /** 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, + /** number of pixels to set max-width on tooltip */ + maxWidth: PropTypes.number.isRequired, + + /** maximum number of lines to set on tooltip */ + numberOfLines: PropTypes.number.isRequired, }; @@ -58,10 +54,16 @@ class TooltipRenderedOnPageBody extends React.Component { super(props); this.state = { // Set maxWidth as initial so we can get width of word wrapped text - tooltipTextWidth: variables.sideBarWidth, + tooltipTextWidth: this.props.maxWidth, + + // The width and height of the tooltip itself + tooltipWidth: 0, + tooltipHeight: 0, }; this.textRef = null; + + this.measureTooltip = this.measureTooltip.bind(this); } componentDidMount() { @@ -70,6 +72,18 @@ class TooltipRenderedOnPageBody extends React.Component { }); } + /** + * Measure the size of the tooltip itself. + * + * @param {Object} nativeEvent + */ + measureTooltip({nativeEvent}) { + this.setState({ + tooltipWidth: nativeEvent.layout.width, + tooltipHeight: nativeEvent.layout.height, + }); + } + render() { const { animationStyle, @@ -84,19 +98,20 @@ class TooltipRenderedOnPageBody extends React.Component { this.props.yOffset, this.props.wrapperWidth, this.props.wrapperHeight, - this.props.tooltipWidth, - this.props.tooltipHeight, + this.state.tooltipWidth, + this.state.tooltipHeight, this.props.shiftHorizontal, this.props.shiftVertical, this.state.tooltipTextWidth, + this.props.maxWidth, ); return ReactDOM.createPortal( - + this.textRef = ref}>{this.props.text} diff --git a/src/components/Tooltip/index.js b/src/components/Tooltip/index.js index 942029b4ab46..8059e8e14f27 100644 --- a/src/components/Tooltip/index.js +++ b/src/components/Tooltip/index.js @@ -26,9 +26,6 @@ class Tooltip extends PureComponent { wrapperWidth: 0, wrapperHeight: 0, - // The width and height of the tooltip itself - tooltipWidth: 0, - tooltipHeight: 0, }; // The wrapper view containing the wrapped content along with the Tooltip itself. @@ -43,7 +40,6 @@ class Tooltip extends PureComponent { this.animation = new Animated.Value(0); this.getWrapperPosition = this.getWrapperPosition.bind(this); - this.measureTooltip = this.measureTooltip.bind(this); this.showTooltip = this.showTooltip.bind(this); this.hideTooltip = this.hideTooltip.bind(this); } @@ -86,18 +82,6 @@ class Tooltip extends PureComponent { })); } - /** - * Measure the size of the tooltip itself. - * - * @param {Object} nativeEvent - */ - measureTooltip({nativeEvent}) { - this.setState({ - tooltipWidth: nativeEvent.layout.width, - tooltipHeight: nativeEvent.layout.height, - }); - } - /** * Display the tooltip in an animation. */ @@ -200,13 +184,12 @@ class Tooltip extends PureComponent { yOffset={this.state.yOffset} wrapperWidth={this.state.wrapperWidth} wrapperHeight={this.state.wrapperHeight} - tooltipWidth={this.state.tooltipWidth} - tooltipHeight={this.state.tooltipHeight} setTooltipRef={el => this.tooltip = el} shiftHorizontal={_.result(this.props, 'shiftHorizontal')} shiftVertical={_.result(this.props, 'shiftVertical')} - measureTooltip={this.measureTooltip} text={this.props.text} + maxWidth={this.props.maxWidth} + numberOfLines={this.props.numberOfLines} /> )} = variables.sideBarWidth - ? variables.sideBarWidth + maxWidth: tooltipTextWidth >= maxWidth + ? maxWidth // Sum left and right padding : tooltipTextWidth + (spacing.ph2.paddingHorizontal * 2), @@ -151,9 +153,8 @@ export default function getTooltipStyles( color: themeColors.textReversed, fontFamily: fontFamily.GTA, fontSize: tooltipFontSize, - // overflowWrap: 'normal', - // overflow: 'hidden', - // textOverflow: 'ellipsis', + overflowWrap: 'normal', + overflow: 'hidden', }, pointerWrapperStyle: { position: 'fixed', From c8efb522fddfb489fbd68404019f66fa0c0eb2e5 Mon Sep 17 00:00:00 2001 From: LucioChavezFuentes Date: Wed, 6 Apr 2022 01:28:00 +0200 Subject: [PATCH 09/33] remove maxWidth numberOfLines default values --- src/components/MultipleAvatars.js | 3 ++- src/components/Tooltip/TooltipRenderedOnPageBody.js | 9 ++++++--- src/components/Tooltip/tooltipPropTypes.js | 3 --- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/components/MultipleAvatars.js b/src/components/MultipleAvatars.js index 993d36123c3a..bbe8e96bfa51 100644 --- a/src/components/MultipleAvatars.js +++ b/src/components/MultipleAvatars.js @@ -3,6 +3,7 @@ import PropTypes from 'prop-types'; import {Image, View} from 'react-native'; import _ from 'underscore'; import styles from '../styles/styles'; +import variables from '../styles/variables'; import Avatar from './Avatar'; import Tooltip from './Tooltip'; import Text from './Text'; @@ -80,7 +81,7 @@ const MultipleAvatars = (props) => { /> ) : ( - + diff --git a/src/components/Tooltip/TooltipRenderedOnPageBody.js b/src/components/Tooltip/TooltipRenderedOnPageBody.js index fbaea286f035..ffdcf7453bf0 100644 --- a/src/components/Tooltip/TooltipRenderedOnPageBody.js +++ b/src/components/Tooltip/TooltipRenderedOnPageBody.js @@ -40,14 +40,17 @@ const propTypes = { text: PropTypes.string.isRequired, /** number of pixels to set max-width on tooltip */ - maxWidth: PropTypes.number.isRequired, + maxWidth: PropTypes.number, /** maximum number of lines to set on tooltip */ - numberOfLines: PropTypes.number.isRequired, + numberOfLines: PropTypes.number, }; -const defaultProps = {}; +const defaultProps = { + maxWidth: undefined, + numberOfLines: undefined, +}; class TooltipRenderedOnPageBody extends React.Component { constructor(props) { diff --git a/src/components/Tooltip/tooltipPropTypes.js b/src/components/Tooltip/tooltipPropTypes.js index ac1d43cb73e4..bbcec77327f3 100644 --- a/src/components/Tooltip/tooltipPropTypes.js +++ b/src/components/Tooltip/tooltipPropTypes.js @@ -1,6 +1,5 @@ import PropTypes from 'prop-types'; import {windowDimensionsPropTypes} from '../withWindowDimensions'; -import variables from '../../styles/variables'; const propTypes = { /** Enable support for the absolute positioned native(View|Text) children. It will only work for single native child */ @@ -40,8 +39,6 @@ const defaultProps = { shiftVertical: 0, containerStyles: [], text: '', - maxWidth: variables.sideBarWidth, - numberOfLines: 3, }; export { From 2c68ac7672df1135ff776d5397408243ae0780e1 Mon Sep 17 00:00:00 2001 From: LucioChavezFuentes Date: Wed, 6 Apr 2022 03:19:33 +0200 Subject: [PATCH 10/33] fix minor capital letter comments Signed-off-by: LucioChavezFuentes --- src/components/Tooltip/TooltipRenderedOnPageBody.js | 4 ++-- src/components/Tooltip/tooltipPropTypes.js | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/components/Tooltip/TooltipRenderedOnPageBody.js b/src/components/Tooltip/TooltipRenderedOnPageBody.js index ffdcf7453bf0..9633ed381d04 100644 --- a/src/components/Tooltip/TooltipRenderedOnPageBody.js +++ b/src/components/Tooltip/TooltipRenderedOnPageBody.js @@ -39,10 +39,10 @@ const propTypes = { /** Text to be shown in the tooltip */ text: PropTypes.string.isRequired, - /** number of pixels to set max-width on tooltip */ + /** Number of pixels to set max-width on tooltip */ maxWidth: PropTypes.number, - /** maximum number of lines to set on tooltip */ + /** Maximum number of lines to set on tooltip */ numberOfLines: PropTypes.number, }; diff --git a/src/components/Tooltip/tooltipPropTypes.js b/src/components/Tooltip/tooltipPropTypes.js index bbcec77327f3..b0d54e94c0e9 100644 --- a/src/components/Tooltip/tooltipPropTypes.js +++ b/src/components/Tooltip/tooltipPropTypes.js @@ -25,10 +25,10 @@ const propTypes = { A positive value shifts the tooltip down, and a negative value shifts it up. */ shiftVertical: PropTypes.oneOfType([PropTypes.number, PropTypes.func]), - /** number of pixels to set max-width on tooltip */ + /** Number of pixels to set max-width on tooltip */ maxWidth: PropTypes.number, - /** maximum number of lines to set on tooltip */ + /** Maximum number of lines to set on tooltip */ numberOfLines: PropTypes.number, }; From 7b42881bc2567b4ad4347f246e2f610742db9cf0 Mon Sep 17 00:00:00 2001 From: LucioChavezFuentes Date: Wed, 6 Apr 2022 03:25:58 +0200 Subject: [PATCH 11/33] set tooltipTextWidth and maxWidth as optional Signed-off-by: LucioChavezFuentes --- src/styles/getTooltipStyles.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/styles/getTooltipStyles.js b/src/styles/getTooltipStyles.js index b6af267aca18..f1093ed3ba6d 100644 --- a/src/styles/getTooltipStyles.js +++ b/src/styles/getTooltipStyles.js @@ -67,8 +67,8 @@ function computeHorizontalShift(windowWidth, xOffset, componentWidth, tooltipWid * and a negative value shifts it to the left. * @param {Number} [manualShiftVertical] - Any additional amount to manually shift the tooltip up or down. * A positive value shifts it down, and a negative value shifts it up. - * @param {Number} tooltipTextWidth - Tooltip's inner text width - * @param {Number} maxWidth - Max-width for tooltip's wrapper + * @param {Number} [tooltipTextWidth] - Tooltip's inner text width + * @param {Number} [maxWidth] - Max-width for tooltip's wrapper * @returns {Object} */ export default function getTooltipStyles( From 6e0bfe6cdd029d4fac88f4d3ac4605cb6a48e31a Mon Sep 17 00:00:00 2001 From: LucioChavezFuentes Date: Wed, 6 Apr 2022 03:04:02 +0200 Subject: [PATCH 12/33] remove displayName --- src/components/Tooltip/TooltipRenderedOnPageBody.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/components/Tooltip/TooltipRenderedOnPageBody.js b/src/components/Tooltip/TooltipRenderedOnPageBody.js index 9633ed381d04..7444f9acc2c1 100644 --- a/src/components/Tooltip/TooltipRenderedOnPageBody.js +++ b/src/components/Tooltip/TooltipRenderedOnPageBody.js @@ -128,7 +128,6 @@ class TooltipRenderedOnPageBody extends React.Component { 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. From 25e29381ad38d7b41c34cc9120cd3b74215321db Mon Sep 17 00:00:00 2001 From: LucioChavezFuentes Date: Fri, 8 Apr 2022 02:59:15 +0200 Subject: [PATCH 13/33] avoid set state on tooltips with no maxWidth --- src/components/Tooltip/TooltipRenderedOnPageBody.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/components/Tooltip/TooltipRenderedOnPageBody.js b/src/components/Tooltip/TooltipRenderedOnPageBody.js index 7444f9acc2c1..69336ad73ce4 100644 --- a/src/components/Tooltip/TooltipRenderedOnPageBody.js +++ b/src/components/Tooltip/TooltipRenderedOnPageBody.js @@ -70,6 +70,9 @@ class TooltipRenderedOnPageBody extends React.Component { } componentDidMount() { + if (!this.props.maxWidth) { + return; + } this.setState({ tooltipTextWidth: this.textRef.offsetWidth, }); From 764c569d1ec64f7ed61b05882fb8f55065faae84 Mon Sep 17 00:00:00 2001 From: LucioChavezFuentes Date: Fri, 8 Apr 2022 03:27:47 +0200 Subject: [PATCH 14/33] remove memo, make component Pure, minor changes --- .../Tooltip/TooltipRenderedOnPageBody.js | 22 +++++++------------ src/components/Tooltip/index.js | 4 ---- src/components/Tooltip/tooltipPropTypes.js | 2 ++ 3 files changed, 10 insertions(+), 18 deletions(-) diff --git a/src/components/Tooltip/TooltipRenderedOnPageBody.js b/src/components/Tooltip/TooltipRenderedOnPageBody.js index 69336ad73ce4..5457d29bd2d4 100644 --- a/src/components/Tooltip/TooltipRenderedOnPageBody.js +++ b/src/components/Tooltip/TooltipRenderedOnPageBody.js @@ -1,4 +1,4 @@ -import React, {memo} from 'react'; +import React from 'react'; import PropTypes from 'prop-types'; import {Animated, View} from 'react-native'; import ReactDOM from 'react-dom'; @@ -33,9 +33,6 @@ const propTypes = { A positive value shifts the tooltip down, and a negative value shifts it up. */ shiftVertical: PropTypes.number.isRequired, - /** Callback to set the Ref to the Tooltip */ - setTooltipRef: PropTypes.func.isRequired, - /** Text to be shown in the tooltip */ text: PropTypes.string.isRequired, @@ -52,7 +49,12 @@ const defaultProps = { numberOfLines: undefined, }; -class TooltipRenderedOnPageBody extends React.Component { +// 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. +class TooltipRenderedOnPageBody extends React.PureComponent { constructor(props) { super(props); this.state = { @@ -64,8 +66,6 @@ class TooltipRenderedOnPageBody extends React.Component { tooltipHeight: 0, }; - this.textRef = null; - this.measureTooltip = this.measureTooltip.bind(this); } @@ -113,7 +113,6 @@ class TooltipRenderedOnPageBody extends React.Component { ); return ReactDOM.createPortal( @@ -132,9 +131,4 @@ class TooltipRenderedOnPageBody extends React.Component { TooltipRenderedOnPageBody.propTypes = propTypes; TooltipRenderedOnPageBody.defaultProps = defaultProps; -// 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); +export default TooltipRenderedOnPageBody; diff --git a/src/components/Tooltip/index.js b/src/components/Tooltip/index.js index 8059e8e14f27..d7995e1bd979 100644 --- a/src/components/Tooltip/index.js +++ b/src/components/Tooltip/index.js @@ -31,9 +31,6 @@ class Tooltip extends PureComponent { // The wrapper view containing the wrapped content along with the Tooltip itself. this.wrapperView = null; - // The tooltip (popover) itself. - this.tooltip = null; - // Whether the tooltip is first tooltip to activate the TooltipSense this.isTooltipSenseInitiator = false; this.shouldStartShowAnimation = false; @@ -184,7 +181,6 @@ class Tooltip extends PureComponent { yOffset={this.state.yOffset} wrapperWidth={this.state.wrapperWidth} wrapperHeight={this.state.wrapperHeight} - setTooltipRef={el => this.tooltip = el} shiftHorizontal={_.result(this.props, 'shiftHorizontal')} shiftVertical={_.result(this.props, 'shiftVertical')} text={this.props.text} diff --git a/src/components/Tooltip/tooltipPropTypes.js b/src/components/Tooltip/tooltipPropTypes.js index b0d54e94c0e9..a46d73d50da2 100644 --- a/src/components/Tooltip/tooltipPropTypes.js +++ b/src/components/Tooltip/tooltipPropTypes.js @@ -39,6 +39,8 @@ const defaultProps = { shiftVertical: 0, containerStyles: [], text: '', + maxWidth: undefined, + numberOfLines: undefined, }; export { From c2e46ad1832b9a03555ae98375999934a563a265 Mon Sep 17 00:00:00 2001 From: LucioChavezFuentes Date: Sat, 9 Apr 2022 01:12:58 +0200 Subject: [PATCH 15/33] tooltip settings, create setWrapperWidth --- src/components/DisplayNames/index.js | 7 +++++- src/styles/getTooltipStyles.js | 34 ++++++++++++++++++++++------ 2 files changed, 33 insertions(+), 8 deletions(-) diff --git a/src/components/DisplayNames/index.js b/src/components/DisplayNames/index.js index 07e71e25a44f..c9656715b5fc 100644 --- a/src/components/DisplayNames/index.js +++ b/src/components/DisplayNames/index.js @@ -5,6 +5,7 @@ import {propTypes, defaultProps} from './displayNamesPropTypes'; import styles from '../../styles/styles'; import Tooltip from '../Tooltip'; import Text from '../Text'; +import variables from '../../styles/variables'; class DisplayNames extends PureComponent { constructor(props) { @@ -110,7 +111,11 @@ class DisplayNames extends PureComponent { {this.props.displayNamesWithTooltips.length > 1 && this.state.isEllipsisActive && ( - + {/* There is some Gap for real ellipsis so we are adding 4 `.` to cover */} .... diff --git a/src/styles/getTooltipStyles.js b/src/styles/getTooltipStyles.js index f1093ed3ba6d..71db9f3ad830 100644 --- a/src/styles/getTooltipStyles.js +++ b/src/styles/getTooltipStyles.js @@ -49,6 +49,30 @@ function computeHorizontalShift(windowWidth, xOffset, componentWidth, tooltipWid return 0; } +/** + * + * @param {Number} [maxWidth] - Max-width for tooltip's wrapper. + * @param {Number} [tooltipTextWidth] - Max-width for tooltip's wrapper. + * @returns {Object} + */ +function setWrapperWidth(maxWidth, tooltipTextWidth) { + if (!maxWidth) { + return ({}); + } + + if (tooltipTextWidth >= maxWidth) { + return ({maxWidth}); + } + + // Sum left and right wrapper padding, otherwise the wrapper will clip the text. + // Add 1 px safe buffer to avoid last word wrap or clip on special cases + // where the real text width is float but a little larger than ref.offsetWidth, which will return integer + // ex. real text width: 172.2 px, ref.offsetWidth: 172px + return ({ + maxWidth: tooltipTextWidth + (spacing.ph2.paddingHorizontal * 2) + 1, + }); +} + /** * Generate styles for the tooltip component. * @@ -67,8 +91,8 @@ function computeHorizontalShift(windowWidth, xOffset, componentWidth, tooltipWid * and a negative value shifts it to the left. * @param {Number} [manualShiftVertical] - Any additional amount to manually shift the tooltip up or down. * A positive value shifts it down, and a negative value shifts it up. - * @param {Number} [tooltipTextWidth] - Tooltip's inner text width - * @param {Number} [maxWidth] - Max-width for tooltip's wrapper + * @param {Number} [tooltipTextWidth] - Tooltip's inner text width. + * @param {Number} [maxWidth] - Max-width for tooltip's wrapper. * @returns {Object} */ export default function getTooltipStyles( @@ -113,11 +137,7 @@ export default function getTooltipStyles( ...tooltipVerticalPadding, ...spacing.ph2, zIndex: variables.tooltipzIndex, - maxWidth: tooltipTextWidth >= maxWidth - ? maxWidth - - // Sum left and right padding - : tooltipTextWidth + (spacing.ph2.paddingHorizontal * 2), + ...setWrapperWidth(maxWidth, tooltipTextWidth), // Because it uses fixed positioning, the top-left corner of the tooltip is aligned // with the top-left corner of the window by default. From 259f44e3547b5169e6154093c19f88c0013d425b Mon Sep 17 00:00:00 2001 From: LucioChavezFuentes Date: Mon, 11 Apr 2022 22:19:15 +0200 Subject: [PATCH 16/33] fix comment about adding 1px, add consts, minor changes --- src/components/DisplayNames/index.js | 4 +++- src/components/MultipleAvatars.js | 4 +++- src/styles/getTooltipStyles.js | 15 ++++++++------- 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/src/components/DisplayNames/index.js b/src/components/DisplayNames/index.js index c9656715b5fc..1f224c21ca8e 100644 --- a/src/components/DisplayNames/index.js +++ b/src/components/DisplayNames/index.js @@ -7,6 +7,8 @@ import Tooltip from '../Tooltip'; import Text from '../Text'; import variables from '../../styles/variables'; +const TOOLTIP_MAX_LINES = 3; + class DisplayNames extends PureComponent { constructor(props) { super(props); @@ -113,7 +115,7 @@ class DisplayNames extends PureComponent { {/* There is some Gap for real ellipsis so we are adding 4 `.` to cover */} diff --git a/src/components/MultipleAvatars.js b/src/components/MultipleAvatars.js index bbe8e96bfa51..1529e0e958e5 100644 --- a/src/components/MultipleAvatars.js +++ b/src/components/MultipleAvatars.js @@ -11,6 +11,8 @@ import themeColors from '../styles/themes/default'; import * as StyleUtils from '../styles/StyleUtils'; import CONST from '../CONST'; +const TOOLTIP_MAX_LINES = 3; + const propTypes = { /** Array of avatar URLs or icons */ avatarIcons: PropTypes.arrayOf(PropTypes.oneOfType([PropTypes.string, PropTypes.func])), @@ -81,7 +83,7 @@ const MultipleAvatars = (props) => { /> ) : ( - + diff --git a/src/styles/getTooltipStyles.js b/src/styles/getTooltipStyles.js index 71db9f3ad830..62d5f7d02607 100644 --- a/src/styles/getTooltipStyles.js +++ b/src/styles/getTooltipStyles.js @@ -57,20 +57,21 @@ function computeHorizontalShift(windowWidth, xOffset, componentWidth, tooltipWid */ function setWrapperWidth(maxWidth, tooltipTextWidth) { if (!maxWidth) { - return ({}); + return {}; } if (tooltipTextWidth >= maxWidth) { - return ({maxWidth}); + return {maxWidth}; } // Sum left and right wrapper padding, otherwise the wrapper will clip the text. - // Add 1 px safe buffer to avoid last word wrap or clip on special cases - // where the real text width is float but a little larger than ref.offsetWidth, which will return integer - // ex. real text width: 172.2 px, ref.offsetWidth: 172px - return ({ + // Sometimes the real width of a text is a value with fractional digits (i.e. 172.2px) + // but the text.offsetWidth returns a truncated integer (172px) making this function + // to set the wrapper width slightly smaller than text but enough to make the last word + // of a line to wrap or clip. So is neccesary add 1px extra to the sum so wrapper contains text entirely. + return { maxWidth: tooltipTextWidth + (spacing.ph2.paddingHorizontal * 2) + 1, - }); + }; } /** From 24bcfabe82ee41df6f122503dc7379dcb809d495 Mon Sep 17 00:00:00 2001 From: LucioChavezFuentes Date: Tue, 12 Apr 2022 20:33:22 +0200 Subject: [PATCH 17/33] move TOOLTIP_MAX_LINES to CONST --- src/CONST.js | 2 ++ src/components/DisplayNames/index.js | 5 ++--- src/components/MultipleAvatars.js | 4 +--- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/CONST.js b/src/CONST.js index 0ecb1aedf48a..a9c0531d894e 100755 --- a/src/CONST.js +++ b/src/CONST.js @@ -368,6 +368,8 @@ const CONST = { EMOJI_FREQUENT_ROW_COUNT: 3, + TOOLTIP_MAX_LINES: 3, + LOGIN_TYPE: { PHONE: 'phone', EMAIL: 'email', diff --git a/src/components/DisplayNames/index.js b/src/components/DisplayNames/index.js index 1f224c21ca8e..f8ed09a6bdb1 100644 --- a/src/components/DisplayNames/index.js +++ b/src/components/DisplayNames/index.js @@ -6,8 +6,7 @@ import styles from '../../styles/styles'; import Tooltip from '../Tooltip'; import Text from '../Text'; import variables from '../../styles/variables'; - -const TOOLTIP_MAX_LINES = 3; +import CONST from '../../CONST'; class DisplayNames extends PureComponent { constructor(props) { @@ -115,7 +114,7 @@ class DisplayNames extends PureComponent { {/* There is some Gap for real ellipsis so we are adding 4 `.` to cover */} diff --git a/src/components/MultipleAvatars.js b/src/components/MultipleAvatars.js index 1529e0e958e5..e1c2ab4097f7 100644 --- a/src/components/MultipleAvatars.js +++ b/src/components/MultipleAvatars.js @@ -11,8 +11,6 @@ import themeColors from '../styles/themes/default'; import * as StyleUtils from '../styles/StyleUtils'; import CONST from '../CONST'; -const TOOLTIP_MAX_LINES = 3; - const propTypes = { /** Array of avatar URLs or icons */ avatarIcons: PropTypes.arrayOf(PropTypes.oneOfType([PropTypes.string, PropTypes.func])), @@ -83,7 +81,7 @@ const MultipleAvatars = (props) => { /> ) : ( - + From 39e859bfe18e753cdaa0017955a34ce2aadb9d8f Mon Sep 17 00:00:00 2001 From: LucioChavezFuentes Date: Tue, 12 Apr 2022 20:39:28 +0200 Subject: [PATCH 18/33] change comment about 1px add, minor change --- src/components/Tooltip/tooltipPropTypes.js | 1 - src/styles/getTooltipStyles.js | 7 ++----- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/src/components/Tooltip/tooltipPropTypes.js b/src/components/Tooltip/tooltipPropTypes.js index a46d73d50da2..6013b577d34f 100644 --- a/src/components/Tooltip/tooltipPropTypes.js +++ b/src/components/Tooltip/tooltipPropTypes.js @@ -30,7 +30,6 @@ const propTypes = { /** Maximum number of lines to set on tooltip */ numberOfLines: PropTypes.number, - }; const defaultProps = { diff --git a/src/styles/getTooltipStyles.js b/src/styles/getTooltipStyles.js index 62d5f7d02607..24caf2abcaaf 100644 --- a/src/styles/getTooltipStyles.js +++ b/src/styles/getTooltipStyles.js @@ -64,11 +64,8 @@ function setWrapperWidth(maxWidth, tooltipTextWidth) { return {maxWidth}; } - // Sum left and right wrapper padding, otherwise the wrapper will clip the text. - // Sometimes the real width of a text is a value with fractional digits (i.e. 172.2px) - // but the text.offsetWidth returns a truncated integer (172px) making this function - // to set the wrapper width slightly smaller than text but enough to make the last word - // of a line to wrap or clip. So is neccesary add 1px extra to the sum so wrapper contains text entirely. + // Add horizontal padding to the text width to get the wrapper width. + // tooltipTextWidth ignores the fractions (OffsetWidth) so add 1px to fit the text properly. return { maxWidth: tooltipTextWidth + (spacing.ph2.paddingHorizontal * 2) + 1, }; From f0524e7b2789605cf9dd7bdc25c5c398b89873bf Mon Sep 17 00:00:00 2001 From: LucioChavezFuentes Date: Fri, 15 Apr 2022 23:08:37 +0200 Subject: [PATCH 19/33] set default values numberOfLines maxWidth, remove check --- .../Tooltip/TooltipRenderedOnPageBody.js | 17 +++++------------ src/components/Tooltip/tooltipPropTypes.js | 6 ++++-- 2 files changed, 9 insertions(+), 14 deletions(-) diff --git a/src/components/Tooltip/TooltipRenderedOnPageBody.js b/src/components/Tooltip/TooltipRenderedOnPageBody.js index 5457d29bd2d4..50fa8332d75a 100644 --- a/src/components/Tooltip/TooltipRenderedOnPageBody.js +++ b/src/components/Tooltip/TooltipRenderedOnPageBody.js @@ -37,17 +37,13 @@ const propTypes = { text: PropTypes.string.isRequired, /** Number of pixels to set max-width on tooltip */ - maxWidth: PropTypes.number, + maxWidth: PropTypes.number.isRequired, /** Maximum number of lines to set on tooltip */ - numberOfLines: PropTypes.number, - + numberOfLines: PropTypes.number.isRequired, }; -const defaultProps = { - maxWidth: undefined, - numberOfLines: undefined, -}; +const defaultProps = {}; // Props will change frequently. // On every tooltip hover, we update the position in state which will result in re-rendering. @@ -58,8 +54,8 @@ class TooltipRenderedOnPageBody extends React.PureComponent { constructor(props) { super(props); this.state = { - // Set maxWidth as initial so we can get width of word wrapped text - tooltipTextWidth: this.props.maxWidth, + + tooltipTextWidth: undefined, // The width and height of the tooltip itself tooltipWidth: 0, @@ -70,9 +66,6 @@ class TooltipRenderedOnPageBody extends React.PureComponent { } componentDidMount() { - if (!this.props.maxWidth) { - return; - } this.setState({ tooltipTextWidth: this.textRef.offsetWidth, }); diff --git a/src/components/Tooltip/tooltipPropTypes.js b/src/components/Tooltip/tooltipPropTypes.js index 6013b577d34f..09a1d224db9c 100644 --- a/src/components/Tooltip/tooltipPropTypes.js +++ b/src/components/Tooltip/tooltipPropTypes.js @@ -1,5 +1,7 @@ import PropTypes from 'prop-types'; import {windowDimensionsPropTypes} from '../withWindowDimensions'; +import variables from '../../styles/variables'; +import CONST from '../../CONST'; const propTypes = { /** Enable support for the absolute positioned native(View|Text) children. It will only work for single native child */ @@ -38,8 +40,8 @@ const defaultProps = { shiftVertical: 0, containerStyles: [], text: '', - maxWidth: undefined, - numberOfLines: undefined, + maxWidth: variables.sideBarWidth, + numberOfLines: CONST.TOOLTIP_MAX_LINES, }; export { From 9229a4c54ac3a2e175e34ebde6c8b99e135945cf Mon Sep 17 00:00:00 2001 From: LucioChavezFuentes Date: Fri, 15 Apr 2022 23:12:32 +0200 Subject: [PATCH 20/33] remove unneccesary props --- src/components/DisplayNames/index.js | 8 +------- src/components/MultipleAvatars.js | 3 +-- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/src/components/DisplayNames/index.js b/src/components/DisplayNames/index.js index f8ed09a6bdb1..07e71e25a44f 100644 --- a/src/components/DisplayNames/index.js +++ b/src/components/DisplayNames/index.js @@ -5,8 +5,6 @@ import {propTypes, defaultProps} from './displayNamesPropTypes'; import styles from '../../styles/styles'; import Tooltip from '../Tooltip'; import Text from '../Text'; -import variables from '../../styles/variables'; -import CONST from '../../CONST'; class DisplayNames extends PureComponent { constructor(props) { @@ -112,11 +110,7 @@ class DisplayNames extends PureComponent { {this.props.displayNamesWithTooltips.length > 1 && this.state.isEllipsisActive && ( - + {/* There is some Gap for real ellipsis so we are adding 4 `.` to cover */} .... diff --git a/src/components/MultipleAvatars.js b/src/components/MultipleAvatars.js index e1c2ab4097f7..993d36123c3a 100644 --- a/src/components/MultipleAvatars.js +++ b/src/components/MultipleAvatars.js @@ -3,7 +3,6 @@ import PropTypes from 'prop-types'; import {Image, View} from 'react-native'; import _ from 'underscore'; import styles from '../styles/styles'; -import variables from '../styles/variables'; import Avatar from './Avatar'; import Tooltip from './Tooltip'; import Text from './Text'; @@ -81,7 +80,7 @@ const MultipleAvatars = (props) => { /> ) : ( - + From 0042574e3d763de98406a3360f6ff7c9fe4797f3 Mon Sep 17 00:00:00 2001 From: LucioChavezFuentes Date: Fri, 15 Apr 2022 23:38:57 +0200 Subject: [PATCH 21/33] return maxWidth if tooltipTextWidth is not set --- src/components/Tooltip/TooltipRenderedOnPageBody.js | 2 +- src/styles/getTooltipStyles.js | 10 +++------- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/src/components/Tooltip/TooltipRenderedOnPageBody.js b/src/components/Tooltip/TooltipRenderedOnPageBody.js index 50fa8332d75a..54826ad7a967 100644 --- a/src/components/Tooltip/TooltipRenderedOnPageBody.js +++ b/src/components/Tooltip/TooltipRenderedOnPageBody.js @@ -54,7 +54,7 @@ class TooltipRenderedOnPageBody extends React.PureComponent { constructor(props) { super(props); this.state = { - + // tooltipTextWidth will update with wrapped text offsetWidth based on maxWidth prop tooltipTextWidth: undefined, // The width and height of the tooltip itself diff --git a/src/styles/getTooltipStyles.js b/src/styles/getTooltipStyles.js index 24caf2abcaaf..44f53a42283f 100644 --- a/src/styles/getTooltipStyles.js +++ b/src/styles/getTooltipStyles.js @@ -51,16 +51,12 @@ function computeHorizontalShift(windowWidth, xOffset, componentWidth, tooltipWid /** * - * @param {Number} [maxWidth] - Max-width for tooltip's wrapper. + * @param {Number} maxWidth - Max-width for tooltip's wrapper. * @param {Number} [tooltipTextWidth] - Max-width for tooltip's wrapper. * @returns {Object} */ function setWrapperWidth(maxWidth, tooltipTextWidth) { - if (!maxWidth) { - return {}; - } - - if (tooltipTextWidth >= maxWidth) { + if (!tooltipTextWidth || tooltipTextWidth >= maxWidth) { return {maxWidth}; } @@ -90,7 +86,7 @@ function setWrapperWidth(maxWidth, tooltipTextWidth) { * @param {Number} [manualShiftVertical] - Any additional amount to manually shift the tooltip up or down. * A positive value shifts it down, and a negative value shifts it up. * @param {Number} [tooltipTextWidth] - Tooltip's inner text width. - * @param {Number} [maxWidth] - Max-width for tooltip's wrapper. + * @param {Number} maxWidth - Max-width for tooltip's wrapper. * @returns {Object} */ export default function getTooltipStyles( From 707a4ff1d13dd3be0e3b785ebfce5ffedf4dc384 Mon Sep 17 00:00:00 2001 From: LucioChavezFuentes Date: Fri, 15 Apr 2022 23:42:51 +0200 Subject: [PATCH 22/33] minor comment change --- src/styles/getTooltipStyles.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/styles/getTooltipStyles.js b/src/styles/getTooltipStyles.js index 44f53a42283f..bc5159adc90e 100644 --- a/src/styles/getTooltipStyles.js +++ b/src/styles/getTooltipStyles.js @@ -52,7 +52,7 @@ function computeHorizontalShift(windowWidth, xOffset, componentWidth, tooltipWid /** * * @param {Number} maxWidth - Max-width for tooltip's wrapper. - * @param {Number} [tooltipTextWidth] - Max-width for tooltip's wrapper. + * @param {Number} [tooltipTextWidth] - Tooltip's inner text width. * @returns {Object} */ function setWrapperWidth(maxWidth, tooltipTextWidth) { From f6a28327b619d9b136ffc4b6a2a32e4fb10a6558 Mon Sep 17 00:00:00 2001 From: LucioChavezFuentes Date: Wed, 20 Apr 2022 03:24:58 +0200 Subject: [PATCH 23/33] avoid re-render on single line tooltips, minor changes --- src/components/Tooltip/TooltipRenderedOnPageBody.js | 8 +++++--- src/components/Tooltip/index.js | 1 - src/styles/getTooltipStyles.js | 7 ++++--- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/components/Tooltip/TooltipRenderedOnPageBody.js b/src/components/Tooltip/TooltipRenderedOnPageBody.js index 54826ad7a967..99a2a9c033b4 100644 --- a/src/components/Tooltip/TooltipRenderedOnPageBody.js +++ b/src/components/Tooltip/TooltipRenderedOnPageBody.js @@ -43,8 +43,6 @@ const propTypes = { numberOfLines: PropTypes.number.isRequired, }; -const defaultProps = {}; - // 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. @@ -66,6 +64,11 @@ class TooltipRenderedOnPageBody extends React.PureComponent { } componentDidMount() { + // Is unneccesary to get new wrapper width if text is not longer enough to fit more than one line + if (this.textRef.getClientRects().length <= 1) { + return; + } + this.setState({ tooltipTextWidth: this.textRef.offsetWidth, }); @@ -122,6 +125,5 @@ class TooltipRenderedOnPageBody extends React.PureComponent { } TooltipRenderedOnPageBody.propTypes = propTypes; -TooltipRenderedOnPageBody.defaultProps = defaultProps; export default TooltipRenderedOnPageBody; diff --git a/src/components/Tooltip/index.js b/src/components/Tooltip/index.js index d7995e1bd979..d8098c9688d7 100644 --- a/src/components/Tooltip/index.js +++ b/src/components/Tooltip/index.js @@ -25,7 +25,6 @@ class Tooltip extends PureComponent { // The width and height of the wrapper view wrapperWidth: 0, wrapperHeight: 0, - }; // The wrapper view containing the wrapped content along with the Tooltip itself. diff --git a/src/styles/getTooltipStyles.js b/src/styles/getTooltipStyles.js index bc5159adc90e..7e8ca1605002 100644 --- a/src/styles/getTooltipStyles.js +++ b/src/styles/getTooltipStyles.js @@ -50,18 +50,19 @@ function computeHorizontalShift(windowWidth, xOffset, componentWidth, tooltipWid } /** + * Generate wrapper's width using text offsetWidth based on maxWidth prop * * @param {Number} maxWidth - Max-width for tooltip's wrapper. * @param {Number} [tooltipTextWidth] - Tooltip's inner text width. * @returns {Object} */ -function setWrapperWidth(maxWidth, tooltipTextWidth) { +function getWrapperWidth(maxWidth, tooltipTextWidth) { if (!tooltipTextWidth || tooltipTextWidth >= maxWidth) { return {maxWidth}; } // Add horizontal padding to the text width to get the wrapper width. - // tooltipTextWidth ignores the fractions (OffsetWidth) so add 1px to fit the text properly. + // TooltipTextWidth ignores the fractions (OffsetWidth) so add 1px to fit the text properly. return { maxWidth: tooltipTextWidth + (spacing.ph2.paddingHorizontal * 2) + 1, }; @@ -131,7 +132,7 @@ export default function getTooltipStyles( ...tooltipVerticalPadding, ...spacing.ph2, zIndex: variables.tooltipzIndex, - ...setWrapperWidth(maxWidth, tooltipTextWidth), + ...getWrapperWidth(maxWidth, tooltipTextWidth), // Because it uses fixed positioning, the top-left corner of the tooltip is aligned // with the top-left corner of the window by default. From 7b6abee9f0589d370cfee5d7d12072faed9e3ea2 Mon Sep 17 00:00:00 2001 From: LucioChavezFuentes Date: Mon, 25 Apr 2022 21:35:59 +0200 Subject: [PATCH 24/33] use width instead of max-width --- src/styles/getTooltipStyles.js | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/styles/getTooltipStyles.js b/src/styles/getTooltipStyles.js index 7e8ca1605002..36c6914a444c 100644 --- a/src/styles/getTooltipStyles.js +++ b/src/styles/getTooltipStyles.js @@ -57,15 +57,14 @@ function computeHorizontalShift(windowWidth, xOffset, componentWidth, tooltipWid * @returns {Object} */ function getWrapperWidth(maxWidth, tooltipTextWidth) { - if (!tooltipTextWidth || tooltipTextWidth >= maxWidth) { - return {maxWidth}; + if (tooltipTextWidth && tooltipTextWidth < maxWidth) { + return { + // Add horizontal padding to the text width to get the wrapper width. + // TooltipTextWidth ignores the fractions (OffsetWidth) so add 1px to fit the text properly. + width: tooltipTextWidth + (spacing.ph2.paddingHorizontal * 2) + 1, + }; } - - // Add horizontal padding to the text width to get the wrapper width. - // TooltipTextWidth ignores the fractions (OffsetWidth) so add 1px to fit the text properly. - return { - maxWidth: tooltipTextWidth + (spacing.ph2.paddingHorizontal * 2) + 1, - }; + return {width: maxWidth}; } /** From 3b6048e7ee98b7ec6e70f7bb26ff753bc6882d69 Mon Sep 17 00:00:00 2001 From: LucioChavezFuentes Date: Mon, 25 Apr 2022 21:36:27 +0200 Subject: [PATCH 25/33] remove check on componentDidMount --- src/components/Tooltip/TooltipRenderedOnPageBody.js | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/components/Tooltip/TooltipRenderedOnPageBody.js b/src/components/Tooltip/TooltipRenderedOnPageBody.js index 99a2a9c033b4..f3024366681b 100644 --- a/src/components/Tooltip/TooltipRenderedOnPageBody.js +++ b/src/components/Tooltip/TooltipRenderedOnPageBody.js @@ -64,11 +64,6 @@ class TooltipRenderedOnPageBody extends React.PureComponent { } componentDidMount() { - // Is unneccesary to get new wrapper width if text is not longer enough to fit more than one line - if (this.textRef.getClientRects().length <= 1) { - return; - } - this.setState({ tooltipTextWidth: this.textRef.offsetWidth, }); From b66427b22296134d5698a001fad2426e156bd0e9 Mon Sep 17 00:00:00 2001 From: LucioChavezFuentes Date: Mon, 25 Apr 2022 22:17:54 +0200 Subject: [PATCH 26/33] remove unneccesary function, simplify code --- .../Tooltip/TooltipRenderedOnPageBody.js | 2 +- src/styles/getTooltipStyles.js | 30 +++++++------------ 2 files changed, 11 insertions(+), 21 deletions(-) diff --git a/src/components/Tooltip/TooltipRenderedOnPageBody.js b/src/components/Tooltip/TooltipRenderedOnPageBody.js index f3024366681b..4c44cdc68a52 100644 --- a/src/components/Tooltip/TooltipRenderedOnPageBody.js +++ b/src/components/Tooltip/TooltipRenderedOnPageBody.js @@ -53,7 +53,7 @@ class TooltipRenderedOnPageBody extends React.PureComponent { super(props); this.state = { // tooltipTextWidth will update with wrapped text offsetWidth based on maxWidth prop - tooltipTextWidth: undefined, + tooltipTextWidth: this.props.maxWidth, // The width and height of the tooltip itself tooltipWidth: 0, diff --git a/src/styles/getTooltipStyles.js b/src/styles/getTooltipStyles.js index 36c6914a444c..3adaa5aecc1e 100644 --- a/src/styles/getTooltipStyles.js +++ b/src/styles/getTooltipStyles.js @@ -49,24 +49,6 @@ function computeHorizontalShift(windowWidth, xOffset, componentWidth, tooltipWid return 0; } -/** - * Generate wrapper's width using text offsetWidth based on maxWidth prop - * - * @param {Number} maxWidth - Max-width for tooltip's wrapper. - * @param {Number} [tooltipTextWidth] - Tooltip's inner text width. - * @returns {Object} - */ -function getWrapperWidth(maxWidth, tooltipTextWidth) { - if (tooltipTextWidth && tooltipTextWidth < maxWidth) { - return { - // Add horizontal padding to the text width to get the wrapper width. - // TooltipTextWidth ignores the fractions (OffsetWidth) so add 1px to fit the text properly. - width: tooltipTextWidth + (spacing.ph2.paddingHorizontal * 2) + 1, - }; - } - return {width: maxWidth}; -} - /** * Generate styles for the tooltip component. * @@ -85,7 +67,7 @@ function getWrapperWidth(maxWidth, tooltipTextWidth) { * and a negative value shifts it to the left. * @param {Number} [manualShiftVertical] - Any additional amount to manually shift the tooltip up or down. * A positive value shifts it down, and a negative value shifts it up. - * @param {Number} [tooltipTextWidth] - Tooltip's inner text width. + * @param {Number} tooltipTextWidth - Tooltip's inner text width. * @param {Number} maxWidth - Max-width for tooltip's wrapper. * @returns {Object} */ @@ -131,7 +113,15 @@ export default function getTooltipStyles( ...tooltipVerticalPadding, ...spacing.ph2, zIndex: variables.tooltipzIndex, - ...getWrapperWidth(maxWidth, tooltipTextWidth), + + // Generate wrapper's width using text's offsetWidth based on maxWidth prop + width: tooltipTextWidth < maxWidth + + // Add horizontal padding to the text width to get the wrapper width. + // TooltipTextWidth ignores the fractions (OffsetWidth) so add 1px to fit the text properly. + ? tooltipTextWidth + (spacing.ph2.paddingHorizontal * 2) + 1 + + : maxWidth, // Because it uses fixed positioning, the top-left corner of the tooltip is aligned // with the top-left corner of the window by default. From 706f227b9d8d7a02af3c3368687632ef90e0cfb3 Mon Sep 17 00:00:00 2001 From: LucioChavezFuentes Date: Wed, 27 Apr 2022 01:45:40 +0200 Subject: [PATCH 27/33] move wrapper width logic close to its variables --- .../Tooltip/TooltipRenderedOnPageBody.js | 16 ++++++++++++---- src/styles/getTooltipStyles.js | 16 +++------------- 2 files changed, 15 insertions(+), 17 deletions(-) diff --git a/src/components/Tooltip/TooltipRenderedOnPageBody.js b/src/components/Tooltip/TooltipRenderedOnPageBody.js index 4c44cdc68a52..75d5fbeef3d3 100644 --- a/src/components/Tooltip/TooltipRenderedOnPageBody.js +++ b/src/components/Tooltip/TooltipRenderedOnPageBody.js @@ -4,6 +4,7 @@ import {Animated, View} from 'react-native'; import ReactDOM from 'react-dom'; import getTooltipStyles from '../../styles/getTooltipStyles'; import Text from '../Text'; +import spacing from '../../styles/utilities/spacing'; const propTypes = { /** Window width */ @@ -52,8 +53,8 @@ class TooltipRenderedOnPageBody extends React.PureComponent { constructor(props) { super(props); this.state = { - // tooltipTextWidth will update with wrapped text offsetWidth based on maxWidth prop - tooltipTextWidth: this.props.maxWidth, + // The width of toolttip's inner text + tooltipTextWidth: 0, // The width and height of the tooltip itself tooltipWidth: 0, @@ -82,6 +83,14 @@ class TooltipRenderedOnPageBody extends React.PureComponent { } render() { + // We get wrapper width based on tooltip's inner text width so the wrapper + // is just big enough to fit text and prevent white space + const wrapperWidth = this.state.tooltipTextWidth && this.state.tooltipTextWidth < this.props.maxWidth + + // Add horizontal padding to the text width to get the wrapper width. + // TooltipTextWidth ignores the fractions (OffsetWidth) so add 1px to fit the text properly. + ? this.state.tooltipTextWidth + (spacing.ph2.paddingHorizontal * 2) + 1 + : this.props.maxWidth; const { animationStyle, tooltipWrapperStyle, @@ -99,8 +108,7 @@ class TooltipRenderedOnPageBody extends React.PureComponent { this.state.tooltipHeight, this.props.shiftHorizontal, this.props.shiftVertical, - this.state.tooltipTextWidth, - this.props.maxWidth, + wrapperWidth, ); return ReactDOM.createPortal( Date: Wed, 27 Apr 2022 21:29:28 +0200 Subject: [PATCH 28/33] remove unneeded wrapperView initializing --- src/components/Tooltip/index.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/components/Tooltip/index.js b/src/components/Tooltip/index.js index d8098c9688d7..af46f8154d00 100644 --- a/src/components/Tooltip/index.js +++ b/src/components/Tooltip/index.js @@ -27,9 +27,6 @@ class Tooltip extends PureComponent { wrapperHeight: 0, }; - // The wrapper view containing the wrapped content along with the Tooltip itself. - this.wrapperView = null; - // Whether the tooltip is first tooltip to activate the TooltipSense this.isTooltipSenseInitiator = false; this.shouldStartShowAnimation = false; From e076b954aad1d03213c3408f743bc9594b959b66 Mon Sep 17 00:00:00 2001 From: LucioChavezFuentes Date: Tue, 3 May 2022 01:36:24 +0200 Subject: [PATCH 29/33] move wrapper width condition to getTooltipStyles --- .../Tooltip/TooltipRenderedOnPageBody.js | 12 ++---------- src/styles/getTooltipStyles.js | 15 +++++++++++++-- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/src/components/Tooltip/TooltipRenderedOnPageBody.js b/src/components/Tooltip/TooltipRenderedOnPageBody.js index 75d5fbeef3d3..66ba3a57cd44 100644 --- a/src/components/Tooltip/TooltipRenderedOnPageBody.js +++ b/src/components/Tooltip/TooltipRenderedOnPageBody.js @@ -4,7 +4,6 @@ import {Animated, View} from 'react-native'; import ReactDOM from 'react-dom'; import getTooltipStyles from '../../styles/getTooltipStyles'; import Text from '../Text'; -import spacing from '../../styles/utilities/spacing'; const propTypes = { /** Window width */ @@ -83,14 +82,6 @@ class TooltipRenderedOnPageBody extends React.PureComponent { } render() { - // We get wrapper width based on tooltip's inner text width so the wrapper - // is just big enough to fit text and prevent white space - const wrapperWidth = this.state.tooltipTextWidth && this.state.tooltipTextWidth < this.props.maxWidth - - // Add horizontal padding to the text width to get the wrapper width. - // TooltipTextWidth ignores the fractions (OffsetWidth) so add 1px to fit the text properly. - ? this.state.tooltipTextWidth + (spacing.ph2.paddingHorizontal * 2) + 1 - : this.props.maxWidth; const { animationStyle, tooltipWrapperStyle, @@ -106,9 +97,10 @@ class TooltipRenderedOnPageBody extends React.PureComponent { this.props.wrapperHeight, this.state.tooltipWidth, this.state.tooltipHeight, + this.state.tooltipTextWidth, this.props.shiftHorizontal, this.props.shiftVertical, - wrapperWidth, + this.props.maxWidth, ); return ReactDOM.createPortal( Date: Tue, 3 May 2022 01:42:07 +0200 Subject: [PATCH 30/33] move maxWidth before optional parameters --- src/components/Tooltip/TooltipRenderedOnPageBody.js | 2 +- src/styles/getTooltipStyles.js | 5 ++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/components/Tooltip/TooltipRenderedOnPageBody.js b/src/components/Tooltip/TooltipRenderedOnPageBody.js index 66ba3a57cd44..562842260f6c 100644 --- a/src/components/Tooltip/TooltipRenderedOnPageBody.js +++ b/src/components/Tooltip/TooltipRenderedOnPageBody.js @@ -95,12 +95,12 @@ class TooltipRenderedOnPageBody extends React.PureComponent { this.props.yOffset, this.props.wrapperWidth, this.props.wrapperHeight, + this.props.maxWidth, this.state.tooltipWidth, this.state.tooltipHeight, this.state.tooltipTextWidth, this.props.shiftHorizontal, this.props.shiftVertical, - this.props.maxWidth, ); return ReactDOM.createPortal( Date: Tue, 3 May 2022 23:43:17 +0200 Subject: [PATCH 31/33] update comment and change offsetWidth to measure function --- src/components/Tooltip/TooltipRenderedOnPageBody.js | 6 ++++-- src/styles/getTooltipStyles.js | 3 +-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/components/Tooltip/TooltipRenderedOnPageBody.js b/src/components/Tooltip/TooltipRenderedOnPageBody.js index 562842260f6c..11278650a913 100644 --- a/src/components/Tooltip/TooltipRenderedOnPageBody.js +++ b/src/components/Tooltip/TooltipRenderedOnPageBody.js @@ -64,8 +64,10 @@ class TooltipRenderedOnPageBody extends React.PureComponent { } componentDidMount() { - this.setState({ - tooltipTextWidth: this.textRef.offsetWidth, + this.textRef.measure((x, y, width) => { + this.setState({ + tooltipTextWidth: width, + }); }); } diff --git a/src/styles/getTooltipStyles.js b/src/styles/getTooltipStyles.js index e2f53956392b..baca706d652a 100644 --- a/src/styles/getTooltipStyles.js +++ b/src/styles/getTooltipStyles.js @@ -97,8 +97,7 @@ export default function getTooltipStyles( const tooltipVerticalPadding = spacing.pv1; const tooltipFontSize = variables.fontSizeSmall; - // We get wrapper width based on tooltip's inner text width so the wrapper - // is just big enough to fit text and prevent white space + // We get wrapper width based on tooltip's inner text width so the wrapper is just big enough to fit text and prevent white space const wrapperWidth = tooltipTextWidth && tooltipTextWidth < maxWidth // Add horizontal padding to the text width to get the wrapper width. From 89c578184ea9475e4f43531eb84e5eb42d2964f8 Mon Sep 17 00:00:00 2001 From: LucioChavezFuentes Date: Wed, 4 May 2022 22:49:32 +0200 Subject: [PATCH 32/33] revert to offsetWidth, minor comment changes --- src/components/Tooltip/TooltipRenderedOnPageBody.js | 10 ++++------ src/components/Tooltip/tooltipPropTypes.js | 2 +- src/styles/getTooltipStyles.js | 6 +++--- 3 files changed, 8 insertions(+), 10 deletions(-) diff --git a/src/components/Tooltip/TooltipRenderedOnPageBody.js b/src/components/Tooltip/TooltipRenderedOnPageBody.js index 11278650a913..21c68780da65 100644 --- a/src/components/Tooltip/TooltipRenderedOnPageBody.js +++ b/src/components/Tooltip/TooltipRenderedOnPageBody.js @@ -39,7 +39,7 @@ const propTypes = { /** Number of pixels to set max-width on tooltip */ maxWidth: PropTypes.number.isRequired, - /** Maximum number of lines to set on tooltip */ + /** Maximum number of lines to show in tooltip */ numberOfLines: PropTypes.number.isRequired, }; @@ -47,7 +47,7 @@ const propTypes = { // 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. +// It's good to memorize this one. class TooltipRenderedOnPageBody extends React.PureComponent { constructor(props) { super(props); @@ -64,10 +64,8 @@ class TooltipRenderedOnPageBody extends React.PureComponent { } componentDidMount() { - this.textRef.measure((x, y, width) => { - this.setState({ - tooltipTextWidth: width, - }); + this.setState({ + tooltipTextWidth: this.textRef.offsetWidth, }); } diff --git a/src/components/Tooltip/tooltipPropTypes.js b/src/components/Tooltip/tooltipPropTypes.js index 09a1d224db9c..627f1c017190 100644 --- a/src/components/Tooltip/tooltipPropTypes.js +++ b/src/components/Tooltip/tooltipPropTypes.js @@ -30,7 +30,7 @@ const propTypes = { /** Number of pixels to set max-width on tooltip */ maxWidth: PropTypes.number, - /** Maximum number of lines to set on tooltip */ + /** Maximum number of lines to show in tooltip */ numberOfLines: PropTypes.number, }; diff --git a/src/styles/getTooltipStyles.js b/src/styles/getTooltipStyles.js index baca706d652a..e2d0fd60fb93 100644 --- a/src/styles/getTooltipStyles.js +++ b/src/styles/getTooltipStyles.js @@ -98,12 +98,12 @@ export default function getTooltipStyles( const tooltipFontSize = variables.fontSizeSmall; // We get wrapper width based on tooltip's inner text width so the wrapper is just big enough to fit text and prevent white space + // Add horizontal padding to the text width to get the wrapper width. + // TooltipTextWidth ignores the fractions (OffsetWidth) so add 1px to fit the text properly. const wrapperWidth = tooltipTextWidth && tooltipTextWidth < maxWidth - - // Add horizontal padding to the text width to get the wrapper width. - // TooltipTextWidth ignores the fractions (OffsetWidth) so add 1px to fit the text properly. ? tooltipTextWidth + (spacing.ph2.paddingHorizontal * 2) + 1 : maxWidth; + return { animationStyle: { // remember Transform causes a new Local cordinate system From 34231f9267895a3a135726a7f70235a747a1fdd2 Mon Sep 17 00:00:00 2001 From: LucioChavezFuentes Date: Mon, 9 May 2022 20:09:35 +0200 Subject: [PATCH 33/33] change on comments --- src/components/Tooltip/TooltipRenderedOnPageBody.js | 2 +- src/styles/getTooltipStyles.js | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/components/Tooltip/TooltipRenderedOnPageBody.js b/src/components/Tooltip/TooltipRenderedOnPageBody.js index 21c68780da65..644c9deae093 100644 --- a/src/components/Tooltip/TooltipRenderedOnPageBody.js +++ b/src/components/Tooltip/TooltipRenderedOnPageBody.js @@ -52,7 +52,7 @@ class TooltipRenderedOnPageBody extends React.PureComponent { constructor(props) { super(props); this.state = { - // The width of toolttip's inner text + // The width of tooltip's inner text tooltipTextWidth: 0, // The width and height of the tooltip itself diff --git a/src/styles/getTooltipStyles.js b/src/styles/getTooltipStyles.js index e2d0fd60fb93..086a1069d74d 100644 --- a/src/styles/getTooltipStyles.js +++ b/src/styles/getTooltipStyles.js @@ -97,9 +97,9 @@ export default function getTooltipStyles( const tooltipVerticalPadding = spacing.pv1; const tooltipFontSize = variables.fontSizeSmall; - // We get wrapper width based on tooltip's inner text width so the wrapper is just big enough to fit text and prevent white space - // Add horizontal padding to the text width to get the wrapper width. - // TooltipTextWidth ignores the fractions (OffsetWidth) so add 1px to fit the text properly. + // We get wrapper width based on the tooltip's inner text width so the wrapper is just big enough to fit text and prevent white space. + // If the text width is less than the maximum available width, add horizontal padding. + // Note: tooltipTextWidth ignores the fractions (OffsetWidth) so add 1px to fit the text properly. const wrapperWidth = tooltipTextWidth && tooltipTextWidth < maxWidth ? tooltipTextWidth + (spacing.ph2.paddingHorizontal * 2) + 1 : maxWidth;