Sky color chooser example

sky/examples/color/color-wheel.sky

+<import src="../../framework/sky-element/sky-element.sky" as="SkyElement" />

[abarth-chromium, 2015/01/31 03:20:18]

Please add a copyright block.

[hansmuller1, 2015/02/02 16:32:30]

Done.

+
+<sky-element name="color-wheel"
+    attributes="color:string"
+    on-pointerdown="handlePointerDown"
+    on-pointermove="handlePointerMove"
+    on-pointerup="handlePointerUp">

[abarth-chromium, 2015/01/31 03:20:18]

You should listen for on-pointercancel as well.  You get that event instead of
pointerup when the finger moves off the screen.

[hansmuller1, 2015/02/02 16:32:30]

Done.

+<template>
+  <style>
+    color-picker {
+      margin: 20px;
+    }

[abarth-chromium, 2015/01/31 03:20:18]

This style doesn't appear to apply to anything.

[abarth-chromium, 2015/01/31 03:20:18]

This style doesn't appear to apply to anything.

[hansmuller1, 2015/02/02 16:32:30]

Oops. Removed that.

+    .color-wheel-img {

[abarth-chromium, 2015/01/31 03:20:18]

There's no need for this class.  You can just use

img { ... }

The style is scoped to this component so there's no risk of styling other
things.

[hansmuller1, 2015/02/02 16:32:30]

Done.

+      min-width: 100%;
+      height: auto;
+    }
+  </style>
+  <img class="color-wheel-img" src="color-wheel.png">
+</template>
+<script>
+function toHexString(n) {
+  var s = Number(n).toString(16).toUpperCase();
+  return (s.length == 1) ? "0" + s : s;
+}
+
+class Color {
+  constructor(r, g, b) {
+    this.r = r;
+    this.g = g;
+    this.b = b;

[esprehn, 2015/02/01 12:28:43]

Object.preventExtensions(this)

[hansmuller1, 2015/02/02 16:32:30]

I got rid of this class, since it ended up with the same just-a-function purpose
as ColorWheel. 

+  }
+  toString() {
+    return "#" + toHexString(this.r) + toHexString(this.g) + toHexString(this.b);
+  }
+}
+
+function HSVtoRGB(h, s, v) {

[esprehn, 2015/02/01 12:28:43]

hsvToRgb()

Treat acronyms like words.

[hansmuller1, 2015/02/02 16:32:30]

Done.

+  var i = Math.floor(h * 6);
+  var f = h * 6 - i;
+  var p = v * (1 - s);
+  var q = v * (1 - f * s);
+  var t = v * (1 - (1 - f) * s);
+  var r, g, b;
+  switch (i % 6) {
+    case 0: r = v, g = t, b = p; break;
+    case 1: r = q, g = v, b = p; break;
+    case 2: r = p, g = v, b = t; break;
+    case 3: r = p, g = q, b = v; break;
+    case 4: r = t, g = p, b = v; break;
+    case 5: r = v, g = p, b = q; break;

[esprehn, 2015/02/01 12:28:43]

All these single letter vars are pretty hard to read, also all the assignments
on one line.

[hansmuller1, 2015/02/02 16:32:30]

I removed the re-declaration of r,g,b (and oops). The arithmetic for RGB to HSV
is intrinsically hard to read, I don't think making up long variable names for
the intermediate results would help.

+  }
+  var r = Math.floor(r * 255);
+  var g = Math.floor(g * 255);
+  var b = Math.floor(b * 255);
+  return new Color(r, g, b);
+}
+
+class ColorWheel {

[esprehn, 2015/02/01 12:28:43]

This should just be a function, it has no state.

[hansmuller1, 2015/02/02 16:32:30]

Done.

+  xyToRGB(x, y, radius) {
+    var rx = x - radius;
+    var ry = y - radius;
+    var d = radius * radius;
+    if (rx * rx + ry * ry > d)
+      return undefined;
+    var h = (Math.atan2(ry, rx) + Math.PI) / (2 * Math.PI);
+    var s = Math.sqrt(d) / radius;
+    return HSVtoRGB(h, s, 1);
+  }
+}
+
+module.exports = class extends SkyElement {
+  created() {
+    super.created();
+    this.pointerIsDown = false;
+    this.colorWheel = new ColorWheel;
+    this.color = "#xFF00FF";
+  }
+  updateColor(event) {
+    var bounds = event.target.getBoundingClientRect();
+    var x = event.x - bounds.left;
+    var y = event.y - bounds.top;
+    var r = Math.min(bounds.width, bounds.height) / 2.0;
+    var rgb = this.colorWheel.xyToRGB(x, y, r);
+    if (rgb)
+      this.color = rgb.toString();
+  }
+  handlePointerDown(event) {
+    this.pointerIsDown = true;
+    this.updateColor(event);
+  }
+  handlePointerMove(event) {
+    if (this.pointerIsDown)

[abarth-chromium, 2015/01/31 03:20:18]

You'll never get a pointermove event unless the pointer is down, so you don't
need the pointerIsDown member variable.

[hansmuller1, 2015/02/02 16:32:30]

Done.

+      this.updateColor(event);
+  }
+  handlePointerUp(event) {
+   this.pointerIsDown = false;
+  }
+}.register();
+</script>
+</sky-element>