Gallery.app: Add touch handlers for the zoom/scroll feature.

Gallery.app: Add touch handlers for the zoom/scroll feature.

Previously touch operations are handled by SwipeOverlay class. Overlay is a
common way to obtain mouse/touch events in Gallery, but it does not support
touch specific capability (e.g. multi fingers). The CL newly adds TouchHandlers
class to the SlideMode class and let it call the zoom/scroll feature of the
slide mode depending on user's touch gestures.

BUG=245926
TEST=on link

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

[hirono, 2014-07-23 07:59:38]

PTAL the CL? Thank you!

[mtomasz, 2014-07-23 08:15:08]

https://codereview.chromium.org/411853002/diff/40001/ui/file_manager/gallery/...
File ui/file_manager/gallery/js/image_editor/viewport.js (right):

https://codereview.chromium.org/411853002/diff/40001/ui/file_manager/gallery/...
ui/file_manager/gallery/js/image_editor/viewport.js:142: this.setZoom(zoom);
nit: How about initializing zoom in #136 or adding an console.assert that zoom
is undefined? WDYT?

https://codereview.chromium.org/411853002/diff/40001/ui/file_manager/gallery/...
ui/file_manager/gallery/js/image_editor/viewport.js:162:
Viewport.prototype.isZooming = function() {
nit: How about just isZoomed?

https://codereview.chromium.org/411853002/diff/40001/ui/file_manager/gallery/...
File ui/file_manager/gallery/js/slide_mode.js (right):

https://codereview.chromium.org/411853002/diff/40001/ui/file_manager/gallery/...
ui/file_manager/gallery/js/slide_mode.js:1324: /**
@param, @constructor missing

https://codereview.chromium.org/411853002/diff/40001/ui/file_manager/gallery/...
ui/file_manager/gallery/js/slide_mode.js:1327: function
TouchHandlers(targetElement, slideMode) {
I'm wondering if the plural naming is fine. How about just TouchHandler? It is
one big handler for all touch events. WDYT?

https://codereview.chromium.org/411853002/diff/40001/ui/file_manager/gallery/...
ui/file_manager/gallery/js/slide_mode.js:1344: this.enable_ = true;
nit: enabled?

https://codereview.chromium.org/411853002/diff/40001/ui/file_manager/gallery/...
ui/file_manager/gallery/js/slide_mode.js:1352: this.inTouch_ = false;
nit: How about simply touchStarted?

https://codereview.chromium.org/411853002/diff/40001/ui/file_manager/gallery/...
ui/file_manager/gallery/js/slide_mode.js:1443: event.type === 'touchstart' &&
nit: Why not splitting this to a separate method? like onTouchStartEvent? This
code is never executed for other events.

https://codereview.chromium.org/411853002/diff/40001/ui/file_manager/gallery/...
ui/file_manager/gallery/js/slide_mode.js:1450: // Check if the current touch
operaiton ends with the event.
typo: operation

https://codereview.chromium.org/411853002/diff/40001/ui/file_manager/gallery/...
ui/file_manager/gallery/js/slide_mode.js:1505: if (distance1 == 0.0)
nit: Simply === 0?

[hirono, 2014-07-23 09:27:23]

https://codereview.chromium.org/411853002/diff/40001/ui/file_manager/gallery/...
File ui/file_manager/gallery/js/image_editor/viewport.js (right):

https://codereview.chromium.org/411853002/diff/40001/ui/file_manager/gallery/...
ui/file_manager/gallery/js/image_editor/viewport.js:142: this.setZoom(zoom);
On 2014/07/23 08:15:08, mtomasz wrote:
> nit: How about initializing zoom in #136 or adding an console.assert that zoom
> is undefined? WDYT?

Done.

https://codereview.chromium.org/411853002/diff/40001/ui/file_manager/gallery/...
ui/file_manager/gallery/js/image_editor/viewport.js:162:
Viewport.prototype.isZooming = function() {
On 2014/07/23 08:15:08, mtomasz wrote:
> nit: How about just isZoomed?

Done.

https://codereview.chromium.org/411853002/diff/40001/ui/file_manager/gallery/...
File ui/file_manager/gallery/js/slide_mode.js (right):

https://codereview.chromium.org/411853002/diff/40001/ui/file_manager/gallery/...
ui/file_manager/gallery/js/slide_mode.js:1324: /**
On 2014/07/23 08:15:08, mtomasz wrote:
> @param, @constructor missing

Done.

https://codereview.chromium.org/411853002/diff/40001/ui/file_manager/gallery/...
ui/file_manager/gallery/js/slide_mode.js:1327: function
TouchHandlers(targetElement, slideMode) {
On 2014/07/23 08:15:08, mtomasz wrote:
> I'm wondering if the plural naming is fine. How about just TouchHandler? It is
> one big handler for all touch events. WDYT?

Done.

https://codereview.chromium.org/411853002/diff/40001/ui/file_manager/gallery/...
ui/file_manager/gallery/js/slide_mode.js:1344: this.enable_ = true;
On 2014/07/23 08:15:08, mtomasz wrote:
> nit: enabled?

Done.

https://codereview.chromium.org/411853002/diff/40001/ui/file_manager/gallery/...
ui/file_manager/gallery/js/slide_mode.js:1352: this.inTouch_ = false;
On 2014/07/23 08:15:08, mtomasz wrote:
> nit: How about simply touchStarted?

Done.

https://codereview.chromium.org/411853002/diff/40001/ui/file_manager/gallery/...
ui/file_manager/gallery/js/slide_mode.js:1443: event.type === 'touchstart' &&
On 2014/07/23 08:15:08, mtomasz wrote:
> nit: Why not splitting this to a separate method? like onTouchStartEvent? This
> code is never executed for other events.

Done.

https://codereview.chromium.org/411853002/diff/40001/ui/file_manager/gallery/...
ui/file_manager/gallery/js/slide_mode.js:1450: // Check if the current touch
operaiton ends with the event.
On 2014/07/23 08:15:08, mtomasz wrote:
> typo: operation

Done.

https://codereview.chromium.org/411853002/diff/40001/ui/file_manager/gallery/...
ui/file_manager/gallery/js/slide_mode.js:1505: if (distance1 == 0.0)
On 2014/07/23 08:15:08, mtomasz wrote:
> nit: Simply === 0?

Done.

[mtomasz, 2014-07-23 09:38:14]

lgtm

[hirono, 2014-07-23 09:43:36]

The CQ bit was checked by hirono@chromium.org

[hirono, 2014-07-23 09:43:41]

On 2014/07/23 09:38:14, mtomasz wrote:
> lgtm

Thank you!

[commit-bot: I haz the power, 2014-07-23 09:44:58]

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

[commit-bot: I haz the power, 2014-07-23 11:44:08]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_chromium_gn_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_gn_rel...)

[commit-bot: I haz the power, 2014-07-23 15:02:29]

Change committed as 284926