From 2c75daecb1b568999b9505cc9563f61025ae5494 Mon Sep 17 00:00:00 2001
From: jdescottes <julian.descottes@gmail.com>
Date: Sat, 10 Oct 2015 12:59:02 +0200
Subject: [PATCH] Issue #287 : Move shortcut definition to tool instances

---
 src/js/controller/ToolController.js           | 45 +++++++++----------
 .../controller/TransformationsController.js   | 17 +++----
 .../controller/dialogs/DialogsController.js   |  1 +
 src/js/service/HistoryService.js              |  3 +-
 src/js/service/keyboard/CheatsheetService.js  |  2 +-
 src/js/tools/drawing/Circle.js                |  1 +
 src/js/tools/drawing/ColorPicker.js           |  1 +
 src/js/tools/drawing/ColorSwap.js             |  2 +-
 src/js/tools/drawing/DitheringTool.js         |  1 +
 src/js/tools/drawing/Eraser.js                |  2 +
 src/js/tools/drawing/Lighten.js               |  3 +-
 src/js/tools/drawing/Move.js                  |  5 +++
 src/js/tools/drawing/PaintBucket.js           |  1 +
 src/js/tools/drawing/Rectangle.js             |  1 +
 src/js/tools/drawing/SimplePen.js             |  1 +
 src/js/tools/drawing/Stroke.js                |  1 +
 src/js/tools/drawing/VerticalMirrorPen.js     |  1 +
 .../drawing/selection/AbstractDragSelect.js   |  1 +
 src/js/tools/drawing/selection/LassoSelect.js |  5 ++-
 .../drawing/selection/RectangleSelect.js      |  4 +-
 src/js/tools/drawing/selection/ShapeSelect.js |  8 ++--
 .../tools/transform/AbstractTransformTool.js  |  2 +-
 22 files changed, 60 insertions(+), 48 deletions(-)

diff --git a/src/js/controller/ToolController.js b/src/js/controller/ToolController.js
index 6de16235..31f596a8 100644
--- a/src/js/controller/ToolController.js
+++ b/src/js/controller/ToolController.js
@@ -2,26 +2,23 @@
   var ns = $.namespace('pskl.controller');
 
   ns.ToolController = function () {
-    var toDescriptor = function (id, shortcut, instance) {
-      return {id:id, shortcut:shortcut, instance:instance};
-    };
 
     this.tools = [
-      toDescriptor('simplePen', 'P', new pskl.tools.drawing.SimplePen()),
-      toDescriptor('verticalMirrorPen', 'V', new pskl.tools.drawing.VerticalMirrorPen()),
-      toDescriptor('paintBucket', 'B', new pskl.tools.drawing.PaintBucket()),
-      toDescriptor('colorSwap', 'A', new pskl.tools.drawing.ColorSwap()),
-      toDescriptor('eraser', 'E', new pskl.tools.drawing.Eraser()),
-      toDescriptor('stroke', 'L', new pskl.tools.drawing.Stroke()),
-      toDescriptor('rectangle', 'R', new pskl.tools.drawing.Rectangle()),
-      toDescriptor('circle', 'C', new pskl.tools.drawing.Circle()),
-      toDescriptor('move', 'M', new pskl.tools.drawing.Move()),
-      toDescriptor('shapeSelect', 'Z', new pskl.tools.drawing.selection.ShapeSelect()),
-      toDescriptor('rectangleSelect', 'S', new pskl.tools.drawing.selection.RectangleSelect()),
-      toDescriptor('lassoSelect', 'H', new pskl.tools.drawing.selection.LassoSelect()),
-      toDescriptor('lighten', 'U', new pskl.tools.drawing.Lighten()),
-      toDescriptor('dithering', 'T', new pskl.tools.drawing.DitheringTool()),
-      toDescriptor('colorPicker', 'O', new pskl.tools.drawing.ColorPicker())
+      new pskl.tools.drawing.SimplePen(),
+      new pskl.tools.drawing.VerticalMirrorPen(),
+      new pskl.tools.drawing.PaintBucket(),
+      new pskl.tools.drawing.ColorSwap(),
+      new pskl.tools.drawing.Eraser(),
+      new pskl.tools.drawing.Stroke(),
+      new pskl.tools.drawing.Rectangle(),
+      new pskl.tools.drawing.Circle(),
+      new pskl.tools.drawing.Move(),
+      new pskl.tools.drawing.selection.ShapeSelect(),
+      new pskl.tools.drawing.selection.RectangleSelect(),
+      new pskl.tools.drawing.selection.LassoSelect(),
+      new pskl.tools.drawing.Lighten(),
+      new pskl.tools.drawing.DitheringTool(),
+      new pskl.tools.drawing.ColorPicker()
     ];
 
     this.toolIconRenderer = new pskl.tools.IconMarkupRenderer();
@@ -53,8 +50,8 @@
       stage.removeClass(previousSelectedToolClass);
       stage.removeClass(pskl.tools.drawing.Move.TOOL_ID);
     }
