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): default open position #3029

Merged
merged 19 commits into from
Jan 28, 2019
Merged

fix(Popup): default open position #3029

merged 19 commits into from
Jan 28, 2019

Conversation

levithomason
Copy link
Member

@levithomason levithomason commented Jul 22, 2018

Fixes #2991.


This PR was branched from #3020 to perform some cleanup and updates since the original PR was from a fork's master and couldn't be updated.

I've also added a "Default Open" example to the docs for this use case.

It seems there is a bug with the use of ReactDOM.createPortal in InnerPortal. When rendering a Portal with open set to true on first render, an infinite loop is entered upon rendering the InnerPortal. If createPortal is bypassed, there is no infinite loop. I do not yet know why this is :/

@codecov-io
Copy link

codecov-io commented Jul 22, 2018

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3029      +/-   ##
==========================================
+ Coverage   99.89%   99.89%   +<.01%     
==========================================
  Files         170      170              
  Lines        2809     2818       +9     
==========================================
+ Hits         2806     2815       +9     
  Misses          3        3
Impacted Files Coverage Δ
src/modules/Popup/Popup.js 99.25% <100%> (+0.02%) ⬆️
src/addons/Portal/Portal.js 100% <100%> (ø) ⬆️
src/addons/Portal/PortalInner.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 4f8b001...b5405c0. Read the comment docs.

@layershifter
Copy link
Member

layershifter commented Jul 24, 2018

@levithomason Seems that it's really an bug in ReactDOM, it tried to debug this. The issue comes from the renderToStaticMarkup, similar issue: reactjs/react-modal#593.

The simpliest and stupid solution, disable html preview for specific examples. However, I'm not sure, that it's the best solution.

….com/Semantic-Org/Semantic-UI-React into fix/pop-open-position

# Conflicts:
#	docs/src/components/ComponentDoc/ComponentControls/ComponentControls.js
#	docs/src/components/ComponentDoc/ComponentExample/ComponentExample.js
@layershifter layershifter merged commit a263d64 into master Jan 28, 2019
@layershifter layershifter deleted the fix/pop-open-position branch January 28, 2019 11:58
@levithomason
Copy link
Member Author

Released in semantic-ui-react@0.85.0.

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