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.

Sign up for GitHub

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

Remove multiple sources of tooltip size #19097

Merged
merged 8 commits into from
May 19, 2023
Merged

Remove multiple sources of tooltip size #19097

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
885ff70
remove multiple sources of tooltip size
bernhardoj May 17, 2023
aa07307
add comment
bernhardoj May 18, 2023
e023c47
Merge branch 'main' into fix/17555-regression
bernhardoj May 18, 2023
31e6257
Merge branch 'main' into fix/17555-regression
bernhardoj May 19, 2023
a99316a
update comment
bernhardoj May 19, 2023
2c20c8a
fix lint
bernhardoj May 19, 2023
be85b0c
Merge branch 'main' into fix/17555-regression
bernhardoj May 19, 2023
7cc963b
Merge branch 'main' into fix/17555-regression
bernhardoj May 19, 2023
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
21 changes: 8 additions & 13 deletions src/components/Tooltip/TooltipRenderedOnPageBody.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,11 @@ 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 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,
// 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,
// which prevents us from measuring it correctly.
const [contentMeasuredWidth, setContentMeasuredWidth] = useState(undefined);
const [wrapperMeasuredWidth, setWrapperMeasuredWidth] = useState(0);
const [wrapperMeasuredHeight, setWrapperMeasuredHeight] = useState(0);
const [contentMeasuredHeight, setContentMeasuredHeight] = useState(undefined);
const contentRef = useRef();
const rootWrapper = useRef();

Expand All @@ -79,11 +78,9 @@ 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 = rootWrapper.current.getBoundingClientRect();

setWrapperMeasuredWidth(rect.width);
setWrapperMeasuredHeight(rect.height);
setContentMeasuredWidth(contentRef.current.offsetWidth);
const rect = contentRef.current.getBoundingClientRect();
setContentMeasuredWidth(rect.width);
setContentMeasuredHeight(rect.height);
}, []);

