-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
[WIP] Remove Ember.ContainerView. #13188
Conversation
@HeroicEric - Want another challenge? |
heh, I actually tried this one already the other day but got stuck. I can try though 😄 |
@@ -10,44 +9,8 @@ import viewKeyword from 'ember-htmlbars/keywords/view'; | |||
import { objectAt } from 'ember-runtime/mixins/array'; | |||
|
|||
|
|||
// ....................................................... | |||
// removeChild() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rwjblue was this meant to be removed? If so, should View#removeAllChildren
and friends have the same fate?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was uncertain if removeAllChildren is a "normal" view method or just for container view.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rwjblue removeAllChildren
is from the LegacyViewSupport
mixin but it uses removeChild
https://github.com/rwjblue/ember.js/blob/remove-container-view/packages/ember-views/lib/mixins/legacy_view_support.js#L35-L39
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
kk, seems good lets kill it for now and we can see where that puts us
☔ The latest upstream changes (presumably #13190) made this pull request unmergeable. Please resolve the merge conflicts. |
I started working on this a few days ago, but haven't had a chance to circle around to it.