Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Given the following HTML form (fragment):

<fieldset id="timesheet-rows">
    <legend>Add Entries</legend>

    <div id="timesheetrow-0" class="timesheet-row">
      <label for="project-0">Project</label>
      <select id="project-0" name="project-0" required>
        <option value="" />
      </select>

      <label for="department-0">Department</label>
      <select id="department-0" name="department-0" required>
        <option value="" />
      </select>

      <label for="task-0">Task: </label>
      <select id="task-0" name="task-0" required>
        <option value="" />
      </select>

      <label for="hours-0">Hours: </label>
      <input type="number" step="0.25" id="hours-0" name="hours-0" width="1" placeholder="2.0" required />

      <label for="comment-0">Comment: </label>
      <input type="text" id="comment-0" name="comment-0" width="50" />
    </div>

    <input type="button" id="add-row" name="add-row" value="Add row" />

  </fieldset>

I have implemented the following jQuery (fragment) to clone each 'timesheet row' <div /> along with the <label /> tags, which are not caught by $(':input'):

$(document).ready(function() {

  var current_id = 0;
  $('#add-row').click(function(){
    next_element($('#timesheetrow-0'));
  })

  function next_element(element){
    var new_element = element.clone(),
        id = current_id + 1;
    current_id = id;
    new_element.attr("id",element.attr("id").split("-")[0]+"-"+id);

    // Ajuster les `id` et `name` dans les <input />s et <select />s
    $(':input', new_element).each(function(){
      var field_id = $(this).attr("id"),
          field_name = $(this).attr("name");
      $(this).attr("id", field_id.split("-")[0]+"-"+id );
      $(this).attr("name", field_name.split("-")[0]+"-"+id );
    });

    // Ajuster le for="" dans les <label />s
    $('label', new_element).each(function(){
      field_for = $(this).attr("for");
      $(this).attr("for", field_for.split("-")[0]+"-"+id );
    });
    new_element.appendTo($("#timesheet-rows"));
  };
});

(Above jQuery was inspired by http://jsfiddle.net/32RgL/ ).

Is there a more elegant or complete way of doing this?

share|improve this question

1 Answer 1

up vote 2 down vote accepted

Most of this code is due to the fact that you're increasing the numbers in your attributes. I think that's a mistake. Your rows should all have identical name attributes, stored in an array (e.g. name="task[]").

The only problem would then be the identical IDs. Frankly, you shouldn't be using IDs in the first place. The only advantage of using IDs in this case is to associate the labels to the form fields (so that clicking the label focuses on the form field). This could easily be accomplished by wrapping the label around the form element.

So, to summarize, here are some points to consider:

  1. Get rid of the IDs
  2. Wrap the labels around the form elements to associate them with one another
  3. Get rid of the numbers from the names
  4. Use array like notation (e.g. name="task[]") for your names

Once you've incorporated all this, you can simply clone the row when needed.

HTML:

<fieldset id="timesheet-rows">
    <legend>Add Entries</legend>

    <div class="timesheet-row">

        <label>Project:
            <select name="project[]" required>
                <option value=""></option>
            </select>
        </label>

        <label>Department:
            <select name="department[]" required>
                <option value=""></option>
            </select>
        </label>

        <label>Task: 
            <select name="task[]" required>
                <option value=""></option>
            </select>
        </label>

        <label>Hours:
            <input type="number" step="0.25" name="hours[]" width="1" placeholder="2.0" required />
        </label>

        <label>Comment:
            <input type="text" name="comment[]" width="50" />
        </label>

    </div>

    <input type="button" id="add-row" name="add-row" value="Add row" />
</fieldset>​

Javascript:

jQuery(function($) {
    var $button = $('#add-row'),
        $row = $('.timesheet-row').clone();

    $button.click(function() {
        $row.clone().insertBefore( $button );
    });
});​

Here's the fiddle: http://jsfiddle.net/wd5y9/

share|improve this answer
    
Thanks for your response, Joseph. Having worked in a11y design, I would note that implicit label tags are generally discouraged for WCAG 2.0 compliance as not all assistive technologies handle them correctly: "The HTML and XHTML specifications allow both implicit and explicit labels. However, some assistive technologies do not correctly handle implicit labels (for example, <label>First name <input type="text" name="firstname"></label>)." source: w3.org/TR/WCAG-TECHS/H44 I had forgotten the name="array[]" method though! Thanks for that. –  msanford Jul 18 '12 at 18:01
    
Just saw your added jQuery; love it. –  msanford Jul 18 '12 at 18:04
    
@msanford - The info you're quoting from the W3C website is pretty outdated. I doubt that's still true today... –  Joseph Silber Jul 18 '12 at 18:08
    
Joseph, it applies to a version of JAWS which while indeed quite outdated is still in use by a lot of people (see IE6). Still, for my internal application, it's not going to be a problem. –  msanford Jul 24 '12 at 14:37
    
Follow-up: I implemented the changes you recommended and like them a lot. Thanks! –  msanford Jul 30 '12 at 17:34

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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