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

breaking(Portal|Popup): replace mouseOver event listener with mouseEnter #989

Merged
merged 1 commit into from
Dec 20, 2016

Conversation

dvdzkwsk
Copy link
Member

@dvdzkwsk dvdzkwsk commented Dec 6, 2016

Breaking Change

Fixes #978.

This PR replaces all mouseOver-related <Portal /> events with mouseEnter to be consistent with mouseLeave. This affects the <Portal /> component directly, as well as any which implement it, which at this time includes <Popup />.

Upgrade Guide

All related mouseOver props should now be mouseEnter. This will also impact the logic of these handlers since they are now only called when the element is entered, not (incorrectly) on enter and exit.

BEFORE

const onMouseOver = () => {
  console.log('Entered (or exited!!) the element in the portal')
}

<Portal onMouseOver={onMouseOver} trigger={<h2>hello</h2>} />

AFTER

const onMouseEnter = () => {
  console.log('Entered the element in the portal')
}

<Portal onMouseEnter={onMouseEnter} trigger={<h2>hello</h2>} />

@codecov-io
Copy link

codecov-io commented Dec 6, 2016

Current coverage is 95.81% (diff: 100%)

Merging #989 into master will not change coverage

@@             master       #989   diff @@
==========================================
  Files           870        870          
  Lines          4801       4801          
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
  Hits           4600       4600          
  Misses          201        201          
  Partials          0          0          

Powered by Codecov. Last update 8d96e53...3f947da

@levithomason
Copy link
Member

I know you're a busy fellow, any progress here @davezuko? Icon docs are borked without this guy.

@dvdzkwsk
Copy link
Member Author

No, perhaps I will take a stab again tonight. There's a failing test or two that I'm absolutely stumped on, but maybe a fresh look will solve it.

@levithomason
Copy link
Member

Cool, LMK.

@levithomason
Copy link
Member

Nice, this looks good. Want to add some breaking change notes as laid out in #978 (comment)? Also, the title will then be breaking(Portal): ....

@dvdzkwsk dvdzkwsk force-pushed the fix/portal-mouse-over branch 3 times, most recently from 2f013a9 to 4db517a Compare December 20, 2016 02:34
@dvdzkwsk
Copy link
Member Author

@levithomason how is this?

@levithomason levithomason changed the title fix(Portal): replace mouseover event listener with mouseenter fix(PortalPopup): replace mouseover event listener with mouseenter Dec 20, 2016
@levithomason levithomason changed the title fix(PortalPopup): replace mouseover event listener with mouseenter breaking(PortalPopup): replace mouseOver event listener with mouseEnter Dec 20, 2016
@levithomason levithomason changed the title breaking(PortalPopup): replace mouseOver event listener with mouseEnter breaking(Portal|Popup): replace mouseOver event listener with mouseEnter Dec 20, 2016
@levithomason levithomason merged commit d685d52 into master Dec 20, 2016
@levithomason levithomason deleted the fix/portal-mouse-over branch December 20, 2016 07:50
@levithomason
Copy link
Member

Looks great, thanks!

@levithomason
Copy link
Member

Released in [email protected].

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.

3 participants