[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..282dabdc27223dfb8d3ff4f79a0907500a0a7f5b 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,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++)
+          this.idToNodeMap_[results[i].id].searchResultIndex = i;

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

Why do we have to update the nodes inside the tree still?

Ah. After reading more of the patch, I realize it's because I told you to send
the id through the event. This was a mistake. You should send the item up so
that you can directly deal with them and not have to access things through the
idToNodeMap_.

From a data model perspective, we want a clear separation of things in the tree
and things in the search results. Trying to match the search results with the
tree is messy and error-prone, and we should only do it when absolutely
necessary.

This property should be getting set on the search results. Not on the tree.

[jiaxi, 2017/01/23 06:14:46]

Done.

+
         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,35 @@ 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 {string} id
+   * @return {number}
+   * @private
+   */
+  getIndexInList_: function(id) {
+    if (this.searchTerm)
+      return this.idToNodeMap_[id].searchResultIndex;
+
+    return this.idToNodeMap_[id].index;
+  },
+
 ////////////////////////////////////////////////////////////////////////////////
 // bookmarks-store, bookmarks API event listeners:
 
@@ -163,18 +205,55 @@ 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/23 00:44:27]

Does this cancel all existing selections?

[jiaxi, 2017/01/23 06:14:46]

All existing selections will be canceled when the displayedList gets set.

+      var isSelected = this.isAncestorOfSelected_(this.idToNodeMap_[id]);
+      // Check if the removed item is from the list.

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

This only checks if the removed item is the previously selected item? Is this
intentional?

[jiaxi, 2017/01/23 06:14:46]

Now it checks if the removed item is in the displayedList or not instead. In old
bmm no matter how many items we removed, there's only one item selected after
the removal.

+      var isRemovedFromList = false;
+      if (this.prevSelectedItemIndex_ != undefined) {
+        if (id == this.displayedList[this.prevSelectedItemIndex_].id) {

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

Use && for these 2 conditions?

[jiaxi, 2017/01/23 06:14:46]

This part has been changed a lot so...

+          if (this.prevSelectedItemIndex_ == this.displayedList.length - 1)
+            this.prevSelectedItemIndex_--;
+          isRemovedFromList = true;
+        }
+      }
+
+      // Update the tree.
+      this.removeDescendantsFromMap_(id);
+      parentNode[0].path = this.idToNodeMap_[parentNode[0].id].path;

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

Change the arg to the function to parentNodes and then make a var parentNode =
parentNodes[0]. I don't understand why the API gives back an array.

[jiaxi, 2017/01/23 06:14:46]

Done.

+      BookmarksStore.generatePaths(parentNode[0], 0);
+      BookmarksStore.initNodes(parentNode[0], this.idToNodeMap_);
+      this.set(parentNode[0].path, parentNode[0])
+
+      // Update selectedId.
+      if (isSelected)
+        this.fire('selected-folder-changed', removeInfo.parentId);
+
+      // Update the displayedList.
+      if (this.searchTerm) {
+        // Regenerate the search list if its displayed.
+        chrome.bookmarks.search(this.searchTerm, function(results) {
+          for (var i = 0; i < results.length; i++)
+            this.idToNodeMap_[results[i].id].searchResultIndex = i;
+
+          this._setDisplayedList(results);
+          if (!isRemovedFromList)
+            return
+
+          this.set(
+              'displayedList.#' + this.prevSelectedItemIndex_ +
+                  '.isSelectedItem',
+              true);
+        }.bind(this));
+      } else {
+        this._setDisplayedList(parentNode[0].children);

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

This assumes that removed bookmarks are always being displayed. As mentioned,
this gets called if bookmarks are removed from the bookmarks bar and should be
responsive.

[jiaxi, 2017/01/23 06:14:46]

Done.

+        if (!isRemovedFromList)
+          return
+
+        this.set(
+            'displayedList.#' + this.prevSelectedItemIndex_ + '.isSelectedItem',
+            true);

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

I'm not sure what this does?

[jiaxi, 2017/01/23 06:14:46]

This selects the item in the displayedList since we cleared everything selected.

+      }
+    }.bind(this));
   },
 
   /**
@@ -214,12 +293,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 +314,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 {string} id
+   * @private
+   */
+  onSingleItemSelected_: function(id) {
+    this.clearSelectedItems_();
+    this.prevSelectedItemIndex_ = this.getIndexInList_(id);
+    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 {string} id
+   * @private
+   */
+  onMultipleItemsShiftSelected_: function(id) {
+    this.clearSelectedItems_();
+    var startIndex, endIndex;
+    if (this.prevSelectedItemIndex_ == undefined) {
+      this.prevSelectedItemIndex_ = this.getIndexInList_(id);
+      startIndex = this.prevSelectedItemIndex_;
+      endIndex = this.prevSelectedItemIndex_;
+    } else {
+      var currIndex = this.getIndexInList_(id);
+      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 {string} id
+   * @private
+   */
+  onMultipleItemsCtrlSelected_: function(id) {
+    this.prevSelectedItemIndex_ = this.getIndexInList_(id);
+    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.id);
+    else if (e.detail.ctrlKey)
+      this.onMultipleItemsCtrlSelected_(e.detail.id);
+    else
+      this.onSingleItemSelected_(e.detail.id);
+  },
 });
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -263,13 +405,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);