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

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

Previously the aspect ratio is applied at the beginning of dragging after
pressing the buttons.

The CL also temporarily removes dimmable class from the container of aspect
 buttons to prevent the aspect buttons from being dimmed until a user starts
 dragging.

BUG=401312
TEST=manually

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=288320

[hirono, 2014-08-08 04:52:53]

PTAL the CL? Thank you!

[mtomasz, 2014-08-08 04:55:24]

On 2014/08/08 04:52:53, hirono wrote:
> PTAL the CL? Thank you!

In description:
typo: temprarily

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

https://codereview.chromium.org/454743002/diff/1/ui/file_manager/gallery/js/i...
File ui/file_manager/gallery/js/image_editor/image_transform.js (right):

https://codereview.chromium.org/454743002/diff/1/ui/file_manager/gallery/js/i...
ui/file_manager/gallery/js/image_editor/image_transform.js:137: this.toolbar_ =
null;
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.

https://codereview.chromium.org/454743002/diff/1/ui/file_manager/gallery/js/i...
ui/file_manager/gallery/js/image_editor/image_transform.js:596: if
(!this.dragMode)
dragMode or dragMode_?

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

https://codereview.chromium.org/454743002/diff/1/ui/file_manager/gallery/js/i...
File ui/file_manager/gallery/js/image_editor/image_transform.js (right):

https://codereview.chromium.org/454743002/diff/1/ui/file_manager/gallery/js/i...
ui/file_manager/gallery/js/image_editor/image_transform.js:137: this.toolbar_ =
null;
On 2014/08/08 05:00:31, mtomasz wrote:
> 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.

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.

https://codereview.chromium.org/454743002/diff/1/ui/file_manager/gallery/js/i...
ui/file_manager/gallery/js/image_editor/image_transform.js:596: if
(!this.dragMode)
On 2014/08/08 05:00:31, mtomasz wrote:
> dragMode or dragMode_?

Oops. dragMode_ is correct.

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

lgtm

https://codereview.chromium.org/454743002/diff/1/ui/file_manager/gallery/js/i...
File ui/file_manager/gallery/js/image_editor/image_transform.js (right):

https://codereview.chromium.org/454743002/diff/1/ui/file_manager/gallery/js/i...
ui/file_manager/gallery/js/image_editor/image_transform.js:137: this.toolbar_ =
null;
On 2014/08/08 05:15:30, hirono wrote:
> On 2014/08/08 05:00:31, mtomasz wrote:
> > 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.
> 
> 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.

Got it.

[hirono, 2014-08-08 05:23:50]

The CQ bit was checked by hirono@chromium.org

[hirono, 2014-08-08 05:24:12]

Thank you!

[commit-bot: I haz the power, 2014-08-08 05:29:38]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/hirono@chromium.org/454743002/20001

[commit-bot: I haz the power, 2014-08-08 10:48:40]

Change committed as 288320