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

Mouse enter leave #9824

Closed
wants to merge 3 commits into from
Closed

Mouse enter leave #9824

wants to merge 3 commits into from

Conversation

jquense
Copy link
Contributor

@jquense jquense commented May 31, 2017

Updated version of #5762 maybe there is a new failing test tho with fiber?

cc @nhunzaker

@nhunzaker
Copy link
Contributor

Thanks for reviving this! On the surface, this looks great. Looks like CI is just failing because of a linting issue.

I'll be able to dig in for a deeper review on Friday. I'll setup a DOM test fixture and do some browser QA. From there, we just need to figure out what's going on with Fiber.

@spicyj Could you take a look at this?

@jquense
Copy link
Contributor Author

jquense commented Jun 1, 2017

If I'm remembering correctly from way back when I originally did this, the other approach is to use the plugin add/remove listener hooks to circumvent the top level delegate for the polyfill (and attach a listener directly to the node). This may be a preferable approach judging from @sebmarkbage comment in the old PR.

@jquense
Copy link
Contributor Author

jquense commented Jul 20, 2017

So I'm revisiting this. From what I can tell native mouseenter and mouseleave are supported on all modern (and not modern) browsers. https://developer.mozilla.org/en-US/docs/Web/Events/mouseleave

Why is there a polyfill to begin with? Can we just remove at this point?

@sebmarkbage
Copy link
Collaborator

WebKit browsers (including Chrome at the time) didn't get support for it until late 2013. After React. There was also a long tail of upgrades that took a while but I think we can remove it now.

Similarly focusin/out wasn't supported in Firefox until this year(!) but once it has enough adoption we can kill the polyfilling around that too. However, that one is a bit different since we have slightly different behavior.

Also, Portals currently support bubbling through to the parent. That can be replicated in other ways though (e.g. redispatching the event).

@jquense
Copy link
Contributor Author

jquense commented Jul 20, 2017

Also, Portals currently support bubbling through to the parent. That can be replicated in other ways though (e.g. redispatching the event).

This is a killer feature for dealing with focus in fixed position dropdowns btw 👍

If we remove the polyfill we can rely on the normal event dispatch methods vs the fun custom one for ME, does that handle the portal case automatically or does it need to be handled further up or down?

@sebmarkbage
Copy link
Collaborator

Another feature of portals is that the events don't fire when you move between the parent and the portal. That bit might need a bit more complicated logic.

@gaearon
Copy link
Collaborator

gaearon commented Jan 5, 2018

Closing as stale.

@gaearon gaearon closed this Jan 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants