Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

7486 reduce tooltip over group #8494

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
6caff3c
convert to class, get ref, add styles
LucioChavezFuentes Mar 21, 2022
2674c63
finish proposal
LucioChavezFuentes Mar 22, 2022
aeb366d
move getCorrectWidth func
LucioChavezFuentes Mar 23, 2022
3c506fa
create tooltipHorizontalPadding, add new param
LucioChavezFuentes Mar 23, 2022
c3aa571
remove unneeded maximumWords prop
LucioChavezFuentes Mar 30, 2022
9e8271c
simplify code
LucioChavezFuentes Mar 31, 2022
5163c67
remove unneeded maximumWords prop
LucioChavezFuentes Mar 31, 2022
66c912e
insert maxWidth and numberLines props, move measureTooltip
LucioChavezFuentes Apr 4, 2022
3232093
Merge branch 'main' into 7486_reduce-tooltip-over-group
LucioChavezFuentes Apr 4, 2022
c8efb52
remove maxWidth numberOfLines default values
LucioChavezFuentes Apr 5, 2022
2c68ac7
fix minor capital letter comments
LucioChavezFuentes Apr 6, 2022
7b42881
set tooltipTextWidth and maxWidth as optional
LucioChavezFuentes Apr 6, 2022
6e0bfe6
remove displayName
LucioChavezFuentes Apr 6, 2022
25e2938
avoid set state on tooltips with no maxWidth
LucioChavezFuentes Apr 8, 2022
764c569
remove memo, make component Pure, minor changes
LucioChavezFuentes Apr 8, 2022
c2e46ad
tooltip settings, create setWrapperWidth
LucioChavezFuentes Apr 8, 2022
259f44e
fix comment about adding 1px, add consts, minor changes
LucioChavezFuentes Apr 11, 2022
24bcfab
move TOOLTIP_MAX_LINES to CONST
LucioChavezFuentes Apr 12, 2022
39e859b
change comment about 1px add, minor change
LucioChavezFuentes Apr 12, 2022
f0524e7
set default values numberOfLines maxWidth, remove check
LucioChavezFuentes Apr 15, 2022
9229a4c
remove unneccesary props
LucioChavezFuentes Apr 15, 2022
0042574
return maxWidth if tooltipTextWidth is not set
LucioChavezFuentes Apr 15, 2022
707a4ff
minor comment change
LucioChavezFuentes Apr 15, 2022
f6a2832
avoid re-render on single line tooltips, minor changes
LucioChavezFuentes Apr 20, 2022
7b6abee
use width instead of max-width
LucioChavezFuentes Apr 25, 2022
3b6048e
remove check on componentDidMount
LucioChavezFuentes Apr 25, 2022
b66427b
remove unneccesary function, simplify code
LucioChavezFuentes Apr 25, 2022
706f227
move wrapper width logic close to its variables
LucioChavezFuentes Apr 26, 2022
b64b96e
Merge branch 'main' into 7486_reduce-tooltip-over-group
LucioChavezFuentes Apr 27, 2022
7895f01
remove unneeded wrapperView initializing
LucioChavezFuentes Apr 27, 2022
e076b95
move wrapper width condition to getTooltipStyles
LucioChavezFuentes May 2, 2022
6ade224
move maxWidth before optional parameters
LucioChavezFuentes May 2, 2022
e098176
update comment and change offsetWidth to measure function
LucioChavezFuentes May 3, 2022
89c5781
revert to offsetWidth, minor comment changes
LucioChavezFuentes May 4, 2022
34231f9
change on comments
LucioChavezFuentes May 9, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/CONST.js
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,8 @@ const CONST = {

EMOJI_FREQUENT_ROW_COUNT: 3,

TOOLTIP_MAX_LINES: 3,

LOGIN_TYPE: {
PHONE: 'phone',
EMAIL: 'email',
Expand Down
133 changes: 80 additions & 53 deletions src/components/Tooltip/TooltipRenderedOnPageBody.js
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -25,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,
Expand All @@ -39,59 +33,92 @@ 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,

/** 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,

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(
<Animated.View
ref={props.setTooltipRef}
onLayout={props.measureTooltip}
style={[tooltipWrapperStyle, animationStyle]}
>
<Text style={tooltipTextStyle} numberOfLines={1}>{props.text}</Text>
<View style={pointerWrapperStyle}>
<View style={pointerStyle} />
</View>
</Animated.View>,
document.querySelector('body'),
);
/** Maximum number of lines to show in tooltip */
numberOfLines: PropTypes.number.isRequired,
};

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);
// It's good to memorize this one.
class TooltipRenderedOnPageBody extends React.PureComponent {
constructor(props) {
super(props);
this.state = {
// The width of tooltip's inner text
tooltipTextWidth: 0,

// The width and height of the tooltip itself
tooltipWidth: 0,
tooltipHeight: 0,
};

this.measureTooltip = this.measureTooltip.bind(this);
}

componentDidMount() {
this.setState({
tooltipTextWidth: this.textRef.offsetWidth,
});
}

/**
* 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,
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.maxWidth,
this.state.tooltipWidth,
this.state.tooltipHeight,
this.state.tooltipTextWidth,
this.props.shiftHorizontal,
this.props.shiftVertical,
);
return ReactDOM.createPortal(
<Animated.View
onLayout={this.measureTooltip}
style={[tooltipWrapperStyle, animationStyle]}
>
<Text numberOfLines={this.props.numberOfLines} style={tooltipTextStyle}>
<Text style={tooltipTextStyle} ref={ref => this.textRef = ref}>{this.props.text}</Text>
</Text>
<View style={pointerWrapperStyle}>
<View style={pointerStyle} />
</View>
</Animated.View>,
document.querySelector('body'),
);
}
}

TooltipRenderedOnPageBody.propTypes = propTypes;

export default TooltipRenderedOnPageBody;
29 changes: 2 additions & 27 deletions src/components/Tooltip/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,25 +25,14 @@ class Tooltip extends PureComponent {
// The width and height of the wrapper view
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.
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;
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);
}
Expand Down Expand Up @@ -86,18 +75,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.
*/
Expand Down Expand Up @@ -200,13 +177,11 @@ 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}
/>
)}
<Hoverable
Expand Down
10 changes: 10 additions & 0 deletions src/components/Tooltip/tooltipPropTypes.js
Original file line number Diff line number Diff line change
@@ -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 */
Expand All @@ -24,6 +26,12 @@ const propTypes = {
/** Any additional amount to manually adjust the vertical position of the tooltip.
A positive value shifts the tooltip down, and a negative value shifts it up. */
shiftVertical: PropTypes.oneOfType([PropTypes.number, PropTypes.func]),

/** Number of pixels to set max-width on tooltip */
maxWidth: PropTypes.number,

/** Maximum number of lines to show in tooltip */
numberOfLines: PropTypes.number,
};

