Creating  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

&lt;div class="row"&gt;
    ...
    &lt;!-- empty div will hold &lt;select&gt; which will be crated on ajax success --&gt; 
    &lt;div class="append-select col-md-3"&gt;&lt;/div&gt;
    ...
&lt;/div&gt;
...
&lt;!-- onLoad =&gt; 'display: none', &lt;tr&gt; will be dynamically crated on Ajax Success --&gt;
&lt;div class="row"&gt;
    &lt;div class="col-md-4 col-md-offset-2"&gt;
      &lt;table id="options-table" class="table table-striped"&gt;
        &lt;thead&gt; &lt;tr&gt;&lt;th&gt; Selected options &lt;/th&gt;&lt;/tr&gt; &lt;/thead&gt;
        &lt;tbody&gt;&lt;/tbody&gt;
      &lt;/table&gt;
    &lt;/div&gt;
&lt;/div&gt;


JavaScript code

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

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

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

        $.ajax({
            type: 'get',
            url: '../option_types/' + selectedOptionId,
            success: function(data) {
                var element = '&lt;select id="value-select" class="form-control"&gt;';
                element += '&lt;option selected disabled&gt;Select value...&lt;/option&gt;';

                for(var i=0; i&lt;data.length; i++) {
                    element += '&lt;option value="' + data[i].id + '"&gt;' + data[i].name + '&lt;/option&gt;';
                }

                element += '&lt;/select&gt;';
                $('.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 = '&lt;tr&gt;&lt;td&gt;' + selectedValue + '&lt;/td&gt;';
                        element += '&lt;td&gt;&lt;a class="remove"&gt; x &lt;/a&gt;&lt;/tr&gt;';

                    $('#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');
                    });
                });
            }
        });
    });
});
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 &lt; 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 = '&lt;tr&gt;&lt;td&gt;' + selectedValue + '&lt;/td&gt;';
          element += '&lt;td&gt;&lt;a class="remove"&gt; x &lt;/a&gt;&lt;/tr&gt;';

      $('#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');
      });
  });
}