[Bugfix beta] Settable layout cp #10920
Conversation
stefanpenner
force-pushed
the
settable-layout-cp
branch
2 times, most recently
from
5b86dc0
to
69d796a
Compare
Previously setting `layout` would replace the underlying CP. This would prevent `layoutName` from working when a component had an existing template.
stefanpenner
force-pushed
the
settable-layout-cp
branch
from
69d796a
to
5f0d067
Compare
|
travis claims this is actually green: https://travis-ci.org/emberjs/ember.js/builds/59537515 ... ready for review: @mmun / @krisselden / @rwjblue |
| var layoutName = get(this, 'layoutName'); | ||
| var layout = this.templateForName(layoutName, 'layout'); | ||
| */ | ||
| layout: computed('layoutName', { |
There was a problem hiding this comment.
Shouldn't we also add defaultLayout to the dependent keys?
There was a problem hiding this comment.
I don't believe so. I am not sure why defaultLayout still exists, maybe it should be on the chopping block.
Regardless I don't believe a scenario exists where one would invalidate defaultLayout instead of just setting to layout directly or changing layoutName. If some actual use-case does exist, maybe my above statement needs to be taken into re-evaluate. If not, I would prefer not fixing something that serves no purpose and should likely just be removed.
There was a problem hiding this comment.
Sounds good, thank you for clarifying. 😄
|
Travis appears to be green but here it is reported red. I suspect this is good to go – just pending review. |
[Bugfix beta] Settable layout cp
depends on: #10920 will rebase, when that lands.
Also I will add a separate PR for beta branch, as the syntax is different.