-    stage.addClass(tool.instance.toolId);
-    stage.data('selected-tool-class', tool.instance.toolId);
+    stage.addClass(tool.toolId);
+    stage.data('selected-tool-class', tool.toolId);
   };
 
   ns.ToolController.prototype.onSelectToolEvent_ = function(event, toolId) {
@@ -72,12 +69,12 @@
     this.activateToolOnStage_(this.currentSelectedTool);
 
     var selectedToolElement = $('#tool-section .tool-icon.selected');
-    var toolElement = $('[data-tool-id=' + tool.instance.toolId + ']');
+    var toolElement = $('[data-tool-id=' + tool.toolId + ']');
 
     selectedToolElement.removeClass('selected');
     toolElement.addClass('selected');
 
-    $.publish(Events.TOOL_SELECTED, [tool.instance]);
+    $.publish(Events.TOOL_SELECTED, [tool]);
   };
 
   /**
@@ -107,7 +104,7 @@
 
   ns.ToolController.prototype.getToolById_ = function (toolId) {
     return pskl.utils.Array.find(this.tools, function (tool) {
-      return tool.instance.toolId == toolId;
+      return tool.toolId == toolId;
     });
   };
 
@@ -118,7 +115,7 @@
     var html = '';
     for (var i = 0 ; i < this.tools.length ; i++) {
       var tool = this.tools[i];
-      html += this.toolIconRenderer.render(tool.instance, tool.shortcut);
+      html += this.toolIconRenderer.render(tool, tool.shortcut);
     }
     $('#tools-container').html(html);
   };
diff --git a/src/js/controller/TransformationsController.js b/src/js/controller/TransformationsController.js
index 56c6f1fd..bee10a1c 100644
--- a/src/js/controller/TransformationsController.js
+++ b/src/js/controller/TransformationsController.js
@@ -2,15 +2,10 @@
   var ns = $.namespace('pskl.controller');
 
   ns.TransformationsController = function () {
-
-    var toDescriptor = function (id, shortcut, instance) {
-      return {id:id, shortcut:shortcut, instance:instance};
-    };
-
     this.tools = [
-      toDescriptor('flip', '', new pskl.tools.transform.Flip()),
-      toDescriptor('rotate', '', new pskl.tools.transform.Rotate()),
-      toDescriptor('clone', '', new pskl.tools.transform.Clone())
+      new pskl.tools.transform.Flip(),
+      new pskl.tools.transform.Rotate(),
+      new pskl.tools.transform.Clone()
     ];
 
     this.toolIconRenderer = new pskl.tools.IconMarkupRenderer();
@@ -25,9 +20,9 @@
 
   ns.TransformationsController.prototype.applyTool = function (toolId, evt) {
     this.tools.forEach(function (tool) {
-      if (tool.instance.toolId === toolId) {
+      if (tool.toolId === toolId) {
         $.publish(Events.TRANSFORMATION_EVENT, [toolId, evt]);
-        tool.instance.apply(evt);
+        tool.applyTransformation(evt);
       }
     }.bind(this));
   };
@@ -39,7 +34,7 @@
 
   ns.TransformationsController.prototype.createToolsDom_ = function() {
     var html = this.tools.reduce(function (p, tool) {
-      return p + this.toolIconRenderer.render(tool.instance, tool.shortcut, 'left');
+      return p + this.toolIconRenderer.render(tool, tool.shortcut, 'left');
     }.bind(this), '');
     this.toolsContainer.innerHTML = html;
   };
diff --git a/src/js/controller/dialogs/DialogsController.js b/src/js/controller/dialogs/DialogsController.js
index 6ab23f67..a3924bf5 100644
--- a/src/js/controller/dialogs/DialogsController.js
+++ b/src/js/controller/dialogs/DialogsController.js
@@ -27,6 +27,7 @@
     $.subscribe(Events.DIALOG_DISPLAY, this.onDialogDisplayEvent_.bind(this));
     $.subscribe(Events.DIALOG_HIDE, this.onDialogHideEvent_.bind(this));
 
+    // TODO : JD : should be moved to a main controller
     pskl.app.shortcutService.registerShortcut('alt+P', this.onDialogDisplayEvent_.bind(this, null, 'create-palette'));
 
     this.dialogWrapper_.classList.add('animated');
diff --git a/src/js/service/HistoryService.js b/src/js/service/HistoryService.js
index 4fd09a09..653f0f26 100644
--- a/src/js/service/HistoryService.js
+++ b/src/js/service/HistoryService.js
@@ -27,8 +27,7 @@
     $.subscribe(Events.PISKEL_SAVE_STATE, this.onSaveStateEvent.bind(this));
 
     this.shortcutService.registerShortcut('ctrl+Z', this.undo.bind(this));
-    this.shortcutService.registerShortcut('ctrl+Y', this.redo.bind(this));
-    this.shortcutService.registerShortcut('ctrl+shift+Z', this.redo.bind(this));
+    this.shortcutService.registerShortcuts(['ctrl+Y', 'ctrl+shift+Z'] , this.redo.bind(this));
 
     this.saveState({
       type : ns.HistoryService.SNAPSHOT
diff --git a/src/js/service/keyboard/CheatsheetService.js b/src/js/service/keyboard/CheatsheetService.js
index 53df638b..faf6a19e 100644
--- a/src/js/service/keyboard/CheatsheetService.js
+++ b/src/js/service/keyboard/CheatsheetService.js
@@ -66,7 +66,7 @@
 
   ns.CheatsheetService.prototype.initMarkupForTools_ = function () {
     var descriptors = pskl.app.toolController.tools.map(function (tool) {
-      return this.toDescriptor_(tool.shortcut, tool.instance.getHelpText(), 'tool-icon ' + tool.instance.toolId);
+      return this.toDescriptor_(tool.shortcut, tool.getHelpText(), 'tool-icon ' + tool.toolId);
     }.bind(this));
 
     var container = this.cheatsheetEl.querySelector('.cheatsheet-tool-shortcuts');
diff --git a/src/js/tools/drawing/Circle.js b/src/js/tools/drawing/Circle.js
index 5285e21b..42c12364 100644
--- a/src/js/tools/drawing/Circle.js
+++ b/src/js/tools/drawing/Circle.js
@@ -11,6 +11,7 @@
 
     this.toolId = 'tool-circle';
     this.helpText = 'Circle tool';
+    this.shortcut = 'C';
   };
 
   pskl.utils.inherit(ns.Circle, ns.ShapeTool);
diff --git a/src/js/tools/drawing/ColorPicker.js b/src/js/tools/drawing/ColorPicker.js
index 62e2238c..d25dedb5 100644
--- a/src/js/tools/drawing/ColorPicker.js
+++ b/src/js/tools/drawing/ColorPicker.js
@@ -9,6 +9,7 @@
   ns.ColorPicker = function() {
     this.toolId = 'tool-colorpicker';
     this.helpText = 'Color picker';
+    this.shortcut = 'O';
   };
 
   pskl.utils.inherit(ns.ColorPicker, ns.BaseTool);
diff --git a/src/js/tools/drawing/ColorSwap.js b/src/js/tools/drawing/ColorSwap.js
index 7c1f2817..a043a623 100644
--- a/src/js/tools/drawing/ColorSwap.js
+++ b/src/js/tools/drawing/ColorSwap.js
@@ -7,8 +7,8 @@
 
   ns.ColorSwap = function() {
     this.toolId = 'tool-colorswap';
-
     this.helpText = 'Paint all pixels of the same color';
+    this.shortcut = 'A';
 
     this.tooltipDescriptors = [
       {key : 'ctrl', description : 'Apply to all layers'},
diff --git a/src/js/tools/drawing/DitheringTool.js b/src/js/tools/drawing/DitheringTool.js
index d8de3382..4fd2b5b7 100644
--- a/src/js/tools/drawing/DitheringTool.js
+++ b/src/js/tools/drawing/DitheringTool.js
@@ -10,6 +10,7 @@
     ns.SimplePen.call(this);
     this.toolId = 'tool-dithering';
     this.helpText = 'Dithering tool';
+    this.shortcut = 'T';
   };
   pskl.utils.inherit(ns.DitheringTool, ns.SimplePen);
 
diff --git a/src/js/tools/drawing/Eraser.js b/src/js/tools/drawing/Eraser.js
index 977f830e..036b0892 100644
--- a/src/js/tools/drawing/Eraser.js
+++ b/src/js/tools/drawing/Eraser.js
@@ -9,8 +9,10 @@
 
   ns.Eraser = function() {
     this.superclass.constructor.call(this);
+
     this.toolId = 'tool-eraser';
     this.helpText = 'Eraser tool';
+    this.shortcut = 'E';
   };
 
   pskl.utils.inherit(ns.Eraser, ns.SimplePen);
diff --git a/src/js/tools/drawing/Lighten.js b/src/js/tools/drawing/Lighten.js
index 0c3b9a53..7be4ef64 100644
--- a/src/js/tools/drawing/Lighten.js
+++ b/src/js/tools/drawing/Lighten.js
@@ -10,9 +10,10 @@
 
   ns.Lighten = function() {
     this.superclass.constructor.call(this);
-    this.toolId = 'tool-lighten';
 
+    this.toolId = 'tool-lighten';
     this.helpText = 'Lighten';
+    this.shortcut = 'U';
 
     this.tooltipDescriptors = [
       {key : 'ctrl', description : 'Darken'},
diff --git a/src/js/tools/drawing/Move.js b/src/js/tools/drawing/Move.js
index 0c01e60d..a5fd0ea8 100644
--- a/src/js/tools/drawing/Move.js
+++ b/src/js/tools/drawing/Move.js
@@ -9,6 +9,7 @@
   ns.Move = function() {
     this.toolId = ns.Move.TOOL_ID;
     this.helpText = 'Move tool';
+    this.shortcut = 'M';
 
     this.tooltipDescriptors = [
       {key : 'ctrl', description : 'Apply to all layers'},
@@ -21,6 +22,10 @@
     this.startRow = null;
   };
 
+  /**
+   * The move tool id is used by the ToolController and the BaseSelect and needs to be
+   * easliy accessible
+   */
   ns.Move.TOOL_ID = 'tool-move';
 
   pskl.utils.inherit(ns.Move, ns.BaseTool);
