From 341ba443560a959d0e853e22958c93ec20426084 Mon Sep 17 00:00:00 2001 From: Katrina Uychaco Date: Mon, 12 Sep 2016 00:54:18 -0700 Subject: [PATCH] WIP multi-selection improvements --- lib/multi-list-collection.js | 159 ++++++++++++++----- package.json | 1 + test/multi-list-collection.test.js | 247 +++++++++++++++++++++++++++-- 3 files changed, 357 insertions(+), 50 deletions(-) diff --git a/lib/multi-list-collection.js b/lib/multi-list-collection.js index 7a48dac5bc..e33f8d61d8 100644 --- a/lib/multi-list-collection.js +++ b/lib/multi-list-collection.js @@ -1,20 +1,31 @@ /** @babel */ +import compareSets from 'compare-sets' + import MultiList from './multi-list' export default class MultiListCollection { constructor (lists, didChangeSelection) { this.list = new MultiList(lists, (item, key) => { - this.lastSelectedItem = item - this.lastSelectedKey = key + // QUESTION: remove this and just rely on multi-list item and key + // this.lastSelectedItem = item + // this.lastSelectedKey = key didChangeSelection && didChangeSelection(item, key) }) - const selectedKey = this.list.getSelectedListKey() - const selectedItem = this.list.getSelectedItem() - this.lastSelectedItem = selectedItem - this.lastSelectedKey = selectedKey - this.selectedKeys = new Set(selectedKey ? [selectedKey] : []) - this.selectedItems = new Set(selectedItem ? [selectedItem] : []) + const key = this.list.getSelectedListKey() + const item = this.list.getSelectedItem() + this.tail = {item, key} + // QUESTION: remove this and just rely on multi-list item and key + // this.lastSelectedItem = selectedItem + // this.lastSelectedKey = selectedKey + this.selectedKeys = { + stash: new Set(key ? [key] : []), + store: new Set() + } + this.selectedItems = { + stash: new Set(item ? [item] : []), + store: new Set() + } } updateLists (lists, {suppressCallback} = {}) { @@ -22,20 +33,40 @@ export default class MultiListCollection { this.updateSelections() } - clearSelectedItems () { - this.selectedItems = new Set() + clearSelectedItemsStash () { + this.selectedItems.stash = new Set() + } + + clearSelectedItemsStore () { + this.selectedItems.store = new Set() + } + + clearSelectedKeysStash () { + this.selectedKeys.stash = new Set() } - clearSelectedKeys () { - this.selectedKeys = new Set() + clearSelectedKeysStore () { + this.selectedKeys.store = new Set() + } + + clearState () { + this.clearSelectedItemsStash() + this.clearSelectedItemsStore() + this.clearSelectedKeysStash() + this.clearSelectedKeysStore() + this.tail = null + } + + getTail () { + return this.tail } getSelectedItems () { - return this.selectedItems + return new Set(Array.from(this.selectedItems.stash).concat(Array.from(this.selectedItems.store))) } getSelectedKeys () { - return this.selectedKeys + return new Set(Array.from(this.selectedKeys.stash).concat(Array.from(this.selectedKeys.store))) } getItemsForKey (key) { @@ -43,11 +74,13 @@ export default class MultiListCollection { } getLastSelectedListKey () { - return this.lastSelectedKey + // return this.lastSelectedKey + return this.list.getSelectedKey() } getLastSelectedItem () { - return this.lastSelectedItem + // return this.lastSelectedItem + return this.list.getSelectedItem() } selectNextList ({wrap, addToExisting} = {}) { @@ -71,43 +104,89 @@ export default class MultiListCollection { } updateSelections ({addToExisting} = {}) { - const selectedKey = this.list.getSelectedListKey() - const selectedItem = this.list.getSelectedItem() - this.selectItems(selectedItem ? [selectedItem] : [], {addToExisting, suppressCallback: true}) - this.selectKeys(selectedKey ? [selectedKey] : [], {addToExisting, suppressCallback: true}) + const selectedKey = this.getLastSelectedListKey() + const selectedItem = this.getLastSelectedItem() + this.selectItemForKey(selectedItem, selectedKey, {tail: true, addToExisting, suppressCallback: true}) } - selectItems (items, {addToExisting, suppressCallback} = {}) { - if (!addToExisting) this.clearSelectedItems() - items.forEach(item => this.selectedItems.add(item)) - const lastItem = items[items.length - 1] - this.lastSelectedItem = lastItem - this.list.selectItem(lastItem, {suppressCallback}) + selectItems (items, {addToExisting} = {}) { + if (!addToExisting) { + this.clearSelectedItemsStash() + this.clearSelectedItemsStore() + } + items.forEach(item => this.selectedItems.stash.add(item)) + this.list.selectItem(items[items.length - 1], {suppressCallback: this.selectedItems.size === 1}) } selectKeys (keys, {addToExisting, suppressCallback} = {}) { - if (!addToExisting) this.clearSelectedKeys() - keys.forEach(key => this.selectedKeys.add(key)) - const lastKey = keys[keys.length - 1] - this.lastSelectedKey = lastKey - this.list.selectListForKey(lastKey, {suppressCallback}) + if (!addToExisting) { + this.clearSelectedKeysStash() + this.clearSelectedKeysStore() + } + keys.forEach(key => this.selectedKeys.stash.add(key)) + this.list.selectListForKey(keys[keys.length - 1], {suppressCallback}) + } + + // foo (item) { + // if (this.selectedItems.has(item)) { + // toggleItemForKey() + // } else { + // + // } + // selectItemForKey() + // } + + selectItemForKey (item, key, {tail, addToExisting} = {}) { + if (!this.getItemsForKey(key).includes(item)) throw new Error(`item ${item} not found for key ${key}`) + if (!addToExisting) { + this.clearState() + } else if (tail && tail !== this.tail) { + // move stash items to store and then clear stash + this.selectedItems.stash.forEach(v => this.selectedItems.store.add(v)) + this.selectedKeys.stash.forEach(v => this.selectedKeys.store.add(v)) + } + this.clearSelectedItemsStash() + this.clearSelectedKeysStash() + + if (tail || !this.tail) this.tail = {key, item} + this.selectItemsAndKeysInRange(this.tail, {key, item}) + } + + toggleItemForKey (item, key) { + const itemsForKey = this.getItemsForKey(key) + if (!itemsForKey.includes(item)) throw new Error(`item ${item} not found for key ${key}`) + const intersection = compareSets(this.getSelectedItems(), new Set(itemsForKey)).retained + if (intersection.has(item)) { + this.selectedItems.stash.delete(item) + this.selectedItems.store.delete(item) + if (intersection.size === 1) { + this.selectedKeys.stash.delete(key) + this.selectedKeys.store.delete(key) + } + this.selectNewTailNearItemForKey(item, key) + // if (this.getSelectedItems().size === 0) this.tail = null + } else { + // this.selectItems([item], {addToExisting: true}) + // this.selectKeys([key], {addToExisting: true}) + this.selectItemForKey(item, key, {tail: true, addToExisting: true}) + } + } + + selectNewTailNearItemForKey (item, key) { + // look in selection, order it based on keys and location } selectAllItemsForKey (key, addToExisting) { this.selectKeys([key], {addToExisting}) - this.selectItems(this.list.getItemsForKey(key), {addToExisting}) + this.selectItems(this.getItemsForKey(key), {addToExisting}) } selectFirstItemForKey (key, {addToExisting} = {}) { this.selectKeys([key], {addToExisting}) - this.selectItems([this.list.getItemsForKey(key)[0]], {addToExisting}) + this.selectItems([this.getItemsForKey(key)[0]], {addToExisting}) } - selectItemsAndKeysInRange (endPoint1, endPoint2, addToExisting) { - if (!addToExisting) { - this.clearSelectedItems() - this.clearSelectedKeys() - } + selectItemsAndKeysInRange (endPoint1, endPoint2) { // TODO: optimize const listKeys = this.list.getListKeys() const index1 = listKeys.indexOf(endPoint1.key) @@ -133,7 +212,7 @@ export default class MultiListCollection { if (endItemIndex < 0) throw new Error(`item "${endPoint.item}" not found`) if (startKeyIndex === endKeyIndex) { - const items = this.list.getItemsForKey(listKeys[startKeyIndex]) + const items = this.getItemsForKey(listKeys[startKeyIndex]) const indexes = [startItemIndex, endItemIndex].sort() this.selectKeys([startPoint.key], {addToExisting: true, suppressCallback: true}) this.selectItems(items.slice(indexes[0], indexes[1] + 1), {addToExisting: true}) @@ -143,7 +222,7 @@ export default class MultiListCollection { for (let i = startKeyIndex; i <= endKeyIndex; i++) { const key = listKeys[i] this.selectKeys([key], {addToExisting: true, suppressCallback: true}) - const items = this.list.getItemsForKey(key) + const items = this.getItemsForKey(key) if (i === startKeyIndex) { this.selectItems(items.slice(startItemIndex), {addToExisting: true}) } else if (i === endKeyIndex) { diff --git a/package.json b/package.json index 5de68a4a2a..0196501847 100644 --- a/package.json +++ b/package.json @@ -14,6 +14,7 @@ }, "atomTestRunner": "./test/runner", "dependencies": { + "compare-sets": "^1.0.1", "diff": "2.2.2", "etch": "0.6.3", "etch-stateless": "^1.0.0", diff --git a/test/multi-list-collection.test.js b/test/multi-list-collection.test.js index cd842d3dea..bd5d10f775 100644 --- a/test/multi-list-collection.test.js +++ b/test/multi-list-collection.test.js @@ -2,6 +2,14 @@ import MultiListCollection from '../lib/multi-list-collection' +function assertSelectedItemsEqual (mlc, expectedItems) { + assert.deepEqual(Array.from(mlc.getSelectedItems()).sort(), expectedItems.sort()) +} + +function assertSelectedKeysEqual (mlc, expectedKeys) { + assert.deepEqual(Array.from(mlc.getSelectedKeys()).sort(), expectedKeys.sort()) +} + describe('MultiListCollection', () => { describe('selectItemsAndKeysInRange(endPoint1, endPoint2)', () => { it('takes endpoints ({key, item}) and returns an array of items between those points', () => { @@ -11,28 +19,33 @@ describe('MultiListCollection', () => { { key: 'list3', items: ['f', 'g', 'h'] } ]) + mlc.clearState() mlc.selectItemsAndKeysInRange({key: 'list1', item: 'b'}, {key: 'list1', item: 'c'}) - assert.deepEqual([...mlc.getSelectedItems()], ['b', 'c']) - assert.deepEqual([...mlc.getSelectedKeys()], ['list1']) + assertSelectedItemsEqual(mlc, ['b', 'c']) + assertSelectedKeysEqual(mlc, ['list1']) // endpoints can be specified in any order + mlc.clearState() mlc.selectItemsAndKeysInRange({key: 'list1', item: 'c'}, {key: 'list1', item: 'b'}) - assert.deepEqual([...mlc.getSelectedItems()], ['b', 'c']) - assert.deepEqual([...mlc.getSelectedKeys()], ['list1']) + assertSelectedItemsEqual(mlc, ['b', 'c']) + assertSelectedKeysEqual(mlc, ['list1']) // endpoints can be in different lists + mlc.clearState() mlc.selectItemsAndKeysInRange({key: 'list1', item: 'c'}, {key: 'list3', item: 'g'}) - assert.deepEqual([...mlc.getSelectedItems()], ['c', 'd', 'e', 'f', 'g']) - assert.deepEqual([...mlc.getSelectedKeys()], ['list1', 'list2', 'list3']) + assertSelectedItemsEqual(mlc, ['c', 'd', 'e', 'f', 'g']) + assertSelectedKeysEqual(mlc, ['list1', 'list2', 'list3']) + mlc.clearState() mlc.selectItemsAndKeysInRange({key: 'list3', item: 'g'}, {key: 'list1', item: 'c'}) - assert.deepEqual([...mlc.getSelectedItems()], ['c', 'd', 'e', 'f', 'g']) - assert.deepEqual([...mlc.getSelectedKeys()], ['list1', 'list2', 'list3']) + assertSelectedItemsEqual(mlc, ['c', 'd', 'e', 'f', 'g']) + assertSelectedKeysEqual(mlc, ['list1', 'list2', 'list3']) // endpoints can be the same + mlc.clearState() mlc.selectItemsAndKeysInRange({key: 'list1', item: 'c'}, {key: 'list1', item: 'c'}) - assert.deepEqual([...mlc.getSelectedItems()], ['c']) - assert.deepEqual([...mlc.getSelectedKeys()], ['list1']) + assertSelectedItemsEqual(mlc, ['c']) + assertSelectedKeysEqual(mlc, ['list1']) }) it('throws error when keys or items aren\'t found', () => { @@ -57,4 +70,218 @@ describe('MultiListCollection', () => { }, 'item "x" not found') }) }) + + describe('selectItemForKey(item, key, {tail, addToExisting})', () => { + it('selects the items between the given item and the tail', () => { + const mlc = new MultiListCollection([ + { key: 'list1', items: ['a', 'b', 'c'] }, + { key: 'list2', items: ['d', 'e'] }, + { key: 'list3', items: ['f', 'g', 'h'] } + ]) + + // initially tail is first item + assert.deepEqual(mlc.getTail(), {key: 'list1', item: 'a'}) + assertSelectedItemsEqual(mlc, ['a']) + + // tail is set to 'b' + mlc.selectItemForKey('b', 'list1') + assert.deepEqual(mlc.getTail(), {key: 'list1', item: 'b'}) + assertSelectedItemsEqual(mlc, ['b']) + + // addToExisting + mlc.selectItemForKey('e', 'list2', {addToExisting: true}) + assert.deepEqual(mlc.getTail(), {key: 'list1', item: 'b'}) + assertSelectedItemsEqual(mlc, ['b', 'c', 'd', 'e']) + + mlc.selectItemForKey('d', 'list2', {addToExisting: true}) + assert.deepEqual(mlc.getTail(), {key: 'list1', item: 'b'}) + assertSelectedItemsEqual(mlc, ['b', 'c', 'd']) + + // create new tail and addToExisting + mlc.selectItemForKey('f', 'list3', {tail: true, addToExisting: true}) + assert.deepEqual(mlc.getTail(), {key: 'list3', item: 'f'}) + assertSelectedItemsEqual(mlc, ['b', 'c', 'd', 'f']) + + // addToExisting + mlc.selectItemForKey('h', 'list3', {addToExisting: true}) + assert.deepEqual(mlc.getTail(), {key: 'list3', item: 'f'}) + assertSelectedItemsEqual(mlc, ['b', 'c', 'd', 'f', 'g', 'h']) + + // addToExisting + mlc.selectItemForKey('g', 'list3', {addToExisting: true}) + assert.deepEqual(mlc.getTail(), {key: 'list3', item: 'f'}) + assertSelectedItemsEqual(mlc, ['b', 'c', 'd', 'f', 'g']) + + // new tail without addToExisting + mlc.selectItemForKey('e', 'list2', {tail: true}) + assert.deepEqual(mlc.getTail(), {key: 'list2', item: 'e'}) + assertSelectedItemsEqual(mlc, ['e']) + }) + }) + + describe.only('toggleItemForKey(item, key)', () => { + describe('when item was not previously selected', () => { + it('selects item and sets it as new tail', () => { + const mlc = new MultiListCollection([ + { key: 'list1', items: ['a', 'b', 'c'] }, + { key: 'list2', items: ['d', 'e'] } + ]) + // first item is is initially set to tail + assert.deepEqual(mlc.getTail(), {key: 'list1', item: 'a'}) + assertSelectedItemsEqual(mlc, ['a']) + assertSelectedKeysEqual(mlc, ['list1']) + + // new tail 'd' + mlc.toggleItemForKey('d', 'list2') + assert.deepEqual(mlc.getTail(), {key: 'list2', item: 'd'}) + assertSelectedItemsEqual(mlc, ['a', 'd']) + assertSelectedKeysEqual(mlc, ['list1', 'list2']) + + // new tail 'c' + mlc.toggleItemForKey('c', 'list1') + assert.deepEqual(mlc.getTail(), {key: 'list1', item: 'c'}) + assertSelectedItemsEqual(mlc, ['a', 'c', 'd']) + assertSelectedKeysEqual(mlc, ['list1', 'list2']) + }) + }) + + describe('when item was previously selected', () => { + it('unselects item and sets new tail as closest selected item, giving preference to those that come after', () => { + const mlc = new MultiListCollection([ + { key: 'list1', items: ['a', 'b', 'c'] }, + { key: 'list2', items: ['d', 'e'] } + ]) + // first item is is initially set to tail + assert.deepEqual(mlc.getTail(), {key: 'list1', item: 'a'}) + + // addToExisting + mlc.selectItemForKey('e', 'list2', {addToExisting: true}) + assert.deepEqual(mlc.getTail(), {key: 'list1', item: 'a'}) + assertSelectedItemsEqual(mlc, ['a', 'b', 'c', 'd', 'e']) + assertSelectedKeysEqual(mlc, ['list1', 'list2']) + + mlc.toggleItemForKey('d', 'list2') + // new tail is selected item after 'd' + assert.deepEqual(mlc.getTail(), {key: 'list2', item: 'e'}) + assertSelectedItemsEqual(mlc, ['a', 'b', 'c', 'e']) + assertSelectedKeysEqual(mlc, ['list1', 'list2']) + + mlc.toggleItemForKey('e', 'list2') + // new tail is selected item before 'e', since there are no selected items after + assert.deepEqual(mlc.getTail(), {key: 'list1', item: 'c'}) + assertSelectedItemsEqual(mlc, ['a', 'b', 'c']) + assertSelectedKeysEqual(mlc, ['list1']) + + mlc.toggleItemForKey('c', 'list1') + // new tail is selected item before 'c', since there are no selected items after + assert.deepEqual(mlc.getTail(), {key: 'list1', item: 'b'}) + assertSelectedItemsEqual(mlc, ['a', 'b']) + assertSelectedKeysEqual(mlc, ['list1']) + }) + + describe('when tail was unselected', () => { + it('sets new tail as closest selected item after', () => { + + }) + + describe('when there are no selected items after', () => { + it('sets new tail as closest selected item before', () => { + + }) + }) + }) + }) + + it('selects/unselects item as appropriate and updates tail', () => { + const mlc = new MultiListCollection([ + { key: 'list1', items: ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h'] }, + { key: 'list2', items: ['i', 'j'] } + ]) + + assert.deepEqual(mlc.getTail(), {key: 'list1', item: 'a'}) + assertSelectedItemsEqual(mlc, ['a']) + assertSelectedKeysEqual(mlc, ['list1']) + + mlc.toggleItemForKey('a', 'list1') + assert.deepEqual(mlc.getTail(), null) + assertSelectedItemsEqual(mlc, []) + assertSelectedKeysEqual(mlc, []) + + mlc.toggleItemForKey('b', 'list1') + assert.deepEqual(mlc.getTail(), {key: 'list1', item: 'b'}) + assertSelectedItemsEqual(mlc, ['b']) + assertSelectedKeysEqual(mlc, ['list1']) + + + + mlc.selectItemForKey('e', 'list1', {addToExisting: true}) + assert.deepEqual(mlc.getTail(), {key: 'list1', item: 'b'}) + assertSelectedItemsEqual(mlc, ['b', 'c', 'd', 'e']) + assertSelectedKeysEqual(mlc, ['list1']) + + + }) + it('unselects item if already selected', () => { + + }) + + it('selects item if not yet selected and sets as new tail', () => { + + }) + + it('sets a new tail if item is tail', () => { + + }) + + it('sets a new head if item is head', () => { + + }) + }) + + // toggleItemForKey + // when this.tail should be set to null + + +}) + + +it('restores previous tails if current is unselected', () => { + const mlc = new MultiListCollection([ + { key: 'list1', items: ['a', 'b', 'c'] }, + { key: 'list2', items: ['d', 'e'] } + ]) + // first item is is initially set to tail + assert.deepEqual(mlc.getTail(), {key: 'list1', item: 'a'}) + assertSelectedItemsEqual(mlc, ['a']) + assertSelectedKeysEqual(mlc, ['list1']) + + // new tail 'd' + mlc.toggleItemForKey('d', 'list2') + assert.deepEqual(mlc.getTail(), {key: 'list2', item: 'd'}) + assertSelectedItemsEqual(mlc, ['a', 'd']) + assertSelectedKeysEqual(mlc, ['list1', 'list2']) + + // new tail 'c' + mlc.toggleItemForKey('c', 'list1') + assert.deepEqual(mlc.getTail(), {key: 'list1', item: 'c'}) + assertSelectedItemsEqual(mlc, ['a', 'c', 'd']) + assertSelectedKeysEqual(mlc, ['list1', 'list2']) + + // restore previous tail when current is unselected + mlc.toggleItemForKey('c', 'list1') + assert.deepEqual(mlc.getTail(), {key: 'list2', item: 'd'}) + assertSelectedItemsEqual(mlc, ['a', 'd']) + assertSelectedKeysEqual(mlc, ['list1', 'list2']) + + // restore previous tail when current is unselected + mlc.toggleItemForKey('d', 'list2') + assert.deepEqual(mlc.getTail(), {key: 'list1', item: 'a'}) + assertSelectedItemsEqual(mlc, ['a']) + assertSelectedKeysEqual(mlc, ['list1']) + + // tail is undefined when current is unselected + mlc.toggleItemForKey('a', 'list1') + assert.isUndefined(mlc.getTail()) + assertSelectedItemsEqual(mlc, []) + assertSelectedKeysEqual(mlc, []) })