Skip to content

Conversation

@EstefaSora125
Copy link
Owner

Dom exercise by Estefanía Sora

Comment on lines +8 to +12
let name = document.querySelector('#contact_form').querySelector('#user_name'),
email = document.querySelector('#contact_form').querySelector('#email_this'),
number = document.querySelector('#contact_form').querySelector('#user_phone_number'),
textBox = document.querySelector('#contact_form').querySelector('#text-box'),
box = document.querySelector('#contact_form').querySelectorAll('input[type="checkbox"]');
Copy link
Collaborator

Choose a reason for hiding this comment

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

you can save the form reference so you get the elements faster. It is also better to use const since these values will never change. Grouping multiple variable definitions is not easy to read, so it's better if you keep them separated.

Suggested change
let name = document.querySelector('#contact_form').querySelector('#user_name'),
email = document.querySelector('#contact_form').querySelector('#email_this'),
number = document.querySelector('#contact_form').querySelector('#user_phone_number'),
textBox = document.querySelector('#contact_form').querySelector('#text-box'),
box = document.querySelector('#contact_form').querySelectorAll('input[type="checkbox"]');
const form = document.querySelector('#contact_form');
const name = form.querySelector('#user_name');
const email = form.querySelector('#email_this');
const number = form.querySelector('#user_phone_number');
const textBox = form.querySelector('#text-box');
const box = form.querySelectorAll('input[type="checkbox"]');

validateName(name);
validarEmail(email);
validatePhone(number);
validateChecBox(box);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
validateChecBox(box);
validateCheckBox(box);

Comment on lines +19 to +27
let valu = [name.value, email.value, number.value, validateChecBox(box), textBox.value];
let pu = [" Name: ", "Email: ", "Phone number: ", "Courses: ", "Message: "];

for (let index = 0; index < valu.length; index++) {
for (let ind = 0; ind < pu.length; ind++) {
showInfotmation(pu, valu);
}
}
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

you don't need to do this, in Javascript you can use objects as dictionaries:

let formValues = {
  name:  'name',
   email:  'email'
};
// and then you can access these key/pair values

for (const property in formValues) {
  console.log(`${property}: ${formValues[property]}`);
}

In this way you don't need to do a double for loop which is performance heavy.

Comment on lines +59 to +71
let select = null;
let count = [];
for (let index = 0; index < courses.length; index++) {
if (courses[index].checked ==true) {
select = courses[index].value;
let show = count.push(select);
}
}
if(select == null){
alert ('You have to select a course');
}
return count;
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

for some reason this validation is being executed twice and then a double alert is being displayed to the user, can you double check.

return count;
};

function showInfotmation(label, text=[]){
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
function showInfotmation(label, text=[]){
function showInformation(label, text=[]){

Comment on lines +29 to +30
<label for="email_this">E-mail address
<input id="email_this" name="Email" aria-required="true" >
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
<label for="email_this">E-mail address
<input id="email_this" name="Email" aria-required="true" >
<label for="email">E-mail address
<input id="email" name="Email" aria-required="true" >

<legend>Please select the courses thar are you interested in
<abbr title="required">*</abbr>
</legend>
<div id="course" >
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
<div id="course" >
<div id="course">

Comment on lines +53 to +55
<div>
<textarea name="message" id="text-box" cols="40" rows="10"></textarea>
</div>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
<div>
<textarea name="message" id="text-box" cols="40" rows="10"></textarea>
</div>
<textarea name="message" id="text-box" cols="40" rows="10"></textarea>

<div>
<textarea name="message" id="text-box" cols="40" rows="10"></textarea>
</div>
<button>Send mesagge</button>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
<button>Send mesagge</button>
<button>Send message</button>

</article>
</main>
<footer>
<p>This example was created <time datetime="2020-04-12 11:21">at 11:21 AM</time></p>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
<p>This example was created <time datetime="2020-04-12 11:21">at 11:21 AM</time></p>
<p>This example was created at<time datetime="2020-04-12 11:21">11:21 AM</time></p>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants