fix(react/tooltip): resolve tooltip misalignment issue with popup modal items for V1

[tinaClin]

For Tonic V1, the Tooltip also misaligns with popup modal items.

Issue: #893

Refer to the PR: #894

Demo:

• https://trendmicro-frontend.github.io/tonic-ui-demo/react/pr-925/components/tooltip
• https://trendmicro-frontend.github.io/tonic-ui-demo/react/pr-925/components/overflow-tooltip

[codesandbox]

Review or Edit in CodeSandbox

Open the branch in Web Editor • VS Code • Insiders

Open Preview

[changeset-bot]

🦋 Changeset detected

Latest commit: 4e7a4f0

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@tonic-ui/react	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[codiumai-pr-agent-free]

PR Reviewer Guide 🔍

(Review updated until commit 4e7a4f0)

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Key issues to review Performance Concern The computation of tooltip position in the offset modifier is complex and may impact performance, especially with frequent cursor movements. Potential Bug The removal of the placement override for followCursor and nextToCursor might cause unexpected tooltip positioning in these modes.

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 71.48%. Comparing base (5072b31) to head (4e7a4f0).
Report is 1 commits behind head on v1.

Additional details and impacted files

@@            Coverage Diff             @@
##               v1     #925      +/-   ##
==========================================
+ Coverage   71.33%   71.48%   +0.14%     
==========================================
  Files         374      375       +1     
  Lines        6273     6284      +11     
==========================================
+ Hits         4475     4492      +17     
+ Misses       1798     1792       -6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[trendmicro-frontend-bot]

Tonic UI Demo

On 2024-09-19 07:00:42 +0000, PR #925 (4e7a4f0) was successfully deployed. You can view it at the following link:
https://trendmicro-frontend.github.io/tonic-ui-demo/react/pr-925/

[cheton]

/review

[cheton]

/improve

[codiumai-pr-agent-free]

Persistent review updated to latest commit 4e7a4f0

[codiumai-pr-agent-free]

PR Code Suggestions ✨

Latest suggestions up to 4e7a4f0

