[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..17154f827911344f720b19a0c94af610abfed6bb 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,
   },
 
   /** @private {Object} */
@@ -45,9 +47,14 @@ var BookmarksStore = Polymer({
   /** @override */
   attached: function() {
     this.documentListeners_ = {
-      'selected-folder-changed': this.onSelectedFolderChanged_.bind(this),
+      'ctrl-select-multiple-items':
+          this.onMultipleItemsCtrlSelected_.bind(this),
       'folder-open-changed': this.onFolderOpenChanged_.bind(this),
       'search-term-changed': this.onSearchTermChanged_.bind(this),
+      'select-single-item': this.onSingleItemSelected_.bind(this),
+      'selected-folder-changed': this.onSelectedFolderChanged_.bind(this),
+      'shift-select-multiple-items':
+          this.onMultipleItemsShiftSelected_.bind(this),
     };
     for (var event in this.documentListeners_)
       document.addEventListener(event, this.documentListeners_[event]);
@@ -72,8 +79,8 @@ var BookmarksStore = Polymer({
     chrome.bookmarks.onChanged.addListener(this.onBookmarkChanged_.bind(this));
   },
 
-////////////////////////////////////////////////////////////////////////////////
-// bookmarks-store, private:
+  //////////////////////////////////////////////////////////////////////////////
+  // bookmarks-store, private:
 
   /**
    * @param {BookmarkTreeNode} rootNode
@@ -91,7 +98,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,10 +122,21 @@ var BookmarksStore = Polymer({
       this.fire('selected-folder-changed', this.rootNode.children[0].id);
     } else {
       chrome.bookmarks.search(this.searchTerm, function(results) {
+        if (this.displayedList)
+          this.clearSelectedItems_();
+
+        this.prevSelectedItemIndex_ = undefined;

[calamity, 2017/01/17 06:12:22]

Can this and the if clause above move into this.clearSelectedItems_?

[jiaxi, 2017/01/20 04:51:09]

The prevSelectedItemIndex_ cannot be reset in this.clearSelectedItems_,
this.clearSelectedItems is ran by all select events at the beginning but we want
to preserve the prevSelectedItemIndex_. The only time prevSelectedItemIndex_
should be reset is when the displayedList is changed either by a search or by
changing selected folder.

[calamity, 2017/01/23 00:44:27]

Acknowledged.

+
         if (this.selectedId)
           this.deselectFolders_();
 
-        this._setDisplayedList(results);
+        var searchedList = [];
+        for (var i = 0; i < results.length; i++) {
+          searchedList.push(this.idToNodeMap_[results[i].id]);
+          this.linkPaths(
+              'displayedList.#' + i, this.idToNodeMap_[results[i].id].path);
+        }

[calamity, 2017/01/18 03:25:37]

Can we just set isSelected on the nodes inside results instead?

[jiaxi, 2017/01/20 04:51:09]

There's a bunch of changes around this part of the code and it is no longer
needed to set the isSelectedItem here.

+        this._setDisplayedList(searchedList);
       }.bind(this));
     }
   },
@@ -128,6 +147,11 @@ var BookmarksStore = Polymer({
     if (!this.selectedId)
       return;
 
+    if (this.displayedList)
+      this.clearSelectedItems_();
+
+    this.prevSelectedItemIndex_ = undefined;
+
     var selectedNode = this.idToNodeMap_[this.selectedId];
     this.linkPaths('displayedList', selectedNode.path + '.children');
     this._setDisplayedList(selectedNode.children);
@@ -150,8 +174,38 @@ var BookmarksStore = Polymer({
     delete this.idToNodeMap_[id];
   },
 
-////////////////////////////////////////////////////////////////////////////////
-// bookmarks-store, bookmarks API event listeners:
+  /**
+   * Remove all selected items in the list.
+   * @private
+   */
+  clearSelectedItems_: function() {
+    for (var i = 0; i < this.displayedList.length; i++) {
+      if (this.displayedList[i].isSelected) {

[calamity, 2017/01/18 03:25:37]

Invert and early continue.

[jiaxi, 2017/01/20 04:51:09]

Done.

+        this.set(
+            this.idToNodeMap_[this.displayedList[i].id].path +
+                '.isSelected',
+            false);
+      }
+    }
+  },
+
+  /**
+   * Return the index in the search result of an item.
+   * @param {BookmarkTreeNode} item
+   * @return {number}
+   * @private
+   */
+  getSelectedIndex_: function(item) {

[calamity, 2017/01/17 06:12:22]

It would be better to add the index to the BookmarkTreeNode when the search
comes back. Do it under a different field so that it doesn't interfere with
other code that uses item.index. Then this code becomes 

return item.searchResultIndex || item.index;

Also, getSelectedIndex_ sounds like something that gets the currently selected
index. getIndexInList_ is probably better.

[jiaxi, 2017/01/20 04:51:09]

Done.

+    if (this.searchTerm == '')
+      return item.index;
+
+    for (var i = 0; i < this.displayedList.length; i++) {
+      if (this.displayedList[i].id == item.id)
+        return i;
+    }
+  },
+  //////////////////////////////////////////////////////////////////////////////
+  //////// bookmarks-store, bookmarks API event listeners:

[calamity, 2017/01/17 06:12:22]

clang-format janked this up. Delete back to 2 slashes. Check the others too.

[jiaxi, 2017/01/20 04:51:09]

Done.

 
   /**
    * Callback for when a bookmark node is removed.
@@ -192,8 +246,8 @@ var BookmarksStore = Polymer({
       this.updateSearchDisplay_();
   },
 
-////////////////////////////////////////////////////////////////////////////////
-// bookmarks-store, bookmarks app event listeners:
+  //////////////////////////////////////////////////////////////////////////////
+  // bookmarks-store, bookmarks app event listeners:
 
   /**
    * @param {Event} e
@@ -215,11 +269,12 @@ var BookmarksStore = Polymer({
 
     // Deselect the old folder if defined.
     if (this.selectedId)
-      this.set(this.idToNodeMap_[this.selectedId].path + '.isSelected', false);
+      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,15 +289,62 @@ var BookmarksStore = Polymer({
     if (!folder.isOpen && this.isAncestorOfSelected_(folder))
       this.fire('selected-folder-changed', folder.id);
   },
+
+  /**
+   * Select a single item in the list.
+   * @param {CustomEvent} e
+   * @private
+   */
+  onSingleItemSelected_: function(e) {
+    this.clearSelectedItems_();
+    this.set(this.idToNodeMap_[e.detail.item.id].path + '.isSelected', true);
+    this.prevSelectedItemIndex_ = this.getSelectedIndex_(e.detail.item);

[calamity, 2017/01/17 06:12:22]

What happens if the item at prevSelectedItemIndex_ gets deleted?

[jiaxi, 2017/01/20 04:51:09]

The prevSelectedItemIndex_ will not change unless it's the last item in the
displayedList, then it will turn into prevSelectedItem_--. Behaviors for
selected item under deletion has been added.

+  },
+
+  /**
+   * Select multiple items based on |prevSelectedItemIndex_| and the selected
+   * item. If |prevSelectedItemIndex_| is not set, single select the item.
+   * @param {CustomEvent} e
+   * @private
+   */
+  onMultipleItemsShiftSelected_: function(e) {
+    this.clearSelectedItems_();
+    var startIndex, endIndex;
+    if (this.prevSelectedItemIndex_ == undefined) {
+      this.prevSelectedItemIndex_ = this.getSelectedIndex_(e.detail.item);
+      startIndex = this.prevSelectedItemIndex_;
+      endIndex = this.prevSelectedItemIndex_;
+    } else {
+      var currIndex = this.getSelectedIndex_(e.detail.item);
+      startIndex = Math.min(this.prevSelectedItemIndex_, currIndex);
+      endIndex = Math.max(this.prevSelectedItemIndex_, currIndex);
+    }
+    for (var i = startIndex; i <= endIndex; i++)

[calamity, 2017/01/17 06:12:22]

Braces around non-single line clauses.

[jiaxi, 2017/01/20 04:51:09]

Done.

+      this.set(
+          this.idToNodeMap_[this.displayedList[i].id].path +
+              '.isSelected',
+          true);
+  },
+
+  /**
+   * Select multiple items with the index of the last elected item as
+   * |prevSelectedItemIndex_|.
+   * @param {CustomEvent} e
+   * @private
+   */
+  onMultipleItemsCtrlSelected_: function(e) {
+    this.set(this.idToNodeMap_[e.detail.item.id].path + '.isSelected', true);
+    this.prevSelectedItemIndex_ = this.getSelectedIndex_(e.detail.item);
+  },
 });
 
 ////////////////////////////////////////////////////////////////////////////////
 // bookmarks-store, static methods:
 
 /**
- * Stores the path from the store to a node inside the node.
- * @param {BookmarkTreeNode} bookmarkNode
- * @param {number} startIndex
+* Stores the path from the store to a node inside the node.
+* @param {BookmarkTreeNode} bookmarkNode
+* @param {number} startIndex
  */
 BookmarksStore.generatePaths = function(bookmarkNode, startIndex) {
   if (!bookmarkNode.children)
@@ -263,13 +365,14 @@ BookmarksStore.generatePaths = function(bookmarkNode, startIndex) {
  * @param {Object=} idToNodeMap
  */
 BookmarksStore.initNodes = function(bookmarkNode, idToNodeMap) {
+  bookmarkNode.isSelected = false;

[calamity, 2017/01/17 06:12:22]

Maybe isSelectedItem so it's more obvious which is which.

[jiaxi, 2017/01/20 04:51:08]

Done.

   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);