Skip to content

Conversation

@DkSanjed
Copy link
Owner

@DkSanjed DkSanjed commented Jun 2, 2020

Exercise dom

@@ -0,0 +1,65 @@
let form = document.querySelector('form');
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 use classes. e.g: .contact-form to reuse code.

nameCondition.classList.add('required');
return "";
} else {
nameCondition.classList.remove("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.

Use ' instead of " to keep the standard.

}

function validEmail(){
if (/^\w+([\.-]?\w+)*@\w+([\.-]?\w+)*(\.\w{2,3})+$/.test(Email.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 EMAIL_REGEX = /^\w+([\.-]?\w+)*@\w+([\.-]?\w+)*(\.\w{2,3})+$/

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

Choose a reason for hiding this comment

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

you are mixing ' with ".

}

function validNumber(){
if (/\d+$/.test(Phone.value)) {
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 RegEx in a const. It can be at the top of the code.
const PHONE_REGEX = /\d+$/

function structureIf(nameCondition){
if (nameCondition.value == '') {
console.log(`${nameCondition.id}: It's need value`);
nameCondition.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.

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

Copy link
Collaborator

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.


function validEmail(){
if (/^\w+([\.-]?\w+)*@\w+([\.-]?\w+)*(\.\w{2,3})+$/.test(Email.value)) {
Email.classList.remove("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.

Copy link
Collaborator

@juancho11gm juancho11gm left a comment

Choose a reason for hiding this comment

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

Hi Raúl, please see the comments and send the changes.

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

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.

}

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

Choose a reason for hiding this comment

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

you are mixing ' with ".


function validCheckbox2(array){
let check = "";
if (array[0].checked == false && array[1].checked == false && array[2].checked == false && array[3].checked == false) {
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 find an easier way to validate it

Copy link
Collaborator

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.remove("required");
for (let index = 0; index < array.length; index++) {
if (array[index].checked == true) {
check += array[index].value+ ". ";
Copy link
Collaborator

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.

@@ -0,0 +1,3 @@
.required{
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
.required{
.required {

function structureIf(nameCondition){
if (nameCondition.value == '') {
console.log(`${nameCondition.id}: It's need value`);
nameCondition.classList.add('required');
Copy link
Collaborator

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.

Comment on lines +57 to +60
<p id="print_name">Name:</p>
<p id="print_email">Email:</p>
<p id="print_number">Number:</p>
<p id="print_checkbox">Checkbox:</p>
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.

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

let form = document.querySelector('form');

function structureIf(nameCondition){
if (nameCondition.value == '') {
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
if (nameCondition.value == '') {
if (nameCondition.value === '') {


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

Choose a reason for hiding this comment

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

alert it too

<div>
<textarea name="text" id="textArea" cols="50" rows="10"></textarea>
</div>
<button id="Button_submit">Send message</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 id="Button_submit">Send message</button>
<button id="submit-button">Send message</button>

@juancho11gm
Copy link
Collaborator

Tip: follow this convention.
image

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.

3 participants