[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} */

[tsergeant, 2017/01/25 00:23:18]

There's now no way for this to be null, right? You should remove this
annotation, and then go through and remove any null-checks which are no longer
needed.

[angelayang, 2017/01/31 02:25:03]

Done.

     selectedId: {
       type: String,
       observer: 'updateSelectedDisplay_',
       notify: true,
     },
 
     searchTerm: {
       type: String,
       observer: 'updateSearchDisplay_',
       notify: true,
@@ skipping 64 matching lines @@
     if (this.$.router.searchTerm)
       this.searchTerm = this.$.router.searchTerm;
     else
       this.fire('selected-folder-changed', this.$.router.selectedId);
   },
 
   /** @private */
   deselectFolders_: function() {
     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.searchTerm) {
-      this.fire('selected-folder-changed', this.rootNode.children[0].id);
+      var selectedFolder =
+          this.selectedId ? this.selectedId : this.rootNode.children[0].id;
+      this.fire('selected-folder-changed', selectedFolder);

[tsergeant, 2017/01/25 00:23:18]

I mentioned this before, but I think now is the right time to do this: We should
stop firing this event from inside store. It makes the code harder to read, and
prevents closure from doing a little bit of type checking.

Instead:

1. Move the code from onSelectedFolderChanged_ into a new (public, maybe?)
method which takes an integer id and selects that folder
2. Call that method from onSelectedFolderChanged
3. Call that method from everywhere that currently fires
'selected-folder-changed'
4. Maybe? Move the code from updateSelectedDisplay_() into that new method. I'm
not sure that there are any places where we don't want those two pieces of code
to be called at the same time, so it probably makes sense to put them together.

[angelayang, 2017/01/31 02:25:03]

Yep all that works

+      this.updateSelectedDisplay_();
     } else {
       chrome.bookmarks.search(this.searchTerm, function(results) {
         if (this.selectedId)
           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
    */
@@ skipping 86 matching lines @@
   },
 
   /**
    * 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))
+    if (folder.isOpen || !this.isAncestorOfSelected_(folder))
+      return;
+
+    // Save the closed folder as the selectedId.
+    if (this.searchTerm)

[angelayang, 2017/01/31 02:25:03]

This piece of logic reoccurs a few times so i'm going to move it into a separate
function updateSelectedId

+      this.selectedId = folder.id;
+    else
       this.fire('selected-folder-changed', folder.id);
   },
 });
 
 ////////////////////////////////////////////////////////////////////////////////
 // bookmarks-store, static methods:
 
 /**
  * Stores the path from the store to a node inside the node.
  * @param {BookmarkTreeNode} bookmarkNode
@@ skipping 22 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);
 };