Skip to content

Exercise Dom by Raul Valencia #2

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
DkSanjed wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Exercise Dom by Raul Valencia #2

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 65 additions & 0 deletions 2-dom-manipulation/form.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
let form = document.querySelector('form');
Copy link
Collaborator

@juancho11gm juancho11gm Jun 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use classes. e.g: .contact-form to reuse code.


function structureIf(nameCondition){
if (nameCondition.value == '') {
Copy link
Collaborator

@juancho11gm juancho11gm Jun 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (nameCondition.value == '') {
if (nameCondition.value === '') {

console.log(`${nameCondition.id}: It's need value`);
Copy link
Collaborator

@juancho11gm juancho11gm Jun 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

alert it too

nameCondition.classList.add('required');
Copy link
Collaborator

@juancho11gm juancho11gm Jun 4, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can save the class name in a const.
const REQUIRED_CLASS = 'required';

Copy link
Collaborator

@juancho11gm juancho11gm Jun 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would name it const ERROR_CLASS = 'error';.
Because I can enter a value, but with the wrong format. So it would be a format problem and not a required problem.

return "";
} else {
nameCondition.classList.remove("required");
Copy link
Collaborator

@juancho11gm juancho11gm Jun 4, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use ' instead of " to keep the standard.

return nameCondition.value;
}
}

function validEmail(){
if (/^\w+([\.-]?\w+)*@\w+([\.-]?\w+)*(\.\w{2,3})+$/.test(Email.value)) {
Copy link
Collaborator

@juancho11gm juancho11gm Jun 4, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can save the RegEx in a const. It can be at the top of the code.
const EMAIL_REGEX = /^\w+([\.-]?\w+)*@\w+([\.-]?\w+)*(\.\w{2,3})+$/

Email.classList.remove("required");
Copy link
Collaborator

@juancho11gm juancho11gm Jun 4, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here you can use the const. REQUIRED_CLASS.

return Email.value;
} else {
Email.classList.add("required");
Copy link
Collaborator

@juancho11gm juancho11gm Jun 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here you can use the const. REQUIRED_CLASS.

console.log(`${Email.id}: You have entered an invalid email address`)
alert("You have entered an invalid email address!");
Comment on lines +20 to +21
Copy link
Collaborator

@juancho11gm juancho11gm Jun 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you are mixing ' with ".

return "";
}
}

function validNumber(){
if (/\d+$/.test(Phone.value)) {
Copy link
Collaborator

@juancho11gm juancho11gm Jun 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can save the RegEx in a const. It can be at the top of the code.
const PHONE_REGEX = /\d+$/

Phone.classList.remove("required");
return Phone.value;
} else {
Phone.classList.add("required");
console.log(`${Phone.id}: You can only write numbers`)
alert("You can only write numbers!");
return "";
}
}

function validCheckbox2(array){
let check = "";
Copy link
Collaborator

@juancho11gm juancho11gm Jun 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you are mixing ' with ".

if (array[0].checked == false && array[1].checked == false && array[2].checked == false && array[3].checked == false) {
Copy link
Collaborator

@juancho11gm juancho11gm Jun 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can find an easier way to validate it

Copy link
Collaborator

@juancho11gm juancho11gm Jun 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Imagine that you have more checkboxes. Would you do it in the same way?

checkbox_id.classList.add("required");
alert('At least one course must be selected');
return "";
} else {
checkbox_id.classList.remove("required");
for (let index = 0; index < array.length; index++) {
if (array[index].checked == true) {
check += array[index].value+ ". ";
Copy link
Collaborator

@juancho11gm juancho11gm Jun 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you want to concat strings use template literals. I think that t's better to return the array with the checked values.

}
}
return check;
}
}

function validaTeForm(event){
event.preventDefault();
let array = [form.course1, form.course2, form.course3, form.course4];
Copy link
Collaborator

@juancho11gm juancho11gm Jun 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use a better name for your variables.

print_name.textContent = `Name: ${structureIf(Name)}`;
structureIf(Email);
print_email.textContent = `Email: ${validEmail()}`;
print_number.textContent = `Number: ${validNumber()}`;
print_checkbox.textContent = `Checkbox: ${validCheckbox2(array)}`;
Comment on lines +58 to +62
Copy link
Collaborator

@juancho11gm juancho11gm Jun 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Print the form values in the HTML (below the form) if all of them are correct only

Copy link
Collaborator

@juancho11gm juancho11gm Jun 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, are print_name, print_email, print_number and print_number elements, consts?
You can declare it at the top.

let nameAnswer = document.querySelector('#print_name');
let emailAnswer = document.querySelector('#print_email');
let phoneAnswer = document.querySelector('#print_number');
let coursesAnswer = document.querySelector('#print_checkbox');

Copy link
Collaborator

@juancho11gm juancho11gm Jun 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same with Name, Phone and Email

}

form.addEventListener('submit', validaTeForm);
64 changes: 64 additions & 0 deletions 2-dom-manipulation/forms.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8">
<meta name="viewport" content="width=device-width, initial-scale=1.0">
<link rel="stylesheet" href="style.css">
<title>Forms</title>
</head>
<body>
<main>
<form>
<fieldset>
<legend><strong>Please enter your contact details</strong></legend>
<div>
<label for="Name"><strong>Your name (required):</strong></label>
<input type="text" id="Name" name="user_name" aria-label="required">
<abbr title="required"></abbr>
</div>
<div>
<label for="Email"><strong>E-Mail address (required):</strong> </label>
<input id="Email" name="user_email" aria-label="required">
<abbr title="required"></abbr>
</div>
<div>
<label for="Phone"><strong>Phone number (+57) (optional):</strong> </label>
<input id="Phone" name="user_number">
</div>
</fieldset>
<fieldset id="checkbox_id">
<legend><strong>Please select the courses that you are interested in (required)</strong></legend>
<div>
<input type="checkbox" id="webDeveloper" name="course1" value="Web Developer">
<label for="webDeveloper"> <strong>Web Developer</strong></label>
</div>
<div>
<input type="checkbox" id="reactDeveloper" name="course2" value="React Developer">
<label for="reactDeveloper"> <strong>React Developer</strong></label>
</div>
<div>
<input type="checkbox" id="fullStack" name="course3" value="Fullstack Developer">
<label for="fullStack"> <strong>Fullstack Developer</strong></label>
</div>
<div>
<input type="checkbox" id="other" name="course4" value="Other">
<label for="other"> <strong>Other</strong></label>
</div>
</fieldset>
<div>
<label for="textArea"><strong>Please enter a message (optional):</strong></label>
</div>
<div>
<textarea name="text" id="textArea" cols="50" rows="10"></textarea>
</div>
<button id="Button_submit">Send message</button>
Copy link
Collaborator

@juancho11gm juancho11gm Jun 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<button id="Button_submit">Send message</button>
<button id="submit-button">Send message</button>

<button type="reset">Reset form</button>
</form>
<p id="print_name">Name:</p>
<p id="print_email">Email:</p>
<p id="print_number">Number:</p>
<p id="print_checkbox">Checkbox:</p>
Comment on lines +57 to +60
Copy link
Collaborator

@juancho11gm juancho11gm Jun 4, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not wrong. If you want to create it from js file you can use document.createElement.

<script src="form.js"></script>
</main>
</body>
</html>
3 changes: 3 additions & 0 deletions 2-dom-manipulation/style.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
.required{
Copy link
Collaborator

@juancho11gm juancho11gm Jun 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
.required{
.required {

border: 1px solid red;
}