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(Popup): Remove hideOnScroll from window scroll #2063

Merged

Conversation

mkarajohn
Copy link
Contributor

@mkarajohn mkarajohn commented Sep 9, 2017

Fixes #2062

@mkarajohn
Copy link
Contributor Author

mkarajohn commented Sep 9, 2017

Hmm, tests are passing on my machine. What should I look to correct? Probably the commit name

@mkarajohn mkarajohn force-pushed the fix-popup-component-unmount-issue branch from 849cf22 to b0dc313 Compare September 9, 2017 22:21
@codecov-io
Copy link

codecov-io commented Sep 9, 2017

Codecov Report

Merging #2063 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2063      +/-   ##
==========================================
+ Coverage   99.76%   99.76%   +<.01%     
==========================================
  Files         148      148              
  Lines        2561     2562       +1     
==========================================
+ Hits         2555     2556       +1     
  Misses          6        6
Impacted Files Coverage Δ
src/modules/Popup/Popup.js 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 19e0056...faa9eb1. Read the comment docs.

@mkarajohn mkarajohn changed the title Properly remove Popup component's hideOnScroll from window's scroll event fix(Popup): Remove hideOnScroll from window scroll Sep 9, 2017
Copy link
Member

@layershifter layershifter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, you're right. I'm performing refactoring under #1146 and saw same thing.

@levithomason levithomason merged commit 4f48c58 into Semantic-Org:master Sep 18, 2017
@levithomason
Copy link
Member

Thank you!

@levithomason
Copy link
Member

Released in semantic-ui-react@0.73.1.

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.

4 participants