Add delete context menu for custom wallpapers and some ui tweaks

chrome/browser/resources/chromeos/wallpaper_manager/js/wallpaper_manager.js

Index: chrome/browser/resources/chromeos/wallpaper_manager/js/wallpaper_manager.js
diff --git a/chrome/browser/resources/chromeos/wallpaper_manager/js/wallpaper_manager.js b/chrome/browser/resources/chromeos/wallpaper_manager/js/wallpaper_manager.js
index 76c3a21c1e49422fa017b40f032293b270ad5ae4..6e1b25019d57b5c6fbf78b50abaa26ad4e4f022b 100644
--- a/chrome/browser/resources/chromeos/wallpaper_manager/js/wallpaper_manager.js
+++ b/chrome/browser/resources/chromeos/wallpaper_manager/js/wallpaper_manager.js
@@ -252,6 +252,69 @@ function WallpaperManager(dialogDom) {
     });
 
     this.onResize_();
+    this.initContextMenuAndCommand_();
+  };
+
+  /**
+   * One-time initialization of context menu and command.
+   */
+  WallpaperManager.prototype.initContextMenuAndCommand_ = function() {
+    this.wallpaperContextMenu_ = $('wallpaper-context-menu');
+    cr.ui.Menu.decorate(this.wallpaperContextMenu_);
+    cr.ui.contextMenuHandler.setContextMenu(this.wallpaperGrid_,
+                                            this.wallpaperContextMenu_);
+    var commands = this.dialogDom_.querySelectorAll('command');
+    for (var i = 0; i < commands.length; i++)
+      cr.ui.Command.decorate(commands[i]);
+
+    var doc = this.document_;
+    doc.addEventListener('command', this.onCommand_.bind(this));
+    doc.addEventListener('canExecute', this.onCommandCanExecute_.bind(this));
+  };
+
+  /**
+   * Handles a command being executed.
+   * @param {Event} event A command event.
+   */
+  WallpaperManager.prototype.onCommand_ = function(event) {
+    if (event.command.id == 'delete') {
+      var wallpaperGrid = this.wallpaperGrid_;
+      var selectedIndex = wallpaperGrid.selectionModel.selectedIndex;
+      var item = wallpaperGrid.dataModel.item(selectedIndex);
+      if (!item || item.source != wallpapers.WallpaperSourceEnum.Custom)
+        return;
+      this.removeCustomWallpaper(item.baseURL);
+      wallpaperGrid.dataModel.splice(selectedIndex, 1);
+      // The number of remaining custom wallpapers. The add new button in data

[flackr, 2013/03/21 21:51:19]

s/The number of/Calculate the number of

The second sentence is slightly confusing otherwise.

[bshe, 2013/03/22 15:11:57]

Done.

+      // model needs to be excluded.
+      var number = wallpaperGrid.dataModel.length - 1;

[flackr, 2013/03/21 21:51:19]

s/number/customWallpaperCount

[bshe, 2013/03/22 15:11:57]

Done.

+      if (number == 0) {
+        // Active custom wallpaper is also copied in chronos data dir. It needs
+        // to be deleted.
+        chrome.wallpaperPrivate.resetWallpaper();
+      } else {
+        selectedIndex = Math.min(selectedIndex, number - 1);
+        wallpaperGrid.selectionModel.selectedIndex = selectedIndex;
+      }
+      event.cancelBubble = true;
+    }
+  };
+
+  /**
+   * Decides if a command can be executed on current target.
+   * @param {Event} event A command event.
+   */
+  WallpaperManager.prototype.onCommandCanExecute_ = function(event) {
+    if (event.command.id == 'delete') {
+      var wallpaperGrid = this.wallpaperGrid_;
+      var selectedIndex = wallpaperGrid.selectionModel.selectedIndex;
+      if (selectedIndex != -1 &&
+          selectedIndex != this.wallpaperGrid_.dataModel.length - 1) {
+        event.canExecute = true;
+        return;
+      }

[flackr, 2013/03/21 21:51:19]

No check for if this is a custom wallpaper or not?

[bshe, 2013/03/22 15:11:57]

Good call. I have hide the context menu for online thumbnails. But it is even
safer to disable the command.
Done.

On 2013/03/21 21:51:19, flackr wrote:

+    }
+    event.canExecute = false;

[flackr, 2013/03/21 21:51:19]

Nit: Usually the pattern followed is to have multiple if lines that return if
conditions aren't met rather than nesting if's. i.e.
event.canExecute = false;
if (event.command.id != 'delete')
  return;

var selectedIndex = ...
if (selectedIndex == -1 || selectedIndex ==this.wallpaperGrid_.dataModel.length
-1)
  return;

event.canExecute = true;

[bshe, 2013/03/22 15:11:57]

I was thinking to use this function if we ever want to add more commands to
context menu. The above pattern may not best fit if we want to do that, although
I am not sure if we would ever do it. For extensibility, maybe use a switch
cause a better fit here? But I am fine to change to the suggest pattern if you
insist.


On 2013/03/21 21:51:19, flackr wrote:

   };
 
   /**
@@ -428,7 +491,6 @@ function WallpaperManager(dialogDom) {
     var item = this.wallpaperGrid_.dataModel.item(oldestIndex);
     if (!item || item.source != wallpapers.WallpaperSourceEnum.Custom)
       return;
-    console.error(item.baseURL);
     if (item.baseURL == this.currentWallpaper_)
       item = this.wallpaperGrid_.dataModel.item(--oldestIndex);
     if (item) {
@@ -782,7 +844,7 @@ function WallpaperManager(dialogDom) {
     var wallpapersDataModel = new cr.ui.ArrayDataModel([]);
     var selectedItem;
     if (selectedListItem.custom) {
-      $('online-wallpaper-attribute').hidden = true;
+      this.document_.body.setAttribute('custom', '');
       var errorHandler = this.onFileSystemError_.bind(this);
       var toArray = function(list) {
         return Array.prototype.slice.call(list || [], 0);
@@ -839,7 +901,7 @@ function WallpaperManager(dialogDom) {
       this.wallpaperDirs_.getDirectory(WallpaperDirNameEnum.ORIGINAL,
                                        success, errorHandler);
     } else {
-      $('online-wallpaper-attribute').hidden = false;
+      this.document_.body.removeAttribute('custom');
       for (var key in this.manifest_.wallpaper_list) {
         if (selectedIndex == AllCategoryIndex ||
             this.manifest_.wallpaper_list[key].categories.indexOf(