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

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: {
       type: Object,
       notify: true,
     },
 
-    /** @type {?string} */
+    /** @type {string} */
     selectedId: {
       type: String,
-      observer: 'updateSelectedDisplay_',
       notify: true,
     },
 
     searchTerm: {
       type: String,
       value: '',
       observer: 'updateSearchDisplay_',
       notify: true,
     },
 
@@ skipping 52 matching lines @@
    * @private
    */
   setupStore_: function(rootNode) {
     this.rootNode = rootNode;
     this.idToNodeMap_ = {};
     this.rootNode.path = 'rootNode';
     BookmarksStore.generatePaths(rootNode, 0);
     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;
   },
 
   /**
    * @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.rootNode)
       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
    * @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];
   },
 
+  /**
+   * 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:
 
   /**
    * 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);
+      this.selectFolder(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_();
   },
@@ skipping 24 matching lines @@
     this.searchTerm = /** @type {string} */ (e.detail);
   },
 
   /**
    * 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);
-
-    // 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);
   },
 
   /**
    * 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);
+      this.updateSelectedId_(folder.id);
   },
 });
 
 ////////////////////////////////////////////////////////////////////////////////
 // bookmarks-store, static methods:
 
 /**
  * Stores the path from the store to a node inside the node.
  * @param {BookmarkTreeNode} bookmarkNode
  * @param {number} startIndex
@@ skipping 21 matching lines @@
     idToNodeMap[bookmarkNode.id] = bookmarkNode;
 
   if (bookmarkNode.url)
     return;
 
   bookmarkNode.isSelected = false;
   bookmarkNode.isOpen = true;
   for (var i = 0; i < bookmarkNode.children.length; i++)
     BookmarksStore.initNodes(bookmarkNode.children[i], idToNodeMap);
 };