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

Refactor an each/else to an if/each/else to work around an Ember bug #31

Merged
merged 1 commit into from
Feb 13, 2016
Merged

Refactor an each/else to an if/each/else to work around an Ember bug #31

merged 1 commit into from
Feb 13, 2016

Conversation

pgengler
Copy link
Contributor

@pgengler pgengler commented Feb 4, 2016

See emberjs/ember.js#12716

This problem occurs when using a 'collapsable' table.
When a row group is initially rendered in the collapsed state it yields; any {{table-column}} components rendered inside here aren't cleaned up properly (they're removed from the DOM but willDestroyElement is never called so they never unregister). The lack of unregistration means that the corresponding header will always hold a reference to this "frozen-in-time" component.

See emberjs/ember.js#12716

This problem occurs when using a 'collapsable' table.
When a row group is initially rendered in the collapsed state it `yield`s; any {{table-columns}} used inside here aren't cleaned up properly (they're removed from the DOM but `willDestroyElement` is never called so they never unregister, and the corresponding header will always hold a reference to this outdated component.)
@cball
Copy link
Owner

cball commented Feb 13, 2016

👍 For some reason I never got the notification about this and totally missed the PR. Sorry for the delay.

cball added a commit that referenced this pull request Feb 13, 2016
Refactor an each/else to an if/each/else to work around an Ember bug
@cball cball merged commit dd2a54c into cball:master Feb 13, 2016
@pgengler pgengler deleted the eliminate-each-else branch February 17, 2016 14:41
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.

2 participants