[MD Bookmarks] Modify search to retain the previously selected folder.

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 a5f3790860df23d81c4a276fef5d107f6ad26afe..7c10f4de2923ab50bd30d9395f87ced82e14efd6 100644
--- a/chrome/browser/resources/md_bookmarks/store.js
+++ b/chrome/browser/resources/md_bookmarks/store.js
@@ -12,10 +12,9 @@ var BookmarksStore = Polymer({
       notify: true,
     },
 
-    /** @type {?string} */
+    /** @type {string} */
     selectedId: {
       type: String,
-      observer: 'updateSelectedDisplay_',
       notify: true,
     },
 
@@ -88,17 +87,17 @@ var BookmarksStore = Polymer({
     BookmarksStore.initNodes(this.rootNode, this.idToNodeMap_);
 
     // Initialize the store's fields from the router.
-    if (this.$.router.searchTerm)
-      this.searchTerm = this.$.router.searchTerm;
-    else
-      this.fire('selected-folder-changed', this.$.router.selectedId);
+    this.searchTerm = this.$.router.searchTerm;
+    this.updateSelectedId_(this.$.router.selectedId);
   },
 
   /** @private */
   deselectFolders_: function() {
+    if (!this.selectedId)
+      return;
+
     this.unlinkPaths('displayedList');
     this.set(this.idToNodeMap_[this.selectedId].path + '.isSelected', false);
-    this.selectedId = null;
   },
 
   /**
@@ -107,9 +106,6 @@ var BookmarksStore = Polymer({
    * @return {boolean}
    */
   isAncestorOfSelected_: function(folder) {
-    if (!this.selectedId)
-      return false;
-
     var selectedNode = this.idToNodeMap_[this.selectedId];
     return selectedNode.path.startsWith(folder.path);
   },
@@ -120,28 +116,15 @@ var BookmarksStore = Polymer({
       return;
 
     if (!this.searchTerm) {
-      this.fire('selected-folder-changed', this.rootNode.children[0].id);
+      this.selectFolder(this.selectedId);
     } else {
       chrome.bookmarks.search(this.searchTerm, function(results) {
-        if (this.selectedId)
-          this.deselectFolders_();
-
+        this.deselectFolders_();
         this._setDisplayedList(results);
       }.bind(this));
     }
   },
 
-  /** @private */
-  updateSelectedDisplay_: function() {
-    // Don't change to the selected display if ID was cleared.
-    if (!this.selectedId)
-      return;
-
-    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
@@ -159,6 +142,39 @@ var BookmarksStore = Polymer({
     delete this.idToNodeMap_[id];
   },
 
+  /**
+   * Update the selectedId according to the state of the store.
+   * @private
+   */
+  updateSelectedId_: function(id) {

[tsergeant, 2017/01/31 03:21:01]

The API here is really unclear. If I want to select a folder, which function do
I need to call? My expectation from reading the code is that I would call the
public `selectFolder` function, but that doesn't deselect any existing selected
folder.

Is there any reason why we can't have one `selectFolder` function that does all
the work of both these functions? That would simplify things a lot.

[angelayang, 2017/01/31 06:29:25]

yeah that works much better and clearer thanks

+    if (!this.idToNodeMap_)
+      return;
+
+    if (!this.idToNodeMap_[id] || this.idToNodeMap_[id].url)
+      id = this.rootNode.children[0].id;
+
+    this.deselectFolders_();
+    if (this.searchTerm)
+      this.selectedId = id;
+    else
+      this.selectFolder(id);
+  },
+
+  /**
+   * Sets |displayedList| to the folder corresponding to the id.
+   * @param {string} id The id of the folder to be displayed.
+   */
+  selectFolder: function(id) {
+    var newFolder = this.idToNodeMap_[id];
+    this.set(newFolder.path + '.isSelected', true);
+    this.selectedId = id;
+
+    // Update the displayed list to the selected folder.
+    var selectedNode = this.idToNodeMap_[this.selectedId];
+    this.linkPaths('displayedList', selectedNode.path + '.children');
+    this._setDisplayedList(selectedNode.children);
+  },
+
   //////////////////////////////////////////////////////////////////////////////
   // bookmarks-store, bookmarks API event listeners:
 
@@ -173,7 +189,7 @@ var BookmarksStore = Polymer({
    */
   onBookmarkRemoved_: function(id, removeInfo) {
     if (this.isAncestorOfSelected_(this.idToNodeMap_[id]))
-      this.fire('selected-folder-changed', removeInfo.parentId);
+      this.selectFolder(removeInfo.parentId);
 
     var parentNode = this.idToNodeMap_[removeInfo.parentId];
     this.splice(parentNode.path + '.children', removeInfo.index, 1);
@@ -218,21 +234,8 @@ var BookmarksStore = Polymer({
    * @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);
-
-    // Check if the selected id is that of a defined folder.
     var id = /** @type {string} */ (e.detail);
-    if (!this.idToNodeMap_[id] || this.idToNodeMap_[id].url)
-      id = this.rootNode.children[0].id;
-
-    var newFolder = this.idToNodeMap_[id];
-    this.set(newFolder.path + '.isSelected', true);
-    this.selectedId = id;
+    this.updateSelectedId_(id);
   },
 
   /**
@@ -244,7 +247,7 @@ var BookmarksStore = Polymer({
     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);
+      this.updateSelectedId_(folder.id);
   },
 });