fix(utils): fix tooltip arrow safe padding #3354
Conversation
WalkthroughThe change updates the arrow positioning logic in the PopperJS utility by reducing the safe margin around the arrow from 8 pixels to 4 pixels. This is achieved by introducing a constant for the margin value and replacing previous hardcoded values with this constant. The adjustment affects how closely the arrow can be positioned to the edges of the popper. Comments are also updated to reflect the new margin value. No changes are made to exported or public entity declarations. Changes
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
packages/utils/src/popper/index.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-vue". (The package "eslint-plugin-vue" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-vue" was referenced from the config file in ".eslintrc.js » @antfu/eslint-config » @antfu/eslint-config-vue". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Walkthrough此PR修改了tooltip箭头的安全边距，将popper的边距从8px改为4px，以解决在reference元素宽度较小时，tooltip箭头无法对齐的问题。 Changes
|
![petercat-assistant[bot]](https://avatars.githubusercontent.com/in/875736?s=60&v=4){kind=small}
| // 猜测是上下边距留下4px的距离。 确保箭头不太靠顶靠底。 | ||
| // 此时sideValue为“popper的顶- 箭头 - 4px” 的位置。 | ||
| const safeLeft = 4 | ||
| sideValue = Math.max(Math.min(popper[calcProp] - arrowSize - safeLeft, sideValue), safeLeft) | ||
| arrowStyle[side] = sideValue | ||
| arrowStyle[altSide] = '' | ||
|
|
||
| // adjustArrow此处还要校正一下，但不明白为什么只校正left, 不校正top的位置？ |
There was a problem hiding this comment.
The adjustment logic for the arrow position only considers the 'left' side when adjustArrow is true and the placement is 'top' or 'bottom'. Ensure that this logic is correct and that 'top' adjustments are not needed in this context.
WalkthroughThis PR modified the safe margin of the tooltip arrow and changed the margin of the popper from 8px to 4px to solve the problem that the tooltip arrow cannot be aligned when the reference element width is small. Changes
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/utils/src/popper/index.ts (2)
706-710: Suggest updating comments to reflect new margin value.The comments still reference "4px" explicitly which could cause confusion if the value changes in the future. Consider updating the comment to use the variable name instead of the explicit value.
- // 猜测是上下边距留下4px的距离。 确保箭头不太靠顶靠底。 - // 此时sideValue为"popper的顶- 箭头 - 4px" 的位置。 + // 猜测是上下边距留下安全距离。 确保箭头不太靠顶靠底。 + // 此时sideValue为"popper的顶- 箭头 - safeLeft" 的位置。
708-708: Enhance code maintainability with descriptive naming.Consider renaming
safeLeftto something more descriptive of its purpose likearrowSafeMarginorarrowEdgePadding, as it's used for both left/right and top/bottom positioning.-const safeLeft = 4 +const arrowSafeMargin = 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/utils/src/popper/index.ts(1 hunks)
🔇 Additional comments (3)
packages/utils/src/popper/index.ts (3)
708-709: Good improvement to the arrow padding.The change introduces a named constant
safeLeftwith a value of 4px (reduced from the previous 8px) and uses it to constrain the arrow position. This adjustment allows the arrow to be positioned closer to the edges of the popper, which should help with arrow alignment when the reference element has a small width, as mentioned in the PR objectives.
717-717: Consistent use of the new constant value.Good job consistently applying the new
safeLeftconstant (4px) here as well, replacing the previous hardcoded value of 8px. This ensures the arrow positioning is consistent throughout the code.
708-709:❓ Verification inconclusive
Verify the fix works on all browsers and with RTL layouts.
The change looks good, but it would be valuable to verify that this adjustment works properly in all supported browsers, particularly with right-to-left language layouts where alignment might be different.
🏁 Script executed:
#!/bin/bash # Check for RTL-specific handling in the codebase echo "Checking for RTL-specific handling that might be affected by the arrow positioning change:" rg --ignore-case "rtl" -A 3 -B 3Length of output: 66153
🏁 Script executed:
#!/bin/bash # Display context around the arrow positioning adjustment in popper utils sed -n '650,780p' packages/utils/src/popper/index.tsLength of output: 4988
Please verify cross-browser and RTL behavior of the arrow offset change
I don’t see any RTL-specific adjustments in the
arrowmethod (packages/utils/src/popper/index.ts around lines 708–709), so you’ll want to:• Test the new
safeLeftconstraint under right-to-left layouts (e.g.direction="rtl") for both top/bottom and left/right placements
• Confirm the arrow still aligns correctly in all supported browsers (Chrome, Firefox, Safari, IE11, etc.)
PR
修改popper的边距 从8px改完4px， 解决refrence元素宽度小的情况下，tooltip箭头对不齐问题
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit