HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Getting JSON with jQuery and creating lists with click events calling more jQuery and JSON

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
clickeventswithcreatingandmoregettingjquerylistsjson

Problem

I have two elements on my page:



I'm calling some JSON with jQuery and loading elements in the divs. I'm curious if there are things I can be doing more efficiently, and better ways to use selectors and JSON.

```
$().ready(function() {
//Load sections
GetCarlineSections(_BucketID);
});

function GetCarlineSections(bucketID) {
//Get section json list
$.ajax(
{
type: "POST",
url: _ApplicationRootURL + 'GetChildBucketList', //Call the ActionResult to return the JSON object
data: 'parentID=' + bucketID,
success: function (sections) { //'sections' is an array of JSON objects returned by GetChildBucketList
$(sections).each(function () {
$('#VerticalMenu') //Append each item to the #VerticalMenu
.append(
$('') //Append a new element to #VerticalMenu
.addClass('Section')
.html(
$('') //Create a new inside of the
.addClass(this.BucketName)
.html(this.BucketName)
.click({ parentID: this.BucketID }, function (event) { //Attach a click event to the element
$('#AccordionMenu').empty();
GetSectionSubSections(event.data.parentID); //Event.data.parentID is the id of the bucket represented by this element
})
)
);
});
}
});
}

function GetSectionSubSections(bucketID) {
$.ajax(
{
type: "POST",
url: _ApplicationRootURL + 'GetChildBucketList',
data: 'parentID=' + bucketID,
success: function (SubSections) { //SubSections are the children buckets of Section, local var bucketID
$(SubSections).each(function () {
$('#AccordionMenu')
.append(

Solution

Two small nickpicks at the start:

  • Empty HTML lists (ul, ol) are strictly spoken invalid. They should contain at least one list item. That said, I believe there is no browser limitation or other technical reason an empty list shouldn't work.



  • It is custom for the names of functions, variables and object fields in JavaScript to be written with a small letter at the beginning.



You can optimize adding the list item to the list, by using .map() to create the items and appending them all at once.

$('#VerticalMenu') 
  .append(
    $(sections).map(function () {
      $('') 
      // ...
    })
  );


BTW, since sections is an array of simple JavaScript objects and not DOM objects it's a bit weird to wrap them in a jQuery object ($(sections).each(...)), because jQuery unwraps them immediately anyway. You should use $.each(sections, ... ) (or $.map(sections, ... )) instead.

You are assigning all items in your lists the same class (Section and SubSection respectively). Unless you remove the class later, this is usually a sign of wrong CSS design. If you leave out the class, then instead of the selector .Section { ... } you can use a descendent selector: #VerticalMenu li { ... }.

One last thing: The use of h4 elements seems wrong to me. Either this is a menu, then you shouldn't be using header elements at all, or the items in AccordionMenu are subheadlines and be using h5 instead.

Code Snippets

$('#VerticalMenu') 
  .append(
    $(sections).map(function () {
      $('<li/>') 
      // ...
    })
  );

Context

StackExchange Code Review Q#2028, answer score: 3

Revisions (0)

No revisions yet.