Allow CollectionView to be populated with pre-rendered DOM #3135
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
If you were to override `buildChildView` instantiating the childViews with an `el` then you can create a collectionView from pre-rendered DOM
CollectionView can’t be initially “rendered” because it does its initialization of many things on first render.
|
looks reasonable. need help with the test? |
|
sure thing. this branch is on the core, so you can commit right to it if ya want. |
|
I'll git to it sometime today. |
when a collection view is DOM * and it's not attached to the document ** should have _isAttached set to false * and it's attached to the document ** should have _isAttached set to true when rendering a childView * should not render childView twice
| @@ -28,6 +28,34 @@ describe('collection view', function() { | |||
| // Collection View Specs | |||
| // --------------------- | |||
|
|
|||
| describe('when a collection view is DOM', function() { | |||
| beforeEach(function() { | |||
| this.$fixtureEl = $('<div id="fixture-collectionview"></div>'); | |||
There was a problem hiding this comment.
can we change the fixture to cater for when childViews are pre-rendered as well?
There was a problem hiding this comment.
something like
this.$fixtureEl = $('<div id="fixture-collectionview"><span id="el1">1</span></div>');
this.view1 = new this.ChildView({el: this.$fixtureEl.find('#el1')[0]});
then
this.collectionView.addChildView(this.view1, 0);
although I haven't tried that addChildView nor do I know if it will work
There was a problem hiding this comment.
it can also just be new this.ChildView({ el: '#el1' }); and I do think addChildView would work work.
when a collection view is DOM * and it's not attached to the document ** should have a child view without `_isAttached` * and it's attached to the document ** should have a child view with `_isAttached` set to `true`
|
@paulfalgout @ahumphreys87 updated. |
|
Slightly related issue #2154 |
|
👍 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
https://jsfiddle.net/t29dkss1/1/
I don't really know if this would be considered breaking.. I don't really think so..
This will need tests..thanks @rafdeAlso one point to note.. overriding
buildChildViewwas easy.. however that same function is used to build the emptyView which is kind of unfortunate.. it means that when an emptyView is built we're making an empty model to pass to this function.. and when you override it, we'll have to first detect if the view being passed is the childView or the emptyView before doing something out of the ordinary with the el.