Repository navigation
fix(navbar): stop dropdown scroll-lock and keep navbar during lazy navigation - #91
Merged
creatorcluster merged 1 commit intoOct 5, 2026
Merged
Conversation
…vigation Two root causes for issue #18: 1. DropdownMenu.Root defaulted to Radix `modal`, so react-remove-scroll locked `body` (the app's real scroll container) and removed the scrollbar without compensation, shifting the page. The custom PreventLayoutShift effects measured `document.documentElement` instead of the scroller, so their guard never ran. Remove the dead effects and default the root to `modal={false}`. 2. Every page rendered its own <Navbar/> inside the single top-level Suspense. Navigating to a lazy route hid the previous subtree (including the navbar) while the body-portaled dropdown escaped the hide and floated over the full-screen loading fallback. Render one global <Navbar/> outside Suspense, drop the per-page instances, and reset dropdown/drawer state on route change.
|
@Coder-soft is attempting to deploy a commit to the yamura3'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.
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 41 minutes. View limit detailsLimit details: You’ve used the included review currently available. Review configuration: ⚙️ Run configuration
📒 Files selected for processing (37)
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 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes issue Coder-soft#18
Summary
Two independent root causes made the navbar dropdowns feel broken.
1. Scroll lock hid the scrollbar and shifted the page (
src/components/ui/dropdown-menu.tsx)Radix
Rootdefaulted tomodal, soreact-remove-scrolllocked scrolling onbody(the real scroll container) and removed its ~11px scrollbar with no compensation. The two custom "prevent layout shift" effects measureddocument.documentElement, whoseinnerWidth - clientWidthis0in this layout, so the guard never ran. Both effects are gone and the root now defaults tomodal={false}.2. Lazy navigation hid the navbar but not the portaled menu (
src/App.tsx, pages)Every page rendered its own
<Navbar/>inside the single top-level<Suspense>. Navigating to a not-yet-loaded chunk made React hide the previous subtree (navbar included) while the menu, portaled todocument.body, escaped the hide and floated over the full-screen loader. A single global<Navbar/>now renders outsideSuspense, andactiveDropdown/ drawer state resets onlocation.pathnamechanges.Evidence
Measured in Chrome against
vite dev, viewport 1280x800, with a space-taking scrollbar:bodyscrollbar while dropdown open11px → 011pxgetComputedStyle(body).overflowYhidden(scroll locked)autoh1center-x671 → 676(shift)671Lazy-navigation test (delayed route chunk): before, 400ms after clicking a dropdown link the header box collapsed to
0x0while the menu was still224x190and visible over theLoading...fallback. After, the header stays1280x72during the load and the menu closes on navigation.Checks:
eslint .(0 errors),tsc --noEmit(clean),vite build(success).Merge Danger
Door: two-way
Blast Radius: app-wide navigation shell
All routes now share one global navbar instead of a per-page instance. Layout is unchanged because the header is
position: fixedand pages already reserve space for it. Thedropdown-menuprimitive is also used by the avatar menu and favorites sidebar, which now share the non-modal (no scroll-lock) behavior.