-
Notifications
You must be signed in to change notification settings - Fork 116
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.
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
improve nav-faq,signup,conatct,feedback #235
base: main
Are you sure you want to change the base?
improve nav-faq,signup,conatct,feedback #235
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, Web-Dev-Learner, for creating this pull request and contributing to WasteManagement! 💗
The maintainers will review this Pull Request and provide feedback as soon as possible! 😇
We appreciate your patience and contribution, Keep up the great work! 😀
|
@Web-Dev-Learner resolve conflicts and tag me again |
I resolved the conflicts @GarimaSingh0109 label#116 , level 3 , gssoc-ext , hackberfest-accepted changes
screenshots |
WalkthroughThe changes in this pull request involve a comprehensive redesign of the webpage, including significant updates to the HTML structure, JavaScript functionality, and CSS styling. Key modifications include the replacement of the Boxicons script with Material Icons, a restructured header, and new sections for FAQs and classification. JavaScript enhancements focus on improved validation for forms and image uploads, providing better user feedback. The CSS has been extensively revised to modernize the design, improve layout consistency, and ensure responsiveness across devices. Changes
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
@Web-Dev-Learner resolve conflict |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 18
🧹 Outside diff range and nitpick comments (7)
script.js (3)
1-25
: Approve changes and suggest error handling improvement.The addition of the image selection check (lines 8-11) is a good improvement for user experience. However, the error handling for the fetch call could be enhanced to provide user feedback.
Consider updating the error handling to provide user feedback:
.then(data => { classificationResult.textContent = data.classification; disposalInformation.textContent = data.disposalInformation; }) -.catch(error => console.error('Error:', error)); +.catch(error => { + console.error('Error:', error); + classificationResult.textContent = 'An error occurred during classification.'; + disposalInformation.textContent = 'Please try again later.'; +});
36-64
: Approve feedback form validation with a minor suggestion.The feedback form validation implementation is well-structured and provides good user feedback. The use of inline error messages is a good UX practice.
Consider using a more robust email regex for better validation:
-if (!/^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(email)) { +if (!/^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$/.test(email)) { formErrorMessage.textContent = 'Invalid email format.'; return; }This regex provides a more comprehensive check for email formats while still being relatively simple.
66-79
: Approve newsletter form validation with suggestions for improvement.The newsletter form validation is consistent with the feedback form implementation, which is good for maintaining code consistency.
Consider the following improvements:
- Use the same improved email regex as suggested for the feedback form.
- Clear the error message when the email is valid:
const newsletterForm = document.getElementById('newsletter-form'); +const newsletterErrorMessage = document.getElementById('newsletter-error-message'); newsletterForm.addEventListener('submit', (event) => { event.preventDefault(); const email = newsletterForm.querySelector('input[type="email"]').value.trim(); - if (!/^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(email)) { - document.getElementById('newsletter-error-message').textContent = 'Please enter a valid email address.'; + if (!/^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$/.test(email)) { + newsletterErrorMessage.textContent = 'Please enter a valid email address.'; } else { + newsletterErrorMessage.textContent = ''; // Clear error message alert('Subscribed to newsletter successfully!'); newsletterForm.reset(); } });These changes will improve the robustness of the email validation and ensure that the error message is cleared when the input becomes valid.
styles.css (2)
193-193
: Use consistent color notationIn the
.mui-btn
class, thecolor
property is defined as#fff
in one instance andwhite
in another. For consistency and readability, it's advisable to use a consistent color notation throughout the stylesheet.Also applies to: 227-227
196-196
: Ensure consistentborder-radius
for buttonsThe
.mui-btn
class has differingborder-radius
values (4px
and5px
) in its definitions. To maintain uniform button styles, choose a singleborder-radius
value.Also applies to: 230-230
index.html (2)
548-550
: Reset form fields after successful signup submissionTo enhance user experience, consider clearing the form fields after displaying the success message. This provides visual feedback that the form has been submitted.
Add the following line after displaying the success message:
successMessage.style.display = 'block'; +document.getElementById('signup-form').reset();
519-521
: Reset form fields after successful feedback submissionSimilarly, resetting the feedback form after a successful submission can improve user experience.
Add the following line after displaying the success message:
successMessage.style.display = 'block'; +document.getElementById('feedback-form').reset();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
assets/faq-w.jpg
is excluded by!**/*.jpg
📒 Files selected for processing (3)
- index.html (1 hunks)
- script.js (1 hunks)
- styles.css (1 hunks)
🧰 Additional context used
🔇 Additional comments (2)
script.js (2)
28-34
: LGTM: FAQ toggle implementation.The FAQ toggle implementation is clean and efficient. It uses event delegation, which is a good practice for performance.
1-79
: Overall assessment: Good improvements with minor suggestions for refinement.The changes in this file significantly improve form validation and user feedback across different sections (image upload, FAQ, feedback form, and newsletter subscription). The implementations are consistent and follow good practices. The suggestions provided are for further refinement and mainly focus on improving the email validation regex and error message handling. These changes align well with the PR objectives of enhancing the navigation section and improving content.
color: white; | ||
padding: 80px 20px; | ||
text-align: center; | ||
background: url('your-hero-image.jpg') no-repeat center center/cover; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replace placeholder hero image URL with actual image path
The background image URL in the .hero
class is set to 'your-hero-image.jpg'
, which appears to be a placeholder. Please replace it with the correct path to the actual hero image to ensure the background displays correctly.
styles.css
Outdated
#feedback-form { | ||
max-width: 600px; | ||
margin: 0 auto; | ||
padding: 20px; | ||
background-color: #fff; | ||
border: 1px solid #e0e0e0; | ||
border-radius: 8px; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consolidate duplicate #feedback-form
definitions
There are two separate definitions for the #feedback-form
selector with differing properties. This can lead to unexpected styling issues. Consider merging them into a single definition to maintain consistent styling.
Apply this diff to consolidate the definitions:
/* Feedback Form */
#feedback-form {
- max-width: 600px;
+ max-width: 500px;
margin: 0 auto;
+ padding: 20px;
+ background-color: #fff;
+ border: 1px solid #e0e0e0;
+ border-radius: 8px;
}
Also applies to: 210-212
styles.css
Outdated
.mui-input { | ||
width: 100%; | ||
padding: 10px; | ||
border: 1px solid #ccc; | ||
border-radius: 4px; | ||
font-size: 1em; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consolidate duplicate .mui-input
definitions
The .mui-input
class is defined multiple times with conflicting properties such as border-radius
and font-size
. This may result in inconsistent input field styles. Recommend unifying these definitions to ensure consistency.
Consider merging the properties:
.mui-input {
width: 100%;
padding: 10px;
margin-bottom: 15px;
border: 1px solid #ccc;
border-radius: 5px; /* Choose consistent value */
font-size: 16px; /* Choose consistent value */
}
Also applies to: 215-221
styles.css
Outdated
.mui-btn { | ||
padding: 10px 20px; | ||
background-color: #2a9d8f; | ||
color: #fff; | ||
border: none; | ||
cursor: pointer; | ||
border-radius: 4px; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consolidate duplicate .mui-btn
definitions
The .mui-btn
class is defined twice with different styles, including background-color
and border-radius
. This can cause buttons to appear inconsistently across the site. Unify these definitions to maintain consistent button styling.
Consider the following unified definition:
.mui-btn {
padding: 10px 20px;
background-color: #2a9d8f; /* Choose one color */
color: #fff; /* Use consistent color notation */
border: none;
border-radius: 5px; /* Choose consistent value */
font-size: 16px;
cursor: pointer;
}
Also applies to: 224-231
.form-error { | ||
color: red; | ||
margin-top: 10px; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consolidate duplicate .form-error
definitions
The .form-error
class is defined more than once with slightly different properties. This redundancy can lead to maintenance challenges and unexpected styling issues. Merge them into a single definition to ensure consistent error message styling.
Combine the properties:
.form-error {
color: red;
margin-top: 10px; /* or margin-bottom: 10px; decide based on design */
}
Also applies to: 238-240
const fullName = document.querySelector('#signup-form input[aria-label="Full Name"]').value.trim(); | ||
const email = document.querySelector('#signup-form input[aria-label="Email Address"]').value.trim(); | ||
const password = document.querySelector('#signup-form input[aria-label="Password"]').value.trim(); | ||
const confirmPassword = document.querySelector('#signup-form input[aria-label="Confirm Password"]').value.trim(); | ||
const errorMessage = document.getElementById('signup-error-message'); | ||
const successMessage = document.getElementById('signup-success-message'); | ||
|
||
if (!fullName || !email || !password || !confirmPassword) { | ||
errorMessage.textContent = 'Please fill in all fields.'; | ||
errorMessage.style.display = 'block'; | ||
successMessage.style.display = 'none'; | ||
} else if (password !== confirmPassword) { | ||
errorMessage.textContent = 'Passwords do not match.'; | ||
errorMessage.style.display = 'block'; | ||
successMessage.style.display = 'none'; | ||
} else if (!validateEmail(email)) { | ||
errorMessage.textContent = 'Please enter a valid email address.'; | ||
errorMessage.style.display = 'block'; | ||
successMessage.style.display = 'none'; | ||
} else { | ||
errorMessage.style.display = 'none'; | ||
successMessage.textContent = 'Account created successfully!'; | ||
successMessage.style.display = 'block'; | ||
} | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensure secure handling of user passwords
While client-side validation improves user experience, handling passwords on the client side requires caution. Ensure that passwords are transmitted securely over HTTPS and that sensitive data is appropriately managed to protect user information.
question.addEventListener('click', () => { | ||
question.classList.toggle('active'); | ||
const answer = question.nextElementSibling; | ||
answer.style.display = answer.style.display === 'block' ? 'none' : 'block'; | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Use CSS classes to toggle FAQ answers instead of inline styles
Manipulating inline styles directly can lead to maintenance challenges and unexpected behavior. Consider using CSS classes to show or hide FAQ answers, which enhances readability and scalability.
Define the classes in your CSS:
.faq-answer {
display: none;
}
.faq-answer.active {
display: block;
}
Update the JavaScript:
question.addEventListener('click', () => {
question.classList.toggle('active');
const answer = question.nextElementSibling;
- answer.style.display = answer.style.display === 'block' ? 'none' : 'block';
+ answer.classList.toggle('active');
});
// Contact form validation and submission | ||
document.getElementById('contact-form').addEventListener('submit', function(event) { | ||
event.preventDefault(); | ||
const name = document.getElementById('contact-name').value.trim(); | ||
const email = document.getElementById('contact-email').value.trim(); | ||
const message = document.getElementById('contact-message').value.trim(); | ||
const errorMessage = document.getElementById('contact-error-message'); | ||
const successMessage = document.getElementById('contact-success-message'); | ||
|
||
if (!name || !email || !message) { | ||
errorMessage.textContent = 'Please fill in all fields.'; | ||
errorMessage.style.display = 'block'; | ||
successMessage.style.display = 'none'; | ||
} else if (!validateEmail(email)) { | ||
errorMessage.textContent = 'Please enter a valid email address.'; | ||
errorMessage.style.display = 'block'; | ||
successMessage.style.display = 'none'; | ||
} else { | ||
errorMessage.style.display = 'none'; | ||
successMessage.textContent = 'Your message has been sent!'; | ||
successMessage.style.display = 'block'; | ||
} | ||
}); | ||
|
||
// Email validation function | ||
function validateEmail(email) { | ||
const re = /^[^\s@]+@[^\s@]+\.[^\s@]+$/; | ||
return re.test(email); | ||
} | ||
</script> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Extract contact form script to an external JavaScript file
The JavaScript handling the contact form submission is embedded within the HTML. For better organization and reusability, consider moving this script to an external JS file.
</div> | ||
</div> | ||
<h2>Upload Image</h2> | ||
<input type="file" id="image-input" accept="image/*"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a label to the file input for better accessibility
The file input lacks an associated <label>
element, which can make it difficult for users relying on screen readers to identify its purpose. Consider adding a label to improve accessibility.
Apply this diff to include a label:
+<label for="image-input">Choose an image:</label>
<input type="file" id="image-input" accept="image/*">
Alternatively, if you prefer not to display a visible label, you can add an aria-label
attribute:
<input type="file" id="image-input" accept="image/*" +aria-label="Choose an image">
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
<input type="file" id="image-input" accept="image/*"> | |
<label for="image-input">Choose an image:</label> | |
<input type="file" id="image-input" accept="image/*"> |
</nav> | ||
<div class="navbar-container"> | ||
<a href="#" class="logo"> | ||
<img src="logo.png" alt="Waste Management Logo"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct the logo image path for consistency
The src
attribute of the logo image is set to logo.png
, while other assets are located in the assets
directory. To maintain consistency and prevent potential broken links, update the image path if necessary.
Apply this diff if the image is in the assets
folder:
-<img src="logo.png" alt="Waste Management Logo">
+<img src="assets/logo.png" alt="Waste Management Logo">
Committable suggestion was skipped due to low confidence.
@Web-Dev-Learner there are 7 conflicts!! Solve them hen re-tag. |
❌ Deploy Preview for manageyourwaste failed. Why did it fail? →
|
This PR has been merged by the GSSoC team Don't add labels to it now. You'll get the points in the leaderboard update. |
label
#116 , label 3 , gssoc-ext , hackberfest-accepted
changes
scrrenshots
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Style