This repository was archived by the owner on May 7, 2026. It is now read-only.
Implement hamburger navigation mobile-friendly sidebar and topbar - #186
Merged
Conversation
…h toggle functionality
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughAdds a mobile-responsive off-canvas sidebar: new Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Topbar as Topbar Component
participant Store as sidebarOpen Store
participant Sidebar as Sidebar Component
participant CSS as Styles
User->>Topbar: click hamburger
Topbar->>Store: sidebarOpen = !sidebarOpen
Store-->>Sidebar: state updated ($sidebarOpen)
Sidebar->>CSS: apply/remove .open class
CSS-->>Sidebar: slide in/out + backdrop shown/hidden
Sidebar-->>User: visible/hidden sidebar
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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
🧹 Nitpick comments (1)
rocky-interface/src/lib/components/Sidebar.svelte (1)
153-163: RedundantsidebarOpen.set(false)call.Line 157 calls
sidebarOpen.set(false), buthandleFrameChange('courses')on line 155 already sets this tofalseat line 112. This is harmless but redundant.♻️ Suggested simplification
async function openCourse(courseId: number) { selectedCourseId.set(courseId); await handleFrameChange('courses'); courseMenuOpen = false; - sidebarOpen.set(false); requestAnimationFrame(() => { requestAnimationFrame(() => { scrollToTopOfApp(); }); }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rocky-interface/src/lib/components/Sidebar.svelte` around lines 153 - 163, The openCourse function contains a redundant sidebarOpen.set(false) call because handleFrameChange('courses') already clears the sidebar (see handleFrameChange implementation); remove the extra sidebarOpen.set(false) line from openCourse and keep selectedCourseId.set(courseId), await handleFrameChange('courses'), courseMenuOpen = false, and the nested requestAnimationFrame(...) call that triggers scrollToTopOfApp() so behavior remains identical but without duplication.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@rocky-interface/src/lib/components/Topbar.svelte`:
- Around line 12-16: The hamburger spans in Topbar.svelte use
class="hamburger-line" but the stylesheet defines .hamburger-span, so the lines
are unstyled; fix by making the class names consistent—either change the three
<span> elements in Topbar.svelte to use class="hamburger-span" or update the CSS
selector in components.css from .hamburger-span to .hamburger-line so that the
styles apply to the spans rendered by the Topbar component.
In `@rocky-interface/src/lib/styles/components/components.css`:
- Around line 117-125: The .sidebar CSS rule uses an invalid var() syntax:
replace the incorrect token in the top property (currently "top:
--var(--size-topbar-height)") with the proper CSS var() call so it reads top:
var(--size-topbar-height); update the .sidebar rule in components.css (the rule
that sets position, top, left, height, transform, transition, z-index) to use
var(--size-topbar-height) so the mobile sidebar positions correctly below the
topbar.
---
Nitpick comments:
In `@rocky-interface/src/lib/components/Sidebar.svelte`:
- Around line 153-163: The openCourse function contains a redundant
sidebarOpen.set(false) call because handleFrameChange('courses') already clears
the sidebar (see handleFrameChange implementation); remove the extra
sidebarOpen.set(false) line from openCourse and keep
selectedCourseId.set(courseId), await handleFrameChange('courses'),
courseMenuOpen = false, and the nested requestAnimationFrame(...) call that
triggers scrollToTopOfApp() so behavior remains identical but without
duplication.
🪄 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: b1b0ce17-64e5-425e-9b17-5ffefb6fbbf1
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
rocky-interface/src/lib/components/Sidebar.svelterocky-interface/src/lib/components/Topbar.svelterocky-interface/src/lib/stores/sidebarStore.tsrocky-interface/src/lib/styles/components/components.css
Comment on lines
+12
to
+16
| <button class="hamburger" aria-label="Toggle menu" onclick={() => sidebarOpen.update(open => !open)}> | ||
| <span class="hamburger-line"></span> | ||
| <span class="hamburger-line"></span> | ||
| <span class="hamburger-line"></span> | ||
| </button> |
There was a problem hiding this comment.
Class name mismatch: hamburger lines won't be styled.
The spans use class="hamburger-line" but the CSS in components.css (lines 61-67) defines .hamburger-span. This mismatch will cause the hamburger icon bars to be invisible/unstyled.
🐛 Fix: Use consistent class name
Either update the HTML to match the CSS:
<button class="hamburger" aria-label="Toggle menu" onclick={() => sidebarOpen.update(open => !open)}>
- <span class="hamburger-line"></span>
- <span class="hamburger-line"></span>
- <span class="hamburger-line"></span>
+ <span class="hamburger-span"></span>
+ <span class="hamburger-span"></span>
+ <span class="hamburger-span"></span>
</button>Or update the CSS in components.css to use .hamburger-line instead of .hamburger-span.
📝 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.
Suggested change
| <button class="hamburger" aria-label="Toggle menu" onclick={() => sidebarOpen.update(open => !open)}> | |
| <span class="hamburger-line"></span> | |
| <span class="hamburger-line"></span> | |
| <span class="hamburger-line"></span> | |
| </button> | |
| <button class="hamburger" aria-label="Toggle menu" onclick={() => sidebarOpen.update(open => !open)}> | |
| <span class="hamburger-span"></span> | |
| <span class="hamburger-span"></span> | |
| <span class="hamburger-span"></span> | |
| </button> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rocky-interface/src/lib/components/Topbar.svelte` around lines 12 - 16, The
hamburger spans in Topbar.svelte use class="hamburger-line" but the stylesheet
defines .hamburger-span, so the lines are unstyled; fix by making the class
names consistent—either change the three <span> elements in Topbar.svelte to use
class="hamburger-span" or update the CSS selector in components.css from
.hamburger-span to .hamburger-line so that the styles apply to the spans
rendered by the Topbar component.
The widget now goes to the bottom of all the sidebar components.
I made the box to fit the content of the course classes. I didn't want the blank space when min-height was 100%. Change it if you don't like the style.
This was referenced Apr 29, 2026
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Summary by CodeRabbit
New Features
Bug Fixes
Style