Add getElementByClass and getElementsByClass to goog.ui.Component

[GoogleCodeExporter]

It is common to fetch an element by class name under the DOM of a component. 
Using this.dom_.getElementByClass(className, this.element_) is fairly verbose, 
so this would provide convenience accessors.

Code review: http://codereview.appspot.com/4507041/

Includes unit test.

Original issue reported on code.google.com by bolinf...@gmail.com on 6 May 2011 at 11:13

[GoogleCodeExporter]

The components' internal structure including the structure of their DOM tree 
should be a black box to the rest of the code.

Adding getElementBy... methods would encourage programmers to directly access 
the internals of the component, which would lead to very fragile code.

If you want to find an element of a given class from inside the component, it's 
more efficient to keep a direct reference to it as a member variable.

Original comment by pall...@google.com on 7 May 2011 at 12:43

• Changed state: WontFix

[GoogleCodeExporter]

I came to Google yesterday and discussed this with Nathan Naze, Garry Boyer, 
and Dan Pupius, and all were in agreement that this should go in ASAP.

Original comment by bolinf...@gmail.com on 7 May 2011 at 12:45

[GoogleCodeExporter]

It would be helpful to remove the WontFix status so this will come up in the 
Issues UI so it is easier to find and we can continue discussion.

Original comment by bolinf...@gmail.com on 7 May 2011 at 12:46

[GoogleCodeExporter]

If you believe the methods should be protected rather than private, then that's 
a reasonable objection, but these are doing something that's already possible 
using the public API of goog.ui.Component.

Original comment by bolinf...@gmail.com on 7 May 2011 at 12:47

[GoogleCodeExporter]

I changed the status back to new but would like to see a legitimate use case 
behind this change.

Original comment by pall...@google.com on 7 May 2011 at 12:51

• Changed state: New

[GoogleCodeExporter]

Could you also ask the original designer of the Component and Control classes, 
Attila's opinion? IMO he would be the best person to decide on such changes.

Original comment by pall...@google.com on 7 May 2011 at 12:57

• Added labels: Type-CodeReview
• Removed labels: Type-Defect

[GoogleCodeExporter]

This change is one of three:

http://codereview.appspot.com/4507041/
http://codereview.appspot.com/4509041/
(third which will introduce addChildAtClass)

This is designed to facilitate the common pattern where:

(1) You define the HTML for your template in a Soy file.
(2) You use the template to create the DOM in createDom().
(3) You do not want to be forced to add all child components under the same 
content element. Instead, you should define classes in your template that 
represent "attachment points" where child components should be added. Using 
getElementByClass() makes it straightforward to grab that attachment point in 
createDom() and add the child there, as you should.

This is also a benefit because many developers do things in enterDocument() 
that they should do in createDom() because it is so much easier to use 
getElement() than getElementByTagNameAndClass(); however, getElement() only 
works once the DOM is in the document, which is why the code ends up in the 
wrong spot (in enterDocument()). By providing better accessors for elements by 
class name within a component, it would help discourage that incorrect behavior.

Given all that, I would be fine removing the getElementsByClass() method from 
this changelist and add only getElementByClass(), but it was just as easy to 
include it for completeness.

Original comment by bolinf...@gmail.com on 7 May 2011 at 1:05

[GoogleCodeExporter]

I agree -- looping Attila in sounds like a good idea.

Original comment by bolinf...@gmail.com on 7 May 2011 at 1:06

[GoogleCodeExporter]

Seeing Component usage on my current project I think that this is a definite 
improvement to the API.  I see too many cases of people forgetting to scope 
domHelper.getElementByClassName to the current element.

It is also such a common task when dealing with template driven components that 
I think it'd be worth making it a shorter name, e.g. byClass

Original comment by dan.pup...@gmail.com on 14 May 2011 at 12:13

[GoogleCodeExporter]

I would also be in favor of a shorter name, like "byClass." If you're
willing to approve it, then I'm happy to change it.

Original comment by bolinf...@gmail.com on 14 May 2011 at 3:27

[GoogleCodeExporter]

Anyone object?

Original comment by dan.pup...@gmail.com on 16 May 2011 at 4:20

[GoogleCodeExporter]

fixed.

http://code.google.com/p/closure-library/source/detail?r=989

Original comment by nn...@google.com on 2 Jun 2011 at 5:03

[GoogleCodeExporter]

Original comment by nn...@google.com on 2 Jun 2011 at 5:04

• Changed state: Fixed