From 885ff708dfd6528813c4a86e9c27f84ebce3df53 Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Wed, 17 May 2023 12:02:03 +0800 Subject: [PATCH 1/4] remove multiple sources of tooltip size --- .../Tooltip/TooltipRenderedOnPageBody.js | 27 ++--- src/styles/getTooltipStyles.js | 112 ++++++++++-------- 2 files changed, 69 insertions(+), 70 deletions(-) diff --git a/src/components/Tooltip/TooltipRenderedOnPageBody.js b/src/components/Tooltip/TooltipRenderedOnPageBody.js index 360bcca2d790..79798c1b747d 100644 --- a/src/components/Tooltip/TooltipRenderedOnPageBody.js +++ b/src/components/Tooltip/TooltipRenderedOnPageBody.js @@ -60,14 +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 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 [tooltipContentWidth, setTooltipContentWidth] = useState(undefined); - const [tooltipWidth, setTooltipWidth] = useState(0); - const [tooltipHeight, setTooltipHeight] = useState(0); + const [tooltipContentHeight, setTooltipContentHeight] = useState(undefined); const contentRef = useRef(); - const wrapper = useRef(); useEffect(() => { if (!props.renderTooltipContent || !props.text) { @@ -79,11 +77,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 = wrapper.current.getBoundingClientRect(); - - setTooltipWidth(rect.width); - setTooltipHeight(rect.height); - setTooltipContentWidth(contentRef.current.offsetWidth); + const rect = contentRef.current.getBoundingClientRect(); + setTooltipContentWidth(rect.width); + setTooltipContentHeight(rect.height); }, []); const {animationStyle, tooltipWrapperStyle, tooltipTextStyle, pointerWrapperStyle, pointerStyle} = useMemo( @@ -96,9 +92,8 @@ const TooltipRenderedOnPageBody = (props) => { props.wrapperWidth, props.wrapperHeight, props.maxWidth, - tooltipWidth, - tooltipHeight, tooltipContentWidth, + tooltipContentHeight, props.shiftHorizontal, props.shiftVertical, ), @@ -110,9 +105,8 @@ const TooltipRenderedOnPageBody = (props) => { props.wrapperWidth, props.wrapperHeight, props.maxWidth, - tooltipWidth, - tooltipHeight, tooltipContentWidth, + tooltipContentHeight, props.shiftHorizontal, props.shiftVertical, ], @@ -138,10 +132,7 @@ const TooltipRenderedOnPageBody = (props) => { } return ReactDOM.createPortal( - + {content} diff --git a/src/styles/getTooltipStyles.js b/src/styles/getTooltipStyles.js index 42ad2df12474..a5a4b1ef1811 100644 --- a/src/styles/getTooltipStyles.js +++ b/src/styles/getTooltipStyles.js @@ -88,9 +88,8 @@ function isOverlappingAtTop(xOffset, yOffset) { * @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} tooltipContentWidth - The tooltip's inner content width. + * @param {Number} tooltipContentHeight - The tooltip's inner content 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. @@ -106,47 +105,53 @@ export default function getTooltipStyles( componentWidth, componentHeight, maxWidth, - tooltipWidth, - tooltipHeight, tooltipContentWidth, + tooltipContentHeight, manualShiftHorizontal = 0, manualShiftVertical = 0, ) { - // 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); - - // 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, componentWidth, 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 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: tooltipContentWidth ignores the fractions (OffsetWidth) so add 1px to fit the text properly. - const wrapperWidth = tooltipContentWidth && tooltipContentWidth + spacing.ph2.paddingHorizontal * 2 + 1; + // We get 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 position hasn't finished measuring yet. This prevents UI jank where the tooltip flashes in the top left corner of the screen. const opacity = xOffset === 0 && yOffset === 0 ? 0 : 1; - const isTooltipSizeReady = tooltipWidth !== 0 && tooltipHeight !== 0; - const scale = !isTooltipSizeReady ? 1 : currentSize; + const isTooltipSizeReady = tooltipWidth !== undefined && tooltipHeight !== undefined; + let shouldShowBelow = false; + let scale = 1; + let horizontalShift = 0; + let horizontalShiftPointer = 0; let wrapperTop = 0; let wrapperLeft = 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); + + 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, componentWidth, 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 @@ -174,6 +179,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. wrapperLeft = xOffset + (componentWidth / 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) Due to the tip start from the left edge of wrapper Tooltip so we have to remove the + // horizontalShift which is added to adjust it into the Window + pointerWrapperLeft = horizontalShiftPointer + (tooltipWidth / 2 - POINTER_WIDTH / 2); + + pointerAdditionalStyle = shouldShowBelow ? styles.flipUpsideDown : {}; } return { @@ -190,7 +216,7 @@ export default function getTooltipStyles( ...tooltipVerticalPadding, ...spacing.ph2, zIndex: variables.tooltipzIndex, - width: wrapperWidth, + width: tooltipWidth, maxWidth, top: wrapperTop, left: wrapperLeft, @@ -202,32 +228,14 @@ export default function getTooltipStyles( tooltipTextStyle: { 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 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 - top: 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) Due to the tip start from the left edge of wrapper Tooltip so we have to remove the - // horizontalShift which is added to adjust it into the Window - left: horizontalShiftPointer + (tooltipWidth / 2 - POINTER_WIDTH / 2), - + top: pointerWrapperTop, + left: pointerWrapperLeft, opacity, }, pointerStyle: { @@ -241,7 +249,7 @@ export default function getTooltipStyles( borderLeftColor: colors.transparent, borderRightColor: colors.transparent, borderTopColor: themeColors.heading, - ...(shouldShowBelow ? styles.flipUpsideDown : {}), + ...pointerAdditionalStyle, }, }; } From aa073073d07f5f3a872d99db4da70bb23796fb3d Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Thu, 18 May 2023 16:19:35 +0800 Subject: [PATCH 2/4] add comment --- src/styles/getTooltipStyles.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/styles/getTooltipStyles.js b/src/styles/getTooltipStyles.js index a5a4b1ef1811..61761b88df2a 100644 --- a/src/styles/getTooltipStyles.js +++ b/src/styles/getTooltipStyles.js @@ -121,8 +121,10 @@ export default function getTooltipStyles( const opacity = xOffset === 0 && yOffset === 0 ? 0 : 1; const isTooltipSizeReady = tooltipWidth !== undefined && tooltipHeight !== undefined; - let shouldShowBelow = false; + + // 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 wrapperTop = 0; @@ -138,6 +140,7 @@ export default function getTooltipStyles( // we'll display it beneath its wrapped component rather than above it as usual. shouldShowBelow = yOffset - tooltipHeight < GUTTER_WIDTH || isOverlappingAtTop(xOffset, yOffset); + // 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 a99316a90e700d63107fa450a1040bf9b3ca2006 Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Fri, 19 May 2023 12:39:55 +0800 Subject: [PATCH 3/4] update comment --- 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 61761b88df2a..96c0657ec878 100644 --- a/src/styles/getTooltipStyles.js +++ b/src/styles/getTooltipStyles.js @@ -198,8 +198,8 @@ export default function getTooltipStyles( // 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) Due to the tip start from the left edge of wrapper Tooltip so we have to remove the - // horizontalShift which is added to adjust it into the Window + // 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 : {}; From 2c20c8a2a515c0ce637fb32881885b168f4db87c Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Fri, 19 May 2023 12:45:54 +0800 Subject: [PATCH 4/4] fix lint --- 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 96c0657ec878..9fa33577798c 100644 --- a/src/styles/getTooltipStyles.js +++ b/src/styles/getTooltipStyles.js @@ -198,7 +198,7 @@ export default function getTooltipStyles( // 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 + // 3) Remove the wrapper's horizontalShift to maintain the pointer // at the center of the hovered component. pointerWrapperLeft = horizontalShiftPointer + (tooltipWidth / 2 - POINTER_WIDTH / 2);