CODEWITHSUNDEEP
javascriptjquery
ehThind
ehThind

Reputation: 31

Checking a div for duplicates before appending to the list using jQuery

This should be trivial but I'm having issues...

Basically what I am trying to do is append a new "div" to "selected-courses" when a user clicks on a "course". This should happen if and only if the current course is not already in the "selected-courses" box.

The problem I'm running into is that nothing is appended to the "selected-courses" section when this is executed. I have used alert statements to make sure the code is in fact being run. Is there something wrong with my understanding of the way .on and .each work ? can I use them this way.

Here is a fiddle http://jsfiddle.net/jq9dth4j/

$(document).on("click", "div.course", function() {
    var title = $( this ).find("span").text();
    var match_found = 0;

    //if length 0 nothing in list, no need to check for a match     
    if ($(".selected-course").length > 0) {
        match_found = match(title);
    }
    if (matched == 0) {
        var out = '<div class="selected-course">' + '<a href="#">' + title + '</a>'+'</div>';
        $("#selected-box").append(out); 
    }       
});

//checks to see if clicked course is already in list before adding.
function match(str) {
    $(".selected-course").each(function() {
        var retval = 0;
        if(str == this.text()) {
            //course already in selected-course section
            retval = 1;
            return false;
        }
    });
    return retval;
}

Upvotes: 1

Views: 908

Answers (4)

Anders
Anders

Reputation: 8577

Relying on checking what text elements contain is not the best approach to solve this kind of question. It is prone to errors (as you have found out), it can be slow, it gives you long code and it is sensitive to small changes in the HTML. I would recommend using custom data-* attributes instead.

So you would get HTML like this:

<div class="course" data-course="Kite Flying 101"> 
    <a href="#">
        <span>Kite Flying 101</span> 
    </a>
</div>

Then the JS would be simple like this:

$(document).on('click', 'div.course', function() {
    // Get the name of the course that was clicked from the attribute.
    var title = $(this).attr('data-course');
    // Create a selector that selects everything with class selected-course and the right data-course attribute.
    var selector = '.selected-course[data-course="' + title + '"]';
    if($(selector).length == 0) {
        // If the selector didn't return anything, append the div.
        // Do note that we need to add the data-course attribute here.
        var out = '<div class="selected-course" data-course="' + title + '"><a href="#">' + title + '</a></div>';
        $('#selected-box').append(out);    
    }
});

Beware of case sensitivity in course names, though!

Here is a working fiddle.

Upvotes: 1

J Santosh
J Santosh

Reputation: 3847

Your Issues are :

1.this.text() is not valid. you have to use $(this).text().
2.you defined var retval = 0; inside each statement and trying to return it outside each statement. so move this line out of the each statement.
3.matched is not defined . it should be match_found in line if (matched == 0) {.
4. use trim() to get and set text, because text may contain leading and trailing spaces.

Your updated JS is 

$(document).on("click", "div.course", function () {
var title = $(this).find("span").text();
var match_found = 0;

if ($(".selected-course").length > 0) {
    match_found = match(title);
}
if (match_found == 0) {
    var out = '<div class="selected-course">' + '<a href="#">' + title + '</a>' + '</div>';
    $("#selected-box").append(out);
}
});

function match(str) {
var retval = 0;
$(".selected-course").each(function () {
    if (str.trim() == $(this).text().trim()) {
        retval = 1;
        return false;
    }
});
return retval;
}

Updated you Fiddle

Upvotes: 0

ste2425
ste2425

Reputation: 4776

There was a couple of little issues in your fiddle.

See fixed fiddle: http://jsfiddle.net/jq9dth4j/1/

function match(str) {
    var retval = 0;
    $(".selected-course").each(function() {
        if(str == $(this).text()) {
            retval = 1;
            return false;
        }
    });
    return retval;
}

You hadn't wrapped your this in a jquery object. So it threw an exception saying this had no method text().

Second your retval was declared inside the each so it wasn't available to return outside the each, wrong scope.

Lastly the if in the block:

if (matched== 0) {
    var out = '';
    out += '<div class="selected-course">' + '<a href="#">' + title + '</a>'+'</div>';
    $("#selected-box").append(out); 
}

was looking at the wrong variable it was looking at matched which didn't exist causing an exception.

Upvotes: 2

Norlihazmey Ghazali
Norlihazmey Ghazali

Reputation: 9060

Try this code, read comment for where the changes are :

$(document).on("click", "div.course", function () {
  var title = $(this).find("span").text().trim(); // use trim to remove first and end whitespace
  var match_found = 0;

  if ($(".selected-course").length > 0) {
    match_found = match(title);
  }
  if (match_found == 0) { // should change into match_found 
    var out = '';
    out += '<div class="selected-course">' + '<a href="#">' + title + '</a>' + '</div>';
    $("#selected-box").append(out);
  }
});

function match(str) {
  var retval = 0; // this variable should place in here
  $(".selected-course").each(function () {

    if (str == $(this).find('a').text().trim()) { // find a tag to catch values, and use $(this) instead of this
        retval = 1;
        return false;
    }
  });
  return retval; // now can return variable, before will return undefined
}

Updated DEMO

Upvotes: 0

Related Questions