feat: support disabled prop#638
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
Walkthrough新增 ChangesTrigger 禁用可见性
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
actor User
participant DisabledDemo
participant Trigger
participant UniqueProvider
participant Popup
User->>DisabledDemo: 点击切换 disabled
DisabledDemo->>Trigger: 更新 disabled 属性
User->>Trigger: 悬浮触发 Tooltip
Trigger->>UniqueProvider: 同步 mergedOpen
Trigger->>Popup: 传递 mergedOpen
Popup-->>User: 显示或隐藏弹层
Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
|
React Doctor found 8 issues in 1 file · 8 warnings · score 71 / 100 (Needs work) · vs 8 warnings
Reviewed by React Doctor for commit |
❌ Deploy failed
📋 Build log (last lines)🤖 Powered by surge-preview |
|||||||||
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #638 +/- ##
=======================================
Coverage 97.28% 97.28%
=======================================
Files 17 17
Lines 956 957 +1
Branches 268 275 +7
=======================================
+ Hits 930 931 +1
Misses 26 26 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a disabled prop to the Trigger component, allowing users to temporarily suppress popup visibility without resetting its current open state. To support this, UniqueProvider is updated with a setDisabled method and references to track active and disabled open states. Comprehensive tests are also added to verify this behavior. A review comment identifies a potential bug where outside click detection fails to clear the logical open state when autoDestroy is enabled and the trigger is disabled, as the unmounted popup removes the window click listeners; a fix is suggested to register listeners using rawOpen and targetEle instead.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| // Click to hide is special action since click popup element should not hide | ||
| const onPopupPointerDown = useWinClick( | ||
| mergedOpen, | ||
| rawOpen, |
There was a problem hiding this comment.
Potential Bug: Outside click detection fails when autoDestroy is enabled and trigger is disabled\n\nWhen disabled is true, mergedOpen becomes false, which causes the popup to hide. If autoDestroy is true, the popup element is completely unmounted once the hide transition completes, setting popupEle to null.\n\nIn useWinClick.ts, the window click listener is only registered if popupEle is truthy:\ntypescript\nReact.useEffect(() => {\n if (clickToHide && popupEle && (!mask || maskClosable)) {\n // ...\n\nBecause popupEle becomes null, the window click listeners are removed. Consequently, clicking outside the trigger while it is disabled will not clear the logical open state (rawOpen remains true). When disabled is later toggled back to false, the popup will unexpectedly reappear.\n\n#### Suggested Fix\nUpdate useWinClick.ts to register the event listeners when open (which is rawOpen) is true, even if popupEle is null (using targetEle to resolve the window/document):\n\ntypescript\n// src/hooks/useWinClick.ts\nexport default function useWinClick(\n open: boolean,\n clickToHide: boolean,\n targetEle: HTMLElement,\n popupEle: HTMLElement,\n mask: boolean,\n maskClosable: boolean,\n inPopupOrChild: (target: EventTarget) => boolean,\n triggerOpen: (open: boolean) => void,\n) {\n const openRef = React.useRef<boolean>(open);\n openRef.current = open;\n\n const popupPointerDownRef = React.useRef<boolean>(false);\n\n React.useEffect(() => {\n if (clickToHide && (popupEle || open) && (!mask || maskClosable)) {\n const onPointerDown = () => {\n popupPointerDownRef.current = false;\n };\n\n const onTriggerClose = (e: MouseEvent) => {\n if (\n openRef.current &&\n !inPopupOrChild(e.composedPath?.()?.[0] || e.target) &&\n !popupPointerDownRef.current\n ) {\n triggerOpen(false);\n }\n };\n\n const win = getWin(popupEle || targetEle);\n\n win.addEventListener('pointerdown', onPointerDown, true);\n win.addEventListener('mousedown', onTriggerClose, true);\n win.addEventListener('contextmenu', onTriggerClose, true);\n // ...\n
zombieJ
force-pushed
the
codex/trigger-disabled
branch
from
1ad1832 to
ff57868
Compare
zombieJ
force-pushed
the
codex/trigger-disabled
branch
from
ff57868 to
9335127
Compare
zombieJ
force-pushed
the
codex/trigger-disabled
branch
2 times, most recently
from
1cdbb28 to
788c423
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| } | ||
| } | ||
| }, [mergedOpen, targetEle]); | ||
| }, [mergedOpen, targetEle, disabled]); |
There was a problem hiding this comment.
React Doctor · react-doctor/exhaustive-deps (warning)
useLayoutEffect can run with a stale uniqueContext, unique, openUncontrolled, parentContext, uniqueContext.show, mouseEnterDelay, uniqueContext.hide, mouseLeaveDelay & show your users old data.
Fix → Don't blindly add missing dependencies. Read the hook callback first.
Bad:
useEffect(() => {
setCount(count + 1);
}, [count]);
Better:
useEffect(() => {
setCount((currentCount) => currentCount + 1);
}, []);
If the missing value is recreated every render, move it inside the hook or stabilize it before adding it to deps.
zombieJ
force-pushed
the
codex/trigger-disabled
branch
from
788c423 to
780b8aa
Compare
zombieJ
force-pushed
the
codex/trigger-disabled
branch
from
780b8aa to
49bcd0c
Compare
1 participant
Summary
disabledprop that masks effective popup visibility while preserving the existing controlled or uncontrolled open statedisabledon clickBehavior
When a hover-triggered popup is already open, setting
disabledhides it immediately. Settingdisabledback tofalserestores it while the existing open state remains active, without requiring another mouse enter.disabledonly affectsmergedOpen; it does not add separate event guards, effects, open-state storage, orUniqueProviderstate.Related: ant-design/ant-design#57290
Validation
ut coverage --runInBand(17 suites, 132 passed, 1 skipped)ut tscut docs:buildut x prettier@3.9.4 --check docs/examples/disabled.tsx docs/demos/disabled.mdSummary by CodeRabbit
新功能
disabled配置，可暂时隐藏弹层，同时保留当前打开状态。文档
disabled配置说明及默认值。测试