Hide widget panel, add course dropdown, and burger easter egg for mobile view - #214
Conversation
I've kept the help view attached with the other bars and not on the bottom cause I thought it looked better. lmk if you want me to change it to match the desktop view.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (2)
📝 WalkthroughWalkthroughAdds a probabilistic hamburger-button visual variant to Topbar, updates mobile responsive rules for spacer, course-popout, and widget-panel, and adds Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 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: 4
🧹 Nitpick comments (1)
rocky-interface/src/lib/styles/components/components.css (1)
294-297: Consider conditionally rendering the widget panel on mobile instead of hiding it with CSS.
display: nonehides the panel visually, butWidgetPanel.sveltestill mounts on all screen sizes and callsfetchDefaultWidgets(). On mobile (max-width: 768px), this results in unnecessary network requests for data that won't be displayed. Gate rendering in the parent component (+page.svelte) to avoid mounting the widget panel entirely on smaller viewports.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rocky-interface/src/lib/styles/components/components.css` around lines 294 - 297, The widget panel is only hidden via CSS which still mounts WidgetPanel.svelte and triggers fetchDefaultWidgets(); modify the parent (+page.svelte) to conditionally render the WidgetPanel component instead of relying on .widget-panel {display:none}; detect small viewports (e.g., using svelte:window innerWidth or window.matchMedia('(max-width:768px)')/a reactive store) and only include <WidgetPanel /> when the viewport is wider than 768px so fetchDefaultWidgets() is never called on mobile.
🤖 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 14-17: The easter-egg timeout uses setTimeout directly and can
overlap or outlive the component; introduce a local timeout handle (e.g.,
burgerClickTimeout) and when you trigger the easter egg (where showBurgerClick
is set) clear any existing timeout via clearTimeout(burgerClickTimeout) before
scheduling a new one, assign the result to burgerClickTimeout, and add an
onDestroy hook to clearTimeout(burgerClickTimeout) to avoid leaks and stray
callbacks; ensure you null/undefine burgerClickTimeout after clearing to keep
state consistent.
- Around line 9-10: isHamburgerDay is seeded with Math.random() at module init
which causes SSR/client hydration mismatch; change the initial declaration to
let isHamburgerDay = false and then in an onMount() callback (import onMount
from 'svelte') set isHamburgerDay = Math.random() < 0.1 so the random easter-egg
runs only after hydration; leave showBurgerClick as-is and ensure the onMount
block updates the same isHamburgerDay variable used in the template.
In `@rocky-interface/src/lib/styles/components/components.css`:
- Around line 256-271: The .course-popout rule for mobile sets position: static
and max-height: 400px but the surrounding mobile sidebar container lacks
overflow, causing content to become unreachable on small screens; update the
mobile CSS so that the parent mobile sidebar (the element that contains
.course-popout, e.g., the mobile nav/sidebar container) is allowed to scroll
and/or make .course-popout able to expand within a scrollable
container—specifically add overflow-y: auto (or overflow: auto) to the mobile
sidebar/container selector and ensure .course-popout can use max-height with
overflow-y: auto so the dropdown and other nav items remain reachable on short
viewports.
- Around line 273-277: The media query hides the entire .course-popout-header
which also removes the .course-popout-create-btn used by Sidebar.svelte
(rendered at the create button in line ~190), so update the CSS so mobile still
exposes the create button: instead of display:none on .course-popout-header,
either hide only non-action parts (e.g., title/metadata) or add an override for
.course-popout-header .course-popout-create-btn to keep it visible on
max-width:768px (ensure layout and spacing still work and consider using
!important if specificity requires it), or move the .course-popout-create-btn
out of .course-popout-header in the component and style it for mobile.
---
Nitpick comments:
In `@rocky-interface/src/lib/styles/components/components.css`:
- Around line 294-297: The widget panel is only hidden via CSS which still
mounts WidgetPanel.svelte and triggers fetchDefaultWidgets(); modify the parent
(+page.svelte) to conditionally render the WidgetPanel component instead of
relying on .widget-panel {display:none}; detect small viewports (e.g., using
svelte:window innerWidth or window.matchMedia('(max-width:768px)')/a reactive
store) and only include <WidgetPanel /> when the viewport is wider than 768px so
fetchDefaultWidgets() is never called on mobile.
🪄 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: 8c85595b-72cd-418e-8d95-7dd2c38d86ad
⛔ Files ignored due to path filters (1)
rocky-interface/static/hamburger.svgis excluded by!**/*.svg
📒 Files selected for processing (2)
rocky-interface/src/lib/components/Topbar.svelterocky-interface/src/lib/styles/components/components.css
| const isHamburgerDay = Math.random() < 0.1; // x% chance to show the hamburger icon | ||
| let showBurgerClick = $state(false); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Read-only check: inspect Svelte/SvelteKit versions and whether SSR is disabled.
fd -i 'package.json' -x sh -c '
echo "== $1 ==";
jq ".dependencies.svelte, .devDependencies.svelte, .dependencies[\"@sveltejs/kit\"], .devDependencies[\"@sveltejs/kit\"]" "$1"
' sh {}
fd -i 'svelte.config.*' -x sh -c '
echo "== $1 ==";
sed -n "1,220p" "$1"
' sh {}
rg -nP 'export\s+const\s+ssr\s*=\s*false|ssr\s*:\s*false' -C2Repository: Spring-2026-Software-Engineering/Rocky
Length of output: 658
🏁 Script executed:
cat -n rocky-interface/src/lib/components/Topbar.svelte | sed -n '1,40p'Repository: Spring-2026-Software-Engineering/Rocky
Length of output: 1691
🏁 Script executed:
rg 'onMount|import.*onMount' rocky-interface/src/lib/components/ -lRepository: Spring-2026-Software-Engineering/Rocky
Length of output: 491
Move random initial rendering behind onMount to prevent hydration mismatch.
With SSR enabled (default configuration), Line 9 evaluates Math.random() independently on server and client during initialization. This causes the button to render as an <img> element on one side and three <span> elements on the other, creating a hydration failure. Initialize isHamburgerDay deterministically to false, then roll the easter egg in onMount() after hydration completes.
Proposed fix
<script lang="ts">
+ import { onMount } from 'svelte';
import type { User } from '$lib/types/user';
import '$lib/styles/components/topbar.css';
import {sidebarOpen} from '$lib/stores/sidebarStore';
@@
- const isHamburgerDay = Math.random() < 0.1; // x% chance to show the hamburger icon
+ let isHamburgerDay = $state(false); // 10% chance to show the hamburger icon after hydration
let showBurgerClick = $state(false);
+
+ onMount(() => {
+ isHamburgerDay = Math.random() < 0.1;
+ });📝 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.
| const isHamburgerDay = Math.random() < 0.1; // x% chance to show the hamburger icon | |
| let showBurgerClick = $state(false); | |
| <script lang="ts"> | |
| import { onMount } from 'svelte'; | |
| import type { User } from '$lib/types/user'; | |
| import '$lib/styles/components/topbar.css'; | |
| import {sidebarOpen} from '$lib/stores/sidebarStore'; | |
| let isHamburgerDay = $state(false); // 10% chance to show the hamburger icon after hydration | |
| let showBurgerClick = $state(false); | |
| onMount(() => { | |
| isHamburgerDay = Math.random() < 0.1; | |
| }); |
🤖 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 9 - 10,
isHamburgerDay is seeded with Math.random() at module init which causes
SSR/client hydration mismatch; change the initial declaration to let
isHamburgerDay = false and then in an onMount() callback (import onMount from
'svelte') set isHamburgerDay = Math.random() < 0.1 so the random easter-egg runs
only after hydration; leave showBurgerClick as-is and ensure the onMount block
updates the same isHamburgerDay variable used in the template.
| if(Math.random() < 0.125) { | ||
| showBurgerClick = true; | ||
| setTimeout(() => { showBurgerClick = false; }, 3000); //shows hamburger for x seconds | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -t f "Topbar.svelte" --exec cat -n {}Repository: Spring-2026-Software-Engineering/Rocky
Length of output: 1691
Store and clear the timeout handle to prevent overlapping timeouts and memory leaks.
When the easter egg triggers multiple times within 3 seconds, each click schedules an independent timeout. The first timeout still fires after 3 seconds from its trigger, cutting short any subsequent clicks. Additionally, a pending timeout can fire after the component unmounts, causing a memory leak.
Store the timeout handle, clear it before scheduling a new one on retrigger, and clean it up in an onDestroy hook.
🤖 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 14 - 17, The
easter-egg timeout uses setTimeout directly and can overlap or outlive the
component; introduce a local timeout handle (e.g., burgerClickTimeout) and when
you trigger the easter egg (where showBurgerClick is set) clear any existing
timeout via clearTimeout(burgerClickTimeout) before scheduling a new one, assign
the result to burgerClickTimeout, and add an onDestroy hook to
clearTimeout(burgerClickTimeout) to avoid leaks and stray callbacks; ensure you
null/undefine burgerClickTimeout after clearing to keep state consistent.
| /* Mobile View for Courses Sidebar */ | ||
| @media (max-width: 768px) { | ||
| .course-popout { | ||
| position: static; | ||
| width: 100%; | ||
| height: auto; | ||
| max-height: 400px; | ||
| border-radius: 0; | ||
| border-left: none; | ||
| border-right: none; | ||
| box-shadow: none; | ||
| border-top: 1px solid var(--color-gray-200); | ||
| border-bottom: 1px solid var(--color-gray-200); | ||
| overflow-y: auto; | ||
| } | ||
| } |
There was a problem hiding this comment.
Make the mobile sidebar scroll when the static course dropdown opens.
Line 259 moves the dropdown into normal sidebar flow, but the mobile sidebar has a fixed viewport-height container and no overflow-y. On short phones/landscape, the 400px dropdown plus nav items can become unreachable.
Proposed CSS adjustment
`@media` (max-width: 768px) {
+ .sidebar {
+ overflow-y: auto;
+ }
+
.course-popout {
position: static;
width: 100%;
height: auto;
- max-height: 400px;
+ max-height: min(400px, calc(100dvh - var(--size-topbar-height) - var(--space-xl)));
border-radius: 0;
border-left: none;
border-right: none;
box-shadow: none;📝 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.
| /* Mobile View for Courses Sidebar */ | |
| @media (max-width: 768px) { | |
| .course-popout { | |
| position: static; | |
| width: 100%; | |
| height: auto; | |
| max-height: 400px; | |
| border-radius: 0; | |
| border-left: none; | |
| border-right: none; | |
| box-shadow: none; | |
| border-top: 1px solid var(--color-gray-200); | |
| border-bottom: 1px solid var(--color-gray-200); | |
| overflow-y: auto; | |
| } | |
| } | |
| /* Mobile View for Courses Sidebar */ | |
| `@media` (max-width: 768px) { | |
| .sidebar { | |
| overflow-y: auto; | |
| } | |
| .course-popout { | |
| position: static; | |
| width: 100%; | |
| height: auto; | |
| max-height: min(400px, calc(100dvh - var(--size-topbar-height) - var(--space-xl))); | |
| border-radius: 0; | |
| border-left: none; | |
| border-right: none; | |
| box-shadow: none; | |
| border-top: 1px solid var(--color-gray-200); | |
| border-bottom: 1px solid var(--color-gray-200); | |
| overflow-y: auto; | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rocky-interface/src/lib/styles/components/components.css` around lines 256 -
271, The .course-popout rule for mobile sets position: static and max-height:
400px but the surrounding mobile sidebar container lacks overflow, causing
content to become unreachable on small screens; update the mobile CSS so that
the parent mobile sidebar (the element that contains .course-popout, e.g., the
mobile nav/sidebar container) is allowed to scroll and/or make .course-popout
able to expand within a scrollable container—specifically add overflow-y: auto
(or overflow: auto) to the mobile sidebar/container selector and ensure
.course-popout can use max-height with overflow-y: auto so the dropdown and
other nav items remain reachable on short viewports.
| @media (max-width: 768px) { | ||
| .course-popout-header { | ||
| display: none; | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify where the course popout header and create button are rendered.
# Expectation: if .course-popout-create-btn is inside .course-popout-header,
# there should be another mobile-visible way to trigger the same action.
rg -n -C5 'course-popout-header|course-popout-create-btn' --iglob '*.svelte' --iglob '*.css'Repository: Spring-2026-Software-Engineering/Rocky
Length of output: 5377
Mobile users without canCreateCourse permissions lose access to the create course button.
The .course-popout-create-btn is only rendered inside .course-popout-header (Sidebar.svelte line 190). Hiding the entire header at ≤768px viewport removes this action from mobile users who have permission to create courses, with no visible alternative UI to trigger the same functionality.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rocky-interface/src/lib/styles/components/components.css` around lines 273 -
277, The media query hides the entire .course-popout-header which also removes
the .course-popout-create-btn used by Sidebar.svelte (rendered at the create
button in line ~190), so update the CSS so mobile still exposes the create
button: instead of display:none on .course-popout-header, either hide only
non-action parts (e.g., title/metadata) or add an override for
.course-popout-header .course-popout-create-btn to keep it visible on
max-width:768px (ensure layout and spacing still work and consider using
!important if specificity requires it), or move the .course-popout-create-btn
out of .course-popout-header in the component and style it for mobile.
2 participants
Summary by CodeRabbit
New Features
Style
Chores