Category	Suggestion	Score
Maintainability	Extract complex placement logic into a separate function Consider extracting the complex placement logic into a separate function to improve code readability and maintainability. packages/react/src/tooltip/TooltipContent.js [122-136] -if (placement.startsWith('top') || placement.startsWith('bottom')) { +const adjustPlacementForTopBottom = () => { if (placement.endsWith('start')) { computedSkidding += ensureFiniteNumber(mousePageX - elementPageX - popper.width); } else if (placement.endsWith('end')) { computedSkidding += ensureFiniteNumber(mousePageX - elementPageX - reference.width + popper.width); } else { computedSkidding += ensureFiniteNumber(mousePageX - elementPageX - reference.width / 2); } if (placement.startsWith('top')) { computedDistance += ensureFiniteNumber(elementPageY - mousePageY); } else if (placement.startsWith('bottom')) { computedDistance += ensureFiniteNumber(mousePageY - elementPageY - reference.height); } +}; + +if (placement.startsWith('top') || placement.startsWith('bottom')) { + adjustPlacementForTopBottom(); } Apply this suggestion Suggestion importance[1-10]: 8 Why: Extracting complex logic into a separate function enhances code readability and maintainability, making it easier to understand and modify in the future.	8
	Improve variable naming for better code readability Consider using a more descriptive variable name for computedSkidding and computedDistance to improve code readability. packages/react/src/tooltip/TooltipContent.js [109-110] -let computedSkidding = ensureFiniteNumber(skidding); -let computedDistance = ensureFiniteNumber(distance); +let adjustedSkidding = ensureFiniteNumber(skidding); +let adjustedDistance = ensureFiniteNumber(distance); Apply this suggestion Suggestion importance[1-10]: 6 Why: The suggestion to use more descriptive variable names improves code readability and maintainability, but it does not address any functional issues.	6
Enhancement	Add explicit placement control to the OverflowTooltip component Consider adding a placement prop to the OverflowTooltip component to explicitly control its positioning relative to the menu items. packages/react-docs/pages/components/overflow-tooltip/faq-misalignment-with-menu-items.js [13-18] <OverflowTooltip label={children} PopperProps={{ usePortal: true }} + placement="right" > {children} </OverflowTooltip> Apply this suggestion Suggestion importance[1-10]: 7 Why: Adding a placement prop to the OverflowTooltip component can improve its positioning control, which is beneficial for aligning tooltips with menu items. This suggestion enhances usability but is not critical.	7
Performance	Use switch statements for handling different placement scenarios Consider using a switch statement instead of multiple if-else conditions for handling different placement scenarios to improve code readability and performance. packages/react/src/tooltip/TooltipContent.js [122-154] -if (placement.startsWith('top') || placement.startsWith('bottom')) { - if (placement.endsWith('start')) { - computedSkidding += ensureFiniteNumber(mousePageX - elementPageX - popper.width); - } else if (placement.endsWith('end')) { - computedSkidding += ensureFiniteNumber(mousePageX - elementPageX - reference.width + popper.width); - } else { - computedSkidding += ensureFiniteNumber(mousePageX - elementPageX - reference.width / 2); - } - - if (placement.startsWith('top')) { - computedDistance += ensureFiniteNumber(elementPageY - mousePageY); - } else if (placement.startsWith('bottom')) { - computedDistance += ensureFiniteNumber(mousePageY - elementPageY - reference.height); - } +switch (true) { + case placement.startsWith('top'): + case placement.startsWith('bottom'): + switch (true) { + case placement.endsWith('start'): + computedSkidding += ensureFiniteNumber(mousePageX - elementPageX - popper.width); + break; + case placement.endsWith('end'): + computedSkidding += ensureFiniteNumber(mousePageX - elementPageX - reference.width + popper.width); + break; + default: + computedSkidding += ensureFiniteNumber(mousePageX - elementPageX - reference.width / 2); + } + computedDistance += placement.startsWith('top') + ? ensureFiniteNumber(elementPageY - mousePageY) + : ensureFiniteNumber(mousePageY - elementPageY - reference.height); + break; + case placement.startsWith('left'): + case placement.startsWith('right'): + switch (true) { + case placement.endsWith('start'): + computedSkidding += ensureFiniteNumber(mousePageY - elementPageY - popper.height); + break; + case placement.endsWith('end'): + computedSkidding += ensureFiniteNumber(mousePageY - elementPageY - reference.height + popper.height); + break; + default: + computedSkidding += ensureFiniteNumber(mousePageY - elementPageY - reference.height / 2); + } + computedDistance += placement.startsWith('left') + ? ensureFiniteNumber(elementPageX - mousePageX) + : ensureFiniteNumber(mousePageX - elementPageX - reference.width); + break; } -if (placement.startsWith('left') || placement.startsWith('right')) { - if (placement.endsWith('start')) { - computedSkidding += ensureFiniteNumber(mousePageY - elementPageY - popper.height); - } else if (placement.endsWith('end')) { - computedSkidding += ensureFiniteNumber(mousePageY - elementPageY - reference.height + popper.height); - } else { - computedSkidding += ensureFiniteNumber(mousePageY - elementPageY - reference.height / 2); - } - - if (placement.startsWith('left')) { - computedDistance += ensureFiniteNumber(elementPageX - mousePageX); - } else if (placement.startsWith('right')) { - computedDistance += ensureFiniteNumber(mousePageX - elementPageX - reference.width); - } -} - Apply this suggestion Suggestion importance[1-10]: 7 Why: Using switch statements can improve code readability and potentially performance, but the impact is minor since the existing if-else structure is already functional.	7

---

Previous suggestions

Suggestions up to commit 471e1bd

Category	Suggestion	Score
Maintainability	Extract complex offset calculation logic into a separate function Consider extracting the complex offset calculation logic into a separate function to improve readability and maintainability of the code. packages/react/src/tooltip/TooltipContent.js [102-151] offset: ({ placement, reference, popper }) => { - let computedSkidding = ensureFiniteNumber(skidding); - let computedDistance = ensureFiniteNumber(distance); - - if (isHTMLElement(tooltipTriggerElement) && (followCursor || nextToCursor)) { - // @see https://sentry.io/answers/how-do-i-get-the-position-x-y-of-an-html-element/ - - // Get the window coordinate - const rect = tooltipTriggerElement.getBoundingClientRect(); - - // Get the page coordinate - const elementPageX = rect.x + globalThis.scrollX; - const elementPageY = rect.y + globalThis.scrollY; - - // top, top-start, top-end, bottom, bottom-start, bottom-end - if (placement.startsWith('top') || placement.startsWith('bottom')) { - if (placement.endsWith('start')) { - computedSkidding += ensureFiniteNumber(mousePageX - elementPageX - popper.width); - } else if (placement.endsWith('end')) { - computedSkidding += ensureFiniteNumber(mousePageX - elementPageX - reference.width + popper.width); - } else { - computedSkidding += ensureFiniteNumber(mousePageX - elementPageX - reference.width / 2); - } - - if (placement.startsWith('top')) { - computedDistance += ensureFiniteNumber(elementPageY - mousePageY); - } else if (placement.startsWith('bottom')) { - computedDistance += ensureFiniteNumber(mousePageY - elementPageY - reference.height); - } - } - - // left, left-start, left-end, right, right-start, right-end - if (placement.startsWith('left') || placement.startsWith('right')) { - if (placement.endsWith('start')) { - computedSkidding += ensureFiniteNumber(mousePageY - elementPageY - popper.height); - } else if (placement.endsWith('end')) { - computedSkidding += ensureFiniteNumber(mousePageY - elementPageY - reference.height + popper.height); - } else { - computedSkidding += ensureFiniteNumber(mousePageY - elementPageY - reference.height / 2); - } - - if (placement.startsWith('left')) { - computedDistance += ensureFiniteNumber(elementPageX - mousePageX); - } else if (placement.startsWith('right')) { - computedDistance += ensureFiniteNumber(mousePageX - elementPageX - reference.width); - } - } - } - - return [computedSkidding, computedDistance]; + return calculateTooltipOffset({ + placement, + reference, + popper, + skidding, + distance, + tooltipTriggerElement, + followCursor, + nextToCursor, + mousePageX, + mousePageY + }); }, Suggestion importance[1-10]: 8 Why: Extracting the complex offset calculation into a separate function significantly improves code readability and maintainability, making it easier to understand and modify in the future.	8
Possible issue	Add a check for the existence of globalThis before using it Consider adding a check for the existence of globalThis before using it, as it might not be available in all JavaScript environments. packages/react/src/tooltip/TooltipContent.js [114-115] // Get the page coordinate -const elementPageX = rect.x + globalThis.scrollX; -const elementPageY = rect.y + globalThis.scrollY; +const scrollX = typeof globalThis !== 'undefined' ? globalThis.scrollX : window.pageXOffset; +const scrollY = typeof globalThis !== 'undefined' ? globalThis.scrollY : window.pageYOffset; +const elementPageX = rect.x + scrollX; +const elementPageY = rect.y + scrollY; Suggestion importance[1-10]: 7 Why: Adding a check for globalThis ensures compatibility across different JavaScript environments, which is important for preventing potential runtime errors.	7
Enhancement	Improve variable naming for better code readability Consider using a more descriptive variable name for mousePageX and mousePageY. For example, cursorPageX and cursorPageY would better convey that these variables represent the cursor position on the page. packages/react/src/tooltip/TooltipContent.js [93-96] const [ skidding = 0, distance = 8, ] = ensureArray(offset); +const cursorPageX = mousePageX; +const cursorPageY = mousePageY; Suggestion importance[1-10]: 5 Why: The suggestion to rename mousePageX and mousePageY to cursorPageX and cursorPageY improves code readability by making the variable purpose clearer, but it is not crucial for functionality.	5
	Use object destructuring for offset array to improve readability Consider using object destructuring for the offset array to improve code readability and make it clearer what each value represents. packages/react/src/tooltip/TooltipContent.js [93-96] -const [ - skidding = 0, - distance = 8, -] = ensureArray(offset); +const [skidding = 0, distance = 8] = ensureArray(offset); Suggestion importance[1-10]: 3 Why: The suggestion to use object destructuring for the offset array is a minor readability improvement and does not significantly impact the code's functionality or maintainability.	3