feat: add scroll progress indicator - #2030
Conversation
Signed-off-by: SagarSingh01 <sagarsingh07789@gmail.com>
|
@SagarSingh01 is attempting to deploy a commit to the s3dfx-cyber's projects Team on Vercel. A member of the Team first needs to authorize it. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
👋 Thanks for opening a PR, @SagarSingh01!Your PR has entered the 🚦 PR Review Pipeline.
🔄 Review Flow
A pipeline status comment may appear automatically as your PR progresses. ✅ Contributor Checklist
|
💬 Faster Reviews & AssignmentsHi @SagarSingh01, for faster coordination and smoother communication, consider joining our Discord community: 👉 https://discord.gg/jYrbHKtn8t Useful Channels
|
✅ DCO Sign-off VerifiedHi @SagarSingh01 👋 All commits in this PR contain valid Thank you for following the DCO requirements 🚀 |
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📜 Recent review details
|
| Layer / File(s) | Summary |
|---|---|
Progress bar markup and page wiring index.html, landing.html |
Adds the fixed progress bar and loads src/js/scroll-progress.js on both pages. The landing page also reformats stylesheet links and adds whitespace. |
Scroll progress calculation and updates src/js/scroll-progress.js |
Calculates scroll percentage, updates the bar width, throttles updates with requestAnimationFrame, observes body size changes when supported, and initializes the display. |
Estimated code review effort: 2 (Simple) | ~10 minutes
Sequence Diagram(s)
sequenceDiagram
participant Browser
participant ScrollProgressScript
participant scrollProgressBar
participant ResizeObserver
Browser->>ScrollProgressScript: Emit scroll or resize event
ScrollProgressScript->>ScrollProgressScript: Calculate scroll percentage
ScrollProgressScript->>scrollProgressBar: Set width style
ScrollProgressScript->>scrollProgressBar: Apply calculated width
ResizeObserver->>ScrollProgressScript: Report body size change
Possibly related issues
- feat: Add Reading Progress Indicator for Long Pages #2042: Both issues implement a fixed orange scroll progress bar whose width tracks page scrolling.
Possibly related PRs
- S3DFX-CYBER/GSoC-Org-Finder-#2029: Modifies the same scroll progress bar markup and update logic.
- S3DFX-CYBER/GSoC-Org-Finder-#2052: Implements the same fixed progress bar and scroll-percentage updates on both pages.
Suggested labels: enhancement, type:feature
Suggested reviewers: s3dfx-cyber, tarunyaprogrammer, kumarnirupam1
Poem
I’m a rabbit watching pixels glide,
The orange progress line grows wide.
Scroll down the page; I hop along.
Resize the view; the bar stays strong.
At the bottom, the trail is complete! 🐇
🚥 Pre-merge checks | ✅ 3 | ❌ 2
❌ Failed checks (2 warnings)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Linked Issues check | The implementation covers the progress bar and scroll updates, but the provided change summary does not show prefers-reduced-motion support. | Add a prefers-reduced-motion media rule that disables or makes the progress-bar transition instant, then verify the behavior. | |
| Out of Scope Changes check | The landing.html Google Fonts reformatting and inserted blank line are unrelated to the linked scroll-progress requirement. | Remove unrelated formatting-only changes from landing.html, including the Google Fonts reformatting and blank line. |
✅ Passed checks (3 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | The title clearly summarizes the main change: adding a scroll progress indicator. |
| Description check | ✅ Passed | The description covers the change, issue, testing, screenshots, and checklist, despite using headings different from the repository template. |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
- Create stacked PR
- Commit on current branch
🧪 Generate unit tests (beta)
- Create PR with unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
🚦 PR Review Pipeline
Last updated: Fri, 07 Aug 2026 03:49:50 GMT |
🤖 TENET Agent Review📋 SummaryThis PR introduces an animated scroll progress indicator, displayed as a thin bar at the bottom of the sticky header, for both 🔐 Security FindingsNo security issues found. 🧹 Code Quality
✅ What's Done Well
📝 Overall VerdictREQUEST CHANGES - The significant code duplication and inline script placement need to be addressed for improved maintainability and adherence to best practices. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
landing.html (1)
1124-1126: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove duplicate comment.
The section comment for the scroll progress bar is duplicated.
♻️ Proposed fix
- <!-- ═══════════════════ Scroll ProgressBar ═══════════════════ --> <!-- ═══════════════════ Scroll ProgressBar ═══════════════════ --> <script>🤖 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 `@landing.html` around lines 1124 - 1126, Remove the duplicated Scroll ProgressBar section comment, keeping a single comment immediately before the related script block.
🤖 Prompt for all review comments with 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.
Inline comments:
In `@index.html`:
- Around line 1203-1205: Update the scrollProgressBar in index.html (lines
1203-1205) and landing.html (lines 67-69) to use absolute bottom-0 within a
relative header container, or otherwise position it at the header’s bottom
rather than the viewport bottom; ensure both pages preserve the intended
progress-bar placement beneath the header.
---
Nitpick comments:
In `@landing.html`:
- Around line 1124-1126: Remove the duplicated Scroll ProgressBar section
comment, keeping a single comment immediately before the related script block.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 25ef11d6-f1c8-40ba-b2a6-c9176de15a36
📒 Files selected for processing (2)
index.htmllanding.html
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: cubic · AI code reviewer
⚠️ CI failures not shown inline (1)
Commit Status: Vercel: Vercel
Conclusion: failure
Authorization required to deploy.
🔇 Additional comments (1)
index.html (1)
5815-5841: LGTM!
Signed-off-by: SagarSingh01 <sagarsingh07789@gmail.com>
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
🤖 TENET Agent Review📋 SummaryThis pull request introduces an animated scroll progress indicator, displayed as a thin bar at the bottom of the sticky header. The progress bar dynamically updates its width based on the user's scroll position, providing visual feedback. The implementation uses standard HTML, CSS (Tailwind), and JavaScript, including support for 🔐 Security FindingsNo security issues found. 🧹 Code Quality
✅ What's Done Well
📝 Overall Verdict[APPROVE] - The feature is well-implemented with good practices and no security concerns. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed
Confidence score: 5/5
- Safe to merge after the addressed issues were fixed.
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/js/scroll-progress.js (1)
5-21: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrefer
globalThisoverwindowand throttle layout updates.Reading layout properties (
scrollHeight,innerHeight) and writing styles directly on everyscrollevent can cause performance jank. Wrapping these updates inrequestAnimationFrameis a recommended practice to ensure smooth scrolling. Additionally, based on learnings, this repository prefers usingglobalThisoverwindowto keep the code environment-agnostic and satisfy SonarCloud static analysis.♻️ Proposed refactor
- const updateProgress = () => { - const scrollTop = window.scrollY; - const scrollHeight = - document.documentElement.scrollHeight - window.innerHeight; - - const progress = - scrollHeight > 0 ? (scrollTop / scrollHeight) * 100 : 0; - - progressBar.style.width = `${progress}%`; - }; - - window.addEventListener("scroll", updateProgress, { passive: true }); - window.addEventListener("resize", updateProgress); - - if ("ResizeObserver" in window) { - new ResizeObserver(updateProgress).observe(document.body); - } + let ticking = false; + + const updateProgress = () => { + const scrollTop = globalThis.scrollY; + const scrollHeight = + document.documentElement.scrollHeight - globalThis.innerHeight; + + const progress = + scrollHeight > 0 ? (scrollTop / scrollHeight) * 100 : 0; + + progressBar.style.width = `${progress}%`; + ticking = false; + }; + + const requestUpdate = () => { + if (!ticking) { + globalThis.requestAnimationFrame(updateProgress); + ticking = true; + } + }; + + globalThis.addEventListener("scroll", requestUpdate, { passive: true }); + globalThis.addEventListener("resize", requestUpdate); + + if ("ResizeObserver" in globalThis) { + new ResizeObserver(requestUpdate).observe(document.body); + }🤖 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/js/scroll-progress.js` around lines 5 - 21, Update the scroll-progress implementation around updateProgress to use globalThis instead of window for global APIs, and throttle layout reads and progressBar.style.width writes through requestAnimationFrame. Ensure scroll, resize, and ResizeObserver callbacks schedule at most one pending frame while preserving the current progress calculation.Source: Learnings
🤖 Prompt for all review comments with 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.
Nitpick comments:
In `@src/js/scroll-progress.js`:
- Around line 5-21: Update the scroll-progress implementation around
updateProgress to use globalThis instead of window for global APIs, and throttle
layout reads and progressBar.style.width writes through requestAnimationFrame.
Ensure scroll, resize, and ResizeObserver callbacks schedule at most one pending
frame while preserving the current progress calculation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3aa2a94d-1396-4971-b863-708794d57305
📒 Files selected for processing (3)
index.htmllanding.htmlsrc/js/scroll-progress.js
🚧 Files skipped from review as they are similar to previous changes (1)
- index.html
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: cubic · AI code reviewer
⚠️ CI failures not shown inline (1)
Commit Status: Vercel: Vercel
Conclusion: failure
Authorization required to deploy.
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-06-15T18:15:28.688Z
Learnt from: arghya29
Repo: S3DFX-CYBER/GSoC-Org-Finder- PR: 1882
File: src/js/footer.js:33-33
Timestamp: 2026-06-15T18:15:28.688Z
Learning: In this repo’s JavaScript (e.g., footer.js), `globalThis` is intentionally preferred over `window` to keep code environment-agnostic and to satisfy SonarCloud static analysis. The project targets modern browsers (ES2021) with no transpilation, so `globalThis` is fully supported—do not flag `globalThis` usage as a browser compatibility concern or suggest replacing it with `window` during review.
Applied to files:
src/js/scroll-progress.js
🔇 Additional comments (1)
landing.html (1)
1125-1125: LGTM!
|
Hi! I've addressed all the requested review comments and pushed the fixes. All checks are passing now. I'd appreciate a review when convenient. Thank you! |
There was a problem hiding this comment.
0 issues found across 3 files (changes from recent commits).
Requires human review: Auto-approval blocked by 3 unresolved issues from previous reviews.
Re-trigger cubic
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
🤖 TENET Agent Review📋 SummaryThis pull request introduces a scroll progress indicator to the sticky header of both 🔐 Security FindingsNo security issues found. 🧹 Code Quality
✅ What's Done Well
📝 Overall VerdictREQUEST CHANGES - Address the minor code quality issue regarding the missing newline in the new JS file. |
There was a problem hiding this comment.
0 issues found across 2 files (changes from recent commits).
Auto-approved: Adds a scroll progress bar to the header. Feature is purely visual, bounded, respects reduced-motion, and poses no operational, security, or data risks.
Re-trigger cubic
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
🤖 TENET Agent Review📋 SummaryThis PR introduces an animated scroll progress indicator, displayed as a thin bar at the bottom of the sticky header. The progress bar dynamically updates its width based on the user's scroll position, utilizing the brand's orange accent. The implementation includes considerations for accessibility ( 🔐 Security FindingsNo security issues found. 🧹 Code Quality
✅ What's Done Well
📝 Overall Verdict[APPROVE] - The feature is well-implemented, secure, and considers accessibility and performance. Review powered by TENET Agent 🛡️ | Triggered automatically on PR #2030 |
|
Hi maintainers! Just following up on this PR. All requested changes have been addressed, and the automated checks are passing. I'd appreciate a review whenever you have time. Thank you! |
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Auto-approved: Adds a purely visual scroll progress indicator to the header. No behavioral or operational changes, just a bounded UI enhancement.
Re-trigger cubic
|
Hi! @S3DFX-CYBER |
S3DFX-CYBER
left a comment
S3DFX-CYBER
left a comment
There was a problem hiding this comment.
This adds a scroll progress bar to the header on both index.html and landing.html, backed by a new src/js/scroll-progress.js. The approach is clean — a self-executing IIFE, passive scroll listener, ResizeObserver for layout changes. But there are a couple of things to fix, and this is also a duplicate of #2052 which adds the same feature.
First, the duplicate: #2052 adds a reading progress indicator to the same pages with slightly different implementation. You should coordinate with that PR's author and pick one to go forward with — having both open will just cause merge conflicts. I'd lean toward this one since it's simpler and doesn't introduce unrelated scope, but either way one needs to be closed.
On the code itself: src/js/scroll-progress.js uses window.scrollY, window.innerHeight, window.addEventListener, and "ResizeObserver" in window throughout. The repo convention is globalThis over window in src/js/ files — this is intentional, to keep the code environment-agnostic and satisfy SonarCloud. Switch all the window references to globalThis.
The scroll handler runs updateProgress directly on every scroll event, which reads layout properties (scrollHeight, innerHeight) and writes to style.width each time. Wrap it in requestAnimationFrame with a ticking guard so you batch at most one update per frame — the scroll event fires far more often than the screen refreshes, and the direct style write will cause jank on slower devices.
The landing.html diff includes unrelated reformatting of the Google Fonts <link> tags (splitting them across multiple lines) and whitespace changes to the <a href="/"> tag. Those aren't part of the scroll progress feature — drop them so the diff stays focused.
4a10c53
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
github-actions
Bot
added
dco-missing
needs-stage-1-fixes
and removed
dco-verified
stage-1-approved
labels
🤖 TENET Agent Review📋 SummaryThis pull request introduces an animated scroll progress indicator to the sticky header of the application. The feature is implemented using HTML, Tailwind CSS, and a dedicated JavaScript file. The JavaScript logic efficiently calculates scroll progress, updates the indicator's width using 🔐 Security FindingsNo security issues found. 🧹 Code Quality
✅ What's Done Well
📝 Overall VerdictREQUEST CHANGES - Address minor code quality issues, specifically the missing newlines and verbose comments. |
Signed-off-by: SagarSingh01 <sagarsingh07789@gmail.com>
Signed-off-by: SagarSingh01 <sagarsingh07789@gmail.com>
Signed-off-by: SagarSingh01 <sagarsingh07789@gmail.com>
4a10c53 to
8b0dd19
Compare
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
github-actions
Bot
added
dco-verified
stage-1-approved
and removed
dco-missing
needs-stage-1-fixes
labels
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
🤖 TENET Agent Review📋 SummaryThis pull request introduces an animated scroll progress indicator, displayed as a thin bar at the bottom of the sticky header. The progress bar dynamically updates its width based on the user's scroll position, providing visual feedback. The implementation is robust, utilizing 🔐 Security FindingsNo security issues found. 🧹 Code Quality
✅ What's Done Well
📝 Overall Verdict[APPROVE] - The feature is well-implemented, robust, and adheres to good performance and accessibility practices with no security concerns. |
2 participants
Related Issue
Closes #2028
Description of Changes
This PR adds an animated scroll progress indicator to the bottom edge of the sticky header, providing visual feedback as users navigate through the page.
The progress line fills smoothly from left to right based on the user's scroll position while preserving the existing sticky navigation, backdrop blur, and overall layout.
What Changed
prefers-reduced-motionby disabling animated transitions for users who prefer reduced motion.Screenshot
Testing
prefers-reduced-motiondisables transition animationsnpm run lintnpm testPR Checklist
main