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

[RenderToLayout] Remove the debounce #2362

Merged
merged 1 commit into from
Dec 3, 2015
Merged

[RenderToLayout] Remove the debounce #2362

merged 1 commit into from
Dec 3, 2015

Conversation

oliviertassinari
Copy link
Member

@chrismcv My goal here is to remove the debounce of the render-to-layout component.
This is what was causing me issues when I was unmounting an open Popover.
Feedbacks are welcomed.

@chrismcv
Copy link
Contributor

chrismcv commented Dec 3, 2015

I'll take a look shortly, initial question - does the unmount animation on dialog/popover still run in this setup? (that was the reason for the debounce in first place)

@oliviertassinari
Copy link
Member Author

does the unmount animation on dialog/popover still run in this setup

The animations are still working on the examples of the documentation.

Regarding the unmonting of a component, I think that it shouldn't be delayed, that the reason of this PR.
IMHO, the only think that could be challenged is: Should we animate when we unmount an open Popever or a Dialog or an IconMenu?
I think that we shouldn't but you may argue the opposite.

@chrismcv
Copy link
Contributor

chrismcv commented Dec 3, 2015

seeing errors in
https://github.com/callemall/material-ui/pull/2362/files#diff-6b9f922d719aadb7183ece1140a1d82dR105

think componentWillUnmount can be removed

@chrismcv
Copy link
Contributor

chrismcv commented Dec 3, 2015

other than that, it looks good - happy for you to merge

@oliviertassinari
Copy link
Member Author

@chrismcv Good catch! I will check that everything is right (that's where unit test would have saved us) and I will merge.

@oliviertassinari
Copy link
Member Author

I have found one issue, fixed. I'm gonna let you merge #2359 first.

oliviertassinari added a commit that referenced this pull request Dec 3, 2015
…nmount

[RenderToLayout] Remove the debounce
@oliviertassinari oliviertassinari merged commit dd9ec03 into mui:master Dec 3, 2015
@oliviertassinari oliviertassinari deleted the render-to-layout-fix-unmount branch December 3, 2015 23:15
@zannager zannager added component: dialog This is the name of the generic UI component, not the React module! component: Popover The React component. labels Mar 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: dialog This is the name of the generic UI component, not the React module! component: Popover The React component.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants