Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds CSS to implement a full-viewport-height mobile navbar sidebar, using 100dvh with 100vh fallback. Sets calculated heights for sidebar items, enables inner scrolling, prevents clipping, and adjusts pointer events and z-index layering when the sidebar is shown. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
✨ Finishing Touches🧪 Generate 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
| .navbar.navbar-sidebar--show .navbar-sidebar { | ||
| z-index: 10001; | ||
| } |
There was a problem hiding this comment.
this is needed to show in front of the "ask ai" box
![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
🧹 Nitpick comments (2)
src/css/custom.css (2)
373-380: Use modern viewport units + safe-area; avoid over-constraining heightPrefer a resilient combo and account for notches. Also, setting both top/bottom and height over-constrains the box.
.navbar-sidebar { - top: 0; /* start at very top */ - bottom: 0; - height: 100dvh; /* modern mobile viewport unit */ + top: 0; /* start at very top */ + /* Let height be driven by min/height, not bottom */ + /* bottom: 0; */ /* remove to avoid over-constraint */ + /* Prefer small/large/dynamic vh mix for mobile chrome/safari bars */ + min-height: 100svh; + height: 100dvh; /* dynamic viewport */ + padding-bottom: env(safe-area-inset-bottom, 0); display: flex; flex-direction: column; }
401-404: pointer-events: none may disable the close toggleIn Docusaurus, the mobile toggle lives in .navbar__inner. Disabling pointer events there can prevent closing the sidebar via the toggle.
.navbar.navbar-sidebar--show .navbar__inner { pointer-events: none; } +/* Keep the toggle clickable while sidebar is open */ +.navbar.navbar-sidebar--show .navbar__inner .navbar__toggle { + pointer-events: auto; +}To verify, open mobile view, open the sidebar, and try tapping the toggle icon. If it doesn’t respond, apply the patch.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/css/custom.css(1 hunks)
🔇 Additional comments (1)
src/css/custom.css (1)
406-412: Align sidebar z-index with project stack
- In src/css/custom.css (lines 407 & 411), replace the hard-coded 10001/10000 with values just above the navbar (e.g. 1100/1090) or use existing CSS variables if defined.
- Audit shows no other z-index declarations in src/css; confirm the base navbar z-index (and any CSS vars) before merging.
| @supports not (height: 100dvh) { | ||
| .navbar-sidebar { | ||
| height: 100vh; /* fallback */ | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fallback incomplete: .navbar-sidebar__items still uses 100dvh
On browsers without 100dvh, the calc() using 100dvh in the items block becomes invalid, breaking layout.
@supports not (height: 100dvh) {
.navbar-sidebar {
height: 100vh; /* fallback */
}
+ .navbar-sidebar__items {
+ height: calc(100vh - var(--ifm-navbar-height));
+ max-height: calc(100vh - var(--ifm-navbar-height));
+ }
}📝 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.
| @supports not (height: 100dvh) { | |
| .navbar-sidebar { | |
| height: 100vh; /* fallback */ | |
| } | |
| } | |
| @supports not (height: 100dvh) { | |
| .navbar-sidebar { | |
| height: 100vh; /* fallback */ | |
| } | |
| .navbar-sidebar__items { | |
| height: calc(100vh - var(--ifm-navbar-height)); | |
| max-height: calc(100vh - var(--ifm-navbar-height)); | |
| } | |
| } |
| .navbar-sidebar__items { | ||
| margin-top: var(--ifm-navbar-height); | ||
| height: calc(100dvh - var(--ifm-navbar-height)); | ||
| max-height: calc(100dvh - var(--ifm-navbar-height)); | ||
| overflow: visible; /* avoid clipping links; let inner list handle scroll */ | ||
| flex: 1 1 auto; | ||
| } | ||
|
|
||
| /* Let the inner list area scroll instead of the container */ | ||
| .navbar-sidebar__item.menu { | ||
| overflow-y: auto; | ||
| } |
There was a problem hiding this comment.
Scrollable area assigned to wrong element; results in no scrolling
The container (.navbar-sidebar__items) has fixed height but overflow: visible; putting overflow-y: auto on the inner .menu won’t create a scrollable region. The scroll should live on the sized container.
.navbar-sidebar__items {
margin-top: var(--ifm-navbar-height);
height: calc(100dvh - var(--ifm-navbar-height));
max-height: calc(100dvh - var(--ifm-navbar-height));
- overflow: visible; /* avoid clipping links; let inner list handle scroll */
+ overflow: hidden; /* contain contents */
+ overflow-y: auto; /* scrolling lives here */
+ -webkit-overflow-scrolling: touch; /* iOS smooth scrolling */
flex: 1 1 auto;
}
/* Let the inner list area scroll instead of the container */
.navbar-sidebar__item.menu {
- overflow-y: auto;
+ overflow: visible; /* no scroll here */
}📝 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-sidebar__items { | |
| margin-top: var(--ifm-navbar-height); | |
| height: calc(100dvh - var(--ifm-navbar-height)); | |
| max-height: calc(100dvh - var(--ifm-navbar-height)); | |
| overflow: visible; /* avoid clipping links; let inner list handle scroll */ | |
| flex: 1 1 auto; | |
| } | |
| /* Let the inner list area scroll instead of the container */ | |
| .navbar-sidebar__item.menu { | |
| overflow-y: auto; | |
| } | |
| .navbar-sidebar__items { | |
| margin-top: var(--ifm-navbar-height); | |
| height: calc(100dvh - var(--ifm-navbar-height)); | |
| max-height: calc(100dvh - var(--ifm-navbar-height)); | |
| overflow: hidden; /* contain contents */ | |
| overflow-y: auto; /* scrolling lives here */ | |
| -webkit-overflow-scrolling: touch; /* iOS smooth scrolling */ | |
| flex: 1 1 auto; | |
| } | |
| /* Let the inner list area scroll instead of the container */ | |
| .navbar-sidebar__item.menu { | |
| overflow: visible; /* no scroll here */ | |
| } |
🤖 Prompt for AI Agents
In src/css/custom.css around lines 388 to 399, the scrollable area is assigned
to the inner .navbar-sidebar__item.menu while the sized container
.navbar-sidebar__items has overflow: visible, so no scrolling occurs; change
overflow on the sized container to create the scrollable region by setting
.navbar-sidebar__items to overflow-y: auto (or overflow: auto) and remove
overflow-y: auto from .navbar-sidebar__item.menu so the element with fixed
height handles scrolling.
2 participants
open the docs.envio.dev from your phone and click on the nav bar to see what it was doing before
Summary by CodeRabbit