Collection Fast Path Drop Bug On Empty List #402

Closed
hungle opened this Issue Apr 30, 2011 · 3 comments

5 participants

@hungle

This is happening on 1.5 stable with the Collection Fast Path mixin. If I drag an item from one list and move it to an empty list, I get the following error at the console:

Uncaught TypeError: Cannot call method 'get' of null
SC.ListView.SC.CollectionView.extend.showInsertionPointlist.js:465
SC.CollectionView.SC.View.extend.dragUpdatedcollection.js:2898
SC.Drag.SC.Object.extend.mouseDraggeddrag.js:496
SC.Object.tryToPerformobject.js:630
(anonymous function)javascript.js:10562
SC.runrun_loop.js:305
SC.RootResponder.SC.Object.extend.mousemovejavascript.js:10548
SC.mixin.handlejavascript.js:4283
listener

Digging further, it seems that "showInsertionPoint" implemented in SC.ListView is trying to perform:

var index = itemView.get('contentIndex');

...which is NULL due to the following line in "dragUpdated" in SC.CollectionView:

var itemView = this.itemViewForContentIndex(idx);

It appears that when dropping an item on an empty SC.ListView, the index is -1. When using the SC.CollectionFastPath mixin, "itemViewForContentIndex" does not properly return an itemView if index is -1.

I am not familiar with how the mixin speeds things up and thus what the right fix.

@ialexi ialexi was assigned Apr 30, 2011
@annismckenzie

Yep, had the same problem. The implementations of these methods are fundamentally different. The only thing I could do was to remove the mixin. :(

@publickeating
SproutCore member

The sign of a premier framework is a high functioning collection view and as such I think we should target bringing fast path into SC.CollectionView and addressing these issues in the next Major release.

@dcporter
SproutCore member

The offending code is still in there, and calling SC.ListView#showInsertionPoint(null, SC.DRAG_MOVE) still triggers an error, but I failed at a cursory attempt at reproduction. Is this issue still an issue? My hunch is yes. The fix would be to update showInsertionPoint to check itemView for existence and provide default values in case it doesn't.

@dcporter dcporter closed this in 9f22325 Jan 2, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment