Creating <select> element and append data to it

I've made the following and have a few questions:

-
Do I have some (big) mistakes in this approach? (bad practices, 'this code is trash'...)

1.1 If I do, can you suggest what to fix?

• Are appended element created in good way?

• Code in success function seems to be a little long. Any way to refactor?

CSS code

.no-display-table {
    display: none;
}

HTML code

    ...
     which will be crated on ajax success --> 
    
    ...

...
 'display: none',  will be dynamically crated on Ajax Success -->

    
      
          Selected options  
        
      
    

JavaScript code

```
$(function() {
$('#options-table').addClass('no-display-table');

$('#product-option-select').change(function() {
if($('.append-select select').length > 0)
$('.append-select select').remove();

var selectedOptionId = $(this).val();

$.ajax({
type: 'get',
url: '../option_types/' + selectedOptionId,
success: function(data) {
var element = '';
element += 'Select value...';

for(var i=0; i' + data[i].name + '';
}

element += '';
$('.append-select').append(element);

$('.append-select').on('change', '#value-select', function() {
$('#options-table').removeClass('no-display-table');

var selectedValue = $('#value-select option:selected').text();
var element = '' + selectedValue + '';
element += ' x ';

$('#options-table tbody').append(element);

$('#options-table').on('mouseover', '.remove', function() {
$('.remove').css('cursor', 'pointer');
});

$('#options-table').on('click', '.remove', function() {
$(this).parent().parent().remove();

if($('#opti

My 2 cents:

-
You could use $().hide() instead of assigning your custom hiding css class

-
Similarly you could use $().show() instead of removing your custom hiding css class

-
You do not have to check the length of a jQuery query, just call remove(), it will work.

-
Your building of html inside the success callback is ugly, have a helper function to construct it and consider using DOM functions instead of HTML building. I personally tend to show/hide the select element and then rebuild the list with this helper function:

function setOptions( id , array )
{
  //Get the element
  var select = document.getElementById( id ) , i 
  //Remove potentially existing children, somewhat heartless
  while (select.firstChild)
    select.removeChild(select.firstChild);
  //Add the new children
  for( i = 0 ; i < array.length ; i++ )
  {
    var element = new Option( array[i].id , array[i].name );
    select.appendChild(element);        
  }      
}

-
Assiging the listener in the success callback is also too much, consider building a listener that takes into account new elements on document instead $(document').on('change', etc.

-
Same for the mouseover / click event listeners

Simply having the document element ( or any other parent element that stays in the DOM ) and the selector inside the on call should activate the listeners for new elements.

function initializeListeners()
{
  $(document).on('change', '#value-select', function() {
      $('#options-table').removeClass('no-display-table');

      var selectedValue = $('#value-select option:selected').text();
      var element = '' + selectedValue + '';
          element += ' x ';

      $('#options-table tbody').append(element);

      $('#options-table').on('mouseover', '.remove', function() {
          $('.remove').css('cursor', 'pointer');
      });

      $('#options-table').on('click', '.remove', function() {
          $(this).parent().parent().remove();

          if($('#options-table tbody tr').length == 0)
              $('#options-table').addClass('no-display-table');
      });
  });
}

StackExchange Code Review Q#37711, answer score: 2