const defaultProps = {
Expand All @@ -32,6 +40,8 @@ const defaultProps = {
shiftVertical: 0,
containerStyles: [],
text: '',
maxWidth: variables.sideBarWidth,
numberOfLines: CONST.TOOLTIP_MAX_LINES,
};

export {
Expand Down
14 changes: 14 additions & 0 deletions src/styles/getTooltipStyles.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,10 @@ function computeHorizontalShift(windowWidth, xOffset, componentWidth, tooltipWid
* and the top edge of the wrapped component.
* @param {Number} componentWidth - The width of the wrapped component.
* @param {Number} componentHeight - The height of the wrapped component.
* @param {Number} maxWidth - The tooltip's max width.
* @param {Number} tooltipWidth - The width of the tooltip itself.
* @param {Number} tooltipHeight - The height of the tooltip itself.
* @param {Number} tooltipTextWidth - The tooltip's inner text width.
* @param {Number} [manualShiftHorizontal] - Any additional amount to manually shift the tooltip to the left or right.
* A positive value shifts it to the right,
* and a negative value shifts it to the left.
Expand All @@ -76,8 +78,10 @@ export default function getTooltipStyles(
yOffset,
componentWidth,
componentHeight,
maxWidth,
tooltipWidth,
tooltipHeight,
tooltipTextWidth,
manualShiftHorizontal = 0,
manualShiftVertical = 0,
) {
Expand All @@ -93,6 +97,13 @@ export default function getTooltipStyles(
const tooltipVerticalPadding = spacing.pv1;
const tooltipFontSize = variables.fontSizeSmall;

// 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;

return {
animationStyle: {
// remember Transform causes a new Local cordinate system
Expand All @@ -109,6 +120,7 @@ export default function getTooltipStyles(
...tooltipVerticalPadding,
...spacing.ph2,
zIndex: variables.tooltipzIndex,
width: wrapperWidth,

// Because it uses fixed positioning, the top-left corner of the tooltip is aligned
// with the top-left corner of the window by default.
Expand Down Expand Up @@ -144,6 +156,8 @@ export default function getTooltipStyles(
color: themeColors.textReversed,
fontFamily: fontFamily.GTA,
fontSize: tooltipFontSize,
overflowWrap: 'normal',
overflow: 'hidden',
},
pointerWrapperStyle: {
position: 'fixed',
Expand Down