Feat: Updated the complete webpage of the extension#34
Conversation
WalkthroughReplaces site styling with a dark-themed design system, updates HTML to use local assets and a theme toggle, and adds JS for theme persistence, hero-card parallax, IntersectionObserver reveals, GSAP-enhanced CTA animations, and a fullscreen infinite-loop carousel. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant D as Document
participant CSS as style.css
participant JS as main.js
participant NAV as Navbar
participant HERO as Hero Card
participant CAR as Carousel
participant OBS as IntersectionObserver
participant LOT as Lottie
U->>D: Load page
D-->>CSS: Apply design tokens & animations
D-->>JS: DOMContentLoaded → init
JS->>NAV: Attach scroll listener (toggle .scrolled)
JS->>HERO: Bind mousemove/leave parallax + glow
JS->>LOT: Init Lottie array
JS->>CAR: Init infinite carousel (clone slides, autoplay)
JS->>OBS: Observe sections → trigger fadeUp reveals
U->>NAV: Scroll
NAV-->>D: Toggle scrolled styles
U->>HERO: Mousemove/leave
HERO-->>U: 3D tilt & glow motion
U->>CAR: Hover/Swipe/Keys
CAR-->>U: Looping slides, dots update
sequenceDiagram
autonumber
participant U as User
participant BTN as CTA Button
participant JS as main.js
participant GSAP as GSAP/Physics2D
U->>BTN: Click
JS->>GSAP: Attempt register Physics2DPlugin
alt Plugin available
GSAP-->>JS: Registered
JS->>BTN: Run particle/physics animation
else Fallback
JS->>BTN: Run simplified GSAP timeline
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
|
Hi @Harshdev625, I have updated the current Web Page for the extension, Hope you'll like it, Plz review :) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (5)
docs/index.html (2)
28-28: Consider consolidating font imports.The page now imports both Inter (line 27) and Poppins (line 28) from Google Fonts. If Poppins is the primary font for the redesign, consider removing the Inter import to reduce the number of external requests and improve page load performance.
117-117: Section title change is less descriptive.The section title was changed from "Everything You Need for Digital Productivity" to "Your Digital Productivity". While more concise, the new title is less informative about what features are being presented. Consider a middle-ground option like "Your Complete Digital Productivity Suite" or "Everything for Your Digital Productivity".
docs/js/main.js (3)
480-485: Physics2DPlugin fallback is incomplete.The code checks for
Physics2DPluginand logs a warning if unavailable, but doesn't prevent the subsequent code from attempting to use it. The fallback logic in theparticlesfunction (lines 569-582) handles this correctly, making the warning misleading.Either remove the warning since a proper fallback exists, or make it more informative:
if (window.gsap && window.Physics2DPlugin) { gsap.registerPlugin(Physics2DPlugin); -} else if (window.gsap) { - // Fallback: If Physics2DPlugin is not available, create a simple fallback - console.warn('Physics2DPlugin not available, using fallback animation'); }The
particlesfunction already has complete fallback logic at lines 574-581.
487-584: Button animation code is complex and may impact performance.The button animation code initializes elaborate GSAP timelines with particles for every
.btnelement on the page. This includes:
- Complex path calculations and proxy objects
- Repeating animations on hover and page load
- Particle generation with physics simulation
With multiple buttons on the page, this could impact performance, especially on lower-end devices.
Consider:
- Debouncing the mouseover event to prevent rapid animation restarts
- Limiting particle count or simplifying the animation for mobile devices
- Using
will-change: transformhints for animated elements- Adding
requestAnimationFramethrottling for performance-critical sectionsExample throttling:
let isAnimating = false; button.addEventListener('mouseover', () => { if (isAnimating) return; isAnimating = true; tl.restart(); interval = setInterval(() => tl.restart(), 1500); setTimeout(() => isAnimating = false, 100); });
569-573: Missing null safety check for gsap.plugins.The code checks
window.Physics2DPluginandgsap.plugins.physics2D, but doesn't verify thatgsap.pluginsexists before accessing it. While unlikely to fail, this could cause an error if GSAP is in an unexpected state.-if (window.Physics2DPlugin && gsap.plugins.physics2D) { +if (window.Physics2DPlugin && gsap.plugins?.physics2D) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
docs/css/style.css(1 hunks)docs/index.html(4 hunks)docs/js/main.js(10 hunks)
🔇 Additional comments (1)
docs/css/style.css (1)
333-346: Verify shine animation positioning across button sizes.The
.btn-primary:beforeshine effect uses absolute positioning with percentage-based coordinates that may not scale well across different button sizes or text lengths. The-40%top and-60%left values could cause clipping or misalignment.Test the shine animation with buttons of varying widths (short vs. long text) to ensure the effect remains visible and properly positioned throughout the animation.
| /* .navbar.scrolled .logo { | ||
| width: 28px; | ||
| height: 28px; | ||
| } */ |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove commented-out code.
The commented logo sizing rule serves no purpose and clutters the stylesheet.
-/* .navbar.scrolled .logo {
- width: 28px;
- height: 28px;
-} */📝 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.
| /* .navbar.scrolled .logo { | |
| width: 28px; | |
| height: 28px; | |
| } */ |
🤖 Prompt for AI Agents
In docs/css/style.css around lines 145 to 148, there is a commented-out
.navbar.scrolled .logo rule that is unused and clutters the stylesheet; remove
the entire commented block so only active CSS remains, then run a quick
lint/format to ensure no leftover whitespace or formatting issues.
Docstrings generation was requested by @Harshdev625. * #34 (comment) The following files were modified: * `docs/js/main.js`
|
Note Generated docstrings for this pull request at #35 |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
docs/js/main.js (2)
114-114: Remove unused hero element reference.The
herovariable is queried but never used in the code. This appears to be leftover from when the parallax effect existed (mentioned in past reviews but now removed).- const hero = document.querySelector('.hero');
373-432: Consider moving dynamic styles to CSS file.The style block is injected at runtime, which works but adds to the initial JavaScript execution time. These styles could be moved to
style.cssfor better separation of concerns and performance.Move these styles to
docs/css/style.css:.navbar.scrolled { background: rgba(255, 255, 255, 0.98); box-shadow: 0 2px 10px rgba(0, 0, 0, 0.1); } .nav-links.active { display: flex; position: absolute; top: 100%; left: 0; right: 0; background: white; flex-direction: column; padding: 1rem; box-shadow: 0 4px 6px rgba(0, 0, 0, 0.1); } /* ... rest of the styles ... */Then remove lines 373-432 from the JS file.
docs/css/style.css (2)
1-1: Consider preconnect for Google Fonts.The Google Fonts import at the top works, but adding a
<link rel="preconnect">in the HTML head would improve loading performance.Add to
docs/index.htmlbefore the stylesheet link:<link rel="preconnect" href="https://fonts.googleapis.com"> <link rel="preconnect" href="https://fonts.gstatic.com" crossorigin>This allows the browser to establish connections earlier in the page load.
99-116: Document or simplify navbar width. The.navbarrule useswidth: 110%; max-width: 100%, which is redundant and may cause overflow. If the wider width is intentional (e.g., a visual effect), add a comment explaining why; otherwise, simplify to:.navbar { - width: 110%; - max-width: 100%; + width: 100%; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
docs/css/style.css(1 hunks)docs/index.html(4 hunks)docs/js/main.js(10 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/index.html
🔇 Additional comments (9)
docs/js/main.js (4)
1-37: LGTM! Critical race condition fixed.The hero card parallax code is now properly wrapped inside the
DOMContentLoadedevent listener with null checks, addressing the critical race condition from the previous review.
140-318: LGTM! Well-implemented infinite carousel.The carousel implementation correctly handles infinite looping by cloning slides, adjusting dimensions (700% width for 7 slides, 14.2857% per slide), and seamlessly jumping between clones and real slides. The keyboard navigation, touch support, and autoplay with hover-pause are good UX additions.
434-440: Good fallback for missing GSAP plugin.The conditional registration with a clear warning message when Physics2DPlugin is unavailable is a good practice. The fallback animation logic at lines 524-536 ensures functionality even without the plugin.
442-539: Well-structured button animation with robust fallback.The GSAP-based button animation is complex but well-organized:
- Uses a Proxy for reactive SVG path updates
- Includes proper fallback when Physics2DPlugin is missing
- Particle effects gracefully degrade to simple easing
The code is production-ready with good error handling.
docs/css/style.css (5)
8-76: LGTM! CSS variables successfully consolidated.All custom properties are now in a single
:rootblock, addressing the previous maintenance concern about duplicate declarations. The organization is logical with clear groupings (colors, typography, spacing, etc.).
242-253: LGTM! Transform restriction removed.The
.heroclass no longer usestransform: none !important;, which was blocking JavaScript-driven parallax effects. This resolves the conflict mentioned in previous reviews.
703-719: LGTM! Responsive carousel navigation positioning fixed.The carousel navigation now uses
var(--space-6)instead of hard-codedcalc(50% - 560px), making it properly responsive across all screen sizes. This addresses the previous breakpoint-specific positioning issue.
304-417: Well-crafted primary and secondary CTA buttons.The button implementations showcase excellent attention to detail:
- Animated gradient backgrounds with smooth transitions
- Shine/glow effects using pseudo-elements
- Pulsing animation for visual interest
- Proper hover states with scale transforms
- Good separation between primary and secondary styles
The CSS is production-ready and follows modern best practices.
431-499: Impressive 3D hero card with animation.The hero card implementation demonstrates strong CSS skills:
- Proper use of
perspectiveandtransform-style: preserve-3d- Smooth slide-in animation with blur and brightness transitions
- Glow effect using radial gradients with proper z-indexing
- Will-change properties for performance optimization
The animation sequence (0% → 60% overshoot → 100% settle) creates a polished, professional feel.
|
Hi @Harshdev625 , I noticed that my previous pull request got a bunch of reviews regarding the code quality. I have now updated the code as per the reviews. Please let me know if any further improvements are needed. Also, if it seems good, can you plz add "Hacktoberfest-accepted" label to this and merge :) Thank you |
Hi @harshitrwt 👋, great work on this PR! Before we merge, please make the following changes: Add full light and dark mode support for the webpage. Provide a visible theme toggle and respect the user's system preference (prefers-color-scheme) where possible. The dark mode looks good overall, but check the attached image — the “Rating: 5.0★ (4 reviews)” section isn’t very appealing. Improve its styling (contrast, padding, font weight, or background) so it stands out. The footer blends into the dark background. Use a lighter black/grey shade, a subtle border, or a divider so it’s visibly separated from the main content. The TimeTracker icon isn’t very visible. Add a white/alternate icon or adjust its styling (e.g., background circle, increased contrast) to improve visibility. Make sure the download button exactly matches the design shown in the attached image (size, border-radius, padding, hover state). The layout is currently not fully responsive on all devices. Either fix responsiveness in this commit or create a new issue and address it in the next PR — I’ll be happy to assign it. Thanks again for the contribution — I’ll review once these changes are pushed! 🙌 |
|
Hi @Harshdev625 Thanks for the review, sure I will work on the recommended changes. What do you think about this approach? |
|
Yes, I agree with your approach. Creating a separate issue for responsiveness makes sense it allows us to focus on the top 5 issues and ensure the desktop version is fully polished first. Once that’s done, we can address responsiveness in the next PR. Thanks for suggesting this! |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
docs/js/main.js (1)
393-452: Consider moving dynamic styles to CSS file.The dynamic style injection works but mixing CSS definitions in JavaScript reduces maintainability. These styles are static and could be moved to
docs/css/style.cssfor better separation of concerns.Consider moving the style definitions to the CSS file:
- Move the
.navbar.scrolled,.nav-links.active,.mobile-menu-btn.activestyles, and other classes tostyle.css- Remove the dynamic style injection from JavaScript
This improves maintainability and allows the browser to parse styles earlier during page load.
docs/index.html (1)
92-109: Simplify hero stats markup for better semantics.The hero stats structure is now valid HTML, but the aria-label placement is semantically awkward. The aria-label should describe the stat-number, not wrap the visible label.
Apply this diff for cleaner semantics:
<div class="stat"> - <span class="stat-number">👥 10+ </span> - <span aria-label="active users"> - <span class="stat-label">Active Users</span> - </span> + <span class="stat-number" aria-label="10+ active users">👥 10+</span> + <span class="stat-label">Active Users</span> </div> <div class="stat"> - <span class="stat-number">⭐ 5.0★ </span> - <span aria-label="chrome star rating"> - <span class="stat-label">Chrome Store Rating</span> - </span> + <span class="stat-number" aria-label="5.0 star chrome store rating">⭐ 5.0★</span> + <span class="stat-label">Chrome Store Rating</span> </div> <div class="stat"> - <span class="stat-number">✨ 178KB </span> - <span aria-label="lightweight"> - <span class="stat-label">Lightweight</span> - </span> + <span class="stat-number" aria-label="178KB lightweight">✨ 178KB</span> + <span class="stat-label">Lightweight</span> </div>docs/css/style.css (1)
78-90: Remove unnecessary duplicate properties for theme system.The CSS contains duplicate property declarations where a static color is set first, then immediately overridden with a CSS variable. This pattern repeats throughout the file (lines 87, 263, 305, 553, 591, 660, 674, 1010, 1052, 1087) and is flagged by static analysis.
Since CSS variables are always defined in
:root, the initial static values serve no purpose as fallbacks and only add confusion.Apply this pattern throughout the file - remove the first static declaration:
body { font-family: var(--font-primary); font-size: var(--font-size-base); line-height: 1.6; - color: var(--gray-800); - background-color: var(--white); scroll-behavior: smooth; --bg-color: black; --text-color: white; background-color: var(--bg-color); color: var(--text-color); transition: background-color 0.3s, color 0.3s; }Repeat this cleanup for all sections with duplicate properties.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
docs/assets/logo.pngis excluded by!**/*.png
📒 Files selected for processing (3)
docs/css/style.css(1 hunks)docs/index.html(7 hunks)docs/js/main.js(10 hunks)
🧰 Additional context used
🪛 Biome (2.1.2)
docs/css/style.css
[error] 87-87: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
background-color is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
[error] 263-263: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
color is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
[error] 305-305: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
color is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
[error] 553-553: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
color is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
[error] 591-591: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
color is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
[error] 660-660: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
color is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
[error] 674-674: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
color is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
[error] 1010-1010: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
color is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
[error] 1052-1052: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
color is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
[error] 1087-1087: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
color is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
🔇 Additional comments (8)
docs/js/main.js (3)
32-57: LGTM!The hero card parallax effect is properly implemented with null checks and event listeners inside
DOMContentLoaded. The previous race condition issue has been correctly addressed.
231-270: Consider edge case: rapid navigation during animation.The
isAnimatingflag prevents concurrent animations, but if a user rapidly clicks navigation buttons or dots during the transition timeout (line 255-267), the flag is set for 600ms. This could feel unresponsive if users try to navigate quickly.Consider reducing the lock duration or making it more granular:
updateCarousel(skipTransition = false) { if (this.isAnimating || !this.container) return; this.isAnimating = true; const slideWidth = 14.2857; // 100% / 7 slides const translateX = -this.currentSlide * slideWidth; // Apply or skip transition if (skipTransition) { this.container.style.transition = 'none'; + this.isAnimating = false; } else { this.container.style.transition = 'transform 0.6s cubic-bezier(0.25, 1, 0.5, 1)'; }This allows instant jumps (when
skipTransitionis true) to complete immediately, reducing perceived lag.
455-460: LGTM!The GSAP plugin registration properly handles the missing Physics2DPlugin case with a clear warning and fallback logic. This ensures the animation degrades gracefully if the plugin isn't loaded.
docs/index.html (2)
48-56: LGTM!The theme toggle button is properly implemented with:
- Accessible
aria-labelattribute- SVG with
aria-hidden="true"to avoid duplicate announcements- Clear visual icon that matches the JS implementation
377-377: Verify rating section styling meets requirements.The reviewer specifically requested improvements to make the rating section stand out more (contrast, padding, font weight, or background). The current implementation uses
info-value-1class which appears identical toinfo-valuein the CSS.Confirm that the rating styling adequately stands out as requested, or enhance it further:
<div class="info-item"> <span class="info-label">Rating:</span> - <span class="info-value-1">5⭐ (4 reviews)</span> + <span class="info-value-1" style="font-weight: 800; color: #FFD700;">5⭐ (4 reviews)</span> </div>Or add specific CSS rules for
.info-value-1to make it more prominent:.info-value-1 { font-weight: 800; font-size: 1rem; color: #FFD700; text-shadow: 0 0 10px rgba(255, 215, 0, 0.5); }docs/css/style.css (3)
8-76: LGTM!The CSS custom properties are well-organized in a single
:rootblock with clear categorization (Colors, Typography, Spacing, Shadows, Border Radius, Transitions, buttons). The previous review's concern about duplicate:rootblocks has been properly addressed.
92-95: Verify light theme completeness.The light theme implementation only swaps background and text colors. Per the reviewer's requirement for "full light and dark mode support," verify that all UI elements are properly styled in light mode:
- Feature cards may need adjusted backgrounds/borders
- Buttons and CTAs may need color adjustments
- Navbar, hero, screenshots sections may need refinement
Test the light theme by adding the
light-themeclass to the body and check if:
- All text remains readable with adequate contrast
- Card backgrounds and borders are appropriately styled
- Buttons and interactive elements remain visually distinct
- The overall aesthetic remains cohesive
If issues are found, consider adding more comprehensive light theme overrides:
body.light-theme { --bg-color: white; --text-color: black; /* Additional overrides for light mode */ .feature-card { background: rgba(240, 242, 247, 0.85); border-color: rgba(0, 0, 0, 0.1); } .carousel-nav { background: linear-gradient(135deg, #4facfe 0%, #00f2fe 100%); } /* Add more overrides as needed */ }
1210-1228: Verify footer separation in dark mode meets requirements.The reviewer requested that the footer be "visually separate in dark mode (lighter shade, subtle border, or divider)." The current implementation uses
rgb(25, 23, 23)which is only slightly lighter than the black background. This subtle difference may not provide adequate visual separation.Test if the footer stands out enough in dark mode. If not, consider enhancing separation:
.footer { padding: var(--space-16) 0 var(--space-8); - background: rgb(25, 23, 23); + background: rgb(35, 35, 40); color: var(--gray-300); position: relative; overflow: hidden; + border-top: 1px solid rgba(79, 172, 254, 0.2); }Or add a gradient transition:
.footer { background: linear-gradient(to bottom, black, rgb(30, 30, 35)); border-top: 1px solid rgba(255, 255, 255, 0.05); }
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
docs/js/main.js (4)
88-99: Guard against missing Lottie to avoid ReferenceErrorIf the lottie script isn’t loaded, this throws at page load. Add a safe check.
- lottieAnimations.forEach(a => { + lottieAnimations.forEach(a => { const el = document.getElementById(a.id); - if (el) { - lottie.loadAnimation({ + if (el && window.lottie && typeof window.lottie.loadAnimation === 'function') { + lottie.loadAnimation({ container: el, renderer: 'svg', loop: true, autoplay: true, path: a.path }); + } else if (el) { + console.warn('Lottie not available; skipping animation init for', a.id); } });
103-116: Navbar may be null; also set initial scrolled statePrevent null deref and initialize state on load.
- function updateNavbar() { - if (window.scrollY > 50) { - navbar.classList.add('scrolled'); - } else { - navbar.classList.remove('scrolled'); - } - } + function updateNavbar() { + if (!navbar) return; + if (window.scrollY > 50) { + navbar.classList.add('scrolled'); + } else { + navbar.classList.remove('scrolled'); + } + } window.addEventListener('scroll', updateNavbar); + // Initialize once + updateNavbar();
160-172: Hard-coded slide count/width; compute dynamically to avoid breakageUsing fixed 5 slides and 14.2857% breaks when content changes or slides differ. Compute from DOM and handle 0/1 slides.
- totalSlides: 5, + totalSlides: 0, ... init() { if (!this.container) return; + this.totalSlides = this.slides.length; + if (this.totalSlides < 1) return; this.setupInfiniteLoop(); this.bindEvents(); this.updateCarousel(); this.startAutoplay(); }, ... setupInfiniteLoop() { // Clone first and last slides for seamless infinite loop const firstSlide = this.slides[0].cloneNode(true); const lastSlide = this.slides[this.totalSlides - 1].cloneNode(true); // Add cloned slides this.container.appendChild(firstSlide); // Add first slide to end this.container.insertBefore(lastSlide, this.slides[0]); // Add last slide to beginning - // Update container width for new slides (7 slides now: clone, 1, 2, 3, 4, 5, clone) - this.container.style.width = '700%'; // 7 slides + // Update container width for new slides (real + 2 clones) + const total = this.totalSlides + 2; + this.container.style.width = `${total * 100}%`; // Update slide width const allSlides = this.container.querySelectorAll('.carousel-slide'); - allSlides.forEach(slide => { - slide.style.width = '14.2857%'; // 100% / 7 slides - }); + const widthPercent = 100 / total; + allSlides.forEach(slide => { slide.style.width = `${widthPercent}%`; }); // Start at the real first slide (index 1, since we added a clone at index 0) - this.currentSlide = 1; - this.container.style.transform = `translateX(-${this.currentSlide * 14.2857}%)`; - this.container.style.transition = 'none'; // No transition for initial positioning + this.container.style.transition = 'none'; // No transition for initial positioning + this.currentSlide = 1; + this.container.style.transform = `translateX(-${this.currentSlide * (100 / total)}%)`; }, ... - updateCarousel(skipTransition = false) { + updateCarousel(skipTransition = false) { if (this.isAnimating || !this.container) return; this.isAnimating = true; - const slideWidth = 14.2857; // 100% / 7 slides + const slideWidth = 100 / (this.totalSlides + 2); const translateX = -this.currentSlide * slideWidth; // Apply or skip transition if (skipTransition) { this.container.style.transition = 'none'; } else { this.container.style.transition = 'transform 0.6s cubic-bezier(0.25, 1, 0.5, 1)'; } this.container.style.transform = `translateX(${translateX}%)`;Also applies to: 182-205, 231-246
341-366: Null-safe scroll buttons (prevents TypeError when missing)Guard element access and listeners.
- const handleScrollButtons = () => { + const handleScrollButtons = () => { const scrollHeight = document.documentElement.scrollHeight; const clientHeight = document.documentElement.clientHeight; const scrollPosition = window.scrollY; // --- Scroll to Top Button Logic --- // Show the button if user has scrolled down more than 300px - if (scrollPosition > 300) { - scrollTopBtn.classList.add('visible'); - } else { - scrollTopBtn.classList.remove('visible'); - } + if (scrollTopBtn) { + if (scrollPosition > 300) scrollTopBtn.classList.add('visible'); + else scrollTopBtn.classList.remove('visible'); + } // --- Scroll to Bottom Button Logic --- // Show the button if user is near the top, but hide it when they get close to the bottom const isNearBottom = scrollPosition + clientHeight >= scrollHeight - 300; - if (scrollPosition > 100 && !isNearBottom) { - scrollBottomBtn.classList.add('visible'); - } else { - scrollBottomBtn.classList.remove('visible'); - } + if (scrollBottomBtn) { + if (scrollPosition > 100 && !isNearBottom) scrollBottomBtn.classList.add('visible'); + else scrollBottomBtn.classList.remove('visible'); + } }; // --- Click Event Listeners --- // Smoothly scroll to the top of the page - scrollTopBtn.addEventListener('click', () => { + scrollTopBtn?.addEventListener('click', () => { window.scrollTo({ top: 0, behavior: 'smooth' }); }); // Smoothly scroll to the bottom of the page - scrollBottomBtn.addEventListener('click', () => { + scrollBottomBtn?.addEventListener('click', () => { window.scrollTo({ top: document.body.scrollHeight, behavior: 'smooth' }); }); // Add a scroll event listener to the window to check button visibility window.addEventListener('scroll', handleScrollButtons); // Initial check when the page loads handleScrollButtons();Also applies to: 371-385, 387-391
🧹 Nitpick comments (6)
docs/js/main.js (6)
164-165: Scope carousel dots and avoid hijacking arrow keys in inputsLimit dot selection to the carousel container and ignore key events while typing.
- dots: document.querySelectorAll('.dot'), + dots: document.querySelectorAll('.screenshot-carousel .dot'), ... - document.addEventListener('keydown', (e) => { + document.addEventListener('keydown', (e) => { + const t = e.target; + if ((t && (t.isContentEditable || /^(input|textarea|select)$/i.test(t.tagName)))) return; if (e.key === 'ArrowLeft') this.prevSlide(); if (e.key === 'ArrowRight') this.nextSlide(); });Also applies to: 217-220
4-20: Theme toggle: add ARIA and react to system theme changesImprove a11y and keep in sync with system preference when no saved theme exists.
if (themeToggleBtn) { themeToggleBtn.addEventListener('click', () => { document.body.classList.toggle('light-theme'); // Update localStorage based on the current theme if (document.body.classList.contains('light-theme')) { localStorage.setItem('theme', 'light'); } else { localStorage.setItem('theme', 'dark'); } + // Reflect toggle state for assistive tech + themeToggleBtn.setAttribute('aria-pressed', document.body.classList.contains('light-theme') ? 'true' : 'false'); }); const savedTheme = localStorage.getItem('theme'); if (savedTheme === 'light' || (!savedTheme && window.matchMedia('(prefers-color-scheme: light)').matches)) { document.body.classList.add('light-theme'); } + // Initialize ARIA state + themeToggleBtn.setAttribute('aria-pressed', document.body.classList.contains('light-theme') ? 'true' : 'false'); + // React to system theme changes only if user hasn't chosen a theme + if (!savedTheme) { + const mql = window.matchMedia('(prefers-color-scheme: light)'); + mql.addEventListener('change', (e) => { + document.body.classList.toggle('light-theme', e.matches); + themeToggleBtn.setAttribute('aria-pressed', e.matches ? 'true' : 'false'); + }); + } }
503-505: Avoid perpetual GSAP restart interval on loadThe always-on interval runs forever and can add up with multiple buttons. Trigger once; rely on hover for repeats.
- window.addEventListener('load', () => { - tl.restart(); - setInterval(() => tl.restart(), 1500); // repeat every 1.5s - }); + window.addEventListener('load', () => { + tl.restart(); + // No perpetual idle interval; hover handles repeat + });
134-136: Remove unused variable
herois declared but not used.- const hero = document.querySelector('.hero'); -
69-73: Make smooth-scroll offset dynamicUse navbar height instead of magic 80px.
- const offsetTop = targetSection.offsetTop - 80; + const nav = document.querySelector('.navbar'); + const navHeight = nav ? nav.getBoundingClientRect().height : 0; + const offsetTop = targetSection.getBoundingClientRect().top + window.scrollY - navHeight;
393-452: Inline CSS in JS is not theme-awareThese styles hardcode white backgrounds and may conflict with dark mode. Move to CSS and use design tokens/CSS variables.
Consider moving this block to docs/css/style.css and replace colors with theme variables (e.g., var(--surface-elevated), var(--shadow-color)).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/index.html(8 hunks)docs/js/main.js(10 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/index.html
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/js/main.js (2)
103-114: Add null check for navbar element.The
updateNavbarfunction will throw a TypeError if the navbar element doesn't exist. Add a null check to prevent runtime errors.Apply this diff:
const navbar = document.querySelector('.navbar'); function updateNavbar() { + if (!navbar) return; if (window.scrollY > 50) { navbar.classList.add('scrolled'); } else { navbar.classList.remove('scrolled'); } }
341-391: Add null checks for scroll button elements.The scroll button logic will throw TypeErrors if the elements don't exist. Add guards to prevent runtime errors.
Apply this diff:
// Get the button elements from the HTML const scrollTopBtn = document.getElementById('scrollTopBtn'); const scrollBottomBtn = document.getElementById('scrollBottomBtn'); +// Early return if buttons don't exist +if (!scrollTopBtn || !scrollBottomBtn) { + console.warn('Scroll buttons not found in DOM'); +} else { // Function to handle the visibility of both scroll buttons const handleScrollButtons = () => { const scrollHeight = document.documentElement.scrollHeight; const clientHeight = document.documentElement.clientHeight; const scrollPosition = window.scrollY; // --- Scroll to Top Button Logic --- // Show the button if user has scrolled down more than 300px if (scrollPosition > 300) { scrollTopBtn.classList.add('visible'); } else { scrollTopBtn.classList.remove('visible'); } // --- Scroll to Bottom Button Logic --- // Show the button if user is near the top, but hide it when they get close to the bottom const isNearBottom = scrollPosition + clientHeight >= scrollHeight - 300; if (scrollPosition > 100 && !isNearBottom) { scrollBottomBtn.classList.add('visible'); } else { scrollBottomBtn.classList.remove('visible'); } }; // --- Click Event Listeners --- // Smoothly scroll to the top of the page scrollTopBtn.addEventListener('click', () => { window.scrollTo({ top: 0, behavior: 'smooth' }); }); // Smoothly scroll to the bottom of the page scrollBottomBtn.addEventListener('click', () => { window.scrollTo({ top: document.body.scrollHeight, behavior: 'smooth' }); }); // Add a scroll event listener to the window to check button visibility window.addEventListener('scroll', handleScrollButtons); // Initial check when the page loads handleScrollButtons(); +}
🧹 Nitpick comments (3)
docs/js/main.js (3)
16-19: Consider listening for system theme changes.The current implementation checks the system preference once on load. For better UX, consider listening for changes to the user's system theme preference dynamically.
Add this code after the initial theme application:
// Listen for system theme changes window.matchMedia('(prefers-color-scheme: light)').addEventListener('change', (e) => { // Only update if user hasn't manually set a preference if (!localStorage.getItem('theme')) { if (e.matches) { document.body.classList.add('light-theme'); } else { document.body.classList.remove('light-theme'); } } });
394-452: Consider moving dynamic styles to CSS file.Injecting styles dynamically via JavaScript is not ideal for maintainability and can cause a Flash of Unstyled Content (FOUC). These styles should be in the main CSS file for better organization and performance.
Move the styles from lines 396-450 to
docs/css/style.cssand remove the dynamic injection. This will improve maintainability, enable CSS minification, and prevent potential FOUC issues.
462-465: Use a more specific selector for button animations.The selector
.btnis very generic and will apply the GSAP animation to all buttons with that class. Consider using a more specific selector to target only the intended buttons (e.g.,.btn.animatedor.cta-button).Apply this diff to use a more specific selector:
-document.querySelectorAll('.btn').forEach(button => { +document.querySelectorAll('.btn.animated, .cta-button').forEach(button => { const icon = button.querySelector('.icon'); if (!icon) return;Then update the HTML to add the appropriate class to buttons that should have this animation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/js/main.js(10 hunks)
🔇 Additional comments (3)
docs/js/main.js (3)
32-57: LGTM!The hero card parallax effect is well-implemented with proper null checks and optional chaining. The transform calculations are appropriate for the interactive effect.
544-556: LGTM!The fallback animation for Physics2DPlugin is well-implemented. The trigonometric calculations correctly simulate particle motion when the plugin is unavailable.
160-338: LGTM!The carousel implementation is well-structured with proper null checks, infinite loop handling, touch/keyboard support, and autoplay functionality. The code correctly clones slides for seamless transitions and includes proper cleanup.
| if (mobileMenuBtn) { | ||
| + mobileMenuBtn.addEventListener('click', () => { | ||
| if (navLinks) navLinks.classList.toggle('active'); | ||
| mobileMenuBtn.classList.toggle('active'); | ||
| }); | ||
| } |
There was a problem hiding this comment.
Fix indentation and brace placement.
The mobile menu toggle has incorrect indentation and brace structure. The closing brace at line 30 appears misaligned with the opening statement.
Apply this diff to fix the structure:
- if (mobileMenuBtn) {
-+ mobileMenuBtn.addEventListener('click', () => {
- if (navLinks) navLinks.classList.toggle('active');
- mobileMenuBtn.classList.toggle('active');
- });
-}
+ if (mobileMenuBtn) {
+ mobileMenuBtn.addEventListener('click', () => {
+ if (navLinks) navLinks.classList.toggle('active');
+ mobileMenuBtn.classList.toggle('active');
+ });
+ }Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In docs/js/main.js around lines 25 to 30 the if (mobileMenuBtn) block has the
event listener brace mispositioned; move the opening brace so it follows the
arrow in addEventListener (i.e., addEventListener('click', () => {), indent the
two toggle lines inside that function, then close the function with a }) and
finally close the outer if with a }. Ensure there are two closing braces in
sequence (one for the arrow function, one for the if) and correct indentation
for readability.
|
Hi @Harshdev625 , can you please review the updated changes and merge if everything looks good so far. |
|
@harshitrwt Great work on the TimeMachine icon! However, please remove it — it’s not the icon I asked you to change. I’ve attached a screenshot for reference. In dark mode, the footer, navigation bar, and tabs are difficult to distinguish — please adjust the styling for better visibility. Regarding the download button, it doesn’t match the previous design. Please check and restore the proper CSS. Lastly, the light mode appearance isn’t very appealing; refer to the attached screenshot for improvements. |
|
@Harshdev625 my bad , should i put this icon only ? Perfect! Next, I will work on improving the icons in the features section. Regarding the navbar and footer theme visibility, I thought having them float and change to white as we scroll down would look nice, as both the navbar and tabs stay visible. The same happens in the white theme without any issues. How about I change the navbar to black in the white theme and white in the black theme? Does that sound good? |
|
… feat/web-interface
|
Hi @Harshdev625 , I have updated the webpage as per suggestions, you can review and let me know if this one works well :) |
@harshitrwt Don’t add any CSS here, it’s not needed. Just ensure the icon is displayed properly. |
|
@Harshdev625 I have used a new theme button now (hope this one is better) and changed the CSS to original for the logo. |
|
@harshitrwt Great work, man! ⭐ |
|
@Harshdev625, Thank you so much! I genuinely enjoyed working on this project. Absolutely looking forward to starring your repo and sharing my feedback soon! Thanks again for reaching out. |
2 participants
Summary
What: Updated the extension’s UI with a modern, responsive design, including a redesigned hero section, improved navigation, smoother animations, and overall better layout consistency.
Why: The previous interface felt static and disconnected across sections and not upto the mark, this update enhances visual appeal, user experience, and seamless interaction across all devices.
Linked Issues: Closes #33
Type of Change
Screenshots (if UI changes)
|
|
|
Testing Instructions
For Reviewers:
See it in Action (if applicable):
Extension Setup (if applicable):
chrome://extensions/extensionfolderTested Environments:
Checklist
General
Code Quality
Functionality
Summary by CodeRabbit