Calculator App for ChromeOS with fixes for templates/CSS to support this application.

Calculator App for ChromeOS with fixes for templates/CSS to support this application.

TBR=jacobr@google.com
BUG=
TEST=

Committed: https://code.google.com/p/dart/source/detail?r=6231

[Jacob, 2012-04-05 18:06:01]

https://chromiumcodereview.appspot.com/10010009/diff/6004/samples/calculator/...
File samples/calculator/calcui.tmpl (right):

https://chromiumcodereview.appspot.com/10010009/diff/6004/samples/calculator/...
samples/calculator/calcui.tmpl:989: font-family: arial;
include fallback font options or remove line

https://chromiumcodereview.appspot.com/10010009/diff/6004/samples/calculator/...
samples/calculator/calcui.tmpl:1023: font-family: arial;
remove or add fallbacks

https://chromiumcodereview.appspot.com/10010009/diff/6004/samples/calculator/...
File samples/calculator/calculator.dart (right):

https://chromiumcodereview.appspot.com/10010009/diff/6004/samples/calculator/...
samples/calculator/calculator.dart:10: var padUI;          // Calculator Pad UI.
can this be given a type?

https://chromiumcodereview.appspot.com/10010009/diff/6004/samples/calculator/...
samples/calculator/calculator.dart:29: numberKeyPresses = new Set.from([48 /* 0
*/,
you could make this self documenting by instead of adding a comment defining
constants something like

const KEY_CODE_0 = 48
const KEY_CODE_PAD_0 = 96
and then reference those constants in the two places they are used

https://chromiumcodereview.appspot.com/10010009/diff/6004/samples/calculator/...
samples/calculator/calculator.dart:108:
padUI.keyZero.on.click.remove((MouseEvent e) { doCalc(48); });
this won't remove the event listeners attached above.
On modern browsers you likely don't actually need to remove event listeners for
this app. If you really want to remove event listeners for this, you'll need to
define the event listeners as static methods.

https://chromiumcodereview.appspot.com/10010009/diff/6004/samples/calculator/...
samples/calculator/calculator.dart:338: for (final String cls in key.classes) {
Looping through all css class names should be avoided as it is somewhat slow and
fragile.  add a data attribute if you need to store some data with an element.
e.g.
use
element.dataAttributes['someName']

and set an attribute with 
data-someName="value" in the html

https://chromiumcodereview.appspot.com/10010009/diff/6004/samples/calculator/...
samples/calculator/calculator.dart:354: window.setTimeout(() => resetKey(key,
classToAdd), 80);
make 80 a constant

https://chromiumcodereview.appspot.com/10010009/diff/6004/samples/calculator/...
samples/calculator/calculator.dart:360: final Element element = new
Element.tag('div');
nit:
final Element element ==> final element;

Also, why this extra layer of divs around the UI?

https://chromiumcodereview.appspot.com/10010009/diff/6004/samples/calculator/...
File samples/calculator/calculator.html (right):

https://chromiumcodereview.appspot.com/10010009/diff/6004/samples/calculator/...
samples/calculator/calculator.html:4: <script type="application/javascript"
src="calculator.js"></script>
why is this script in the body instead of the head?
Also, make script loading async.

https://chromiumcodereview.appspot.com/10010009/diff/6004/samples/calculator/...
File samples/calculator/settings.dart (right):

https://chromiumcodereview.appspot.com/10010009/diff/6004/samples/calculator/...
samples/calculator/settings.dart:11: int theme;
looks like 
int theme;
could be private

https://chromiumcodereview.appspot.com/10010009/diff/6004/samples/calculator/...
samples/calculator/settings.dart:49: "transparent" : "#333";
make #333 a constant at the top of the file

https://chromiumcodereview.appspot.com/10010009/diff/6004/samples/calculator/...
File samples/calculator/tape.dart (right):

https://chromiumcodereview.appspot.com/10010009/diff/6004/samples/calculator/...
samples/calculator/tape.dart:6: static final int OP_NOOP = 0;
looks like these could be const not final. same for other *enums*

https://chromiumcodereview.appspot.com/10010009/diff/6004/samples/calculator/...
samples/calculator/tape.dart:53: total /= numberAsValue;
do you want to do anything to avoid div by 0.?

https://chromiumcodereview.appspot.com/10010009/diff/6004/samples/calculator/...
samples/calculator/tape.dart:100: opAsStr = "+ ";
perhaps use a the concise Dart syntax for a map literal instead for this case?

https://chromiumcodereview.appspot.com/10010009/diff/6004/samples/calculator/...
samples/calculator/tape.dart:118: DivElement active = activeInput;
DivElement --> Element here and elsewhere
Casting to DivElement tends to serve no purpose other than to make code fragile.

https://chromiumcodereview.appspot.com/10010009/diff/6004/samples/calculator/...
samples/calculator/tape.dart:235: DivElement get activeInput() =>
window.document.query("#activeInput");
window.document.query --> document.query

or better yet

tapeUI.query if this item is under the tape ui.

https://chromiumcodereview.appspot.com/10010009/diff/6004/samples/calculator/...
samples/calculator/tape.dart:241: tapeUI.tape.elements.add(element.elements[0]);
i see the pattern of creating a div, setting some inner text and adding it to
the tape elements a lot. add a helper fn for it.

https://chromiumcodereview.appspot.com/10010009/diff/6004/utils/template/code...
File utils/template/codegen.dart (right):

https://chromiumcodereview.appspot.com/10010009/diff/6004/utils/template/code...
utils/template/codegen.dart:54: 
unreleated to this change but perhaps emit final instead of var. Looking at the
generated code a lot of the vars could by final

https://chromiumcodereview.appspot.com/10010009/diff/6004/utils/template/code...
utils/template/codegen.dart:472: buff.add("  String safeHTML(String html) {\n");
use multiline strings to make this cleaner

https://chromiumcodereview.appspot.com/10010009/diff/6004/utils/template/html...
File utils/template/htmltree.dart (right):

https://chromiumcodereview.appspot.com/10010009/diff/6004/utils/template/html...
utils/template/htmltree.dart:185: buff.add(' ${attr.toString()}');
i don't think the toString() is needed