11
\$\begingroup\$

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.

\$\endgroup\$
  • \$\begingroup\$ 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 \$\endgroup\$ – Vogel612 Sep 6 '15 at 23:06
  • 2
    \$\begingroup\$ 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. \$\endgroup\$ – Flambino Sep 7 '15 at 1:48
  • \$\begingroup\$ 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? \$\endgroup\$ – Flambino Sep 9 '15 at 14:23
  • \$\begingroup\$ 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 \$\endgroup\$ – Vogel612 Sep 9 '15 at 14:25
  • 1
    \$\begingroup\$ 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) \$\endgroup\$ – Flambino Sep 9 '15 at 14:30
1
\$\begingroup\$

console.warn:

  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;
  }

Instead of using console.warn with a return, use throw new Error instead:

It kills the function entirely and provides a stack trace.


Big base64 strings:

I would store those big strings as constants at the top of the function as to improve readability, adding b64_ to the beginning: b64_add_icon

Then, you can use ternaries instead of the if statements:

var add_icon = coreConfig.add_icon === undefined ? b64_add_icon : coreConfig.add_icon;

snake_case:

You shouldn't be using snake_case in JavaScript, you should be using camelCase.


var self = {:

You shouldn't work it like that, just assign self as an empty object, and then work from there, or use the prototype chain.


for:

I'm not sure why the content of the for loop is on a different line, perhaps design influence from another language?

for (
  var config of inputOpts) {

also in another for loop, you can declare the length in the loop to save constant calls to the length property:

for (var r = 0; r < rows.length; r++) {

into:

for (var r = 0, length = rows.length; r < length; r++) {

onclick:

Instead of onclick, you should use addEventListener("click", ...

  remove.onclick = function () {
    self.remove(this.parentElement.parentElement); // remove row on click!
  }

into:

  remove.addEventListener('click', function () {
    self.remove(this.parentElement.parentElement); // remove row on click!
  });

Truthy and Falsy checks:

Be careful when using the == and != operators as they can falsely compare values, see here for more information.

if (r == rows.length - 1) {

Misc:

  • document.createElement("INPUT"): html tags aren't uppercase, and neither does the parameter string need to be.
  • Ternary improvement; I like to structure my ternaries the following way for maximum prettiness:
  input.className = config.classname === undefined ? coreConfig.defaultClass :
    config.className;

into:

  input.className = config.classname === undefined
                        ? coreConfig.defaultClass
                        : config.className;
\$\endgroup\$
1
\$\begingroup\$
  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.

\$\endgroup\$
  • 1
    \$\begingroup\$ 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 :) \$\endgroup\$ – Vogel612 Sep 18 '15 at 11:26

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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