0
\$\begingroup\$

I am building a small tool to generate a copy and paste HTML block, based on the user inputs.

In the HTML file, I have all my inputs (select, checkbox, text inputs etc).

I based myself on the JS of an existing tool on the web, which has the following form. Everything works, but I wonder if there is an opportunity to improve or simplify the code.

(function( $ ) { $(function() { var a; var b; var c; function updateHtml() { a=$('#inputA').val(); b=$('#inputB').val(); c=$('#inputC').val(); html = '<div><p>My name : '+a+'</p><p>My phone : '+b+'</p><p>My mail : '+c+'</p></div>'; $('#code').html(html); } $( document ).ready(function() { updateHtml(); }); $( "input" ).on('change',function() { updateHtml(); }); }); })(jQuery); 

The code seems unstructured to me. It works without prior declaration of variables

var a; var b; var c; 

Inside the main function

(function( $ ) {})(jQuery); 

There is a first function that allows me to retrieve the value of inputs and display the HTML bloc
Then a function to execute the update function when document is ready and finally a function to execute the update function when the inputs are changed..

The code works, but isn't there a more academic or logical way to build it?

Any help is appreciated.

\$\endgroup\$
2
  • \$\begingroup\$Welcome to Code Review! While there are declarations and assignments for a, b and c there do not appear to be assignments for $inputA, $inputB and $inputC\$\endgroup\$CommentedApr 1, 2023 at 23:22
  • \$\begingroup\$HI, Thanks for your reply, sorry I didn't understand...\$\endgroup\$
    – blogob
    CommentedApr 2, 2023 at 7:56

1 Answer 1

1
\$\begingroup\$

There are a few things that jump out at me.

  1. Variables a, b and c are only used in the updateHtml function, so these don't seem to have a reason to be declared outside of this function.
  2. It's probably better to give more meaningful names to a, b and c, in this case maybe name, phone and mail would be better.
  3. By using the text function (or textContent in vanilla JS) you can have the conversion to encoded tags (e.g. <div>) done for you, making the HTML string much more readable.

In the example below I've also put a var keyword infront of the html declaration, removed the inner wrapper function that starts with $(function() { as I'm not sure that's necessary (but my jQuery is a bit rusty, so maybe it's there for a good reason), renamed the input ids and fixed the indentation.

(function( $ ) { function updateHtml() { var name = $('#input_name').val(); var phone = $('#input_phone').val(); var mail = $('#input_mail').val(); var html = '<div>' + '<p>My name : ' + name + '</p>' + '<p>My phone : ' + phone + '</p>' + '<p>My mail : ' + mail + '</p>' + '</div>'; $('#code').text(html); } $( document ).ready(function() { updateHtml(); }); $( "input" ).on('change', function() { updateHtml(); }); })(jQuery);
label { display: block; margin-bottom: 8px; } label span { display: inline-block; width: 100px; text-align: right; font-weight: bold; }
<script src="https://cdnjs.cloudflare.com/ajax/libs/jquery/3.3.1/jquery.min.js"></script> <label><span>Name</span> <input type="text" id="input_name" /></label> <label><span>Phone</span> <input type="text" id="input_phone" /></label> <label><span>Mail</span> <input type="text" id="input_mail" /></label> <h4>HTML Output</h4> <pre id="code"></pre>

You might be able to save a bit of time in the future (in case you're adding more fields) by using something like serializeArray to grab all the input values in one go and use the input name attributes to as the object keys when creating the HTML string. e.g.:

(function( $ ) { const get_input_values = () => Object.fromEntries( $('#input_fields') .serializeArray() .map( (field) => [field.name, field.value] ) ); const get_html = ({ name, phone, mail }) => '<div>' + '<p>My name : ' + name + '</p>' + '<p>My phone : ' + phone + '</p>' + '<p>My mail : ' + mail + '</p>' + '</div>'; function updateHtml() { $('#code').text( get_html( get_input_values() ) ); } $( document ).ready(updateHtml); $( "input" ).on('change', updateHtml); })(jQuery);
label { display: block; margin-bottom: 8px; } label span { display: inline-block; width: 100px; text-align: right; font-weight: bold; }
<script src="https://cdnjs.cloudflare.com/ajax/libs/jquery/3.3.1/jquery.min.js"></script> <form id="input_fields"> <label><span>Name</span> <input type="text" id="input_name" name="name" /></label> <label><span>Phone</span> <input type="text" id="input_phone" name="phone" /></label> <label><span>Mail</span> <input type="text" id="input_mail" name="mail" /></label> </form> <h4>HTML Output</h4> <pre id="code"></pre>

\$\endgroup\$
1
  • \$\begingroup\$Thank you very much Ben, I'll test it following your example..\$\endgroup\$
    – blogob
    CommentedApr 2, 2023 at 22:47

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.