Recipes for using CSS Selectors with dart:html

Recipes for using CSS Selectors with dart:html

[shailentuli, 2013-04-29 20:31:57]

Please review this short CL with 2 dart:html recipes.  The files to review are:

selectors.asciidoc
scoped_selectors.asciidoc

The staged versions can be read at: 

https://github.com/dart-lang/cookbook/blob/html/selectors.asciidoc
https://github.com/dart-lang/cookbook/blob/html/scoped_selectors.asciidoc

I have also pushed a whole bunch of .html files that contain the code of these
are other (in-progress) recipes.  *You don't have to review these*. But if you
want to refer to them, the files that are relevant to these 2 recipes are:

selectors.html
manipulating_queryAll_results.html
scoped_selectors.html

[sethladd, 2013-04-29 20:59:01]

https://chromiumcodereview.appspot.com/14109034/diff/2001/selectors.asciidoc
File selectors.asciidoc (right):

https://chromiumcodereview.appspot.com/14109034/diff/2001/selectors.asciidoc#...
selectors.asciidoc:9: Use the top level `query()` and `queryAll()` functions
provided in the
provided in or provided from?

https://chromiumcodereview.appspot.com/14109034/diff/2001/selectors.asciidoc#...
selectors.asciidoc:27: <li class='mustHave'>Cereal
typically, class names are like must-have

https://chromiumcodereview.appspot.com/14109034/diff/2001/selectors.asciidoc#...
selectors.asciidoc:44: window.console.log(element.id);                 //
'first'
please use print() instead

https://chromiumcodereview.appspot.com/14109034/diff/2001/selectors.asciidoc#...
selectors.asciidoc:52: elements = queryAll('#first, .mustHave');
a comma is like an "or", so this is #first OR .mustHave. Is this what you mean?

https://chromiumcodereview.appspot.com/14109034/diff/2001/selectors.asciidoc#...
selectors.asciidoc:61: window.console.log(elements.first.innerHtml);   // 'Bran
Flakes'
wouldn't elements.first.text work here?

https://chromiumcodereview.appspot.com/14109034/diff/2001/selectors.asciidoc#...
selectors.asciidoc:83: Calling `queryAll()` returns a readonly list of DOM
elements:
read-only ?

https://chromiumcodereview.appspot.com/14109034/diff/2001/selectors.asciidoc#...
selectors.asciidoc:100: void doSomethingWith(element) {}
You don't use this function here. Old?

https://chromiumcodereview.appspot.com/14109034/diff/2001/selectors.asciidoc#...
selectors.asciidoc:112: You can perform regular List operations on the results
of `queryAll()`:
this is confusing to me, injecting this text in the middle of a snippet. Can you
reorder things so that the code all flows?

https://chromiumcodereview.appspot.com/14109034/diff/2001/selectors.asciidoc#...
selectors.asciidoc:116: window.console.log(elements[2].innerHtml);   // 'Reddit'
global: use print()

https://chromiumcodereview.appspot.com/14109034/diff/2001/selectors.asciidoc#...
selectors.asciidoc:125: var sites = elements.map((site) => site.innerHtml);
you can pull .join() up to this line

https://chromiumcodereview.appspot.com/14109034/diff/2001/selectors.asciidoc#...
selectors.asciidoc:125: var sites = elements.map((site) => site.innerHtml);
use .text if it works

https://chromiumcodereview.appspot.com/14109034/diff/2001/selectors.asciidoc#...
selectors.asciidoc:128: // Filter results.
you say filter, but you also reverse. Make this more simple, pull out the
reversed

https://chromiumcodereview.appspot.com/14109034/diff/2001/selectors.asciidoc#...
selectors.asciidoc:131: sites.map((site) => site.innerHtml).first); //
"StackOverflow"
pull the map out of the print()

https://chromiumcodereview.appspot.com/14109034/diff/2001/selectors.asciidoc#...
selectors.asciidoc:143: Trying to modify the results of `queryAll()` generates
an error:
Please explain why.

[Kathy Walrath, 2013-04-29 21:17:02]

https://chromiumcodereview.appspot.com/14109034/diff/2001/scoped_selectors.as...
File scoped_selectors.asciidoc (right):

https://chromiumcodereview.appspot.com/14109034/diff/2001/scoped_selectors.as...
scoped_selectors.asciidoc:5: You have a handle on a selected DOM element and
want to search for other
handle on -> handle to

(Now I'm getting confused about what "selected" means here. I was thinking CSS,
but then I started wondering if you meant radio button or checkbox.)

Is it necessary to use "selected" here? How about just:

You want to find elements that are contained by a particular element.

https://chromiumcodereview.appspot.com/14109034/diff/2001/scoped_selectors.as...
scoped_selectors.asciidoc:10: Use the `query()` and `queryAll()` functions as
methods of the currently
I found this a bit confusing and wasn't sure it was correct. How about just:

Instead of using the top-level `query()` or `queryAll()` function, use the
Element method with the same name.

https://chromiumcodereview.appspot.com/14109034/diff/2001/scoped_selectors.as...
scoped_selectors.asciidoc:11: selected element. This restricts the scope of the
query to the currently
"This" -> Using one of these methods restricts the scope of the query to the
element's descendants.

https://chromiumcodereview.appspot.com/14109034/diff/2001/scoped_selectors.as...
scoped_selectors.asciidoc:15: currentlySelectedElement.query(cssSelector);
currentlySelectedElement -> containerElement

(global)

