Use a specialized new tab page in TOUCH_UI builds

chrome/browser/resources/touch_ntp/grabber.js

+// Copyright (c) 2011 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.
+
+/**
+ * @fileoverview Grabber implementation.
+ * Allows you to pick up objects (with a long-press) and drag them around the
+ * screen.
+ *
+ * Note: This should perhaps really use standard drag-and-drop events, but there
+ * is no standard for them on touch devices.  We could define a model for
+ * activating touch-based dragging of elements (programatically and/or with
+ * CSS attributes) and use it here (even have a JS library to generate such
+ * events when the browser doesn't support them).
+ */
+
+/**
+ * @constructor

[arv (Not doing code reviews), 2011/03/10 19:25:59]

Missing description

[Rick Byers, 2011/03/11 02:44:33]

Done.

+ * @param {!Element} element The element that can be grabbed and moved.
+ */
+function Grabber(element) {
+  /**
+   * @type {!Element}
+   * @private
+   */
+  this.element_ = element;
+
+  /**
+   * @type {!TouchHandler}
+   * @private
+   */
+  this.touchHandler_ = new TouchHandler(this.element_);
+
+  /**
+   * @type {boolean}
+   * @private
+   */
+  this.grabbed_ = false;
+
+  /**
+   * @type {boolean}
+   * @private
+   */
+  this.dragging_ = false;
+
+  /**
+   * @type {EventTracker}
+   * @private
+   */
+  this.events_ = new EventTracker();
+
+  this.touchHandler_.enable(/* opt_capture */ false);

[arv (Not doing code reviews), 2011/03/10 19:25:59]

this.touchHandler_.enabled = false;

seems like a better API

[Rick Byers, 2011/03/11 02:44:33]

enabled=false would suggest to me that the touchHandler was being disabled. 
That's not what this does - it enables but without capture.  I've added a
comment to help clarify.  Let me know if I've misunderstood your suggestion.

Note that I did also add a 'disable' method.  In this code I don't ever use a
TouchHandler that does capturing, so I could replace enable(bool)/disable with a
boolean property as you suggest with some loss of flexibility in other scenarios
if you think it's better.

[arv (Not doing code reviews), 2011/03/11 20:08:46]

OK, then it makes sense.

+
+  // Prevent any built-in drag-and-drop support from activating for the element.
+  // Note that we don't want details of how we're implementing dragging here to
+  // leak out of this file (eg. we may switch to using webkit drag-and-drop).
+  this.events_.add(this.element_, 'dragstart', function(e) {
+    e.preventDefault();
+  }, true);
+
+  // Add our TouchHandler event listeners
+  this.events_.add(this.element_, TouchHandler.EventType.TOUCH_START,
+      this.onTouchStart_.bind(this), false);
+  this.events_.add(this.element_, TouchHandler.EventType.LONG_PRESS,
+      this.onLongPress_.bind(this), false);
+  this.events_.add(this.element_, TouchHandler.EventType.DRAG_START,
+      this.onDragStart_.bind(this), false);
+  this.events_.add(this.element_, TouchHandler.EventType.DRAG_MOVE,
+      this.onDragMove_.bind(this), false);
+  this.events_.add(this.element_, TouchHandler.EventType.DRAG_END,
+      this.onDragEnd_.bind(this), false);
+  this.events_.add(this.element_, TouchHandler.EventType.TOUCH_END,
+      this.onTouchEnd_.bind(this), false);
+}
+
+/**
+ * Events fired by the grabber.
+ * Events are fired at the element affected (not the element being dragged).

[arv (Not doing code reviews), 2011/03/10 19:25:59]

@enum {string}

[Rick Byers, 2011/03/11 02:44:33]

Done.

+ */
+Grabber.EventType = {
+  // Fired at the grabber element when it is first grabbed
+  GRAB: 'grabber:grab',
+  // Fired at the grabber element when dragging begins (after GRAB)
+  DRAG_START: 'grabber:dragstart',
+  // Fired at an element when something is dragged over top of it.
+  DRAG_ENTER: 'grabber:dragenter',
+  // Fired at an element when something is no longer over top of it.
+  // Not fired at all in the case of a DROP
+  DRAG_LEAVE: 'grabber:drag',
+  // Fired at an element when something is dropped on top of it.
+  DROP: 'grabber:drop',
+  // Fired at the grabber element when dragging ends (successfully or not) -
+  // after any DROP or DRAG_LEAVE
+  DRAG_END: 'grabber:dragend',
+  // Fired at the grabber element when it is released (even if no drag
+  // occured) - after any DRAG_END event.
+  RELEASE: 'grabber:release'
+};
+
+/**
+ * The CSS class to apply when an element is touched but not yet
+ * grabbed.
+ * @type {string}
+ */
+Grabber.PRESSED_CLASS = 'grabber-pressed';
+
+/**
+ * The class to apply when an element has been held (including when it is
+ * being dragged.
+ * @type {string}
+ */
+Grabber.GRAB_CLASS = 'grabber-grabbed';
+
+/**
+ * The class to apply when a grabbed element is being dragged.
+ * @type {string}
+ */
+Grabber.DRAGGING_CLASS = 'grabber-dragging';
+
+/**
+ * The webkitTransform applied to the element when it first started being
+ * dragged.
+ * @type {string|undefined}
+ */
+Grabber.prototype.baseTransform_;
+
+/**
+ * The element for which a DRAG_ENTER event was last fired
+ * @type {Element|undefined}
+ */
+Grabber.prototype.lastEnter_;
+
+/**
+ * Clean up all event handlers (eg. if the underlying element will be removed)
+ */
+Grabber.prototype.dispose = function() {
+  this.touchHandler_.disable();
+  this.events_.removeAll();
+
+  // Clean-up any active touch/drag
+  if (this.dragging_) {

[arv (Not doing code reviews), 2011/03/10 19:25:59]

Chrome style is to not use curly braces for single line if statements

[Rick Byers, 2011/03/11 02:44:33]

Done.
Does this apply to if/else blocks to?  Eg. I've got some code in touchhandler.js
like:

if (c) {
  a();
} else {
  b();
}

Should it be this instead?

if (c)
  a();
else
  b();

[arv (Not doing code reviews), 2011/03/11 20:08:46]

Yes.

If either the if or else part is more than one line both should have curly
braces. For example:

if (long_expression_that ==
    wraps) {
  singleStatement();
} else {
  singleStatement();
}

[Rick Byers, 2011/03/15 21:47:54]

Ok, thanks.  I believe I've updated for this bracing style everywhere.  Let me
know if you see anywhere I've missed though.

+    this.stopDragging_();
+  }
+  this.onTouchEnd_();
+};
+
+/**
+ * Invoked whenever this element is first touched
+ * @param {!CustomEvent} e The TouchHandler event.

[arv (Not doing code reviews), 2011/03/10 19:25:59]

CustomEvent is a useless DOM interface. Just use Event instead?

[Rick Byers, 2011/03/11 02:44:33]

The only reason I used CustomEvent is that it adds the 'detail' property where I
can stash extra stuff.  Are you saying that it's better to just rely on expando
properties on the JS Event object?  That would certainly be simpler if it's OK.

+ * @private
+ */
+Grabber.prototype.onTouchStart_ = function(e) {
+  this.element_.classList.add(Grabber.PRESSED_CLASS);
+
+  // Always permit the touch to perhaps trigger a drag
+  e.detail.enableDrag = true;
+};
+
+/**
+ * Invoked whenever the element stops being touched.
+ * Can be called explicitly to cleanup any active touch.
+ * @param {!CustomEvent=} opt_e The TouchHandler event.
+ * @private
+ */
+Grabber.prototype.onTouchEnd_ = function(opt_e) {
+  if (this.grabbed_) {
+    // Mark this element as no longer being grabbed
+    this.element_.classList.remove(Grabber.GRAB_CLASS);
+    this.grabbed_ = false;
+
+    this.dispatchEvent_(Grabber.EventType.RELEASE);

[arv (Not doing code reviews), 2011/03/10 19:25:59]

cr.dispatchSimpleEvent(this, Grabber.EventType.RELEASE);

[Rick Byers, 2011/03/11 02:44:33]

I should have looked more closely at the utilities in cr.js to see what I could
use there - thanks for pointing that out.  But in this case, dispatchSimpleEvent
doesn't set 'sender' and my clients rely on being able to get the Grabber
instance that raised the event (eg. so a single function can handle the grab of
any app - rather than needing a closure per app).

Should I expand dispatchSimpleEvent to add an optional sender argument at the
end perhaps?

[arv (Not doing code reviews), 2011/03/11 20:08:46]

(I misunderstood this. See more in next comment)

The sender is called 'target' in DOM. You can also use 'this' and
'currentTarget'. 'currentTarget' is the node that the current listener was
attached to.

<a>
  <b>
     <c/>
  </b>
</a>

b.addEventListener(type, function(e) {
  assertEquals(b, e.currentTarget);
  assertEquals(c, e.target);
  assertEquals(b, this)
});

c.addEventListener(type, function(e) {
  assertEquals(c, e.currentTarget);
  assertEquals(c, e.target);
  assertEquals(c, this)
});

var e = document.createEvent('Event');
e.initEvent(type, true, true);
c.dispatchEvent(e);

To play around with this you can use:

http://plexode.com/eval3/#ht=%3Cdiv%20id%3Da%3E%0A%20%20%3Cdiv%20id%3Db%3E%0A...


With this info do you still need to have a sender?

[Rick Byers, 2011/03/15 21:47:54]

See below.

+  }
+  else
+  {
+    this.element_.classList.remove(Grabber.PRESSED_CLASS);
+  }
+};
+
+/**
+ * Handler for TouchHandler's LONG_PRESS event
+ * Invoked when the element is held (without being dragged)
+ * @param {!CustomEvent} e The TouchHandler event.
+ * @private
+ */
+Grabber.prototype.onLongPress_ = function(e) {
+  assert(!this.grabbed_, 'Got longPress while still being held');
+
+  this.element_.classList.remove(Grabber.PRESSED_CLASS);
+  this.element_.classList.add(Grabber.GRAB_CLASS);
+  this.grabbed_ = true;
+
+  this.dispatchEvent_(Grabber.EventType.GRAB);
+};
+
+/**
+ * Invoked when the element is dragged.
+ * @param {!CustomEvent} e The TouchHandler event.
+ * @private
+ */
+Grabber.prototype.onDragStart_ = function(e) {
+  var that = this;
+  assert(!this.lastEnter_, 'only expect one drag to occur at a time');
+  assert(!this.dragging_);
+
+  // We only want to drag the element if its been grabbed
+  if (this.grabbed_) {
+    // Mark the item as being dragged
+    // Ensures our translate transform won't be animated and cancels any
+    // outstanding animations.
+    this.element_.classList.add(Grabber.DRAGGING_CLASS);
+
+    // Determine the webkitTransform currently applied to the element.
+    // Note that it's important that we do this AFTER cancelling animation,
+    // otherwise we could see an intermediate value.
+    // We'll assume this value will be constant for the duration of the drag
+    // so that we can combine it with our translate3d transform.
+    this.baseTransform_ = window.getComputedStyle(this.element_, null).

[arv (Not doing code reviews), 2011/03/10 19:25:59]

is this the right window?

this.ownerDocument.defaultView.getComputedStyle(this.element_);

Who designed these aweful DOM APIs? ;)

[arv (Not doing code reviews), 2011/03/10 19:25:59]

skip null here?

[Rick Byers, 2011/03/11 02:44:33]

Interesting - thanks.  I had to read up on this a bit - I'm still new to
HTML/JavaScript and didn't appreciate the impact of IFrames.

[Rick Byers, 2011/03/11 02:44:33]

Done.  I thought JSCompiler was complaining if I omitted the optional 2nd param,
but it seems happy now.

[arv (Not doing code reviews), 2011/03/11 20:08:46]

I assume most of the Chrome JS would fail in the presence of frames but
sometimes it is easy to get the right document and window so it might be worth
remembering that for the future.

+      webkitTransform;
+
+    this.dispatchEvent_(Grabber.EventType.DRAG_START);
+    e.detail.enableDrag = true;
+    this.dragging_ = true;
+  } else {
+    // Hasn't been grabbed - don't drag, just unpress
+    this.element_.classList.remove(Grabber.PRESSED_CLASS);
+    e.detail.enableDrag = false;
+  }
+};
+
+/**
+ * Invoked when a grabbed element is being dragged
+ * @param {!CustomEvent} e The TouchHandler event.
+ * @private
+ */
+Grabber.prototype.onDragMove_ = function(e) {
+  assert(this.grabbed_ && this.dragging_);
+
+  this.translateTo_(e.detail.dragDeltaX, e.detail.dragDeltaY);
+
+  var target = this.getCoveredElement_(e.detail);
+  if (target && target != this.lastEnter_) {
+    // Send the events
+    this.sendDragLeave_(e);
+    this.dispatchEventTo_(Grabber.EventType.DRAG_ENTER, target);
+  }
+  this.lastEnter_ = target;
+};
+
+/**
+ * Send DRAG_LEAVE to the element last sent a DRAG_ENTER if any.
+ * @param {!CustomEvent} e The event triggering this DRAG_LEAVE.
+ * @private
+ */
+Grabber.prototype.sendDragLeave_ = function(e) {
+  if (this.lastEnter_) {
+    this.dispatchEventTo_(Grabber.EventType.DRAG_LEAVE, this.lastEnter_);
+    this.lastEnter_ = undefined;
+  }
+};
+
+/**
+ * Moves the element to the specified position.
+ * @param {number} x Horizontal position to move to.
+ * @param {number} y Vertical position to move to.
+ * @private
+ */
+Grabber.prototype.translateTo_ = function(x, y) {
+  // Order is important here - we want to translate before doing the zoom
+  this.element_.style.webkitTransform = 'translate3d(' + x + 'px, ' +
+    y + 'px, 0) ' + this.baseTransform_;

[arv (Not doing code reviews), 2011/03/10 19:25:59]

+2 indentation

[Rick Byers, 2011/03/11 02:44:33]

Done.

+};
+
+/**
+ * Get the element being covered by a given touch.
+ * @param {TouchHandler.EventDetail} touch The details of the touch event
+ *        indicating the position to check.

[arv (Not doing code reviews), 2011/03/10 19:25:59]

indent 4 spaces

[Rick Byers, 2011/03/11 02:44:33]

Done.

+ * @return {Element} The element under the touch or null.
+ * @private
+ */
+Grabber.prototype.getCoveredElement_ = function(touch) {
+  // Ensure the element being dragged doesn't get in the way
+  // It's unfortunate that there's not a better way to do this.
+  // This will probably cause a re-layout, but shouldn't cause flicker as
+  // repaint shouldn't happen in the middle of this function.
+  // We could also use zIndex, but that's more complicated and harder to
+  // reliably get the element out of the way.
+  var origDisplay = this.element_.style.display || '';
+  this.element_.style.display = 'none';

[arv (Not doing code reviews), 2011/03/10 19:25:59]

I would prefer pointerEvents = 'none' since that does not require layout but I'm
not 100% sure that elementFromPoint is aware of pointer events

[Rick Byers, 2011/03/11 02:44:33]

elementFromPoint does appear to be aware of pointerEvents - thanks!

+  var target = document.elementFromPoint(touch.clientX, touch.clientY);
+  this.element_.style.display = origDisplay;
+
+  return target;
+};
+
+/**
+ * @private
+ * @param {CustomEvent} e The TouchHandler event.
+ */
+Grabber.prototype.onDragEnd_ = function(e) {
+  // We should get this before the onTouchEnd.  Don't change
+  // this.grabbed_ - it's onTouchEnd's responsibility to clear it.
+  assert(this.grabbed_ && this.dragging_);
+  var event;
+
+  // Try to determine what element is underneath us
+  var target = this.getCoveredElement_(e.detail);
+  if (target) {
+    this.dispatchEventTo_(Grabber.EventType.DROP, target);
+  }
+
+  // Cleanup and send DRAG_END
+  // Note that like HTML5 DND, we don't send DRAG_LEAVE on drop
+  this.stopDragging_();
+};
+
+/**
+ * Clean-up the active drag and send DRAG_LEAVE
+ * @private
+ */
+Grabber.prototype.stopDragging_ = function() {
+  assert(this.dragging_);
+  this.lastEnter_ = undefined;
+
+  // Mark the element as no longer being dragged
+  this.element_.classList.remove(Grabber.DRAGGING_CLASS);
+  this.element_.style.webkitTransform = '';
+
+  this.dragging_ = false;
+  this.dispatchEvent_(Grabber.EventType.DRAG_END);
+};
+
+/**
+ * @return {!Element} The element the grabber is attached to.
+ */
+Grabber.prototype.getElement = function() {

[arv (Not doing code reviews), 2011/03/10 19:25:59]

Use getters and setters for getters and setters

Grabber.prototype = {
  ...
  get element() {
    return this.element_;
  },
  ...
};

  

[Rick Byers, 2011/03/11 02:44:33]

Thanks, but JSCompiler doesn't yet support this ES5 syntax.  Instead I've used
the Object.defineProperty syntax for now.  I discovered that an option was
recently added to the JSCompiler sources to enable support for this, but there
hasn't been an official release yet - so I'll leave a TODO saying to upgrade.  I
could create a new jscompiler build for myself, but I wouldn't want that to be a
pre-req for anyone else looking to modify the code here... Agree?   

[arv (Not doing code reviews), 2011/03/11 20:08:46]

It is a such a relief to not have to be hamstrung by old browser limitations and
bugs that I think letting JSCompiler get in our way is not an option.

I'm fine with requiring tip of tree Closure Compiler too if that is what it
takes. We should make changes to GJSLint (Closure Linter) and JSCompiler instead
of changes to best practices coding practices for ToT Chrome.

[Rick Byers, 2011/03/15 21:47:54]

Ok.  I agree we shouldn't be sacrificing good style for these tools.  I'll
switch to using ToT jscompiler to get around this for now.  We can discuss the
bigger question of using JSCompiler at all off-line.

+  return this.element_;
+};
+
+/**
+ * Send a Grabber event to a specific element
+ * @param {string} eventType The type of event to send.
+ * @param {!Element} target The element to send the event to.
+ * @private
+ */
+Grabber.prototype.dispatchEventTo_ = function(eventType, target) {
+  var event = document.createEvent('Event');

[arv (Not doing code reviews), 2011/03/10 19:25:59]

cr.dispatchSimpleEvent

[Rick Byers, 2011/03/11 02:44:33]

See above.

[arv (Not doing code reviews), 2011/03/11 20:08:46]

Ah, I was confused by the overloaded term, sender. How about renaming this
grabber?

var e = new cr.Event(eventType, true, true);
e.grabber = this;
target.dispatchEvent(e);

but then the cr utilities are not that useful any more.

I think another approach to this is to make your elements into grabbers (no
wrapper object) much like how the rest of cr.ui is done. Then the sender and the
target would always be the same.

[Rick Byers, 2011/03/15 21:47:54]

Darn - I forgot that I added a 'sender' property myself, I was thinking it was a
built-in property of the DOM event model (like it is in other eventing patterns)
that I was just setting.  Sorry for the confusion.  I'm having trouble getting
used to the fact that adding a new property is syntactically identical to
changing the value of an existing property - seems like a flaw in the language
to me, but maybe I just need to get into the dynamic mindset more.

The cr.ui approach sounds interesting.  Is there a design doc or anything
anywhere for it?  I've looked over the code a bit and have the general idea, but
I don't have a clear picture of the pros/cons relative to normal wrapper objects
(a pattern I got from google3 code).

Note however that I don't think this would make 'the sender and the target
always be the same'.  The whole reason I need something in addition to target is
that in the HTML5 DND event model (which Grabber copies), some events
(enter/leave/drop) have two elements of interest - the source and the target.

So for now I've made my own custom event type that adds a property for the
source (potentially distinct from the target).  Since at this point the cr
utilities don't seem to buy me anything, I didn't bother taking the dependency. 
Let me know if you have any better ideas.  Also to help make this source/target
distinction more clear, I now always specify the target explicitly in calls to
'sendEvent'.

+  event.initEvent(eventType, true, true);
+  event.sender = this;
+  target.dispatchEvent(event);
+};
+
+/**
+ * Send a Grabber event to the grabbed element
+ * @param {string} eventType The type of event to send.
+ * @private
+ */
+Grabber.prototype.dispatchEvent_ = function(eventType) {
+  this.dispatchEventTo_(eventType, this.element_);
+};
+