Files.app: Add subfolders in the left nav

chrome/browser/resources/file_manager/js/sidebar.js

+// Copyright (c) 2012 The Chromium Authors. All rights reserved.

[mtomasz, 2013/03/14 02:52:05]

2012 -> 2013

[yoshiki, 2013/03/14 07:57:53]

Done.

+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+/**
+ * Update sub-elements of {@code parentElement} reading {@code DirectoryEntry}
+ * with calling {@code iterator}.
+ *
+ * @param {DirectoryItem|DirectoryTree} parentElement Parent element of newly
+ *     created items.
+ * @param {function(number): DirectoryEntry} iterator Function which returns
+ *     the n-th Entry in the directory.
+ * @param {DirectoryModel} directoryModel Current DirectoryModel.
+ */
+function updateSubElementsFromList(parentElement, iterator, directoryModel) {
+  var index = 0;
+  while (iterator(index)) {
+    var currentEntry = iterator(index);
+    var currentElement = parentElement.items[index];
+
+    if (index >= parentElement.items.length) {
+      var item = new DirectoryItem(currentEntry, parentElement, directoryModel);
+      parentElement.add(item);
+      index++;
+    } else if (currentEntry.fullPath === currentElement.fullPath) {
+      index++;
+    } else if (currentEntry.fullPath < currentElement.fullPath) {
+      var item = new DirectoryItem(currentEntry, parentElement, directoryModel);
+      parentElement.addAt(item, index);
+      index++;
+    } else if (currentEntry.fullPath > currentElement.fullPath) {
+      parentElement.remove(currentElement);
+    }
+  }
+
+  var removedChild;
+  while (removedChild = parentElement.items[index]) {
+    parentElement.remove(removedChild);
+  }
+
+  if (index === 0) {
+    parentElement.hasChildren = false;
+    parentElement.expanded = false;
+  } else {
+    parentElement.hasChildren = true;
+  }
+}
+
+/**
+ * @param {DirectoryEntry} parentDirEntry DirectoryEntry of this item.
+ * @param {DirectoryItem|DirectoryTree} parentDirItem Parent of this item.
+ * @param {DirectoryModel} directoryModel Current DirectoryModel.
+ * @extends {cr.ui.TreeItem}
+ * @constructor
+ */
+function DirectoryItem(parentDirEntry, parentDirItem, directoryModel) {
+  var item = cr.doc.createElement('div');
+  DirectoryItem.decorate(item, parentDirEntry, parentDirItem, directoryModel);
+  return item;
+}
+
+/**
+ * @param {HTMLElement} el Element to be DirectoryItem.
+ * @param {DirectoryEntry} parentDirEntry DirectoryEntry of this item.
+ * @param {DirectoryItem|DirectoryTree} parentDirItem Parent of this item.
+ * @param {DirectoryModel} directoryModel Current DirectoryModel.
+ */

[mtomasz, 2013/03/14 02:52:05]

Can we move to Class.prototype.method = function() style instead? We should
choose one style.

[yoshiki, 2013/03/14 07:57:53]

Done.

