Aggregate CSS manipulation functions in html lib.

Aggregate CSS manipulation functions in  html lib.

BUG=
R=blois@google.com

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

[Emily Fortuna, 2013-05-03 20:03:50]

https://codereview.chromium.org/14941002/diff/2001/tools/dom/src/CssClassSet....
File tools/dom/src/CssClassSet.dart (right):

https://codereview.chromium.org/14941002/diff/2001/tools/dom/src/CssClassSet....
tools/dom/src/CssClassSet.dart:197: void modify( f(Set<String> s)) {
The alternative here is to make _MultiElementCssClassSet implement CssClassSet
instead of extend, but there are a lot of methods defined in this class that
would add a lot of boilerplate to the other class. I figure "modify" is an
acceptable method to expose.

https://codereview.chromium.org/14941002/diff/2001/tools/dom/templates/html/i...
File tools/dom/templates/html/impl/impl_Element.darttemplate (right):

https://codereview.chromium.org/14941002/diff/2001/tools/dom/templates/html/i...
tools/dom/templates/html/impl/impl_Element.darttemplate:148: class HtmlList<T
extends Element> extends ListBase<T> {
Is this "T extends Element" correct? I'm confused because we're storing a
_nodeList of List<Node> and Node does not extend Element.... It affects whether
I need to filter the list in my "classes" methods.

[blois, 2013-05-03 20:13:37]

https://codereview.chromium.org/14941002/diff/2001/tools/dom/src/CssClassSet....
File tools/dom/src/CssClassSet.dart (right):

https://codereview.chromium.org/14941002/diff/2001/tools/dom/src/CssClassSet....
tools/dom/src/CssClassSet.dart:197: void modify( f(Set<String> s)) {
On 2013/05/03 20:03:50, Emily Fortuna wrote:
> The alternative here is to make _MultiElementCssClassSet implement CssClassSet
> instead of extend, but there are a lot of methods defined in this class that
> would add a lot of boilerplate to the other class. I figure "modify" is an
> acceptable method to expose.

Can we actually refactor this so CssClassSet is just an interface, then we have
an impl which resides in html_common?

I'm a bit worried about the public surface on this.

https://codereview.chromium.org/14941002/diff/2001/tools/dom/templates/html/i...
File tools/dom/templates/html/impl/impl_Document.darttemplate (right):

https://codereview.chromium.org/14941002/diff/2001/tools/dom/templates/html/i...
tools/dom/templates/html/impl/impl_Document.darttemplate:35: HtmlList<Element>
queryAll(String selectors) {
Can we make this ElementList<>?

https://codereview.chromium.org/14941002/diff/2001/tools/dom/templates/html/i...
File tools/dom/templates/html/impl/impl_Element.darttemplate (right):

https://codereview.chromium.org/14941002/diff/2001/tools/dom/templates/html/i...
tools/dom/templates/html/impl/impl_Element.darttemplate:148: class HtmlList<T
extends Element> extends ListBase<T> {
On 2013/05/03 20:03:50, Emily Fortuna wrote:
> Is this "T extends Element" correct? I'm confused because we're storing a
> _nodeList of List<Node> and Node does not extend Element.... It affects
whether
> I need to filter the list in my "classes" methods.

Can we make this HtmlList and have it just be an interface, then make
_FrozenElementList implement that?

I believe that <T extends Element> is correct.

[Emily Fortuna, 2013-05-03 21:14:22]

ptal

[blois, 2013-05-03 22:16:06]

https://codereview.chromium.org/14941002/diff/6001/tools/dom/templates/html/d...
File tools/dom/templates/html/dartium/html_dartium.darttemplate (right):

https://codereview.chromium.org/14941002/diff/6001/tools/dom/templates/html/d...
tools/dom/templates/html/dartium/html_dartium.darttemplate:33: part
'$AUXILIARY_DIR/CssClassSetImpl.dart';
This file isn't included in the CL.

It might need to be in html_common instead (does it need to be shared with SVG?,
any changes needed for SVG?)

https://codereview.chromium.org/14941002/diff/6001/tools/dom/templates/html/i...
File tools/dom/templates/html/impl/impl_Element.darttemplate (right):

https://codereview.chromium.org/14941002/diff/6001/tools/dom/templates/html/i...
tools/dom/templates/html/impl/impl_Element.darttemplate:144: abstract class
ElementList extends ListBase {
I'd make it:
ElementList<T extends Element> extends ListBase<T>

This way we can expose ElementList<OptionElement>.

https://codereview.chromium.org/14941002/diff/6001/tools/dom/templates/html/i...
tools/dom/templates/html/impl/impl_Element.darttemplate:197: e.classes.clear();
already a setter for classes, just:
e.classes = value?

[Emily Fortuna, 2013-05-03 23:00:09]

updated. ptal

https://codereview.chromium.org/14941002/diff/6001/tools/dom/templates/html/d...
File tools/dom/templates/html/dartium/html_dartium.darttemplate (right):

https://codereview.chromium.org/14941002/diff/6001/tools/dom/templates/html/d...
tools/dom/templates/html/dartium/html_dartium.darttemplate:33: part
'$AUXILIARY_DIR/CssClassSetImpl.dart';
On 2013/05/03 22:16:06, blois wrote:
> This file isn't included in the CL.
> 
> It might need to be in html_common instead (does it need to be shared with
SVG?,
> any changes needed for SVG?)

Done.

https://codereview.chromium.org/14941002/diff/6001/tools/dom/templates/html/i...
File tools/dom/templates/html/impl/impl_Element.darttemplate (right):

https://codereview.chromium.org/14941002/diff/6001/tools/dom/templates/html/i...
tools/dom/templates/html/impl/impl_Element.darttemplate:144: abstract class
ElementList extends ListBase {
On 2013/05/03 22:16:06, blois wrote:
> I'd make it:
> ElementList<T extends Element> extends ListBase<T>
> 
> This way we can expose ElementList<OptionElement>.
> 

Done.

https://codereview.chromium.org/14941002/diff/6001/tools/dom/templates/html/i...
tools/dom/templates/html/impl/impl_Element.darttemplate:197: e.classes.clear();
On 2013/05/03 22:16:06, blois wrote:
> already a setter for classes, just:
> e.classes = value?

Done.

[blois, 2013-05-03 23:20:18]

https://codereview.chromium.org/14941002/diff/18001/sdk/lib/html/html_common/...
File sdk/lib/html/html_common/css_class_set.dart (right):

https://codereview.chromium.org/14941002/diff/18001/sdk/lib/html/html_common/...
sdk/lib/html/html_common/css_class_set.dart:8: abstract class CssClassSet
implements Set<String> {
The public interface should remain in dart:html, not html_common.

https://codereview.chromium.org/14941002/diff/18001/tools/dom/templates/html/...
File tools/dom/templates/html/dart2js/html_dart2js.darttemplate (right):

https://codereview.chromium.org/14941002/diff/18001/tools/dom/templates/html/...
tools/dom/templates/html/dart2js/html_dart2js.darttemplate:35: part
'$AUXILIARY_DIR/CssClassSetImpl.dart';
Missing file?

[blois, 2013-05-03 23:32:27]

lgtm

[Emily Fortuna, 2013-05-06 00:04:26]

Committed patchset #6 manually as r22400 (presubmit successful).