Created a validation form in vanilla JS. Due to my little experience, I have a bunch of "if" here, I can not transfer the coordinates of Yandex.Maps to another function for validation, and it is still unknown what. With the coordinates, I "crutch" by saving them to an invisible div :-) I deleted my Api key, so I won't be able to get the coordinates in the example, I don't know if it's worth giving the key to public access :-) The code is working.
Tell me how I can reduce the code or how to perform form validation without my crutch using a div :-) (pass the variable to another function so that when the button is clicked, it checks that the coordinates are entered)
let errorName = document.querySelector(".name-error");
let errorNumber = document.querySelector(".number-error");
let errorEmail = document.querySelector(".email-error");
let errorMap = document.querySelector(".map-error");
let completed = document.querySelector(".mailForm__completed");
function viodChecker() {
let name = document.querySelector(".mailForm__name").value;
let number = document.querySelector(".mailForm__number").value;
let Email = document.querySelector(".mailForm__email").value;
let Coords = document.querySelector(".coords");
if (name === "") {
errorName.style.display = "block";
}
if (name !== "") {
errorName.style.display = "none";
}
if (number === "") {
errorNumber.style.display = "block";
}
if (number !== "") {
errorNumber.style.display = "none";
}
if (Email.indexOf("@") === -1) {
errorEmail.style.display = "block";
}
if (Email.indexOf("@") !== -1) {
errorEmail.style.display = "none";
}
if (Email.indexOf("@") !== -1) {
errorEmail.style.display = "none";
}
if (Coords.textContent === "") {
errorMap.style.display = "block";
}
if (Coords.textContent !== "") {
errorMap.style.display = "none";
}
if (name !== "" && number !== "" && Email.indexOf("@") !== -1 && Coords.textContent !== "") {
completed.style.display = "block";
}
}
ymaps.ready(init);
function init() {
var myPlacemark,
myMap = new ymaps.Map('map', {
center: [40.391917, -73.524590],
zoom: 17,
controls: ['geolocationControl', 'searchControl']
});
// We listen to the click on the map.
myMap.events.add('click', function (e) {
var coords = e.get('coords');
// If the label has already been created, just move it.
if (myPlacemark) {
myPlacemark.geometry.setCoordinates(coords);
}
// If not, we create.
else {
myPlacemark = createPlacemark(coords);
myMap.geoObjects.add(myPlacemark);
// Listen to the drag-and-drop event on the label.
myPlacemark.events.add('dragend', function () {
getAddress(myPlacemark.geometry.getCoordinates());
});
}
getAddress(coords);
});
// Create a label.
function createPlacemark(coords) {
return new ymaps.Placemark(coords, {
iconCaption: 'search...'
}, {
preset: 'islands#violetDotIconWithCaption',
draggable: true
});
}
// Coords
function getAddress(coords) {
myPlacemark.properties.set('iconCaption', 'поиск...');
ymaps.geocode(coords).then(function (res) {
var firstGeoObject = res.geoObjects.get(0);
myPlacemark.properties
.set({
// We form a line with data about the object.
iconCaption: [
firstGeoObject.geometry.getCoordinates(),
].filter(Boolean).join(', '),
// We set the line with the object address as the balloon content.
balloonContent: firstGeoObject
});
document.querySelector(".coords").textContent = firstGeoObject.geometry.getCoordinates();
});
}
}body, .mailForm {
display: flex;
align-items: center;
justify-content: center;
flex-direction: column;
}
input, textarea {
font-size: 14px;
margin: 10px 0 0 0;
width: 350px;
resize: none;
}
.mailForm__map {
margin-top: 10px;
}
button {
margin: 10px 0 0 0;
font-size: 14px;
cursor: pointer;
}
.mailForm__comment {
display: inline-block;
height: 200px;
}
.mailForm__error, .mailForm__completed, .coords {
display: none;
color: red;
font-size: 16px;
font-weight: bold;
margin-top: 10px;
}
.mailForm__completed {
color: green;
font-size: 20px;
}<!doctype html>
<html lang="en">
<head>
<meta charset="UTF-8">
<meta name="viewport"
content="width=device-width, user-scalable=no, initial-scale=1.0, maximum-scale=1.0, minimum-scale=1.0">
<meta http-equiv="X-UA-Compatible" content="ie=edge">
<link href="style.css" rel="stylesheet">
<title>Mail Form</title>
<script src="https://api-maps.yandex.ru/2.1/?apikey=5200498b-0123-4796-1-bee4ea5473212&lang=ru_RU" type="text/javascript">
</script>
</head>
<body>
<div class="mailForm">
<input class="mailForm__name" placeholder="Name" type="text" value="">
<input class="mailForm__number" placeholder="Tel" type="tel" value="">
<input class="mailForm__email" placeholder="Email" type="email" value="">
<div id="map" class="mailForm__map" style="width: 100%; height: 200px">
</div>
<textarea class="mailForm__comment" maxlength="500" placeholder="Comment" type="text" value=""></textarea>
<button onclick="viodChecker()">SEND</button>
<div class="mailForm__error name-error">NAME ERROR</div>
<div class="mailForm__error number-error">TEL ERROR</div>
<div class="mailForm__error email-error">@ ERROR</div>
<div class="mailForm__error map-error">COORDS ERROR</div>
<div class="mailForm__completed">NICE!</div>
<div class="coords"></div>
</div>
<script src="script.js"></script>
</body>
</html>1 Answer 1
DRYing the ifs One option would be to use else, or even better, the conditional (ternary) operator instead of having two separate blocks for each validator:
errorName.style.display = name ? 'none' : 'block';
errorNumber.style.display = number ? 'none' : 'block';
errorEmail.style.display = email.includes('@') ? 'none' : 'block';
errorMap.style.display = Coords.textContent ? 'block' : 'none';
(You could also use a CSS class for this instead, and use classList.toggle and a second argument)
But for the inputs, rather than having these manual error divs, consider using the HTML to force validation instead. Put the inputs into a <form>, and then:
- For the name and number, use the
requiredattribute - the user's browser will tell them to fill out the fields - For the email, use the
type="email"attribute, and the user's browser will validate the email address (more reliably than just checking for@s).
Div crutch Rather than using a <div> to populate the getCoordinates results, you can save in an outer variable, eg:
// top level
let coords;
// ...lots of code
coords = firstGeoObject.geometry.getCoordinates();
Then rather than checking Coords.textContent, check the contents of the variable coords.
Other suggestions:
Always use const when you can - only use let when you must warn readers of the code that you may be reassigning the variable later in the code. 95% of variable declarations can be with const, usually.
On a similar note:
If you're writing in modern syntax (which you are, and you should!), never use var - var has the same problems as let, but also has issues with having unintuitive function scope instead of block scope, and of being automatically assigned to a property of the global object when on the top level.
Catch asynchronous errors, don't ignore them - currently, if .geocode throws, no indication will be given to the user, and they may well be confused. When dealing with a Promise, usually you'll want to follow a pattern like:
someProm
.then((result) => {
// do stuff with result
})
.catch((err) => {
// there was an error, inform the user and maybe log it for debugging
});
Avoid inline event handlers, they have too many problems to be worth using nowadays, including a demented scope chain and requiring global pollution. Instead, add the event listener using JavaScript instead, eg:
document.querySelector('.mailform > button').addEventListener('click', viodChecker);