Skip to content

Commit

Permalink
Merge pull request #20340 from bernhardoj/fix/17555-regression-2
Browse files Browse the repository at this point in the history
Fix tooltip wrong height and recalculate tooltip size when locale changes
  • Loading branch information
amyevans authored Jun 21, 2023
2 parents c2059ef + b438183 commit 0b7455f
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 14 deletions.
16 changes: 8 additions & 8 deletions src/components/Tooltip/TooltipRenderedOnPageBody.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,12 @@ const defaultProps = {
// There will be n number of tooltip components in the page.
// It's good to memoize this one.
const TooltipRenderedOnPageBody = (props) => {
// The width and height of tooltip's inner content. Has to be undefined in the beginning
// as a width/height of 0 will cause the content to be rendered of a width/height of 0,
// The width of tooltip's inner content. Has to be undefined in the beginning
// as a width of 0 will cause the content to be rendered of a width of 0,
// which prevents us from measuring it correctly.
const [contentMeasuredWidth, setContentMeasuredWidth] = useState(undefined);
const [contentMeasuredHeight, setContentMeasuredHeight] = useState(undefined);
// The height of tooltip's wrapper.
const [wrapperMeasuredHeight, setWrapperMeasuredHeight] = useState(undefined);
const contentRef = useRef();
const rootWrapper = useRef();

Expand All @@ -78,9 +79,8 @@ const TooltipRenderedOnPageBody = (props) => {
useLayoutEffect(() => {
// Calculate the tooltip width and height before the browser repaints the screen to prevent flicker
// because of the late update of the width and the height from onLayout.
const rect = contentRef.current.getBoundingClientRect();
setContentMeasuredWidth(rect.width);
setContentMeasuredHeight(rect.height);
setContentMeasuredWidth(contentRef.current.getBoundingClientRect().width);
setWrapperMeasuredHeight(rootWrapper.current.getBoundingClientRect().height);
}, []);

const {animationStyle, rootWrapperStyle, textStyle, pointerWrapperStyle, pointerStyle} = useMemo(
Expand All @@ -94,7 +94,7 @@ const TooltipRenderedOnPageBody = (props) => {
props.targetHeight,
props.maxWidth,
contentMeasuredWidth,
contentMeasuredHeight,
wrapperMeasuredHeight,
props.shiftHorizontal,
props.shiftVertical,
rootWrapper.current,
Expand All @@ -108,7 +108,7 @@ const TooltipRenderedOnPageBody = (props) => {
props.targetHeight,
props.maxWidth,
contentMeasuredWidth,
contentMeasuredHeight,
wrapperMeasuredHeight,
props.shiftHorizontal,
props.shiftVertical,
],
Expand Down
6 changes: 4 additions & 2 deletions src/components/Tooltip/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import withWindowDimensions from '../withWindowDimensions';
import * as tooltipPropTypes from './tooltipPropTypes';
import TooltipSense from './TooltipSense';
import * as DeviceCapabilities from '../../libs/DeviceCapabilities';
import compose from '../../libs/compose';
import withLocalize from '../withLocalize';

/**
* A component used to wrap an element intended for displaying a tooltip. The term "tooltip's target" refers to the
Expand Down Expand Up @@ -137,7 +139,7 @@ class Tooltip extends PureComponent {
renderTooltipContent={this.props.renderTooltipContent}
// We pass a key, so whenever the content changes this component will completely remount with a fresh state.
// This prevents flickering/moving while remaining performant.
key={[this.props.text, ...this.props.renderTooltipContentKey]}
key={[this.props.text, ...this.props.renderTooltipContentKey, this.props.preferredLocale]}
/>
)}
<BoundsObserver
Expand All @@ -158,4 +160,4 @@ class Tooltip extends PureComponent {

Tooltip.propTypes = tooltipPropTypes.propTypes;
Tooltip.defaultProps = tooltipPropTypes.defaultProps;
export default withWindowDimensions(Tooltip);
export default compose(withWindowDimensions, withLocalize)(Tooltip);
8 changes: 4 additions & 4 deletions src/styles/getTooltipStyles.js
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ function isOverlappingAtTop(xOffset, yOffset, tooltip, tooltipTargetWidth, toolt
* @param {Number} tooltipTargetHeight - The height of the tooltip's target
* @param {Number} maxWidth - The tooltip's max width.
* @param {Number} tooltipContentWidth - The tooltip's inner content measured width.
* @param {Number} tooltipContentHeight - The tooltip's inner content measured height.
* @param {Number} tooltipWrapperHeight - The tooltip's wrapper measured height.
* @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 @@ -131,18 +131,18 @@ export default function getTooltipStyles(
tooltipTargetHeight,
maxWidth,
tooltipContentWidth,
tooltipContentHeight,
tooltipWrapperHeight,
manualShiftHorizontal = 0,
manualShiftVertical = 0,
tooltip,
) {
const tooltipVerticalPadding = spacing.pv1;

// We calculate tooltip width and height based on the tooltip's content width and height
// We calculate tooltip width based on the tooltip's content width
// so the tooltip wrapper is just big enough to fit content and prevent white space.
// NOTE: Add 1 to the tooltipWidth to prevent truncated text in Safari
const tooltipWidth = tooltipContentWidth && tooltipContentWidth + spacing.ph2.paddingHorizontal * 2 + 1;
const tooltipHeight = tooltipContentHeight && tooltipContentHeight + tooltipVerticalPadding.paddingVertical * 2;
const tooltipHeight = tooltipWrapperHeight;

const isTooltipSizeReady = tooltipWidth !== undefined && tooltipHeight !== undefined;

Expand Down

0 comments on commit 0b7455f

Please sign in to comment.