feat: add Back to Top button for better navigation on long pages#222
feat: add Back to Top button for better navigation on long pages#222Vaishnavi22Thakur wants to merge 2 commits into
Conversation
Signed-off-by: Vaishnavi Thakur <vaaishnavi4713@gmail.com>
|
Warning Review limit reached
More reviews will be available in 1 minute and 38 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughA new ChangesBack to Top Navigation Feature
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@apps/web-dashboard/src/components/Layout/BackToTop.jsx`:
- Around line 27-30: The icon-only button rendered in BackToTop.jsx (the button
with onClick={handleClick} and content "↑") lacks an explicit accessible name;
add aria-label="Back to Top" to that button element (and the second similar
button instance around line 49) so assistive technologies receive a reliable
name; locate the button elements that use handleClick and add the aria-label
attribute without changing visual text or behavior.
- Around line 4-16: The effect currently registers handleScroll but never runs
it on mount, so BackToTop can remain hidden if the page loads with
window.scrollY > 300; after adding the scroll listener in the useEffect, call
the existing handleScroll() once to initialize visibility (i.e., invoke
handleScroll right after window.addEventListener('scroll', handleScroll)) so
setIsVisible is set correctly on mount; keep the existing cleanup that removes
the listener.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: c41caf8f-3d3d-41c6-9082-3e429e518c75
📒 Files selected for processing (3)
apps/web-dashboard/src/components/Layout/BackToTop.jsxapps/web-dashboard/src/components/Layout/MainLayout.jsxapps/web-dashboard/src/pages/LandingPage/index.jsx
| useEffect(() => { | ||
| const handleScroll = () => { | ||
| if (window.scrollY > 300) { | ||
| setIsVisible(true); | ||
| } else { | ||
| setIsVisible(false); | ||
| } | ||
| }; | ||
| window.addEventListener('scroll', handleScroll); | ||
| return () => { | ||
| window.removeEventListener('scroll', handleScroll); | ||
| }; | ||
| }, []); |
There was a problem hiding this comment.
Initialize visibility once on mount to avoid stale hidden state.
At Line 4, visibility is only updated after a scroll event. If the page restores at scrollY > 300, the button stays hidden until the user scrolls again. Call handleScroll() once in the effect after registering the listener.
Proposed fix
useEffect(() => {
const handleScroll = () => {
if (window.scrollY > 300) {
setIsVisible(true);
} else {
setIsVisible(false);
}
};
window.addEventListener('scroll', handleScroll);
+ handleScroll();
return () => {
window.removeEventListener('scroll', handleScroll);
};
}, []);📝 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.
| useEffect(() => { | |
| const handleScroll = () => { | |
| if (window.scrollY > 300) { | |
| setIsVisible(true); | |
| } else { | |
| setIsVisible(false); | |
| } | |
| }; | |
| window.addEventListener('scroll', handleScroll); | |
| return () => { | |
| window.removeEventListener('scroll', handleScroll); | |
| }; | |
| }, []); | |
| useEffect(() => { | |
| const handleScroll = () => { | |
| if (window.scrollY > 300) { | |
| setIsVisible(true); | |
| } else { | |
| setIsVisible(false); | |
| } | |
| }; | |
| window.addEventListener('scroll', handleScroll); | |
| handleScroll(); | |
| return () => { | |
| window.removeEventListener('scroll', handleScroll); | |
| }; | |
| }, []); |
🤖 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 `@apps/web-dashboard/src/components/Layout/BackToTop.jsx` around lines 4 - 16,
The effect currently registers handleScroll but never runs it on mount, so
BackToTop can remain hidden if the page loads with window.scrollY > 300; after
adding the scroll listener in the useEffect, call the existing handleScroll()
once to initialize visibility (i.e., invoke handleScroll right after
window.addEventListener('scroll', handleScroll)) so setIsVisible is set
correctly on mount; keep the existing cleanup that removes the listener.
Signed-off-by: Vaishnavi Thakur <vaaishnavi4713@gmail.com>
|
BRo @coderabbitai
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
echo "=== BackToTop.jsx ==="
cat apps/web-dashboard/src/components/Layout/BackToTop.jsx
echo ""
echo "=== MainLayout.jsx ==="
cat apps/web-dashboard/src/components/Layout/MainLayout.jsx
echo ""
echo "=== LandingPage/index.jsx ==="
cat apps/web-dashboard/src/pages/LandingPage/index.jsx
echo ""
echo "=== Layout folder structure ==="
tree apps/web-dashboard/src/components/Layout/ 2>/dev/null || ls -la apps/web-dashboard/src/components/Layout/Length of output: 58232 PR description image 1 (production at urbackend.bitbros.in): Shows the dark landing page "Build Anything." with three use-case cards — button not visible because the page hasn't been scrolled, which is correct expected behavior. PR description image 2 (localhost:5173): Shows the dev server with the cyan/teal circular back-to-top button visibly rendered in the bottom-right corner with an upward arrow — feature working correctly.
✅ Feature — Implemented CorrectlyAll requirements from issue
✅ File Placement — Correct
|
| Check | Result |
|---|---|
Feature works as per issue #219 |
✅ |
| File placed correctly | ✅ |
| Code structure | ✅ Clean |
| Scroll performance | |
| Icon consistency (Lucide) | ↑ |
| Hover feedback |
Overall solid work — the core feature is well done. The three
|
please fix then will merge |
2 participants
🚀 Pull Request Description
Fixes #219
🛠️ Type of Change
🧪 Testing & Validation
Backend Verification:
Frontend Verification:
📸 Screenshots / Recordings
✅ Checklist
📝 Changes Made
BackToTop.jsxcomponent inapps/web-dashboard/src/components/Layout/<BackToTop />toMainLayout.jsxfor dashboard pages<BackToTop />toLandingPage/index.jsxfor the landing pageSummary by CodeRabbit