Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(4118)

Unified Diff: chrome/browser/resources/md_bookmarks/store.js

Issue 2639453002: [MD Bookmarks] Add Select for Bookmarks. (Closed)
Patch Set: clean up Created 3 years, 11 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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 620a1231ec176ce28c6c269607e8b331be132c11..bcc4c0fcd2f38e7716c7257c5559ae612c600a01 100644
--- a/chrome/browser/resources/md_bookmarks/store.js
+++ b/chrome/browser/resources/md_bookmarks/store.js
@@ -37,7 +37,12 @@ var BookmarksStore = Polymer({
readOnly: true,
},
+ /** @type {Object<string, !BookmarkTreeNode>} */
idToNodeMap_: Object,
+
+ anchorIndex_: Number,
+
+ searchResultSet_: Object,
},
/** @private {Object} */
@@ -46,9 +51,10 @@ var BookmarksStore = Polymer({
/** @override */
attached: function() {
this.documentListeners_ = {
- 'selected-folder-changed': this.onSelectedFolderChanged_.bind(this),
'folder-open-changed': this.onFolderOpenChanged_.bind(this),
'search-term-changed': this.onSearchTermChanged_.bind(this),
+ 'select-item': this.onItemSelected_.bind(this),
+ 'selected-folder-changed': this.onSelectedFolderChanged_.bind(this),
};
for (var event in this.documentListeners_)
document.addEventListener(event, this.documentListeners_[event]);
@@ -73,8 +79,8 @@ var BookmarksStore = Polymer({
chrome.bookmarks.onChanged.addListener(this.onBookmarkChanged_.bind(this));
},
- //////////////////////////////////////////////////////////////////////////////
- // bookmarks-store, private:
+////////////////////////////////////////////////////////////////////////////////
+// bookmarks-store, private:
/**
* @param {BookmarkTreeNode} rootNode
@@ -97,7 +103,8 @@ var BookmarksStore = Polymer({
/** @private */
deselectFolders_: function() {
this.unlinkPaths('displayedList');
- this.set(this.idToNodeMap_[this.selectedId].path + '.isSelected', false);
+ this.set(
+ this.idToNodeMap_[this.selectedId].path + '.isSelectedFolder', false);
this.selectedId = null;
},
@@ -114,8 +121,15 @@ var BookmarksStore = Polymer({
return selectedNode.path.startsWith(folder.path);
},
- /** @private */
- updateSearchDisplay_: function() {
+ /**
+ * When being called as an observer, |searchObserver| is the search term. When
+ * being called elsewhere, |searchObserver| is undefined. This helps the
+ * function with doing observer specific actions and can be reused in other
+ * functions to update |displayedList|.
+ * @param {?string} searchObserver
+ * @private
+ */
+ updateSearchDisplay_: function(searchObserver) {
if (!this.rootNode)
return;
@@ -123,10 +137,18 @@ var BookmarksStore = Polymer({
this.fire('selected-folder-changed', this.rootNode.children[0].id);
} else {
chrome.bookmarks.search(this.searchTerm, function(results) {
+ if (searchObserver)
+ this.anchorIndex_ = undefined;
+ this.clearSelectedItems_();
+ this.searchResultSet_ = new Set();
+
if (this.selectedId)
this.deselectFolders_();
- this._setDisplayedList(results);
+ this.setupSearchResults_(results);
+
+ this.set(
+ 'displayedList.#' + this.anchorIndex_ + '.isSelectedItem', true);
}.bind(this));
}
},
@@ -137,6 +159,9 @@ var BookmarksStore = Polymer({
if (!this.selectedId)
return;
+ this.clearSelectedItems_();
+ this.anchorIndex_ = undefined;
+
var selectedNode = this.idToNodeMap_[this.selectedId];
this.linkPaths('displayedList', selectedNode.path + '.children');
this._setDisplayedList(selectedNode.children);
@@ -159,8 +184,93 @@ var BookmarksStore = Polymer({
delete this.idToNodeMap_[id];
},
- //////////////////////////////////////////////////////////////////////////////
- // bookmarks-store, bookmarks API event listeners:
+ /**
+ * 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
+ */
+ getIndexInList_: function(item) {
+ return this.searchTerm ? item.searchResultIndex : item.index;
+ },
+
+ /**
+ * @param {BookmarkTreeNode} item
+ * @return {boolean}
+ * @private
+ */
+ isInDisplayedList_: function(id) {
+ return this.searchTerm ? this.searchResultSet_.has(id) :
+ this.idToNodeMap_[id].parentId == this.selectedId;
+ },
+
+ /**
+ * Initializes the search results returned by the API as follows:
+ * - Populates |searchResultSet_| with a mapping of all result ids to
+ * their corresponding result.
+ * - Sets up the |searchResultIndex|.
+ * @param {Array<BookmarkTreeNode>} item
+ * @private
+ */
+ setupSearchResults_: function(results) {
+ for (var i = 0; i < results.length; i++) {
+ results[i].searchResultIndex = i;
+ results[i].isSelectedItem = false;
+ this.searchResultSet_.add(results[i].id);
+ }
+
+ this._setDisplayedList(results);
+ },
+
+ /**
+ * Select multiple items based on |anchorIndex_| and the selected
+ * item. If |anchorIndex_| is not set, single select the item.
+ * @param {BookmarkTreeNode} item
+ * @private
+ */
+ selectRange_: function(item) {
+ var startIndex, endIndex;
+ if (this.anchorIndex_ == undefined) {
+ this.anchorIndex_ = this.getIndexInList_(item);
+ startIndex = this.anchorIndex_;
+ endIndex = this.anchorIndex_;
+ } else {
+ var selectedIndex = this.getIndexInList_(item);
+ startIndex = Math.min(this.anchorIndex_, selectedIndex);
+ endIndex = Math.max(this.anchorIndex_, selectedIndex);
+ }
+ for (var i = startIndex; i <= endIndex; i++)
+ this.set('displayedList.#' + i + '.isSelectedItem', true);
+ },
+
+ /**
+ * Selects a single item in the displayedList.
+ * @param {BookmarkTreeNode} item
+ * @private
+ */
+ selectItem_: function(item) {
+ this.anchorIndex_ = this.getIndexInList_(item);
+ this.set('displayedList.#' + this.anchorIndex_ + '.isSelectedItem', true);
+ },
+
+////////////////////////////////////////////////////////////////////////////////
+// bookmarks-store, bookmarks API event listeners:
/**
* Callback for when a bookmark node is removed.
@@ -172,18 +282,44 @@ var BookmarksStore = Polymer({
* node: BookmarkTreeNode}} removeInfo
*/
onBookmarkRemoved_: function(id, removeInfo) {
- if (this.isAncestorOfSelected_(this.idToNodeMap_[id])) {
- 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_();
+ chrome.bookmarks.getSubTree(removeInfo.parentId, function(parentNodes) {
+ var parentNode = parentNodes[0];
+ var isAncestor = this.isAncestorOfSelected_(this.idToNodeMap_[id]);
+ var isInDisplayedList = this.isInDisplayedList_(id);
calamity 2017/01/31 05:05:15 Maybe call this 'wasInDisplayedList' to indicate t
jiaxi 2017/02/01 03:19:25 Done.
tsergeant 2017/02/02 00:16:02 It makes sense to call the var `wasInDisplayedList
jiaxi 2017/02/02 04:11:55 Done.
+
+ // Polymer doesn't update the indexes after a deletion. Manually updates
+ // the indexes.
calamity 2017/01/31 05:05:15 What does indexes mean? How about 'Refresh the par
jiaxi 2017/02/01 03:19:25 Done.
+ 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);
+
+ // Only update the displayedList if the removed node is in the
+ // displayedList.
+ if (!isInDisplayedList)
+ return;
+
+ if (this.anchorIndex_ == this.displayedList.length - 1)
+ this.anchorIndex_--;
calamity 2017/01/31 05:05:15 I'm pretty happy for everything to just be deselec
jiaxi 2017/02/01 03:19:25 Done.
+
+ if (this.searchTerm) {
calamity 2017/01/31 05:05:15 Comment above here: // Update the currently displa
jiaxi 2017/02/01 03:19:25 Done.
+ this.updateSearchDisplay_();
+ } else {
+ if (!isAncestor)
+ this.fire('selected-folder-changed', this.selectedId);
+
+ this._setDisplayedList(parentNode.children);
+
+ this.set(
+ 'displayedList.#' + this.anchorIndex_ + '.isSelectedItem', true);
+ }
+ }.bind(this));
},
/**
@@ -201,8 +337,8 @@ var BookmarksStore = Polymer({
this.updateSearchDisplay_();
},
- //////////////////////////////////////////////////////////////////////////////
- // bookmarks-store, bookmarks app event listeners:
+////////////////////////////////////////////////////////////////////////////////
+// bookmarks-store, bookmarks app event listeners:
/**
* @param {Event} e
@@ -223,8 +359,9 @@ var BookmarksStore = Polymer({
this.searchTerm = '';
// Deselect the old folder if defined.
- if (this.selectedId)
- this.set(this.idToNodeMap_[this.selectedId].path + '.isSelected', false);
+ 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);
@@ -232,7 +369,7 @@ var BookmarksStore = Polymer({
id = this.rootNode.children[0].id;
var newFolder = this.idToNodeMap_[id];
- this.set(newFolder.path + '.isSelected', true);
+ this.set(newFolder.path + '.isSelectedFolder', true);
this.selectedId = id;
},
@@ -247,6 +384,21 @@ var BookmarksStore = Polymer({
if (!folder.isOpen && this.isAncestorOfSelected_(folder))
this.fire('selected-folder-changed', folder.id);
},
+
+ /**
+ * Selects items according to keyboard behaviours.
+ * @param {CustomEvent} e
+ * @private
+ */
+ onItemSelected_: function(e) {
+ if (!e.detail.add)
+ this.clearSelectedItems_();
+
+ if (e.detail.range)
+ this.selectRange_(e.detail.item);
+ else
+ this.selectItem_(e.detail.item);
+ },
});
////////////////////////////////////////////////////////////////////////////////
@@ -276,13 +428,14 @@ BookmarksStore.generatePaths = function(bookmarkNode, startIndex) {
* @param {Object=} idToNodeMap
*/
BookmarksStore.initNodes = function(bookmarkNode, idToNodeMap) {
+ bookmarkNode.isSelectedItem = false;
if (idToNodeMap)
idToNodeMap[bookmarkNode.id] = bookmarkNode;
if (bookmarkNode.url)
return;
- bookmarkNode.isSelected = false;
+ bookmarkNode.isSelectedFolder = false;
bookmarkNode.isOpen = true;
for (var i = 0; i < bookmarkNode.children.length; i++)
BookmarksStore.initNodes(bookmarkNode.children[i], idToNodeMap);

Powered by Google App Engine
This is Rietveld 408576698