JavaScript module to render and handle a form to add users Announcing the arrival of Valued...

Are my PIs rude or am I just being too sensitive?

How do I stop a creek from eroding my steep embankment?

Is there a documented rationale why the House Ways and Means chairman can demand tax info?

Gastric acid as a weapon

I am not a queen, who am I?

WAN encapsulation

Did Kevin spill real chili?

Does the Giant Rocktopus have a Swim Speed?

Models of set theory where not every set can be linearly ordered

Can inflation occur in a positive-sum game currency system such as the Stack Exchange reputation system?

Java 8 stream max() function argument type Comparator vs Comparable

How can I fade player when goes inside or outside of the area?

How to recreate this effect in Photoshop?

"Seemed to had" is it correct?

Is 1 ppb equal to 1 μg/kg?

Why was the term "discrete" used in discrete logarithm?

Is it possible to boil a liquid by just mixing many immiscible liquids together?

How discoverable are IPv6 addresses and AAAA names by potential attackers?

Check which numbers satisfy the condition [A*B*C = A! + B! + C!]

How to deal with a team lead who never gives me credit?

How widely used is the term Treppenwitz? Is it something that most Germans know?

How to draw this diagram using TikZ package?

Does accepting a pardon have any bearing on trying that person for the same crime in a sovereign jurisdiction?

What is a Meta algorithm?



JavaScript module to render and handle a form to add users



Announcing the arrival of Valued Associate #679: Cesar Manara
Planned maintenance scheduled April 17/18, 2019 at 00:00UTC (8:00pm US/Eastern)Showing statistics of popular JavaScript frameworks from GitHub APICode to call Space X API and display resultseDomForm - dynamic forms without writing any JavaScriptManaging a user databaseHow to automatically insert HTML to a form and obtain the inputs?Making an array editable by userRecognizing elements and doing event handling for repeating templatesUsing/storing server-side data on the client sideBest practice of HTML DOM template in javascriptJS micro-view libraryExpand form at runtime using a templateAuthentication system (no browser) with mail verification





.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty,.everyoneloves__bot-mid-leaderboard:empty{ margin-bottom:0;
}







4












$begingroup$


I recently switched to modular JavaScript and really like the idea of having the state of your application in JavaScript and not in the DOM. I want to know if what I am doing is considered best practices or not and how I can avoid re-rendering the entire list after each change.



First, the code:



HTML



<form id="add-user-form">
<input type="text" placeholder="username" name="username">
<button>add User</button>
</form>
<hr>
<ul id="user-list"></ul>


<script id="user-template" type="text/html">
 <li data-key="${key}">${name}</li>
</script>


JavaScript



// lets you get templates from HTML ( as you can see in the HTML above ). 
// This is to avoid writing out HTML inside JavaScript
const Template = {
cached: {},
 get(templateId, placeholders) {
let templateHtml = ''
if (!this.cached[templateId]) {
templateHtml = document.getElementById(templateId).innerHTML.trim()
this.cached[templateId] = templateHtml
}
else {
templateHtml = this.cached[templateId]
}
 
for (key in placeholders) {
templateHtml = templateHtml.replace(new RegExp("\${\s*" + key + "\s*}", "g"), placeholders[key]);
}
return templateHtml
 }
}

const Users = {
users: [],
init() {
this.cacheDom()
this.bindEvents()
},
cacheDom() {
this.addUserFormEl = document.getElementById('add-user-form')
this.userListEl = document.getElementById('user-list')
},
bindEvents() {
this.addUserFormEl.addEventListener('submit', this.handleAddUser.bind(this))
},
render() {
// gets the HTML from ALL users of our user list and replaces the current user list
const userHtml = this.users.map( user => Template.get('user-template', user) ).join('')
this.userListEl.innerHTML = userHtml
},
handleAddUser(e) {
e.preventDefault()

const username = e.currentTarget.elements['username'].value
if (!username) return
e.currentTarget.elements['username'].value = ''

const key = this.users.length // I know I know this is bad practice, please ignore :)

this.users.push({
key: key,
name: username
})

this.render()
}
}

Users.init()


The code has a form with an input field where you can type in a name that, once the form gets submitted, will be added to the user list below. It would probably be a good idea to separate the userlist and the userform into seperate concerns, but for the sake of this example I kept it simple.



I want to keep DOM modifications to a minimum so I put them inside the method Users.render. This however has one side-effect. Whenever I add an user, it reloads the entire list. It might work with this amount of data, but let's say I have thousands of records, maybe even with input fields. These would all be emptied again.



I could of course just add the new entry to the DOM inside the Users.handleAddUser method but what if later I also want to delete users? I would have to delete the entry within Users.handleRemoveUser and would then have two methods that handle DOM modifications for the same thing.



I know this all can be easily and beautifully achieved with frameworks such as VueJs that use a vDOM but I am looking for a vanillaJs way of handling this.
(Please note that the code is just an example. Yes Unit testing and typeScript are missing and I am well aware of the fact that the code will not run in browsers that do not support ES-6).










share|improve this question











$endgroup$



















    4












    $begingroup$


    I recently switched to modular JavaScript and really like the idea of having the state of your application in JavaScript and not in the DOM. I want to know if what I am doing is considered best practices or not and how I can avoid re-rendering the entire list after each change.



    First, the code:



    HTML



    <form id="add-user-form">
    <input type="text" placeholder="username" name="username">
    <button>add User</button>
    </form>
    <hr>
    <ul id="user-list"></ul>


    <script id="user-template" type="text/html">
     <li data-key="${key}">${name}</li>
    </script>


    JavaScript



    // lets you get templates from HTML ( as you can see in the HTML above ). 
    // This is to avoid writing out HTML inside JavaScript
    const Template = {
    cached: {},
     get(templateId, placeholders) {
    let templateHtml = ''
    if (!this.cached[templateId]) {
    templateHtml = document.getElementById(templateId).innerHTML.trim()
    this.cached[templateId] = templateHtml
    }
    else {
    templateHtml = this.cached[templateId]
    }
     
    for (key in placeholders) {
    templateHtml = templateHtml.replace(new RegExp("\${\s*" + key + "\s*}", "g"), placeholders[key]);
    }
    return templateHtml
     }
    }

    const Users = {
    users: [],
    init() {
    this.cacheDom()
    this.bindEvents()
    },
    cacheDom() {
    this.addUserFormEl = document.getElementById('add-user-form')
    this.userListEl = document.getElementById('user-list')
    },
    bindEvents() {
    this.addUserFormEl.addEventListener('submit', this.handleAddUser.bind(this))
    },
    render() {
    // gets the HTML from ALL users of our user list and replaces the current user list
    const userHtml = this.users.map( user => Template.get('user-template', user) ).join('')
    this.userListEl.innerHTML = userHtml
    },
    handleAddUser(e) {
    e.preventDefault()

    const username = e.currentTarget.elements['username'].value
    if (!username) return
    e.currentTarget.elements['username'].value = ''

    const key = this.users.length // I know I know this is bad practice, please ignore :)

    this.users.push({
    key: key,
    name: username
    })

    this.render()
    }
    }

    Users.init()


    The code has a form with an input field where you can type in a name that, once the form gets submitted, will be added to the user list below. It would probably be a good idea to separate the userlist and the userform into seperate concerns, but for the sake of this example I kept it simple.



    I want to keep DOM modifications to a minimum so I put them inside the method Users.render. This however has one side-effect. Whenever I add an user, it reloads the entire list. It might work with this amount of data, but let's say I have thousands of records, maybe even with input fields. These would all be emptied again.



    I could of course just add the new entry to the DOM inside the Users.handleAddUser method but what if later I also want to delete users? I would have to delete the entry within Users.handleRemoveUser and would then have two methods that handle DOM modifications for the same thing.



    I know this all can be easily and beautifully achieved with frameworks such as VueJs that use a vDOM but I am looking for a vanillaJs way of handling this.
    (Please note that the code is just an example. Yes Unit testing and typeScript are missing and I am well aware of the fact that the code will not run in browsers that do not support ES-6).










    share|improve this question











    $endgroup$















      4












      4








      4


      3



      $begingroup$


      I recently switched to modular JavaScript and really like the idea of having the state of your application in JavaScript and not in the DOM. I want to know if what I am doing is considered best practices or not and how I can avoid re-rendering the entire list after each change.



      First, the code:



      HTML



      <form id="add-user-form">
      <input type="text" placeholder="username" name="username">
      <button>add User</button>
      </form>
      <hr>
      <ul id="user-list"></ul>


      <script id="user-template" type="text/html">
       <li data-key="${key}">${name}</li>
      </script>


      JavaScript



      // lets you get templates from HTML ( as you can see in the HTML above ). 
      // This is to avoid writing out HTML inside JavaScript
      const Template = {
      cached: {},
       get(templateId, placeholders) {
      let templateHtml = ''
      if (!this.cached[templateId]) {
      templateHtml = document.getElementById(templateId).innerHTML.trim()
      this.cached[templateId] = templateHtml
      }
      else {
      templateHtml = this.cached[templateId]
      }
       
      for (key in placeholders) {
      templateHtml = templateHtml.replace(new RegExp("\${\s*" + key + "\s*}", "g"), placeholders[key]);
      }
      return templateHtml
       }
      }

      const Users = {
      users: [],
      init() {
      this.cacheDom()
      this.bindEvents()
      },
      cacheDom() {
      this.addUserFormEl = document.getElementById('add-user-form')
      this.userListEl = document.getElementById('user-list')
      },
      bindEvents() {
      this.addUserFormEl.addEventListener('submit', this.handleAddUser.bind(this))
      },
      render() {
      // gets the HTML from ALL users of our user list and replaces the current user list
      const userHtml = this.users.map( user => Template.get('user-template', user) ).join('')
      this.userListEl.innerHTML = userHtml
      },
      handleAddUser(e) {
      e.preventDefault()

      const username = e.currentTarget.elements['username'].value
      if (!username) return
      e.currentTarget.elements['username'].value = ''

      const key = this.users.length // I know I know this is bad practice, please ignore :)

      this.users.push({
      key: key,
      name: username
      })

      this.render()
      }
      }

      Users.init()


      The code has a form with an input field where you can type in a name that, once the form gets submitted, will be added to the user list below. It would probably be a good idea to separate the userlist and the userform into seperate concerns, but for the sake of this example I kept it simple.



      I want to keep DOM modifications to a minimum so I put them inside the method Users.render. This however has one side-effect. Whenever I add an user, it reloads the entire list. It might work with this amount of data, but let's say I have thousands of records, maybe even with input fields. These would all be emptied again.



      I could of course just add the new entry to the DOM inside the Users.handleAddUser method but what if later I also want to delete users? I would have to delete the entry within Users.handleRemoveUser and would then have two methods that handle DOM modifications for the same thing.



      I know this all can be easily and beautifully achieved with frameworks such as VueJs that use a vDOM but I am looking for a vanillaJs way of handling this.
      (Please note that the code is just an example. Yes Unit testing and typeScript are missing and I am well aware of the fact that the code will not run in browsers that do not support ES-6).










      share|improve this question











      $endgroup$




      I recently switched to modular JavaScript and really like the idea of having the state of your application in JavaScript and not in the DOM. I want to know if what I am doing is considered best practices or not and how I can avoid re-rendering the entire list after each change.



      First, the code:



      HTML



      <form id="add-user-form">
      <input type="text" placeholder="username" name="username">
      <button>add User</button>
      </form>
      <hr>
      <ul id="user-list"></ul>


      <script id="user-template" type="text/html">
       <li data-key="${key}">${name}</li>
      </script>


      JavaScript



      // lets you get templates from HTML ( as you can see in the HTML above ). 
      // This is to avoid writing out HTML inside JavaScript
      const Template = {
      cached: {},
       get(templateId, placeholders) {
      let templateHtml = ''
      if (!this.cached[templateId]) {
      templateHtml = document.getElementById(templateId).innerHTML.trim()
      this.cached[templateId] = templateHtml
      }
      else {
      templateHtml = this.cached[templateId]
      }
       
      for (key in placeholders) {
      templateHtml = templateHtml.replace(new RegExp("\${\s*" + key + "\s*}", "g"), placeholders[key]);
      }
      return templateHtml
       }
      }

      const Users = {
      users: [],
      init() {
      this.cacheDom()
      this.bindEvents()
      },
      cacheDom() {
      this.addUserFormEl = document.getElementById('add-user-form')
      this.userListEl = document.getElementById('user-list')
      },
      bindEvents() {
      this.addUserFormEl.addEventListener('submit', this.handleAddUser.bind(this))
      },
      render() {
      // gets the HTML from ALL users of our user list and replaces the current user list
      const userHtml = this.users.map( user => Template.get('user-template', user) ).join('')
      this.userListEl.innerHTML = userHtml
      },
      handleAddUser(e) {
      e.preventDefault()

      const username = e.currentTarget.elements['username'].value
      if (!username) return
      e.currentTarget.elements['username'].value = ''

      const key = this.users.length // I know I know this is bad practice, please ignore :)

      this.users.push({
      key: key,
      name: username
      })

      this.render()
      }
      }

      Users.init()


      The code has a form with an input field where you can type in a name that, once the form gets submitted, will be added to the user list below. It would probably be a good idea to separate the userlist and the userform into seperate concerns, but for the sake of this example I kept it simple.



      I want to keep DOM modifications to a minimum so I put them inside the method Users.render. This however has one side-effect. Whenever I add an user, it reloads the entire list. It might work with this amount of data, but let's say I have thousands of records, maybe even with input fields. These would all be emptied again.



      I could of course just add the new entry to the DOM inside the Users.handleAddUser method but what if later I also want to delete users? I would have to delete the entry within Users.handleRemoveUser and would then have two methods that handle DOM modifications for the same thing.



      I know this all can be easily and beautifully achieved with frameworks such as VueJs that use a vDOM but I am looking for a vanillaJs way of handling this.
      (Please note that the code is just an example. Yes Unit testing and typeScript are missing and I am well aware of the fact that the code will not run in browsers that do not support ES-6).







      javascript html template ecmascript-6 dom






      share|improve this question















      share|improve this question













      share|improve this question




      share|improve this question








      edited Dec 5 '18 at 22:54









      Sᴀᴍ Onᴇᴌᴀ

      10.6k62168




      10.6k62168










      asked Mar 21 '18 at 2:53









      MichaelMichael

      212




      212






















          1 Answer
          1






          active

          oldest

          votes


















          0












          $begingroup$

          Your Concerns/Questions




          Whenever I add an user, it reloads the entire list.




          One alternate approach would be to have the handleAddUser() store the newly added user in a property (e.g. recentlyAdded) and then have the render() method look for that property - if it is set (to something other than null) then add the rendered template of the new user and clear that property.



          When the page loads, is there an existing list of users that gets added to the list? If so, maybe those could be stored in a different property and the existing code in render() could look for that property for rendering the existing records.




          I could of course just add the new entry to the DOM inside the Users.handleAddUser method but what if later, I also want to delete users. I would have to delete the entry within Users.handleRemoveUser and would so already have two methods that handle DOM modifications for the same thing.




          Going along with the alternate approach above, one could have the handleRemoveUser method also store the key of the recently removed user, and then the render() method can look for that property too and remove such an element associated with that key - perhaps by adding an id or other data attribute to the template.



          You didn't include an implementation for handleRemoveUser so I can only guess as to what it would be: some way of removing the user from the list this.user. If this.users is still an array, then looking for the user to remove might require a loop. However if this.users is an associative-array (i.e. object with keys corresponding to the key of each user) (or a Map), then looking for the user to remove can be achieved without a loop.



          See this demonstrated in the snippet below.





          In revision 2 you had this question, altered in revision 5:




          As you can see I am not getting the input by ID but by form.elements and then referencing the name.




          It is fine to do that, but because the form data doesn't get sent to the server-side, you could also use the id attribute on the input instead of the name attribute, and then fetch that element by id.



          Other Feedback



          The Template.get() function loops over the keys in placeholders:




          for (key in placeholders) {



          For this loop, key is a global variable, which may or not be intentional. In general it is best to avoid global variables. One reason would be that if code in a separate function makes its own variable with the same name, there would be no chance of overwriting the value if it isn't a global variable. Use const to declare key as local to that loop.



          for (const key in placeholders) {


          I recently saw this SO answer that offers a solution to interpolating a string literal similar to how a template literal would be interpolated. I compared it with the regular expression approach in your code and found it to be much slower (see this jsPerf test). I was hoping I could offer that suggestion but unfortunately it can be much slower.






          // lets you get templates from HTML ( as you can see in the HTML above ). 
          // This is to avoid writing out HTML inside JavaScript
          const Template = {
          cached: {},
          get(templateId, placeholders) {
          let templateHtml = ''
          if (!this.cached[templateId]) {
          templateHtml = document.getElementById(templateId).innerHTML.trim()
          this.cached[templateId] = templateHtml
          }
          else {
          templateHtml = this.cached[templateId]
          }

          for (let key in placeholders) {
          templateHtml = templateHtml.replace(new RegExp("\${\s*" + key + "\s*}", "g"), placeholders[key]);
          }
          return templateHtml
          }
          }

          const Users = {
          users: {},
          recentlyAdded: null,
          recentlyRemovedKey: null,
          nextKey: 1,
          init() {
          this.cacheDom()
          this.bindEvents()
          },
          cacheDom() {
          this.addUserFormEl = document.getElementById('add-user-form')
          this.userListEl = document.getElementById('user-list')
          },
          bindEvents() {
          this.addUserFormEl.addEventListener('submit', this.handleAddUser.bind(this))
          this.userListEl.addEventListener('click', this.handleListClick.bind(this));
          },
          render() {
          // gets the HTML from ALL users of our user list and replaces the current user list
          /*const userHtml = this.users.map( user => Template.get('user-template', user) ).join('')
          this.userListEl.innerHTML = userHtml*/
          if (this.recentlyAdded) {
          this.userListEl.innerHTML += Template.get('user-template', this.recentlyAdded);
          this.recentlyAdded = null;
          }
          if (this.recentlyRemovedKey) {
          const element = document.getElementById(this.recentlyRemovedKey);
          if (element) {
          element.remove();
          }
          this.recentlyRemoved = null;
          }
          console.log('userlist after render: ',this.users);
          },
          handleAddUser(e) {
          e.preventDefault()

          const username = e.currentTarget.elements['username'].value
          if (!username) return
          e.currentTarget.elements['username'].value = ''

          const key = this.nextKey++;

          this.users[key] = {
          key: key,
          name: username
          };
          this.recentlyAdded = this.users[key];

          this.render();
          },
          handleListClick(e) {
          const target = e.target;
          if (target.classList.contains('remove')) {
          this.handleRemoveUser(target.parentNode.id);
          }
          },
          handleRemoveUser(key) {
          delete this.users[key];
          this.recentlyRemovedKey = key;
          this.render();
          }
          }

          Users.init()

          .remove:before {
          content: 'X';
          }

          <form id="add-user-form">
          <input type="text" placeholder="username" name="username">
          <button>add User</button>
          </form>
          <hr>
          <ul id="user-list"></ul>


          <script id="user-template" type="text/html">
          <li data-key="${key}" id="${key}">${name} <button class="remove"></button></li>
          </script>








          share|improve this answer











          $endgroup$














            Your Answer






            StackExchange.ifUsing("editor", function () {
            StackExchange.using("externalEditor", function () {
            StackExchange.using("snippets", function () {
            StackExchange.snippets.init();
            });
            });
            }, "code-snippets");

            StackExchange.ready(function() {
            var channelOptions = {
            tags: "".split(" "),
            id: "196"
            };
            initTagRenderer("".split(" "), "".split(" "), channelOptions);

            StackExchange.using("externalEditor", function() {
            // Have to fire editor after snippets, if snippets enabled
            if (StackExchange.settings.snippets.snippetsEnabled) {
            StackExchange.using("snippets", function() {
            createEditor();
            });
            }
            else {
            createEditor();
            }
            });

            function createEditor() {
            StackExchange.prepareEditor({
            heartbeatType: 'answer',
            autoActivateHeartbeat: false,
            convertImagesToLinks: false,
            noModals: true,
            showLowRepImageUploadWarning: true,
            reputationToPostImages: null,
            bindNavPrevention: true,
            postfix: "",
            imageUploader: {
            brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
            contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
            allowUrls: true
            },
            onDemand: true,
            discardSelector: ".discard-answer"
            ,immediatelyShowMarkdownHelp:true
            });


            }
            });














            draft saved

            draft discarded


















            StackExchange.ready(
            function () {
            StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f190088%2fjavascript-module-to-render-and-handle-a-form-to-add-users%23new-answer', 'question_page');
            }
            );

            Post as a guest















            Required, but never shown

























            1 Answer
            1






            active

            oldest

            votes








            1 Answer
            1






            active

            oldest

            votes









            active

            oldest

            votes






            active

            oldest

            votes









            0












            $begingroup$

            Your Concerns/Questions




            Whenever I add an user, it reloads the entire list.




            One alternate approach would be to have the handleAddUser() store the newly added user in a property (e.g. recentlyAdded) and then have the render() method look for that property - if it is set (to something other than null) then add the rendered template of the new user and clear that property.



            When the page loads, is there an existing list of users that gets added to the list? If so, maybe those could be stored in a different property and the existing code in render() could look for that property for rendering the existing records.




            I could of course just add the new entry to the DOM inside the Users.handleAddUser method but what if later, I also want to delete users. I would have to delete the entry within Users.handleRemoveUser and would so already have two methods that handle DOM modifications for the same thing.




            Going along with the alternate approach above, one could have the handleRemoveUser method also store the key of the recently removed user, and then the render() method can look for that property too and remove such an element associated with that key - perhaps by adding an id or other data attribute to the template.



            You didn't include an implementation for handleRemoveUser so I can only guess as to what it would be: some way of removing the user from the list this.user. If this.users is still an array, then looking for the user to remove might require a loop. However if this.users is an associative-array (i.e. object with keys corresponding to the key of each user) (or a Map), then looking for the user to remove can be achieved without a loop.



            See this demonstrated in the snippet below.





            In revision 2 you had this question, altered in revision 5:




            As you can see I am not getting the input by ID but by form.elements and then referencing the name.




            It is fine to do that, but because the form data doesn't get sent to the server-side, you could also use the id attribute on the input instead of the name attribute, and then fetch that element by id.



            Other Feedback



            The Template.get() function loops over the keys in placeholders:




            for (key in placeholders) {



            For this loop, key is a global variable, which may or not be intentional. In general it is best to avoid global variables. One reason would be that if code in a separate function makes its own variable with the same name, there would be no chance of overwriting the value if it isn't a global variable. Use const to declare key as local to that loop.



            for (const key in placeholders) {


            I recently saw this SO answer that offers a solution to interpolating a string literal similar to how a template literal would be interpolated. I compared it with the regular expression approach in your code and found it to be much slower (see this jsPerf test). I was hoping I could offer that suggestion but unfortunately it can be much slower.






            // lets you get templates from HTML ( as you can see in the HTML above ). 
            // This is to avoid writing out HTML inside JavaScript
            const Template = {
            cached: {},
            get(templateId, placeholders) {
            let templateHtml = ''
            if (!this.cached[templateId]) {
            templateHtml = document.getElementById(templateId).innerHTML.trim()
            this.cached[templateId] = templateHtml
            }
            else {
            templateHtml = this.cached[templateId]
            }

            for (let key in placeholders) {
            templateHtml = templateHtml.replace(new RegExp("\${\s*" + key + "\s*}", "g"), placeholders[key]);
            }
            return templateHtml
            }
            }

            const Users = {
            users: {},
            recentlyAdded: null,
            recentlyRemovedKey: null,
            nextKey: 1,
            init() {
            this.cacheDom()
            this.bindEvents()
            },
            cacheDom() {
            this.addUserFormEl = document.getElementById('add-user-form')
            this.userListEl = document.getElementById('user-list')
            },
            bindEvents() {
            this.addUserFormEl.addEventListener('submit', this.handleAddUser.bind(this))
            this.userListEl.addEventListener('click', this.handleListClick.bind(this));
            },
            render() {
            // gets the HTML from ALL users of our user list and replaces the current user list
            /*const userHtml = this.users.map( user => Template.get('user-template', user) ).join('')
            this.userListEl.innerHTML = userHtml*/
            if (this.recentlyAdded) {
            this.userListEl.innerHTML += Template.get('user-template', this.recentlyAdded);
            this.recentlyAdded = null;
            }
            if (this.recentlyRemovedKey) {
            const element = document.getElementById(this.recentlyRemovedKey);
            if (element) {
            element.remove();
            }
            this.recentlyRemoved = null;
            }
            console.log('userlist after render: ',this.users);
            },
            handleAddUser(e) {
            e.preventDefault()

            const username = e.currentTarget.elements['username'].value
            if (!username) return
            e.currentTarget.elements['username'].value = ''

            const key = this.nextKey++;

            this.users[key] = {
            key: key,
            name: username
            };
            this.recentlyAdded = this.users[key];

            this.render();
            },
            handleListClick(e) {
            const target = e.target;
            if (target.classList.contains('remove')) {
            this.handleRemoveUser(target.parentNode.id);
            }
            },
            handleRemoveUser(key) {
            delete this.users[key];
            this.recentlyRemovedKey = key;
            this.render();
            }
            }

            Users.init()

            .remove:before {
            content: 'X';
            }

            <form id="add-user-form">
            <input type="text" placeholder="username" name="username">
            <button>add User</button>
            </form>
            <hr>
            <ul id="user-list"></ul>


            <script id="user-template" type="text/html">
            <li data-key="${key}" id="${key}">${name} <button class="remove"></button></li>
            </script>








            share|improve this answer











            $endgroup$


















              0












              $begingroup$

              Your Concerns/Questions




              Whenever I add an user, it reloads the entire list.




              One alternate approach would be to have the handleAddUser() store the newly added user in a property (e.g. recentlyAdded) and then have the render() method look for that property - if it is set (to something other than null) then add the rendered template of the new user and clear that property.



              When the page loads, is there an existing list of users that gets added to the list? If so, maybe those could be stored in a different property and the existing code in render() could look for that property for rendering the existing records.




              I could of course just add the new entry to the DOM inside the Users.handleAddUser method but what if later, I also want to delete users. I would have to delete the entry within Users.handleRemoveUser and would so already have two methods that handle DOM modifications for the same thing.




              Going along with the alternate approach above, one could have the handleRemoveUser method also store the key of the recently removed user, and then the render() method can look for that property too and remove such an element associated with that key - perhaps by adding an id or other data attribute to the template.



              You didn't include an implementation for handleRemoveUser so I can only guess as to what it would be: some way of removing the user from the list this.user. If this.users is still an array, then looking for the user to remove might require a loop. However if this.users is an associative-array (i.e. object with keys corresponding to the key of each user) (or a Map), then looking for the user to remove can be achieved without a loop.



              See this demonstrated in the snippet below.





              In revision 2 you had this question, altered in revision 5:




              As you can see I am not getting the input by ID but by form.elements and then referencing the name.




              It is fine to do that, but because the form data doesn't get sent to the server-side, you could also use the id attribute on the input instead of the name attribute, and then fetch that element by id.



              Other Feedback



              The Template.get() function loops over the keys in placeholders:




              for (key in placeholders) {



              For this loop, key is a global variable, which may or not be intentional. In general it is best to avoid global variables. One reason would be that if code in a separate function makes its own variable with the same name, there would be no chance of overwriting the value if it isn't a global variable. Use const to declare key as local to that loop.



              for (const key in placeholders) {


              I recently saw this SO answer that offers a solution to interpolating a string literal similar to how a template literal would be interpolated. I compared it with the regular expression approach in your code and found it to be much slower (see this jsPerf test). I was hoping I could offer that suggestion but unfortunately it can be much slower.






              // lets you get templates from HTML ( as you can see in the HTML above ). 
              // This is to avoid writing out HTML inside JavaScript
              const Template = {
              cached: {},
              get(templateId, placeholders) {
              let templateHtml = ''
              if (!this.cached[templateId]) {
              templateHtml = document.getElementById(templateId).innerHTML.trim()
              this.cached[templateId] = templateHtml
              }
              else {
              templateHtml = this.cached[templateId]
              }

              for (let key in placeholders) {
              templateHtml = templateHtml.replace(new RegExp("\${\s*" + key + "\s*}", "g"), placeholders[key]);
              }
              return templateHtml
              }
              }

              const Users = {
              users: {},
              recentlyAdded: null,
              recentlyRemovedKey: null,
              nextKey: 1,
              init() {
              this.cacheDom()
              this.bindEvents()
              },
              cacheDom() {
              this.addUserFormEl = document.getElementById('add-user-form')
              this.userListEl = document.getElementById('user-list')
              },
              bindEvents() {
              this.addUserFormEl.addEventListener('submit', this.handleAddUser.bind(this))
              this.userListEl.addEventListener('click', this.handleListClick.bind(this));
              },
              render() {
              // gets the HTML from ALL users of our user list and replaces the current user list
              /*const userHtml = this.users.map( user => Template.get('user-template', user) ).join('')
              this.userListEl.innerHTML = userHtml*/
              if (this.recentlyAdded) {
              this.userListEl.innerHTML += Template.get('user-template', this.recentlyAdded);
              this.recentlyAdded = null;
              }
              if (this.recentlyRemovedKey) {
              const element = document.getElementById(this.recentlyRemovedKey);
              if (element) {
              element.remove();
              }
              this.recentlyRemoved = null;
              }
              console.log('userlist after render: ',this.users);
              },
              handleAddUser(e) {
              e.preventDefault()

              const username = e.currentTarget.elements['username'].value
              if (!username) return
              e.currentTarget.elements['username'].value = ''

              const key = this.nextKey++;

              this.users[key] = {
              key: key,
              name: username
              };
              this.recentlyAdded = this.users[key];

              this.render();
              },
              handleListClick(e) {
              const target = e.target;
              if (target.classList.contains('remove')) {
              this.handleRemoveUser(target.parentNode.id);
              }
              },
              handleRemoveUser(key) {
              delete this.users[key];
              this.recentlyRemovedKey = key;
              this.render();
              }
              }

              Users.init()

              .remove:before {
              content: 'X';
              }

              <form id="add-user-form">
              <input type="text" placeholder="username" name="username">
              <button>add User</button>
              </form>
              <hr>
              <ul id="user-list"></ul>


              <script id="user-template" type="text/html">
              <li data-key="${key}" id="${key}">${name} <button class="remove"></button></li>
              </script>








              share|improve this answer











              $endgroup$
















                0












                0








                0





                $begingroup$

                Your Concerns/Questions




                Whenever I add an user, it reloads the entire list.




                One alternate approach would be to have the handleAddUser() store the newly added user in a property (e.g. recentlyAdded) and then have the render() method look for that property - if it is set (to something other than null) then add the rendered template of the new user and clear that property.



                When the page loads, is there an existing list of users that gets added to the list? If so, maybe those could be stored in a different property and the existing code in render() could look for that property for rendering the existing records.




                I could of course just add the new entry to the DOM inside the Users.handleAddUser method but what if later, I also want to delete users. I would have to delete the entry within Users.handleRemoveUser and would so already have two methods that handle DOM modifications for the same thing.




                Going along with the alternate approach above, one could have the handleRemoveUser method also store the key of the recently removed user, and then the render() method can look for that property too and remove such an element associated with that key - perhaps by adding an id or other data attribute to the template.



                You didn't include an implementation for handleRemoveUser so I can only guess as to what it would be: some way of removing the user from the list this.user. If this.users is still an array, then looking for the user to remove might require a loop. However if this.users is an associative-array (i.e. object with keys corresponding to the key of each user) (or a Map), then looking for the user to remove can be achieved without a loop.



                See this demonstrated in the snippet below.





                In revision 2 you had this question, altered in revision 5:




                As you can see I am not getting the input by ID but by form.elements and then referencing the name.




                It is fine to do that, but because the form data doesn't get sent to the server-side, you could also use the id attribute on the input instead of the name attribute, and then fetch that element by id.



                Other Feedback



                The Template.get() function loops over the keys in placeholders:




                for (key in placeholders) {



                For this loop, key is a global variable, which may or not be intentional. In general it is best to avoid global variables. One reason would be that if code in a separate function makes its own variable with the same name, there would be no chance of overwriting the value if it isn't a global variable. Use const to declare key as local to that loop.



                for (const key in placeholders) {


                I recently saw this SO answer that offers a solution to interpolating a string literal similar to how a template literal would be interpolated. I compared it with the regular expression approach in your code and found it to be much slower (see this jsPerf test). I was hoping I could offer that suggestion but unfortunately it can be much slower.






                // lets you get templates from HTML ( as you can see in the HTML above ). 
                // This is to avoid writing out HTML inside JavaScript
                const Template = {
                cached: {},
                get(templateId, placeholders) {
                let templateHtml = ''
                if (!this.cached[templateId]) {
                templateHtml = document.getElementById(templateId).innerHTML.trim()
                this.cached[templateId] = templateHtml
                }
                else {
                templateHtml = this.cached[templateId]
                }

                for (let key in placeholders) {
                templateHtml = templateHtml.replace(new RegExp("\${\s*" + key + "\s*}", "g"), placeholders[key]);
                }
                return templateHtml
                }
                }

                const Users = {
                users: {},
                recentlyAdded: null,
                recentlyRemovedKey: null,
                nextKey: 1,
                init() {
                this.cacheDom()
                this.bindEvents()
                },
                cacheDom() {
                this.addUserFormEl = document.getElementById('add-user-form')
                this.userListEl = document.getElementById('user-list')
                },
                bindEvents() {
                this.addUserFormEl.addEventListener('submit', this.handleAddUser.bind(this))
                this.userListEl.addEventListener('click', this.handleListClick.bind(this));
                },
                render() {
                // gets the HTML from ALL users of our user list and replaces the current user list
                /*const userHtml = this.users.map( user => Template.get('user-template', user) ).join('')
                this.userListEl.innerHTML = userHtml*/
                if (this.recentlyAdded) {
                this.userListEl.innerHTML += Template.get('user-template', this.recentlyAdded);
                this.recentlyAdded = null;
                }
                if (this.recentlyRemovedKey) {
                const element = document.getElementById(this.recentlyRemovedKey);
                if (element) {
                element.remove();
                }
                this.recentlyRemoved = null;
                }
                console.log('userlist after render: ',this.users);
                },
                handleAddUser(e) {
                e.preventDefault()

                const username = e.currentTarget.elements['username'].value
                if (!username) return
                e.currentTarget.elements['username'].value = ''

                const key = this.nextKey++;

                this.users[key] = {
                key: key,
                name: username
                };
                this.recentlyAdded = this.users[key];

                this.render();
                },
                handleListClick(e) {
                const target = e.target;
                if (target.classList.contains('remove')) {
                this.handleRemoveUser(target.parentNode.id);
                }
                },
                handleRemoveUser(key) {
                delete this.users[key];
                this.recentlyRemovedKey = key;
                this.render();
                }
                }

                Users.init()

                .remove:before {
                content: 'X';
                }

                <form id="add-user-form">
                <input type="text" placeholder="username" name="username">
                <button>add User</button>
                </form>
                <hr>
                <ul id="user-list"></ul>


                <script id="user-template" type="text/html">
                <li data-key="${key}" id="${key}">${name} <button class="remove"></button></li>
                </script>








                share|improve this answer











                $endgroup$



                Your Concerns/Questions




                Whenever I add an user, it reloads the entire list.




                One alternate approach would be to have the handleAddUser() store the newly added user in a property (e.g. recentlyAdded) and then have the render() method look for that property - if it is set (to something other than null) then add the rendered template of the new user and clear that property.



                When the page loads, is there an existing list of users that gets added to the list? If so, maybe those could be stored in a different property and the existing code in render() could look for that property for rendering the existing records.




                I could of course just add the new entry to the DOM inside the Users.handleAddUser method but what if later, I also want to delete users. I would have to delete the entry within Users.handleRemoveUser and would so already have two methods that handle DOM modifications for the same thing.




                Going along with the alternate approach above, one could have the handleRemoveUser method also store the key of the recently removed user, and then the render() method can look for that property too and remove such an element associated with that key - perhaps by adding an id or other data attribute to the template.



                You didn't include an implementation for handleRemoveUser so I can only guess as to what it would be: some way of removing the user from the list this.user. If this.users is still an array, then looking for the user to remove might require a loop. However if this.users is an associative-array (i.e. object with keys corresponding to the key of each user) (or a Map), then looking for the user to remove can be achieved without a loop.



                See this demonstrated in the snippet below.





                In revision 2 you had this question, altered in revision 5:




                As you can see I am not getting the input by ID but by form.elements and then referencing the name.




                It is fine to do that, but because the form data doesn't get sent to the server-side, you could also use the id attribute on the input instead of the name attribute, and then fetch that element by id.



                Other Feedback



                The Template.get() function loops over the keys in placeholders:




                for (key in placeholders) {



                For this loop, key is a global variable, which may or not be intentional. In general it is best to avoid global variables. One reason would be that if code in a separate function makes its own variable with the same name, there would be no chance of overwriting the value if it isn't a global variable. Use const to declare key as local to that loop.



                for (const key in placeholders) {


                I recently saw this SO answer that offers a solution to interpolating a string literal similar to how a template literal would be interpolated. I compared it with the regular expression approach in your code and found it to be much slower (see this jsPerf test). I was hoping I could offer that suggestion but unfortunately it can be much slower.






                // lets you get templates from HTML ( as you can see in the HTML above ). 
                // This is to avoid writing out HTML inside JavaScript
                const Template = {
                cached: {},
                get(templateId, placeholders) {
                let templateHtml = ''
                if (!this.cached[templateId]) {
                templateHtml = document.getElementById(templateId).innerHTML.trim()
                this.cached[templateId] = templateHtml
                }
                else {
                templateHtml = this.cached[templateId]
                }

                for (let key in placeholders) {
                templateHtml = templateHtml.replace(new RegExp("\${\s*" + key + "\s*}", "g"), placeholders[key]);
                }
                return templateHtml
                }
                }

                const Users = {
                users: {},
                recentlyAdded: null,
                recentlyRemovedKey: null,
                nextKey: 1,
                init() {
                this.cacheDom()
                this.bindEvents()
                },
                cacheDom() {
                this.addUserFormEl = document.getElementById('add-user-form')
                this.userListEl = document.getElementById('user-list')
                },
                bindEvents() {
                this.addUserFormEl.addEventListener('submit', this.handleAddUser.bind(this))
                this.userListEl.addEventListener('click', this.handleListClick.bind(this));
                },
                render() {
                // gets the HTML from ALL users of our user list and replaces the current user list
                /*const userHtml = this.users.map( user => Template.get('user-template', user) ).join('')
                this.userListEl.innerHTML = userHtml*/
                if (this.recentlyAdded) {
                this.userListEl.innerHTML += Template.get('user-template', this.recentlyAdded);
                this.recentlyAdded = null;
                }
                if (this.recentlyRemovedKey) {
                const element = document.getElementById(this.recentlyRemovedKey);
                if (element) {
                element.remove();
                }
                this.recentlyRemoved = null;
                }
                console.log('userlist after render: ',this.users);
                },
                handleAddUser(e) {
                e.preventDefault()

                const username = e.currentTarget.elements['username'].value
                if (!username) return
                e.currentTarget.elements['username'].value = ''

                const key = this.nextKey++;

                this.users[key] = {
                key: key,
                name: username
                };
                this.recentlyAdded = this.users[key];

                this.render();
                },
                handleListClick(e) {
                const target = e.target;
                if (target.classList.contains('remove')) {
                this.handleRemoveUser(target.parentNode.id);
                }
                },
                handleRemoveUser(key) {
                delete this.users[key];
                this.recentlyRemovedKey = key;
                this.render();
                }
                }

                Users.init()

                .remove:before {
                content: 'X';
                }

                <form id="add-user-form">
                <input type="text" placeholder="username" name="username">
                <button>add User</button>
                </form>
                <hr>
                <ul id="user-list"></ul>


                <script id="user-template" type="text/html">
                <li data-key="${key}" id="${key}">${name} <button class="remove"></button></li>
                </script>








                // lets you get templates from HTML ( as you can see in the HTML above ). 
                // This is to avoid writing out HTML inside JavaScript
                const Template = {
                cached: {},
                get(templateId, placeholders) {
                let templateHtml = ''
                if (!this.cached[templateId]) {
                templateHtml = document.getElementById(templateId).innerHTML.trim()
                this.cached[templateId] = templateHtml
                }
                else {
                templateHtml = this.cached[templateId]
                }

                for (let key in placeholders) {
                templateHtml = templateHtml.replace(new RegExp("\${\s*" + key + "\s*}", "g"), placeholders[key]);
                }
                return templateHtml
                }
                }

                const Users = {
                users: {},
                recentlyAdded: null,
                recentlyRemovedKey: null,
                nextKey: 1,
                init() {
                this.cacheDom()
                this.bindEvents()
                },
                cacheDom() {
                this.addUserFormEl = document.getElementById('add-user-form')
                this.userListEl = document.getElementById('user-list')
                },
                bindEvents() {
                this.addUserFormEl.addEventListener('submit', this.handleAddUser.bind(this))
                this.userListEl.addEventListener('click', this.handleListClick.bind(this));
                },
                render() {
                // gets the HTML from ALL users of our user list and replaces the current user list
                /*const userHtml = this.users.map( user => Template.get('user-template', user) ).join('')
                this.userListEl.innerHTML = userHtml*/
                if (this.recentlyAdded) {
                this.userListEl.innerHTML += Template.get('user-template', this.recentlyAdded);
                this.recentlyAdded = null;
                }
                if (this.recentlyRemovedKey) {
                const element = document.getElementById(this.recentlyRemovedKey);
                if (element) {
                element.remove();
                }
                this.recentlyRemoved = null;
                }
                console.log('userlist after render: ',this.users);
                },
                handleAddUser(e) {
                e.preventDefault()

                const username = e.currentTarget.elements['username'].value
                if (!username) return
                e.currentTarget.elements['username'].value = ''

                const key = this.nextKey++;

                this.users[key] = {
                key: key,
                name: username
                };
                this.recentlyAdded = this.users[key];

                this.render();
                },
                handleListClick(e) {
                const target = e.target;
                if (target.classList.contains('remove')) {
                this.handleRemoveUser(target.parentNode.id);
                }
                },
                handleRemoveUser(key) {
                delete this.users[key];
                this.recentlyRemovedKey = key;
                this.render();
                }
                }

                Users.init()

                .remove:before {
                content: 'X';
                }

                <form id="add-user-form">
                <input type="text" placeholder="username" name="username">
                <button>add User</button>
                </form>
                <hr>
                <ul id="user-list"></ul>


                <script id="user-template" type="text/html">
                <li data-key="${key}" id="${key}">${name} <button class="remove"></button></li>
                </script>





                // lets you get templates from HTML ( as you can see in the HTML above ). 
                // This is to avoid writing out HTML inside JavaScript
                const Template = {
                cached: {},
                get(templateId, placeholders) {
                let templateHtml = ''
                if (!this.cached[templateId]) {
                templateHtml = document.getElementById(templateId).innerHTML.trim()
                this.cached[templateId] = templateHtml
                }
                else {
                templateHtml = this.cached[templateId]
                }

                for (let key in placeholders) {
                templateHtml = templateHtml.replace(new RegExp("\${\s*" + key + "\s*}", "g"), placeholders[key]);
                }
                return templateHtml
                }
                }

                const Users = {
                users: {},
                recentlyAdded: null,
                recentlyRemovedKey: null,
                nextKey: 1,
                init() {
                this.cacheDom()
                this.bindEvents()
                },
                cacheDom() {
                this.addUserFormEl = document.getElementById('add-user-form')
                this.userListEl = document.getElementById('user-list')
                },
                bindEvents() {
                this.addUserFormEl.addEventListener('submit', this.handleAddUser.bind(this))
                this.userListEl.addEventListener('click', this.handleListClick.bind(this));
                },
                render() {
                // gets the HTML from ALL users of our user list and replaces the current user list
                /*const userHtml = this.users.map( user => Template.get('user-template', user) ).join('')
                this.userListEl.innerHTML = userHtml*/
                if (this.recentlyAdded) {
                this.userListEl.innerHTML += Template.get('user-template', this.recentlyAdded);
                this.recentlyAdded = null;
                }
                if (this.recentlyRemovedKey) {
                const element = document.getElementById(this.recentlyRemovedKey);
                if (element) {
                element.remove();
                }
                this.recentlyRemoved = null;
                }
                console.log('userlist after render: ',this.users);
                },
                handleAddUser(e) {
                e.preventDefault()

                const username = e.currentTarget.elements['username'].value
                if (!username) return
                e.currentTarget.elements['username'].value = ''

                const key = this.nextKey++;

                this.users[key] = {
                key: key,
                name: username
                };
                this.recentlyAdded = this.users[key];

                this.render();
                },
                handleListClick(e) {
                const target = e.target;
                if (target.classList.contains('remove')) {
                this.handleRemoveUser(target.parentNode.id);
                }
                },
                handleRemoveUser(key) {
                delete this.users[key];
                this.recentlyRemovedKey = key;
                this.render();
                }
                }

                Users.init()

                .remove:before {
                content: 'X';
                }

                <form id="add-user-form">
                <input type="text" placeholder="username" name="username">
                <button>add User</button>
                </form>
                <hr>
                <ul id="user-list"></ul>


                <script id="user-template" type="text/html">
                <li data-key="${key}" id="${key}">${name} <button class="remove"></button></li>
                </script>






                share|improve this answer














                share|improve this answer



                share|improve this answer








                edited 9 mins ago

























                answered Apr 9 '18 at 18:36









                Sᴀᴍ OnᴇᴌᴀSᴀᴍ Onᴇᴌᴀ

                10.6k62168




                10.6k62168






























                    draft saved

                    draft discarded




















































                    Thanks for contributing an answer to Code Review Stack Exchange!


                    • Please be sure to answer the question. Provide details and share your research!

                    But avoid



                    • Asking for help, clarification, or responding to other answers.

                    • Making statements based on opinion; back them up with references or personal experience.


                    Use MathJax to format equations. MathJax reference.


                    To learn more, see our tips on writing great answers.




                    draft saved


                    draft discarded














                    StackExchange.ready(
                    function () {
                    StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f190088%2fjavascript-module-to-render-and-handle-a-form-to-add-users%23new-answer', 'question_page');
                    }
                    );

                    Post as a guest















                    Required, but never shown





















































                    Required, but never shown














                    Required, but never shown












                    Required, but never shown







                    Required, but never shown

































                    Required, but never shown














                    Required, but never shown












                    Required, but never shown







                    Required, but never shown







                    Popular posts from this blog

                    Webac Holding Inhaltsverzeichnis Geschichte | Organisationsstruktur | Tochterfirmen |...

                    What's the meaning of a knight fighting a snail in medieval book illustrations?What is the meaning of a glove...

                    Salamanca Inhaltsverzeichnis Lage und Klima | Bevölkerungsentwicklung | Geschichte | Kultur und...