https://chromiumcodereview.appspot.com/14109034/diff/2001/scoped_selectors.as...
scoped_selectors.asciidoc:36: queryAll('tr').forEach((element) {
I was initially a bit confused when I saw this because the queryAll wasn't
invoked on an element.

https://chromiumcodereview.appspot.com/14109034/diff/2001/scoped_selectors.as...
scoped_selectors.asciidoc:44: Because of the scoped query, non-descendant <td>s
with the '.status' class are
Could be clearer. How about:

Because the query() method is invoked on the just-clicked row, status cells in
other rows are ignored.

(I can't think of a good way to get rid of the passive voice. It might be
appropriate here.)

https://chromiumcodereview.appspot.com/14109034/diff/2001/selectors.asciidoc
File selectors.asciidoc (right):

https://chromiumcodereview.appspot.com/14109034/diff/2001/selectors.asciidoc#...
selectors.asciidoc:9: Use the top level `query()` and `queryAll()` functions
provided in the
top level -> top-level

And I'd say "provided by"

https://chromiumcodereview.appspot.com/14109034/diff/2001/selectors.asciidoc#...
selectors.asciidoc:37: 
delete this blank line?

https://chromiumcodereview.appspot.com/14109034/diff/2001/selectors.asciidoc#...
selectors.asciidoc:42: // Find by id.
id => ID

https://chromiumcodereview.appspot.com/14109034/diff/2001/selectors.asciidoc#...
selectors.asciidoc:83: Calling `queryAll()` returns a readonly list of DOM
elements:
On 2013/04/29 20:59:01, sethladd wrote:
> read-only ?

Yes.

Although I think you can just leave out "read only". It made me question, for a
moment, whether it was the list or the elements that are read only. I think you
can leave it for later.

[Andrei Mouravski, 2013-04-29 21:21:43]

Part 1.

https://codereview.chromium.org/14109034/diff/2001/scoped_selectors.asciidoc
File scoped_selectors.asciidoc (right):

https://codereview.chromium.org/14109034/diff/2001/scoped_selectors.asciidoc#...
scoped_selectors.asciidoc:5: You have a handle on a selected DOM element and
want to search for other
A handle?

https://codereview.chromium.org/14109034/diff/2001/scoped_selectors.asciidoc#...
scoped_selectors.asciidoc:5: You have a handle on a selected DOM element and
want to search for other
You want to find an element that is a child of a selected DOM element. Or
something.

https://codereview.chromium.org/14109034/diff/2001/scoped_selectors.asciidoc#...
scoped_selectors.asciidoc:10: Use the `query()` and `queryAll()` functions as
methods of the currently
"As methods of the..." is weird.

Call the `query` or `queryAll` methods on the selected element.

https://codereview.chromium.org/14109034/diff/2001/scoped_selectors.asciidoc#...
scoped_selectors.asciidoc:29: </table>
Have two tables, or maybe they can be ULs and give them unique IDs or something,
and then have the second one have a group of people, one of whom is Donny.

Then, use "queryAll" on the first table/UL and do the same as you do before, but
Donny is not affected, because he's out of his element.

https://codereview.chromium.org/14109034/diff/2001/scoped_selectors.asciidoc#...
scoped_selectors.asciidoc:44: Because of the scoped query, non-descendant <td>s
with the '.status' class are
"Because the query is scoped to whatever, ..."

[shailentuli, 2013-04-30 02:59:31]

Ready for second round of reviews.  Many thanks for your comments.

https://chromiumcodereview.appspot.com/14109034/diff/2001/scoped_selectors.as...
File scoped_selectors.asciidoc (right):

https://chromiumcodereview.appspot.com/14109034/diff/2001/scoped_selectors.as...
scoped_selectors.asciidoc:5: You have a handle on a selected DOM element and
want to search for other
On 2013/04/29 21:17:02, Kathy Walrath wrote:
> handle on -> handle to
> 
> (Now I'm getting confused about what "selected" means here. I was thinking
CSS,
> but then I started wondering if you meant radio button or checkbox.)
> 
> Is it necessary to use "selected" here? How about just:
> 
> You want to find elements that are contained by a particular element.

Done.

https://chromiumcodereview.appspot.com/14109034/diff/2001/scoped_selectors.as...
scoped_selectors.asciidoc:5: You have a handle on a selected DOM element and
want to search for other
On 2013/04/29 21:21:44, Andrei Mouravski wrote:
> You want to find an element that is a child of a selected DOM element. Or
> something.

Using Kathy's suggestion, above.

https://chromiumcodereview.appspot.com/14109034/diff/2001/scoped_selectors.as...
scoped_selectors.asciidoc:10: Use the `query()` and `queryAll()` functions as
methods of the currently
On 2013/04/29 21:17:02, Kathy Walrath wrote:
> I found this a bit confusing and wasn't sure it was correct. How about just:
> 
> Instead of using the top-level `query()` or `queryAll()` function, use the
> Element method with the same name.

Done.

https://chromiumcodereview.appspot.com/14109034/diff/2001/scoped_selectors.as...
scoped_selectors.asciidoc:10: Use the `query()` and `queryAll()` functions as
methods of the currently
On 2013/04/29 21:21:44, Andrei Mouravski wrote:
> "As methods of the..." is weird.
> 
> Call the `query` or `queryAll` methods on the selected element.

Done.

https://chromiumcodereview.appspot.com/14109034/diff/2001/scoped_selectors.as...
scoped_selectors.asciidoc:11: selected element. This restricts the scope of the
query to the currently
On 2013/04/29 21:17:02, Kathy Walrath wrote:
> "This" -> Using one of these methods restricts the scope of the query to the
> element's descendants.

Done.

https://chromiumcodereview.appspot.com/14109034/diff/2001/scoped_selectors.as...
scoped_selectors.asciidoc:15: currentlySelectedElement.query(cssSelector);
On 2013/04/29 21:17:02, Kathy Walrath wrote:
> currentlySelectedElement -> containerElement
> 
> (global)

Done.

https://chromiumcodereview.appspot.com/14109034/diff/2001/scoped_selectors.as...
scoped_selectors.asciidoc:29: </table>
On 2013/04/29 21:21:44, Andrei Mouravski wrote:
> Have two tables, or maybe they can be ULs and give them unique IDs or
something,
> and then have the second one have a group of people, one of whom is Donny.
> 
> Then, use "queryAll" on the first table/UL and do the same as you do before,
but
> Donny is not affected, because he's out of his element.

WITTIEST CL COMMENT EVER. But, no, alas, Donny will have to stay out of his
element without any help from me.  Leaving this as is.

https://chromiumcodereview.appspot.com/14109034/diff/2001/scoped_selectors.as...
scoped_selectors.asciidoc:29: </table>
On 2013/04/29 21:21:44, Andrei Mouravski wrote:
> Have two tables, or maybe they can be ULs and give them unique IDs or
something,
> and then have the second one have a group of people, one of whom is Donny.
> 
> Then, use "queryAll" on the first table/UL and do the same as you do before,
but
> Donny is not affected, because he's out of his element.

Done.

https://chromiumcodereview.appspot.com/14109034/diff/2001/scoped_selectors.as...
scoped_selectors.asciidoc:36: queryAll('tr').forEach((element) {
On 2013/04/29 21:17:02, Kathy Walrath wrote:
> I was initially a bit confused when I saw this because the queryAll wasn't
> invoked on an element.

Yup, it is a bit confusing. Adding a note at the end of the recipe about the
scope of a top-level queryAll().

https://chromiumcodereview.appspot.com/14109034/diff/2001/scoped_selectors.as...
scoped_selectors.asciidoc:44: Because of the scoped query, non-descendant <td>s
with the '.status' class are
On 2013/04/29 21:17:02, Kathy Walrath wrote:
> Could be clearer. How about:
> 
> Because the query() method is invoked on the just-clicked row, status cells in
> other rows are ignored.
> 
> (I can't think of a good way to get rid of the passive voice. It might be
> appropriate here.)

Done.

https://chromiumcodereview.appspot.com/14109034/diff/2001/scoped_selectors.as...
scoped_selectors.asciidoc:44: Because of the scoped query, non-descendant <td>s
with the '.status' class are
On 2013/04/29 21:17:02, Kathy Walrath wrote:
> Could be clearer. How about:
> 
> Because the query() method is invoked on the just-clicked row, status cells in
> other rows are ignored.
> 
> (I can't think of a good way to get rid of the passive voice. It might be
> appropriate here.)

Done.

https://chromiumcodereview.appspot.com/14109034/diff/2001/selectors.asciidoc
File selectors.asciidoc (right):

https://chromiumcodereview.appspot.com/14109034/diff/2001/selectors.asciidoc#...
selectors.asciidoc:9: Use the top level `query()` and `queryAll()` functions
provided in the
On 2013/04/29 20:59:01, sethladd wrote:
> provided in or provided from?

provided by

https://chromiumcodereview.appspot.com/14109034/diff/2001/selectors.asciidoc#...
selectors.asciidoc:9: Use the top level `query()` and `queryAll()` functions
provided in the
On 2013/04/29 21:17:02, Kathy Walrath wrote:
> top level -> top-level
> 
> And I'd say "provided by"

Done.

https://chromiumcodereview.appspot.com/14109034/diff/2001/selectors.asciidoc#...
selectors.asciidoc:27: <li class='mustHave'>Cereal
On 2013/04/29 20:59:01, sethladd wrote:
> typically, class names are like must-have

Done.

https://chromiumcodereview.appspot.com/14109034/diff/2001/selectors.asciidoc#...
selectors.asciidoc:37: 
On 2013/04/29 21:17:02, Kathy Walrath wrote:
> delete this blank line?

Done.

https://chromiumcodereview.appspot.com/14109034/diff/2001/selectors.asciidoc#...
selectors.asciidoc:42: // Find by id.
On 2013/04/29 21:17:02, Kathy Walrath wrote:
> id => ID

Done.

https://chromiumcodereview.appspot.com/14109034/diff/2001/selectors.asciidoc#...
selectors.asciidoc:44: window.console.log(element.id);                 //
'first'
On 2013/04/29 20:59:01, sethladd wrote:
> please use print() instead

Done.

https://chromiumcodereview.appspot.com/14109034/diff/2001/selectors.asciidoc#...
selectors.asciidoc:52: elements = queryAll('#first, .mustHave');
On 2013/04/29 20:59:01, sethladd wrote:
> a comma is like an "or", so this is #first OR .mustHave. Is this what you
mean?

Yes. Comma acts like an "or".

https://chromiumcodereview.appspot.com/14109034/diff/2001/selectors.asciidoc#...
selectors.asciidoc:61: window.console.log(elements.first.innerHtml);   // 'Bran
Flakes'
On 2013/04/29 20:59:01, sethladd wrote:
> wouldn't elements.first.text work here?

Yes. Making change global.

https://chromiumcodereview.appspot.com/14109034/diff/2001/selectors.asciidoc#...
selectors.asciidoc:83: Calling `queryAll()` returns a readonly list of DOM
elements:
On 2013/04/29 21:17:02, Kathy Walrath wrote:
> On 2013/04/29 20:59:01, sethladd wrote:
> > read-only ?
> 
> Yes.
> 
> Although I think you can just leave out "read only". It made me question, for
a
> moment, whether it was the list or the elements that are read only. I think
you
> can leave it for later.

Done.

https://chromiumcodereview.appspot.com/14109034/diff/2001/selectors.asciidoc#...
selectors.asciidoc:83: Calling `queryAll()` returns a readonly list of DOM
elements:
On 2013/04/29 20:59:01, sethladd wrote:
> read-only ?

Leaving it at based on Kathy's suggestion.

https://chromiumcodereview.appspot.com/14109034/diff/2001/selectors.asciidoc#...
selectors.asciidoc:100: void doSomethingWith(element) {}
On 2013/04/29 20:59:01, sethladd wrote:
> You don't use this function here. Old?

Removing it.

https://chromiumcodereview.appspot.com/14109034/diff/2001/selectors.asciidoc#...
selectors.asciidoc:112: You can perform regular List operations on the results
of `queryAll()`:
On 2013/04/29 20:59:01, sethladd wrote:
> this is confusing to me, injecting this text in the middle of a snippet. Can
you
> reorder things so that the code all flows?

I've broken things up into several little sections.  I think that makes it more
clear.

https://chromiumcodereview.appspot.com/14109034/diff/2001/selectors.asciidoc#...
selectors.asciidoc:116: window.console.log(elements[2].innerHtml);   // 'Reddit'
On 2013/04/29 20:59:01, sethladd wrote:
> global: use print()

Done.

https://chromiumcodereview.appspot.com/14109034/diff/2001/selectors.asciidoc#...
selectors.asciidoc:125: var sites = elements.map((site) => site.innerHtml);
On 2013/04/29 20:59:01, sethladd wrote:
> you can pull .join() up to this line

The print results don't fit on the line then. Leaving it as is.

https://chromiumcodereview.appspot.com/14109034/diff/2001/selectors.asciidoc#...
selectors.asciidoc:125: var sites = elements.map((site) => site.innerHtml);
On 2013/04/29 20:59:01, sethladd wrote:
> use .text if it works

Done.

https://chromiumcodereview.appspot.com/14109034/diff/2001/selectors.asciidoc#...
selectors.asciidoc:128: // Filter results.
On 2013/04/29 20:59:01, sethladd wrote:
> you say filter, but you also reverse. Make this more simple, pull out the
> reversed

Done.

https://chromiumcodereview.appspot.com/14109034/diff/2001/selectors.asciidoc#...
selectors.asciidoc:131: sites.map((site) => site.innerHtml).first); //
"StackOverflow"
On 2013/04/29 20:59:01, sethladd wrote:
> pull the map out of the print()

Done.

https://chromiumcodereview.appspot.com/14109034/diff/2001/selectors.asciidoc#...
selectors.asciidoc:143: Trying to modify the results of `queryAll()` generates
an error:
On 2013/04/29 20:59:01, sethladd wrote:
> Please explain why.

Done.

[sethladd, 2013-04-30 16:36:40]

https://chromiumcodereview.appspot.com/14109034/diff/10001/selectors.asciidoc
File selectors.asciidoc (right):

https://chromiumcodereview.appspot.com/14109034/diff/10001/selectors.asciidoc...
selectors.asciidoc:27: <li class='mustHave'>Cereal
still missing must-have

https://chromiumcodereview.appspot.com/14109034/diff/10001/selectors.asciidoc...
selectors.asciidoc:50: // Find by ID and class.
and is confusing here. I think you mean 'or'

https://chromiumcodereview.appspot.com/14109034/diff/10001/selectors.asciidoc...
selectors.asciidoc:128: var sites = elements.map((site) => site.text);
please annotate sites

https://chromiumcodereview.appspot.com/14109034/diff/10001/selectors.asciidoc...
selectors.asciidoc:144: generates an error:
where is the explanation for why it generates an error?

[shailentuli, 2013-04-30 21:52:08]

Incorporated changes requested by Seth.

https://chromiumcodereview.appspot.com/14109034/diff/10001/selectors.asciidoc
File selectors.asciidoc (right):

https://chromiumcodereview.appspot.com/14109034/diff/10001/selectors.asciidoc...
selectors.asciidoc:27: <li class='mustHave'>Cereal
On 2013/04/30 16:36:40, sethladd wrote:
> still missing must-have

Done.

https://chromiumcodereview.appspot.com/14109034/diff/10001/selectors.asciidoc...
selectors.asciidoc:50: // Find by ID and class.
On 2013/04/30 16:36:40, sethladd wrote:
> and is confusing here. I think you mean 'or'

Done.

https://chromiumcodereview.appspot.com/14109034/diff/10001/selectors.asciidoc...
selectors.asciidoc:128: var sites = elements.map((site) => site.text);
On 2013/04/30 16:36:40, sethladd wrote:
> please annotate sites

Making it Iterable sites ...

Calling map() returns a MappedListIterable, and calling where() returns a
WhereIterable. So, Iterable, is the best I can do here. I don't think its a good
idea to expose all the different kinds of Iterables in this short example.

https://chromiumcodereview.appspot.com/14109034/diff/10001/selectors.asciidoc...
selectors.asciidoc:144: generates an error:
On 2013/04/30 16:36:40, sethladd wrote:
> where is the explanation for why it generates an error?

Done.

[Kathy Walrath, 2013-04-30 22:48:54]

lgtm!

I noticed a few small things.

https://chromiumcodereview.appspot.com/14109034/diff/15001/scoped_selectors.a...
File scoped_selectors.asciidoc (right):

https://chromiumcodereview.appspot.com/14109034/diff/15001/scoped_selectors.a...
scoped_selectors.asciidoc:10: these methods on an element restricts the scope of
the query to the that
the that -> that

https://chromiumcodereview.appspot.com/14109034/diff/15001/scoped_selectors.a...
scoped_selectors.asciidoc:44: class in in other rows are not affected.
in in -> in

https://chromiumcodereview.appspot.com/14109034/diff/15001/selectors.asciidoc
File selectors.asciidoc (right):

https://chromiumcodereview.appspot.com/14109034/diff/15001/selectors.asciidoc...
selectors.asciidoc:141: Since the list returned by `queryAll()` is read-only,
you cannot add, modify,
read-only -> read only
since this follows the noun.
"Read-only" is appropriate if it precedes the noun.

https://chromiumcodereview.appspot.com/14109034/diff/15001/selectors.asciidoc...
selectors.asciidoc:150: into the DOM, add modify existing DOM elements.
Links?

[shailentuli, 2013-05-06 08:33:21]

https://chromiumcodereview.appspot.com/14109034/diff/15001/scoped_selectors.a...
File scoped_selectors.asciidoc (right):

https://chromiumcodereview.appspot.com/14109034/diff/15001/scoped_selectors.a...
scoped_selectors.asciidoc:10: these methods on an element restricts the scope of
the query to the that
On 2013/04/30 22:48:54, Kathy Walrath wrote:
> the that -> that

Done.

https://chromiumcodereview.appspot.com/14109034/diff/15001/scoped_selectors.a...
scoped_selectors.asciidoc:44: class in in other rows are not affected.
On 2013/04/30 22:48:54, Kathy Walrath wrote:
> in in -> in

Done.

https://chromiumcodereview.appspot.com/14109034/diff/15001/selectors.asciidoc
File selectors.asciidoc (right):

https://chromiumcodereview.appspot.com/14109034/diff/15001/selectors.asciidoc...
selectors.asciidoc:150: into the DOM, add modify existing DOM elements.
On 2013/04/30 22:48:54, Kathy Walrath wrote:
> Links?

Punting on this. These recipes are still in a bit of flux.