[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,
     },
 
     /**
      * This updates to either the result of a search or the contents of the
      * selected folder.
      * @type {Array<BookmarkTreeNode>}
      */
     displayedList: {
       type: Array,
       notify: true,
       readOnly: true,
     },
 
     /** @type {Object<?string, !BookmarkTreeNode>} */
     idToNodeMap_: Object,
 
     /** @type {?number} */
-    anchorIndex_: Number,
+    anchorIndex_: {

[tsergeant, 2017/02/08 03:11:10]

Any particular reason why this needed to change? Nothing else in the CL touches
anchorIndex

[angelayang, 2017/02/09 04:17:36]

Previously anchorIndex is set to null on initialization by updateSelectedDisplay
i believe which is now gone in this refactor. There is a test to check if it's
null on initialization. (It's now undefined on initialization) Perhaps I could
change test to match new behavior? what do you think?

+      type: Number,
+      value: null,
+    },
 
     /** @type {Set<string>} */
     searchResultSet_: Object,
   },
 
   /** @private {Object} */
   documentListeners_: null,
 
   /** @override */
   attached: function() {
@@ skipping 34 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.selectFolder_(this.$.router.selectedId);
+    this.searchTerm = this.$.router.searchTerm;
   },
 
   /** @private */
   deselectFolders_: function() {
+    if (!this.idToNodeMap_[this.selectedId])
+      return;
+
     this.unlinkPaths('displayedList');
     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.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) {
         this.anchorIndex_ = null;
         this.clearSelectedItems_();
         this.searchResultSet_ = new Set();
 
-        if (this.selectedId)
-          this.deselectFolders_();
-
+        this.deselectFolders_();
         this.setupSearchResults_(results);
       }.bind(this));
     }
   },
 
-  /** @private */
-  updateSelectedDisplay_: function() {
-    // Don't change to the selected display if ID was cleared.
-    if (!this.selectedId)
-      return;
-
-    this.clearSelectedItems_();
-    this.anchorIndex_ = null;
-
-    var selectedNode = this.idToNodeMap_[this.selectedId];
-    this.linkPaths('displayedList', selectedNode.path + '.children');
-    this._setDisplayedList(
-        /** @type {Array<BookmarkTreeNode>} */ (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];
   },
 
   /**
+   * Changes the selected folder to that of the id or else defaults to the
+   * Bookmarks Bar.
+   * @param {string} id The id of the folder to be selected.
+   * @private
+   */
+  selectFolder_: function(id) {

[tsergeant, 2017/02/08 03:11:10]

I think we should make this a public method.

This method is an important part of the public API for the element, and it's
already publicly accessible through the selected-folder-changed event.

Then, in the tests, anywhere where you don't specifically want to test that the
event is working correctly, you can just call the `store.selectFolder()` method
instead of firing the event, which is much easier to read.

+    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 {
+      var newFolder = this.idToNodeMap_[id];
+      this.set(newFolder.path + '.isSelectedFolder', 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);
+    }
+  },
+
+  /**
    * 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
    */
@@ skipping 82 matching lines @@
       // indexes will have changed and Polymer doesn't update them.
       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);
+        this.selectFolder_(removeInfo.parentId);
 
       // Only update the displayedList if the removed node is in the
       // displayedList.
       if (!wasInDisplayedList)
         return;
 
       this.anchorIndex_ = null;
 
       // Update the currently displayed list.
       if (this.searchTerm) {
         this.updateSearchDisplay_();
       } else {
         if (!isAncestor)
-          this.fire('selected-folder-changed', this.selectedId);
+          this.selectFolder_(this.selectedId);
 
         this._setDisplayedList(parentNode.children);
       }
     }.bind(this));
   },
 
   /**
    * Called when the title of a bookmark changes.
    * @param {string} id The id of changed bookmark node.
    * @param {!Object} changeInfo
@@ skipping 19 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)
+    if (/** @type {boolean} */ (e.detail.clearSearch))
       this.searchTerm = '';
 
-    // Deselect the old folder if defined.
-    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);
-    if (!this.idToNodeMap_[id] || this.idToNodeMap_[id].url)
-      id = this.rootNode.children[0].id;
-
-    var newFolder = this.idToNodeMap_[id];
-    this.set(newFolder.path + '.isSelectedFolder', true);
-    this.selectedId = id;
+    this.selectFolder_(/** @type {string} */ (e.detail.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.selectFolder_(folder.id);
   },
 
   /**
    * Selects items according to keyboard behaviours.
    * @param {CustomEvent} e
    * @private
    */
   onItemSelected_: function(e) {
     if (!e.detail.add)
       this.clearSelectedItems_();
@@ skipping 37 matching lines @@
     idToNodeMap[bookmarkNode.id] = bookmarkNode;
 
   if (bookmarkNode.url)
     return;
 
   bookmarkNode.isSelectedFolder = false;
   bookmarkNode.isOpen = true;
   for (var i = 0; i < bookmarkNode.children.length; i++)
     BookmarksStore.initNodes(bookmarkNode.children[i], idToNodeMap);
 };