[MD Bookmarks] Add Select for Bookmarks.

chrome/browser/resources/md_bookmarks/store.js

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 var BookmarksStore = Polymer({
   is: 'bookmarks-store',
 
   properties: {
     /** @type {BookmarkTreeNode} */
     rootNode: {
@@ skipping 19 matching lines @@
      * selected folder.
      * @type {Array<BookmarkTreeNode>}
      */
     displayedList: {
       type: Array,
       notify: true,
       readOnly: true,
     },
 
     idToNodeMap_: Object,
+
+    prevSelectedItemIndex_: Number,
   },
 
   /** @private {Object} */
   documentListeners_: null,
 
   /** @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]);
   },
 
   /** @override */
   detached: function() {
     for (var event in this.documentListeners_)
       document.removeEventListener(event, this.documentListeners_[event]);
   },
@@ skipping 23 matching lines @@
     this.idToNodeMap_ = {};
     this.rootNode.path = 'rootNode';
     BookmarksStore.generatePaths(rootNode, 0);
     BookmarksStore.initNodes(this.rootNode, this.idToNodeMap_);
     this.fire('selected-folder-changed', this.rootNode.children[0].id);
   },
 
   /** @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;
   },
 
   /**
    * @param {BookmarkTreeNode} folder
    * @private
    * @return {boolean}
    */
   isAncestorOfSelected_: function(folder) {
     if (!this.selectedId)
       return false;
 
     var selectedNode = this.idToNodeMap_[this.selectedId];
     return selectedNode.path.startsWith(folder.path);
   },
 
   /** @private */
   updateSearchDisplay_: function() {
     if (this.searchTerm == '') {
       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));
     }
   },
 
   /** @private */
   updateSelectedDisplay_: function() {
     // Don't change to the selected display if ID was cleared.
     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);
   },
 
   /**
    * Remove all descendants of a given node from the map.
    * @param {string} id
    * @private
    */
   removeDescendantsFromMap_: function(id) {
     var node = this.idToNodeMap_[id];
     if (!node)
       return;
 
     if (node.children) {
       for (var i = 0; i < node.children.length; i++)
         this.removeDescendantsFromMap_(node.children[i].id);
     }
     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:
 
   /**
    * Callback for when a bookmark node is removed.
    * If a folder is selected or is an ancestor of a selected folder, the parent
    * of the removed folder will be selected.
    * @param {string} id The id of the removed bookmark node.
    * @param {!{index: number,
    *           parentId: string,
    *           node: BookmarkTreeNode}} removeInfo
    */
   onBookmarkRemoved_: function(id, removeInfo) {
-    if (this.isAncestorOfSelected_(this.idToNodeMap_[id])) {
-      this.fire('selected-folder-changed', removeInfo.parentId);
-    }
+    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;
+        }
+      }
 
-    var parentNode = this.idToNodeMap_[removeInfo.parentId];
-    this.splice(parentNode.path + '.children', removeInfo.index, 1);
-    this.removeDescendantsFromMap_(id);
-    BookmarksStore.generatePaths(parentNode, removeInfo.index);
+      // 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])
 
-    // Regenerate the search list if its displayed.
-    if (this.searchTerm)
-      this.updateSearchDisplay_();
+      // 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));
   },
 
   /**
    * Called when the title of a bookmark changes.
    * @param {string} id The id of changed bookmark node.
    * @param {!Object} changeInfo
    */
   onBookmarkChanged_: function(id, changeInfo) {
     if (changeInfo.title)
       this.set(this.idToNodeMap_[id].path + '.title', changeInfo.title);
@@ skipping 19 matching lines @@
    * Selects the folder specified by the event and deselects the previously
    * selected folder.
    * @param {CustomEvent} e
    * @private
    */
   onSelectedFolderChanged_: function(e) {
     if (this.searchTerm)
       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;
   },
 
   /**
    * Handles events that open and close folders.
    * @param {CustomEvent} e
    * @private
    */
   onFolderOpenChanged_: function(e) {
     var folder = this.idToNodeMap_[e.detail.id];
     this.set(folder.path + '.isOpen', e.detail.open);
     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);
+  },
 });
 
 ////////////////////////////////////////////////////////////////////////////////
 // bookmarks-store, static methods:
 
 /**
  * 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)
     return;
 
   for (var i = startIndex; i < bookmarkNode.children.length; i++) {
     bookmarkNode.children[i].path = bookmarkNode.path + '.children.#' + i;
     BookmarksStore.generatePaths(bookmarkNode.children[i], 0);
   }
 };
 
 /**
  * Initializes the nodes in the bookmarks tree as follows:
  * - Populates |idToNodeMap_| with a mapping of all node ids to their
  *   corresponding BookmarkTreeNode.
  * - Sets all the nodes to not selected and open by default.
  * @param {BookmarkTreeNode} bookmarkNode
  * @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);
 };