Skip to content
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

Merged
merged 4 commits into from
Feb 20, 2024

Conversation

SpaNb4
Copy link
Contributor

@SpaNb4 SpaNb4 commented Oct 16, 2023

Fixes the issue described in ant-design/ant-design#44135

Why the issue exists:
The highlighted code in rc-dropdown returns true for react and false for preact
https://github.com/react-component/dropdown/blob/master/src/Overlay.tsx#L25

Screenshot

image

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

image

And here are the results:

react

image

preact

image

So, based on the screenshots above, we can conclude that in the case of preact, the nodeOrComponent is a function, not an object, so the second condition is fulfilled and false 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 to Symbol(react.forward_ref)
image

@vercel
Copy link

vercel bot commented Oct 16, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
util ❌ Failed (Inspect) Dec 27, 2023 11:58am

@SpaNb4
Copy link
Contributor Author

SpaNb4 commented Oct 31, 2023

@MadCcc @zombieJ Could you please review this PR?

@mellis481
Copy link

@MadCcc @afc163 @zombieJ Can you please review and merge this PR? It's critical for my team.

Copy link

codecov bot commented Dec 21, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (5a3231f) 91.51% compared to head (105a436) 91.54%.

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.
📢 Have feedback on the report? Share it here.

@afc163
Copy link
Member

afc163 commented Dec 27, 2023

@MadCcc 看看这个

@SpaNb4
Copy link
Contributor Author

SpaNb4 commented Jan 26, 2024

@MadCcc Could you please review this PR?

@mellis481
Copy link

@MadCcc Bump.

@mellis481
Copy link

@MadCcc Help!

@mellis481
Copy link

@afc163 Can you please help get this PR merged?

@afc163
Copy link
Member

afc163 commented Feb 20, 2024

ping @MadCcc @zombieJ

@MadCcc
Copy link
Member

MadCcc commented Feb 20, 2024

Could we add test for preact?

@zombieJ
Copy link
Member

zombieJ commented Feb 20, 2024

Checked for React 16, seems safe to merge.

@zombieJ zombieJ merged commit 60c3785 into react-component:master Feb 20, 2024
10 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants