Work around for content select not supporting touch events on it's children.

Work around for content select not supporting touch events on it's children.

BUG=327528
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=245626

[kevers, 2014-01-15 23:33:07]

https://codereview.chromium.org/140123002/diff/50001/ui/keyboard/resources/el...
File ui/keyboard/resources/elements/kb-keyboard.html (right):

https://codereview.chromium.org/140123002/diff/50001/ui/keyboard/resources/el...
ui/keyboard/resources/elements/kb-keyboard.html:342: // children.
Don't think it's necessary to revert to old method once supported.

https://codereview.chromium.org/140123002/diff/50001/ui/keyboard/resources/el...
ui/keyboard/resources/elements/kb-keyboard.html:366: // displayed by
content-select.
Nuke it.  Content select offers no real advantage over "display: none" for our
use case.

https://codereview.chromium.org/140123002/diff/50001/ui/keyboard/resources/el...
ui/keyboard/resources/elements/kb-keyboard.html:822: * @param {string | null}
oldLayout The previous layout.
?string

https://codereview.chromium.org/140123002/diff/50001/ui/keyboard/resources/el...
ui/keyboard/resources/elements/kb-keyboard.html:828:
keyset.classList.remove("activeKeyset");
keyset.hide() or alternatively why not leave the old keyset visible until the
new one is ready to display in order to avoid a flicker.  keyset.show() can take
care of hiding the previous keyset since only one keyset can be visible at a
time.

https://codereview.chromium.org/140123002/diff/50001/ui/keyboard/resources/el...
ui/keyboard/resources/elements/kb-keyboard.html:927: var nodes =
this.$.content.getDistributedNodes();
If we simply append children to the keyboard, I don't think there is any need to
use distributed nodes.

https://codereview.chromium.org/140123002/diff/50001/ui/keyboard/resources/el...
File ui/keyboard/resources/elements/kb-keyset.html (right):

https://codereview.chromium.org/140123002/diff/50001/ui/keyboard/resources/el...
ui/keyboard/resources/elements/kb-keyset.html:86:
this.classList.add('activeKeyset');
Worth checking if you're already active.  This method could also take care of
hiding the previous keyset.

https://codereview.chromium.org/140123002/diff/50001/ui/keyboard/resources/el...
ui/keyboard/resources/elements/kb-keyset.html:100: if (this.isDefault)
The stutter can be addressed by doing the equivalent of Polymer's veil/unveil. 
Simply add a class to the keyset that forces opacity to 0.  Once the keyset is
loaded and ready for display, pop off the "hide-me" class.

[rsadam, 2014-01-17 15:43:53]

https://codereview.chromium.org/140123002/diff/50001/ui/keyboard/resources/el...
File ui/keyboard/resources/elements/kb-keyboard.html (right):

https://codereview.chromium.org/140123002/diff/50001/ui/keyboard/resources/el...
ui/keyboard/resources/elements/kb-keyboard.html:342: // children.
On 2014/01/15 23:33:07, kevers wrote:
> Don't think it's necessary to revert to old method once supported.

Done.

https://codereview.chromium.org/140123002/diff/50001/ui/keyboard/resources/el...
ui/keyboard/resources/elements/kb-keyboard.html:366: // displayed by
content-select.
On 2014/01/15 23:33:07, kevers wrote:
> Nuke it.  Content select offers no real advantage over "display: none" for our
> use case.

Done.

https://codereview.chromium.org/140123002/diff/50001/ui/keyboard/resources/el...
ui/keyboard/resources/elements/kb-keyboard.html:822: * @param {string | null}
oldLayout The previous layout.
On 2014/01/15 23:33:07, kevers wrote:
> ?string

No longer needed

