This is my first time using Javascript, so please forgive me. I'm still not solid on the terminology or best practices, but I'm losing my mind with how complex this script is getting for such a simple thing.
Also if there's a better way, in general, to do what I'm trying to do, let me know because boy I have lost sleep on this one.
Context:
I have a form to build standardized email signatures. The user puts their info into the inputs and then copies the output from a copyable area with standardized styling specific for email markup. In each signature, someone may include the information for another person in their company.
I have a set of radio buttons that enable 0, 1, 2, or 3 additional fieldsets of input for these team members. In addition to adding fieldsets of inputs, they also enable outputs in the copyable area. The outputs have to be "display: none" so that someone who does not include this information in their signature doesn't end up with blank table cells in their copied signature.
https://jsfiddle.net/slingtruchoice/jrem21yb/
Here's the whole, ugly thing. I'm proud but also so not proud of it. Like I said, literally the first time I've ever used javascript. Specifically, I'm looking at this:
var radios = document.getElementsByName('addTeam');
for (var i = 0; i < radios.length; i ) {
radios[i].addEventListener('change', function() {
//--------------------------------- || RADIO BUTTONS || -------------------------------------------
let fieldset = document.getElementsByClassName('sigform-fieldset'), //CLASS fieldset for all radio buttons
inputs = document.getElementsByClassName('sigform-team'), //CLASS all radio buttons
izero = document.getElementById('add-team__0'), //ID radio button, input 0
ione = document.getElementById('add-team__1'), //ID radio button, input 1
itwo = document.getElementById('add-team__2'), //ID radio button, input 2
ithree = document.getElementById('add-team__3'); //ID radio button, input 3
//--------------------------------- || INPUT SECTIONS || -------------------------------------------
let divs = document.getElementsByClassName('sigform__team-inputs'), //CLASS all input wrapper divs
done = document.getElementById('team-inputs__1'), //ID div of input section, team 1
dtwo = document.getElementById('team-inputs__2'), //ID div of input section, team 2
dthree = document.getElementById('team-inputs__3'); //ID div of input section, team 3
//--------------------------------- || SIGNATURE OUTPUT || -------------------------------------------
let // ------------------------ Table Rows -------------------------------------------
teamsrows = document.getElementsByClassName('extraTeamWrap'), //CLASS of tr wrap each output table
teamwrap1 = document.getElementById('extraTeamWrap1'), //ID tr wrap of output table team 1
teamwrap2 = document.getElementById('extraTeamWrap2'), //ID tr wrap of output table team 2
teamwrap3 = document.getElementById('extraTeamWrap3'), //ID tr wrap of output table team 3
// ------------------------ Tables -------------------------------------------
teamtables = document.getElementsByClassName('extraTeamTable'), //CLASS of table for each output
teamtable1 = document.getElementById('extraTeamTable-one'), // ID table wrap of output table team 1
teamtable2 = document.getElementById('extraTeamTable-two'), // ID table wrap of output table team 2
teamtable3 = document.getElementById('extraTeamTable-three'); // ID table wrap of output table team 3
if (ione.checked == false && itwo.checked == false && ithree.checked == false || izero.checked == true){
done.style.display = 'none';
dtwo.style.display = 'none';
dthree.style.display = 'none';
teamsrows.style.display = 'none';
} else if (ione.checked == true && itwo.checked == false && ithree.checked == false) {
done.style.display = 'block';
teamsrows.style.display = "block";
dtwo.style.display = 'none';
dthree.style.display = 'none';
} else if (ione.checked == false && itwo.checked == true && ithree.checked == false) {
done.style.display = 'block';
dtwo.style.display = 'block';
dthree.style.display = 'none';
} else if (ione.checked == false && itwo.checked == false && ithree.checked == true) {
done.style.display = 'block';
dtwo.style.display = 'block';
dthree.style.display = 'block';
} else {
return false;
}
});
}
And it's not even done. (Oh yeah, by the way, please don't expect this fiddle to work. It's far from there.)
How do I go about this better? I'm having difficulty googling answers for my questions since I don't really know how to say "how to make an argument equal to multiple IDs that are paired specifically with other IDs to do something to that ID when the other ID is activated... javascript" in a way that yields useful results.
My only request is that any explanation come in very simple language. I really can't iterate enough how much of an absolute beginner I am. Most responses on StackExchange I've found for other questions have just blown right over my head.
And really, thank you for any help you're able to give!
CodePudding user response：
Well, that took more than a minute. Here is the updated code. Are there more efficient ways of doing this? Probably. Here, I want you take away how data structures can simplify your code.
let radios = document.getElementsByName('addTeam');
for (let i = 0; i < radios.length; i ) {
radios[i].addEventListener('change', function() {
//Based on whether it is a class or an id
const elementClasses = [
'sigform-fieldset',
'sigform-team',
'sigform__team-inputs',
'extraTeamWrap',
'extraTeamTable',
];
const elementIds = [
'add-team__0',
'add-team__1',
'add-team__2',
'add-team__3',
'team-inputs__1',
'team-inputs__2',
'team-inputs__3',
'extraTeamWrap1',
'extraTeamWrap2',
'extraTeamWrap3',
'extraTeamTable-one',
'extraTeamTable-two',
'extraTeamTable-three'
];
//Adding elements to the loop
elementsTotal = [];
//For each loop to iterate the array
elementClasses.forEach(element => {
push(document.getElementsByClassName(element));
});
elementIds.forEach(element => {
push(document.getElementsByClassName(element));
});
//The commented code is for classes.
//document.getElementsByClassName returns a collection, so you must iterate that collection to style.
if (elementsTotal[6].checked == false && elementsTotal[7].checked == false && elementsTotal[8].checked == false || elementsTotal[5].checked == true){
elementsTotal[9].style.display = 'none';
elementsTotal[10].style.display = 'none';
elementsTotal[11].style.display = 'none';
//teamsrows.style.display = 'none';
} else if (elementsTotal[6].checked == true) {
elementsTotal[9].style.display = 'block';
//teamsrows.style.display = "block";
elementsTotal[10].style.display = 'none';
elementsTotal[11].style.display = 'none';
} else if (elementsTotal[7].checked == true) {
elementsTotal[9].style.display = 'block';
elementsTotal[10].style.display = 'block';
elementsTotal[11].style.display = 'none';
} else if (elementsTotal[8].checked == true) {
elementsTotal[9].style.display = 'block';
elementsTotal[10].style.display = 'block';
elementsTotal[11].style.display = 'block';
} else {
return false;
}
});
}
So I talked about objects, but I can't really see how it might be implemented here. Thus, I would like to show an example of how it might be used.
Example from: https://betterprogramming.pub/stop-putting-so-many-if-statements-in-your-javascript-3b65aaa4b86b
Here, is a pretty simple if statement approach, not the most clean code.
function getStatusColor (status) {
if (status === 'success') {
return 'green'
}
if (status === 'warning') {
return 'yellow'
}
if (status === 'info') {
return 'blue'
}
if (status === 'error') {
return 'red'
}
}
Better code:
function getStatusColor (status) {
return {
success: 'green',
warning: 'yellow',
info: 'blue',
error: 'red'
}[status]
}
With an object, it is easier to tell what is happening. I couldn't fit an object anywhere in your code as the if statements had booleans instead of strings. (I merely thought about strings, I wish I knew how to do it with booleans, though!)
Again, use data structures! I hope you learned some things, I did too.
CodePudding user response：
Pretty impressive starter project and results!!
This might be an unexpected answer. I am not sure whether some repetition is really harmful. Especially, as you just started coding in Javascript. I am also learning Javascript and the most important thing for me right now are speaking and short variable names and methods.
I took the chance to look into your code and refactored it a bit. The code still cannot write the extra fields to the right. I also changed some unrelated code and created a function. I am sure that some more optimizations are possible to remove repetition, maybe I come back to this code. If you can still understand it after 3 or 6 months, it's good code :)
The new code:
- adds the extra fields at the bottom and on the right depending on your team checkboxes.
- uses shorter and more speaking variable names. I have also shortened the names of all elements in your HTML, repeating the word "sigform" all the time is not necessary.
- contains a cleaner way to access/add/remove all the fields (at least for me).
I was unsure how to add the click/change eventlistener to the checkboxes and ran out of time. You can check out this SO post: https://stackoverflow.com/a/27622415/3842598
See https://jsfiddle.net/kL3qogv7/1/ (or better below).
Let me know if you have any questions or issues to understand my changes. I hope this helps.
const phonePrinter = function() {
// using more speaking names here
let phoneNumberDir = document.querySelector('#input__phone__dir').value.replace(/\D/g, "");
let directPhone = document.querySelector('#phone-output__dir');
let officePhone = document.querySelector('#input__phone__off').value.replace(/\D/g, "");
let extension = document.querySelector('#input__ext__off').value;
let officePhoneWithExtension = officePhone ',' extension;
let phoneLinkOff = document.querySelector('#phone-output__off');
// phoneNumber is the input
function validatePhoneNumber(phoneNumber) {
// removed the two lines to be able to test with random numbers
// var re = /^\(?(\d{3})\)?[- ]?[. ]?(\d{3})[- ]?[. ]?(\d{4})$/;
// return re.test(phoneNumber);
return phoneNumber
}
if (officePhone == '' && phoneNumberDir == '') {
alert("Please enter at least one phone number.")
} else if (officePhone !== '' && !validatePhoneNumber(officePhone)) {
alert("Please enter a valid office phone number.");
document.getElementById('input__phone__off').value = '';
return;
} else if (phoneNumberDir !== '' && !validatePhoneNumber(phoneNumberDir)) {
alert("Please enter a valid direct phone number.");
document.getElementById('input__phone__dir').value = '';
return;
}
//populating the phone in the href
directPhone.setAttribute("href", "tel:" phoneNumberDir);
directPhone.innerHTML = addDots(phoneNumberDir);
//populating the phone ext in the href
phoneLinkOff.setAttribute("href", "tel:" officePhoneWithExtension);
phoneLinkOff.innerHTML = addDots(phoneNumberDir, extension);
event.preventDefault();
};
// function to handle the dots in the phone number, externalNumber is a default parameter, not set by default
function addDots(phoneNumber, externalNumber='') {
var number = phoneNumber.slice(0, 3) "." phoneNumber.slice(3, 6) "." phoneNumber.slice(6, 10);
// if external number is set, add it to the number output
if (externalNumber != '') { return number " ext " externalNumber }
return number
}
// this should be much clearer now. New order and shorter names
function printName() {
var name = document.querySelector('#input__name').value;
var title = document.querySelector('#input__title').value;
if (name == '') alert("Please enter your name.");
if (title == '') alert('Please enter your title.');
if (name != '' && title != '') {
const nameOutput = document.querySelector('#nameOutput');
const titleOutput = document.querySelector('#titleOutput');
nameOutput.innerHTML = name;
titleOutput.innerHTML = title;
}
}
// removed the uncommented code here, just for testing
// checkLimiter = document.getElementsByTagName('banner-checks'); ...
var checkbox = document.querySelectorAll(".check");
for (var i = 0; i < checkbox.length; i )
checkbox[i].onclick = selectiveCheck;
function selectiveCheck(event) {
var checked = document.querySelectorAll(".check:checked");
// only used here, updated the name and use > maxBanners below instead of = and 1
const maxBanners = 2
if (checked.length > maxBanners) {
alert("You may only have up to two additional banners in your signature.");
return false;
}
}
// default hidden, not required to set this within your html
var teamFields1 = document.getElementById('team-inputs__1');
var teamFields2 = document.getElementById('team-inputs__2');
var teamFields3 = document.getElementById('team-inputs__3');
// I used classList.add("hidden") to hide the elements
// Later, you will see classList.remove("hidden") to make it visible again
teamFields1.classList.add("hidden");
teamFields2.classList.add("hidden");
teamFields3.classList.add("hidden");
// this name is maybe still not optimal, maybe better additionalTeam1..3 or so
var table1 = document.getElementById('teamTable1');
var table2 = document.getElementById('teamTable2');
var table3 = document.getElementById('teamTable3');
table1.classList.add("hidden");
table2.classList.add("hidden");
table3.classList.add("hidden");
// Not sure how to handle/improve this right now
var radios = document.getElementsByName('addTeam');
for (let i = 0; i < radios.length; i ) {
radios[i].addEventListener('click', function() {
// I could remove some of your variables but of course not all
// We have of course still some repetition.
const input0 = document.getElementById('add-team__0');
const input1 = document.getElementById('add-team__1');
const input2 = document.getElementById('add-team__2');
const input3 = document.getElementById('add-team__3');
if (input0.checked == true){
table1.classList.add('hidden');
table2.classList.add('hidden');
table3.classList.add('hidden');
teamFields1.classList.add("hidden");
teamFields2.classList.add("hidden");
teamFields3.classList.add("hidden");
} else if (input1.checked == true) {
table1.classList.remove('hidden');
table2.classList.add('hidden');
table3.classList.add('hidden');
teamFields1.classList.remove("hidden");
teamFields2.classList.add("hidden");
teamFields3.classList.add("hidden");
} else if (input2.checked == true) {
table1.classList.remove('hidden');
table2.classList.remove('hidden');
table3.classList.add('hidden');
teamFields1.classList.remove("hidden");
teamFields2.classList.remove("hidden");
teamFields3.classList.add("hidden");
} else if (input3.checked == true) {
table1.classList.remove('hidden');
table2.classList.remove('hidden');
table3.classList.remove('hidden');
teamFields1.classList.remove("hidden");
teamFields2.classList.remove("hidden");
teamFields3.classList.remove("hidden");
}
});
}
function populate() {
phonePrinter();
printName();
}@import url('https://fonts.googleapis.com/css2?family=Source Sans Pro:wght@400;600&display=swap');
body {
font-size: 16;
font-family: "Source Sans Pro", Arial, sans-serif;
margin: 0;
padding: 0;
box-sizing: border-box;
background-color: #095285;
}
.columns {
display: flex;
flex-direction: row;
background: linear-gradient(135deg, rgba(255, 255, 255, 0.7) 0%, rgba(255, 255, 255, 0.4) 100%);
}
form {
padding: 5vw;
width: 40vw;
margin: 30px auto;
display: flex;
flex-direction: column;
}
.input {
display: block;
padding: 10px 10px 5px 0;
width: 100%;
box-sizing: border-box;
margin: 10px;
background-color: transparent;
border-top: none;
border-left: none;
border-right: none;
border-bottom: 2px solid #095285;
font-size: 24px;
font-weight: 600;
font-family: "Source Sans Pro", Arial, sans-serif;
color: #095285;
}
.input::placeholder {
font-weight: 400;
font-size: 24px;
color: #095285;
font-family: "Source Sans Pro", Arial, sans-serif;
opacity: 1;
text-transform: uppercase;
}
.output {
width: 40vw;
padding: 5vw;
margin: 30px auto;
display: flex;
flex-direction: column;
justify-content: center;
}
.output > .table-wrapper {
font-family: "Source Sans Pro", sans-serif;
color: #095285;
font-size: 16px;
background-color: white;
border: 2px solid #095285;
padding: 2vw;
min-width: 375px;
}
.instructions {
font-size: 24px;
font-weight: bolder;
text-align: center;
color: #095285;
}
.no-select::selection {
-webkit-touch-callout: none;
-webkit-user-select: none;
-khtml-user-select: none;
-moz-user-select: none;
-ms-user-select: none;
user-select: none;
cursor: no-drop;
}
.selectable::selection,
table.selectable>*::selection {
-webkit-touch-callout: auto;
-webkit-user-select: auto;
-khtml-user-select: auto;
-moz-user-select: auto;
-ms-user-select: auto;
user-select: auto;
cursor: copy !important;
}
/* banners, checkboxes */
/* checkboxes */
.dropdown-hidden {
display: none;
}
.dropdown-visible {
display: block;
}
/* banners */
.banner-hidden {
display: none;
}
.banner-visible {
display: block;
}
.visible {
display: block;
}
.hidden {
display: none;
}<div >
<!-- the only place where I kept sigform in the name. Other refs have been removed for shorter names -->
<form id="sigform" name="sigform">
<input id="input__name" type="text" placeholder="Name:">
<input id="input__title" type="text" placeholder="Title:">
<input id="input__phone__dir" type="tel" placeholder="Direct phone number:">
<input id="input__phone__off" type="tel" placeholder="Office phone number:">
<input id="input__ext__off" type="number" placeholder="Office Extension:">
<input id="input__website" type="url" placeholder="Team Website:">
<label id="label__banner-inst" for="field__banner-fs">Add banners to your signature. You may choose up to 2.
<fieldset id="field__banner-fs" labeledby="label__banner-inst">
<label id="label__banner-one">
<input type="checkbox" name="banner-checks">
</label>
<label id="label__banner-two">
<input type="checkbox" name="banner-checks">
</label>
<label id="label__banner-three">
<input type="checkbox" name="banner-checks">
</label>
<label id="label__banner-four">
<input type="checkbox" name="banner-checks">
</label>
<label id="label__banner-five">
<input type="checkbox" name="banner-checks">
</label>
<label id="label__banner-six">
<input type="checkbox" name="banner-checks">
</label>
</fieldset>
</label>
<label id="label__add-team">How many additional team members would you like represented in your signature?</label>
<fieldset id="field__add-team" labeledby="label__add-team">
<label id="label__add-team__0" for="add-team__0">0
<input id="add-team__0" type="radio" name="addTeam" value="0" labeledby="label__add-team__0" checked>
</label>
<label id="label__add-team__1" for="add-team__1">1
<input id="add-team__1" type="radio" name="addTeam" value="1" labeledby="label__add-team__1">
</label>
<label id="label__add-team__2" for="add-team__2">2
<input id="add-team__2" type="radio" name="addTeam" value="2" labeledby="label__add-team__2">
</label>
<label id="label__add-team__3" for="add-team__3">3
<input id="add-team__3" type="radio" name="addTeam" value="3" labeledby="label__add-team__3">
</label>
</fieldset>
<fieldset id="field__team-sigs">
<div id="team-inputs__1">
<input type="text" id="team-1__name" placeholder="Team Member Name:">
<input type="text" id="team-1__title" placeholder="Title:">
<input type="text" id="team-1__email" placeholder="Email:">
<input type="text" id="team-1__phone" placeholder="Direct Phone Number:">
</div>
<div id="team-inputs__2">
<input type="text" id="team-2__name" placeholder="Team Member Name:">
<input type="text" id="team-2__title" placeholder="Title:">
<input type="text" id="team-2__email" placeholder="Email:">
<input type="text" id="team-2__phone" placeholder="Direct Phone Number:">
</div>
<div id="team-inputs__3">
<input type="text" id="team-3__name" placeholder="Team Member Name:">
<input type="text" id="team-3__title" placeholder="Title:">
<input type="text" id="team-3__email" placeholder="Email:">
<input type="text" id="team-3__phone" placeholder="Direct Phone Number:">
</div>
</fieldset>
<button id="submit" type="button" onClick="populate()">Submit</button>
<img src="">
</form>
<div >
<p >
HIGHLIGHT EVERYTHING BELOW TO COPY AND PASTE IN YOUR SIGNATURE
</p>
<div >
<table >
<tr>
<td colspan="1" id="nameOutput">Bobby Joe</td>
</tr>
<tr>
<td colspan="1" id="titleOutput">Web Developer | Graphic Designer</td>
</tr>
<tr>
<td colspan="1"><span id="phoneWrapper">Direct: <a href="tel:1112223333,8888" id="phone-output__dir">111.222.3333</a> | Office: <a href="#" id="phone-output__off">111.222.3333 ext. 123</a></span></td>
</tr>
<tr>
<td colspan="1"><img src="https://via.placeholder.com/350x65">
</td>
</tr>
<tr id="extraTeamWrap1">
<table id="teamTable1">
<tr>
<td id="team-name__one">Extra Team member</td>
</tr>
<tr>
<td id="team-title__one">Extra Team title</td>
</tr>
<tr>
<td id="team-email__one"><a href="mailto:[email protected]">Extra Team email</a></td>
</tr>
<tr>
<td id="team-phone__one"><a href="tel:1112223333">111.222.6666</a></td>
</tr>
</table>
</tr>
<tr id="extraTeamWrap2">
<table id="teamTable2">
<tr>
<td id="team-name__two">Extra Team member</td>
</tr>
<tr>
<td id="team-title__two">Extra Team title</td>
</tr>
<tr>
<td id="team-email__two"><a href="mailto:[email protected]">Extra Team email</a></td>
</tr>
<tr>
<td id="team-phone__two"><a href="tel:1112223333">111.222.6666</a></td>
</tr>
</table>
</tr>
<tr id="extraTeamWrap3">
<table id="teamTable3">
<tr>
<td id="team-name__three">Extra Team member</td>
</tr>
<tr>
<td id="team-title__three">Extra Team title</td>
</tr>
<tr>
<td id="team-email__three"><a href="mailto:[email protected]">Extra Team email</a></td>
</tr>
<tr>
<td id="team-phone__three"><a href="tel:1112223333">111.222.6666</a></td>
</tr>
</table>
</tr>
</table>
<table id="banner-table">
<tr id="tr__banner-opt__one">
<td id="td__banner-opt__one">Banner One</td>
</tr>
<tr id="tr__banner-opt__two">
<td id="td__banner-opt__two">Banner Two</td>
</tr>
<tr id="tr__banner-opt__three">
<td id="td__banner-opt__three">Banner Three</td>
</tr>
<tr id="tr__banner-opt__four">
<td id="td__banner-opt__four">Banner Four</td>
</tr>
<tr id="tr__banner-opt__five">
<td id="td__banner-opt__five">Banner Five</td>
</tr>
<tr id="tr__banner-opt__six">
<td id="td__banner-opt__six">Banner Six</td>
</tr>
</table>
</div>
</div>
</div>