Major revision of page cycler UI.

chrome/common/extensions/docs/examples/apps/cycler/cycler_ui.js

+// Copyright (c) 2012 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.
+
+function $(criterion) {
+  return document.querySelector(criterion);
+}
+
+var cyclerUI = new (function () {
+
+  /**
+   * Enum for different UI states, either capture or playback.

[Jeffrey Yasskin, 2012/08/15 01:03:59]

You can omit "capture or playback" since those are the names of the enum values.

[clintstaley, 2012/08/15 23:54:20]

Done.

+   * @enum {number}
+   * @private
+   */
+  var EnableState_ = {capture: 0, playback: 1};
+
+  // Members for all UI elements subject to programmatic adjustment.
+  this.captureTab_ = $('#capture-tab');
+  this.captureTabLabel_ = $('#capture-tab-label');
+
+  this.playbackTab_ = $('#playback-tab');
+  this.playbackTabLabel_ = $('#playback-tab-label');
+
+  this.popupDialog_ = $('#popup');
+  this.popupContent_ = $('#popup-content');
+  this.doPopupDismiss_ = $('#do-popup-dismiss');
+
+  /**
+   * Name of the most recent capture made, or the one most recently chosen
+   * for playback.
+   * @type {!string}
+   */
+  Object.defineProperty(this, 'currentCaptureName',

[Jeffrey Yasskin, 2012/08/15 01:03:59]

Why use defineProperty instead of simple assignment?

[clintstaley, 2012/08/15 23:54:20]

Seemed it was widely used in other webui JS, and also just good general OO
discipline.  I suppose I could remove it and put it back only if some
housekeeping turns out to be needed.  IIUC, the syntax of using the property
won't change if I do so.

Done.

[Jeffrey Yasskin, 2012/08/16 20:19:31]

I think the good OO practice is to write getters and setters for properties, or
to use defineProperty to, say, restrict the writability or deletability of a
field. In Python, where @property acts similarly to Object.defineProperty, it's
better practice to leave user-accessible fields directly accessible until you
need to restrict them or modify their behavior, and only then write a full
property. I assume the guidelines are the same in JS.

[clintstaley, 2012/08/17 04:12:25]

This way in C# too.  Wish they'd add properties to Java.  So much nicer than all
the getter/setter noise.

+      {value: null, writable: true});
+
+  this.cyclerData_ = new CyclerData();
+  this.playbackTab_ = new PlaybackTab(this, this.cyclerData_);

[Jeffrey Yasskin, 2012/08/15 01:03:59]

This overwrites an existing variable. Which did you want to keep?

[clintstaley, 2012/08/15 23:54:20]

Oops :).  Thanks.  (Ditto for captureTab, of course.)

Done

+  this.captureTab_ = new CaptureTab(this, this.cyclerData_, this.playbackTab_);
+
+  /*
+   * Enable the capture tab, changing tab labels approproiately.
+   * @private
+   */
+  this.enableCapture_ = function() {
+    if (this.enableState != EnableState_.capture) {

[Jeffrey Yasskin, 2012/08/15 01:03:59]

Where does enableState get initialized? I guess this works because it's
initially 'undefined', which is != to everything? I'm not sure that's good
style.

[clintstaley, 2012/08/15 23:54:20]

Yes, that was my intent. Wasn't sure if it was good JS style or not.  People
tend to just make up properties as they like in this language :).  I'll add a
default setting to null.  I don't want to set it to EnableState_.capture, since
that should go along with the housekeeping that matches it, and will happen in
the call at the bottom.

+      this.enableState = EnableState_.capture;
+
+      this.captureTabLabel_.classList.add('selected');
+      this.captureTab_.enable();

[Jeffrey Yasskin, 2012/08/15 01:03:59]

Arguably, it seems like CaptureTab.enable should imply marking the label as
selected.

[clintstaley, 2012/08/15 23:54:20]

It also would have to know to deselect all the other labels.  Global knowledge
of what tab labels there are, and which need selection and deselection, seemed
the realm of the overarching UI class.  But now I realize I could simply do
label deselection in the disable methods.  So, fixed.

+
+      this.playbackTabLabel_.classList.remove('selected');
+      this.playbackTab_.disable();
+    }
+  };
+
+  /*
+   * Enable the playback tab, changing tab labels approproiately.
+   * @private
+   */
+  this.enablePlayback_ = function() {
+    if (this.enableState != EnableState_.playback) {
+      this.enableState = EnableState_.playback;
+
+      this.captureTabLabel_.classList.remove('selected');
+      this.captureTab_.disable();
+
+      this.playbackTabLabel_.classList.add('selected');
+      this.playbackTab_.enable();
+    }
+  };
+
+  /**
+   * Show an overlay with a message, a dismiss button with configurable
+   * label, and an action to call upon dismissal.
+   * @param {!string} content The message to display.
+   * @param {!string} dismissLabel The label on the dismiss button.
+   * @param {function()} action Additional action to take, if any, upon
+   *     dismissal.
+   */
+  this.showMessage = function(content, dismissLabel, action) {
+    this.popupContent_.innerText = content;
+    this.doPopupDismiss_.innerText = dismissLabel;
+    this.popupDialog_.hidden = false;
+    if (action != null)
+      doPopupDismiss_.addEventListener('click', action);
+  }
+
+  /**
+   * Default action for popup dismissal button, performed in addition to
+   * any other actions that may be specified in showMessage_ call.
+   * @private
+   */
+  this.clearMessage_ = function() {
+    this.popupDialog_.hidden = true;
+  }
+
+  // Set up listeners on all buttons.
+  this.doPopupDismiss_.addEventListener('click', this.clearMessage_.bind(this));
+
+  // Set up listeners on tab labels.
+  this.captureTabLabel_.addEventListener('click',
+      this.enableCapture_.bind(this));
+  this.playbackTabLabel_.addEventListener('click',
+      this.enablePlayback_.bind(this));
+
+  // Start with capture tab displayed.
+  this.enableCapture_();
+})();