-
Notifications
You must be signed in to change notification settings - Fork 114
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
Implemented seamless realtime waste monitoring feature #260
Implemented seamless realtime waste monitoring feature #260
Conversation
@Nkovaturient solve conflicts |
hey @khushi-joshi-05 @GarimaSingh0109 , I have fixed the conflicts. Kindly look into it and guide me. |
@khushi-joshi-05 @GarimaSingh0109 |
@khushi-joshi-05 @GarimaSingh0109 Hey there?! |
Caution Review failedThe pull request is closed. WalkthroughThe pull request introduces extensive modifications to a Waste Management website, enhancing user interface and functionality. Key updates include a revamped eco-tips section, a new real-time monitoring dashboard, and improved responsiveness across various components. The index page now features a progress bar and a slider for displaying features. CSS updates ensure a cohesive design, while JavaScript enhancements add interactivity, including form validation and a "Scroll to Top" button. Overall, these changes aim to improve user engagement and streamline waste management tracking. Changes
Assessment against linked issues
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 (
|
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: 25
🧹 Outside diff range and nitpick comments (13)
realtime-monitoring.css (7)
1-16
: LGTM! Consider adjusting the z-index value.The body and header styles are well-structured and follow good practices. The sticky header with flexbox layout is a good choice for consistent navigation.
Consider using a lower z-index value for the header, such as 100. The current value of 999 is unnecessarily high and might cause issues with other positioned elements in the future.
18-36
: LGTM! Consider adding hover styles for navigation links.The navigation styles create a clean, horizontal menu layout that integrates well with the header.
To enhance user experience, consider adding hover styles for the navigation links. For example:
nav ul li a:hover { text-decoration: underline; color: #f0f0f0; }This will provide visual feedback when users interact with the menu items.
38-67
: Good design, but consider some refinements.The container and card styles create an attractive layout for data presentation. The use of flexbox and creative styling for depth effects is commendable.
Consider the following refinements:
- Replace the vw unit for .card-input width with a max-width in pixels or percentage:
.card-input { width: 100%; max-width: 400px; /* other properties */ }
- The negative transform on .card-input might cause layout shifts. Consider using margin-top instead:
.card-input { margin-top: 10px; /* remove transform: translateY(-10px); */ /* other properties */ }These changes will improve responsiveness and prevent potential layout issues.
100-163
: LGTM! Consider optimizing for larger screens.The monitor dashboard styles create a well-structured, responsive layout. The use of flexbox and media queries demonstrates good responsive design practices.
To further optimize the layout for larger screens, consider adding another breakpoint:
@media (min-width: 1200px) { .dashboard-card { width: 23%; /* Allows for 4 cards in a row on very large screens */ } }This change will make better use of space on larger displays while maintaining the current layout on smaller screens.
165-211
: LGTM! Consider more flexible image handling.The waste segregation guide styles create an attractive and informative layout. The use of flexbox and visual elements like dashed borders enhances the user experience.
To improve flexibility with different image sizes, consider the following changes:
- For bin images:
.bin img { max-width: 100px; max-height: 100px; width: auto; height: auto; object-fit: contain; }
- For waste items:
.waste-item { width: 50px; height: 50px; display: flex; align-items: center; justify-content: center; } .waste-item img { max-width: 100%; max-height: 100%; object-fit: contain; }These changes will ensure that images maintain their aspect ratios and fit well within their containers, regardless of their original dimensions.
213-220
: LGTM! Consider future-proofing the footer.The footer styles are clean, consistent with the overall design, and functional for the current needs.
To accommodate potential future expansion of the footer content, consider updating the styles as follows:
#footer { background-color: #00796b; color: white; padding: 20px; display: flex; flex-wrap: wrap; justify-content: space-around; align-items: center; } #footer > * { margin: 10px; }This change allows for easy addition of multiple elements (like links, social media icons, etc.) while maintaining a clean layout on various screen sizes.
1-220
: Great work overall! Consider using CSS variables for improved maintainability.The CSS file for the real-time monitoring dashboard is well-structured and creates a cohesive, visually appealing interface. The use of responsive design techniques and a consistent color scheme enhances the user experience across different devices.
To improve maintainability and make future theme changes easier, consider using CSS variables for colors and other repeated values. Here's an example of how you could implement this:
:root { --primary-color: #00796b; --secondary-color: #83C5BE; --text-color: #fff; --background-color: #f4f4f4; --accent-color: #2E7D32; /* Add other colors and common values */ } /* Then use these variables throughout your CSS */ body { background-color: var(--background-color); } header { background-color: var(--primary-color); color: var(--text-color); } /* And so on... */This approach will make it much easier to maintain a consistent theme and make global color changes in the future.
script.js (3)
190-190
: Remove unnecessary placeholder commentThe comment
// feature/your-new-feature
appears to be a leftover placeholder or template text. Removing it will help keep the codebase clean and free of irrelevant comments.Apply this diff to remove the unnecessary comment:
-// feature/your-new-feature
Line range hint
283-303
: Fix the syntax error due to an extra closing parenthesisThere is an extra closing parenthesis
)
after theloginForm
event listener, causing a syntax error as indicated by the static analysis tool.Apply this diff to fix the syntax error:
}); - - }); + }Explanation:
- Replace
});
with}
to properly close theif (loginForm)
block without the extra parenthesis.
Line range hint
283-303
: Ensure form event listeners are added after DOM content is loadedThe code attaching event listeners to
signupForm
andloginForm
is currently outside theDOMContentLoaded
event listener. This can lead to issues if the script is executed before the DOM elements are available. To ensure that the forms are present in the DOM when you try to access them, move this code inside theDOMContentLoaded
event listener.Apply this refactor to move the form event listener code inside the
DOMContentLoaded
event listener:+ // Attach form validation to respective forms + const signupForm = document.getElementById('form1'); + if (signupForm) { + signupForm.addEventListener('submit', (event) => { + if (!validateSignup()) { + event.preventDefault(); // Prevent form submission + } + }); + } + + const loginForm = document.getElementById('form2'); + if (loginForm) { + loginForm.addEventListener('submit', (event) => { + if (!validateLogin()) { + event.preventDefault(); // Prevent form submission + } + }); + }This ensures that the event listeners are attached only after the DOM is fully loaded.
ecotips.html (2)
28-28
: Use a more appropriate icon for the "Tips" navigation link.The
"fa-brands fa-pagelines"
icon represents the Pagelines brand, which may not clearly signify "Tips" to users. For better semantic meaning, consider using"fa-solid fa-leaf"
to represent eco-tips.Apply this diff to update the icon:
- ><i class="fa-brands fa-pagelines"></i> Tips</a + ><i class="fa-solid fa-leaf"></i> Tips</a
376-376
: Remove unnecessary commented-out code.The line
// updateSlider();
is commented out and may be redundant sinceupdateSlider
is already called on theload
event. Consider removing it to clean up the code.Apply this diff:
- // updateSlider();
styles.css (1)
1298-1310
: Inconsistent button styling in dark modeIn lines 1298-1310, the
.dark-mode .feedback
and.dark-mode .feedback button
styles are specified. Ensure that these button styles are consistent with other buttons in dark mode to provide a uniform user experience.Review and align button styles across the dark mode theme for consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (3)
assets/organic.jpg
is excluded by!**/*.jpg
assets/peel.jpg
is excluded by!**/*.jpg
assets/rbin.jpg
is excluded by!**/*.jpg
📒 Files selected for processing (6)
- ecotips.html (1 hunks)
- index.html (4 hunks)
- realtime-monitoring.css (1 hunks)
- realtime-monitoring.html (1 hunks)
- script.js (2 hunks)
- styles.css (4 hunks)
🧰 Additional context used
🪛 Biome
script.js
[error] 303-304: Expected a statement but instead found ')'.
Expected a statement here.
(parse)
🔇 Additional comments (8)
index.html (6)
16-18
: Progress Bar Added CorrectlyThe implementation of the progress bar within the
progress-container
is well-structured and integrates smoothly at the top of the page. This enhances the user interface by providing visual feedback on page scroll.
24-25
: Semantic Structure of Header ElementsWrapping the
<h1>
tag within an<a>
tag is acceptable in HTML5 and aids in making the site title clickable, redirecting users to the homepage. Ensure that this structure is accounted for in your CSS to maintain consistent styling and that it complies with accessibility standards.
Line range hint
82-110
: 'Waste Categories' Section Enhances User EducationThe addition of the "Waste Categories" section with interactive cards for Biodegradable and Non-Biodegradable waste is well-executed. It effectively educates users on waste types and promotes engagement through intuitive design.
116-141
: Optimize Image Accessibility with Alt AttributesIt appears that images within the feature cards or elsewhere in the provided code might be missing
alt
attributes.Please ensure that all
<img>
tags include descriptivealt
attributes to improve accessibility for users with screen readers.If images are added in these sections, apply this diff:
-<img src="image.jpg"> +<img src="image.jpg" alt="Descriptive text about the image">
382-382
: Include Necessary JavaScript FilesYou've included a reference to
script.js
at the end of the body:<script src="script.js"></script>Ensure that
script.js
contains the necessary JavaScript code to handle new interactive elements like the progress bar, feature slider, "Back to Top" button, and any form validations. Verify that this file is correctly linked and that all functions work as intended.
129-141
:⚠️ Potential issueDuplicate IDs on Multiple Elements
In the feature cards, the
id="eco-fact"
is used more than once within the page. IDs must be unique within an HTML document to prevent conflicts in CSS and JavaScript.Apply this diff to assign unique IDs or convert them to classes if the styling is shared:
-<blockquote id="eco-fact"> +<blockquote class="eco-fact">Update your CSS and JavaScript accordingly to reference the class selector instead of the ID.
Likely invalid or redundant comment.
styles.css (2)
973-1085
: Ensure cross-browser compatibility for slider componentThe slider styles introduced in lines 973-1085 include advanced CSS features and animations. To guarantee consistent behavior across all browsers, it's important to test the slider component in multiple browsers and add vendor prefixes where necessary.
Please confirm cross-browser compatibility and consider using tools like Autoprefixer to automatically add vendor prefixes.
139-145
:⚠️ Potential issueDuplicate definition of
nav ul
stylesThe styles for
nav ul
are redefined in lines 139-145, which duplicate the earlier declaration. This redundancy can lead to confusion and potential inconsistencies. Consider consolidating these styles to improve maintainability.Apply this diff to remove the duplicate definition:
- nav ul { - list-style-type: none; - padding: 0; - margin: 0; - display: flex; - justify-content: center; - }Likely invalid or redundant comment.
.chart-container { | ||
position: relative; | ||
height: 400px; | ||
} | ||
.card-chart-btn{ | ||
display: flex; | ||
gap: 20px; | ||
flex-wrap: wrap; | ||
align-items: center; | ||
justify-content: center; | ||
} | ||
#error{ | ||
color: red; | ||
font-weight: 600; | ||
} | ||
.card-chart-btn button{ | ||
border-radius: 24px; | ||
margin: 1.2rem; | ||
} | ||
button { | ||
display: inline-block; | ||
padding: 10px 20px; | ||
background-image: linear-gradient(to right, #334ee7, #04484e); | ||
border-radius: 12px; | ||
font-size: 1.1rem; | ||
transition: all 0.3s ease; | ||
} | ||
|
||
button:hover { | ||
background-image: linear-gradient(to right, #006d77, #edf6f9); | ||
} |
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
Enhance chart responsiveness and button accessibility.
The chart and button styles add visual appeal to the interface. However, there are areas for improvement in terms of responsiveness and accessibility.
- Make the chart container responsive:
.chart-container {
position: relative;
height: 50vw;
max-height: 400px;
min-height: 200px;
}
- Improve button accessibility:
button {
/* existing styles */
color: white; /* Ensure text is visible on gradient background */
border: none; /* Remove default border */
cursor: pointer; /* Indicate clickable */
}
button:focus {
outline: 2px solid #fff; /* Visible focus indicator */
outline-offset: 2px;
}
- Consider adding a transition to the error message for smoother appearance:
#error {
color: red;
font-weight: 600;
transition: opacity 0.3s ease;
}
These changes will improve the overall user experience and accessibility of your interface.
function moveSlider(direction) { | ||
const sliderWrapper = document.querySelector('.slider-wrapper'); | ||
const totalSlides = document.querySelectorAll('.feature-card').length; | ||
currentSlide += direction; | ||
|
||
// Loop the slider | ||
if (currentSlide >= totalSlides) { | ||
currentSlide = 0; | ||
} else if (currentSlide < 0) { | ||
currentSlide = totalSlides - 1; | ||
} | ||
|
||
// Move the slider | ||
sliderWrapper.style.transform = `translateX(-${currentSlide * 100}%)`; |
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
Consolidate duplicate slider functionality
There are two implementations for the slider functionality:
- The
moveSlider
function (lines 204-217) - The
updateSlider
function and related event listeners (lines 226-255)
Having two separate implementations for the same feature can lead to confusion and maintenance challenges. Consider consolidating the slider code into a single, cohesive implementation to improve code maintainability and readability.
You could choose one of the implementations that best fits your requirements and remove the redundant code. Ensure that all necessary features are retained in the consolidated version.
Also applies to: 220-255
<form id="newsletter-form"> | ||
<input type="email" placeholder="Your Email" required /> | ||
<button type="submit">Subscribe</button> | ||
</form> |
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
Implement functionality for newsletter subscription
The newsletter subscription form in the footer doesn't have any JavaScript code handling its submission. Consider adding an event listener to process the form data and provide feedback to the user upon subscription.
Would you like assistance in implementing the newsletter subscription functionality? I can help you write the JavaScript code to handle the form submission and provide user feedback.
class="bin" | ||
id="recycle-bin" | ||
ondrop="drop(event)" | ||
ondragover="allowDrop(event)" | ||
> | ||
<img src="./assets/rbin.jpg" alt="Recycle Bin" /> | ||
<p>Recyclable</p> | ||
</div> | ||
<div | ||
class="bin" | ||
id="organic-bin" | ||
ondrop="drop(event)" | ||
ondragover="allowDrop(event)" | ||
> | ||
<img src="./assets/organic.jpg" alt="Organic Bin" /> | ||
<p>Organic</p> | ||
</div> | ||
<div | ||
class="bin" | ||
id="landfill-bin" | ||
ondrop="drop(event)" | ||
ondragover="allowDrop(event)" | ||
> | ||
<img src="./assets/landfill.avif" alt="Landfill Bin" /> | ||
<p>Landfill</p> | ||
</div> | ||
</div> | ||
|
||
<div class="waste-container mb-4"> | ||
<div | ||
class="waste-item" | ||
draggable="true" | ||
ondragstart="drag(event)" | ||
id="item1" |
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
Refactor inline event handlers to use addEventListener
Using inline event handlers like ondrop
, ondragover
, and ondragstart
can make the HTML cluttered and less maintainable. Consider attaching event listeners via JavaScript to improve separation of concerns and code readability.
Example refactoring:
In your JavaScript code:
// Add event listeners for bins
document.querySelectorAll('.bin').forEach(bin => {
bin.addEventListener('drop', drop);
bin.addEventListener('dragover', allowDrop);
});
// Add event listeners for waste items
document.querySelectorAll('.waste-item').forEach(item => {
item.addEventListener('dragstart', drag);
});
In your HTML, remove the inline event handlers:
-<div class="bin" id="recycle-bin" ondrop="drop(event)" ondragover="allowDrop(event)">
+<div class="bin" id="recycle-bin">
Repeat for other bins and waste items.
Also applies to: 388-408
wasteChart = createChart(selectedType); | ||
} | ||
|
||
// Fetch user wasteData from localStorage | ||
const fetchBtn = document.getElementById("fetch"); | ||
fetchBtn.addEventListener("click", async () => { | ||
await fetchFromLocalStorage(); | ||
}); | ||
|
||
function fetchFromLocalStorage() { | ||
const retrieveData = | ||
JSON.parse(localStorage.getItem("userData")) || null; | ||
if (retrieveData) { | ||
wasteData = retrieveData; | ||
|
||
wasteChart.data.datasets[0].data = [ | ||
wasteData.plastic, | ||
wasteData.organic, | ||
wasteData.paper, | ||
wasteData.glass, | ||
]; | ||
wasteChart.update(); | ||
} else { | ||
document.getElementById( | ||
"error" | ||
).innerHTML = `No Data Found! Kindly input your data`; | ||
} | ||
} | ||
|
||
// Clear history | ||
const clearBtn = document.getElementById("clear"); | ||
clearBtn.addEventListener("click", () => { | ||
localStorage.removeItem("userData"); | ||
location.reload(); | ||
}); | ||
</script> | ||
|
||
<script> | ||
//dashboard | ||
function updateDashboard() { | ||
const wasteGenerated = Math.floor(Math.random() * 100); | ||
const wasteRecycled = Math.floor(Math.random() * 50); | ||
const binCapacity = Math.floor(Math.random() * 100); | ||
|
||
document.getElementById( | ||
"wasteGenerated" | ||
).innerText = `${wasteGenerated}%`; | ||
document.getElementById( | ||
"wasteProgress" | ||
).style.width = `${wasteGenerated}%`; | ||
document.getElementById( | ||
"wasteProgress" | ||
).innerText = `${wasteGenerated}%`; | ||
|
||
document.getElementById( | ||
"wasteRecycled" | ||
).innerText = `${wasteRecycled}%`; | ||
document.getElementById( | ||
"recycleProgress" | ||
).style.width = `${wasteRecycled}%`; | ||
document.getElementById( | ||
"recycleProgress" | ||
).innerText = `${wasteRecycled}%`; | ||
|
||
document.getElementById("binCapacity").innerText = `${binCapacity}%`; | ||
document.getElementById( | ||
"capacityProgress" | ||
).style.width = `${binCapacity}%`; | ||
document.getElementById( | ||
"capacityProgress" | ||
).innerText = `${binCapacity}%`; | ||
} | ||
|
||
setInterval(updateDashboard, 5000); | ||
</script> | ||
|
||
<script> | ||
function allowDrop(ev) { | ||
ev.preventDefault(); | ||
} | ||
|
||
function drag(ev) { | ||
ev.dataTransfer.setData("text/plain", ev.target.id); | ||
} | ||
|
||
function drop(ev) { | ||
ev.preventDefault(); | ||
// Get the dragged item ID | ||
let data = ev.dataTransfer.getData("text/plain"); | ||
|
||
// Ensure we append to the drop target's parent (bin), not internal elements like images | ||
let draggedElement = document.getElementById(data); | ||
if(ev.target.classList.contains('bin')){ | ||
ev.currentTarget.appendChild(draggedElement); | ||
} else { | ||
ev.currentTarget.closest('.bin').appendChild(draggedElement); | ||
} | ||
} | ||
</script> | ||
</body> |
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
Consider moving JavaScript code to external files
Embedding large amounts of JavaScript within HTML can make the file harder to maintain. Consider moving your JavaScript code into external .js
files and linking them in your HTML. This improves code organization and allows for better caching by browsers.
width: 300%; | ||
} |
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
Fixed width of .slider-wrapper
may affect responsiveness
The .slider-wrapper
class has a width
of 300%
set in lines 986-987. This fixed width might cause layout issues on different screen sizes and doesn't adapt well to responsive designs.
Consider refactoring the slider to use flexbox or CSS grid to create a responsive layout without relying on widths greater than 100%
. This will enhance the slider's adaptability to various screen sizes.
width: 300%; | ||
} | ||
|
||
/* Individual Feature Card */ | ||
.feature-card { | ||
flex: 1 0 100%; | ||
padding: 2rem; | ||
text-align: center; | ||
background-color: #EDF6F9; | ||
box-shadow: 0 4px 10px rgba(0, 0, 0, 0.1); | ||
border-radius: 8px; | ||
margin: 2rem; | ||
transition: background-color 0.3s ease; | ||
} | ||
|
||
/* Feature card specific styles */ | ||
.feature-card h3 { | ||
font-size: 2rem; | ||
margin-bottom: 1rem; | ||
color: #006D77; | ||
} | ||
|
||
.feature-card p, blockquote { | ||
font-size: 1.1rem; | ||
color: #555; | ||
line-height: 1.6; | ||
} | ||
|
||
blockquote { | ||
background-color: #C8E6C9; | ||
padding: 1rem; | ||
border-left: 4px solid #43A047; | ||
border-radius: 5px; | ||
margin: 1.5rem 0; | ||
} | ||
|
||
#fact-highlight { | ||
font-weight: bold; | ||
color: #E53935; | ||
} | ||
|
||
.highlight { | ||
color: #2e7d32; | ||
font-weight: bold; | ||
} | ||
|
||
/* Call-to-action button */ | ||
.cta-button { | ||
display: inline-block; | ||
padding: 10px 20px; | ||
background-color: #4CAF50; | ||
color: white; | ||
text-decoration: none; | ||
border-radius: 5px; | ||
font-size: 1.1rem; | ||
transition: background-color 0.3s ease; | ||
} | ||
|
||
.cta-button:hover { | ||
background-image: linear-gradient(to right, #006D77, #EDF6F9); | ||
} | ||
|
||
/* Slider navigation buttons */ | ||
.slider-btn { | ||
position: absolute; | ||
top: 50%; | ||
transform: translateY(-50%); | ||
background-color: rgba(0, 0, 0, 0.5); | ||
color: white; | ||
border: none; | ||
padding: 1rem; | ||
cursor: pointer; | ||
z-index: 10; | ||
font-size: 1.5rem; | ||
} | ||
|
||
.slider-btn.left { | ||
left: 10px; | ||
} | ||
|
||
.slider-btn.right { | ||
right: 10px; | ||
} | ||
|
||
/* Responsive design */ | ||
@media (max-width: 768px) { | ||
.feature-card { | ||
padding: 1rem; | ||
margin: 1rem; | ||
} | ||
|
||
.cta-button { | ||
padding: 8px 16px; | ||
font-size: 1rem; | ||
} | ||
|
||
h3 { | ||
font-size: 1.5rem; | ||
} | ||
} |
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
Consider modularizing CSS for better maintainability
The stylesheet has grown significantly, especially with the addition of the slider component between lines 973-1085. For improved maintainability, consider splitting the CSS into separate files for different components or sections, such as slider.css
, dark-mode.css
, etc.
Organizing styles into modular files can make it easier to manage and update specific components without affecting the entire stylesheet.
styles.css
Outdated
.dark-mode header { | ||
background-color: #1f1f1f; | ||
/* Darker header */ | ||
} | ||
|
||
.dark-mode .hero { | ||
background: linear-gradient(rgba(0, 77, 64, 0.7), rgba(0, 77, 64, 0.8)), url('https://images.unsplash.com/photo-1628638428099-48fc6fdb98c2?q=80&w=1470&auto=format&fit=crop&ixlib=rb-4.0.3&ixid=M3wxMjA3fDB8MHxwaG90by1wYWdlfHx8fGVufDB8fHx8fA%3D%3D') center/cover no-repeat; | ||
} | ||
|
||
.dark-mode .feature-card { | ||
background-color: #00796b; | ||
/* Card color in dark mode */ | ||
} | ||
|
||
.dark-mode .feedback { | ||
background-color: #1f1f1f; | ||
/* Feedback section background */ | ||
} | ||
|
||
.dark-mode .feedback button { | ||
background-color: #1c1c1c; | ||
/* Dark mode button color (matches dark header) */ | ||
color: #ffffff; | ||
/* Light text color for contrast */ | ||
} | ||
|
||
#newsletter-form input:focus { | ||
outline: none; | ||
} | ||
|
||
.moon-icon { | ||
font-size: 30px; | ||
/* Adjust size of the moon icon */ | ||
margin-right: 8px; | ||
/* Match the header color */ | ||
color: white; | ||
/* Set the icon color to white for contrast */ | ||
border-radius: 50%; | ||
/* Optional: add rounded corners */ | ||
padding: 5px; | ||
/* Optional: add some padding */ | ||
} | ||
|
||
body.dark-mode { | ||
background-color: black; | ||
/* Dark background matching navbar */ | ||
color: white; | ||
/* Light text color */ | ||
} | ||
|
||
#dark-mode-toggle { | ||
background-color: #00796b; | ||
/* Match the navbar color */ | ||
color: white; | ||
/* White text for contrast */ | ||
|
||
border: none; | ||
|
||
cursor: pointer; | ||
transition: background-color 0.3s ease; | ||
} | ||
|
||
body.dark-mode #dark-mode-toggle { | ||
background-color: #1c1c1c; | ||
/* Dark mode button color */ | ||
color: white; | ||
/* Keep text white */ | ||
} |
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
Consolidate .dark-mode
styles for consistency
The .dark-mode
styles are scattered across lines 1285-1352, with some duplicate or conflicting definitions. Consolidating these styles into a unified section or grouping related styles together enhances readability and ensures consistency across the dark mode theme.
Gather all .dark-mode
styles into one section and remove any redundant or conflicting definitions.
Hey @GarimaSingh0109 , @khushi-joshi-05 One thing I would like to say--the main styles.css file is becoming bulky and inefficient due to continuous css integrations and changes, this may impact the overall code and website performance + loading. Kindly look into it and suggest guidance to contributors to only make necessary changes, while ensuring code efficiency and modularity. |
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
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: 2
🧹 Outside diff range and nitpick comments (3)
styles.css (3)
973-1085
: Feature slider implementation looks great, with a minor suggestionThe new feature slider component enhances the user interface and content presentation. The implementation includes proper responsive design considerations.
However, there's a potential issue with the fixed width of 300% for the slider wrapper (line 986). This might cause layout issues on different screen sizes.
Consider using a more flexible approach for the slider wrapper width:
.slider-wrapper { display: flex; transition: transform 0.5s ease; - width: 300%; + width: 100%; + flex-wrap: nowrap; } .feature-card { - flex: 1 0 100%; + flex: 0 0 100%; /* ... other styles ... */ }This change allows the slider to adapt to the number of cards dynamically without relying on a fixed percentage width.
1367-1419
: Impressive button styles with a suggestion for accessibilityThe new button styles significantly enhance the visual appeal and interactivity of the website. The use of CSS variables for colors is a good practice, and the creative hover effect adds a unique touch.
To improve accessibility, consider adding a
transition
property for the text color change:.button { /* ... existing styles ... */ + transition: .5s, color .3s; }
This will make the text color change smoother when hovering, improving the user experience for all users, including those with visual sensitivities.
1421-1483
: Well-implemented custom checkbox stylesThe custom checkbox styles significantly enhance the visual appeal of the form elements. The use of SVG icons for different states is a good approach for scalability, and the implementation is thorough.
To improve usability on touch devices, consider increasing the touch target size:
.checkbox-con input[type="checkbox"] { /* ... existing styles ... */ + min-width: 48px; + min-height: 48px; } .checkbox-con label { /* ... existing styles ... */ + padding: 12px 0; }This change ensures that the checkbox and its label are easier to interact with on touch devices, improving the overall user experience.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- styles.css (4 hunks)
🧰 Additional context used
🔇 Additional comments (4)
styles.css (4)
128-179
: Responsive design improvements look good!The added media queries for screen widths of 768px and 480px enhance the website's responsiveness. The adjustments to font sizes, padding, and layout are appropriate for different screen sizes.
139-154
: Navigation styles updatedThe changes to the navigation list styles improve the layout. However, the link color inconsistency has already been addressed in a previous review.
1221-1243
: "Back to Top" button implementationThe "Back to Top" button is well-implemented with proper positioning and styling. However, the visibility issue has already been addressed in a previous review.
Line range hint
1-1489
: Overall assessment: Good improvements with some areas for refinementThe changes to
styles.css
significantly enhance the website's functionality, responsiveness, and visual appeal. Key improvements include:
- Responsive design through media queries
- Implementation of a feature slider
- Addition of dark mode styles
- Custom button and checkbox styles
Main areas for improvement:
- Consolidate and ensure consistency in dark mode styles
- Refine the feature slider wrapper for better responsiveness
- Minor adjustments to button and checkbox styles for improved accessibility and user experience
These refinements will further enhance the overall quality and maintainability of the stylesheet.
.dark-mode header { | ||
background-color: #1f1f1f; | ||
/* Darker header */ | ||
} | ||
|
||
.dark-mode .hero { | ||
background: linear-gradient(rgba(0, 77, 64, 0.7), rgba(0, 77, 64, 0.8)), url('https://images.unsplash.com/photo-1628638428099-48fc6fdb98c2?q=80&w=1470&auto=format&fit=crop&ixlib=rb-4.0.3&ixid=M3wxMjA3fDB8MHxwaG90by1wYWdlfHx8fGVufDB8fHx8fA%3D%3D') center/cover no-repeat; | ||
} | ||
|
||
.dark-mode .feature-card { | ||
background-color: #00796b; | ||
/* Card color in dark mode */ | ||
} | ||
|
||
.dark-mode .feedback { | ||
background-color: #1f1f1f; | ||
/* Feedback section background */ | ||
} | ||
|
||
.dark-mode .feedback button { | ||
background-color: #1c1c1c; | ||
/* Dark mode button color (matches dark header) */ | ||
color: #ffffff; | ||
/* Light text color for contrast */ | ||
} | ||
|
||
#newsletter-form input:focus { | ||
outline: none; | ||
} | ||
|
||
.moon-icon { | ||
font-size: 30px; | ||
/* Adjust size of the moon icon */ | ||
margin-right: 8px; | ||
/* Match the header color */ | ||
color: white; | ||
/* Set the icon color to white for contrast */ | ||
border-radius: 50%; | ||
/* Optional: add rounded corners */ | ||
padding: 5px; | ||
/* Optional: add some padding */ | ||
} | ||
|
||
body.dark-mode { | ||
background-color: #1f1f1f; | ||
/* Consistent dark background */ | ||
color: white; | ||
/* Light text color */ | ||
} |
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
Consolidate dark mode styles and address inconsistencies
The addition of dark mode styles enhances the user experience. However, there are some issues to address:
- Dark mode styles are scattered across the file, which might lead to maintenance difficulties.
- There are inconsistencies in the background colors used for dark mode.
Consider consolidating all dark mode styles into a single section for better maintainability. Also, ensure consistent use of background colors in dark mode.
Example of consolidation and consistency:
/* Dark Mode Styles */
body.dark-mode {
background-color: #1f1f1f;
color: white;
}
.dark-mode header,
.dark-mode .feedback button,
.dark-mode #dark-mode-toggle {
background-color: #1c1c1c;
}
.dark-mode .feature-card {
background-color: #00796b;
}
/* ... other dark mode styles ... */
This approach improves code organization and ensures consistency across dark mode styles.
styles.css
Outdated
|
||
.btn-nav #checkbox:hover { | ||
transform: scale(1.7); | ||
} No newline at end of file | ||
} | ||
|
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
Adjust checkbox hover effect for better user experience
The addition of a hover effect to the checkbox improves interactivity. However, the current scale value of 1.7 might be too large and could potentially cause layout issues.
Consider using a more subtle scale effect:
.btn-nav #checkbox:hover {
- transform: scale(1.7);
+ transform: scale(1.1);
}
This change will still provide a noticeable hover effect without risking layout disruptions or overlapping with nearby elements.
📝 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.
.btn-nav #checkbox:hover { | |
transform: scale(1.7); | |
} | |
\ No newline at end of file | |
} | |
.btn-nav #checkbox:hover { | |
transform: scale(1.1); | |
} |
❌ Deploy Preview for manageyourwaste failed. Why did it fail? →
|
Issues Identification
Closes: #166
Description
Summary
Added RTM feature for users to input their waste data and track their waste disposals.
Details
Here is illustrated preview video of the feature:-
Waste.Management.-.RTM.mp4
Types of Changes
Please check the boxes that apply
Checklist
Please check the boxes that apply
Summary by CodeRabbit
New Features
Bug Fixes
Style