+DirectoryItem.decorate =
+    function(el, parentDirEntry, parentDirItem, directoryModel) {
+  el.__proto__ = DirectoryItem.prototype;
+  (/** @type {DirectoryItem} */ el).decorate(
+      parentDirEntry, parentDirItem, directoryModel);
+};
+
+DirectoryItem.prototype = {
+  __proto__: cr.ui.TreeItem.prototype,
+
+  /**
+   * @param {DirectoryEntry} parentDirEntry DirectoryEntry of this item.
+   * @param {DirectoryItem|DirectoryTree} parentDirItem Parent of this item.
+   * @param {DirectoryModel} directoryModel Current DirectoryModel.
+   */
+  decorate: function(parentDirEntry, parentDirItem, directoryModel) {
+    var path = parentDirEntry.fullPath;
+    var label = PathUtil.isRootPath(path) ?
+                    PathUtil.getRootLabel(path) : parentDirEntry.name;
+
+    this.className = 'tree-item';
+    this.innerHTML =
+          '<div class=tree-row>' +
+          '<span class=expand-icon></span>' +
+          '<span class=icon></span>' +
+          '<span class=label></span>' +
+          '<div class=root-eject></div>' +
+          '</div>' +
+          '<div class=tree-children></div>';
+    this.setAttribute('role', 'treeitem');
+
+    this.directoryModel_ = directoryModel;
+    this.parent_ = parentDirItem;
+    this.label = label;
+    this.fullPath = path;
+
+    // Sets hasChildren=true tentatively. This will be overridden after
+    // scanning sub-directories in updateSubElementsFromList.
+    this.hasChildren = true;
+
+    this.addEventListener('expand', this.onExpand_.bind(this), true);
+
+    var icon = this.querySelector('.icon');
+    if (PathUtil.isRootPath(path)) {
+      var iconType = PathUtil.getRootType(path);
+      if (iconType === RootType.REMOVABLE)
+        iconType = this.volumeManager_.getDeviceType(path);
+
+      icon.classList.add('volume-icon');
+      icon.setAttribute('volume-type-icon', iconType);
+    }
+
+    var eject = this.querySelector('.root-eject');
+    eject.hidden = !PathUtil.isUnmountableByUser(path);
+    eject.addEventListener('click',
+        function(event) {
+          event.stopPropagation();

[mtomasz, 2013/03/14 02:52:05]

1. Is stopPropagation() necessary here?
2. Is checking if its unmountable again necessary?
3. Umounting seem not to be working on my linux box. Is it ok?

[yoshiki, 2013/03/14 07:57:53]

Yes, it prevents selecting the item when user trys to unmount.
I'm not sure but I added it just in case.
It's working on a real device.

[yoshiki, 2013/03/14 09:05:48]

I found some bugs on removable device, but it is fixed on Patch Set #2.

+          if (!PathUtil.isUnmountableByUser(path))
+            return;
+
+          var volumeManager = VolumeManager.getInstance();
+          volumeManager.unmount(path, function() {}, function() {});
+        }.bind(this));
+
+    if ('expanded' in parentDirItem || parentDirItem.expanded)
+      this.updateSubDirectoriesWithEntry_(parentDirEntry);
+  },
+
+  /**
+   * The element containing the label text and the icon.
+   * @type {!HTMLElement}
+   * @override
+   **/
+  get labelElement() {
+      return this.firstElementChild.querySelector('.label');

[mtomasz, 2013/03/14 02:52:05]

Indentation is off?

[yoshiki, 2013/03/14 07:57:53]

Done.

+  },
+
+  /**
+   * Invoked when the item is being expended.

[mtomasz, 2013/03/14 02:52:05]

expended -> expanded

[yoshiki, 2013/03/14 07:57:53]

Done.

+   * @param {!UIEvent} e Event.
+   * @private
+   **/
+  onExpand_: function(e) {
+    this.updateSubDirectories(function() {
+      this.expanded = false;
+    });

[mtomasz, 2013/03/14 02:52:05]

Add .bind(this)? UpdateSubDirectories calls resolveDirectory, etc which may
change the this context I think.

[yoshiki, 2013/03/14 07:57:53]

Done.

+
+    e.stopPropagation();

[mtomasz, 2013/03/14 02:52:05]

Is this necessary?

[yoshiki, 2013/03/14 07:57:53]

It prevents selecting the item when clicking expand mark.

+  },
+
+  /**

[mtomasz, 2013/03/14 02:52:05]

Please add a short description.

[yoshiki, 2013/03/14 07:57:53]

Done.

+   * @param {function()=} opt_errorCallback Callback called on error.
+   */
+  updateSubDirectories: function(opt_errorCallback) {
+    this.directoryModel_.resolveDirectory(
+        this.fullPath,
+        function(entry) {
+          this.updateSubDirectoriesWithEntry_(entry, opt_errorCallback);
+        },

[mtomasz, 2013/03/14 02:52:05]

.bind(this) missing. Causes js errors.

[yoshiki, 2013/03/14 07:57:53]

Done.

+        opt_errorCallback);
+  },
+
+  /**
+   * @param {!DirectoryEntry} dirEntry DirectoryEntry to read from.
+   * @param {function()=} opt_errorCallback Callback called on error.
+   * @private
+   */
+  updateSubDirectoriesWithEntry_: function(dirEntry, opt_errorCallback) {
+    // Skips if the entry is dummy.
+    if (!dirEntry.createReader) {

[mtomasz, 2013/03/14 02:52:05]

nit: How about instanceof? Sounds cleaner and less tricky.

[yoshiki, 2013/03/14 07:57:53]

Done.

+      if (opt_errorCallback)
+        opt_errorCallback();
+      return;
+    }
+
+    var reader = dirEntry.createReader();
+    var entries = [];
+
+    var readEntry = function() {
+      reader.readEntries(function(results) {
+        if (results.length === 0) {
+          this.entries_ = entries.sort();
+          this.redrawSubDirectoryList_();
+          return;
+        }
+
+        for (var i = 0; i < results.length; i++) {
+          var entry = results[i];
+          if (entry.isDirectory)
+            entries.push(entry);
+        }
+        readEntry();
+      }.bind(this));
+    }.bind(this);
+    readEntry();
+  },
+
+  /**
+   * @private
+   */
+  redrawSubDirectoryList_: function() {
+    var entries = this.entries_;
+    updateSubElementsFromList(this,
+                              function(i) { return entries[i]; },
+                              this.directoryModel_);
+  }
+};
+
+/**

[mtomasz, 2013/03/14 02:52:05]

I think we should have description for the classes, even if it is quite obvious.

[yoshiki, 2013/03/14 07:57:53]

Done.

+ * @constructor
+ * @extends {cr.ui.Tree}
+ */
+function DirectoryTree() {}
+
+/**
+ * Decorate element.

[mtomasz, 2013/03/14 02:52:05]

nit: I think we should consistently use passive form. Decorate -> decorates.

[yoshiki, 2013/03/14 07:57:53]

Done.

+ * @param {HTMLElement} el Element to be DirectoryTree.
+ * @param {DirectoryModel} directoryModel Current DirectoryModel.
+ */
+DirectoryTree.decorate = function(el, directoryModel) {
+  el.__proto__ = DirectoryTree.prototype;
+  (/** @type {DirectoryTree} */ el).decorate(directoryModel);

[mtomasz, 2013/03/14 02:52:05]

I think this virtual cast is unnecessary, since el.__proto__ already changed the
class to DirectoryTree.

[yoshiki, 2013/03/14 07:57:53]

This does nothing on JavaScript interpreter. It's just teaching a cast here to
human and JSdoc.

On 2013/03/14 02:52:05, mtomasz wrote:

+};
+
+DirectoryTree.prototype = {
+  __proto__: cr.ui.Tree.prototype,
+
+  /**
+   * Decorate element.
+   * @param {DirectoryModel} directoryModel Current DirectoryModel.
+   */
+  decorate: function(directoryModel) {
+    cr.ui.Tree.prototype.decorate.call(this);
+
+    this.directoryModel_ = directoryModel;
+
+    this.rootsList_ = this.directoryModel_.getRootsList();
+    this.rootsList_.addEventListener('change',
+                                     this.onRootsListChanged_.bind(this));
+    this.rootsList_.addEventListener('permuted',
+                                     this.onRootsListChanged_.bind(this));
+    this.onRootsListChanged_();
+  },
+
+  /**
+   * @param {cr.ui.Menu} menu Context menu.
+   */
+  setContextMenu: function(menu) {
+    this.contextMenu_ = menu;
+
+    for (var i = 0; i < this.rootsList_.length; i++) {
+      var item = this.rootsList_.item(i);
+      var type = PathUtil.getRootType(item.fullPath);
+      // Context menu is set only to archive and removable volumes.
+      if (type === RootType.ARCHIVE || type === RootType.REMOVABLE) {
+        cr.ui.contextMenuHandler.setContextMenu(this.items[i].rowElement,
+                                                this.contextMenu_);
+      }
+    }
+  },
+
+  /**
+   * Invoked when the root list is changed.
+   * @private
+   */
+  onRootsListChanged_: function() {
+    var rootsList = this.rootsList_;
+    updateSubElementsFromList(this,
+                              rootsList.item.bind(rootsList),
+                              this.directoryModel_);
+    this.setContextMenu(this.contextMenu_);
+  },
+
+  /**
+   * Returns the path of the current selected item.
+   * @return {string} The current path.
+   */
+  getCurrentPath: function() {
+    return this.selectedItem ? this.selectedItem.fullPath : null;
+  }
+};