Extend the sky color picker example

Extend the sky color picker example

Displays the last six selected colors.


BUG=
R=abarth@chromium.org

Committed: https://chromium.googlesource.com/external/mojo/+/1442f3f9566eed618a392590efa9c1fda6b60830

[hansmuller1, 2015-02-03 20:40:44]

hansmuller@google.com changed reviewers:
+ abarth@chromium.org, esprehn@chromium.org

[hansmuller1, 2015-02-03 20:45:19]

hansmuller@google.com changed reviewers:
+ hansmuller@google.com

[hansmuller1, 2015-02-03 20:45:20]

I wrote this to familiarize myself with the sky repeating template. I was
pleased to learn that one could track foo property changes by just adding a
function valued attribute called fooChanged. It would be nice if one could
override the sky-box title font. I wish the CSS last-child: psuedo-selector
worked.

PTAL.

[abarth-chromium, 2015-02-04 02:40:31]

https://codereview.chromium.org/897683002/diff/20002/sky/examples/color/color...
File sky/examples/color/color-picker.sky (right):

https://codereview.chromium.org/897683002/diff/20002/sky/examples/color/color...
sky/examples/color/color-picker.sky:37: var colorWheelElt =
this.shadowRoot.getElementById('color-wheel-element');
Usually you want to do this sort of thing in shadowRootReady.

https://codereview.chromium.org/897683002/diff/20002/sky/examples/color/color...
sky/examples/color/color-picker.sky:38: colorWheelElt.colorChanged = (function
() {
Rather than overwriting this project on the child element, I'd recommend having
the color-wheel watch for colorChanged and then fire an event on itself.  Then
this element can watch for the event with the on-foo syntax.

[esprehn, 2015-02-04 02:46:23]

My design for the framework is to have <sky-theme> that defines a sheet to
inject into elements. So you can do <sky-box theme="foo"> and elsewhere write
<sky-theme name="foo"><style>...</style></sky-theme> to shove styles into it.

For last-child we need to think harder, we can either use virtual DOM and tree
diffing like react, or we can expose a callback that runs whenever the <template
repeat> changes so you can "fix up" the classes inside it.

[hansmuller1, 2015-02-04 17:00:12]

On 2015/02/04 02:46:23, esprehn wrote:
> My design for the framework is to have <sky-theme> that defines a sheet to
> inject into elements. So you can do <sky-box theme="foo"> and elsewhere write
> <sky-theme name="foo"><style>...</style></sky-theme> to shove styles into it.

That sounds like a good idea. Layout dependencies might make themes complicated.
Do I switch themes for different aspect ratios and resolutions? 

> 
> For last-child we need to think harder, we can either use virtual DOM and tree
> diffing like react, or we can expose a callback that runs whenever the
<template
> repeat> changes so you can "fix up" the classes inside it.

I guess the last-child: pseudo-selector hides a pile of complexity although the
result is easier to read then getting the same effect with a callback. Doing the
work with a callback might be brittle wrt the structure of the template and it
wouldn't work if the tree changed for some other reason (which doesn't matter in
this case). But a callback would be sufficient in this case.

[hansmuller1, 2015-02-04 17:02:24]

Thanks for the feedback. I've made the suggested changes.

https://codereview.chromium.org/897683002/diff/20002/sky/examples/color/color...
File sky/examples/color/color-picker.sky (right):

https://codereview.chromium.org/897683002/diff/20002/sky/examples/color/color...
sky/examples/color/color-picker.sky:37: var colorWheelElt =
this.shadowRoot.getElementById('color-wheel-element');
On 2015/02/04 02:40:31, abarth wrote:
> Usually you want to do this sort of thing in shadowRootReady.

Done.

https://codereview.chromium.org/897683002/diff/20002/sky/examples/color/color...
sky/examples/color/color-picker.sky:38: colorWheelElt.colorChanged = (function
() {
On 2015/02/04 02:40:31, abarth wrote:
> Rather than overwriting this project on the child element, I'd recommend
having
> the color-wheel watch for colorChanged and then fire an event on itself.  Then
> this element can watch for the event with the on-foo syntax.

Done.

[abarth-chromium, 2015-02-04 17:23:52]

LGTM

https://codereview.chromium.org/897683002/diff/40001/sky/examples/color/color...
File sky/examples/color/color-picker.sky (right):

https://codereview.chromium.org/897683002/diff/40001/sky/examples/color/color...
sky/examples/color/color-picker.sky:12: <sky-element name="color-picker"
on-color-change="updateColorSamples">
You can also put this directly on the <color-wheel> element if you like.

https://codereview.chromium.org/897683002/diff/40001/sky/examples/color/color...
File sky/examples/color/color-samples.sky (right):

https://codereview.chromium.org/897683002/diff/40001/sky/examples/color/color...
sky/examples/color/color-samples.sky:24: }
I'd remove this given that it doesn't work.

[abarth-chromium, 2015-02-04 17:24:48]

On 2015/02/04 at 17:00:12, hansmuller wrote:
> I guess the last-child: pseudo-selector hides a pile of complexity although
the result is easier to read then getting the same effect with a callback. Doing
the work with a callback might be brittle wrt the structure of the template and
it wouldn't work if the tree changed for some other reason (which doesn't matter
in this case). But a callback would be sufficient in this case.

Yeah, we've removed many features like last-child from selectors.  That's why
we're able to compute style so fast.

[hansmuller, 2015-02-04 17:29:25]

New patchsets have been uploaded after l-g-t-m from abarth@chromium.org

[hansmuller1, 2015-02-04 17:30:32]

https://codereview.chromium.org/897683002/diff/40001/sky/examples/color/color...
File sky/examples/color/color-samples.sky (right):

https://codereview.chromium.org/897683002/diff/40001/sky/examples/color/color...
sky/examples/color/color-samples.sky:24: }
On 2015/02/04 17:23:51, abarth wrote:
> I'd remove this given that it doesn't work.

Good point. I'd only put it there to get some feedback about :last-child.

[hansmuller, 2015-02-04 17:30:55]

Committed patchset #5 (id:60001) manually as
1442f3f9566eed618a392590efa9c1fda6b60830 (presubmit successful).