-
Notifications
You must be signed in to change notification settings - Fork 361
fix(Popup): triggerElement string not working as CSS selector
#3940
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.
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
Conversation
triggerElement string not working as CSS selector
commit: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors the popup trigger mechanism by replacing inline event handler props with direct DOM event listeners, fixes a typo in the TooltipLite component's displayName, and updates snapshot tests accordingly.
- Migrated trigger event handling from React props to native DOM event listeners in
useTrigger.tsx - Fixed displayName from 'Tooltiplite' to 'TooltipLite' for consistency
- Enhanced event listener utilities to support options like
passive
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/snap/snapshots/csr.test.jsx.snap | Updated snapshot to reflect reordered CSS classes in button element |
| packages/components/tooltip/TooltipLite.tsx | Fixed displayName capitalization from 'Tooltiplite' to 'TooltipLite' |
| packages/components/popup/hooks/useTrigger.tsx | Refactored trigger event handling from React synthetic events to native DOM listeners |
| packages/components/popup/PopupPlugin.tsx | Added dynamic componentName using classPrefix from config |
| packages/components/popup/Popup.tsx | Updated to use refactored useTrigger hook API |
| packages/components/_util/ref.ts | Added mergeRefs utility function for combining multiple refs |
| packages/components/_util/listener.ts | Added options parameter support for addEventListener/removeEventListener |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
557a202 to
8fc31af
Compare
c19761d to
029f191
Compare
c96c8cc to
a68802e
Compare
|
/update-common |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 22 out of 22 changed files in this pull request and generated 9 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

🤔 这个 PR 的性质是?
🔗 相关 Issue
💡 需求背景和解决方案
ref和props的穿透useInnerPopupVisible解决Note
该 PR 解决了
children为普通二次封装组件,无法正常穿透的问题。但是如果它使用了useImperativeHandle覆盖ref,从而误判导致拿不到 DOM 节点,仍需要用户自己手动使用<></>包裹,例如 #3990Ant Design 底层也有类似的问题:https://ant.design/components/popover-cn#faq
📝 更新日志
fix(Popup): 修复
triggerElement类型为字符串时未正确作为元素选择器解析的问题fix(Popup): 修复
children为不支持ref穿透的封装组件时,弹窗无法正常出现的问题fix(PopupPlugin): 修复
classPrefix不生效的问题本条 PR 不需要纳入 Changelog
☑️ 请求合并前的自查清单