[MD Bookmarks] Add search.

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 bdbcb2ebd10de8400a8af2ac44628d861266aede..3ef4521e917cc270a2673f9ee0016c925b4b7558 100644
--- a/chrome/browser/resources/md_bookmarks/store.js
+++ b/chrome/browser/resources/md_bookmarks/store.js
@@ -14,13 +14,19 @@ var BookmarksStore = Polymer({
 
     selectedId: {
       type: String,
-      observer: 'updateSelectedNode_',
+      observer: 'updateSelectedDisplay_',
+      notify: true,
+    },
+
+    searchTerm: {
+      type: String,
+      observer: 'updateSearchDisplay_',
       notify: true,
     },
 
     /** @type {BookmarkTreeNode} */

[tsergeant, 2017/01/12 06:45:06]

Might be good to leave a comment here explaining what this does now: It's no
longer super straightforward.

[angelayang, 2017/01/12 07:51:24]

Done.

-    selectedNode: {
-      type: Object,
+    displayedList: {
+      type: Array,
       notify: true,
       readOnly: true,
     },
@@ -36,6 +42,7 @@ var BookmarksStore = Polymer({
     this.documentListeners_ = {
       'selected-folder-changed': this.onSelectedFolderChanged_.bind(this),
       'folder-open-changed': this.onFolderOpenChanged_.bind(this),
+      'search-term-changed': this.onSearchTermChanged_.bind(this),
     };
     for (var event in this.documentListeners_)
       document.addEventListener(event, this.documentListeners_[event]);
@@ -76,20 +83,48 @@ var BookmarksStore = Polymer({
     this.fire('selected-folder-changed', this.rootNode.children[0].id);
   },
 
+  /** @private */
+  deselectFolders_: function() {

[tsergeant, 2017/01/12 06:45:06]

You should probably do an unlinkPaths that's the equivalent of the linkPaths in
updateSelectedDisplay(). This stops Polymer from thinking that displayedList
connects to anything in the tree.

[angelayang, 2017/01/12 07:51:24]

Done.

+    this.set(this.idToNodeMap_[this.selectedId].path + '.isSelected', false);
+    this.selectedId = '';

[angelayang, 2017/01/12 06:08:06]

Closure doesn't like it when i set this to null, so i've set it to '' instead.

[tsergeant, 2017/01/12 06:45:06]

What specifically doesn't it like? I think using null is probably better, so we
can try and fix any problems with Closure to allow it.

[angelayang, 2017/01/12 07:51:24]

Closure says it should be of type string. So i'll put it to null and we can fix
the closure issue after yep

+  },
+
   /**
    * @param {BookmarkTreeNode} folder
    * @private
    * @return {boolean}
    */
   isAncestorOfSelected_: function(folder) {
-    return this.selectedNode.path.startsWith(folder.path);
+    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) {
+        if (this.selectedId)
+          this.deselectFolders_();
+
+        this._setDisplayedList(results);
+      }.bind(this));
+    }
   },
 
   /** @private */
-  updateSelectedNode_: function() {
+  updateSelectedDisplay_: function() {
+    // Don't do change to selected display if ID was cleared.
+    if (!this.selectedId)
+      return;
+
     var selectedNode = this.idToNodeMap_[this.selectedId];
-    this.linkPaths('selectedNode', selectedNode.path);
-    this._setSelectedNode(selectedNode);
+    this.linkPaths('displayedList', selectedNode.path + '.children');
+    this._setDisplayedList(selectedNode.children);
   },
 
   /**
@@ -122,13 +157,47 @@ var BookmarksStore = Polymer({
    *           node: BookmarkTreeNode}} removeInfo
    */
   onBookmarkRemoved_: function(id, removeInfo) {
-    if (this.isAncestorOfSelected_(this.idToNodeMap_[id]))
+    if (this.isAncestorOfSelected_(this.idToNodeMap_[id])) {
+      // TODO (angelayang): does this ever happen?

[angelayang, 2017/01/12 06:08:06]

Sorry i'll delete this comment. Just checking whether i can remove this piece of
logic.  i think it's no longer possible to remove the selectedNode unless you're
in search. In which case we don't need to update the selectedId. 

[tsergeant, 2017/01/12 06:45:06]

Try:
- Create 'folder'
- Create 'subfolder'
- Select 'subfolder' in MD Bookmarks 
- Delete 'folder' from bookmarks bar

Then the selected item no longer exists and this line selects the parent of
'folder'

It's important to remember that modifications we see here could be coming from
anywhere: the bookmarks bar, the bookmark manager open in another tab, or even
synced from another device.

[angelayang, 2017/01/12 07:51:24]

Cool thanks! I'll definitely keep that in mind! I forgot about the bookmarks bar
etc.

       this.fire('selected-folder-changed', 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_();
+  },
+
+  /**
+   * 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
+   * @private
+   */
+  initNodes_: function(bookmarkNode) {

[tsergeant, 2017/01/12 06:45:06]

This function got moved by Charlotte: It now exists below as
BookmarksStore.initNodes(). You should delete it again.

[angelayang, 2017/01/12 07:51:24]

oh thanks i didn't spot that

+    this.idToNodeMap_[bookmarkNode.id] = bookmarkNode;
+    if (bookmarkNode.url)
+      return;
+
+    bookmarkNode.isSelected = false;
+    bookmarkNode.isOpen = true;
+    for (var i = 0; i < bookmarkNode.children.length; i++) {
+      this.initNodes_(bookmarkNode.children[i]);
+    }
+  },
+
+  /**
+   * @param {Event} e
+   * @private
+   */
+  onSearchTermChanged_: function(e) {

[tsergeant, 2017/01/12 06:45:06]

Also, Charlotte's patch added sections to store.js to try and keep everything in
order. This new function should go under "bookmarks app event listeners"

[angelayang, 2017/01/12 07:51:24]

Done.

+    this.searchTerm = /** @type {string} */ (e.detail);
   },
 
   /**
@@ -153,6 +222,9 @@ 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);