-
Notifications
You must be signed in to change notification settings - Fork 190
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
Fix supportRef function for preact #477
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #477 +/- ##
==========================================
+ Coverage 91.51% 91.54% +0.03%
==========================================
Files 38 38
Lines 919 923 +4
Branches 280 284 +4
==========================================
+ Hits 841 845 +4
Misses 76 76
Partials 2 2 ☔ View full report in Codecov by Sentry. |
@MadCcc 看看这个 |
@MadCcc Could you please review this PR? |
@MadCcc Bump. |
@MadCcc Help! |
@afc163 Can you please help get this PR merged? |
Could we add test for preact? |
Checked for React 16, seems safe to merge. |
Fixes the issue described in ant-design/ant-design#44135
Why the issue exists:
The highlighted code in
rc-dropdown
returnstrue
for react andfalse
for preacthttps://github.com/react-component/dropdown/blob/master/src/Overlay.tsx#L25
Screenshot
That's why the focus doesn't shift to the menu when using preact.
Now let's take a look at the
supportRef
code and add some console.log to it for debugging:Screenshot
And here are the results:
react
preact
So, based on the screenshots above, we can conclude that in the case of preact, the
nodeOrComponent
is afunction
, not anobject
, so the second condition is fulfilled andfalse
is returned.And to fix this I added an additional condition because in the screenshot above you can see that preact as well as react have a
$$typeof
property equal toSymbol(react.forward_ref)