diff --git a/src/js/tools/drawing/PaintBucket.js b/src/js/tools/drawing/PaintBucket.js
index 5c9034de..d264b939 100644
--- a/src/js/tools/drawing/PaintBucket.js
+++ b/src/js/tools/drawing/PaintBucket.js
@@ -9,6 +9,7 @@
   ns.PaintBucket = function() {
     this.toolId = 'tool-paint-bucket';
     this.helpText = 'Paint bucket tool';
+    this.shortcut = 'B';
   };
 
   pskl.utils.inherit(ns.PaintBucket, ns.BaseTool);
diff --git a/src/js/tools/drawing/Rectangle.js b/src/js/tools/drawing/Rectangle.js
index 80b9c2e0..8074e97b 100644
--- a/src/js/tools/drawing/Rectangle.js
+++ b/src/js/tools/drawing/Rectangle.js
@@ -11,6 +11,7 @@
 
     this.toolId = 'tool-rectangle';
     this.helpText = 'Rectangle tool';
+    this.shortcut = 'R';
   };
 
   pskl.utils.inherit(ns.Rectangle, ns.ShapeTool);
diff --git a/src/js/tools/drawing/SimplePen.js b/src/js/tools/drawing/SimplePen.js
index ec665749..7b8c5a2c 100644
--- a/src/js/tools/drawing/SimplePen.js
+++ b/src/js/tools/drawing/SimplePen.js
@@ -9,6 +9,7 @@
   ns.SimplePen = function() {
     this.toolId = 'tool-pen';
     this.helpText = 'Pen tool';
+    this.shortcut = 'P';
 
     this.previousCol = null;
     this.previousRow = null;
diff --git a/src/js/tools/drawing/Stroke.js b/src/js/tools/drawing/Stroke.js
index 6f7a3e55..b86622f5 100644
--- a/src/js/tools/drawing/Stroke.js
+++ b/src/js/tools/drawing/Stroke.js
@@ -9,6 +9,7 @@
   ns.Stroke = function() {
     this.toolId = 'tool-stroke';
     this.helpText = 'Stroke tool';
+    this.shortcut = 'L';
 
     // Stroke's first point coordinates (set in applyToolAt)
     this.startCol = null;
diff --git a/src/js/tools/drawing/VerticalMirrorPen.js b/src/js/tools/drawing/VerticalMirrorPen.js
index b98c4934..22a0d805 100644
--- a/src/js/tools/drawing/VerticalMirrorPen.js
+++ b/src/js/tools/drawing/VerticalMirrorPen.js
@@ -6,6 +6,7 @@
 
     this.toolId = 'tool-vertical-mirror-pen';
     this.helpText = 'Vertical Mirror pen';
+    this.shortcut = 'V';
 
     this.tooltipDescriptors = [
       {key : 'ctrl', description : 'Use horizontal axis'},
diff --git a/src/js/tools/drawing/selection/AbstractDragSelect.js b/src/js/tools/drawing/selection/AbstractDragSelect.js
index 42aa6c7a..3b9f4ead 100644
--- a/src/js/tools/drawing/selection/AbstractDragSelect.js
+++ b/src/js/tools/drawing/selection/AbstractDragSelect.js
@@ -8,6 +8,7 @@
 
   ns.AbstractDragSelect = function () {
     ns.BaseSelect.call(this);
+
     this.hasSelection = false;
   };
 
diff --git a/src/js/tools/drawing/selection/LassoSelect.js b/src/js/tools/drawing/selection/LassoSelect.js
index c49471dc..d34ae20d 100644
--- a/src/js/tools/drawing/selection/LassoSelect.js
+++ b/src/js/tools/drawing/selection/LassoSelect.js
@@ -7,10 +7,11 @@
   var ns = $.namespace('pskl.tools.drawing.selection');
 
   ns.LassoSelect = function() {
+    ns.AbstractDragSelect.call(this);
+
     this.toolId = 'tool-lasso-select';
     this.helpText = 'Lasso selection';
-
-    ns.AbstractDragSelect.call(this);
+    this.shortcut = 'H';
   };
 
   pskl.utils.inherit(ns.LassoSelect, ns.AbstractDragSelect);
diff --git a/src/js/tools/drawing/selection/RectangleSelect.js b/src/js/tools/drawing/selection/RectangleSelect.js
index fa4aa062..26dfcc71 100644
--- a/src/js/tools/drawing/selection/RectangleSelect.js
+++ b/src/js/tools/drawing/selection/RectangleSelect.js
@@ -7,10 +7,12 @@
   var ns = $.namespace('pskl.tools.drawing.selection');
 
   ns.RectangleSelect = function() {
+    ns.AbstractDragSelect.call(this);
+
     this.toolId = 'tool-rectangle-select';
     this.helpText = 'Rectangle selection';
+    this.shortcut = 'S';
 
-    ns.AbstractDragSelect.call(this);
   };
 
   pskl.utils.inherit(ns.RectangleSelect, ns.AbstractDragSelect);
diff --git a/src/js/tools/drawing/selection/ShapeSelect.js b/src/js/tools/drawing/selection/ShapeSelect.js
index 2849a93b..7bd72c9a 100644
--- a/src/js/tools/drawing/selection/ShapeSelect.js
+++ b/src/js/tools/drawing/selection/ShapeSelect.js
@@ -7,11 +7,11 @@
   var ns = $.namespace('pskl.tools.drawing.selection');
 
   ns.ShapeSelect = function() {
-    this.toolId = 'tool-shape-select';
-
-    this.helpText = 'Shape selection';
-
     ns.BaseSelect.call(this);
+
+    this.toolId = 'tool-shape-select';
+    this.helpText = 'Shape selection';
+    this.shortcut = 'Z';
   };
 
   pskl.utils.inherit(ns.ShapeSelect, ns.BaseSelect);
diff --git a/src/js/tools/transform/AbstractTransformTool.js b/src/js/tools/transform/AbstractTransformTool.js
index 98c06be0..db30203d 100644
--- a/src/js/tools/transform/AbstractTransformTool.js
+++ b/src/js/tools/transform/AbstractTransformTool.js
@@ -5,7 +5,7 @@
 
   pskl.utils.inherit(ns.AbstractTransformTool, pskl.tools.Tool);
 
-  ns.AbstractTransformTool.prototype.apply = function (evt) {
+  ns.AbstractTransformTool.prototype.applyTransformation = function (evt) {
     var allFrames = evt.shiftKey;
     var allLayers = evt.ctrlKey;