Sign up ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

This code is in a really early state in the lifecycle. I'm still in the process of implementing some core features, but nevertheless the code up to here works and accomplishes its task.

I have no idea as to how well, though.

This code is intended to give a dead-simple way of creating growable and shrinkable form content for things like: multiple addresses, multiple courses, multiple X.

As such, it is intended (but not yet able) to fully support all kinds of form elements, including radio buttons and dropdowns.

The goal is providing a single registration once on page-load and then automatize generation of input-names from a template, generation of new form fields for new "rows", removal of rows as well as providing a way to simplistically style the form-fields.

To stay lightweight I decided to implement this in vanilla-js. The current state of the code can be found at this revision in my GitHub repository

I am aware and intend to timely "fix" following shortcomings:

  • Lack of documentation
  • Constraint to table-structure in html
  • Lacking support for all types of inputs currently supported are only the inputs that have the tagname input
  • Actual numbering of inputs by row through the nametemplate

These "features" are currently marked with FIXME comments.

// FIXME jsdoc
function DynamicForm(parent, coreConfig, formOptions) {
  // require input data from formOptions
  if (formOptions === undefined || formOptions.inputs === undefined) {
    console.warn("Cannot create a DynamicForm without formOptions");
    return false;
  }
  if (parent === undefined) {
    console.warn("Cannot create a DynamicForm without a structure to bind to");
    return false;
  }

  var add_icon;
  if (coreConfig.add_icon === undefined) {
    add_icon =
      'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABgAAAAYCAYAAADgdz34AAACyUlEQVR42q2UTWgTQRTH/7NJiujBY0HqByJqFRVEoaAIEUEoiLXEUrGFKh5qFamg4sGb54IgiiD05EFFi7TgSa2aRk0/Y2mKlopapcQ2281Ho202s+Pb2WySBdsI3cDwJm/f/H/vY3YZyvwOdm4RletXwVvhc/hzWR2/fiyg7+wkW+78sg9tQHt9LTavrXL4v6dj6HjSvXLA3nsbxPXGOkTnHjj8yYyCt6HVGG6dcgHQcAITqccOv5YCet/53AFcC9Rjcv6R/C9oGQYQTwKhsEuAKwT4QgBTmNMyhIJ4Augf8rgEqD+OT6kuErYqMCFxDRiOuAS4dKwOn+efkjiTELOSGZVjLPofLTKvYTnIuaO1GE935dtDVZCdS3CawWK5o2D+zp3ixhk/phPfIJhCIswapBRSILiCcTVEuXulsAFFWp0CFrnA7soa8hmFyuRZDnh9Hjx8HgY7dL9anD+1FZHpEB31WEGCQacgMnLPaKgGPStUAEvMEAKclkEgI+8TJoj2CiU0+CENduButWhr2kGAPsrS5wjiMiMlL1YUtmchgbBaBlHcm/ZPVsHYQBKs5vZ20da8CyM/g1CYtyhsKFZWpcJUDbezt9uRn2DOsG6MlZwC+lTh43sC7L9lAYYJwJSKQgalwlywYltKhO1YlAgb0sGR43SFgwTY17FNHPFvRHQ6uuRN2FPpx5QRdAjHYjn5bE3Fv89kssDEaAas+uYmkU5llxTXfs/hcksAX3nQkfEMAcIv1fLXtFxA1dV1oqXhMCZ1uwJFUtTZrARk7iys7E02Ae0tJxFOdBf6a/ZbVQ0Mvoi7A7jQHMBg8ll+kB5ZiTarY+iVS4C20wH0p3qksD0ElQAjr10CtDYFMKD1wNY3h63FdUTeuAWgCsKJHsct0mZ0jAZdAlxsbkSIZiAryL+5CQJEemfdAZg2nU0WWiRtjktbDvAXpTXf5N+BV58AAAAASUVORK5CYII=';
  } else {
    add_icon = coreConfig.add_icon;
  }
  var remove_icon;
  if (coreConfig.remove_icon === undefined) {
    remove_icon =
      'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABgAAAAYCAYAAADgdz34AAABqElEQVR42u1TO0sDQRD+NsFGBEubxNI/YC/YWPiKqNHK3ka0s/EXWWjjT9BCfDQiEhUshCghL9AYvdvdcfZmL3cGBUFSCHewzM3OzPfNaxUG/KmMICP4BwQ0NUMojgHG/iIH6rPRVz+bus/lgGqNCcrrhN0NoF5jn1zib32wDdmfg8nInfaSSKS1AvjhJB+ywptneXDGBPMlwtYSULkSVpdJFOz8AwF3ID1Ah6e5YkdmE1Jnd10w/K85qTxjXTxDBXOLNLS9DNxe82WeA5QAdN+ATkcqC5mo1eJgtjXrDAAhDY3IuJLQV2cQVfJ+14bqzC7Q8E6ZCW6AWpUB+TQaTNAV59D1yUjm2rcmAtI+GTYEMbAjIamO9dYDE7RLKzS6Ogkc7XMftfQvAiEB1pS0w3wDbDxw7BdaT8AzfnqBeiwUqbA5DZyeywyCUIar47LJA8dD9vpPwLHO6n3zFaoyMkITe2vA8YkbfdLPwA8yBo4AbAIcenCdaov2i+B0rXHZ1VCHvEjjvJ7GrWFqs9MbbvtkbKfevfK68oei19OOLIN+yRlBRvDn7xOEbDnnrmtLswAAAABJRU5ErkJggg==';
  } else {
    remove_icon = coreConfig.remove_icon;
  }
  // FIXME: add a way to use nested divs
  var tbody = parent.getElementsByTagName('tbody')[0];
  var inputOpts = formOptions.inputs;

  var self = {
    add: function () {
      var newRow = tbody.insertRow(-1);
      addInputs(newRow);
      organizeForm();
    }
    , remove: function (node) {
      if (tbody.children.length == 1) {
        return false;
      }
      tbody.removeChild(node);
      organizeForm();
      return true;
    }
  };

  // run that for each field actually..
  var addInputs = function (tableRow) {
    // iterate over configurated inputs
    for (
      var config of inputOpts) {
      var cell = tableRow.insertCell(-1);
      // FIXME: correct the behaviour..
      var input = document.createElement("INPUT");
      input.type = config.type;
      input.className = config.classname === undefined ? coreConfig.defaultClass :
        config.className;
      input.name = config.nametemplate;

      cell.appendChild(input);
    }
  };

  var organizeForm = function () {
    var rows = tbody.children;
    for (var r = 0; r < rows.length; r++) {
      while (rows[r].childElementCount > inputOpts.length) {
        rows[r].deleteCell(inputOpts.length);
      }
      var cell = rows[r].insertCell(-1);
      var remove = new Image();
      remove.src = remove_icon;
      remove.onclick = function () {
        self.remove(this.parentElement.parentElement); // remove row on click!
      }
      cell.appendChild(remove);
      // last row!
      if (r == rows.length - 1) {
        var add = new Image();
        add.src = add_icon;
        add.onclick = function () {
          self.add();
        }
        cell.appendChild(add);
      }
      // FIXME correct this behaviour
      // var txt = rows[r].children[1].children[0];
      // txt.name = inputOpts.nametemplate.replace(/%INDEX%/, r);
    }
  };
  organizeForm();
  return self;
}
        img {
          margin: 5px;
        }
        .df-input {
          padding: 5px;
        }
<html>
  <head>
  </head>
  <body>
    <script>
      window.onload = function() {
        var df = DynamicForm(document.getElementById('df-parent-0'), {
          defaultClass : 'df-input'
        }, {
          inputs: [
            {type : 'text', nametemplate : 'textfield'}
            , {type : 'checkbox', nametemplate : 'checkbox'}
          ]
        });
        df.add();
      };
    </script>
    This is some random content :D
    <form action="#" name="example">
      <table id="df-parent-0">
        <tbody>
        </tbody>
      </table>
    </form>
  </body>
</html>

I added some skeleton HTML and CSS to demonstrate that the code can currently do the following:

  • Add rows to a table according to a simplistic configuration
  • Remove rows from a table

Yes, it is intended that the minus sign does not do anything if there is only one row left.

Please note that the code is also licensed under the MIT License.

Concerns

I am concerned about:

  • Proper use of DOM manipulation
  • Idiomaticness of creating a DynamicForm

Note that I'm still getting into the guts of atom to make the automatic formatting work as I want it to, so comments on what needs cleanup there are much appreciated.

share|improve this question
    
For whatever reason it seems I cannot bind a listener to window.onload.... if there's someone that can fix the code-snippet in that respect, that would be absolutely awesome – Vogel612 Sep 6 at 23:06
2  
I think I got your snippet working. The visible: false flags for the HTML and CSS did hide those blocks, but also caused them to be completely ignored by the snippet system (couldn't "copy to answer" either). So the snippet was only loading your JS, but no content. I tried a similar thing once, but it seems it's all or nothing with snippets; can't hide/show individual parts. – Flambino Sep 7 at 1:48
    
I'm wondering; if the minus-button can't remove the last row, that implies you'll always want at least 1 row. If so, it would let you put that 1st row directly in the HTML and use it as a template for the following ones. Would that be an option? – Flambino Sep 9 at 14:23
    
while that would definitely be an option I am not sure I want to constrain myself to that behaviour. Especially when I want this to also work for nested div-structures that emulate tables... then again that might just be wishful thinking. That definitely sounds like an idea worth exploring :) Also the "minimum 1 row" thing is actually a crutch to always have a plus there, that doesn't want to be all alone :D – Vogel612 Sep 9 at 14:25
1  
True, it's a different approach. Yet it could also allow for more complex HTML. Anyway, I've been meaning to answer this question, just haven't had the time yet. Will get to it (and perhaps propose an alternative solution) – Flambino Sep 9 at 14:30

1 Answer 1

  1. Don't create unnecessary if-else block to check for add_icon and remove_icon respectively.

    var add_icon = coreConfig.add_icon || 'data:...BV58AAAAASUVORK5CYII=';
    

    would work just as well.

  2. I'd favour usage of tbody.rows over tbody.children.
  3. Try to have all your functions return something, just to be consistent.

    var self = {
        add: function () {
          var newRow = tbody.insertRow(-1);
          addInputs(newRow);
          return organizeForm();
        },
        remove: function (node) {
          if (tbody.children.length == 1) {
            return false;
          }
          tbody.removeChild(node);
          return organizeForm();
        }
    };
    

    and just put a return true in the organizeForm declaration. I am unaware whether JavaScript happens to have tail calls or not!

  4. Let the users' use actual attribute names when providing inputOpts. This will not confuse the new users, and they can always reference W3C definitions to see which attributes they can use. So, instead of classname and nametemplate; let there be className and name respectively.

    With the above in place, you won't need to individually assign the properties. This can be done with a simple deep-copy of properties across objects.

share|improve this answer
1  
I think you misunderstood the Goal of the nametemplate. The nametemplate is intended to allow the user to configure a Name, that changes depending on what row the Input has. That functionality is not quite finished yet though, since I wanted to get a very early Review. But all the other Points you make are very interesting and I think I will get all of them into the code. Thank you :) – Vogel612 Sep 18 at 11:26

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.