[MD Bookmarks] Add Select for Bookmarks.

chrome/browser/resources/md_bookmarks/store.js

Index: chrome/browser/resources/md_bookmarks/store.js
diff --git a/chrome/browser/resources/md_bookmarks/store.js b/chrome/browser/resources/md_bookmarks/store.js
index 15b270fbe23c31b922c2bb02f27f902a2a59c9db..bae0e930c369f2b4e48b6929efa6eda4b02377e2 100644
--- a/chrome/browser/resources/md_bookmarks/store.js
+++ b/chrome/browser/resources/md_bookmarks/store.js
@@ -37,6 +37,8 @@ var BookmarksStore = Polymer({
     },
 
     idToNodeMap_: Object,
+
+    prevSelectedItemIndex_: Number,

[calamity, 2017/01/24 06:39:17]

Hmm. I got confused by this thinking it was the actual previous selection index.
This is probably better called anchorIndex as in list_selection_controller.js.

[jiaxi, 2017/01/25 03:26:10]

Done.

   },
 
   /** @private {Object} */
@@ -45,9 +47,10 @@ var BookmarksStore = Polymer({
   /** @override */
   attached: function() {
     this.documentListeners_ = {
-      'selected-folder-changed': this.onSelectedFolderChanged_.bind(this),
       'folder-open-changed': this.onFolderOpenChanged_.bind(this),
       'search-term-changed': this.onSearchTermChanged_.bind(this),
+      'select-item': this.onItemSelected_.bind(this),
+      'selected-folder-changed': this.onSelectedFolderChanged_.bind(this),
     };
     for (var event in this.documentListeners_)
       document.addEventListener(event, this.documentListeners_[event]);
@@ -91,7 +94,8 @@ var BookmarksStore = Polymer({
   /** @private */
   deselectFolders_: function() {
     this.unlinkPaths('displayedList');
-    this.set(this.idToNodeMap_[this.selectedId].path + '.isSelected', false);
+    this.set(
+        this.idToNodeMap_[this.selectedId].path + '.isSelectedFolder', false);
     this.selectedId = null;
   },
 
@@ -114,9 +118,15 @@ var BookmarksStore = Polymer({
       this.fire('selected-folder-changed', this.rootNode.children[0].id);
     } else {
       chrome.bookmarks.search(this.searchTerm, function(results) {
+        this.clearSelectedItems_();
+        this.prevSelectedItemIndex_ = undefined;
+
         if (this.selectedId)
           this.deselectFolders_();
 
+        for (var i = 0; i < results.length; i++)
+          results[i].searchResultIndex = i;
+
         this._setDisplayedList(results);
       }.bind(this));
     }
@@ -128,6 +138,9 @@ var BookmarksStore = Polymer({
     if (!this.selectedId)
       return;
 
+    this.clearSelectedItems_();
+    this.prevSelectedItemIndex_ = undefined;
+
     var selectedNode = this.idToNodeMap_[this.selectedId];
     this.linkPaths('displayedList', selectedNode.path + '.children');
     this._setDisplayedList(selectedNode.children);
@@ -150,6 +163,46 @@ var BookmarksStore = Polymer({
     delete this.idToNodeMap_[id];
   },
 
+  /**
+   * Remove all selected items in the list.
+   * @private
+   */
+  clearSelectedItems_: function() {
+    if (!this.displayedList)
+      return;
+
+    for (var i = 0; i < this.displayedList.length; i++) {
+      if (!this.displayedList[i].isSelectedItem)
+        continue;
+
+      this.set('displayedList.#' + i + '.isSelectedItem', false);
+    }
+  },
+
+  /**
+   * Return the index in the search result of an item.
+   * @param {BookmarkTreeNode} item
+   * @return {number}
+   * @private
+   */
+  getIndexInList_: function(item) {
+    if (this.searchTerm)
+      return item.searchResultIndex;
+
+    return item.index;

[calamity, 2017/01/24 06:39:17]

I think this can just be return item.searchResultIndex || item.index;

[jiaxi, 2017/01/25 03:26:10]

This will return undefined if we select the first item in the displayedList
while searching (item.searchResultIndex = 0 = false in js).

+  },
+
+  isInDisplayedList_: function(id) {
+    if (this.searchTerm) {
+      for (var i = 0; i < this.displayedList.length; i++) {
+        if (this.displayedList[i].id == id)
+          return true;
+      }
+      return false;
+    }

[calamity, 2017/01/24 06:39:17]

You may want to think about caching this information in a map for fast lookup
instead of iterating.

[jiaxi, 2017/01/25 03:26:10]

Done.

+
+    return (this.idToNodeMap_[id].parentId == this.selectedId);

[calamity, 2017/01/24 06:39:17]

nit: No parens.

[jiaxi, 2017/01/25 03:26:10]

Done.

+  },
 ////////////////////////////////////////////////////////////////////////////////
 // bookmarks-store, bookmarks API event listeners:
 
@@ -163,18 +216,51 @@ var BookmarksStore = Polymer({
    *           node: BookmarkTreeNode}} removeInfo
    */
   onBookmarkRemoved_: function(id, removeInfo) {
-    if (this.isAncestorOfSelected_(this.idToNodeMap_[id])) {
-      this.fire('selected-folder-changed', removeInfo.parentId);
-    }
-
-    var parentNode = this.idToNodeMap_[removeInfo.parentId];
-    this.splice(parentNode.path + '.children', removeInfo.index, 1);
-    this.removeDescendantsFromMap_(id);
-    BookmarksStore.generatePaths(parentNode, removeInfo.index);
-
-    // Regenerate the search list if its displayed.
-    if (this.searchTerm)
-      this.updateSearchDisplay_();
+    chrome.bookmarks.getSubTree(removeInfo.parentId, function(parentNode) {

[calamity, 2017/01/24 06:39:17]

Call this parentNodes so it's not confused with the var below.

[jiaxi, 2017/01/25 03:26:11]

Done.

+      var parentNode = parentNode[0];
+      var isAncestor = this.isAncestorOfSelected_(this.idToNodeMap_[id]);
+      var isInDisplayedList = this.isInDisplayedList_(id);
+
+      // Update the tree.
+      this.removeDescendantsFromMap_(id);

[calamity, 2017/01/24 06:39:17]

Should this be replacing the parent node? Is this even necessary since you're
setting a new parentNode later anyway?

[jiaxi, 2017/01/25 03:26:11]

Discussed offline. This will remove all the extra nodes in the idToNodeMap_.

+      parentNode.path = this.idToNodeMap_[parentNode.id].path;
+      BookmarksStore.generatePaths(parentNode, 0);
+      BookmarksStore.initNodes(parentNode, this.idToNodeMap_);
+      this.set(parentNode.path, parentNode)
+
+      // Update selectedId if the removed node is an ancestor of the current
+      // selected node.
+      if (isAncestor)
+        this.fire('selected-folder-changed', removeInfo.parentId);
+
+      // Only update the displayedList if the removed node is in the
+      // displayedList.
+      if (isInDisplayedList) {

[calamity, 2017/01/24 06:39:17]

Invert and early return.

[jiaxi, 2017/01/25 03:26:11]

Done.

+        if (this.prevSelectedItemIndex_ == this.displayedList.length - 1)
+          this.prevSelectedItemIndex_--;
+
+        if (this.searchTerm) {
+          chrome.bookmarks.search(this.searchTerm, function(results) {
+            for (var i = 0; i < results.length; i++)
+              results[i].searchResultIndex = i;

[calamity, 2017/01/24 06:39:17]

This code should really be shared with the searching code.

[jiaxi, 2017/01/25 03:26:11]

Done.

+
+            this._setDisplayedList(results);
+
+            this.set(
+                'displayedList.#' + this.prevSelectedItemIndex_ +
+                    '.isSelectedItem',
+                true);

[calamity, 2017/01/24 06:39:17]

What if prevSelectedItemIndex is null?

[jiaxi, 2017/01/25 03:26:11]

The prevSelectedItemIndex(anchorIndex) can only be either undefined or a number.
When it is undefined it will not select anything.

+          }.bind(this));
+        } else {
+          this._setDisplayedList(parentNode.children);
+
+          this.set(
+              'displayedList.#' + this.prevSelectedItemIndex_ +
+                  '.isSelectedItem',
+              true);
+        }
+      }
+    }.bind(this));
   },
 
   /**
@@ -214,12 +300,13 @@ var BookmarksStore = Polymer({
       this.searchTerm = '';
 
     // Deselect the old folder if defined.
-    if (this.selectedId)
-      this.set(this.idToNodeMap_[this.selectedId].path + '.isSelected', false);
+    if (this.selectedId && this.idToNodeMap_[this.selectedId])
+      this.set(
+          this.idToNodeMap_[this.selectedId].path + '.isSelectedFolder', false);
 
     var selectedId = /** @type {string} */ (e.detail);
     var newFolder = this.idToNodeMap_[selectedId];
-    this.set(newFolder.path + '.isSelected', true);
+    this.set(newFolder.path + '.isSelectedFolder', true);
     this.selectedId = selectedId;
   },
 
@@ -234,6 +321,68 @@ var BookmarksStore = Polymer({
     if (!folder.isOpen && this.isAncestorOfSelected_(folder))
       this.fire('selected-folder-changed', folder.id);
   },
+
+  /**
+   * Select a single item in the list.
+   * @param {BookmarkTreeNode} item
+   * @private
+   */
+  onSingleItemSelected_: function(item) {
+    this.clearSelectedItems_();
+    this.prevSelectedItemIndex_ = this.getIndexInList_(item);
+    this.set(
+        'displayedList.#' + this.prevSelectedItemIndex_ + '.isSelectedItem',
+        true);
+  },
+
+  /**
+   * Select multiple items based on |prevSelectedItemIndex_| and the selected
+   * item. If |prevSelectedItemIndex_| is not set, single select the item.
+   * @param {BookmarkTreeNode} item
+   * @private
+   */
+  onMultipleItemsShiftSelected_: function(item) {
+    this.clearSelectedItems_();
+    var startIndex, endIndex;
+    if (this.prevSelectedItemIndex_ == undefined) {
+      this.prevSelectedItemIndex_ = this.getIndexInList_(item);
+      startIndex = this.prevSelectedItemIndex_;
+      endIndex = this.prevSelectedItemIndex_;
+    } else {
+      var currIndex = this.getIndexInList_(item);

[calamity, 2017/01/24 06:39:17]

nit: selectedIndex.

[jiaxi, 2017/01/25 03:26:11]

Done.

+      startIndex = Math.min(this.prevSelectedItemIndex_, currIndex);
+      endIndex = Math.max(this.prevSelectedItemIndex_, currIndex);
+    }
+    for (var i = startIndex; i <= endIndex; i++)
+      this.set('displayedList.#' + i + '.isSelectedItem', true);
+  },
+
+  /**
+   * Select multiple items with the index of the last elected item as
+   * |prevSelectedItemIndex_|.
+   * @param {BookmarkTreeNode} item
+   * @private
+   */
+  onMultipleItemsCtrlSelected_: function(item) {
+    this.prevSelectedItemIndex_ = this.getIndexInList_(item);
+    this.set(
+        'displayedList.#' + this.prevSelectedItemIndex_ + '.isSelectedItem',
+        true);
+  },
+
+  /**
+   * Select item according to keyboard behaviours.
+   * @param {CustomEvent} e
+   * @private
+   */
+  onItemSelected_: function(e) {
+    if (e.detail.shiftKey)
+      this.onMultipleItemsShiftSelected_(e.detail.item);
+    else if (e.detail.ctrlKey)
+      this.onMultipleItemsCtrlSelected_(e.detail.item);
+    else
+      this.onSingleItemSelected_(e.detail.item);
+  },
 });
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -263,13 +412,14 @@ BookmarksStore.generatePaths = function(bookmarkNode, startIndex) {
  * @param {Object=} idToNodeMap
  */
 BookmarksStore.initNodes = function(bookmarkNode, idToNodeMap) {
+  bookmarkNode.isSelectedItem = false;
   if (idToNodeMap)
     idToNodeMap[bookmarkNode.id] = bookmarkNode;
 
   if (bookmarkNode.url)
     return;
 
-  bookmarkNode.isSelected = false;
+  bookmarkNode.isSelectedFolder = false;
   bookmarkNode.isOpen = true;
   for (var i = 0; i < bookmarkNode.children.length; i++)
     BookmarksStore.initNodes(bookmarkNode.children[i], idToNodeMap);