Fix popover invisible in horizontal scroll #702
Conversation
Walkthrough该拉取请求包含对多个文件的更改，主要涉及版本更新和功能增强。在 Changes
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (
|
![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
🧹 Outside diff range and nitpick comments (3)
packages/shineout/src/popover/__doc__/changelog.cn.md (1)
1-5: 更新看起来不错！新的更新日志条目格式正确，内容清晰。它准确地描述了修复的问题，并提供了相关的 PR 链接。这与之前提供的 PR 目标一致。
为了保持一致性，建议在版本号和日期之间添加一个空行。例如：
## 3.4.3-beta.4 + 2024-10-09packages/hooks/src/utils/dom/element.tsx (1)
106-137: 代码实现良好，建议小幅优化性能这个新增的
getClosestScrollContainer函数实现得很好，逻辑清晰，考虑了各种边界情况。以下是一些观察和建议：
- 函数正确处理了输入元素为 null 的情况。
- 内部的
isScrollable函数全面检查了元素的可滚动性，包括样式和实际尺寸。- DOM 树的遍历逻辑正确。
- 当找不到可滚动祖先时，使用
document.scrollingElement或document.documentElement作为后备方案是恰当的。为了稍微优化性能，可以考虑缓存
window.getComputedStyle的结果：const isScrollable = (el: HTMLElement) => { - const style = window.getComputedStyle(el); + const style = el.style || window.getComputedStyle(el); const overflowX = style.overflowX; const overflowY = style.overflowY; return (overflowX === 'auto' || overflowX === 'scroll' || overflowY === 'auto' || overflowY === 'scroll') && (el.scrollHeight > el.clientHeight || el.scrollWidth > el.clientWidth); };这样可以避免对已经有内联样式的元素重复调用
getComputedStyle，从而略微提高性能。packages/hooks/src/common/use-position-style/index.ts (1)
344-344: 建议移除或条件化控制台日志在
updateStyle函数中添加的console.log语句可能对调试很有帮助，但不适合保留在生产代码中。建议采取以下措施之一：
- 如果这个日志仅用于临时调试，请在提交前删除它。
- 如果这个日志对于长期调试或监控很重要，考虑使用条件语句来控制它的输出，例如：
if (process.env.NODE_ENV === 'development') { console.log('show, position, absolute, updateKey, fixedWidth: >>', show, position, absolute, updateKey, fixedWidth); }这样可以确保日志只在开发环境中输出，不会影响生产环境的性能。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
- package.json (2 hunks)
- packages/hooks/src/common/use-position-style/index.ts (4 hunks)
- packages/hooks/src/components/use-popup/use-popup.ts (2 hunks)
- packages/hooks/src/utils/dom/element.tsx (1 hunks)
- packages/shineout/src/popover/doc/changelog.cn.md (1 hunks)
✅ Files skipped from review due to trivial changes (1)
- package.json
🧰 Additional context used
🔇 Additional comments (4)
packages/hooks/src/components/use-popup/use-popup.ts (1)
62-68: 代码改进：提高了弹出框定位的响应性和稳定性这个改动很好地解决了PR中提到的问题：
- 移除了
setTimeout，有助于解决间歇性闪烁的问题。- 立即计算和更新位置，提高了弹出框在滚动容器中的响应性。
- 只在位置发生变化时更新状态，减少了不必要的重渲染。
这些变更应该能有效改善弹出框在水平滚动场景中的可见性和稳定性。
packages/hooks/src/common/use-position-style/index.ts (3)
6-6: 新导入函数有助于改进定位逻辑引入
getClosestScrollContainer函数是一个好的改进。这个函数可以帮助我们找到最近的可滚动容器，从而更准确地计算弹出元素的位置，特别是在复杂的嵌套滚动容器中。
158-159: 改进了水平滚动情况下的定位逻辑这个修改很好地解决了在水平滚动容器中弹出元素可能变得不可见的问题。通过考虑最近滚动容器的滚动偏移量，弹出元素的位置现在可以更准确地计算，确保它始终保持在正确的位置上。这对于提高用户体验和组件的可用性非常重要。
建议：
- 考虑添加注释解释这个计算的目的，以便其他开发者更容易理解。
- 可以考虑将这个逻辑抽取为一个单独的函数，以提高代码的可读性和可维护性。
Also applies to: 176-176
Line range hint
1-355: 总体评价：有效解决了报告的问题这些更改很好地解决了水平滚动场景中弹出元素可见性的问题。主要改进包括：
- 引入了
getClosestScrollContainer函数，用于找到最近的可滚动容器。- 更新了
getAbsolutePositionStyle函数中的定位逻辑，考虑了最近滚动容器的滚动偏移量。- 添加了调试日志，有助于问题排查（建议在生产环境中移除或条件化）。
这些修改提高了组件在复杂布局中的可用性和可靠性。建议在合并这些更改之前，进行全面的测试，特别是在各种滚动场景下，以确保修复能够在所有情况下正常工作。
Types of changes
Changelog
Summary by CodeRabbit
getClosestScrollContainer函数，以改进弹出窗口的定位逻辑。Popover组件不可见的问题。Popover组件的变更日志，记录了版本3.4.3-beta.4的更新。