Skip to content

Obj/contact us updates#1

Open
Alvi24 wants to merge 8 commits into
mainfrom
obj/contactUs-updates
Open

Obj/contact us updates#1
Alvi24 wants to merge 8 commits into
mainfrom
obj/contactUs-updates

Conversation

@Alvi24
Copy link
Copy Markdown
Owner

@Alvi24 Alvi24 commented Jun 29, 2023

No description provided.

@Alvi24 Alvi24 requested a review from edemiraj-innvis June 29, 2023 08:36
Comment thread js/script.js Outdated
);

radioButtonsEmailSpecification.forEach((radioButtonEmailSpecification) => {
if (clickedRadioButtonTelEmail.id == "email") {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Used strict equality (===) instead of loose equality (==) for comparison to ensure consistent behavior.

Comment thread js/script.js
isInputValid = TelRegEx.test(input.value);
break;
case "select-one":
isInputValid = input.value !== "";
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Used strict equality (===) instead of loose equality (==) for comparison to ensure consistent behavior.

Comment thread js/script.js
if (!isFormValid) return;

const inputsText = inputs
.filter((input) => input.value !== "on")
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Used strict equality (===) instead of loose equality (==) for comparison to ensure consistent behavior.

Comment thread js/script.js Outdated
if (clickedRadioButtonTelEmail.id == "email") {
radioButtonEmailSpecification.parentElement.classList.remove("hidden");
} else {
radioButtonEmailSpecification.parentElement.classList = "hidden";
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Replaced classList = with classList.add and classList.remove to correctly add or remove classes.

Comment thread js/script.js Outdated
messageTextInput.classList.remove("hidden");
} else {
messageTextInput.value = "";
messageTextInput.classList = "hidden";
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Replaced classList = with classList.add and classList.remove to correctly add or remove classes.

Comment thread js/script.js

if (isInputValid) input.classList.remove("invalid");
else input.classList.add("invalid");
return isInputValid;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Refactor to:
if (isInputValid) {
input.classList.remove("invalid");
} else {
input.classList.add("invalid");
}

return isInputValid;

Comment thread js/script.js Outdated
);
const dropdown = document.querySelector("select");

window.addEventListener("load", function () {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I would be better to refactor the code:
// Function to reset and add event listeners to text inputs and buttons
function initializeInputs() {
const textInputs = document.querySelectorAll(
'select, input:not([type="checkbox"]):not([type="radio"]):not(.notRequired)'
);
const buttons = document.querySelectorAll(
'input[type="checkbox"], input[type="radio"]:not(.notRequired)'
);

textInputs.forEach((textInput) => {
textInput.value = "";
textInput.addEventListener("input", (e) => validateInput(e.target));
});

buttons.forEach((button) => {
button.checked = false;
button.addEventListener("click", (e) => validateInput(e.target));
});

messageCheckBox.checked = false;
}

// Event listener for window load
window.addEventListener("load", function () {
initializeInputs();
});

Comment thread js/script.js
'select,input:not(:is([type="checkbox"],[type="radio"]),.notRequired)'
);
let buttons = document.querySelectorAll(
'input:is([type="checkbox"],[type="radio"]):not(.notRequired)'
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Simplified the selectors for text inputs and buttons to exclude the unnecessary :is and :not pseudo-classes.

Comment thread js/script.js Outdated
);
let isFormValid = true;

for (const input of inputs) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Used inputs.forEach instead of for...of loop in the validateForm function for consistency.
Refactor:
inputs.forEach((input) => {
if (!validateInput(input)) {
isFormValid = false;
}
});

if (!isFormValid) {
return;
}

Comment thread styles/styles.scss Outdated
grid-column: 1/-1;
}

input,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

SASS nesting - Refactor code: From the HTML structure input in nested within fieldset. Please refactor the code and follow the SASS nesting rules.

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.

2 participants