Merge pull request #1 from sglanzer/frosetti/range_select

Looks good.

addon/components/frost-list.js

@@ -3,7 +3,7 @@
  */
 
 import Ember from 'ember'
-const {$, A, isEmpty, run, set} = Ember
+const {$, A, get, isEmpty, isNone, run, set} = Ember
 import computed, {readOnly} from 'ember-computed-decorators'
 import {Component} from 'ember-frost-core'
 import {selection} from 'ember-frost-list'
@@ -86,8 +86,8 @@ export default Component.extend({
 
       // State
       _rangeState: {
-        anchor: Ember.Object.create(),
-        endpoint: Ember.Object.create()
+        anchor: null,
+        endpoint: null
       }
     }
   },
@@ -96,17 +96,18 @@ export default Component.extend({
 
   @readOnly
   @computed('expandedItems.[]', 'items.[]', 'selectedItems.[]', '_itemComparator')
-  _items (expandedItems, items, selectedItems, itemComparator) {
+  _items (expandedItems, items, selectedItems, _itemComparator) {
     if (isEmpty(items)) {
       return []
     }
+
     return items.map(item => {
       run.next(() => {
         set(item, 'isExpanded', isEmpty(expandedItems) ? false : expandedItems.some(
-          selectedItem => itemComparator(selectedItem, item))
+          selectedItem => _itemComparator(selectedItem, item))
         )
         set(item, 'isSelected', isEmpty(selectedItems) ? false : selectedItems.some(
-          selectedItem => itemComparator(selectedItem, item))
+          selectedItem => _itemComparator(selectedItem, item))
         )
       })
       return item
@@ -141,7 +142,7 @@ export default Component.extend({
     const itemKey = this.get('itemKey')
     if (itemKey) {
       this.set('_itemComparator', function (lhs, rhs) {
-        return lhs.get(itemKey) === rhs.get(itemKey)
+        return isNone(lhs) || isNone(rhs) ? false : get(lhs, itemKey) === get(rhs, itemKey)
       })
     } else {
       this.set('_itemComparator', function (lhs, rhs) {

addon/utils/selection.js

@@ -2,9 +2,8 @@
  * TODO Selection utilities
  */
 
-import Ember from 'ember'
-
 export default {
+
   /**
    * Basic selection acts conditionally based on the presence of additional selections.
    *
@@ -29,23 +28,22 @@ export default {
       rangeState['anchor'] = item
 
       // New anchor, clear any previous endpoint
-      rangeState['endpoint'] = Ember.Object.create()
+      rangeState['endpoint'] = null
     } else {
       // Toggle the item selection
-
-      const isCurrentlySelected = (index >= 0)
+      const isCurrentlySelected = index >= 0
       const isSelected = !isCurrentlySelected
       if (isSelected) {
-        selectedItems.pushObject(item)
+        selectedItems.addObject(item)
       } else {
         selectedItems.removeAt(index)
       }
 
       // Set the range anchor if selected, otherwise clear the anchor
-      rangeState['anchor'] = isSelected ? item : Ember.Object.create()
+      rangeState['anchor'] = isSelected ? item : null
 
       // New or no anchor, clear any previous endpoint
-      rangeState['endpoint'] = Ember.Object.create()
+      rangeState['endpoint'] = null
     }
   },
 
@@ -63,22 +61,18 @@ export default {
    */
   /* eslint-disable complexity */
   range (items, selectedItems, item, rangeState, itemComparator, itemKey) {
-    // If an anchor isn't set, then set the anchor and exit
+    // If an anchor isn't set or in the current list of items, then set the anchor and exit
     const rangeAnchor = rangeState['anchor']
-
-    const anchor = items.findIndex(currentItem => itemComparator(currentItem, rangeState['anchor']))
-    // If anchor is -1 then it was a anchor from a previous page that we can not find, so reset
-    if (Ember.isEmpty(rangeAnchor) || anchor === -1) {
+    const anchor = rangeAnchor ? items.findIndex(currentItem => itemComparator(currentItem, rangeAnchor)) : -1
+    if (anchor === -1) {
       // Range select is always a positive selection (no deselect)
       rangeState['anchor'] = item
 
       // New anchor, clear any previous endpoint
-      rangeState['endpoint'] = Ember.Object.create()
+      rangeState['endpoint'] = null
 
-      // Add the anchor to the selected items if not already in it from a previous page
-      if (!selectedItems.some(selectedItem => itemComparator(selectedItem, item))) {
-        selectedItems.pushObject(item)
-      }
+      // Add the anchor to the selected items
+      selectedItems.addObject(item)
 
       return
     }
@@ -87,9 +81,9 @@ export default {
 
     // Select all of the items between the anchor and the item (inclusive)
     if (anchor < endpoint) {
-      selectedItems.pushObjects(items.slice(anchor, endpoint + 1))
+      selectedItems.addObjects(items.slice(anchor, endpoint + 1))
     } else {
-      selectedItems.pushObjects(items.slice(endpoint, anchor + 1))
+      selectedItems.addObjects(items.slice(endpoint, anchor + 1))
     }
 
     // If an endpoint was already selected remove selected items that were
@@ -116,12 +110,10 @@ export default {
       }
     }
 
-    // Wipe out duplicates if range selection covered already selected items
-    // e.g item2-3 already selected but shift click item0 through item5 happens
+    // If items in the list are compared using itemKey rather than by reference
+    // then addObject(s) won't guarentee uniqueness, so do a uniqueness pass
     if (itemKey) {
       selectedItems.setObjects(selectedItems.uniqBy(itemKey))
-    } else {
-      selectedItems.setObjects(selectedItems.uniq())
     }
 
     // Store the new endpoint
@@ -144,13 +136,13 @@ export default {
     const isSelected = !isCurrentlySelected
 
     // Set the range anchor if selected, otherwise clear the anchor
-    rangeState['anchor'] = isSelected ? item : Ember.Object.create()
+    rangeState['anchor'] = isSelected ? item : null
     // New or no anchor, clear any previous endpoint
-    rangeState['endpoint'] = Ember.Object.create()
+    rangeState['endpoint'] = null
 
     // Store the selection
     if (isSelected) {
-      selectedItems.pushObject(item)
+      selectedItems.addObject(item)
     } else {
       selectedItems.removeAt(index)
     }

tests/dummy/app/pods/paged/template.hbs

@@ -3,6 +3,7 @@
   hook='demo'
   item=(component 'list-item')
   itemExpansion=(component 'list-item-expansion')
+  itemKey='id'
   items=items
   scrollTop=scrollTop
   expandedItems=expandedItems

tests/integration/components/frost-list-test.js

@@ -627,6 +627,8 @@ describe(test.label, function () {
 
     describe('When Paged', function () {
       beforeEach(function () {
+        // Note: DON'T change the seeding, the object creation/destruction is intentional
+        // to prove that comparison of selected items only via key works!
         const testItems = A([
           Ember.Object.create({id: '0'}),
           Ember.Object.create({id: '1'}),
@@ -723,7 +725,7 @@ describe(test.label, function () {
             expect($hook('my-list-item-container', {index: 1}).hasClass('is-selected')).to.eql(false)
           })
 
-          it('item 1 is not selected', function () {
+          it('item 2 is not selected', function () {
             expect($hook('my-list-item-container', {index: 2}).hasClass('is-selected')).to.eql(false)
           })
 
@@ -763,6 +765,7 @@ describe(test.label, function () {
           })
         })
       })
+
       describe('When using specific select on item 2', function () {
         beforeEach(function () {
           $hook('my-list-selection', {index: 1}).click()