Gallery: Apply aspect settings immediately when pressing the aspect buttons.

ui/file_manager/gallery/js/image_editor/image_transform.js

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 'use strict';
 
 /**
  * Crop mode.
  *
  * @extends {ImageEditor.Mode}
@@ skipping 36 matching lines @@
   this.middleBox_.appendChild(this.cropFrame_);
 
   this.shadowRight_ = doc.createElement('div');
   this.shadowRight_.className = 'shadow';
   this.middleBox_.appendChild(this.shadowRight_);
 
   this.shadowBottom_ = doc.createElement('div');
   this.shadowBottom_.className = 'shadow';
   this.domOverlay_.appendChild(this.shadowBottom_);
 
+  this.toolBar_ = null;
+
   var cropFrame = this.cropFrame_;
   function addCropFrame(className) {
     var div = doc.createElement('div');
     div.className = className;
     cropFrame.appendChild(div);
   }
 
   addCropFrame('left top corner');
   addCropFrame('top horizontal');
   addCropFrame('right top corner');
@@ skipping 28 matching lines @@
           if (button.classList.contains('selected')) {
             button.classList.remove('selected');
             this.cropRect_.fixedAspectRatio = null;
           } else {
             var selectedButtons =
                 toolbar.element.querySelectorAll('button.selected');
             for (var i = 0; i < selectedButtons.length; i++) {
               selectedButtons[i].classList.remove('selected');
             }
             button.classList.add('selected');
+            var clipRect = this.viewport_.screenToImageRect(
+                this.viewport_.getImageBoundsOnScreenClipped());
             this.cropRect_.fixedAspectRatio = aspect;
+            this.cropRect_.forceAspectRatio(aspect, clipRect);
+            this.markUpdated();
+            this.positionDOM();
+            this.toolbar_.element.classList.remove('dimmable');
+            this.toolbar_.element.removeAttribute('dimmed');
           }
         }.bind(this, aspects[name]));
   }
+  this.toolbar_ = toolbar;
 };
 
 /**
  * Handles resizing of the window and updates the crop rectangle.
  * @private
  */
 ImageEditor.Mode.Crop.prototype.onResized_ = function() {
   this.positionDOM();
 };
 
 /**
  * Resets the mode.
  */
 ImageEditor.Mode.Crop.prototype.reset = function() {
   ImageEditor.Mode.prototype.reset.call(this);
   this.createDefaultCrop();
+  if (this.toolbar_) {
+    this.toolbar_.element.classList.add('dimmable');
+    this.toolbar_ = null;

[mtomasz, 2014/08/08 05:00:31]

Why do you need to set toolbar_ to null? Seems confusing, since the toolbar is
still there, but we just lose a reference to it.

[hirono, 2014/08/08 05:15:30]

I followed the existing design of ImageEditor.Mode. It looks the toobar is
designed to be shared among all ImageEditor.Mode.
ImageEditor.Mode.prototype.reset called at #133 is removes all the buttons added
by this mode. It means the mode loses the ownership of the toolbar. We should
not modify the toolbar after calling the reset method.

[mtomasz, 2014/08/08 05:21:10]

Got it.

+  }
 };
 
 /**
  * Updates the position of DOM elements.
  */
 ImageEditor.Mode.Crop.prototype.positionDOM = function() {
   var screenClipped = this.viewport_.getImageBoundsOnScreenClipped();
 
   var screenCrop = this.viewport_.imageToScreenRect(this.cropRect_.getRect());
   var delta = ImageEditor.Mode.Crop.MOUSE_GRAB_RADIUS;
@@ skipping 84 matching lines @@
  * @return {function(number,number,boolean)} A function to be called on mouse
  *     drag. It takes x coordinate value, y coordinate value, and shift key
  *     flag.
  */
 ImageEditor.Mode.Crop.prototype.getDragHandler = function(x, y, touch) {
   var cropDragHandler = this.cropRect_.getDragHandler(x, y, touch);
   if (!cropDragHandler)
     return null;
 
   return function(x, y, shiftKey) {
+    if (this.toolbar_)
+      this.toolbar_.element.classList.add('dimmable');
     cropDragHandler(x, y, shiftKey);
     this.markUpdated();
     this.positionDOM();
   }.bind(this);
 };
 
 /**
  * Obtains the double tap action depending on the coordinate.
  *
  * @param {number} x X coordinate of the event.
@@ skipping 331 matching lines @@
  * Forces the aspect ratio.
  *
  * @param {number} aspectRatio Aspect ratio.
  * @param {Object} clipRect Clip rect.
  */
 DraggableRect.prototype.forceAspectRatio = function(aspectRatio, clipRect) {
   // Get current rectangle scale.
   var width = this.bounds_.right - this.bounds_.left;
   var height = this.bounds_.bottom - this.bounds_.top;
   var currentScale;
-  if (this.dragMode_.xSide === 'none')
+  if (!this.dragMode)

[mtomasz, 2014/08/08 05:00:31]

dragMode or dragMode_?

[hirono, 2014/08/08 05:15:30]

Oops. dragMode_ is correct.

+    currentScale = ((width / aspectRatio) + height) / 2;
+  else if (this.dragMode_.xSide === 'none')
     currentScale = height;
   else if (this.dragMode_.ySide === 'none')
     currentScale = width / aspectRatio;
   else
     currentScale = Math.max(width / aspectRatio, height);
 
   // Get maximum width/height scale.
   var maxWidth;
   var maxHeight;
   var center = (this.bounds_.left + this.bounds_.right) / 2;
   var middle = (this.bounds_.top + this.bounds_.bottom) / 2;
-  switch (this.dragMode_.xSide) {
+  var xSide = this.dragMode_ ? this.dragMode_.xSide : 'none';
+  var ySide = this.dragMode_ ? this.dragMode_.ySide : 'none';
+  switch (xSide) {
     case 'left':
       maxWidth = this.bounds_.right - clipRect.left;
       break;
     case 'right':
       maxWidth = clipRect.left + clipRect.width - this.bounds_.left;
       break;
     case 'none':
       maxWidth = Math.min(
           clipRect.left + clipRect.width - center,
           center - clipRect.left) * 2;
       break;
   }
-  switch (this.dragMode_.ySide) {
+  switch (ySide) {
     case 'top':
       maxHeight = this.bounds_.bottom - clipRect.top;
       break;
     case 'bottom':
       maxHeight = clipRect.top + clipRect.height - this.bounds_.top;
       break;
     case 'none':
       maxHeight = Math.min(
           clipRect.top + clipRect.height - middle,
           middle - clipRect.top) * 2;
       break;
   }
 
   // Obtains target scale.
   var targetScale = Math.min(
       currentScale,
       maxWidth / aspectRatio,
       maxHeight);
 
   // Update bounds.
   var newWidth = targetScale * aspectRatio;
   var newHeight = targetScale;
-  switch (this.dragMode_.xSide) {
+  switch (xSide) {
     case 'left':
       this.bounds_.left = this.bounds_.right - newWidth;
       break;
     case 'right':
       this.bounds_.right = this.bounds_.left + newWidth;
       break;
     case 'none':
       this.bounds_.left = center - newWidth / 2;
       this.bounds_.right = center + newWidth / 2;
       break;
   }
-  switch (this.dragMode_.ySide) {
+  switch (ySide) {
     case 'top':
       this.bounds_.top = this.bounds_.bottom - newHeight;
       break;
     case 'bottom':
       this.bounds_.bottom = this.bounds_.top + newHeight;
       break;
     case 'none':
       this.bounds_.top = middle - newHeight / 2;
       this.bounds_.bottom = middle + newHeight / 2;
       break;
   }
 };