Skip to content

Solved the broken progress bar issue #226

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.

Sign up for GitHub

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

Open
Shayan-Bhowmik wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Solved the broken progress bar issue #226

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 17 additions & 16 deletions src/components/ScrollProgressBar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,15 @@ const ScrollProgressBar = () => {
const [scrollWidth, setScrollWidth] = useState(0);
const [isAnimating, setIsAnimating] = useState(true); // Tracks if the page load animation is active

const handleScroll = () => {
const scrollTop = document.documentElement.scrollTop;
const scrollHeight =
document.documentElement.scrollHeight -
document.documentElement.clientHeight;
const width = (scrollTop / scrollHeight) * 100;
Comment on lines +8 to +12
setScrollWidth(width);
};
Comment on lines +7 to +14
Comment on lines +7 to +14
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 12, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Guard against division by zero and wrap in useCallback.

Two issues:

  1. When page content fits on screen without scrolling, scrollHeight - clientHeight equals 0, causing width to become NaN and breaking the progress bar rendering.

  2. The function is recreated on every render. The cleanup at line 31 attempts to remove a different function reference than was registered at line 28, preventing proper cleanup and causing a memory leak.

🔧 Proposed fix 
-import { useEffect, useState } from "react";
+import { useCallback, useEffect, useState } from "react";
...
-  const handleScroll = () => {
+  const handleScroll = useCallback(() => {
     const scrollTop = document.documentElement.scrollTop;
     const scrollHeight =
       document.documentElement.scrollHeight -
       document.documentElement.clientHeight;
+    
+    // Guard against division by zero when content doesn't scroll
+    if (scrollHeight === 0) {
+      setScrollWidth(0);
+      return;
+    }
+    
     const width = (scrollTop / scrollHeight) * 100;
     setScrollWidth(width);
-  };
+  }, []);
📝 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.

Suggested change
const handleScroll = () => {
const scrollTop = document.documentElement.scrollTop;
const scrollHeight =
document.documentElement.scrollHeight -
document.documentElement.clientHeight;
const width = (scrollTop / scrollHeight) * 100;
setScrollWidth(width);
};
const handleScroll = useCallback(() => {
const scrollTop = document.documentElement.scrollTop;
const scrollHeight =
document.documentElement.scrollHeight -
document.documentElement.clientHeight;
// Guard against division by zero when content doesn't scroll
if (scrollHeight === 0) {
setScrollWidth(0);
return;
}
const width = (scrollTop / scrollHeight) * 100;
setScrollWidth(width);
}, []);
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/components/ScrollProgressBar.tsx` around lines 7 - 14, The handleScroll
function can produce NaN when document.documentElement.scrollHeight -
document.documentElement.clientHeight is 0 and is recreated each render,
breaking cleanup; wrap handleScroll in useCallback and inside it compute denom =
document.documentElement.scrollHeight - document.documentElement.clientHeight,
if denom === 0 setScrollWidth(0) else setScrollWidth((scrollTop / denom) * 100);
then use that same memoized handleScroll in the effect that adds/removes the
'scroll' listener (include handleScroll in the effect deps) so addEventListener
and removeEventListener receive the identical function reference.


useEffect(() => {
// Simulate the page load animation
const animationTimeout = setTimeout(() => {
Expand All @@ -14,21 +23,13 @@ const ScrollProgressBar = () => {
return () => clearTimeout(animationTimeout);
}, []);

const handleScroll = () => {
const scrollTop = document.documentElement.scrollTop;
const scrollHeight =
document.documentElement.scrollHeight -
document.documentElement.clientHeight;
const width = (scrollTop / scrollHeight) * 100;
setScrollWidth(width);
};

useEffect(() => {
if (!isAnimating) {
window.addEventListener("scroll", handleScroll);
}
// Always listen for scroll events
window.addEventListener("scroll", handleScroll);
// Call handleScroll once on mount to set initial position
handleScroll();
return () => window.removeEventListener("scroll", handleScroll);
Comment on lines 26 to 31
}, [isAnimating]);
}, []);

return (
<>
Expand Down Expand Up @@ -56,10 +57,10 @@ const ScrollProgressBar = () => {
top: 0,
left: 0,
width: `${scrollWidth}%`,
height: "5px", // Thicker line for scroll progress
backgroundColor: "grey",
height: "3px",
backgroundColor: "#3b82f6",
zIndex: 100,
transition: "width 0.2s ease",
transition: "width 0.1s ease",
}}
/>
)}
Expand Down
Loading