const {animationStyle, rootWrapperStyle, textStyle, pointerWrapperStyle, pointerStyle} = useMemo(
Expand All @@ -96,9 +93,8 @@ const TooltipRenderedOnPageBody = (props) => {
props.targetWidth,
props.targetHeight,
props.maxWidth,
wrapperMeasuredWidth,
wrapperMeasuredHeight,
contentMeasuredWidth,
contentMeasuredHeight,
props.shiftHorizontal,
props.shiftVertical,
rootWrapper.current,
Expand All @@ -111,9 +107,8 @@ const TooltipRenderedOnPageBody = (props) => {
props.targetWidth,
props.targetHeight,
props.maxWidth,
wrapperMeasuredWidth,
wrapperMeasuredHeight,
contentMeasuredWidth,
contentMeasuredHeight,
props.shiftHorizontal,
props.shiftVertical,
],
Expand Down
119 changes: 62 additions & 57 deletions src/styles/getTooltipStyles.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,8 @@ function isOverlappingAtTop(xOffset, yOffset, tooltip) {
* @param {Number} tooltipTargetWidth - The width of the tooltip's target
* @param {Number} tooltipTargetHeight - The height of the tooltip's target
* @param {Number} maxWidth - The tooltip's max width.
* @param {Number} tooltipWidth - The measured width of the tooltip
* @param {Number} tooltipHeight - The measured height of the tooltip
* @param {Number} tooltipContentWidth - The tooltip's inner content measured width.
* @param {Number} tooltipContentHeight - The tooltip's inner content 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 @@ -109,49 +108,54 @@ export default function getTooltipStyles(
tooltipTargetWidth,
tooltipTargetHeight,
maxWidth,
tooltipWidth,
tooltipHeight,
tooltipContentWidth,
tooltipContentHeight,
manualShiftHorizontal = 0,
manualShiftVertical = 0,
tooltip,
) {
// Determine if the tooltip should display below the wrapped component.
// If either a tooltip will try to render within GUTTER_WIDTH logical pixels of the top of the screen,
// Or the wrapped component is overlapping at top-left with another element
// we'll display it beneath its wrapped component rather than above it as usual.
const shouldShowBelow = yOffset - tooltipHeight < GUTTER_WIDTH || isOverlappingAtTop(xOffset, yOffset, tooltip);

// Determine if we need to shift the tooltip horizontally to prevent it
// from displaying too near to the edge of the screen.
const horizontalShift = computeHorizontalShift(windowWidth, xOffset, tooltipTargetWidth, tooltipWidth, manualShiftHorizontal);

// Determine if we need to shift the pointer horizontally to prevent it from being too near to the edge of the tooltip
// We shift it to the right a bit if the tooltip is positioned on the extreme left
// and shift it to left a bit if the tooltip is positioned on the extreme right.
const horizontalShiftPointer =
horizontalShift > 0
? Math.max(-horizontalShift, -(tooltipWidth / 2) + POINTER_WIDTH / 2 + variables.componentBorderRadiusSmall)
: Math.min(-horizontalShift, tooltipWidth / 2 - POINTER_WIDTH / 2 - variables.componentBorderRadiusSmall);

const tooltipVerticalPadding = spacing.pv1;
const tooltipFontSize = variables.fontSizeSmall;

// We calculate 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: tooltipContentWidth ignores the fractions (OffsetWidth) so add 1px to fit the text properly.
const rootWrapperWidth = tooltipContentWidth && tooltipContentWidth + spacing.ph2.paddingHorizontal * 2 + 1;

const isTooltipSizeReady = tooltipWidth !== 0 && tooltipHeight !== 0;
// We calculate tooltip width and height based on the tooltip's content width and height
// so the tooltip wrapper is just big enough to fit content and prevent white space.
const tooltipWidth = tooltipContentWidth && tooltipContentWidth + spacing.ph2.paddingHorizontal * 2;
const tooltipHeight = tooltipContentHeight && tooltipContentHeight + tooltipVerticalPadding.paddingVertical * 2;

// Hide the tooltip entirely if it's size hasn't finished measuring yet. This prevents UI jank where the tooltip flashes close to its expected position.
const opacity = isTooltipSizeReady ? 1 : 0;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because now we depends on isTooltipsizeReady to set the opacity (previously xOffset and yOffset), we can safely remove it because the tooltip won't be shown if useLayoutEffect hasn't done yet..

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

const scale = !isTooltipSizeReady ? 1 : currentSize;
// Set the scale to 1 to be able to measure the toolip size correctly when it's not ready yet.
let scale = 1;
let shouldShowBelow = false;
let horizontalShift = 0;
let horizontalShiftPointer = 0;
let rootWrapperTop = 0;
let rootWrapperLeft = 0;
let pointerWrapperTop = 0;
let pointerWrapperLeft = 0;
let pointerAdditionalStyle = {};

if (isTooltipSizeReady) {
// Determine if the tooltip should display below the wrapped component.
// If either a tooltip will try to render within GUTTER_WIDTH logical pixels of the top of the screen,
// Or the wrapped component is overlapping at top-left with another element
// we'll display it beneath its wrapped component rather than above it as usual.
shouldShowBelow = yOffset - tooltipHeight < GUTTER_WIDTH || isOverlappingAtTop(xOffset, yOffset, tooltip);

// When the tooltip size is ready, we can start animating the scale.
scale = currentSize;

// Determine if we need to shift the tooltip horizontally to prevent it
// from displaying too near to the edge of the screen.
horizontalShift = computeHorizontalShift(windowWidth, xOffset, tooltipTargetWidth, tooltipWidth, manualShiftHorizontal);

// Determine if we need to shift the pointer horizontally to prevent it from being too near to the edge of the tooltip
// We shift it to the right a bit if the tooltip is positioned on the extreme left
// and shift it to left a bit if the tooltip is positioned on the extreme right.
horizontalShiftPointer =
horizontalShift > 0
? Math.max(-horizontalShift, -(tooltipWidth / 2) + POINTER_WIDTH / 2 + variables.componentBorderRadiusSmall)
: Math.min(-horizontalShift, tooltipWidth / 2 - POINTER_WIDTH / 2 - variables.componentBorderRadiusSmall);

// Because it uses fixed positioning, the top-left corner of the tooltip is aligned
// with the top-left corner of the window by default.
// we will use yOffset to position the tooltip relative to the Wrapped Component
Expand Down Expand Up @@ -179,6 +183,27 @@ export default function getTooltipStyles(
// 3) Add the horizontal shift (left or right) computed above to keep it out of the gutters.
// 4) Lastly, add the manual horizontal shift passed in as a parameter.
rootWrapperLeft = xOffset + (tooltipTargetWidth / 2 - tooltipWidth / 2) + horizontalShift + manualShiftHorizontal;

// By default, the pointer's top-left will align with the top-left of the tooltip wrapper.
//
// To align it vertically, we'll:
// If the pointer should be below the tooltip wrapper, shift the pointer down (+) by the tooltip height,
// so that the top of the pointer lines up with the bottom of the tooltip
//
// OR if the pointer should be above the tooltip wrapper, then the pointer up (-) by the pointer's height
// so that the bottom of the pointer lines up with the top of the tooltip
pointerWrapperTop = shouldShowBelow ? -POINTER_HEIGHT : tooltipHeight;

// To align it horizontally, we'll:
// 1) Shift the pointer to the right (+) by the half the tooltipWidth's width,
// so the left edge of the pointer lines up with the tooltipWidth's center.
// 2) To the left (-) by half the pointer's width,
// so the pointer's center lines up with the tooltipWidth's center.
// 3) Remove the wrapper's horizontalShift to maintain the pointer
// at the center of the hovered component.
pointerWrapperLeft = horizontalShiftPointer + (tooltipWidth / 2 - POINTER_WIDTH / 2);

pointerAdditionalStyle = shouldShowBelow ? styles.flipUpsideDown : {};
}

return {
Expand All @@ -195,45 +220,25 @@ export default function getTooltipStyles(
...tooltipVerticalPadding,
...spacing.ph2,
zIndex: variables.tooltipzIndex,
width: rootWrapperWidth,
width: tooltipWidth,
maxWidth,
top: rootWrapperTop,
left: rootWrapperLeft,
opacity,

// We are adding this to prevent the tooltip text from being selected and copied on CTRL + A.
...styles.userSelectNone,
},
textStyle: {
color: themeColors.textReversed,
fontFamily: fontFamily.EXP_NEUE,
fontSize: tooltipFontSize,
fontSize: variables.fontSizeSmall,
overflow: 'hidden',
lineHeight: variables.lineHeightSmall,
},
pointerWrapperStyle: {
position: 'fixed',

// By default, the pointer's top-left will align with the top-left of the tooltip tooltip.
//
// To align it vertically, we'll:
// If the pointer should be below the tooltip tooltip, shift the pointer down (+) by the tooltip height,
// so that the top of the pointer lines up with the bottom of the tooltip
//
// OR if the pointer should be above the tooltip tooltip, then the pointer up (-) by the pointer's height
// so that the bottom of the pointer lines up with the top of the tooltip
top: shouldShowBelow ? -POINTER_HEIGHT : tooltipHeight,

// To align it horizontally, we'll:
// 1) Shift the pointer to the right (+) by the half the rootWrapperWidth's width,
// so the left edge of the pointer lines up with the rootWrapperWidth's center.
// 2) To the left (-) by half the pointer's width,
// so the pointer's center lines up with the rootWrapperWidth's center.
// 3) Due to the tip start from the left edge of tooltip Tooltip so we have to remove the
// horizontalShift which is added to adjust it into the Window
left: horizontalShiftPointer + (tooltipWidth / 2 - POINTER_WIDTH / 2),

opacity,
top: pointerWrapperTop,
left: pointerWrapperLeft,
},
pointerStyle: {
width: 0,
Expand All @@ -246,7 +251,7 @@ export default function getTooltipStyles(
borderLeftColor: colors.transparent,
borderRightColor: colors.transparent,
borderTopColor: themeColors.heading,
...(shouldShowBelow ? styles.flipUpsideDown : {}),
...pointerAdditionalStyle,
},
};
}