2
\$\begingroup\$

Purpose:

In an HTML page, a user can filter results, by setting conditions.

At the beginning, there is no condition displayed, but a button to add one.

The code for each condition is always the same, and is know during design.

I am trying to insert the template in the page in the most efficient and standard way.

Code:

I have added the template of the condition, with display:none at the bottom of the page, and when a new condition is requested, the template is cloned and added to the DOM

$("#addConditionButton").click(function() {
  $("#template")
 .clone()
 .attr('id','condition'+$(".condition").length)
 .show()
 .appendTo("#conditions")
})
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<div id="filterDiv">
  <div id="title">
    <h1>
      Filter conditions
    </h1>
  </div>
  <div id=conditions>
  </div>
  <div id="footer">
    <button id="addConditionButton">
      Add Condition
    </button>
  </div>
</div>

<div style="display:none" class='condition' id="template">
  <select>
    <option>&lt;</option>
    <option>&gt;</option>
    <option>=</option>
  </select>
  <input type='text'>
</div>

My concerns:

I am not sure that the method I used, hidden template then cloning is the best way to do this, in terms of performance, but mostly in terms of best practice, since I'm learning about this.

Thanks,

\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

Your code looks good, and very clean. Best practices are usually subjective; there's no absolute best way to clone and insert an element. That being said, I would make two suggestions:

1. Be consistent with your HTML

Your conditions div doesn't have quotes around its ID, whereas your template div does. I'd recommend always surrounding IDs in quotes as a standard practice.

2. Swap the .show() and .appendTo() order

This suggestion is purely subjective, but semantically, I think it makes more sense to add the cloned element to the DOM first, then change its display property, rather than change its display property first (which won't have any affect yet anyway) and then add it to the DOM. Again, that's just my opinion.

Other notes

If you're interested in adding autofocus to your input elements (so that as a new element is added, it automatically gets focus) you could do that fairly easily by inserting this line just before you close your function:

[0].childNodes[3].focus();

Also, if you ever modify your code to where you need to access the cloned elements by ID, keep in mind that your cloned elements will start with condition2. You're using the length to generate the number, so when you add the first element the length will be 2.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.