Skip to content
This repository has been archived by the owner on Mar 4, 2020. It is now read-only.

fix(Popup): opened popup causes infinite rendering loop #705

Merged
merged 4 commits into from
Jan 14, 2019

Conversation

kuzhelov
Copy link
Contributor

@kuzhelov kuzhelov commented Jan 11, 2019

Fixes #706.

TODO

  • provide an example for introduced API
  • update CHANGELOG

The problem was introduced by #678 - essentially, the root cause is the side effect of scheduleUpdate() function that causes React to rerender components tree - and given that each rendering ends up in another render being immediately scheduled, the whole story ends up in an infinite loop. infinite loop.

Provided fix introduces ability to the client to explicitly tell when this popup position update should happen. Please, note that this scenario is fairly rare, so that normally content prop of the Popup will be used, as it was before. However, in cases where fine control is necessary over the moment of popup position update, the introduced approach now can be used:

<Popup 
  trigger={..} 
  renderContent={updatePosition => <AsyncData done={updatePosition} ... />} />

This will prevent us from implicit scheduling of render updates and, thus, remove probability of falling into infinite render loops altogether. At the same time, all the user scenarios mentioned in the following issue (#641) will be covered.

{ ref, scheduleUpdate, style: popupPlacementStyles }: PopperChildrenProps,
) => {
const { content } = this.props
const { content: propsContent, renderContent } = this.props
const content = renderContent ? renderContent(scheduleUpdate) : propsContent
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure but, maybe it will be nice if we can have an example with this, especially that we are adding some new prop for the API. Other then that looks good!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

totally agree, will work on it!

@kuzhelov kuzhelov merged commit 70063f2 into master Jan 14, 2019
@layershifter layershifter deleted the fix/popup-infinite-render branch January 29, 2019 08:38
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🧰 fix Introduces fix for broken behavior. 🚀 ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Opened Popup causes infinite render loop
5 participants