[jQuery] Re: UPDATE: code review, suggestions for condensing / reuse?
Ok i was a bit too quick. You could use an idnumber for each image but
it will add more code to the html page.
On 5 apr, 14:23, "David Duymelinck" <[EMAIL PROTECTED]> wrote:
> How does that makes it possible to change the table?
>
> The solution for all those parents is
>
> var id = curImg.parents('tr').attr('id');
>
> With parents you will get all the parent elements and you can limit it
> using an expression.
>
> http://docs.jquery.com/DOM/Traversing#parents.28_expr_.29
>
> On 5 apr, 14:08, Ronald Haring <[EMAIL PROTECTED]> wrote:
>
> > Looks good, only one suggestion, this line:
>
> > var id =
> > curImg.parent().parent().parent().parent().parent().parent().attr('id'); //
> > get the id of the parent TR tag
>
> > looks very brittle. If you change the table or tr a bit, then you are
> > lost. Why dont you try and add another class to the img e.g. class="nav
> > 1073" and then do something like:
>
> > var curImg = $(this); // store a reference to the image just
> > clicked upon
> > var cssClass = curImg.attr("class"); // fetch the classes e.g. "nav 1073"
> > var idArr = css.split(" "); //split the id
>
> > Regards
> > Ronald
>
> > Andy Matthews wrote:
> > > I've made some additions and updates to the code I posted last week:
> > >http://www.commadelimited.com/uploads/psychic/
>
> > > I wondered if you all would mind looking over it to see if it can be
> > > improved. I've got the functionality the way I like it, but you all
> > > know jQuery way better than I do. So...any suggestions you feel like
> > > making would be welcomed.
>
> > > Andy Matthews
> > > Senior Coldfusion Developer
> > > Office: 877.707.5467 x747
> > > Direct: 615.627.9747
> > > Fax: 615.467.6249
> > > [EMAIL PROTECTED]
> > >www.dealerskins.com
[jQuery] Re: UPDATE: code review, suggestions for condensing / reuse?
How does that makes it possible to change the table?
The solution for all those parents is
var id = curImg.parents('tr').attr('id');
With parents you will get all the parent elements and you can limit it
using an expression.
http://docs.jquery.com/DOM/Traversing#parents.28_expr_.29
On 5 apr, 14:08, Ronald Haring <[EMAIL PROTECTED]> wrote:
> Looks good, only one suggestion, this line:
>
> var id =
> curImg.parent().parent().parent().parent().parent().parent().attr('id'); //
> get the id of the parent TR tag
>
> looks very brittle. If you change the table or tr a bit, then you are
> lost. Why dont you try and add another class to the img e.g. class="nav
> 1073" and then do something like:
>
> var curImg = $(this); // store a reference to the image just
> clicked upon
> var cssClass = curImg.attr("class"); // fetch the classes e.g. "nav 1073"
> var idArr = css.split(" "); //split the id
>
> Regards
> Ronald
>
> Andy Matthews wrote:
> > I've made some additions and updates to the code I posted last week:
> >http://www.commadelimited.com/uploads/psychic/
>
> > I wondered if you all would mind looking over it to see if it can be
> > improved. I've got the functionality the way I like it, but you all
> > know jQuery way better than I do. So...any suggestions you feel like
> > making would be welcomed.
>
> > Andy Matthews
> > Senior Coldfusion Developer
> > Office: 877.707.5467 x747
> > Direct: 615.627.9747
> > Fax: 615.467.6249
> > [EMAIL PROTECTED]
> >www.dealerskins.com
[jQuery] Re: UPDATE: code review, suggestions for condensing / reuse?
Looks good, only one suggestion, this line:
var id =
curImg.parent().parent().parent().parent().parent().parent().attr('id'); // get
the id of the parent TR tag
looks very brittle. If you change the table or tr a bit, then you are
lost. Why dont you try and add another class to the img e.g. class="nav
1073" and then do something like:
var curImg = $(this); // store a reference to the image just
clicked upon
var cssClass = curImg.attr("class"); // fetch the classes e.g. "nav 1073"
var idArr = css.split(" "); //split the id
Regards
Ronald
Andy Matthews wrote:
I've made some additions and updates to the code I posted last week:
http://www.commadelimited.com/uploads/psychic/
I wondered if you all would mind looking over it to see if it can be
improved. I've got the functionality the way I like it, but you all
know jQuery way better than I do. So...any suggestions you feel like
making would be welcomed.
Andy Matthews
Senior Coldfusion Developer
Office: 877.707.5467 x747
Direct: 615.627.9747
Fax: 615.467.6249
[EMAIL PROTECTED]
www.dealerskins.com
[jQuery] Re: UPDATE: code review, suggestions for condensing / reuse?
Andy,
Nice work and some great suggestions from Brandon.
I love this list!
/alex
On 4/3/07, Andy Matthews <[EMAIL PROTECTED]> wrote:
Awesome Brandon!!
Those are PRECISELY the sorts of things I'm looking to learn.
andy
-Original Message-
From: jquery-en@googlegroups.com [mailto:[EMAIL PROTECTED] On
Behalf Of Brandon Aaron
Sent: Tuesday, April 03, 2007 8:50 AM
To: jquery-en@googlegroups.com
Subject: [jQuery] Re: UPDATE: code review, suggestions for condensing /
reuse?
First off, this is functioning much better than before. Good work.
In the click handler you bind to the h1 you have a few areas where you could
optimize your code.
First you declare a var named outerchild like this:
var outerid = $(this).parent().attr("id"); var outerchild = $('#' + outerid
+ '-details');
Then later you use outerchild again but you wrap it in another jQuery
object.
$(outerchild)
This isn't necessary and I often times name my jQuery object vars with a $
to remind me it is the jQuery object and not just the DOM node.
var outerid = $(this).parent().attr("id"); var $outerchild = $('#' + outerid
+ '-details');
Now you can just use $outerchild.attr(...).
Moving on ... there is a handy method called is that lets you test if the
object is something. So lets look at this particular if statement and see
how it can be written using the is method.
As you have it now:
if ($(outerchild).attr("class").indexOf('outeropen') == -1) {
Using the is method:
if ( !$outerchild.is('.outeropen') ) {
Or you could also use the ':hidden' selector like this:
if ( $outerchild.is(':hidden') ) {
Hope that helps optimize the code a little. :)
--
Brandon Aaron
On 4/3/07, Andy Matthews <[EMAIL PROTECTED]> wrote:
>
> I've made some additions and updates to the code I posted last week:
> http://www.commadelimited.com/uploads/psychic/
>
> I wondered if you all would mind looking over it to see if it can be
> improved. I've got the functionality the way I like it, but you all
> know jQuery way better than I do. So...any suggestions you feel like
> making would be welcomed.
>
>
>
> Andy Matthews
> Senior Coldfusion Developer
> Office: 877.707.5467 x747
> Direct: 615.627.9747
> Fax: 615.467.6249
> [EMAIL PROTECTED]
> www.dealerskins.com
>
>
[jQuery] Re: UPDATE: code review, suggestions for condensing / reuse?
Awesome Brandon!!
Those are PRECISELY the sorts of things I'm looking to learn.
andy
-Original Message-
From: jquery-en@googlegroups.com [mailto:[EMAIL PROTECTED] On
Behalf Of Brandon Aaron
Sent: Tuesday, April 03, 2007 8:50 AM
To: jquery-en@googlegroups.com
Subject: [jQuery] Re: UPDATE: code review, suggestions for condensing /
reuse?
First off, this is functioning much better than before. Good work.
In the click handler you bind to the h1 you have a few areas where you could
optimize your code.
First you declare a var named outerchild like this:
var outerid = $(this).parent().attr("id"); var outerchild = $('#' + outerid
+ '-details');
Then later you use outerchild again but you wrap it in another jQuery
object.
$(outerchild)
This isn't necessary and I often times name my jQuery object vars with a $
to remind me it is the jQuery object and not just the DOM node.
var outerid = $(this).parent().attr("id"); var $outerchild = $('#' + outerid
+ '-details');
Now you can just use $outerchild.attr(...).
Moving on ... there is a handy method called is that lets you test if the
object is something. So lets look at this particular if statement and see
how it can be written using the is method.
As you have it now:
if ($(outerchild).attr("class").indexOf('outeropen') == -1) {
Using the is method:
if ( !$outerchild.is('.outeropen') ) {
Or you could also use the ':hidden' selector like this:
if ( $outerchild.is(':hidden') ) {
Hope that helps optimize the code a little. :)
--
Brandon Aaron
On 4/3/07, Andy Matthews <[EMAIL PROTECTED]> wrote:
>
> I've made some additions and updates to the code I posted last week:
> http://www.commadelimited.com/uploads/psychic/
>
> I wondered if you all would mind looking over it to see if it can be
> improved. I've got the functionality the way I like it, but you all
> know jQuery way better than I do. So...any suggestions you feel like
> making would be welcomed.
>
>
>
> Andy Matthews
> Senior Coldfusion Developer
> Office: 877.707.5467 x747
> Direct: 615.627.9747
> Fax: 615.467.6249
> [EMAIL PROTECTED]
> www.dealerskins.com
>
>
[jQuery] Re: UPDATE: code review, suggestions for condensing / reuse?
First off, this is functioning much better than before. Good work.
In the click handler you bind to the h1 you have a few areas where you
could optimize your code.
First you declare a var named outerchild like this:
var outerid = $(this).parent().attr("id");
var outerchild = $('#' + outerid + '-details');
Then later you use outerchild again but you wrap it in another jQuery object.
$(outerchild)
This isn't necessary and I often times name my jQuery object vars with
a $ to remind me it is the jQuery object and not just the DOM node.
var outerid = $(this).parent().attr("id");
var $outerchild = $('#' + outerid + '-details');
Now you can just use $outerchild.attr(...).
Moving on ... there is a handy method called is that lets you test if
the object is something. So lets look at this particular if statement
and see how it can be written using the is method.
As you have it now:
if ($(outerchild).attr("class").indexOf('outeropen') == -1) {
Using the is method:
if ( !$outerchild.is('.outeropen') ) {
Or you could also use the ':hidden' selector like this:
if ( $outerchild.is(':hidden') ) {
Hope that helps optimize the code a little. :)
--
Brandon Aaron
On 4/3/07, Andy Matthews <[EMAIL PROTECTED]> wrote:
I've made some additions and updates to the code I posted last week:
http://www.commadelimited.com/uploads/psychic/
I wondered if you all would mind looking over it to see if it can be
improved. I've got the functionality the way I like it, but you all
know jQuery way better than I do. So...any suggestions you feel like
making would be welcomed.
Andy Matthews
Senior Coldfusion Developer
Office: 877.707.5467 x747
Direct: 615.627.9747
Fax: 615.467.6249
[EMAIL PROTECTED]
www.dealerskins.com
