[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 620a1231ec176ce28c6c269607e8b331be132c11..b8bb8a62b0c069e610c909dce9e8d22d0b5bd36d 100644
--- a/chrome/browser/resources/md_bookmarks/store.js
+++ b/chrome/browser/resources/md_bookmarks/store.js
@@ -37,7 +37,13 @@ var BookmarksStore = Polymer({
       readOnly: true,
     },
 
+    /** @type{Object<string, !BookmarkTreeNode>} */

[tsergeant, 2017/01/30 06:39:47]

Space after type:

@type {}

(sorry, I typoed this in my comments last time)

[jiaxi, 2017/01/31 00:37:42]

Done.

     idToNodeMap_: Object,
+
+    anchorIndex_: Number,
+
+    /** @type{Object<string, !boolean>} */

[tsergeant, 2017/01/30 06:39:47]

Space after type here, too

[jiaxi, 2017/01/31 00:37:42]

Since this is a set object now do we need closure for it?

[tsergeant, 2017/01/31 01:55:35]

You should be able to add /** @type {Set<string>} */

[jiaxi, 2017/01/31 03:45:03]

Done.

+    searchResultSet_: Object,
   },
 
   /** @private {Object} */
@@ -46,9 +52,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]);
@@ -73,8 +80,8 @@ var BookmarksStore = Polymer({
     chrome.bookmarks.onChanged.addListener(this.onBookmarkChanged_.bind(this));
   },
 
-  //////////////////////////////////////////////////////////////////////////////
-  // bookmarks-store, private:
+////////////////////////////////////////////////////////////////////////////////
+// bookmarks-store, private:
 
   /**
    * @param {BookmarkTreeNode} rootNode
@@ -97,7 +104,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;
   },
 
@@ -115,7 +123,7 @@ var BookmarksStore = Polymer({
   },
 
   /** @private */
-  updateSearchDisplay_: function() {
+  updateSearchDisplay_: function(searchObserver) {

[tsergeant, 2017/01/30 06:39:47]

- Rename `searchObserver` to `searchTerm`, since that's what it actually is

- Add an `@param {?string} searchTerm` annotation to mark searchTerm as optional

- Add a comment explaining why this is necessary.

[jiaxi, 2017/01/31 00:37:42]

We're using this.searchTerm in this function. Will renaming the param to
searchTerm be a bit confusing?

     if (!this.rootNode)
       return;
 
@@ -123,10 +131,18 @@ var BookmarksStore = Polymer({
       this.fire('selected-folder-changed', this.rootNode.children[0].id);
     } else {
       chrome.bookmarks.search(this.searchTerm, function(results) {
+        if (searchObserver)

[tsergeant, 2017/01/30 06:39:47]

This is an interesting way to solve this problem.

@Chris -- I'm happy to go ahead with 'creative solution' in the interest of
making forward progress on this CL and the things it is blocking. Do you agree?

+          this.anchorIndex_ = undefined;
+        this.clearSelectedItems_();
+        this.searchResultSet_ = {};

[tsergeant, 2017/01/30 06:39:47]

There's a Set class that you should use for this:

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Obje...

Which lets you do add(), clear() and has()

[jiaxi, 2017/01/31 00:37:42]

Done.

+
         if (this.selectedId)
           this.deselectFolders_();
 
-        this._setDisplayedList(results);
+        this.setupSearchResults_(results);
+
+        this.set(
+            'displayedList.#' + this.anchorIndex_ + '.isSelectedItem', true);
       }.bind(this));
     }
   },
@@ -137,6 +153,9 @@ var BookmarksStore = Polymer({
     if (!this.selectedId)
       return;
 
+    this.clearSelectedItems_();
+    this.anchorIndex_ = undefined;
+
     var selectedNode = this.idToNodeMap_[this.selectedId];
     this.linkPaths('displayedList', selectedNode.path + '.children');
     this._setDisplayedList(selectedNode.children);
@@ -159,8 +178,62 @@ var BookmarksStore = Polymer({
     delete this.idToNodeMap_[id];
   },
 
-  //////////////////////////////////////////////////////////////////////////////
-  // bookmarks-store, bookmarks API event listeners:
+  /**
+   * 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) {
+    return this.searchTerm ? item.searchResultIndex : item.index;
+  },
+
+  /**
+   * @param {BookmarkTreeNode} item
+   * @return {boolean}
+   * @private
+   */
+  isInDisplayedList_: function(id) {
+    return this.searchTerm ? this.searchResultSet_[id] :
+                             this.idToNodeMap_[id].parentId == this.selectedId;
+  },
+
+  /**
+   * Initializes the search results returned by the API as follows:
+   * - Populates |searchResultSet_| with a mapping of all result ids to
+   *   their corresponding result.
+   * - Sets up the |searchResultIndex|.
+   * @param {Array<BookmarkTreeNode>} item
+   * @private
+   */
+  setupSearchResults_: function(results) {
+    for (var i = 0; i < results.length; i++) {
+      results[i].searchResultIndex = i;
+      results[i].isSelectedItem = false;
+      this.searchResultSet_[results[i].id] = true;
+    }
+
+    this._setDisplayedList(results);
+  },
+
+////////////////////////////////////////////////////////////////////////////////
+// bookmarks-store, bookmarks API event listeners:
 
   /**
    * Callback for when a bookmark node is removed.
@@ -172,18 +245,44 @@ 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(parentNodes) {
+      var parentNode = parentNodes[0];
+      var isAncestor = this.isAncestorOfSelected_(this.idToNodeMap_[id]);
+      var isInDisplayedList = this.isInDisplayedList_(id);
+
+      // Updates the tree with the new subtree we got so every node has the
+      // correct index.

[tsergeant, 2017/01/30 06:39:47]

Update this comment to explain that since Polymer doesn't update the indexes,
you need to do it manually.

[jiaxi, 2017/01/31 00:37:42]

Done. I moved selectRange and selectItem to the right section so this looks a
bit messy. The only thing changed in this part of the code is the comment.

+      this.removeDescendantsFromMap_(id);
+      parentNode.path = this.idToNodeMap_[parentNode.id].path;
+      BookmarksStore.generatePaths(parentNode, 0);
+      BookmarksStore.initNodes(parentNode, this.idToNodeMap_);
+      this.set(parentNode.path, parentNode);
+
+      // Updates 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)
+        return;
+
+      if (this.anchorIndex_ == this.displayedList.length - 1)
+        this.anchorIndex_--;
+
+      if (this.searchTerm) {
+        this.updateSearchDisplay_();
+      } else {
+        if (!isAncestor)
+          this.fire('selected-folder-changed', this.selectedId);
+
+        this._setDisplayedList(parentNode.children);
+
+        this.set(
+            'displayedList.#' + this.anchorIndex_ + '.isSelectedItem', true);
+      }
+    }.bind(this));
   },
 
   /**
@@ -201,8 +300,39 @@ var BookmarksStore = Polymer({
       this.updateSearchDisplay_();
   },
 
-  //////////////////////////////////////////////////////////////////////////////
-  // bookmarks-store, bookmarks app event listeners:
+  /**
+   * Select multiple items based on |anchorIndex_| and the selected
+   * item. If |anchorIndex_| is not set, single select the item.
+   * @param {BookmarkTreeNode} item
+   * @private
+   */
+  selectRange_: function(item) {
+    var startIndex, endIndex;
+    if (this.anchorIndex_ == undefined) {
+      this.anchorIndex_ = this.getIndexInList_(item);
+      startIndex = this.anchorIndex_;
+      endIndex = this.anchorIndex_;
+    } else {
+      var selectedIndex = this.getIndexInList_(item);
+      startIndex = Math.min(this.anchorIndex_, selectedIndex);
+      endIndex = Math.max(this.anchorIndex_, selectedIndex);
+    }
+    for (var i = startIndex; i <= endIndex; i++)
+      this.set('displayedList.#' + i + '.isSelectedItem', true);
+  },
+
+  /**
+   * Selects a single item in the displayedList.
+   * @param {BookmarkTreeNode} item
+   * @private
+   */
+  selectItem_: function(item) {
+    this.anchorIndex_ = this.getIndexInList_(item);
+    this.set('displayedList.#' + this.anchorIndex_ + '.isSelectedItem', true);
+  },
+
+////////////////////////////////////////////////////////////////////////////////
+// bookmarks-store, bookmarks app event listeners:
 
   /**
    * @param {Event} e
@@ -223,8 +353,9 @@ 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);
 
     // Check if the selected id is that of a defined folder.
     var id = /** @type {string} */ (e.detail);
@@ -232,7 +363,7 @@ var BookmarksStore = Polymer({
       id = this.rootNode.children[0].id;
 
     var newFolder = this.idToNodeMap_[id];
-    this.set(newFolder.path + '.isSelected', true);
+    this.set(newFolder.path + '.isSelectedFolder', true);
     this.selectedId = id;
   },
 
@@ -247,6 +378,21 @@ var BookmarksStore = Polymer({
     if (!folder.isOpen && this.isAncestorOfSelected_(folder))
       this.fire('selected-folder-changed', folder.id);
   },
+
+  /**
+   * Selects items according to keyboard behaviours.
+   * @param {CustomEvent} e
+   * @private
+   */
+  onItemSelected_: function(e) {
+    if (!e.detail.add)
+      this.clearSelectedItems_();
+
+    if (e.detail.range)
+      this.selectRange_(e.detail.item);
+    else
+      this.selectItem_(e.detail.item);
+  },
 });
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -276,13 +422,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);