https://codereview.chromium.org/140123002/diff/50001/ui/keyboard/resources/el...
ui/keyboard/resources/elements/kb-keyboard.html:828:
keyset.classList.remove("activeKeyset");
On 2014/01/15 23:33:07, kevers wrote:
> keyset.hide() or alternatively why not leave the old keyset visible until the
> new one is ready to display in order to avoid a flicker.  keyset.show() can
take
> care of hiding the previous keyset since only one keyset can be visible at a
> time.

Done.

https://codereview.chromium.org/140123002/diff/50001/ui/keyboard/resources/el...
ui/keyboard/resources/elements/kb-keyboard.html:927: var nodes =
this.$.content.getDistributedNodes();
On 2014/01/15 23:33:07, kevers wrote:
> If we simply append children to the keyboard, I don't think there is any need
to
> use distributed nodes.

As we discussed offline, Polymer replaces appendChild, meaning it gets added
into the shadow dom.

https://codereview.chromium.org/140123002/diff/50001/ui/keyboard/resources/el...
File ui/keyboard/resources/elements/kb-keyset.html (right):

https://codereview.chromium.org/140123002/diff/50001/ui/keyboard/resources/el...
ui/keyboard/resources/elements/kb-keyset.html:86:
this.classList.add('activeKeyset');
On 2014/01/15 23:33:07, kevers wrote:
> Worth checking if you're already active.  This method could also take care of
> hiding the previous keyset.

Done.

https://codereview.chromium.org/140123002/diff/50001/ui/keyboard/resources/el...
ui/keyboard/resources/elements/kb-keyset.html:100: if (this.isDefault)
On 2014/01/15 23:33:07, kevers wrote:
> The stutter can be addressed by doing the equivalent of Polymer's veil/unveil.

> Simply add a class to the keyset that forces opacity to 0.  Once the keyset is
> loaded and ready for display, pop off the "hide-me" class.
Didn't have luck with veil/unveil. endOrMicroTask should suffice.

[kevers, 2014-01-17 17:56:35]

https://codereview.chromium.org/140123002/diff/120001/ui/keyboard/resources/e...
File ui/keyboard/resources/elements/kb-keyboard.html (right):

https://codereview.chromium.org/140123002/diff/120001/ui/keyboard/resources/e...
ui/keyboard/resources/elements/kb-keyboard.html:938: var allKeys =
keysets[i].querySelectorAll('*').array();
childNodes?

Also believe that this code belongs in the keyset expansion code and not here.

[rsadam, 2014-01-17 18:38:48]

https://codereview.chromium.org/140123002/diff/120001/ui/keyboard/resources/e...
File ui/keyboard/resources/elements/kb-keyboard.html (right):

https://codereview.chromium.org/140123002/diff/120001/ui/keyboard/resources/e...
ui/keyboard/resources/elements/kb-keyboard.html:938: var allKeys =
keysets[i].querySelectorAll('*').array();
On 2014/01/17 17:56:35, kevers wrote:
> childNodes?
> 
> Also believe that this code belongs in the keyset expansion code and not here.

Done.

[kevers, 2014-01-17 18:50:29]

lgtm with nit.

https://codereview.chromium.org/140123002/diff/190001/ui/keyboard/resources/m...
File ui/keyboard/resources/main.js (right):

https://codereview.chromium.org/140123002/diff/190001/ui/keyboard/resources/m...
ui/keyboard/resources/main.js:63: }
nit: Remove brackets.

[rsadam, 2014-01-17 19:12:33]

https://codereview.chromium.org/140123002/diff/190001/ui/keyboard/resources/m...
File ui/keyboard/resources/main.js (right):

https://codereview.chromium.org/140123002/diff/190001/ui/keyboard/resources/m...
ui/keyboard/resources/main.js:63: }
On 2014/01/17 18:50:30, kevers wrote:
> nit: Remove brackets.

Done.

[commit-bot: I haz the power, 2014-01-17 19:14:39]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rsadam@chromium.org/140123002/240001

[commit-bot: I haz the power, 2014-01-17 21:35:02]

Change committed as 245626