Skip to content

Commit

Permalink
Fix: Chat - Link in end of line displays tooltip over text and not on…
Browse files Browse the repository at this point in the history
… link

Add a new prop to Tooltip called shouldUseMultilinePositioning. This
enables a new algorithm for finding the bounding box target for the
tooltip. In the case where the target has wrapped onto multiple lines
(e.g. it is a link in a chat window) then the link has multiple bounding
boxes, and we want to show the tooltip against the one that the user is
hovering over.

As part of this, extend Hoverable to pass the Event to its onHoverIn /
onHoverOut callbacks.

Enable this new algorithm from BaseAnchorForCommentsOnly, which is the
base class for links that show up in chat.
  • Loading branch information
ewanmellor committed Sep 19, 2023
1 parent 6d67552 commit c8257c9
Show file tree
Hide file tree
Showing 4 changed files with 139 additions and 60 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,10 @@ function BaseAnchorForCommentsOnly({onPressIn, onPressOut, href = '', rel = '',
accessibilityRole={CONST.ACCESSIBILITY_ROLE.LINK}
accessibilityLabel={href}
>
<Tooltip text={href}>
<Tooltip
text={href}
shouldUseMultilinePositioning
>
<Text
ref={(el) => (linkRef = el)}
style={StyleSheet.flatten([style, defaultTextStyle])}
Expand Down
20 changes: 12 additions & 8 deletions src/components/Hoverable/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,9 @@ class Hoverable extends Component {
* Sets the hover state of this component to true and execute the onHoverIn callback.
*
* @param {Boolean} isHovered - Whether or not this component is hovered.
* @param {Event} ev - The event that triggered this state change.
*/
setIsHovered(isHovered) {
setIsHovered(isHovered, ev) {
if (this.props.disabled) {
return;
}
Expand All @@ -94,7 +95,7 @@ class Hoverable extends Component {
if (this.isScrollingRef && this.props.shouldHandleScroll && !this.state.isHovered) return;

if (isHovered !== this.state.isHovered) {
this.setState({isHovered}, isHovered ? this.props.onHoverIn : this.props.onHoverOut);
this.setState({isHovered}, () => (isHovered ? this.props.onHoverIn : this.props.onHoverOut)(ev));
}
}

Expand All @@ -113,15 +114,18 @@ class Hoverable extends Component {
return;
}

this.setIsHovered(false);
this.setIsHovered(false, e);
}

handleVisibilityChange() {
/**
* @param {Event} ev - The visibility-change event object.
*/
handleVisibilityChange(ev) {
if (document.visibilityState !== 'hidden') {
return;
}

this.setIsHovered(false);
this.setIsHovered(false, ev);
}

render() {
Expand Down Expand Up @@ -154,14 +158,14 @@ class Hoverable extends Component {
}
},
onMouseEnter: (el) => {
this.setIsHovered(true);
this.setIsHovered(true, el);

if (_.isFunction(child.props.onMouseEnter)) {
child.props.onMouseEnter(el);
}
},
onMouseLeave: (el) => {
this.setIsHovered(false);
this.setIsHovered(false, el);

if (_.isFunction(child.props.onMouseLeave)) {
child.props.onMouseLeave(el);
Expand All @@ -171,7 +175,7 @@ class Hoverable extends Component {
// Check if the blur event occurred due to clicking outside the element
// and the wrapperView contains the element that caused the blur and reset isHovered
if (!this.wrapperView.contains(el.target) && !this.wrapperView.contains(el.relatedTarget)) {
this.setIsHovered(false);
this.setIsHovered(false, el);
}

if (_.isFunction(child.props.onBlur)) {
Expand Down
156 changes: 105 additions & 51 deletions src/components/Tooltip/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,49 @@ import useWindowDimensions from '../../hooks/useWindowDimensions';

const hasHoverSupport = DeviceCapabilities.hasHoverSupport();

/**
* Choose the correct bounding box for the tooltip to be positioned against.
* This handles the case where the target is wrapped across two lines, and
* so we need to find the correct part (the one that the user is hovering
* over) and show the tooltip there.
*
* This is only used when shouldUseMultilinePositioning == true.
*
* @param {Element} target The DOM element being hovered over.
* @param {number} clientX The X position from the MouseEvent.
* @param {number} clientY The Y position from the MouseEvent.
* @param {number} slop An allowed slop factor when searching for the bounding
* box. If the user is moving the mouse quickly we can end up getting a
* hover event with the position outside any of our bounding boxes. We retry
* with a small slop factor in that case, so if we have a bounding box close
* enough then we go with that.
* @return {DOMRect} The chosen bounding box.
*/
function chooseBoundingBox(target, clientX, clientY, slop = 0) {
const bbs = target.getClientRects();
for (let i = 0; i < bbs.length; i++) {
const bb = bbs[i];
if (bb.x - slop <= clientX && bb.x + bb.width + slop >= clientX && bb.y - slop <= clientY && bb.y + bb.height + slop >= clientY) {
return bb;
}
}
if (slop === 0) {
// Retry with a slop factor, in case the user is moving the mouse quickly.
return chooseBoundingBox(target, clientX, clientY, 5);
}
// Fall back to the full bounding box if we failed to find a matching one
// (shouldn't happen).
return target.getBoundingClientRect();
}

/**
* A component used to wrap an element intended for displaying a tooltip. The term "tooltip's target" refers to the
* wrapped element, which, upon hover, triggers the tooltip to be shown.
* @param {propTypes} props
* @returns {ReactNodeLike}
*/
function Tooltip(props) {
const {children, numberOfLines, maxWidth, text, renderTooltipContent, renderTooltipContentKey} = props;
const {children, numberOfLines, maxWidth, text, renderTooltipContent, renderTooltipContentKey, shouldUseMultilinePositioning} = props;

const {preferredLocale} = useLocalize();
const {windowWidth} = useWindowDimensions();
Expand All @@ -44,33 +79,59 @@ function Tooltip(props) {
const prevText = usePrevious(text);

/**
* Display the tooltip in an animation.
* Update the tooltip bounding rectangle.
*
* @param {Object} bounds - updated bounds
*/
const showTooltip = useCallback(() => {
if (!isRendered) {
setIsRendered(true);
const updateBounds = useCallback((bounds) => {
if (bounds.width === 0) {
setIsRendered(false);
}
setWrapperWidth(bounds.width);
setWrapperHeight(bounds.height);
setXOffset(bounds.x);
setYOffset(bounds.y);
}, []);

setIsVisible(true);

animation.current.stopAnimation();

// When TooltipSense is active, immediately show the tooltip
if (TooltipSense.isActive()) {
animation.current.setValue(1);
} else {
isTooltipSenseInitiator.current = true;
Animated.timing(animation.current, {
toValue: 1,
duration: 140,
delay: 500,
useNativeDriver: false,
}).start(({finished}) => {
isAnimationCanceled.current = !finished;
});
}
TooltipSense.activate();
}, [isRendered]);
/**
* Display the tooltip in an animation.
*/
const showTooltip = useCallback(
(ev) => {
if (shouldUseMultilinePositioning) {
if (ev) {
const {clientX, clientY, target} = ev;
const bb = chooseBoundingBox(target, clientX, clientY);
updateBounds(bb);
}
}

if (!isRendered) {
setIsRendered(true);
}

setIsVisible(true);

animation.current.stopAnimation();

// When TooltipSense is active, immediately show the tooltip
if (TooltipSense.isActive()) {
animation.current.setValue(1);
} else {
isTooltipSenseInitiator.current = true;
Animated.timing(animation.current, {
toValue: 1,
duration: 140,
delay: 500,
useNativeDriver: false,
}).start(({finished}) => {
isAnimationCanceled.current = !finished;
});
}
TooltipSense.activate();
},
[isRendered, shouldUseMultilinePositioning, updateBounds],
);

// eslint-disable-next-line rulesdir/prefer-early-return
useEffect(() => {
Expand All @@ -82,21 +143,6 @@ function Tooltip(props) {
}
}, [isVisible, text, prevText, showTooltip]);

/**
* Update the tooltip bounding rectangle
*
* @param {Object} bounds - updated bounds
*/
const updateBounds = (bounds) => {
if (bounds.width === 0) {
setIsRendered(false);
}
setWrapperWidth(bounds.width);
setWrapperHeight(bounds.height);
setXOffset(bounds.x);
setYOffset(bounds.y);
};

/**
* Hide the tooltip in an animation.
*/
Expand Down Expand Up @@ -126,6 +172,16 @@ function Tooltip(props) {
return children;
}

const hoverableChildren = (
<Hoverable
onHoverIn={showTooltip}
onHoverOut={hideTooltip}
shouldHandleScroll={props.shouldHandleScroll}
>
{children}
</Hoverable>
);

return (
<>
{isRendered && (
Expand All @@ -147,18 +203,16 @@ function Tooltip(props) {
key={[text, ...renderTooltipContentKey, preferredLocale]}
/>
)}
<BoundsObserver
enabled={isVisible}
onBoundsChange={updateBounds}
>
<Hoverable
onHoverIn={showTooltip}
onHoverOut={hideTooltip}
shouldHandleScroll={props.shouldHandleScroll}
{shouldUseMultilinePositioning ? (
hoverableChildren
) : (
<BoundsObserver
enabled={isVisible}
onBoundsChange={updateBounds}
>
{children}
</Hoverable>
</BoundsObserver>
{hoverableChildren}
</BoundsObserver>
)}
</>
);
}
Expand Down
18 changes: 18 additions & 0 deletions src/components/Tooltip/tooltipPropTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,23 @@ const propTypes = {

/** passes this down to Hoverable component to decide whether to handle the scroll behaviour to show hover once the scroll ends */
shouldHandleScroll: PropTypes.bool,

/**
* Whether to use a different algorithm for positioning the tooltip.
*
* If true, when the user hovers over an element, we check whether that
* has multiple bounding boxes (i.e. it is text that has wrapped onto two
* lines). If it does, we select the correct bounding box to use for the
* tooltip.
*
* If false, the tooltip is positioned relative to the center of the
* target element. This is more performant, because it does not need to
* do any extra work inside the onmouseenter handler.
*
* Defaults to false. Set this to true if your tooltip target could wrap
* onto multiple lines.
*/
shouldUseMultilinePositioning: PropTypes.bool,
};

const defaultProps = {
Expand All @@ -42,6 +59,7 @@ const defaultProps = {
renderTooltipContent: undefined,
renderTooltipContentKey: [],
shouldHandleScroll: false,
shouldUseMultilinePositioning: false,
};

export {propTypes, defaultProps};

0 comments on commit c8257c9

Please sign in to comment.