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

Ensure DelayHide component disappears correctly #917

Merged
merged 4 commits into from
Jun 11, 2018

Conversation

sorenlouv
Copy link
Member

@sorenlouv sorenlouv commented Jun 10, 2018

A regression was introduced in #879 which causes the component to be displayed indefinitely even though the hide parameter is true and the timeout has passed.

I added a tests that reproduces this scenario, where an update to the parent component will cause getDerivedStateFromProps to be called again, which will corrupt the internal state.

There is a good blog post about this exact pitfall here: https://reactjs.org/blog/2018/06/07/you-probably-dont-need-derived-state.html

@chandlerprall I'm not sure the solution presented here is the best. If you have anything more elegant I'm all ears!

fixes elastic/kibana#19785

@sorenlouv sorenlouv requested a review from chandlerprall June 10, 2018 12:48
@sorenlouv sorenlouv changed the title Ensure delay_hide disappears correctly Ensure DelayHide component disappears correctly Jun 10, 2018
Copy link
Contributor

@chandlerprall chandlerprall left a comment

Choose a reason for hiding this comment

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

Nice, and thanks for the two test cases!

LGTM, pulled and verified docs example still works as expected.

@sorenlouv sorenlouv merged commit d222621 into elastic:master Jun 11, 2018
@sorenlouv sorenlouv deleted the fix-bug-in-delay-hide branch June 11, 2018 15:13
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.

[APM] Loading indicator never disappears
2 participants