-
Notifications
You must be signed in to change notification settings - Fork 35
Add responsive homepage navigation menu for better section navigation #86
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.
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
Conversation
|
@Shaily-62 is attempting to deploy a commit to the idan lodzki's projects Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughAdds a new responsive NavMenu React component (with mobile toggle) and its CSS module, and integrates it into the main index page by importing the component and adding Changes
Suggested reviewers
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
Comment |
|
Thanks for your contribution! Join our Slack: https://join.slack.com/t/opsimate/shared_invite/zt-39bq3x6et-NrVCZzH7xuBGIXmOjJM7gA Please make sure to include an image with your PR — it really helps us review and understand the changes better. Only in rare cases will we accept a PR without one. Also, take a moment to review your code to ensure it’s clear, readable, and easy to follow. PRs from contributors who haven’t joined our Slack community or starred the repository won’t be reviewed — we’d love to see you join and be part of the project! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (1)
opsimate-docs/src/components/NavMenu.js (1)
7-12: Consider moving sections array outside the component.The sections array is currently redefined on every render. Moving it outside the component as a constant would prevent unnecessary recreation.
Apply this diff:
+const SECTIONS = [ + { id: 'features', label: 'Features' }, + { id: 'integrations', label: 'Integrations' }, + { id: 'community', label: 'Community' }, + { id: 'contact', label: 'Contact' }, +]; + export default function NavMenu() { const [menuOpen, setMenuOpen] = useState(false); - const sections = [ - { id: 'features', label: 'Features' }, - { id: 'integrations', label: 'Integrations' }, - { id: 'community', label: 'Community' }, - { id: 'contact', label: 'Contact' }, - ]; - return ( <nav className={styles.navMenu}> {/* Mobile Toggle Button */} <button className={styles.menuToggle} aria-label="Toggle menu" onClick={() => setMenuOpen(!menuOpen)} > <span className={styles.bar}></span> <span className={styles.bar}></span> <span className={styles.bar}></span> </button> {/* Menu Links */} <ul className={`${styles.menuList} ${menuOpen ? styles.open : ''}`}> - {sections.map((section) => ( + {SECTIONS.map((section) => ( <li key={section.id}>
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
opsimate-docs/src/components/NavMenu.js(1 hunks)opsimate-docs/src/components/NavMenu.module.css(1 hunks)opsimate-docs/src/pages/index.jsx(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
opsimate-docs/src/pages/index.jsx (1)
opsimate-docs/src/components/NavMenu.js (1)
NavMenu(4-42)
🔇 Additional comments (9)
opsimate-docs/src/pages/index.jsx (3)
5-5: LGTM!The NavMenu import is correctly placed and follows standard React import conventions.
106-106: Anchor target correctly added for features section.The
id="features"attribute enables navigation from the NavMenu to this section.
116-116: Anchor target correctly added for community section.The
id="community"attribute enables navigation from the NavMenu to this section.opsimate-docs/src/components/NavMenu.js (3)
1-2: LGTM!The imports are correct for React 19 and follow standard conventions.
28-39: Menu rendering logic works correctly.The conditional className application and the onClick handler to close the menu on link selection provide good mobile UX.
17-25: Add keyboard event handling for accessibility.The toggle button has an appropriate
aria-labelbut is missing keyboard event handlers. Users navigating via keyboard should be able to activate the button using Enter or Space keys.Apply this diff to add keyboard support:
<button className={styles.menuToggle} aria-label="Toggle menu" onClick={() => setMenuOpen(!menuOpen)} + onKeyDown={(e) => { + if (e.key === 'Enter' || e.key === ' ') { + e.preventDefault(); + setMenuOpen(!menuOpen); + } + }} >Note: Native
<button>elements already handle Enter and Space by default, triggering onClick. However, explicitly handling these keys ensures consistent behavior and demonstrates clear intent for keyboard accessibility.Likely an incorrect or invalid review comment.
opsimate-docs/src/components/NavMenu.module.css (3)
15-35: Desktop menu styles are well-implemented.The horizontal flex layout, hover states, and use of Docusaurus CSS variables ensure consistent theming across light and dark modes.
58-88: Mobile responsive design is well-structured.The breakpoint at 768px and the toggle behavior using the
.openclass provide good mobile UX. The absolute positioning and styling maintain theme consistency.
1-13: No z-index conflict exists. The Docusaurus navbar uses --ifm-z-index-fixed, which defaults to 3, not the higher values claimed in the original review. With z-index: 90, the NavMenu properly sits above the navbar.Likely an incorrect or invalid review comment.
|
Sorry, it looks like the right place to implement this is on the official website — it doesn’t look as good on the docs. |
Issue Reference
[UI] Add Navigation Menu to Jump Between Sections on Homepage #64
What Was Changed
src/components/NavMenu.js— handles the menu logic, responsive toggle, and section navigation.src/components/NavMenu.module.css— includes light/dark mode styles and responsive layout.NavMenucomponent intosrc/pages/index.jsxso it appears below the main Docusaurus navbar.Features,Integrations,Community,Contact).Why Was It Changed
Screenshots
Desktop View
Mobile View
Summary by CodeRabbit