HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Limiting the number of text fields created

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
numberthecreatedfieldstextlimiting

Problem

It's a script that I've ripped off elsewhere, and have modified it via help from Stack Overflow on a couple of other questions, so that it tops out at 10 fields.

My knowledge of JS/jQuery is barely there. I'm terrified that I did it badly or wrongly. Could you please let me know if this is the correct way of doing it, or if there's another way that I should have done it differently?

I need to make sure that the max number of text fields created is limited to 10 at most.

Here's a demo and a JSFiddle.



jQuery(document).ready(function($) {
var max_fields = 10;
var x = 1;

$('.my-form .add-box').click(function() {
var n = $('.text-box').length + 1;

if(x Name ' + n + ' Remove');
box_html.hide();
$('.my-form p.text-box:last').after(box_html);
box_html.fadeIn('slow');
x++;
}
});

$('.my-form').on('click', '.remove-box', function() {
$(this).parent().css('background-color', '#FF6C6C').fadeOut("slow", function() {
$(this).remove();
$('.text-box').each(function(index) {
var label = $(this).children('label');
label.attr('for', 'box' + (index + 1));
label.children('.box-number').text(index + 1);
$(this).children('input').attr({
name: 'on' + index,
id: 'box' + (index + 1)
});
});
});
return false;
});
});




Name 1

Add More Names

Solution

Bored, so I made a few improvements as I mentioned (in the comments that have vanished when this was moved to Code Review):

Use the jQuery shortcut DOM-ready handler

A handy short-cut for your DOM ready, with locally scoped $ is just jQuery(function($){...});

Store templates in HTML, not JavaScript strings:


    
       Name {n}  Remove
    


and access like this:

var box_html = $('#template').html().replace(/{n}/g, n).replace(/{-1}/g, n-1);
var $box = $(box_html);


Count the current items when you need them

var n = $('.text-box').length + 1;
if (n <= max_fields) {


Restrict to the current "group" of inputs:

var $form = $(this).closest('.my-form');
$('p.text-box:last', $form).after($box);


This also means it can now handle multiple sets of inputs, each with different name/id prefixes.

Combine operations when you can:

$box.hide().fadeIn('slow');


Putting it all together:

JSFiddle: http://jsfiddle.net/TrueBlueAussie/q2emvzdd/4/

Notes:

  • A prefix was needs to be supplied by the parent form (e.g. a data attribute) so that each set of inputs has unique IDs and names. This is not applied to the first entry yet, but I have to go now, so something for you to finish :)

Code Snippets

<script id="template" type="text/template">
    <p class="text-box">
       <label for="box{n}">Name <span class="box-number">{n}</span></label> <input type="text" name="on{-1}" value="" id="box{n}" /> <a href="#" class="remove-box">Remove</a>
    </p>
</script>
var box_html = $('#template').html().replace(/{n}/g, n).replace(/{-1}/g, n-1);
var $box = $(box_html);
var n = $('.text-box').length + 1;
if (n <= max_fields) {
var $form = $(this).closest('.my-form');
$('p.text-box:last', $form).after($box);
$box.hide().fadeIn('slow');

Context

StackExchange Code Review Q#102127, answer score: 3

Revisions (0)

No revisions yet.