Skip to content

Commit

Permalink
Fixes exception when hitting the return key on editable collection vi…
Browse files Browse the repository at this point in the history
…ew with no selection. Refactored the method to prevent attempting to edit invalid item views. Includes unit test.
  • Loading branch information
publickeating committed Mar 27, 2014
1 parent f7c347f commit bee352e
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 12 deletions.
19 changes: 19 additions & 0 deletions frameworks/desktop/tests/views/collection/keyboard.js
Expand Up @@ -63,4 +63,23 @@ test("deselectAll", function() {
equals(view.getPath('selection.length'), 10, "deselectAll has no effect when allowsEmptySelection is NO")
});

// There was a specific bug in which insertNewLine when no selection was set, but
// isEditable & canEditContent were true, that it would throw an exception.
test("insertNewline doesn't throw exception when no selection", function() {
var collection = pane.view('default');

// Prep.
collection.set('isEditable', true);
collection.set('canEditContent', true);

SC.run(function() {
try {
collection.insertNewline();
ok(true, "Calling insertNewline without a selection should not throw an exception.");
} catch (ex) {
ok(false, "Calling insertNewline without a selection should not throw an exception. %@".fmt(ex));
}
});
});

// TODO: yeah all the other keyboard stuff.
22 changes: 10 additions & 12 deletions frameworks/desktop/views/collection.js
Expand Up @@ -2024,30 +2024,28 @@ SC.CollectionView = SC.View.extend(SC.CollectionViewDelegate, SC.CollectionConte
otherwise, invoke action.
*/
insertNewline: function (sender, evt) {
var canEdit = this.get('isEditable') && this.get('canEditContent'),
sel, content, set, idx, itemView;
var wantsEdit = this.get('isEditable') && this.get('canEditContent'),
canEdit = true, //false,
sel, content, set, idx, itemView;

// first make sure we have a single item selected; get idx
if (canEdit) {
// Make sure we have a single item selected and the item view supports beginEditing
if (wantsEdit) {
sel = this.get('selection');
content = this.get('content');

if (sel && sel.get('length') === 1) {
set = sel.indexSetForSource(content);
idx = set ? set.get('min') : -1;
canEdit = idx >= 0;
}
}

// next find itemView and ensure it supports editing
if (canEdit) {
itemView = this.itemViewForContentIndex(idx);
canEdit = itemView && SC.typeOf(itemView.beginEditing) === SC.T_FUNCTION;
// next find itemView and ensure it supports editing
itemView = this.itemViewForContentIndex(idx);
canEdit = itemView && SC.typeOf(itemView.beginEditing) === SC.T_FUNCTION;
}
}

// ok, we can edit..
if (canEdit) {
this.scrollToContentIndex(idx);
itemView = this.itemViewForContentIndex(idx); // just in case
itemView.beginEditing();

// invoke action
Expand Down

0 comments on commit bee352e

Please sign in to comment.