Merge pull request #74 from chrometoasters/pulls/next-id-fix

FIX Correctly increment ID

client/javascript/multivaluefield.js

@@ -2,12 +2,13 @@ jQuery(function($) {
 	function addNewField() {
 		var self = $(this);
 		var val = self.val();
+		var keySep = '__';
 
 		// check to see if the one after us is there already - if so, we don't need a new one
 		var li = $(this).closest('li').next('li');
 
 		if (!val) {
-			// lets also clean up if needbe
+			// lets also clean up if need be
 			var nextText = li.find('input.mventryfield');
 			var detach = true;
 
@@ -32,11 +33,16 @@ jQuery(function($) {
 
 			// Assign the new inputs a unique ID, so that chosen picks up
 			// the correct container.
-			append.find("input, select, textarea").val("").attr("id", function() {
-				var pos = this.id.lastIndexOf("__");
-				var num = parseInt(this.id.substr(pos + 1));
+			append.find('input, select, textarea').val('').each(function () {
+				var pos = this.id.lastIndexOf(keySep);
+				if (pos !== -1) {
+					pos = pos + keySep.length;
 
-				return this.id.substr(0, pos + 1) + (num + 1).toString();
+					var maxId  = parseInt(this.id.substr(pos));
+					var nextId = maxId + 1;
+
+					this.id = this.id.substr(0, pos) + nextId; // nextId auto-converted to string here
+				}
 			});
 
 			append.appendTo(self.parents("ul.multivaluefieldlist"));

src/Fields/MultiValueTextField.php

@@ -59,7 +59,10 @@ public function Field($properties = [])
             }
         }
 
+        // add an empty row
         if (!$this->readonly) {
+            // assume next pos equals to the number of existing fields which gives index+1 in a zero-indexed list
+            $attributes['id'] = $this->id().MultiValueTextField::KEY_SEP.count($fields);
             $fields[] = $this->createInput($attributes);
         }
 

tests/MultiValueTextFieldTest.php

@@ -0,0 +1,32 @@
+<?php
+
+namespace Symbiote\MultiValueField\Tests;
+
+use SilverStripe\Dev\SapphireTest;
+use Symbiote\MultiValueField\Fields\MultiValueTextField;
+
+/**
+ * @author Marcus Nyeholt <[email protected]>
+ */
+class MultiValueTextFieldTest extends SapphireTest
+{
+    protected static $extra_dataobjects = [
+        'MultiValueFieldTest_DataObject'
+    ];
+
+    public function testAttributesGeneration()
+    {
+        $field = MultiValueTextField::create('TestTextField', 'Test Text Field');
+
+        $this->assertContains('id="' . $field->ID() . '"', $field->forTemplate());
+        $this->assertContains('id="' . $field->ID() . MultiValueTextField::KEY_SEP . '0"', $field->forTemplate());
+        $this->assertNotContains('id="' . $field->ID() . MultiValueTextField::KEY_SEP . '1"', $field->forTemplate());
+
+        $field->setValue(['one']);
+        $this->assertContains('id="' . $field->ID() . MultiValueTextField::KEY_SEP . '1"', $field->forTemplate());
+        $this->assertNotContains('id="' . $field->ID() . MultiValueTextField::KEY_SEP . '2"', $field->forTemplate());
+
+        $field->setValue(['one', 'two']);
+        $this->assertContains('id="' . $field->ID() . MultiValueTextField::KEY_SEP . '2"', $field->forTemplate());
+    }
+}